Skip to content

DO NOT MERGE — merge train: 2614,2601,2608 - #2630

Closed
vybe wants to merge 24 commits into
devfrom
train/20260908-1521
Closed

vybe wants to merge 24 commits into
devfrom
train/20260908-1521

Conversation

@vybe

@vybe vybe commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Integration surface for #2614, #2601, #2608 (#2607 ejected: its own new test fails the sys.modules lint). Never merged; members merge individually once green.

AndriiPasternak31 and others added 21 commits September 7, 2026 23:03
#2582, Abilityai/trinity-enterprise#548)

Trinity Rule #1 — the docs land before the code.

- requirements/core-agent.md: new §5.30 (the six ACs, the permission matrix,
  the storage decision, the stated limits); §5.20 AC-2/AC-4 amended — the
  Files signal now covers uploads and the refresh triggers widen.
- requirements/content-files.md: §13.10's flat "Content-Disposition:
  attachment" bullet was stale against shipped ent#461; rewritten as the
  server-decided allowlist plus the one-way ?download=1.
- architecture/integrations.md: the ent#461 delivery-policy paragraph records
  the one-way flag, why it is applied in the handlers, why it is parsed
  tolerantly, and that sig is a stored bearer token rather than an HMAC over
  the URL — so appending the flag cannot invalidate it.
- architecture/api-endpoints.md: GET row updated, the missing HEAD row added,
  and the three new client-portal routes catalogued.
- architecture/workspace.md + database.md: the upload announcement, the
  session-type-dependent matrix, and portal_file_dismissals with the three
  reasons its shape is what it is.
- feature-flows: workspace-rail.md gains Slice 3; file-sharing-outbound.md
  corrected on three counts that were stale since ent#461 and #568;
  workspace-agents-at-the-centre.md and the index updated.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq
…, Abilityai/trinity-enterprise#548)

A Workspace viewer needs to remove an agent-shared file from THEIR list without
revoking the share. agent_shared_files has no audience column, so
portal_documents lists every active share of an agent to every rostered client;
and the one generic per-user preference store is FK'd to users.id, which a
portal principal has no row in. So: new storage.

- tables.py / schema.py (DDL + the sweeper's file_id index) / migrations.py
  (SQLite) / migrations/versions/0058 (PostgreSQL) — Invariant #9, both tracks,
  single head confirmed by scripts/ci/check_alembic_heads.py.
- agent_name is on the table FOR the AgentRef registration: agent_shared_files
  is a CASCADE ref, so deleting an agent hard-deletes its shares without going
  through the revoke sweeper — every dismissal keyed on those ids would be
  orphaned forever, and a table with no agent column would sidestep the parity
  guard that exists to catch exactly this.
- Both purge paths in db/agent_shared_files.py delete the matching dismissals
  in the same transaction.
- PK leads with client_email (the read is WHERE client_email = ?, once per
  participant per turn end); the file_id index serves the sweeper.

test_agent_cleanup_parity + test_schema_parity: 8 passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq
…bs (#2582, Abilityai/trinity-enterprise#548)

routers/files.py
- GET and HEAD accept `download`, which may only ever force `attachment`.
  There is no ?disposition= and no way to force `inline` — that direction is
  ent#461's XSS allowlist. `_format_disposition`'s docstring now records the
  asymmetry, so the next reader does not resolve the apparent contradiction in
  the wrong direction.
- Typed Optional[str] with a truthy check, NOT bool: a bool query param 422s on
  ?download= or ?download=x, and this is the public link opened from Telegram /
  WhatsApp / iOS — a malformed query it ignores today must keep being ignored.
- Applied in the handlers, never in _validate_download_request, whose arg list
  is AST-pinned by test_file_download_no_session_gate.
- A ranged PREFIX read no longer bumps download_count. The Workspace preview
  reads from byte 0, so without this every preview would inflate the owner's
  numbers and bury the audit log. The audit row is kept and made separable
  instead: details.ranged_prefix.
- `# mcp: none` header (Invariant #13).

client_portal
- portal_owns_agent + `owned` on the roster row and the card: the SAME
  membership the card renders, so the UI's "Delete for everyone" and the
  service's gate cannot disagree.
- Three routes: read one of your own uploads back (attachment, nosniff,
  no-store), delete one (idempotent), and remove/revoke an agent share. Each is
  _require_roster -> rate_limiter -> service -> audit.
- TWO limiter tiers, env-tunable, per router.py's own stated rule — and the
  burst tier is 20, tighter than upload's, because this is the first time a
  rostered client can reach extract_from_agent (sync docker-py iteration on the
  global 4-worker pool, ~3x file size resident). Delete gets its own looser
  counter; an rm is not a get_archive.
- portal_revoke_shared_file is access-first (roster 404 -> owner 403 -> row
  404), never existence-then-access (Invariant #8), and 404s where its operator
  sibling 204s — enumeration-uniformity on an external surface, stated in the
  docstring so nobody aligns it.
- portal_dismiss_shared_file does NOT validate the file_id (an existence oracle
  over every share in the install) and caps rows instead — the same fork
  set_chat_star already resolved.
- _inbox_path_for: `_safe_filename(name) == name` or a uniform 404. Every shell
  use is shlex.quote plus `--`, because _safe_filename admits a leading hyphen.
- _read_inbox guesses mime_type, and PortalUploadItem declares it — the row's
  FileIcon has been rendering the generic icon since it shipped because the
  response model stripped the undeclared field.
- portal_documents drops dismissed ids and appends &download=1. Only this base
  URL gets the flag; the agent's chat link is untouched.

test_ent79_portal_exposure: the two download_url assertions updated (the AC
changes the URL) and the fixtures gained the table portal_documents now reads.
71 passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq
…te (#2582, Abilityai/trinity-enterprise#548)

stores/clientPortal.js
- uploadDocument queues the agent in a pending-agent SET. It is the ONE funnel
  all three upload surfaces already call, so notifying here makes "Files you
  sent" update before any agent reply WITHOUT touching PortalConversation.vue
  or PortalRoom.vue — the files the delivery sequence is serialized to protect.
- A set, not a scalar, because both real gestures defeat a scalar: a sequential
  multi-file batch (a consumer joining the in-flight read gets a listing taken
  before the later files landed) and a room's fan-out across three DIFFERENT
  agents in one Vue flush window (only the last value survives).
- fetchUploadBlob / deleteUpload / deleteDocument.

stores/portalRailFeeds.js
- noteUpload(agent): shares _fetchToken with refresh() (a refresh issued before
  the upload but resolving after it would otherwise silently clobber the fresh
  listing), coalesces leading AND trailing, and re-checks participation after
  the await. upload() now delegates to it rather than carrying a second read.

portalRail.js / usePortalRailFeeds.js
- filesSignalItems(documents, uploads) projects uploads onto the created_at key
  the Files dot already reads, and BOTH the signal and markSeen read it — one
  mechanism, or opening the tab would mark documents seen and leave the
  uploads' dot lit forever. The collections stay separate; only the signal
  merges them.
- The pending set is drained by the owner: clear-then-read, so a note arriving
  mid-drain is a new entry rather than one this drain already claimed.

portalFiles.js (new, pure — vitest pins environment: node)
- flattenFiles owns BOTH render order and preview index; two orderings drift and
  the lightbox opens the wrong file, silently. `groups` is a parameter so
  ent#484's shared folder becomes a third entry with no structural change.
- previewKind is extension-first for text: Python's mimetypes maps .ts to
  video/mp2t and .toml to nothing, and a shared .md arrives as text/plain.
- neighbour skips non-previewable rows and stops at the ends.
- errorDetail reads a Blob body first: with responseType 'blob' the usual
  err.response.data.detail idiom yields undefined, so the promised "server's
  named reason" would degrade to a generic line for exactly the new verbs.

PortalFilePreview.vue (new)
- Images ONLY through <img :src>, never inline <svg>, never v-html.
- Text capped at 256 KB, fetched whole and sliced: CORS allow_headers omits
  Range, so a ranged preview dies silently cross-origin — and slicing keeps
  preview off the transfer-start counter path. The cap is stated in the UI.
- Escape/arrows registered with { capture: true } + preventDefault, because the
  conversation's turn-cancel listener is on document in the BUBBLE phase.
- v-if not v-show (the column and the mobile sheet are siblings), z-40 below
  ConfirmDialog's z-50, focus on the safe action, a Tab trap.
- Never a blank modal: an unpreviewable type, an over-cap image AND a failed
  byte fetch all land on the same name/size/type + Download card.

PortalRailFiles.vue
- One v-for over the flat list; Download on both lists; the delete matrix
  mirrored off the roster card's `owned`; one ConfirmDialog with copy that
  restates the consequence per case; per-row InlineError with the server's
  reason. The dead single-file upload() (zero callers) is gone.
- AC-3's `download` attribute is deliberately dropped: the control is a button
  driving a blob save and cannot carry it, and it is inert on a cross-origin
  anchor anyway — which is why the server-side flag exists.

npm run test:unit: 101 files, 2238 passed. check:tokens OK. Both new files
scan 0 raw_nongray / 0 hardcoded; loading gates total 70, equal to baseline.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq
#2582, Abilityai/trinity-enterprise#548)

Three new files, registered in tests/registry.json, plus the ent#461 guard
extended so the asymmetry is asserted where a future widening would be edited
past.

test_2582_download_flag.py (35)
- Proves the ASYMMETRY, not the feature: the flag forces attachment and NO
  input forces inline (?download=0 on text/html stays attachment).
- ?download= / ?download=x / a repeated pair never 422 — the regression a bool
  annotation would have shipped on the public link ent#461 exists to keep
  opening from a phone.
- HEAD agrees with GET; the sig survives the extra query pair; every other
  ent#461 header is carried through; the flag rides the 206 branch.
- A ranged prefix read is audited ranged_prefix:true and does NOT bump
  download_count; a full-file range and a plain GET do; a mid-file seek does
  neither. Mutation control: forcing is_ranged_prefix=False fails that test.

test_2582_portal_uploads.py (28)
- The traversal 404 asserted at the handler AND through the mounted route,
  with a positive control — %2F shapes never match the route while %2E%2E
  reaches the handler, and a status-only test cannot tell the two apart.
- Gate order (off-roster before any docker work), ent#308's collision on the
  read side, the translated extract_from_agent exceptions (its 404 echoes the
  container path), the deliberate 409/502 on a stopped/missing agent, rm -f --
  with quoting proven by a name that needs it, and both limiter tiers.

test_ent548_portal_share_delete.py (23)
- The matrix, including the two cases that read as bugs without the rule: a
  non-owner admin is a viewer, and so is an owner on a portal token.
- Access-first revoke proven with a call-recording monkeypatch (the row must
  not be read before the caller is authorized). Mutation control: reordering
  to existence-then-access fails it.
- The dismissal's non-validation and its row cap as a PAIR, both purge paths,
  the AgentRef registration, both migration tracks, the PK's leading column.
  Mutation control: dropping the delete_for_agent purge fails it.

218 passed across the plan's verification set; alembic heads still 1.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq
…iew rules (#2582, Abilityai/trinity-enterprise#548)

portalRailFiles.spec.js (19) — and it found a real defect.

The room fan-out test failed against my own first cut: `noteUpload` shared
`_fetchToken` with `refresh()`, so three concurrent per-agent reads each
invalidated the last and two of three listings were discarded. The two
questions are different — "has the chat moved on?" is global, "is this agent's
listing still the newest?" is per agent — so the store now carries a
`_scopeToken` (participant changes and clear only) and a per-agent
`_uploadEpoch` that `refresh()` snapshots before its awaits. That snapshot is
also what stops a refresh issued before an upload and resolving after it from
silently clobbering the fresh listing.

Three mutation controls, each failing exactly its own test:
- a shared counter instead of the per-agent epoch -> the fan-out test and the
  clobber test fail;
- dropping the trailing re-fire -> the two-file batch test fails;
- refresh ignoring the epoch snapshot -> the clobber test fails.

The spec mounts the composable inside an effectScope stopped after each test:
its watcher on the SHARED portal mock has no component to unmount it, so the
oldest surviving watcher drained the queue against a previous test's Pinia
store. Diagnosed from the symptom (uploads.scout undefined, a fan-out noting
one agent), not guessed.

Also pinned: the dot lights with Files CLOSED after one targeted read; opening
the tab clears it (markSeen must read the same projection or the upload's dot
stays lit forever); a non-participant bump does nothing; a hidden rail drops
the note rather than queueing it; and source guards that PortalConversation.vue
and PortalRoom.vue are untouched and know nothing of the rail.

portalFiles.spec.js (47) — previewKind's extension-first rule with the three
cases that motivate it (.md arriving as text/plain, .ts as video/mp2t, .toml as
nothing), neighbour skipping and stopping, flattenFiles order equalling render
order, the fileActions matrix failing closed, sameOriginPath on a RELATIVE url
(the default install), the Blob error path, and the component's source guards:
an <img> and no v-html, no Range, capture + preventDefault, revokeObjectURL,
z-40 below ConfirmDialog, v-if not v-show, never a blank modal including on a
failed fetch, zero raw palette classes and zero hex.

portalRailFeeds.spec.js: the portal mock is now reactive() with the new fields,
so the owner's watcher is not permanently inert there.

103 files, 2304 passed. check:tokens OK; loading gates 70 (= baseline); both
new files 0 raw_nongray / 0 hardcoded.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq
…e third Escape owner (#2582, Abilityai/trinity-enterprise#548)

Two corrections and one addition from the tail steps.

The docs written before implementation said the drain "shares the feed store's
_fetchToken with refresh()". That is no longer true and was never sufficient:
the room fan-out test proved a shared counter makes three concurrent per-agent
reads invalidate each other. The mechanism is a _scopeToken (chat identity) plus
a per-agent _uploadEpoch that refresh() snapshots before its awaits. Corrected
in requirements/core-agent.md §5.30, architecture/workspace.md,
feature-flows/workspace-rail.md (Slice 3, with the finding recorded as the
reason) and the feature-flows.md changelog row.

/sync-feature-flows also surfaced one flow the docs commit missed:
chat-turn-cancellation.md owns the "what may take Escape" rule, and this PR adds
a THIRD way of owning it — a rail-mounted overlay with no ref the conversation
could name, taking Escape in the capture phase with preventDefault() rather than
joining the per-surface overlay list. The known residual (the voice-call branch
above shouldCancelOnEscape never consults defaultPrevented) is recorded there
too, with a revision-history row, and the index row now points at it.

The Slice 3 Testing block names the effectScope the spec needs and the three
mutation controls.

NOTE: /update-tests also updated .claude/agents/test-runner.md, which lives in
the private .claude submodule (detached HEAD in this worktree) — that edit is
left UNCOMMITTED there rather than creating a dangling commit and dirtying the
gitlink. It needs landing in trinity-dev separately.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq
… blob (#2582)

Self-review finding. The first cut fetched every row as a blob and handed it
back through a synthetic <a download> — which made the one-way ?download=1 flag
decorative on the one surface it was added for, pulled up to 50 MB into the tab
to save a file the browser could stream itself, and used exactly the
programmatic-blob-save path the plan's own risk register flags as the classic
iOS Safari failure, on a surface whose primary form is a phone sheet.

An agent share now saves by an anchor click on its already-attachment URL. A
client upload keeps the blob path, because it has no URL at all — no DB row, no
token — which is the whole reason that path exists.

AC-3's `download` attribute rides on that anchor as belt-and-braces; a browser
ignores it cross-origin, which is precisely why the server-side flag is the
mechanism rather than the attribute.

Pinned by a source guard with a mutation control (collapsing the branch fails
it). 103 files, 2305 passed. Docs corrected in §5.30 and workspace-rail.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq
… turn (#2582, Abilityai/trinity-enterprise#548)

Review finding. The preview modal took Escape in the capture phase with
preventDefault() so it could not reach the conversation's bubble-phase
turn-cancel listener — but the ConfirmDialog it raises has no key handling of
its own, and nothing was added for it. So Escape on an open delete confirm
dismissed nothing and arrived at shouldCancelOnEscape with defaultPrevented
still false: the dialog stayed up and an in-flight turn was destroyed.

Same shape, same reason. And because two capture listeners on `document` fire
in registration order — the tab body mounts before the modal it opens — the
preview now returns early on event.defaultPrevented, so one keystroke closes
one overlay rather than both.

Also documents the three PORTAL_FILE_* limiters in .env.example, beside the
PORTAL_CHAT_*/PORTAL_UPLOAD_* pair they were modelled on. They are the only
bound on the newly-exposed extract_from_agent path, and an operator cannot
tune a knob they cannot discover.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq
…/trinity-enterprise#548)

Review finding. `portalRailFeeds.upload()` awaited `noteUpload()` itself while
`clientPortal.uploadDocument()` — which it calls one line earlier — already
queued the same agent for the rail owner's drain. The drain's note therefore
arrived while the store's own read was in flight, became its trailing re-fire,
and every drop-zone upload cost two container execs: eight for a four-file
batch, on the shared 4-worker executor.

The funnel is the mechanism, so let it be the only one. `upload()` now just
sends. The two specs that pinned "delegates to noteUpload / re-reads its own
agent" pinned the redundancy, so they now pin ZERO reads from `upload()` and
move the "skips a chat that moved on" property onto `noteUpload`, where it
actually lives.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq
…nity-enterprise#548)

The plan listed four things to file and the docs said "filed as a follow-up"
while nothing existed on either tracker. Filed, and named where a reader lands:

  trinity#2598  Escape during a voice call ends the call even when an overlay
                already claimed the keystroke — PortalConversation.vue's voice
                branch sits above shouldCancelOnEscape and reads no
                defaultPrevented, so preventDefault() cannot reach it
  trinity#2599  the Workspace Files routes are uncatalogued in api-endpoints.md
  ent#549       shared files have no audience — every rostered client sees
                every share of an agent, ?sig= included; a dismissal is a
                preference, not authorization
  ent#550       a client upload has no DB row, which is why listing, download,
                delete, preview and MIME are five different mechanisms

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq
An overlay that claims Escape must claim it for the dialogs it raises too —
the 4.14 incomplete-fix class in UI clothes, and destructive-and-silent here
because the wrong Escape kills billed work with no sign. Plus the two rules
that fall out: capture listeners on `document` fire in registration order, not
z-order; and a `preventDefault()` protocol is only as good as the consumer
branch that reads it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxW9tCTd3RnrWrxN8Yk8Bq
…mid-call (#2598)

ent#534 gave the voice call the first branch of `onEscapeKeydown` — correct,
because while a call is up the composer and the picker are inert and there is
no turn to cancel. What was wrong is that the branch re-decided the
preconditions instead of asking for them: it tested `voiceCallActive &&
event.key === 'Escape'` and nothing else.

`defaultPrevented` is the protocol on this surface. #2582's file preview and
delete confirm claim Escape in the capture phase precisely so they cannot
destroy an in-flight turn, and that works only because `shouldCancelOnEscape`
reads it. It was invisible to the branch above it, so one keystroke closed the
overlay AND ended the call. `preventDefault()` cannot help where nothing reads
it.

Fixed as a sibling rule rather than a guard clause. Both branches now start
from one private `ownsEscape(event)` — is it Escape, is an IME composing, has
something nearer already claimed it — and `shouldEndCallOnEscape(event,
{ callActive })` sits beside `shouldCancelOnEscape` in `utils/turnCancel.js`.

The issue offered `if (event.defaultPrevented) return` at the top, which is
smaller and worse twice over: it fixes the reported symptom while leaving the
next Escape owner free to re-decide the preconditions the same way, and it
keeps the decision inside an SFC that `vitest` cannot mount (`environment:
'node'`, no harness) — which is exactly how the original shipped able to ignore
`defaultPrevented` and `isComposing` at once.

That second one is a defect nobody reported and this closes with it: the old
branch ignored `isComposing`, so abandoning an IME candidate with Escape ended
the call.

`portalVoiceMode.spec.js` pinned the literal old condition AND its ordering.
The ordering is the property ent#534 actually cares about and is unchanged, so
that assertion is re-pointed at the new spelling rather than dropped — and
`turnCancel.spec.js` gains a source guard that the branch dispatches THROUGH
the rule, because a predicate the branch bypasses passes its own tests.

Frontend suite: 2254 passed (101 files).

Related to #2598

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ
Self-review finding. `turnCancel.spec.js` reads sources RAW — fine for every
guard that predates this change, and not fine for the two #2598 added:

  * the POSITIVE guard would pass on a comment alone if the call were deleted,
    which is a vacuous guard on the one assertion that proves the branch
    dispatches through the rule rather than around it;
  * the NEGATIVE guard asserts the old condition is gone, over a region whose
    comments now quote that condition verbatim to explain the fix — one
    explanatory sentence away from failing spuriously.

Both now read a `stripComments` view. The comment guard keeps reading RAW on
purpose: the comment IS the thing under test there.

Mutation-checked rather than assumed: restoring the old inline condition turns
the dispatch guard red, and reverting the mutation turns it green again.

Related to #2598
… it is computed (#2582)

`_apply_download_flag` is a pure function, and the existing test pins its
asymmetry exactly. But a pure function cannot tell you the route handed it the
right `inline=`. The plausible regression — a handler deriving the disposition
from the query parameter instead of from `is_inline_safe(row["mime_type"])` —
passes every helper-level assertion in this file and ships stored XSS on a
public token-gated link.

So the route fixture is now parametrized by type, and a `text/html` row is
asserted `attachment` across every flag value, on GET and HEAD both. Verified
by mutation: deriving the disposition from the parameter turns 6 tests red,
5 of them these.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
, Abilityai/trinity-enterprise#548)

The three knobs were read by `client_portal/router.py` and documented in
`.env.example` while being forwarded by NONE of the three compose files, and
neither prod nor hosted uses `env_file:` — so the advertised `.env` lever was
inert on every install. That is worse than an undocumented knob: the operator
sets it, sees no effect, and has nothing to debug. The #1056 / trinity-enterprise#31
packaging-gap class, caught at the `/validate-pr` gate.

Proven by render rather than grep — `docker compose config` showed NONE before
and honours `PORTAL_FILE_HOURLY_LIMIT=7` over the 100 default after, on both
dev and prod.

Wiring prod also broke #2280's wholesale env parity against
`docker-compose.hosted.yml`, which is the guard doing its job: the gap was three
files wide, not two.

Guarded so it cannot recur: the knobs must be forwarded by all three composes,
with defaults agreeing across them AND with the module default. #2433's guard is
scoped to its own two vars by a hardcoded tuple, so extending it would have been
the wrong home. Mutation-verified — dropping one var from prod turns 2 tests red.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e-2582

# Conflicts:
#	docs/memory/feature-flows.md
#	docs/memory/learnings.md
#	docs/memory/requirements/core-agent.md
Multi-agent rooms went dead after ~6h. A room stores a `--resume` handle per
(agent, room) in `enterprise_room_participants.cached_session_id` and passes it
to `execute_task(resume_session_id=...)`, but the reaper's keep set was built
from `agent_sessions` and `enterprise_portal_sessions` only. Every room handle
was therefore an orphan by construction: kept until the 1h age guard, deleted on
the next 6h sweep. The next mention of any agent in the room then failed with
`No conversation found with session ID: <uuid>`.

Reported from production as "the 3-agent room is dead, the 1-agent chat from the
same day is fine" — the single-agent chat was a Workspace thread, and its ids
were in the keep set. Same class as ent#358 one surface over, which that fix's
own comment predicted.

`shared_sessions.db.list_active_claude_session_ids` mirrors `_wake_agent`'s early
returns rather than returning every stored handle, and the two must move
together: `left_at IS NULL` and room `status = 'open'` are both permanent
(`close_room` is a one-way CAS; `add_participant` is a no-op that never clears
`left_at`), so those handles can never resume, and keeping them would trade this
bug for an unbounded leak — every room eventually expires and closes. The
`kind = 'agent'` filter is load-bearing, not decoration: `identity` is
polymorphic (ent#443), so without it a human participant whose username equals
an agent name would inject a handle into that agent's keep set.

The union is read in its own try/except that ABORTS the sweep on failure,
matching the ent#358 half: skipping a cycle costs disk, reaping against a
partial keep set costs users their conversations.

Two shipped occurrences means the mechanism is the defect, so the guard is
anchored on the SCHEMA, not on the reaper: `test_2610_resume_surface_parity`
fails when a table gains a resume-handle column that no keep-set accessor
covers. A test asserting only "the reaper calls three accessors" would pass on
the very next occurrence, because the new surface simply would not be in it.

Verified on both backends: the unit suite on SQLite, and the accessor's exact
SQL against a live PostgreSQL, which returns only the open room's agent handle
(excluding a sibling agent, a `user`-kind row with the same identity, and a
closed room). Each guard mutation-tested — removing the union kills the two
reaper tests and the parity guard; dropping `kind='agent'` or the
`left_at`/`status` filter each kills exactly its own test.

The second half of the report — rooms lack the inline cold retry the other two
resumable surfaces have, so a dead handle is user-visible — is split to #2613:
it needs the working-state lifecycle restructured and `delta`/`top_seq`
recomputed for the cold attempt, which does not belong in a P1 fix.

Related to #2610

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ
`workspace-absorbs-session.md` still described the keep set as "the union of
both tables". #2610 made it three, and the section's own warning — a live JSONL
deleted an hour after it was written, no error anywhere — is exactly what
happened to rooms. Point at the guard so the next resume surface is caught by
CI rather than by a user reporting an agent that forgot.

Related to #2610

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ
@vybe vybe added the ui PR touches the frontend UI — triggers Playwright e2e tests label Sep 8, 2026
sim added 3 commits September 8, 2026 17:02
# Conflicts:
#	docs/memory/feature-flows/chat-turn-cancellation.md
@vybe vybe closed this Sep 8, 2026
@vybe
vybe deleted the train/20260908-1521 branch September 8, 2026 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ui PR touches the frontend UI — triggers Playwright e2e tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants