fix(mcp): honor the create-issue bootstrap in anti-stomp preflight (Closes #757)
The sanctioned create_issue bootstrap from #749/#750 was unreachable in
production. Two guards assessed the same workspace for the same task and
reached opposite conclusions: the #274 branches-only guard consulted the
bootstrap and permitted a clean canonical control checkout, then the #604
anti-stomp preflight -- which never consulted it -- rejected that same
checkout as wrong_worktree.
The defect was wiring, not policy: the bootstrap decision was computed in
one guard and discarded, while the other re-derived a conflicting answer
from a lower-level assessor with no notion of the bootstrap phase.
Fix: one computation site, one interpretation site.
* create_issue_bootstrap.bootstrap_permits_control_checkout() is the single
predicate both guards use to interpret an assessment. It is fail-closed by
construction: missing, malformed, refused, incomplete, or contradictory
evidence returns False and leaves the ordinary block in force. It also
verifies the assessment describes the exact workspace and canonical root
being guarded, so a stale or foreign assessment cannot be reused.
* _create_issue_bootstrap_assessment() computes the assessment once per
preflight from inspected repository state. verify_preflight_purity threads
that single result into both guards.
* The #604 assessor accepts the assessment and waives ONLY the wrong-worktree
verdict. Root checkout, repo, role, stale runtime, lease, head-SHA,
workflow-hash, and contamination checks are evaluated independently and
still apply.
Evidence is server-derived only and travels an internal path: no MCP tool
signature gains a bootstrap argument, and no caller-controlled boolean can
manufacture eligibility. Behavior is unchanged for callers that supply no
evidence, and for every non-create_issue author mutation.
No issue or PR number is special-cased in production behavior.
Tests: new tests/test_issue_757_bootstrap_guard_agreement.py (38 tests, 26
subtests) covering the shared predicate, the narrow waiver, guard agreement
across the full workspace-state matrix, non-forgeable eligibility, and an
end-to-end native gitea_create_issue run with the #604 gate LIVE. All 38
fail against unfixed sources; the e2e reproduces the production error text
verbatim ("Anti-stomp preflight (#604) blocked mutation [wrong_worktree]").
tests/test_reconciler_close_workspace_guard.py: one case asserted that
create_issue stays blocked on the control checkout, which only held because
the bootstrap-blind #604 guard was overriding #750 -- it encoded the defect.
Re-pointed to lock_issue, which is issue-backed and legitimately still
requires a branches/ worktree. Its teardown now restores the module-level
preflight task/role so test order cannot leak resolved state.
Full suite: 3624 passed, 2 failed, 6 skipped (426 subtests).
Baseline at bde5c5fb on a clean detached worktree: 3586 passed, 2 failed,
6 skipped (400 subtests). The same 2 failures reproduce identically on
pristine master and are unrelated to this change
(test_issue_702_review_findings_f1_f6 F1 worktree recovery;
test_reconciler_supersession_close org/repo forwarding).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
+86
-18
@@ -817,10 +817,56 @@ def _enforce_canonical_repository_root(
|
||||
)
|
||||
|
||||
|
||||
# #757: distinguishes "caller supplied no bootstrap evidence" (compute it) from
|
||||
# "caller supplied a not-applicable/refused assessment" (honour it, fail closed).
|
||||
_BOOTSTRAP_UNSET = object()
|
||||
|
||||
|
||||
def _create_issue_bootstrap_assessment(
|
||||
task: str | None,
|
||||
worktree_path: str | None = None,
|
||||
) -> dict | None:
|
||||
"""#757: the single server-derived create_issue bootstrap assessment.
|
||||
|
||||
Computed once per preflight from inspected repository state, then consumed
|
||||
by BOTH the #274 branches-only guard and the #604 anti-stomp preflight so
|
||||
the two cannot reach opposite conclusions about identical evidence.
|
||||
|
||||
Returns ``None`` when *task* is not a create_issue mutation, which leaves
|
||||
every other author mutation on the ordinary branches-only path. The result
|
||||
is derived from inspected repository state only — never from an MCP tool
|
||||
argument.
|
||||
"""
|
||||
import create_issue_bootstrap as _cib
|
||||
|
||||
if not _cib.is_create_issue_task(task):
|
||||
return None
|
||||
|
||||
ctx = _resolve_namespace_mutation_context(worktree_path)
|
||||
workspace = ctx["workspace_path"]
|
||||
git_state = issue_lock_worktree.read_worktree_git_state(workspace)
|
||||
try:
|
||||
remote_master_sha = root_checkout_guard.resolve_remote_master_sha(
|
||||
ctx["canonical_repo_root"]
|
||||
)
|
||||
except Exception:
|
||||
remote_master_sha = None
|
||||
return _cib.assess_create_issue_bootstrap(
|
||||
workspace_path=workspace,
|
||||
canonical_repo_root=ctx["canonical_repo_root"],
|
||||
current_branch=git_state.get("current_branch"),
|
||||
head_sha=git_state.get("head_sha"),
|
||||
porcelain_status=git_state.get("porcelain_status") or "",
|
||||
remote_master_sha=remote_master_sha,
|
||||
task=task,
|
||||
)
|
||||
|
||||
|
||||
def _enforce_branches_only_author_mutation(
|
||||
worktree_path: str | None = None,
|
||||
*,
|
||||
task: str | None = None,
|
||||
bootstrap_assessment: Any = _BOOTSTRAP_UNSET,
|
||||
) -> None:
|
||||
"""#274: author file/branch mutations must run from a branches/ worktree.
|
||||
|
||||
@@ -859,27 +905,29 @@ def _enforce_branches_only_author_mutation(
|
||||
return
|
||||
|
||||
# #749 create_issue bootstrap: narrow phase exemption only.
|
||||
# #757: consume the caller-computed assessment when one was threaded in, so
|
||||
# this guard and the later #604 anti-stomp guard judge identical evidence.
|
||||
# Falling back to computing it here preserves behaviour for callers that
|
||||
# supply none.
|
||||
import create_issue_bootstrap as _cib
|
||||
|
||||
remote_master_sha = None
|
||||
try:
|
||||
remote_master_sha = root_checkout_guard.resolve_remote_master_sha(
|
||||
ctx["canonical_repo_root"]
|
||||
)
|
||||
except Exception:
|
||||
remote_master_sha = None
|
||||
bootstrap = _cib.assess_create_issue_bootstrap(
|
||||
bootstrap = (
|
||||
_create_issue_bootstrap_assessment(task, worktree_path)
|
||||
if bootstrap_assessment is _BOOTSTRAP_UNSET
|
||||
else bootstrap_assessment
|
||||
)
|
||||
if _cib.bootstrap_permits_control_checkout(
|
||||
bootstrap,
|
||||
task=task,
|
||||
workspace_path=workspace,
|
||||
canonical_repo_root=ctx["canonical_repo_root"],
|
||||
current_branch=git_state.get("current_branch"),
|
||||
head_sha=git_state.get("head_sha"),
|
||||
porcelain_status=git_state.get("porcelain_status") or "",
|
||||
remote_master_sha=remote_master_sha,
|
||||
task=task,
|
||||
)
|
||||
if bootstrap.get("allowed"):
|
||||
):
|
||||
return
|
||||
if bootstrap.get("block") and not bootstrap.get("not_applicable"):
|
||||
if (
|
||||
isinstance(bootstrap, dict)
|
||||
and bootstrap.get("block")
|
||||
and not bootstrap.get("not_applicable")
|
||||
):
|
||||
raise RuntimeError(_cib.format_create_issue_bootstrap_error(bootstrap))
|
||||
raise RuntimeError(
|
||||
author_mutation_worktree.format_author_mutation_worktree_error(assessment)
|
||||
@@ -926,6 +974,7 @@ def _run_anti_stomp_preflight(
|
||||
workflow_hash_valid: bool | None = None,
|
||||
workflow_hash_reasons: list[str] | None = None,
|
||||
raise_on_block: bool = True,
|
||||
bootstrap_assessment: Any = _BOOTSTRAP_UNSET,
|
||||
) -> dict | None:
|
||||
"""Shared #604 anti-stomp preflight for MCP mutation entrypoints.
|
||||
|
||||
@@ -1106,6 +1155,14 @@ def _run_anti_stomp_preflight(
|
||||
source_contaminated=source_contaminated,
|
||||
contamination_reasons=contamination_reasons,
|
||||
manual_bypass_attempted=False,
|
||||
# #757: same server-derived bootstrap decision the #274 guard consumed.
|
||||
# Computing it here when unsupplied keeps standalone callers consistent
|
||||
# rather than silently bootstrap-blind.
|
||||
create_issue_bootstrap_assessment=(
|
||||
_create_issue_bootstrap_assessment(task, worktree_path)
|
||||
if bootstrap_assessment is _BOOTSTRAP_UNSET
|
||||
else bootstrap_assessment
|
||||
),
|
||||
)
|
||||
if not assessment.get("block"):
|
||||
return None
|
||||
@@ -1248,7 +1305,12 @@ def verify_preflight_purity(
|
||||
# Historical path: root + branches after purity-order when dirty paths live.
|
||||
_enforce_canonical_repository_root(worktree_path, remote=remote)
|
||||
_enforce_root_checkout_guard(worktree_path)
|
||||
_enforce_branches_only_author_mutation(worktree_path, task=task)
|
||||
# #757: compute the create_issue bootstrap decision ONCE and hand the
|
||||
# same result to both the #274 and #604 guards, so they cannot disagree.
|
||||
bootstrap_assessment = _create_issue_bootstrap_assessment(task, worktree_path)
|
||||
_enforce_branches_only_author_mutation(
|
||||
worktree_path, task=task, bootstrap_assessment=bootstrap_assessment
|
||||
)
|
||||
_enforce_issue_scope_guard(
|
||||
worktree_path,
|
||||
task=task,
|
||||
@@ -1258,6 +1320,7 @@ def verify_preflight_purity(
|
||||
# #604: common anti-stomp preflight after legacy + #683 enforcers.
|
||||
_run_anti_stomp_preflight(
|
||||
task,
|
||||
bootstrap_assessment=bootstrap_assessment,
|
||||
remote=remote,
|
||||
worktree_path=worktree_path,
|
||||
org=org,
|
||||
@@ -1282,7 +1345,11 @@ def verify_preflight_purity(
|
||||
if production_active:
|
||||
_enforce_canonical_repository_root(worktree_path, remote=remote)
|
||||
_enforce_root_checkout_guard(worktree_path)
|
||||
_enforce_branches_only_author_mutation(worktree_path, task=task)
|
||||
# #757: one shared bootstrap decision for both guards (see above).
|
||||
bootstrap_assessment = _create_issue_bootstrap_assessment(task, worktree_path)
|
||||
_enforce_branches_only_author_mutation(
|
||||
worktree_path, task=task, bootstrap_assessment=bootstrap_assessment
|
||||
)
|
||||
_enforce_issue_scope_guard(
|
||||
worktree_path,
|
||||
task=task,
|
||||
@@ -1292,6 +1359,7 @@ def verify_preflight_purity(
|
||||
if force_anti_stomp:
|
||||
_run_anti_stomp_preflight(
|
||||
task,
|
||||
bootstrap_assessment=bootstrap_assessment,
|
||||
remote=remote,
|
||||
worktree_path=worktree_path,
|
||||
org=org,
|
||||
|
||||
Reference in New Issue
Block a user