fix(review): make review.impactMap fully control activation once the kill-switch is on

[JSONbored]

Summary

• shouldComputeImpactMap (src/review/impact-map-wire.ts) previously required isImpactMapEnabled(env) && manifestImpactMapEnabled — a required AND where every call site had to pre-collapse the manifest's boolean | undefined toggle to a strict boolean (args.reviewImpactMap === true) before calling in.
• Migrated it onto the same precedence shape inlineComments/fixHandoff already use (Upgrade inlineComments + fixHandoff from required-AND opt-in to full config-as-code substitutes #4099, PR feat(review): upgrade inlineComments + fixHandoff to full config-as-code substitutes #4116): the operator's GITTENSORY_REVIEW_IMPACT_MAP flag is an absolute master kill-switch (off ⇒ always false, no per-repo bypass), and once it's on, the manifest's review.impactMap toggle (manifestToggle: boolean | undefined) fully decides via manifestToggle === true. impact-map has never had a GITTENSORY_REVIEW_REPOS cutover-allowlist fallback, so there was nothing to "bypass" in that dimension — this PR closes the config-as-code gap by moving the === true collapse into the single resolver instead of leaving it scattered at each call site.
• Updated both call sites in src/queue/processors.ts (the AI-review prompt gate and the cache-bypass dynamicReviewFeatures.impactMap gate) to pass the raw manifest value straight through instead of pre-coercing it.
• Behavior is byte-identical for every existing repo: given the same (env, manifestValue) pair, the old and new resolvers compute the exact same boolean (verified with a regression test asserting an unset manifest toggle still resolves to false even with the env flag on).
• src/review/ai-review-cache-input.ts and src/review/unified-comment-bridge.ts were checked and need no changes — both consume the already-resolved boolean/array after activation, not the activation decision itself.

Closes #4102

Scope

• The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
• 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 (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

• git diff --check
• npm run actionlint
• npm run typecheck
• npm run test:coverage locally; codecov/patch requires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.
• 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

If any required check was skipped, explain why:

• This is a backend-only src/review + src/queue change with no UI/MCP/OpenAPI/wrangler surface touched, so ui:*, build:mcp/test:mcp-pack, test:workers, and ui:openapi:check were not run locally (nothing in the diff can affect them). npm run actionlint was not run locally (no .github/workflows/** changes in this diff). Coverage was measured via npx vitest run --coverage --pool=forks test/unit/impact-map-wire.test.ts test/unit/queue.test.ts (targeted, not the full unsharded suite) — src/review/impact-map-wire.ts's shouldComputeImpactMap shows 100% line/branch coverage (LF:19 LH:19, BRF:12 BRH:12 in coverage/lcov.info), and the two src/queue/processors.ts call-site lines are executed on every AI-review pass (verified hit counts in the same lcov output). The full test/unit/queue.test.ts suite (697 tests) was run in full and passes, both before and after rebasing onto origin/main.

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.
• API/OpenAPI/MCP behavior is updated and tested where needed. (N/A — no API/OpenAPI/MCP surface touched.)
• 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 with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository. (N/A — no visible UI change.)
• Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. (N/A — no docs/changelog change needed.)

UI Evidence

N/A — backend-only change, no visible UI/frontend/docs/extension surface touched.

Notes

• This mirrors the exact fix shape used for inlineComments/fixHandoff (Upgrade inlineComments + fixHandoff from required-AND opt-in to full config-as-code substitutes #4099, merged as PR feat(review): upgrade inlineComments + fixHandoff to full config-as-code substitutes #4116) and is one of a set of sibling issues in the same epic (Investigate + close the config-as-code gap for reviewMemory #4101 for reviewMemory, Investigate + close the config-as-code gap for cultureProfile #4103) applying the same precedence pattern to other standalone review-feature toggles.

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

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4125   +/-   ##
=======================================
  Coverage   93.65%   93.65%           
=======================================
  Files         384      384           
  Lines       35865    35866    +1     
  Branches    13160    13160           
=======================================
+ Hits        33589    33590    +1     
  Misses       1618     1618           
  Partials      658      658           

Files with missing lines	Coverage Δ
src/queue/processors.ts	95.15% <100.00%> (ø)
src/review/impact-map-wire.ts	100.00% <100.00%> (ø)

🚀 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-08 02:59:17 UTC}

5 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 refactors `shouldComputeImpactMap` (src/review/impact-map-wire.ts) to accept the raw `manifestToggle: boolean | undefined` and perform the `=== true` collapse internally, rather than requiring callers to pre-coerce with `args.reviewImpactMap === true`. The two call sites in src/queue/processors.ts are updated to pass the raw manifest value straight through, and the logic is behaviorally identical to before (kill-switch off ⇒ false; kill-switch on ⇒ `manifestToggle === true`), which is exactly what the old `isImpactMapEnabled(env) && manifestImpactMapEnabled` produced when `manifestImpactMapEnabled` was always pre-coerced via `=== true` at the call site. New tests correctly cover the previously implicit unset-manifest-toggle arm. This is a small, well-scoped, byte-identical config-plumbing refactor closing #4102 with matching test coverage.

Nits — 5 non-blocking

• src/review/impact-map-wire.ts:26 and src/queue/processors.ts:6938 use "master kill-switch" terminology — consider "primary/main kill-switch" for consistency with the repo's other converged-feature comments.
• The precedence rationale (kill-switch + manifestToggle === true, no allowlist fallback) is now duplicated near-verbatim across src/review/impact-map-wire.ts, src/queue/processors.ts, and src/env.d.ts — consider having two of the three point back to the canonical explanation in impact-map-wire.ts to avoid drift next time the precedence changes.
• Since this same `env kill-switch` + `manifestToggle === true` shape now exists in three places (inlineComments, fixHandoff, impactMap per the PR description), consider whether a small shared helper (e.g. `resolveConvergedFeature`, already referenced in the updated comments) could replace the hand-written `if (!enabled) return false; return manifestToggle === true;` pattern to prevent the next feature from re-diverging.
• 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	#4102
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: 51 registered-repo PR(s), 43 merged, 509 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	JSONbored; Gittensor profile; 51 PR(s), 509 issue(s).
Gate result	⚠️ Not blocking	Advisory; not blocking this PR.

Linked issue satisfaction

Addressed
The PR replaces the required-AND gate in shouldComputeImpactMap with the same precedence used for #4099/#4101 (env var as an absolute kill-switch, manifest 'review.impactMap' fully deciding via === true once the switch is on), updates both call sites in processors.ts to stop pre-coercing the manifest value, and adds tests including an explicit regression case showing an unset manifest toggle prese

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: 51 PR(s), 509 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

[JSONbored]

Closing unmerged — see #4102's closing comment. No behavior gap existed to fix; impactMap never had an allowlist term to bypass, so this is a byte-identical code-shape change, not a real config-as-code fix.