feat(runtime): Goal-based autonomous execution (Issue #15 P6) - #625
Conversation
Astro-Han
left a comment
There was a problem hiding this comment.
Approved. The goal loop is a good direction, and the current issues I found are follow-ups rather than review blockers from my side.
- P2: The desktop goal evaluator uses the global default connection while evaluating recent messages from the current session. If a session is using a different provider/model, the evaluator can route session text to the wrong provider. I’d prefer this to use the session header’s
llmConnectionSlugandmodel, unless the evaluator provider becomes an explicit setting. - P2: Goal continuation can still inject a new turn while the session is
waiting_for_user.canContinuecurrently rejectsrunning,blocked, andaborted, but notwaiting_for_user, andinjectTurngoes straight toruntime.sendMessage. It would be safer to rejectwaiting_for_userand reuse the same send guard used by normal user sends. - P3: The evaluator passes
maxTokens: 250togenerateText; AI SDK v6 expectsmaxOutputTokens. As written, the cap is likely ignored. - P3:
wait_secondsis parsed from evaluator output but not used before continuing. Either honoring it or removing it from the contract would avoid surprising behavior.
jackwener
left a comment
There was a problem hiding this comment.
First-principles review done (core decision logic + tests read as high quality — evaluate-before-caps, neutral evaluator-failure handling, re-entrancy guard, and behavioral tests are all genuinely good). Three things block merge:
1. BLOCKER — CLI interrupt doesn't stop the loop. pi-tui-runner.ts:293-296 gates continuation on !closed, but the CLI's primary interrupt is double-Escape (:820-832 → driver.stop()), which never sets closed. Interrupting a goal-driven turn settles it and onTurnComplete immediately re-injects — the user cannot stop an autonomous loop without quitting the app. Thread the stop-reason out of the transcript (it already sees user_stop/abort events) into onTurnComplete and skip continuation on interrupt, mirroring the desktop turnAborted guard.
2. BLOCKER — merge-order conflict with #643. Both PRs rewrite the same turn-boundary seam: main.ts streamEvents tail (#643 changes its return type to { turnId, ok, error }; this PR assumes Promise<void>), the same runAgentTurn hunk in pi-tui-runner.ts, runtime-bootstrap.ts allTools, index.ts exports, cli-system-prompt.ts. Decision: #643 (unified Automation) lands first — it's being adopted with review fixes now. Please rebase onto main after it merges and reconcile the streamEvents signature. Worth considering: both features build 'inject a turn after settle' machinery independently — a single shared post-turn trigger seam would serve both.
3. Required before user-facing ship — a visible kill switch. An unattended re-prompting loop that burns tokens has zero renderer surface: no indicator it's active, no iteration counter, no pause/clear control (only the model-facing GoalStatus tool). At minimum emit a session event on goal set/continue/terminal so the UI can badge the session and offer 清除目标.
Also noted (fix or annotate): the evaluator actually runs on the session's full defaultModel while comments claim a cheap/fast model — implement the cheap-model mapping or correct the comments; a turn ending in error still continues (evaluator hits the same failing connection → up to ~50 failing turns before max_iterations — treat turn-level error as non-continuable or back off on repeated evaluatorFailed); goals are in-memory only and silently vanish on restart — defensible default, but document it.
Happy to re-review promptly after the #643 rebase.
CC-style external evaluator + Codex lifecycle (pause/resume/blocked, token budget, block cap, agent-settable goals). GoalManager + evaluateGoal (30s timeout, evaluator-failure neutral) + handleGoalContinuation (evaluate-first, re-entrancy guard, block cap -> stalled, token budget -> budget_limited). Wired at the turn boundary in both cli (runtime-bootstrap + pi-tui onTurnComplete) and desktop (goal-wiring + main.ts continuation), with an active-goal turn-tail fragment in both system-prompt builders. Independent of Automation. Addresses review on apache#625 (jackwener BLOCKERs + Astro-Han follow-ups): - CLI interrupt now halts the loop: submitPromptToTranscript reports turn outcome (aborted/errored); pi-tui-runner gates onTurnComplete on a clean turn, mirroring the desktop turnAborted/turnError guard. An errored turn is non-continuable (no hammering a failing connection). - Visible kill switch: GoalManager onChange -> 'goal-change' session event -> desktop header pill (turn counter, warning-tinted) with one-click clear via goal:get / goal:clear IPC. Rebased onto main after apache#643 (unified Automation). - Evaluator runs on the session's own connection/model (not the global default). canContinue also rejects waiting_for_user. - Evaluator uses maxOutputTokens (AI SDK v6), raised to 1024 so model-side reasoning before the JSON verdict doesn't truncate it to empty output (found via a real-chain e2e). Unused wait_seconds dropped from the contract. - Goals are in-memory / session-lifetime by design (documented); a restart stops the loop rather than silently resuming.
|
Rebased onto @jackwener — 3 blockers1. CLI interrupt now stops the loop. Threaded the turn stop-reason out of the transcript: 2. Merge-order with #643. Done — #643 landed first; this is rebased onto 3. Visible kill switch. Also noted:
@Astro-Han — follow-ups
VerificationFull dependency-order build green · runtime 986/0 (goal 30/0) · cli 154/0 (incl. the new interrupt/error-outcome tests) · live e2e through the real Maka chain (natural phrasing arms a goal → real external evaluator judges on the session model → Ready for another look 🙏 |
CC-style external evaluator + Codex lifecycle (pause/resume/blocked, token budget, block cap, agent-settable goals). GoalManager + evaluateGoal (30s timeout, evaluator-failure neutral) + handleGoalContinuation (evaluate-first, re-entrancy guard, block cap -> stalled, token budget -> budget_limited). Wired at the turn boundary in both cli (runtime-bootstrap + pi-tui onTurnComplete) and desktop (goal-wiring + main.ts continuation), with an active-goal turn-tail fragment in both system-prompt builders. Independent of Automation. Addresses review on apache#625 (jackwener BLOCKERs + Astro-Han follow-ups): - CLI interrupt now halts the loop: submitPromptToTranscript reports turn outcome (aborted/errored); pi-tui-runner gates onTurnComplete on a clean turn, mirroring the desktop turnAborted/turnError guard. An errored turn is non-continuable (no hammering a failing connection). - Visible kill switch: GoalManager onChange -> 'goal-change' session event -> desktop header pill (turn counter, warning-tinted) with one-click clear via goal:get / goal:clear IPC. Rebased onto main after apache#643 (unified Automation). - Evaluator runs on the session's own connection/model (not the global default). canContinue also rejects waiting_for_user. - Evaluator uses maxOutputTokens (AI SDK v6), raised to 1024 so model-side reasoning before the JSON verdict doesn't truncate it to empty output (found via a real-chain e2e). Unused wait_seconds dropped from the contract. - Goals are in-memory / session-lifetime by design (documented); a restart stops the loop rather than silently resuming.
970a502 to
f7fdb08
Compare
CC-style external evaluator + Codex lifecycle (pause/resume/blocked, token budget, block cap, agent-settable goals). GoalManager + evaluateGoal (30s timeout, evaluator-failure neutral) + handleGoalContinuation (evaluate-first, re-entrancy guard, block cap -> stalled, token budget -> budget_limited). Wired at the turn boundary in both cli (runtime-bootstrap + pi-tui onTurnComplete) and desktop (goal-wiring + main.ts continuation), with an active-goal turn-tail fragment in both system-prompt builders. Independent of Automation. Addresses review on apache#625 (jackwener BLOCKERs + Astro-Han follow-ups): - CLI interrupt now halts the loop: submitPromptToTranscript reports turn outcome (aborted/errored); pi-tui-runner gates onTurnComplete on a clean turn, mirroring the desktop turnAborted/turnError guard. An errored turn is non-continuable (no hammering a failing connection). - Visible kill switch: GoalManager onChange -> 'goal-change' session event -> desktop header pill (turn counter, warning-tinted) with one-click clear via goal:get / goal:clear IPC. Rebased onto main after apache#643 (unified Automation). - Evaluator runs on the session's own connection/model (not the global default). canContinue also rejects waiting_for_user. - Evaluator uses maxOutputTokens (AI SDK v6), raised to 1024 so model-side reasoning before the JSON verdict doesn't truncate it to empty output (found via a real-chain e2e). Unused wait_seconds dropped from the contract. - Goals are in-memory / session-lifetime by design (documented); a restart stops the loop rather than silently resuming.
CC-style external evaluator + Codex lifecycle (pause/resume/blocked, token budget, block cap, agent-settable goals). GoalManager + evaluateGoal (30s timeout, evaluator-failure neutral) + handleGoalContinuation (evaluate-first, re-entrancy guard, block cap -> stalled, token budget -> budget_limited). Wired at the turn boundary in both cli (runtime-bootstrap + pi-tui onTurnComplete) and desktop (goal-wiring + main.ts continuation), with an active-goal turn-tail fragment in both system-prompt builders. Independent of Automation. Addresses review on apache#625 (jackwener BLOCKERs + Astro-Han follow-ups): - CLI interrupt now halts the loop: submitPromptToTranscript reports turn outcome (aborted/errored); pi-tui-runner gates onTurnComplete on a clean turn, mirroring the desktop turnAborted/turnError guard. An errored turn is non-continuable (no hammering a failing connection). - Visible kill switch: GoalManager onChange -> 'goal-change' session event -> desktop header pill (turn counter, warning-tinted) with one-click clear via goal:get / goal:clear IPC. Rebased onto main after apache#643 (unified Automation). - Evaluator runs on the session's own connection/model (not the global default). canContinue also rejects waiting_for_user. - Evaluator uses maxOutputTokens (AI SDK v6), raised to 1024 so model-side reasoning before the JSON verdict doesn't truncate it to empty output (found via a real-chain e2e). Unused wait_seconds dropped from the contract. - Goals are in-memory / session-lifetime by design (documented); a restart stops the loop rather than silently resuming.
CC-style external evaluator + Codex lifecycle (pause/resume/blocked, token budget, block cap, agent-settable goals). GoalManager + evaluateGoal (30s timeout, evaluator-failure neutral) + handleGoalContinuation (evaluate-first, re-entrancy guard, block cap -> stalled, token budget -> budget_limited). Wired at the turn boundary in both cli (runtime-bootstrap + pi-tui onTurnComplete) and desktop (goal-wiring + main.ts continuation), with an active-goal turn-tail fragment in both system-prompt builders. Independent of Automation. Addresses review on apache#625 (jackwener BLOCKERs + Astro-Han follow-ups): - CLI interrupt now halts the loop: submitPromptToTranscript reports turn outcome (aborted/errored); pi-tui-runner gates onTurnComplete on a clean turn, mirroring the desktop turnAborted/turnError guard. An errored turn is non-continuable (no hammering a failing connection). - Visible kill switch: GoalManager onChange -> 'goal-change' session event -> desktop header pill (turn counter, warning-tinted) with one-click clear via goal:get / goal:clear IPC. Rebased onto main after apache#643 (unified Automation). - Evaluator runs on the session's own connection/model (not the global default). canContinue also rejects waiting_for_user. - Evaluator uses maxOutputTokens (AI SDK v6), raised to 1024 so model-side reasoning before the JSON verdict doesn't truncate it to empty output (found via a real-chain e2e). Unused wait_seconds dropped from the contract. - Goals are in-memory / session-lifetime by design (documented); a restart stops the loop rather than silently resuming.
CC-style external evaluator + Codex lifecycle (pause/resume/blocked, token budget, block cap, agent-settable goals). GoalManager + evaluateGoal (30s timeout, evaluator-failure neutral) + handleGoalContinuation (evaluate-first, re-entrancy guard, block cap -> stalled, token budget -> budget_limited). Wired at the turn boundary in both cli (runtime-bootstrap + pi-tui onTurnComplete) and desktop (goal-wiring + main.ts continuation), with an active-goal turn-tail fragment in both system-prompt builders. Independent of Automation. Addresses review on apache#625 (jackwener BLOCKERs + Astro-Han follow-ups): - CLI interrupt now halts the loop: submitPromptToTranscript reports turn outcome (aborted/errored); pi-tui-runner gates onTurnComplete on a clean turn, mirroring the desktop turnAborted/turnError guard. An errored turn is non-continuable (no hammering a failing connection). - Visible kill switch: GoalManager onChange -> 'goal-change' session event -> desktop header pill (turn counter, warning-tinted) with one-click clear via goal:get / goal:clear IPC. Rebased onto main after apache#643 (unified Automation). - Evaluator runs on the session's own connection/model (not the global default). canContinue also rejects waiting_for_user. - Evaluator uses maxOutputTokens (AI SDK v6), raised to 1024 so model-side reasoning before the JSON verdict doesn't truncate it to empty output (found via a real-chain e2e). Unused wait_seconds dropped from the contract. - Goals are in-memory / session-lifetime by design (documented); a restart stops the loop rather than silently resuming.
jackwener
left a comment
There was a problem hiding this comment.
Re-review — all prior blockers resolved. Excellent turnaround.
Re-read the updated PR against the earlier review (which requested changes on abort-halt parity, missing user control, and zero test coverage of the halt path). Every point is addressed, cleanly:
1. CLI/desktop abort-halt parity — FIXED. submitPromptToTranscript now returns {aborted, errored} (pi-transcript.ts): aborted on an abort event or a user_stop completion, errored on error/failed. runAgentTurn's finally gates goal continuation on !closed && !turnOutcome.aborted && !turnOutcome.errored — real parity with the desktop !turnAborted guard. My earlier concern (the CLI's !closed guard missed double-Escape → driver.stop(), relying implicitly on canContinue's status check) is gone; the comment now correctly names double-Escape as a Stop affordance.
2. Errored-turn guard — NEW, good. Neither host re-injects into a failed turn now (!turnError/!turnOutcome.errored), so a persistently-failing turn can't be hammered by the loop. That's the right hardening beyond what I asked for.
3. Test coverage of the halt path — ADDED. packages/cli/src/__tests__/pi-transcript.test.ts pins the aborted/errored outcome that drives the guard — the exact path that was previously untested.
4. User-facing kill switch + visibility — ADDED, well-designed. My deepest concern was that all goal controls were model-facing, so a user couldn't see or stop a running autonomous loop. Now: GoalManager.onChange → 'goal-change' session event → goal:get / goal:clear IPC, surfaced by the standalone useSessionGoal hook (running-goals-only) into a header pill with a one-click stop (app-shell + chat-view). goal:clear → manager.clear → terminal cleared → getActive returns undefined → the loop halts. Decoupling it as a tiny hook "regardless of where the chat surface renders it" is the right call.
5. Merge order — resolved. Rebased onto main after #643 (unified Automation); Goal reviews standalone now.
The core engineering was already strong (external judge that never self-grades, fail-open-but-flag-neutral evaluator, iteration/stall/token caps, token re-baselining, re-entrancy guard, in-memory/restart-clears-by-design). With the human-control story now closed and tested, this is in good shape.
One non-blocking note for the record: "Stop" (interrupt / pill-stop-of-the-turn) halts the current continuation but a still-active goal resumes on the next non-aborted turn — that's the intended "pause autonomy, let me steer, then resume" semantics, and the pill's clear button is the definitive stop. Worth a line in the goal pill's tooltip so users know interrupt ≠ clear.
Approving. I'll run a clean isolated build + the runtime/cli goal suites before landing.
…mary, honest fade (round 16) (#722) Post-pull discovery sweep over the externally-merged tip (LiteLLM, Goal #625, FormatJson, tool-row rework — all reviewed clean, incl. the Goal kill-switch pill which correctly absorbed the earlier review blockers). Three finds, fixed: - 每日回顾 已生成 chip still success-green — the EXPECTED outcome per the #651 restraint rule (健康 正常 and 权限 已授权 are already neutral); failures keep destructive. - 机器人对话 had NO primary at all — 测试并连接 (the form's completion action) promoted from secondary to default. - first-run provider list: the 28px bottom fade left a half-clipped row still half-legible — read as broken clipping; 56px fully veils it. Also re-learned: the desktop test suite does not compile renderer pages (a JSX syntax error sailed through 2309 green tests and only vite/ typecheck caught it) — full typecheck now part of this round's gate. Desktop 2309/2309 + typecheck + dead-css clean; CDP captures verified.
What
P6 Goal-based autonomous execution (Issue #15) — CC-style external evaluator + Codex lifecycle:
GoalManager+evaluateGoal(30s timeout; evaluator failure treated as neutral, never blocks the turn) +handleGoalContinuation(evaluate-first ordering, in-flight re-entrancy guard, block cap →stalled, token budget →budget_limited, agent-settable goals)runtime-bootstrap+ pi-tuionTurnComplete) and desktop (goal-wiring+main.tscontinuation, gated on!turnAbortedso Stop halts the loop)Why standalone
Goal (P6) and Automation (P4) are independent primitives; keeping this PR standalone on
main(rather than stacked) makes the review surface focused and avoids repeated rebases asmainadvances.Tests