fix(sessions): enforce per-session exclusivity independently of max_concurrent_sessions

[Futahua]

The defect

Two processes can resume the same stored session and both run turns in it. Neither is refused.

The second process loads its own snapshot of the transcript, reasons from a history that does not contain the first one's in-flight turn, and appends anyway — so the stored conversation ends up holding two replies that never saw each other. Reproduced with two python -m tui_gateway.entry processes.

The registry that would prevent this already exists, with a file lock, pid + process-start identity, dead-owner pruning, lease ids and release. It simply was not consulted unless an operator had configured a capacity cap:

max_sessions = resolve_max_concurrent_sessions(config)
if max_sessions is None:
    return ActiveSessionLease(..., enabled=False), None   # registry untouched

So "no cap configured" silently meant "concurrent writers to one session are fine". With the cap unset — the default — nothing is recorded, so nothing can be refused.

The change

The two concerns are uncoupled:

per-session exclusivity	always enforced — correctness
max_concurrent_sessions	optional — capacity policy

Both are decided inside the existing lock, immediately after the existing prune, so a dead owner is never mistaken for a live one and a live one is never overlooked. prompt.submit is unchanged as the enforcement point: the check already happens there, before the busy-queue check, before _ensure_session_db_row and before _start_agent_build, so a refusal persists no user row and starts no model turn.

Re-entrancy, not a hole. A live session whose record is rebuilt in place loses its reference to the lease it already holds, and would then be fenced out of its own session by its own leak — permanently, since pruning only removes entries whose process is dead and that one is alive. Identity is therefore (pid, live_session_id): another process differs by pid, another tab in this process differs by live id, and only the same writer re-acquiring its own session matches.

A session with no stored id yet is exempt; treating "" as an identity would make the first unsaved composer refuse every other one.

The refusal is machine-readable. An automated client has to tell "the machine is at capacity, retry later" from "this session has a live owner and your write would interleave with theirs". Those call for different behaviour, and a client forced to match prose changes behaviour silently whenever the wording improves. prompt.submit now returns data.reason of SESSION_NOT_OWNED or MAX_CONCURRENT_SESSIONS. The refusal is carried by a str subclass, so all existing call sites — which format it, hand it back as a JSON-RPC message, or test it for None — keep working untouched.

gateway.capabilities advertises per_session_exclusive_submit so a client can distinguish a build that enforces this from one that does not, sourced from the module that performs the check rather than from config: a capability an operator can switch on without the mechanism is worse than none, because it is believed.

Tests

tests/test_active_session_exclusivity.py — 11 cases: cap unset still fences one session; different sessions still run concurrently; capacity still applies independently and refuses for its own reason; dead owner pruned and successor acquires; recycled pid does not keep a lease alive; release lets the next owner in; release is idempotent; empty id exempt; same live session may re-acquire.

scripts/probe_active_session_exclusivity.py — the unit tests share one interpreter and so cannot exercise the actual failure. This drives two real gateway processes over stdio and checks the whole sequence, including the parts easy to get wrong:

session.create claims nothing        an idle composer must not hold a session
the lease keys on the STORED id      a lease on the runtime handle would fence
                                     nothing — two processes resuming one
                                     conversation have different runtime ids
B may still RESUME                   reading is never fenced, only writing
B's submit -> SESSION_NOT_OWNED      typed, and the registry is unchanged
A killed, B retries -> accepted      a dead owner is pruned, not permanent

Against the parent commit the probe stops at the second check with an empty registry — the defect stated exactly. No provider or credentials are needed: the fence is checked before the agent is built, so a submit that later fails for want of a model still proves who owns the session.

Compatibility

The registry is now written whenever a turn claims a session, including when no cap is configured; previously it was only written under a configured cap. Leases are correspondingly always releasable rather than sometimes being disabled no-ops.

One existing test in tests/test_tui_gateway_server.py released its session by popping it from _sessions without finalizing, leaking the lease its first turn claimed and then fencing itself out of the same session_key. It now releases the slot the way _finalize_session does.

Verified no regressions against this branch's merge base on the same machine: identical failure sets before and after across tests/test_tui_gateway_server.py, tests/gateway/test_max_concurrent_sessions.py, tests/gateway/test_clarify_active_session_bypass.py and tests/gateway/test_command_bypass_active_session.py.

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

This PR fixes a real correctness bug: without max_concurrent_sessions configured, try_acquire_active_session returned a disabled no-op lease and never touched the registry, allowing two gateway processes to write to the same stored session simultaneously — each reasoning from a stale transcript snapshot. The fix correctly separates per-session exclusivity (a correctness property) from capacity limits (a resource policy), checks exclusivity first under the same lock that prunes dead owners, and introduces ActiveSessionRefusal with a machine-readable reason field so clients can distinguish "capacity full, retry later" from "session owned, do not interleave." The re-entrancy exception via (pid, live_session_id) identity is well-reasoned and prevents a leaked lease from fencing a live session out of itself permanently. The cross-process falsification probe is the right testing approach — the defect is invisible to same-interpreter unit tests.

Two concerns:

1. live_session_id instability could cause self-fencing (hermes_cli/active_sessions.py:139): _is_same_writer requires both pid and live_session_id to match. If a session restore or internal rebuild generates a new live_session_id for the same logical session (e.g., a crash recovery that mints a fresh runtime id), the session would be fenced out of its own lease by its own earlier acquisition, with no way to recover until the process dies and pruning kicks in. Consider whether a fallback path should allow re-acquisition when the pid matches but the live id differs — perhaps with a warning log — or document explicitly that live_session_id must be stable for the lifetime of the process.

2. Cross-process probe is not in CI (scripts/probe_active_session_exclusivity.py): The probe script is the only test that can actually verify the cross-process fence, but it's in scripts/ rather than tests/ and requires a full venv with gateway entry point available. If the exclusivity logic is refactored in the future, the unit tests (which share one interpreter) won't catch a regression in the cross-process behavior. Consider either adding the probe to CI as an integration test (perhaps gated behind a marker like @pytest.mark.integration), or adding a comment at the top of try_acquire_active_session pointing future maintainers to the probe script so it isn't forgotten during refactors.

The fix to existing tests in test_tui_gateway_server.py (releasing active session slots before popping) is a good catch — without it, the new fence would cause cascading test failures from leaked leases.

[andrexibiza]

Review of exact head def357331c888ea483e131f088335b15cc9dee7a against current main (1bbb6e5bce56e721ab685af4cd87df21bbff4d35). The core direction is correct: writer exclusivity is a correctness invariant, not a capacity policy, and the existing cross-process lease registry is the right primitive to reuse. I found two merge-blocking gaps in the current carrier, plus an exact-head verification gap.

Blocker 1 — the fence is not on the common turn-admission path; crash auto-continue bypasses it

This PR puts _ensure_active_session_slot() in the prompt.submit RPC handler, but not in _run_prompt_submit() itself. On this exact head, _maybe_schedule_auto_continue() is reached from cold session.resume, where the live session is deliberately published with no lease, and its kickoff() calls:

_emit("message.start", sid)
_run_prompt_submit(rid, sid, session, text, display_kind="auto_continue")

That path never goes through methods_prompt.prompt.submit, so it never runs the new ownership check. _ensure_active_session_slot( occurs in server.py only as the helper definition; the actual foreground call is in methods_prompt.py.

This is no longer hypothetical repository context. #94778 reports the exact opposite-side failure: two live backends share one HERMES_HOME; backend A is actively running stored session S; backend B resumes S, sees A's fresh interrupted-turn marker, and schedules an auto-continue while A is still alive. Both turns then execute concurrently and complete. #94595 would fence a manual B prompt.submit, as the probe demonstrates, but B's auto-continue still enters _run_prompt_submit() directly and can run the duplicate turn.

Required fix: put the ownership admission at a turn-start chokepoint that every fresh turn source must cross, or explicitly acquire/fail before auto-continue can emit message.start or call _run_prompt_submit. Add a real two-process regression for the #94778 shape: A owns/runs S with a live marker; B resumes S; B must not start a second turn. #94778 still needs its own marker-writer identity fix to eliminate the false “backend stopped” recovery signal, so that issue is complementary rather than superseded by this PR.

Blocker 2 — a capacity registry that fails open cannot become an advertised correctness authority without changing its error semantics

hermes_cli.active_sessions._read_entries() still converts any unreadable/corrupt registry into []. Separately, _claim_active_session_slot() catches any exception from try_acquire_active_session() and returns (None, None). _ensure_active_session_slot() then treats that as success and the turn proceeds with no lease.

That behavior was defensible when this registry only enforced an optional resource cap. It is not compatible with this PR's new contract that per-session exclusivity is “always enforced” correctness, and it makes gateway.capabilities.per_session_exclusive_submit = true stronger than the mechanism actually guarantees. A corrupt registry, lock-acquisition failure, read error, or write/replace failure can silently reopen the exact double-writer state this PR says is impossible.

Required fix: ownership uncertainty needs a distinct fail-closed result (for example SESSION_OWNERSHIP_UNKNOWN / SESSION_COORDINATION_UNAVAILABLE) while capacity policy may retain its existing degradation semantics if desired. Do not collapse “could not prove ownership” into “no owner exists.” There is adjacent work worth reusing rather than duplicating: open #65059 already introduces ActiveSessionRegistryError, strict registry reads/shape validation, and fail-closed cross-process ownership decisions. #65059 and this PR both modify active_sessions.py/TUI ownership semantics, so merge order needs to be explicit; whichever lands second should absorb the other's strict-authority primitive rather than create two incompatible registry contracts.

Interlocks / supersession map

• Concurrent turns on same session act on stale snapshots, causing duplicate execution/side effects #84235 is the broader stale-snapshot / duplicate-side-effect defect class. This PR closes an important TUI/Desktop cross-process admission surface, but not every surface.
• fix(api_server): serialize concurrent agent turns per session #84876 is complementary, not duplicate: it serializes same-session turns inside APIServerAdapter only and explicitly does not provide a cross-process queue.
• fix(protocol): let clients recover turn outcomes after reconnects #91475 is also complementary: durable turn receipts/idempotent reconnect recovery address ambiguous retry after transport loss; they do not establish one live writer for a stored session.
• RFC: Per-session storage with central pointer index (structural fix for shared-writer corruption class) #93244's invariant 3 (“one authoritative writer per session”) is exactly the architectural rule this PR is implementing at the gateway boundary.
• fix(status): show live sessions when no concurrency cap is set #80034 overlaps only the “uncapped registry is still written” mechanism. fix(sessions): enforce per-session exclusivity independently of max_concurrent_sessions #94595 supersedes that narrow early-return removal for correctness purposes; fix(status): show live sessions when no concurrency cap is set #80034's repo-attribution/status work remains separate. Do not merge both implementations of the same uncapped registry behavior blindly.
• The cross-process lease registry itself predates this PR (feat(sessions): add optional max session cap #42355/feat(sessions): add optional max session cap #42389 and subsequent lease lifecycle fixes); credit for that foundation should remain distinct from this PR's correctness-policy change.

Exact-head readiness

At review time main is 1bbb6e5bce56e721ab685af4cd87df21bbff4d35. GitHub compare reports this head as diverged, 2 commits ahead and 61 behind current main. The workflows attached to exact head def3573 are all action_required with zero jobs, so there is no exact-head CI/Docker/Nix execution receipt to treat as green. The local unit/probe evidence is useful, but the cross-process probe is also not currently part of CI.

After the two correctness gaps above are fixed: rebase/materialize onto current main, run the cross-process auto-continue + manual-submit cases, and obtain fresh exact-head CI. Until then I would not merge this carrier, because its advertised invariant is stronger than the paths it actually fences.

[teknium1]

Maintainer-side review (triage sweep). Verdict: right direction, not salvageable as-is — keeping open for revision rather than closing.

Premise confirmed on current main: hermes_cli/active_sessions.py still returns a disabled no-op lease when max_concurrent_sessions is unset (try_acquire_active_session → enabled=False, registry never consulted), so two processes can indeed both run turns in one stored session by default. Separating per-session exclusivity (correctness) from the capacity cap (policy) is the right architectural call, and reusing the existing lease registry is the right primitive. The (pid, live_session_id) re-entrancy identity and the machine-readable ActiveSessionRefusal.reason are both well-designed.

What blocks a straight salvage today (largely echoing @andrexibiza's review, which we verified):

1. The fence isn't at a turn-admission chokepoint. It lives in the prompt.submit RPC handler, but _maybe_schedule_auto_continue() (cold session.resume) calls _run_prompt_submit() directly — the exact double-writer shape reported in Auto-continue false positive: interrupted-turn marker shared across backends has no writer identity (duplicate turns, misleading "backend stopped" notice) #94778 sails past the check. The admission needs to sit where every fresh turn source must cross, with a two-process regression for the auto-continue path.
2. Fail-open registry semantics contradict the advertised invariant. _read_entries() turns corrupt/unreadable registries into [] and _claim_active_session_slot() swallows exceptions into "proceed with no lease" — defensible for an optional capacity cap, not for an always-enforced correctness guarantee advertised via gateway.capabilities.per_session_exclusive_submit. Ownership uncertainty needs a distinct fail-closed result; fix(desktop): coordinate orphan reaping across profile backends #65059's ActiveSessionRegistryError/strict-read work is the primitive to reuse, and merge order with that PR needs to be explicit.
3. Branch is now well behind main (diverged, ~61+ commits at last check) with no exact-head CI receipt, so a rebase is needed regardless.

If you rebase onto current main and address 1–2 (or narrow the claim: enforce at a shared chokepoint, fail closed on registry uncertainty), this looks salvageable — the core change and the test/probe approach are solid. Happy to re-review a revised head.

[teknium1]

Salvaged via PR #99719. Both of your commits were cherry-picked onto current main with your authorship preserved in git log, and the two review blockers were implemented on top in a follow-up commit:

1. the ownership admission now sits at the _run_prompt_submit chokepoint (and crash auto-continue acquires before message.start), closing the Auto-continue false positive: interrupted-turn marker shared across backends has no writer identity (duplicate turns, misleading "backend stopped" notice) #94778 bypass
2. registry uncertainty now fails closed with a typed SESSION_COORDINATION_UNAVAILABLE refusal instead of degrading to an untracked go-ahead

Your cross-process probe was re-run live on both sides: on main, B's submit was accepted (double-writer confirmed); on the salvage head, all 9 checks pass with B refused SESSION_NOT_OWNED. Thanks for a well-designed fix — the correctness/policy separation, the typed refusal, and the falsification probe all survived intact.