fix: slice 1 robustness follow-ups — record-store re-arm race, login code expiry, control-plane readiness, relay teardown (#426)

[pdettori]

Summary

Code-only robustness follow-ups from the P6-on-Kubernetes slice 1 reviews (epic #426). It doesn't touch deploy/k8s/ or the manifest tests, so it does not conflict with slice 2 (#424). The setup.sh, manifest and smoke items of #433 and #434 stay open for a follow-up after slice 2 merges.

Closes #428. Closes #431. Part of #434 and #433.

Changes

• RedisRecordStore re-arm: a failed attempt's .catch can clear a newer memo (microtask race) #428, harness/src/pool-records.ts: connectOnce holds the attempt in a local p, and its .catch clears the memo only if the memo is still p. A failed re-arm can no longer wipe a newer attempt.
  • Test: a hand-settled client interleaves three callers around a failed re-arm. Before the fix, the third caller got Socket already opened.
• mocactl login: say how long the device code is valid, and handle expired_token #431, mocactl and control plane:
  • mocactl login prints how long the device code is valid, for example (valid for 15 minutes), from expiresIn.
  • The token poll used to collapse GitHub's expired_token into unauthorized. It now answers 410 device_code_expired. errors.ts, identity.ts, docs/api/openapi.yaml and the client spec are updated.
  • On expiry, from the server or from the local deadline, mocactl re-issues the code once and prints the new one. The Login overlay swaps it in with a fresh countdown. A second expiry fails with "run mocactl login again" and exits 1.
• deploy/k8s, control plane, relay: small robustness follow-ups from the slice 1 reviews #434, control plane:
  • The Redis reconnect strategy is exported (redisReconnectStrategy, redisClientOptions), and tests pin it: 100 ms per retry, capped at 2 s, never gives up, and not node-redis's default.
  • /readyz bounds index.get at 500 ms, then answers 503 redis_unavailable. A connected but wedged Redis now fails before the kubelet's 1 s default probe timeout.
  • SIGTERM closes the HTTP server, then deps.close(), then exits 0. A throwing close() is logged, and the process still exits.
• deploy/k8s, control plane, relay: small robustness follow-ups from the slice 1 reviews #434, relay (relay.ts): each stream remembers the session its own Hello created. teardown acts only while the map still holds that exact session, and at most once. A stray 'error' after 'end' can therefore no longer evict a same-id reattach or cancel its presence retry.
  • Test: end, reattach with a retry pending, then a late 'error'. Before the fix, the new session vanished from parked().
• deploy/k8s: test-strength follow-ups from the slice 1 reviews #433, relay test: covers a presence put still in flight at teardown, whether it resolves or rejects: no further put, no timer scheduled, nothing logged. It was checked against a mutation: removing the guard that stops a gone session's retry makes it fail.

For review

• mocactl login: say how long the device code is valid, and handle expired_token #431 re-issues once instead of failing straight away. The case seen on OpenShift was a user away from the browser: a fresh code costs one request, while a failure costs a re-run. Allowing only one re-issue means an unattended login still ends, after two code lifetimes.
• New API error device_code_expired (410). A new mocactl talking to an older control plane still sees unauthorized. Its own expiresIn deadline usually catches the expiry, but a poll that lands between GitHub's expiry and mocactl's own deadline still fails as before. deploy/knative/demo-multiuser.sh now prints device_code_expired.
• The /readyz 500 ms bound is a constant. CpDeps.readyzTimeoutMs exists for tests.

Test plan

• make lint (exit 0)

• make typecheck (exit 0)

• vitest with Redis available, all exit 0. The skipped tests are only the env-gated live ones.

  Package	Passed	Skipped
  harness	514	5
  control-plane	452	3
  mocactl	557	2
  sandbox-relay	49	0

• Each new test failed before its fix (deploy/k8s: test-strength follow-ups from the slice 1 reviews #433 is test-only and was mutation-checked).

🤖 Generated with Claude Code

[pdettori]

(Review posted as COMMENT — GitHub won't let the PR author's token submit an APPROVE; verdict would be approve.)

Reviewed the four fixes plus the relay test — all look correct, each pinned by a test that fails without it.

• #428 (pool-records.ts): clearing the memo only when this.ready === p is the right fix for the microtask window between node-redis's synchronous isOpen = false and the rejection reaching .catch. The hand-settled stand-in client makes the interleaving exact rather than a timing race.
• #431 (identity / mocactl): device_code_expired (410) is consistently wired through errors.ts, openapi.yaml, the client spec table, and the client mapping — no string-matching of GitHub's text anywhere. Re-issue-once bounded by MAX_CODES = 2 is a reasonable call; an unattended login still terminates.
• #434 (control-plane): the /readyz Promise.race bound with clearTimeout in finally is clean (the losing get rejection is absorbed by the race), and exporting redisReconnectStrategy / redisClientOptions so a test pins both the backoff and the wiring is exactly how this kind of thing should be locked down. SIGTERM ordering (server → deps → exit 0) with logged close() failures is right.
• #434 (relay.ts): the identity guard (sessions.get(sandboxId) !== own) plus own = undefined correctly stops both the double-teardown from end+error and the same-id reattach eviction. The test covers the exact interleaving.

One suggestion inline; nothing blocking.

🤖 Generated with Claude Code

[pdettori]

suggestion: server.close(cb) waits for all connections to end, including idle keep-alive ones from the ingress. In practice k8s SIGKILLs after the grace period so this can't hang forever, but a long-lived idle connection would delay deps.close() (Redis release) and the exit for the full grace period. Consider calling server.closeIdleConnections() (Node ≥ 18.2) right before server.close(...) so only in-flight requests hold the shutdown open.

[pdettori]

Addressed in bf95af7, but not the way the comment suggests, because I tested it on Node 22 and 25 first:

• Since Node 19, server.close() already closes connections that are idle when it is called. A socket that was idle at SIGTERM is closed in about 1 ms.
• The real gap is a keep-alive socket whose request is in flight at SIGTERM. It goes idle after close() runs, and nothing closes it, so the exit and deps.close() wait for keepAliveTimeout. Calling closeIdleConnections() before close() doesn't help there because the socket is busy at that moment. I measured the full keep-alive wait with it in place.

Fix: while draining, every response that finishes calls server.closeIdleConnections() (on setImmediate, so the socket has been marked idle). New test SIGTERM > does not wait out keep-alive on a connection whose request was in flight at SIGTERM sets keepAliveTimeout=60s, reuses one agent socket for /healthz and then a slow /readyz, and checks that the process exits within 2 s. It fails without the fix.