Attach lifecycle: track the gateway's ephemeral port (endpoint-aware markers + manual attach reads status.json)

[philcunliffe]

Problem

The daemon's gateway binds an ephemeral port by default (DEFAULT_LISTEN = '127.0.0.1:0'), so the port changes on every restart. The attach lifecycle (LLP 0044 / 0045) was built as attach once, done forever and never modeled a moving endpoint. Two gaps, one root cause.

Gap 1 - manual hyp attach <client> cannot resolve the live port. On a default install localEndpoint() throws (no in-process gateway in a CLI boot) and configuredGatewayEndpoint is undefined (endpointFromListen rejects port 0), so attach never reached a success path - even though the daemon persists its bound port to status.json.

Gap 2 - nothing re-attaches when the daemon rebinds. The boot reconcile pass is level-triggered on marker existence: a done attach marker short-circuits forever, and the marker recorded settings_path/prev_value but not the endpoint, so "attached, but at a stale endpoint" was unrepresentable. Every restart bound a new port, env.ANTHROPIC_BASE_URL kept pointing at the old one, and capture silently stopped.

Fix

• Endpoint-aware markers + re-attach on drift. action_attach.js perform() records the endpoint it attached at; the generic reconciler gains one optional, provider-agnostic hook ActionHandler.isCurrent(), so a done marker at a stale endpoint is a forward gap that re-perform()s instead of short-circuiting. The reconciler stays generic; the attach handler owns the marker.endpoint === ctx.endpoint comparison. Backward compatible: a pre-fix marker (no recorded endpoint) is treated as stale and self-heals on the first pass. Preserves LLP 0045's invariant - no live endpoint this pass leaves the existing attach untouched (auto-attach never records a URL for a port nothing bound).
• Manual attach reads the live port. hyp attach falls back to status.json's sources[].details.port via resolveLiveGatewayEndpointFromStatus, guarded by a daemon-liveness check (pid file + processIsAlive) - a dead-daemon snapshot is never used. The "already attached" branch now validates the recorded port against the live one: a stale-port marker re-attaches; a matching one is a genuine no-op.
• hyp status drift diagnostic (optional, included). A non-degrading client_attach_stale warning when a client's recorded port no longer matches the live gateway port (both already on disk).

Design

New decision LLP 0086 records the endpoint-aware-marker + re-attach-on-drift choice; additive Extended-by forward-refs added to LLP 0044 / 0045 / 0041 (settled text untouched). Code annotated with @ref LLP 0086#....

Verification

Reproduce tests (fail before, pass after):

• test/core/attach-live-port.test.js - Gap 1 (live-port discovery, liveness gate, stale-port re-attach, port-match no-op)
• test/core/attach-endpoint-drift.test.js - Gap 2 (rebind re-attaches, legacy endpoint-less marker self-heals, unresolved endpoint leaves attach untouched)

npm test 2098 pass / 0 fail (1 pre-existing skip); npm run typecheck and npm run build:types clean. Existing action-attach expectations updated for the newly-recorded endpoint detail.

Fixes #277

[philcunliffe]

🔍 neutral review (Codex + 2 Claude lenses): request_changes

Round 1, head 181f767. Thorough pass across the re-attach correctness, the LLP/contract/guidance, and Codex as the second family. The core fix is sound: both gaps are addressed, the LLP 0045 Part-1 invariant holds (auto-attach never records a URL for an unbound port, and when no live endpoint is resolvable this pass the existing attach is left alone), there is no re-attach storm on a stable install (endpoints compared as strings, a stable install short-circuits to skipped), legacy endpoint-less markers self-heal once without crashing, the pid-liveness gate rejects a dead daemon's status.json, and the new optional ActionHandler.isCurrent is backward compatible (a handler without it still short-circuits a done marker, so non-attach actions do not re-run every boot). LLP 0086 is coherent with resolving anchors, and the forward-refs to 0041/0044/0045 are additive (LLP 0045 Part-1 explicitly preserved). Three things to fix before merge:

