fix(cli): wait for armed goal completion - #4192
Conversation
Keep blocking maka run invocations attached to a self-armed Goal until its durable terminal turn is available. Generated-by: OpenAI Codex
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for addressing a demonstrated and important maka run contract violation. Keeping the CLI attached to a self-armed Goal and projecting the final durable Turn from Runtime Host is the correct ownership direction.
I reviewed exact head 527acd9501d9eae6c32d63b36cd0642cffb28ded. The exact-head checks are green, but the PR body contains no before/after terminal transcript or recording for the changed CLI behavior. Please attach evidence covering the first answer versus final Goal answer, final exit code, timeout, and SIGINT.
I found two P1 lifecycle issues:
- A timeout or SIGINT during the gap between Goal Turns does not stop the Goal. The existing stop path only stops the current root Turn. When no Turn is active, the local waiter closes but the Host-owned Goal remains active and can continue executing tools after the CLI returns. This is a normal supported cancellation path and recreates the permission/exit boundary the PR is intended to close. The stop operation must atomically pause or cancel the current Goal at its Host owner, not rely only on
turn.stop. - When the Session subscription exhausts recovery, the new Goal waiter receives no terminal failure signal. The channel can fail permanently while the prior
goal.querystill reported an active Goal; without anonFailedpath or a bounded failure, a defaultmaka runcan wait forever. Please propagate terminal channel failure into the Goal waiter and cover real recovery exhaustion.
There is also a P2 fast-completion race: if the first goal.query already returns a terminal Goal, waitForGoalCompletion() returns immediately without reading and projecting the final Goal Turn, so stdout can still contain the initial answer. The long-Goal event queue also needs an explicit drain or discard path rather than retaining every Host-started Turn until context close.
The current tests cover active → achieved and local close, but not timeout, SIGINT, subscription failure, fast terminal discovery, or multi-turn queue pressure. Please extend the real lifecycle coverage around those supported boundaries.
Review analysis was assisted by Codex and independent @reviewer agents. Astro-Han verified the exact head, Host and CLI ownership paths, cancellation and recovery reachability, CI, and severity judgment, and owns this review.
中文对照
谢谢你处理这个已经得到实际复现、而且很重要的 maka run 契约问题。让 CLI 持续等待 self-armed Goal,并从 Runtime Host 投影最终持久化 Turn,是正确的 owner 方向。
我审查了精确 head 527acd9501d9eae6c32d63b36cd0642cffb28ded。exact-head checks 已通过,但 PR 正文没有这次 CLI 行为变化的前后终端输出或录屏。请补充首轮答案与最终 Goal 答案、最终退出码、timeout 和 SIGINT 的证据。
有两个 P1 生命周期问题:
- timeout 或 SIGINT 落在两轮 Goal Turn 之间时不会停止 Goal。现有 stop 只停止当前 root Turn;没有 active Turn 时,本地 waiter 会关闭,但 Host-owned Goal 仍保持 active,CLI 返回后仍可能继续执行工具。这是正常支持的取消路径,也重新制造了本 PR 要关闭的权限与退出边界。停止动作必须在 Host owner 原子 pause/cancel 当前 Goal,不能只依赖
turn.stop。 - Session subscription 恢复耗尽后,新 Goal waiter 收不到最终失败信号。channel 可能永久失败,而之前的
goal.query已经看到 active Goal;没有onFailed或有界失败路径时,默认maka run会永久等待。请把 channel terminal failure 传递给 Goal waiter,并覆盖真实 recovery exhaustion。
另有一个 P2 快速完成竞态:第一次 goal.query 如果已经返回 terminal Goal,waitForGoalCompletion() 会直接返回,不读取并投影最终 Goal Turn,stdout 仍可能只有首轮答案。长 Goal 的 event queue 也需要明确 drain 或 discard,而不是把所有 Host-started Turn 保留到 context close。
当前测试覆盖 active → achieved 和本地 close,但没有覆盖 timeout、SIGINT、subscription failure、快速 terminal discovery 或多轮 queue pressure。请围绕这些受支持边界补充真实生命周期测试。
本次审查分析由 Codex 和独立的 @reviewer 子代理协助;Astro-Han 核验了精确 head、Host 与 CLI owner 路径、取消和恢复可达性、CI 和问题分级,并对本次 Review 负责。
hqhq1025
left a comment
There was a problem hiding this comment.
Codex-assisted review performed under the maintainer-approved review workflow.
Reviewed exact head 527acd9501d9eae6c32d63b36cd0642cffb28ded. This change makes blocking maka run wait for a self-armed Goal and project the final Goal Turn into stdout and the exit code. I inspected the complete three-file diff plus the Runtime Host session channel, Goal continuation state machine, stop path, and current-main synthetic merge.
I independently confirmed the same-head lifecycle concerns already documented by Astro-Han, so I am not duplicating those comments. I found one additional P1 inline: an automatically paused Goal is ignored as non-terminal, leaving the default no-timeout CLI blocked indefinitely.
Validation: exact-head maka-agent suite passed 634/634; synthetic merge with current main (d2346707) passed 635/635; CLI typecheck and Biome passed. A focused runtime-level reproduction against the built exact head showed an active-to-paused Goal update leaves waitForCompletion() pending.
Unable to determine: I did not run a live provider-backed multi-turn Goal because that requires external model credentials and nondeterministic execution. The deterministic runtime reproduction covers the state transition responsible for the hang.
Result: NO-GO while this P1 and the already-reported same-head P1 lifecycle issues remain.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
| async waitForGoalCompletion(sessionId: string): Promise<void> { | ||
| await this.#attach(sessionId); | ||
| const current = (await this.#connection.request('goal.query', { sessionId })).goal; | ||
| if (!current || TERMINAL_GOAL_STATUSES.has(current.status)) return; |
There was a problem hiding this comment.
[P1] Terminate the blocking CLI wait when the Goal becomes paused. TERMINAL_GOAL_STATUSES intentionally excludes paused, so this initial check falls through, and the subscription callback below also ignores an active-to-paused update. Goal continuation reaches paused on ordinary failures such as an aborted/failed continuation Turn, evaluation-context failure, or unavailable admission (packages/runtime/src/goal-continuation.ts:643-649, 670-678, and 805-806). Because maka run has no default timeout and a paused Goal will not continue without an external resume/clear, the command then waits forever. I reproduced this against the built exact head: after publishing an active-to-paused projection, waitForCompletion() remained pending. Please treat paused as a terminating error for this CLI wait (while preserving Goal durability) and add a regression test.
There was a problem hiding this comment.
Addressed in f23b11d: both initially paused and active/waiting-to-paused projections now terminate the blocking CLI wait with Goal paused before completion. The regression test covers the pushed transition without clearing the durable Goal.
Automated response: posted by OpenAI Codex on behalf of cat0825.
Generated-by: OpenAI Codex
|
Review follow-up for head
Local verification passed: Not run: a live provider-backed terminal recording for first-answer versus final-Goal-answer, timeout, and SIGINT. It requires external model credentials and nondeterministic execution; I did not fabricate this evidence. The deterministic command/runtime and real recovery-chain tests cover those ownership transitions. Automated response: posted by OpenAI Codex on behalf of cat0825. |
|
CI follow-up: the previous run failed in Post-merge Automated response: posted by OpenAI Codex on behalf of cat0825. |
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for extending maka run through the Host-owned Goal terminal boundary. I reviewed exact head ef7191a94569a535221f011b1248b5eb5b7ef40b and left two suggestions around cancellation and Goal-generation ownership. The previously reported paused-Goal hang is fixed; these are separate recovery/concurrency paths. The exact-head test check is currently failing, so I have not treated CI as a gate here. Please do push back if the Host lifecycle has an invariant I have missed—the author may have context that is not visible from this lane.
中文摘要
感谢把 maka run 的阻塞等待延伸到 Host 权威的 Goal 终态。旧的 paused Goal 卡死已经修复;这里另外留下两条关于取消竞态和 Goal generation 所有权的建议。当前 exact-head 的 test 仍失败。若 Host 生命周期还有我未看到的不变量,也欢迎直接反驳或补充上下文。
AI-assisted review disclosure: Codex ran an independent analysis lane; Astro-Han independently verified the exact head, production path, and severity, and owns this review.
| if (this.#graphEnabled) { | ||
| stops.push(this.#stopGraph(sessionId)); | ||
| } | ||
| stops.push(this.#pauseGoal(sessionId)); |
There was a problem hiding this comment.
[P1] Suggestion (category ② — reasonable SIGINT/timeout racing a Host-started Goal Turn): could cancellation stop the exact active Goal execution at the Host authority instead of relying on this client snapshot? Here driver.stop() only emits turn.stop when its local channel already projects a nonterminal rootTurn (runtime-host-session-driver.ts:1037-1055). If the Host has started the Goal Turn but its start frame has not reached this client, the snapshot can still show the previous terminal Turn, so no turn.stop is sent. The concurrent goal.control(pause) only invalidates the continuation lane (goal-coordinator.ts:423-425; goal-continuation.ts:938-949) and does not stop an already hosted execution. In that window stopSession() can return to the timeout/SIGINT path while the Goal Turn continues running tools. I suggest binding cancellation to the queried Goal/active Host execution and adding a gated start-frame → cancel regression that proves no post-return tool work occurs. Please push back if another Host invariant already fences this race.
| }; | ||
| const accept = (goal: GoalProjection | null) => { | ||
| if ( | ||
| goal?.goalId === goalId && |
There was a problem hiding this comment.
[P2] Suggestion (category ② — reasonable multi-client clear/re-arm): could this waiter stay bound to the original goalId/generation? A projection for any different Goal falls through to finish(goal), so if another client clears A and arms B while this command waits, it can complete against B and later read/print B’s terminal output. The cancellation path has the same ownership drift: #pauseGoal() re-queries the current Goal and can pause B rather than the A that this invocation was awaiting. Carrying the original goalId (and revision/generation) through both wait and cancel would keep this command from observing or mutating a successor Goal. A clear-A/re-arm-B concurrent regression would make the boundary explicit. Please push back if concurrent Goal control is intentionally serialized elsewhere.
hqhq1025
left a comment
There was a problem hiding this comment.
Reviewed exact head ef7191a94569a535221f011b1248b5eb5b7ef40b. The prior paused-Goal hang, recovery-exhaustion hang, terminal projection race, and undrained Host-started queues are fixed. I independently confirmed the two current-head lifecycle findings already left inline by Astro-Han, so I am not duplicating those comments.
P1: timeout/SIGINT can race a Host-started Goal Turn before its start frame reaches this client. In that window driver.stop() sees no nonterminal rootTurn, while goal.control(pause) only invalidates future continuation admission; stopSession() can return while the already admitted execution continues tools.
P2: #waitForGoalTerminal() treats any different Goal projection as completion, and #pauseGoal() re-queries whichever Goal is current. A concurrent clear-A/re-arm-B can therefore make this invocation print B’s terminal output or pause B instead of the Goal it originally awaited. A focused exact-head probe confirmed that publishing an achieved successor Goal resolves the waiter for Goal A.
Validation: npm run build:test, full workspace typecheck, maka-agent 641/641, changed-file checks, git diff --check, and a synthetic merge with current main passed. The exact-head hosted test is red only in the renderer architecture ledger; the PR has no Desktop diff, the same failure reproduced against its base, and current main contains the later ledger fix.
Unable to determine: no live provider-backed timeout/SIGINT run was executed. The cancellation P1 is established from the Host/driver ownership path; the successor-Goal P2 has a deterministic local reproduction.
Result: NO-GO until both current-head lifecycle issues are fixed.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
Summary
Keep blocking
maka runinvocations attached to a self-armed Goal until the Goal reaches a durable terminal boundary, then report the final Goal Turn through the existing stdout and exit-code projection. A paused Goal now terminates the CLI with an error, timeout and SIGINT pause active or waiting Host-owned Goals under revision control, exhausted Session recovery fails pending waiters, and Host-started Turn queues are drained.Fixes #3854
Verification
npm --workspace maka-agent test— 640 passed, 0 failednpm run build— passednpm run typecheck— passednpm run lint— passednpx biome checkon the four changed files — passednpx knip --workspace apps/desktop— passednpx knip --workspace packages/ui— passedgit diff --check— passedAI use
Select exactly one:
Tool(s) and scope: OpenAI Codex investigated the failure, implemented the CLI/runtime-host change, and authored the regression tests under human direction and review.
Checklist
Does this PR entail a change in behavior?