Hold the public realm-server port across the SF harness startup gap#5129
Merged
Conversation
The software-factory test harness allocated its public (compat-proxy) realm-server port with the non-holding findAvailablePort(): it picked an ephemeral port, released it immediately, and only bound it much later when the in-stack compat proxy listened on that port. In the gap, the sibling findAndHoldAvailablePort() allocations the harness makes for support services, the worker-manager, the realm-server child, and the prerender could be handed that same released port number by the OS and bind it first, so the compat-proxy bind died with EADDRINUSE and the whole harness failed to start (intermittently, under CI ephemeral-port churn). resolveFactoryRealmServerURL now holds the freshly-allocated public port with a holder socket and returns the reservation, which is threaded through the template build into startIsolatedRealmStack and released the instant before the compat proxy binds — the same release-before-bind handoff the worker-manager and realm-server ports already use. For a public port inherited from a reused context, startIsolatedRealmStack holds the specific number itself via the new holdSpecificPort(). Every harness port is now held across its allocation->bind gap. The compat-proxy bind also probes the contested port on EADDRINUSE so any residual collision names its holder in the failed-job log. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![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: 9e25a18aa7
ℹ️ 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".
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an intermittent EADDRINUSE startup failure in the software-factory harness by ensuring the public (compat-proxy) realm-server port is held from allocation time through the startup “gap”, preventing sibling port-0 allocations from stealing it before the compat proxy binds.
Changes:
- Add
holdSpecificPort()to reserve a caller-chosen port with the same holder-socket strategy used byfindAndHoldAvailablePort(), including conflict diagnostics. - Thread a held public-port reservation from
resolveFactoryRealmServerURL()through the template build intostartIsolatedRealmStack(), releasing immediately before the compat proxy binds. - Add unit tests validating
holdSpecificPort()behavior, idempotent release, and allocator invariants.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/software-factory/tests/hold-specific-port.test.ts | Adds unit tests for holding/releasing a specific port and verifying allocator behavior under contention. |
| packages/realm-test-harness/src/shared.ts | Introduces holdSpecificPort() and changes resolveFactoryRealmServerURL() to optionally return a held public-port reservation. |
| packages/realm-test-harness/src/isolated-realm-stack.ts | Adopts/creates a public-port hold before sibling allocations and releases it immediately before compat-proxy bind; adds bind-time diagnostics. |
| packages/realm-test-harness/src/index.ts | Re-exports holdSpecificPort() as part of the harness public API. |
| packages/realm-test-harness/src/database.ts | Threads publicPortReservation through buildCombinedTemplateDatabase() into startIsolatedRealmStack(). |
| packages/realm-test-harness/src/api.ts | Threads the public-port reservation through global-context/template paths with idempotent release backstops. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When startFactoryRealmServer runs with an existing support context but no prepared template, it delegates its held public-port reservation to ensureFactoryRealmTemplate. Because realmServerURL is also supplied, that callee allocates no reservation of its own, so its cache-hit/finally releases only the (undefined) local holder — and the delegating call sits outside startFactoryRealmServer's try/catch. A template-build failure before startIsolatedRealmStack adopts the holder (e.g. a Postgres drop/clone error) therefore leaked the holder socket. ensureFactoryRealmTemplate now releases the *effective* holder (the caller's delegated reservation when present, else its own) at both exit points. The release stays idempotent and is a no-op on the happy path, where startIsolatedRealmStack already released it before the compat-proxy bind. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Report the actually-bound port from server.address() rather than echoing the input, so a stray holdSpecificPort(0) reports the OS-assigned ephemeral port instead of a misleading 0. - Guard the bind-error handler's server.close() against the never-listened case so a synchronous throw there can't leave the outer promise unsettled. - Correct the startIsolatedRealmStack publicPortReservation doc: it's also omitted for a caller-supplied realmServerURL or an explicit compatRealmServerPort, not only a reused context. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jurgenwerk
approved these changes
Jun 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The software-factory test harness intermittently fails to start with
EADDRINUSEon its public realm-server port — e.g. this run, wherebuildCombinedTemplateDatabasedied withlisten EADDRINUSE: address already in use 127.0.0.1:43569.Cause
The harness exposes a fixed public (compat-proxy) realm-server port that tests connect to. That port was allocated with the non-holding
findAvailablePort(): it probe-binds an ephemeral port, closes the probe, and returns the bare number — leaving the port free until the in-stack compat proxy binds it much later. Between those two moments the harness makes several holdingfindAndHoldAvailablePort()allocations (support services, worker-manager, realm-server child, prerender). The OS port-0 allocator can legitimately hand one of them the public port's number, and it binds first — so the compat-proxy bind dies withEADDRINUSE. Every other harness port was already converted to the hold-across-the-gap pattern; the public port was the last one still on the racy path.Fix
resolveFactoryRealmServerURLnow holds the freshly-allocated public port with a holder socket and returns the reservation. The reservation is threaded through the template build intostartIsolatedRealmStack, which adopts it and releases it the instant before the compat proxy binds — the same release-before-bind handoff the worker-manager and realm-server ports already use. When the public port is inherited from a reused context (no upstream holder),startIsolatedRealmStackholds the specific number itself via the newholdSpecificPort().If it ever recurs
holdSpecificPort()and the compat-proxy bind both rundiagnosePortConflict()onEADDRINUSE, so a residual collision lands in the failed-job log naming the interface/pid that holds the port instead of an opaque error.Tests
Unit tests in
hold-specific-port.test.tscover the holder, idempotent release, the fail-fast-with-probe path, and the core invariant that a held specific port is never handed to a siblingfindAndHoldAvailablePort(). The realm-bench and software-factory CI jobs exercise the harness-bootstrap path end to end.🤖 Generated with Claude Code