feat: add Plan mode - #232
Conversation
Normalize Default and Plan modes across Codex collaboration mode and Claude permission mode. Add the /plan command, Shift+Tab cycling, session persistence, status projection, and busy-turn protection. Update chat-tui to the merged mode-cycling protocol and cover both harness adapters plus the TUI and store behavior.
| (candidate.id === "mode" || candidate.category === "mode"), | ||
| ); | ||
| if (!option || option.type !== "select" || option.options.length === 0) { | ||
| throw new Error(`${target} does not support Plan mode`); |
There was a problem hiding this comment.
🤖 devloop code-review · seed-2.1-pro
The error message in modeOption says "does not support Plan mode", but this method is also called from cycleMode() (Shift+Tab), where the user isn't necessarily trying to switch to Plan mode — they might just be cycling through modes. The error message is misleading in that context. Consider using a more generic message like "does not support mode switching" or "has no configurable modes" to accurately reflect both callers.
ccr:fp=e2a7dbc144a2
There was a problem hiding this comment.
ccr:label=minor — finding 成立；modeOption 同时服务 /plan 与 Shift+Tab，错误文案已改为不支持 mode switching，避免把通用模式切换误写成仅 Plan。
| throw new Error("Cannot switch Claude mode while a turn is running"); | ||
| } | ||
| if (rt.activeQuery) await rt.activeQuery.setPermissionMode(value); | ||
| rt.permissionMode = value; |
There was a problem hiding this comment.
🤖 devloop code-review · deepseek-v4-pro
When setConfig is called with value === "default", rt.permissionMode is set to "default" (a truthy string). This means permissionMode: "default" will be passed to the SDK options at line 870, unlike setModel and setEffort which treat "default" as undefined (omitting the option entirely). This inconsistency means a session that explicitly selects "default" mode may behave differently from a freshly opened session where permissionMode is undefined. Consider setting rt.permissionMode to undefined when value is "default", consistent with the pattern used in setModel and setEffort.
ccr:fp=21f3a0fc5d4f
There was a problem hiding this comment.
ccr:label=minor — finding 成立；切回 default 后仍对活动 Query 显式调用 setPermissionMode(default)，但运行时清除覆盖，使后续 Query 恢复 SDK 缺省值。
| rt.peer.request("model/list", { limit: 200 }), | ||
| ]); | ||
| const selected = codexModes(modeResult).find((candidate) => candidate.id === value); | ||
| if (!selected) throw new Error(`Unknown Codex mode: ${value}`); |
There was a problem hiding this comment.
🤖 devloop code-review · seed-2.1-pro
In setConfig for mode, the value validation happens after two parallel API calls. If the value is invalid (e.g., not "default" or "plan"), both API calls are wasted. The Claude adapter (src/harness/claude/adapter.ts:741-742) validates the value first before any API calls. Consider moving the validation before the Promise.all to avoid unnecessary network requests.
ccr:fp=961f4085c4aa
There was a problem hiding this comment.
ccr:label=minor — finding 成立；default/plan 的输入校验已前移到任何 catalog 请求之前，并新增零请求回归测试。
| const modes = await rt.peer | ||
| .request("collaborationMode/list", {}) | ||
| .then(codexModes) | ||
| .catch(() => []); |
There was a problem hiding this comment.
🤖 devloop code-review · seed-2.1-pro
In getConfig, the collaborationMode/list API call uses .catch(() => []) which silently swallows errors. If rt.mode is already set (e.g., from a previous successful setConfig call) but the API temporarily fails, the mode option won't appear in the UI even though the mode is still active on the runtime. This creates an inconsistency where the mode is silently active but invisible to the user. Consider preserving the existing mode option from rt.mode when the API call fails, or at least logging the error.
ccr:fp=de64be33af36
There was a problem hiding this comment.
ccr:label=minor — finding 成立；旧 app-server 首次探测失败仍隐藏不支持的能力，但已选择模式后遇到临时 catalog 失败会保留 Default/Plan 本地快照和当前值。
|
🤖 devloop code-review · 4 finding(s)（4 条已作为独立 review thread 发布） |
Clarify mode-switching errors, restore Claude SDK defaults, validate Codex mode values before catalog requests, and retain an active mode during transient collaboration-mode catalog failures. Add regression coverage and update chat-tui to the merged PR #71 revision.
2 participants
Normalize Default and Plan modes across Codex collaboration mode and Claude
permission mode. Add the /plan command, Shift+Tab cycling, session persistence,
status projection, and busy-turn protection.
Update chat-tui to the merged mode-cycling protocol and cover both harness
adapters plus the TUI and store behavior.
Clarify mode-switching errors, restore Claude SDK defaults, validate Codex
mode values before catalog requests, and retain an active mode during
transient collaboration-mode catalog failures.
Add regression coverage and update chat-tui to the merged PR #71 revision.