feat: replace output_budget_guard with lossless shell capture (#4286)#4288
Merged
Conversation
Trecek
commented
Jul 18, 2026
Trecek
left a comment
Trecek
left a comment
Collaborator
Author
There was a problem hiding this comment.
AutoSkillit PR Review — Verdict: changes_requested
Trecek
commented
Jul 18, 2026
Trecek
left a comment
Trecek
left a comment
Collaborator
Author
There was a problem hiding this comment.
AutoSkillit review: blocking findings detected. Changes are requested; see inline comments.
Trecek
commented
Jul 18, 2026
Trecek
left a comment
Trecek
left a comment
Collaborator
Author
There was a problem hiding this comment.
AutoSkillit PR Review — Verdict: approved_with_comments
Trecek
commented
Jul 18, 2026
… Phase A) - Add _policy_event.py: typed PolicyEvent dataclass + render_provenance_prefix - Gate output_budget_guard on Codex sessions (env var OR turn_id in payload) - Replace R1-R3 jargon with provenance-bearing deny messages - Add _suggest_bounded_rewrite: classifier-validated rewrite suggestions - Update HookDef enforcement_strength to not-applicable for claude_code - Create ADR-0006 (output-boundary containment), amend ADR-0005 Layer 3 - Update all dependent tests for new Codex-only scope and message format Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cture) - Add CapturedStream dataclass to core types - Add CaptureSetupError/CaptureReadError exceptions and summarize_capture() - Extend create_temp_io with capture_dir and keep_streams parameters - Thread capture_dir through SubprocessRunner protocol and all conformers - Add _run_subprocess_captured for capture-mode subprocess execution - Extract run_cmd_artifact_root helper from spill_run_cmd_result - Rewrite spill_run_cmd_result to accept CapturedStream objects - Update run_cmd to use capture mode with exhaustive TerminationReason handling Tests not yet updated — next commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Re-export CaptureReadError/CaptureSetupError/summarize_capture through facade - Update tools_execution.py imports to use facade (not private _process_io) - Extract _summarize_streams helper to satisfy no-for-loop-in-handlers rule - Update MockSubprocessRunner to handle capture_dir in tests/fakes.py - Update spill/run_cmd tests for capture-mode SubprocessResult - Add test_process_io_capture.py and TestCaptureMode integration tests - Fix arch exemptions: import allowlists, cross-submodule exemptions, pyright budget - Regenerate codex hook format snapshot fixture - Rename ADR-0006 to comply with 3-segment filename rule Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…4286) Replace output_budget_guard with shell_capture_hook (PreToolUse input-rewrite). Delete classification machinery, remove small_file_max_bytes config surface. Conformance corpus (13 rows) as hard gate. ADR-0005/0006 updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add 5 new tests to test_tools_execution_spill.py covering capture-mode error paths (CaptureReadError, CaptureSetupError, timeout, signal death, orphan prevention). Add 6 conformance corpus rows (rg, jq, heredoc normalization, trailing backslash, self-signal, unicode boundary) to satisfy OBB-038/OBB-041. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ables
The printf marker argument was single-quoted, suppressing expansion of
$__as_sz, $__as_f, and $__as_sha. Use POSIX quote-break technique
('literal'"$var"'literal') to allow variable expansion.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
OSError from NamedTemporaryFile (disk/fd exhaustion, permission race) now raises CaptureSetupError instead of bypassing the structured capture-failure handler. Cleans up stdout file if stderr creation fails. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Non-dict JSON (null, scalar, list) would crash _is_codex_session or .get() calls with TypeError/AttributeError instead of graceful exit(0). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
os.open() returns a fd that was passed to fsync() but never closed, leaking one descriptor per promoted capture stream. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Expose CaptureReadError, CaptureSetupError, and summarize_capture from autoskillit.execution instead of requiring direct submodule import. Remove the bespoke REQ-ARCH-001 exemption that weakened the guard. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When summarize_capture fails, the proc_*.tmp file was left on disk with no reference in the output. Now log the path in the error message and unlink the file to prevent accumulation of undiscoverable orphans. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cover stream retention, path population, and setup-error handling for the synchronous capture_dir path, matching the async test coverage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The existing oracle concatenates separately captured stdout+stderr which does not match real 2>&1 interleaving. Add _run_raw_merged helper and a test that alternates both descriptors to validate ordering. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The existing mock-based test bypasses the streaming path. Add a complementary test that verifies summarize_capture returns bounded slices from a real 200KB file without reading it fully into memory. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The test previously asserted no double quotes in the marker, which conflicts with the fix for variable expansion. Now validates that each shell variable is wrapped in expansion-safe '"$var"' pattern instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ailure Moves the function-local os/uuid imports in _process_capture_stream and _uuid8 to module scope (os was already imported), and wraps os.replace in try/except OSError so a failed artifact promotion returns a structured capture_failed error instead of crashing the tool call.
test_policy_event.py exercised render_provenance_prefix() using the retired output_budget_guard hook name and its old reason code. Update both cases to shell_capture_hook v1 / SHELL_OUTPUT_CAPTURED, matching what the production hook actually emits.
test_run_cmd_partial_capture_error_leaves_no_orphan only checked the successful stdout capture file was unlinked, despite its name implying full orphan coverage. Add the matching assertion for the stderr file that failed summarize_capture().
Trecek
force-pushed
the
impl-output-boundary-bounding-20260718-113106
branch
from
July 18, 2026 22:46
e9be65d to
fbb79bd
Compare
…e limit tools_execution.py exceeded the 1600-line exemption after rebase. Move _summarize_streams (and its CaptureReadError/summarize_capture deps) to _execution_helpers.py where _process_capture_stream already lives. Update test monkeypatch targets accordingly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Jul 22, 2026
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
output_budget_guardto Codex-only with typed provenance messages; Claude Code sessions no longer see deny surfacesrun_cmd— subprocess output goes directly to artifact-directory files with bounded inline slices and fail-stop capture semanticsshell_capture_hook.py(PreToolUse input-rewrite) that wraps every Codex native shell command in a capture harness with trap-EXIT postlude, delete the command-shape classifier andoutput_budget_guard.pyentirelyImplements ADR-0006 (Output-Boundary Containment). The pre-execution shape classifier is retired in favor of per-backend output-boundary bounding on measured bytes.
Key changes
shell_capture_hook.py— every Codex shell command's complete output captured to.autoskillit/temp/shell_capture/shell_<uuid8>.log; bounded inline slice with provenance markerclassify_command_output_budgetand ~380 lines of classification machinery deleted from_command_classification.pysmall_file_max_bytesremoved from config surface (existed solely for classifier's JSONL exception)Closes #4286
Test plan
task test-checkpasses (29,718 passed, 553 skipped, 55 xfailed)pre-commit run --all-filescleantest_capture_dir_uncreatable_fail_stops: fail-stop verified🤖 Generated with Claude Code