Skip to content

feat(tandem): the layer on top of Main — brief delivery, the user-facing room signal, report-a-problem (abilityai/trinity-enterprise#498, abilityai/trinity-enterprise#363, abilityai/trinity-enterprise#499) - #2568

Merged
vybe merged 27 commits into
devfrom
dolho/issue-498-tandem
Sep 7, 2026

Conversation

@dolho

@dolho dolho commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Journey Impact

Journey Impact: new: J11

A companion's brief reaches me where I already work, without me asking. Every other
journey is a promise the user reaches for; J11 is the first that reaches for them. J05
("I can schedule work, walk away, and find out what happened") ends at the executions list
— the operator's surface — so it does not cover this. Catalog row + regenerated
JOURNEYS.md + a strict=True xfail skeleton are in the diff; the harness is
#2565.

Summary

The Tandem layer (abilityai/trinity-enterprise#497), stacked on Main (ent#523). Three small
features that only make sense once every (user, agent) pair has one constant chat.

Based on dolho/issue-523 (PR #2558), which must merge first.

ent#498 — the brief lands in Main

A schedule names one Workspace user (agent_schedules.deliver_to_workspace_email); when it
fires, the output lands as a turn in that person's Main chat with the agent. API/MCP
only this cut — the schedule form is untouched.

Almost all of it already existed: report_completion is trigger-agnostic and schedule is
deliberately not in INLINE_CHANNEL_TRIGGERS, because a scheduled run has no surface
that already answered. The only missing fact was that a scheduled execution row never
carried source_channel='portal'. So this is a stamp and nothing else — no new delivery
path, no second applier, no terminal writer changed.

The scheduler carries the address only, for two independent reasons: it cannot import
the portal package to resolve a session, and it always sends execution_id, so
execute_task's channel-persisting branch can never run for a cron fire and channel columns
passed as kwargs would be silently inert (the #2426 class). execute_task_internal
resolves Main and stamps the pre-created row before dispatch.

  • stamp_execution_channel_context is the first UPDATE of those columns — every other
    writer sets them at INSERT. Guarded on source_channel IS NULL, so it only ever adds
    a destination and can never repoint an inbound channel turn whose adapter is waiting on
    that reply.
  • Access is checked against where the message will land — agent_on_roster(..., include_owned=True), the Workspace's own roster — not email_has_agent_access, which
    admits any admin; an admin who neither owns the agent nor is shared it cannot open that
    thread, so a brief delivered there would be invisible.
  • Every refusal (blocked client, unreachable address, unreadable roster → fail closed,
    unavailable session, already-stamped row) fails the pre-created row with a named reason
    and releases the idempotency claim. A visible failure, never a silent no-op — running
    the turn anyway spends the tokens and puts the answer where nobody can read it.
  • C9 — the portal leg now waits, bounded, on the ent#286 in-flight marker and then
    writes regardless. PortalConversation detects a reply by an assistant-row count
    delta, so a report landing mid-turn can be read as that turn's answer; deferring removes
    that for the common case, while a wait that could refuse would trade a cosmetic misread
    for a lost brief. Applies to every portal report, not only scheduled ones.
  • At-most-once per fire is inherited, not rebuilt: the effect guard is keyed on the
    execution id and a fire is one execution.

ent#363 — the user-facing room signal

An agent woken in a room containing a human gets an explicit signal that a person outside
the fleet reads every line, plus guidance on what that changes. Filed theme-security, and
it is one: full transcript visibility is only safe while the agents know they are watched.

  • Derived from membership (room_is_user_facing), never asserted by a participant.
    NON_HUMAN_PARTICIPANT_KINDS is the complement of "human" on purpose — a kind added later
    (ent#171's external A2A sender) is likelier to be a person than a machine, and an
    allow-list of human kinds would silently classify it as fleet-internal.
  • The block names no participant: it is handed to every woken agent, so an address would
    be disclosed sideways to agents that person never addressed.
  • Derived per wake rather than threaded from post_message (which does hold the list) —
    _wake_agent calls post_message back, so a threaded value could go stale when a reply
    recruits a human.
  • An unreadable roster assumes a person is reading — the inverse of the usual capability
    default, because the mistakes are not symmetrical.
  • The turn header stops claiming "Other agents and people are in this room" unconditionally.

ent#499 — report-a-problem

A thumbs-down raises a rate-bounded operator_queue item naming the agent, the person, what
was rated and their comment. ent#366's redaction is untouched: the operator sees the words,
the rated agent still does not.

Two live bugs fixed on the way

Operator-queue items of an unrecognised type could never be closed. type is free TEXT
and the platform has emitted skill_not_found since #1410, but QueueCard.vue and
QueueItemDetail.vue each hardcoded an approval → question → alert chain and rendered no
control
for anything else. For a budgeted type, five such items jam the pending cap
permanently — so ent#499 could not ship without this. Both cards now consume
utils/operatorQueue.js::queueResponseKind (the module that already declares itself the one
home of the controls-kind switch), whose unknown-type default moves from question to
acknowledge. An approval without options still gets a text box.

A test that has never run. test_ent362_workspace_participant.py's (human) assertion
called _render_transcript — a name that has never existed; the renderer is _format_delta
— behind a hasattr guard that skipped silently, leaving the exact labelling ent#363 builds
on unprotected. It now calls the real function and asserts the label discriminates.

Documentation

  • requirements/scheduling.md §10.18; requirements/core-agent.md §5.24, §5.25, §5.26
  • New flow feature-flows/schedule-workspace-delivery.md + the flows index
  • architecture/workspace.md (a Tandem section) and architecture/database.md (the DDL)

Testing

Upgrade note for anyone who ran #2558 before its dev merge

0053_portal_session_main_chat was renamed 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 is singular, so a forked
graph applies zero revisions.

A database stamped with the old id crash-loops the backend on boot:

alembic.util.exc.CommandError: Can't locate revision identified by '0053_portal_session_main_chat'

The DDL the two revisions apply is identical, so the marker is re-stamped, not the
data:

UPDATE alembic_version SET version_num='0054_portal_session_main_chat'
 WHERE version_num='0053_portal_session_main_chat';

This affects local branch checkouts only — 0053_portal_session_main_chat
never reached dev or main, so no deployed instance can hold it. Hit on my own
dev stack while bringing it onto this branch.

Not in this cut — stated so it is not inferred from the merge

  • No schedule-form UI for the delivery target (API/MCP only, by decision).
  • Rooms are not a delivery destination, and there is no agent-initiated post_to_room —
    both remain trinity-enterprise#442.
  • Delivery is durable, not live-pushed: the Workspace does not poll thread history, so a
    brief appears on the next load or thread switch. Acceptable at daily cadence.
  • create_item has no UPDATE path, so an edited rating comment does not reach an item
    already raised — the same residual ent#434's alert carries; the fix belongs at the sink.

Fixes abilityai/trinity-enterprise#498
Fixes abilityai/trinity-enterprise#363
Fixes abilityai/trinity-enterprise#499
Related to #2565

🤖 Generated with Claude Code

https://claude.ai/code/session_01WLerYYUUEEmf43UKF2VVRY

dolho and others added 23 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
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
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
# Conflicts:
#	docs/memory/learnings.md
#	src/backend/db/migrations.py
# Conflicts:
#	docs/memory/learnings.md
#	src/frontend/src/components/StackedBarChart.vue
…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
…already work (#2565)

Gate G1 makes a `new:` journey declaration load-bearing: the PR that declares
one owes a skeleton in the journey tier. This is the record half — the catalog
row, the regenerated JOURNEYS.md, and the count the density assertion checks.

The fixed J01..J10 list becomes a dense J01..JNN run with the count named as
DECLARED_JOURNEY_COUNT, so growing the catalog is a deliberate one-line act in
the same commit rather than a test nobody can add a promise past.

J11 is genuinely new rather than an extension of J05: J05 ends at the executions
list — the operator surface — and every other journey is something the user
reaches for. This is the first one that reaches for them.

Related to Abilityai/trinity-enterprise#498

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

Full transcript visibility is the deliberate choice for Workspace rooms — watching
the team work is the differentiator over a summary — and that choice is only safe
while the agents know they are being watched. Without a signal, agent-to-agent
messages in a user-facing room discuss internals, other customers, costs and
platform mechanics in front of the customer.

What existed was not enough. ent#362 labels each transcript LINE whose sender is
human with '(human)', so an agent woken into a room where the person is reading
silently sees agents talking to agents and nothing else. The room header then
said 'Other agents and people are in this room' unconditionally — false in an
agent-only room, and scene-setting rather than a disclosure in a user-facing one.

The signal is derived from MEMBERSHIP and nothing a participant writes reaches it
(AC 2). `room_is_user_facing` is pure and is the one place the question is
answered; `NON_HUMAN_PARTICIPANT_KINDS` is written as the complement of 'human'
because a kind added later — ent#171's external A2A sender is already anticipated
in count_budget_messages — is likelier to be a person than a machine, and an
allow-list of human kinds would classify it as fleet-internal.

The block names no participant: it is composed into a prompt handed to every
woken agent, so an address would be disclosed sideways to agents that person
never addressed, and it buys nothing since the behaviour change is the same
whoever is reading.

Derived per wake rather than threaded from post_message (which does hold the
list): _wake_agent calls post_message back with the reply and that wakes the next
agent, so a threaded value would have to survive a round trip through a public
function and could go stale when a reply recruits a human.

An unreadable roster assumes a person IS reading — the inverse of the usual
capability default, because the mistakes are not symmetrical: needless caution
costs a more careful answer, a missed signal is the disclosure.

Also fixes a test that has never run: test_ent362's '(human)' assertion called
_render_transcript, a name that has never existed (the renderer is _format_delta),
behind a hasattr guard that skipped silently — leaving the exact labelling this
feature builds on unprotected. Now calls the real function and asserts the label
discriminates.

Related to Abilityai/trinity-enterprise#363

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

ent#366 records the rating and, on down+comment, hands the words to the agent's
own capture-feedback skill. Nothing reached the person who runs the instance.

The rated agent is deliberately not in this loop: a readable score is a loop an
agent may optimise for, and a stranger's verbatim words handed to the thing being
criticised is a prompt-injection path into it. The operator's copy goes straight
to the queue and the agent-facing redaction (comment_withheld) is untouched — the
operator sees the comment, the agent still does not.

ROUTED through create_bounded_alert, never a direct create. The volume is driven
by a client clicking, which is the agent-influenceable side of the #1677
classification; a direct create_operator_queue_item would fail the CI emitter
guard. It gets its own registered type rather than the generic 'alert', because
the budget counts pending rows OF THAT TYPE including ones other emitters wrote —
reusing 'alert' would let five unrelated alerts silence every problem report. The
id prefix is reserved so an agent cannot pre-create the id of a complaint about
itself and swallow it through ON CONFLICT, and the id is hashed rather than
interpolated because an email is not confined to the sink validator's alphabet.

The operator's copy fires on EVERY thumbs-down, not only one carrying a comment:
'this was not useful' is the report and the words are the elaboration, so gating
on them would mean the quietest complaints — a bare thumb, which is what most
people leave — reach nobody.

PREREQUISITE, and a live bug found while building it: operator_queue.type is free
TEXT and the platform has emitted non-protocol types since #1410, but QueueCard
and QueueItemDetail each carried their own approval-question-alert v-if chain and
rendered NO control for anything else — so skill_not_found items have never been
closeable from the queue, and for a BUDGETED type five of them jam the pending cap
forever. Both cards now consume utils/operatorQueue.js::queueResponseKind, the
module that already declares itself the one home of the controls-kind switch, and
its unknown-type default moves from 'question' to 'acknowledge': an unrecognised
item is informational, a freeform box invites a reply that goes nowhere, and under
ent#329 answering can spend a turn. An approval without options still gets a box —
there the operator has a real decision to express.

Stated residual: create_item has no UPDATE path, so an edited comment does not
reach an item already raised. Same residual ent#434's alert carries; the fix is at
the sink, not per-emitter — a comment-dependent id would trade one bounded item
per person for one per keystroke-set.

Related to Abilityai/trinity-enterprise#499

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

A role companion's daily brief is a scheduled playbook call whose output has to
reach one primary human where they already work. Until now every scheduled run
terminated in an execution row — the operator's surface — and a person who is not
the operator never saw it.

Almost all of this already existed. channel_completion_report has resolved,
persisted and effect-guarded a portal-bound completion since ent#457, and
report_completion is trigger-agnostic: `schedule` is deliberately NOT in
INLINE_CHANNEL_TRIGGERS, because a scheduled run has no surface that already
answered. The only missing fact was that a scheduled execution row never carried
source_channel='portal'. So this is a STAMP and nothing else — no new delivery
path, no second applier, no terminal writer changed.

The scheduler carries the ADDRESS only, for two independent reasons: it is a
separate process that cannot import the portal package to resolve a session, and
it always sends execution_id, so execute_task's channel-persisting branch can
never run for a cron fire and channel columns passed as kwargs would be silently
inert (the #2426 class). execute_task_internal resolves Main and stamps the
pre-created row before dispatch.

stamp_execution_channel_context is the first UPDATE of those columns — every
other writer sets them at INSERT, which is why no updater existed. Guarded on
source_channel IS NULL and returns whether it landed, so it only ever ADDS a
destination: a row that already has one belongs to an inbound channel turn whose
adapter is waiting on that reply.

Access is checked against where the message will LAND —
agent_on_roster(..., include_owned=True), the Workspace's own roster — not
email_has_agent_access, which admits any admin; an admin who neither owns the
agent nor is shared it cannot open that thread, so a brief delivered there would
be invisible. An unreachable address, a blocked client, an unreadable roster
(fail closed) or an already-stamped row all REFUSE with a named reason, fail the
pre-created row and release the idempotency claim: a visible failure, never a
silent no-op. Running the turn anyway would spend the tokens and put the answer
where nobody can read it.

C9: the portal delivery leg now waits, bounded, on the ent#286 in-flight marker
and then writes regardless. PortalConversation detects a reply by an
assistant-row count delta and renders the last assistant row, so a report landing
mid-turn can be read as that turn's answer; deferring removes that for the common
case while a wait that could REFUSE would trade a cosmetic misread for a lost
brief. Applies to every portal report, not only scheduled ones.

At-most-once per fire is inherited, not rebuilt: report_completion's effect_guard
is keyed on the execution id and a fire is one execution.

Schema on both tracks (Invariant #3): SQLite schedule_workspace_delivery, Alembic
0055 off 0054_portal_session_main_chat. Nullable, no backfill, no index.
_fail_execution_row is allowlisted in the #1804 parity guard as an admission-path
terminal (the refusal precedes execute_task, so no dispatch activity exists), and
the stamp is registered in _EXPECTED_UPDATE_SITES as a non-status writer.

Journey J11 is declared with a strict=True xfail skeleton (#2565).

Related to Abilityai/trinity-enterprise#498

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

The backend unit suite's regression diff caught this: _fail_execution_row
persists an error string via update_execution_status without passing it through
runtime_secret_scrub, which test_ent279_scrub_parity fails on. Real, and my local
suite never ran to catch it.

Scrubbed rather than allowlisted, though the allowlist would have been defensible
— today's only caller passes a platform-composed refusal built from the
schedule's target address and the agent name, before execute_task, which is the
same shape the guard already allowlists for _admission_gate. The signature takes
an arbitrary error: str, and an allowlist entry is pinned to a FUNCTION NAME, so
it would silently extend the exemption to a future caller that does pass agent
output. The seam fails open with a [] fast path.

Related to Abilityai/trinity-enterprise#498

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

Two acceptance criteria I had marked done on reasoning rather than evidence.

ent#498 AC 6 says the not-live-pushed caveat is 'stated in the user docs'. It was
stated in requirements, the flow doc and the PR — none of which a person setting
up a schedule reads. docs/user-docs/automation/scheduling.md now carries the
field, where the message lands, who may be named, that an unreachable address
fails the run rather than delivering nowhere, and that it appears on the next
Workspace load rather than mid-conversation.

AC 7 (the delivered message is rateable like any agent message) was claimed 'by
construction' — and construction is exactly what a later edit changes.
_rating_target_is_visible demands an agent match, a client match and
role == 'assistant'; the delivery leg satisfies all three from the SESSION row.
Pinned from both ends: the predicate accepts the delivered shape and rejects a
system-role one (so the test discriminates), and a source assertion that the
delivery leg still writes an assistant row addressed from the session.

Related to Abilityai/trinity-enterprise#498

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

**The complaint travelled back to the agent it was about.** ent#499's docstring
promised "the operator sees the comment; the agent still does not" — and two
pre-existing return paths falsified it, both keyed on agent_name alone:
operator_queue_service._write_responses_to_agent copies a RESPONDED item's
`question` and `context` verbatim into the agent's own ~/.trinity/operator-queue.json,
and routers/operator_queue's respond hook spawns the ent#329 resume dispatch whose
prompt embeds item["question"]. So an operator clicking "Got it" handed the rated
agent the client's address and verbatim words within one 5s sync cycle, and on an
agent with operator_resume_enabled spent one of its turns doing it — the ent#366
disclosure this feature exists to avoid, plus an untrusted-text-into-prompt path.

Both sinks exist to close a loop the AGENT opened: it parked a question, a human
answered, the answer goes back. A platform alarm opened no such loop, and for a
problem report the agent is the SUBJECT. One shared predicate, is_platform_minted,
keyed on the reserved id prefixes that already mark "an agent may not mint this",
gates both so they cannot drift. It discriminates: an agent-authored item still
round-trips, or every parked question would go dead.

Also narrowed the alert's `context` to identifiers — no comment text, no address.
The operator reads who is unhappy from `question`; `context` is the field most
likely to be forwarded or logged.

**ent#363 fired in every room, including an operator's own.** The ticket says
"a room containing a workspace user"; I generalised it to "any non-agent kind",
which swept in the platform `user`. Since create_room always seats its creator and
the only removal path is kind="agent", a human can never leave — so every room was
client-facing, the quiet branch was unreachable, and an ops room's agents were told
to keep infrastructure, costs and queue plumbing out of it, which is the subject
those rooms exist for. FLEET_INTERNAL_PARTICIPANT_KINDS now names agent, system and
user; an unrecognised kind still counts as a reader, which was the right half.

I had shipped a test asserting the generalisation, so nothing could catch it — the
same guard-rail-pointing-the-wrong-way shape recorded for ent#523. That test is
inverted and joined by one that drives the participant shape create_room really
produces, since every existing test fed synthetic lists.

Related to Abilityai/trinity-enterprise#499
Related to Abilityai/trinity-enterprise#363

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WLerYYUUEEmf43UKF2VVRY
…nsumers, and generalising a requirement's noun

Related to Abilityai/trinity-enterprise#499
Related to Abilityai/trinity-enterprise#363

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 — #2568 (self-review + independent adversarial pass)

I wrote this code, so a self-review has an obvious blind spot. I ran an independent
adversarial pass over the merge-base diff alongside my own and reconciled the two.
It found three real defects, two of them disclosures. All three are fixed in
48a2f5c4; I verified each myself before acting rather than taking the finding at
face value.

Scope: 43 files, +2245/−34 against merge-base 95e42030.


[C1] The complaint travelled back to the agent it was about — FIXED

raise_problem_report's docstring promised "the operator sees the comment; the
agent still does not"
. Two pre-existing consumers of operator_queue
falsified it, and neither sits anywhere near the emitter:

# services/operator_queue_service.py — _write_responses_to_agent
for resp in responded_items:
    requests.append({..., "question": resp.get("question", ""),
                          "context": resp.get("context"), ...})
# db/operator_queue.py — get_responded_items_for_agent
and_(operator_queue.c.agent_name == agent_name,
     operator_queue.c.status == "responded")      # ← no origin filter

So: operator clicks Got it → status becomes responded → within one 5s sync
cycle the item's question (the client's address and verbatim words) is written
into ~/.trinity/operator-queue.json on the rated agent, which reads that file
routinely. And separately:

# routers/operator_queue.py
if item:
    operator_resume_service.spawn_resume_dispatch(item, ...)
# services/operator_resume_service.py
f"Question: {item.get('question') or item.get('title') or '(none recorded)'}",

"Got it" posts to /respond like any other answer, so on an agent with
operator_resume_enabled (ent#329) acknowledging spends one of that agent's
turns
on a prompt containing the complaint. Both the ent#366 disclosure this
feature exists to avoid, and an untrusted-text-into-prompt path.

It also falsified my own comment in operatorQueue.js, which argued acknowledge
was safe because "under ent#329 an answer can spend a turn" — acknowledge is
an answer.

Fix, at the sink rather than the emitter. Both paths exist to close a loop the
agent opened: it parked a question, a human answered, the answer returns. A
platform alarm opened no such loop, and for a problem report the agent is the
subject. One shared predicate, is_platform_minted, keyed on the reserved id
prefixes that already mean "an agent may not mint this" (#1632), gates both so they
cannot drift. It discriminates — an agent-authored item still round-trips, or every
parked question would silently go dead — and that discrimination is tested.

skill_not_found has ridden these same paths for months without harm because its
body is platform text about the agent's own missing skill. This is the first
budgeted type whose body is third-party PII, which is what made the class bite.

Also narrowed the alert's context to identifiers — no comment text, no address.
The operator reads who is unhappy from question; context is the field most
likely to be forwarded or logged.

[C2] ent#363 fired in every room, including an operator's own — FIXED

The ticket says, three times, "a room containing a workspace user". I
generalised it to "any non-agent kind", with a comment arguing the complement was
safer for future kinds. Half right — an unrecognised kind is likelier to be an
outside person — but it swept in the platform user, i.e. the operator.

That is not cosmetic, because the generalisation is unfalsifiable in production:

creator_kind, creator_identity = _caller(current_user)   # create_room, always seats them
removed = db.remove_participant(room_id, "agent", agent_name, ...)  # the ONLY removal path

A human participant can never leave. So every room was client-facing, the
else branch was unreachable, and an operator's own ops room got its agents told
to "keep platform mechanics out of it — infrastructure, container and model
internals, costs and token spend, queue and scheduling plumbing"
— the subject
those rooms exist for.

And nothing could catch it: I had shipped test_a_platform_user_makes_the_room_user_facing
asserting the generalisation, and every other test fed room_is_user_facing
synthetic dicts rather than the shape create_room emits. That test is now
inverted and joined by one driving the real constructor's shape.

This is the second instance of this shape in one session — an implementation
changing a stated requirement, with a comment justifying it and a test pinning it.
The first was ent#523's archived-tab rule. Logged as a class in learnings.md.

[C3] The refusal terminal skipped the secret-scrub seam — FIXED earlier (9edb2d63)

Caught not by review but by the suite, and only because I dispatched it manually:
test_ent279_scrub_parity went red on head across all three seeds while every
other check stayed green. _fail_execution_row persisted an error via
update_execution_status without passing it through runtime_secret_scrub.

Scrubbed rather than allowlisted, though an allowlist entry would have been
defensible (the only caller passes a platform-composed string, pre-dispatch — the
shape the guard already allowlists for _admission_gate). An allowlist entry is
pinned to a function name, and that signature takes an arbitrary error: str,
so it would silently extend the exemption to a future caller that does pass agent
output.


Findings I checked and dismissed

  • literal_eval-style narrow except — not applicable here, but the same
    instinct: I nearly filed the queueResponseKind default change as risky before
    enumerating its consumers. All four (QueueCard, QueueItemDetail,
    MobileAdmin ×2, PortalAsks) still reach every branch; PortalAsks cannot see
    an unknown kind at all (client_portal/asks/service.py filters to
    ("question","approval","alert")), and buildQueueResponse is untouched.
  • The stamp is not a TOCTOU. stamp_execution_channel_context is a single
    UPDATE … WHERE id = :id AND source_channel IS NULL returning rowcount > 0 — a
    real CAS — and the caller refuses on a lost race rather than assuming.
  • The recipient check cannot disagree with the stamp. source_channel_client
    and source_channel_chat_id both derive from the same normalised address, and
    both create_portal_session and get_main_portal_session_id lower-case it, so
    _norm_email(context_client) != _norm_email(client_email) cannot fire on a case
    difference.
  • The bounded in-flight wait is correctly scoped. report_completion has one
    caller, spawn_completion_report, reached only from three spawn_* sites — no
    request-serving coroutine awaits it.

Known residuals, stated rather than buried

  • resolve_and_stamp runs synchronous DB calls on the event loop. It widens an
    exposure the same handler already has (idempotency_service.begin writes
    synchronously two lines above), and the canonical use case fires at ~03:30 UTC
    when db_backup_service holds SQLite's lock. Worth a follow-up; not worth
    blocking on, since it is pre-existing on this path.
  • A cancellation inside the ≤120s in-flight wait loses that report.
    CancelledError is a BaseException, so except Exception does not catch it and
    nothing re-applies the terminal. My comment claims "never dropped, only deferred"
    — true of the timeout path, not of a restart landing in the window.
  • A resolved complaint suppresses later ones about the same target.
    ON CONFLICT DO NOTHING ignores the existing row's status, so once acknowledged
    that person cannot raise another report about that target. Same shape as the
    documented edited-comment residual; the fix belongs at the sink.
  • A shared (non-owner) user can schedule a delivery into another roster member's
    Workspace.
    Schedule creation is assert_agent_access, not owner-only, and
    allow_proactive is not consulted. Bounded by roster co-membership, so not
    cross-tenant — but it is a third party the ent#457 "no third party to protect"
    reasoning did not anticipate. Worth a decision before this reaches a customer.

Acceptance criteria

All three tickets' ACs are met. Two I had marked done on reasoning and have since
closed with evidence: ent#498's "stated in the user docs" (now in
docs/user-docs/automation/scheduling.md) and "rateable like any agent message"
(now pinned from both ends, including a discriminating negative case).

Two deliberate deviations, both stated in the PR body rather than inferred:
ent#499's alert fires on every thumbs-down rather than only commented ones, and
the item does not carry the "acted on at the next wake-up" wording, because ent#329
has shipped and is per-agent opt-in.

Behavioural proof of C2, on a live stack

Before the fix, in an operator's own room (creator = a platform user, plus one
agent), asking the agent about token spend produced hedging — it had been told to
keep costs and platform internals out of the conversation. After it:

"Rough estimate: my last response was around 150–200 output tokens. The input
context — system prompt, CLAUDE.md, conversation history, all the system
reminders — was probably 3,000–5,000 tokens."

Direct, in the room where that is exactly what the operator wanted. The signal now
fires only where a workspace client is present.

Verification

Backend pytest matrix green on head across all three seeds with the regression
diff clean on the run covering the scrub fix; the run covering the three fixes
above is in flight as I post this and I will report it below; 361 passed locally across every affected suite; frontend 96 files /
2158 tests; vite build clean; tsc --noEmit clean on the MCP server. All three
features exercised end-to-end on a live stack against PostgreSQL, including the new
Alembic revision applying to a real database.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WLerYYUUEEmf43UKF2VVRY

dolho and others added 3 commits September 7, 2026 16:35
**A shared user could deliver into a colleague's Workspace.** Schedule creation is
assert_agent_access — owner OR shared OR admin — and the delivery target may be any
roster member, so a merely-shared user could schedule a recurring message with a
prompt of their choosing into someone else's Main chat, rendered as an ordinary turn
from the agent. ent#457's "no third party for allow_proactive to protect" does not
carry over: ent#498's author need not be its recipient. Rule: address yourself
freely, address anyone else only as the owner. Enforced on create AND update
(create-only is a formality — create without the field, PUT it after), fail-closed
on an unreadable ownership read, and clearing the target is never privileged.

**A cancellation inside the in-flight wait lost the report.** CancelledError is a
BaseException, so `except Exception` did not catch it: a restart landing in the
≤120s window unwound the effect guard with the terminal already applied and nothing
to re-apply it. The docstring's "never dropped, only deferred" was true of the
timeout path only. Now caught explicitly and the report is written immediately — a
report beside an in-flight turn is a cosmetic misread, one that never lands is the
failure the contract exists to prevent.

**A resolved complaint suppressed every later one about the same target.**
create_item's ON CONFLICT ignores the existing row's status, so once acknowledged
that person could never raise another report about that target — silently, forever,
which is worse than a duplicate. The id is now quantised to the UTC day (ent#434's
bucketing, same reason): one item per person per target per day, and tomorrow gets
through.

**Two synchronous DB paths on the event loop.** resolve_and_stamp makes 4–6
synchronous SQLAlchemy calls including two writes, and the in-flight marker read is
a synchronous Redis GET called up to 60 times per report. Both now go through
asyncio.to_thread. The delivery case fires at ~03:30 UTC, inside the window where
db_backup_service holds SQLite's lock.

Related to Abilityai/trinity-enterprise#498
Related to Abilityai/trinity-enterprise#499

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WLerYYUUEEmf43UKF2VVRY
Found re-reviewing my own residual fix. can_user_share_agent resolves via
get_user_by_username, so str(current_user.id) finds no user and returns False —
refusing the OWNER as well, which turns the gate from a narrowing into a
functional break. Every other call site in the codebase passes
current_user.username.

Every test I wrote for the gate stubs that function, so all six passed against the
broken argument. Added one that inspects what is passed rather than what comes
back — the only shape that could have caught this.

Related to Abilityai/trinity-enterprise#498

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

Re-reviewing my own residual fixes found the cancel fix was itself unreliable:
after catching CancelledError it fell through to `await asyncio.to_thread(_write)`.
A task is cancelled because the loop is going away, so planning to reach another
await point is planning on the thing that just stopped being available. `_write`
is now hoisted above the wait and called SYNCHRONOUSLY in the handler, then the
cancellation is re-raised — swallowing it would leave the task running against a
closing loop and rob the caller's shutdown path of its exception.

The two structural tests are now AST reads rather than string slices. Both earlier
revisions cut the source on a neighbouring token — `def _write()` (which this
commit moves) and `except Exception` (which appears inside the cancel handler's
own comment explaining why it does not catch) — and failed for reasons unrelated
to what they assert. Third instance of that trap in this branch; a structure
question deserves a structural read.

Related to Abilityai/trinity-enterprise#498

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 13:51
@dolho

dolho commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

/review — re-review of #2568 (b85cad25)

Six defects have been fixed since the first pass. Two of them were in the fixes
themselves
, which is the part of this worth reading — the residual list I posted
was not a to-do list I could work through mechanically.

From the review: three defects, all fixed

C1 the complaint travelled back to the agent it was about 48a2f5c4
C2 ent#363 fired in every room, including an operator's own 48a2f5c4
C3 the refusal terminal skipped the secret-scrub seam 9edb2d63

C2 has a behavioural proof on a live stack: in an operator's own room the agent now
answers "my last response was around 150–200 output tokens; the input context was
probably 3,000–5,000"
— direct, where before it had been told to keep costs and
platform internals out of the conversation.

From the residuals: four more, all fixed (74a2e79b)

A shared user could deliver into a colleague's Workspace. I posted this as
needing your decision. It didn't — the safe rule costs nothing. Schedule creation is
assert_agent_access (owner or shared or admin) and the target may be any roster
member, so a merely-shared user could schedule a recurring message, with a prompt of
their choosing, into a colleague's Main chat as an ordinary turn from the agent. Rule
now: address yourself freely, address anyone else only as the owner — which leaves
the common self-service case open to shared users. On create and update, because
create-only is a formality (create without the field, PUT it a second later).

A resolved complaint suppressed every later one about the same target.
ON CONFLICT DO NOTHING ignores the existing row's status, so once acknowledged that
person could never report that target again — silently, forever, which is worse than
a duplicate. Id quantised to the UTC day (ent#434's bucketing).

A cancellation inside the in-flight wait lost the report, and two synchronous DB
paths ran on the event loop
— resolve_and_stamp (4–6 blocking calls, two of them
writes) and the marker read (a blocking Redis GET up to 60× per report). Both moved
to asyncio.to_thread; the delivery case fires at ~03:30 UTC, inside the window
where db_backup_service holds SQLite's lock.

The two the fixes introduced

[F1] The authority gate was broken, not merely narrow (4df4dd72).

can_own = db.can_user_share_agent(str(current_user.id), name)   # ← id
def can_user_share_agent(self, username: str, agent_name: str) -> bool:
    user = self._user_ops.get_user_by_username(username)
    if not user:
        return False

An id finds no user → False → the owner is refused too. A gate meant to narrow
who may address a colleague would instead have blocked everyone, including the person
it exists to permit. Every other call site in the codebase passes
current_user.username.

All six tests I wrote for that gate stubbed can_user_share_agent, so every one
passed against the broken argument. The test that catches this inspects what is
passed rather than what comes back — a shape none of the others had.

[F2] The cancel fix was itself unreliable (b85cad25).

After catching CancelledError it fell through to await asyncio.to_thread(_write).
But a task is cancelled because the loop is going away, so planning to reach another
await point is planning on the thing that just stopped being available. _write is now
hoisted above the wait and called synchronously in the handler, then the
cancellation is re-raised — swallowing it would leave the task running against a
closing loop and rob the caller's shutdown path of its exception.

A pattern worth naming

Three times on this branch a structural test failed because I sliced source on a
neighbouring token — and each time that token appeared inside a comment explaining
the very thing being asserted
: create_operator_queue_item named in a docstring to
say why it is forbidden; def _write() after I moved it; except Exception inside
the cancel handler's own note about why it does not catch. All three are now ast
reads. A structure question deserves a structural read; string-slicing over commented
code produces a test that fails for reasons unrelated to its subject.

Both review classes are recorded in docs/memory/learnings.md: a new item on a
shared queue inherits every consumer that queue already had
, and generalising a
requirement's noun is a deviation, and a conditional that can never be false is how
you find out
.

Still standing — stated, not buried

  • create_item has no UPDATE path, so an edited comment does not reach an item
    already raised that day. Shared with ent#434; belongs at the sink.
  • _fail_execution_row's pre-read is redundant — db/schedules/executions.py
    already guards non-success terminal writes inside the transaction. Harmless, and I
    left it rather than widen the diff.
  • The agent-only room branch is now reachable but only via an operator-created
    room; a room whose creator is a workspace client is always client-facing, which is
    correct and worth knowing when reading room_is_user_facing's tests.

Verification

Green: 361 backend tests locally across every affected file; frontend 96 files /
2158 tests; vite build clean; MCP tsc --noEmit clean; all 13 PR checks including
pg-migrations, schema-parity, e2e and gitleaks. All three features exercised
end-to-end on a live stack against PostgreSQL — the new Alembic revision applying to
a real database, a real scheduled brief landing in a Main chat, a thumbs-down raising
a queue item, and the ent#363 signal correctly absent from an operator's own room.

Still running as I post this: the backend unit matrix on b85cad25
(run 34128948926). The previous matrix was green on head across all three seeds
with the regression diff clean, on 48a2f5c4 — every commit since is the residual
fixes plus their tests, all of which pass locally, but I have not yet seen a full
matrix on the final commit and am not claiming one. I will report it here either way.

Note on this PR's CI: backend-unit-test.yml gates pull_request on
branches: [dev, main], and this PR is stacked on dolho/issue-523 — so the unit
matrix does not run automatically here and every result above came from a manual
workflow_dispatch. Once #2558 merges and this rebases onto dev, it runs on every
push. Worth knowing when reading the green check list: pg-migrations and
schema-parity cover the migration, but the ~90 new tests are not in it.

Out of draft at the operator's request; #2558 still merges first.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WLerYYUUEEmf43UKF2VVRY

@dolho

dolho commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Final matrix: green ✅

The run I flagged as outstanding in the re-review above has completed on b85cad25
— run 34128948926:

conclusion: success
  lint (sys.modules pollution check): success
  pytest (base, seed 12345):  success      pytest (head, seed 12345):  success
  pytest (base, seed 67890):  success      pytest (head, seed 67890):  success
  pytest (base, seed 99999):  success      pytest (head, seed 99999):  success
  regression diff:            success

regression diff is the one that matters here: it compares head against base across
all three seeds and fails on any test failing under HEAD that did not fail under
BASE. It is what caught the ent#279 scrub regression earlier in this branch, and it
is clean.

Every claim in the re-review is now backed. Nothing outstanding on my side.

…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
@vybe
vybe changed the base branch from dolho/issue-523 to dev September 7, 2026 15:14
@vybe vybe closed this Sep 7, 2026
@vybe vybe reopened this Sep 7, 2026
Comment thread src/frontend/tests/unit/operatorQueueUnknownType.spec.js Dismissed

@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: docs (requirements §5.24–5.26, scheduling §10.18, flow + index, workspace/database architecture), both migration tracks (renumbered to Alembic 0056 off 0055_portal_session_main_chat in the dev merge), security greps clean, 70+ named tests. Merging.

@vybe
vybe merged commit 15dc9c7 into dev Sep 7, 2026
32 of 33 checks passed
AndriiPasternak31 added a commit that referenced this pull request Sep 7, 2026
Three conflicts, all "both sides added at the same insertion point":

- `operator_queue_service.py` — ent#499 (#2568) added `workspace_problem_report`
  to `_BUDGETED_ALERT_TYPES` and `workspace-problem-` to `_RESERVED_ID_PREFIXES`;
  #2529 adds `gitignore_untracked` / `gitignore-untracked-` to the same two.
  Pure additions, both kept.

  Real interaction, not just a textual merge: ent#499 introduced
  `is_platform_minted`, keyed on `_RESERVED_ID_PREFIXES`. Listing
  `gitignore-untracked-` there therefore ALSO keeps the sweep alert out of the
  agent's own `~/.trinity/operator-queue.json` write-back. That is the correct
  outcome — the sweep alarm is a platform alarm ABOUT the agent, not a loop the
  agent opened and is waiting on — so the prefix comment now records it rather
  than leaving a future reader to rediscover it. Verified by executing the
  predicate against the id `git_service` actually emits.

- `docs/memory/learnings.md`, `docs/memory/feature-flows.md` — newest-first
  lists, both sides prepended an entry. Both kept.

Merged tree re-verified: 2027 passed, 4 skipped across every `git_service`- and
`operator_queue`-touching unit test plus `test_models_centralized`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvPBC72QFs1kXSA7YUyiS1
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.

3 participants