fix(selfhost): disambiguate red-CI hold reasons and enrich hold audit metadata - #3288
Conversation
…nrich hold audit metadata agentHoldAuditDetail's ciState==="failed" branch returned a bare, unexplained "auto-action held because CI is failing but no close action was planned" for EVERY red-CI hold, unconditionally -- it never checked protected-author or close-autonomy the way the gate-blocker-codes branch a few lines below it already did. That made a protected owner/admin/automation author and a "close autonomy isn't auto yet" repo indistinguishable from each other, and from a genuinely unexplained hold, in the single most common real-world cause of this message on a self-hosted instance. Extract the shared protected-author/close-autonomy disambiguation into closeWithheldReason() and apply it to BOTH branches, so a red-CI hold now reports "close withheld for protected author" or "close withheld because close autonomy is <level>" exactly like a gate-blocker hold already does. Also persist the previously-missing structured fields on the agent.action.hold audit event's metadata (headSha, gateBlockerTitles, ciFailingCheckNames, closeEligible, closeAutonomy, mergeAutonomy, protectedAuthor flags, closeOwnerAuthors, precisionBreakerEngaged/directions) so a hold is fully debuggable from the audit table alone. Investigated whether contributor + red CI + close=auto genuinely fails to plan a close (the reported "biggest product issue"): it does not -- planAgentMaintenanceActions' willClose condition already covers this case correctly (see the existing "blocked contributor PR ... close=auto" regression test), and this fix's own new tests confirm a hold only ever reaches the generic fallback when the author is protected AND close autonomy isn't auto -- i.e. every reachable hold now has a specific, correct explanation.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Tip 🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩 ✅ Gittensory review result - approve/merge recommendedReview updated: 2026-07-05 01:02:23 UTC
✅ Suggested Action - Approve/Merge
Review summary Nits — 4 non-blocking
Review context
Contributor next steps
Signal definitions
🟩 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.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3288 +/- ##
=======================================
Coverage 94.13% 94.14%
=======================================
Files 276 276
Lines 30236 30246 +10
Branches 11016 11021 +5
=======================================
+ Hits 28464 28474 +10
Misses 1127 1127
Partials 645 645
🚀 New features to boost your workflow:
|
Summary
agentHoldAuditDetail'sciState === "failed"branch returned a bare "auto-action held because CI is failing but no close action was planned" for every red-CI hold, unconditionally — it never checked protected-author or close-autonomy the way thegateBlockerCodes.length > 0branch a few lines below it already did. That made "protected owner/admin/automation author" and "close autonomy isn't set to auto yet" indistinguishable from each other and from a genuinely unexplained hold — the single most common real-world cause of this exact opaque message.closeWithheldReason()and applied it to both branches, so a red-CI hold now reports"close withheld for protected author"or"close withheld because close autonomy is <level>"exactly like a gate-blocker hold already did.agent.action.holdaudit event'smetadata:headSha,gateBlockerTitles,ciFailingCheckNames,closeEligible,closeAutonomy,mergeAutonomy,protectedAuthor(owner/admin/automation flags),closeOwnerAuthors,precisionBreakerEngaged/precisionBreakerDirections— so a hold is fully debuggable from the audit table alone, without re-deriving eligibility from settings by hand.planAgentMaintenanceActions'swillClosecondition already covers this correctly (see the pre-existing"blocked contributor PR ... close=auto is actually closed"regression test), and this PR's new tests confirm a hold only ever reaches the generic fallback when the author is protected and close autonomy isn't auto — i.e. every reachable hold now has a specific, correct explanation rather than the previous unconditional opaque one.This is the first of several follow-ups from a live Grafana/Loki review of the self-host stack; the other four (Orb relay registration alerting, Codex provider timeout fallback/correlation, retryable PR-freshness reclassification, and the maintenance-admission log-level downgrade) are separate, independently-isolated PRs in progress.
Scope
type(scope): short summaryConventional Commit format.src/queue/processors.ts) plus its two test files, one coherent fix.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run typechecknpx vitest run test/unit/precision-breakers-chain.test.ts test/unit/queue.test.ts— 512/512 passing, including new regression tests for: red-CI + protected author (owner/admin/automation) → specific reason; red-CI +closeOwnerAuthors: true→ falls through correctly; red-CI +closeAutonomy: observe→ specific reason; protected-author precedence over close-autonomy; and an end-to-end webhook-driven test asserting the enrichedmetadata_jsonshape.npm run test:changed— 56 affected test files, 1577 tests, all greennpm run test:ci— full local gate, green?? nullfallback marked/* v8 ignore next */(a real GitHub PR always carries a head sha by the time it reaches this path — mirrors 8 other existing/* v8 ignore next */annotations already in this same file for the same class of "structurally unreachable on the real webhook path" fallback).Safety
planAgentMaintenanceActions,applyPrecisionBreakers) is untouched — only the after-the-fact audit description of an already-decided hold changed, plus new metadata fields on an existing audit event. No repo's automation behavior changes as a result of this PR.Notes
.gittensory.ymldoesn't need any change — this is a pure engine fix, not a production config change.