feat(cli): add one-shot user commands - #3210
Conversation
📝 WalkthroughSummaryThis PR adds Codex-style The implementation extends the existing Runtime Resource, session-driver, transcript, and client-inspection paths. It does not create a separate command execution system. User commands use The solution is the smallest coherent path shown by the diff. It reuses shell-run resources and existing update handling. The added race handling is necessary because terminal updates can arrive before the local transcript card exists. The new protocol validation and caller checks are necessary to enforce command bounds and access restrictions. Complexity delta
No test or implementation area is clearly removable without weakening regression coverage. The total maintenance complexity increases, but the increase is justified by the new user-command behavior, access boundary, protocol validation, and race-replay requirements. ValidationThe reported validation includes package builds, CLI, Runtime, and Runtime Host suites, formatting, lint, and diff checks. Additional tests cover command routing, no-agent-turn behavior, transcript projection, raced update replay, caller restrictions, client inspection, protocol bounds, and Runtime Host launch parameters. The final check status remains subject to independent verification. Review-relevant risks
The person performing the merge must review the final diff. A maintainer makes the final determination. WalkthroughThe PR adds ChangesOne-shot user command flow
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🔵 Low · up to The PR adds one-shot user commands, but completed commands can currently produce an extra background-task notice in addition to their local result card. This creates bounded duplicate or misleading TUI output; the change is otherwise mergeable with explicit owner follow-up to suppress that notice and cover the behavior with a test. Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant TUI
participant SessionDriver
participant RuntimeHost
participant Runtime
TUI->>SessionDriver: runUserCommand(command)
SessionDriver->>RuntimeHost: runtime.resource.start(command)
RuntimeHost->>Runtime: create user-visible shell run
Runtime-->>RuntimeHost: snapshot and shell-run update
RuntimeHost-->>SessionDriver: command result and raced update
SessionDriver-->>TUI: local transcript card data
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoAdd Runtime Host-backed one-shot user commands to the TUI
AI Description
Diagram
High-Level Assessment
Files changed (16)
|
Code Review by Qodo
1. Reconnect erases command cards
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
76f6a88 to
313edbb
Compare
me2seeks
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/runtime/src/shell-run-contract.ts (1)
74-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the canonical visibility type.
Use
ShellRunVisibilityfrompackages/core/src/shell-run.tshere. The literal union duplicates the persisted visibility authority. A later visibility change can otherwise create a contract mismatch.Disposition: optional.
As per path instructions, consolidate duplicated authority and reuse the closest existing seam.
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ab21c065-4510-4f86-b3a6-a9717fa3999f
📒 Files selected for processing (16)
packages/cli/src/__tests__/pi-transcript.test.tspackages/cli/src/__tests__/pi-tui-runner.test.tspackages/cli/src/__tests__/runtime-host-session-driver.test.tspackages/cli/src/pi-transcript.tspackages/cli/src/pi-tui-runner.tspackages/cli/src/runtime-host-session-driver.tspackages/cli/src/session-driver.tspackages/cli/src/tui-primary-guidance.tspackages/core/src/shell-run.tspackages/runtime-host/src/__tests__/runtime-resource-coordinator.test.tspackages/runtime-host/src/__tests__/runtime-resource-protocol.test.tspackages/runtime-host/src/protocol/runtime-resource.tspackages/runtime-host/src/server/runtime-resource-coordinator.tspackages/runtime/src/__tests__/shell-run-manager.test.tspackages/runtime/src/shell-run-contract.tspackages/runtime/src/shell-run-manager.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 0 remain after this review.
|
Code review by qodo was updated up to the latest commit 313edbb |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The core direction is sound: Runtime Host remains the sole execution and resource-lifecycle authority, and the CLI adds only a local projection. I found three concrete lifecycle/projection gaps that should be addressed before merge: commands have no cancellation or close-time cleanup, reconnect discards their only UI projection, and terminal updates reuse the model-background-task notice. The smallest design is to keep Runtime Host authoritative while having the CLI retain active user-command refs, stop them through the existing client-owned resource API, and reconstruct their cards from user-visible resource snapshots after reconnect.
AI-assisted review: Codex coordinated two independent reviewer passes and an OpenCode Go DeepSeek V4 Flash high-effort adversarial pass. I verified these findings against exact head 313edbbcf18f961714c116ebb21669f313e299cb, the relevant call paths, and current CI. No local tests were run in this review.
中文审查
整体方向正确：执行权和资源生命周期仍由 Runtime Host 统一负责，CLI 只增加本地投影。当前有三个需要修复的具体缺口：用户命令没有取消或退出清理路径；重连会删除唯一的 UI 卡片；终态更新会错误复用模型后台任务通知。最小方案是不新增执行权，而是让 CLI 保存活跃 user-command ref，通过已有 client-owned resource stop API 清理，并在重连后从用户可见资源快照重建卡片。
本次为 AI 辅助审查：Codex 协调两轮独立 reviewer 审查及 OpenCode Go DeepSeek V4 Flash high 对抗性复核，并核验了精确 head、相关调用链和当前 CI。本轮未运行本地测试。
|
This PR adds a new user-visible TUI flow. Could you please add a screenshot showing a ! command in the idle TUI and the resulting User command card, including its completed output state? One annotated screenshot is fine. Thanks! Posted by Codex on behalf of Astro-Han. |
|
Added the requested screenshot to the PR body. It shows an idle |
313edbb to
2c65cc5
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The current-head rereview confirms that the prior teardown, same-session reconnect, and duplicate-settlement findings are fixed. The actual terminal capture also matches the current UX: completed user commands are expanded by default and the bare ! hint is visible. All six commits carry Generated-by: Codex, and the PR body discloses the substantive scope.
One nonduplicate session-transition P2 remains below. Exact-head CI has not run yet because the Apache Actions lanes are still queued/unavailable, so this head is not merge-ready independently of the finding.
AI-assisted review by OpenAI Codex. I verified the range-diff from the previously reviewed head, lifecycle and projection paths, existing threads, current screenshot, provenance, and live check state.
中文
当前 head 已修复上一轮的退出清理、同 Session 重连和重复终态通知问题；真实终端截图也与当前 UX 一致。6 个提交均保留 Generated-by: Codex，PR body 的 AI 披露完整。
下面仍有一个新的 Session 切换 P2。当前 exact-head CI 还没有实际运行，因此即使不考虑 finding，也尚不可合并。
本次由 OpenAI Codex 辅助，已核对上一轮 head 的 range-diff、生命周期与投影路径、已有线程、截图、来源披露和实时检查状态。
3225392 to
787ad6a
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
COMMENT. One P1 that I think blocks the merge, plus four smaller findings and a contract question.
Scope note first: I reviewed this at 787ad6aae, after the rebase and the fix(cli): stop user commands before leaving their Session commit. The start-barrier and stop-generation machinery in stopUserCommands is a real improvement over the previous head, and the two threads I opened earlier — cancellation/teardown at runtime-resource-coordinator.ts:352 and stop-on-session-change at runtime-host-session-driver.ts:355 — are both genuinely addressed. Two of the findings below are residuals of those fixes rather than repeats of them.
The P1 is the compatibility epoch. This PR widens runtime.resource.start's input to accept command and leaves RUNTIME_HOST_COMPATIBILITY_EPOCH at 27, which is what main has, so a new CLI and an old remote Runtime Host still shake hands and then fail on the first ! command. Details inline. The same PR also adds visibility to the durable ShellRunRecord (packages/core/src/shell-run.ts:117, key set at :305), which an older binary reading the same workspace operational-state DB rejects as a malformed field — and listSessionShellRuns maps over the whole session, so one such row takes out that session's entire shell-run projection. Same root cause, same fix.
Two things I could not confirm and am not filing:
- The close path.
beginClose(pi-tui-runner.ts:695-704) firesvoid input.driver.stop().catch(() => {})and returns, so/exitwith a running!sleep 3600does not await theruntime.resource.stopbefore the caller tears the connection down, andbeginCloseskipsdriver.stop()entirely wheninterruptRequestedis already set. Against a managed host that leaves the command running and holding amaxLiveShellRunsslot. I am reasonably confident this is real but I could not construct the failure end to end at this head, so I am recording it as an observation on the existing teardown thread rather than as a numbered finding. - Whether
runtime.resource.startbeing inREMOTE_OWNER_OPERATION_GRANTSis a privilege change. I do not think it is — a remote owner could already start the login-shell PTY, acquire a controller and type anything into it — but the new surface is non-interactive and scriptable, which is a different exposure profile. An explicit "no new privilege" line in the PR body would settle it.
Test gaps, none of which I am giving a P level:
runtime-resource-coordinator.test.ts'sstarts a one-shot user command in pipes without exposing it to the modelasserts only theShellRunBashInputhanded to a fakerunBackgroundBash; the harness'sreadRuntimeResourcehas no visibility gate, so the test cannot prove non-exposure. The real coverage is inshell-run-manager.test.ts; the name overclaims.stops a running user command before switching Sessions (#3210)asserts that aruntime.resource.stopexists in the request log, not that it preceded the identity change — which is the actual claim.UserCommandDriverinpi-tui-runner.test.tsreturnsstatus: 'completed'fromrunUserCommand, so the running-to-terminal card transition is never exercised in the runner suite.- The PR body's "removes
/shell" is stale: neither/shellnoropenInteractiveShellexists onmainor in this diff.
biome format is clean on every changed TS file at this head.
AI disclosure: this review was assisted by Claude (Opus), which performed the initial code search and cross-checking. Every finding published here I re-derived myself against the source at 787ad6aae, and I dropped the ones I could not confirm at that head — the two observations above are marked as such rather than filed.
d7552f7 to
a87fc52
Compare
|
Rebased onto current main; new head
Everything else from the 2026-08-20 11:31 review is unchanged from 中文说明已 rebase 到最新 main；epoch 因 main 占用 28 而推进到 29，gate 测试改为相对断言，tip 提交补上合规的 Generated-by trailer。本地套件全绿。11:31 review 的全部修复保持不变，等待重新 review。 |
1808710 to
8d9da7d
Compare
|
Rebased onto current
Verified on new head |
8d9da7d to
7949592
Compare
|
Hi — this PR was behind Please CI should now run against the rebased head. I will pick this up for review once the checks are terminal. AI-assisted maintenance action. It is not a review and does not count as the required human review under |
956ba32 to
f0149f9
Compare
ARE404
left a comment
ARE404
left a comment
There was a problem hiding this comment.
Exact-head review at f0149f9d5ea6d191f0708ec7b98c1da35de95f20: NO-GO. Spec axis: three P2 lifecycle/atomicity findings inline. Standards axis: one P3 evidence gap inline; no hard code-standard violation. The permission review found no exposed new grant or model confirmation bypass: remote owners already had the broader interactive-shell path, while model Read/Write/Stop and context-summary paths are gated for visibility: user. The only path-filtered check expected here is test; it is terminal failure because packages/cli/src/pi-tui-runner.ts has a formatter-rejected blank line at EOF. No audit or Windows sandbox check is expected for these paths.
f0149f9 to
73ba78b
Compare
|
Rebased onto
Verified locally: CLI workspace 439/439, runtime-host 1127 green including the new regression tests; epoch guard 44 → 45 passes against the new base. The [P3] before/after captures will be refreshed from this head once CI is green. |
…leaning Three review findings on apache#3210: - startNewSession is now awaited end-to-end: the driver runs the barrier-aware user-command stop before any identity change and rejects on failure, so /new aborts with nothing committed instead of stranding a running command without its card or Ctrl+C affordance. The runner surfaces the reason and leaves transcript/state intact; its control gate lets /new through while a user command runs (that stop is the command's first step), fenced by the existing generation guards. - The one-shot resource start performs header read, shell resolution, launch, and the initial snapshot inside one Session admission section, closing the relocate-vs-cwd gap; when the post-launch snapshot fails, the launched process is stopped so a client retry cannot double-execute. Generated-by: maka
|
Heads-up on CI, not a review finding: this head's CI run is red on the formatter, and it fails before any test runs. Its checks had never executed — the run was parked awaiting maintainer approval. I released it ( Where it stops: the What it found — reproduced locally at
One of them is a line-length wrap Biome wants split: plus one stray blank line before a
中文CI 提示，不是 review finding：这个 head 红在格式化检查上，而且它在任何测试之前就失败了。 它的检查此前从没执行过——run 卡在等放行，我已放行（ 停在哪： 报了什么（我在
|
…leaning Three review findings on apache#3210: - startNewSession is now awaited end-to-end: the driver runs the barrier-aware user-command stop before any identity change and rejects on failure, so /new aborts with nothing committed instead of stranding a running command without its card or Ctrl+C affordance. The runner surfaces the reason and leaves transcript/state intact; its control gate lets /new through while a user command runs (that stop is the command's first step), fenced by the existing generation guards. - The one-shot resource start performs header read, shell resolution, launch, and the initial snapshot inside one Session admission section, closing the relocate-vs-cwd gap; when the post-launch snapshot fails, the launched process is stopped so a client retry cannot double-execute. Generated-by: maka
720151a to
228b94f
Compare
…leaning Three review findings on apache#3210: - startNewSession is now awaited end-to-end: the driver runs the barrier-aware user-command stop before any identity change and rejects on failure, so /new aborts with nothing committed instead of stranding a running command without its card or Ctrl+C affordance. The runner surfaces the reason and leaves transcript/state intact; its control gate lets /new through while a user command runs (that stop is the command's first step), fenced by the existing generation guards. - The one-shot resource start performs header read, shell resolution, launch, and the initial snapshot inside one Session admission section, closing the relocate-vs-cwd gap; when the post-launch snapshot fails, the launched process is stopped so a client retry cannot double-execute. Generated-by: maka
228b94f to
b5d18be
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Update on f6139efc15:
[P1] RUNTIME_HOST_COMPATIBILITY_EPOCH must strictly increase — still 49
PR claims epoch bumped to handle new command + visibility fields, but head f6139e sets RUNTIME_HOST_COMPATIBILITY_EPOCH = 49 which is already used by main for WorkHub (also 49). Rebased PR re-used main's occupied epoch instead of allocating a fresh one (needs 50). A main-built Host (epoch 49 WorkHub) will accept a PR-built Client but reject the new command key / visibility record — incompatible change not fenced.
Fix: bump epoch to a new value strictly greater than main's 49 and keep comment load-bearing to force conflict.
Also mergeStateStatus=CONFLICTING/DIRTY — needs rebase.
Checks on f6139efc15ce735e8878e2313d704db7060b8153 are test: SUCCESS only — not full green.
简体中文
epoch 未真正递增，已占用，与 main 冲突。
5d57650 to
1d9c615
Compare
Generated-by: Codex
e030f03 to
3277eb1
Compare
# Conflicts: # packages/cli/src/pi-transcript.ts # packages/runtime-host/src/protocol/index.ts
4 participants
Summary
Adds idle TUI
!<command>execution through the selected Runtime Host and current Session workspace. The command appears as a local user-command card and does not create an agent turn. Submitting bare!showsUsage: !<command>;!!pwdexecutes!pwd.Runtime boundary. Runtime Host remains the execution and resource-lifecycle authority. One-shot user commands persist
visibility: "user". Runtime model-facing Read, Write, and Stop return the same non-disclosing not-found result used for inaccessible resources, while the owning Client can inspect and stop the exact command resource.Lifecycle boundary. Host admission rechecks that the Session is active and resolves its authoritative working directory before launch. A failed post-launch inspection triggers best-effort cleanup. The CLI driver owns observation ordering and stops active user commands before Session switch, new Session, close, or driver disposal; TUI reconnect preserves local command cards without turning them into transcript messages.
Protocol compatibility.
runtime.resource.startaccepts an optional bounded one-shotcommand, and durable Shell Run records carryvisibility.RUNTIME_HOST_COMPATIBILITY_EPOCHis 57, so epoch-56 peers are rejected during admission rather than failing on the widened wire shape.Privilege.
runtime.resource.startalready belongs toREMOTE_OWNER_OPERATION_GRANTSfor the interactive login shell. This change adds no grant and no CLI-owned executor.Fixes #3209
中文说明
TUI 空闲时输入
!<command>，命令会由当前选中的 Runtime Host 在当前 Session 工作目录执行，并显示为本地 User command 卡片，不创建 agent turn。提交裸!会显示Usage: !<command>；!!pwd会执行!pwd。Runtime Host 继续拥有执行、工作目录准入和资源生命周期。一次性用户命令持久化为
visibility: "user"：模型侧 Read、Write、Stop 均得到非披露的 not-found；拥有该资源的 Client 仍可检查和停止它。Host 在真正启动前会在同一准入边界内再次确认 Session 仍为 active，并读取权威工作目录；启动后检查失败会尽力停止资源。CLI driver 在切换 Session、新建 Session、关闭或释放前停止活动命令。重连会保留本地命令卡片，但不会把它写进对话 transcript。
协议增加可选且有长度上限的
command，持久化 Shell Run 增加visibility，兼容 epoch 升为 57；epoch 56 的旧对端会在准入时被拒绝。Verification
npm testpassed with all workspace suites at 0 failures.git diff --checkpassed.!, and!!command.AI use
Select exactly one:
Tool(s) and scope: Codex prepared the implementation, tests, Issue, and PR description. The human contributor reviews the diff and remains responsible for submission, correctness, provenance, and licensing.
Checklist
Does this PR entail a change in behavior?
Visual evidence
Not included. The prior screenshots represented an earlier implementation with an inline localized hint that is intentionally outside this reconstructed scope.