fix(review): require persisted visual captures

[JSONbored]

Motivation

• The screenshot-table gate could be bypassed because on-demand /shot?url= fallback links (returned when rendering failed or R2 was missing) were treated as evidence of a real bot-captured before/after pair.
• Require only persisted /shot?key= URLs as machine-proven evidence so a failed/no-render on-demand URL cannot mark a PR as botCaptureSatisfied and bypass maintainer-configured checks.

Description

• Replace the loose "real shot" predicate with isPersistedShotUrl(...) which requires a /shot? URL that contains a key= and excludes placeholder= markers in src/review/visual/capture.ts.
• Use the new isPersistedShotUrl predicate in the per-route pair check so hasSuccessfulBotCapture only returns true for persisted cached PNGs.
• Update test/unit/visual-capture.test.ts to assert persisted ?key= URLs satisfy the predicate and add a regression test proving on-demand ?url= fallback URLs do not satisfy bot-capture evidence.

Testing

• Ran the visual-capture unit suite with npx vitest run test/unit/visual-capture.test.ts and the hasSuccessfulBotCapture tests passed (all tests in that file passed).
• Ran TypeScript checks with npm run typecheck and it succeeded with no errors.
• Attempted npm run test:coverage but the full coverage run did not complete within the session (the broader queue test suite exercised and long-running tasks prevented finishing in this environment).
• Attempted npm audit --audit-level=moderate and it failed due to the registry audit endpoint returning 403 Forbidden in this environment.

---

Codex Task

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.10%. Comparing base (efcd25d) to head (056ced2).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4136      +/-   ##
==========================================
- Coverage   94.11%   94.10%   -0.01%     
==========================================
  Files         432      432              
  Lines       38348    38349       +1     
  Branches    13979    13979              
==========================================
- Hits        36091    36089       -2     
- Misses       1600     1602       +2     
- Partials      657      658       +1     

Files with missing lines	Coverage Δ
src/review/visual/capture.ts	94.77% <100.00%> (+0.03%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[loopover-orb]

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - fixes required

_{Review updated: 2026-07-10 08:22:00 UTC}

2 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI failing · blocked

🛑 Suggested Action - Manual Review

• No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
• Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This PR tightens the screenshot-table gate's proof-of-capture predicate from 'any non-empty, non-placeholder shot URL' to 'a persisted /shot?key= URL', closing a real bypass where an on-demand ?url= fallback (returned when R2 is unavailable or rendering fails) could satisfy hasSuccessfulBotCapture without a real cached before/after pair. The change is narrowly scoped to one function (isPersistedShotUrl) and its two call sites in routeHasRealBeforeAfterPair, is strictly more restrictive than the prior logic (so it can only flip prior true-positives to false, never introduce new false positives), and ships a regression test that exercises the real code path with the exact on-demand-URL shape being excluded. The description does not link or close a tracked issue for this fix, which this repo's contribution conventions call for even on security-motivated changes.

Nits — 5 non-blocking

• src/review/visual/capture.ts: isPersistedShotUrl relies on substring checks (url.includes("/shot?") && url.includes("key=")) rather than parsing query params, so a URL with a stray 'key=' substring elsewhere could in theory pass; this mirrors the pre-existing placeholder= substring convention, so it's a minor robustness nit rather than a regression.
• PR description does not close or link a tracked issue for this fix (only an incidental feat(review): wire bot-generated visual captures into screenshotTableGate #4110 reference inside a pre-existing test describe block) — worth citing the issue/discussion that surfaced the gate bypass.
• Only the on-demand-fallback negative case was added as a regression test; consider also asserting a URL containing both 'key=' and 'placeholder=' still returns false, since that boundary combination isn't exercised by the diff's new test.
• test/unit/visual-capture.test.ts: add a case for '/shot?key=x&placeholder=failed' to lock in that placeholder exclusion still wins even when key= is present.
• Link the PR to the issue/discussion that identified the bot-capture-gate bypass for future readers.

Why this is blocked

• No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
• Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

CI checks failing

• validate
• validate-code

Signal	Result	Evidence
Code review	❌ 2 blockers	1 reviewer
Linked issue	⚠️ Missing	No linked issue or no-issue rationale found.
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (no linked issue context).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 48 registered-repo PR(s), 40 merged, 334 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	JSONbored; Gittensor profile; 48 PR(s), 334 issue(s).
Gate result	❌ Blocking	Repo-configured hard blocker found.

Review context

• Author: JSONbored
• Role context: owner (maintainer lane)
• Public audience mode: oss maintainer
• Lane context: Repository is configured for direct PR review.
• Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
• Official Gittensor activity: 48 PR(s), 334 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Treat this as maintainer-lane context rather than normal contributor-lane activity.
• Explain no-issue PR.
• Link the issue being solved, or explicitly explain why this is a no-issue PR.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
• Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
• Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

---

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

• Re-run Gittensory review

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Updated (UTC)
🔵 In progress View logs	gittensory-ui	cf94e10	Jul 10 2026, 08:23 AM