feat(stack): add lazy service lifecycle - #6070
Conversation
![depthfirst-app[bot]](https://avatars.githubusercontent.com/in/1021002?s=60&v=4){kind=small}
| }; | ||
| daemonState = state; | ||
| await Effect.runPromise(stateManager.write(state)); | ||
| await Effect.runPromise(localStateManager.claim(state)); |
There was a problem hiding this comment.
🟡 Severity: MEDIUM
Claiming the live-state file before startup is acknowledged persists secretKey, serviceRoleJwt, and the daemon socket path. If later initialization or IPC fails, the catch path disposes runtimes and releases ports but never removes that claim, leaving credentials and stale ownership metadata on disk.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: The fix requires changes at multiple locations in runDaemon:
- Before the
tryblock (around line 54, alongsideportLease): Hoist the state manager and a claim tracker out of thetryscope so they are accessible in thecatchblock:
let localStateManager: Awaited<ReturnType<typeof appRuntime.runPromise<StateManager>>> | undefined;
let claimedStateName: string | undefined;- Line 84: Change
const localStateManagerto assign to the outer variable:
localStateManager = await appRuntime.runPromise(StateManager);- After line 114 (right after the successful
claimcall): Record that a claim was made:
await Effect.runPromise(localStateManager.claim(state));
claimedStateName = state.name; // track for catch-block cleanup- In the
catchblock (around line 131-133, aftershutdownDaemon): Add cleanup of the on-disk claim before releasing ports and exiting:
if (localStateManager !== undefined && claimedStateName !== undefined) {
await Effect.runPromise(localStateManager.remove(claimedStateName)).catch(() => {});
}This mirrors the existing pattern used for portLease and ensures that if any step after claim() fails (IPC process.send!, DaemonServer initialization, etc.), the on-disk state file containing secretKey, serviceRoleJwt, and socket path is removed before the process exits, preventing credential and ownership-metadata leakage from failed daemon startups.
| * Sends POST /stop to the daemon's Unix socket and waits for it to exit. | ||
| * The daemon owns its own state cleanup; this function only removes stale | ||
| * state after confirming the process is no longer alive. | ||
| * Removes the live-state pointer only after confirming the process is no |
There was a problem hiding this comment.
🟡 Severity: MEDIUM
Removing state by stack name after observing the original PID exit is not generation-safe. A replacement daemon can claim the same state.json between the liveness check and cleanup; the stop command then deletes the replacement's ownership record, allowing further starts to bypass the single-daemon guard and leaving stale control metadata.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: Before calling stateManager.remove(state.name) at line 215, perform a generation check: read the current state.json for the same stack name and compare its pid field against the original state.pid. Only proceed with the remove if the PIDs match (or if no state file exists, meaning it was already cleaned up). If the PIDs differ, a replacement daemon has already claimed ownership of state.json between the liveness-poll exit and the cleanup step — in that case, skip the remove entirely to preserve the new daemon's state record.
Concretely, replace yield* stateManager.remove(state.name); with something like:
const currentState = yield* stateManager.read(state.name).pipe(
Effect.map((s) => Option.some(s)),
Effect.catchTag("StateNotFoundError", () => Effect.succeed(Option.none())),
Effect.catchTag("InvalidStackStateError", () => Effect.succeed(Option.none())),
);
if (Option.isNone(currentState) || currentState.value.pid === state.pid) {
yield* stateManager.remove(state.name);
}This also requires importing Option from "effect" at the top of the file.
f5a641c to
b874f95
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5a641c892
ℹ️ 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".
| }), | ||
| }).pipe( | ||
| Effect.onError(() => Ref.set(phaseRef, "stopped")), | ||
| withLifecycleLock, |
There was a problem hiding this comment.
When a daemon /start call reaches a service whose health check never becomes ready (for example an infinite restart loop), this keeps the lifecycle semaphore held while waitAllReady() is pending. Since /stop also goes through stack.stop() and must acquire the same permit, supabase stop cannot interrupt or clean up the stuck startup; start the services under the lock, but wait for readiness after releasing it, like activateService does.
Useful? React with 👍 / 👎.
| yield* output.intro("Start local Supabase stack"); | ||
| yield* ensureProjectStateIgnored(projectHome.projectRoot); | ||
|
|
||
| const stackLayer = yield* daemonLayer( |
There was a problem hiding this comment.
Because daemonLayer now forks the daemon and the child claims state.json before this effect returns, any subsequent failure in command setup, such as stateManager.writeMetadata() failing on a read-only or broken .supabase directory, aborts the command without stopping that daemon. This leaves an idle background stack recorded as running even though supabase start failed; persist the metadata before forking, or add cleanup that stops the daemon on later setup failures.
Useful? React with 👍 / 👎.
|
Consolidated into #6072 to simplify review and merging. |
Consolidates the complete lazy-stack v2 implementation and its architecture hardening into one reviewable change. The implementation replaces the earlier user-space coordination protocols with explicit ownership boundaries and operating-system primitives: - centralizes Docker and native artifact policy in one service catalog - publishes complete native caches through private staging directories and atomic rename - models lifecycle intent directly on each service as inactive, running, or explicitly stopped - activates HTTP services at the existing proxy boundary while keeping direct-listener services eager - reserves real TCP ports until each service reaches its spawn boundary - gives foreground and detached stacks the same allocation, readiness, and lifecycle behavior - enables lazy startup for CLI-managed local stacks while preserving eager startup as the package default The hardening pass makes each service's stable state stream the single lifecycle coordination primitive, removes generation-specific waiter and relaunch machinery, keeps healthy requests off the global lifecycle lock, starts independent eager roots concurrently, recovers incomplete artifact-cache destinations, and makes service port mappings exhaustive. This keeps the simpler v2 architecture while closing the highest-impact concurrency, recovery, and shutdown races identified during review. Realtime remains eager because the HTTP proxy does not bridge its WebSocket traffic, and concurrent artifact downloaders may duplicate work while still publishing through an atomic winner. Supersedes supabase#6041 Supersedes supabase#6042 Supersedes supabase#6043 Supersedes supabase#6044 Supersedes supabase#6045 Supersedes supabase#6046 Supersedes supabase#6047 Supersedes supabase#6069 Supersedes supabase#6070 Supersedes supabase#6071
1 participant
Replacement stack 2 of 3, based on #6069.
Stack: #6069 → #6070 → #6071
Adds lazy startup around explicit ownership boundaries:
process-composesole ownership of desired service state and restart behaviorDormantas an explicit service state instead of parallel boolean bookkeepingThis deliberately omits the custom Realtime WebSocket bridge, managed port-lock protocol, and duplicate lifecycle intent sets from the previous stack.
Supersedes #6042
Supersedes #6043
Supersedes #6044
Supersedes #6045
Supersedes #6046