Skip to content

feat(workspace): opening an agent starts a new chat with the cursor in the message field (abilityai/trinity-enterprise#784) - #3219

Draft
vybe wants to merge 8 commits into
devfrom
feature/ent784-new-chat-default
Draft

vybe wants to merge 8 commits into
devfrom
feature/ent784-new-chat-default

Conversation

@vybe

@vybe vybe commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Fixes abilityai/trinity-enterprise#784. A single issue, not stacked: the base is dev. Draft: it merges on the operator's call. Squash merge only (see Before merge).

What

Opening an agent in the Workspace lands you where you can keep typing, with the cursor in the message field. It no longer reopens the agent's most recent chat by default. This deliberately reverses the earlier landing rule (most recent chat, with Main as the floor).

Landing precedence (updated 2026-10-05 after the first frontend-e2e run went red on the drafts specs):

  1. A link to a specific chat opens that chat (unchanged; not this helper's job).
  2. The lastOpenSessionId seam (no caller yet, see Handoffs).
  3. Otherwise, the agent's chat holding an unsent draft. With several, the most recently edited wins. An unsaved new-chat draft lands on a new chat, which restores it.
  4. Otherwise, the agent's existing empty chat is reused. There is at most one per agent. With legacy duplicates the choice is deterministic: Main first, then the newest.
  5. Otherwise, a new chat.
  • One landing rule (components/portal/portalUtils.js): a pure agentLanding({ agentName, threads, drafts, lastOpenSessionId }) helper replaces landingThread. Every door that resolves "open this agent" calls it (landOnAgent, resolveAgentLanding), and future callers reuse the same seam. agentEmptyChat defines "empty": unarchived, not a room, no last_message_at, and message_count 0 or absent.
  • Drafts (portalDrafts.js): isDraftedThread is now the ONE predicate behind both the sidebar's agent-row draft mark (agentsWithDrafts) and the landing, so "this row is marked" and "clicking lands there" cannot drift. Rooms and archived threads are excluded on both sides. draftedLandingFor picks the newest updatedAt.
  • Shell (views/Portal.vue):
    • A live call is guarded first: guardLeaveCall runs before any landing, so back/forward can no longer silently leave a call.
    • The /workspace/a/:name watcher is idempotent. A thread-list refresh after the first send keeps the adopted chat instead of minting a new composer over it.
    • ensureMainListed (a list read that creates a pinned Main) returns early for an agent that already has an empty chat, so opening an agent never adds a second empty row.
    • The focus mode is set before the landing branch, so a drafted-thread landing follows the same pointer rule as every other landing.
  • Focus (PortalConversation.vue, PortalRoom.vue, portalDrafts.js):
    • The composer takes focus on mount when the landing asks for it, gated on (pointer: fine), so phones and tablets do not raise the keyboard unprompted.
    • An explicit New chat gesture still focuses on touch devices, as before. ?new=1 is the one way past the whole precedence.
  • Docs: docs/memory/architecture/workspace.md, the workspace-agents-at-the-centre feature flow, and requirements/core-agent.md describe the rule.

Rulings carried (on the operator's behalf; recorded in the plan file)

  • Operator, 2026-10-05 (binding, on the issue): drafts win, newest edited first. The existing empty chat is reused, with at most one per agent. Precedence is link > draft > empty > new. The drafts e2e specs describe the wanted behaviour and were not edited.
  • T1 (a), superseded in part by the ruling above: the unused Main is still hidden from the chat list. It is now also reused (arm 4), and no second one is created when an empty chat exists.
  • T2 (a): the last-open-chat exception is a helper argument only. There is no per-agent memory store until a caller needs one.
  • T3 (b), a deviation from the planner's recommendation: focus without a keyboard applies only to the unprompted landing. An explicit New chat / ⌘J / picker gesture still focuses on touch devices, because one rule would regress the earlier Android behaviour (bug(workspace): New chat shows no tab and no focus, Main tab absent, titles fall back to the first message, tabs not fixed-width #2579 AC 2). Operator: overrule to the single rule here if you prefer it.
  • T4 (a): the agent page stays on /workspace/a/:name, with an idempotency guard.
  • feat: SMARTS trading pipeline with Telegram notifications and Miro visualization #13: guardLeaveCall comes first in landOnAgent.

Review + security

  • /review (claude-fable-5-1, report-only, range 482ad074..040285ae): MERGEABLE, 0 critical. /cso --diff: 0 findings at the 8/10 gate.
  • Fixed in 4462f3dc and c8af3d70:
    • I1: a thread-list re-fire after adoption could re-mint over the live chat. It is now guarded, and the guard was checked against same-agent back/forward and against the cold deep link.
    • I3: decision feat: SMARTS trading pipeline with Telegram notifications and Miro visualization #13 and the T4 guard were pinned only by source text. They now have jsdom mount cases (portalAgentLandingRemount.mount.spec.js, 8 cases), mutation-verified in both directions.
    • portalUnavailableTargets.mount.spec.js still asserted the old landing rule; fixed.
  • Fixed in ccfe0fa0 (drafts win) and 24fb2be1 (empty-chat reuse) after the operator reopened the PR on red frontend-e2e (workspace-drafts.spec.js :92 and :116). Those specs were not edited.
  • The commits after the review (4462f3dc..24fb2be1) were not re-reviewed. They are covered by the targeted tests and the full CI below.
  • No Alembic revision; frontend and docs only. On a merge-tree against live dev (482ad074) the merge is clean.

Tests

  • CI on 24fb2be1: every check is green.
    • frontend-e2e: 94 passed. The run at c8af3d70 had 92 passed and 2 failed; both drafts specs now pass. agent-detail-request-dedupe.spec.js:73 was flaky in both runs and passed on retry. It is unrelated to this change.
    • frontend-build runs the full frontend unit suite. backend-unit-test, CodeQL, gitleaks and schema-parity are also green.
  • Targeted vitest/jsdom at the tip: 19 files, 394 tests green.
    • New: portalDraftLanding.spec.js covers each precedence arm, newest-draft-wins and archived/room exclusion.
    • New: portalDraftLanding.mount.spec.js replays both e2e flows. It was red at c8af3d70, with sessionId: null where the draft was, and is green at the tip.
  • Not run here: Playwright locally and any live stack.

Before merge

Handoffs

  • A follow-on issue supplies lastOpenSessionId to the same helper, which brings the "return to the last-open chat" exception to life. Later callers use the same seam rather than a second rule.
  • Follow-up candidate, not filed (I2, pre-existing on dev): a cold deep link for a viewer with zero threads never lands. The watcher needs the roster-loaded signal as a source.

🤖 Generated with Claude Code

@vybe vybe added the ui PR touches the frontend UI — triggers Playwright e2e tests label Oct 4, 2026
Trinity Agent (trinity) and others added 7 commits October 5, 2026 17:05
…rinity-enterprise#784)

Replaces `landingThread` with one pure rule, `agentLanding`, that every door
which has to RESOLVE a landing calls. ent#523 landed you in the chat you were
most recently active in; most visits to an agent start new work, so resuming
cost two actions every time. The default is now a new, empty chat.

Nothing is minted by the landing: the helper is pure and returns a null
session, and the row is born on the first send (`newThread`, ent#451), so
repeated visits accumulate no empty chats. An unused Main is already kept out
of the chat list by `sidebarThreadsOf` — the AC 4 guard, still green.

`agentLanding` takes `lastOpenSessionId` as the seam ent#621's agent-switch
keys will pass, honoured only when it still names a live, unarchived chat of
this agent in the principal's own thread list — so a stale or forged id falls
back to the default rather than landing somewhere it should not (#3140 class).
No Map is built here; ent#621 adds the state and the handler.

`resolveAgentLanding` keeps its signature and delegates, so the `?agent=` deep
link and the sidebar row cannot drift. `forceNew` stays accepted for existing
`?new=1` links and is now redundant rather than wrong.

Shell wiring (`landOnAgent`, the focus gate) follows in the next commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lityai/trinity-enterprise#784)

Wires the shell to the ent#784 rule and gates the composer focus.

`landOnAgent` is now synchronous. The awaited `ensureMainListed` is gone from
the landing path — the rule needs nothing from the network — and with it the
overtake race that awaited round trip required. The pinned Main is still
minted on the first visit; `watch(activeAgentName)` owns that promise (ent#523)
and no longer blocks the landing.

The landed chat KEEPS `/workspace/a/:name` rather than escaping to bare
`/workspace`, so a reload, a bookmark or a copied link still names the agent;
the first send replaces it with the thread's own URL as today. An idempotency
guard makes the second watcher fire (the thread list arriving) a no-op, so it
cannot remount an unsent chat and throw away what was being typed.

`landOnAgent` now asks `guardLeaveCall` BEFORE touching `activeAgentName`,
which feeds `convKey`. Back/forward and a typed `/workspace/a/:name` reach the
landing without passing a click door, so they could end a live voice call
without a word (the ent#551 class; the click doors were already guarded).

Focus is gated on WHY the fresh composer mounted, not on the pointer alone.
A gesture (New chat, ⌘J, the agent picker, switch-agent) passes `always` and
focuses on any pointer, keeping #2579 AC 2 on Android. A landing passes
`fine-pointer` and focuses only where that cannot summon an on-screen
keyboard — the "unprompted" the issue names. `shouldFocusOnRestore` is renamed
`shouldAutoFocusComposer`: two reasons now share the one rule.

Tests: the focus gate is proven by MOUNTING PortalConversation under a stubbed
`matchMedia` (#2918), asserting `document.activeElement`, with focus shown to
be elsewhere first. Reverting either the rule or the gate turns 10 cases red on
behaviour — session ids and activeElement, not source text.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…y-enterprise#784)

Tiered docs for a feature change: the owning area file, the requirement, and
the feature flow.

- `architecture/workspace.md` — `agentLanding` is the one landing rule and what
  it now answers; the synchronous `landOnAgent`, the kept URL, the guard order,
  and the `focusOnMount` mode.
- `requirements/core-agent.md` — the ent#523 "One page" rule amended rather
  than rewritten: what reversed, and that landing mints no row.
- `feature-flows/workspace-agents-at-the-centre.md` — the rule table, the "a
  tab is not a landing" note (the strip is now the only way back), the two
  doors that RESOLVE vs the gesture doors that ASSERT, and a `## Changed by
  ent#784` section rather than edited history. Names the follow-up: backend
  adoption of an empty Main on `new_thread=True` is not done here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ted chat (Abilityai/trinity-enterprise#784)

ent#784's landing stays on `/workspace/a/:name`, so the watcher that resolves
that route can now re-fire with the route unchanged — once per thread-list
refresh — for as long as the person stays there. `landOnAgent`'s idempotency
guard keys on `startingNewChat`, which the first send clears, so a refresh
settling in the window between `onSessionAdopted` and the asynchronous
`router.replace` nulled `pendingSession` and bumped `convGen`, remounting an
empty composer over the thread whose first reply was streaming (and handing the
`/c/:id` watcher a #3140 "chat isn't available" for the chat just created).

Guarded in the watcher instead: a list-only re-fire is a no-op, keyed on "the
route did not change on this fire" AND on having already landed this name. Both
clauses are load-bearing — the name alone swallows the cold deep link, whose
only fire IS the list arriving with the param already in place. Deliberately
NOT keyed on `pendingSession`: back/forward from `/workspace/c/:id` arrives
with a session set and must still land fresh (T4).

New behavioural pin `portalAgentLandingRemount.mount.spec.js` (shallow-mounts
Portal.vue, asserts the conversation's props and instance identity, not source
text): the adopted-session case went red before this change with
`sessionId: null`; the three regression cases — composing, same-agent
back/forward, cold deep link — were green before and stay green.

Also fixes `portalUnavailableTargets.mount.spec.js`, which still asserted
ent#523's landing (`/workspace/a/scout` → `/workspace/c/t1`) and was failing on
this branch; it now proves the page opened by the conversation on screen.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…source-text pins (Abilityai/trinity-enterprise#784)

`landOnAgent` was pinned only by regexes over its own function body
(`workspaceNewChat.spec.js:181-211`), so nothing executed it: an inverted
guard, or the right lines in the wrong order, reads byte-identically to a
regex. The shell is mount-testable (`portalUnavailableTargets.mount.spec.js`),
so these assert STATE instead.

Decision #13 (ent#551 class) — back/forward reaches the landing without passing
a click door, and `activeAgentName` feeds `convKey`, so a live call must be
asked about BEFORE anything is written: mid-call, the dialog opens and the chat,
its props and its instance identity are untouched; confirming then performs the
landing it deferred; cancelling leaves call and chat exactly as they were.

T4 — one navigation to an agent page mounts exactly ONE conversation, however
often the thread list moves underneath it.

Mutation-verified, both directions: with `guardLeaveCall` removed from
`landOnAgent`, the two mid-call cases go red; with the T4 guards removed, the
composing and one-mint cases go red. The source-text pins are kept as
supplements (they still pin "no await / no ensureMainListed / no router.push"
inside the function, which state cannot see).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…inity-enterprise#784)

F1 of the operator's 2026-10-05 reopen. `agentLanding` always answered a new
chat, so a draft typed into an existing chat was not where opening the agent
landed — and the draft mark on the agent's row pointed at words that clicking
it could not get back to. `e2e/workspace-drafts.spec.js:92` and `:116` describe
the wanted behaviour and were red at c8af3d7.

The rule is now the ruling's precedence: a link naming a chat (never reaches
here), then the ent#621 `lastOpenSessionId` seam unchanged as the first arm,
then the agent's chat holding an unsent draft — newest `updatedAt`, with the
unsaved `new:<agent>` chat weighed on the same clock — then a new chat. A
`new:` winner is `sessionId: null`, which is where those words already live; a
thread winner opens that thread and its remount restores the draft.

The candidate set is the set that lights the sidebar's mark: `isDraftedThread`
is shared by `agentsWithDrafts` and the new `draftedLandingFor` rather than
copied, so "the mark means click here to continue" cannot drift — a room and an
archived thread are excluded on both sides. The drafts map is passed IN by both
doors (`landOnAgent`, `resolveAgentLanding`), never read inside the rule, so
one rule serves every door and stays pure.

`composerFocusMode` moves above the branch in `landOnAgent`: a landing on a
drafted chat is the same kind of arrival, and `openThread` does not touch that
ref, so a previous gesture's `always` would otherwise have made the next
landing focus on a touch device (T3(b)). The restore's own caret already shares
`shouldAutoFocusComposer`. The T4 remount guard is untouched: the watcher still
returns on a list-only re-fire, and the drafted-thread arm leaves the agent URL
through `openThread`, so the guard is never reached with a stale answer.

Tests: `portalDraftLanding.spec.js` (each arm, newest-wins both ways, the tie,
room/archived exclusion, and the property that every MARKED row is a landing
candidate) and `portalDraftLanding.mount.spec.js`, which replays both e2e flows
at the shell's seam — red at c8af3d7 on the e2e's own symptom, 2 of 3 failing
with `sessionId: null` where the draft was.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…y-enterprise#784)

F2 of the operator's 2026-10-05 reopen: "opening an agent must not create a new
chat every time; if an empty chat with that agent already exists it is reused,
and at most one empty chat per agent exists at any time."

Arm 4 of the precedence, under the drafts arm and above a new chat.
`agentEmptyChat` is the read side: an unarchived, non-room thread of this agent
with no message sent — ent#523's own "unused Main" test, widened to any row
that fits it. Both fields are required because the two reads disagree: the
cross-agent batch omits `message_count` (so an absent count must not read as
used) while the per-agent read carries it (so a count of 2 must not read as
empty just because `last_message_at` is missing). Several empty rows — legacy
data — resolve Main first, then newest `created_at`, then id, so list order
never decides and two doors reading one list cannot reuse two different rows.

`ensureMainListed` is the write side, and the only place the "at most one" half
can be held: it is a GET that INSERTS (`list_sessions` → `ensure_main_session`),
so for an agent with an empty chat but no Main — legacy, since nothing mints a
non-Main empty row today — visiting it would add a SECOND empty chat and then
land on one of the two. It now returns early for that case. #2579's "the pinned
tab has to be there" still holds for every agent whose chats are all used,
which is the case it was about; that is pinned as its own mount case rather
than left to the comment. No backend change: nothing here asks the server for
anything it does not already do.

Three existing specs carried fixtures with neither message field, which under
the old rule meant "a thread exists" and under the ruling means "an empty chat
to reuse" — the assertion they were written for. Each is updated to a USED row
and the empty case is asserted separately, so none of them went green by
accident: `portalAgentsAtCentre` (the landing arms), `workspaceAgentLanding`
(the `?agent=` door, both id shapes) and `workspaceNewChat`, where `?new=1` is
load-bearing again — it is the one way past the whole precedence, so the two
answers now have to differ.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vybe
vybe force-pushed the feature/ent784-new-chat-default branch from 24fb2be to dace0c2 Compare October 5, 2026 16:06
…and the caret (Abilityai/trinity-enterprise#784 review)

ent#784 made /workspace/a/:name the URL a landed new chat RESTS on. Three
things had only been true because nobody stayed there; each was reproduced
in a browser against the rebased branch.

- Clicking the row of the agent whose new chat is already on stage cleared
  `startingNewChat` in `openAgentPage` and then pushed the URL it was
  already on, so the landing never re-ran. The composer still read "New
  chat" and sent its first message without `new_thread`, which the server
  resolves to the agent's Main chat. The click is now a no-op that hands
  the caret back.
- The rail rules still read the agent URL as "a page, not a conversation",
  so a landed new chat had no rail, and the first send (which moves the URL
  to /workspace/c/:id) slid one in beside a reply mid-stream. The URL is
  rail-free only until the landing has put the named agent on stage, and
  its column is reserved mid-load like any 1:1 route.
- A landing that reuses a chat (the agent's empty one) goes through
  `openThread`, and a thread mount focuses nothing, so the caret stayed on
  the sidebar row. The shell hands it over under the landing's own
  fine-pointer rule.

Tests: portalAgentLandingStage.mount.spec.js (10 cases, mounted shell).
Four were red before the fix, for these reasons.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ui PR touches the frontend UI — triggers Playwright e2e tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants