Restore loopback port publishing in the llama.cpp qualification runner #8667

Activity

1. apurvvkumaria

   self-assigned this

   on Aug 10, 2026
2. AzeelSajjad commented on Aug 10, 2026

   

   AzeelSajjad

   on Aug 10, 2026

   Contributor

   More actions

   Opened #8676 for this.

   What it does

   buildServerContainerArgv() keeps delegating to the shared buildLlamaCppRequestGuardDockerArgv(), then splices one qualification-only --publish pair into the returned argv immediately before the image reference:

   … --entrypoint /usr/local/bin/nemoclaw-llama-cpp-request-guard \
      --publish 127.0.0.1::8081 \
      localhost:5000/…@sha256:… --listen-host 0.0.0.0 …
   

   • Without hostPort: --publish 127.0.0.1::8081
   • With hostPort: 8081: --publish 127.0.0.1:8081:8081

   Position is load-bearing. In the request-guard argv the image reference is immediately preceded by --entrypoint <guardPath>, so the pair lands between the guard path and the image. Anything after the image reference would be passed to the request guard as a program argument instead of to Docker, which would fail quietly rather than loudly.

   Nothing under src/ changed. The shared materializer and the Docker lifecycle provider are untouched, so ordinary product launches still leave host bridging to the lifecycle provider — which, as this issue notes, actively rejects a published container via the must not publish ownership rollback from #8544.

   Acceptance criteria

   Criterion	Status
   Ephemeral mapping when no host port is requested	Done — 127.0.0.1::8081
   Requested fixed mapping when hostPort is provided	Done — 127.0.0.1:8081:8081
   Ordinary materializer still leaves bridging to the lifecycle provider	Done — pinned by host-local-runtime.test.ts (16/16) and the lifecycle provider's suite (41/41), both untouched
   Targeted qualification-runner and host-local runtime tests pass	Done — qualification runner 12/12, and the failing assertion this issue reports is now green
   Fixed-port availability race handled independently	Not touched, as directed

   One correction to the issue's proposed fix

   The issue asks not to change the assertion to expect an empty result, and #8676 does not. But there is a second, separate assertion in the same test that also needed changing, in the opposite direction:

   expect(valuesAfter(agentQualificationArgv, "--publish")).toEqual([]);

   That one passed on main only because nothing was published at all — it asserted the bug. It now expects ["127.0.0.1:8081:8081"], matching the hostPort behavior this issue specifies.

   While strengthening it, one further gap turned up: because the test passes hostPort: 8081 and serve.port is also 8081, toEqual(["127.0.0.1:8081:8081"]) cannot distinguish an implementation that renders the requested host port from one that duplicates the container port onto both sides. #8676 therefore adds a row with a host port that differs from the served port, and mutation testing confirms it is the only assertion that catches that specific bug.

   Two adjacent findings, deliberately not fixed here

   1. .mts files are exempt from formatting enforcement. The prek biome-format hook's files pattern ends in .*\.ts$, which does not match a .mts extension, while biome.json includes **/*.mts. Both npm run validate:pr and the prek run --from-ref invocation in .github/workflows/pr.yaml inherit the gap, so formatting in every .mts file under scripts/checks/ and tools/e2e/ is unchecked. Happy to open a separate issue if that is worth tracking.
   2. resolveLoopbackPort() accepts only 4-and-5-digit ports. Its regex is /^127\.0\.0\.1:([1-9][0-9]{3,4})$/u, while positiveInteger(contract.serve.port, ..., 65_535) permits any port from 1. A recipe declaring a shorter port would publish fine and then fail to resolve. Not reachable today, since the pinned recipe declares port: 8081.

   One incidental improvement worth noting: restoring the mapping also re-arms a cleanup assertion that had become vacuous. cleanupOwnedRuntime() computes loopbackListenerClosed: loopbackPort === undefined || !(await tcpOpen(loopbackPort)); on main, resolveLoopbackPort() always threw, so loopbackPort stayed undefined and that teardown evidence field was unconditionally true.

   The broad npm test gate is deferred to CI — a local run on an 8 GB development host produced only memory-pressure timeouts in files unrelated to this change, which pass in isolation. Details are in the PR's Verification section.
3. github-actions

   assigned

   AzeelSajjad

   on Aug 10, 2026
4. prekshivyas

   closed this as completedin #8672on Aug 10, 2026
5. prekshivyas

   added a commit that references this issue on Aug 10, 2026

   fix(inference): restore qualification loopback publish (#8672)

   ...

   This issue will close once commit ae25d72 is merged into the 'main' branch.

   ae25d72