Take session leases in CLI, ACP, and serve / CLI、ACP、serve 写绑定会话时接入会话租约#6034
Merged
SivanCola merged 3 commits intoJul 5, 2026
Merged
Conversation
…iting Phase 2b of esengine#6027: the desktop has held session leases since v1.16, but CLI --resume/--continue, the chat TUI's session switches, serve, and ACP never acquired one — leaving desktop+CLI on the same session as the last real silent dual-writer entrance (CAS only detected it after the fact by forking recovery copies). - control.SessionLeaseKeeper: single-holder lease follower for the non-desktop frontends. Rebind acquires the new path before releasing the old one, no-ops on the already-held canonical path, releases on empty path, and leaves the previous lease untouched on failure. control.SessionInUseMessage renders the shared holder line (pid, host, since; never the writer id or the session path). - CLI: run/chat resume targets fail fast before setup when held, with the holder line plus the two ways out (close the holder, or rerun with --copy). New --copy flag duplicates the resume target beside the original (event-log-aware Session.Save; parent/preview/turns/model meta; ' (copy)' title suffix) and continues writable on the copy. In-TUI /resume, /switch refuse held targets and keep the current session; /switch re-points the lease if the switch itself fails after the lease moved. /new, /clear, /branch, and rewind-fork follow the controller onto fresh paths. Leases release after the controller closes on exit. - serve: POST /resume returns 409 with the holder line when held (state unchanged); /new, /fork, and model-switch path rotations follow their new paths. Read-only endpoints are unaffected. - ACP: session/new, session/load, and session/resume guard their bind; held sessions surface a protocol error with the holder line. session/close, session/delete, and closeAll release; delete releases before removing files so no lock sidecar is left behind on Windows. - controller: export ResolveBranchRef (wrapper over resolveBranch) so the TUI can learn a /switch target's path before moving its lease. - Crash leftovers need no special casing: the OS lock is the sole arbiter, so a dead holder's lease is simply acquirable. Not wired in this round (documented in esengine#6027): the bot gateway's attached-session path (needs desktop-style lease handover semantics) and destructive-op guards for serve/ACP delete endpoints. Refs esengine#6027. Cache-impact: none - CLI/serve/ACP session binding and help text only; no provider-visible prompts, tool schemas, or request serialization.
github-actions
Bot
added
v2
…utex Review finding on this PR: /resume, /new, and /fork ran their snapshot -> lease rebind -> controller bind sequences without any cross-request serialization. net/http runs handlers concurrently and one serve instance backs multiple browser tabs, so two interleaved requests could leave the controller writing session B while the lease keeper guards session C - B unprotected, C wrongly occupied - exactly the split this PR exists to prevent. switchModel was serialized by its own switchMu but nothing stopped the other handlers from rebinding the same controller mid-switch. Widen switchMu into bindMu and take it in every entry point that changes the active session path (/resume, /new, /fork, switchModel). The lock is acquired after body decoding and path validation, so a slow client cannot hold the binding lock while uploading; lock order stays bindMu -> s.mu with no reverse path. Regressions: a deterministic interleave test parks the first resume between its rebind and its Resume via a test hook (gated by an atomic CAS, not sync.Once - Once.Do's internal mutex would accidentally re-serialize the window) and asserts the second resume waits and both land with controller and lease aligned; removing the guard makes it fail. Plus two hammer tests (concurrent resume x resume and resume x fork) asserting the same alignment invariant under -race. Refs esengine#6034 review.
This was referenced Jul 5, 2026
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
1 participant
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.
Summary
Phase 2b of #6027: put the session lease where the writers are. The desktop has held leases since v1.16, but CLI
--resume/--continue, the chat TUI's in-session switches,serve, and ACP never acquired one — leaving "desktop + CLI on the same session" as the last real silent dual-writer entrance. Until now CAS only caught it after the fact by forking recovery copies; with this PR the second writer is refused up front with a clear way out.Per the approved UX in #6027: conflict → refuse with a holder-identity line + escape hatch; never silent double-writing, never raw lease details.
What's new
control.SessionLeaseKeeper— a single-holder lease follower for the non-desktop frontends (one active session at a time).Rebindacquires the new path before releasing the old one, no-ops on the already-held canonical path (same canonical form as the lease registry — the #6023 lesson), releases on empty path, and on failure leaves the previous lease untouched.control.SessionInUseMessagerenders the shared holder line:pid/host/time are local diagnostics on the operator's own machine; the writer id (random nonce) and the session file path are deliberately never included.
CLI
reasonix [run] --resume/--continueon a held session fails fast before setup: holder line + "close the other Reasonix window or process, or rerun with--copyto continue in a duplicated session" (exit 1).--copyflag: duplicates the resume target beside the original — written throughSession.Saveso the copy is event-log-aware, carries parent/preview/turns/model meta and a " (copy)" title suffix, and starts with no lease/lock sidecars — then continues writable on the copy.--copywithout a resume target exits 2./resumeand/switchrefuse held targets and stay on the current session (holder line + close hint). The outgoing session is snapshotted before the lease moves; if/switchfails after the lease already moved, the lease is re-pointed at the session the controller still owns./new,/clear,/branch,/branch N, and rewind-fork follow the controller onto their fresh paths (fresh paths cannot be held; failure is theoretical but never silent). Leases release after the controller closes on exit — verified by tests that the sidecars are gone and reacquirable.serve —
POST /resumeon a held session returns 409 Conflict with the holder line and leaves serve's state unchanged;/new,/fork, and model-switch path rotations follow their new paths. Read-only endpoints unaffected.ACP —
session/new,session/load,session/resumeguard their bind; held sessions surface a protocol error with the holder line (no copy hatch — that is an editor-side decision).session/close/session/delete/ close-all release; delete releases before removing files so no locked sidecar survives on Windows.Crash resilience needs no special casing: the OS lock is the sole arbiter (kernel releases it with the process), so a dead holder's session is simply acquirable and its stale
lease.jsongets overwritten — same self-healing the desktop relies on.Deliberately not wired (documented for #6027 tracking)
TryAcquireSessionRemovalGuardexists in agent) — protection for deletes, not write-binds; separate concern.--copycovers the need without inventing a new mode.Tests
19 new regressions across four packages: keeper semantics (handover, canonical no-op, held-refusal keeps prior lease, empty-path release, idempotent release, holder-line content and non-leak assertions), CLI (held resume refusal with
--copyhint and no path leak,--copycorrectness — copy equals source transcript, source bytes untouched, copy's lease released on exit — normal resume releases on exit, TUI held-refusal / lease handover / fresh-path follow), serve (409 + unchanged state; successful resume moves the lease), ACP (held load refused and unregistered; close releases).go test ./internal/cli ./internal/acp ./internal/serve ./internal/control ./internal/agent— all pass; new cases pass with-race;gofmt/go vetclean;GOOS=windows GOARCH=amd64 go build ./...clean; desktop module untouched (builds/vets clean).Cache impact
Cache-impact: none. CLI/serve/ACP session binding, help text, and an exported wrapper (
ResolveBranchRef) only; no provider-visible prompts, tool schemas, request serialization, or compaction changes.Refs #6027.