Skip to content

Commit 1c5443e

Browse files
fix(setup): scope the compose port skip to published ports; print both k8s forwards
Review round on #5964: - ensureComposePortsFree skipped conflict handling whenever the project had any container running, so leftover db/redis (which publish neither app port) waved through a foreign process on :3000 — it then surfaced as a raw compose bind error instead of the prompt. Read the host ports the project actually publishes and skip only those; the remaining ports still get the full check. Reading from the containers rather than the file matters because what counts is what is bound right now. - The post-install note and the skip path documented only the app forward, while offerPortForward runs two. Skipping the prompt or copying the printed command left the editor's socket dead — the exact failure this change set exists to fix. Both commands now come from one forwardCommands() helper, so what is printed and what is run cannot drift.
1 parent bb2fd2a commit 1c5443e

2 files changed

Lines changed: 74 additions & 18 deletions

File tree

scripts/setup/modes/compose.ts

Lines changed: 49 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,28 +19,62 @@ import {
1919
} from '../steps.ts'
2020
import { glyph, theme } from '../theme.ts'
2121

22+
const REQUIRED_PORTS = [3000, 3002] as const
23+
24+
/**
25+
* Host ports this compose project currently publishes. Read from the containers
26+
* rather than assumed from the file, because what matters is what is bound right
27+
* now — a project with only db/redis up publishes neither app port, so those
28+
* still need the conflict check.
29+
*/
30+
function composePublishedPorts(composeFile: string): Set<number> {
31+
const ids = spawnSync('docker', ['compose', '-f', composeFile, 'ps', '-q'], {
32+
cwd: ROOT,
33+
encoding: 'utf8',
34+
})
35+
const containers = ids.status === 0 ? ids.stdout.split('\n').filter(Boolean) : []
36+
if (containers.length === 0) return new Set()
37+
38+
const inspect = spawnSync(
39+
'docker',
40+
[
41+
'inspect',
42+
...containers,
43+
'--format',
44+
'{{range $port, $bindings := .HostConfig.PortBindings}}{{range $bindings}}{{.HostPort}} {{end}}{{end}}',
45+
],
46+
{ encoding: 'utf8' }
47+
)
48+
if (inspect.status !== 0) return new Set()
49+
const published = new Set<number>()
50+
for (const token of inspect.stdout.split(/\s+/)) {
51+
const port = Number(token)
52+
if (Number.isInteger(port) && port > 0) published.add(port)
53+
}
54+
return published
55+
}
56+
2257
/**
2358
* Compose publishes 3000 and 3002 — resolve conflicts before touching docker,
2459
* instead of letting `docker compose up` die halfway through startup. Aborting
2560
* is fatal here: compose can't come up while the ports are held.
2661
*/
2762
async function ensureComposePortsFree(composeFile: string): Promise<void> {
28-
// This stack already holding 3000/3002 is not a conflict — `docker compose up
29-
// -d` reconciles its own containers. Without this, re-running setup against a
30-
// running install reports its own realtime container as a blocker and offers
31-
// to kill Docker's listener, which is never the right move. A genuinely
32-
// foreign process still gets caught below, and a foreign *container* surfaces
33-
// as a clear bind error from compose itself.
34-
const ours = spawnSync('docker', ['compose', '-f', composeFile, 'ps', '-q'], {
35-
cwd: ROOT,
36-
encoding: 'utf8',
37-
})
38-
if (ours.status === 0 && ours.stdout.trim()) {
39-
p.log.step('Existing Sim containers hold :3000/:3002 — compose will reconcile them')
40-
return
63+
// A port this stack already publishes is not a conflict — `docker compose up
64+
// -d` reconciles its own containers, and reporting the install's own realtime
65+
// container as a blocker (offering to kill Docker's listener) is never right.
66+
// Skip only the ports this project actually publishes: leftover db/redis
67+
// containers must not wave through a foreign process sitting on 3000, which
68+
// would otherwise surface as a raw compose bind error instead of the prompt.
69+
const ours = composePublishedPorts(composeFile)
70+
const toCheck = REQUIRED_PORTS.filter((port) => !ours.has(port))
71+
if (toCheck.length < REQUIRED_PORTS.length) {
72+
const skipped = REQUIRED_PORTS.filter((port) => ours.has(port))
73+
p.log.step(`Existing Sim containers hold :${skipped.join(', :')} — compose will reconcile them`)
4174
}
42-
if (await ensurePortsFree([3000, 3002])) return
43-
throw new SetupError('ports 3000/3002 are in use', [
75+
if (toCheck.length === 0) return
76+
if (await ensurePortsFree(toCheck)) return
77+
throw new SetupError(`ports ${toCheck.map((port) => `:${port}`).join('/')} are in use`, [
4478
`free the ports, then re-run: ${theme.command('bun run setup')}`,
4579
`see what holds them: ${theme.command('lsof -nP -iTCP:3000 -sTCP:LISTEN')}`,
4680
`stop a container publishing them: ${theme.command('docker ps')}`,

scripts/setup/modes/k8s.ts

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -386,10 +386,14 @@ export async function runK8sMode(detection: Detection): Promise<void> {
386386

387387
p.note(
388388
[
389-
`open ${APP_URL} (needs the port-forward below)`,
389+
`open ${APP_URL} (needs both forwards below)`,
390390
`pods: kubectl --context ${shq(context)} -n ${NAMESPACE} get pods`,
391391
`app logs: kubectl --context ${shq(context)} -n ${NAMESPACE} logs deploy/${RELEASE}-app --tail 50`,
392-
`forward: kubectl --context ${shq(context)} -n ${NAMESPACE} port-forward svc/${RELEASE}-app 3000:3000`,
392+
// Both, always: the app alone loads but the editor's socket has nothing to
393+
// reach, which is the reconnect failure this change set exists to fix.
394+
...forwardCommands(context).map(
395+
(command, index) => `${index === 0 ? 'forward: ' : ' '} ${command}`
396+
),
393397
`tear down: helm uninstall ${RELEASE} --kube-context ${shq(context)} -n ${NAMESPACE}`,
394398
].join('\n'),
395399
'Reach your cluster'
@@ -398,6 +402,16 @@ export async function runK8sMode(detection: Detection): Promise<void> {
398402
await offerPortForward(context)
399403
}
400404

405+
/**
406+
* Both forwards, in the order a user should run them. Kept in one place so the
407+
* printed instructions and what the wizard actually runs cannot drift — the app
408+
* alone leaves the editor's socket dead.
409+
*/
410+
function forwardCommands(context: string): string[] {
411+
const scope = `kubectl --context ${shq(context)} -n ${NAMESPACE} port-forward`
412+
return [`${scope} svc/${RELEASE}-app 3000:3000`, `${scope} svc/${RELEASE}-realtime 3002:3002`]
413+
}
414+
401415
/**
402416
* The services are ClusterIP, so a healthy release is still unreachable from the
403417
* host — "Sim is ready" with nothing on :3000 is the least satisfying way to end
@@ -414,7 +428,15 @@ async function offerPortForward(context: string): Promise<void> {
414428
initialValue: true,
415429
})
416430
if (!forward) {
417-
p.log.info(theme.muted('Skipped — run the forward command above when you want to reach it.'))
431+
p.log.info(
432+
theme.muted(
433+
`Skipped — run both forwards when you want to reach it (the second keeps the editor's socket alive):\n${forwardCommands(
434+
context
435+
)
436+
.map((command) => ` ${command}`)
437+
.join('\n')}`
438+
)
439+
)
418440
return
419441
}
420442

0 commit comments

Comments
 (0)