Major: a failed re-attach can orphan a client attach

When a stale-but-applied done attach marker is re-performed after a rebind and that re-attach transiently FAILS, the marker is overwritten to failed with no recorded endpoint. If the action then becomes undesired (a config drop) before a successful retry, the reconciler treats the failed marker as never-applied and deletes it WITHOUT running reverse, so the client's ANTHROPIC_BASE_URL from the original attach is left orphaned on disk (untracked, pointing at a now-dead or possibly reused port). The PR's new re-attach path is what makes this reachable for attach.
Fix: preserve the applied state when a re-attach of a previously-done action fails (keep the prior endpoint, or mark failed-but-applied) so a later reverse still detaches. Add a regression: stale re-attach fails, action becomes undesired, reverse still detaches.

Must-fix (house style): em dashes and the @ref separator

CLAUDE.md now forbids the U+2014 em dash anywhere, and CI does not catch it. There are 15 new lines carrying em dashes, including the three additive forward-ref notes appended to the immutable LLPs (0041/0044/0045) and five @ref glosses. Five @ref annotations also use [implements] <emdash> gloss instead of the documented [implements]: gloss (the status.js refs already use the correct : form, so the file set is internally inconsistent).
Fix: replace every U+2014 on the new lines (use -, : , or restructure, matching the LLP 0086 body, which is already clean), and switch the five @ref separators to : .

Minor: TOML-probed clients (Codex) only partially covered

Gap 2's reconciler re-attach works for Codex too (it keys on marker.endpoint, not the probe port), so the core silent-capture-stop is fixed for both clients. But the MANUAL hyp attach "already attached at the live port" validation and the client_attach_stale diagnostic rely on a JSON probe port, which Codex (TOML) does not return, so manual hyp attach codex re-runs even when current and drift will not surface for Codex in hyp status. Non-blocking. Recommend a follow-up to extract the managed port from the Codex TOML block (with current and stale Codex tests), or fold it in here if straightforward.

Verdict: request_changes. neutral is applying the major and the house-style fix and will re-review the new head (round 2). Everything else verified clean.

Automated neutral review (Codex = independent second family; 2 Claude review lenses).

[philcunliffe]

Neutral review round - PR #278 (attach tracks the gateway's ephemeral port)

Head reviewed: 181f767feea1b474ffa2d2151c3e79c2f099e463
Verdict: approve with minor notes - no blocker/major correctness, security, or performance defect. Two minor findings below; neither meets the actionable-fix bar, so no code was changed and the head SHA is unmoved.

Reviewers: Claude (5 parallel focus passes + maintainer synthesis). Codex was launched but failed on an environment error (its own local proxy dropped the connection: stream disconnected before completion ... 127.0.0.1:60541/backend-api/codex/responses), so it contributed no review this round. Verdict computed on the Claude side alone.

What the change does

Extends the LLP 0044/0045 "attach once, done forever" model to an endpoint-aware one (LLP 0086): the attach marker records the endpoint it attached at; the reconciler gains an optional ActionHandler.isCurrent(marker, action, ctx) freshness hook so a done attach whose endpoint no longer matches the live gateway becomes a forward gap and re-attaches; manual hyp attach resolves the daemon's live ephemeral port from status.json (pid-liveness gated) instead of giving up; and hyp status emits a non-degrading client_attach_stale warning on drift.

Verified correct (no defect)

• Reconciler boolean logic (action_reconciler.js markerIsCurrent, used at the forward-gap guard): no inversion. isCurrent() === false -> not skipped -> re-perform; missing hook and a thrown predicate both degrade to "current" (skip), i.e. the pre-0086 level-triggered behavior. A successful re-perform rewrites marker.endpoint, so the next pass short-circuits: no churn loop.
• isCurrent guard (action_attach.js): live empty/undefined -> return true (leave the done in place, never churn it to failed); a pre-0086 endpoint-less marker self-heals with exactly one re-attach. isCurrent only returns false when live is a non-empty string, so the follow-up perform() is guaranteed to clear its own missing-endpoint guard.
• perform() records endpoint: endpoint is guaranteed a non-empty string at const detail = { endpoint } (the missing-endpoint guard returns failed earlier), so the marker can never record endpoint: undefined.
• Type consistency of the port compare: probe.port is normalized to a string (String(markerObj.port) in probeClientAttachFromDescriptor), and both portFromEndpoint(...) and liveGatewayPort (String(liveGateway.port)) are strings - all comparisons are string-vs-string, no 55555 === '55555' trap.
• resolveLiveGatewayEndpointFromStatus / gatewaySourceDetails: pid-liveness gate runs first (dead/absent daemon -> undefined), a readStatusFile throw is caught -> undefined, port validated as a positive integer, host defaults to 127.0.0.1, Array.isArray guard - no crash on malformed status.json, never returns a live endpoint for a dead daemon.
• clients.js control flow: every path through the unbound-gateway catch either continues, throws, or assigns endpoint = liveEndpoint, so endpoint is never read unassigned at client.attach. The old endpoint = configured line was correctly folded into endpoint = configuredGatewayEndpoint(...); no dangling variable.
• client_attach_stale: only computed when daemon.running; string-vs-string port compare; non-degrading (never flips overall), matching client_attach_missing per LLP 0041.
• Tests: the 12 new attach tests (endpoint-drift, live-port, dead-daemon liveness gate, backward-compat legacy marker) plus the updated action-attach suite all pass at this head (31/31). Coverage matches the decision doc's guards.

Findings

F1 - Minor (style, not fixed): new lines introduce U+2014 em dashes, violating the repo rule.
CLAUDE.md states "No em dashes (the U+2014 character) anywhere: code, comments, JSDoc, strings, or docs." The diff adds em dashes on new comment/JSDoc lines in src/core/commands/clients.js (the two @ref LLP 0086 ... [implements] — lines), src/core/config/action_attach.js (3), src/core/config/action_reconciler.js (3), src/core/config/types.d.ts (1), src/core/daemon/status.js (2), and test/core/attach-endpoint-drift.test.js (1).
Why not fixed: it is a cosmetic house-style nit with no functional effect, and these four files are already saturated with pre-existing em dashes (the @ref ... [implements] — gloss form is the established local convention on prior lines in the same files), so touching only the new lines would be inconsistent and would churn the head SHA for zero functional gain. Flagged for a human/author to sweep if desired; the .md (LLP) forward-refs added by the same PR also contain em dashes.

F2 - Minor (behavior change, not a defect): TOML-probe clients (codex) lose the "already attached, nothing to do" fast path against a live daemon.
probeClientAttachFromDescriptor never populates port for a TOML-format probe (codex), so probe.port is always undefined. In runClientLifecycle, when a live daemon endpoint is discovered, alreadyCurrent reduces to attached && (false || (undefined !== undefined && ...)) = false, so an already-correctly-attached codex client falls through to client.attach on every hyp attach instead of taking the no-op branch it took pre-#277. client.attach is idempotent, so this is a marker re-write, not a correctness bug - and it is arguably intended (a port we cannot verify is conservatively re-pointed). Note the same clients cannot surface the client_attach_stale diagnostic (no recorded port to compare), though the reconciler's isCurrent still corrects their drift via the marker endpoint. If the no-op path is wanted for codex, treat liveEndpoint !== undefined && probe.port === undefined as "cannot verify -> assume current" rather than "stale."

Disposition

Sound, well-tested, faithful to LLP 0086 and preserves the LLP 0045 Part 1 proven-bound-endpoint invariant. No actionable (blocker/major) finding; no fix pushed; head SHA unchanged at 181f767. Held for the human merge gate.

[philcunliffe]

neutral PR-health review, round 2

Head reviewed: 181f767feea1b474ffa2d2151c3e79c2f099e463 (unchanged since round 1)
Verdict: findings, none blocking. No actionable (correctness / security / performance) defect. No code pushed; head unchanged.

Codex: failed again on the same local-proxy environment error as round 1 (stream disconnected before completion ... /backend-api/codex/responses, plus a model-refresh failure against 127.0.0.1:60541). Round 2 is again effectively Claude-only. This is an environment fault, not a signal about the PR.

Independent Claude review: full read of the diff plus two parallel review subagents (bug/logic scan and test/contract scan), each reading the actual files (producers and comparers), and the 31 affected tests run green. The core logic is sound and all endpoint/port comparisons are internally consistent:

• The reconciler compares the full URL marker.endpoint === ctx.endpoint, both sourced from the daemon's in-process localEndpoint(), so they share a format by construction.
• The manual-attach alreadyCurrent check and the client_attach_stale diagnostic both compare String(number) ports: probe.port (String(markerObj.port) in probeClientAttachFromDescriptor) vs portFromEndpoint() / String(liveGateway.port). No number-vs-string mismatch.
• isCurrent is throw-safe (markerIsCurrent catch to true), both documented guards behave (no live endpoint to true; pre-0086 marker with no endpoint to re-attach once), the liveness gate in resolveLiveGatewayEndpointFromStatus correctly rejects a dead-daemon snapshot (attach-live-port test 4: dead pid to exit 1, zero attach calls), and the optional isCurrent hook breaks no existing handler (backfill omits it and stays permanently done).

Findings

F1 (style, non-actionable) - em dashes on new lines. 12 added source/test lines carry U+2014 em dashes (@ref annotations and JSDoc in src/core/commands/clients.js, src/core/config/action_attach.js, src/core/config/action_reconciler.js, src/core/daemon/status.js), violating the repo "no em dashes anywhere" rule (CLAUDE.md Code Style). Left unfixed, matching round 1: these four files already carry 55 pre-existing em dashes on master (20 / 17 / 3 / 15), so a new-lines-only sweep would be inconsistent and churn the head for no real gain. Worth a batch cleanup someday, not on this PR.

F2 (behavior, non-actionable) - TOML-probe clients (codex) re-attach every manual hyp attach. probeClientAttachFromDescriptor records a port only for JSON-format clients; TOML clients (codex, matched by marker_header) return port: undefined. So in runClientLifecycle, alreadyCurrent is always false whenever a live endpoint is discoverable, and hyp attach codex on a running daemon always falls through and re-writes config.toml (changed) instead of the pre-#277 no-op. It is idempotent and prev_value-safe, so this is a louder-output / redundant-write behavior change, not a defect, and arguably intended since a portless probe cannot self-diagnose drift. Same conclusion as round 1. (src/core/commands/clients.js:227-264)

F3 (test gap, non-blocking - NEW this round) - the client_attach_stale diagnostic has zero test coverage. The PR's third pillar (about 96 new lines in status.js, src/core/daemon/status.js:508-527) is not exercised by any test: neither the firing case nor the non-firing guards. The guard predicate liveGatewayPort !== undefined && probe.port !== undefined && probe.port !== liveGatewayPort is exactly the false-positive-prone shape where a dropped guard would nag client_attach_stale on every hyp status for a portless (codex/TOML) client, or for a stopped daemon. The code is verified correct by independent trace, so this is a coverage gap, not a runtime defect - left for the author because a faithful test needs a new collectHypAwareStatus integration fixture (a discoverable client descriptor with an attachProbe, a written client-settings marker recording a port, and a live status.json gateway source at a mismatched port), which the existing status suite does not build. Suggested follow-up: a status test asserting the diagnostic (1) fires on a port mismatch, (2) stays silent on a match, (3) stays silent when probe.port is undefined, (4) stays silent when the daemon is not running. A companion minor gap: the isCurrent-throws-to-current branch (action_reconciler.js:283-285) is also untested.

