refactor(review): unify six advisory-feature activation checks behind one shared resolver

[JSONbored]

Summary

Closes #4616 (the review-stack architecture audit's flagship config-sprawl finding).

• Consolidates the independently hand-rolled activation checks scattered across src/review/*-wire.ts
  behind one shared, pure precedence resolver.
• Adds resolveFeatureActivation (src/review/feature-activation.ts) — a single pure core that every
  per-feature activation precedence shape in this codebase now reduces to, expressed as one of four named
  modes: standard, forceOnOnly (safety, fix(review): features.safety is per-repo disableable via .gittensory.yml even when the operator enabled it globally #2269), allowlistRequired (grounding), and the new
  manifestOnly. resolveConvergedFeature (the features:-block keys) and the new
  resolveManifestOnlyFeature (the review:-block keys with no allowlist role) are thin adapters over it.
• Migrates six of the currently-inconsistent features onto the shared resolver, each verified to produce
  the identical activation decision before/after across every yml-set / yml-unset / allowlist-set /
  allowlist-unset / env-flag-on / env-flag-off combination (see "No-behavior-change verification" below):
  • screenshots joins the six existing ConvergedFeatureKeys (features.screenshots is now a real,
    bidirectional per-repo override) — this closes the exact gap the audit called out: it previously had
    env-flag-AND-allowlist only, with no features: override at all, unlike its six documented siblings.
    A self-hoster who guessed features.screenshots: true (a natural guess given the sibling keys right next
    to it in .gittensory.yml.example) previously found it silently did nothing.
  • impactMap, reviewMemory, cultureProfile, inlineComments, fixHandoff now
    delegate to the new manifestOnly mode instead of each independently re-implementing the identical
    isXEnabled(env) && <manifest toggle> === true boolean pair. cultureProfile additionally gained a
    proper shouldApplyRepoCultureProfile function (mirroring its four siblings) — it previously had no
    dedicated resolver at all and was inlined twice at its call sites in processors.ts.
• .gittensory.yml.example (and its mirror, config/examples/gittensory.full.yml) document the new
  features.screenshots key alongside its six siblings, with an updated precedence comment.

Scope cut (disclosed)

Given the size (~16 call sites across distinct features, per the issue), this PR lands the shared resolver
plus the clearest, individually-lowest-risk inconsistent cases, and explicitly leaves the rest for a
fast-follow rather than trying to migrate all ~16 in one PR:

• content-lane (src/review/content-lane/spec-resolver.ts's resolveRegistryLaneSpec): already
  implements the same env-kill-switch → override → allowlist-default precedence one-for-one (verified by
  reading the function in full), it just isn't literally routed through resolveConvergedFeature yet — it
  resolves a whole RegistryLaneSpec object, not a boolean, so migrating it is a larger, more speculative
  refactor for zero behavior-consistency gain (it wasn't inconsistent to begin with).
• selftune: already self-documented in the codebase (CONVERGED_FEATURE_KEYS's own comment, and
  selftune-wire.ts) as a deliberate non-fit — its base eligibility is isAgentConfigured (the
  acting-autonomy consent boundary), not the GITTENSORY_REVIEW_REPOS allowlist, so it has no "allowlist"
  input to plug into either resolveConvergedFeature or the new resolveManifestOnlyFeature without a
  larger redesign (a pluggable base-eligibility predicate, not just a pinned boolean).
• enrichment (REES): gated by GITTENSORY_REVIEW_ENRICHMENT and REES_URL being configured — a
  deployment-integration toggle, not a per-repo features:/review: boolean; per-repo review.enrichment
  only narrows which analyzers run, not whether the capability exists at all.
• ops, outcomes/parity-audit, maintainer-recap: deployment-level (ops/telemetry
  recording, cross-repo digest cadence) rather than per-PR, per-repo advisory capabilities — none has ever had
  a features:/review: per-repo toggle to begin with, so there's no inconsistency to fix here.
• issue-rag-wire.ts: not an activation-gate file at all — a query-composition re-export shim.

No-behavior-change verification

For every migrated feature, resolveFeatureActivation's mode was derived to be mathematically identical
to the feature's prior hand-rolled expression, not just "similar":

• screenshots: new capability (a features: override that didn't exist before) — for every repo that has not
  set features.screenshots, the override resolves to null and falls back to the allowlist exactly as
  screenshotsAllowed's old isScreenshotsEnabled(env) && isConvergenceRepoAllowed(env, repo) already did.
  Existing review.visual.enabled/review.visual.production_url config (a separate, richer layer) is
  untouched.
• impactMap / reviewMemory / inlineComments / fixHandoff: resolveManifestOnlyFeature(flag, override)
  with allowlisted pinned to false collapses to exactly flag && override === true for every input —
  algebraically identical to each feature's prior isXEnabled(env) && manifestToggle === true (or && manifestXEnabled for the two that took an already-collapsed boolean).
• cultureProfile: the two processors.ts call sites' inline isRepoCultureProfileEnabled(env) && reviewCultureProfile === true now read shouldApplyRepoCultureProfile(env, reviewCultureProfile === true)
  — same expression, centralized into one named function.

This is backed by:

1. A dedicated resolveFeatureActivation/resolveManifestOnlyFeature unit-test suite in
   feature-activation.test.ts exercising every (globalFlag × override × allowlist × mode) combination
   explicitly.
2. Every pre-existing exhaustive truth-table test for the six migrated features'
   public functions (shouldComputeImpactMap, shouldApplyReviewMemory, shouldRequestInlineFindings,
   shouldEmitFixHandoff, plus the new shouldApplyRepoCultureProfile, plus a rewritten
   visual-wire.test.ts covering screenshots' new override) — all pass unchanged against the refactored
   internals, since the public signatures and behavior are unchanged.
3. Precise line/branch-level verification via the raw lcov.info output (not just the terminal summary):
   feature-activation.ts, visual-wire.ts, impact-map-wire.ts, inline-comments.ts, fix-handoff.ts,
   and repo-culture-profile-wire.ts all show zero uncovered lines and zero uncovered branches; the
   three touched call sites in processors.ts do not appear in that file's uncovered-line list either.

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 PR touches only backend review-config-resolution logic (src/review/**, src/queue/processors.ts,
  packages/gittensory-engine/src/focus-manifest.ts) plus two mirrored yml docs — no apps/gittensory-ui,
  packages/gittensory-mcp, packages/gittensory-miner, wrangler.jsonc, or Cloudflare Workers-runtime code
  is touched, so the UI/MCP/miner/workers checks are not applicable and were not run.
• In place of the full unsharded npm run test:coverage, I ran (a) npm run typecheck (clean), (b) every
  test file that imports or exercises a file I changed — feature-activation.test.ts, visual-wire.test.ts,
  impact-map-wire.test.ts, review-memory-wire.test.ts, repo-culture-profile-wiring.test.ts,
  repo-culture-profile.test.ts, inline-comments.test.ts, inline-comments-select.test.ts,
  review-fix-handoff.test.ts, fix-handoff-collapsible.test.ts, fix-handoff-render.test.ts,
  focus-manifest.test.ts, content-lane-wire.test.ts, cutover-gate.test.ts, config-templates.test.ts,
  and the full queue.test.ts (807 tests, the integration suite for processors.ts) — all green, with
  --coverage scoped to the changed files and cross-checked against the raw lcov.info per-line/per-branch
  data (see "No-behavior-change verification" above); and (c) npm run test:changed (Vitest's diff-aware
  test selection against origin/main) as a broader safety net — 9084 tests across 368 files, all green.
  That sweep caught a real gap my initial file list missed (config/examples/gittensory.full.yml, a mirrored
  copy of .gittensory.yml.example checked by config-templates.test.ts), now fixed and included. I also ran
  npm run docs:drift-check, npm run manifest:drift-check, and npm run engine-parity:drift-check
  (all relevant given the packages/gittensory-engine and .gittensory.yml.example edits) — all pass. CI
  runs the full test:ci gate as the authoritative signal.

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. — not applicable, no auth/session/CORS surface touched.
• API/OpenAPI/MCP behavior is updated and tested where needed. — not applicable, no API routes or MCP surface touched.
• UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — not applicable, no UI changed.
• Visible UI changes include a UI Evidence section — not applicable, no visible UI change.
• Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. (.gittensory.yml.example and its config/examples/ mirror updated; CHANGELOG.md untouched.)

Notes

• Rebased onto fresh origin/main (c3b10f296) immediately before pushing; the only upstream change since
  branching (perf(review): downscale before/after screenshots before the local VLM, perf(review): downscale before/after screenshots before sending to the local VLM #4370/perf(review): downscale before/after screenshots before the local VLM #4687) touched
  src/review/visual/capture.ts and src/review/visual/image-downscale.ts — no file overlap with this PR, so
  the rebase applied cleanly with no conflicts. Re-ran npm ci, npm run typecheck, and the full touched-test
  list (1683 tests) after the rebase — all green.
• Deliberately did not touch src/queue/processors.ts beyond the three call sites this migration actually
  requires (two cultureProfile sites, one screenshots site) plus the corresponding import updates.
• resolveFeatureActivation takes already-resolved primitives (globalFlagOn: boolean, override: boolean | null, allowlisted: boolean), not Env or a raw manifest — so a future per-tenant resolution path (a
  tenant DB row standing in for the global env var, or a tenant's own allowlist) can supply the same three
  inputs without this function, or its two callers' precedence logic, changing at all — the reuse target the
  issue's acceptance criteria calls for.

Update (rebase + independent re-verification)

Rebased onto the latest origin/main to resolve a conflict with improvementSignal (#4738), which landed
on CONVERGED_FEATURE_KEYS after this branch forked — merged both screenshots and improvementSignal
into the array/doc-comments/yml examples/tests, full suite re-verified green (687 files / 13,706 tests).

Independently re-researched the "Scope cut" list above from scratch tonight (fresh read of all 9 excluded
*-wire.ts files, not just a recap) before deciding whether more of #4616 could land in a follow-up. It
reached the identical conclusion: every one of content-lane, selftune, enrichment, ops, outcomes,
parity, maintainer-recap, and issue-rag-wire is either already self-documented in-repo as a deliberate
non-fit, structurally incompatible with the resolver's boolean/per-repo shape (cross-repo cron jobs, an
object-valued resolver, telemetry-only recorders), or not an activation gate at all. There is no clean,
low-risk second increment left — this PR is the full realistic scope of #4616, hence Closes.

[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	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	gittensory-ui	4c5556e	Commit Preview URL Branch Preview URL	Jul 10 2026, 11:56 PM

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

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4692   +/-   ##
=======================================
  Coverage   94.22%   94.22%           
=======================================
  Files         441      441           
  Lines       38740    38742    +2     
  Branches    14108    14106    -2     
=======================================
+ Hits        36503    36505    +2     
  Misses       1577     1577           
  Partials      660      660           

Files with missing lines	Coverage Δ
packages/gittensory-engine/src/focus-manifest.ts	99.11% <100.00%> (ø)
src/queue/processors.ts	95.70% <100.00%> (-0.01%)	⬇️
src/review/feature-activation.ts	100.00% <100.00%> (ø)
src/review/fix-handoff.ts	100.00% <100.00%> (ø)
src/review/impact-map-wire.ts	100.00% <100.00%> (ø)
src/review/inline-comments.ts	100.00% <100.00%> (ø)
src/review/repo-culture-profile-wire.ts	100.00% <100.00%> (ø)
src/review/review-memory-wire.ts	100.00% <100.00%> (ø)
src/review/visual-wire.ts	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-10 23:43:11 UTC}

15 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI green · dirty

⏸️ 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 refactor consolidates six independently hand-rolled activation checks (screenshots, impactMap, reviewMemory, cultureProfile, inlineComments, fixHandoff) behind one pure resolver `resolveFeatureActivation`, with `resolveConvergedFeature` and the new `resolveManifestOnlyFeature` as thin adapters over it. I traced the four modes (`standard`/`forceOnOnly`/`allowlistRequired`/`manifestOnly`) against the prior inline logic in each wire file and they reduce to byte-identical boolean arithmetic — the safety force-on-only and grounding allowlist-required asymmetries (#2269) are preserved verbatim, and the five manifestOnly features' `override === true` semantics are unchanged. `screenshots` gaining a real `features:` override (previously env-flag-AND-allowlist only, with no override at all) is the one actual behavior change, and it's called out honestly rather than hidden inside a 'no-behavior-change' claim. Both `.gittensory.yml.example` and `config/examples/gittensory.full.yml` were updated in lockstep, `CONVERGED_FEATURE_KEYS`/`EMPTY_FEATURES_CONFIG` were extended together, and the test suite (feature-activation.test.ts, visual-wire.test.ts, repo-culture-profile-wiring.test.ts) exercises every mode's boundary cases including the new screenshots override paths.

Nits — 5 non-blocking

• `master kill-switch` wording in src/review/feature-activation.ts:71 and :112 (and the mirrored test comments) could use non-inclusive-terminology-neutral phrasing like `primary kill-switch`.
• The doc comments repeat `(Unify per-feature *-wire.ts activation behind one shared resolver #4616)` as an inline citation on nearly every function/block across 8+ files — consider trimming to one reference per file now that the pattern is established, since it reads as noise on re-read.
• config/examples/gittensory.full.yml and .gittensory.yml.example are large files (1100+ lines) that this PR nudges further; not this PR's problem to fix, but worth a follow-up to split per-feature docs out of the single monolithic example file.
• src/queue/processors.ts's screenshots callsite now reads `resolveConvergedFeature(env, repoFocusManifestForComment, "screenshots", repoFullName)` inline at the call site rather than through a small named wrapper like the culture-profile one — worth confirming this matches the codebase's existing convention for e2eTests/rag callsites for consistency.
• Consider a lint rule or exhaustiveness test asserting `FEATURE_GLOBAL_FLAG` and `FEATURE_MODE` stay in sync with `CONVERGED_FEATURE_KEYS` so a future ConvergedFeatureKey addition can't forget its flag entry (partial wiring silently no-ops, per this repo's own config-parity concern).

Concerns raised — review before merging

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

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, 275 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	JSONbored; Gittensor profile; 48 PR(s), 275 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: not available
• Official Gittensor activity: 48 PR(s), 275 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