Observer verification: fix coverage bookkeeping, restart sessions when scope widens

docs/observers.md

@@ -91,6 +91,8 @@ This feature replaces that all-or-nothing posture with a per-user, gatekeeper-me
 |---|---|
 | Gatekeeper RPC API (the committed surface) | `packages/workshop-shared/src/gatekeeper.ts` |
 | Overseer DO, `open()` auth entry point | `packages/workshop-backend/src/overseer.ts:2714` |
+| Authorization gate shared by `open()` and `receiveExternalMessage()` | `overseer.ts` `authorizeCollaborator()` |
+| Session restart when verification scope widens | `overseer.ts` (`#restartIfShared`, `scheduleAccessRestart`) |
 | Server `openGadget` path | `packages/workshop-backend/src/server.ts:206` |
 | Role resolution / permission graph | `packages/workshop-backend/src/sharing.ts` (`getEffectiveRole`, `computeEffectiveRoles`, `hasAnyShares`) |
 | `prohibitAllSharing` enforcement | `overseer.ts:1171` (`authorizeObservation`), `:1207` (web fetch), `:1258` (`submitAction`) |
@@ -254,7 +256,11 @@ Logic:
    - A `creationSpec` with a `vendorId` requires an account; other specs need no verifier or account
      choice.
 
-2. **Load the observer record** for `profileId` (may be absent).
+2. **Load the observer record** for `profileId` (may be absent), and **prune** any
+   `accountChoices` entry that is no longer in scope. The record must state only what this open
+   verifies: `use` scope is live binding state, so an entry left from before a connection was
+   unbound would otherwise silently re-register the collaborator off a choice made for a scope the
+   workspace no longer has, instead of asking them again after a rebind.
 
 3. **Determine uncovered bindings**: in-scope account-requiring gatekeepers with no
    `accountChoices` entry in the record. Before prompting, automatically fill ambient bindings from
@@ -276,9 +282,20 @@ Logic:
      a verifier minted by its own vendor; filtering account choices in the client is only a
      user-interface convenience.
    - If any `addObserver` **throws** (or `getVerifier` throws on vendor mismatch), the user is not
-     (or no longer) allowed: best-effort `removeObserver(record.observerId)` on the gatekeepers
-     added in *this* pass, do **not** persist the working record, and deny the open with a clear
-     message.
+     (or no longer) allowed. Every such failure goes through one `fail()` path that synchronously
+     scrubs the failed gatekeeper from the *persisted* record, so the record stops claiming a
+     verification that no longer holds, and the user is offered a bounded number of re-prompts to
+     repair (e.g. re-authenticate an expired account). On terminal failure the open is denied with
+     a message naming each refused binding, and the registrations this call added are
+     best-effort-removed while no record is persisted.
+   - Roll back only what *this call* added. A returning observer's registrations are kept: their
+     `observerId` is already persisted, a registration can only ever add exclusion names (so
+     keeping it is fail-closed), and the next successful open's `addObserver` overwrites its
+     verifier. Only a *first-ever* verification rolls back fully, since that collaborator was never
+     admitted and the minted id would otherwise linger unresolvable.
+   - A terminal failure that scrubbed a *previously-persisted* choice also restarts the workspace
+     (see "Restarting when verification scope widens" below), because the collaborator may hold
+     other sessions that opened while that choice still verified them.
 
 6. **Persist the observer record** (with merged `accountChoices` and `observerId`) only after all
    `addObserver` calls succeed. Storing/creating the record is the canonical moment the user
@@ -300,6 +317,56 @@ Notes:
   stored account choices. The modal is only for genuinely uncovered bindings (first open, a binding
   the owner added after this user last configured, or an ambient binding without a matching provided
   account).
