Skip to content

fix(agent-actions): close hard-blocked contributor PRs in auto mode - #2856

Merged
JSONbored merged 7 commits into
mainfrom
claude/close-hard-blocked-contributor-prs
Jul 4, 2026
Merged

fix(agent-actions): close hard-blocked contributor PRs in auto mode#2856
JSONbored merged 7 commits into
mainfrom
claude/close-hard-blocked-contributor-prs

Conversation

@JSONbored

@JSONbored JSONbored commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Summary

  • Live VPS evidence (system_flags on edge-us-01) showed closehold:<repo> engaged for all three review-active repos since 2026-06-28 07:00:17, silently downgrading every heuristic close to a human hold — regardless of autonomy.close=auto, agent_dry_run=0, and correct repo config. Investigated the full gate-to-action decision path (planAgentMaintenanceActionsapplyPrecisionBreakersexecuteAgentMaintenanceActions) and found three compounding, generic self-host engine bugs (no repo-specific logic anywhere in this diff):
    1. The close-precision breaker was too coarse. downgradeCloseToHold downgraded every closeKind: "heuristic" close identically, including ones backed by concrete, non-judgment evidence — a committed secret, red CI, a base conflict, or a deterministic linked-issue-overlap duplicate. Added closeConcreteEvidence to PlannedAgentAction, round-tripped it through the approval-queue staging path (AgentPendingActionParams, actionParams), and scoped the breaker to exempt it — a hard blocker now closes even while the breaker is engaged, matching the existing exemption already given to blacklist/contributor_cap/review_nag/linked-issue-hard-rule closes. Both ai_consensus_defect (a dual-model AI verdict) and ai_review_split stay fully subject to the breaker — the gate review (round 2) correctly flagged an earlier version of this diff for treating a dual-model AI consensus as "concrete": two models agreeing is still a judgment call, not deterministic evidence, and a systematically-wrong AI-driven close is exactly the failure mode this breaker exists to catch. hasConcreteCloseEvidence now also defensively excludes anything in advisory.ts's own AI_JUDGMENT_BLOCKER_CODES, so this can't silently regress again.
    2. The self-tune breaker's auto-clear tick had a starvation deadlock. runSelfTuneBreaker only reconsidered projects present in the current computeGateEval report, but a project whose closes are 100% suppressed stops producing new decided samples for that action class and can drop out of the report entirely — stranding its flag engaged forever, no matter how long the 24h cooldown has elapsed. Widened the auto-clear candidate set to the union of the eval report and every currently-engaged per-project flag (still never touches a human-set :global flag).
    3. The eval was scored against dead data. computeGateEval read across every review_audit source, including the frozen, no-longer-written reviewbot source from before the gittensory-native convergence (verified on the VPS: 256 old close predictions vs. zero from gittensory-native, which only ever records merge/hold for its own gate check). A dead system's historical close predictions could permanently anchor a live repo's measured close precision, so it could never legitimately "recover." Scoped the tick to source: 'gittensory-native'.
  • Also adds a bounded-cardinality gittensory_precision_breaker_downgrades_total{direction} counter (extracted as a pure, directly-tested precisionBreakerDowngradeDirections helper) so an engaged breaker is visible without querying review_audit directly — no raw SHAs/PR numbers/free text. precisionBreakerDowngradeDirections compares each planned merge/close action's survival by reference identity (not "is any action of that class still present anywhere"), so a plan carrying both a kept deterministic close and a dropped heuristic close is still correctly counted as a close downgrade — another defect the round-2 gate review caught in the first version of this metric.
  • Verified current-head reconciliation (missing last_regated_at/no current-head gate outcome) is already adequately covered by the existing hourly per-repo sweep plus the foreground-liveness/reconciliation-repair work already merged in fix(selfhost): keep review queue live and verify configured CI #2843/fix(queue): prioritize live PR work under GitHub rate pressure #2847 — no additional code needed there.
  • Two of the nine gate-blocker codes in CONCRETE_EVIDENCE_BLOCKER_CODES now reuse advisory.ts's/pre-merge-checks.ts's existing exported constants (DUPLICATE_ONLY_BLOCKER_CODES, PRE_MERGE_CHECK_BLOCKING_CODE) instead of retyping the literal; the remaining seven have no single canonical export to import (each is either module-private or a literal duplicated across several unrelated producer files), so a new source-text parity test guards them against silent producer-side drift instead.

Scope

  • Conventional Commit title
  • Focused: src/settings/agent-actions.ts, src/types.ts, src/services/agent-action-executor.ts, src/review/outcomes-wire.ts, src/queue/processors.ts, src/selfhost/metrics.ts + their test files only
  • Generic self-host engine behavior — no repo names, no hardcoded rules

Validation

  • git diff --check
  • npm run typecheck
  • Full affected suite: agent-actions.test.ts, agent-action-executor.test.ts, outcomes-wire.test.ts, precision-breakers-chain.test.ts, agent-approval-queue.test.ts, auto-tune.test.ts — 415/415 passing
  • npm audit --audit-level=moderate — 0 vulnerabilities
  • Verified 100% branch coverage on every changed line via v8 coverage report cross-referenced against the diff
  • npm run test:coverage (full unsharded) — not re-run locally beyond the directly-affected suite; CI runs the full gate unsharded

