fix(#3210): gate unmet preconditions as blocking-human; cap blocker retries at needs_human (#3528)

* fix(#3210): gate unmet preconditions as blocking-human and cap blocker retries at needs_human

* chore(#3210): add changeset fragment for PR #3528

* fix(#3210): restore blocking-human carve-out and CRLF-safe split

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-08-14 23:01:30 -04:00
committed by GitHub
parent d2fa696a30
commit 8fc88f663d
13 changed files with 154 additions and 11 deletions

View File

@@ -286,3 +286,42 @@ describe('autonomous verification deferral contract', () => {
assert.match(iterateStep, /drop deferred phases from the autonomous queue/);
});
});
// ─── Issue #3210: bounded blocker retries, needs_human escalation ────────────
//
// handle_blocker's "Fix and retry" path had no attempt ceiling across
// invocations and no automatic escalation to a terminal needs_human state, so
// a non-converging blocker (e.g. an operator gate the executor cannot satisfy)
// looped indefinitely. Regression coverage lives here because this file owns
// the autonomous.md host-workflow contract.
describe('issue #3210: autonomous handle_blocker has a retry ceiling with needs_human escalation', () => {
function stepOf(content, name) {
const open = `<step name="${name}">`;
const from = content.indexOf(open);
assert.ok(from !== -1, `step "${name}" not found`);
const to = content.indexOf('</step>', from);
assert.ok(to !== -1, `step "${name}" has no closing tag`);
return content.slice(from, to);
}
test('handle_blocker bounds "Fix and retry" attempts per phase step', () => {
const step = stepOf(read(WORKFLOW_PATH), 'handle_blocker');
assert.match(
step,
/\b3\b.*retr|\bretr.*\b3\b|RETRY_COUNT|retry (ceiling|limit|count)/i,
'handle_blocker must track a bounded retry count for the same phase step instead of ' +
're-presenting "Fix and retry" indefinitely (#3210)'
);
});
test('handle_blocker auto-escalates to a terminal needs_human halt once the ceiling is exceeded', () => {
const step = stepOf(read(WORKFLOW_PATH), 'handle_blocker');
assert.match(
step,
/needs_human/,
'once the retry ceiling is exceeded, handle_blocker must halt autonomously in a terminal ' +
'needs_human state (surfacing the unmet items) instead of looping or asking again (#3210)'
);
});
});

View File

@@ -1,6 +1,6 @@
{
"version": 1,
"paths": {
"gsd-executor.md": "#2943: the context7 doc-lookup block's ctx7 CLI-fallback rationale was rewritten to describe the real mechanism (custom subagents cannot see project-scoped .mcp.json; they inherit only user-scoped ~/.claude/mcp.json), replacing the wrong anthropics/claude-code#13898 'tools: frontmatter restriction' attribution. The tool name was also corrected (get-library-docs -> query-docs) and the params renamed (context7CompatibleLibraryId/topic -> libraryId/query). The +95 bytes is the longer-but-accurate mechanism description; it is the literal fix for the mis-attribution, not incidental prose growth, and the rationale must be correct because agents read it to decide when to fall back to the CLI. #3021 amendment: branch allow-list regex widened to accept worktree-wf_* (Workflow backend naming) alongside agent-*/worktree-agent-*."
"gsd-executor.md": "#2943: the context7 doc-lookup block's ctx7 CLI-fallback rationale was rewritten to describe the real mechanism (custom subagents cannot see project-scoped .mcp.json; they inherit only user-scoped ~/.claude/mcp.json), replacing the wrong anthropics/claude-code#13898 'tools: frontmatter restriction' attribution. The tool name was also corrected (get-library-docs -> query-docs) and the params renamed (context7CompatibleLibraryId/topic -> libraryId/query). The +95 bytes is the longer-but-accurate mechanism description; it is the literal fix for the mis-attribution, not incidental prose growth, and the rationale must be correct because agents read it to decide when to fall back to the CLI. #3021 amendment: branch allow-list regex widened to accept worktree-wf_* (Workflow backend naming) alongside agent-*/worktree-agent-*. — #3210 append: the unmet-<precondition> branch now reports the returned checkpoint with **Gate:** blocking-human (both auto-mode bypass layers key on that gate; without it auto-mode silently auto-approved the checkpoint with a synthetic 'approved'), the auto-mode human-verify rule names precondition-unmet checkpoints as exempt, and checkpoint_return_format's Gate line notes precondition-unmet checkpoints report blocking-human; +134 bytes, still under the 49152 cap."
}
}

View File

@@ -0,0 +1,6 @@
{
"version": 1,
"paths": {
"autonomous.md": "#3210: handle_blocker's 'Fix and retry' path gained a retry ceiling — a per-phase-step RETRY_COUNT that escalates to a terminal needs_human halt (STATE.md '## Needs Human' row + unmet items surfaced) after 3 failed fix attempts, instead of re-presenting the options indefinitely. This is the loop-fixing behavior the issue title names; +704 bytes is the ceiling rule itself, not incidental prose. (The matching gsd-executor.md growth is acknowledged in the 2943 fragment and the execute-phase.md growth in the 3370 fragment, which already name those paths.)"
}
}

View File

@@ -1,7 +1,7 @@
{
"version": 1,
"paths": {
"execute-phase.md": "#3370: the step-3 executor-routing line now also cites #3370 — the checkpoint gate rule itself lives in the execute-phase/steps/per-plan-executor-routing.md fragment (loaded per plan in every isolation mode immediately before the dispatch prompt is composed), the same keep-the-host-lean pattern #1689/#3417 used, because the host sits under the frozen ADR-857 Phase 6 ceiling (≤93400). Net growth is 6 bytes (93386 -> 93392): the routing citation only; the rule text, which forbids the orchestrator from composing dispatch text that refuses or overrides auto-approval for the default gate=\"blocking\" (only blocking-human always surfaces), is in the fragment. Supersedes the spent #3324 fragment (merged into next), which also named execute-phase.md and would otherwise double-ack the same path. — #3423 append (epic #1891 F8): grew 12 bytes with the <files_to_read> -> <required_reading> tag rename (4 tag tokens, +3 bytes each), then trimmed 8 bytes of redundant prose (dropped 'current' from the model-inheritance note; #3478's growth had left only 8 bytes of ADR-857 margin) to stay under the Phase 6 margin ceiling; net +4 bytes against the #3370 baseline.",
"execute-phase.md": "#3370: the step-3 executor-routing line now also cites #3370 — the checkpoint gate rule itself lives in the execute-phase/steps/per-plan-executor-routing.md fragment (loaded per plan in every isolation mode immediately before the dispatch prompt is composed), the same keep-the-host-lean pattern #1689/#3417 used, because the host sits under the frozen ADR-857 Phase 6 ceiling (≤93400). Net growth is 6 bytes (93386 -> 93392): the routing citation only; the rule text, which forbids the orchestrator from composing dispatch text that refuses or overrides auto-approval for the default gate=\"blocking\" (only blocking-human always surfaces), is in the fragment. Supersedes the spent #3324 fragment (merged into next), which also named execute-phase.md and would otherwise double-ack the same path. — #3423 append (epic #1891 F8): grew 12 bytes with the <files_to_read> -> <required_reading> tag rename (4 tag tokens, +3 bytes each), then trimmed 8 bytes of redundant prose (dropped 'current' from the model-inheritance note; #3478's growth had left only 8 bytes of ADR-857 margin) to stay under the Phase 6 margin ceiling; net +4 bytes against the #3370 baseline. — #3210 append: the checkpoint_handling blocking-human carve-out names precondition-unmet checkpoints (+29-byte marker '(precondition-unmet, #3210)'), and the decision bullet's 'Except blocking-human' conditional — required by tests/package-legitimacy-gate.test.cjs — was restored (+29), funded by trimming ', regardless of type' (-20, the carve-out already overrides all branches) and '(standard flow below)' -> '(standard flow)' (-6); net +3 bytes (93395 -> 93398), still under the frozen ADR-857 Phase 6 ceiling (<=93400).",
"execute-plan.md": "#3370: the Pattern A dispatch prompt spec gained the gate-semantics clause (gate=\"blocking\" (the default) is auto-approvable in auto-mode per the executor's own checkpoint protocol, gate=\"blocking-human\" always surfaces to a human; add no instruction overriding that protocol), closing the identically-shaped dispatch-time gap on the single-plan path named in the issue. Growth ~248 bytes (38913 -> 39161, still under the DEFAULT 40 KiB ceiling). Supersedes the spent #2652 fragment (merged into next), which also named execute-plan.md and would otherwise double-ack the same path."
}
}

View File

@@ -908,3 +908,30 @@ describe('bug #3096: ai-integration-phase sequential ordering and Edit-only disc
});
});
}
// ─── Issue #3210: auto-mode carve-out exempts precondition-unmet checkpoints ─
//
// The checkpoint_handling auto-spawn rule dispatched on checkpoint type alone;
// a checkpoint returned because a task's <precondition> was unmet would have
// been auto-approved with a synthetic "approved" — re-approving the very
// checkpoint the executor refused to auto-approve (it reports Gate:
// blocking-human). This file owns the execute-phase.md host-workflow contract.
describe('issue #3210: execute-phase auto-mode carve-out exempts precondition-unmet checkpoints', () => {
test('the checkpoint_handling auto-spawn rule names precondition-unmet checkpoints', () => {
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
const open = content.indexOf('<step name="checkpoint_handling">');
assert.ok(open !== -1, 'checkpoint_handling step not found');
const close = content.indexOf('</step>', open);
const step = content.slice(open, close);
const splitLines = require('../gsd-core/bin/lib/text-lines.cjs').splitLines;
const carveOut = splitLines(step).find((l) => l.includes('Carve-out'));
assert.ok(carveOut, 'checkpoint_handling must keep the blocking-human carve-out');
assert.match(
carveOut,
/precondition/i,
'the auto-mode carve-out must state that a precondition-unmet checkpoint reports ' +
'blocking-human and is never auto-approved (#3210)'
);
});
});