+- **Role resolution and verification belong together.** Both live behind one
+  `authorizeCollaborator(profileId, clientUser, {configureCb?, requireRole?})`, so every non-owner
+  entry point applies the same gate. `receiveExternalMessage()` — the chat-integration path, whose
+  agent reply can surface anything the workspace has already read — passes `requireRole: "build"`
+  and no `configureCb`: it has no channel to prompt on, so an unverified caller is told to open the
+  workspace in a browser, and an insufficient role is denied *before* verification runs rather than
+  being sent to fix a failure that could never grant them access anyway.
+
+#### Restarting when verification scope widens
+
+Verification runs at `open()` and nowhere else, so a live session is only ever as verified as the
+scope that existed when it opened. When that scope **widens**, the overseer restarts the workspace
+rather than trying to re-verify sessions in place: `#restartIfShared(reason)` delegates to
+`scheduleAccessRestart(reason)` — the same DO abort used to revoke a collaborator (see
+`docs/sharing.md`) — so every client's browser reconnects and re-runs
+`authorizeCollaborator`/`ensureObserver` against the new scope. It is a no-op when the workspace
+has no collaborators: the owner is never an observer, so there is nobody to re-verify.
+
+Four events trigger it:
+
+| Event | What grows |
+|---|---|
+| `addGatekeeper()` with a vendor-backed `creationSpec` | **build** scope — a live `build` session can `getGatekeeperById()`/`openSession()` on it with no observer check |
+| `bindWorkpiece()` for a permanent (non-`chatId`) edge onto a vendor-backed connection | **use** scope — the gadget UI a `use` session drives can now invoke it |
+| A merge that promotes a pending gadget or a pending binding edge into `use` scope | **use** scope, same reason |
+| A terminal `ensureObserver()` failure that scrubbed a previously-persisted account choice | Coverage *shrank*: the collaborator's other sessions still hold access the scrubbed choice used to justify. Scheduled when the failure becomes terminal, which a re-prompt the failing client never answers can defer (edge case 3) |
+
+The merge trigger compares the effective `use` scope before and after promotion rather than firing
+on any promotion: most merges promote something, and a promoted gadget with no bindings — or an edge
+onto a vendorless connection nobody is verified against — widens nothing and must not sever a
+shared workspace for nothing.
+
+Shrinking scope needs no restart (`unbindWorkpiece`, `removeGatekeeper`): the prune in step 2
+handles it at the next open, and a narrower scope can never under-verify. Role *rises*
+(`addCollaborator`, share-key redemption) are deliberately not triggers either — a live session's
+capability set is fixed at open, so raising someone's graph role does not widen the session they
+already hold.
+
+The restart is what makes `addGatekeeper()`'s publication order load-bearing. The DO's input gate
+is open across the gatekeeper's `describe()` and ids are allocated sequentially, so publishing the
+record before that await would let a live `build` session guess the id and `openSession()` on the
+owner's brand-new connection — which gates on nothing but record existence — for as long as
+`describe()` took, all of it before the restart severed it. The record is therefore published
+exactly once, after `describe()` resolves; `getGatekeeperFacet(id, cls?)` takes the class directly
+so nothing needs the early put.
+
+Enforcement is therefore at admission, within the ~100 ms abort delay of the moment the change is
+determined. For the three widening triggers that moment is the change itself; for the scrub trigger
+it is the point at which the failure becomes terminal, which the failing collaborator can defer by
+leaving a re-prompt unanswered — worth no more to them than never re-opening at all (edge case 3).
 
 ### Step 4 — Frontend: the configuration modal
 