If any required check was skipped, explain why:

  • Full test:coverage/test:workers/UI checks were not re-run locally beyond the directly-affected suite, per this repo's own guidance that CI already runs the full gate unsharded.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, trust scores, or private maintainer evidence touched or exposed
  • No public-facing text changes
  • Not an auth/CORS/session change

Notes

  • Live VPS evidence used to diagnose this (read-only SELECT queries against system_flags/review_audit over the existing Tailscale SSH access) is not reproduced here — only the code fix and its reasoning.
  • Round 2 of the gate review flagged two real defects in the first version of this diff (the AI-consensus-as-concrete-evidence misclassification, and the downgrade-metric undercounting a mixed close plan) plus five related nits; all seven items are addressed in the latest commit.

@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.02%. Comparing base (883a237) to head (ad04158).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2856   +/-   ##
=======================================
  Coverage   96.01%   96.02%           
=======================================
  Files         259      259           
  Lines       28304    28334   +30     
  Branches    10291    10298    +7     
=======================================
+ Hits        27177    27207   +30     
  Misses        491      491           
  Partials      636      636           
Files with missing lines Coverage Δ
src/queue/processors.ts 92.67% <100.00%> (+0.02%) ⬆️
src/review/outcomes-wire.ts 89.13% <100.00%> (+1.63%) ⬆️
src/selfhost/metrics.ts 100.00% <ø> (ø)
src/services/agent-action-executor.ts 96.36% <100.00%> (+0.01%) ⬆️
src/settings/agent-actions.ts 94.48% <100.00%> (+0.15%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

loopover-orb Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-04 02:33:52 UTC

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

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
This change coherently threads a concrete-evidence marker through heuristic close planning, approval-queue persistence, breaker application, and observability, while also fixing the self-tune tick to use native gate data and reconsider currently engaged project-scoped flags. The close-breaker exemption is narrowly scoped away from AI-derived findings, and the added tests exercise the key planner, breaker, persistence, and auto-clear paths visible in the diff. I do not see a reachable correctness break in the provided hunks.

Nits — 6 non-blocking
  • nit: src/settings/agent-actions.ts:116 carries a hand-maintained blocker-code allowlist whose source-text parity test can still pass if the real producer stops feeding that code into gateBlockerCodes, so this remains a drift-prone contract.
  • nit: src/review/outcomes-wire.ts:126 intentionally uses a plain else to classify every non-holdonly matched row as closehold, which is safe for today's SQL but fragile if the query grows another prefix.
  • nit: src/review/outcomes-wire.ts:144 only logs engaged-scope scan failures, so an operator may miss that stale breaker flags are no longer being reconsidered during that tick.
  • nit: test/unit/agent-actions.test.ts:1002 uses cwd-relative source scanning as a contract test, which is brittle under test-runner cwd changes and refactors that keep behavior but change source shape.
  • src/services/agent-action-executor.ts:676: add one accept-time integration test with closehold engaged and a staged closeConcreteEvidence=true action, so the test covers the actual persisted-action execution path rather than only actionParams plus pendingActionToPlanned.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No 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: 60 registered-repo PR(s), 51 merged, 440 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 60 PR(s), 440 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 60 PR(s), 440 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.
  • No action.
  • 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

@JSONbored JSONbored self-assigned this Jul 4, 2026
@JSONbored
JSONbored force-pushed the claude/close-hard-blocked-contributor-prs branch 4 times, most recently from ea61c4b to 87a286b Compare July 4, 2026 01:38
JSONbored added 6 commits July 3, 2026 18:52
VPS evidence (system_flags) showed closehold:<repo> engaged for all
three review-active repos since 2026-06-28, silently downgrading every
heuristic close to a human hold regardless of autonomy.close=auto.
Three compounding, generic self-host engine bugs:

- The close-precision circuit breaker treated every heuristic close
  identically, including ones backed by concrete, non-judgment
  evidence (a committed secret, red CI, a base conflict, a
  deterministic linked-issue duplicate, a dual-model AI consensus
  defect). Added closeConcreteEvidence, round-tripped through the
  approval-queue staging path, and scoped downgradeCloseToHold to
  exempt it - a hard blocker now closes even while the breaker is
  engaged, matching every other deterministic close kind.
- The self-tune breaker's auto-clear tick only reconsidered projects
  present in the current gate-eval report, but a project whose closes
  are 100% suppressed stops producing new decided samples for that
  action class and can drop out of the report entirely - stranding
  its flag engaged forever regardless of cooldown. Widened the
  auto-clear candidate set to the union of the eval report and every
  currently-engaged per-project flag.
- The eval read across every review_audit source, including the
  frozen, no-longer-written 'reviewbot' source from before the
  gittensory-native convergence. A dead system's historical close
  predictions could permanently anchor a live repo's measured close
  precision. Scoped the tick to source='gittensory-native'.

Also adds a bounded-cardinality gittensory_precision_breaker_downgrades_total
counter so an engaged breaker is visible without querying review_audit
directly.
…tric miscount

Gate review (round 2) found two reachable defects in the prior commit:

- ai_consensus_defect was wrongly classified as closeConcreteEvidence,
  letting a green PR with only an AI-derived blocker bypass the
  close-precision breaker it exists to check. Two independent models
  agreeing is still a judgment call, not deterministic evidence -
  removed it from CONCRETE_EVIDENCE_BLOCKER_CODES, and hardened
  hasConcreteCloseEvidence to also exclude anything in advisory.ts's
  own AI_JUDGMENT_BLOCKER_CODES so this can't silently regress again.
- precisionBreakerDowngradeDirections detected a close downgrade by
  checking whether any close action remained in the post-breaker plan,
  so a plan with a kept deterministic close plus a dropped heuristic
  close recorded no downgrade. Switched to reference-identity survival
  checks (does this specific planned action still exist in the
  breaker's output), which correctly handles multiple close actions.

Also reuses advisory.ts's/pre-merge-checks.ts's existing exported code
constants for two of the nine concrete-evidence codes instead of
retyping them, and adds a source-text parity test guarding the
remaining seven against silent producer-side drift.
… fix

npm run test:coverage (and CI's validate/validate-code) failed with
"DUPLICATE_ONLY_BLOCKER_CODES is not iterable": agent-actions.ts sits
inside a genuine module-load cycle (scoring/model.ts -> db/repositories.ts
-> agent-actions.ts -> rules/advisory.ts -> scoring/preview.ts ->
scoring/model.ts), and eagerly spreading another module's export into a
top-level array literal reads it before that module has necessarily
finished initializing on the cycle's first pass. Reverted the two
CONCRETE_EVIDENCE_BLOCKER_CODES entries back to plain string literals
and extended the parity test to cover all nine codes instead of seven.
AI_JUDGMENT_BLOCKER_CODES stays imported - it is only read inside a
function body, not at module-eval time, so it isn't exposed to the
same hazard.
codecov/patch flagged src/queue/processors.ts's incr() call inside
runAgentMaintenancePlanAndExecute as uncovered: precisionBreakerDowngradeDirections
and applyPrecisionBreakers were already exercised as pure functions in
precision-breakers-chain.test.ts, but nothing drove the LIVE integration
path (a real webhook, a real engaged breaker, a real withheld mutation)
through this exact call site. Added an end-to-end regression alongside
the existing convergence-chain test, seeding a holdonly breaker flag
before the CI-completion re-review step and asserting both the withheld
merge and the resulting counter.
…dProjectScopes

codecov/patch flagged an else-if arm as a partial branch: the query's
own WHERE clause (key LIKE 'holdonly:%' OR 'closehold:%') guarantees
prefix can only ever be "holdonly" or "closehold", so the second ===
check could never see any other value. Collapsed to a plain else --
same behavior, no unreachable branch left to cover.
…r, address gate nits

The gate's deterministic secret scanner flagged reason: "leaked secret"
/ "secret leaked" test fixtures as a possible generic_secret_assignment
hit - no real credential, just wording that matched the heuristic's
shape. Reworded to "hard blocker", which the tests don't depend on
semantically.

Also addresses the round-3 review nits:
- Added a dedicated regression test that forces agent-actions.ts and
  scoring/model.ts to load together in the same module graph, so a
  reintroduced eager cross-module read on the documented load cycle
  fails a test directly instead of only being caught incidentally by
  unrelated suites.
- Tightened the CONCRETE_EVIDENCE_BLOCKER_CODES parity test to require
  the actual producer assignment shape (code: "..." or a SOME_CONST =
  "..." export) rather than the bare literal appearing anywhere in the
  file, which a stale comment could have satisfied.
- Documented the maintenance path in listEngagedProjectScopes's plain
  else: if the WHERE clause ever grows a third prefix, it must go back
  to an explicit branch with its own test, not stay silently bucketed
  as closehold.
@JSONbored
JSONbored force-pushed the claude/close-hard-blocked-contributor-prs branch from 87a286b to 400e03d Compare July 4, 2026 01:53
…gression

The deterministic secret scanner's generic_secret_assignment pattern
matches any `token: "..."` (or api_key/secret/password/etc.) assignment
whose value is 16+ characters and doesn't look like a known placeholder
string. The new holdonly-breaker test's hand-rolled fetch stub used
"installation-token" (18 chars, no placeholder markers) in an ADDED
line, which the scanner flags regardless of the hundreds of pre-existing,
unchanged occurrences of that same literal elsewhere in this file.
Shortened to a value under the length threshold; the token's actual
content was never asserted on.
@JSONbored
JSONbored merged commit ab91d82 into main Jul 4, 2026
13 checks passed
@JSONbored
JSONbored deleted the claude/close-hard-blocked-contributor-prs branch July 4, 2026 02:39
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.

1 participant