fix(runtime): full STARTING→RUNNING→STOPPED lifecycle timeline (#6) - #88
Conversation
…ne (#6) The live proof showed only a STOPPED event persisted. Two emit-timing bugs: - STARTING was emitted at the create-route entry, BEFORE ensureRuntimeWorkspace Record inserts the Workspace row, so the workspaceId FK rejected the lifecycle insert and it was silently dropped. Moved the emit to AFTER the row exists (and after authorized.workspaceId is reassigned to the record id). - RUNNING only emitted on the create route's synchronous-reachable branch, which a COLD provision skips (it returns 'starting' early). The IDE's /status poll is where the pod is actually observed live, so emit RUNNING there when the manager reports running — and heal the Workspace row to RUNNING at the same point (it otherwise lags at PENDING/STARTING). Idempotent: repeated polls are a no-op. Net: a cold provision now records STARTING (create) -> RUNNING (status poll) -> STOPPED (stop) instead of a lone STOPPED. Typecheck clean; full api suite 1352 passed / 0 failed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 362d64e057
ℹ️ 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".
| await store | ||
| .updateWorkspaceStatus({ workspaceId: authorized.workspaceId, status: 'RUNNING' }) | ||
| .catch(() => undefined); |
There was a problem hiding this comment.
Treat DELETED manager records as stopped before persisting
When GC deletes a workspace, services/workspace-manager/src/app.ts:368 continues returning its tombstone and deleteWorkspace persists status: 'DELETED' (services/workspace-manager/src/manager.ts:1173). The status normalization above falls through to managerStatus === 'running' for that value, so this new write can change an API row previously reconciled to STOPPED back to RUNNING even though its pod and PVC are gone, reoccupying an active-workspace quota slot and emitting a false lifecycle event. Only promote the row for an explicit manager RUNNING status, and classify DELETED as stopped.
Useful? React with 👍 / 👎.
| await store | ||
| .updateWorkspaceStatus({ workspaceId: authorized.workspaceId, status: 'RUNNING' }) | ||
| .catch(() => undefined); |
There was a problem hiding this comment.
Prevent a stale status poll from undoing a completed stop
If this GET observes manager status RUNNING while a concurrent /stop request is completing, the stop handler can subsequently stop the manager and persist STOPPED, after which this unconditional update may overwrite that terminal state with stale RUNNING. This can leave a successfully stopped workspace counting against quota once the IDE closes and stops polling. Make the promotion conditional on the API row still being PENDING/STARTING, or otherwise serialize it with stop/restart transitions.
Useful? React with 👍 / 👎.
| .updateWorkspaceStatus({ workspaceId: authorized.workspaceId, status: 'RUNNING' }) | ||
| .catch(() => undefined); | ||
|
|
||
| emitLifecycle(authorized.workspaceId, 'RUNNING', 'status.running'); |
There was a problem hiding this comment.
Serialize lifecycle writes from concurrent status polls
The claimed idempotence only holds for sequential polls: two overlapping status requests can both invoke this fire-and-forget emission, both read the same last STARTING event in recordLifecycleEvent, and both append RUNNING because the read/validate/create sequence is not atomic and the schema has no uniqueness constraint. Multiple tabs or overlapping slow polls therefore produce duplicate lifecycle events and can race terminal emissions; serialize or atomically guard lifecycle transitions before emitting from a polling endpoint.
Useful? React with 👍 / 👎.
Fix-forward: the live proof showed only a lone
STOPPEDlifecycle event. Two emit-timing bugs:STARTINGwas emitted at create-route entry beforeensureRuntimeWorkspaceRecordinserts theWorkspacerow → theworkspaceIdFK rejected the insert (silently dropped). Moved to after the row exists.RUNNINGonly emitted on the create route's synchronous-reachable branch, which a cold provision skips (returnsstartingearly). Now also emitted from the/statuspoll when the manager reports running — and theWorkspacerow is healed toRUNNINGthere (it otherwise lags atPENDING/STARTING). Idempotent.Cold provisions now record
STARTING → RUNNING → STOPPEDinstead of justSTOPPED. Typecheck clean; full@vibecore/apisuite 1352 passed / 0 failed. (#5 already proven end-to-end live on the prior deploy.)🤖 Generated with Claude Code