feat(workspace): New chat means a new chat (ent#451 — the fresh-thread slice)

[dolho]

Journey Impact: none: interaction change within the existing Workspace chat surface — no journey in the current deck (J01–J10) covers Workspace chat.

Related to ent#451 (the fresh-thread slice — see "what this does not close" below)

The bug

Pressing New chat drops you back into the existing conversation with that agent.

One value carrying two meanings. An absent session_id meant both "I don't know which thread" and "I want a fresh one", and both readers resolved it as the first:

_resolve_session_id(..., None)  → resume the client's latest
get_history(..., None)          → return the most-recent thread

Both readings are right for the case they were written for — a deep link, a refresh, an API caller that never held a session id — so neither could simply be inverted. The intent had to become sayable.

The frontend tell was an asymmetry: New chat with the agent you were already on started fresh; New chat with a different agent resumed.

// PortalConversation.vue — before
if (props.agent.name !== oldName || sid) await loadThread(sid)   // sid === null
else { messages.value = [] }                                      // "brand-new chat"

A changed agent was read as "load that agent's history", discarding the pendingSession = null that newChatWithAgent had just set to mean the opposite.

Most of ent#451 is already built

Worth recording, because the issue is complexity-high and this PR is not:

AC	State
#2 chats listed per agent, switchable, title + recency	already shipped — sidebar with titles, recency, starred lifted out, search, per-agent avatars
#3 landing rule for agent-initiated items	already decided and documented in ensure_thread_for_ask
#4 existing history migrates cleanly	nothing to migrate — no UNIQUE constraint, title column, (agent_name, client_email, last_message_at) index, auto-titling all present
#1 start a new chat without hijacking the existing one	this PR

AC #3 is untouched and pinned by a test: asks keep reusing the latest thread so they don't accumulate beside the conversation. That rule matters more once several chats exist, not less — an ask opening its own thread each time would bury itself.

Four properties

• An explicit session_id wins over the flag. A caller sending both contradicts itself; the id is a fact, the flag an intent, and abandoning a named thread would strand a turn meant for a conversation the caller could see.
• The ownership check runs first either way — the flag is never a route past it.
• Both turn entry points carry it. The Workspace uses /chat/stream and falls back to /chat; a flag honoured by only one brings the bug back exactly when streaming fails.
• The intent is spent on adoption. The send guard already ANDs on "no session yet", so a second turn was never going to open a third thread — clearing it in onSessionAdopted keeps the two bits from disagreeing after a navigation.

Test doubles updated, not worked around

Seven _resolve_session_id lambdas and four _fake_chat stubs didn't accept the new keyword; they take **kw now — a stub that must be edited for every new parameter is a second signature. One hand-rolled _Body model double gained the field. All stale stubs, no behaviour changes.

Verification

pytest -k "portal or ent451 or ent358 or ent429 or ent430 or ent286 or ent287"
  → 392 passed, 1 skipped
npm run test:unit  → 1497 passed

Mutation-checked:

Mutation	Result
Make the flag inert in the resolver	1 failed
Let the flag override an explicit session id	2 failed

The full backend suite exceeds a local foreground run — left to CI rather than claimed.

What this does not close

ent#451's headline is topic-scoped conversations. This makes New chat honest and lets several threads per agent exist and be switched between — which is the daily annoyance and the blocking half. Not here: naming/renaming a chat by topic, and grouping the sidebar list per agent rather than by recency across agents. I'd keep ent#451 open for those, or split them out.

Not from this branch

test_ent457_portal_turn_kwargs and test_both_portal_row_creation_sites_name_the_chat fail on dev today — backend-unit-test is red there. Both are repaired in #2427.

[obasilakis]

The backend design is sound and well tested, and CI is fully green. Two gaps hold it: the same bug survives on a sibling entry point, and the half of the fix that lives in the frontend has no test.

Blockers

1. src/frontend/src/views/Portal.vue:791-819 — the ?new=1 deep link still resumes.

resolveAgentQuery() passes forceNew: !!route.query.new to resolveAgentLanding and sets pendingSession = null, but never sets startingNewChat = true. So /workspace?agent=X&new=1 renders an empty conversation, PortalConversation.vue:974 computes newThread: props.newChat && !currentSessionId.value as false, and the backend resolves session_id=None, new_thread=False and resumes the existing thread. That is the reported bug intact on the documented ?new=1 contract (portalUtils.js:215), which workspaceAgentLanding.spec.js:35 pins with the header "an explicit ask for a fresh thread must not silently resume the last one". One-line fix.

2. No frontend test.

Thirteen files and seven test files, all Python, six of them mechanical **kw stub edits. The literal bug has zero coverage: the watcher-branch ordering at PortalConversation.vue:660 ahead of the agent-changed branch at :661, the first-paint guard at :679, the two newThread send expressions at :974/:989, and the startingNewChat lifecycle. src/frontend/tests/unit/ already holds ~27 portal/workspace specs using two cheap established patterns — a pure portalUtils function plus vitest, or a source grep as in portalLeaveSpecificRoute.spec.js. The npm run test:unit → 1497 passed in the body is the pre-existing suite, not new coverage.

3. tests/unit/test_ent451_new_chat.py (test_history_without_a_session_is_unchanged) cites a frontend spec that does not exist in this PR. The docstring points at "the spec in tests/unit/… frontend suite" as where the frontend rule is pinned — a dangling reference asserting coverage that isn't there.

Comments

• Portal.vue:775-785 — the deep-link watcher, which the code itself calls "the commonest way in — back/forward, a bookmark and a reload all land here rather than in openThread", sets pendingSession = sid without clearing startingNewChat. Benign today only because both consumers AND on !sid, but it contradicts the :438-440 comment claiming the flag is cleared the moment a real thread exists.
• Portal.vue:586,600 — openRoom / openAgentPage null pendingSession without clearing startingNewChat. Same latent-desync class.
• test_both_turn_entry_points_forward_it is inspect.getsource plus a "new_thread" substring, so it passes on a comment or a misspelled kwarg — the weakest guard on the property the PR calls load-bearing ("a flag honoured by only one brings the bug back"). And since the seven _resolve_session_id lambdas and four _fake_chat stubs now absorb new_thread via **kw, no pre-existing portal test can observe the flag's propagation, which makes that grep the only propagation check.
• Sequencing: #2427 is open and edits the same client_portal/service.py regions (_precreate_sync_execution, start_portal_turn). Semantic conflict is low, textual conflict likely — land #2427 first. Relatedly, the body's "fails on dev today" is imprecise: test_ent457_portal_turn_kwargs.py doesn't exist on dev, #2427 introduces it.
• No closing keyword, so the linked issue won't auto-promote — a manual bump is needed. Deliberate per the body ("I'd keep ent#451 open"), noting only so it isn't missed. ent#451 also isn't a GitHub autolink and public #451 is a different, closed issue, so the shorthand reads ambiguously. The issue itself carries both status-in-progress and status-ready.

Docs and gating

• docs/memory/feature-flows/workspace-absorbs-session.md documents both seams this PR changes — _resolve_session_id's placement (:195) and the resolveAgentLanding landing rule (:124-125) — and is not updated. Per the tiered rule that's the required doc surface here.
• docs/memory/architecture.md's Workspace/Client Portal section documents the portal turn surface in detail, and PortalChatRequest gains a public field on the ent#83-documented headless integration endpoint (POST .../chat and /chat/stream) with no architecture note.
• Requirements untouched is fine — this is a behaviour correction inside an existing capability.
• Gating: OSS-core, undocumented. No requires_entitlement, and the logic lands in src/backend/client_portal/, which architecture.md records as OSS core since ent#356 ("mounted in every build"), so it inherits that ruling and is de facto right. But the default for an enterprise-tracker feature is gated-unless-explicitly-ruled, and the convention is to state it — "OSS-core by decision (ent#384)", "(ent#326)", "(ent#392)" all appear verbatim in architecture.md. One sentence, so it's never later inferred from the mere fact that it merged.

Design system is clean — no template or style changes beyond one :new-chat prop binding, and no hex literals or raw Tailwind palette classes added. Backend tests are good: explicit-id precedence, ownership 404 with the flag set, first-ever-chat, and an AC#3 drift pin.

[dolho]

Addressed — 1302233e

Blocker 1 was real and I had not seen it

resolveAgentQuery passed forceNew to resolveAgentLanding and set pendingSession = null, but never raised startingNewChat. So ?new=1 rendered an empty conversation and then sent new_thread: false — the reported bug, intact on the documented contract, in the PR that exists to fix it.

And the cause is this PR's own thesis one level up: route.query.new was read in two places for two different decisions — which thread to land on and what the first send asks for — and only the first honoured it. Now read once into a local that feeds both:

const forceNew = !!route.query.new
const landing = resolveAgentLanding({ agent: route.query.agent, forceNew, ... })
...
startingNewChat.value = forceNew && !landing.sessionId

AND-ed with the landing so a ?new=1 that still resolved a thread never claims a fresh start. A test pins that route.query.new is read exactly once.

Blocker 2 — you're right, there was no coverage

1497 passed was the pre-existing suite. workspaceNewChat.spec.js, 9 tests, using the two patterns you named (pure function + source assertion in the portalLeaveSpecificRoute.spec.js shape):

• the ?new=1 deep link, both halves
• the watcher branch ordering at :660 ahead of :661 — ordering is the fix, and both branches are individually correct
• the first-paint guard
• both newThread send conjunctions (asserted as a count of 2, since the failure mode is unbounded — every turn after the second opens another thread)
• the settle-everywhere rule

Mutation-checked, and M1 is your blocker: reverting the startingNewChat line turns it red. So the spec would have caught what I missed.

Comment lines are stripped before the source assertions — a comment naming new_thread must not satisfy an assertion about the code (the #2415 lesson, which I'd otherwise have repeated here).

Blocker 3

Fixed — the docstring now names src/frontend/tests/unit/workspaceNewChat.spec.js instead of "the spec in tests/unit/… frontend suite".

Comments

Latent desyncs — all four settled: the deep-link watcher, openRoom, openAgentPage, and the unreachable-agent branch. You're right that it's benign today only because both consumers AND on !sid; a flag that is correct only because of a second variable is one refactor from being wrong, and the :441 declaration claimed otherwise. Now true as written.

The weak propagation guard — agreed, it was the weakest guard on the property the PR calls load-bearing. It now binds the keyword against each service signature (sig.bind_partial(..., new_thread=True)) and asserts the routes forward body.new_thread through comment-stripped source. Verified by mutation: deleting the forward turns it red.

Docs — workspace-absorbs-session.md updated at both seams you cited (:124-125 landing rule, :195 resolver), and architecture.md's Workspace section now documents the public new_thread field on the ent#83 headless surface.

Gating — stated rather than inferred: "OSS-core by decision (ent#451): deliberately ungated", matching the ent#326/#384/#392 convention.

Sequencing — agreed, #2427 first. I'll rebase if it lands first.

ent#451 shorthand — noted; using the full URL in future references. The issue's double status-in-progress + status-ready labels I'll clean up.

---

One correction, with evidence rather than silently applied

the body's "fails on dev today" is imprecise: test_ent457_portal_turn_kwargs.py doesn't exist on dev, #2427 introduces it

It does exist on dev:

$ git cat-file -e origin/dev:tests/unit/test_ent457_portal_turn_kwargs.py   # exits 0
$ git log --diff-filter=A -1 -- tests/unit/test_ent457_portal_turn_kwargs.py
d6a4bc10 fix(portal): execute_task must accept the client stamp it is handed (ent#457)

#2427 modifies it (repairing an AST walk that had gone inert), it doesn't introduce it. And independently of any PR:

$ gh run list --branch dev   →   failure   backend-unit-test

So "fails on dev today" stands. Everything else in the review is accepted as written — thank you, blocker 1 in particular was a genuine miss and the frontend-coverage point was the right thing to hold this on.

Verification

frontend  1497 → 1506  (+9 new)
backend    392 passed on the portal selection
           2 pre-existing dev failures unchanged (fixed in #2427)

[obasilakis]

Re-validated on 1302233e. All three blockers and all five comments from the previous pass are addressed with evidence — thanks.

First, a correction to my own last review. I claimed test_ent457_portal_turn_kwargs.py does not exist on dev and that #2427 introduces it. That was wrong: git cat-file -e origin/dev:tests/unit/test_ent457_portal_turn_kwargs.py succeeds, ent#457 added the file and #2427 only modifies it. Your correction stands. #2427 has since merged, and merge-tree against current dev gives 0 conflicts, so the sequencing concern is moot.

Prior items — verified fixed

#	Item	Evidence on 1302233e
B1	?new=1 deep link resumed	Portal.vue:812 reads route.query.new once; :845 startingNewChat.value = forceNew && !landing.sessionId
B2	No frontend test	src/frontend/tests/unit/workspaceNewChat.spec.js, 9 tests, runs in CI (frontend-build.yml:39). codeOnly() stops a comment satisfying a code assertion
B3	Dangling frontend-spec reference	docstring now names the real file
C1	Deep-link watcher didn't settle	Portal.vue:793
C2	openRoom / openAgentPage didn't settle	Portal.vue:597, :610, :829
C3	getsource + substring propagation guard	now binds the keyword against each service signature and asserts against comment-stripped router source — a typo or rename fails. Still source-level; see W3
D1–D3	flow, architecture, gating ruling	workspace-absorbs-session.md updated at both seams; +17 lines in architecture.md covering new_thread on /chat and /chat/stream, the id-wins rule, and ensure_thread_for_ask's deliberate exclusion; "OSS-core by decision (ent#451)" stated per the ent#326/#384/#392 convention

The backend design holds up on re-read: start_portal_turn resolves the thread once (service.py:2159) and its background _run() passes the resolved session_id without the flag, so streaming cannot create two threads; the ownership check precedes the flag (service.py:1152-1155) and test_a_named_thread_that_is_not_yours_still_404s pins it. No schema, endpoint, auth or migration surface. CI 24/24 green on the head commit.

Approving. Three follow-ups, none blocking, but the first is worth a commit before this merges because it is a stated invariant that is currently false.

W1 — the invariant the PR declares is false at two sites, and its guard cannot detect that

Portal.vue:592-597 and the declaration at :438-441 both say every site that nulls pendingSession also settles the intent. Two do not:

• Portal.vue:455 — leaveRoomRoute()
• Portal.vue:915 — onSignOut()

Measured on head: 6 pendingSession.value = null sites (:455, :591, :609, :629, :828, :915) against 8 settles. The guard at workspaceNewChat.spec.js:110-119 asserts settles >= nulls, which is a count and not a pairing, so it passes green with both sites unpaired and will keep passing as more are added. Both are benign today — onSignOut converges on the next resolveAgentQuery(), and leaveRoomRoute is only reachable from openRoom, which already sets the flag false — but "correct only because of a second variable" is the exact argument this PR used to justify fixing the other three.

Two lines in Portal.vue, plus turning the spec's toBeGreaterThanOrEqual into a per-call-site pairing.

W2 — onMounted gives the flag precedence over an explicit id, inverting the backend rule

PortalConversation.vue:679 is if (props.sessionId && !props.newChat) await loadThread(props.sessionId). The backend rule, documented and tested at service.py:1152-1155 / test_an_explicit_thread_beats_the_flag, is the reverse: an explicit id wins over the flag.

In the sessionId && newChat case this branch empties messages while currentSessionId keeps props.sessionId (initialised at :473), so the send guard at :974 is false and the turn resumes the named thread behind a blank transcript. Unreachable today, since every writer of startingNewChat = true (:628, :845) nulls pendingSession in the same tick — but the watcher at :660 gets this right (props.newChat && !sid) and the mount path does not, so the two paths already disagree about the precedence rule.

W3 — no behavioural propagation test

Every propagation check in test_ent451_new_chat.py is a shape assertion — model_fields, inspect.signature(...).bind_partial, comment-stripped router source. The six updated fixtures absorb the kwarg via **kw, so no test observes the value flowing. Nothing asserts that POST .../chat with new_thread: true returns a different session_id than with false.

test_ent287_portal_rate_limits.py already drives the router with a _Body double that carries the field, so one assertion on the returned session_id closes this in two lines. Given the PR's own framing — that a flag honoured by only one entry point brings the bug back — I would rather this were behavioural than grepped.

Smaller notes

• PortalConversation.vue:983-989 — routeMissing = status === 404 || status === 405 || !dispatchErr?.response cannot distinguish a lost request from a lost response, so a streaming dispatch that succeeded server-side but whose 202 never arrived falls through to sendPortalChat(..., { newThread: true }) because currentSessionId is still null. The double-spend on that path is pre-existing; the empty orphan thread it now leaves in the sidebar is new. Related: the Workspace sends no Idempotency-Key on either route (clientPortal.js:493-513), so the replay protection router.py:1053-1078 implements is inert for its only client.
• Nothing bounds portal thread count. new_thread=True mints an enterprise_portal_sessions row per turn, bounded only by the burst/hourly limiters at router.py:722-729. There is no per-(agent, client) thread cap and no retention sweep named for these rows. Worth a follow-up issue now that opening a thread is a deliberate user action rather than an accident.
• test_history_without_a_session_is_unchanged asserts "new_thread" not in inspect.getsource(svc.get_history). A negative source assertion breaks on a mention in a comment and proves nothing about behaviour; the positive form — get_history(agent, email, None) still returns the latest thread — is available and cheap.
• workspaceNewChat.spec.js's "reads route.query.new exactly once" asserts toHaveLength(1) over the whole SFC. Correct today, but it fails the moment anyone reads that query elsewhere in Portal.vue for an unrelated reason.
• The body's "Not from this branch: test_ent457_portal_turn_kwargs … fixed in #2427" is now stale — #2427 merged 2026-08-28. Worth trimming so the caveat does not outlive itself.

ent#451 is cross-tracker, so the merge automation will not promote it. Please set status-in-dev on abilityai/trinity-enterprise#451 manually after merge.