Skip to content

Upgrade inlineComments + fixHandoff from required-AND opt-in to full config-as-code substitutes #4099

Description

@JSONbored

Part of #4092.

Problem

shouldRequestInlineFindings (src/review/inline-comments.ts) and shouldEmitFixHandoff (src/review/fix-handoff.ts) both require manifestToggle === true && envFlag(env) && isConvergenceRepoAllowed(env, repo) — ALL THREE. A repo can't turn either feature on through config alone; the env var and allowlist remain hard requirements, unlike rag/reputation/unifiedComment/safety which already route through src/review/feature-activation.ts's resolveConvergedFeature/convergedFeatureActive (env kill-switch → per-repo override → allowlist default, false override for security-sensitive features downgrades to "no opinion" rather than active force-off — see #2386's precedent for safety).

(Correction from this issue's original scope: there is no new "shared resolver" to build — feature-activation.ts already exists. This issue is about MIGRATING these two features onto it.)

Resolved design decisions (2026-07-08, corrects the original text below)

  • The operator's env flag is an ABSOLUTE kill-switch, never bypassable by per-repo config — this matches resolveConvergedFeature's actual behavior for every other converged feature (the env flag always wins; only the allowlist is per-repo-overridable). The deliverable below originally said config-true could turn a feature on "even if the env var... would otherwise say no" — that was imprecise/wrong; only the ALLOWLIST is bypassable, never the env var. An operator-level kill-switch should not be overridable by lower-trust, repo-level config.
  • Namespace: option (b) — kept the existing top-level review.inlineComments/review.fixHandoff field names rather than migrating into the features: block. Lower-risk (no field rename for anyone already using them).

Deliverables

  • Explicit written decision on the namespace question (option b, see above)
  • shouldRequestInlineFindings/shouldEmitFixHandoff migrated: an explicit manifest true/false now fully controls the feature, bypassing the cutover allowlist (NOT the env flag, which stays an absolute kill-switch); unset stays byte-identical to today — regression-tested specifically
  • shouldRenderSuggestions checked — no assumption tied to the old three-gate shape
  • Tests mirroring feature-activation.test.ts's pattern

Expected outcome

An operator can fully enable inline comments or fix-handoff for a specific repo through .gittensory.yml alone, without needing the corresponding allowlist entry (the env var remains the deployment-wide on/off switch either way).

Effort

M — two call sites, careful default-preserving regression tests. Shipped in #4116.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions