chore: reconcile main into dev — rooms OSS core + the Alembic merge revision (ent#443) - #2364
Conversation
Swap the hero video slot to the v0.9.0 release review (live workshop walkthrough), link it plus the docs.ability.ai what's-new overview from the release callout, and add the review to the Watch more list. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Bumps [brace-expansion](https://github.com/juliangruber/brace-expansion) from 2.1.0 to 2.1.4. - [Release notes](https://github.com/juliangruber/brace-expansion/releases) - [Commits](juliangruber/brace-expansion@v2.1.0...v2.1.4) --- updated-dependencies: - dependency-name: brace-expansion dependency-version: 2.1.4 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* refactor(rooms): move multi-agent rooms into OSS core (ent#443) Rooms were the entitled `shared_sessions` module, 404ing in community builds — while the frontend that drives them (`components/rooms/`, `stores/rooms.js`, the ent#392 composer typeahead) and the MCP tools (`tools/rooms.ts`) shipped in EVERY build and self-disabled. Three of four surfaces were already public, so gating only the backend left an OSS install rendering an affordance it then refused. Workspace itself moved for the same adoption reason (ent#356); rooms are the half that makes it the place people work with agents rather than a second 1:1 chat. The public docs had already drifted ahead of the code: the Workspace guide presents `@`-mention → room as the ordinary continuation of a 1:1, while the FAQ called multi-agent chat "an enterprise capability". Both are live on docs.ability.ai; this makes the guide true and corrects the FAQ. Engine ported VERBATIM — mention-wake turn-taking, per-room budgets, chain depth, wake caps, the ent#218 overshoot rule and the ent#220 cancellation shield are unchanged. A port that also fixes things is a port nobody can review. Adoption, not creation: * DDL to `db/schema.py` + `db/tables.py`, versioned on the OSS two-track runner (`shared_sessions_tables_to_oss` + Alembic `0042_shared_sessions_oss`). 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. * Table names keep the `enterprise_` prefix. Renaming them IS the data migration this forbids; the prefix is provenance, not a licensing claim (ent#356). * The enterprise Alembic `0011_shared_sessions` stays on its own line — `0012` revises it, and deleting an applied revision is a PG boot failure (ent#431). One deliberate behaviour change, and it fixes a latent bug the move surfaced: `participants.identity` / `messages.sender_identity` are now in AGENT_REFS, so a rename re-keys and a purge cascades. Before this, rooms sat outside the registry (it covers OSS tables), so a renamed agent silently stopped being woken — its participant row still named the old agent, which is what mention resolution matches against — and a purge orphaned participants and transcript. Both columns are POLYMORPHIC, so each ref carries a `kind` predicate: unscoped, a rename would rewrite (and a purge DELETE) a human participant whose user id or verified email happened to equal the agent's name. The forward parity regex cannot see either column, so `test_agent_cleanup_parity` gains a documented `_POLYMORPHIC_AGENT_COLUMNS` set for the backward direction and `test_ent443_rooms_oss_core` pins the predicates explicitly. Transition safety: the OSS routers are included BEFORE `register_enterprise(app)`, so on an install whose submodule is not yet bumped both mount and the ungated OSS one wins the match order. Pinned by test, source-asserted so a reorder fails loudly. `multi_agent_chat_available` is now unconditionally true but STAYS on the roster: 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, so deleting it would hide the feature this move exposes. Tests: five suites ported verbatim (94 pass), `test_2128` rewritten around the new unconditional answer, new `test_ent443_rooms_oss_core.py` (16) covering mount, ungated first-match, no feature-id registration, both migration tracks, and the kind-scoped rename contract. Companion PR in trinity-enterprise deletes the module; the submodule bump follows its merge. Related to ent#443. Blocks ent#442. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * ci(guard): exempt the three room tables the OSS move adopted (ent#443) `enterprise-docs-guard` failed on the architecture.md section documenting the move: the pattern forbids `enterprise_[a-z_]+` in public docs, and naming `enterprise_rooms` / `enterprise_room_participants` / `enterprise_room_messages` reads as a private-schema disclosure. It isn't one any more, and the guard already carries this exact carve-out for ent#356's portal tables: those names moved onto the OSS track and their `enterprise_` prefix is retained history, because renaming them would be a data migration on every existing install. The room tables are the same case one module later, so they join the same allowlist with the same stated reason. Listed one by one rather than as a `room` prefix. The exemption should cover the three tables that actually moved — a future private `enterprise_room_*` table must still trip the guard, and it does (verified: `enterprise_room_secrets` still matches, `enterprise_rooms` no longer does). Related to ent#443. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(docker): ship shared_sessions in the prod image, and guard the COPY list (ent#443) `prod-image-smoke` failed: 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' The backend Dockerfile globs top-level MODULES (`COPY src/backend/*.py` — #1033's fix, after redis_breaker_util.py crash-looped the backend) but enumerates PACKAGES one COPY line at a time. So a new package builds a clean image that dies at import. The `client_portal` COPY line carries a comment predicting exactly this; ent#443 is the prediction coming true one module later. Two changes: * the missing `COPY ../../src/backend/shared_sessions /app/shared_sessions/`; * a source-level parity guard, because the only thing standing between this class of bug and production was a job that builds the entire image and reports a runtime crash. `test_ent443_backend_package_copy_parity` parses main.py's module-scope imports and the Dockerfile's COPY lines and fails in milliseconds, naming the exact line to add. Verified by deleting the COPY and watching it go red — a guard that has never been seen failing is a guess. It does NOT replace prod-image-smoke: that job proves the image boots, which no static check can. This only proves the COPY list has not fallen behind the import list. `enterprise` is exempt (optional submodule, guarded import). Related to ent#443. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(rename): teach the #1819 seeder about kind-scoped AGENT_REFS (ent#443) `regression diff` caught this: `test_rename_rekeys_every_registered_table` failed under HEAD and not under BASE, and it is a HARNESS gap, not a rename bug. The test seeds one row per AGENT_REFS entry, renames, and asserts nothing is left behind. ent#443 registered the room tables with a predicate — the `identity` / `sender_identity` columns are POLYMORPHIC, so each ref carries `kind = 'agent'` to keep a rename off a human participant whose user id or verified email happens to equal the agent's name. A seeded row that does not satisfy that predicate is skipped BY DESIGN, and then reported as a strand the seeder itself caused. The harness already knew about this shape: it hard-coded `scope = "agent"` for the scope-filtered `mcp_api_keys` refs, with a comment saying a synthetic value "matches no filter, which would look like a strand when it is really this seeder's fault". That special case is now generalized — `_filter_values` parses any ref's `extra_filter` and seeds the columns it requires — so the next filtered ref is covered without touching this file again. Behaviour is unchanged and was never wrong: `test_ent443_rooms_oss_core` proves a rename re-keys the agent participant and leaves the `user` and `workspace_user` rows (seeded with colliding identities) untouched. Related to ent#443. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(migrations): renumber the room revision to 0044 and pin the hotfix line (ent#443) Review finding 2: `0039_shared_sessions_oss` shared a numeric prefix with `dev`'s `0039_operator_queue_addressed_to`, and both chain off `0038_portal_chat_state`. The fork itself is not the defect — it is forced. This PR is a hotfix onto `main`, whose Alembic head IS `0038_portal_chat_state`; 0039-0043 exist only on `dev`, so chaining off `dev`'s head would name a revision `main` does not have and fail boot on the line this ships to. `main` must resolve to one head, and it does: verified with the repo's own guard (40 revisions, 1 head) and with a real `alembic upgrade head` against PostgreSQL 16 from a narrow (VARCHAR(32)) `alembic_version`, reaching `0044_shared_sessions_oss` with the column widened to 255. A downgrade leaves all three room tables in place and the re-upgrade over pre-existing tables is a clean no-op — the adoption contract, exercised. So what is fixed here is the part that was avoidable: the duplicated numeric prefix, and the fact that the back-merge obligation lived nowhere. - Renumbered to `0044_shared_sessions_oss` (parent unchanged, still 0038) so the filename does not collide with `dev`'s 0039 after the back-merge. Revision ids are strings, but the numeric prefix is the graph's only human ordering cue, and a duplicate would be the first in the repo — on exactly the graph whose failure mode is invisible to git. - Documented in the revision docstring, architecture.md and a new test: the back-merge MUST add an `alembic merge` revision with `down_revision = ("0043_subscription_headroom_history", "0044_shared_sessions_oss")`. It cannot be added 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. - `test_the_room_revision_chains_off_the_hotfix_line_and_numbers_uniquely` asserts the parent is 0038 and that no two revision files share a prefix. Simulated the back-merge to prove both halves rather than assert them: dev's `versions/` plus this file fails the guard at exactly `0038_portal_chat_state`, and adding the prescribed merge revision collapses it to one head. Suites: test_ent443_rooms_oss_core (17 pass), and the `-k "schema or alembic or parity or migration"` sweep (562 pass, 1 skip). Related to ent#443. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(learnings): capture the Dockerfile package-COPY and hotfix-Alembic-fork classes (ent#443) Two entries from the #2355 re-review, both third-or-later occurrences. The COPY class has now bitten three times — #1033 (a module, fixed by the `*.py` glob), ent#356 (`client_portal`), ent#443 (`shared_sessions`) — and is invisible to every source-only check by construction, because the source tree always has the directory. #2355 added the static parity guard; the entry records why the guard needs an anti-vacuity anchor and where it is still blind (transitive imports). The Alembic entry records the shape a hotfix migration takes: the single-head guard is green on BOTH branches while the graph is forked, because the fork only exists in the relationship between them. It names the four things that make it safe — chain off the branch you ship to, number past the other branch's head, simulate the merge with the repo's own guard, and exercise an adoption revision by downgrading and re-upgrading over existing tables. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(rooms): finish the docs half, and stop reporting a refusal as an absence (ent#443) Four review findings from the #2355 re-review, none blocking, all cheap. 1. The docs half of the move was incomplete. The PR corrected three lines and left nine, including the feature's own page opening with "**Enterprise feature.**" and a sentence in `sharing-access-and-monetization.md` eight lines below the one it fixed. Since the stated motivation was that the published guide had drifted ahead of the code, a half-corrected guide is the same defect with a smaller radius. Six files updated; the only surviving mentions of "enterprise" beside rooms now say it *used to be* one. Two of those lines were stale twice over: `faq/collaboration.md` and `guides/using-trinity.md` both described reaching rooms through a **Sessions** view that ent#381 retired. Rooms open from the Workspace by @mentioning a second agent, which is what they now say. 2. `rooms.ts` treated any 403 or 404 as "shared sessions are not enabled on this instance". That was true while the module was entitled — an unentitled 403 really did mean absent — and ent#443 makes it false: the rooms module authors 403 for an agent the caller may not reach, and a deliberately uniform 404 for a room it is not a member of. Both were being reported to the agent as a switched-off feature. The status alone cannot answer the question, so the discriminator is now the shape of `detail`: the serving module authors `{code, message}`, absence is a plain string (FastAPI's own "Not Found", or an older build's entitlement sentence). That is the rule the Workspace already follows for the same distinction (#2128) — this is the MCP surface catching up. Everything unrecognised still degrades to the friendly result, so the default is unchanged. `ApiError` retains the raw body for it rather than having callers regex the message, and a non-ApiError keeps the old string test. Seven cases pinned in `rooms-availability.test.ts`, including the coded-404 one, which is the case a status-only rule gets wrong most often. 3. The new COPY parity guard keyed on `main.py`'s imports, so a package pulled in only transitively would still die at import. Every non-exempt package under `src/backend/` is copied today, so asserting that directly is free and closes it; `_EXEMPT` stays as the escape hatch. 4. `router.py`'s gate comment, which this PR rewrote from "Triple-gated" to "Double-gated", kept claiming `require_admin` alone would admit an agent. #1890 / ent#297 moved that rejection into the gate itself. The code is right — `reject_agent_principal` is belt-and-braces — but a rationale this repo treats as load-bearing should not assert something the gate stopped doing. Tests: 511 passed, 1 skipped (`-k "ent443 or 2128 or parity or schema or alembic"`); MCP server 161/161. Related to ent#443. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…evision (ent#443) A MERGE, not a rebase. `dev` is shared and protected-by-convention: every open PR is based on it, so rewriting its history would invalidate every clone and orphan those bases. This repo has said so before — `7dc2aaef chore: reconcile main into dev (merge, not rebase)`. Brings #2355 (multi-agent rooms → OSS core), plus two docs/dependabot commits. ## The Alembic fork, and why a merge revision rather than a renumber `0044_shared_sessions_oss` chained off `0038_portal_chat_state` because that IS `main`'s head; pointing it at `dev`'s head would have named a revision `main` does not carry — a boot failure on the line it shipped to. `dev` had meanwhile grown `0039`…`0043` off the same parent, so the graph has two children of 0038. Neither branch could see it alone: `check_alembic_heads` was green on the hotfix PR (single-head tree) and green on `dev` (never saw 0044). It becomes two heads here, at the back-merge, which is exactly where it was predicted to. That matters more than it looks: `alembic upgrade head` is singular and resolves its target BEFORE applying anything, so a two-head graph applies ZERO revisions, silently — not just the forked one, but every revision since the fork. `0045_merge_rooms_hotfix` is an empty merge revision with a tuple `down_revision`. Repointing `0044` at `0043` would also give one head and would LOOK fixed; it is not. An install already stamped at 0044 would then be at head, so `upgrade head` would apply nothing and `0039`…`0043` would never reach it — permanently, silently. The tuple converges from any starting state. ## Conflicts, both append-only * `db/migrations.py` — both lines added a migration function and a MIGRATIONS entry at the same spot. BOTH kept: dropping either silently stops that line's schema arriving. main's rooms adoption is appended last (independent tables, and appending is this list's convention). * `docs/memory/learnings.md` — both appended entries. Both kept, re-sorted chronologically; my first pass kept each side intact but interleaved the dates wrongly, which reads as fine and is not. Related to ent#443. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
| dev | main | merged | ||
|---|---|---|---|---|
.claude |
e1229b39 |
92a56ec8 |
e1229b39 |
main's is an ancestor → fast-forward |
src/backend/enterprise |
a4198127 |
65182c14 |
a4198127 |
main's is an ancestor → fast-forward |
Neither is a regression.
Migration graph
- OSS line: 46 revisions, 1 head (
0045_merge_rooms_hotfix) - Enterprise line: 17 revisions, 1 head — untouched
check_alembic_parity: PASS · revision-id lint: 48 passed
Convergence was proven against a real PostgreSQL for all three install populations before this PR was opened — fresh (46 applied), stamped 0044 (6 applied), stamped 0043 (2 applied), each ending at 0045 with both lines' schema (rooms=3 addressed_to=1 headroom=1).
Informational — 1
dev will run the rooms double-mount until the enterprise submodule is bumped.
dev's enterprise pin still contains _register_module(app, "shared_sessions") (backend/__init__.py:265), so after this lands both routers mount. Verified on this merged tree that the OSS one wins the match order, which is the transition state #2355 was designed for:
POST /api/rooms: 2 handler(s)
[0] shared_sessions.router deps=['get_room_principal']
[1] enterprise.backend.shared_sessions.router deps=['requires_entitlement…', …]
Not a defect in this PR, and not fixable here. But it means one action is still outstanding: bump src/backend/enterprise on dev once trinity-enterprise's rooms-removal PR merges — otherwise enterprise_features keeps advertising shared_sessions as an entitled module for a surface every build now serves.
Clean
- SQL / data safety — no new DDL;
0045is an empty merge revision (upgrade()is a docstring only). - Auth — no route definitions changed by the resolution; the only route-table effect is ordering, verified above.
- Enum completeness — no new enum values; the
MIGRATIONSlist is the analogous closed set and is counted exhaustively above. - Credentials — the one credential-adjacent file (
_migrate_secret_settings_encryption, dev-side) arrives byte-identical to dev. - Frontend / tests — arrive unmodified from their respective sides.
Nothing to fix in this PR.
|
Drive-by review — I merged the three PRs this reconciles (#2355 → trinity-enterprise#444 → #2360) while this branch was being cut, so flagging one thing it is missing. The enterprise submodule pin is not in this diffBut the branch does bring the OSS half across: And the tree still pinned at Both routers carry identical prefixes — What that actually costsNot a crash. That is precisely what #2360 was cut to prevent on Why git will not tell youThe gitlink and the new directory are independent paths with no textual conflict, so a merge that takes one without the other is a perfectly clean merge. No public CI catches it either — no workflow checks out the submodule ( FixMove the pin in this same commit — to Worth considering The rest looks right on the two items I checked: |
…2365 One conflict: src/backend/main.py router mounts — both sides appended at the same point (rooms routers from main's ent#443, portal_asks_router from dev's ent#428). Both kept; both mount before register_enterprise(app). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
vybe
left a comment
There was a problem hiding this comment.
Validated via /validate-pr: correct reconcile shape (merge, not rebase — 7dc2aae precedent), both append-only conflicts kept both sides, and the 0045 Alembic merge revision converges all three stamp states on real PG (pg-migrations green here). I updated the branch for the 4 dev commits that landed after the cut (merge cc735b0): one both-sides-append conflict in main.py router mounts — rooms routers (ent#443) + portal_asks_router (ent#428) both kept, both before register_enterprise. Zero markers tree-wide, imports verified, compiles. Head seeds + regression diff green; base seed 99999 red is the known baseline-job noise. Merging as a merge commit per reconcile convention.
Brings #2355 (multi-agent rooms → OSS core, ent#443) back from
main, plus two docs/dependabot commits, and adds the Alembic merge revision the hotfix's landing now requires.A merge, not a rebase
devis shared: every open PR is based on it, so rewriting its history would invalidate every clone and orphan those bases. This repo has already settled the question —7dc2aaef chore: reconcile main into dev (merge, not rebase).The Alembic fork — the part that actually needs care
0044_shared_sessions_osschained off0038_portal_chat_state, which ismain's head and was the only correct parent: namingdev's head would have referenced a revisionmaindoes not carry, i.e. a boot failure on the line it shipped to.devhad meanwhile grown0039…0043off that same parent — two children of 0038.Neither branch could see it alone.
check_alembic_headswas green on the hotfix PR (single-head tree) and green ondev(never saw 0044). It becomes two heads here, which is where it was predicted to.Why that is worse than it looks:
alembic upgrade headis singular and resolves its target before applying anything, so a two-head graph applies zero revisions — silently, and not just the forked one. Every revision since the fork stops arriving.Why a merge revision and not a renumber
Repointing
0044.down_revisionat0043also yields one head and looks fixed. It is not: an install already stamped at 0044 would then be at head, soupgrade headapplies nothing and0039…0043never reach it — permanently, silently.0045_merge_rooms_hotfixcarries a tupledown_revision, which converges from any starting state.All three verified against real PostgreSQL:
0045_merge_rooms_hotfix0044(took the hotfix)0039…0043+ merge)0045_merge_rooms_hotfix0043(dev line)0044+ merge)0045_merge_rooms_hotfixand all three end with both lines' schema:
rooms=3 addressed_to=1 headroom=1.SQLite track re-checked too —
init_database()recordsshared_sessions_tables_to_ossand the dev-side migrations, with the room tables, theaddressed_to_emailcolumn andsubscription_headroom_historyall present.Conflicts, both append-only
db/migrations.py— both lines added a migration function and aMIGRATIONSentry at the same spot. Both kept: dropping either silently stops that line's schema arriving. main's rooms adoption is appended last (independent tables; appending is this list's convention).docs/memory/learnings.md— both appended. Both kept and re-sorted chronologically. My first pass kept each side intact but interleaved the dates wrongly — which reads as fine and is not, so it is worth saying it was caught rather than avoided.mainitself is functionalBooted a pure-OSS image built from
366cf505and exercised the merged feature:Clean boot, no errors in the log beyond the expected "no Docker socket" warnings for a socket-less test container.
Related to ent#443.
🤖 Generated with Claude Code