Skip to content

fix(cosh-ng): forward shell cwd to registry mode - #2686

Open
KaiLongZhou wants to merge 1 commit into
alibaba:mainfrom
KaiLongZhou:fix/cosh-ng/registry-workspace
Open

fix(cosh-ng): forward shell cwd to registry mode#2686
KaiLongZhou wants to merge 1 commit into
alibaba:mainfrom
KaiLongZhou:fix/cosh-ng/registry-workspace

Conversation

@KaiLongZhou

Copy link
Copy Markdown
Collaborator

Why

When cosh-core is not yet running in headless mode, slash commands such as /skills list fall back to registry_query_short, which spawns a one-shot cosh-core --registry process. Previously this path did not pass --workspace, so cosh-core fell back to the cosh-shell process cwd. When the process cwd differs from the user's shell cwd (e.g. daemon-style startup or cd after launch), project-level skills under <workspace>/.copilot-shell/skills/ were not discovered.

What changed

  • Added a shell_cwd field to CoshCoreAdapter to track the latest shell working directory reported by OSC 1337.
  • Updated render_slash_command to synchronize shell_cwd before dispatching any slash command.
  • Modified registry_query_short to forward shell_cwd as --workspace to the registry-mode cosh-core process, falling back to the active session's workspace scope when unavailable.

Related issue

no-issue: internal bug report

User / Agent impact

/skills list, /hooks, /extensions and other slash registry queries now correctly discover project-level resources even when no Agent turn has been run yet.

Risk and compatibility

Low risk. The change only affects the registry_query_short fallback path. The live IPC registry path and the headless prepare_invocation path are unchanged.

Validation

  • cargo check --workspace
  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets --locked -- -D warnings
  • cargo test --workspace (888 passed, 1 pre-existing macOS-specific failure unrelated)
  • cargo test -p cosh-shell (5 pre-existing macOS hook environment failures unrelated)

Documentation and rollback

No documentation changes. Rollback: revert commit 72be99f97 or checkout the previous main state.

registry_query_short spawns a one-shot cosh-core --registry
process without forwarding --workspace, causing workspace_root()
to fall back to the cosh-shell process cwd. When the process cwd
differs from the user's shell cwd (e.g. daemon-style startup or
cd after launch), project-level skills under
<workspace>/.copilot-shell/skills/ are not discovered.

Add a shell_cwd field to CoshCoreAdapter, set by
render_slash_command from the OSC 1337 shell cwd.
registry_query_short forwards it as --workspace so the
registry-mode cosh-core resolves the same project root as the
headless runtime. Falls back to active_workspace_scope() when
shell cwd is unavailable.

Only affects the registry_query_short path;
live_registry_query (IPC) and headless prepare_invocation are
unaffected.

Assisted-by: Qoder:1.22.0
Signed-off-by: Kailong Zhou <zhoukailong.zkl@alibaba-inc.com>
@KaiLongZhou
KaiLongZhou requested a review from SunnyQjm as a code owner August 20, 2026 02:22
@github-actions github-actions Bot added the component:cosh-ng src/cosh-ng label Aug 20, 2026

@qoderai qoderai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

本次审查范围内未发现需要修改的问题。


🤖 Generated by QoderView workflow run

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 72be99f977

ℹ️ 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".

// resolves the same project root as the headless runtime. Falls
// back to the active session's workspace scope when the shell
// cwd is unavailable (e.g. programmatic callers without a PTY).
let workspace = self

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add the mandatory Fixes attribution

This standalone fix is based on a commit already contained in main, but its commit body has no Fixes: <hash> ("<subject>") entry. Add the commit that introduced the incorrect registry workspace behavior so the fix complies with the repository's required history attribution.

AGENTS.md reference: AGENTS.md:L378-L384

Useful? React with 👍 / 👎.

Comment on lines +40 to +42
/// Shell cwd tracked from the PTY host so that short-lived `--registry`
/// processes resolve the same project root as the headless runtime.
pub(crate) shell_cwd: Arc<Mutex<Option<String>>>,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Include the cosh-shell crate scope in the subject

The subject fix(cosh-ng): forward shell cwd to registry mode omits the mandatory crate scope for a change confined to cosh-shell. Rewrite it with [shell] while keeping the complete subject under 50 characters so it follows the component's commit format.

AGENTS.md reference: src/cosh-ng/AGENTS.md:L132-L139

Useful? React with 👍 / 👎.

@SunnyQjm

SunnyQjm commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

PR number: #2686
head_sha: 72be99f
reviewed_at: 2026-08-20T02:55:12Z

评审结论

Findings

未发现 blocking package/module/public API 组织问题。

  • [P3] src/cosh-ng/crates/cosh-shell/src/slash/commands.rs:33 —— render_slash_command 在每次 slash 分发前无条件调用 set_shell_cwd(shell_cwd)。当某次调用传入 None(无 PTY 上报的程序化路径)时,会清空此前已同步的真实 shell cwd,随后 registry_query_short 静默退回 session workspace scope(adapter/cosh_core_registry.rs:110or_else 兜底),而非最近一次真实 cwd。若存在"先 SomeNone"的混合调用序列,registry 解析的项目根会发生漂移。建议仅在 Some 时更新,或由作者确认所有 caller 的时序不会出现该序列。
  • [P3] diff 中未见针对新 workspace 解析优先级(shell_cwd > session scope > 进程 cwd)的测试。--workspace 转发是本 PR 的核心行为变化,建议补一条 focused 测试钉住优先级与回退路径,或在 PR 中说明不补的理由。

结构合规检查(对照 code organization 规则)

  • Owner 归属:变更落在 adapter/(registry 进程装配属 provider process driver 职责)与 slash/(命令分发入口),符合 standard.md §2;--workspace 参数装配未越界写 journal/activity/shell_host。
  • 跨 owner 说明:patch 同时改 adapterslash 两个 owner,PR body 已说明配套契约(slash 分发时同步 cwd 供 adapter registry 查询使用),属同一能力的最小配套,可接受。
  • Public API:新增字段 shell_cwd 与方法 set_shell_cwd 均为 pub(crate),未触碰 lib.rs,未新增 pub mod/pub use,inventory 分类计数不受影响。CoshCoreAdapter 已有 pub(crate) 字段,外部字面量构造本就不可能,无兼容性破坏。
  • Root src / self-crate path:未新增 root implementation 文件,未引入 cosh_shell::... 自引用路径。
  • 依赖方向slash -> adapter 为既有方向(render_slash_command 原签名即接收 &AdapterInstance),未命中 forbidden dependency 清单;两处 Mutex 顺序获取、未嵌套持锁,无新死锁路径。
  • 大文件:三个被改文件均未登记在 large-file-inventory;按 diff 增量估算(各净增约 10–25 行)不越 1000 行增长线,精确行数以 CI gate 实测为准。

Open Questions

  • shell_cwd 与 session workspace scope 的优先级语义目前仅服务 slash registry 路径;若 registry_query_short 存在不经过 render_slash_command 的其他调用入口(如启动期查询),这些入口始终走 session scope 兜底,请作者确认覆盖完整性。
  • Codex bot 提出的 commit message 规范意见([shell] scope、Fixes: attribution)属仓库提交约定,不在本结构评审范围内,留待作者按 AGENTS.md 处理。

Validation

  • Passed:结构规则静态比对(owner、public surface、依赖方向、root 文件、跨 owner 说明)。
  • Not run:cargo check/test/clippycheck-layout.shinventory-public-api.sh(本评审不执行工具;PR body 报告 workspace 测试通过,CI Test cosh-ng 与 fast checks 均已 SUCCESS)。

剩余风险:shell_cwd 字段的长期归属(adapter 持有 shell 会话状态)目前只被本 PR 单侧定义,若后续 headless runtime 或其他入口接入同一字段,需在 adapter owner 内复核一致性。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants