Skip to content

Negotiate cell execution start and resume#282

Merged
jlewi merged 7 commits into
mainfrom
codex/fix-stale-cell-execution
Jul 22, 2026
Merged

Negotiate cell execution start and resume#282
jlewi merged 7 commits into
mainfrom
codex/fix-stale-cell-execution

Conversation

@jlewi

@jlewi jlewi commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • negotiate explicit START or RESUME intent as the first WebSocket application message
  • wait for the runner acknowledgement before releasing execute requests or starting heartbeats
  • restore a persisted active cell by resuming its saved runID
  • mark the execution unknown only when the runner returns authoritative NOT_FOUND
  • preserve the running state and keep reconnecting when the runner is unreachable
  • document the protocol semantics, compatibility matrix, recovery behavior, runner-first rollout, and future terminal-state retention

Root cause

The old runner could not distinguish a new execution from a reconnect. Connecting with an unknown historical runID created an empty multiplexer that answered heartbeats but had no process and would never emit an exit event. A persisted PID therefore left the cell displaying as running indefinitely.

Behavior

runID continues to identify the execution; each WebSocket gets a fresh streamID.

  • New run: send START, expect CREATED.
  • Reconnect or browser refresh: send RESUME, expect RUNNING.
  • Missing/expired run: receive NOT_FOUND, clear the stale PID, and record unknown.
  • Unreachable runner: retain the PID and running state and keep reconnecting because transport failure is not a terminal execution result.
  • Legacy clients remain supported by the new runner’s existing create-or-attach path.

Users may select another runner and rerun the cell, which creates a new runID and stops monitoring the old one. If the old runner remains unreachable, the client cannot guarantee that its process stopped. A future explicit abandon action can clear local running state without claiming remote cancellation.

Completed terminal results are not retained in this version. If a run finishes while the browser is disconnected, reconnect returns NOT_FOUND; retaining and replaying the exit result is documented as a future protocol extension.

Coordinated runner change: runmedev/runme#1247

Design doc: https://github.com/runmedev/web/blob/codex/fix-stale-cell-execution/docs-dev/design/20260715_execution_monitoring.md

Validation

  • pnpm --filter @runmedev/renderers exec vitest run src/streams.test.ts
  • pnpm --filter @runmedev/app exec vitest run src/lib/notebookData.test.ts
  • runme run build test

@jlewi
jlewi force-pushed the codex/fix-stale-cell-execution branch from 76c7d67 to 9a41930 Compare July 18, 2026 22:55
@jlewi jlewi changed the title Fix stale running state after execution monitor loss Negotiate cell execution start and resume Jul 18, 2026
sourishkrout pushed a commit to runmedev/runme that referenced this pull request Jul 22, 2026
## Summary

- add a first-message `OpenRunRequest` negotiation with explicit `START`
and `RESUME` intent
- return authoritative `CREATED` or `RUNNING` acknowledgements
- reject duplicate starts with `ALREADY_EXISTS` and missing resumes with
`NOT_FOUND`
- preserve the existing create-or-attach behavior for legacy clients
that do not negotiate
- add runner coverage for start, resume, missing resume, duplicate
start, and legacy behavior

## Why

The previous WebSocket contract used an unknown `runID` for both a
genuinely new execution and a reconnect to a forgotten execution. On
reconnect, a completed or lost run could therefore create an empty
multiplexer that answered heartbeats but never emitted an exit event,
leaving the web UI stuck in a running state.

Explicit intent removes that ambiguity without breaking older clients.
This is intended for a runner-first rollout.

Coordinated web change: runmedev/web#282

Design doc:
https://github.com/runmedev/web/blob/codex/fix-stale-cell-execution/docs-dev/design/20260715_execution_monitoring.md

## Validation

- `go test -timeout 120s ./pkg/agent/runme/stream`
- `runme run lint`
- targeted protobuf generation for
`api/proto/runme/stream/v1/websockets.proto`

`runme run test` was also attempted. It reaches unrelated
environment-dependent failures in the existing suite because Dagger/Deno
tooling is unavailable and the local shell startup prints a `z` upgrade
notice. The changed stream package and all new negotiation tests pass.

---------

Signed-off-by: Jeremy lewi <jeremy@lewi.us>
jlewi added 6 commits July 22, 2026 14:43
Signed-off-by: Jeremy lewi <jeremy@lewi.us>
Signed-off-by: Jeremy lewi <jeremy@lewi.us>
Signed-off-by: Jeremy lewi <jeremy@lewi.us>
Signed-off-by: Jeremy lewi <jeremy@lewi.us>
Signed-off-by: Jeremy lewi <jeremy@lewi.us>
Signed-off-by: Jeremy lewi <jeremy@lewi.us>
@jlewi
jlewi force-pushed the codex/fix-stale-cell-execution branch from 75e7c6e to 471fa6b Compare July 22, 2026 21:44
@jlewi
jlewi marked this pull request as ready for review July 22, 2026 21:44

@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: 471fa6b31a

ℹ️ 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 +391 to +392
negotiationComplete = true
this.nextIntent = RunIntent.RESUME

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Retry lost START acknowledgements as RESUME

If the first socket drops after the runner creates the run but before the browser receives RUN_STATE_CREATED, nextIntent is still START; the reconnect sends START for the same runID, which the new protocol reports as ALREADY_EXISTS, and the status path then treats it as terminal instead of attaching. In that race the queued execute request is never released, leaving an empty/orphaned run and a cell that stays locally running without output; handle ALREADY_EXISTS after a START attempt by retrying with RESUME or otherwise making the start handshake idempotent.

Useful? React with 👍 / 👎.

Signed-off-by: Jeremy lewi <jeremy@lewi.us>
@jlewi
jlewi merged commit 18b939f into main Jul 22, 2026
3 checks passed
@jlewi
jlewi deleted the codex/fix-stale-cell-execution branch July 22, 2026 22:15
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.

1 participant