Skip to content

coop: make the agent reliably wait for developer confirmation - #1862

Open
tomelm wants to merge 11 commits into
stripe:coopfrom
tomelm:tomer/coop-await-pingpong
Open

coop: make the agent reliably wait for developer confirmation#1862
tomelm wants to merge 11 commits into
stripe:coopfrom
tomelm:tomer/coop-await-pingpong

Conversation

@tomelm

@tomelm tomelm commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

The agent frequently failed to wait for, or wake up on, the developer's confirmation — worst with cheaper models. The cause is a timeout race, not prompting.

await-review blocked for five minutes and asked agents to allow a six-minute shell timeout. That only holds if the model actually sets one. Claude Code's Bash tool defaults to 120s and its override env vars have open bugs, so a model that does not set a timeout has the command killed at two minutes — before the internal deadline — and receives a raw harness string with no ok and no next instead of structured JSON. Cheap models respond by assuming the step passed, or by stopping to ask a question in a pane the developer is not watching.

Each call now waits at most AwaitTimeout (45s, under the harness default) and returns state: "waiting" with advance_allowed: false and a next repeating the exact command just run. The developer is not rushed: review state lives in the session file, so the total wait stays unbounded across calls.

next-action was untouched by the earlier review-wait work and still had the original bug in its worst form — a ten-minute block that returned ErrSelectionTimeout, surfacing as JSON on stderr with a non-zero exit. An agent reads that as a broken session and gives up.

Relationship to #1826

#1826 fixed the same class of problem by tagging the tmux agent pane and typing a resume prompt into it. That mechanism is removed here: it only works when Co-op launched the agent under tmux, and driving another program's input is not something to depend on. stripe coop agent resume is kept — it is read-only, types nothing, and is now the shared lifecycle query behind both the Stop hook and manual recovery.

AwaitTimeout / AwaitHarnessTimeout and the harness-headroom invariant from #1826 are kept, retuned to 45s / 90s so the interval sits under the harness default rather than only under an advertised timeout.

Also fixed

Races that are reachable today and become routine once calls are frequent and short:

  • Requesting changes between two calls moved the node back to active, so AwaitReview fell through to alreadyMovedResponse, whose next pointed at the following pending task. The agent skipped the rejected work and never saw the feedback.
  • ShowSuggestions cleared NextSteps.Selected unconditionally, erasing a choice the developer made while the previous call was exiting.
  • Confirming the final task completed the session, and the next await-review then failed requireActiveSession — non-zero exit, empty stdout, recovery pointing at status rather than next-action. That is the end of every session.

Stop hook

Returning on an interval removes the hard failure, but a model can still stop mid-session and the CLI cannot force it back. A Stop hook can: it fires exactly when the agent tries to end its turn. It asks resume what the session needs rather than recomputing the lifecycle, and blocks with that exact command.

Injection is via launch flags only — claude --settings and codex -c — so nothing is written into the user's repo. Bounded by a 30-minute wall-clock window (what it detects is an absent developer; attempt count is a poor proxy), with a count ceiling as a runaway guard. When it does release the agent stops for good, so the TUI now reports the exact stripe coop agent resume command to paste, and keeps it on screen.

Test plan

  • make test (52 packages), go test -race ./pkg/coop/... ./pkg/cmd/coop, make lint — all clean
  • Real end-to-end with Claude Code and Codex in tmux, driving actual sessions to review
  • Verified the ping-pong loop by the elapsed counter resetting on a ~45s cycle across four await-review calls, then confirming from the TUI and watching the agent advance within seconds
  • Verified the Stop hook fires in a live session by instructing the agent to stop: it was blocked and pulled back into the lifecycle
  • Verified crash recovery (SIGKILL mid-await leaves no lock; next call works), the completion boundary, and the rejection path
  • TUI frames rendered at 69/100/120 columns for the new status line

Three adversarial reviews found nine defects, fixed in the last four commits — including an escape hatch that could never fire because the command the hook ordered bumped the version the budget keyed on, and a Codex auto-approve path that silently dropped the hook.

Notes for review

  • Codex gates hooks behind an interactive trust prompt ("Hooks need review"); until the developer accepts it the hook is installed but inert and Co-op falls back to the interval alone. Claude Code loads settings-provided hooks directly.
  • The 30-minute window is a judgement call, informed by "long enough to click through a checkout" rather than data. In both real-agent runs the hook never fired naturally — the models stayed in the loop on their own — so this layer may be insurance rather than a hot path.
  • A stale stripe earlier on PATH silently reintroduces the old behavior, since agent instructions invoke bare stripe. Worth a version check in coop run as a follow-up; not included here.

