fix(session-start): stand down instead of an unfiltered watcher when a resumed seat is unidentified (#982) - #993
Open
fujibee wants to merge 2 commits into
Open
fix(session-start): stand down instead of an unfiltered watcher when a resumed seat is unidentified (#982)#993fujibee wants to merge 2 commits into
fujibee wants to merge 2 commits into
Conversation
…a resumed seat is unidentified (#982) On a resumed session with no role-session record, the fallback emitted the generic watch.sh directive (no 4th <agent> arg). On a project with several registered seats that watcher subscribes to every pair and, as it delivers, stamps read_at and advances each pair's read cursor to the tip -- so it consumes other seats' unread mail, which those seats then never receive. Fail-open is the wrong default here. Fail closed instead: - Narrowing: when no role-session record matches but an actas <team>__<agent>.session lock is owned by this session's bare sid, re-seat to that pair and emit the role-filtered directive. The record the branch wanted and the lock that exists carry the same fact; an ambiguous 2+ match is left unidentified rather than guessed. - When the seat still cannot be established: emit a generic watcher only for a single-pair project (nobody else's mail to take); with more than one pair, emit no watcher and say why, naming `/agmsg actas <name>`. A silent no-watcher is indistinguishable from "no messages arriving", so the stand-down is explicit and points at the recovery. This is a delivery miss, not a loss: messages remain in the store and history.sh returns them. Tests (tests/test_resume_seat_guard.bats) run the directive as emitted and observe which pairs' read cursors move -- a string check on the directive text would stay green if the wrong watcher were emitted or watch.sh ignored its 4th arg. A fail-open regression advances a bystander seat's cursor (red); a broken narrowing fails to deliver the seat's own mail (red).
…d vs inferred from the actas lock (#982) The role-filtered directive is prose the next session reads and acts on: it launches the watcher the text describes. The narrowing path (#982) reused the record path's wording — "this session was recorded as that role's seat" — but it has no record; it inferred the seat from an actas lock this sid still owns. Telling the reader a thing was recorded when it was inferred hands them a claim they cannot check, and this whole fix exists because a directive was trusted and executed as-is. Split the sentence on how the seat was established: - record path: "was recorded as that role's seat" (unchanged). - narrowing path: names the actas lock as the basis and why it stands in for the record, so the reader can weigh "what if that lock is stale?". Tests assert both directions of the distinction, so an inferred seat can never again read as a recorded one.
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.
Fixes #982.
Problem
On a resumed session with no
role-sessionrecord,session-start.shfell back to the genericwatch.shdirective (no 4th<agent>arg). On a project with several registered seats, that watcher subscribes to every pair and, as it delivers, stampsread_atand advances each pair's read cursor to the tip — so it consumes other seats' unread mail, and those seats never get those messages delivered. This actually happened: 37 messages addressed to two other seats were marked read by a mis-seated resume. Fail-open is the wrong default here.Change — fail closed
role-sessionrecord matches but anactas.<team>__<agent>.sessionlock is owned by this session's bare sid, re-seat to that pair and emit the role-filtered directive. The record the branch wanted and the lock that exists carry the same fact. An ambiguous 2+ match is left unidentified rather than guessed./agmsg actas <name>. A silent no-watcher is indistinguishable from "no messages arriving", so the stand-down is explicit and points at the recovery.Severity is accurate: this is a delivery miss, not a loss — messages stay in the store and
history.sh <team> <agent>returns them. No storage-layer change.Tests
tests/test_resume_seat_guard.bats(5 tests, on a two-seat project — the shape in which the defect exists; a single-seat project can't reproduce it). The load-bearing tests run the directive as emitted and observe which pairs' read cursors move — a string check on the directive text would stay green if the wrong watcher were emitted orwatch.shignored its 4th arg.Verified by mutation:
Existing
#339role-directive tests and the full delivery/session-start/role-session suites stay green. Enforced-assertions checker at baseline (638).