refactor(rooms): move multi-agent rooms into OSS core (ent#443)

[dolho]

Moves multi-agent rooms from the entitled shared_sessions module into OSS core, in the shape ent#356 used for the Workspace itself. PR 1 of 2 — the companion PR in trinity-enterprise deletes the module there; the submodule bump follows its merge.

Related to ent#443 (private tracker — it carries the edition/monetization rationale, which enterprise-docs-guard keeps out of public docs). Blocks ent#442.

Why

Three of the four surfaces were already public. components/rooms/, stores/rooms.js, the ent#392 composer typeahead and src/mcp-server/src/tools/rooms.ts ship in every build and self-disable; only the backend was gated. So an OSS install rendered the room affordance and then refused it — an advert for a missing feature rather than a clean absence.

The docs had already drifted ahead of the code, publicly:

• docs/user-docs/sharing-and-access/workspace.md:60 presents @-mention → room as the ordinary continuation of a 1:1, with no edition qualifier
• docs/user-docs/faq/sharing-access-and-monetization.md:59 said flatly "Multi-agent chat is an enterprise capability"

Both live on docs.ability.ai. This PR makes the guide true and corrects the FAQ.

Verbatim port

The engine is unchanged: mention-wake turn-taking, per-room budgets, chain depth, the per-participant wake cap, the ent#218 overshoot rule and the ent#220 cancellation shield. A port that also fixes things is a port nobody can review — the five ported suites are the evidence, and they were not edited beyond their import paths.

Adoption, not creation

• DDL → db/schema.py + db/tables.py, versioned on the OSS two-track runner: SQLite shared_sessions_tables_to_oss, Alembic 0042_shared_sessions_oss (single head; check_alembic_heads green).
• Both tracks are CREATE TABLE IF NOT EXISTS, so the migration is a no-op on every entitled install. No data migration, no lost transcripts. downgrade drops nothing — this revision adopted the tables, it did not create them.
• Table names keep the enterprise_ prefix. Renaming them is the data migration this forbids; the prefix is provenance, not a licensing claim (the ent#356 precedent, quoted in the code).
• Enterprise Alembic 0011_shared_sessions stays on its own line — 0012_slack_agent_bindings revises it, and deleting an applied revision is a PostgreSQL boot failure that takes the whole paid tier with it (ent#431).
• shared_sessions/schema.py is now a test/bootstrap applier over the canonical OSS statements (the client_portal/schema.py shape) and owns no DDL of its own.

One deliberate behaviour change — it fixes a latent bug the move surfaced

enterprise_room_participants.identity and enterprise_room_messages.sender_identity are now registered in AGENT_REFS. Before this, rooms sat outside the registry (it covers OSS tables), so:

• an agent rename silently stopped it being woken — its participant row still named the old agent, and that row is what mention resolution matches against;
• an agent purge orphaned participants and transcript.

Both columns are polymorphic — kind / sender_kind decides whether the value is an agent name, a platform user id, or a workspace client's verified email — so each ref carries a kind = 'agent' predicate. Unscoped, a rename would rewrite (and a purge DELETE) a human participant whose id or email happened to equal the agent's name. test_ent443_rooms_oss_core.py proves both directions with colliding human rows.

The forward parity regex structurally cannot see either column (identity is far too generic to add to _AGENT_ID_COLUMNS), so test_agent_cleanup_parity.py gains a documented _POLYMORPHIC_AGENT_COLUMNS set for the backward direction — still verified against the real DDL, so a drop or rename still fails.

This is the same fix ent#356 made for the portal tables, and it is the answer to open question 1 on the issue.

Transition safety

The OSS routers are included in main.py before register_enterprise(app). On an install whose submodule has not yet been bumped, both routers mount and the ungated OSS one wins the match order, so an entitled install keeps serving rooms throughout the window. Pinned behaviourally and source-asserted, so moving the mounts below the seam fails loudly instead of silently handing the paths back to a 403.

Capability bit

multi_agent_chat_available is unconditionally true now but stays on the roster payload. It is the portal's only capability channel (#2128) — a portal principal cannot read /api/settings/feature-flags — and the shipped bundle gates the picker, five room store actions and /workspace/r/:roomId on it. Deleting the field would make all of those read undefined and hide the very feature this move exposes.

Tests

Suite	Result
5 ported suites (test_ent169/220/361/362/387_*)	94 pass, 1 skip
test_ent443_rooms_oss_core.py (new)	16 pass
test_2128_workspace_rooms_capability.py (rewritten)	9 pass
test_agent_cleanup_parity.py	4 pass
-k "schema or alembic or parity" sweep	488 pass

The new suite covers what the ported ones structurally cannot see (they import the module directly and would pass even if it were never mounted): both routers mounted, the first-matching route is the OSS one and carries no entitlement dependency, no feature id registered, DDL on both tracks under the historical names, the bootstrap helper owns no DDL, and the kind-scoped rename contract.

Pre-existing, unrelated: tests/unit/test_ent399_ipv6_origin.py has 2 failures on pristine origin/dev (verified in a throwaway worktree) — environmental IPv6 formatting, untouched by this PR.

Open questions from the issue

1. AGENT_REFS — answered above: it was a live bug, fixed here, kind-scoped.
2. Does any install break by rooms appearing unentitled? No — additive. Nothing is withdrawn, no route changes shape, and an entitled install keeps its existing tables and transcripts untouched.
3. docs/planning/SHARED_SESSIONS_DESIGN.md — still in the private repo; not moved by this PR. Reviewer call whether it follows the code.

Reviewer notes

• Read shared_sessions/__init__.py first — it carries the why, the prefix rationale and the two-router split.
• The diff looks large but is dominated by the verbatim module and the ported tests; the reviewable surface is main.py, client_portal/service.py, the two migration tracks, db/agent_cleanup.py, and the new test file.

🤖 Generated with Claude Code

[dolho]

Rebased onto main (single commit) and force-pushed.

The Alembic chain had to move with the base. main's head is 0038_portal_chat_state; the revision was written against dev's 0041_secret_settings_encryption, which does not exist on main — an unresolvable down_revision is a PostgreSQL boot failure, not a migration warning. Renamed to 0039_shared_sessions_oss with down_revision = "0038_portal_chat_state"; check_alembic_heads reports 40 revisions, 1 head.

The SQLite conflict resolved the same way: this branch no longer carries secret_settings_encryption (dev-only), just shared_sessions_tables_to_oss at the end of MIGRATIONS.

⚠️ Reconciliation will fork the version line. dev has 0039_operator_queue_addressed_to … 0043_subscription_headroom_history, all chained from 0038. This revision is now a second child of 0038, so whenever dev and main meet, that line has two heads — and alembic upgrade head is singular, so it resolves its target first and applies nothing: every revision merged since the fork stops arriving, silently, with no git conflict. It needs an alembic merge revision at the reconciliation point. Flagging rather than guessing which direction that merge should run.

Re-verified on this base (nothing below was inherited from the dev-based run):

• room suites + capability + cleanup parity: 123 passed, 1 skipped
• guard families (schema|alembic|parity|migration|cleanup|rename): 682 passed, 1 skipped
• SQLite entitled-install adoption: seeded 0011-era DDL + live rows → init_database() → migration recorded, data identical, module reads the old room, cached_session_id and last_read_seq intact
• PostgreSQL entitled-install adoption (real PG, scratch DB, dropped after): upgrade_to_head() → alembic_version = 0039_shared_sessions_oss, room + 4 participants + 3 messages intact, agent cursors preserved

One regression this surfaced and fixed: test_1819_rename_cascade_parity failed after the AGENT_REFS addition — its seeder inserts one row per ref and could not satisfy a kind = 'agent' predicate, so both room refs looked stranded. It already had a hard-coded scope special case for the scope-filtered key refs; that is now generalized to parse any ref's extra_filter, so a future filtered ref is covered without touching the harness again.

Known cosmetic during the transition: with a not-yet-bumped submodule both routers mount; route matching is first-wins (OSS serves, proven at runtime under TRINITY_OSS_ONLY=1 where the gated router would 403) but the OpenAPI path map is last-wins, so /docs shows tags: [enterprise:rooms] for /api/rooms until #444 merges.

[vybe]

The port itself reads well — the verbatim-port discipline, the CREATE TABLE IF NOT EXISTS adoption on both tracks, keeping enterprise 0011 on its own line, and the polymorphic AGENT_REFS scoping with colliding-human-row tests are all the right calls, and the FAQ/guide correction is overdue. Sending it back for four things, two of which are landing blockers.

1. Blocker — the prod image does not boot: shared_sessions/ is not in the backend Dockerfile

prod-image-smoke is red, and it is a real failure, not flake:

File "/app/main.py", line 128, in <module>
    from shared_sessions.router import budget_router as room_budget_router
ModuleNotFoundError: No module named 'shared_sessions'

docker/backend/Dockerfile copies backend packages via an explicit per-directory list (routers, services, adapters, utils, db, canary, client_portal, migrations); the COPY src/backend/*.py glob above it catches loose top-level modules only, not packages. shared_sessions/ is a new package and is not on that list — and this PR does not touch docker/ at all.

main.py imports it unconditionally at module scope, so this is a crash loop on boot in production, not a degraded surface.

The precedent is already in the file, four lines above where the fix goes — written for client_portal during ent#356, the very move this PR follows:

# Workspace / client portal (ent#356). A new top-level backend package must be
# listed here or the image builds fine and the app dies at import — `main.py`
# does `from client_portal.router import ...` and the container has no such
# module (#1033's class). `prod-image-smoke` is what catches it; source-only CI
# cannot, because the source tree always has the directory.
COPY ../../src/backend/client_portal /app/client_portal/

One line beside it:

COPY ../../src/backend/shared_sessions /app/shared_sessions/

Worth checking whether anything else needs the same treatment before re-running.

2. Blocker — Alembic branch point: this merges two heads into dev

0042_shared_sessions_oss declares down_revision = "0041_secret_settings_encryption". But dev already carries 0042_agent_ownership_operator_resume off that same parent, with 0043_subscription_headroom_history chained onto it. Both revisions become heads the moment this lands.

I reproduced it by dropping this PR's revision file into a copy of dev's versions/ and running the repo's own guard:

alembic-heads: FAIL — resolves to 2 heads across 45 revision(s); exactly 1 is required.
  • 0042_shared_sessions_oss  (0042_shared_sessions_oss.py)
  • 0043_subscription_headroom_history  (0043_subscription_headroom_history.py)

They fork at: 0041_secret_settings_encryption

upgrade head is singular and resolves its target before applying anything, so the graph applies zero revisions — 0042_agent_ownership_operator_resume and 0043_subscription_headroom_history stop arriving too, not just this one. Git reports no conflict, because each file is individually valid.

The PR body says check_alembic_heads is green, and it is — but only against main, where 0042/0043 do not exist yet. That is finding 3 hiding this one.

Fix: rebase onto dev and renumber to 0044_shared_sessions_oss with down_revision = "0043_subscription_headroom_history". Since nothing has applied 0042_shared_sessions_oss anywhere, a plain renumber is correct here — no merge revision needed.

Related, and much less serious: the SQLite list in db/migrations.py was also cut from main, so its tail is missing agent_ownership_operator_resume and subscription_headroom_history_table. That one will conflict loudly on rebase and is name-tracked rather than order-critical, so it should resolve cleanly — just flagging it so it does not get resolved in the wrong direction.

3. Base is main; it should be dev

Per the SDLC, feature work targets dev and only a release PR targets main. As it stands this PR would land 100 files into main outside the release process — its real change set against dev is 25 files; the other ~75 are just everything already sitting on dev.

The more consequential effect is on CI: schema-parity is passing because the base is main. Retarget and it turns red on finding 2, which is the guard doing its job. Retargeting first also means the re-run after the Dockerfile fix is testing the thing that will actually merge.

4. Title is still wip:

Presumably intentional while the above is open — flagging it so it does not survive to the merge.

Merge order

Your body has it right and I want to confirm it rather than have it re-derived later: this PR → trinity-enterprise#444 → the submodule pointer bump. #444 is green and clean and will sit and wait; nothing there needs changing.

Happy to re-review as soon as 1 and 2 are in — the substance of the port is not what is holding this up.

[dolho]

Re-review — all four findings addressed; no blockers remain

Findings 1 and 4 are fixed as asked. Finding 2 is fixed, but not the way the review prescribed, and the difference matters — details below. Finding 3 is a deliberate decision rather than a mistake.

#	Finding	State
1	shared_sessions missing from the backend Dockerfile	✅ 7c55bd46 — plus a static guard so the class stops recurring
2	Alembic branch point	✅ 34813911 — renumbered to 0044, parent deliberately unchanged
3	Base is main	⚖️ intentional: this is a hotfix to main
4	wip: title	✅ dropped

---

1 — Dockerfile (blocker, fixed)

COPY ../../src/backend/shared_sessions /app/shared_sessions/, one line below the client_portal precedent you quoted. prod-image-smoke is green.

You asked "worth checking whether anything else needs the same treatment before re-running". It does not today, and I made that answer durable rather than a one-time grep — tests/unit/test_ent443_backend_package_copy_parity.py asserts the COPY list has not fallen behind main.py's module-scope imports, and fails in milliseconds naming the exact line to add. It carries an anti-vacuity anchor (both real incidents, client_portal and shared_sessions, as fixtures) because a parser that quietly stops matching makes both assertions pass on nothing. Deliberately not a replacement for prod-image-smoke — that job proves the image boots, which no static check can.

Third occurrence of this shape (#1033 module → ent#356 → ent#443), so it is now in docs/memory/learnings.md.

2 — Alembic (blocker, fixed — but the prescribed fix would have broken main)

The suggested fix was "renumber to 0044 with down_revision = "0043_subscription_headroom_history"". That is right for a dev-based PR and wrong for this one: main's head is 0038_portal_chat_state, and 0039–0043 exist only on dev, so a down_revision naming 0043 is a revision the map cannot resolve — a boot failure on the very line this ships to.

So the fork at 0038 is forced, not accidental, and main resolves to exactly one head as-is. What was avoidable — and is fixed — is the duplicated numeric prefix and the fact that the back-merge obligation lived nowhere:

• Renumbered 0039_shared_sessions_oss → 0044_shared_sessions_oss, parent still 0038, so the filename does not collide with dev's 0039_operator_queue_addressed_to after the back-merge. Ids are strings, but the prefix is the graph's only human ordering cue, and a duplicate would be the first in the repo — on exactly the failure git shows no conflict for.
• The back-merge must add an alembic merge revision with down_revision = ("0043_subscription_headroom_history", "0044_shared_sessions_oss"). It cannot ship here (it would name revisions main does not carry). check_alembic_heads fails that PR loudly until it exists; the fix there is the merge revision, never renumbering a revision already applied wherever the hotfix went. Written into the revision docstring, architecture.md, and a test.
• test_the_room_revision_chains_off_the_hotfix_line_and_numbers_uniquely pins parent = 0038 and prefix uniqueness.

Simulated rather than asserted — dev's versions/ + this file fails the repo's own guard at exactly the fork point, and adding the prescribed merge revision collapses it:

alembic-heads: FAIL — resolves to 2 heads across 45 revision(s)
  They fork at: 0038_portal_chat_state
# + the merge revision:
alembic-heads: 46 revision(s), 1 head (0045_merge_shared_sessions_oss) — PASS

Verified against a real PostgreSQL 16 from a narrow VARCHAR(32) alembic_version (the pg-migrations shape): reaches 0044_shared_sessions_oss, column widened to 255, three room tables created. Then the part that actually exercises an adoption revision — downgrade -1 leaves all three tables in place, and re-upgrade over the pre-existing tables is a clean no-op.

The SQLite list you flagged conflicts loudly on rebase: not applicable while the base is main; it will be a name-tracked keep-both resolution at the back-merge.

3 — Base branch

Confirmed with the author: this is intentionally a hotfix into main, not ordinary feature work that drifted. That resolves the CI concern too, but from the other direction than the review assumed — schema-parity and check_alembic_heads are green because main genuinely is single-head with this revision applied, not because the base is hiding finding 2.

4 — Title

wip: dropped.

---

What I verified independently

Since the port's substance was not what held this up, I checked the claims rather than re-reading the engine:

• Verbatim port — service.py and db.py differ from the enterprise originals by docstrings only; models.py is byte-identical. router.py's delta is exactly the entitlement removal, schema.py is the intended rewrite to a bootstrap applier over db/schema.py (owns no DDL).
• Adoption contract on an entitled PG install — the enterprise 0011_shared_sessions DDL is byte-identical to the OSS 0044 DDL (statement order aside): same columns, same defaults, same UNIQUE (room_id, kind, identity) / UNIQUE (room_id, seq), same four indexes. So the IF NOT EXISTS no-op leaves the correct shape, not merely a shape — which is the thing that would silently break a paying install if it were false.
• DDL agreement across all four sources — db/schema.py, db/tables.py, db/migrations.py, Alembic 0044: identical column sets on all three tables, correct per-dialect types (SERIAL/DOUBLE PRECISION vs AUTOINCREMENT/REAL).
• extra_filter reaches every consumer — the kind-scoping is applied in all three: cascade_delete, cascade_rename, and find_orphan_agent_names. A predicate honoured on two of three would have left the polymorphic hazard open on the third.
• Companion side is consistent — enterprise 0011 kept, 0012_slack_agent_bindings still chains off it, _register_module("shared_sessions") gone, no stale reference to the OSS revision id.
• Neither submodule pointer is committed (both are dirty working-tree only), so the merge order in the body still holds.
• The parity guards were widened, not weakened — _POLYMORPHIC_AGENT_COLUMNS is still validated against the real DDL, and the _seed_row change replaces a hard-coded scope special case with a parsed predicate, which is what stops the harness reporting its own unsatisfied filter as a strand.
• The ent399_ipv6 failures are pre-existing — reproduced identically on a pristine origin/main worktree.
• Rooms suites: 131 passed, 1 skipped. -k "schema or alembic or parity or migration": 562 passed, 1 skipped.

Non-blocking findings

[I1] The docs half of this PR is incomplete — 9 live lines still say rooms are enterprise-gated (confidence 10/10)

The PR's stated motivation is that the guide had drifted publicly. Three lines were corrected; the feature's own page was not:

• docs/user-docs/collaboration/rooms.md:5 — opens > **Enterprise feature.** Shared sessions are available on the enterprise tier…; also :53 and :68
• docs/user-docs/faq/sharing-access-and-monetization.md:67 — "only multi-agent rooms within it are enterprise-gated" — eight lines below the line this PR fixed, in the same file
• docs/user-docs/faq/collaboration.md:51 — "This is an enterprise-tier capability… In a community build the Sessions view is hidden and the feature is unavailable"
• docs/user-docs/README.md:57, docs/user-docs/guides/using-trinity.md:21, docs/user-docs/integrations/mcp-server.md:82 and :140

All live on docs.ability.ai. Point-in-time docs (whats-new/, dev-announcements/) are correctly out of scope. Not a blocker for the code, but it leaves the FAQ contradicting itself in one file.

[I2] The MCP room tools now mis-diagnose a genuine 403 (confidence 8/10)

src/mcp-server/src/tools/rooms.ts:27-30:

function unavailable(e: unknown): boolean {
  const msg = e instanceof Error ? e.message : String(e);
  return msg.includes("404") || msg.includes("403");
}

Unchanged by this PR, but its meaning changed. Before, a 403 genuinely meant mounted-but-unentitled. Now the entitlement gate is gone, so 403 can only be a real refusal — non-member, agent principal on the budget setter, "you cannot reach that agent" — and the agent is told shared_sessions_not_enabled: Shared sessions are not enabled on this Trinity instance, which sends whoever reads it after the wrong thing entirely.

The frontend got exactly this distinction (coded detail = refusal, plain string = absence, per the #2128 note in architecture.md); the MCP surface did not. Narrowing unavailable() to 404-only would be the equivalent fix. Fine as a follow-up.

[I3] The new COPY guard is blind to transitive imports (confidence 7/10)

test_ent443_backend_package_copy_parity.py keys on main.py's module-scope imports, so a package pulled in only by a router or service is invisible — the image would still die at import. Today _top_level_packages() - _EXEMPT and the COPY list are exactly equal (verified: 8 packages, all copied), so tightening the assertion to "every non-exempt top-level package is copied" is free and closes the hole; _EXEMPT already exists for anything that legitimately should not ship. Left alone rather than widened mid-review.

[I4] A rationale comment this PR edited is now false (confidence 8/10)

src/backend/shared_sessions/router.py:111-116, rewritten from "Triple-gated" to "Double-gated", keeps:

require_admin alone would admit an agent to widen the budget that bounds it.

That stopped being true at #1890/ent#297 — require_admin rejects agent principals itself (dependencies.py:882, "ent#293 — an ADMIN gate is never agent-callable"). The code is right and reject_agent_principal is correct belt-and-braces; only the stated reason is stale, in a sentence the PR touched. Worth a one-line fix since this repo treats these rationales as load-bearing.

---

Verdict: approve. Both blockers are closed with evidence, and finding 2's resolution is stronger than the one prescribed — chaining off main's real head, with the back-merge's merge revision written down and tested instead of left to be rediscovered when CI goes red. I1 is the only thing I would genuinely like landed before this ships, and it is docs-only: it leaves the feature's own guide page announcing the opposite of what merged.

Merge order unchanged and confirmed: this PR → trinity-enterprise#444 → submodule bump. The back-merge to dev owes the alembic merge revision.

🤖 Generated with Claude Code

[vybe]

Re-review after the 5 commits pushed since my CHANGES_REQUESTED. Both blockers are closed and verified:

1. Dockerfile — COPY ../../src/backend/shared_sessions /app/shared_sessions/ is present beside the client_portal precedent, and prod-image-smoke is green. The added test_ent443_backend_package_copy_parity.py guard is a better answer than the one-time grep I asked for.

2. Alembic — resolved to 0044_shared_sessions_oss with down_revision = 0038_portal_chat_state. I verified the graph directly: main tails at 0038, so this is single-head on this base and check_alembic_heads is green on the thing that actually merges. pg-migrations green.

4. Title — wip: dropped.

3 — base branch: accepting main, with the cost stated. Confirmed with the author. The consequence is real and I want it on the record rather than rediscovered when CI goes red: dev carries 0039…0043 off the same 0038 parent, so the moment the two branches meet, that version line has two heads and alembic upgrade head — being singular — applies zero revisions. check_alembic_heads is a required context via schema-parity, so the next release PR will block loudly rather than ship silently; it owes an alembic merge revision at that point. Your PR comment already says this; approving on that basis.

All 21 non-skipped checks pass. Security scan clean, no new env var, no host paths or mode flips.

Merge order confirmed and being executed now: this PR → trinity-enterprise#444 → submodule pin bump on main.

Not blocking, but I I1'd it and it is still open: 9 live doc lines still describe rooms as enterprise-gated (docs/user-docs/collaboration/rooms.md:5,53,68, faq/sharing-access-and-monetization.md:67, faq/collaboration.md:51, README.md:57, guides/using-trinity.md:21, integrations/mcp-server.md:82,140) — all on docs.ability.ai, and sync-docs-to-vertex fires on this push. Filing as a follow-up.

[vybe]

Correction to my approval, so nobody chases this: the I1 docs finding is already fixed — I quoted it from the review thread instead of checking the merged tree, and it was closed by the later commits. Verified against origin/main at 366cf505; no follow-up issue filed.

Every surviving enterprise-adjacent line about rooms now reads correctly:

• docs/user-docs/collaboration/rooms.md:5 — "Shared sessions ship in every Trinity build. They used to be an enterprise capability…"
• docs/user-docs/faq/sharing-access-and-monetization.md:59 — no edition claim at all
• docs/user-docs/guides/using-trinity.md:21 — scopes "entitled" to the Enterprise nav entry and states rooms are not a nav entry

faq/collaboration.md, README.md and integrations/mcp-server.md carry no stale claim. Nothing to file.

Merge order completed: this PR (366cf505) → trinity-enterprise#444 (2a5def35) → submodule pin bump #2360.