Overnight automated review: 98 test-gated fixes (11 bug, 6 risk, 81 quality)

[hb-agent]

Overnight automated review — Phase 1 (survey) + Phase 2 (test-gated fixes)

Two-phase autonomous code review of feat/positioning-redesign, run as sequential workflows on this branch. Nothing here changes external behavior beyond the specific defects fixed — anything that would (API/contract/schema/dependency/architecture) was held for human decision.

Read these two files first:

• REVIEW.md — the prioritized findings report (86 findings, each independently verified).
• CHANGES.md — per-item change log + the Held list awaiting your decision.

Phase 1 — survey (read-only)

18 module reviewers + 6 cross-cutting lenses (authz/repo-ownership, input/XSS, session/CSRF, data-loss/privacy, dead-code/dup, deps/config). Every bug/risk/high finding was re-derived by an independent skeptic agent before inclusion. Result: 86 findings — 11 bug, 9 risk, 56 quality, 10 judgment (2 refuted/dropped).

Phase 2 — implementation (test-gated, one commit per item)

Each item: failing test first where behavior is testable → minimal fix → commit only if the full vitest suite + tsc --noEmit + lint stay green; otherwise reverted and logged BLOCKED.

Category	Result
Bugs	11 / 11 implemented
Risks	6 / 6 actionable implemented (3 held — see below)
Quality	81 implemented, 3 blocked, 3 skipped
Total	98 IMPLEMENTED · 3 BLOCKED · 3 SKIPPED · 107 commits

Highlights (fixed)

• Stored XSS — context-update attachment URIs were rendered as href with no scheme allowlist (javascript: was one-click executable); now gated through safeHttpUrl.
• Secret leakage — XRPC proxy echoed un-redacted 4xx upstream messages; groups/register logged raw DPoP/Bearer-bearing errors. Both redacted/logSafe'd.
• Silent data/edit loss — group-write InvalidSwap detection was dead (conflict-rebase never ran); inline-edit Save dropped in-flight avatar/banner uploads; project location was written as a strongRef but read as a string (never displayed). All fixed.
• Broken share previews — 5 pages referenced a non-existent OG image (404 unfurls).
• Plus crawlability/noindex compliance, dead-code removal, effect-cleanup/abort guards, a11y, design-token compliance, shared-helper extraction, and new unit tests for previously-untested security-relevant utilities.

Verification

• Final gate: vitest 474 passing (from 291) · tsc --noEmit 0 errors · lint 0 errors / 67 warnings (down from 69; now over the broader eslint . scope).
• No baseline test was weakened — every test-file change is an addition (audited across all 107 commits; the only deletion is an import-line expansion in badges.test.ts).
• main untouched. No force-push. No --no-verify.

⚠️ Held for your decision (NOT implemented — see CHANGES.md § Held)

• 10 judgment items (judgment-001…010): OAuth prompt allowlist, BFF defense-in-depth role checks, custom-domain group-awareness, profile swap-protection (lost-update), autoprefixer dependency, popover-primitive consolidation, canonical landing URL, foreign-blob caching cap, etc.
• risk-005 / risk-007 / risk-009: write-envelope contract change, /api/resolve-did rate limiter (= judgment-002), and a new deleteFollow API surface.
• quality-024 (= judgment-007), quality-042 (refuted), quality-004 (clean fix would require deleting a test), and 056 sub-items approot-sitemap-public-gap (coupled to judgment-005) + quality-overexport-1 (too broad).

Blocked (reverted, branch stayed green)

• quality-002 — deleting dead cert-context.ts: 4 other branches import it (its own recommendation gated deletion on this). quality-003 still fixed the module's pagination bug.
• quality-033 — useBskyPosts race not reproducible (existing requestIdRef guard already covers it).
• quality-045 — the two list-modals diverge in user-visible behavior; no pure extraction exists → escalated to judgment.

Known follow-up (not blocking; not in CI)

quality-010 added a typecheck:test tool (tsconfig.test.json) that surfaces ~21 pre-existing type errors in test files (mostly notifications.test.ts), never type-checked before. ci.yml runs only the main tsc --noEmit (excludes tests), so CI stays green; cleaning these up is a separate effort.

Test plan

• npm test — 474 passing
• npx tsc --noEmit — 0 errors
• npm run lint — 0 errors / 67 warnings
• Review REVIEW.md judgment items and decide the Held list
• Decide whether to delete cert-context.ts (quality-002) given cross-branch usage

Opened as Draft — not merging; that's your call.

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
certified-app	Ready	Preview, Comment	May 29, 2026 9:33am

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f5c1ffd9-23dd-4fc9-9653-d85edc322ce6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch overnight-review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

_{Tip: disable this comment in your organization's Code Review settings.}