Skip to content

fix(selfhost): distinguish a stale codex-reviewer flag name from unconfigured and cool down its breaker - #7473

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
bitfathers94:fix/issue-7466
Jul 20, 2026
Merged

fix(selfhost): distinguish a stale codex-reviewer flag name from unconfigured and cool down its breaker#7473
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
bitfathers94:fix/issue-7466

Conversation

@bitfathers94

Copy link
Copy Markdown
Contributor

fix(selfhost): distinguish a stale codex-reviewer flag name from unconfigured and cool down its breaker

The GITTENSORY_→LOOPOVER_ env rebrand (#5652) retired dual-read of
GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER and hardcoded a strict
LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER === "1" check. An operator whose .env
still uses the legacy name silently reverts to fully-disabled and gets the
exact same generic codex_credential_isolation_required error as someone who
never opted in at all — no signal telling them to rename the var.

Recognize the retired flag name in assertCodexCredentialIsolation (without
honoring it) and throw an actionable "rename it to
LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER" message, but only when the current flag
isn't already correctly set so a mounted CODEX_HOME isn't mislabeled a rename
problem. The message keeps the codex_credential_isolation_required prefix so
the structural circuit breaker still recognizes it. Read via a constant rather
than env.GITTENSORY_... so the retired name stays out of the generated
self-host env reference.

Also widen isStructuralProviderConfigError's regex to match
codex_credential_isolation_required (bare or with a ": rename …" suffix). It's
a deterministic config failure, so it now earns the hour-long structural
cooldown instead of retrying every 60s — the flood pattern that breaker exists
to prevent.

Closes #7466

…nfigured and cool down its breaker

The GITTENSORY_→LOOPOVER_ env rebrand (JSONbored#5652) retired dual-read of
GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER and hardcoded a strict
LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER === "1" check. An operator whose .env
still uses the legacy name silently reverts to fully-disabled and gets the
exact same generic codex_credential_isolation_required error as someone who
never opted in at all — no signal telling them to rename the var.

Recognize the retired flag name in assertCodexCredentialIsolation (without
honoring it) and throw an actionable "rename it to
LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER" message, but only when the current flag
isn't already correctly set so a mounted CODEX_HOME isn't mislabeled a rename
problem. The message keeps the codex_credential_isolation_required prefix so
the structural circuit breaker still recognizes it. Read via a constant rather
than env.GITTENSORY_... so the retired name stays out of the generated
self-host env reference.

Also widen isStructuralProviderConfigError's regex to match
codex_credential_isolation_required (bare or with a ": rename …" suffix). It's
a deterministic config failure, so it now earns the hour-long structural
cooldown instead of retrying every 60s — the flood pattern that breaker exists
to prevent.

Closes JSONbored#7466
@bitfathers94
bitfathers94 requested a review from JSONbored as a code owner July 20, 2026 11:30
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.36%. Comparing base (fbd1cba) to head (8f3d2e9).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7473   +/-   ##
=======================================
  Coverage   91.36%   91.36%           
=======================================
  Files         717      717           
  Lines       73016    73020    +4     
  Branches    21628    21632    +4     
=======================================
+ Hits        66713    66717    +4     
  Misses       5265     5265           
  Partials     1038     1038           
Flag Coverage Δ
shard-1 34.16% <100.00%> (+<0.01%) ⬆️
shard-2 39.31% <60.00%> (+<0.01%) ⬆️
shard-3 32.60% <20.00%> (-0.01%) ⬇️
shard-4 39.52% <20.00%> (-0.01%) ⬇️
shard-5 36.58% <60.00%> (+<0.01%) ⬆️
shard-6 34.32% <20.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/selfhost/ai.ts 98.18% <100.00%> (+0.01%) ⬆️
src/services/ai-review.ts 96.75% <100.00%> (+<0.01%) ⬆️

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 20, 2026
@loopover-orb

loopover-orb Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-20 11:39:57 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR adds a targeted detection for the retired GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER flag name in assertCodexCredentialIsolation, emitting an actionable rename error while still throwing the generic message when the flag was never set or when the current flag is correctly configured alongside a mounted CODEX_HOME. It also widens isStructuralProviderConfigError's regex to recognize the codex_credential_isolation_required prefix (bare or with a rename suffix) so this deterministic misconfiguration gets the long structural cooldown instead of retrying every 60s. The logic is correctly gated (rename branch only fires when the current flag isn't already '1'), the regex anchor prevents false positives on longer look-alike tokens, and tests cover the legacy-flag, never-configured, and CODEX_HOME-with-valid-opt-in cases distinctly.

Nits — 3 non-blocking
  • The magic numbers in the updated doc comment (2094, 544, 16) at src/services/ai-review.ts:1029 are informational history, not logic, so naming them as constants would be unnecessary busywork.
  • src/selfhost/ai.ts and src/services/ai-review.ts are both growing past typical file-length conventions (501 and 1040 lines respectively) — consider whether the credential-isolation logic warrants its own module eventually.
  • Consider adding a brief note in the self-host docs/env reference pointing out that GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER is retired, for operators who search docs rather than hit the runtime error.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #7466
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 ⚠️ 20/25 Preflight is ready, but the PR body does not name the validation run.
Contributor workload ✅ 10/10 Author activity: 45 registered-repo PR(s), 29 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor bitfathers94; Gittensor profile; 45 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The PR adds detection of the legacy GITTENSORY_ env var to throw a distinct, actionable rename error (only when the current flag isn't already valid) and widens isStructuralProviderConfigError's regex to match codex_credential_isolation_required, giving it the hour-long cooldown — both directly satisfying the issue's two expected behaviors, with tests covering the new branches.

Review context
  • Author: bitfathers94
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 45 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Add validation command/output.
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.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(selfhost): distinguish stale codex-reviewer env var from unconfigured, and cool down its circuit breaker

1 participant