[bong-water-water-bong]

Refs #1485, batch 3. Same rule as #1537, applied in the same commit this time: a migration moves the census with it.

What

• tests/serve_e2e.sh: --port 0 + serve_port_wait with a 12000-tenths budget (this one loads a real model before it is ready; its own health window is 600 s).
• tests/anchor_rerun.sh: same, 18000 tenths to match the script's own 30-minute health window. A serve that never reports a port now names the alias and tails the log, instead of continuing to poll a port nobody is listening on.
• _onebit_route_tests drops serve_e2e_cpu/serve_e2e_hrx — they no longer pre-pick, so the lock serialises nothing. The census floor is unchanged at 12: those two only exist in model-enabled builds and were never part of the bare-build count.

Two things I checked rather than assumed

1. hrx_cpu_weight_parity is not a 1bit serve script. It runs $server — i.e. llama-server — so --port 0 and the ready-line parse do not apply to it; it keeps free_port() and its lock. That is also why the lock-list comment now says so: the next person migrating "all the route scripts" would otherwise break it silently.
2. The remaining laya/mem-admit scripts are mostly failure-path runs (out=$("$bin" serve … 2>&1) expecting a refusal), so they need individual reading rather than a mechanical --port 0. Deliberately not in this batch — see below.

Verification

• bash -n clean on both scripts; shellcheck warning counts unchanged against main (serve_e2e 2, anchor_rerun 0) — the measure the review on tests: five more scripts take a port serve reports instead of guessing one (refs #1485) #1537 asked for, counted against main rather than against zero.
• Census rehearsed against a stand-in ctest built from the new lists: 3/3 cases pass.
• Not run end to end: serve_e2e needs a model and a device, anchor_rerun needs the anchor models; neither exists on this box. CI runs the census (which is the part of this batch it can run) and will say so loudly if the lists disagree.

Remaining in #1485

laya_gguf_route, laya_models_e2e, laya_route_e2e, mem_admit_route, npu_repack_path — all 1bit serve, but each mixes real serves with deliberate-failure runs; and kvshare_fd_handoff.py, which is python and spawns serve itself. hrx_cpu_weight_parity is explicitly out (llama-server).

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1485 - Partially compliant Compliant requirements: serve --port 0 is used in tests/serve_e2e.sh and tests/anchor_rerun.sh Both scripts now parse the port from the serve output using serve_port_wait The census is updated to reflect the change in resource locking Non-compliant requirements: The PR does not include a second script as a reference for the migration pattern Requires further human verification: Verification that the port parsing works correctly in all environments Confirmation that the resource lock behavior is as expected

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Port Binding Race Condition The script now uses --port 0 and parses the bound port from the output, which eliminates the race condition that previously occurred when picking and releasing a port. This change aligns with the ticket's goal of removing the race condition by using the port reporting mechanism instead of retrying around it. # --port 0: the kernel picks and serve reports the port it bound (engine #1485), so nothing is picked and released # here. The budget matches this script's own 30-minute health window: these are real models. "$bin" serve -m "$gguf" --device "$device" --port 0 --alias "$alias" >"$log" 2>&1 & pid=$! port=$(serve_port_wait "$log" "$pid" 18000) || { echo "FAIL $alias: serve never reported a port; log=$log"; tail -25 "$log" kill "$pid" 2>/dev/null; wait "$pid" 2>/dev/null; continue } Port Binding Race Condition The script now uses --port 0 and parses the bound port from the output, which eliminates the race condition that previously occurred when picking and releasing a port. This change aligns with the ticket's goal of removing the race condition by using the port reporting mechanism instead of retrying around it. log=$(mktemp) # --port 0: the kernel picks and serve reports the port it bound, so nothing here picks a port and releases it before # serve binds (engine #1444/#1485). The budget is generous because this test loads a real model before it is ready. "$bin" serve -m "$model" --device "$device" --port 0 --alias e2e-model "$@" >"$log" 2>&1 & pid=$! port=$(serve_port_wait "$log" "$pid" 12000) || { echo "FAIL serve never reported a port"; cat "$log"; exit 1; } cleanup() { kill "$pid" 2>/dev/null; wait "$pid" 2>/dev/null; rm -f "$log"; }

[bong-water-water-bong]

VERDICT: READY at ed3bbb4f.

Both scripts migrated, and the arithmetic on the budgets — which is the part that decides whether a slow start fails a correct build — checks out for one
and is a 2x margin on the other:

anchor_rerun  health wait  3600 x sleep 0.5 = 1800 s = 30 min   port budget 18000 tenths = 1800 s   exact match
serve_e2e     health wait  1200 x sleep 0.5 =  600 s = 10 min   port budget 12000 tenths = 1200 s   2x the window
no pick-and-release left in either; --port 0 + serve_port_wait in both; bash -n clean
shellcheck per file identical to main (serve_e2e 2 SC2034, anchor_rerun none)

The one thing to adjust or explain is serve_e2e's 12000. The ready line is printed before the health loop, so a port budget of 1200 s is twice the
600 s the test then waits for health — and a serve that is alive but never ready costs 20 minutes before this row says anything, against the 10 the rest
of the script is willing to spend. If the 2x is deliberate (a cold start on a loaded runner can be slow, and the failure it guards is "no line at all"),
one clause in the comment says so and the number is fine; if it was meant to mirror the health window, 6000 is the number that matches. Not blocking either
way — the margin errs toward waiting rather than failing a good build, and serve_port_wait's kill -0 covers the dead-serve case at once.

And the census moved in the same commit, which is the lesson from #1537 applied rather than repeated —

-  … pm_laya_route serve_e2e_cpu serve_e2e_hrx hrx_cpu_weight_parity
+  … pm_laya_route hrx_cpu_weight_parity

with the reason written where the list is ("serve_e2e_ left this list with engine #1485 … it picks nothing and there is no window for the lock to
close"). hrx_cpu_weight_parity staying is right and I checked why: the script runs llama-server itself (3 references), whose port is not ours to
choose, so it is out of the migration and keeps its lock — the distinction the comment draws is the one that decides it.

anchor_rerun was the one I flagged as unconfigured in #1537 — it is still not in CMakeLists, so it is migrated now but still outside the census; when
it is wired in it should be added to _onebit_route_tests? No: it now takes --port 0, so it should not be — it simply needs to be configured, and the
migration means it arrives without a window. That ordering (migrate, then configure) is the right one and worth keeping.

[bong-water-water-bong]

VERDICT: CHANGES — one line, and it is the exact hazard this fleet has already paid for once.

serve_e2e.sh leaks the serve process on the failure path it just added. Current order in the file:

44  pid=$!
45  port=$(serve_port_wait "$log" "$pid" 12000) || { echo "FAIL serve never reported a port"; cat "$log"; exit 1; }
46  cleanup() { kill "$pid" 2>/dev/null; wait "$pid" 2>/dev/null; rm -f "$log"; }
47  trap cleanup EXIT

trap cleanup EXIT is registered two lines after the exit 1 it is supposed to cover, so on the readiness-failure path the trap does not exist yet and the script exits leaving serve running. That is the worst case for the leak, not an edge case: this branch fires when serve never became ready, i.e. while it is still loading a model and holding the GTT its weights need — the 51 GiB orphan class from 11-08 that knocked out the PR-agent LLM. anchor_rerun.sh is safe in this same PR because it kills in the branch (kill "$pid" … ; continue), which is why I think this is an ordering slip rather than a design choice.

Two ways to fix, both one line: hoist cleanup and trap cleanup EXIT above line 44 (so the trap covers every exit after the spawn — my preference, since it also covers a set -e abort anywhere later), or add kill "$pid" inside the failure branch as anchor_rerun does. Either way the property to state is: after pid=$!, every exit path kills the child.

Everything else in the PR is right, and two things are better than right:

• >"$log" 2>&1 gives serve_port_from_log raw serve output, which is the precondition #1531's parse actually has (it anchors ^1bit serve: and indexes $5, so a prefixed log yields nothing and would look like "serve never reported a port"). Both migrations satisfy it, and the failure branch now names the alias and tails the log instead of polling a port nobody is listening on — that is the difference between a 30-minute stall and a readable failure.
• hrx_cpu_weight_parity is kept, with the reason in the CMake comment — it starts llama-server, whose port is not ours to choose. That is exactly the distinction a mechanical "migrate all the route scripts" sweep would have destroyed silently, and putting the reason next to the list is what stops the next sweep from destroying it.

Also good: the census moving in the same commit (the rule from #1537), shellcheck counted against main rather than against zero (a warning count of 2 is not a regression if main is 2), and the end-to-end runs declared as not possible on this box. And the deferred batch is deferred for the right reason — the remaining laya/mem-admit scripts mix real serves with deliberate-failure runs, where --port 0 would change what the test is asserting (a refusal path needs a port that is genuinely taken, which is the one place free_port() is still correct).

[bong-water-water-bong]

Still live at ed3bbb4f — cleanup and the trap are two lines below the exit 1 they must cover, so the readiness-failure path still leaves serve running while it is loading a model. Re-posting with the patch inline so it is a one-click apply rather than a re-read of my earlier comment:

# --port 0: the kernel picks and serve reports the port it bound, so nothing here picks a port and releases it before
# serve binds (engine #1444/#1485). The budget is generous because this test loads a real model before it is ready.
"$bin" serve -m "$model" --device "$device" --port 0 --alias e2e-model "$@" >"$log" 2>&1 &
pid=$!
# Before the wait, not after it: this branch fires when serve never became ready, i.e. while it is still loading and
# holding its weights' GTT, and an `exit 1` above the trap registration leaves it running (the 51 GiB orphan class).
cleanup() { kill "$pid" 2>/dev/null; wait "$pid" 2>/dev/null; rm -f "$log"; }
trap cleanup EXIT
port=$(serve_port_wait "$log" "$pid" 12000) || { echo "FAIL serve never reported a port"; cat "$log"; exit 1; }

My reason for pressing on one line: this is the same shape as engine#1624, which I fixed in #1629 the same day — there the watchdog was killed before its own kill completed, and here serve is left alive on the one path where it is guaranteed to still hold memory. In both cases the fix is one ordering change and the failure is invisible in a green run. anchor_rerun.sh in this same PR already gets it right by killing inside the branch, which is why I read this as a slip rather than a choice.

Everything else in the PR still stands approved from my earlier verdict: raw >"$log" 2>&1 feeding the helper (its parse precondition), the census moving with the migration, and hrx_cpu_weight_parity kept with its reason in the CMake comment.