Rectify: Guard Decision Architecture, Capture Diagnostics & Store Reconciliation Immunity - #4468
Merged
Trecek merged 84 commits intoAug 6, 2026
Merged
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…subcommands Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cution-cwd semantics Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…to command position Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mantics Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds a state-derived diagnostic severity model to hooks/_capture/_types.py: CleanupSeverity (HEALTHY/DEFERRED/STALLED/ FAILED), an exhaustive BLOCKER_FAMILY mapping every CleanupBlocker member to "healthy"/"budget"/"external", and the pure total function classify_cleanup_outcome(progress, blocker, errors) that derives severity from them. errors always wins; a healthy-family blocker is always healthy; a budget-family blocker is deferred with progress and stalled without; an external-family blocker is always stalled regardless of progress, so an externally-blocked store never goes silent. A future CleanupBlocker member without a family assignment raises KeyError immediately via the plain-dict subscript, rather than silently defaulting. Part of Workstream 2 (diagnostic severity & declared session identity) of the guard/capture-immunity rectify plan. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mission Rebuilds hooks/_capture/_reconcile.py's cleanup_diagnostic on the new severity classifier and replaces emit_runner_diagnostic with an owner-neutral emit_owner_diagnostic(outcome, owner, write): silent for HEALTHY/DEFERRED, a neutral one-line note for STALLED, a failure-worded line for FAILED. Every non-silent message is rendered via hooks/_policy_event.py's PolicyEvent + render_provenance_prefix instead of a hand-rolled "[AutoSkillit ...]" literal — decision carries the severity token, so FAILED is the only rendered text that can contain "failed". A single shared DIAGNOSTIC_MAX_BYTES bound replaces the former 240 (runner-tail) vs. 512 (SessionStart) divergence. This fixes the incident: a healthy, by-design bounded deferral (progress=retired blocker=record_budget errors=0) previously always rendered "cleanup failed" on every command; it is now silent, and an externally-blocked store (e.g. migration_blocked) still surfaces a neutral line every pass rather than going silent. capture_lifecycle_hook.py and _capture_artifacts.py's runner-tail sweep migrate to the shared emitter: capture_lifecycle_hook.py drops its private _bounded_stderr/_MAX_DIAGNOSTIC_BYTES entirely, and both hooks' generic crash-fallback paths (an exception escaping reconcile itself, distinct from outcome.errors > 0) also render through PolicyEvent rather than a bare literal. Test updates mechanically required by this rebuild: the frozen narrow-import AST assertion in test_capture_lifecycle_hook.py now expects the shared emitter's import set; the two test_capture_artifacts.py tests asserting the old "shell capture cleanup failed" wording now assert the PolicyEvent-rendered text; new tests cover the severity classifier's full cross-product, the emission-policy contract (incident silence via both a real seeded store and direct outcome construction, neutral STALLED wording, FAILED wording preservation), and DEFERRED silence through both the runner-tail and SessionStart wrappers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds tests/arch/test_hook_message_provenance.py: a static AST scan of _reconcile.py, shell_capture_hook.py, and capture_lifecycle_hook.py asserting no string literal carrying the "AutoSkillit" provenance token (bracketed or bare) exists outside hooks/_policy_event.py, plus a standing check that _policy_event.py has at least one production importer so it can no longer regress to an orphaned component. One pre-existing literal is narrowly exempted: shell_capture_hook.py's _LOCAL_REJECTION_DETAIL, which is baked into the generated shell harness itself (a `printf ... >&2` inside the wrapped command) rather than emitted as a hook policy/diagnostic message by the hook process — a companion test asserts the exemption list never goes stale. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Splits shell_capture_hook.py's single "managed launch controls missing or invalid" fallback into a three-way declared-mode contract, each rendered via PolicyEvent: (i) mode=capture with no managed-identity vars is a normal declared state with no diagnostic at all — previously this exact shape hit the same "missing or invalid" fallback as a genuine anomaly; (ii) a complete, valid managed=direct identity tuple resolves to the managed path unchanged; (iii) a declared mode with a partial/invalid identity tuple falls back to capture with a distinct "incomplete managed native-shell controls; falling back to capture" note; (iv) no declaration at all falls back to capture with a neutral "native-shell control undeclared; using capture" note. codex.py's build_interactive_cmd (the Codex cook/interactive launch path) now positively injects AUTOSKILLIT_NATIVE_SHELL_CAPTURE_MODE= "capture" after CodexEnvPolicy().build_env() returns — extras-side injection would be silently stripped by build_env's protected native-shell env filter, so it must follow the same post-build_env env.update() pattern already used by build_skill_session_cmd, build_food_truck_cmd, and build_resume_cmd. Cook remains structurally unmanaged (no managed-identity params added) but is now declaredly so: case (i) above is what cook triggers, so shell_capture_hook.py no longer needs to infer "normal" from the absence of a declaration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ADR-0006 gains an "Implemented" note under the Provenance Rule naming the now-wired PolicyEvent formatter, the severity vocabulary (healthy/deferred/stalled/failed), and the owner-neutral emission path. docs/safety/hooks.md's capture-lifecycle section gains matching "Cleanup diagnostic severity" and "Declared native-shell control mode" subsections describing the classifier's trigger table and cook's capture-normal declared state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bounded lock retry (2a): CaptureLifecycleStore._locked(blocking=False) —
the only non-blocking caller shape, used exclusively by sweep-path helpers
while a sweep is active — now retries on EAGAIN/EWOULDBLOCK with jittered,
doubling backoff (5-20ms base, capped) bounded by the sweep's own existing
max_duration_seconds budget instead of aborting on first contention.
LOCK_CONTENDED is only ever returned once the entire budget elapsed
without acquisition; blocking callers are unaffected; a non-blocking call
outside an active sweep falls back to today's single-attempt behavior.
Directory-reconciliation orphan-adoption scan phase (2b): new stdlib-only
hooks/_capture/_orphan_scan.py performs a budget-bounded, cursor-resumed
directory scan (SweepBudgetSpec gains max_directory_entries_scanned, 0 by
default so RUNNER_TAIL_BUDGET is unaffected; SESSION_START_BUDGET sets
512) identifying shell_[0-9a-f]{16}.log files that are regular (lstat, no
symlink traversal — #4319), aged past a 24h threshold, and not the public
name of any non-DELETED-phase ledger record (so a DELETING-phase record's
file mid-quarantine is never re-adopted — #4321). _ledger.py gains
adopted_orphan_record(), the LEGACY_CLEANUP_ONLY constructor for adopted
orphans; CaptureLifecycleStore._admit_new_record() admits them through the
same capacity check and max_transitions budget reserve_capture() uses, so
orphan adoption can only ever compete for the active-record ceiling real
captures do, never bypass or starve it (#4440). run_bounded_sweep runs the
scan phase after record-sweep work, only while duration budget remains.
Also adds capture_store_stats()/CaptureStoreStats — a read-only ledger and
directory statistics adapter shared by the upcoming doctor check and CLI
command (2c).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
doctor is a flat @app.command running a read-only check battery under a standing read-only architectural guard (tests/arch/test_doctor_readonly.py) — there is no subcommand mechanism, and a mutating reclaim cannot live under doctor. Two pieces: - cli/doctor/_doctor_capture_store.py: a read-only capture-store stats check appended to the existing doctor battery, calling hooks._capture._reconcile.capture_store_stats() — never reconcile_capture_store, never anything from the orphan-adoption path. - cli/_capture_store.py: a new flat @app.command capture-store (CLI-visible as capture-store, matching quota_status -> quota-status), printing the same stats by default; --reclaim loops reconcile_capture_store with a generous one-time RECLAIM_BUDGET until a clean pass (no due records, no adoptable orphans) or a hard iteration cap, printing per-pass progress. This is the one-time bulk path for pre-existing debris; the SessionStart scan phase (previous commit) keeps new debris from ever accumulating again. Imports hooks._capture._reconcile directly — none of pyproject's import-linter contracts mention hooks or restrict cli imports. Both surfaces call the same capture_store_stats() adapter so neither can drift from what a real reconciliation pass would find. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
docs/safety/hooks.md capture-lifecycle section gains three subsections after declared native-shell control mode: the directory-reconciliation scan phase (budget field, cursor sidecar, adoption gate table, capacity sharing with reserve_capture(), Codex hook-generation coverage), bounded lock-contention retry (jittered backoff mechanics, budget bound, when LOCK_CONTENDED is actually returned), and the stats/reclamation CLI (shared read-only adapter, --reclaim's one-time bulk purpose). ADR-0006 gains a Resolved section acknowledging both gaps this workstream closes: unledgered orphan files (previously permanently invisible to cleanup) and lock contention (previously aborted a sweep immediately, including the 256-attempt SessionStart pass). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…, and CLI/doc tests tests/hooks/test_capture_lifecycle.py: - Production-scale convergence (1a): seeds ~1,024 file-less reserved records via the store API, asserts remaining_due strictly decreases every SESSION_START_BUDGET invocation until 0 within ceil(N/max_attempts)+2 invocations — the documented 'one record cannot starve the backlog' guarantee, previously validated by nothing (issue #4440 regression guard: cleanup capacity must outpace admission at scale, not merely drain a static backlog eventually). A single RUNNER_TAIL_BUDGET call against a non-empty backlog always retires >= 1 record. - Lock-contention recovery (1b): replaces test_sweep_defers_without_blocking_on_contended_ledger_lock, whose premise the bounded lock-retry change removes by design, with two tests matching the new contract — a lock released a few ms into a sweep recovers within the same invocation and makes progress; a lock held for the whole budget reports a bounded, clean STALLED-class LOCK_CONTENDED outcome. - Orphan adoption (1c): the plan's exact seeded corpus (20 aged unledgered, 5 fresh unledgered, 3 active-record, 1 aged DELETING-tracked, 1 aged symlink, 1 aged non-matching name) — only the 20 aged orphans are ever adopted and deleted; no duplicate ledger record is ever created for the DELETING-tracked name. Plus budget-bounded scan/cursor-resume coverage and RUNNER_TAIL_BUDGET no-scanning coverage. tests/cli/test_capture_store.py (1d): the doctor check and autoskillit capture-store (without --reclaim) report stats without mutating a seeded ledger+orphan backlog (filesystem snapshot before/after); --reclaim drains both backlogs to convergence. tests/docs/test_capture_store_reconciliation_docs.py (1e): ratchets presence of the scan-phase, adoption-gate, and contention-retry documentation in docs/safety/hooks.md and the Resolved acknowledgment in ADR-0006 — the doc gap was itself a finding. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
reconcile_capture_store's call chain is open_capture_lifecycle(...) -> lifecycle.sweep(budget). open_capture_lifecycle -> from_open_authorities unconditionally calls _normalize_interrupted_deliveries(), which acquires the ledger lock via store._locked() (blocking=True, the default) BEFORE .sweep(budget) is ever reached — so _sweep_budget/_sweep_started_monotonic were still None at that point, and this specific lock acquisition never went through the bounded-retry path added for the sweep body. A contended lock at store-open time blocked the whole reconcile_capture_store call indefinitely regardless of budget.max_duration_seconds, exactly the incident scenario this workstream exists to close (session_scope="any" means every concurrent SessionStart hook opens the store simultaneously). Reproduced concretely: holding the lock 2.0s and calling reconcile_capture_store(cwd, SweepBudgetSpec(max_duration_seconds=0.15)) returned after ~2.0s with blocker=none — the budget was silently ignored. Fix: open_capture_lifecycle and CaptureLifecycleStore.from_open_authorities gain an optional open_budget parameter. When supplied, it primes _sweep_budget/_sweep_started_monotonic before _normalize_interrupted_deliveries runs and clears them when the open_capture_lifecycle 'with' block exits. _delivery.py's normalize_interrupted_deliveries — the only blocking=True caller reachable during that window — now acquires its two ledger locks with blocking=(store._sweep_budget is None), so store-open lock acquisition participates in the same jittered-backoff bounded-retry mechanism the sweep body already used, and LockContended surfaces through reconcile_capture_store as the same LOCK_CONTENDED outcome the sweep body reports (new except clause + local duration tracking). capture_store_stats() gets the identical treatment with RUNNER_TAIL_BUDGET as its own open_budget (and its own record-load lock made conditional) — a diagnostic read must never hang either. Every other from_open_authorities/open_capture_lifecycle caller (create_artifact, direct test construction, ...) passes nothing and keeps today's blocking-until-acquired behavior, verified concretely. Verified against the exact reported repro plus both RUNNER_TAIL_BUDGET and SESSION_START_BUDGET: contention bounded to ~budget.max_duration_seconds with blocker=LOCK_CONTENDED, errors=0; a lock released within budget recovers and completes successfully; non-sweep callers still block exactly as before (measured elapsed matches the holder's real hold duration). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ook payload module resolve_state_root(payload_cwd) resolves the project root whose .autoskillit/ directory holds session state, per the plan's three-tier order: AUTOSKILLIT_STATE_ROOT env var (production signal set by launch preparation) wins unconditionally; failing that, an upward walk from payload_cwd looking for a directory containing .autoskillit/, operating on resolved (symlink-free) paths so a symlinked .autoskillit escaping the trust anchor is never accepted (issue #4319); failing that, the process cwd, unchanged from every existing guard's pre-migration behavior. extract_apply_patch_text(data) extracts the patch text from a Codex apply_patch tool payload's "command" field — a distinct semantic use of that key name from a shell command, kept in the shared stdlib-only module so write_guard.py's read of it stays inside the payload-extraction AST conformance boundary in the same commit that migrates write_guard.py. _hook_settings.read_merged_hook_config's bare default (root=None) now resolves via a new _default_state_root() helper (AUTOSKILLIT_STATE_ROOT env var, else process cwd) instead of bare Path.cwd(). Implemented as an inline duplicate of resolve_state_root's env-var tier rather than an import, because _hook_settings.py is imported both as a bare sibling module by guard subprocesses (which pre-insert hooks/ onto sys.path) and via the normal package path by in-process test imports — only the latter resolves a cross-sibling-module import correctly, so importing _hook_payload here would break guard subprocess execution. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ommand Mechanical extraction swap for pr_create_guard.py, git_ops_guard.py, unsafe_install_guard.py, compose_pr_body_guard.py, planner_gh_discovery_guard.py, artifact_download_guard.py, test_runner_guard.py, recipe_read_guard.py, write_guard.py — behavior-identical except write_guard.py. pr_create_guard.py, git_ops_guard.py, and compose_pr_body_guard.py also migrate their state/body-path Path.cwd() resolution to resolve_state_root(parsed.payload_cwd), and pass the resolved root through read_merged_hook_config's existing root parameter rather than re-resolving. write_guard.py additionally resolves relative write targets against parsed.execution_cwd when non-empty, falling back to the AUTOSKILLIT_CWD env var — fixes the cwd-source divergence where relative Bash/run_cmd write targets were resolved only against the env var, ignoring the per-call cwd available in worktree flows. Its apply_patch handling now reads the patch text via the new extract_apply_patch_text helper instead of a direct tool_input["command"] read, since that field is patch text rather than a shell command but shares the same key name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…uards reset_resume_gate.py, open_kitchen_guard.py (three sites), pipeline_step_guard.py (two Path.cwd() sites plus its read_merged_hook_config() call), resume_ownership_guard.py, ask_user_question_guard.py, and review_loop_gate.py all resolved state files via bare Path.cwd(), which in worktree topologies silently looks in the wrong .autoskillit/temp/ tree: pr_create_guard/git_ops_guard (migrated in the prior commit) then fail open in the unsafe direction (kitchen "not open" -> check skipped), and ask_user_question_guard fails closed (spurious deny) — the exact incident symptom. Each guard now threads the hook payload's top-level cwd (best-effort; several of these MCP-tool-matched guards receive no payload cwd at all in current fixtures) through resolve_state_root(), which checks the new AUTOSKILLIT_STATE_ROOT env var first — the load-bearing signal for guards with no payload cwd — then falls back to the payload-cwd walk, then process cwd (no worse than today when both are absent). open_kitchen_guard.py and resume_ownership_guard.py keep their existing AUTOSKILLIT_STATE_DIR override as the first-checked branch (test isolation, pre-existing) and only replace the bare Path.cwd() fallback. review_loop_gate.py now captures the parsed stdin payload (previously discarded after the try/except) to extract its cwd field. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…and cook sessions Adds AUTOSKILLIT_STATE_ROOT_ENV_VAR to core/types/_type_constants_env.py (named constant for core-side producer code; hooks/_hook_payload.py's resolve_state_root reads the same value as a bare string literal, since stdlib-only hook scripts cannot import this constant) and its private-env scrub/re-inject list, plus the core/__init__.pyi re-export. _assemble_shared_env_extras (shared by both backends' skill-session and food-truck builders) now injects AUTOSKILLIT_STATE_ROOT alongside AUTOSKILLIT_CWD whenever cwd is set — cwd is the orchestrating project root at every current call site. The interactive cook launch path (_session_launch.py's _run_interactive_session) injects it from its resolved _project_dir into extra_env before either build_interactive_cmd call, since cook sessions have no other path through _assemble_shared_env_extras. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n coverage test_guard_payload_extraction.py (new): AST scan of hooks/guards/*.py asserting no guard reads tool_input["cmd"/"command"/"cwd"] directly outside hooks/_hook_payload.py's parse_hook_command. EXEMPT is empty — every guard listed in the plan is already migrated as of the prior two commits. Also asserts EXEMPT entries (currently none) reference live files and still need the exemption, so a stale entry fails the same as a missing migration. test_state_root_resolution.py (new): resolve_state_root's three-tier resolution order (env var wins; payload-cwd upward walk fallback finds the nearest ancestor's .autoskillit/; process cwd last resort, matching pre-migration Path.cwd() behavior exactly when both signals are absent), symlink handling for both the env-var path and the walk, and the worktree-topology scenario for pr_create_guard, git_ops_guard, and ask_user_question_guard — state under an orchestrating project root, reachable only via AUTOSKILLIT_STATE_ROOT while the hook payload's own cwd points into an unrelated sibling worktree with its own checked-in .autoskillit/. Fixes a real gap resolve_state_root's symlink-defense tests surfaced: the upward walk accepted a symlinked .autoskillit/ entry via bare is_dir() (which follows symlinks), which would let every caller's subsequent `root / ".autoskillit" / "temp" / ...` file access transparently follow the symlink outside the trust anchor (issue #4319). Now rejects any .autoskillit entry that is itself a symlink and keeps walking upward past it. Also updates test_backend_cmd_builder_base.py's exact-key-count assertion (12 -> 13) and adds AUTOSKILLIT_STATE_ROOT coverage now that _assemble_shared_env_extras injects it, and adds cook-launch producer-side coverage to test_session_launch.py for the AUTOSKILLIT_STATE_ROOT injection from _project_dir. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
test_session_replay.py drives a fixture-described session through the
full PreToolUse hook chain via the real hooks/_dispatch.py subprocess —
for each event, selects every HOOK_REGISTRY guard whose matcher
fullmatches the tool name AND whose session_scope is compatible with the
fixture's simulated session (mirroring hooks.json generation-time gating,
which _dispatch.py itself has no notion of), runs each via subprocess
with the fixture's session_env applied, and asserts every allowed:true
event is denied by no matched guard while no stderr line anywhere in the
replay carries failure-grade wording (failed/error/invalid) unless the
owning event's max_severity is "failure".
Fixture format: tests/hooks/fixtures/session_replays/*.jsonl — first
line a header ({session_env, state_setup}), each subsequent line one
event ({payload, expectations}). String values may carry
{{ORCHESTRATING_ROOT}}/{{WORKTREE_ROOT}} placeholders the harness
substitutes with real per-test tmp_path directories.
incident_transcript_v1.jsonl is modeled on the incident session: three
dual-cwd worktree run_cmd calls (pwd, git rev-parse, sed -n) with
tool_cwd pointing into a worktree while the payload's top-level cwd is
the orchestrating repo root, two Bash reads (a file read, a benign
loop), and one genuine gh pr review --approve mutation that must still
be denied by github_mutation_guard.py — proving the W1 fix and this
workstream's guard migrations compose correctly end to end. Verified by
direct execution: the five benign events pass through all 11-13 matched
guards per event with zero denials, and the one deny-expected event is
denied with the review_mutation trigger, exactly as expected.
Registers the new .jsonl fixture pattern in
.autoskillit/test-filter-manifest.yaml so changes to it route to the
hooks/ test suite, matching the existing tests/fixtures/codex/*.jsonl
precedent.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
_segment_has_possible_github_exec_token previously checked every token in a shell segment for exact equality to the exec name set, causing a bare-word argument that merely mentions the two-character executable name (e.g. echo "gh", which shlex collapses to the same tokens as an unquoted echo gh) to be misclassified as a possible exec reachable through a loop, denying a benign command that never invokes it in command position. The check now derives the segment's actual command verb via command_verb_and_args (which already skips env/wrapper prefixes and loop control words), narrowing to command position only. An inline function definition (name() open-brace) or bare compound-command opener fuses its body's first command into the same shlex segment as the opener, so the check re-derives the verb from the token immediately following such an opener to keep catching the exec name reachable through a function body -- verified this does not regress the existing "function" id case in test_multiple_or_repeatable_mutations_are_not_single and test_wrappers_and_repeatable_shell_constructs_cannot_bypass_guard. Adds the plan's literal named test case (test_bare_gh_token_as_argument_inside_a_loop_is_none) as its own test, distinct from the existing multi-word-variant test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…esolution write_guard.py's write-target resolution was migrated to prefer parse_hook_command's execution_cwd (the run_cmd tool's own cwd argument, independent of the payload's session-level cwd) over the AUTOSKILLIT_CWD env var fallback, fixing a cwd-source divergence in worktree flows -- but shipped with no standing test coverage of that behavior. Adds TestWriteGuardRunCmdExecutionCwd with: - an allow case: a relative write resolves inside the allowed prefix via the run_cmd tool's own cwd, even though AUTOSKILLIT_CWD points elsewhere (would incorrectly deny if still consulted first) - a deny case: the same shape, but the run_cmd tool's own cwd resolves the target outside the allowed prefix, even though AUTOSKILLIT_CWD points inside it (would incorrectly allow if still consulted first) - a regression case: when the run_cmd tool omits its own cwd argument, resolution still falls back to AUTOSKILLIT_CWD as before Reuses the shared make_hook_event harness from conftest.py (already used by test_github_mutation_guard.py for the same execution_cwd vs payload_cwd distinction). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The investigation confirmed two live defects and one debris field:
github_mutation_guard.pydenies everyrun_cmdwhosecwdtool argument differs from the hook payload's session cwd, before any mutation classification runs (src/autoskillit/hooks/guards/github_mutation_guard.py:84-85). A worktree cwd is the standard — effectively the only — way for a Codex cook session to operate in a worktree, so every such call is denied, includingpwd, with a reason about GitHub mutations that has nothing to do with the trigger. All nine deny paths share one hardcoded reason string (_DENY_REASON, :47-51).emit_runner_diagnostic(src/autoskillit/hooks/_capture/_reconcile.py:121-126) hardcodes "cleanup failed" around healthy, by-design bounded deferral (progress=retired blocker=record_budget errors=0), andshell_capture_hook.py:68-70reports the permanent, by-design absence of managed launch controls in cook sessions as transient breakage — on every command.shell_*.logfiles (~650 MB) are permanently invisible to every cleanup path (zero directory enumeration exists inhooks/_capture/), and the SessionStart sweep — where drain capacity lives — aborts on the firstflockcontention with no retry, so the ledger-visible backlog (~3.7k eligible records) sits in equilibrium.This PR makes each bug class structurally impossible or instantly caught, in four workstreams:
PolicyEventformatter mandated by ADR-0006; cook builders positively declarecapturemode so absence-of-declaration once again signals a genuine anomaly.The implementation was independently audited against the plan (
audit-impl, structural diff review) — plan fidelity is solid across all four workstreams. Actually running the test suite (task test-check, not part of the structural audit by design) surfaced 59 genuine regressions the structural review couldn't catch: a dropped test fixture (umask), a deleted architectural-contract constant, a cross-package import boundary violation, and several legitimate test-registry/budget counts needing updates for this plan's own additions. All were root-caused and fixed with real test verification —tests/hooks/+tests/arch/now passes cleanly except 4 pre-existing, unrelated failures that also fail identically ondevelop.A handful of additional failures outside that scope (
tests/cli/,tests/docs/test_doc_counts.py) surfaced in a full unscoped run and have not yet been triaged — tracked as follow-up, not blocking this PR.Implementation Plan
Plan file:
.autoskillit/temp/rectify/rectify_guard_capture_immunity_2026-08-04_200806.md🤖 Generated with Claude Code via AutoSkillit