feat(cli,mcp)!: clients renew their own leases (ADR 0004, PR A) - #124
Merged
Merged
Conversation
V3RON
force-pushed
the
claude/adr-0004-a-client-renew
branch
from
September 7, 2026 13:05
c81c644 to
5db801d
Compare
V3RON
changed the base branch from
claude/simlock-host-worker-prd-ohc30e
to
claude/adr-0005-120-docs
September 7, 2026 13:12
V3RON
force-pushed
the
claude/adr-0004-a-client-renew
branch
from
September 7, 2026 14:08
5db801d to
13a1800
Compare
V3RON
added this pull request to stack #123
September 9, 2026 16:16
ADR 0004 makes a client-initiated `lease.renew` the only thing that keeps a lease alive, and turns "held" into a client policy rather than a daemon mode. This is the first slice: the client side starts behaving that way while the daemon, the contract, the core, and the HTTP gateway stay exactly as they are. - new `src/lease-policy`: a renew timer that fires at a third of the remaining TTL, recomputed from the deadline the daemon returned with every grant and renewal (never from a config value the client does not have), stops on the first failure, and guarantees nothing runs after `stop()`. - `simlock lease` (held mode) runs that timer through the `Clock` port and releases explicitly on exit, parent death, or SIGINT/SIGTERM, before the connection closes. `--detach` is untouched: print, exit, no timer. - the MCP session does the same for its own lease, and releases it on session end (bounded, best-effort) instead of letting the socket do it. - neither frontend declares the `heartbeat` capability any more, and `simlock/client`'s `heartbeat` connect option defaults to `false`. The client still starts no renew loop of its own: that stays a frontend concern (ADR 0003 section 10). `mode: "held"` still goes over the wire, and the daemon still releases on connection close; PR B removes both. Part of #114 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz
Findings from the adversarial review of the first commit: - a single failed renewal ended liveness for good, which is worse than the daemon push it replaces (that one retried every interval, forever). An attempt is now retried on the same one-third cadence until the lease's own last known deadline has passed, and every attempt is reported. - a renewal the daemon accepts and never answers is now bounded by the interval it was scheduled on: the wire has no request timeout, so a hung daemon used to cost the whole lease in silence. - a deadline that comes back not in the future ends renewal instead of spinning at the 250ms floor. - the CLI's release moved into the `finally`: a throw between the grant and the wait (a `--export-env` key that is not a shell identifier, an EPIPE on a closed pipe) skipped it, and only the daemon's release-on-close -- which PR B removes -- was covering that. - the MCP session stops renewing only once a release has actually succeeded. Stopping before the call left a session that still held the device with no timer keeping it; a renew that overtakes a successful release cannot resurrect anything, since the registry has already dropped the record. Tests: retry, give-up-at-the-deadline, unanswered-renewal and expired-answer cadences; the CLI's stderr report for a failed renewal, its release on a throwing print, and its held-mode `heartbeat: false` at hello (the previous test only covered `--detach`); the MCP session's renewal surviving a failed release. Part of #114 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz
- an abandoned renewal that turns out to have reached the daemon now contributes its deadline, so the give-up check can no longer fire against a deadline the daemon has already moved. - a throwing `onError` can no longer take the process down through the timer's own `void tick()`, which would have skipped the release the holder still owes; giving up permanently now reports itself distinctly from the attempt that failed. - the CLI's farewell release is bounded exactly like MCP's, through one shared helper rather than two opposite decisions in one change. - `SIGHUP` joins SIGINT/SIGTERM: ADR 0004 §2 says "a catchable signal", and a closing terminal is the likeliest way a backgrounded holder dies. - `close()` no longer sends a second `lease.release` for a lease whose release a tool call already has in flight. - `CliEnvironment.now` is gone: `clock` is required now, and the old field's `Date.now` fallback was a standing architecture-rule-9 violation. Tests: the cadence test now answers a deadline the grant's own could never produce (it passed with the reschedule deleted); the give-up test walks the retry ladder instead of one attempt; new coverage for a late answer moving the deadline, for release-on-SIGHUP/SIGINT/SIGTERM, and for the release-in-flight guard on close. Part of #114 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz
… the lease A renewal answered `UNKNOWN_LEASE` or `FORBIDDEN` is the daemon's final word, not a failed attempt: the record is gone, or was never this principal's. Retrying it printed the same rejection on every step of the ladder, gave up at the deadline, and left the holder alive -- with a doomed farewell release still to come. `startLeaseRenewal` now stops on those two codes and calls a new `onLeaseGone`; the CLI ends exactly as it does for a `lease-lost` push (exit 14, no release), and the MCP session clears its held lease and fans out a lease-lost notice, which is the only signal an agent gets when the lease expired while its connection was idle. Also from the same review: - `McpSession.release` marks the release in flight before `#clientForUse()`, which can reconnect: `close()` landing in that window no longer sends a second release. - a `renew` that throws synchronously is a failed attempt like any other, rather than a silent end swallowed by the unawaited tick. - SIGHUP is dropped again. It changes behaviour a user can see (`nohup simlock lease … &` would start dying on hangup) and MCP does not trap it, so the parity it claimed was not real. SIGINT/SIGTERM only, as before. - comment and test-description fixes: an attempt is bounded by a third of what is left of the TTL when it starts, not "the interval it was scheduled on"; `e2e/heartbeat-ttl.test.ts` says renew where it meant heartbeat, and drops a `lastHeartbeatAt > 0` assertion that never discriminated anything. Tests: the terminal answer per frontend and in the timer itself (both codes), a synchronously throwing renew, `awaitWithin`'s three outcomes on a fake clock, and a CLI farewell release the daemon never answers. Part of #114 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz
… release
Two ways `close()` could go wrong while a tool call was in flight, both found
by review:
- a `lease()` whose grant landed after `close()` set `#heldLeaseId` and armed
a renew timer on a session that was already gone. Nothing stopped it again,
and since the MCP runner never calls `process.exit`, `simlock mcp` could sit
alive renewing a lease on a dead client until the backstop. A grant that
arrives after the session closed is now handed straight back (bounded,
best-effort) and the call fails `SESSION_CLOSED`.
- the release-in-flight guard added last round turned "two releases" into
none: `close()` skipped its farewell release because a tool call had one on
the way, then closed the wire, which rejected that very request. The guard
is gone -- `close()` always sends its own release, and a duplicate costs one
`UNKNOWN_LEASE` it already swallows. Under PR A the daemon's release-on-close
hid this; PR B removes that crutch.
Also:
- lease-lost notices are deduplicated by lease id, so a lease that a renewal
found gone and the daemon then pushed is one ending, not two.
- the CLI writes the same `{"push":"lease-lost",...,"reason":"renew-rejected"}`
stderr line for a renewal the daemon rejects as it does for the push, with
the error line after it as detail: a script tailing for `push: "lease-lost"`
sees both ways of losing a lease.
- an abandoned renewal's late answer may only move the deadline while nothing
newer has been answered, which is what its own comment claimed; a fresh
answer stays authoritative in both directions.
- the module doc says what deriving the cadence from `ttlDeadline - now()`
assumes (one clock, true on the unix socket) and how PR B's stored `ttlMs`
turns it into `ttlMs / 3` with the deadline as a bound.
- the two dead `termination !== undefined` conditions collapse to one, and
`mcp/connect.ts`'s now-inert `heartbeat` option is labelled for PR B.
Part of #114
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz
…rs newest-first Three findings from the third isolated review: - two abandoned renewals that both answered late were merged by taking the larger deadline, so a stale 900_000 could override a newer, shorter 12_000 and leave the holder renewing long after the lease was gone. Every attempt is numbered now and every answer -- late or in time -- is applied by that number, newest wins, in both directions. - a second `lease()` overwrote the session's single renewal, leaving the first lease neither renewed nor released: a regression against the daemon heartbeat, which slid every held lease on the connection. Renewals are keyed by lease id, every one is renewed, and every one is released on close. No client-side "one lease per session" rule is added: the daemon's one-lease-per-requester check is the authority (ADR 0003 section 11), and if it refuses the second request the map simply holds one entry. - `close()` ran straight into the wire close, so a grant landing from an in-flight `lease()` had no live connection to be handed back on. It now waits for the tool call in flight, bounded by the release timeout, before releasing and closing. `FakeSimlockClient` rejects every call after `close()` the way the wire does, so a test cannot pass on a dead connection any more. Also: the scheduled delay is capped at the largest delay a Node timer can express, so a deadline more than ~24 days out cannot truncate into a hot loop; `#announcedLost` entries are dropped with the lease they belong to; the CLI's single `lease-lost` line is now a flag both paths share rather than an ordering accident; and the MCP frontend builds one `Clock` for the session and the auto-launch retry loop. BREAKING CHANGE: `simlock/client` and `simlock/admin` default the `heartbeat` connect option to `false`, so a held lease is kept alive only by its holder's own `lease.renew` (ADR 0004 sections 1, 2 and 4), and `client.heartbeat()` answers `BAD_REQUEST` unless the caller opts back in. Part of #114 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz
…renewal gives up Two findings from the fourth isolated review: - `McpSession#endSession` bounded each step of the shutdown separately -- the wait for the in-flight tool call, then one more per lease -- so an MCP server facing a dead daemon could sit for a multiple of the budget after stdin EOF. The whole tail now runs inside one `RELEASE_TIMEOUT_MS`: wait for the call in flight, release every lease, and close the wire in a `finally`, whichever comes first. - renewal's two "this lease cannot be saved" endings -- the deadline passing mid-ladder, and a daemon answering with a deadline already behind us -- were reported through `onError` and left the holder alive with no timer: the CLI printed an INTERNAL line and exited 0 on Ctrl-C, and MCP raised no notice, leaked the renewal entry, and released a dead lease on close. Both now end through `onLeaseGone`, which carries a reason: `renew-rejected` when the daemon said the lease is gone or not ours, `renew-failed` when renewal ran out of runway. The CLI writes its one `lease-lost` line with that reason and exits 14 either way; the MCP session raises the matching notice and stops counting the lease as its own. Also: `stop()` now prevents the renewal call itself when it lands in the microtask between the timer firing and the request going out, and `#announcedLost` is capped rather than growing with every lease a long session loses. Tests: the total shutdown budget (one `RELEASE_TIMEOUT_MS`, not two); a grant that lands late in that window still released before the wire closes; the give-up ending per frontend; one `lease-lost` line when a rejected renewal and the daemon's push both arrive; a stop that beats the request to the wire; a 90-day deadline clamped to the largest delay a timer can express; and the fake client rejecting calls after `close()` the way the wire does. Part of #114 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz
V3RON
force-pushed
the
claude/adr-0004-a-client-renew
branch
from
September 9, 2026 16:18
13a1800 to
e58c9ef
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First of three slices of ADR 0004. The client side starts behaving the way the ADR describes; the daemon, the contract, the core, and the HTTP gateway are untouched, and
mode: "held"still goes over the wire. Behaviour is otherwise unchanged from a user's point of view: a held lease still ends promptly when its holder exits, and still ends on the daemon's backstop if the holder isSIGKILLed.Part of #114What changed, and which part of the ADR it implements
§2 — "'Held' is a client policy, not a daemon mode." New module
src/lease-policy, the renew timer both frontends share: it fires at one third of the remaining TTL, computed from the deadline the daemon returned with the grant and with every renewal since — never from a config value the client does not have. A failed attempt is retried on the same shrinking ladder while the lease can still be saved; each attempt is bounded by a third of what is left of the TTL when it starts, because the wire has no request timeout and a daemon that acceptslease.renewand never answers would otherwise cost the whole lease in silence. Every attempt is numbered and every answer applied by that number — two abandoned attempts can answer in either order, and the newer one's deadline is the truth, shorter or longer.Renewal ends for exactly two reasons, and says which:
renew-rejected(the daemon answeredUNKNOWN_LEASE/FORBIDDEN— the lease is gone or was never ours) andrenew-failed(the deadline passed with no usable answer, or the daemon answered with one already behind us). Both reach the frontend throughonLeaseGone;onErroris only for attempts that failed while the lease was still savable. A holder never ends up alive with no timer and no lease.§2 —
simlock lease(held mode) runs that timer through theClockport and releases explicitly on exit, parent death,SIGINT, orSIGTERM. The release moved into the command'sfinally, so no exit path can skip it — including a throw between the grant and the wait, such as a--export-envkey that is not a shell identifier — and it is bounded, so an unresponsive daemon costs the wait rather than the exit. Losing the lease writes exactly one{"push":"lease-lost",…}stderr line however it is found out — the daemon's push, or renewal ending with its reason — and exits 14 without a farewell release.§2 — the MCP session does the same for every lease it holds: one renew timer per lease id, each stopped when that lease stops being the session's (a successful release, a
lease-lostpush, renewal ending, orclose()), and every held lease released on close. Nothing here limits a session to one lease — the daemon's one-lease-per-requester rule is the authority (ADR 0003 §11) — so a second grant can no longer strand the first, which the daemon's heartbeat used to keep alive.close()runs its whole tail (wait for the tool call in flight, release every lease, close the wire) inside oneRELEASE_TIMEOUT_MSbudget, so a grant landing just after shutdown is still handed back on a live wire while a dead daemon can only ever cost that one budget. Lease-lost notices are deduplicated by lease id.§4 — the daemon-initiated heartbeat goes away. Neither frontend declares the
heartbeatcapability athello, and the client default flips — see the breaking-change note at the top. The client itself still starts no renew loop: renew and reconnect policy stays a frontend concern (ADR 0003 §10,docs/CLIENT.md), which is whysrc/lease-policyis a helper frontends choose rather than somethingconnectSimlockdoes for them.§3 — connection close means nothing to a lease is not implemented here: the daemon still releases on close, which is what keeps a
SIGKILLed holder's device coming back immediately today.--detachis byte-identical: it prints, exits, and arms no timer.Deliberately left for PR B
lease.heartbeat, theheartbeathello capability, and the daemon's held set / release-on-close.modeonlease.request(still sent as"held"), and thelease.heldTtlBackstopMs/lease.heartbeatIntervalMs/lease.detachedTtlMsconfig keys.src/mcp/connect.ts'sheartbeatoption is left in place, labelled, so B does not get a conflict on the file it is editing.ttlDeadline - clock.now()presumes daemon and client share a clock epoch — true on the unix socket, not across a network hop. ADR 0004's storedttlMs(PR B) turns the interval intottlMs / 3, a duration that needs no shared epoch, with the deadline left as the bound it really is. The module doc says so at the one place that changes.e2e/heartbeat-ttl.test.ts's filename still names the heartbeat, and itsheartbeatIntervalMs: 800override has to stay for now — the config validator still requiresheartbeatIntervalMs <= heldTtlBackstopMs / 4, and that suite pins the backstop to 4s.SIGHUP. ADR §2 says "a catchable signal", and one review round argued for adding it here; it was tried and reverted, because catching it changes behaviour a user can see (nohup simlock lease … &would start dying on hangup) and MCP traps no signals at all, so the parity it claimed was not real. It belongs with PR B, where explicit release actually becomes load-bearing.Docs
No doc changes here, and no ADR status change: PR C (#122) owns exactly those lines, and the stack merges in one go, so editing them here would only hand C a rebase conflict.
Intended merge order: #121 (the ADR branch, which this PR is based on) → #122 (docs rewritten, ADR 0004 marked Accepted — not yet implemented, which
AGENTS.mdexplicitly allows ahead of the code) → #124 (this PR) → #125 (PR B), then a one-line follow-up flips ADR 0004 to Accepted. That is what keepsmainfrom contradicting itself: the accepted record describes the end state before any slice of code lands, and the docs and the code become true together at the end of the stack. It is also why theheartbeatdefault flip ships here rather than waiting: PR B deletes the option one PR later and PR C documents the end state, and the stack merges in order, so no consumer ever sees the intermediate state in a release.The passages #122 rewrites, so they are not lost:
docs/ARCHITECTURE.md(~line 420, thelease.heartbeatpush and held-lease liveness),docs/CLI.md:251-256(a held lease'slease renew --ttlnow does stick),docs/known-pitfalls.md:32("CLI held mode now declares theheartbeatcapability"),docs/EVENTS.md:17,19(lease.renewed/lease.expireddescribed in terms of the push),docs/CONFIGURATION.md:19(lease.heartbeatIntervalMs, now inert), anddocs/CLIENT.md(no lease-liveness section at all, and now theheartbeatdefault flip to document).Adversarial review
Seven isolated rounds, each by a reviewer given only the ADR, the agent rules, the issue, and the diff.
Round 1 — MERGE AFTER FIXES. A single transient renew failure ended liveness for good (fixed: retry until the deadline); a failed MCP
release()left a lease nothing renewed (fixed); the CLI's release was not on every exit path (fixed: moved intofinally); the 250ms floor bounded a hot loop at 4 req/s rather than ending it (fixed); no watchdog on the renew request (fixed); the capability test only covered--detach(fixed); missing failure-path tests (fixed); a false comment (fixed). Docs/ADR status declined (see above); theheartbeatdefault flip kept.Round 2 — MERGE AFTER FIXES. An abandoned renewal's answer was discarded, so the give-up check could fire against a stale deadline (fixed); a throwing
onErrorcould reject the timer's unawaited tick and kill the process past the release (fixed); the CLI's farewell release was unbounded while MCP's was bounded (fixed: one shared helper);close()could double-release (guarded, then reworked in round 4); the cadence test did not discriminate the returned deadline (fixed); the give-up test exercised one attempt, not the ladder (fixed);CliEnvironmentcarried bothclockand aDate.now-falling-backnow(fixed).Round 3 — MERGE AFTER FIXES, run with test execution.
UNKNOWN_LEASEon renew treated as transient (fixed:onLeaseGone); no tests forawaitWithin(fixed);#releaseInFlightset after a call that can reconnect (fixed, then removed); SIGHUP dropped (done); a synchronously throwingrenewended renewal silently (fixed); comment and e2e-wording nits (fixed).Round 4 — MERGE AFTER FIXES, two blocking findings in
close(). Alease()racingclose()armed a timer on a closed session (fixed: the grant is released and the call failsSESSION_CLOSED); the release-in-flight guard turned two releases into zero (fixed: the guard is gone). Plus notice dedupe, thelease-lostline on the renew-rejected path, dead conditions collapsed, the clock-epoch note, and the breaking-change marker — all done.Round 5 — MERGE AFTER FIXES. Two late answers merged by taking the larger deadline (fixed: every attempt numbered, newest answer wins); a second
lease()overwrote the session's single renewal (fixed: renewals keyed by lease id, all renewed, all released);close()ran straight into the wire close so a late grant had no live connection (fixed: it waits for the tool call in flight, andFakeSimlockClientnow rejects calls afterclose()the way the wire does). Plus the timer-delay ceiling,#announcedLostpruning, the structurallease-lostflag, and oneClockfor the MCP frontend.Round 6 — MERGE AFTER FIXES.
simlock mcpcould sit 10-15s after stdin EOF against a dead daemon. Fixed:#endSessionputs the whole tail (wait, then every release) inside a singleawaitWithin(RELEASE_TIMEOUT_MS)and closes the wire in afinally, so the connection stays open until the last release settles or the budget is spent. Covered by a test asserting the total is exactly one budget, and by the existing late-grant test, which still gets its release completed inside it.onErrorand stopped, so nobody learned the lease was gone — the CLI sat alive and exited 0 on Ctrl-C, MCP leaked the renewal entry and released a dead lease on close. Fixed: both go throughonLeaseGonewith reasonrenew-failed(againstrenew-rejectedfor the daemon's own refusal). The CLI writes itslease-lostline with that reason and exits 14; the MCP session raises the matching notice, drops the renewal, and releases nothing for it. Tested per frontend.simlock/clientdefault flip stays. Declined as directed, with the reasoning in the Docs paragraph above.4-5. Docs and ADR status. Declined as before — docs: describe TTL-first leases end state, accept ADR 0004 (PR C) #122's, merge order above.
stop()must prevent therenewcall itself. Fixed: the deferred call checksstoppedand abandons instead of calling. Test asserts zerorenewcalls for a stop landing in that microtask.push: "lease-lost"line and exit 14; aFakeSimlockClientcall afterclose()rejectsDAEMON_CONNECTION_LOST; a ~90-day deadline schedules a delay clamped toMAXIMUM_RENEW_DELAY_MS.#announcedLostis capped (oldest id dropped past 64);mcp/connect.ts's dead option and the CLIfinallyordering left as they are.Final review — MERGE, with no blocking findings. Its remaining non-blocking items are carried by #125 (PR B), which has rebased onto this branch and owns
src/mcp/session.tsandsrc/lease-policy/index.tsfrom here: a self-initiatedrelease()racing the renew timer into a falselease-lostnotice;giveUpnot honouring a re-entrantstop(); tests for the never-launch property, the#announcedLostcap, and the fake's dead guard on every method; the abandoned shutdown loop; and the nested#releaseQuietlytimer.Also noted by the reviewers, not defects:
lastHeartbeatAtinsimlock status/list --leasesnow reflects a renewal everyheldTtlBackstopMs / 3instead of a ping everyheartbeatIntervalMs.Validation
pnpm check— typecheck, e2e typecheck, lint, format check, 1366 unit tests, and the fake-driver e2e suite (15 files, 47 tests;slow-*excluded, they need a Mac). Run twice at the end of every review round; green both times, most recently onc81c644.The e2e suites this touches most —
held-lease-liveness,heartbeat-ttl,parent-watch,mcp-session— pass unchanged;heartbeat-ttl's "both held leases slide past the backstop" assertions now hold because the clients renew, not because the daemon pings. The only e2e edits are comments, a test title, and one dropped assertion (lastHeartbeatAt > 0, which held for any lease at any time).Several of the new tests were mutation-checked against the code they cover (the close-race guard, the newest-answer guard, both late-answer orderings): each fails when its guard is removed.
🤖 Generated with Claude Code
https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz