fix(stack): reserve ports through service startup - #6045
Conversation
c0631bd to
37acb4b
Compare
e273d2d to
706b171
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 706b171d88
ℹ️ 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".
37acb4b to
8e31b5c
Compare
706b171 to
a6e0729
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6e072975c
ℹ️ 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".
8e31b5c to
0398257
Compare
a6e0729 to
fad354d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fad354d893
ℹ️ 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".
0398257 to
c18d586
Compare
fad354d to
4265a74
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4265a74ba6
ℹ️ 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".
4265a74 to
c2cdb81
Compare
c18d586 to
5fbe0b5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2cdb81572
ℹ️ 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".
5fbe0b5 to
1b41b91
Compare
c2cdb81 to
ac00e70
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac00e70644
ℹ️ 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".
1b41b91 to
2d2fccd
Compare
ac00e70 to
3ea143a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ea143a742
ℹ️ 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".
2d2fccd to
3be802b
Compare
3ea143a to
1f4a18a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f4a18a420
ℹ️ 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".
3be802b to
2fde245
Compare
d96ca0e to
86c6a70
Compare
## TL;DR Fixing the `parallelStacks.e2e.test.ts` flake that's hit CI three times now: always the same pair of `90000ms` + `60000ms` hook timeouts with nothing useful in the log. Turns out the harness could hang three ways: a stack that died cleanly never settled its promise (the exit handler only checked for non-zero), stack that wedged had no timeout at all, and if one stack failed, its healthy sibling's handle got dropped so teardown killed nothing and the leak-check spun for 60s chasing a process it couldn't reach... The spawn logic now lives in a small `spawnStandaloneStack` helper that settles on every path with the child's actual output attached, and teardown tracks children from the moment they spawn. Also fixed `terminateChildProcess` quietly burning 2×30s on an already-dead child, that alone would've recreated the afterAll timeout. Repro'd all three failure modes with stubs first, each one has a pinning test, and the real suite ran green 6 times in a row.... ## refs - Deflakes the shard-1/3 failures from the supabase#6038 and supabase#6004 CI runs - Pairs with supabase#6045 which fixes the underlying startup port race properly
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86c6a707a6
ℹ️ 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".
414d65e to
3428039
Compare
86c6a70 to
9f2f7d4
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f2f7d4a62
ℹ️ 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".
3428039 to
7304cfa
Compare
9f2f7d4 to
868f236
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 868f236aba
ℹ️ 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".
7304cfa to
a4277ce
Compare
868f236 to
1ebb475
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ebb475a62
ℹ️ 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".
a4277ce to
7e4eede
Compare
1ebb475 to
cc5effc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc5effc281
ℹ️ 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".
7e4eede to
78e2903
Compare
cc5effc to
9639149
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 963914941c
ℹ️ 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".
9639149 to
8d6ede9
Compare
78e2903 to
ac2d69a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d6ede95b5
ℹ️ 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".
| Math.abs(owner.bootMinute - currentBootMinute()) <= 2 && | ||
| processIsAlive(owner.pid) && | ||
| (owner.startIdentity === undefined || | ||
| observedStartIdentity === undefined || | ||
| observedStartIdentity === owner.startIdentity) |
There was a problem hiding this comment.
Authenticate owners of the shared allocation lock
On a multi-user Unix host, any user can create allocation.lock/owner beneath the intentionally world-writable shared root, name a long-lived foreign PID such as PID 1, and omit startIdentity. ownerIsActive() accepts the missing identity, while processIsAlive() treats EPERM as alive, so this forged owner is never age-reclaimed and all other users' stack starts poll indefinitely. Use an OS-authenticated host-wide lock or otherwise prevent untrusted users from forging active ownership.
Useful? React with 👍 / 👎.
| unlinkSync(config.spawnGate.requestPath); | ||
| unlinkSync(config.spawnGate.releasePath); | ||
| } |
There was a problem hiding this comment.
Register shutdown handling before spawning the gated child
When a stop or restart interrupts supervised startup just after the release file appears, the supervisor unlinks the gate and spawns the detached child before installing its SIGTERM/stdin handlers at lines 315-327. The orchestrator finalizer can therefore terminate the supervisor with the default signal disposition during this window, leaving the newly spawned service unmanaged. Fresh evidence after the prior handshake-cleanup thread is that cleanup is registered in the parent, but the supervisor still cannot run killChildTree() until after the child has spawned; install shutdown handling before consuming the gate and cover cancellation after release.
AGENTS.md reference: AGENTS.md:L126-L126
Useful? React with 👍 / 👎.
| const service = services.get(affectedDef.name); | ||
| if (service !== undefined) service.requested = true; | ||
| yield* FiberMap.run(fibers, affectedDef.name, runServiceSafe(affectedDef)); | ||
| yield* FiberMap.run(fibers, affectedDef.name, runServiceSafe(affectedDef, options)); |
There was a problem hiding this comment.
Re-reserve ports before tearing down the restart closure
When restarting a running service with active dependents, restartService() completes every stopForRestart() at lines 908-910 before these relaunch fibers can execute beforeStart and reacquire their leases. Ports belonging to dependents stopped early in the closure can consequently remain unreserved throughout the rest of the shutdown sequence, allowing another process to bind them and make the eventual restart fail. Fresh evidence after the prior restart-lease thread is this current stop-all-then-relaunch ordering; reacquire each reservation immediately when its process scope closes rather than waiting until all affected services are stopped.
AGENTS.md reference: AGENTS.md:L123-L126
Useful? React with 👍 / 👎.
| ...opts.stackConfig, | ||
| }), | ||
| ); | ||
| return yield* daemonLayer(config, opts.daemonEntryPoint); |
There was a problem hiding this comment.
Bound the daemon handshake while holding the global lock
When the forked daemon remains alive but never sends an IPC response—for example, because a custom daemon entry point or platform layer stalls—waitForDaemonResponse() has no timeout. Since this call now runs while projectDaemonLayer() owns the host-wide managed-port lock, that live owner prevents every other foreground and detached stack allocation on the host from progressing indefinitely. Apply a startup timeout that terminates the unresponsive child and releases the lock.
Useful? React with 👍 / 👎.
|
|
||
| // The previous process scope has closed, so external resources can | ||
| // be reserved safely for the duration of this restart's backoff. | ||
| yield* prepareStart(); |
There was a problem hiding this comment.
Fail unhealthy restarts when port re-reservation fails
When an unhealthy service loses its port before this restart-time beforeStart call, the reservation failure occurs while its state is still Unhealthy. runServiceSafe() then emits SpawnFailed, but the transition table accepts that event only from Pending or Starting, so the fiber exits with no process while the old completed healthy signal can make a later waitReady() report success. Fresh evidence beyond the prior initial pre-start failure thread is that this call runs inside the unhealthy restart loop before RestartTriggered; transition the service to a restart state first or handle SpawnFailed from Unhealthy.
AGENTS.md reference: AGENTS.md:L126-L126
Useful? React with 👍 / 👎.
Stack layer 5 of 7, based on #6044.
Closes the lazy-start port race by replacing probe-only allocation with real OS leases: