CS-10564, CS-10699: Fix env var port leaks and prerender heartbeat escape#4364
Open
CS-10564, CS-10699: Fix env var port leaks and prerender heartbeat escape#4364
Conversation
…cape in test harness Replace module-level env var constants (DEFAULT_REALM_SERVER_PORT, DEFAULT_COMPAT_REALM_SERVER_PORT, DEFAULT_PRERENDER_PORT, CONFIGURED_PRERENDER_URL) with explicit CLI args and function parameters, following the pattern established in CS-10560 for the worker-manager port. Changes: - shared.ts: Strip ambient port env vars at module load (like HOST_URL), remove module-level constants, accept ports via function params - isolated-realm-stack.ts: Accept realmServerPort and prerenderURL as explicit params - api.ts: Thread new port params through FactoryRealmOptions to startIsolatedRealmStack - serve-realm.ts: Parse --realmServerPort, --compatRealmServerPort, --prerenderURL CLI args - fixtures.ts: Pass ports as CLI args to serve-realm.ts instead of env vars - support-services.ts: Set PRERENDER_MANAGER_URL to unreachable address when spawning test prerender servers, preventing them from registering with external managers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
test-run-execution.ts defaulted hostDistDir to a worktree-relative path (../../../host/dist), which fails in worktrees where the host app hasn't been built locally. Now uses findHostDistPackageDir() to check the root repo checkout first, matching how support-services.ts resolves the host dist for the main app. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens the software-factory harness’ test isolation by removing ambient env-var–driven port configuration in favor of explicit CLI args / function params, and prevents harness-spawned prerender servers from registering heartbeats with external prerender managers. It also improves host dist resolution for worktrees when executing QUnit runs via Playwright.
Changes:
- Replace env-var-based default port/prerender configuration with explicit
--realmServerPort,--compatRealmServerPort, and--prerenderURLplumbing through the harness. - Prevent harness prerender servers from heartbeating to external prerender managers by overriding
PRERENDER_MANAGER_URLwhen spawning the prerender server. - Fix worktree failures by resolving the host dist directory via
findHostDistPackageDir()before falling back to a worktree-local path.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/software-factory/tests/fixtures.ts | Spawn serve-realm.ts with explicit port/prerender CLI flags instead of env vars. |
| packages/software-factory/src/harness/support-services.ts | Default prerender port selection to dynamic ports; force PRERENDER_MANAGER_URL to a local unreachable URL for harness prerender servers. |
| packages/software-factory/src/harness/shared.ts | Strip several ambient env vars at module load; add explicit port/prerender options; remove module-level defaults sourced from env. |
| packages/software-factory/src/harness/isolated-realm-stack.ts | Thread explicit realmServerPort and prerenderURL into isolated stack startup and prerender reuse logic. |
| packages/software-factory/src/harness/api.ts | Thread new options through to location resolution and isolated stack startup. |
| packages/software-factory/src/cli/serve-realm.ts | Add simple CLI parsing for the new flags and adjust positional arg handling. |
| packages/software-factory/scripts/lib/test-run-execution.ts | Resolve host dist using findHostDistPackageDir() to improve worktree behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Extract findHostDistPackageDir, findRootRepoCheckoutDir, fileExists into a side-effect-free module (src/host-dist.ts) so scripts/lib can import without pulling in harness env var stripping from shared.ts - Add Number.isFinite validation to parseCliNumber in serve-realm.ts - Reword PRERENDER_MANAGER_URL comment (port 1 is "expected closed", not "unreachable") Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
lukemelia
approved these changes
Apr 8, 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.
Summary
DEFAULT_REALM_SERVER_PORT,DEFAULT_COMPAT_REALM_SERVER_PORT,DEFAULT_PRERENDER_PORT,CONFIGURED_PRERENDER_URL) with explicit CLI args and function parameters. Strips ambient port env vars atshared.tsmodule load to prevent dev shell pollution. Follows the pattern from CS-10560 (worker-manager port fix).:4222), causing stale heartbeat entries likeHeartbeat sweep: prerender server http://localhost:44289 is stale; removing. Fixed by settingPRERENDER_MANAGER_URLto an unreachable address (http://127.0.0.1:1) when spawning test harness prerender servers, so their heartbeats never reach the outside manager. This is safe because the test harness doesn't use a prerender manager at all — the worker-manager and realm-server talk directly to the prerender server via--prerendererUrl, bypassing the manager entirely. The heartbeat registration is a side-effect from the prerender server code designed for the production/dev-all multi-server setup.test-run-execution.tsdefaultedhostDistDirto a worktree-relative path that fails in worktrees where the host app hasn't been built locally. Now usesfindHostDistPackageDir()to resolve from the root repo checkout first, matching howsupport-services.tsalready resolves the host dist for the main app.Files changed
shared.tscompatRealmServerPort/realmServerPort/prerenderURLtoFactoryRealmOptionsisolated-realm-stack.tsrealmServerPortandprerenderURLas explicit paramsapi.tsstartIsolatedRealmStackandresolveFactoryRealmLocationserve-realm.ts--realmServerPort,--compatRealmServerPort,--prerenderURLCLI argsfixtures.tsserve-realm.tsinstead of env varssupport-services.tsDEFAULT_PRERENDER_PORTimport, setPRERENDER_MANAGER_URLto unreachable address so test prerender servers can't register with external managerstest-run-execution.tsfindHostDistPackageDir()for host dist lookup, fixing worktree test executionTest plan
factory-test-realm.spec.tsto confirm host dist fix🤖 Generated with Claude Code