Repository navigation
Avoid echoing denied shell commands into model context - #256
Merged
Merged
Conversation
qduc
force-pushed
the
codex/efficient-file-guidance
branch
from
November 12, 2026 01:05
968f523 to
c5541db
Compare
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
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.
An automatically denied shell command is already present in the tool call, but the non-interactive policy repeats the entire command in the rejection reason sent back to the model. A long heredoc can therefore be carried twice through subsequent requests. Keep the RED/YELLOW decision and explanation while omitting the duplicate command from generated heuristic reasons. Human-supplied reasons are unchanged. The CLI retains its existing tool preview, and the full call remains in JSON events and session history; the rejection line itself no longer contains the full command.
This is a bounded context reduction, not proof of end-to-end parity with Pi. In one concurrent Sonnet comparison, recorded heuristic-rejection strings totaled 130,993 characters across baseline requests and 8,232 with this change. Both Term2 samples made seven denials and 27 requests. Across three concurrent Term2 pairs, generated tokens were 16,976 / 20,010 / 15,276 for baseline and 17,998 / 17,190 / 13,946 with the change. The small sample is variable; it does not establish a causal or quality-preserving improvement.
In the latest comparisons, all outputs built, typechecked and passed the two recovered ACP wire regressions. One Term2 baseline and the Pi control passed 18/18 rebuilt classification cases; both changed outputs and the other baseline passed 16/18. Four other shell tests failed with the same signatures on the historical reference in the nested sandbox; the original SDK exception fixtures were unavailable. The PR remains a draft while the end-to-end benefit and quality remain uncertain.
The normal application launcher uses the changed policy. The exported headless fallback also avoids repeating the command; user-provided and injected reasons are passed through unchanged. The benchmark runners also included the Sonnet compatibility fix from #255; this change itself does not depend on that fix. An independent verifier checked the fallback decision and direct-connection output, including custom reasons and both risk levels.
Validation: focused CLI-to-provider RED/YELLOW tests pass and fail against the baseline; existing approval-reuse tests, typecheck, lint, provider black-box suite (270 passed, 5 skipped), human rejection-reason coverage and independent safety verification pass.
The earlier generation environment lacked the
socatexecutable needed by Term2’s sandbox. A repaired isolated grader reran all five outputs from the latest comparison and the full recovered ACP wire suite passed; the original runs were still generated in the impaired environment. The previously reported model comparisons therefore do not establish general quality or cost parity.A later smoke evaluation used three different small tasks (retry policy, invoice aggregation, and path containment) and two models (Sonnet and Sol). The Term2 baseline, this change, and Pi passed all 18 held-out checks. No run hit an approval denial, so this is cross-task quality smoke evidence, not evidence of savings from this change. Pi had skills disabled while Term2 retained its skills. Provider cache sharing also differed across some runs; no stable cost-parity claim is made.