View File

@@ -231,3 +231,64 @@ describe('issue #1949: parity between plan-md.md and planner-preconditions.md',
assert.ok(ref.includes('<precondition>'), 'planner-preconditions.md must spell <precondition>');
});
});
// ─── Issue #3210: the precondition-unmet checkpoint must be human-gated ──────
//
// The executor's own invariant ("unmet preconditions are NEVER auto-approved,
// even under AUTO_CFG=true") was unenforceable: the checkpoint it returns for an
// unmet <precondition> carried no gate, and both auto-mode bypass rules only
// exempt `gate="blocking-human"`. Auto-mode therefore auto-approved it with a
// synthetic "approved", the task's <verify> failed closed, and the loop read
// the failure as an executable gap — retrying an operator gate forever (#3210).
describe('issue #3210: executor gates the precondition-unmet checkpoint as blocking-human', () => {
const CHECKPOINTS_REF = path.join(ROOT, 'gsd-core', 'references', 'checkpoints.md');
test('unmet-precondition branch stamps the returned checkpoint with Gate: blocking-human', () => {
const exec = read(EXECUTOR);
const unmet = exec.split('\n').find((l) => l.includes('**Unmet:**'));
assert.ok(unmet, 'gsd-executor.md must keep the **Unmet:** precondition branch');
assert.match(
unmet,
/blocking-human/,
'the unmet-precondition branch must report the checkpoint with **Gate:** blocking-human — ' +
'that gate is what both auto-mode bypass layers check (#3210)'
);
});
test('auto-mode human-verify rule explicitly exempts precondition-unmet checkpoints', () => {
const exec = read(EXECUTOR);
const rule = exec.split('\n').find((l) => l.includes('**checkpoint:human-verify**') && /Auto-approve/.test(l));
assert.ok(rule, 'gsd-executor.md must keep the auto-mode human-verify bypass rule');
assert.match(
rule,
/precondition/i,
'the executor auto-mode bypass rule must name precondition-unmet checkpoints as never ' +
'auto-approved, not only gate="blocking-human" tasks (#3210)'
);
});
test('checkpoints.md maps precondition-unmet checkpoints onto the blocking-human gate', () => {
const ref = read(CHECKPOINTS_REF);
const rule = ref.split('\n').find((l) => l.includes('never auto-approved'));
assert.ok(rule, 'checkpoints.md must keep golden rule 6 (blocking-human is never auto-approved)');
assert.match(
rule,
/precondition/i,
'golden rule 6 must name the executor\'s precondition-unmet checkpoint as a carrier of ' +
'gate="blocking-human" (#3210)'
);
});
test('planner-preconditions.md documents the blocking-human gate on the unmet path', () => {
const ref = read(PRECONDITIONS_REF);
const unmet = ref.split('\n').find((l) => l.includes('**Unmet**'));
assert.ok(unmet, 'planner-preconditions.md must keep the executor-behavior Unmet row');
assert.match(
unmet,
/blocking-human/,
'the Unmet row must state the returned checkpoint carries gate="blocking-human", matching ' +
'agents/gsd-executor.md (#3210)'
);
});
});