🤖 Generated with Claude Code

@tomelm
tomelm requested review from a team as code owners July 30, 2026 22:26
return outputStopHookDecision(out, stopHookDecision{})
}

resume, err := resumer.Resume(session.ID)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we store if the developer selects Finish? Right now the agent runs stripe coop stop, but this hook then checks the completed session, receives another next-action command. and prevents the agent from existing.

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.

Good catch — reproduced exactly as you describe, and fixed in bfa8070e.

The gap was that nothing recorded the Finish choice. Every other next action reports back through --completed=<id> when its follow-up finishes; Finish has no follow-up, so consuming the selection is the only chance to record it. Without that the session sat as "completed with a next action still owed" forever, Resume kept answering with the next-action command, and the hook kept blocking — so the agent could never exit and the suggestions would reappear for the developer.

Two changes:

  • consumeSelection records done (coop.FinishActionID) into NextSteps.Completed when that is what the developer picked.
  • Resume returns no next for a finished session, which is already the hook's "nothing to do" signal, so it allows the stop.

Verified end to end against a real session:

1. next-action -> next = stripe coop stop --session=coop_06c23062
2. coop stop   -> state = completed
3. stop-hook   -> {}          # allow, agent exits

Step 3 previously returned decision: "block" with another next-action command.

The case that must not regress — a completed session where the developer has not chosen Finish — still blocks with next-action. Both directions are covered by TestResumeReportsNothingToRunAfterDeveloperFinishes, TestResumeStillOffersNextActionBeforeFinish, and TestRunRecordsTheFinishSelection.

Note the branch has been rebased onto the rewritten coop (it had been rewritten onto newer master, which was making this PR show a 102-file diff). The fix above was re-verified after that rebase, since it landed on a rewritten resume.go/nextaction.go.

tomelm added a commit to tomelm/stripe-cli that referenced this pull request Aug 13, 2026
Selecting Finish hands the agent "stripe coop stop", which completes the
session. Resume answered any completed session with the next-action command, so
the stop hook then held the turn open and handed back next-action again — the
agent could never exit, and the developer would see the post-completion
suggestions reappear.

Nothing recorded the Finish choice. Other next actions report back through
--completed when their follow-up finishes; Finish has no follow-up, so
consuming the selection is the only chance to record it. It is now written to
NextSteps.Completed there, and Resume treats a finished session as having
nothing left to run.

Reported by @kwu-stripe on stripe#1862.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tomelm and others added 11 commits August 13, 2026 16:13
The tmux resume path sent literal keystrokes into the agent's TUI after a
review decision — a prompt plus Enter, submitted as a turn on the agent's
behalf. That only works when Co-op launched the agent under tmux, and driving
another program's input is not a mechanism we want to depend on.

Removed: tmuxAgentResumer, the @stripe_coop_agent pane tagging that existed
only so the resumer could find the pane, and the TUI ReviewDecisionNotifier
wiring. Panes are split plainly again.

"stripe coop agent resume" is kept. It is read-only, types nothing, and remains
useful as a manual recovery command when an agent loses track of the lifecycle.

Review decisions now reach the agent solely through its own await-review call,
which the following commits make survive an agent harness's command timeout.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two paths could silently lose a developer decision that landed while the agent
was between commands. Both are reachable today and become routine once
await-review returns on an interval.

await-review: requesting changes moves the node out of review and back to
active with a rejection note. AwaitReview only special-cased the in-review
state, so the next call fell through to alreadyMovedResponse, whose next points
at the following *pending* task. The agent skipped the rejected work and never
saw the feedback. It now reports the rejection, reusing rejectedStepResponse.
The check requires a rejection note so a step with one task in review and
another legitimately active is not misread as a rejection.

next-action: ShowSuggestions unconditionally clears NextSteps.Selected, so a
choice the developer made while the previous invocation was exiting was erased
by the next one and the agent waited on a selection that no longer existed. Run
now consumes a pending selection before republishing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
await-review blocked for five minutes and asked agents to allow a six-minute
shell timeout. That only holds if the model actually sets one. Claude Code's
Bash tool defaults to 120s and its override env vars have open bugs, so a model
that does not set a timeout has the command killed at two minutes — before the
internal deadline — and receives a raw harness string with no ok and no next
instead of structured JSON. Cheap models respond by assuming the step passed or
by stopping to ask a question in a pane the developer is not watching.

