Pipeline Tracker Split-Brain — Server-Authoritative Step Completion by Trecek · Pull Request #4296 · TalonT-Org/AutoSkillit

Trecek · 2026-07-19T21:00:24Z

Summary

The remediation pipeline for #4291 aborted on a false DEPENDENCY UNMET because three architectural weaknesses composed: split-brain state ownership between client-side hook and server-side enforcer, unmanaged tracker lifecycle allowing stale files to accumulate, and a lossy presentation layer that dropped the diagnostic error field.

This PR delivers three architectural immunities:

Phase A — Step completion becomes server-authoritative, written at run_skill's adjudication point through the same resolve_tracker_order_id() function the enforcement check uses. Enforcer and recorder become structurally unable to disagree. The fragile single-file discovery heuristic is deleted; an architectural guard prevents future client-side tracker writes.
Phase B — Kitchen-scoped state gains a liveness-gated pruning lifecycle at every open_kitchen, using active_kitchens.json liveness checks with an any-entry-alive rule (fleet-safe). Close-only cleanup stops being load-bearing.
Phase C — The error field becomes structurally indestructible through the formatter layer via a single-exit-point pass-through gate. All pre-flight deny sites use deny_envelope() with stage/retriable fields. DEPENDENCY UNMET becomes an actionable denial routed like QUOTA WAIT and INGREDIENT LOCK.

Requirements

Make step-completion server-authoritative
Fix tracker lifecycle — prune/archive stale trackers at open_kitchen time
Guarantee failure-envelope fidelity through formatting
Make the deny actionable — include recovery guidance in DEPENDENCY UNMET
Regression tests covering two-tracker scenario, open-no-close-open lifecycle, deny-envelope fidelity, end-to-end handoff

Closes #4293

Implementation Plan

Plan file: .autoskillit/temp/rectify/rectify_pipeline_tracker_split_brain_2026-07-18_214000_combined.md

🤖 Generated with Claude Code via AutoSkillit

Step completion marking moves from the client-side PostToolUse hook to the run_skill adjudication point in tools_execution.py, using a shared tracker resolver that guarantees recorder and enforcer agree on which tracker file to target. Key changes: - Extract resolve_tracker_order_id() shared resolver - Add op="complete" to record_pipeline_step MCP tool - Mark steps complete at run_skill adjudication point - Retire pipeline_step_post_hook.py (RETIRED_SCRIPT_BASENAMES) - Migrate pipeline_step_guard.py to kitchen_id-based resolution - Delete discover_single_tracker_order_id fragile heuristic - Add pipeline_tracker field to RunSkillResult TypedDict - Add architectural guard test_tracker_write_provenance.py Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add prune_stale_kitchen_state() that removes tracker files belonging to dead kitchens at every open_kitchen, using active_kitchens.json liveness checks. Stale kitchen_state markers are also swept. The overlay lock sidecar is now cleaned up at close_kitchen. Key changes: - Add kitchen_entry_alive() public liveness wrapper - Add prune_stale_kitchen_state() with any-entry-alive rule - Wire pruning into both fresh-open and deferred-recall paths - Wire clear_kitchens_for_pid + sweep_stale_markers at open - Clean up .hook_config_overlay.lock at close_kitchen - Add lifecycle tests for dead/live/orphan tracker pruning Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The error field is now structurally indestructible through the formatter layer via a dispatch-layer pass-through gate. DEPENDENCY UNMET becomes an actionable denial routed like QUOTA WAIT and INGREDIENT LOCK. Key changes: - Add deny_envelope() canonical constructor in _types.py - Add error pass-through gate in pretty_output_hook._format_response - Move error from suppressed to rendered in run_cmd/token/timing formatters - Add recovery instruction to DEPENDENCY UNMET deny text - Add DEPENDENCY UNMET to orchestrator/fleet prompt denial patterns - Add DEPENDENCY UNMET PROTOCOL to sous-chef SKILL.md - Add stage/retriable fields to deny envelopes for orchestrator routing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Remediate 18 MISSING findings from audit-impl: Phase A gaps: - Add advisory field for dependents with unmet prerequisites in mark_step_complete (REQ-013) - Add fleet checkpoint test for server-marked tracker entries (REQ-009) Phase B gaps: - Add test_multi_entry_same_kitchen_one_alive_preserves_tracker (REQ-048) - Add test_same_process_reopen_replaces_registry_entry (REQ-043) - Add prune-safety unit tests: malformed JSON, missing kitchen_id, registry failure (REQ-046) - Add fleet checkpoint bridge test (REQ-009) Phase C gaps: - Convert ALL ad-hoc deny dict sites to deny_envelope() with stage and retriable fields (REQ-065): ingredient locks, multi-pipeline, plan path, cwd, ambiguous-step - Add error rendering to _fmt_kitchen_status (REQ-070) - Add test_error_field_survives_every_registered_formatter (REQ-058) - Add test_dependency_unmet_deny_envelope_fidelity (REQ-059) - Add test_dependency_deny_carries_recovery_instruction (REQ-060) - Add test_preflight_denials_use_canonical_envelope (REQ-061) - Add DEPENDENCY UNMET prompt routing content tests (REQ-062) Docs: - Update server/tools/AGENTS.md with new capabilities (REQ-038/056/075) - Update hooks/formatters/AGENTS.md with error-fidelity invariant (REQ-075) - File follow-up issues #4294, #4295 (REQ-057) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The gate_error and tool_exception branches in _format_response used early returns that bypassed the error-fidelity backstop gate. Restructure to an if/elif chain so all branches flow through the single exit point where the error text is guaranteed to survive. Found by test_error_field_survives_every_registered_formatter which parametrizes over all formatters × subtypes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The integration tests use bare skill commands that don't satisfy the input contract gate added in #4279. Set input_contract_resolver=None in the test setup to bypass the gate (same pattern as test_tools_run_skill_retry.py). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…t_deferred_recall_open_still_prunes (REQ-044, REQ-047) Add the two specifically named tests required by Phase B Step 1 #6 and #9: - test_open_kitchen_sweeps_stale_kitchen_state_markers: verifies sweep_stale_markers removes aged kitchen_state markers (25h old) while preserving fresh ones - test_deferred_recall_open_still_prunes: verifies prune_stale_kitchen_state reaps a dead kitchen's tracker, covering the deferred-recall path wiring Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Full test-check now passes with zero failures (29899 passed). Key fixes: - Hoist deny_envelope import to module level in tools_execution.py (no circular dependency; fixes guard-clause single-statement pattern broken by the earlier inline import, restoring never-raises structural enforcement) - Add read_active_kitchens_registry() public API to _plugin_cache.py and export via core/__init__.pyi — prune_stale_kitchen_state no longer reaches into the private _plugin_cache submodule (REQ-ARCH-001) - Tag remaining deferred autoskillit.server.tools.* imports with # circular-break (IL-3 same-layer deferred imports) - Fix integration test fixtures to use real RecipeStep objects instead of plain dicts — the run_skill handler accesses .provider/.with_args/ .stale_threshold attributes on active_recipe_steps entries - Add error/stage/retriable fields to RunSkillResult TypedDict - Narrow test_tracker_write_provenance's AST write-detector to os.write(fd, ...) specifically, fixing a false positive on sys.stdout.write() in session_start_hook.py - Extract _maybe_tracker_line() helper in _fmt_execution.py to stay under file size budget - Regenerate codex hook snapshot fixture and registry.sha256 - Update stale line numbers in the JSON-write-site allowlist and add the new mark_step_complete write site (same tracker schema as init) - Update doc counts (43->42 hooks, 10->9 PostToolUse) after hook retirement - Add step_name to record_pipeline_step's _TOOL_PARAMS entry - Add test_tracker_write_provenance.py to the resolve-review Architectural Constraint Catalog - Bump tools_kitchen.py/tools_execution.py line-limit exemptions with rationale for the new pruning/deny-envelope functionality Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Trecek

AutoSkillit PR Review — Verdict: approved_with_comments

Trecek

AutoSkillit review: warning-only findings detected. See inline comments — no blocking changes required.

…_server_side_marking The test only asserted absence of an error message substring, which would pass even if the call failed with a differently-worded denial. Add an explicit success assertion.

…with typed field Add multi_pipeline bool field to ResolutionRefusal so _check_pipeline_deps can branch on the field instead of substring-matching the reason text. Prevents silent breakage if the reason wording changes.

…le_pipeline Sibling tests assert on error message substrings; this test only checked success=False. Add substring check for consistency.

The function is defined in core/_plugin_cache.py and imported by tools_kitchen.py — the table entry conflated 'used by' with 'defined in'.

Fix test_errors_on_unresolvable_pipeline to assert on the actual error path hit (pipeline_id is required). Update schema version convention allowlist for line number shifts in tools_pipeline_tracker.py.

Trecek

AutoSkillit PR Review — Verdict: approved_with_comments

…ove fcntl to top-level Refactor record_pipeline_step and _handle_complete error branches to use deny_envelope() instead of raw json.dumps({...}) dicts, aligning the pipeline tracker module's error shape with the rest of the run_skill pre-flight deny surface. Move fcntl import to module top-level. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Remove _WRITE_FUNC_NAMES and _TRACKER_PATH_SEGMENTS frozensets that were defined but never referenced — _has_file_write and _has_tracker_reference hardcode their own literal checks directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Update _resolve_order_id_from_kitchen to skip the self-named file from candidates, matching resolve_tracker_order_id in the server. Prevents the hook from producing advisory context that disagrees with the server-side enforcer. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Trecek

AutoSkillit PR Review — Verdict: approved_with_comments

…, allow kitchen-only complete - mark_step_complete error paths now use deny_envelope() for consistency with _handle_complete and tools_execution.py deny surface - Renamed misleading preflight:ambiguous_step stage to preflight:unresolved_step on the non-ambiguous empty-step branch - Moved op=complete dispatch above the pipeline_id gate so resolve_tracker_order_id kitchen_id-only fallback is reachable - Also used deny_envelope for the pipeline_id-required gate itself - Updated test assertions and schema allowlist line numbers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…avoidance Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Trecek

AutoSkillit PR Review — Verdict: approved_with_comments

Trecek

AutoSkillit review: warning-only findings detected. See inline comments — no blocking changes required.

Extract _write_tracker and _setup_project into shared tests/server/_pipeline_test_helpers.py. Move _write_registry into tests/server/_helpers.py. Update all consumer test files to import from the shared locations. Also rewrites test_open_kitchen_sweeps_stale_kitchen_state_markers to actually call _open_kitchen_handler() instead of testing sweep_stale_markers() directly, so it genuinely validates the wiring. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…vior resolve_tracker_order_id() was falling through to kitchen_id when exactly one non-self candidate existed, diverging from _resolve_order_id_from_kitchen in pipeline_step_guard.py which returns the single candidate's stem. Now both paths agree: single candidate → use its stem. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…SUFFIX_RE Both pipeline_step_guard.py and token_summary_hook.py defined identical _SUFFIX_RE = re.compile(r"-\d+$"). Centralized as STEP_SUFFIX_RE in _hook_utils.py (stdlib-only shared module). The server-layer copy in tools_pipeline_tracker.py is intentionally kept separate due to the import layer boundary. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Prior commits in this PR shifted json.dumps sites in tools_pipeline_tracker.py by 2 lines (254→256, 400→402). Update the _LEGACY_JSON_WRITES allowlist to match. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Trecek and others added 8 commits July 19, 2026 08:16

Trecek commented Jul 19, 2026

View reviewed changes

Comment thread tests/integration/test_pipeline_step_completion_flow.py

Comment thread src/autoskillit/server/tools/tools_execution.py Outdated

Trecek commented Jul 19, 2026

View reviewed changes

Comment thread tests/server/test_record_pipeline_step_complete.py

Trecek commented Jul 19, 2026

View reviewed changes

Comment thread src/autoskillit/server/tools/_types.py

Trecek commented Jul 19, 2026

View reviewed changes

Comment thread src/autoskillit/server/tools/tools_pipeline_tracker.py

Trecek commented Jul 19, 2026

View reviewed changes

Comment thread src/autoskillit/server/tools/AGENTS.md Outdated

Trecek commented Jul 19, 2026

View reviewed changes

Comment thread src/autoskillit/server/tools/tools_execution.py

Trecek commented Jul 19, 2026

View reviewed changes

Comment thread src/autoskillit/server/tools/tools_pipeline_tracker.py

Trecek commented Jul 19, 2026

View reviewed changes

Comment thread src/autoskillit/server/tools/tools_execution.py

Trecek commented Jul 19, 2026

View reviewed changes

Comment thread src/autoskillit/server/tools/tools_pipeline_tracker.py Outdated

Trecek added 5 commits July 19, 2026 15:48

fix(review): assert success=True in test_dependent_step_allowed_after…

4b7fdfa

…_server_side_marking The test only asserted absence of an error message substring, which would pass even if the call failed with a differently-worded denial. Add an explicit success assertion.

fix(review): add error content assertion to test_errors_on_unresolvab…

d6b2aff

…le_pipeline Sibling tests assert on error message substrings; this test only checked success=False. Add substring check for consistency.

fix(review): correct AGENTS.md attribution for kitchen_entry_alive

daf895d

The function is defined in core/_plugin_cache.py and imported by tools_kitchen.py — the table entry conflated 'used by' with 'defined in'.

fix(review): correct test assertion and update allowlist line numbers

efbc415

Fix test_errors_on_unresolvable_pipeline to assert on the actual error path hit (pipeline_id is required). Update schema version convention allowlist for line number shifts in tools_pipeline_tracker.py.

Trecek commented Jul 19, 2026

View reviewed changes

Trecek and others added 4 commits July 19, 2026 16:20

fix(review): update allowlist line numbers after deny_envelope refactor

edbab66

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Trecek commented Jul 19, 2026

View reviewed changes

fix(review): add docstring to _TrackerCtx explaining circular-import …

59b0a16

…avoidance Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Trecek commented Jul 20, 2026

View reviewed changes

Trecek and others added 5 commits July 19, 2026 18:23

ci: re-trigger workflow after queued run cancellation

9180ae1

Trecek added this pull request to the merge queue Jul 20, 2026

Merged via the queue into develop with commit 5b78e97 Jul 20, 2026
3 checks passed

Trecek deleted the impl-rectify-pipeline-tracker-split-brain-20260719-080146 branch July 20, 2026 03:12

Trecek mentioned this pull request Jul 21, 2026

check_review_posted false negative: codex review-pr writes receipt to flat dir #4311

Open

Uh oh!

Conversation

Trecek commented Jul 19, 2026

Summary

Requirements

Implementation Plan

Uh oh!

Trecek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Trecek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Trecek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Trecek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Trecek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Trecek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant