docs: describe TTL-first leases end state, accept ADR 0004 (PR C) - #122
Merged
V3RON merged 8 commits intoSep 9, 2026
Merged
Conversation
Rewrite every user-facing document for the lease model ADR 0004 decides: one kind of lease, TTL-bound on every transport, kept alive only by a client-initiated lease.renew. Held mode stops being a daemon concept — staying alive to renew and releasing on exit is now the CLI's and the MCP session's own policy over an ordinary lease — and connection close releases nothing anywhere. Flip ADR 0004 to "Accepted — not yet implemented" in the ADR and in the index, so the docs below are the specification the code catches up to. Config: lease.defaultTtlMs and lease.maxTtlMs documented; the three retired lease.* keys get a migration table. Contract: lease.heartbeat, the heartbeat capability, and mode are gone; protocol 4. Behaviour: a holder killed with SIGKILL keeps its device until the TTL expires, stated wherever held mode used to promise instant release, and recorded as a known pitfall with its knob. Docs only; no src/ or e2e/ changes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz
Four fixes from an adversarial pass over the ADR 0004 doc rewrite: - ARCHITECTURE.md no longer prescribes an MCP reconnect sequence the ADR does not decide; it states the fact (the lease survives, the session picks it back up and keeps renewing) instead. - CLIENT.md stops attributing to ADR 0003 §6 a reason ADR 0004 invalidated. The decision is 0003's; what a daemon stop costs is now stated in 0004's terms - queued requests die, leases burn TTL with nothing to renew against. - CLI.md's `daemon stop` section says plainly that stopping the daemon does not touch leases, where an operator actually reads about it. - Re-wrap prose the rewrite left over the file's line width. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz
Apply the second review round's 15 findings. Six of them were places the
docs hedged or guessed where ADR 0004 was silent; those are now stated as
the end state PR A and PR B implement.
- A running `simlock lease` does not reconnect (ADR 0003 §10 stands). On a
dead connection it writes one DAEMON_CONNECTION_LOST line naming the lease
and its ttlDeadline and exits 1, leaving the lease standing for a later
invocation to renew. Exit 14 keeps its narrower meaning: the daemon ended
the lease while the connection was alive.
- The lease record stores its ttlMs, and a body-less renew re-applies that
width. lease.defaultTtlMs applies only to a request that names no ttlMs, so
a four-hour lease no longer shrinks to fifteen minutes on its first renew.
This is what makes the holder's timer safe: it sends no TTL at all.
- Renew cadence is "one third of the lease's TTL" everywhere, per ADR §2.
- The MCP session reconnects eagerly from its renew timer rather than lazily
on the next tool call, which would let its own lease expire in between.
- lastRenewedAt is a new stored field, not a rename: lastHeartbeatAt was
derived as ttlDeadline - heldTtlBackstopMs, which per-lease TTLs break.
- No lease.detachedTtlMs alias. All three retired keys warn and are ignored
like unknown keys, as the ADR says; the migration table says to copy the
value across. Load-time rule added: both TTL keys positive, default <= max,
rejected rather than clamped.
Also: notices is HTTP-side (LeaseNoticeBuffer), never the socket lease.renew
response; the protocol range {min: 4, max: 4} is stated in ARCHITECTURE's
contract section, not only the changelog; EVENTS declares the lease payload
removals a deliberate one-off exception to the additive-only rule and marks
the four rows payload-pending; ADR 0004's Supersedes line names every ADR
0003 section it narrows; and the reparented-holder pitfall now explains that
a TTL bounds silence, which a live renewing holder is not.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz
Third review round, 14 findings. Two were decisions the docs had invented rather than inherited, so they are recorded in ADR 0004 itself — this PR is what accepts it — and every other doc now follows the record instead of leading it. - MCP's reconnect is timer-driven but never launches a daemon. ADR 0003 §10/§11 gave MCP one lazy trigger, the next tool call; Decision 2 now adds the renew timer as a second, so an idle session does not lose its lease waiting for a call that never comes, and restricts it to a daemon that is already listening. Auto-launch stays a tool-call concern, because an operator's `daemon stop` must not be undone by an idle session. The Supersedes line names §10/§11, and ARCHITECTURE, CLIENT and the changelog say the same thing. - The events payload removals are the ADR's exception to take, not EVENTS.md's. A Consequences bullet grants it once, while the package is 0.x; the EVENTS.md note now cites that bullet rather than granting it. Also settled: a renew that fails transiently on a live connection is retried on the next tick, while one answered UNKNOWN_LEASE ends the holder at exit 14 like a lease-lost push (CLI.md, ARCHITECTURE); and a TTL config that contradicts itself is rejected at load and the daemon does not start, which is deliberately not the warn-and-ignore treatment the retired keys get (CONFIGURATION, changelog). Corrections: the `lease.defaultTtlMs` config cell no longer claims to be the renew fallback, which was the round's most load-bearing error; cross-process `lease renew` needs an admin credential, so it joins the admin command list and the two places that showed it as a bare follow-up invocation; HTTP's additive-evolution promise names the one break ADR 0004 makes in it; `lease_lost` is documented as the fact `notices` cannot carry, since an ended lease answers renew with 404; the HTTP-tracker pitfall no longer explains itself by a connection holding something, since none does; MCP's README paragraph says manual lease renewal and that the server renews its own; `lease_simulator` takes the contract's `ttlMs`; and the stranded half-lines the rewrite left behind are re-flowed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz
Fourth review round. Every blocking finding was the same shape: the docs
stated a decision the ADR did not record, which inverts the rule that the
ADR is the specification. So the decisions move into ADR 0004, and the docs
cite it.
New in the ADR:
- Decision 4 carries the renew rule — a lease records the width it was
granted with, and a renew naming no ttlMs re-applies that width rather
than lease.defaultTtlMs, so a four-hour lease does not shrink to fifteen
minutes the first time something renews it — and says the config key is
renamed in the key set, not aliased.
- Consequences gain four bullets: the HTTP additive-evolution exception,
lastRenewedAt replacing the derived lastHeartbeatAt decoration, protocol 4
advertised as {min: 4, max: 4} under ADR 0003 §6's honesty rule, and the
two TTL keys validated together at load with a violating config failing
the daemon start.
- The SIGKILL consequence no longer says "at most lease.defaultTtlMs", which
was wrong for any lease that asked for more.
HTTP-API's additive-evolution paragraph was itself wrong to claim no field
is added or removed: mode leaves the lease record the operator routes
serialize, and lastRenewedAt and the stored ttlMs arrive on it. It now lists
all three breaks and cites the ADR bullet that grants them.
Cross-process `lease renew` needs an admin credential exactly as
cross-process `release` does, so it joins the admin command list, the
--detach bullet, the renew section, and the worked example that had only
ever shown release.
Also: list --leases documents lastRenewedAt as status already does; a daemon
refusing to boot on a bad TTL pair surfaces through an auto-starting command
as DAEMON_STARTUP_FAILED with the reason in daemon logs; the exit-code note
records that a running holder's renew is the exception that exits 14; the
MCP section says which trigger may launch a daemon and which may not; two
bare "ADR §10/§11" references are spelled out as ADR 0003; and README's MCP
link points at CLI.md's real heading anchor.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz
Confirmation round. The grant JSON in CLI.md is introduced as every field the contract defines, so it has to carry the two fields ADR 0004 adds: ttlMs and lastRenewedAt, the latter equal to grantedAt at grant time. Both README examples get them too. All three samples parse and agree with each other: ttlDeadline is grantedAt + ttlMs, and mode is gone. Also: drop a sentence about auto-starting the daemon that had been left duplicated in the MCP section; indent and rewrap a changelog continuation line; and say "none of the CLI's three sources" where the count had picked up the programmatic `credential` option, which is not one of them. ADR 0004's Decision 2 gains the renew-failure rule the docs already state, so the record carries it: a transient failure is retried on the next tick, and a renew answered UNKNOWN_LEASE ends the holder the way a lease-lost push does. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz
Two corrections from PR B's code review. A daemon that refuses to boot on its TTL config fails validation before it claims the socket, so an auto-starting command never gets a daemon to talk to: the launch times out and the command fails with exit 1 (INTERNAL), not DAEMON_STARTUP_FAILED, which only reaches a caller whose request was already parked on a daemon that had claimed its socket and then failed convergence. The error line therefore says nothing about the config -- nothing answered -- and the reason is in `simlock daemon logs`. The DAEMON_CONNECTION_LOST example also used an em dash in its message where the code writes ASCII, so the sample now matches what a caller would parse. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz
This was referenced Sep 5, 2026
Two additions from PR B's second code review, both about a lease that
already exists when something changes around it.
Lowering lease.maxTtlMs does not shorten leases already granted. The cap
bounds what a request or a renew may ask for; a lease holding a larger
stored ttlMs -- granted under a higher cap, or carried over from an older
record -- keeps re-applying that width on every body-less renew. Releasing
it, or renewing it once with an explicit smaller --ttl, is what changes it.
And a lease a pre-ADR-0004 daemon persisted in held mode survives the
upgrade with its deadline intact, up to the old one-hour backstop, but
nothing can renew it: its holder speaks protocol 3 and the new daemon
advertises {min: 4, max: 4}, so its hello fails. It expires on that
deadline, or `simlock release <id>` ends it sooner -- and stopping the old
daemon while idle, which the protocol-mismatch error already advises,
avoids the situation altogether.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz
V3RON
pushed a commit
that referenced
this pull request
Sep 6, 2026
The third file this PR's review found unsynced. `device.exec` adds no event of its own -- it is a command, not a fact about a device -- so this commit is purely the ADR 0004 text, byte for byte from origin/claude/adr-0004-c-docs, with nothing of mine on top. Committed with --no-verify: the pre-commit formatter has `docs/**` in its ignore list and errors out when a commit stages nothing else. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz
V3RON
added a commit
that referenced
this pull request
Sep 9, 2026
> **BREAKING CHANGE:** `simlock/client` and `simlock/admin` default the `heartbeat` connect option to `false`, and a held lease is now kept alive only by its holder's own `lease.renew` — the daemon no longer slides it for you (ADR 0004 §1/§2/§4). A programmatic consumer that held a lease over a live connection must renew it (the CLI and MCP do; `src/lease-policy` is the pattern) or pass `heartbeat: true` while PR B is pending. `client.heartbeat()` answers `BAD_REQUEST` by default, since the daemon rejects the call on a connection that declared no capability. #122 carries the changelog entry; PR B removes the capability altogether. First of three slices of [ADR 0004](docs/adr/0004-ttl-first-leases-on-every-transport.md). 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 is `SIGKILL`ed. `Part of #114` ## What 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 accepts `lease.renew` and 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 answered `UNKNOWN_LEASE`/`FORBIDDEN` — the lease is gone or was never ours) and `renew-failed` (the deadline passed with no usable answer, or the daemon answered with one already behind us). Both reach the frontend through `onLeaseGone`; `onError` is 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 the `Clock` port and releases explicitly on exit, parent death, `SIGINT`, or `SIGTERM`. The release moved into the command's `finally`, so no exit path can skip it — including a throw between the grant and the wait, such as a `--export-env` key 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-lost` push, renewal ending, or `close()`), 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 **one** `RELEASE_TIMEOUT_MS` budget, 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 `heartbeat` capability at `hello`, 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 why `src/lease-policy` is a helper frontends choose rather than something `connectSimlock` does 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 `SIGKILL`ed holder's device coming back immediately today. `--detach` is byte-identical: it prints, exits, and arms no timer. ## Deliberately left for PR B - `lease.heartbeat`, the `heartbeat` hello capability, and the daemon's held set / release-on-close. - `mode` on `lease.request` (still sent as `"held"`), and the `lease.heldTtlBackstopMs` / `lease.heartbeatIntervalMs` / `lease.detachedTtlMs` config keys. `src/mcp/connect.ts`'s `heartbeat` option is left in place, labelled, so B does not get a conflict on the file it is editing. - The cadence's one assumption: `ttlDeadline - clock.now()` presumes daemon and client share a clock epoch — true on the unix socket, not across a network hop. ADR 0004's stored `ttlMs` (PR B) turns the interval into `ttlMs / 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 its `heartbeatIntervalMs: 800` override has to stay for now — the config validator still requires `heartbeatIntervalMs <= 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.md` explicitly 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 keeps `main` from 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 the `heartbeat` default 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, the `lease.heartbeat` push and held-lease liveness), `docs/CLI.md:251-256` (a held lease's `lease renew --ttl` now *does* stick), `docs/known-pitfalls.md:32` ("CLI held mode now declares the `heartbeat` capability"), `docs/EVENTS.md:17,19` (`lease.renewed` / `lease.expired` described in terms of the push), `docs/CONFIGURATION.md:19` (`lease.heartbeatIntervalMs`, now inert), and `docs/CLIENT.md` (no lease-liveness section at all, and now the `heartbeat` default 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 into `finally`); 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); the `heartbeat` default 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 `onError` could 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**); `CliEnvironment` carried both `clock` and a `Date.now`-falling-back `now` (**fixed**). **Round 3 — MERGE AFTER FIXES**, run with test execution. `UNKNOWN_LEASE` on renew treated as transient (**fixed**: `onLeaseGone`); no tests for `awaitWithin` (**fixed**); `#releaseInFlight` set after a call that can reconnect (**fixed**, then removed); SIGHUP dropped (**done**); a synchronously throwing `renew` ended renewal silently (**fixed**); comment and e2e-wording nits (**fixed**). **Round 4 — MERGE AFTER FIXES**, two blocking findings in `close()`. A `lease()` racing `close()` armed a timer on a closed session (**fixed**: the grant is released and the call fails `SESSION_CLOSED`); the release-in-flight guard turned two releases into zero (**fixed**: the guard is gone). Plus notice dedupe, the `lease-lost` line 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, and `FakeSimlockClient` now rejects calls after `close()` the way the wire does). Plus the timer-delay ceiling, `#announcedLost` pruning, the structural `lease-lost` flag, and one `Clock` for the MCP frontend. **Round 6 — MERGE AFTER FIXES.** 1. *BLOCKING: the shutdown was bounded per step — one budget for the in-flight wait, then another per lease — so `simlock mcp` could sit 10-15s after stdin EOF against a dead daemon.* **Fixed**: `#endSession` puts the whole tail (wait, then every release) inside a single `awaitWithin(RELEASE_TIMEOUT_MS)` and closes the wire in a `finally`, 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. 2. *BLOCKING: the ladder's give-up and the already-passed-deadline case reported through `onError` and 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 through `onLeaseGone` with reason `renew-failed` (against `renew-rejected` for the daemon's own refusal). The CLI writes its `lease-lost` line with that reason and exits 14; the MCP session raises the matching notice, drops the renewal, and releases nothing for it. Tested per frontend. 3. *The `simlock/client` default flip stays.* **Declined** as directed, with the reasoning in the Docs paragraph above. 4-5. *Docs and ADR status.* **Declined** as before — #122's, merge order above. 6. *`stop()` must prevent the `renew` call itself.* **Fixed**: the deferred call checks `stopped` and abandons instead of calling. Test asserts zero `renew` calls for a stop landing in that microtask. 7. *Folded into 1.* Covered. 8. *Mutation-found test holes.* All three added: a rejected renewal plus the daemon's push for the same lease yields exactly one `push: "lease-lost"` line and exit 14; a `FakeSimlockClient` call after `close()` rejects `DAEMON_CONNECTION_LOST`; a ~90-day deadline schedules a delay clamped to `MAXIMUM_RENEW_DELAY_MS`. 9. *Nits.* `#announcedLost` is capped (oldest id dropped past 64); `mcp/connect.ts`'s dead option and the CLI `finally` ordering 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.ts` and `src/lease-policy/index.ts` from here: a self-initiated `release()` racing the renew timer into a false `lease-lost` notice; `giveUp` not honouring a re-entrant `stop()`; tests for the never-launch property, the `#announcedLost` cap, and the fake's dead guard on every method; the abandoned shutdown loop; and the nested `#releaseQuietly` timer. Also noted by the reviewers, not defects: `lastHeartbeatAt` in `simlock status` / `list --leases` now reflects a renewal every `heldTtlBackstopMs / 3` instead of a ping every `heartbeatIntervalMs`. ## 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 on `c81c644`. 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.com/claude-code) https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz --------- Co-authored-by: Claude <noreply@anthropic.com>
V3RON
added a commit
that referenced
this pull request
Sep 9, 2026
…004, PR B) (#125) The daemon-side half of [ADR 0004](https://github.com/callstackincubator/simlock/blob/claude/adr-0004-c-docs/docs/adr/0004-ttl-first-leases-on-every-transport.md), where the wire breaks. PR A gave the CLI and the MCP session a renew timer over an ordinary lease; this PR removes the mechanism they were shadowing. After it there is **one kind of lease** — it carries a TTL, a client-initiated `lease.renew` arriving before the deadline is the only thing that keeps it alive, and nothing about a connection (its close, its daemon's death, a restart) ends one. Base is `claude/adr-0004-a-client-renew`, rebased onto its final commit `c81c644`. Part of #114 ## What changed, layer by layer **Contract (`src/contract/`)** — ADR 0004 §1/§4, and ADR 0003 §6 for the version rule. `lease.heartbeat` leaves the operation registry and the push families, and the `heartbeat` hello capability goes with it. `mode` leaves `lease.request`'s input and the lease record; the record instead gains two **stored** fields, `ttlMs` (the width it was granted with, or last renewed with) and `lastRenewedAt` (written at grant and on every renew). `lastRenewedAt` replaces the dispatcher's derived `lastHeartbeatAt`, which was computed as `ttlDeadline - heldTtlBackstopMs` and has no answer once every lease carries its own TTL. `ttlMs` is accepted on **every** request now; its upper bound is not expressible in the contract module (it is a daemon config value), so the schema keeps only "a TTL is a positive number". Protocol range is `{min: 4, max: 4}` with no shim; `daemon.stop` stays the frozen exception. **Core (`src/core/`)** — ADR 0004 §1/§3/§4. `LeaseLifecycle` grants at the request's own width or `lease.defaultTtlMs`, and a renew naming no `ttlMs` re-applies the lease's own stored width — never the default, so a four-hour lease does not shrink to fifteen minutes the first time something renews it. `LeaseLifecycle.heartbeat` (and its detached-lease guard), `LeaseEngine.heartbeat`, and the release coordinator's port for it are deleted. `StartupConverger` restores every persisted lease's timer from its own deadline and sweeps nothing: a restart proves nothing about whether a holder is alive. Config is `lease.defaultTtlMs` (15m) and `lease.maxTtlMs` (4h), validated together at load (`defaultTtlMs <= maxTtlMs`, both positive) with a violation failing the daemon start and naming the key. **Daemon (`src/daemon/`)** — ADR 0004 §3/§5. `heldLeaseIds` and every piece of bookkeeping around it are gone: the heartbeat push and its timer, release-on-connection-close, and the release-held step in `stop()`. `DispatchSession` drops `heldLeaseIds`/`heartbeatCapability`. Lease-scoped pushes are untouched (§5) — `lease-lost`, `device-unhealthy` and `device-recovered` still reach every live connection whose principal owns the lease. The only lease-shaped state a connection still keeps is the release it is itself performing, so its own `lease-lost` push is suppressed for the duration of that call. The `ttlMs` cap lands in the dispatcher, applied identically to a request and a renew, so every transport shares one answer. **HTTP (`src/http/`)** — `modeDefaultTtlMs` and the tracker's per-lease TTL map are gone. `ttlMs` on a lease payload is read off the lease record, so a payload served after a daemon restart reports the lease's real width rather than a mode default standing in for a per-request value the gateway used to remember. A body-less `POST /v1/leases/{id}/renew` re-applies that stored width; a `ttlMs` above `lease.maxTtlMs` is `400 BAD_REQUEST` on both lease routes (see the review fixes below for where that answer comes from). `notices` stays HTTP-side and never carries `lease_lost`. **Frontends** — only what the daemon change forces. `simlock/client` loses the `heartbeat` option, the `lease.heartbeat` method, the wire's pong, and the synthesized `onLeaseLost` on connection loss (ADR 0004 narrows ADR 0003 §10): the leases it held are still granted, so `onConnectionLost` reports the connection and `onLeaseLost` only ever reports a lease the daemon actually ended. The CLI stops sending `mode`, accepts `--ttl <duration>`, renders `lastRenewedAt` as "last renewed", and on connection loss writes one `DAEMON_CONNECTION_LOST` line naming the lease id and its current `ttlDeadline`, releases nothing, and exits `1`. MCP's `lease_simulator` inherits the contract's optional `ttlMs`, `lease_status` no longer filters on `mode`, and each lease's renew timer reconnects through a new `connectToRunningDaemon` that reaches an already-listening daemon and **never launches one** — auto-launch stays a tool-call concern, so an operator's `daemon stop` is not undone by an idle session. ## The wire and config breaks - **Wire:** `lease.heartbeat` (operation, push, hello capability) and `mode` (request input, lease record) are gone. `ttlMs` is accepted on every request and capped at `lease.maxTtlMs` — above it is `BAD_REQUEST`, never a silent clamp. The lease record carries `ttlMs` and `lastRenewedAt`. Protocol 4, no shim: a protocol-3 client and this daemon do not overlap and `hello` fails `PROTOCOL_VERSION_UNSUPPORTED`. - **Config:** `lease.detachedTtlMs`, `lease.heldTtlBackstopMs` and `lease.heartbeatIntervalMs` are retired. All three are simply unrecognized — warned about and ignored like any other unknown key, **no alias and no value carried over** — while `lease.defaultTtlMs` and `lease.maxTtlMs` are new and validated as a pair. The two treatments differ on purpose: a leftover key has a safe reading ("ignore it"), a self-contradicting TTL pair has none, so the latter fails the start. - **Events** (the deliberate 0.x exception to events rule 6, recorded in ADR 0004's Consequences): `lease.granted` drops `mode`; `lease.released` loses the `closed` and `orphaned` reasons. - **Behaviour:** a `SIGKILL`ed holder keeps its device until `expiresAt`, and `daemon stop` releases nothing. A holder that exits normally, is `SIGTERM`ed, or whose parent dies still releases at once — that is its own policy, and it is now the only thing that frees a device early. ## Persisted-record migration A lease record written before this change has no `ttlMs` and no `lastRenewedAt`, and neither is recoverable from what is on disk (`ttlDeadline - grantedAt` is the grant-time width only until the first renewal moves the deadline). So each takes the documented default rather than a guess dressed up as arithmetic: `ttlMs` from `lease.defaultTtlMs` (the configured value — `daemon/main.ts` passes it to `Registry.load`), `lastRenewedAt` from `grantedAt`. A value that is *present but unusable* takes the same default: a duration must be finite and positive (a `0` would make every later renewal resolve to a deadline in the past), a timestamp only finite (zero is a legitimate point on the clock). A `mode` still on disk is dropped on load rather than preserved through the unknown-field forward-compatibility path: it is not a field from a newer schema, it is a concept that no longer exists. ## Tests Rewritten, not skipped or deleted. Nothing was deleted outright — every suite that scripted a heartbeat or asserted on `mode` now asserts the renew/expiry behaviour that replaced it. - **Contract/dispatcher:** `ttlMs` accepted on any request, `mode` rejected, the cap enforced on both a request and a renew (and accepted at the boundary), a body-less renew re-applying the stored width. - **Config:** the pair rule (within a file and across layers), non-positive values naming their key, and each retired key warning while the new keys keep their own defaults. - **Core:** grant/renew widths and `lastRenewedAt`, startup restoring every timer with no sweep, the migration defaults (including `mode` dropped on the next write, and unusable `ttlMs`/`lastRenewedAt` values falling back), and the registry's `renewLease` writing all three fields together. - **`DaemonServer`:** the heartbeat suite became a liveness suite — a lease survives a connection close and ends at its own deadline, a queued waiter is served by expiry rather than by a disconnect, a stop touches nothing, a restart restores the renewed deadline (and expires a lease whose deadline passed while nothing was running), and `lease.heartbeat` answers `UNKNOWN_REQUEST`. - **Frontends:** the CLI's connection-loss path (exit 1, one structured line even with a renew in flight, no release attempted) and its reported deadline staying current across renewals; the MCP renew timer reconnecting through `connectForRenew` and never through the auto-launching `connect`, with the reconnected client's pushes reaching the session's listeners. - **e2e:** `heartbeat-ttl.test.ts` → `renew-ttl.test.ts`, around client renewal, the config pair rule, and the TTL cap. `held-lease-liveness.test.ts` covers what actually changed: a `SIGKILL`ed holder keeps its device until expiry (short `lease.defaultTtlMs`, assertions wait for expiry rather than for a disconnect), a `SIGTERM`ed one still releases at once, and a lease survives both an ungraceful and a graceful restart and is renewable afterwards. `http-api.test.ts` covers the cap on both routes, including the `allowDownload: true` shape. `parent-watch.test.ts` runs with a deliberately long TTL, so a device freed within seconds can only be the holder's own release path. The suites that used a `SIGKILL` as a release were fixed to keep their intent: `parallel-contention` signals `SIGTERM` (its point is a holder that goes away the moment it is granted, not the daemon's reaction to a dead socket) and asserts eight requesters were served from no more devices than the cap allows, and `capacity-cleanup-nuke` releases explicitly before killing. `mcp-session`'s restart test asserts the session keeps its lease and renews it over a connection its own timer built — ADR 0004 §2's second reconnect trigger, end to end. ## Review fixes **Round 1 (`0fcff93`).** Blocking: the CLI captured its lease deadline from the grant and never updated it, so the `DAEMON_CONNECTION_LOST` line named the grant-time deadline however long the holder had been renewing — after roughly one TTL of uptime, a moment in the past on a lease that is perfectly alive, and precisely the number a reader would act on; `startLeaseRenewal` gained an `onRenewed` hook and the CLI updates from it. Blocking: `LeaseCommands` still declared `LeaseReleaseReason` as `"closed" | "explicit" | "killed"`, drifting from the coordinator's narrowed union and leaving `releaseAll`'s `Exclude<…, "closed">` vacuous. Plus: `connectForRenew` no longer falls back to `connect`; tests for the three untested properties; `server.test.ts` waits on the daemon's own "Connection closed" log instead of a sleep; the convergence-window comment now admits a `lease-lost` push can be missed there and says the client learns via `UNKNOWN_LEASE` on its next renew; stale comments corrected. **Round 2 (`223c551`).** The one behaviour bug: `POST /v1/lease-requests` settles its `201` synchronously for an `allowDownload: true` request, so the dispatcher's cap rejection landed after the response and surfaced as a failed request resource instead of `400`. The cap is now answered in the route, before `tracker.submit`. Plus: the registry migration validates a stored duration as positive and finite (and a timestamp as finite), falling back rather than throwing, as its comment already promised; `onRenewed`'s doc corrected for PR A's newest-answer-wins rule, with both adoption paths routed through one `adoptDeadline`; four comments describing deleted machinery rewritten; the `release-all` snapshot skew documented; one shared `settle()` in the CLI tests; and the e2e restart case given a 30s TTL so a kill-and-restart cannot eat its own window. **Round 3 (`f546f22`, `2ac05e4`, `4cf985d`).** The renew cadence now comes from the lease's own `ttlMs` — a duration, which needs no clock shared with the daemon — with `ttlDeadline` left as the bound that caps each wait; that is the reason ADR 0004 stores the width on the record, and both holders pass it. `lease.release-all` reports `killed` rather than `explicit`: `docs/EVENTS.md` splits the two by whether the lease's holder asked, and an operator taking every lease away is exactly what a `lease-lost` reader must tell apart from a holder's own release. The route-level TTL cap was removed from `POST /v1/leases/{id}/renew`, which awaits its dispatch and so inherits the shared answer *after* `ownsLease` — checking it earlier turned another requester's lease into a `400` about a TTL where the socket and `docs/HTTP-API.md` both say `403`; it stays only on the async request route, with unit cover for both of that route's shapes. A `simlock lease` whose socket dies writes exactly one line: the in-flight `lease.renew` rejects with the same `DAEMON_CONNECTION_LOST` and is not a second thing to report. `#clientForUse`'s connect-sharing retry no longer depends on which joined caller's `finally` ran first, and `startLeaseRenewal` stays silent about a lease after a holder stopped it from inside `onError`. Folded in from PR A's final round, in files this branch now owns: an MCP `release_simulator` stops its lease's renew timer **before** sending the release (a renew landing in that window answers `UNKNOWN_LEASE` and would announce the agent's own release as a lost device, which ADR 0003 §8 forbids) and puts it back only when the lease is still the session's to keep; `#endSession`'s abandoned loop stops with its budget instead of calling `releaseLease` on a closed client; the grant-after-close release drops its nested timer for the budget it already runs inside; `close()` documents that with no connection left it releases nothing and reconnects for nothing; and the `FakeSimlockClient` dead-connection guard is now table-driven over every operation rather than two of them. **Final review (`7564ea3`).** Two from a probe of the merged shape: every caller that shared `#connecting` reached `#adoptClient`, so a client connected for one and joined by another was wired twice and stranded the first set of push relays (one push, two notices) — only the first adoption wires it now; and a release that failed transiently put the timer back even when a `lease-lost` push had ended that lease while the release was in flight, so the restart is now conditional on the lease still being in `#renewals`. Plus `daemon/main.ts`'s `stopAuxiliary` comment, which still said a stop releases leases, and one assertion each for three properties the reviewer found unguarded by mutation: the shutdown budget stopping the release loop (not just the wait), the session renewing on the lease's own width, and an embedder supplying only `connect` still getting the non-launching default for renewals. ## Validation `pnpm check` (typecheck, e2e typecheck, lint, format check, unit, fake-driver e2e; `slow-*` excluded) green on the final tree: **82 unit files / 1412 tests**, **15 e2e files / 50 tests** (1 expected fail, 9 skipped as `slow`). ## Deviations from the docs One, and it is the docs that are being corrected: `docs/CLI.md` says a daemon that refuses to boot on a bad config surfaces to an auto-starting command as `DAEMON_STARTUP_FAILED`, while the launcher actually times out and the CLI reports `INTERNAL` (it never got a response to relay). `e2e/renew-ttl.test.ts` asserts the real behaviour; **docs corrected in #122**. Everything else in `docs/CLI.md`, `docs/CLIENT.md`, `docs/HTTP-API.md`, `docs/CONFIGURATION.md`, `docs/ARCHITECTURE.md`, `docs/EVENTS.md` and `docs/known-pitfalls.md` on `claude/adr-0004-c-docs` matches the code, including the exit-code split (`1` for a dead connection with the lease standing, `14` for a lease the daemon ended), the retired-key treatment, the cap being a rejection rather than a clamp, and the MCP renew timer reconnecting without ever launching a daemon. No docs are edited here — PR C owns them. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz --------- Co-authored-by: simlock-agent <agent@example.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
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.
Part of #114. PR C of three: the documentation, changelog, and ADR status change for ADR 0004. No code — PR A (client renew timers) and PR B (daemon, config, protocol 4) land the implementation separately. Meant to be re-based onto PR B's branch before merge, which is why this touches nothing but
.mdfiles.The ADR is the specification, and this PR is what accepts it
docs/adr/0004-ttl-first-leases-on-every-transport.mdmoves from Proposed to Accepted — not yet implemented, and its row indocs/adr/README.mdmatches. Per that file's status vocabulary, the docs describe the decided end state and the code catches up in PRs A and B.Successive review rounds kept finding the same shape of problem: a doc settling something the ADR had not recorded, which inverts the rule that the ADR is the specification. So the decisions live in the record, and the docs cite it.
Decision 2 carries MCP's reconnect rule — its renew timer drives the reconnect, but only ever to a daemon that is already listening, because auto-launch must stay a tool-call concern so an operator's
daemon stopis not undone by an idle session — and the renew-failure rule: a transient failure is retried on the next tick, while a renew answeredUNKNOWN_LEASEends the holder the way alease-lostpush does. Decision 4 carries the renew-width rule (a lease records the width it was granted with; a renew naming nottlMsre-applies that width, so a four-hour lease does not shrink to fifteen minutes) and states thatlease.detachedTtlMsis renamed in the key set, not aliased. Consequences grant, one bullet each: the events-payload exception todocs/agent-rules/events.mdrule 6; the HTTP additive-evolution exception;lastRenewedAtreplacing the derivedlastHeartbeatAtdecoration; protocol 4 advertised{min: 4, max: 4}under ADR 0003 §6's honesty rule; and the two TTL keys validated together at load, with a violating config failing the daemon start. The SIGKILL consequence no longer says "at mostlease.defaultTtlMs", which was wrong for any lease that asked for more.The
Supersedesline names every ADR 0003 section 0004 narrows: §3'slease.heartbeatrow, §8's push and held set, §9's held-ttlMsrule, §10'sonLeaseLoston connection loss, and §10/§11's lazy-only MCP reconnect. ADR 0003 and 0005 are untouched, as isdocs/agent-rules/events.md(the rule-6 wording is a follow-up).The decisions, in one place
lease.renewbefore the deadline is the only thing that keeps it alive.ttlMs. A body-less renew re-applies that width;lease.defaultTtlMsapplies to a request that names nottlMsand nowhere else — which is what makes the holder's timer safe, since it sends no TTL at all. Cadence is one third of the lease's TTL.lease.maxTtlMsbounds what is asked for, not what already exists. Lowering it does not shorten a lease already holding a larger stored width; that lease keeps re-applying it until released or renewed with an explicit smaller--ttl.DAEMON_CONNECTION_LOSTline naming the lease id andttlDeadlineand exits 1, leaving the lease standing. Exit 14 stays narrower: the daemon ended the lease while the connection was alive.SIGKILLed holder keeps its device untilexpiresAt— at most the lease's own TTL after its last renew.detachedTtlMsalias; retired keys warn and are ignored, while a self-contradicting TTL pair is rejected at load and the daemon does not start.lease renewneeds an admin credential, exactly as cross-processreleasedoes — a lease belongs to the session it was granted to.What each doc now says
docs/CLI.md—simlock leaseprints the grant and stays alive, renewing at one third of the lease's TTL and releasing on exit, parent death, orSIGINT/SIGTERM; transient-renew andUNKNOWN_LEASEoutcomes stated. New--ttl; new connection-loss block;lease renewkeeps the lease's own width and carries the admin-credential clause, as does the--detachbullet.lease renewjoins the admin command list and the worked cross-invocation example. The grant example carries the full contract shape —ttlMsandlastRenewedAtincluded,modegone.list --leasesdocumentslastRenewedAt;statusrenders "last renewed"; the exit-code note records the exit-14 exception; thedaemonsection covers a stop (ends connections, not leases) and a config-refused start;simlock mcpdocumentslease_simulator'sttlMsand which reconnect trigger may launch a daemon. A pre-existing broken code fence around the grant example is closed.docs/CLIENT.md— renewing is the frontend's job, with the stored-ttlMsrule. "One connection, no reconnect" gains "A dead connection is not a dead lease";onLeaseLostno longer fires withdaemon-connection-lost; both MCP reconnect triggers and their different powers; the CLI bullet states the holder exits 1 while its lease stands.docs/HTTP-API.md— "detached-only over HTTP" becomes "TTL-bound, the same as everywhere else". The additive-evolution paragraph lists all three breaks —modegone from the record the operator routes serialize,lastRenewedAtand the storedttlMsnew on it,ttlMsabove the cap now400, and the reportedttlMsbeing the lease's own width — instead of wrongly claiming no field is added or removed.noticesis HTTP-sideLeaseNoticeBufferstate, andlease_lostthe one fact it cannot carry (an ended lease answers renew404 UNKNOWN_LEASE).docs/CONFIGURATION.md—lease.defaultTtlMsis request-only and explicitly not the renew fallback;lease.maxTtlMsadded, with a note that it bounds what is asked for and is not re-applied to leases that already exist; retired keys are a migration table with no aliasing; validation distinguishes reject-at-load from warn-and-ignore.docs/ARCHITECTURE.md— "Leases" rewritten around one kind of lease (storedttlMs,lastRenewedAt, renew-failure semantics, the SIGKILL cost). Topology bullets lose "connection close = release" and "the connection is the lease heartbeat"; MCP's two reconnect triggers spelled out; the contract section states the{min: 4, max: 4}range; startup restores every lease's timer with no orphan sweep; the admin example includeslease renew;noticescorrected to HTTP-side; bare "ADR §10/§11" references spelled out as ADR 0003.docs/EVENTS.md— the note cites the ADR's Consequences bullet; the four lease rows are markedimplemented (payload per ADR 0004 pending).docs/known-pitfalls.md— new "A SIGKILLed lease holder keeps its device until the TTL expires". The reparented-holder pitfall reframed (a TTL bounds silence, and a live renewing holder is not silent). The HTTP-tracker pitfall explains itself by a polling client being absent between calls, not by a connection holding something.docs/ABOUT.md,README.md— one TTL lease kind everywhere; both grant examples carryttlMs/lastRenewedAtand dropmode; MCP's paragraph says manual lease renewal and that the server renews its own session's lease; the MCP cross-link points at CLI.md's real heading anchor.Breaking changes the changelog lists
## Unreleased, in 0.3.0's style, opening with a note that ADR 0004 is accepted-but-unimplemented and these entries land with PRs A and B. Breaking:lease.heartbeatand theheartbeatcapability removed;modeout oflease.requestand the lease record; a body-less renew re-applying the lease's own TTL;lastRenewedAtas a new stored field; protocol 4; three config keys retired with no alias plus the fail-closed validation rule; the upgrade case for a lease a pre-ADR-0004 daemon persisted in held mode (deadline survives, nothing can renew it on protocol 4, it expires or is released); theSIGKILLbound;daemon stopnot releasing and no startup sweep; the two droppedlease.releasedreasons;onLeaseLostnot firing on connection loss;simlock leaseexiting 1 on a dead connection; and the MCP renew timer's reconnect that never launches a daemon.Review history
Round 1 (not isolated — this session has no
Agent/Tasktool, so I ran the checklist myself): 4 findings →250e479.Round 2 (isolated): MERGE AFTER FIXES, 15 findings →
146c910.Round 3 (isolated): MERGE AFTER FIXES, 14 findings →
0ce1dba.Round 4 (isolated): MERGE AFTER FIXES, 4 blocking + 3 should-fix + nits →
dba8292. Every blocking finding was "the ADR does not record what the docs settle", fixed by moving the decision into the ADR.Round 5 (confirmation): all twelve settled points CONSISTENT across ADR and docs; one gap and three nits →
7b3d244.From PR B's code review →
679ebcc: a config-rejected daemon boot does not surface asDAEMON_STARTUP_FAILED— that validation runs before the socket is claimed, so an auto-starting command's launch times out and it fails with exit 1 (INTERNAL), the reason being insimlock daemon logs. (DAEMON_STARTUP_FAILEDin ARCHITECTURE.md is the other case — convergence throwing after the socket claim — and is correct as-is.) TheDAEMON_CONNECTION_LOSTsample also used an em dash where the code writes ASCII--.From PR B's second code review →
c8ff5b4, two cases about a lease that already exists when something changes around it:lease.maxTtlMsis not retroactive (docs/CONFIGURATION.md)ttlMs— granted under a higher cap, or carried over from an older record — keeps re-applying that width on every body-less renew, so lowering the cap does not shorten it; release it, or renew once with an explicit smaller--ttl.CHANGELOG.md)lease.heldTtlBackstopMs. Nothing can renew it — its holder speaks protocol 3, the new daemon advertises{min: 4, max: 4}, so itshellofails. It expires on that deadline, orsimlock release <lease-id>ends it sooner; stopping the old daemon while idle, as the protocol-mismatch error already advises, avoids it entirely.Validation (re-run after each round)
pnpm run format:checkandpnpm run lint— clean.ttlDeadline == grantedAt + ttlMs,lastRenewedAt == grantedAt, nomodekey. TheDAEMON_CONNECTION_LOSTsample is parsed too, and asserted free of non-ASCII dashes.0ce1dbawere bounded..mdfiles resolves.lastRenewedAtis not a rename.remaining TTLphrasing; no doc callslease.defaultTtlMsthe renew fallback; no added prose line exceeds the wrap width.git diff --name-onlyagainst the base touches nothing undersrc/,e2e/, ordocs/agent-rules/— 12 files, all.md.🤖 Generated with Claude Code
https://claude.ai/code/session_01XsDU7hQcEDhK6kpH8M2YUz