Skip to content

feat(review): hard-guardrail + owner-PR guard in auto-maintain (rollout-blocker for autonomy) - #1050

Merged
JSONbored merged 3 commits into
mainfrom
feat/auto-maintain-guardrail
Jun 22, 2026
Merged

feat(review): hard-guardrail + owner-PR guard in auto-maintain (rollout-blocker for autonomy)#1050
JSONbored merged 3 commits into
mainfrom
feat/auto-maintain-guardrail

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Why

Before auto-merge/auto-close autonomy can be enabled on any converged repo, the auto-maintain path needs two safety properties it currently lacks (verified by audit):

  1. No hard-guardrail enforcement. The planner (agent-actions.ts) is structurally blind to which files a PR touches. A gate-passing, mergeable, approved PR would auto-merge regardless of path — reproducing the awesome-claude feat(review): auto-trigger E2E test generation on the manifest_missing_tests signal #4196 incident class (a weakened policy script auto-merged because its path wasn't guarded). Nothing in gittensory guarded src/scoring/**, src/auth/**, scripts/**, .github/workflows/**, etc.
  2. No owner-PR protection. The executed path had no rule against auto-closing the repo owner's own PRs — violating the standing "never auto-close JSONbored's PRs" rule.

What

  • src/signals/change-guardrail.ts — pure glob matcher ported from reviewbot core/change-classifier (* within a segment, ** across /).
  • src/review/guardrail-config.ts — resolves each repo's hardGuardrailGlobs from the shared REVIEW_CONFIG KV (keyed by repo slug), with a conservative .github/workflows/** + scripts/** fallback. Operator tuning stays in KV, never in this repo — the rich per-repo glob lists are private runtime config; only the mechanism + a generic floor are public.
  • planner — a changed path hitting a guardrail suppresses auto-merge, auto-approve, and auto-close (so it can't later self-satisfy a merge). label + request_changes still run, so the PR simply falls through to a human. Owner-authored PRs are never auto-closed (merge still allowed when clean + passing).
  • wiringmaybeRunAgentMaintenance now loads the PR's changed paths + the repo's globs + authorIsOwner and feeds them to the planner. Adds the REVIEW_CONFIG KV binding.
  • tests — glob semantics + guardrail-blocks-merge/close/approve + owner-guard-blocks-close-not-merge + non-matching-path-still-merges.

Fail-safe throughout: no binding ⇒ conservative default globs; config read errors ⇒ default; flag-OFF behavior unchanged (autonomy is observe everywhere until separately enabled).

Part of the convergence epic #983 (prerequisite for the auto-maintain autonomy rollout). Typecheck clean; full unit suite (3301 tests) green.

The converged auto-maintain planner had no path awareness: a gate-passing, mergeable, approved PR would auto-merge regardless of which files it touched, and a non-passing PR tripping a noise signal would auto-close — including the repo owner's own PRs. This reproduces the awesome-claude #4196 incident class (a weakened policy script auto-merging because its path wasn't guarded) and violated the standing 'never auto-close the owner's PRs' rule.

- change-guardrail.ts: pure glob matcher ported from reviewbot core/change-classifier (`*` within a segment, `**` across `/`).
- guardrail-config.ts: resolve per-repo hardGuardrailGlobs from the shared REVIEW_CONFIG KV (operator tuning stays in KV, never in the repo) with a conservative .github/workflows + scripts fallback.
- planner: a changed path hitting a guardrail suppresses auto-merge, auto-approve AND auto-close; label + request_changes still run so the PR falls to a human. Owner-authored PRs are never auto-closed (merge still allowed when clean).
- wire changedPaths + globs + authorIsOwner from maybeRunAgentMaintenance; bind REVIEW_CONFIG KV; tests for glob semantics + guardrail/owner gating.
@dosubot dosubot Bot added the size:L label Jun 22, 2026
@ghost

ghost commented Jun 22, 2026

Copy link
Copy Markdown

Note

🟦🟦🟦🟦🟦🟦🟦🟦🟦🟦🟦🟦

💡 Gittensory review — advisory only

0 files · no blockers · readiness 0/100

💡 Advisory only — no action taken

Signal Result Evidence
Code review ✅ No blockers synthesized
Gate result ⚠️ Skipped #1050 is no longer open.

💰 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.

@ghost ghost added the gittensory:reviewed label Jun 22, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

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

@JSONbored JSONbored self-assigned this Jun 22, 2026
@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.28571% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.85%. Comparing base (5bc8140) to head (7cd9bd3).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/queue/processors.ts 80.00% 0 Missing and 1 partial ⚠️
src/signals/change-guardrail.ts 93.75% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1050       +/-   ##
===========================================
+ Coverage   71.78%   96.85%   +25.06%     
===========================================
  Files         141      143        +2     
  Lines       17506    17538       +32     
  Branches     6334     6347       +13     
===========================================
+ Hits        12567    16986     +4419     
+ Misses       3785      104     -3681     
+ Partials     1154      448      -706     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored
JSONbored merged commit 9d4b852 into main Jun 22, 2026
19 checks passed
@JSONbored
JSONbored deleted the feat/auto-maintain-guardrail branch June 22, 2026 18:02
JSONbored added a commit that referenced this pull request Jun 22, 2026
…ke timers)

The test drove an async flow — real WebCrypto token-decrypt + async D1/fetch interleaved with one setTimeout(sleep 3000) — via a fake-timer pump loop. Under CI full-shard coverage load the real macrotask lagged the microtask flush the pump relied on, so the scheduled sleep was never fired and the test HUNG → 15s timeout (the CI-only flake that blocked #1050/#1051/#1052/#1056).

Fix: drop fake timers entirely. Mock setTimeout so the sleep(3000) backoff fires on a real 0ms tick, then await processSubmitDraft directly — the flow runs to completion on the real event loop (probe 404 → instant backoff → probe 200 → open PR) with no pump, no race, no real 3s wait, no weakened assertions. Verified stable across 6+ consecutive plain + coverage runs locally.
JSONbored added a commit that referenced this pull request Jun 22, 2026
…ke timers) (#1057)

The test drove an async flow — real WebCrypto token-decrypt + async D1/fetch interleaved with one setTimeout(sleep 3000) — via a fake-timer pump loop. Under CI full-shard coverage load the real macrotask lagged the microtask flush the pump relied on, so the scheduled sleep was never fired and the test HUNG → 15s timeout (the CI-only flake that blocked #1050/#1051/#1052/#1056).

Fix: drop fake timers entirely. Mock setTimeout so the sleep(3000) backoff fires on a real 0ms tick, then await processSubmitDraft directly — the flow runs to completion on the real event loop (probe 404 → instant backoff → probe 200 → open PR) with no pump, no race, no real 3s wait, no weakened assertions. Verified stable across 6+ consecutive plain + coverage runs locally.
ghost pushed a commit that referenced this pull request Jun 23, 2026
…hs (#1062)

The auto-maintain planner (#1050) suppressed auto-merge/close/approve only when a
changed path hit a hard-guardrail glob. But maybeRunAgentMaintenance reads the
changed paths from the pull_request_files cache without refreshing, so an empty
(fresh PR pre-backfill) or stale (post-synchronize) cache yields changedPaths=[],
making guardrailHit=false — letting the automation auto-merge/close a PR that
actually touches a guarded path (.github/workflows, scripts, scoring/auth).

When guardrails are configured but the changed-file set is unknown (empty), treat
it as a hit: we cannot prove the PR is safe, so it must fall through to a human.
Repos with no guardrails configured stay permissive.

Closes #1061

Co-authored-by: galuis116 <galuis116@users.noreply.github.com>
Co-authored-by: ghost <49853598+JSONbored@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant