Skip to content

[Bug]: auto-maintain guardrail fails open on empty/stale changed-files cache #1061

Description

@philluiz2323

[Bug]: auto-maintain hard-guardrail fails OPEN on an empty/stale changed-files cache — can auto-merge/close a PR touching guarded paths

Summary

The autonomous auto-maintain planner (#1050, the "rollout-blocker for autonomy"
guardrail) suppresses the irreversible dispositions (auto-merge / auto-close /
auto-approve) only when a PR's changed path hits a hard-guardrail glob
(.github/workflows/**, scripts/**, scoring/auth, …). But the changed paths are
read from the pull_request_files DB cache with no freshness guarantee, and
maybeRunAgentMaintenance never refreshes them. When that cache is empty (a fresh
PR processed before the async file backfill) or stale (a synchronize added a
guarded file after the cache was last populated), changedPaths is []
changedPathsHittingGuardrail([], globs) returns []guardrailHit = false
the automation may auto-merge or auto-close a PR that actually touches a guarded
path. The guardrail fails open, defeating exactly what it was built to prevent.

Evidence

// src/settings/agent-actions.ts:102 — guardrail = "does a changed path hit a guarded glob?"
const guardrailHit = changedPathsHittingGuardrail(input.changedPaths, input.hardGuardrailGlobs).length > 0;
...
const canMerge = passing && acting("merge") && mergeableClean && approvalsSatisfied && !guardrailHit;  // :133
...
} else if (acting("close") && !passing && !guardrailHit && !input.authorIsOwner && !input.authorIsAutomationBot) {  // :141
// src/signals/change-guardrail.ts — empty changedPaths => no hit
export function changedPathsHittingGuardrail(changedPaths: string[], hardGuardrailGlobs: string[]): string[] {
  if (hardGuardrailGlobs.length === 0) return [];
  return changedPaths.filter((path) => path.length > 0 && matchesAny(path, hardGuardrailGlobs));  // [] in -> [] out
}
// src/queue/processors.ts:488 — maybeRunAgentMaintenance reads the CACHE, never refreshes
const [changedFiles, hardGuardrailGlobs] = await Promise.all([
  listPullRequestFiles(env, repoFullName, pr.number),   // DB-only read of pull_request_files
  loadHardGuardrailGlobs(env, repoFullName),
]);
const changedPaths = changedFiles.map((file) => file.path).filter((path) => path.length > 0);  // can be []

listPullRequestFiles (src/db/repositories.ts) never calls GitHub; the
pull_request_files table is populated only by refreshPullRequestDetails, which
on the webhook path runs solely under the slop/manifest condition
(processors.ts:1010, gated by shouldCollectSlopEvidence(settings) || settings.manifestPolicyGateMode !== "off"). maybeRunAgentMaintenance
(processors.ts:1032) is invoked independently of that condition, so with the
slop and manifest gates off (both default "off") no refresh ever runs before
the guardrail read.

The function's own comment states the intended contract — "feed the planner the
PR's changed paths … so guarded paths force manual review" — but it silently
operates on an empty path set.

Reachability

A repo with autonomy auto-merge or auto-close configured
(settings.autonomy = { merge: "auto" } or { close: "auto" }) — which is being
rolled out (#1053 "cut over review to all 3 repos", #1050/#1055 autonomy
guardrails) — and the slop/manifest gates at their default "off":

  1. Empty cache (fresh PR): a PR is opened editing .github/workflows/ci.yml;
    the webhook runs maybeRunAgentMaintenance before the batched, oldest-first
    file backfill reaches this PR → changedPaths = [] → guardrail blind. A
    passing, mergeable, approved such PR is auto-merged; a noisy one is auto-closed.
  2. Stale cache (synchronize): a later commit adds a guarded file to a PR whose
    pull_request_files row was cached at an earlier head (with slop/manifest off,
    nothing re-syncs it) → the guardrail checks the old, guarded-file-free fileset.

Both are exactly the irreversible mis-actions on a CI/policy/auth-touching PR that
the #1050 guardrail exists to block.

Suggested fix

The guardrail's real contract is "never auto-merge/close a PR that touches a
guarded path." If the changed paths are unknown (empty) while guardrails are
configured, the automation cannot prove the PR is safe, so it must fall through to
a human. Make the planner fail SAFE (minimal, defense-in-depth):

// src/settings/agent-actions.ts
const guardrailHit =
  input.hardGuardrailGlobs.length > 0 &&
  (input.changedPaths.length === 0 || changedPathsHittingGuardrail(input.changedPaths, input.hardGuardrailGlobs).length > 0);

(When no guardrails are configured the repo has opted out of path protection, so
empty paths stay permissive — unchanged. An empty-diff PR is not a real
merge/close target, so suppressing it is harmless.)

Complementary (avoids merely delaying a legitimate auto-merge on the
fresh-PR race): in maybeRunAgentMaintenance, refresh the PR files before reading
them when the cache is absent/older than the PR head —
await refreshPullRequestDetails(env, repoFullName, pr.number) — mirroring the
gate path. The fail-safe planner change alone closes the hole; the refresh is the
UX optimization.

Test status

Not covered. There is no test exercising maybeRunAgentMaintenance (grep of
test/ shows zero references). The planner unit tests pass changedPaths
explicitly and the shared helper defaults it to [], so the suite actively
blesses "empty changedPaths ⇒ no guardrail hit" — codifying the fail-open. A
regression test should assert that with guardrails configured and changedPaths: [], the planner emits no merge/close/approve action.

Confidence note

High. The fail-open is provable from the code (empty changedPaths
guardrailHit=false ⇒ merge/close/approve permitted), the wiring demonstrably
reads an unrefreshed cache, and the default gate config leaves the cache
unpopulated. It is an autonomy-safety defect in the exact guardrail #1050
designates a rollout-blocker — the highest-consequence class (irreversible
auto-merge/close of a CI/policy/auth PR). The one judgement call is the fix shape
(fail-safe planner vs refresh-before-read); the fail-safe is the conservative,
contract-upholding minimum.

Distinct from prior reports

A different surface from the gate-refresh fixes (#866/#925, which refresh before
the gate policy evaluation) and the manual-retrigger refresh (#927/#928): this
is the auto-maintain autonomy guardrail (maybeRunAgentMaintenance, #1050)
reading an unrefreshed cache, with an irreversible auto-merge/close consequence
rather than a check-run verdict. Unrelated to predicted-gate, gate-403, or BYOK.

Metadata

Metadata

Assignees

No one assigned

    Labels

    slopAI slop and/or attempts to game additional points via manipulation or alt profiles.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions