fix(runtime): live-proof fixes for readiness (#5) + diagnostics/lifecycle (#6) - #87
Conversation
…ycle (#6) Live proof on a throwaway prod account surfaced four gaps the unit tests could not (all real correctness fixes, not test-bending): 1. /ports manager signal read the Workspace ROW (store.getWorkspace), which legitimately lags at PENDING/STARTING after a cold provision returned early — false-vetoing a genuinely-live preview as blockedBy:'manager'. Now reads the manager's LIVE status (same source as /status); manager-unreachable → neutral. 2. Lifecycle trail recorded nothing: the first emitted event was RUNNING (warm create never passes through the STARTING helper) and PENDING->RUNNING was dropped as illegal, so STOPPED (from PENDING) dropped too. recordLifecycleEvent now records the GENESIS event unconditionally; only transitions BETWEEN recorded events are validated. Create route also emits STARTING on entry so a warm reopen still gets a STARTING->RUNNING->STOPPED trail. 3. captureStopDiagnostics ran AFTER managerRequest(stop) tore the pod down, so the post-mortem froze empty ports/logs. Moved it BEFORE the stop and awaited it — the only moment the last-known ports/processes/logs are still readable. Full @vibecore/api suite: 1352 passed / 0 failed. 34 focused unit tests green. 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: 5ec9d4fdbd
ℹ️ 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".
| * fully best-effort so a slow/absent agent never blocks the stop. | ||
| */ | ||
| if (diagnosticsDb) { | ||
| await captureStopDiagnostics(authorized.workspaceId, 'user.stop', 'STOPPED').catch(() => undefined); |
There was a problem hiding this comment.
Bound diagnostic collection before stopping
When the workspace agent is slow or unreachable, awaiting this call delays the actual manager stop until both agentRequest retry loops finish; with three 15-second attempts this can exceed 45 seconds, and body reads are not covered by the fetch timeout. The user-facing stop request can therefore time out while the pod continues running, so pre-stop collection needs its own short overall deadline rather than being awaited without a bound.
Useful? React with 👍 / 👎.
| if (diagnosticsDb) { | ||
| await captureStopDiagnostics(authorized.workspaceId, 'user.stop', 'STOPPED').catch(() => undefined); |
There was a problem hiding this comment.
Record STOPPED only after the manager confirms it
If the subsequent manager stop returns a genuine fault such as HTTP 500, captureStopDiagnostics has already persisted a post-mortem with finalState: 'STOPPED' and emitted the STOPPED lifecycle event even though the route rethrows and the workspace may still be running. Gather the live data before teardown if necessary, but defer persisting the final state/lifecycle transition until the stop succeeds or is confirmed already gone.
Useful? React with 👍 / 👎.
| * event would be RUNNING and STARTING would be missing. Illegal-from-current | ||
| * (e.g. RUNNING->STARTING when already running) is dropped by the machine. | ||
| */ | ||
| emitLifecycle(authorized.workspaceId, 'STARTING', 'provision.request'); |
There was a problem hiding this comment.
Emit STARTING only after start admission succeeds
For a stopped or failed workspace, this records a legal STARTING transition before the service-shutdown, billing, and quota gates below run. If any gate rejects the request (for example with SERVICE_SHUTDOWN_LIMIT_REACHED), no provisioning occurs but diagnostics permanently claim that the workspace started, corrupting the lifecycle trail; emit this only after all admission checks have passed.
Useful? React with 👍 / 👎.
Fix-forward on #86 (already merged). A live proof on a throwaway prod account surfaced four real correctness gaps the unit tests couldn't — fixed here:
/portsmanager signal read the staleWorkspacerow (lags atPENDING/STARTINGafter a cold provision returns early) → false-vetoed a live preview asblockedBy:manager. Now reads the manager's live status (same source as/status); manager-unreachable → neutral.RUNNING(warm create skips the STARTING helper) andPENDING→RUNNINGwas dropped as illegal, cascading toSTOPPEDtoo.recordLifecycleEventnow records the genesis event unconditionally; only transitions between recorded events are validated. Create route also emitsSTARTINGon entry.captureStopDiagnosticsran after teardown → empty ports/logs. Moved before the manager stop and awaited.Full
@vibecore/apisuite: 1352 passed / 0 failed. 34 focused unit tests green. Migration0081already applied in #86.🤖 Generated with Claude Code