fix(cosh-ng): [shell] intercept zsh slash-bearing natural-language prompts - #2210
fix(cosh-ng): [shell] intercept zsh slash-bearing natural-language prompts#2210SunnyQjm wants to merge 1 commit into
Conversation
![qoderai[bot]](https://avatars.githubusercontent.com/in/1396158?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d1af0b142
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
PR number: #2210 评审结论Findings未发现 blocking package/module/public API 组织问题。 结构核对
大文件 / 阈值
Open Questions / 剩余风险
Validation
|
…ompts Problem: zsh executes a slash-bearing command word as a path without invoking command_not_found_handler, so the natural-language classifier hooked there is unreachable. A prompt like "你读一下，并安装这个skill：/usr/share/.../SKILL.md" (full-width punctuation keeps the whole line one IFS token) surfaces as a raw "zsh: no such file or directory" error instead of being taken over by the agent (#1943). The bash sibling was fixed in #1942 via the extdebug DEBUG-trap veto, but zsh's DEBUG trap has no veto-execution semantics, so that mechanism cannot be ported. Fix (zsh-only, per SDD cosh-1943-zsh-slash-nl-intercept): - marker/zsh.rs: new _cosh_should_intercept_missing_path helper with the same five-gate verdict as the bash fix (slash-bearing first word, AI enabled, path provably ENOENT, missing-path classification says natural_language), reusing the shared input_intent.sh classifier verbatim (single source of truth, zero classifier changes). - marker/zsh.rs: a _cosh_accept_line ZLE widget mounted over the active accept-line (alias-saving chain keeps user widgets working) evaluates the gates at line submission — the only pre-execution seam zsh offers. On a natural_language verdict it re-adds the line to history (print -sr; skipped for secret-bearing lines), re-echoes the prompt+text (ZLE erases the edit line on buffer clear; sensitive lines re-echo the redaction placeholder), emits the same intercept marker shape as the bash missing-path route, clears the buffer and accepts the empty line. Every gate failure and internal error falls open to the original accept-line: worst case is the interception not firing, never a broken native line. CONTEXT!=start submissions (PS2/heredoc continuations, vared) always pass through. - bash side and input_intent.sh are untouched (zero diff). Tests: five zsh-side cases mirroring the bash anchors — positive intercept, sensitive intercept with journal redaction, fail-closed counterproofs (existing path, English typo path, dangling symlink, permission-opaque parent), user accept-line widget compatibility, and heredoc continuation pass-through. Review round 1 (Codex P1/P2 + layout gate): - P1: the manual history re-add now replays the active history policy (HIST_IGNORE_SPACE and the zshaddhistory hook chain) before print -sr, so explicit exclusions and user hooks keep vetoing persistence; any veto skips the add (worst case: non-recallable intercepted prompt). - P2: the accept-line save uses an unconditional zle -A alias, so customizations aliased to another builtin (not only user:* widgets) are preserved through the dispatch chain. - Layout: the zsh marker script body moved verbatim to zsh_marker.sh (include_str!), keeping zsh.rs under the 700-line gate; slash registry tests re-anchored to the script file. Emitted protocol is byte-identical. Assisted-by: Qoder:1.22.0 Signed-off-by: SunnyQjm <mfeng@linux.alibaba.com>
5d1af0b to
27551a8
Compare
|
Review round 1 addressed (Codex P1 history policy, P2 builtin-alias accept-line) plus the layout gate split (
Full verification re-run on the new head: workspace clippy |
|
PR number: #2210 评审结论Findings未发现 blocking package/module/public API 组织问题。 结构核对
大文件 / 阈值
Open Questions / 剩余风险
Validation
|
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Requesting changes for two blocking ZLE/history integration regressions reproduced against 27551a84 with the cosh-shell PTY harness. The PR's seven focused zsh tests pass, but they do not cover these user-customization paths.
| zle -I 2>/dev/null || true | ||
| print -r -- "${(%%)PS1}${echo_line}" 2>/dev/null || true | ||
| _cosh_emit_intercept_marker "$line" "natural_language" false "$sensitive" | ||
| _cosh_dispatch_accept_line |
There was a problem hiding this comment.
[P1] Bypass the saved widget after a successful intercept
At this point BUFFER has been cleared and the intercept marker has already been emitted, but _cosh_dispatch_accept_line invokes an arbitrary user accept-line widget. Such a widget may legitimately synthesize a command for an empty buffer. I reproduced this with a widget that sets an empty buffer to echo review-unexpected-native: the event stream became UserInputIntercepted followed by CommandStarted("echo review-unexpected-native"). This violates the interception guarantee and can execute unintended native commands. Submit the cleared line through .accept-line directly on the successful-intercept path; keep the saved user widget only for pass-through paths.
| local hook | ||
| for hook in "${zshaddhistory_functions[@]}"; do | ||
| (( ${+functions[$hook]} )) || continue | ||
| "$hook" "$line"$'\n' || return 1 |
There was a problem hiding this comment.
[P1] Do not invoke zshaddhistory hooks outside native hook processing
zsh automatically restores history contexts pushed with fc -p after native zshaddhistory processing. Calling each hook function directly here bypasses that restoration. With a valid per-directory-history hook that runs fc -p local-history, one intercepted prompt permanently changed subsequent $HISTFILE from global-history to local-history in the PTY session. Later commands can therefore be persisted to the wrong file. Preserve zsh's native hook-context semantics instead of directly replaying arbitrary hooks; the replacement should also retain status 2 semantics (internal history only, no history-file write).
2 participants
Summary
zsh sibling of #1919 (bash fixed in #1942): a natural-language prompt whose
first IFS token contains a slash and does not resolve to an existing path is
executed by zsh as a path — zsh never invokes
command_not_found_handlerforslash-bearing command words, so the NL classifier hooked there is unreachable
and the user sees a raw
zsh: no such file or directoryerror instead of theagent taking over. zsh's DEBUG trap has no veto-execution semantics (probe
evidence in #1943), so the bash mechanism cannot be ported; the only
pre-execution seam zsh offers is the
accept-lineZLE widget.Changes
marker/zsh.rs: new_cosh_should_intercept_missing_path— the samefive-gate verdict as the bash missing-path fix (slash-bearing first word,
AI enabled, path provably ENOENT, missing-path classification says
natural_language), reusing the shared
input_intent.shclassifier verbatim(single source of truth; classifier and bash side have zero diff).
marker/zsh.rs:_cosh_accept_lineZLE widget mounted over the activeaccept-line(alias-saving chain keeps user widgets working) evaluates thegates at line submission. On a natural_language verdict it re-adds the line
to history (
print -sr; skipped for secret-bearing lines), re-echoes theprompt+text (ZLE erases the edit line on buffer clear; sensitive lines
re-echo the redaction placeholder), emits the same intercept marker shape
as the bash missing-path route (
natural_language,top_level_missing=false,sensitiveas-is), clears the buffer andaccepts the empty line. Every gate failure and internal error falls open to
the original accept-line: the worst case is the interception not firing,
never a broken native line.
CONTEXT != startsubmissions (PS2/heredoccontinuations, vared) always pass through.
The zsh marker script body lives in
zsh_marker.sh(
include_str!, byte-identical move) keepingzsh.rsunder the700-line layout gate; slash registry tests re-anchored to the script
file.
Tests
Seven zsh-side integration tests mirroring the bash anchors
(
tests/shell_host/marker.rs):shell_host_zsh_missing_path_natural_language_intercepts— positiveintercept, no native error, re-echo visible.
shell_host_zsh_sensitive_missing_path_natural_language_intercepts—sensitive flag, journal whole-field redaction, redaction placeholder
re-echo, raw key absent from events/journal.
shell_host_zsh_missing_path_counterproofs_stay_native— existingexecutable, English typo path, dangling symlink, permission-opaque parent
all keep native behavior; slash-free CNF route unaffected.
shell_host_zsh_missing_path_intercepts_with_user_accept_line_widget—interception works with a user accept-line widget present and the user
widget stays in the chain for pass-through lines.
shell_host_zsh_missing_path_heredoc_continuation_stays_native—CONTEXT=cont lines pass through untouched.
shell_host_zsh_missing_path_history_respects_user_policy—HIST_IGNORE_SPACE keeps a space-prefixed intercepted prompt out of
history while the interception itself still fires (review P1).
shell_host_zsh_missing_path_intercepts_with_builtin_alias_accept_line—zle -Abuiltin-alias customizations survive the mount and native lineskeep executing through the preserved alias (review P2).
Verification
Focused (alinux3 arm64 container, zsh 5.5.1):
cargo test -p cosh-shell --test shell_host— new cases green; 37 zshfilter tests green. Pre-existing environment failures unrelated to this
diff (
heavy::raw_relay_host_shows_isolated_sudo_prompt_and_keeps_shell_usable,termios::cosh_owned_timeout_recovery_restores_pty_without_visible_command,tools::readonly_compound_tests::*reap-deadline pair) fail identically onthe clean base commit (stash-control attribution).
cargo test -p cosh-shell --lib/--bin cosh-shell— green except thesame pre-existing
readonly_compoundpair (also fails on clean base).cargo clippy --workspace --all-targets -- -D warnings(container) green;cargo fmt --checkgreen;check-layout.shandcheck-test-inventory.shgreen.
Real-machine acceptance (real PTY 120x40, real cosh-core adapter, real LLM):
single-token absolute path, f2 relative path), slash-free control scenario
unchanged; native error gone, agent takes over.
under a themed user prompt.
Not run: workspace-wide test targets beyond cosh-shell; macOS cosh-core
build (pre-existing Linux-only
rustix::openat2usage, unrelated).Evidence
Fork-hosted, commit-SHA-pinned
(
SunnyQjm/anolisa@c9a65b3e312458743766d4f530ed28b5d4ff0e8c, branchpr-2210-assets). All screenshots are full-size (120x40) final framesrendered from the real-PTY casts.
FAIL→PASS on the exact issue scenario (f1, container zsh 5.5.1):
b0a57a26)Relative-path variant (f2):
Slash-free control (c1, no regression):
macOS zsh 5.9 spot check (f1, themed user prompt):
Casts:
head-f1.cast ·
fixed-f1.cast
Closes #1943