Skip to content

fix: control Codex goals through the ACP extension - #554

Open
lodystage[bot] wants to merge 7 commits into
mainfrom
fix/debug-codex-goal-resume-bug
Open

fix: control Codex goals through the ACP extension#554
lodystage[bot] wants to merge 7 commits into
mainfrom
fix/debug-codex-goal-resume-bug

Conversation

@lodystage

@lodystage lodystage Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Related issue

Problem / pressure

An active Codex goal holds the session's only ACP v1 prompt open across the agent's own continuations — that is what GoalPromptLifecycle (#465) is for, and it is what keeps a goal's turns attributable to one conversation entry.

Lody's goal controls were chat messages. Pressing Pause or Resume wrote a pending user turn (/goal pause, /goal resume) and asked the CLI to dispatch it, but resolveSessionDispatchAction returns noop('active-session') whenever a turn is active, so the turn sat pending until the goal's own prompt closed. Meanwhile the banner's pendingGoalCommand only cleared when the goal status actually changed, with no timeout, and it disabled every goal button.

The user-visible result: a goal shows paused, Resume does nothing, and the only way out is pressing Stop. Pause had already accumulated two compensations for the same root cause — the Stop button sends its own /goal pause, and the adapter pauses the goal itself when a prompt is cancelled — both of which exist because the bridge could not deliver a command during a prompt.

Summary

Goal actions now split by what they do, not by which agent is running.

  • pause / clear move durable state only. They go out of band on the _lody/session/goal extension request and take effect mid-prompt. No turn, no prompt slot, no queue.
  • set / resume start work, and ACP v1 gives a client exactly one way to own running work: its own prompt. They ride a Lody-owned turn carrying _meta.lody.goalControl, so no command text enters the conversation while the resulting turns still belong to an assistant entry the user can see and cancel.
  • An action that arrives while a turn is draining waits for that turn (waitForTurnRelease) instead of being dropped. One queued action per session, newest wins, bounded to three turn waits.
  • The banner reads the runtime's advertised goalActions instead of agentType === 'codex', and its pending state expires after 60s so a slow action cannot leave the controls dead.

A goal turn creates an assistant entry with no user message and carries no run configuration, so resuming a goal cannot silently change model or mode.

New machine RPC session/goal, subject to the same session access verification as a chat message.

Depends on two submodule PRs (see Submodules below). The pointers here currently reference their branch heads and must be re-pinned to the merged commits before this merges, the same way #465 pinned its adapter commit.

Visual explanation

Why the two transports are not symmetric:

sequenceDiagram
    participant UI
    participant CLI
    participant Agent

    Note over Agent: goal active — one prompt open across N native turns

    UI->>CLI: session/goal {pause}
    CLI->>Agent: _lody/session/goal (out of band)
    Agent-->>UI: goal snapshot: paused
    Note over Agent: current native turn drains, prompt returns

    UI->>CLI: session/goal {resume}
    alt a turn is still draining
        CLI-->>UI: accepted, disposition=queued
        Note over CLI: waitForTurnRelease, newest action wins
    end
    CLI->>Agent: prompt(_meta.lody.goalControl={resume})
    Note over CLI,Agent: Lody owns this prompt, so the goal's<br/>turns attach to an assistant entry
Loading

Transport selection (apps/cli/src/agent/goal-control.ts):

resolveGoalActionTransport(capability, action)
├─ action not in capability.actions ......................... null (hidden in UI)
├─ controlActions has it AND action is status-only .......... 'request'     pause | clear
├─ promptActions has it ..................................... 'promptMeta'  set | resume
└─ neither list advertised (pre-split runtime) .............. 'slashCommand'

Before / after

Before After
Pause/Resume dispatch /goal … as a user turn Pause/Clear are an out-of-band ACP request; Set/Resume are prompt metadata
Any goal action during a running prompt is deferred by guard-noop-active-session Status actions land mid-prompt; work actions queue on the turn and then run
Resume on a paused goal does nothing until the user presses Stop Resume runs as soon as the draining turn releases, with no user intervention
One press disables every goal button indefinitely Pending state clears on the goal snapshot or expires after 60s
/goal resume appears in the transcript Nothing is written to the conversation
Goal buttons gated on agentType === 'codex' Gated on the runtime's advertised goalActions

Test plan

Ran:

  • pnpm typecheck (whole workspace), pnpm lint, pnpm lint:i18n, pnpm check:public-boundary, pnpm check:platform-boundaries, pnpm check:code-collab-imports, pnpm format — all pass.
  • pnpm run docs check — no errors; the new Spec and note are draft/implemented with translation pending.
  • @lody/shared 1051 tests, @lody/loro-streams-rpc 112, lody (CLI) 2639 — pass.
  • Codex adapter goal suites (37 tests) pass; the full adapter suite passes except one pre-existing failure (below).

New tests:

  • Adapter: prompt metadata resumes a goal with no command text and no duplicate turnStart; the metadata parser accepts every advertised action and rejects a blank objective, an unknown action, and a future version.
  • CLI (goal-control.test.ts): transport selection prefers the request for status-only actions, keeps work-starting actions on a prompt even when the agent lists them as control actions, falls back to the slash bridge for pre-split runtimes, and refuses unadvertised actions.
  • CLI (session-execution-service.test.ts): out-of-band pause opens no turn; an unadvertised action is refused; a goal turn carries no run configuration; a resume queued behind a draining turn runs when the turn is released (driven by clearing the current turn — no timers, no sleeps); a newer action supersedes an older queued one.
  • Components: goal commands derive from advertised actions, including a partial advertisement.

Not verified / known failures:

  • No live Codex session was driven through a real pause/resume end to end.
  • apps/cli/tests/gh-shim-script.test.ts fails in this sandbox (the spawned shim exits 1). Untouched by this change.
  • @lody/components test suite fails wholesale here with React.act is not a function — the runner resolves a production React build. 154 files, unrelated; the goal-specific component tests were run directly and pass.
  • Codex adapter handles review slash commands through Codex app server times out. Reproduced on the pinned pre-change sources, so it is pre-existing.
  • The managed-runtime build path that consumes a published acp-extension-core is unverified; inside this workspace the pnpm override resolves the local source.

Submodules

Both must merge before this PR's pointers are re-pinned:

  • LodyAI/acp-extension-core → branch feat/goal-control-transports (commit 9e7503c): controlActions/promptActions on the goal capability, LodyGoalPromptControl, version 0.1.2. Publish 0.1.2 before the adapter's pinned dependency resolves outside this workspace.
  • LodyAI/acp-extension-codex → branch feat/goal-prompt-control (commit 1a35bc0): accepts _meta.lody.goalControl, advertises both transports, and corrects stale claims in docs/goal-extension.md.

I could not open those two PRs: the token available to me gets Resource not accessible by integration (createPullRequest) on both repositories. The branches are pushed.

Documentation

  • specs/session-goal-control.md (draft) — intent and guarantees.
  • .agents/notes/implemented/architecture/2026-09-09-goal-control-plane.md — the decision, the rejected alternatives, and the limits.
  • packages/shared/AGENTS.md — the binding invariant.

Both documents are English-only for now (Translation: pending).

🤖 Generated with Claude Code

Context handoff

Instructions for reviewing agents

  • Review focus: apps/cli/src/agent/goal-control.ts (transport selection) and SessionExecutionService.controlSessionGoal + queueGoalTurn — the queue is the only new concurrency in this change and it shares the session's turn-ownership rules. Also CodexAcpServer.prompt's new metadata branch, which must reach exactly the same code as the slash command.
  • Decisions to challenge: that resume may not use the out-of-band request even when an agent advertises it (the adapter can self-start a turn there, which Lody cannot attribute); that a queued goal action is bounded to three turn waits and then dropped with a warning; that ACP_CAPABILITY_CACHE_VERSION bumps to 8 rather than treating a missing goalActions as "assume Codex supports it".
  • Plausible failures / evidence gaps: no live Codex goal was paused or resumed end to end. Until a machine re-probes capabilities the goal buttons hide rather than misfire, which is the safe direction but is a visible regression window. A goal turn whose agent lacks the extension fails inside prompt(); the UI should never offer that, but the path is untested.

Authoring context

  • User goal / directives: investigate why a paused Codex goal could not be resumed without pressing Stop, then replace the /goal … prompt bridge with a real ACP control operation, defining the contract in acp-extension-core and consuming it from the Codex adapter.
  • Constraints / non-goals: every unit of agent work keeps an assistant entry it can be attributed to; no new provider-name branching; /goal stays available for other ACP clients. Not in scope: a first-class UI for setting a goal, and any other agent implementing the extension.
  • Risk-bearing decisions: the new session/goal machine RPC (same access verification as chat); the capability cache version bump, which forces a re-probe; the goal turn deliberately carrying no run configuration.
  • Destructive or irreversible behavior: none. No migration, no data rewrite. A dropped queued goal action is logged and leaves durable goal state untouched.
  • Deliberately not done or tested: no live Codex run; no publish of acp-extension-core@0.1.2; the pre-existing sandbox failures listed in the test plan were not chased.
  • Unknowns / confidence: high on the transport split and the CLI queue (covered by deterministic tests); lower on real-world timing, since the original bug depended on how long a native turn drains after a pause.

Pausing or resuming a goal was delivered as a chat message (`/goal pause`,
`/goal resume`), so it needed the session's ACP prompt slot — the same slot the
running goal holds open across the agent's own continuations. The dispatch
watcher deferred those turns with `guard-noop-active-session` and the banner
disabled every button while it waited, so a paused goal's Resume button did
nothing until the user pressed Stop.

Goal actions now split by what they do. `pause` and `clear` move durable state
only, so they travel out of band on `_lody/session/goal` and land while a prompt
is running. `set` and `resume` start work, so they ride a Lody-owned turn
carrying `_meta.lody.goalControl`: no command text in the conversation, and the
resulting turns still belong to an assistant entry the user can see and cancel.
An action that arrives while a turn is draining waits for that turn instead of
being dropped, newest action wins, and the banner's pending state now expires.

Goal buttons follow the runtime's advertised `goalActions` instead of an
`agentType === 'codex'` check, so any agent implementing the extension gets them.

Model: claude-opus-5

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Remove redundant goal fields, helpers, waiter values, and test details; update the Codex adapter pointer and document the ablation matrix and remaining P1 findings.

Validation: targeted tests, full typecheck and lint, formatting, docs, and public boundary checks passed. Full check was stopped after reproducing the known component act failure.

Model: gpt-5
Restore the pre-review formatting; no behavior changes.

Model: gpt-5
Update acp-extension-codex from d7969e6 to merged PR #39 commit 33d897b.

Validation: 161 targeted tests and docs check passed; pnpm format completed. Codex typecheck fails because the current Core pointer lacks LodyWorktreeProject and worktreeProject capabilities. Full pnpm check timed out. The Core integration remains unresolved; this commit only updates the requested pointer.

Model: gpt-5
Preserve goal capabilities alongside independent plan mode and pin Core to a merge containing both goal transport and worktree project contracts. Retain merged Codex PR #39 and upstream prompt ownership fixes.

Validation: Core build, workspace typecheck and lint, 205 targeted tests, docs and public-boundary checks passed. pnpm check was terminated at five minutes during tests; the full suite did not complete.

Model: gpt-5
Pin Core to its 0.1.4 release and Codex to the matching dependency update. Record successful registry-only installation, Codex typechecks, and 52 targeted tests.

Validation: workspace typecheck, formatting, and docs checks passed. Full pnpm check timed out at two minutes during lint; full-suite success is not claimed.

Model: gpt-5
Update the gitlink to merged PR #40 (5f0aab0). Its tree is identical to the previously validated ce7b0e8 commit; reuse the registry-backed typechecks and 52 passing tests. Docs check and diff check passed; full checks were not rerun for this pointer-only change.

Model: gpt-5
@Leeeon233
Leeeon233 marked this pull request as ready for review September 10, 2026 08:44
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-10T08:52:25.377341Z 925094a Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 925094aec4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1116 to +1120
const response = await this.deps.controlSessionGoal({
sessionId: request.params.sessionId as SessionId,
action: request.params.action,
...(request.params.objective ? { objective: request.params.objective } : {}),
userId: request.params.userId,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Acknowledge goal turns before awaiting the persistent prompt

When resume or set arrives after the previous turn has drained, this awaits controlSessionGoal before sending the RPC response. That call awaits startGoalTurncontinueSession → the goal's persistent agentClient.prompt, while both goal RPC clients time out after 10 seconds. A normally resumed goal can therefore start successfully but still be reported as failed, with the server responding only when the long-running goal eventually stops. Start the turn asynchronously after ownership is established and return turn_started immediately.

AGENTS.md reference: packages/loro-streams-rpc/AGENTS.md:L48-L55

Useful? React with 👍 / 👎.

Comment on lines +1268 to +1269
// No live agent, or an action that needs a turn: the turn boots the session
// when necessary and lets the agent client pick its transport at prompt time.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve request transport when restoring a status-only action

When a daemon restart or memory eviction leaves a durable goal but no live Session, pause and clear enter this fallback and become prompt turns. After the ACP client is restored, buildGoalControlPrompt resolves these advertised actions to request and throws because it only accepts prompt metadata or the legacy slash bridge, so an active persisted goal can no longer be paused or cleared. Restore the session and then invoke controlGoal through _lody/session/goal instead of putting status-only actions into a prompt.

AGENTS.md reference: packages/shared/AGENTS.md:L78-L83

Useful? React with 👍 / 👎.

Comment on lines +1321 to +1325
if (this.getExecutionSnapshot(sessionId).hasActiveTurn) {
this.deps.logger.warn(
`[${sessionId}] Dropping queued goal ${pending.control.action}: session stayed busy`
);
return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep accepted queued goal actions until execution

If three successive turns acquire the session before the queued resume or set can start, this deletes and silently drops the action after the caller already received accepted: true with disposition: 'queued'. That contradicts the guarantee in specs/session-goal-control.md that a work-starting action waits and is not dropped, and the caller has no completion channel or durable retry after its pending UI state expires. Keep accepted work queued durably until it executes, or avoid acknowledging responsibility before that guarantee can be maintained.

AGENTS.md reference: AGENTS.md:L12-L17

Useful? React with 👍 / 👎.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants