feat: support Codex CLI runtime - #2
Conversation
coppynight
left a comment
There was a problem hiding this comment.
感谢这个 PR — 整体设计与现有 CLIAdapter 契约对齐得很整齐，thread.started / item.* / turn.* 的 JSONL 解析也跟官方 cheatsheet 对得上。建议合并前先做以下两件事：
- 修掉下面 5 个必要改动（B-1~B-5）；
- 拆 PR — 先合 adapter，再合 system-agent fallback。
一、必要改动（建议合并前修）
B-1 command_execution.success 误判（真 bug）
packages/server/src/agents/codex-adapter.ts 里：
success: i.exit_code === 0 || i.status === 'completed',status === 'completed' 在 Codex 语义里只表示"item 已结束"，不区分成功失败。一条 grep 找不到（exit 1）的 shell 命令也会拿到 status: 'completed'，于是被错判为成功。
建议：
success: typeof i.exit_code === 'number' ? i.exit_code === 0 : i.status === 'completed',B-2 parseTeamSuggestion 不应让 LLM 自由选 runtime
packages/server/src/system-agents/team-architect.ts：
const runtimeNormalized = runtime === 'cursor' || runtime === 'codex' ? runtime : defaultRuntime;prompt 里写的是 Use runtime="codex" for every agent，但 LLM 完全可能无视。在只有 Codex 装了的机器上，最终会创建一批 runtime: 'cursor' 的 Agent，用户首次 @ 它就会看到 "cursor backend not configured"。
建议直接强制对齐当前可用 runtime：
const runtimeNormalized = defaultRuntime;如果未来要支持 Architect 显式跨 runtime 推荐，再加一个白名单 + 安装检测。
B-3 createSystemAdapter 双重 checkInstallation 在 SDK 模式下浪费 API 调用
const cursor = createCursorAdapter();
const cursorInstall = await cursor.checkInstallation(); // ← 这次
if (cursorInstall.installed) return cursor;调用方（coach / evaluator / scribe / facilitator / onboarder / team-architect）拿到 adapter 后还会再调一次 adapter.checkInstallation()。在 SLARK_CURSOR_BACKEND=sdk 模式下，CursorSdkAdapter.checkInstallation() 是真实 Cursor.me({ apiKey }) 网络调用 —— 每个 system agent 跑一次就多打一次 Cursor API。
建议把签名改成返回 install 让调用方复用：
export async function createSystemAdapter(): Promise<{
adapter: CLIAdapter;
install: { installed: boolean; version?: string; path?: string; error?: string };
}> {
// ...
}或在 CursorSdkAdapter.checkInstallation() 内加 30s in-memory cache（与 runtime-detect.ts 一致）。
B-4 两个 runtime 都没装时 system agents 静悄悄消失
return cursor; // 最后兜底，installed=false返回的 adapter 在 install.installed === false 上会被调用方判掉，结果是 Coach / Evaluator / Scribe 完全无声地不做事。Team Architect 走 fallbackTeam 还有 reason 字符串，其它几个连 log 都没有。
建议至少在 logger 里打一行：
logger.warn('[scribe] no coding runtime available (cursor/codex both missing); skipping');并把 createSystemAdapter 的兜底返回包一层告诉调用方"我俩都没装"。
B-5 --dangerously-bypass-approvals-and-sandbox + -s workspace-write 互相覆盖
if (params.permissive) {
args.push('-s', 'workspace-write');
args.push('--dangerously-bypass-approvals-and-sandbox');
}--yolo 已经覆盖 sandbox + approvals，前一行 -s workspace-write 是死代码。要么删掉 -s workspace-write，要么删掉 --dangerously-bypass-approvals-and-sandbox 走"workspace-write + 不再问 approval"的更稳妥路径（Slark 默认 permissive: true 给 agent，bypass-all 风险偏高）。
建议改为：
if (params.permissive) {
args.push('-s', 'workspace-write');
args.push('-a', 'never');
} else {
args.push('-s', 'read-only');
args.push('-a', 'never');
}二、拆 PR 建议
当前 PR 同时做了两件事：
- adapter 层（纯加法、零回归风险）：
codex-adapter.ts+createCodexAdapter+createAdapterForRuntime+ 类型 / RUNTIME_REGISTRY / 文档 - system-agent 层（行为变更，影响 6 个文件 + Team Architect prompt 大改）：
createSystemAdapter自动 fallback + 双 model catalog
建议拆成两个 PR：
PR-A feat(adapter): Codex CLI adapter（建议先合）
只包含：
packages/server/src/agents/codex-adapter.tspackages/server/src/agents/adapter-factory.ts里只加createCodexAdapter+createAdapterForRuntime，不加createSystemAdapter/runtimeForAdapterpackages/server/src/agents/engine.ts里getAdapterFor改用createAdapterForRuntimepackages/shared/src/constants.ts里RUNTIME_REGISTRY.codex.available = trueBuildTeamDialog.tsx里(a.runtime || 'cursor') as Runtime类型放宽- README 三处 +
WelcomePage.tsx文案 verify-sdk-adapter.ts加一条createCodexAdaptersmoke
效果：用户在 CreateAgentDialog 里手动选 codex runtime 就能用，零 system-agent 行为变化，几乎零风险。
PR-B feat(system-agent): auto-fallback to Codex when Cursor unavailable
在 PR-A 合后基于它再开一个 PR：
createSystemAdapter+runtimeForAdapter（修复 B-3 / B-4 后的版本）- 6 个 system-agent 改用
createSystemAdapter team-architect.ts双 catalog +defaultRuntime注入 + B-2 修复CreateProjectDialog.tsx注释 + 兜底 runtime 改为按检测结果选WelcomePage.tsx"Codex CLI ready" 绿条SLARK_SYSTEM_RUNTIME环境变量文档（README.md+docs/cursorsdkadapter.md都补一段）
这样 PR-B 的 review focus 全在"fallback 策略 + Team Architect prompt 设计"，不会和 adapter 实现细节混在一起。
三、其它非阻塞建议（可拆到 PR-B 或后续 PR）
getSupportedModels()用codex debug models --json取真实 catalog，硬编码列表回落parseItemCompleted把file_change也映射成tool.completed { tool: 'edit' }，否则 Activity Tab 看不到 codex 改了哪些文件turn.failed/error的message字段做对象兜底（避免[object Object]）- spawn spec 里
cwd与-C二选一（保留-C，删cwd，让"目录不存在但能 spawn"成为可靠语义） createAdapterForRuntime(runtime: string)收紧成Runtime类型，未来加claude时编译器能提醒
2df5169 to
cc285b9
Compare
|
Thanks for the detailed review. I split this PR down to the adapter-only scope:
I also updated the PR body with the narrower scope and current verification commands. For the system-agent fallback / Team Architect behavior, I prepared it as a separate stacked follow-up PR that depends on this adapter PR. That follow-up should be reviewed/merged after this PR, so the adapter plumbing stays easy to review first. |
coppynight
left a comment
There was a problem hiding this comment.
B-1 / B-5 已修复到位,PR-A 范围与建议一致,合并。
Adds createSystemAdapter() as the shared system-agent runtime selector with explicit SLARK_SYSTEM_RUNTIME=cursor|codex override, defaulting to Cursor when available and falling back to Codex when Cursor is not. Updates Coach, Evaluator, Facilitator, Onboarder, Scribe, and Team Architect to use the shared selector, and updates Team Architect prompts/fallback so generated agents match the chosen runtime. Follow-up to #2. Review feedback addressed in this PR: - B-3: createSystemAdapter now returns { adapter, install, noRuntimeAvailable } so callers no longer issue a duplicate adapter.checkInstallation() (avoids an extra Cursor.me() roundtrip per system-agent under SDK mode). - B-4: noRuntimeAvailable=true makes "neither cursor nor codex installed" explicit; all six system-agent callers warn-log and surface a clearer fallback_reason instead of silently no-op'ing. - B-2: parseTeamSuggestion forces runtime=defaultRuntime regardless of what the LLM returns; when overridden, model also falls back to defaultModel(...) for the active runtime, and thinking/context are forced null on codex to match the fallback team shape.
Hi, thanks for the careful review. I narrowed this draft PR to the adapter-only scope you suggested.
What changed in this revision:
CodexAdapterbacked bycodex exec --json.runtime: "codex"in the shared runtime registry and widened the team build UI type so Codex agents can be created.Review feedback addressed:
command_execution.successnow prefers numericexit_codewhen present.approval_policy=never.Local verification:
npm exec --yes -- pnpm typechecknpm exec --yes -- pnpm build:servernpm exec --yes -- pnpm build:webnpm exec --yes -- tsx packages/server/scripts/verify-sdk-adapter.tsnpm exec --yes -- prettier --check packages/server/src/agents/codex-adapter.ts packages/server/src/agents/adapter-factory.ts packages/server/scripts/verify-sdk-adapter.tsgit diff --checkcodex -s workspace-write -a never exec --helpNote: this stays intentionally draft/early because the Codex JSONL event surface may need more coverage as real usage expands.