Skip to content

feat(workspace): agents at the centre — the pinned Main chat, Reset, and files onto the conversation (abilityai/trinity-enterprise#523, abilityai/trinity-enterprise#524) - #2558

Merged
vybe merged 16 commits into
devfrom
dolho/issue-523
Sep 7, 2026

Conversation

@dolho

@dolho dolho commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Journey Impact: extends J04 — the execution lifecycle, via the parent epic abilityai/trinity-enterprise#472. Workspace chat itself still has no journey in the current deck — the same finding as ent#451 / ent#472 / ent#523.

Fixes abilityai/trinity-enterprise#523
Fixes abilityai/trinity-enterprise#524
Fixes abilityai/trinity-enterprise#468

Summary

Two rulings from the 2026-09-06 operator session, in one PR because ent#524's drop target lands on the very surface ent#523 rebuilds and the operator asked for one design pass over both.

ent#523 — agents at the centre. Clicking an agent opened a report about it, with the conversation one click further behind Start a chat. Ruled inverse 2026-09-05 ("make agents the central entity, so people think of the agents they talked to"), design approved 2026-09-06 (board A3). Clicking an agent now opens the chat you were last in; every (user, agent) pair gets one pinned Main chat — the place the agent reaches you — and Reset archives it and starts the agent cold.

ent#524 — files onto the conversation. The one surface people work on was the one surface you could not drop a file onto, and both existing upload paths silently discarded every file after the first.

ent#360's reasoning is not reverted: an agent still has a home with its history, what it can do, and a place to ask you something. It is simply no longer a stop on the way to the conversation.

Main and Reset

enterprise_portal_sessions gains is_main and archived_at on both tracks (SQLite portal_session_main_chat + Alembic 0053), plus the partial unique index idx_portal_sessions_main.

  • The index is the invariant, not an optimisation. ensure_main_session is reachable from two request paths and runs in every uvicorn worker, so a check-then-insert races two Mains into existence for one pair — after which "the pinned first tab" has no single answer. The loser catches IntegrityError and adopts the winner's row.
  • WHERE is_main = 1 is load-bearing. An archived row keeps its (agent_name, client_email) pair forever, so an unconditional unique index would refuse the second Reset.
  • No backfill (the ent#473 title_source precedent one revision back). Backfilling would have to anoint one existing thread per pair, and "whichever was most recent when we migrated" is not a fact anyone asked for. Main is minted lazily by list_sessions and _resolve_session_id, and deliberately not by the cross-agent batch (bug: Agent Detail and Workspace over-fetch on load (21 redundant requests, N+1 roster) #2198), which runs on every sidebar refresh and would write a row per agent the person never opened.
  • Reset needs no second reset primitive. A fresh row carries no cached_claude_session_id and the turn engine resumes only on a cached id, so cold is the new row's property rather than an action against the old one. routers/sessions.py::reset_session_memory is untouched and uncalled — clearing a cache and keeping the thread is a different verb from retiring the thread. The archive keeps its own cached id (still resumable, still in session_cleanup_service's keep-set) and its own title. Per-user memory (MEM-001) is not touched. No confirmation dialog (operator, 2026-09-06 09:27) — ConfirmDialog is not a caller.
  • Refused with a named 409 while a turn is in flight (turn_in_flight) or when a concurrent Reset won (reset_raced). Resetting an untouched Main is a no-op reported as archived_session_id: null — archiving anyway mints an empty thread per click and files it under a name nobody chose.
  • The landing rule is one edit. _resolve_session_id(agent, email, None) resolves to Main, which covers an agent-initiated message, an ask raised outside a chat (ent#364/refactor: collapse 9-path cleanup pyramid once agent is authoritative #429) and a scheduled brief (ent#498) — all three already funnel through it. An explicit session id still wins.

One page

PortalAgentPage.vue is dismantled, not deleted — every section has a named home, so "no capability is lost" is checkable rather than asserted:

Was on the agent page Is now
stats strip + Activity chart PortalAgentBand.vue, always visible above the thread
Your chats / What it can do / Reports PortalAgentDetails.vue
Canvas · Files already rail tabs (ent#475)
Recent work / Activity already the rail's Work tab (ent#525)
asks the conversation's mount — the surviving one after #2449
health + availability the details header, still two separate facts (#2196)
Start a chat gone; the row opens the chat
  • Agent details is a sibling of the rail, not a rail tab (ruled 2026-09-05): the rail is participant-scoped with a fixed five-tab set, this is about one agent and is dismissed rather than switched away from. portalRail.js is untouched; the rail's state is a setup ref, so closing details returns it on the tab it was showing.
  • landingThread is the ONE rule for which chat you land in, and the ?agent= deep link's resolveAgentLanding now defers to it. It used to take the first row of a sorted list — which agreed with "most recent" by accident, and an unused Main sorts last on recency, which is exactly where that accident would have surfaced.
  • Main is named by its role in the tab strip and the header and is not renameable: it is the same thread for the life of the pair, and a derived or typed title would make the two disagree about which chat you are in.
  • Archived chats leave the tab strip (Reset would otherwise add a permanent tab per use) and stay in the sidebar and in details. An unused Main is filtered from the sidebar only — a projection, not a filter on threads, because the strip must show Main from the first visit.
  • The roster is ordered before the collapse; bounding first would sort a slice chosen by the old order. This is not ent#491 (incubating) — it ships the order the AC states and leaves primaryName as that issue's seam.
  • The composer labels an unavailable agent, never disables: a client whose agents are all stopped gets an inert Workspace otherwise.

Files onto the conversation

composables/usePortalFileDrop.js is the ONE implementation, used by the conversation, the room and the rail's Files tab — the issue forbids a second, and the reason is in the defect: the gesture already existed on the Files panel and the [0] bug existed there too, because each surface had written its own.

  • both <input type="file"> gain multiple; no path reads [0]
  • one chip per file with its own progress and outcome; a refused file names itself and the limit; a 429 batch says which files landed and when to retry
  • uploads run sequentially — twenty parallel requests is the surest way to trip the per-email limiter (ent#287) on a gesture that would have succeeded spread over a second
  • a room's drop fans out to every participating agent's inbox and the chip names the recipients (operator decision 13)
  • dragenter/dragleave are depth-counted (they fire per child element); the overlay is pointer-events-none so it cannot swallow the drop it announces; isFileDrag keeps a dragged link or text selection from lighting it
  • the destination stays the caller's upload, so ent#484/fix(security): patch 4 Dependabot alerts (happy-dom RCE + vite file read) (#485) #486's working folder can take it over without the gesture changing

The #1927 ratchet caught v-if="f.uploading" and was right to — a scanner cannot tell that from the bare fetch-in-flight gate it exists to stop. Fixed the way the ratchet asks rather than by renaming: a chip has three outcomes and no fourth, so both surfaces render from one derived attachmentState, and PortalConversation's baseline entry drops 1 → 0.

An answered ask says whether work started (ent#468)

ent#468 asked for a decision, render or drop. Render — recorded on the issue with its reason. Dropping loses a fact the person is entitled to: on an operator_resume_enabled agent their answer sets real work in motion and spends the owner's budget, and the client causing that spend was the one party getting no acknowledgement. ent#364's AC ("the answer reaches the agent and it resumes") was true in the backend and invisible in the product.

  • portalUtils.js::answerConfirmation consumes both fields, which is that issue's "either both or neither": status === 'answered' is the gate, resume_requested is the wording.
  • Opt-in ON → "Sent — <agent> is picking this up."; OFF / false / absent → "Sent." The tense follows _resume_requested's own contract — a report of intent, since the dispatch is backgrounded — so never "has done it". Reading null as "started" would re-introduce the over-claim ent#430 spent a blocker removing.
  • The confirmation cannot live on the ask row, because answering removes it — that AC bullet is the structural difficulty. PortalAsks.visible gated on items.length > 0, so the surface unmounted at the same instant the confirmation was created and the message would have rendered for zero frames. It now stays mounted while one is up, clears itself, and clears its timers on unmount (this surface unmounts on every chat switch).
  • No backend change — the fields were already correct and already on the wire. This is the consumer they were missing.

Folded in here because it lands on the same surface: PortalAsks renders above the composer of the conversation ent#523 rebuilt.

Changes

  • Backend: client_portal/{db,service,router,models}.py, db/{schema,tables,migrations}.py, migrations/versions/0053_portal_session_main_chat.py
  • Frontend new: composables/{usePortalAgentPage,usePortalFileDrop}.js, portal/{PortalAgentBand,PortalAgentDetails}.vue
  • Frontend edited: views/Portal.vue, portal/{PortalConversation,PortalRoom,PortalSidebar,PortalRailFiles}.vue, portal/portalUtils.js, stores/clientPortal.js
  • Frontend deleted: portal/PortalAgentPage.vue
  • Docs: requirements §5.23 (and a pre-existing duplicate §5.21 fixed), new feature-flows/workspace-agents-at-the-centre.md + both index entries, architecture/workspace.md, workspace-agent-page.md marked superseded with the mapping, workspace-chat-tabs-and-titles.md closed out
  • ent#468: portal/portalUtils.js (answerConfirmation), portal/PortalAsks.vue
  • Tests: test_ent523_main_chat.py (new, 22), portalAgentsAtCentre.spec.js (new, 48), ten existing specs re-pointed

Ten specs re-pointed rather than deleted. Where a rule MOVED, the guard follows it (portalRatings, portalReportsRendering, portalAvailabilityChip, portalAskSingleSource). Where a rule was RETIRED on purpose, the guard asserts the replacement and names the old one, per the #2169 convention in that file (portalAgentPageUx's two-column Overview guards). Where a guard pinned a v-if / v-else-if spelling on a chain whose membership this changes, it now asserts the rule — keyed on the verdict, branches are its v-else — not the ordinal (portalRail, portalLoadingTreatment, workspaceRoomsGate F23, portalRosterRow, portalSidebarSearch).

Test Plan

  • pytest tests/unit/test_ent523_main_chat.py — 22 passed. Both migration tracks and the index predicate; ensure_main_session idempotency + the lost-race path against a real sqlite carrying the real partial index; the landing rule through _resolve_session_id and ensure_thread_for_ask; Reset's archive/mint/system-line, cold-by-construction (asserting the ARCHIVE keeps its cached id), the in-flight 409, the untouched-Main no-op, repeatability, uniform 404. The index DDL is read out of schema.py rather than retyped, so the test cannot pass against an index the product no longer creates.
  • Neighbours: test_ent451/473/525/358/364, test_2196, test_2198 — 254 passed, and the one break was a test asserting the rule this PR deliberately reverses (updated, not deleted).
  • npm run test:unit — 2036 passed (90 files). Both ratchets green: loading-gate baseline paid down 1 → 0 and diffed (nothing grew); raw-color measured against origin/dev — no touched file grew raw_nongray or hardcoded_colors (0 everywhere), only gray chrome, which the scanner itself calls partially sanctioned.
  • vite build clean. New spec mutation-checked: removing the Main pin fails 2 of its assertions.
  • Live on the Docker stack (backend --reload, PostgreSQL): migration applied with the partial index present; Main minted on first open and pinned first; Reset archives with the title kept, mints a cold Main and writes the system line; a second Reset works (the predicate); reset on an untouched Main is a no-op; unknown agent → 404; exactly one live Main in SQL. In the browser: clicking the agent lands in the conversation with no Start-a-chat button, band shows stats + chart + window + Agent details, Main is the first tab, the archive appears as an ordinary chat, Agent details opens in the rail's place and closing returns the rail, the sidebar row carries the preview and the unused Main is not double-listed. ent#524: dragging files lights the affordance, and dropping two files produces two chips.
  • Reviewer: light + dark pass over the band, the details panel and the drop overlay; a room drop with two participants (recipients named on the chip); --skip-agent verify-local.

Not in this PR

Recorded so the narrowing is never inferred later from the fact that it merged (the ent#474 convention):

  • the State tab (ent#439)
  • the working-folder destination for dropped files (ent#484 / ent#486) — this ships against the existing upload path and the gesture does not change when they land
  • ent#498's brief delivery, which consumes this landing rule but is its own issue
  • ent#492's resize handles; the tab strip repacks without them because OverflowTabs re-measures on ResizeObserver
  • ent#491's roster ordering (incubating) — a deterministic order ships and leaves the seam

🤖 Generated with Claude Code

https://claude.ai/code/session_01WLerYYUUEEmf43UKF2VVRY

dolho and others added 6 commits September 7, 2026 10:36
Every (user, agent) pair gets one Main chat — the place the agent reaches you
when no conversation named itself. Reset retires it and starts the agent cold.

`enterprise_portal_sessions` gains `is_main` and `archived_at` on both
migration tracks (SQLite `portal_session_main_chat` + Alembic 0053), plus the
partial unique index `idx_portal_sessions_main`. That index is the invariant,
not an optimisation: `ensure_main_session` is reachable from two request paths
in every uvicorn worker, so a check-then-insert races two Mains into existence
for one pair. Its `WHERE is_main = 1` predicate is load-bearing — an archived
row keeps its (agent, client) pair forever, so an unconditional unique index
would refuse the second Reset. No backfill; Main is minted lazily, and only
from `list_sessions` and `_resolve_session_id`, never from the cross-agent
batch (#2198) that would then write a row per agent the user never opened.

The landing rule is one edit: `_resolve_session_id(agent, email, None)` now
resolves to Main instead of the most recent thread, which is the whole of
AC 2 because every homeless turn already funnels through it — asks via
`ensure_thread_for_ask` (ent#364/#429), a scheduled brief (ent#498), a headless
API turn. An explicit session id still wins.

Reset needs no second reset primitive: a fresh row carries no
`cached_claude_session_id`, and the turn engine resumes only on a cached id, so
"starts cold" is a property of the new row rather than an action against the
old one. `routers/sessions.py::reset_session_memory` stays untouched — clearing
a cache and retiring a thread are different verbs. Per-user memory is not
touched. Refused with a named 409 while a turn is in flight, since retiring the
thread mid-turn lands the reply somewhere only a search would find.

Resetting an untouched Main is a no-op that says so (`archived_session_id:
null`): archiving anyway mints an empty thread per click and files it under a
name nobody chose. An untitled archive is dated, because these accumulate in
one list.

Related to Abilityai/trinity-enterprise#523

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WLerYYUUEEmf43UKF2VVRY
Clicking an agent opened a report about it, with the conversation one click
further behind "Start a chat". The operator ruled the inverse: agents are the
central entity, so clicking one opens the chat you were last in.

`PortalAgentPage.vue` is dismantled rather than deleted — every section has a
named new home, so "no capability is lost" is checkable rather than asserted:

  stats strip + Activity chart  -> PortalAgentBand.vue, always visible
  chats / what it can do / reports -> PortalAgentDetails.vue
  Canvas, Files                 -> already rail tabs since ent#475
  Recent work / Activity list   -> already the rail's Work tab since ent#525
  asks                          -> the conversation already mounted the
                                   surviving copy; the page's was #2449's
                                   second one

`/workspace/a/:agentName` keeps its URL and resolves to a chat. `landingThread`
is the rule — most recently active, Main as the floor — and the pure
`resolveAgentLanding` that answers the same question for the `?agent=` deep
link now defers to it, so a deep link and a sidebar click cannot land a
first-time visitor in different places. It used to take the first row of a
sorted list, which agreed with "most recent" by accident; an unused Main sorts
last on recency, which is where that accident would have surfaced.

Agent details opens into the RAIL'S PLACE (ruled 2026-09-05) as a sibling of
the rail, not a rail tab: the rail is participant-scoped with a fixed five-tab
set, while this is about one agent and is dismissed rather than switched away
from. The rail's state is a setup ref, so closing details returns it on the tab
it was showing.

Main is named by its role in both the tab strip and the header, and is not
renameable — it is the same thread for the life of the pair, and a title
derived from whatever was said in it first would make the two disagree about
which chat you are in. Archived chats leave the tab strip (Reset would
otherwise grow it by one permanent entry per use) and stay in the sidebar and
in Agent details. An unused Main is filtered from the SIDEBAR only, via a
projection rather than a filter on `threads`, because the tab strip must show
Main from the first visit.

The scanline stays on the chart and nowhere else (#2540): the band's stat
figures load with a skeleton beside a beam that wraps only the chart.

Five existing specs pinned behaviour that MOVED; they are re-pointed at the new
surfaces rather than deleted, and the two guards that pinned the retired
two-column Overview assert the replacement rule and name the old one, per the
#2169 convention in that file. Two more (`portalRail`, `portalLoadingTreatment`,
`workspaceRoomsGate` F23) pinned `v-if`/`v-else-if` spellings on a chain whose
membership this change alters; they now assert the rule — keyed on the verdict,
branches are its v-else — not the ordinal.

npm run test:unit: 1988 passed (89 files). vite build clean. Raw-color ratchet
measured against origin/dev: no touched file grew `raw_nongray` or
`hardcoded_colors` (both 0 everywhere); only gray chrome, which the scanner
itself calls partially sanctioned.

Related to Abilityai/trinity-enterprise#523

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WLerYYUUEEmf43UKF2VVRY
…#523, ent#524)

**ent#523 tail.** The sidebar orders agents by most recent collaboration then
name, applied BEFORE the collapse so the rows that survive the limit are the
ones the person uses — ordering after it would sort a slice chosen by the old
order. This is deliberately NOT ent#491 (still incubating): `orderRosterAgents`
ships the order this AC states and leaves `primaryName` as the seam. Each row
gains the preview line, which is the newest chat's TITLE rather than a message
body: the sidebar list is viewer-scoped metadata (#2198) and carries no message
content, so a body preview would reinstate the N+1 that batch call removed.

The composer says when an agent cannot take a message, reading the same pure
rule as the sidebar chip and the details header. A label, never a disabled
input — disabling relocates the dead state rather than removing it, and a
client whose agents are all stopped gets an entirely inert Workspace.

**ent#524.** One drop/batch implementation (`usePortalFileDrop`), three
consumers — the conversation, the room, and the rail's Files tab — because the
issue forbids a second and the reason is visible in the defect: the gesture
already existed on the Files panel, and the `files?.[0]` bug existed there too,
because each surface had written its own.

  - the whole conversation is the drop target, on by default, with an
    affordance that names what will happen; `isFileDrag` keeps a dragged link
    or selection from lighting it, and the overlay is pointer-events-none so it
    cannot swallow the drop it announces
  - a room's drop fans out to every participating agent's inbox and the chip
    names the recipients (operator decision 13, 2026-09-06)
  - both `<input type="file">` gain `multiple`; `onPickFile` / `onPick` /
    `onDrop` stop reading `[0]`
  - one chip per file with its own outcome; a refused file names itself and the
    limit; a 429 batch reports which files landed and when to retry. Uploads
    run sequentially, not `Promise.all` — firing twenty at once is the surest
    way to trip the per-email limiter (ent#287) on a gesture that would have
    succeeded spread over a second

The #1927 ratchet caught `v-if="f.uploading"` in the room, and it was right to:
a scanner cannot tell that from the bare fetch-in-flight gate it exists to stop.
The fix is the one the ratchet asks for rather than a rename — a chip has three
outcomes and no fourth, so both surfaces render from one derived
`attachmentState`, and PortalConversation's baseline entry drops 1 -> 0.

npm run test:unit: 1988 passed (89 files). vite build clean. Loading-gate
baseline regenerated and diffed: one file shrank, none grew.

Related to Abilityai/trinity-enterprise#523
Related to Abilityai/trinity-enterprise#524

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

Tests

- `tests/unit/test_ent523_main_chat.py` (22) — both migration tracks and the
  index PREDICATE (the difference between "Reset works twice" and "Reset works
  once"); `ensure_main_session` idempotency and the lost-race path, run against
  a throwaway sqlite carrying the real partial index rather than a mock of it;
  the landing rule through `_resolve_session_id` AND `ensure_thread_for_ask`;
  Reset's archive/mint/system-line, its cold-by-construction property (asserting
  the ARCHIVE keeps its cached id, which is what stops a future edit wiring in a
  second reset primitive), the in-flight 409, the untouched-Main no-op,
  repeatability, and the uniform 404. The index DDL is read out of `schema.py`
  rather than retyped, so the test cannot keep passing against an index the
  product no longer creates.
- `src/frontend/tests/unit/portalAgentsAtCentre.spec.js` (41) — every pure rule
  (tab pin, archived exclusion, landing, roster order, preview, composer
  notice), the ent#524 rules (file-vs-text, rejection copy, retry-after,
  attachment state), and the source guards no unit test can reach. Mutation-
  checked: removing the Main pin fails 2 of them.

Docs

- requirements `core-agent.md` §5.23. Also fixes a PRE-EXISTING collision: §5.21
  was used twice (ent#451/473 and ent#525); the second becomes §5.22, with its
  one cross-reference.
- new `feature-flows/workspace-agents-at-the-centre.md` + both index entries.
- `architecture/workspace.md` — Main, Reset, the one page, the file drop.
- `workspace-agent-page.md` marked SUPERSEDED with a table of where each section
  went, rather than deleted: the page's reason for existing is still why its
  content had to go somewhere rather than away.
- `workspace-chat-tabs-and-titles.md` — its "what this leaves to #523" section
  closed out, including that the sidebar criterion needed no routing once the
  agent page IS the conversation.

Related to Abilityai/trinity-enterprise#523
Related to Abilityai/trinity-enterprise#524

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WLerYYUUEEmf43UKF2VVRY
…ver test follows the new rule

Two loose ends from the live pass.

The Agent-details chat list rendered in sidebar order, so Main sat wherever
recency put it while the tab strip above pinned it first — the same list at two
lengths, disagreeing about where Main is. It now uses the strip's comparator.
Archived chats stay included here, unlike the strip: this panel is where the
full history lives, and a retired Main is still a chat you can open.

`test_ent451_new_chat.py::test_no_session_still_resumes_the_latest` asserted the
rule ent#523 reverses. Its original reasoning — a deep link, a refresh and an
API caller that never held a session id all arrive this way, so resume — held
only while there was nowhere designated; "most recent" was a guess, and Main is
the answer that replaces it. Renamed and rewritten to state that, rather than
deleted: a deleted guard leaves no record that a rule was retired on purpose.
What has not changed is asserted too — the branch still resolves to an existing
thread rather than opening one.

Related to Abilityai/trinity-enterprise#523

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WLerYYUUEEmf43UKF2VVRY
… and a late landing

Three findings from the pre-landing review of this branch, plus a stale
docstring.

**C1 — the system line was replayed as the agent's own words.**
`_format_history_context` splits speakers binarily ("Client" for a user row,
"You" for everything else), so Reset's `role='system'` line came back to the
model as something the AGENT said: "You: Main was reset. The previous
conversation is saved as …". Reachable on the first turn after every Reset,
because that turn is cold by construction and therefore always carries the
history prefix. System rows are now skipped — putting words in the agent's
mouth is worse than omitting chrome it did not write. Found by checking the
consumers of the new `role` value outside the diff, which is the one category
where reading only the diff is insufficient.

**C2 — an archived chat is a tab, and I had hidden it.**
The operator ruled it explicitly on 2026-09-06: "one system line in Main names
the archived chat, **which becomes the newest tab**". This branch filtered
archived chats out of the strip, reasoning that Reset would grow it by one
permanent entry per use — solving a problem `OverflowTabs` already solves, by
contradicting a ruling. Restored, with the landing rule left as it was: you can
GO to an archive, you are never PUT in one. Two rules, two questions.

**I1 — a late landing could move you off a chat you had chosen.**
The landing watcher fires on the route param AND on the thread list arriving,
so on a cold deep link two `landOnAgent` calls can be in flight: the first
misses and goes to the network, the second finds the list and navigates. The
first one's late resolution then navigated too. Guarded on the route, which is
the authority — the same staleness check `usePortalAgentPage` already makes.

**I2 — `ensure_thread_for_ask`'s docstring** still described the pre-#523
"reuses the client's latest thread" resolution.

Also re-anchored `test_ent79_portal_exposure::test_portal_chat_feeds_prior_history_as_context`,
which reached its seeded history through "no session_id resumes the latest
thread". That resolution is what this issue changes, so the test now names its
session: its subject is that prior turns are fed back as context, not which
thread gets chosen — which `test_ent523_main_chat.py` owns.

npm run test:unit: 2029 passed. Backend: 156 passed across ent#79 / ent#451 /
ent#473 / ent#523.

Related to Abilityai/trinity-enterprise#523

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WLerYYUUEEmf43UKF2VVRY
@dolho

dolho commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

/review report — pre-landing structural review

Branch: dolho/issue-523 → dev · merge-base 1d5c0422
Files: 40 (+3164 / −785) · Scope: CLEAN, with one requirements deviation found and fixed (C2)
Plan completion: 11 AC done · 0 partial · 0 not done · 3 changed-and-stated · 2 unverifiable (light/dark pass, room drop with two participants — left for the reviewer)

Every finding below was fixed on this branch; the commit is 7a75e0dd.


Critical

[C1] Enum completeness — the system line was replayed to the model as the agent's own words (Confidence: 9/10)
src/backend/client_portal/service.py:1654 (_format_history_context)

who = "Client" if m.get("role") == "user" else "You"

The speaker split is binary, so Reset's new role='system' row fell into the else branch and came back as "You: Main was reset. The previous conversation is saved as …". Not theoretical: the turn after a Reset is cold by construction (fresh row, no cached_claude_session_id), and a cold turn is exactly the one that carries the history prefix — so every Reset was followed by a turn that told the agent it had said something it never said.

Found by 4.12: reading the consumers of a new enum value outside the diff. Three other readers were checked and are safe — _title_plan's opener scan keys on "user", the dedup at service.py:2195 keys on "user", and ratings at service.py:3888 key on "assistant". message_count is bumped by touch_portal_session, not add_portal_message, so the system row also does not inflate it — which is what keeps "an untouched Main is a no-op" reading 0 after a Reset.

Fix: skip role == "system" in the context renderer.

[C2] Requirements deviation — I hid the archived chat the operator ruled must be a tab (Confidence: 10/10)
src/frontend/src/components/portal/portalUtils.js (agentChatTabs)

The ruling, verbatim (ent#523, operator 2026-09-06): "one system line in Main names the archived chat, which becomes the newest tab". This branch filtered archived_at rows out of the strip, on the reasoning that Reset would otherwise grow it by one permanent entry per use.

That reasoning was wrong twice: OverflowTabs already bounds the strip with a counted "N more", so the growth costs nothing visually — and it contradicted an explicit ruling to solve a problem that did not exist. Worse, it hid precisely the thing the system line had just pointed at, in the moment the person is most likely to look for it.

Fix: archived chats are tabs again. landingThread is deliberately left excluding them — you can go to an archive, you are never put in one. Two rules, two questions. Docs and the spec follow.


Informational

[I1] Race — a late landing could move you off a chat you had chosen (Confidence: 8/10)
src/frontend/src/views/Portal.vue (landOnAgent)

The landing watcher fires on activeAgentPageName and on threads.length, so on a cold deep link two calls are in flight: the first misses (no threads yet) and awaits fetchSessions, the second finds the list and navigates. Nothing re-checked the route after the await, so the first one's late resolution navigated too — harmless when it lands on the same chat, wrong when the person had since picked a different one.

Fix: re-check activeAgentPageName.value === name after each await before navigating — the same staleness guard usePortalAgentPage already makes for its own late response.

[I2] Documentation staleness — ensure_thread_for_ask (Confidence: 10/10)
Its docstring still described the pre-#523 resolution ("reuses the client's latest thread"). Rewritten to state that it lands in Main and inherits that without knowing about Main at all.

[I3] Test coupling — an existing test reached its fixture through the rule this PR changes (Confidence: 9/10)
tests/unit/test_ent79_portal_exposure.py::test_portal_chat_feeds_prior_history_as_context seeded history into a thread and called portal_chat with no session id, relying on "no session id resumes the latest thread" to reach it. That resolution is the thing ent#523 replaces, so the test failed — correctly.

It is now re-anchored on an explicit session_id. Its subject is that prior turns are fed back as context; which thread gets chosen is test_ent523_main_chat.py's subject, and a test should not fail for a reason it is not about.


Verified clean

  • SQL & data safety — every new statement is text() with bound parameters (db.py get_main_portal_session_id, archive_main_and_mint, the widened SELECTs). No interpolation, no f-strings in SQL. The migration is additive: two nullable/defaulted columns and one index, no data rewritten, no backfill.
  • Concurrency — the one check-then-act (ensure_main_session) is resolved by the database, not by a check: idx_portal_sessions_main lets exactly one INSERT land and the loser re-reads. archive_main_and_mint is one transaction with the flag cleared before the insert (the index refuses it otherwise) and a WHERE is_main = 1 precondition, so a concurrent Reset yields a named 409 instead of a second Main.
  • Auth boundaries — the new route is Depends(get_portal_principal) with include_owned = principal.is_platform (the ent#358 see/do rule), the service gates on agent_on_roster before touching the table, and every miss is the uniform 404 (Invariant security: implement safe tar extraction with symlink/hardlink validation #8). The UPDATE is additionally scoped to (agent, client). Rate-limited per viewer at 10/60s. main/reset is two path segments and is declared above the {session_id} PATCH neighbour anyway.
  • Credential exposure — nothing new logged; the one new WARNING logs an agent name.
  • Frontend XSS — no v-html added; the system line renders as interpolated text.
  • Performance — the sidebar's cross-agent batch (bug: Agent Detail and Workspace over-fetch on load (21 redundant requests, N+1 roster) #2198) is untouched and deliberately does not mint Main; no per-row fetch was added. The band and the details panel share one payload through usePortalAgentPage, so the split did not double the agent-page read. The band's cached-paint-then-refetch on a chat switch is the ent#253 pattern, not a defect.
  • Error handling — no bare except:; the two broad catches are documented best-effort paths (ensuring Main must not blank a chat list; the system line must not fail a committed Reset) and both log with a stack trace.
  • Product quality bar — new refusals are named 400/409 with a code, not generic 500s; the composer degrades with a label rather than dead-ending; the new columns are defaulted and additive.

Summary

  • Critical: 2 — both fixed on this branch
  • Informational: 3 — all fixed
  • Scope: clean; one deviation from an operator ruling found and reverted to the ruling

Verification after the fixes: npm run test:unit 2029 passed / 90 files; vite build clean; backend 3021 passed, 20 skipped across the whole portal/workspace-adjacent suite (portal|workspace|ask|client|ent1xx–ent5xx). Both ratchets green.

Still on the reviewer: light + dark pass over the band, the details panel and the drop overlay; a room drop with two participants (recipients named on the chip); /verify-local --skip-agent.

dolho and others added 2 commits September 7, 2026 12:11
A new value in a free-text discriminator column lands in the `else` of every
binary split that reads it, and the readers that never mention the new value
are the dangerous ones — a grep for it finds nothing and looks reassuring. The
failure mode is content, not a crash: `role='system'` came back to the model as
words the agent never said.

And: a self-imposed refinement that narrows something a ruling states
positively is a deviation, not a refinement. The tell is a code comment arguing
against a quoted requirement — and the fix here was to notice that an existing
primitive (`OverflowTabs`' counted overflow) already solved the problem the
deviation was defending against.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WLerYYUUEEmf43UKF2VVRY
`/validate-pr` §2.4: a new API endpoint belongs in the endpoint catalogue, and
`POST .../sessions/main/reset` was only described in the workspace architecture
section and the feature flow. Added beside the other client-portal routes, with
the two facts a reader of that table needs to not re-derive: cold is the new
row's property (so there is no second reset primitive to go looking for), and
the two named 409s.

Related to Abilityai/trinity-enterprise#523

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WLerYYUUEEmf43UKF2VVRY
@dolho
dolho marked this pull request as ready for review September 7, 2026 09:19
@dolho dolho closed this Sep 7, 2026
@dolho dolho reopened this Sep 7, 2026
dolho and others added 4 commits September 7, 2026 12:35
ent#468 asked for a decision, render or drop. **Render**, and the reason is that
dropping loses a fact the person is entitled to: on an `operator_resume_enabled`
agent their answer sets real work in motion and spends the owner's budget.
ent#364's AC — "the answer reaches the agent and it resumes" — was true in the
backend and invisible in the product; `resume_requested` and the `answered`
status were on the wire and read by nothing, which is also why ent#430's review
spent a blocker correcting a field with no consumer.

`portalUtils.js::answerConfirmation` consumes BOTH fields, which is the AC's
"either both or neither": `status === 'answered'` is the gate — anything else
means the answer did not land the way this copy would claim — and
`resume_requested` is the wording. The tense follows `_resume_requested`'s own
contract, a report of INTENT rather than a promise of success: "is picking this
up", never "has done it". A failure after that point is an operator-side FAILED
row plus an `operator_resume_dispatch` audit entry. `null` and `false` both say
only "Sent." — reading an absent value as "started" would re-introduce exactly
the over-claim ent#430 removed.

The confirmation cannot live on the ask row, because answering removes it —
which is the AC's third bullet and, structurally, the whole difficulty.
`PortalAsks.visible` gated on `items.length > 0`, so the surface unmounted at
the same instant the confirmation was created; the message would have rendered
for zero frames. It now stays mounted while a confirmation is up, clears itself
after six seconds, and clears its timers on unmount — this surface unmounts on
every chat switch, and a timer that outlived it would write to a dead ref.

Folded into this branch because it lands on the same surface: `PortalAsks`
renders above the composer of the conversation ent#523 rebuilt.

npm run test:unit: 2036 passed (90 files), 7 new. vite build clean.

Fixes Abilityai/trinity-enterprise#468

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WLerYYUUEEmf43UKF2VVRY
# Conflicts:
#	docs/memory/feature-flows.md
…3 (ent#523)

Compared the shipped surface against the design the operator approved on
2026-09-06 and closed five gaps. One of them was a real rendering bug that only
appears once the agent has data.

**The band overflowed onto the tab strip.** `StackedBarChart` renders bars PLUS
a day-label row PLUS a legend — about `height + 38px` — and the band clipped it
into a fixed 52px box, so with any real activity the legend drew on top of the
chat tabs. The row now sizes to the chart. Invisible on an agent with no
executions, which is exactly what the first screenshots had.

**The chart was full-width, so one execution rendered as a slab.** Flexed
across the band a 7-day window gives ~150px columns; A3 draws a compact block
beside the figures. Bounded to 26rem with a spacer holding the window selector
right.

**The legend now sits beside the bars**, as A3 draws it —
`StackedBarChart legend="side"`, an additive prop defaulting to today's
below-the-bars layout so no existing caller changes. Same markup as the
existing block, so the two cannot disagree about what they say.

**Agent details moved from a text link in the band to an info control in the
header**, where A3 puts it, beside the other per-conversation actions. The band
is numbers; this is the door to everything else about the agent.

**Main carries the bookmark A3 draws on it** — `OverflowTabs` gains an optional
`pinned`, rendered in all THREE sites including the hidden mirror row: a glyph
the visible row draws and the measuring row does not is a tab measured narrower
than it renders, which makes the strip overflow one tab too late. The remeasure
key includes it.

**The agent row carries the timestamp A3 shows** (`now` · `12m` · `2d` ·
`Aug 21`). Deliberately a second format rather than reusing `relativeTime`: this
column is a few characters wide beside a name and a preview, so it drops the
"ago" its position already implies. Two jobs, two formats.

Also names the chart ("Activity · last 7 days") instead of leaving a bare plot
beside a row of numbers.

npm run test:unit: 2055 passed (92 files), 7 new pinning the A3 rules — incl.
that the pin is drawn in the mirror row, since that one is invisible until a
strip overflows wrongly. vite build clean. Raw-color vs origin/dev: no file grew
`raw_nongray` or `hardcoded_colors`; gray chrome only.

Related to Abilityai/trinity-enterprise#523

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WLerYYUUEEmf43UKF2VVRY
… row (ent#523 re-review)

Found re-reviewing my own board-A3 commit. `agentPreview` and `agentRowTime`
each scan the whole thread list, and the template called them per row and TWICE
each — a `v-if` and an interpolation — so a 10-agent roster over 200 threads did
~8k iterations per render, on a surface that re-renders on every store tick.

`agentRowMeta` computes both for every agent in one pass and the sidebar
memoizes it, so the cost is O(threads + agents) rather than O(rows × threads ×
4). Same rules and same output — pinned by a test that asserts the map agrees
with the two per-agent helpers it replaced, which stay exported and tested so
the "which chat is newest" rule has one definition. The tight time format is
split into a shared `compactAge` so the two callers cannot drift on what "2d"
means.

Also prunes a fired timer's handle from `PortalAsks`' list instead of letting it
accumulate for the life of the mount.

npm run test:unit: 2059 passed (92 files), 4 new.

Related to Abilityai/trinity-enterprise#523

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WLerYYUUEEmf43UKF2VVRY
@dolho

dolho commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

/review — second pass (the delta since the first review)

Scope: 88056c3d..a9184e7d — ent#468, the dev merge, and the board-A3 alignment. The first pass covered the original diff; this one covers what landed after it, plus a check that its own fixes survived.

Merge-base moved. After merging dev in, the merge-base is dev's tip, so this diff is 46 files that are actually mine — not the 52 a stale merge-base showed, which included four of dev's own commits.

The first pass's fixes are still in place

  • C1 _format_history_context still skips role == "system" — service.py:1663
  • C2 archived_at appears only in landingThread, not in agentChatTabs — archived chats are tabs, as ruled
  • I1 three route-staleness guards in landOnAgent

Findings

[P1] Performance: the sidebar did four whole-list scans per row (Confidence: 9/10) — fixed
src/frontend/src/components/portal/PortalSidebar.vue

agentPreview and agentRowTime each scan the entire thread list, and the template called them per row and twice each — a v-if and an interpolation:

<span v-if="previewFor(a.name)" …>{{ previewFor(a.name) }}</span>
<span v-if="rowTime(a.name)" …>{{ rowTime(a.name) }}</span>

A 10-agent roster over 200 threads is ~8k iterations per render, on a surface that re-renders on every store tick. Mine, introduced in the A3 commit and in the sidebar-preview commit before it.

Fix: agentRowMeta computes both for every agent in one pass, memoized in a computed — O(threads + agents) instead of O(rows × threads × 4). The two per-agent helpers stay exported and tested, and a new test asserts the map agrees with them, so "which chat is newest" keeps one definition. The tight time format is split into a shared compactAge so the two callers cannot drift on what 2d means.

[P2] A fired timer's handle accumulated for the life of the mount (Confidence: 7/10) — fixed
PortalAsks.vue. Minor, bounded by answers-per-mount, but onBeforeUnmount was then clearing a pile of expired ids. The handle is now dropped when the timer fires.


Verified, with the line that proves it

The riskiest assumption in ent#468 — checked against the server, not against my own literal. answerConfirmation gates on status === 'answered'. A wrong literal would make the whole feature silently dead and my tests would still pass, since they use the same constant. The projection maps it (asks/service.py:56,73):

_ANSWERED_STATUSES = frozenset({"responded", "acknowledged"})
...
if (item.get("status") or "") in _ANSWERED_STATUSES:
    return "answered"

So the gate matches the contract, and it correctly treats the operator-side acknowledged terminal as answered too.

Blast radius of the two shared primitives is bounded, and I checked rather than assumed.

  • StackedBarChart — three callers (OverviewPanel, TrendLineChart, the band); grep 'legend=' finds it passed only by the band. Every other caller takes the 'below' default and renders exactly as before. The new wrapper div is a plain block in default mode, so block stacking is unchanged.
  • OverflowTabs — five callers; only PortalChatTabs' Main sets pinned, and an unset pinned renders nothing. The remeasure key includes it, and the glyph is in all three render sites including the hidden mirror row — a glyph the visible row draws and the measuring row does not is a tab measured narrower than it renders, which overflows the strip one tab too late.

The band overflow was a real bug and is fixed at the cause. StackedBarChart is height + ~38px (bars + day labels + legend) and the band clipped it into a fixed 52px box, so with any real activity the legend drew on top of the chat tabs. It was invisible in the first screenshots because that agent had zero executions. The row now sizes to the chart, and legend="side" is what keeps that from doubling the band's height.

Clean categories (nothing found): SQL/data safety and concurrency — no backend change in this delta beyond the two-line history-context skip; auth boundaries — no new route or gate; credential exposure — none; XSS — no v-html added, the confirmation is interpolated text; error handling — no new swallow; enum completeness — role='system' consumers were swept in the first pass, and the new status/resume_requested reads are the first consumers of fields that had none.

Summary

Verification: npm run test:unit 2059 passed / 92 files (11 new since the first pass); vite build clean; backend 172 passed across ent#79 / ent#451 / ent#473 / ent#523 plus dev's new test_2110_widget_type_parity.py. Raw-color vs origin/dev: no file grew raw_nongray or hardcoded_colors; gray chrome only.

CI: 15 checks green including journey-impact, schema-parity, pg-migrations, gitleaks, verify-non-root and the compose/docs guards. The pytest seed matrix, e2e, CodeQL and the image smokes are still running.

Still on the reviewer: light + dark over the band, details panel and drop overlay; a room drop with two participants; /verify-local --skip-agent. Both cross-tracker issues need a manual status-in-dev at merge — the automation is same-repo only.

# Conflicts:
#	docs/memory/learnings.md
#	src/backend/db/migrations.py
# Conflicts:
#	docs/memory/learnings.md
#	src/frontend/src/components/StackedBarChart.vue
@dolho
dolho requested a review from vybe September 7, 2026 11:08
…he dev merge

The revision was rebased from 0053 to 0054 when #2561 landed 0053_user_ui_preferences
on the same parent — two revisions sharing a down_revision is two heads, and
`alembic upgrade head` then applies ZERO revisions. The three docs still named 0053.

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

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated via /validate-pr — APPROVE.

  • Both migration tracks present (SQLite portal_session_main_chat + Alembic 0054), heads check on the branch resolves to one head; schema-parity and pg-migrations green.
  • Re-ran locally on the branch head: backend test_ent523_main_chat + ent451/ent79/2196/2198 neighbours — 161 passed; vitest run (both ratchets) — 95 files, 2147 passed.
  • Security greps clean; no compose/Dockerfile/getenv packaging surface.
  • Doc drift on the Alembic revision number was already corrected in 95e4203.

Cross-tracker refs do not auto-promote — setting status-in-dev on ent#523, ent#524, ent#468 by hand at merge.

…o 0055 off 0054_agent_canvases_template

dev landed 0054_agent_canvases_template (ent#537) off 0053 while this branch
carried its own 0054 off 0053: two heads, the #2068 class, which
`alembic upgrade head` answers by applying nothing. The portal revision is
the one not yet applied anywhere, so it moves: 0055, chained off the canvas
revision. Docs, the SQLite mirror note and the test that pins the parent
id follow it. Both `migrations.py` entries and both learnings blocks kept.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LaBkiyfRkYkmk4iMJkHRdL
@vybe
vybe enabled auto-merge (squash) September 7, 2026 14:58
@vybe
vybe merged commit 9a1dfa7 into dev Sep 7, 2026
28 checks passed
vybe added a commit that referenced this pull request Sep 7, 2026
…sion off 0055_portal_session_main_chat as 0056

dev renamed the Main-chat revision 0054 → 0055 (#2558 landed after #2561's
0053 and #2566's 0054), so this branch's 0055_schedule_workspace_delivery
chained off a parent that no longer exists and the merge left a stale
0054_portal_session_main_chat.py behind — two heads, zero revisions applied.
Renumbered to 0056 off 0055_portal_session_main_chat, stale file dropped,
docs and the ent#498 revision-pin test updated. check_alembic_heads: 1 head.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LaBkiyfRkYkmk4iMJkHRdL
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants