fix(settings): render the Room budget defaults panel — it gates on an entitlement ent#443 removed (#2620)

[dolho]

Fixes #2620.

The bug

The Room budget defaults panel (Settings → Retention) was hidden on every install. It gated on isEntitled('shared_sessions') — correct while rooms were an enterprise module — but ent#443 moved multi-agent rooms into OSS core: main.py mounts both routers unconditionally, shared_sessions/router.py records in its own header that its routes "used to carry requires_entitlement", and nothing registers that feature id any more.

So the predicate was False on every build, OSS and enterprise alike, while the endpoint behind it worked for any admin. Rooms functioned; only their dial was missing. Found by an operator asking why a room stopped at 59/60 messages and closed permanently, with no UI to raise the default.

The fix

Remove the gate. No authorization changes: budget_router is require_admin (plus reject_agent_principal on the write), and the panel only mounts on Settings' Retention tab, which is adminOnly: true. The panel's load() also had an if (!entitled.value) return, so even had it rendered it would have shown empty — that goes too.

Why this is more than one panel

Second instance of one class. ent#356 did exactly this to client_portal, caught in NavBar.vue — whose comment even named the mechanism that would eventually break it:

It survived only because the enterprise submodule has not yet dropped its registration … i.e. it was already relying on a registration that is about to disappear.

The failure is invisible in the worst way: nothing errors, no route 404s, the capability works, and only a control disappears — so it surfaces as a user asking where a setting went, not as a bug report.

Two things to stop a third:

• retiredEntitlementGates.spec.js lists the ids that moved to OSS (client_portal, shared_sessions) and fails on any isEntitled(...) gate naming one, anywhere under src/frontend/src. Adding an id to that list is the second half of moving a module to OSS. Comments are stripped before matching, so the two files that document this history don't read as committing it.
• shared_sessions.service.FEATURE_ID is documented as retired rather than deleted. The issue suggested removing it, but the private enterprise submodule is not visible from here and may still import the name — deleting a module-level constant a hidden consumer might import is a break I could not test. It is now explicitly marked as not-an-entitlement-id, since a surviving constant is precisely what makes this read like a live gate to the next person.

Verification

• Full frontend suite: 2303 passed (104 files)
• Backend room/settings suites: 474 passed
• Mutation-tested: restoring the v-if="entitled" gate and its computed fails 2 of the guard's 3 cases (no gate references a feature id that moved to OSS core, the room budget panel renders without an entitlement check). Panel restored byte-exact afterwards.
• The sweep the issue asked for: isEntitled('shared_sessions') had exactly one occurrence, and the other live gates (slack_per_agent_bots, cross_model_validation, user_management, retention) all name modules that are still entitled.

Docs: learnings.md records the class.

Related to #2620

🤖 Generated with Claude Code

https://claude.ai/code/session_01CdxGmvuKuqaWJZ6pKUgaUJ

[dolho]

Operator feedback handled — pushed as 609f3475.

Defaults raised (your call): 60 → 200 messages, 24h → 1 week TTL, no cost cap. Both limits close a room permanently (close_room is a one-way CAS), so both were ending working conversations rather than catching runaways. Note this makes max_messages the only hard spend bound a room has.

The warning is now proactive and says what happens. Before: a small amber 59/60 messages — the ratio, never the consequence. Now:

Band	Where	What it says
≥80%	header, amber	"40 messages left in this room" + full sentence on hover
last 5	header red and a banner above the composer	"3 messages left. At the limit the room closes for good — the transcript stays readable, but nobody can post again. Start a new chat to carry on."
<80%	nothing	a permanent gauge is how a warning gets ignored

Rule is pure in utils/roomBudgets.js::budgetNotice (vitest is node-env, no mount harness).

Two ent#387 tests asserted the literal 60; bound to service.DEFAULT_MAX_MESSAGES rather than re-pinned to 200, since their property is "the client's value is discarded and the operator default applies".

Suites: backend room/settings 488 passed, full frontend 2314 passed.

One open decision for the reviewer: MAX_TTL_HOURS is also 168, so the default now sits at its own ceiling — the panel can move it down or to 0 (never), but not to, say, two weeks. Raising the ceiling to 720 would restore headroom; I did not take that decision here.

[dolho]

/review report — fix/2620-room-budget-panel-gate → dev

Reviewed against merge-base 818e7f5f. 9 files, +325/−37.

Scope: CLEAN. Intent was the hidden panel (#2620) plus the operator feedback on this PR; the diff is that and nothing else.

Critical — found and fixed in 767a27f1

[C1] The budget banner stole the composer's v-else (confidence 10/10)
src/frontend/src/components/portal/PortalRoom.vue

<div v-if="attachments.length"> …chips… </div>
<form v-else>                    ← the composer

v-else binds to the immediately preceding element. The banner I added in 609f3475 sat between them, so the composer chained to the BANNER's condition — and disappeared at exactly the moment the banner fired, i.e. the last few messages of a room. The warning removed the ability to act on it.

Nothing that ran could see it: the SFC compiles (a v-else after any v-if is valid) and the suite has no mount harness, so no test renders the template. Fixed by moving the banner above the attachments block, plus roomComposerChain.spec.js pinning the sibling relationship.

Worth recording separately: the first version of that guard was vacuous. It matched text — "is the gap after the last </div> empty" — and the intruder's own closing tag becomes the last one, so the mutation passed. It now walks the template AST. Mutation-tested both ways.

Informational

[I1] The default now equals its own ceiling (confidence 9/10) — MAX_TTL_HOURS = 168 and DEFAULT_TTL_HOURS = 168, so the panel can move TTL down or to 0 (never), but not to two weeks. Workable, not broken; raising the ceiling to 720 is a one-line operator decision I did not take.

[I2] max_messages is now the sole hard spend bound (confidence 10/10) — no default cost cap, and TTL is a clock not a budget. Stated in the constant's comment so the next person tuning it knows.

[I3] db/schema.py column default stays 60 (confidence 8/10) — annotated as the vestigial fallback it is; every insert supplies the value, and Alembic 0044 carries the same literal. Rewriting an applied migration would be worse.

Clean

SQL (no new queries) · auth (gate removal is server-enforced: require_admin + adminOnly tab) · concurrency (no shared state) · credentials (none touched) · enum completeness (no new values) · migrations (none needed).

Suites

Full frontend 2317 passed; backend room/settings 488 passed.

[dolho]

/review report — second pass (post-fix)

Re-reviewed the full branch against merge-base 818e7f5f — 4 commits, 10 files, +343/−37. This pass looked hardest at what the first pass changed, plus the parts I had read rather than exercised.

Critical: none. [C1] from the first pass (the banner stealing the composer's v-else) is fixed in 767a27f1 and pinned by an AST guard, mutation-tested in both directions.

What this pass actually did

Rather than re-read budgetNotice, I executed it against adversarial input:

input	result
count > max (205/200)	critical — "reached its message limit", no negative remaining
cost exactly at cap	critical, cost
cost over cap ($12/$10)	reports the overspend honestly
message_count: '199' (string from JSON)	coerced, critical
negative count (bad data)	null — silent, not a false alarm
closed room at cap	null — the room's own "ended" line owns that

Findings (all informational)

[I4] Both budgets critical → messages wins (confidence 9/10). budgetNotice checks messages first, so a room at 199/200 and $9.90/$10 reports only messages. Deliberate — one line, and messages is the bound closing it here — but a cost-capped room can be a dollar from closing while the notice talks about messages. Worth knowing if cost caps get used.

[I5] ParticipantsRail.vue still hard-codes default: 60 (confidence 10/10). Harmless: the component is unreferenced — dead since ent#381 retired the Sessions page — so the stale default never renders. Left alone; it belongs to the dead-component cleanup in #2492, not here.

[I6] One assertion is now coincidentally vacuous-adjacent (confidence 7/10). test_workspace_client_cannot_set_its_own_budget has the client supply ttl_hours=168, which now equals the default. It asserts only max_messages and max_cost_usd, so nothing is weakened today — but a TTL assertion added there later would pass without testing anything.

[I1]–[I3] from the first pass stand: TTL default equals its own ceiling; max_messages is the sole hard spend bound; the DDL's 60 is a documented vestigial fallback.

Fixed in this pass

9db2cf6f — a duplicated "The proactive half" phrase left in the banner comment by the C1 fix. Cosmetic, no behaviour change.

Clean

SQL · auth · concurrency · credentials · enum completeness · migrations (none needed) · no dead code in the diff (the panel's computed import is still used by dirty).

Suites

Full frontend 2317 passed · backend room/settings 488 passed.

[vybe]

merge-train: ejected — required check red. CodeQL (required on dev) reports a new high alert: js/incomplete-multi-character-sanitization at src/frontend/tests/unit/retiredEntitlementGates.spec.js:33 (a <!-- strip that can still leave <!--). Fix or dismiss with a stated reason, then resolve the dev conflict. Rides the next train once green.

[vybe]

merge-train (2026-09-09): pushed 5b9e3b7a to this branch — a merge of origin/dev (105be06) resolving the append collision on docs/memory/learnings.md (dev's #2610 entry first, this PR's #2620 entry after it). No code changed. This also gives head 374691bc its first CI run, which should close CodeQL alert 349 (the fixed-point stripComments loop matches the rule's loop exemption). Verified on the merged tree: test_ent387_room_budget_defaults.py 14 passed; retiredEntitlementGates + roomBudgets + roomComposerChain specs 25 passed. Non-blocking follow-up for you: docs/user-docs/collaboration/rooms.md:14 still states the old 60 / 24h defaults. Riding this train.

[vybe]

merge-train (2026-09-09, follow-up): the fresh run closed alert 349 but CodeQL raised 350 on the fixed-point loop itself (retiredEntitlementGates.spec.js:41) — its loop exemption looks for the result flowing back to the same receiver, and a chained .replace(...).replace(...) doesn't match that shape. The loop is correct (exits only when a pass changes nothing, so no <!-- can survive) and the helper is test-only with regex-matched, never-rendered output, so I dismissed 350 as a false positive with that reason rather than send this back. If you'd rather satisfy the rule structurally, out = out.replace(...) one pattern at a time inside the loop is the shape it recognises.

[dolho]

CodeQL finding addressed in 374691bc.

js/incomplete-multi-character-sanitization on retiredEntitlementGates.spec.js's comment stripper. It is not an XSS sink — the output is only regex-matched against isEntitled('<id>') and never rendered — but the rule points at a real hole in the guard itself: one pass over <!--…--> can leave a fresh <!-- behind (<!-<!---->-> strips to <!-->), so a live gate written inside a comment-shaped string could survive stripping and read as a comment the guard was told to ignore.

The three replaces now run to a fixed point, and the comment says which of the two problems this is about. 3 specs pass.

[vybe]

merge-train (2026-09-09): pushed a merge of origin/dev resolving the append collision on docs/memory/learnings.md left by the previous train member landing (keep-both, dev's entry first). No code changed.

[vybe]

merge-train: batch validated on train/20260909-0822 (#2639) — full suite green on the combined batch.