fix(review): an incomplete secret scan holds the gate instead of closing the PR (#9082)

[JSONbored]

Problem

incompletePatchLessSecretScanFinding (src/queue/patchless-secret-scan.ts) emitted code secret_leak whenever a patch-less file's content couldn't be fetched or fully verified within the scan cap — a Contents API 404/5xx, a file over 4MB, a rate limit, or the fetch concurrency budget being exhausted marks every eligible patch-less file this way. It shared the code with a genuinely matched credential.

secret_leak is designed to be unconditional and breaker-exempt (CONCRETE_EVIDENCE_BLOCKER_CODES in src/settings/agent-actions.ts) — correct for a real leak, catastrophic for "we couldn't read the file." An unreadable file therefore auto-closed a legitimate contributor PR with zero live re-check ever getting a chance to clear it.

Confirmed live (edge-nl-01 audit ledger): 4 contributor PRs were closed this exact way — metagraphed#4600, loopover#7155, #7431, #8376.

Fix

Gives the incomplete case its own code, secret_scan_incomplete, routed through evaluateGateCheckCore's no-deterministic-blocker branch — mirroring the existing ai_review_inconclusive pattern exactly.

Important correctness detail caught while building this: the obvious first approach — adding the new code to isEvaluationBlocker's unconditional first check (same as pre_merge_check_unresolved) — is wrong. That check runs before any deterministic blockers are computed, so it would let one unreadable file's secret_scan_incomplete finding silently mask a real, confirmed secret_leak on a different file in the same PR, downgrading a genuine credential leak to "held for a human" instead of hard-blocking it. ai_review_inconclusive already solves exactly this class of problem by being checked only inside the blockers.length === 0 branch — i.e. only relevant once nothing else has already hard-blocked. The new code follows that same shape, verified with a dedicated regression test (secret_leak + secret_scan_incomplete together on one PR → still failure).

secret_leak itself is completely unchanged — a genuine match still hard-blocks unconditionally, no opt-in, no downgrade.

Also corrects a stale comment claiming secret_leak is "produced ONLY by the flag-gated safety scan" — it's also produced unconditionally by maybeAddSecretLeakFinding (#audit-3.4) regardless of LOOPOVER_REVIEW_SAFETY.

Tests

• Gate-level regression (test/unit/gate-check-policy.test.ts): secret_scan_incomplete alone → neutral hold; secret_leak + secret_scan_incomplete together → still failure with secret_leak in blockers (the exact masking bug this PR avoids).
• End-to-end wiring (test/unit/patchless-secret-scan.test.ts): the maybeAddSecretLeakFinding fetcher-rejection test rewritten to assert the new code is present and secret_leak is absent.
• Unit-level: the direct incompletePatchLessSecretScanFinding code assertion updated.

Validation

• npx vitest run test/unit/gate-check-policy.test.ts test/unit/patchless-secret-scan.test.ts test/unit/rules.test.ts — 282/282 passed.
• Also ran the 5 other test files referencing secret_leak across the repo (configured-gate-blocker-signals, agent-actions, safety-wiring, outcomes-wire, salvageability, backtest-logic-check-core, content-lane-wire) — 670/670 passed, no regressions.
• Coverage on all 3 changed source files: 100% (patchless-secret-scan.ts confirmed directly; the other two sit well outside their files' only uncovered ranges).
• tsc --noEmit --incremental false clean.
• Prettier flags all 5 touched files identically on main (pre-existing repo-wide state) — left unreformatted.

Closes #9082

[superagent-security]

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

[github-actions]

Logic backtest

Replayed 0 historical case(s) for linked_issue_scope_mismatch through the base (3c1173e) and head (9bcfbf3) versions of its detection logic (corpus checksum 4f53cda18c2b).

Backtest comparison: linked_issue_scope_mismatch

Verdict: unchanged — no comparable axis moved.

Advisory only — this check never blocks merge (#8105).

[codecov]

❌ 3 Tests Failed:

Tests completed	Failed	Passed	Skipped
21586	3	21583	21

View the top 3 failed test(s) by shortest run time

test/unit/check-engine-parity-script.test.ts > check-engine-parity script > runEngineParityMain returns 0 for the real monorepo workspace

Stack Traces | 0.0435s run time

AssertionError: expected 1 to be +0 // Object.is equality

- Expected
+ Received

- 0
+ 1

 ❯ test/unit/check-engine-parity-script.test.ts:798:48

test/unit/check-engine-parity-script.test.ts > check-engine-parity script > engine version skew > uses default version readers against the real monorepo workspace

Stack Traces | 0.0547s run time

AssertionError: expected [ Array(1) ] to deeply equal []

- Expected
+ Received

- []
+ [
+   "Gate-decision logic change in src/rules/advisory.ts requires either:
+   • a matching edit to the other twin (.../src/advisory/gate-advisory.ts), or
+   • a version bump in packages/loopover-engine/package.json (currently 3.15.0 vs base 3.15.0).",
+ ]

 ❯ test/unit/check-engine-parity-script.test.ts:785:31

test/unit/check-engine-parity-script.test.ts > check-engine-parity script > prints a clean summary and exits 0 for the real repo state when run as a subprocess

Stack Traces | 0.861s run time

Error: Command failed: .../loopover/node_modules/.bin/tsx scripts/check-engine-parity.ts
Engine-parity check found 1 issue(s):
Gate-decision logic change in src/rules/advisory.ts requires either:
  • a matching edit to the other twin (.../src/advisory/gate-advisory.ts), or
  • a version bump in packages/loopover-engine/package.json (currently 3.15.0 vs base 3.15.0).

 ❯ test/unit/check-engine-parity-script.test.ts:803:20

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { status: 1, signal: null, output: [ null, '', 'Engine-parity check found 1 issue(s):\nGate-decision logic change in src/rules/advisory.ts requires either:\n  • a matching edit to the other twin (.../src/advisory/gate-advisory.ts), or\n  • a version bump in packages/loopover-engine/package.json (currently 3.15.0 vs base 3.15.0).\n' ], pid: 7096, stdout: '', stderr: 'Engine-parity check found 1 issue(s):\nGate-decision logic change in src/rules/advisory.ts requires either:\n  • a matching edit to the other twin (.../src/advisory/gate-advisory.ts), or\n  • a version bump in packages/loopover-engine/package.json (currently 3.15.0 vs base 3.15.0).\n' }

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

[loopover-orb]

Warning

⏸️ LoopOver review result - manual review recommended

_{Review updated: 2026-07-26 17:51:44 UTC}

5 files · 1 AI reviewer · no blockers · CI pending · unstable

⏸️ Suggested Action - Manual Review

Review summary
This PR gives the patch-less secret-scan's incomplete-verification case its own finding code (`secret_scan_incomplete`) instead of overloading `secret_leak`, which previously routed a Contents-API fetch failure/size-cap/rate-limit through the same unconditional, breaker-exempt hard block reserved for a real matched credential. The new code is wired through `evaluateGateCheckCore`'s no-deterministic-blocker branch (mirroring `ai_review_inconclusive`), added to `NEUTRAL_HOLD_REASON_CODES` in `parity-wire.ts`, and deliberately left unhandled (falls to "off") in `resolveConfiguredGateMode` so it can never itself become a configured blocker. Two targeted regression tests confirm both directions: an isolated `secret_scan_incomplete` holds neutral, and a real `secret_leak` alongside it on a different file still hard-fails — exactly the ordering-safety property the fix claims.

Nits — 5 non-blocking

• The rationale comment explaining why `secret_scan_incomplete` gets its own code (and why it must never route through `secret_leak`'s block branch) is duplicated almost verbatim across `src/queue/patchless-secret-scan.ts:159-168` and `src/rules/advisory.ts` (the `resolveConfiguredGateMode` hunk) — consider keeping the full rationale in one place and linking to it from the other.
• The two CI failures (`validate-tests`, `validate`) have no detail provided, and this branch is 2 commits behind the default branch, so the cause can't be confirmed from this diff — rebase onto the latest default branch before assuming these failures indicate a defect in this change.
• `test/unit/patchless-secret-scan.test.ts` content wasn't available for review in this pass, so the two touched test blocks there could only be verified via the diff hunks shown, not the full file.
• Consider extracting the shared `secret_leak` vs `secret_scan_incomplete` rationale into a single doc comment referenced by both files to avoid the two copies drifting apart on a future edit.
• If CI failures persist after rebasing onto the current default branch, re-run and attach the actual failure output so the cause (rather than branch drift) can be confirmed.

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	#9082
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: 13 registered-repo PR(s), 13 merged, 323 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	JSONbored; Gittensor profile; 13 PR(s), 323 issue(s).
Improvement	✅ Minor	risk: clean · value: minor · LLM: significant

Linked issue satisfaction

Addressed
The PR introduces a distinct secret_scan_incomplete code that routes through evaluateGateCheckCore's neutral-hold branch (mirroring ai_review_inconclusive) instead of the unconditional, breaker-exempt secret_leak block, while a regression test confirms a genuine secret_leak on another file in the same PR still hard-blocks.

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, Ruby, Go, MDX, Shell, Solidity, JavaScript
• Official Gittensor activity: 13 PR(s), 323 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Start here: 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.

🧪 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 <question> answers contribution-quality Q&A with source citations and freshness.
• @loopover chat <question> 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