Each call now waits at most AwaitTimeout (45s, under the harness default) and
returns state "waiting" with advance_allowed false and next repeating the exact
command just run. The developer is not rushed: review state lives in the session
file, so the total wait stays unbounded across calls. AwaitHarnessTimeout drops
to 90s and is still advertised as wait_timeout_seconds.

advance_allowed becomes the single field an agent branches on, and the waiting
message deliberately avoids failure vocabulary — a response reading as an error
is what makes models abandon the loop.

The heartbeat is now kept while waiting and cleared on every other outcome.
Clearing it each interval would make the TUI report the agent idle during the
gap between calls on any review longer than two minutes.

A keep-alive line goes to stderr every ten seconds so the wait is never a silent
process; stdout stays a single JSON document.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
next-action blocked for ten minutes and then returned ErrSelectionTimeout,
which reached outputCoopError and left the agent with JSON on stderr and a
non-zero exit. An agent that sees a non-zero exit treats the session as broken
and gives up — exactly wrong when the developer is merely still deciding. This
was the original bug in its worst form, and the review-wait work left it
untouched.

waitForSelection now waits one NextActionInterval (45s, matching await-review's
reasoning about harness defaults) and reports an unmade choice as an empty
selection rather than an error. Run turns that into a waiting response carrying
state "waiting", advance_allowed false, and a next that repeats the invoked
command, so the command exits zero and the agent knows to run it again.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Returning await-review on an interval removes the hard failure, but a model can
still decide to stop mid-session and the CLI has no way to force it back. A
Stop hook does: it fires exactly when the agent tries to end its turn, which is
precisely when it has drifted.

The hook asks "stripe coop agent resume" what the session needs rather than
recomputing the lifecycle. Resume already handles aborted and completed
sessions, rejected work, steps ready for review, and the next pending task, and
its empty next is a ready-made "nothing to do" signal. When it yields an
actionable command the hook blocks and hands that exact command back, so the
agent gets the real next step instead of an instruction to go look one up.

This also gives resume a purpose again now that nothing types into the agent
pane: it is the shared lifecycle query behind both the hook and manual recovery.

Injection is via launch flags only — claude --settings and codex -c — so
nothing is written into the user's repo and there is nothing to clean up.
Agents Co-op did not launch get no hook and rely on the interval alone. Codex
gates hooks behind an interactive trust prompt, so its hook stays inert until
the developer accepts it; that is noted in the code and docs.

Blocking forever would burn tokens whenever the developer walks away, so the
hook stops blocking after three consecutive blocks with no session progress and
lets the turn end, leaving the TUI's idle state to prompt a rejoin.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adversarial review of the previous commit turned up several ways the hook could
burn tokens indefinitely or act on the wrong session.

The block budget was keyed on the session version, but the very command the hook
orders can bump it: next-action republished its suggestions on every call, so
the counter reset each round and the escape hatch could never fire. Verified
before the fix — eight consecutive Stop events all reported block/1. The budget
now keys on the pending command, and ShowSuggestions skips a no-op republish so
the version stops churning at all.

Releasing also cleared the counter, so the next Stop event started from zero and
blocked three more times; the agent could never actually stop. The exhausted
budget is now persisted until the lifecycle moves on.

A failed counter write was swallowed, which left the loop unbounded with no
release — the opposite of the fail-open the comments claimed. It now allows the
stop.

The discovery-mode fallback resolved "latest active session" from a
machine-global directory. With two worktrees running Co-op, or one abandoned
session from an earlier run, the hook could hand agent A a command targeting
session B and mutate it. Sessions do not record which directory they belong to,
so the fallback now requires exactly one active session, recently updated, and
otherwise fails open.

Codex auto-approve replaced the injected hook flag instead of appending to it,
so the mode most sessions use launched with no Stop hook at all. Both launcher
paths now have auto-approve coverage.

Also: drain the harness event payload before any early return, register the
command's error handling like its siblings, address the tmux pane by id rather
than a hardcoded index that breaks under pane-base-index, and clean up sidecar
files on session delete.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three defects in the lifecycle responses, all reachable now that await returns
every 45s instead of every five minutes.

The rejection check keyed on RejectionNote alone, but the note survives until a
task reaches Done — it is not cleared when the task is re-activated for a redo.
So once the agent obeyed a rejection and ran start-work, any further await call
reported the same rejection again and sent it back to start-work on work already
in flight. Resume had this right, requiring an empty Activity; both sites now
share isUnstartedRejection so they cannot drift apart again.

Confirming the final task completes the session, and the next await then failed
requireActiveSession: non-zero exit, empty stdout, JSON on stderr, and a
recovery hint pointing at status rather than next-action. That is the end of
every session, and the agent is under instructions to keep re-running await, so
it would routinely read the finish line as a broken session. It now routes
through Resume, which hands back the next-action command; a genuinely aborted
session still errors.

Resume's "task is already active" branch carries no next, so advertising
advance_allowed true promised a command that was not there. It now leaves the
field unset.

Rejecting two tasks in one step left the agent with "stripe coop status" and
nothing to run, because nextInStepOrStatus only looked for pending siblings. It
now also offers a sibling still carrying unstarted feedback.

Finally, the heartbeat is removed on every await return again. Retaining it
between calls did not prevent the TUI reporting the agent idle — that check
falls through to the last session change, which is frozen for the whole review —
and it left a killed agent indistinguishable from a patient one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The prompt claimed "every response carries advance_allowed". It does not —
start-work, report-check, the session bootstrap, stop, and every next-action
outcome omit it, and error responses carry their command under recovery rather
than a top-level next, which the field's contract never mentioned. A model told
the field is always present has no rule for its absence. The prompt now states
the false case and says any other response is followed normally, and points at
recovery for failures.

It also told the agent to run "stripe coop agent resume" with no session, which
exits non-zero — a regression from when the TUI supplied the full command. It
now shows the flag with the session_id every response carries.

README fixes: next-action and await-review are no longer described as blocking,
the five-minute waiter is gone, and wait_timeout_seconds is documented as
carrying the interval rather than the harness timeout, which is prompt-only and
never on the wire.

Test fixes, all cases that could not fail: the discovery-mode assertion matched
a substring that never occurs even with a session; the cumulative-wait test
passed equally for per-process timing, so it now backdates the review entry and
asserts a span no in-process timer could produce, plus the review prompt it was
named for; and the Codex TOML test claimed verification while only checking a
prefix, so it now actually parses the value.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The block budget counted consecutive drift attempts, which is a poor proxy for
what it is actually detecting: a developer who has walked away. At three
attempts an agent drifting on every await call exhausted it in about three
minutes, well inside a genuine review — someone clicking through a checkout in a
browser easily takes longer.

The budget is now primarily wall-clock: the hook keeps holding turns until
stopBlockWindow (30 minutes) passes with the lifecycle unchanged.
maxConsecutiveStopBlocks stays as a runaway guard for an agent that re-stops
immediately without waiting, where turns would otherwise burn far faster than
the window anticipates. Both restart when the pending command changes, so a
healthy long session is never cut off.

That matters more than it used to because releasing is now terminal. With the
tmux typing gone, a stopped agent never sees a later confirmation: no turn is
running, so no Stop event fires and the hook cannot help. The TUI now closes
that loop. A review decision made while no agent is waiting reports the exact
"stripe coop agent resume --session=<id>" to paste into the agent pane, and
that status persists rather than expiring like an acknowledgement, because it
is an action the developer has to take.

The status line wraps now: it can carry a command, which is longer than the
short acknowledgements it used to hold and would have overflowed the narrow
layout. Verified by rendering frames at 69, 100, and 120 columns.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Selecting Finish hands the agent "stripe coop stop", which completes the
session. Resume answered any completed session with the next-action command, so
the stop hook then held the turn open and handed back next-action again — the
agent could never exit, and the developer would see the post-completion
suggestions reappear.

Nothing recorded the Finish choice. Other next actions report back through
--completed when their follow-up finishes; Finish has no follow-up, so
consuming the selection is the only chance to record it. It is now written to
NextSteps.Completed there, and Resume treats a finished session as having
nothing left to run.

Reported by @kwu-stripe on stripe#1862.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The old maxConsecutiveStopBlocks doc was left stranded above stopBlockWindow
when the budget became wall-clock bound, describing a limit that no longer
exists and a TUI idle path that no longer applies.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tomelm
tomelm force-pushed the tomer/coop-await-pingpong branch from b1acaa3 to 9129360 Compare August 13, 2026 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants