fix(cosh-ng): [shell] default to native mode - #2759
Conversation
github-actions
Bot
added
component:cosh-ng
![qoderai[bot]](https://avatars.githubusercontent.com/in/1396158?s=60&v=4){kind=small}
There was a problem hiding this comment.
[P1] 原生 / 增强集成在 raw_shell 入口上仍有混合路径：run_raw 默认 native_mode=true，且在读取配置前就调用 bootstrap_process_path_from_shell，当配置为 enhanced 时会落入“增强集成 + 原生启动路径”的组合，可能重新暴露 marker 行为。
[P2] 原生集成依赖 marker 缺失来避免 AI 拦截，但配置层仍允许 analysis_mode 和 ai_enabled 处于开启状态，存在“配置开启但行为静默禁用”的认知偏差。
[P2] shell.integration 和 COSH_SHELL_INTEGRATION 对非法值完全静默回退到 native，易导致配置写错时增强模式无法开启且难以排查，建议增加最小可见校验或日志。
🤖 Generated by Qoder • View workflow run
| impl ShellIntegration { | ||
| pub(crate) fn from_config(value: &str) -> Self { |
There was a problem hiding this comment.
[P2] COSH_SHELL_INTEGRATION 环境覆盖与配置含义可能混淆
CoshConfig::default 将 shell_integration 设为 "native"，apply_shell_integration_environment_override 会直接把 COSH_SHELL_INTEGRATION 的原始字符串写回配置，ShellIntegration::from_config 则仅在严格等于 "enhanced" 时切换为增强，否则都视为 Native。这意味着用户传入 COSH_SHELL_INTEGRATION=Native、native （带空格）或其他大小写/拼写变体时不会报错，只是静默回退到原生；同时无论是配置文件还是环境变量，只有 "enhanced" 一档，缺少显式的非法值反馈。
从运维角度看，配置/环境写错时很难排查“为什么始终没有增强集成”。可以考虑在解析时增加简单校验：当 value 既不是 "native" 也不是 "enhanced" 时，在 stderr 打一条提示或者在日志中记录一条配置警告，并在未来需要时扩展为 Result 返回，让 CLI 能够在明显错误配置时显式失败而不是悄然降级。
🤖 Generated by Qoder • Fix in Qoder
There was a problem hiding this comment.
Fixed in 4aa84f61. Parsing already tolerated case and surrounding whitespace, but other invalid values did silently fall back. The parser now accepts only native or enhanced; invalid config or environment input exits with status 2 and an actionable stderr message. Added an exact raw CLI regression test.
![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: bad3096a60
ℹ️ 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".
| config.login_shell = login; | ||
| if config.native_mode { | ||
| let cosh_config = load_config(); | ||
| config.integration = ShellIntegration::from_config(&cosh_config.shell_integration); |
There was a problem hiding this comment.
When cosh --resume or cosh --resume <id> is launched without an explicit integration override, this selects Native but still only queues the request in pending_launch. render_session_launch consumes that request exclusively after a ShellReady event, while the new passthrough parser never emits ShellReady, so the explicit resume flag silently drops the user into an ordinary shell without opening or restoring the requested session. The launch path should either select Enhanced for resume requests or reject the incompatible combination visibly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4aa84f61. --resume is an explicit Agent action, so it now selects Enhanced integration even when Native is configured by default. This restores the ShellReady boundary before consuming pending_launch. The existing value and picker integration test now removes the integration override and covers both default-Native launch paths.
| if config.native_mode && config.integration.uses_markers() { | ||
| bootstrap_process_path_from_shell(&shell_kind, login); | ||
| } |
There was a problem hiding this comment.
With the new default Native integration, the parser cannot produce the ShellReady event required by render_startup_banner, but run_raw still starts the recommendation-store worker, startup health scan, shell-rewrite worker, and—under the default AI-enabled, cosh-core, TTY configuration—the ai_configured subprocess used only by that banner. Consequently every ordinary default shell performs database, diagnostic, and subprocess work whose results can never be displayed or consumed; these Enhanced-only workers should not be started for Native sessions.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4aa84f61. Native sessions no longer start the recommendation store/history observer, shell-rewrite worker, startup health scan, analyzer cancellation, or startup auth probe. Enhanced sessions keep the existing behavior. A raw CLI regression verifies that default Native neither invokes cosh-core for startup auth nor creates the recommendation store.
07989da to
4c7ecd1
Compare
Make shell ownership the default integration policy. Native sessions load user startup files directly. They bypass marker generation, OSC routing, and input interception. Enhanced remains an explicit opt-in while hook isolation is redesigned. Fixes: 9ecda4b ("feat(shell): add cosh-shell crate with Phase 0 fixes") Signed-off-by: kongche-jbw <kongche.jbw@alibaba-inc.com>
Give every card an explicit identity and stable symbol. Do not infer routing or permission behavior from visible text. Permission cards bind to structured runtime requests. Direct execution remains an argv-only contract. Its executor and native-session Agent panel remain future work. Signed-off-by: kongche-jbw <kongche.jbw@alibaba-inc.com>
4c7ecd1 to
bbc0f15
Compare
|
Review summary triage:
|
1 participant
Why
The current marker injection forces Bash tracing options and exposes Cosh traps,
internal helpers, and session tokens to user-visible shell state. This change
makes transparent shell ownership the default and keeps the legacy marker path
behind an explicit enhanced setting.
What changed
ShellIntegration::NativeandShellIntegration::Enhanced.traps, OSC routing, session tokens, or input interception.
shell.integration = "enhanced"andCOSH_SHELL_INTEGRATION=enhancedas explicit opt-ins.hello,/,??, and all other Native input bytes directly to thechild PTY.
$,▶,◆,/,*,!, and·card identities. Permissioncards can only originate from structured tool permission events.
documentation.
Related issue
closes #2687
User / Agent impact
coshnow behaves like the selected native shell by default. Implicit naturallanguage, slash-command routing, command-boundary events, and Agent handoff
require Enhanced integration. Existing enhanced behavior remains available,
and visible UI cards now identify their typed origin with stable symbols.
Risk and compatibility
The default interaction policy changes intentionally. Users who need the legacy
workflow can set
shell.integration = "enhanced"or launch withCOSH_SHELL_INTEGRATION=enhanced. Enhanced still uses the existing global hookimplementation, so its xtrace and helper-isolation work remains tracked by
#2683. Same-child-shell hot switching, a Native Agent panel, and the Direct Exec
executor are not included.
Validation
Final local revision:
bbc0f155a6856ff17039438f9df92d06812cf921.cargo fmt --all -- --checkcrates/cosh-shell/scripts/check-layout.shscripts/check-test-inventory.sh— lib/bin overlap 696/696cargo clippy --package cosh-shell --lib --tests -- -D warningscargo test --package cosh-shell --lib— 1353 passedcargo test --package cosh-shell --test shell_host -- --test-threads=4— 164 passedcargo test --package cosh-shell --test raw_cli renderer:: -- --test-threads=4— 15 passedcargo test --package cosh-shell --test raw_cli splits_agent_cards -- --test-threads=4— 2 passedcargo doc --package cosh-shell --no-depspython3 scripts/docs-link-check.pyThe PTY regressions cover Bash options and traps, user
PROMPT_COMMAND, markerand token absence, xtrace, Native bash/zsh startup, Native handling of
hello,/, and??, and explicit Enhanced marker routing. The CI-failure regressionscover typed card titles, split Agent cards, provider cancellation, startup
banners, and the lib/bin duplicate-test ceiling.
Documentation and rollback
English and Chinese component READMEs, quickstarts, shell guides,
configuration references, and the integration/card design are updated. Roll
back the default behavior per session with
COSH_SHELL_INTEGRATION=enhanced cosh, or persistshell.integration = "enhanced"in the user configuration.