@@ -365,8 +432,10 @@ downgrades — see the matching methods on `OverseerClientInterface` and `Sharin
   Safe to defer — an over-broad observer set only ever errs toward stricter future checks — but
   it keeps gatekeeper state tidy.
 - All these calls are best-effort: log and continue on error. An orphaned observer entry only
-  causes superfluous future checks, never a data leak (the leak-relevant gate is
-  `authorizeObservation`, which keys off the live sharing graph).
+  causes superfluous future checks, never a data leak: a registration is what *admits* an open, and
+  every open re-runs `addObserver`, so a stale one grants nothing on its own — while
+  `authorizeObservation`'s exclusion gate re-checks the live sharing graph for any id a gatekeeper
+  still names.
 
 > Multi-gatekeeper sequencing/atomicity is an overseer implementation detail, not part of the
 > shared interface. Because `addObserver` is re-run every open and `removeObserver` is idempotent,
@@ -401,12 +470,30 @@ already in the JSDoc in `gatekeeper.ts`; add anything missing there rather than
    throws and denies the open.
 3. **Underlying resource access revoked** — caught at the next open because `addObserver`
    re-runs the live check and throws; the open is denied. Consistent with the lazy-revocation
-   model in `sharing.ts`.
+   model in `sharing.ts`. The denial also scrubs each failed gatekeeper from the collaborator's
+   persisted observer record, so the record stops claiming a verification that no longer holds; and
+   because the collaborator may hold *other* sessions that opened while it did, the scrub restarts
+   the workspace when the denial is determined (see "Restarting when verification scope widens"),
+   forcing every session on it to re-open and re-verify. "When determined" is later than the scrub
+   itself: the failing client is offered a re-prompt first, and one it never answers defers the
+   restart for as long as it stays unanswered — the same residual as never re-opening. The residual
+   under the lazy model is otherwise unchanged: a collaborator who never opens again is never
+   asked, so nothing detects their revocation and nothing severs the session they already hold. An
+   operational failure (vendor outage, expired credential) is treated the same way — the overseer
+   cannot tell it from a settled denial — and the collaborator gets back in as soon as a repaired
+   open re-verifies them.
 4. **`prohibitAllSharing` interaction** — unchanged and still authoritative: if set, no non-owner
    can open at all (`overseer.ts:2770`). Observer checks only matter when sharing is allowed.
 5. **Owner adds a new binding after sharing** — existing observers see an incremental modal for
    just the new binding on their next open, and may be denied if they lack access to the new
-   resource (inherent to the security model).
+   resource (inherent to the security model). Because that next open is what verifies them, the
+   addition restarts a shared workspace (see "Restarting when verification scope widens"): every
+   client reconnects within ~100 ms and re-opens at the new scope, so no session keeps watching a
+   connection its holder was never verified against. A connection added *while a collaborator's
+   verification is parked* on an await (the modal, verifier RPCs) is covered by the same restart:
+   their committed record lacks an entry for the new connection, and the restart forces the open
+   that adds one. The residual is the ~100 ms window itself, which is inside the revocation window
+   the sharing model already accepts.
 6. **Performance** — `ensureObserver` does one `getVerifier` + one `addObserver` per in-scope
    gatekeeper per open. Parallelize with `Promise.all` and pipe the verifier promise straight into
    `addObserver`. Expensive gatekeepers cache on their side.

docs/sharing.md

@@ -150,13 +150,15 @@ Authorization is enforced at `open()`: the method computes the caller's effectiv
 
 Because the role is recomputed from the graph on every `open()`, the live computation is the *sole* source of truth for access -- there is no eager cleanup whose bugs could grant access to an unreachable user. This is what makes lazy revocation safe: severing an edge is enough to deny access, even though the unreachable records linger in storage.
 
-### Terminating live sessions on revocation
+### Terminating live sessions on revocation or scope growth
 
 Authorization is only checked at `open()`, so a session that is *already* open is not re-checked per message. Without intervention, a collaborator who was just removed or downgraded could keep using their live session until something else disconnected them. To close this gap, `removeCollaborator`/`revokeShareLink` proactively restart the gadget's Overseer DO via `ctx.abort()` whenever the change actually removed or downgraded someone (i.e. the returned `AffectedCollaborator[]` is non-empty; pure no-op removals don't restart). Aborting forcibly disconnects every client; each reconnects and re-runs `open()`, which re-evaluates the now-changed permission graph -- sending removed users to the terminal access-denied page and handing downgraded users their reduced capability (the editor swaps to the `use` view automatically based on `metadata.role`). Since removals are rare (and DOs restart unpredictably anyway, so reconnects are already cheap), the disruption is acceptable.
 
-Two precautions surround the abort (`OverseerImpl.scheduleRevocationRestart`): the severed edge is flushed with `ctx.storage.sync()` first (because `ctx.abort()` does not respect the output gate, a restart could otherwise come back with the change lost), and the abort is delayed ~100ms so the triggering RPC's response reaches the caller -- typically the owner, who is also connected -- before their own connection drops. The disconnect reaches the browser through the existing `notifyClosed` plumbing: when the Overseer DO aborts, the per-session `notifyClosed` stub is disposed without being called, which `AuthenticatedApiImpl` treats as a lost connection and reacts to by killing the browser WebSocket, forcing a reconnect.
+Two precautions surround the abort (`OverseerImpl.scheduleAccessRestart`): the severed edge is flushed with `ctx.storage.sync()` first (because `ctx.abort()` does not respect the output gate, a restart could otherwise come back with the change lost), and the abort is delayed ~100ms so the triggering RPC's response reaches the caller -- typically the owner, who is also connected -- before their own connection drops. The disconnect reaches the browser through the existing `notifyClosed` plumbing: when the Overseer DO aborts, the per-session `notifyClosed` stub is disposed without being called, which `AuthenticatedApiImpl` treats as a lost connection and reacts to by killing the browser WebSocket, forcing a reconnect.
 
-Note this is only needed for removals/downgrades. Granting or raising access never strands anyone, and `prohibitAllSharing` cannot strand a session either: an observation that would set that flag is *blocked* (rather than applied) if the gadget is already shared, so the flag only ever flips to true on a gadget with no other sessions to evict.
+Granting or raising access never strands anyone: a live session's capability is fixed at open, so a `use` collaborator promoted to `build` in the graph still holds `UseOverseerInterface` until they re-open, and nobody is newly excluded from anything. `prohibitAllSharing` cannot strand a session either: an observation that would set that flag is *blocked* (rather than applied) if the gadget is already shared, so the flag only ever flips to true on a gadget with no other sessions to evict.
+
+The same abort serves a second purpose, though, and there the trigger is a *grant*: observer verification (see docs/observers.md) also runs only at `open()`, so widening the set of gatekeepers a collaborator must be verified against leaves their live session holding access they were never verified for. `OverseerImpl.#restartIfShared` restarts the workspace whenever that happens -- a connection is added, one is bound into a gadget, a merge promotes such a binding, or a re-verification failure scrubs a previously-persisted account choice -- so every client re-opens and re-runs `ensureObserver` at the new scope. It is a no-op when the workspace has no collaborators, so a solo workspace is never disturbed. See docs/observers.md, "Restarting when verification scope widens", for the full trigger list and the reasoning about what deliberately does *not* trigger it.
 
 ## Future work