dev-up: make port release deadline-based and distinguish a live foreign owner from a lingering socket - #2522
Conversation
…ount Stop-LoadedStack reaped both recorded trees cleanly and then failed the whole stop because Wait-PortRelease allowed only 50 x 100 ms. On a loaded hosted runner the kernel can hold the listening socket past that, so a correctly dead stack was reported "still occupied" and the launcher exited 1. Wait-PortRelease / wait_for_port_release now poll to a configurable elapsed deadline (TASKDECK_DEV_UP_PORT_RELEASE_TIMEOUT_MS, default 30000). When the deadline passes, the port's LISTEN owners are inventoried: any live listener still fails closed with the retained PID state, and a port no live process is listening on is treated as a lingering kernel socket behind our own confirmed-dead tree and counts as released. An owner inventory that cannot be read at all also fails closed.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Review (agent half of the ADR-0066 gate; Codex credits exhausted, SC-9): one fresh-context reviewer confirmed no kill/signal path was added or widened (all new process interactions are read-only), enumerated every classification branch as fail-closed on Windows, and found Test-PortBindable unchanged. FIX-FIRST on one HIGH: in scripts/dev-up.sh an unprivileged lsof/ss cannot attribute another user's listener, so a root-owned foreign listener (docker-proxy, a systemd unit) reads as 'no live listener', the port is reported released, the PID file is removed and --stop exits 0 — a false clean report. Plus MEDIUM: the recorded/unrelated survivor label is bare-PID, contradicting the identity discipline of the kill path. Fix round in progress; after it, this PR stays parked for the maintainer's review per SC-10. |
An unprivileged owner lookup cannot attribute another user's listening socket: lsof lists nothing at all, and ss -p prints the row without users:(...). The empty owner list was read as "no live listener", so a root-owned foreign listener - docker-proxy, or a systemd unit on 5000/5173 - would have been classified as a lingering socket, the PID file removed, and the stop reported clean. Get-Process on another account's PID can fail the same way on Windows. Socket existence and PID attribution are now separate. Existence comes from a source that covers every account (ss -ltn, netstat -an on BSD, the kernel TCP table via Get-NetTCPConnection or netstat -ano); attribution is best effort and only decorates the diagnostic. Release is accepted only when the port becomes bindable, or when a readable inventory positively shows no listening socket. Also: drop the "recorded" / "unrelated" survivor label, which classified by bare PID and so contradicted the PID+name+token identity the kill path requires - the warning now reports PID and command name only; bound TASKDECK_DEV_UP_PORT_RELEASE_TIMEOUT_MS to int32 in the Bash launcher to match the PowerShell parse; and report elapsed time in ms in both launchers.
|
Fix round pushed as HIGH (accepted, and it applied to Windows too). The bug was reading an empty owner list as New Bash test MEDIUM (accepted). The LOW (accepted). LOW (declined, tracked here). The release test's 12 s wall-clock hold stays. Verification (local, Windows, Node 24, head Not re-verified in this round: the full per-platform suite (unchanged from the previous round's Still a control-plane change awaiting maintainer review per the ADR-0066 amendment (2026-09-03). |
|
Round 2 (scoped to the fix diff): the fresh-context reviewer confirmed the HIGH and MEDIUM are closed on both launchers — existence is probed separately from attribution (ss without -p / netstat fallback; explicit Listening flag on Windows), release is accepted only on a successful bind or a readable inventory positively showing no listener, unattributable and unreadable both fail closed, the new Bash test's PATH shim genuinely shadows real ss/lsof on hosted Linux and discriminates against the previous script, and no kill path was added. Verdict SHIP (agent half of the gate). One new MEDIUM confined to the netstat fallback (English LISTEN token vs localized Windows) tracked in the issue linked above. This PR now stays parked for the maintainer's review per ADR-0066 / SC-10; hosted Frontend Unit runs at d8d5300 are the R4 proof. |
|
Coordinator note (2026-09-04 overnight run wrap-up): this PR is queued for your review under OUTSTANDING_TASKS SC-10, but GitHub now reports it CONFLICTING / DIRTY — two further dev-up commits landed on main after it opened (#2548 8c0779f and #2556 6c10850, both touching scripts/ci/dev-up.test.mjs). Its last hosted run is therefore against a stale base. It needs a merge from main and a fresh hosted re-prove before a review of it means anything; the agent-side gate (round 1 FIX-FIRST on the unprivileged-lsof/ss privilege hole, fixed, round 2 SHIP) still stands for the content. Its stacked follow-up #2531 (#2526, localized netstat states) should be re-targeted to main only after this lands. |
Re-measured against live GitHub on 2026-09-04: - SC-4 clock: a09d986 did not leave a clean planner. Five same-shape shadow false reds landed 2026-09-04 (PR #2485 twice, #2496, #2515, #2500) from the CONTROL_BASE/merge-ref race, so the clock restarts when PR #2506 lands. #2506 becomes the first open blocker in the clause-5 chain, ahead of #2327. - Human-gate table: add the missing SC-9 and SC-10 rows from OUTSTANDING_TASKS section J, with the five SC-10 PRs measured open and #2522 CONFLICTING. - Clause-4 risks: #2425 (PR #2447) and #2399 (PR #2454) are closed; the open flake pair is #2489 and #2378, and the clause-4 row count follows. - Decision labels: #1936 is closed and #2004 no longer carries decision; the open decision-labelled milestone issues are #2324 and #1772. - Label split: 12 dogfooding / 17 ci / 22 other = 51, with the ci sub-breakdown, the ordinary-backlog list and the gated/un-gated arithmetic following it.
Resolves the single conflict this branch had accumulated. #2522 inserts 182 lines of new port-release tests immediately above the `high-volume stdout and stderr cannot deadlock marker acceptance` declaration; main's 8c0779f modified that exact declaration line (adding `timeout: 60_000`). Git cannot order an insertion against a change to its anchor line, so it conflicted. Resolution keeps both sides: every inserted line survives verbatim and main's declaration — with its timeout — moves below the inserted block. #2522's own copy of that declaration, which lacked the timeout, is dropped. Merged rather than rebased on purpose: #2531's head has this branch's head d8d5300 as an ancestor, so a force-push here would strand the stacked child. Verified: the diff against main contains zero removed lines (pure insertion), `node --check scripts/ci/dev-up.test.mjs` passes, and both of main's fixes survive in the tree - `closeAllConnections` and `timeout: 60_000`.
Conflict resolved — head is now
|
The new red at
|
Second red at
|
| Check | Failure | Tracked as |
|---|---|---|
Frontend Unit (windows-latest) |
spawnSync powershell.exe ETIMEDOUT, dev-up.test.mjs:1157 |
#2378 (third occurrence) |
API Integration (windows-latest) |
fixed-Task.Delay polling assertion, C# |
#2572 (new) |
Both are Windows-runner-contention shapes on a leg where this PR's own content passed. The three tests this branch inserts passed, and main's timeout: 60_000 fix — preserved by the resolution — passed at 12 723 ms.
Disposition unchanged. This PR is T2 control-plane and parked under SC-10 for the maintainer, so neither red blocks anything tonight. When it is taken up: run gh pr update-branch 2522 (main has advanced four merges since this head — 8c511205d, 22ec0d333, 93d021d01, df1559fd8) and take the evidence from that fresh run rather than from this one. I have not re-run the current head, deliberately: a re-run would reuse the stale merge ref, and re-proving against a base that has since moved is the thing that actually needs doing.
Stop rule still stands: if PowerShell: pipeline cancellation runs transactional cleanup from finally fails again on a fresh head, that is a regression to investigate rather than re-run.
…d-head claim, note #2522 needs update-branch
Root cause
Tonight's hosted-Windows red
PowerShell: Vite fallback leaves the foreign frontend-port owner alive(
scripts/ci/dev-up.test.mjs:1716, asserting insidestopSuccessfulStackatscripts/ci/dev-up.test.mjs:955-961) was not about the foreign listener. The failure message namesstate.Frontend.Port— the launcher's own Vite fallback port.Path:
Stop-LoadedStack(scripts/dev-up.ps1:423-434) reaps both recorded trees cleanly.Wait-PortRelease -Port $script:State.Frontend.Port(scripts/dev-up.ps1:367-374),which allowed only 50 iterations x 100 ms = 5 s and required
Test-PortBindable(
scripts/dev-up.ps1:357-365) to get an exclusive bind on both IPv4 and IPv6 loopback.taskkill /T /F,so
Wait-PortReleasereturned$false->"Frontend port N is still occupied. No foreign listener was killed; PID state is retained."->$clean = $false->throw "Stack cleanup was incomplete"(
scripts/dev-up.ps1:699), exit 1.A correctly dead stack was reported incomplete. Same family as the fixed 3 s marker budget in #2157:
a hard-coded bound measured against hosted wall-clock variance.
Occurrences: run
33835062683(#2506) and run33833389166(#2497).Fix
Both launchers (
scripts/dev-up.ps1,scripts/dev-up.sh) had the same{1..50}/50 x 100 msbound.Wait-PortRelease/wait_for_port_releasepoll every 250 msagainst an elapsed-time deadline from
TASKDECK_DEV_UP_PORT_RELEASE_TIMEOUT_MS(default 30000).An unparseable value warns and falls back to the default.
inventoried (
Get-NetTCPConnection -State Listen -LocalPort, falling back tonetstat -ano;lsofthen
sson Bash), filtered to PIDs that are still live:owner is
unrelatedor arecordedPID that outlived its reap, comparing against the reaped tree PIDs);line is printed and the port counts as released;
ever killed, and any live owner still retains state and exits 1.
Refs #1898Refs #2378Refs #2157Verification
All local, Windows, Node 24, in an isolated worktree at head
c4509b0ab.New regression tests (
scripts/ci/dev-up.test.mjs), both platforms, both stopping a syntheticschema-v1 state whose recorded PID is already reaped so only the port-release stage is under test:
a frontend port released after the old fixed budget still stops cleanly— an out-of-process holderoccupies the recorded frontend port and releases it 12 s in; the launcher must exit 0 and remove state.
a live listener still holding the frontend port fails closed— a live listener keeps the port; thelauncher must exit non-zero, keep the PID state, and leave the listener alive.
Negative control (the point of the first test): with
scripts/dev-up.ps1/scripts/dev-up.shstashed back to
origin/mainand only the new test applied —and with the fix applied, both pass. (The Bash launcher's old loop spent ~12 s of wall clock on 50
node-subprocess probes, so its effective budget was already long; its change is consistency and the
same owner classification, and it is still covered by the test.)
Targeted runs —
node --test --test-concurrency=1 --test-timeout=30000 --test-name-pattern=... scripts/ci/dev-up.test.mjs,for the two new tests plus the originally-red
Vite fallback leaves the foreign frontend-port owner alive:Bash: Vite fallback...hit the 30 s--test-timeoutat 30005 msBash: Vite fallback...20771 ms)Full suite, split by platform to fit the local shell timeout:
--test-name-pattern="^PowerShell": tests 44, pass 43, fail 1, duration 128989 ms. The singlefailure was
an exited preflight broker does not fail process identity capture, and it failed ontaskkill—FATAL: ERROR: The process with PID 47068 (child process of PID 27932) could not be terminated— i.e. the [Bug][Priority II] Close remaining dev-up fail-closed availability races #1898 taskkill race, before any port check runs. Re-run by name three times:pass, pass, pass.
--test-name-pattern="^Bash": several reds, all 20-50 s durations against the 30 s--test-timeout.Compared against a clean
origin/mainworktree (C:/Users/jekyt/source/td-wt/base1898) runningthe same cases:
tests 17, pass 6, fail 5— clean main is equally red (and red on cases this branchpassed, e.g.
invalid Vite outcome duplicate,duplicate-property). These are the known Frontend Unit (windows-latest) times out on slow runners: dev-up.test.mjs PowerShell spawns hit ETIMEDOUT and the job hits its timeout #2378Git-Bash-on-Windows slowness class on this box, not this diff; the hosted lane runs Bash on Linux.
Hosted proof is this PR's own Frontend Unit jobs.
Not verified
lingering that motivated the change, so the original red is proven by causal reading of the code
path plus the deadline negative control, not by a local reproduction of the hosted failure.
netstat -anofallback branch ofGet-LivePortListenerOwnerwas not exercised:Get-NetTCPConnectionis present on this box, so only the primary path ran.ssbranch oflive_port_listener_ownerswas not exercised; Git Bash on Windows has neitherlsofnorss, so the Bash fail-closed test took the "could not be inventoried" path locally. Thelsofpath will run for the first time on hosted Linux.PowerShell failure and the Bash failures both reproduce on unmodified
origin/main.Risks
A healthy stop returns on the first successful bind and is unchanged; a genuinely stuck port now
costs up to 30 s before the same fail-closed error.
bounded to: after the deadline, after both recorded trees were proved gone, and only when a listener
inventory was actually readable. A port with no listening process cannot serve anything, and a later
launch that cannot bind it still fails loudly at
Test-PortBindable.Get-NetTCPConnectioncan be slow on a busy machine; it is called at most twice per stop and onlyafter the deadline has already expired.
Control-plane change: awaiting maintainer review per the ADR-0066 amendment (2026-09-03). This PR is
parked ready-for-review and must not be merged by an agent.