fix(cosh-ng): host injection save-compose-restore contract (#2598 U2) - #2632
fix(cosh-ng): host injection save-compose-restore contract (#2598 U2)#2632SunnyQjm wants to merge 4 commits into
Conversation
|
PR number: #2632 Findings未发现 blocking package/module/public API 组织问题。以下为非阻断项:
结构合规确认
Open Questions
Validation
|
736b115 to
c83f0eb
Compare
|
PR number: #2632 Findings未发现 blocking package/module/public API 组织问题。以下为非阻断项(与上一 head 的评审一致,本轮 patch 中仍存在):
结构合规确认
Open Questions
Validation
|
c83f0eb to
a9d6f08
Compare
|
Review round 1 disposition (head moved to the current SHA):
|
|
PR number: #2632 Findings未发现 blocking package/module/public API 组织问题。此前轮次的两条 P3( 结构合规确认
Open Questions
Validation
|
a9d6f08 to
7a8023f
Compare
|
CI follow-up (head Fix (fixed up into the test commit): the drain thread is now deliberately detached with the rationale documented inline, and the test drops both pty fds explicitly after the assertions. Verified on Linux (alinux3 arm64 container, real binary): |
|
PR number: #2632 Findings未发现 blocking package/module/public API 组织问题。本轮 head 与上一轮已评审 head 相同、patch 无变化;此前两条 P3( 结构合规确认
Open Questions
Validation
|
|
CI round-2 disposition (head moved to the current SHA): the
|
7a8023f to
f1a0117
Compare
There was a problem hiding this comment.
- [P1] termios 生命周期测试对 bash 存在隐式前置依赖,在极简 Linux 环境下可能整体被静默跳过,削弱恢复合同的回归覆盖。
- [P2] NS-009 回归测试目前只校验 jobs 输出的并发 job number,未覆盖
$!的作业号语义,存在残余差异未被捕获的空间。 - [P2] termios 恢复轮询的 5 秒固定上限在资源紧张或噪声较大的 CI 环境中可能产生一次性假阴性,建议提供更弹性的恢复窗口或非致命降级路径。
🤖 Generated by Qoder • View workflow run
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1a0117e86
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
PR number: #2632 Findings
结构维度:未发现 blocking package/module/public API 组织问题。本 head 与 10:32 已评审 head 完全相同、patch 无增量,上述为未闭环残留与本轮新增静态推断。 结构合规确认
Open Questions
Validation
|
9250783 to
c65e4ff
Compare
|
PR number: #2632 Findings
结构维度:未发现 blocking package/module/public API 组织问题。 结构合规确认
Open Questions
Validation
|
c65e4ff to
edee581
Compare
|
PR number: #2632 Findings
结构维度:未发现 blocking package/module/public API 组织问题。本 head 与 13:26 已评审 head 相同、patch 无增量;本轮触发为 BryanHeBY 的新人工评审意见(02:34),已逐条静态复核并并入上述 Findings,其余为前轮未闭环残留项的复核确认。 结构合规确认
Open Questions
Validation
|
审阅 @
|
edee581 to
d0c8593
Compare
Round-4 disposition (head d0c8593)All five findings from the 2026-08-18T13:44 auto-review are closed: [P2] prompt-frame tail [P3] comment placement — adopted; the Linux-only note now sits directly above the [P3] nested-bash test never entering the read loop — adopted; the regression now pipes the script through stdin to [P3] nested × errexit corner — acknowledged as the recorded INV-2 trade-off; not expanded here. [P3] Also rebased onto latest main (9 commits) before push; full preflight green (fmt/clippy/layout/test-inventory), Linux container suite 65/65. |
|
PR number: #2632 Findings
结构维度:未发现 blocking package/module/public API 组织问题。本 head 与上一轮已评审 head 相同、patch 无增量,上述为未闭环残留项的静态复核确认。 结构合规确认
Open Questions
Validation
|
d0c8593 to
ae90a91
Compare
人工评审处置(@BryanHeBY,head 移至 ae90a91)三条全部核实成立并处置;另致歉:这条评论发出时间在我上一轮机器评审处置窗口之前 26 分钟,活动过滤窗口把它漏掉了一轮,本轮补上。 P1 — SEM-019 退出码轴:采纳,新增按原文 payload 的主轴回归您的判读完全正确:既有
Shipping target(Linux 容器,真实 PTY)实测:断言通过——即 M2 守卫修复真实钉住了该轴,不再只是终端内容侧写。 P2 — PS2:机制已探针确证,显式摘出本 PR 验收范围按伞单「先确证机制再写码」条款,先做了真实 PTY A/B 探针(容器, P2 — exported guard value 可观测偏差:采纳,已显式声明同意您的定性:INV-2 回归钉的是「无害」而非「等价」, 另谢三处正面确认(flags token 假阳性排除、NS-009 reap 窗口说明、INV-1 反向格披露)——这三处的写法会保持。 |
|
PR number: #2632 Findings
结构维度:未发现 blocking package/module/public API 组织问题。本 head 与上一轮已评审 head 相同、patch 无增量;本轮变化来自状态检查推进(CI 已转绿),上述 P3 为残留项复核确认。 结构合规确认
前轮闭环确认
Open Questions
Validation
|
The intent classifier (_cosh_classify_missing) runs inside $(...) substitutions whose subshells inherit the user's errexit. The bare _cosh_utf8_han_status calls return 1 for plain-ASCII input by design, which aborts the subshell before printf under `set -e` and drifts the classification. Guarded conditional-context calls keep the 0/1/2 tri-state intact (#2598 M2 sweep, umbrella for #2541 sub-item B). Assisted-by: Qoder:1.22.0 Signed-off-by: SunnyQjm <mfeng@linux.alibaba.com>
Converge the four injection-layer contract breaks tracked by umbrella issue #2598 (save-compose-restore semantics, one design): - M2 dispatch-chain failure isolation (SEM-019, #2541-B): guard the bare _cosh_utf8_han_status call in _cosh_begin_attempt and the _cosh_token_fingerprint substitution in command_not_found_handle so internal enum statuses never leak into a user `set -e` session. - M3 PROMPT_COMMAND attribute fidelity (NS-005, Fixes #2540): record the declare -p flags token before the wholesale replacement, restore the export attribute afterwards, and switch the hijack value to a guard form that stays a silent no-op in nested shells. The guard value stages $? into _COSH_PROMPT_STATUS because `declare -F` would otherwise clobber the user's status before the prompt chain reads it. - M1 frame-level errexit protection (SEM-019): wrap the preexec and prompt frames to suspend errexit on entry; the preexec veto path defers restoration to the next frame entry (2541-D4), and the prompt wrapper returns 0 under errexit because a non-zero PROMPT_COMMAND list status kills an interactive session (probed on bash 3.2/5.2; bash itself restores the user's $? at the prompt boundary). - M4 line-execution trap exit (NS-009, Fixes #2539): the first DEBUG firing of a line finishes the preexec work and leaves the trap disarmed for the rest of the line - per-command trap execution is what opens bash's mid-line job reap window that serializes background jobs. The prompt boundary re-arms via an ownership matrix (user-cleared / dormant / combined user trap / foreign trap), and _cosh_now_ms drops the external `date` exec (fork hygiene). Container FAIL->PASS evidence (Apple container, anolisos:23.5 + real binaries on alinux3): SEM-019 session exit 1->0 with payload parity, NS-009 10/10 concurrent job numbers under the unanchored classifier, NS-005 declare -a -> declare -ax with a real cosh-shell/cosh-core run. Design: specs/cosh-2598-host-injection-contract/ (G1 confirmed 2026-08-17). Refs #2598, #2541 Fixes #2540 Fixes #2539 Assisted-by: Qoder:1.22.0 Signed-off-by: SunnyQjm <mfeng@linux.alibaba.com>
- marker.rs: NS-005 export-attribute cells (array/scalar, nested-bash silence), SEM-019 errexit dispatch survival, NS-009 concurrent job numbers, user DEBUG trap chain parity. The unexported reverse cell is documented as container-acceptance scope (the harness always injects an exported PROMPT_COMMAND). - raw_cli/termios_lifecycle.rs (Linux-only): termios roundtrip over exit/EOF/SIGTERM with a real cosh-shell in a pty (#2598 M5 anchor, #2537 RC-4). macOS libtest intermittently reports ENOTTY on a fresh pty slave, so the module is gated to the shipping target. Refs #2598, #2537 Assisted-by: Qoder:1.22.0 Signed-off-by: SunnyQjm <mfeng@linux.alibaba.com>
The #2598 host-injection changes pushed marker/bash.rs past the 1000-line blocking bar in the large-file inventory. Execute the split plan already recorded there: move the embedded script body into marker/bash_marker.sh loaded via include_str! (the same asset pattern as shell_host/input_intent.sh). include_str! preserves the exact bytes of the former raw-string literal; the OSC golden tests and the marker PTY suite pin the emitted protocol. Inventory row updated in the same change. The routing_c4 registry test parses the marker source text for the authoritative slash case list; its include_str! pointer follows the script body to the new asset (caught by CI fast checks). Refs #2598 Assisted-by: Qoder:1.22.0 Signed-off-by: SunnyQjm <mfeng@linux.alibaba.com>
Round-6 disposition (head bc84468)Single remaining [P3] from the 2026-08-19T09:52 auto-review — adopted, verified on the shipping target. Your double-decoding trace was exact: the outer printf format was All other items in your review are closure confirmations; the two open questions ( |
ae90a91 to
bc84468
Compare
|
PR number: #2632 Findings本 head 与上一轮(03:20 已评审)相同、patch 无增量;本轮触发为 kongche-jbw 的新人工评审(03:41),三条发现已逐条对照 patch 静态复核,均成立:
结构维度:未发现 blocking package/module/public API 组织问题。 结构合规确认
Open Questions
Validation
|
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: f94cf16d1f68...bc84468e6d24
[P1] PROMPT_COMMAND 重赋值后 marker 链会失活
src/cosh-ng/crates/cosh-shell/src/shell_host/marker/bash_marker_dispatch.sh:238
把 DEBUG trap 留在撤防状态,并依赖 _cosh_prompt_command 在 prompt 边界重挂。
执行本 PR 新增用例里的 PROMPT_COMMAND=(...) 或标量赋值时,赋值会先替换
bash_marker_frames.sh:189 的 guard,随后 prompt 不再调用 cosh wrapper,
所以 _COSH_AT_PROMPT 和 trap 都无法恢复,之后不再产生 preexec/precmd/intercept。
现有属性测试仍会通过,因为 scripted.rs 的 send_command_line 丢弃了
read_until 返回的 false;本地单跑这三个用例实际累计等待了 20.35 秒超时。
Possible direction: 让边界恢复不依赖可被用户覆盖的 PROMPT_COMMAND,并在数组、
标量重赋值后再执行一条命令,断言新一代 preexec/precmd/intercept 事件按时到达。
[P2] termios 往返断言遗漏控制字符状态
src/cosh-ng/crates/cosh-shell/tests/shell_host/termios_lifecycle.rs:73
只快照四个 flag 字段,但生产路径的 cfmakeraw 还会改写 c_cc[VMIN] 和
c_cc[VTIME]。若退出路径只恢复 flags、遗留 raw 读取参数,exit/EOF/SIGTERM
三条测试仍会全部通过,与“恢复原值”和注释中的“partial restore cannot pass”不符。
Possible direction: 启动前设置非默认 VMIN/VTIME,并比较完整 c_cc;同时覆盖
该平台可观察的其余 termios 字段,证明三种退出路径确实恢复整份快照。
[P2] classifier 的 errexit 修复没有经过目标调用链
src/cosh-ng/crates/cosh-shell/src/shell_host/input_intent.sh:347
修复的是 set -e 继承到命令替换后,ASCII helper 返回 1 导致分类提前退出;
但新增 errexit 用例只执行已有命令,不会进入 _cosh_classify_missing,现有
input_intent 用例也未启用 inherit_errexit。因此这两处 guard 回退后测试仍会绿,
ASCII 自然语言缺失命令会重新降级到 native command-not-found 或终止会话。
Possible direction: 通过真实 marker 会话启用 inherit_errexit 与 set -e,输入
缺失的 ASCII 自然语言命令,断言发生 natural-language intercept、会话存活且退出码为 0。
Summary
Umbrella #2598 (U2, host-injection non-destruction contract): converge four injection-layer contract breaks with one save-compose-restore design, instead of per-issue point fixes. Design record:
specs/cosh-2598-host-injection-contract/in the dev workspace (G1 confirmed 2026-08-17); key invariants and matrices are also carried by code comments, test names and commit bodies in this PR.Fixes #2540 (NS-005), Fixes #2539 (NS-009); Refs #2598, #2541 (sub-item B / SEM-019), #2537 (termios anchor M5).
Changes
_cosh_utf8_han_statusreturns 1 for plain ASCII by design; its bare calls in_cosh_begin_attemptand_cosh_classify_missingleaked an internal "failure" into every dispatch, killing userset -esessions and polluting the session exit code. All enum-status helper calls are now guarded conditional-context calls (tri-state preserved);_cosh_token_fingerprintincommand_not_found_handledegrades to the native-delegate path on failure.declare -pflags token before the wholesale hijack, restore-xafterwards; the hijack value is now a guard form (declare -F ... && ...) that stays a silent no-op when the exported copy leaks into a nested bash, and stages$?into_COSH_PROMPT_STATUSsodeclare -Fcannot clobber the user's status before the prompt chain reads it (ledger exit codes depend on this).$?at the prompt boundary — also probed on both).[1]slot reuse). The prompt boundary re-arms through an ownership matrix: user-cleared / dormant / combined user trap (kept as-is, path generation stays fails-closed) / foreign trap (absorbed into the OLD chain)._cosh_now_msdrops the externaldateexec (fork hygiene, with a bash <4.2 fallback).cosh-shellbinary; SIGKILL is an explicit non-goal (physically uninterceptable).marker/bash_marker.shviainclude_str!(byte-identical; same asset pattern asinput_intent.sh), inventory row updated.Tests
tests/shell_host/marker.rs: +6 regressions — exported PROMPT_COMMAND array/scalar reassignment keeps-x, nested-bash guard-value silence,set -esession survives dispatch with$?/$-fidelity, background jobs keep concurrent job numbers, user DEBUG trap installed mid-session keeps firing.tests/raw_cli/termios_lifecycle.rs(Linux-only): termios roundtrip over exit/EOF/SIGTERM. macOS libtest intermittently reports ENOTTY on a fresh pty slave (stable in a standalone process), so the module is gated to the shipping target.Verification
Focused (green, macOS host unless noted):
cargo test -p cosh-shell --test shell_host -- marker::57/57;--lib -- oscgolden 36/36 (byte identity across the asset split); new regressions 6/6.cargo clippy --workspace -- -D warnings;cargo fmt --check;check-layout.sh;check-test-inventory.sh— all green (cosh-lab preflightdata.ok=true).container):cosh-shell raw cosh-corebuild of this branch):declare -a→declare -ax, driver exit 1→0, oracle parity.Excluded scope (not run here, as-is):
tests/shell_hostfull suite on macOS:relay::routing_c3shows pre-existing flaky/hang behavior under full parallel runs on this host (bare-base comparison: base fails/flakes the same or worse);hooks::enginelib/bins failures reproduce on bare base (pre-existing, macOS-only). Linux CI is the authoritative gate.Known deviations (recorded, not regressions):
env | grep PROMPT_COMMANDshows cosh's guard string in a cosh session vs the user's own value in bare bash. A single exported name cannot both carry the user's value and cosh's hook, so this is an inherent cost of the hijack mechanism (2540 INV-2 trade-off); INV-2 regressions pin it harmless (silent in nested bash), not equivalent. Declared as a non-goal under the [Umbrella][P0][cosh-ng] 宿主注入非破坏合同:PS1/PS2/PROMPT_COMMAND/trap/termios 保存-组合-恢复语义 #2598 adjudication wording.$PS2= env value on both sides). The>default seen in the [P1][cosh-ng] exported 变量数组再赋值丢失 -x 属性(NS-005) #2540 evidence therefore does not originate from this injection layer; its mechanism (suspected host-side env construction) stays unconfirmed and is tracked in [P1][cosh-ng] exported 变量数组再赋值丢失 -x 属性(NS-005) #2540, per the umbrella's "confirm mechanism before coding" rule. "Fixes [P1][cosh-ng] exported 变量数组再赋值丢失 -x 属性(NS-005) #2540" here covers the-xattribute-fidelity half (NS-005 as measured); if the maintainer prefers, it can be downgraded to Refs until the PS2 half lands.Evidence
Assets on fork orphan branch
pr-2632-assets(commit-pinned):Baseline & mechanism-gate reports: BASELINE.md · T1-REPORT.md
NS-005 real-binary FAIL→PASS: fail verdict → pass verdict · oracle verdict · cast · replay GIF & final frame:
NS-009 FAIL→PASS (same v2 classifier on both sides): fail summary-v2 (9/10 SERIALIZED) → pass summary-v2 (10/10 CONCURRENT) · pass transcript · v1 fail summary
SEM-019 PASS: summary (candidate session exit 0, payload parity) · transcript
errexit×veto×prompt parity probe: vb summary