feat(visual-review): worker scaffold + webhook/queue + D1/R2

[Helios531]

Summary

Stands up the visual-review Worker skeleton for owner-led visual-change PR auto-review, mirroring the submission-gate pattern: GitHub App webhook intake → enqueue → queue consumer, with D1 for review-target state and R2 for the intake audit log. Closes #578 (epic #577, roadmap #525).
This lands the state plumbing only — no screenshots yet, per the acceptance criteria. Detection, render/capture, diff, and the managed comment ship in the child issues (#579–#584).

What's included:

• Webhook intake (src/visual/webhook.ts): on an opted-in repo, a non-draft PR opened/synchronize/reopened event records a queued target in D1 and enqueues a visual-review job. Fully fail-safe — any error is swallowed + audited so it can never break submission-gate webhook processing.
• Queue consumer (src/visual/pipeline.ts): drives queued → capturing, writes an R2 intake audit log (optional binding, degrades gracefully), and transitions to failed +rethrows on a real error so the existing queue/DLQ retries instead of silently dropping.
• D1 state (migrations/0037_visual_review_targets.sql, src/db/schema.ts, src/visual/targets.ts): visual_review_targets (one row per repo/PR/head SHA, lifecycle queued → capturing → posted | failed, idempotent upsert) and per-repo opt-in visual_review_settings (absence of row == disabled).
• R2 binding VISUAL_REVIEW_BUCKET (wrangler.jsonc, src/env.d.ts).
• REVIEW_MARKER managed-comment constant for the downstream comment writer (owner-led(visual-agent): R2 image hosting + managed PR comment #583).
• Wiring of the visual-review JobMessage type and dispatch case (src/types.ts, src/queue/processors.ts).

Related Issue

Closes: #578

Change Type

• New feature (non-breaking change that adds functionality)
• Backend / Worker
• Database migration (D1)
• Infrastructure / bindings (R2, queue, wrangler)
• Tests
• Bug fix
• Breaking change
• MCP
• Documentation / changelog

Real behavior proof

All commands run locally on this branch:

• tsc --noEmit → clean, no errors.
• vitest run test/unit/visual-review.test.ts → 19/19 passed (enablement, target state, idempotency, webhook gating, fail-safe, pipeline lifecycle, R2 audit, marker).
• vitest run test/unit → 1815/1815 passed, 126 files — no regressions from the processors.ts integration.
• vitest run --config vitest.workers.config.ts → 1/1 passed.
• wrangler d1 migrations apply gittensory --local → 0037_visual_review_targets.sql ✅, 6 commands executed successfully (migration is sequential and valid).
• wrangler deploy --dry-run → Worker builds; env.VISUAL_REVIEW_BUCKET (gittensory-visual-review) R2 binding confirmed alongside JOBS and DB.

Test Files  126 passed (126)
     Tests  1815 passed (1815)

Validation

• git diff --check
• npm run actionlint (not run — no workflow files changed)
• npm run typecheck
• npm run test:coverage (ran test/unit + test:workers; full coverage gate is CI's job)
• npm run test:workers
• New or changed behavior has unit tests for new branches, fallback paths, and boundaries

[ghost]

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal	Result	Evidence	Action
Gate result	⚠️ Skipped	#757 is no longer open.	No action.

---

💰 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.

[ghost]

gittensory · advisory review

Reviewed 12 changed file(s) — two independent AI reviewers.

Changed files (12)

• migrations/0037_visual_review_targets.sql
• package-lock.json
• src/db/schema.ts
• src/env.d.ts
• src/queue/processors.ts
• src/types.ts
• src/visual/constants.ts
• src/visual/pipeline.ts
• src/visual/targets.ts
• src/visual/webhook.ts
• test/unit/visual-review.test.ts
• wrangler.jsonc

Suggested action: ✅ Safe to merge — both reviewers found no blocking issues.

Reviewer A · gpt-oss-120b — recommends ✅ merge
The PR adds a full D1‑backed visual‑review scaffold: new DB tables, migration, env bindings, constants, target management, webhook enqueueing, queue processor integration, and comprehensive unit tests. The implementation follows existing patterns, respects strict TypeScript settings, and does not introduce any public‑facing forbidden terminology.

Suggestions

• Add a test for transitionVisualReviewTarget with incrementAttempts: true on an existing target to verify the attempts count increments correctly.
• Consider documenting the optional R2 bucket behavior in the README for operators deploying the worker.
• Ensure the new migration is included in the migration run sequence for fresh deployments.

Worth double-checking

• If the R2 bucket is not provisioned, audit logs will be silently omitted – ensure operators are aware of this graceful degradation.
• The new visual-review job type must be supported by any downstream queue consumers; verify compatibility in production environments.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
The PR adds a scaffold for visual review functionality, including D1 state management (targets and settings tables), webhook intake, queue processing, and R2 audit logs. The code follows existing patterns, is well-tested with comprehensive unit tests, and maintains the public/private boundary (no public output yet). Integration points are correctly hooked into the webhook and queue processors. The scaffold is intentionally minimal, leaving actual screenshot capture and diffing for future issues.

Suggestions

• Consider using an atomic increment for attempts in transitionVisualReviewTarget to avoid potential race conditions (though low risk in Cloudflare Workers' single-threaded model).
• In writeIntakeAuditLog, verify the JSON body size is within R2 limits (though current payload is small).
• The test file is large; consider splitting into multiple test files for better maintainability as the feature grows.

Worth double-checking

• Ensure the R2 bucket gittensory-visual-review is created in production before deployment to avoid degraded functionality (audit logs skipped).
• Confirm that the unique index on (repo_full_name, pull_number, head_sha) prevents duplicate targets during high-concurrency webhook redelivery.
• Verify that the VISUAL_REVIEW_BUCKET binding is correctly configured in all environments (preview/prod) where visual review is enabled.

[superagent-security]

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

[Helios531]

@JSONbored Could you plz review my PR? Thanks!

[codecov]

Codecov Report

❌ Patch coverage is 97.40260% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.58%. Comparing base (b2162f7) to head (1276731).

Files with missing lines	Patch %	Lines
src/queue/processors.ts	33.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #757      +/-   ##
==========================================
- Coverage   96.58%   96.58%   -0.01%     
==========================================
  Files          98      102       +4     
  Lines       14196    14273      +77     
  Branches     5172     5196      +24     
==========================================
+ Hits        13711    13785      +74     
- Misses        105      107       +2     
- Partials      380      381       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[JSONbored]

PR #757 — feat(visual-review): worker scaffold + webhook/queue + D1/R2
Verdict: REQUEST CHANGES
Resolves issue #578: FULLY
CI: All required checks green (validate, test 1/2, workers, mcp, ui, lint, security, codecov patch+project).

Highlights:

• Faithfully mirrors the submission-gate pattern: webhook intake records a queued D1 target on opted-in repos and enqueues a job; queue consumer drives queued->capturing with an optional R2 intake audit; per-repo opt-in (absence == disabled).
• Migration 0037 + schema match exactly (visual_review_targets with unique(repo,pr,head_sha) idempotency index; visual_review_settings). REVIEW_MARKER defined for the downstream comment writer.
• Correct fail-safety: webhook intake swallows+audits errors so it can never break submission-gate processing; pipeline transitions to failed and rethrows on real errors so existing queue/DLQ retries. R2 binding optional, degrades gracefully.
• "No screenshots yet" honored — parks at capturing with an audited capture-pending note. 19 strong unit tests (enablement, idempotent upsert, webhook gating, lifecycle, R2 write/failure/no-bucket).

Concerns / required changes:

• Lockfile/dep scope creep: package.json adds a ws override; package-lock.json churns ~3.9k lines — should be isolated.
• Minor: upsertVisualReviewTarget stores base ref into the baseSha column — harmless now but the name implies a SHA; align before the #582 diff step consumes it.
• Migration numbering collides with previous PRs - fix

Completeness vs issue: Fully satisfies #578 — opted-in visual-change PR detected, target recorded in D1 (queued->capturing->posted|failed), enqueued with DLQ, R2 bound for audit, REVIEW_MARKER defined, no screenshots yet. Only the dependency churn detracts.

[JSONbored]

Just realized these are owner assigned - the overnight reviews should've flagged these and closed them, they're issues I'm personally working on. Closing this PR.