Non-issue noted for completeness: auto re-attach self-heals a daemon restart-rebind (the boot reconcile pass observes the drift) but not an in-process SIGHUP/central-config reload-rebind (the client seam endpoint is frozen for the daemon lifetime and reload schedules no reconcile pass). This is pre-existing, not regressed by this PR, honestly scoped out in LLP 0086 (D1 / Consequences say "restart"), and is now at least surfaced by the new client_attach_stale diagnostic and repairable via hyp attach.

Disposition

Hold for the human merge gate. No blocker or major correctness / security / performance defect; F1 and F2 confirm round 1 and stay non-actionable; F3 is a genuine test-coverage gap on correct code, recommended as author follow-up rather than a merge blocker.

[philcunliffe]

Review round 3 (confirmation) — approve

Independent, focused re-read of head 181f767 (unchanged since rounds 1 and 2). Not a full re-audit: two thorough independent Claude reviews already ran and both approved. This round targets any TRUE blocker/major defect the prior rounds might have missed. None found.

Codex

Attempted once (codex exec, gpt-5.5 via the hypaware provider). Failed again on the known local-proxy env error — this repo intercepts Codex's /backend-api/codex/responses traffic (stream disconnected before completion), same failure as rounds 1 and 2. Proceeded Claude-side.

What I traced independently

• clients.js alreadyCurrent control flow — verified all four quadrants of probe.attached === true && (liveEndpoint === undefined || (probe.port !== undefined && probe.port === livePort)): attached@live-port → no-op; attached@stale-port or attached-but-port-unknown against a live daemon → re-attach via endpoint = liveEndpoint fall-through; daemon-down + marker present → no-op success; daemon-down + no marker → actionable error/throw. Matches LLP 0086 D2. The removed trailing endpoint = configured is subsumed by the new endpoint = configuredGatewayEndpoint(ctx.config) at the top of the catch — the configured-listen path is preserved.
• Endpoint format consistency (loop risk) — perform() records detail.endpoint = ctx.endpoint and isCurrent() compares marker.endpoint === ctx.endpoint, both reading the same ctx.endpoint in a given reconcile pass. No format skew, so a re-attach records exactly what the next pass compares against — self-heals to current, no churn loop.
• Reconciler isCurrent hook — throw-safe (markerIsCurrent treats a throwing predicate as current → degrades to pre-0086 level-triggered skip, never a re-perform loop); handlers without the hook (backfill) stay permanently done. Reverse-gap logic untouched.
• resolveLiveGatewayEndpointFromStatus — liveness gate is first and hard (pid file + processIsAlive); a stale snapshot from a dead daemon yields undefined, never a fabricated port. gatewaySourceDetails validates port as a positive integer.
• client_attach_stale diagnostic — non-degrading (never flips overall), string-vs-string port comparison, only fires for a configured+attached client with a known recorded port against a live gateway port. Correct by trace.
• Tests — the 3 attach test files (attach-live-port, attach-endpoint-drift, action-attach) run green locally: 31 pass, 0 fail, covering both gaps, the legacy-marker backward-compat path, the unresolved-endpoint leave-alone guard, and the dead-daemon liveness gate.

Residual findings (all non-blocking; concur with rounds 1 and 2, would ship as-is)

• F1 (style) — 12 new lines add U+2014 em dashes vs the repo no-em-dash rule; the touched files already hold ~55 pre-existing ones, so a new-lines-only sweep is inconsistent churn. Non-blocking.
• F2 (behavior) — TOML-probe clients (codex) record no port, so alreadyCurrent is always false against a live daemon and they idempotently re-attach every hyp attach. Idempotent and prev-value-safe; arguably intended. Non-blocking.
• F3 (test gap) — the client_attach_stale diagnostic in status.js has no direct test. Verified correct by trace; a faithful test needs a new status-collection fixture, out of scope for a review fix. Non-blocking.

Verdict

clean — no blocker/major correctness/security/regression defect. Nothing fixed this round (no truly-actionable finding; F1/F2/F3 remain non-actionable). Concur with rounds 1 and 2: approve.