fix(cosh-ng): keep debug trap on bash 4.2 prompts - #2757
Merged
Conversation
bash 4.x suspends the DEBUG trap while PROMPT_COMMAND runs; the snapshot logic misread the suspended empty output as a user-cleared trap and permanently dropped it after the first command; skip the snapshot on bash < 5 where suspension guarantees the trap is untouched; bash 5.x path unchanged. Closes alibaba#2736 Fixes: 0ad6b4b Assisted-by: Qoder Signed-off-by: Jiangtian Feng <jiangtianf97@163.com>
There was a problem hiding this comment.
- bash 4.2 行为假设合理,但建议在注释中更精确地限定适用版本,减少未来兼容扩展时的误用风险。
- 版本门逻辑本身清晰,未发现功能或安全层面的回归风险。
- 当前修改范围有限,未在审查范围内发现其它需要调整的问题。
🤖 Generated by Qoder • View workflow run
kongche-jbw
self-requested a review
August 22, 2026 09:06
kongche-jbw
reviewed
Aug 22, 2026
kongche-jbw
left a comment
Collaborator
There was a problem hiding this comment.
Review baseline: a4cb23ea27c9645d7294b7afe65309262e6a0e4b...732502d2f4405431f7aca57a8733b70730683f66
未发现阻塞问题。
Remaining risks:
- 精确的 bash 4.2.46 前后对照仍依赖 PR 中报告的本地验证。
Verification scope:
- 复查了完整 diff、相邻 DEBUG/PROMPT_COMMAND 状态机、PTY 测试及既有 review thread。
- 使用源码构建的 bash 4.2.0 与系统 bash 5.2.21 分别运行两项相关 PTY 测试,均通过。
- 当前 cosh-ng CI、release build、fast checks、PR lint 与 CLA 均通过。
kongche-jbw
approved these changes
Aug 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
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.
Why
On bash 4.2.x (e.g. Alibaba Group Enterprise Linux 7 line) the bash marker script permanently loses its DEBUG trap after the first prompt whenever PROMPT_COMMAND is set, so no
preexecOSC markers are emitted from the second command on — silently losing the command audit trail. Closes #2736.Mechanism: bash 4.x suspends the DEBUG trap while PROMPT_COMMAND runs, so
trap -p DEBUGprints nothing inside the hook. The snapshot block in_cosh_prompt_commandread that empty output and cleared_COSH_ACTIVE_DEBUG_TRAP; the subsequenteval "$active_debug_trap"in the preexec handler then re-installed nothing, and the freshly restored trap was permanently dropped.What changed
cosh-shell/src/shell_host/marker/bash.rs(_cosh_prompt_command): the trap-snapshot block is now gated on(( BASH_VERSINFO[0] >= 5 )). On bash < 5 the snapshot is skipped entirely — suspension guarantees the trap cannot change while the hook runs, so the snapshot is meaningless there and its empty read is actively harmful. The bash 5.x execution path is byte-identical (the gate only short-circuits the 4.x branch); the snapshot block body itself is unchanged.Related issue
closes #2736
User / Agent impact
On bash 4.2 hosts with PROMPT_COMMAND set (e.g. the AGL7 audit-hook default), command-level OSC markers, command blocks, and exit codes are captured again from the very first command instead of silently disappearing after the first prompt. bash >= 5 hosts see no change.
Risk and compatibility
Low risk: the change is a version gate in front of one snapshot block; on bash >= 5 the gate is always true. One trade-off: on bash < 5, a user hook that itself installs a new DEBUG trap would no longer be re-snapshotted — but bash < 5 suspends the trap during the hook anyway, so any trap swap could only take effect after the hook returns, which is exactly the window the preexec re-arm path already owns.
Validation
cargo test -p cosh-shell --test shell_host shell_host_runs_bash_pty_and_emits_command_eventsFAILED (assertion failed: ledger.blocks.iter().any(...)atmarker.rs:73— no command block, matching fix(cosh-ng): bash marker permanently loses DEBUG trap after first prompt when PROMPT_COMMAND is set (bash 4.2) #2736)RUSTUP_TOOLCHAIN=1.89.0):cargo test -p cosh-shell --lib— 1347 passed / 0 failed;cargo test -p cosh-shell --test shell_host(full suite) — 160 passed / 0 failed / 1 ignored;cargo fmt --all -- --check— clean;cargo clippy -p cosh-shell --all-targets -- -D warnings— exit 0Documentation and rollback
None. Revert the single commit to restore the previous behavior on bash 4.2 (bash 5.x behavior is identical either way).