fix(review): thread the reputation-skip check from caller to callee instead of re-deriving it (#4507)

[JSONbored]

Summary

• shouldStartAiReviewForAdvisory and runAiReviewForAdvisory each independently called shouldSkipAiForReputation for the same (repo, submitter) within a single webhook-processing pass — a bounded but real duplicate review_targets scan (up to REPUTATION_WINDOW_ROW_CAP = 500 rows) on every reputation-enabled, allowlisted repo.
• The outer caller (maybePublishPrPublicSurface) now computes preComputedReputationSkip once, under the exact gate condition shouldStartAiReviewForAdvisory uses internally, and threads it into both functions — mirroring the existing preAcquiredAiReviewLock threading pattern in the same file.
• Both consumers fall back to their own fresh, independently authoritative check whenever the threaded value is absent — which is exactly the divergent-config case where a per-repo .gittensory.yml manifest override disagrees with the GITTENSORY_REVIEW_REPOS allowlist (only one of the two call sites' gates evaluates true in that case, so the other's threaded value was never computed to begin with). Two regression tests specifically cover both divergence directions, proving the fix never forces a skip (or forces a run) that the two gates would have legitimately disagreed on.

Closes #4507.

Scope

• The PR title follows type(scope): short summary Conventional Commit format.
• This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
• This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
• I linked a currently open issue this PR resolves (Closes #4507).

Validation

• git diff --check
• npm run actionlint
• npm run typecheck
• npm run test:coverage locally — every changed line and branch in src/queue/processors.ts is covered (confirmed via precise lcov.info cross-referencing against the exact diff hunks: zero uncovered lines, zero zero-hit branch sides).
• npm run test:workers
• npm run build:mcp
• npm run test:mcp-pack
• npm run ui:openapi:check
• npm run ui:lint
• npm run ui:typecheck
• npm run ui:build
• npm audit --audit-level=moderate
• New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries — an invariant test (zero extra reputation-scan reads when threaded), two regression tests (both manifest-override divergence directions), and an end-to-end agent-regate-pr pipeline test proving the real outer-caller-scope computation itself is exercised (not just the two consumer functions called directly) — all four empirically verified to fail without their respective fix and pass with it, including one that caught an initially-wrong ?? false fallback during development.

Safety

• No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
• Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
• Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A — no such surface touched.)
• API/OpenAPI/MCP behavior is updated and tested where needed. (N/A — no API/OpenAPI/MCP surface changed.)
• UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A — no UI changes.)
• Visible UI changes include a UI Evidence section below. (N/A — backend-only change, no visible UI.)
• Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. (N/A)

[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.12%. Comparing base (6fee937) to head (1b22397).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4592   +/-   ##
=======================================
  Coverage   94.12%   94.12%           
=======================================
  Files         430      430           
  Lines       38177    38180    +3     
  Branches    13920    13922    +2     
=======================================
+ Hits        35935    35938    +3     
  Misses       1585     1585           
  Partials      657      657           

Files with missing lines	Coverage Δ
src/queue/processors.ts	95.31% <100.00%> (+<0.01%)	⬆️

🚀 New features to boost your workflow:

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

[loopover-orb]

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

_{Review updated: 2026-07-10 06:54:07 UTC}

3 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

⏸️ Suggested Action - Manual Review

• Touches a guarded path — held for manual review: This PR changes guardrail-protected path(s): src/queue/processors.ts (matched src/queue/**).

Review summary
This threads a caller-computed shouldSkipAiForReputation result into both shouldStartAiReviewForAdvisory and runAiReviewForAdvisory via an optional preComputedReputationSkip param, using `??` (not `||`) so a real `false` skip result isn't discarded, and correctly falls back to each function's own fresh, independently-authoritative check whenever the value is absent — which is exactly the manifest-override-divergence case the PR calls out. The three new tests (zero-extra-D1-read invariant, both divergence directions, and a full agent-regate-pr pass) actually assert on the underlying `prepare()` calls rather than just outcome, which is real proof the dedup works and isn't a fabricated-payload test. Logic in both callees is sound: `runAiReviewForAdvisory`'s own `reputationActive` gate is still evaluated fresh every time, so the threaded value is only consulted when that gate agrees, never force-skipping under a stale premise.

Nits — 5 non-blocking

• The `preComputedReputationSkip` is computed once early in `maybePublishPrPublicSurface` (near the `shouldStartAiReviewForAdvisory` call) and reused ~450 lines later at the `runAiReviewForAdvisory` call site (src/queue/processors.ts:10130) — worth confirming nothing in between (e.g. a reputation-outcome write for this same submitter) can mutate the underlying stats within the same pass, since that would make the threaded value stale by the second use.
• test/unit/queue.test.ts:3999 uses `GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem()`, which the secret scanner flagged — worth confirming this is a freshly-generated test-only keypair (not a checked-in credential) since it wasn't visible in the truncated diff context.
• Consider a one-line comment at the second use site (src/queue/processors.ts:10130) cross-referencing where `preComputedReputationSkip` was computed, since the two sites are far apart in a 10k-line file and a future editor could miss the coupling.
• PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
• Touches a guarded path — held for manual review — A maintainer must review and merge this change.

Signal	Result	Evidence
Code review	✅ No blockers	1 reviewer
Linked issue	✅ Linked	#4507
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 (1 linked issue).
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	⚠️ Not blocking	Advisory; not blocking this PR.

Linked issue satisfaction

Addressed
The diff threads a preComputedReputationSkip value from the single outer call site through both shouldStartAiReviewForAdvisory and runAiReviewForAdvisory (mirroring the cited preAcquiredAiReviewLock pattern), with both consumers falling back to independent fresh checks when the value is undefined, which is exactly the divergent-config/manifest-override behavior the issue required to be preserved.

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: not available
• 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.

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