Skip to content

fix(cli): wait for armed goal completion - #4192

Open
cat0825 wants to merge 4 commits into
apache:mainfrom
cat0825:fix/maka-run-goal-exit-3854
Open

fix(cli): wait for armed goal completion#4192
cat0825 wants to merge 4 commits into
apache:mainfrom
cat0825:fix/maka-run-goal-exit-3854

Conversation

@cat0825

@cat0825 cat0825 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Keep blocking maka run invocations 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 failed
  • focused Goal and real Session recovery exhaustion tests — 9 passed, 0 failed
  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint — passed
  • npx biome check on the four changed files — passed
  • npx knip --workspace apps/desktop — passed
  • npx knip --workspace packages/ui — passed
  • git diff --check — passed

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

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

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Keep blocking maka run invocations attached to a self-armed Goal until its durable terminal turn is available.

Generated-by: OpenAI Codex
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Aug 29, 2026

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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:

  1. 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.
  2. When the Session subscription exhausts recovery, the new Goal waiter receives no terminal failure signal. The channel can fail permanently while the prior goal.query still reported an active Goal; without an onFailed path or a bounded failure, a default maka run can 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 生命周期问题:

  1. timeout 或 SIGINT 落在两轮 Goal Turn 之间时不会停止 Goal。现有 stop 只停止当前 root Turn;没有 active Turn 时,本地 waiter 会关闭,但 Host-owned Goal 仍保持 active,CLI 返回后仍可能继续执行工具。这是正常支持的取消路径,也重新制造了本 PR 要关闭的权限与退出边界。停止动作必须在 Host owner 原子 pause/cancel 当前 Goal,不能只依赖 turn.stop
  2. 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 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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
@cat0825

cat0825 commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

Review follow-up for head f23b11dc9fa6e6356da2860edd6d164d2d46157b:

  • timeout/SIGINT now pauses an active or waiting Host-owned Goal through revision-checked goal.control; the between-Turn waiting state has a regression test;
  • bounded Session subscription recovery exhaustion is surfaced to Goal/Graph waiters, with an integration test that exhausts the real eight-attempt recovery path;
  • an already-terminal Goal still reads and projects its final durable Turn;
  • paused Goals fail instead of blocking indefinitely;
  • Host-started Goal/Graph Turn streams are drained to bound live queue retention.

Local verification passed: maka-agent 640/640, root build/typecheck/lint, changed-file Biome, desktop/UI knip, and diff check.

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.

@cat0825

cat0825 commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

CI follow-up: the previous run failed in Check renderer architecture, not in the CLI change. I merged current upstream/main@8c491e64 normally (no rebase/force-push) and reproduced the same ledger mismatch locally with no Desktop diff against that base. Current main itself fails identically in CI run 33300276625, introduced after refactor(desktop): lock renderer root architecture (#4088); this PR does not modify the renderer or its ledger.

Post-merge npm --workspace maka-agent test still passes. I am leaving the unrelated renderer ledger unchanged and will rely on an upstream main fix for that gate.

Automated response: posted by OpenAI Codex on behalf of cat0825.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[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 &&

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[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 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

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

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(cli): maka run returns 0 while a self-armed full-access Goal keeps executing in the Runtime Host

3 participants