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
|
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.
27551a8 to
72c5da8
Compare
|
Review round 2 addressed (kongche-jbw P1 x2: intercept-path user-widget bypass, foreign zshaddhistory hook replay). History rewritten by amend + rebase onto latest main:
Re-verification on the new head: 39 zsh-filter shell_host tests green (incl. 2 new review-anchored tests: @kongche-jbw both reproductions should now be closed off — happy to re-run your PTY scenarios if anything still trips. |
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: 6ece763ed999...72c5da8346bc
[P1] Keep URL-shaped tokens on the native path
src/cosh-ng/crates/cosh-shell/src/shell_host/marker/zsh_marker.sh:185
The missing-path context disables the slash veto without adding the URL counterproof required by
#1943. In a readable working directory, https://example.invalid/path 请帮我打开 proves missing
and classifies as natural_language, so this branch sends a URL-shaped command to the agent
instead of preserving zsh's native result. Please reject URL schemes before this verdict and add
a PTY counterproof with a URL first token plus natural-language text.
[P1] Wrap the widget bound to the submit key
src/cosh-ng/crates/cosh-shell/src/shell_host/marker/zsh_marker.sh:710
Replacing the widget named accept-line does not affect a keymap that directly binds ^J or
^M to accept-line-and-down-history or a user widget. After this mount, bindkey '^J' still
points to that alternate widget, so _cosh_accept_line is unreachable and the slash-bearing NL
input reaches the original raw path error. Preserve and wrap the active submit-key bindings across
keymaps, then add a PTY test using a direct bindkey customization.
[P2] Apply the complete in-memory history policy
src/cosh-ng/crates/cosh-shell/src/shell_host/marker/zsh_marker.sh:687
_cosh_history_policy_allows only honors HIST_IGNORE_SPACE and hooks. print -s also bypasses
options such as HIST_IGNORE_DUPS: with that option set, two identical intercepted prompts are
both inserted, whereas native submission keeps only one. This makes the new route persist history
entries the user asked zsh to suppress. Cover the applicable duplicate/reduction options or avoid
the raw history insertion, with a repeated-prompt regression test.
[P2] Do not evaluate PS1 again while re-echoing
src/cosh-ng/crates/cosh-shell/src/shell_host/marker/zsh_marker.sh:694
${(%%)PS1} performs prompt expansion. With PROMPT_SUBST and a PS1 containing $(...), an
intercepted line runs that command substitution here and then again when zsh renders the next
prompt. Side-effecting or expensive prompt commands therefore run one extra time only on this
route. Reuse the prompt already rendered by ZLE, or re-echo without evaluating PS1, and add a
PROMPT_SUBST counter test.
72c5da8 to
5e4d258
Compare
|
Review round 3 addressed in [P1] URL-shaped tokens — valid (verified: [P1] Submit-key bindings — valid. Redesign: submission is now modeled on keymap bindings, not the widget name. The mount claims [P2] In-memory history policy — valid (reproduced: two identical intercepted prompts both inserted under HIST_IGNORE_DUPS). Redesign: the manual history re-add is removed entirely — options × hooks × fc -p contexts form an open-ended bypass surface, and three rounds each found a new one. Intercepted lines are now never re-added; the failure direction is a non-recallable prompt, never a persisted line the user asked zsh to suppress. Test: [P2] PS1 re-evaluation — valid (reproduced:
Verification on the new head: 42 zsh-filter shell_host tests green (incl. the four new review-anchored tests); workspace clippy |
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: 43d939139df6...5e4d2581784b
[P1] Preserve submit widgets per keymap
src/cosh-ng/crates/cosh-shell/src/shell_host/marker/zsh_marker.sh:733
_COSH_SUBMIT_KEY_WIDGETS is indexed only by $KEYS, although the mount scans four keymaps.
If emacs and viins bind ^M to different widgets, the later scan overwrites the saved
emacs widget. Pressing Enter in emacs then invokes the viins widget for every pass-through
line; a mode-specific widget may rewrite or submit a different buffer.
Possible direction: key saved widgets by keymap plus key (or use per-keymap wrappers), and add
a PTY test with distinct ^M widgets in emacs and viins that exercises both active maps.
5e4d258 to
0c5864e
Compare
|
Review round 4 addressed in [P1] Preserve submit widgets per keymap — valid — accepted and fixed. Verification: confirmed the flat Fix: the delegation table is keyed by Evidence: new test
|
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: 4349e319a682...0c5864e1000e
[P1] Prevent recursive dispatch through accept-line
src/cosh-ng/crates/cosh-shell/src/shell_host/marker/zsh_marker.sh:653
A directly bound submit widget may legitimately finish with zle accept-line. On a pass-through
line, this function invokes that widget; its accept-line call re-enters _cosh_accept_line, which
dispatches the same widget again. Binding both ^M and ^J to such a widget and submitting
echo ok repeats the widget until zsh reports maximum nested function level reached, so native
lines can multiply widget side effects and fail submission.
Possible direction: guard delegated dispatch so a re-entrant accept-line call reaches
_cosh_orig_accept_line, and add a PTY test whose directly bound widget uses zle accept-line
and runs exactly once.
0c5864e to
778126f
Compare
|
Review round 5 addressed in [P1] Prevent recursive dispatch through Verification: confirmed — a delegated widget finishing with the NAMED Fix: delegated dispatch (both the submit-key table hit and the saved-alias branch) now sets an in-progress flag around the Evidence: new test
|
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: 4349e319a682...778126fdbef7
[P1] Preserve the saved accept-line chain on re-entry
src/cosh-ng/crates/cosh-shell/src/shell_host/marker/zsh_marker.sh:679
When ^M is bound to widget A and accept-line is customized to widget B, A may finish with
zle accept-line to invoke B. The new in-progress branch instead jumps directly to
.accept-line, so B never runs on pass-through submissions; its buffer transforms, validation,
or side effects are silently lost. The added test leaves accept-line at the builtin and misses
this combined customization.
Possible direction: Track key-widget delegation separately from saved-accept-line delegation.
First re-entry should invoke _cosh_orig_accept_line; only a further recursive entry should reach
the builtin. Please add a PTY regression with both custom widgets active.
|
@kongche-jbw Rounds 4–5 were consecutive point-fixes in the same delegation-chain domain, which is a signal the input space was never enumerated up front — so instead of waiting for the next counter-example, here is the complete dispatch state machine we now anchor the implementation and tests to. It is also recorded in the design spec as the ruling contract for this domain. Invariants
Entry × context × gate × delegation matrix
Ruling contract: a new finding in this domain should locate its cell — existing cells are ruled by their outcome and invariant; only an entry mode outside the matrix constitutes a design gap (which would trigger a redesign, not another patch). S11 is the one combination we explicitly declare out of scope — please flag if you disagree with that boundary. |
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: 4349e319a682...778126fdbef7
[P1] Preserve the saved accept-line target in S5
src/cosh-ng/crates/cosh-shell/src/shell_host/marker/zsh_marker.sh:679
The proposed S5 outcome still changes a valid combined setup. If Enter is bound to widget A and
accept-line names widget B, A may call zle accept-line specifically to delegate to B. The guard
at this line jumps to .accept-line, so B's buffer edits, validation, and side effects disappear.
This violates I-b even though A runs once: native behavior is A once, followed by B once.
Possible direction: distinguish key-widget dispatch from saved-accept-line dispatch. The first
re-entry from A should call _cosh_orig_accept_line; only a further recursive entry should reach
the builtin. Please add a PTY regression with distinct A and B widgets.
|
PR number: #2210 评审结论Findings未发现 blocking package/module/public API 组织问题。 结构核对
大文件 / 阈值
Open Questions / 剩余风险
Validation
|
778126f to
1cfdacc
Compare
|
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.
Review baseline: 4b131c4faa2a...1cfdaccd2675
Found one new blocking correctness issue in the current head. The previously reported P1 at zsh_marker.sh:679 (re-entry skips the saved accept-line target in the combined A → B widget chain) is unchanged and remains blocking; I have not duplicated that inline comment.
Validation: reproduced the new issue with zsh 5.9 on aarch64 Linux using the exact current gate/classifier logic. Relevant GitHub CI checks are green, but they do not cover this Zsh resolution path.
1cfdacc to
5b35d40
Compare
|
Review round 6 addressed in [P1] Keep Zsh-resolved commands on the native path — valid — fixed by mirroring zsh command-word resolution. Verification: reproduced in a container (zsh 5.5.1 and PTY tests): Fix (design-level): the gate's legitimacy premise is "zsh would necessarily report no such file or directory for this line". This wrapper fires before zsh parses the line (bash's hook fires after resolution already failed), so the gate now reproduces zsh's own resolution order for a slash-bearing word:
The design spec now records the full resolution-order table (alias → function → glob expansion → path exec, each level with its gate outcome) as the ruling contract for this domain; suffix aliases are the one declared non-goal boundary. The shared classifier is intentionally unchanged: on the bash side the hook only fires after expansion/resolution already failed, so the Han-branch metacharacter behavior is not reachable as a false intercept there — the compensation belongs to the pre-parse zsh gate. [P1] Re-entry skips the saved accept-line target (A → B chain) — valid — round 5's implementation deviated from your suggested direction (it terminated on the builtin instead of Evidence: three new PTY tests —
Anti-regression: with the fix reverted (tests kept) all three fail; restored, the zsh-filter suite is 47/47 green. Workspace clippy
|
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: 52eeb520147b...5b35d4058965
[P1] Parse command words before proving them missing
src/cosh-ng/crates/cosh-shell/src/shell_host/marker/zsh_marker.sh:196
first_word is still raw BUFFER text here. Zsh runs a defined 路径/run
function for "路径/run" 帮我运行, but whence receives the quotes and the path
probe treats "路径 as missing, so this gate returns natural_language and
consumes a valid command. Brace expansion has the same problem:
/b{in,路径}/echo 帮我运行 executes /bin/echo natively but is intercepted here.
Possible direction: safely normalize a literal command word, or fail open on
quoting, escaping, and every pre-execution expansion form. Add PTY counterproofs
for quoted slash functions or paths and brace-expanded commands.
[P1] Preserve bindings when keymaps alias the same map
src/cosh-ng/crates/cosh-shell/src/shell_host/marker/zsh_marker.sh:788
If an rcfile binds a custom Enter widget and then runs bindkey -A main vicmd
before this marker loads, main and vicmd share one map. The scan saves the
original only under main; its rebind is already visible through vicmd, so the
second scan skips it. In vicmd, KEYMAP=vicmd, the lookup at line 673 misses, and
the global accept-line target silently replaces the user's submit transform or
validation on every native line. Possible direction: capture every logical map
before rebinding aliased maps, or mirror the saved entry. Add a PTY regression
that aliases vicmd to main, enters vicmd, and verifies the widget runs once.
|
PR number: #2210 评审结论Findings未发现 blocking package/module/public API 组织问题。 结构核对
大文件 / 阈值
Open Questions / 剩余风险
Validation
|
审阅 @
|
5b35d40 to
86e37c7
Compare
|
Review round 7 addressed in [P1] Parse command words before proving them missing — valid — the raw first word has seen no lexing or expansion, so quoting ( Fix (design-level): the round-6 glob veto generalizes into a literal-word whitelist — instead of enumerating dangerous characters (a blacklist always misses the next form), the gate only continues for a first word made purely of non-syntax characters (alphanumerics, [P1] Preserve bindings when keymaps alias the same map — valid — with Evidence: three new PTY tests —
Anti-regression: with the fix reverted (tests kept) all three fail; restored, the zsh-filter suite is 50/50 green. Workspace clippy
|
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: 0a190d509b8e...86e37c72df18
[P1] Claim submit keys in user-defined keymaps
src/cosh-ng/crates/cosh-shell/src/shell_host/marker/zsh_marker.sh:811
The mount scans only main and vicmd. If an rcfile creates a custom keymap,
binds ^M/^J there to a widget that finishes with zle .accept-line, and
selects that map from zle-line-init, Enter never reaches _cosh_accept_line.
The issue input then still produces the native no such file or directory
error and no intercept marker. This setup exists before the marker loads, so it
is not the declared post-mount rebind boundary.
Possible direction: enumerate user-defined keymaps at mount time, preserve
aliases as this table already does, and add a PTY regression that submits from
a custom map.
|
PR number: #2210 评审结论Findings未发现 blocking package/module/public API 组织问题。 结构核对
大文件 / 阈值
Open Questions / 剩余风险
Validation
|
|
Re-verified against The mount loop only claims Minimal zsh 5.9 evidence after applying the current claim loop: Because the custom map and binding exist in the rcfile before the marker loads, this is outside neither the mount contract nor the declared post-mount-rebind boundary. The finding should remain blocking until the mount handles pre-existing user-defined keymaps and a PTY regression covers that path. Related review: #2210 (review) |
|
Review round 8 addressed in [P1] Claim submit keys in user-defined keymaps — valid — the main/vicmd claim enumeration rested on the round-4 probe showing runtime Fix (design-level): zsh's builtin keymap set is closed ( Evidence: new PTY test
|
86e37c7 to
97f275c
Compare
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: 8f2416470233...97f275cb701d
[P1] Preserve whitespace in custom keymap names
src/cosh-ng/crates/cosh-shell/src/shell_host/marker/zsh_marker.sh:818
bindkey -N 'review map' main is valid, and bindkey -l emits that name on one line.
The unquoted command substitution here splits it into review and map, so the actual
map is never claimed. If zle-line-init selects that map and its submit widget calls
zle .accept-line, Enter bypasses _cosh_accept_line; the slash-bearing prompt still
ends in zsh's native no such file or directory instead of an intercept.
Possible direction: iterate bindkey -l line by line and cover a space-bearing map in PTY.
[P1] Mirror aliases from the map they actually share
src/cosh-ng/crates/cosh-shell/src/shell_host/marker/zsh_marker.sh:796
When main and vicmd have different submit widgets and a custom map aliases main,
both source maps have already been rebound when the custom map is scanned. This loop
copies the first saved entry matching the key, without checking which physical map the
custom name aliases. On zsh 5.9 at this head, bindkey -A main review-alias stores
_vicmd_submit for review-alias. Every pass-through Enter in that map then invokes
the wrong user widget, which can apply mode-specific edits or validation.
Possible direction: capture each logical map's binding before any rebind, then add a PTY
regression with distinct main/vicmd widgets and a custom alias of main.
|
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. Review round 2 (kongche-jbw, two blocking P1s reproduced on PTY): - A successful intercept now finalizes through the builtin .accept-line: a saved user widget may synthesize a command for an empty buffer, which would execute a native line the marker already claimed as intercepted. The saved widget stays on pass-through paths. - The history policy check no longer replays foreign zshaddhistory hooks: outside native hook processing zsh does not restore fc -p history contexts and status-2 semantics cannot be honored, so any foreign hook (or standalone zshaddhistory function) fails closed to skipping the manual re-add; only cosh's own filter is consulted. Review round 3 (kongche-jbw, 2 P1 + 2 P2; two domains hit the review circuit-breaker after repeated counter-examples, so both were redesigned instead of point-fixed): - URL gate: a scheme-shaped first word (https://...) proves missing in a readable cwd and classifies as natural language, so the gate now vetoes [a-zA-Z]*://* shapes before any filesystem probe. - Submission is modeled on keymap bindings, not the accept-line name: the mount claims ^M/^J across main/emacs/viins/vicmd when they bind another widget, remembers the original per key, and the pass-through dispatch routes $KEYS back to it. Other keys binding accept variants are an explicit non-goal (fail-open). Requires the self-named zle -N _cosh_accept_line registration. - History replay is abandoned entirely: intercepted lines are never re-added (options x hooks x fc -p contexts form an open-ended bypass surface; three rounds each found a new one). - The re-echo defers to the next precmd as plain text: zle -I repaint and %-expansion both run PROMPT_SUBST command substitutions an extra time, so the intercept route now touches no prompt rendering. Review round 4 (kongche-jbw P1): the submit-key delegation table is now keyed per keymap (${KEYMAP}:${KEYS}) — a flat per-key table let a vicmd-specific widget overwrite the insert-mode entry, so Enter in the insert map invoked the vicmd widget. Probe evidence: at dispatch time ZLE reports the active insert map as "main" (emacs/viins resolve through it) and vicmd by name, so the claim scans exactly those two keymaps; a mode-specific widget is never invoked from the other mode. Review round 5 (kongche-jbw P1): delegated dispatch guards against re-entrant accept-line — a directly bound widget may legitimately finish with the NAMED `zle accept-line`, which re-enters the wrapper while $KEYS still matches the claimed key and would re-dispatch the same widget until the nested-function limit. The in-progress flag (cleared by an always block on every exit path) routes the re-entrant call straight to the builtin: the user's widget runs exactly once per submission and the line still submits. Review round 6 (kongche-jbw): the gate now mirrors zsh command-word resolution instead of probing only the filesystem — `whence -w` keeps anything zsh itself resolves (aliases, functions) on the native path, and a first word carrying unquoted glob metacharacters conservatively stays native because the literal token proving missing on disk proves nothing about its expansion. Unlike bash, whose hook fires after resolution already failed, this wrapper runs before zsh parses the line, so the gate must reproduce that resolution order. Delegated dispatch re-entry now reaches the saved accept-line target first (a user accept-line wrapper stays in the A -> B chain) and only a second-level re-entry terminates on the builtin. Review round 7 (kongche-jbw): the gate's raw first word has seen no lexing or expansion — quoting, escaping and brace expansion can all rewrite the word before zsh resolves it, so probing the literal text proves nothing. The round-6 glob veto generalizes into a literal-word whitelist: only a word made purely of non-syntax characters (alphanumerics, `_./-`, multibyte text) may continue; any other ASCII character fails open to the native path. The submit-key claim scan now mirrors saved entries across aliased keymaps (`bindkey -A main vicmd` shares one physical map, so the second scan would otherwise skip the key and silently replace the user's widget on dispatch). Review round 8 (kongche-jbw): an rcfile may create its own keymap, bind the submit keys there, and select it from zle-line-init via `zle -K` — the previous main/vicmd claim enumeration never saw it, so such submissions bypassed the wrapper entirely. zsh's builtin keymap set is closed, so the claim set generalizes to main, vicmd, and every non-builtin keymap reported by `bindkey -l`; the modal builtins stay unclaimed because their submit keys do not accept a command line. The alias mirror now consults every saved table entry instead of only main/vicmd. Review round 9 (kongche-jbw): keymap names may carry spaces, so the claim enumeration splits `bindkey -l` per line instead of by IFS; and the single-pass alias mirror could copy the wrong source when main and vicmd carry different widgets, so the mount is now two-phase — phase 1 captures every keymap's original binding before any physical map is touched (an aliased map reads the true widget of the map it shares), phase 2 rebinds every captured key. The mirror heuristic is removed entirely; table-key splitting is fixed-length from the tail because keymap names may also contain colons. Assisted-by: Qoder:1.22.0 Signed-off-by: SunnyQjm <mfeng@linux.alibaba.com>
97f275c to
026f6e9
Compare
|
Review round 9 addressed in [P1] Preserve whitespace in custom keymap names — valid — the unquoted command substitution split [P1] Mirror aliases from the map they actually share — valid — the round-7/8 single-pass mirror could not know which physical map an alias shares (associative-array iteration order is unspecified), so with distinct Evidence: two new PTY tests —
Container probe additionally confirms phase-1 capture reads
|
|
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.
Review baseline: 8f2416470233...026f6e9fac91
[P1] Preserve the complete widget name from bindkey
src/cosh-ng/crates/cosh-shell/src/shell_host/marker/zsh_marker.sh:788
ZLE accepts widget names containing whitespace. For a direct binding to submit widget,
bindkey emits "^M" submit widget, but this last-field split records only widget.
The following widgets[$widget] lookup then fails, so Enter remains bound directly to the
user widget and never reaches _cosh_accept_line. A slash-bearing natural-language prompt
therefore still ends in the native path error in this valid configuration.
Possible direction: remove only the rendered key prefix, preserve the remaining widget name
exactly, and add a PTY regression with a space-bearing widget name.
[P1] Do not claim Enter in non-submitting custom maps
src/cosh-ng/crates/cosh-shell/src/shell_host/marker/zsh_marker.sh:811
Every non-builtin keymap is treated as a command-submission map, but user keymaps may be
modal. For example, a map selected by zle-line-init can bind Enter to a widget that edits
BUFFER or accepts a selection without calling accept-line. This scan still captures that
widget and phase 2 rebinds Enter to _cosh_accept_line; on a slash-bearing natural-language
buffer, the gate clears and submits the line instead of invoking the modal widget. Excluding
only zsh's builtin modal maps does not preserve equivalent user-defined maps.
Possible direction: claim custom-map bindings only with a verifiable submission contract,
fail open otherwise, and cover a non-submitting custom-map Enter widget in PTY.
|
CI triage —
|
4 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 bashmissing-path verdict (slash-bearing first word, AI enabled, path provably
ENOENT, missing-path classification says natural_language) plus a URL
veto: scheme-shaped first words (
[a-zA-Z]*://*) always keep the nativeresult. The shared
input_intent.shclassifier is reused 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
Twenty-three 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_intercepts_with_builtin_alias_accept_line—zle -Abuiltin-alias customizations survive the mount and native lineskeep executing through the preserved alias (review P2).
shell_host_zsh_missing_path_intercept_never_runs_user_widget_synthesis—an empty-buffer-synthesizing user widget never executes a native command
after a successful intercept (review round 2).
shell_host_zsh_missing_path_foreign_history_hook_stays_uninvoked—a foreign
fc -pzshaddhistory hook is never replayed and the sessionHISTFILE context stays untouched (review round 2).
shell_host_zsh_missing_path_url_first_word_stays_native— URL firstword + NL text keeps the native error (review round 3).
shell_host_zsh_missing_path_intercepts_with_direct_submit_key_binding—bindkey '^M'/'^J'straight to a user widget: interception reachable,pass-through keeps the user widget (review round 3).
shell_host_zsh_missing_path_intercepted_lines_stay_out_of_history—repeated intercepted prompts leave zero history entries (review round 3).
shell_host_zsh_missing_path_intercept_does_not_reevaluate_ps1—PROMPT_SUBST side effects run exactly as often as native submission
(review round 3).
shell_host_zsh_missing_path_submit_key_widgets_stay_per_keymap—distinct submit widgets in the main and vicmd keymaps never cross modes
(review round 4).
shell_host_zsh_missing_path_reentrant_accept_line_runs_widget_once—a delegated widget finishing with the named
zle accept-linerunsexactly once and the line still submits (review round 5).
shell_host_zsh_missing_path_slash_function_stays_native— a definedslash-bearing function executes natively; the gate mirrors zsh
resolution via
whence -w(review round 6).shell_host_zsh_missing_path_han_glob_stays_native— a Han first wordcarrying an unquoted glob expands and runs natively (review round 6).
shell_host_zsh_missing_path_reentrant_reaches_saved_accept_line— are-entrant named accept-line reaches the saved user wrapper, keeping
the A → B chain intact (review round 6).
shell_host_zsh_missing_path_quoted_function_stays_native— a quotedslash-bearing function call runs natively; the literal-word whitelist
fails open on any lexing/expansion form (review round 7).
shell_host_zsh_missing_path_brace_expansion_stays_native— abrace-expanded first word runs natively (review round 7).
shell_host_zsh_missing_path_aliased_keymap_keeps_user_widget— withbindkey -A main vicmdthe claim scan mirrors saved entries so avicmd submission still reaches the user's widget (review round 7).
shell_host_zsh_missing_path_custom_keymap_still_intercepts— auser-defined keymap selected from zle-line-init is claimed at mount;
interception fires and the user's widget stays (review round 8).
shell_host_zsh_missing_path_space_keymap_still_intercepts— aspace-bearing keymap name is enumerated per line and claimed
(review round 9).
shell_host_zsh_missing_path_alias_mirrors_shared_map_widget— thetwo-phase mount captures bindings before any rebind, so an alias of
main dispatches main's widget, never vicmd's (review round 9).
Verification
Focused (alinux3 arm64 container, zsh 5.5.1):
cargo test -p cosh-shell --test shell_host— new cases green; 53 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