From d479873714755e8d7b9ee00080a079bbe6cd8fa9 Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Sun, 4 Oct 2026 18:10:44 -0400 Subject: [PATCH 01/22] feat(workspace): opening an agent resolves to a new chat (abilityai/trinity-enterprise#784) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../src/components/portal/portalUtils.js | 65 ++++++------- .../tests/unit/portalAgentsAtCentre.spec.js | 95 +++++++++++++------ .../tests/unit/workspaceAgentLanding.spec.js | 12 ++- 3 files changed, 104 insertions(+), 68 deletions(-) diff --git a/src/frontend/src/components/portal/portalUtils.js b/src/frontend/src/components/portal/portalUtils.js index d0a8c7742..bf43cb956 100644 --- a/src/frontend/src/components/portal/portalUtils.js +++ b/src/frontend/src/components/portal/portalUtils.js @@ -568,18 +568,14 @@ function isTransportError(err) { export function resolveAgentLanding({ agent, forceNew = false, agents = [], threads = [] } = {}) { if (!agent || typeof agent !== 'string') return null if (!Array.isArray(agents) || !agents.some((a) => a && a.name === agent)) return null + // ent#784: the same rule the sidebar's agent row uses (`agentLanding`), not a + // second one — a deep link and a sidebar click must land in the same place. + // `forceNew` is kept so existing `?new=1` links keep working, but it is now + // redundant: the default IS a new chat. This door passes no + // `lastOpenSessionId` (a deep link carries no session memory), so it always + // resolves to a fresh chat. if (forceNew) return { agentName: agent, sessionId: null } - - // ent#523: the same rule the sidebar's agent row uses (`landingThread`), not - // a second one. This used to take the first row of an already-sorted list, - // which agreed with "most recent" by accident; once Main exists, an unused - // Main sorts last on recency and "first row" would skip it, so a deep link - // and a sidebar click could land a first-time visitor in different places. - const latest = landingThread(threads, agent) - return { - agentName: agent, - sessionId: latest ? (latest.id || latest.session_id || null) : null, - } + return agentLanding({ agentName: agent, threads }) } // ent#358: the Workspace is now the ONLY continuous-conversation surface, so a @@ -1748,32 +1744,33 @@ export function feedbackAcknowledgement(captureFeedback) { return FEEDBACK_REACHED_AGENT.has(captureFeedback) ? FEEDBACK_SENT_TEXT : FEEDBACK_RECORDED_TEXT } -// --- Agents at the centre (ent#523) ------------------------------------------ +// --- Agents at the centre (ent#523, landing rule replaced by ent#784) -------- -// Which chat opening an agent lands you in. Most recently ACTIVE wins, and Main -// is the floor — never a blank stage, and never "the oldest thread happened to -// sort first". Returns null only when the caller has no chats with this agent at -// all, which the caller reads as "open Main once the list arrives". +// Where opening an agent lands you. ONE rule, called by every door that has to +// RESOLVE a landing (the sidebar/rail row via `landOnAgent`, and the `?agent=` +// deep link via `resolveAgentLanding`). The gesture doors — New chat, the agent +// picker, the switch-agent key — do not call this: they already mean "fresh". // -// Deliberately NOT `agentChatTabs()[0]`: that pins Main to the front, so it -// would land you in Main every time regardless of where you were last — the -// exact behaviour AC 1 replaced. -export function landingThread(threads, agentName) { - if (!agentName) return null - const mine = (Array.isArray(threads) ? threads : []) - .filter((t) => t && !t.is_room && t.agent_name === agentName && !t.archived_at) - if (!mine.length) return null - const ts = (t) => { - const iso = t.last_message_at || t.created_at - const n = iso ? new Date(iso).getTime() : 0 - return Number.isNaN(n) ? 0 : n +// ent#784 reverses ent#523's rule. The default is a NEW, empty chat +// (`sessionId: null`), because most visits to an agent start new work and +// resuming cost two actions every time. Nothing is minted server-side: the row +// is born on the first send (`newThread`, ent#451), so landing repeatedly +// accumulates no empty chats. +// +// The ONE exception is `lastOpenSessionId` (the seam ent#621's agent-switch +// keys will pass). It is honoured only when it still names a live, unarchived +// chat of THIS agent in `threads` — the principal's own list — so a stale or +// forged id falls back to the default rather than landing somewhere it should +// not (the #3140 class). +export function agentLanding({ agentName, threads = [], lastOpenSessionId = null } = {}) { + if (!agentName || typeof agentName !== 'string') return null + if (lastOpenSessionId) { + const row = (Array.isArray(threads) ? threads : []).find((t) => t && !t.is_room + && t.agent_name === agentName && !t.archived_at + && (t.id || t.session_id) === lastOpenSessionId) + if (row) return { agentName, sessionId: lastOpenSessionId } } - // An unused Main has no `last_message_at` and would sort last on recency - // alone, so a first-time visitor would land on nothing. Falling back to it - // explicitly is cheaper to read than a comparator that special-cases zero. - const used = mine.filter((t) => t.last_message_at) - if (used.length) return used.slice().sort((a, b) => ts(b) - ts(a))[0] - return mine.find((t) => t.is_main) || mine[0] + return { agentName, sessionId: null } } // The sidebar's agent order (ent#523 AC 6). diff --git a/src/frontend/tests/unit/portalAgentsAtCentre.spec.js b/src/frontend/tests/unit/portalAgentsAtCentre.spec.js index 341be976f..b0aed9dd3 100644 --- a/src/frontend/tests/unit/portalAgentsAtCentre.spec.js +++ b/src/frontend/tests/unit/portalAgentsAtCentre.spec.js @@ -17,7 +17,7 @@ import { sidebarThreadsOf } from '@/components/portal/portalInbox' import { agentChatTabs, - landingThread, + agentLanding, orderRosterAgents, agentPreview, composerAvailabilityNotice, @@ -96,44 +96,79 @@ describe('ent#523 — Main is pinned, archives stay reachable', () => { // Which chat you land in // --------------------------------------------------------------------------- -describe('ent#523 — landing', () => { - it('opens the chat you were most recently active in', () => { - const landed = landingThread([ - main(), - chat('c1', { last_message_at: '2026-09-07T09:00:00Z' }), - chat('c2', { last_message_at: '2026-09-07T12:00:00Z' }), - ], 'a') - expect(landed.id).toBe('c2') - }) - - it('falls back to Main for a first-time visitor', () => { - // An unused Main has no `last_message_at` and would sort last on recency - // alone, so a first visit would otherwise land on nothing. - expect(landingThread([main()], 'a').id).toBe('main') - }) - - it('never lands you in an archived chat', () => { - // Deliberately unlike the tab rule above: a tab is somewhere you can GO, a - // landing is where you are PUT without asking. - const landed = landingThread([ +describe('ent#784 — landing', () => { + it('lands on a NEW chat even when recent chats exist', () => { + // ent#784 reverses ent#523: most visits start new work, so resuming cost + // two actions every time. Continuity is one click away in the tab strip. + const landed = agentLanding({ + agentName: 'a', + threads: [ + main(), + chat('c1', { last_message_at: '2026-09-07T09:00:00Z' }), + chat('c2', { last_message_at: '2026-09-07T12:00:00Z' }), + ], + }) + expect(landed).toEqual({ agentName: 'a', sessionId: null }) + }) + + it('lands a first-time visitor on a new chat, minting no row', () => { + // AC 4: the landing itself creates nothing server-side. The helper is pure + // and returns a null session; the row is born on the first send + // (`newThread`, ent#451), so repeated visits accumulate no empty chats. + expect(agentLanding({ agentName: 'a', threads: [main()] })) + .toEqual({ agentName: 'a', sessionId: null }) + expect(agentLanding({ agentName: 'a', threads: [] })) + .toEqual({ agentName: 'a', sessionId: null }) + }) + + it('honours a lastOpenSessionId that is still a live chat of this agent', () => { + // The ent#621 seam: the agent-switch keys return you to the chat you last + // had open with that agent since the Workspace loaded. + const landed = agentLanding({ + agentName: 'a', + threads: [main(), chat('c1', { last_message_at: '2026-09-07T09:00:00Z' })], + lastOpenSessionId: 'c1', + }) + expect(landed).toEqual({ agentName: 'a', sessionId: 'c1' }) + }) + + it('accepts a session_id-shaped row for that id too', () => { + const landed = agentLanding({ + agentName: 'a', + threads: [{ session_id: 's9', agent_name: 'a' }], + lastOpenSessionId: 's9', + }) + expect(landed.sessionId).toBe('s9') + }) + + it('falls back to the new-chat rule for an id it cannot vouch for', () => { + // Validated against the principal's OWN thread list, so an archived chat, + // another agent's chat, a room, or an id that simply is not there all fall + // back rather than landing somewhere they should not (the #3140 class). + const threads = [ main(), - chat('old', { archived_at: 'x', last_message_at: '2026-09-07T23:00:00Z' }), - ], 'a') - expect(landed.id).toBe('main') + chat('arch', { archived_at: 'x', last_message_at: '2026-09-07T23:00:00Z' }), + chat('other', { agent_name: 'b', last_message_at: '2026-09-07T23:00:00Z' }), + { id: 'room1', agent_name: 'a', is_room: true }, + ] + for (const id of ['arch', 'other', 'room1', 'nope']) { + expect(agentLanding({ agentName: 'a', threads, lastOpenSessionId: id }).sessionId) + .toBeNull() + } }) - it('returns null when there is nothing for this agent', () => { - expect(landingThread([chat('other', { agent_name: 'b' })], 'a')).toBeNull() - expect(landingThread([], 'a')).toBeNull() - expect(landingThread([main()], '')).toBeNull() + it('returns null only for a caller that named no agent', () => { + expect(agentLanding({ agentName: '', threads: [main()] })).toBeNull() + expect(agentLanding({ agentName: null })).toBeNull() + expect(agentLanding()).toBeNull() }) it('is the SAME rule the ?agent= deep link uses', () => { // Two answers to "which chat do I land in" is how a deep link and a // sidebar click put a first-time visitor in different places. - const threads = [main()] + const threads = [main(), chat('c2', { last_message_at: '2026-09-07T12:00:00Z' })] const agents = [{ name: 'a' }] - expect(resolveAgentLanding({ agent: 'a', agents, threads }).sessionId).toBe('main') + expect(resolveAgentLanding({ agent: 'a', agents, threads }).sessionId).toBeNull() }) }) diff --git a/src/frontend/tests/unit/workspaceAgentLanding.spec.js b/src/frontend/tests/unit/workspaceAgentLanding.spec.js index a127e2c24..19b8f1cba 100644 --- a/src/frontend/tests/unit/workspaceAgentLanding.spec.js +++ b/src/frontend/tests/unit/workspaceAgentLanding.spec.js @@ -22,9 +22,11 @@ const threads = [ ] describe('resolveAgentLanding', () => { - it('resumes the most recent thread with that agent', () => { + it('starts a new chat even when threads with that agent exist (ent#784)', () => { + // ent#784 reversed ent#523's resume-by-default: opening an agent means + // starting new work. The deep link and the sidebar row share one rule. expect(resolveAgentLanding({ agent: 'scribe', agents, threads })) - .toEqual({ agentName: 'scribe', sessionId: 's2' }) + .toEqual({ agentName: 'scribe', sessionId: null }) }) it('opens a fresh thread when the agent has none yet', () => { @@ -32,7 +34,7 @@ describe('resolveAgentLanding', () => { .toEqual({ agentName: 'auditor', sessionId: null }) }) - it('?new=1 starts fresh even when a thread exists', () => { + it('?new=1 still starts fresh — now the same answer as without it', () => { expect(resolveAgentLanding({ agent: 'scribe', forceNew: true, agents, threads })) .toEqual({ agentName: 'scribe', sessionId: null }) }) @@ -50,9 +52,11 @@ describe('resolveAgentLanding', () => { }) it('tolerates a thread list that uses session_id instead of id', () => { + // ent#784: the shape no longer changes the answer for this door — a deep + // link carries no session memory, so it always resolves to a fresh chat. const alt = [{ session_id: 'sX', agent_name: 'scribe' }] expect(resolveAgentLanding({ agent: 'scribe', agents, threads: alt })) - .toEqual({ agentName: 'scribe', sessionId: 'sX' }) + .toEqual({ agentName: 'scribe', sessionId: null }) }) it('survives absent inputs rather than throwing at the landing', () => { From c2d2a2a681a37913c91e01fd2743c519ef5abadc Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Sun, 4 Oct 2026 18:11:02 -0400 Subject: [PATCH 02/22] feat(workspace): land on the new chat, and focus only when asked (abilityai/trinity-enterprise#784) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../components/portal/PortalConversation.vue | 18 ++- .../src/components/portal/PortalRoom.vue | 4 +- .../src/components/portal/portalDrafts.js | 13 +- src/frontend/src/views/Portal.vue | 111 +++++++++-------- src/frontend/tests/unit/portalDrafts.spec.js | 12 +- .../unit/portalNewChatFocus.mount.spec.js | 114 ++++++++++++++++++ .../tests/unit/workspaceNewChat.spec.js | 54 +++++++-- 7 files changed, 253 insertions(+), 73 deletions(-) create mode 100644 src/frontend/tests/unit/portalNewChatFocus.mount.spec.js diff --git a/src/frontend/src/components/portal/PortalConversation.vue b/src/frontend/src/components/portal/PortalConversation.vue index b38ed9d74..e08c05aa6 100644 --- a/src/frontend/src/components/portal/PortalConversation.vue +++ b/src/frontend/src/components/portal/PortalConversation.vue @@ -877,7 +877,7 @@ import { useConversationAnchor } from '@/composables/useConversationAnchor' import { useRoute, useRouter } from 'vue-router' import { useComposerDraft } from '@/composables/useComposerDraft' import { usePortalDraftsStore } from '@/stores/portalDrafts' -import { draftKeyFor, shouldFocusOnRestore } from './portalDrafts' +import { draftKeyFor, shouldAutoFocusComposer } from './portalDrafts' import PortalTypeahead from './PortalTypeahead.vue' import PortalJumpToLatest from './PortalJumpToLatest.vue' import PortalAsks from './PortalAsks.vue' @@ -966,6 +966,11 @@ const props = defineProps({ // This is the second bit that makes them distinguishable, mirroring the // backend's `new_thread`. newChat: { type: Boolean, default: false }, + // ent#784 — WHY this instance is showing a fresh composer, which decides + // whether it may focus itself. `always` = the person just made a gesture that + // asks for a composer. `fine-pointer` = they LANDED here (opening an agent, a + // deep link), so focus only where it cannot summon an on-screen keyboard. + focusOnMount: { type: String, default: 'always' }, prefill: { type: String, default: '' }, // ent#610 round 8: `{ sessionId, messageId, excerpt }` from the Inbox arrow. // Shown only on the chat it belongs to; the shell clears it on `reply-done`. @@ -1044,6 +1049,13 @@ watch(() => props.threads, (list) => { }) function focusComposer() { textarea.value?.focus() } +// ent#784: the landing doors pass `fine-pointer`, so opening an agent on a +// phone shows a fresh composer without the keyboard sliding up over it. A +// gesture door passes `always` and is unaffected. +function mayAutoFocus() { + if (props.focusOnMount !== 'fine-pointer') return true + return shouldAutoFocusComposer(typeof window !== 'undefined' ? window.matchMedia?.bind(window) : null) +} const loadingHistory = ref(false) // #2163 — "a verdict exists for this thread's history" (mirrors `onMounted`'s @@ -1593,7 +1605,7 @@ onMounted(async () => { // fine pointer only (a phone would get the soft keyboard over the thread). // After the prefill line on purpose: an explicit "Ask about it" replaces the // composer today, and the write-through then makes IT the draft. - if (draftRestored && !props.prefill && shouldFocusOnRestore(typeof window !== 'undefined' ? window.matchMedia?.bind(window) : null)) { + if (draftRestored && !props.prefill && shouldAutoFocusComposer(typeof window !== 'undefined' ? window.matchMedia?.bind(window) : null)) { nextTick(() => { const el = textarea.value if (!el || el.disabled) return @@ -1613,7 +1625,7 @@ onMounted(async () => { // instance: pressing New chat bumps `convGen`, which remounts this // component, so any focus set before the press is thrown away. A disabled // textarea (a live voice call) makes it a no-op by construction. - if (props.newChat) nextTick(focusComposer) + if (props.newChat && mayAutoFocus()) nextTick(focusComposer) } autoGrowAfterUpdate() // `props.prefill` was assigned above; wait for the patch }) diff --git a/src/frontend/src/components/portal/PortalRoom.vue b/src/frontend/src/components/portal/PortalRoom.vue index 66d58e4a3..e87a0cb7d 100644 --- a/src/frontend/src/components/portal/PortalRoom.vue +++ b/src/frontend/src/components/portal/PortalRoom.vue @@ -405,7 +405,7 @@ import { workSignalFromRoom } from './portalRail' import { usePortalFileDrop, attachmentState } from '@/composables/usePortalFileDrop' import { useComposerDraft } from '@/composables/useComposerDraft' import { usePortalDraftsStore } from '@/stores/portalDrafts' -import { draftKeyFor, shouldFocusOnRestore } from './portalDrafts' +import { draftKeyFor, shouldAutoFocusComposer } from './portalDrafts' import { shouldCancelOnEscape, cancelOutcome } from '@/utils/turnCancel' import { useStickToBottom } from '@/composables/useStickToBottom' import { @@ -969,7 +969,7 @@ onMounted(async () => { // end, on a fine pointer only (a phone would get the keyboard over the room). if (isClosed.value) drafts.clear(draftKey.value) else if (draftRestored && !props.prefill - && shouldFocusOnRestore(typeof window !== 'undefined' ? window.matchMedia?.bind(window) : null)) { + && shouldAutoFocusComposer(typeof window !== 'undefined' ? window.matchMedia?.bind(window) : null)) { nextTick(() => { const el = textarea.value if (!el || el.disabled) return diff --git a/src/frontend/src/components/portal/portalDrafts.js b/src/frontend/src/components/portal/portalDrafts.js index 4ab900f67..db61c3c52 100644 --- a/src/frontend/src/components/portal/portalDrafts.js +++ b/src/frontend/src/components/portal/portalDrafts.js @@ -211,11 +211,16 @@ export function reconcileDraftOnKeyChange({ oldKey = null, newKey = null, compos } /** - * Focus the restored composer on a fine pointer only. #2579's New chat focus - * is an explicit gesture; arriving at a drafted chat is not, and on a phone a - * focus pops the soft keyboard over the transcript. + * May the composer take focus by itself? On a fine pointer only — on a phone a + * programmatic focus pops the soft keyboard over the transcript. + * + * Two reasons share this one rule, which is why the name says "auto" rather + * than "on restore": arriving at a drafted chat (ent#657), and LANDING on an + * agent's new chat (ent#784). Neither is something the person asked for in + * those words. An explicit gesture — New chat, ⌘J, the agent picker — does not + * consult this at all and focuses on any pointer (#2579 AC 2). */ -export function shouldFocusOnRestore(matchMedia) { +export function shouldAutoFocusComposer(matchMedia) { try { return typeof matchMedia === 'function' && !!matchMedia('(pointer: fine)')?.matches } catch { diff --git a/src/frontend/src/views/Portal.vue b/src/frontend/src/views/Portal.vue index ada793329..c65c7ebcc 100644 --- a/src/frontend/src/views/Portal.vue +++ b/src/frontend/src/views/Portal.vue @@ -414,6 +414,7 @@ :roster="store.agents" :session-id="pendingSession" :new-chat="startingNewChat" + :focus-on-mount="composerFocusMode" :prefill="prefill" :reply-target="replyTarget" :starred="isStarred('thread', activeSessionId || pendingSession)" @@ -901,7 +902,7 @@ import { import { stageZone } from '@/components/portal/portalBriefingState' import { isNewChatHotkey, resolveAgentLanding, shouldMarkTurnRead, shouldEscapeStage, - landingThread, + agentLanding, agentHasMain, titleSettling, shouldFetchTitleHealth, titleGenerationNotice, TITLE_SETTLE_DELAYS_MS, // ent#557: the SAME sum the sidebar renders, so the tab and the rows cannot @@ -1470,6 +1471,13 @@ const pickerBusy = ref(false) // thread exists (`openThread`, and the send that gets a session id back), so it // can never make a SECOND turn open another thread. const startingNewChat = ref(false) +// ent#784 / #2579 — WHY the composer is being focused, which decides whether it +// may summon a soft keyboard. `always` is a gesture the person just made (New +// chat, ⌘J, the agent picker, switch-agent): they asked for a composer, so the +// keyboard is the answer, not an ambush. `fine-pointer` is a LANDING they did +// not ask for in those words (opening an agent, an `?agent=` link), where a +// keyboard sliding up over a phone is exactly the "unprompted" the issue names. +const composerFocusMode = ref('always') function newChat() { if (guardLeaveCall(() => newChat())) return // ent#551: ask, then leave @@ -1728,13 +1736,16 @@ function openRoom(roomId) { // where a row carrying an unread badge opened the unread chat instead. The // count still shows on the row; the page's Overview lists the chats it belongs // to, so the conversation is one click further, not lost. -// ent#523: clicking an agent opens the CONVERSATION you were last in, not a -// report about the agent. The `/workspace/a/:name` URL is kept — every existing -// link, and the sidebar row, still route through it — and `landOnAgent` -// swaps it for the thread's own URL once the list is in hand. Landing here -// rather than pushing the thread URL directly is deliberate: the thread list -// may not have loaded yet on a cold deep link, and this way the URL is honest -// at every instant instead of pointing at a chat we have not resolved. +// ent#784: clicking an agent opens a NEW chat with it, not a report about the +// agent and not the chat you were last in. The `/workspace/a/:name` URL is kept +// — every existing link, and the sidebar row, still route through it — and it +// now STAYS: a landed new chat is "new chat with X", which that URL says +// exactly, so a reload, a bookmark or a copied link keeps the agent. The first +// send replaces it with the thread's own `/workspace/c/:id` (`onSessionAdopted`), +// which is the first moment there is a thread to name. Landing here rather than +// pushing a thread URL directly is deliberate: the thread list may not have +// loaded yet on a cold deep link, and this way the URL is honest at every +// instant instead of pointing at a chat we have not resolved. function openAgentPage(name) { if (!name) return // ent#551 QA: the rail's agent row. It pushed a route with no guard, so the @@ -1747,49 +1758,46 @@ function openAgentPage(name) { router.push(`/workspace/a/${encodeURIComponent(name)}`) } -// ent#523 — turn `/workspace/a/:name` into the chat to land in. +// ent#784 — turn `/workspace/a/:name` into the chat to land in. // Named `landOnAgent` to stay clear of the pure `resolveAgentLanding` above, // which answers the same question for the `?agent=` deep link; both defer to -// `landingThread` so there is ONE rule for which chat you land in. +// `agentLanding` so there is ONE rule for which chat you land in. // -// `landingThread` is the rule (most recently active, Main as the floor); it is -// pure and lives in portalUtils so it is testable without a mount. With no -// chats at all the agent's Main has not been minted yet, so the shell asks the -// server for the list — which is what mints it — and lands on what comes back. -// A failure leaves the caller on the agent URL with the stage's own error -// states, rather than dropping them somewhere unrelated. -async function landOnAgent(name) { +// `agentLanding` is the rule, and since ent#784 its answer is a NEW chat unless +// the caller hands it a session to return to (the ent#621 seam). So this is now +// SYNCHRONOUS: there is nothing to fetch before it can answer, and the awaited +// `ensureMainListed` that used to sit on this path is gone from it. The pinned +// Main is still minted on the first visit — `watch(activeAgentName)` below owns +// that promise (ent#523) — it simply no longer blocks the landing. +// +// No `escapeStage` / no push to bare `/workspace`: staying on this URL is what +// makes the landed chat reloadable and linkable. +function landOnAgent(name) { if (!name) return - activeAgentName.value = name - const target = landingThread(threads.value, name) - if (target) { openThread(target); return } - try { - // #2579: this branch used to destructure `{ sessions }` off the store's - // return value — which is an ARRAY (`data.sessions || []`). `sessions` was - // therefore always `undefined`, `landingThread` always missed, and the - // repair branch this comment describes never once ran: a first-time - // visitor always fell through to a fresh chat, and the Main the call had - // just minted server-side never reached the screen. It now goes through - // `ensureMainListed`, which does the same per-agent read AND folds the - // result into `threads` — one seam for "the list must show this agent's - // Main", shared with the watcher below. - await ensureMainListed(name) - // The watcher fires on the route param AND on the thread list arriving, so - // two landings can be in flight at once on a cold deep link: the first - // misses (no threads yet) and goes to the network, the second finds the - // list and navigates. Without this the first one's late resolution - // navigates too — moving the person off a chat they have since chosen. The - // route is the authority; if it no longer names this agent, this landing - // has been overtaken and has nothing to say. Re-checked HERE, after the - // await: the ensure spends two round trips where the old code spent one. - if (activeAgentPageName.value !== name) return - const landed = landingThread(threads.value, name) - if (landed) { openThread(landed); return } - } catch { - // Fall through: a fresh chat is a better answer than a dead stage. + // ent#551 class: `activeAgentName` feeds `convKey`, so writing it remounts + // the conversation and ends a live call without a word. The click doors ask + // first; back/forward and a typed `/workspace/a/:name` come through here, and + // did not. Ask before touching anything. + if (guardLeaveCall(() => landOnAgent(name))) return + const landing = agentLanding({ agentName: name, threads: threads.value }) + if (!landing) return + if (landing.sessionId) { + const row = threads.value.find((t) => (t.id || t.session_id) === landing.sessionId) + if (row) { openThread(row); return } } - if (activeAgentPageName.value !== name) return - newChatWithAgent(name) + // Idempotent: the watcher fires on the route param AND on the thread list + // arriving, and this landing no longer navigates away — so without this the + // second fire would bump `convGen` and remount an unsent chat, throwing away + // what the person had started typing. + if (activeAgentName.value === name && startingNewChat.value && !pendingSession.value) return + unreachableAgent.value = null + activeAgentName.value = name + activeRoomId.value = null + // ent#784: landing is not a gesture, so the composer focuses only where that + // cannot summon an on-screen keyboard (see `focusOnMount` below). + composerFocusMode.value = 'fine-pointer' + startingNewChat.value = true + pendingSession.value = null; prefill.value = ''; convGen.value++ } // #2579 — make sure this agent's pinned Main is IN the list on screen. @@ -1867,6 +1875,10 @@ function newChatWithAgent(name) { if (guardLeaveCall(() => newChatWithAgent(name))) return // ent#534/ent#551: ask, then leave unreachableAgent.value = null activeAgentName.value = name + // ent#784: every door into here is a gesture the person just made — the New + // chat button, ⌘J, the agent picker, switch-agent — so the composer focuses + // on any pointer. Only the landing doors hold back (#2579 AC 2 kept). + composerFocusMode.value = 'always' // ent#451: this function has always MEANT a fresh chat — it clears // `pendingSession` — but a null session id is also what an unresolved thread // looks like, so the conversation could not tell the two apart and loaded the @@ -2406,7 +2418,12 @@ function resolveAgentQuery() { // ambiguity ent#451 exists to remove — it also means "unresolved". AND-ed // with the landing so a `?new=1` that still resolved a thread (it cannot // today, but the two are independent functions) never claims a fresh start. - startingNewChat.value = forceNew && !landing.sessionId + // ent#784: `forceNew` no longer changes the answer — a landing IS a new chat + // unless the resolver handed back a session — so the intent follows the + // resolved landing rather than the query flag. + startingNewChat.value = !landing.sessionId + // A deep link is a landing, not a gesture (see `composerFocusMode`). + composerFocusMode.value = 'fine-pointer' // The bare path drops the query, so this replace is ALSO the strip on this // branch — recorded so the `finally` does not start a competing navigation. if (landing.sessionId) { landingReplaced = true; router.replace(`/workspace/c/${landing.sessionId}`) } diff --git a/src/frontend/tests/unit/portalDrafts.spec.js b/src/frontend/tests/unit/portalDrafts.spec.js index 16154e882..d93b759c0 100644 --- a/src/frontend/tests/unit/portalDrafts.spec.js +++ b/src/frontend/tests/unit/portalDrafts.spec.js @@ -17,7 +17,7 @@ import { draftKeyFor, threadKey, roomKey, newChatKey, isNewChatKey, agentOfNewChatKey, hasDraftText, draftStorageKey, normalizeDraftsMap, boundDrafts, loadDrafts, persistDraft, removeDraftsBucket, - agentsWithDrafts, reconcileDraftOnKeyChange, shouldFocusOnRestore, + agentsWithDrafts, reconcileDraftOnKeyChange, shouldAutoFocusComposer, } from '../../src/components/portal/portalDrafts' import { usePortalDraftsStore } from '../../src/stores/portalDrafts' @@ -195,12 +195,12 @@ describe('reconcileDraftOnKeyChange — the four in-instance transitions', () => }) }) -describe('shouldFocusOnRestore — a fine pointer only', () => { +describe('shouldAutoFocusComposer — a fine pointer only', () => { it('true on a fine pointer, false on coarse, false with no matchMedia', () => { - expect(shouldFocusOnRestore((q) => ({ matches: q === '(pointer: fine)' }))).toBe(true) - expect(shouldFocusOnRestore(() => ({ matches: false }))).toBe(false) - expect(shouldFocusOnRestore(undefined)).toBe(false) - expect(shouldFocusOnRestore(() => { throw new Error('no') })).toBe(false) + expect(shouldAutoFocusComposer((q) => ({ matches: q === '(pointer: fine)' }))).toBe(true) + expect(shouldAutoFocusComposer(() => ({ matches: false }))).toBe(false) + expect(shouldAutoFocusComposer(undefined)).toBe(false) + expect(shouldAutoFocusComposer(() => { throw new Error('no') })).toBe(false) }) }) diff --git a/src/frontend/tests/unit/portalNewChatFocus.mount.spec.js b/src/frontend/tests/unit/portalNewChatFocus.mount.spec.js new file mode 100644 index 000000000..79d721693 --- /dev/null +++ b/src/frontend/tests/unit/portalNewChatFocus.mount.spec.js @@ -0,0 +1,114 @@ +// @vitest-environment jsdom +/** + * trinity-enterprise#784 — who gets the caret when a fresh composer mounts. + * + * Opening an agent now lands on a NEW chat, which means the #2579 mount focus + * fires on a path the person did not ask for in those words. On a phone that + * slides the on-screen keyboard up over the transcript — the "unprompted" + * the issue names. So the focus is gated on WHY this instance mounted: + * + * `always` — a gesture (New chat, ⌘J, the agent picker, switch-agent). + * They asked for a composer; focus on any pointer. + * `fine-pointer` — a landing (`landOnAgent`, the `?agent=` deep link). + * Focus only where it cannot summon a keyboard. + * + * Mounted (#2918), not read as source: the predicate and the prop default are + * the whole behaviour, and an inverted one reads byte-identically to a regex. + * Recipe from `portalThreadMissing.mount.spec.js`. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' +import { shallowMount, flushPromises } from '@vue/test-utils' +import { createPinia, setActivePinia } from 'pinia' +import { nextTick } from 'vue' + +vi.mock('axios', () => { + const mk = () => ({ + get: vi.fn(() => Promise.resolve({ data: {} })), post: vi.fn(() => Promise.resolve({ data: {} })), + put: vi.fn(), patch: vi.fn(), delete: vi.fn(), + interceptors: { request: { use: vi.fn() }, response: { use: vi.fn() } }, + defaults: { headers: { common: {} } }, + }) + return { + default: Object.assign( + { get: vi.fn(() => Promise.resolve({ data: {} })), post: vi.fn(), put: vi.fn(), delete: vi.fn(), create: mk }, + { interceptors: { request: { use: vi.fn() }, response: { use: vi.fn() } }, + defaults: { headers: { common: {} } } }, + ), + } +}) + +import PortalConversation from '@/components/portal/PortalConversation.vue' + +globalThis.ResizeObserver = globalThis.ResizeObserver || class { observe() {} unobserve() {} disconnect() {} } + +let wrapper +beforeEach(() => { + document.body.innerHTML = '' + setActivePinia(createPinia()) +}) +afterEach(() => { + wrapper?.unmount(); wrapper = null + delete window.matchMedia +}) + +// The pointer is read through `window.matchMedia`, so it has to be stubbed +// BEFORE the mount — the gate runs in `onMounted`. +function pointer(kind) { + window.matchMedia = vi.fn((q) => ({ matches: kind === 'fine' && q === '(pointer: fine)' })) +} + +async function mountFresh(props = {}) { + // The mutation check from the 2026-10-01 learnings: focus has to be provably + // somewhere else first, or "the textarea is focused" can pass on a harness + // that focuses it for unrelated reasons. + expect(document.activeElement).toBe(document.body) + wrapper = shallowMount(PortalConversation, { + props: { agent: { name: 'scout', playbooks: [] }, sessionId: null, newChat: true, ...props }, + attachTo: document.body, + }) + await flushPromises() + await nextTick() + return wrapper.find('textarea').element +} + +describe('ent#784 — a landing does not summon the keyboard', () => { + it('a landing mount on a COARSE pointer leaves the composer unfocused', async () => { + pointer('coarse') + const el = await mountFresh({ focusOnMount: 'fine-pointer' }) + expect(document.activeElement).not.toBe(el) + }) + + it('a landing mount on a FINE pointer focuses the composer', async () => { + // Desktop AC: open an agent and start typing, no click first. + pointer('fine') + const el = await mountFresh({ focusOnMount: 'fine-pointer' }) + expect(document.activeElement).toBe(el) + }) + + it('a phone with no matchMedia at all is treated as coarse', async () => { + const el = await mountFresh({ focusOnMount: 'fine-pointer' }) + expect(document.activeElement).not.toBe(el) + }) +}) + +describe('#2579 AC 2 — an explicit gesture still focuses, on any pointer', () => { + it.each([['fine'], ['coarse']])('a gesture mount on a %s pointer focuses', async (kind) => { + pointer(kind) + const el = await mountFresh({ focusOnMount: 'always' }) + expect(document.activeElement).toBe(el) + }) + + it('defaults to the gesture reading, so a caller that says nothing keeps #2579', async () => { + pointer('coarse') + const el = await mountFresh() + expect(document.activeElement).toBe(el) + }) +}) + +describe('ent#784 — the gate is about a FRESH composer only', () => { + it('a mount that is not a new chat never auto-focuses, fine pointer or not', async () => { + pointer('fine') + const el = await mountFresh({ newChat: false, focusOnMount: 'always' }) + expect(document.activeElement).not.toBe(el) + }) +}) diff --git a/src/frontend/tests/unit/workspaceNewChat.spec.js b/src/frontend/tests/unit/workspaceNewChat.spec.js index 4fd169228..673fcea06 100644 --- a/src/frontend/tests/unit/workspaceNewChat.spec.js +++ b/src/frontend/tests/unit/workspaceNewChat.spec.js @@ -41,8 +41,10 @@ describe('ent#451 — ?new=1 asks for a fresh thread', () => { it('the landing function already honoured it', () => { expect(resolveAgentLanding({ agent: 'sage', forceNew: true, agents, threads })) .toEqual({ agentName: 'sage', sessionId: null }) + // ent#784 made `forceNew` redundant rather than wrong: a plain landing now + // resolves to a fresh chat too, so the flag can never resume a thread. expect(resolveAgentLanding({ agent: 'sage', forceNew: false, agents, threads }).sessionId) - .toBe('ps_old') + .toBeNull() }) it('and the view now carries that same answer into the SEND', () => { @@ -52,7 +54,7 @@ describe('ent#451 — ?new=1 asks for a fresh thread', () => { // `new_thread: false` — resuming the thread the user asked to leave. const src = codeOnly(PORTAL()) expect(src).toMatch(/const forceNew = !!route\.query\.new/) - expect(src).toMatch(/startingNewChat\.value = forceNew && !landing\.sessionId/) + expect(src).toMatch(/startingNewChat\.value = !landing\.sessionId/) }) it('reads route.query.new exactly once, so the two consumers cannot drift', () => { @@ -115,7 +117,11 @@ describe('ent#451 — only the first turn of a new chat opens a thread', () => { // variable is one refactor away from being wrong. const src = codeOnly(PORTAL()) const nulls = (src.match(/pendingSession\.value = null/g) || []).length - const settles = (src.match(/startingNewChat\.value = (false|forceNew)/g) || []).length + // ent#784 widened the spellings rather than keeping a dead `forceNew` in + // the assignment to dodge this count: the deep link now settles on the + // RESOLVED landing, and the two new-chat doors settle on `true`. The case + // counts settles, not spellings. + const settles = (src.match(/startingNewChat\.value = (false|true|!landing\.sessionId)/g) || []).length expect(settles).toBeGreaterThanOrEqual(nulls) }) }) @@ -148,7 +154,11 @@ describe('#2579 — the fresh chat is visible and focused', () => { // New chat bumps `convGen`, which remounts the conversation, so focus set // before the press is thrown away. It has to happen in `onMounted`. const src = codeOnly(CONV()) - expect(src).toMatch(/if \(props\.newChat\) nextTick\(focusComposer\)/) + expect(src).toMatch(/if \(props\.newChat && mayAutoFocus\(\)\) nextTick\(focusComposer\)/) + // ent#784: the gate is a MODE, not a blanket pointer check — a gesture door + // (`always`, the default) keeps #2579 AC 2 on every pointer, and only a + // landing (`fine-pointer`) consults the media query. + expect(src).toMatch(/if \(props\.focusOnMount !== 'fine-pointer'\) return true/) expect(src).toMatch(/function focusComposer\(\) \{ textarea\.value\?\.focus\(\) \}/) // Membership, not the exact shape: #2559 adds `startVoiceCall` beside it for // the Talk door. What this case needs is that the parent can still reach @@ -168,14 +178,36 @@ describe('#2579 — the fresh chat is visible and focused', () => { }) describe('#2579 — Main is listed from the first visit', () => { - it('landOnAgent no longer destructures an array, and re-checks the route AFTER the ensure', () => { - // `store.fetchSessions` returns `data.sessions || []` — an ARRAY. The old - // `const { sessions } = await …` was always undefined, so the repair - // branch never ran once. The ensure awaits two round trips where the old - // code awaited one, so the overtake guard has to sit between them. + it('ent#784 — landOnAgent is synchronous and never waits on the ensure', () => { + // The awaited `ensureMainListed` used to sit on the landing path, with an + // overtake guard after it because two landings could be in flight at once. + // ent#784's rule needs nothing from the network, so the whole await — and + // with it that race — is gone from this function. The pinned Main is still + // minted on the first visit; the watcher below owns it (T1). const src = codeOnly(PORTAL()) - expect(src).not.toMatch(/const \{ sessions \} = await store\.fetchSessions/) - expect(src).toMatch(/await ensureMainListed\(name\)[\s\S]{0,400}?if \(activeAgentPageName\.value !== name\) return/) + const fn = src.match(/function landOnAgent\(name\) \{[\s\S]*?\n\}/) + expect(fn).not.toBeNull() + expect(fn[0]).not.toMatch(/await/) + expect(fn[0]).not.toMatch(/ensureMainListed/) + expect(src).not.toMatch(/async function landOnAgent/) + // ent#551 class (decision #13): back/forward and a typed URL reach the + // landing without passing a click door, and `activeAgentName` feeds + // `convKey` — so the call has to be guarded BEFORE anything is written. + expect(fn[0]).toMatch(/^function landOnAgent\(name\) \{\s*if \(!name\) return\s*if \(guardLeaveCall/) + // T4: the landed chat keeps `/workspace/a/:name`, so a reload or a copied + // link still names the agent. No escape to bare `/workspace` here. + expect(fn[0]).not.toMatch(/escapeStage|router\.push/) + }) + + it('ent#784 — the landing mints no row, and re-firing does not remount an unsent chat', () => { + // AC 4: landing creates nothing server-side — it only raises the intent, + // and the thread is born on the first send (`new_thread`, above). The + // watcher fires on the route param AND on the thread list arriving, and + // this landing no longer navigates away, so the second fire must be a + // no-op or it throws away what the person had started typing. + const fn = codeOnly(PORTAL()).match(/function landOnAgent\(name\) \{[\s\S]*?\n\}/)[0] + expect(fn).not.toMatch(/store\.fetchSessions|createSession|refreshThreads/) + expect(fn).toMatch(/if \(activeAgentName\.value === name && startingNewChat\.value && !pendingSession\.value\) return[\s\S]*?convGen\.value\+\+/) }) it('the ensure is a deduped, capped promise per agent — and both maps die at sign-out', () => { From c79b3e16495883be3387cab112f85cc0ba22ef91 Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Sun, 4 Oct 2026 18:11:53 -0400 Subject: [PATCH 03/22] feat(workspace): document the new-chat landing rule (abilityai/trinity-enterprise#784) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- docs/memory/architecture/workspace.md | 10 +++- .../workspace-agents-at-the-centre.md | 46 ++++++++++++++++--- docs/memory/requirements/core-agent.md | 13 ++++-- 3 files changed, 58 insertions(+), 11 deletions(-) diff --git a/docs/memory/architecture/workspace.md b/docs/memory/architecture/workspace.md index a903885bd..c7363410e 100644 --- a/docs/memory/architecture/workspace.md +++ b/docs/memory/architecture/workspace.md @@ -997,9 +997,15 @@ SIBLING of the rail, never a rail tab (ruled 2026-09-05); **ent#547 reversed tha door, the room restriction and why the 2026-09-05 objections are answered rather than dropped. Both read one payload through `composables/usePortalAgentPage.js`. Canvas, Files and recent work are **not** duplicated here — they have been rail tabs since -ent#475/#525. `portalUtils.js::landingThread` is the one rule for which chat you land in, +ent#475/#525. `portalUtils.js::agentLanding` is the one rule for which chat you land in, and the `?agent=` deep link's `resolveAgentLanding` defers to it so the two entry points -cannot disagree. Main is named by its role in both the tab strip and the header and is +cannot disagree. Since **ent#784** that rule answers "a new, empty chat" (it replaced +ent#523's most-recently-active `landingThread`); only a validated `lastOpenSessionId` +— the ent#621 seam, checked against the principal's own thread list — overrides it. +`landOnAgent` is therefore synchronous, keeps `/workspace/a/:name`, and guards the +call before writing `activeAgentName`; the composer's mount focus is gated by a +`focusOnMount` mode (`always` for a gesture, `fine-pointer` for a landing) so an +unasked-for landing cannot raise a soft keyboard. Main is named by its role in both the tab strip and the header and is not renameable; an archived chat stays a tab (ruled “becomes the newest tab”), though you never LAND in one by default; an unused Main is filtered from the **sidebar** only (a projection, not a filter on `threads`, because the strip must show Main from the first visit). diff --git a/docs/memory/feature-flows/workspace-agents-at-the-centre.md b/docs/memory/feature-flows/workspace-agents-at-the-centre.md index 35d2307fc..61b457f4a 100644 --- a/docs/memory/feature-flows/workspace-agents-at-the-centre.md +++ b/docs/memory/feature-flows/workspace-agents-at-the-centre.md @@ -128,8 +128,9 @@ it attaches to the chat of the turn that raised it — see `/workspace/inbox` before the stage resolves (the route is captured before the first await and the replace is awaited inside the `try`, so no conversation flashes). Every explicit target still wins, and bare `/workspace` keeps its meaning as the new-chat -stage `newChatWithAgent` → `escapeStage` lands on. Clicking an agent is unchanged: it -opens the conversation `landingThread` picks. An addressed report with no chat of its +stage `newChatWithAgent` → `escapeStage` lands on. Clicking an agent opens a NEW chat +with it (ent#784) and STAYS on `/workspace/a/:name`, which says exactly that — so a +reload or a copied link keeps the agent. An addressed report with no chat of its addressee joins the agent-initiated things that land in Main, at publish time (`report_service.resolve_report_session`). @@ -200,8 +201,8 @@ rule is a plain function (the ent#392 precedent): | Function | Rule | |---|---| | `agentChatTabs` | Main first, then recency; an archive stays a tab (ruled “becomes the newest tab”); Main is labelled by its **role** | -| `landingThread` | most recently active, **Main as the floor**; never an archived chat | -| `resolveAgentLanding` | the `?agent=` deep link, delegating to `landingThread` so there is one answer | +| `agentLanding` | **a new, empty chat** (ent#784, replacing ent#523's `landingThread`); the one exception is a `lastOpenSessionId`, honoured only when it still names a live, unarchived chat of this agent | +| `resolveAgentLanding` | the `?agent=` deep link, delegating to `agentLanding` so there is one answer | | `orderRosterAgents` | most recent collaboration, then name; `primaryName` is ent#491's seam | | `agentPreview` | the newest chat's **title** — the sidebar list carries no message content (#2198), so a body preview would reinstate the N+1 | | `composerAvailabilityNotice` | what the composer says for a stopped/unavailable agent | @@ -213,9 +214,18 @@ rule is a plain function (the ent#392 precedent): about which chat you are in. - **A tab is not a landing.** An archived chat stays in the strip — the operator ruled it "becomes the newest tab", and hiding what the system line just pointed - at is exactly where the person looks next — but `landingThread` never puts you + at is exactly where the person looks next — but `agentLanding` never puts you in one without asking. Two rules, two questions: where can I go, and where am - I put. + I put. Since ent#784 the landing answer is "a new chat", so the strip is the + ONLY way back into an old one — which is the point: continuity is one click, + starting fresh is zero. +- **Two doors RESOLVE, the rest ASSERT.** Only `landOnAgent` (the sidebar/rail + row, back/forward, a typed URL) and `resolveAgentLanding` (`?agent=`) ask + `agentLanding` where to land. New chat, ⌘J, the agent picker and switch-agent + do not call it at all — they already mean "fresh". That split is also the + focus rule: a gesture may focus the composer on any pointer (#2579 AC 2), a + landing only on a fine one (ent#784), because a landing nobody asked for in + those words must not slide a keyboard up over a phone. - **An unused Main is filtered from the sidebar only.** It exists for every pair the moment an agent is opened; the tab strip must still show it from the first visit, so this is a projection for one consumer (`sidebarThreads`), not a @@ -468,3 +478,27 @@ Stated so the narrowing is never inferred later from the fact that it merged `OverflowTabs` re-measures on `ResizeObserver` - ent#491's roster ordering (incubating) — a deterministic order ships and leaves the seam + +## Changed by ent#784 (2026-10-04) + +Opening an agent lands on a **new, empty chat**, not the chat you were most +recently active in. `landingThread` is deleted; `agentLanding` is the rule, and +both resolving doors call it. Three consequences worth knowing: + +- **Nothing is minted by landing.** The helper is pure and returns a null + session; the row is born on the first send (`newThread`, ent#451). An unused + Main is still kept out of the chat list by `sidebarThreadsOf`, so repeated + visits accumulate no visible empty chats. +- **`landOnAgent` is synchronous** and no longer awaits `ensureMainListed`. The + pinned Main is still minted on the first visit — `watch(activeAgentName)` owns + that (ent#523) — it just no longer blocks the landing. `landOnAgent` also asks + `guardLeaveCall` first now, so back/forward can no longer end a live call + silently (the ent#551 class). +- **`focusOnMount`** (`always` | `fine-pointer`) carries WHY the composer + mounted, which decides whether it may focus itself. `shouldFocusOnRestore` is + renamed `shouldAutoFocusComposer` — two reasons, one rule. + +Still open above this: ent#621 (agent-switch keys) supplies the +`lastOpenSessionId` the helper already accepts; ent#577 (Spotlight) is a third +caller of the same seam. Backend adoption of an empty Main on `new_thread=True` +is a follow-up, not done here. diff --git a/docs/memory/requirements/core-agent.md b/docs/memory/requirements/core-agent.md index 88e809544..1531d5a2a 100644 --- a/docs/memory/requirements/core-agent.md +++ b/docs/memory/requirements/core-agent.md @@ -2311,9 +2311,16 @@ issue if it's ever wanted. Also deferred: `data.json` caching/streaming. **One page** - `/workspace/a/:agentName` keeps its URL and resolves to a chat. - `portalUtils.js::landingThread` is the rule — most recently active, Main as - the floor — and the `?agent=` deep link's `resolveAgentLanding` defers to it, - so the two entry points cannot land a first-time visitor in different places. + `portalUtils.js::agentLanding` is the rule and the `?agent=` deep link's + `resolveAgentLanding` defers to it, so the two entry points cannot land a + first-time visitor in different places. **ent#784 reversed what the rule + answers**: opening an agent starts a NEW, empty chat (ent#523's + most-recently-active `landingThread` is gone), the URL STAYS on + `/workspace/a/:name` until the first send names a thread, and the composer + takes focus — on a fine pointer for a landing, on any pointer for an explicit + gesture, so a phone never gets an unprompted keyboard. Landing mints no row; + the chat is born on the first send. Returning to a previous chat is the tab + strip, one click. - `PortalAgentPage.vue` is dismantled: stats + the Activity chart to `PortalAgentBand.vue` (always visible); chats / what it can do / reports to `PortalAgentDetails.vue`; Canvas and Files were already rail tabs (ent#475); From b2863a68fcfff215c5362399b7240ab177c356a4 Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Sun, 4 Oct 2026 18:57:01 -0400 Subject: [PATCH 04/22] fix(portal): [I1] a thread-list re-fire must not re-mint over an adopted chat (abilityai/trinity-enterprise#784) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/frontend/src/views/Portal.vue | 24 ++- .../portalAgentLandingRemount.mount.spec.js | 195 ++++++++++++++++++ .../portalUnavailableTargets.mount.spec.js | 6 +- 3 files changed, 220 insertions(+), 5 deletions(-) create mode 100644 src/frontend/tests/unit/portalAgentLandingRemount.mount.spec.js diff --git a/src/frontend/src/views/Portal.vue b/src/frontend/src/views/Portal.vue index c65c7ebcc..bd8f100fe 100644 --- a/src/frontend/src/views/Portal.vue +++ b/src/frontend/src/views/Portal.vue @@ -2331,10 +2331,8 @@ watch([() => route.params.sessionId, () => threads.value.length], () => { // Watched on BOTH the route param and the thread list, for the same reason the // `sessionId` watcher above watches both: on a cold deep link the agent name // arrives long before the threads do, so a param-only watcher would resolve -// against an empty list and always mint a new chat. Re-entrancy is guarded by -// the fact that `landOnAgent` navigates away from this route as soon as it -// succeeds; while it has not, re-running is harmless and idempotent. -watch([activeAgentPageName, () => threads.value.length], ([name]) => { +// against an empty list and always mint a new chat. +watch([activeAgentPageName, () => threads.value.length], ([name], [prevName]) => { if (!name || !store.isClientSignedIn) return // Wait for the roster verdict. Landing before it means `activeAgent` cannot // resolve the name yet, and the "you don't have access" branch would fire for @@ -2347,6 +2345,24 @@ watch([activeAgentPageName, () => threads.value.length], ([name]) => { unreachableAgent.value = name return } + // ent#784: the landing no longer navigates away, so — unlike every earlier + // version of this watcher — it can re-fire with the route unchanged, once per + // thread-list refresh, for as long as the person stays on this URL. The + // landing rule reads nothing from the list, so a list-only re-fire cannot + // change the answer; re-running it would null a session the first send has + // since adopted (`onSessionAdopted` writes the state, then replaces the route + // asynchronously, and itself ends in a `refreshThreads`) and remount an empty + // composer over the thread whose reply is streaming. + // + // Keyed on "the route did not change on this fire", AND on having already + // landed this name — the two have to hold together: + // * the name alone would swallow the cold deep link, whose only fire IS the + // list arriving with the param already in place (the reason above); + // * `pendingSession` must NOT be the key: back/forward from + // `/workspace/c/:id` to `/workspace/a/:name` arrives with a session set + // and has to land fresh. A thread URL carries no `agentName` param, so + // that hop always changes `name` and is never mistaken for a re-fire. + if (name === prevName && activeAgentName.value === name) return landOnAgent(name) }) diff --git a/src/frontend/tests/unit/portalAgentLandingRemount.mount.spec.js b/src/frontend/tests/unit/portalAgentLandingRemount.mount.spec.js new file mode 100644 index 000000000..b804438c7 --- /dev/null +++ b/src/frontend/tests/unit/portalAgentLandingRemount.mount.spec.js @@ -0,0 +1,195 @@ +// @vitest-environment jsdom +/** + * ent#784 [I1] — a thread-list refresh must not re-mint a fresh chat over a live one. + * + * ent#784 made `/workspace/a/:name` LAND on a fresh chat and STAY on that URL. + * The watcher that resolves that route fires on the route param AND on the + * thread list arriving (a cold deep link gets the name long before the + * threads), so once landed, every later list refresh re-fires it with the same + * name. `landOnAgent`'s own idempotency guard keys on `startingNewChat`, which + * the first send clears (`onSessionAdopted`) — so a refresh settling in the + * window between adoption and the route leaving `/a/:name` nulled + * `pendingSession` and bumped `convGen`, remounting an empty composer over the + * thread whose first reply was streaming. `ensureMainListed` ends in + * `refreshThreads()`, and `onSessionAdopted` itself calls it, so that window is + * reached by the ordinary first send, not by a contrived one. + * + * The guard is keyed on "the route did not change on this fire" — deliberately + * NOT on `pendingSession`, because back/forward from `/workspace/c/:id` to + * `/workspace/a/:name` legitimately arrives with a session set and must still + * land fresh (ent#784 T4). + * + * `Portal.vue` is mounted for real (shallow — the harness of + * `portalUnavailableTargets.mount.spec.js`), so the watcher, the guard and the + * conversation's props are the shell's own. + */ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { shallowMount, flushPromises } from '@vue/test-utils' +import { setActivePinia, createPinia } from 'pinia' +import { createRouter, createMemoryHistory } from 'vue-router' + +vi.hoisted(() => { + window.matchMedia = window.matchMedia || ((q) => ({ + matches: false, media: q, addEventListener() {}, removeEventListener() {}, addListener() {}, removeListener() {}, + })) +}) + +vi.mock('@/stores/auth', () => ({ + useAuthStore: () => ({ isAuthenticated: true, authHeader: {}, userEmail: 'me@example.com', logout: vi.fn() }), +})) +vi.mock('axios', () => { + const mk = () => ({ + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), + interceptors: { request: { use: vi.fn() }, response: { use: vi.fn() } }, + defaults: { headers: { common: {} } }, + }) + return { + default: Object.assign( + { get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), create: mk }, + { interceptors: { request: { use: vi.fn() }, response: { use: vi.fn() } }, + defaults: { headers: { common: {} } } }, + ), + } +}) + +import { useClientPortalStore } from '@/stores/clientPortal' +import Portal from '@/views/Portal.vue' + +globalThis.ResizeObserver = globalThis.ResizeObserver || class { observe() {} unobserve() {} disconnect() {} } + +const Blank = { template: '
' } +function makeRouter() { + return createRouter({ + history: createMemoryHistory(), + routes: [ + { path: '/workspace', component: Blank }, + { path: '/workspace/inbox', component: Blank }, + { path: '/workspace/c/:sessionId', component: Blank }, + { path: '/workspace/r/:roomId', component: Blank }, + { path: '/workspace/a/:agentName', component: Blank }, + { path: '/login', component: Blank }, + ], + }) +} + +const T1 = { id: 't1', session_id: 't1', agent_name: 'scout', is_main: true, last_message_at: '2026-09-27T10:00:00Z', unread: 0 } + +let store +function arm(threads = [T1]) { + store = useClientPortalStore() + store.portalToken = 'tok' + store.fetchRoster = vi.fn(async () => { + store.agents = [{ name: 'scout' }, { name: 'sage' }] + store.error = null + store.rosterLoaded = true + }) + store.fetchAllSessions = vi.fn(async () => { store.sessionsFailed = false; return threads }) + store.fetchChatState = vi.fn(async (opts) => (opts ? { state: {}, previews: {} } : {})) + store.fetchSessions = vi.fn(async () => threads) + store.fetchAsks = vi.fn(async () => []) + store.seedAgentRecency = vi.fn() + store.ensureBriefing = vi.fn() + store.loadAgentSuggestions = vi.fn() + store.markChatRead = vi.fn(async () => {}) +} + +async function boot(path) { + const router = makeRouter() + await router.push(path) + await router.isReady() + const w = shallowMount(Portal, { global: { plugins: [router] } }) + await flushPromises(); await flushPromises() + return { w, router } +} + +const conversation = (w) => w.findComponent({ name: 'PortalConversation' }) +// What the conversation is actually showing: the session it was handed, whether +// it is a fresh chat, and the instance identity (a `convGen` bump changes +// `:key`, so a remount is a NEW component instance). +const showing = (w) => { + const c = conversation(w) + return { session: c.props('sessionId'), newChat: c.props('newChat'), agent: c.props('agent')?.name, vm: c.vm } +} + +beforeEach(() => { + localStorage.clear() + setActivePinia(createPinia()) +}) + +describe('ent#784 — landing on an agent page is idempotent against the thread list', () => { + it('a list refresh after the first send keeps the adopted chat (I1)', async () => { + arm() + const { w, router } = await boot('/workspace/a/scout') + expect(showing(w).newChat).toBe(true) + expect(showing(w).session).toBe(null) + + // The first send adopts a real id. `onSessionAdopted` writes the state + // synchronously and THEN replaces the route; stub the replace so the window + // this finding is about — adopted, but still on `/workspace/a/scout` — + // stays open deterministically instead of depending on microtask order. + vi.spyOn(router, 'replace').mockResolvedValue(undefined) + conversation(w).vm.$emit('session-adopted', 'new1') + await flushPromises() + const adopted = showing(w) + expect(adopted.session).toBe('new1') + expect(adopted.newChat).toBe(false) + expect(router.currentRoute.value.path).toBe('/workspace/a/scout') + + // The list refresh that the adoption itself kicks off lands in that window. + store.fetchAllSessions = vi.fn(async () => { + store.sessionsFailed = false + return [T1, { id: 'new1', session_id: 'new1', agent_name: 'scout', last_message_at: '2026-09-28T10:00:00Z', unread: 0 }] + }) + await w.vm.refreshThreads() + await flushPromises() + + const after = showing(w) + expect(after.session).toBe('new1') + expect(after.newChat).toBe(false) + expect(after.vm).toBe(adopted.vm) // no remount + }) + + it('a list refresh while still composing keeps the one unsent chat', async () => { + // The pre-existing guard's case, re-asserted behaviourally: nothing adopted + // yet, so a refresh must not throw away what is being typed either. + arm() + const { w } = await boot('/workspace/a/scout') + const before = showing(w) + store.fetchAllSessions = vi.fn(async () => { + store.sessionsFailed = false + return [T1, { id: 't2', session_id: 't2', agent_name: 'sage', last_message_at: '2026-09-28T10:00:00Z', unread: 0 }] + }) + await w.vm.refreshThreads() + await flushPromises() + expect(showing(w).vm).toBe(before.vm) + expect(showing(w).newChat).toBe(true) + }) + + it('back to the agent page from a chat of the SAME agent still lands fresh (T4)', async () => { + arm() + const { w, router } = await boot('/workspace/a/scout') + await router.push('/workspace/c/t1') + await flushPromises() + expect(showing(w).session).toBe('t1') + expect(showing(w).newChat).toBe(false) + + // Back. Arrives with `pendingSession` set — which is exactly why the guard + // must not key on it. + await router.push('/workspace/a/scout') + await flushPromises() + expect(showing(w).newChat).toBe(true) + expect(showing(w).session).toBe(null) + }) + + it('a cold deep link whose threads arrive after mount still lands', async () => { + // The watcher's reason for watching the list at all: on a cold load the + // route param is already in place, so the ONLY fire is the list arriving — + // with the name unchanged. A guard keyed on the name alone would swallow it. + arm() + const { w } = await boot('/workspace/a/sage') + const seen = showing(w) + expect(seen.agent).toBe('sage') // not the `store.agents[0]` fallback + expect(seen.newChat).toBe(true) + expect(seen.session).toBe(null) + }) +}) diff --git a/src/frontend/tests/unit/portalUnavailableTargets.mount.spec.js b/src/frontend/tests/unit/portalUnavailableTargets.mount.spec.js index 58ef0152e..d2f4d8605 100644 --- a/src/frontend/tests/unit/portalUnavailableTargets.mount.spec.js +++ b/src/frontend/tests/unit/portalUnavailableTargets.mount.spec.js @@ -203,7 +203,11 @@ describe('#3140 — an agent URL that is not shared with you', () => { arm() const { w, router } = await boot('/workspace/a/scout') expect(w.text()).not.toContain("You don't have access to") - expect(router.currentRoute.value.path).toBe('/workspace/c/t1') + expect(conversation(w).exists()).toBe(true) + // ent#784 reversed ent#523's landing: an agent page lands on a FRESH chat + // and stays on its own URL, so the proof that it opened is a conversation + // on screen, not a hop to the most recent thread. + expect(router.currentRoute.value.path).toBe('/workspace/a/scout') }) }) From f7101fee70ebe58b4f85f29d7da2719dc426a920 Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Sun, 4 Oct 2026 19:00:50 -0400 Subject: [PATCH 05/22] fix(portal): [I3] mount cases for decision #13 and the T4 guard, not source-text pins (abilityai/trinity-enterprise#784) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 --- .../portalAgentLandingRemount.mount.spec.js | 95 +++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/src/frontend/tests/unit/portalAgentLandingRemount.mount.spec.js b/src/frontend/tests/unit/portalAgentLandingRemount.mount.spec.js index b804438c7..261eef55d 100644 --- a/src/frontend/tests/unit/portalAgentLandingRemount.mount.spec.js +++ b/src/frontend/tests/unit/portalAgentLandingRemount.mount.spec.js @@ -181,6 +181,31 @@ describe('ent#784 — landing on an agent page is idempotent against the thread expect(showing(w).session).toBe(null) }) + it('one landing mounts one conversation, however often the list moves (T4)', async () => { + // T4's idempotency guard, behaviourally: the watcher fires on the route + // param AND on the list, so a single navigation to an agent page must + // produce exactly ONE fresh conversation — not one per list refresh. + arm() + const { w, router } = await boot('/workspace/a/scout') + await router.push('/workspace/a/sage') + await flushPromises() + const minted = showing(w) + expect(minted.agent).toBe('sage') + expect(minted.newChat).toBe(true) + + await w.vm.refreshThreads() + store.fetchAllSessions = vi.fn(async () => { + store.sessionsFailed = false + return [T1, { id: 't3', session_id: 't3', agent_name: 'scout', last_message_at: '2026-09-29T10:00:00Z', unread: 0 }] + }) + await w.vm.refreshThreads() + await flushPromises() + + expect(showing(w).vm).toBe(minted.vm) // still the one mint + expect(showing(w).agent).toBe('sage') + expect(showing(w).session).toBe(null) + }) + it('a cold deep link whose threads arrive after mount still lands', async () => { // The watcher's reason for watching the list at all: on a cold load the // route param is already in place, so the ONLY fire is the list arriving — @@ -193,3 +218,73 @@ describe('ent#784 — landing on an agent page is idempotent against the thread expect(seen.session).toBe(null) }) }) + +const dialog = (w) => w.findComponent({ name: 'ConfirmDialog' }) + +describe('ent#784 decision #13 — a landing during a live call asks before it writes', () => { + // ent#551 class: back/forward and a typed `/workspace/a/:name` reach the + // landing WITHOUT passing a click door, and `activeAgentName` feeds `convKey` + // — so writing it remounts the conversation and ends the call without a word. + // `landOnAgent` consults `guardLeaveCall` on its first line for that reason. + // Asserted here on STATE (the dialog, the conversation's props, the instance + // identity) rather than on the order of lines in the function body, which is + // all the regex pins in `workspaceNewChat.spec.js` can see. + async function callActiveOnAThread() { + arm() + const { w, router } = await boot('/workspace/a/scout') + await router.push('/workspace/c/t1') + await flushPromises() + conversation(w).vm.$emit('voice-call', { active: true, agentName: 'scout', voiceSessionId: 'v1' }) + await flushPromises() + return { w, router, live: showing(w) } + } + + it('back to the agent page mid-call asks, and writes nothing until it is answered', async () => { + const { w, router, live } = await callActiveOnAThread() + expect(live.session).toBe('t1') + + await router.push('/workspace/a/scout') + await flushPromises() + + expect(dialog(w).props('visible')).toBe(true) + // Nothing written: same chat, same instance, call intact. + const held = showing(w) + expect(held.session).toBe('t1') + expect(held.newChat).toBe(false) + expect(held.vm).toBe(live.vm) + }) + + it('confirming the leave then performs the landing it deferred', async () => { + const { w, router } = await callActiveOnAThread() + await router.push('/workspace/a/scout') + await flushPromises() + // The landing must have been DEFERRED, not merely performed — without the + // ask there is nothing for the confirm to run and this case says nothing. + expect(dialog(w).props('visible')).toBe(true) + expect(showing(w).newChat).toBe(false) + + dialog(w).vm.$emit('confirm') + await flushPromises() + + expect(dialog(w).props('visible')).toBe(false) + const landed = showing(w) + expect(landed.agent).toBe('scout') + expect(landed.newChat).toBe(true) + expect(landed.session).toBe(null) + }) + + it('cancelling leaves the call and the chat exactly as they were', async () => { + const { w, router, live } = await callActiveOnAThread() + await router.push('/workspace/a/scout') + await flushPromises() + + dialog(w).vm.$emit('cancel') + await flushPromises() + + expect(dialog(w).props('visible')).toBe(false) + const after = showing(w) + expect(after.session).toBe('t1') + expect(after.newChat).toBe(false) + expect(after.vm).toBe(live.vm) + }) +}) From 721222366cb39c63f989ff0f5e2e592126d9ae79 Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Mon, 5 Oct 2026 04:48:17 -0400 Subject: [PATCH 06/22] fix(portal): opening an agent lands on its newest draft (abilityai/trinity-enterprise#784) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 c8af3d70. 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:` 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 c8af3d70 on the e2e's own symptom, 2 of 3 failing with `sessionId: null` where the draft was. Co-Authored-By: Claude Opus 5 --- docs/memory/architecture/workspace.md | 17 +- .../workspace-agents-at-the-centre.md | 35 ++- .../src/components/portal/portalDrafts.js | 73 +++++- .../src/components/portal/portalUtils.js | 60 +++-- src/frontend/src/views/Portal.vue | 36 ++- .../unit/portalDraftLanding.mount.spec.js | 208 ++++++++++++++++++ .../tests/unit/portalDraftLanding.spec.js | 164 ++++++++++++++ 7 files changed, 560 insertions(+), 33 deletions(-) create mode 100644 src/frontend/tests/unit/portalDraftLanding.mount.spec.js create mode 100644 src/frontend/tests/unit/portalDraftLanding.spec.js diff --git a/docs/memory/architecture/workspace.md b/docs/memory/architecture/workspace.md index c7363410e..f915c3c85 100644 --- a/docs/memory/architecture/workspace.md +++ b/docs/memory/architecture/workspace.md @@ -999,9 +999,20 @@ dropped. Both read one payload through `composables/usePortalAgentPage.js`. Canvas, Files and recent work are **not** duplicated here — they have been rail tabs since ent#475/#525. `portalUtils.js::agentLanding` is the one rule for which chat you land in, and the `?agent=` deep link's `resolveAgentLanding` defers to it so the two entry points -cannot disagree. Since **ent#784** that rule answers "a new, empty chat" (it replaced -ent#523's most-recently-active `landingThread`); only a validated `lastOpenSessionId` -— the ent#621 seam, checked against the principal's own thread list — overrides it. +cannot disagree. Since **ent#784** that rule replaced ent#523's most-recently-active +`landingThread` with a **precedence**, settled by the operator's 2026-10-05 ruling: a link +naming a CHAT wins (and never reaches the rule); then a validated `lastOpenSessionId` — the +ent#621 seam, checked against the principal's own thread list; then the agent's chat +holding an unsent **draft**, newest `updatedAt` first, where a `new:` winner lands on +a new chat (that key IS where the words are) and a thread winner opens that thread; then +the agent's existing **empty** chat, reused; then a new, empty chat. The drafts arm reads +`portalDrafts.js::isDraftedThread` — the SAME predicate `agentsWithDrafts` lights the +sidebar's agent-row mark from, shared 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 each door (`landOnAgent`, `resolveAgentLanding`), never read +inside the rule, so it stays pure; `?new=1` outranks the whole precedence. Nothing in it +mints a row: the drafted-`new:` and fresh arms are both `sessionId: null`, and the empty +arm opens a row that already exists. `landOnAgent` is therefore synchronous, keeps `/workspace/a/:name`, and guards the call before writing `activeAgentName`; the composer's mount focus is gated by a `focusOnMount` mode (`always` for a gesture, `fine-pointer` for a landing) so an diff --git a/docs/memory/feature-flows/workspace-agents-at-the-centre.md b/docs/memory/feature-flows/workspace-agents-at-the-centre.md index 61b457f4a..65f1f425b 100644 --- a/docs/memory/feature-flows/workspace-agents-at-the-centre.md +++ b/docs/memory/feature-flows/workspace-agents-at-the-centre.md @@ -129,8 +129,10 @@ it attaches to the chat of the turn that raised it — see await and the replace is awaited inside the `try`, so no conversation flashes). Every explicit target still wins, and bare `/workspace` keeps its meaning as the new-chat stage `newChatWithAgent` → `escapeStage` lands on. Clicking an agent opens a NEW chat -with it (ent#784) and STAYS on `/workspace/a/:name`, which says exactly that — so a -reload or a copied link keeps the agent. An addressed report with no chat of its +with it (ent#784) — unless a draft or an existing empty chat is waiting, per the +precedence above — and STAYS on `/workspace/a/:name`, which says exactly that, so a +reload or a copied link keeps the agent. (A drafted THREAD is the one winner that +leaves the URL: it opens that chat, at `/workspace/c/:id`.) An addressed report with no chat of its addressee joins the agent-initiated things that land in Main, at publish time (`report_service.resolve_report_session`). @@ -201,7 +203,7 @@ rule is a plain function (the ent#392 precedent): | Function | Rule | |---|---| | `agentChatTabs` | Main first, then recency; an archive stays a tab (ruled “becomes the newest tab”); Main is labelled by its **role** | -| `agentLanding` | **a new, empty chat** (ent#784, replacing ent#523's `landingThread`); the one exception is a `lastOpenSessionId`, honoured only when it still names a live, unarchived chat of this agent | +| `agentLanding` | a **precedence** (ent#784, replacing ent#523's `landingThread`; operator ruling 2026-10-05): a validated `lastOpenSessionId` (live, unarchived, this agent's), then the chat holding an unsent **draft** (newest `updatedAt`; a `new:` winner = a new chat), then the agent's existing **empty** chat, then a new, empty chat | | `resolveAgentLanding` | the `?agent=` deep link, delegating to `agentLanding` so there is one answer | | `orderRosterAgents` | most recent collaboration, then name; `primaryName` is ent#491's seam | | `agentPreview` | the newest chat's **title** — the sidebar list carries no message content (#2198), so a body preview would reinstate the N+1 | @@ -216,9 +218,21 @@ rule is a plain function (the ent#392 precedent): ruled it "becomes the newest tab", and hiding what the system line just pointed at is exactly where the person looks next — but `agentLanding` never puts you in one without asking. Two rules, two questions: where can I go, and where am - I put. Since ent#784 the landing answer is "a new chat", so the strip is the - ONLY way back into an old one — which is the point: continuity is one click, - starting fresh is zero. + I put. Since ent#784 the landing answer is a new chat unless something of the + person's own is waiting (a draft, or an empty chat already open), so the strip + is the ONLY way back into a USED one — which is the point: continuity is one + click, starting fresh is zero. +- **A draft outranks a fresh chat, and shares the mark's predicate.** The + 2026-10-05 ruling: if a chat with the agent holds unsent words, opening the + agent goes THERE, because the mark on the agent's row already promises + "click here to continue". So the landing's candidate set and that mark's are + one set — `portalDrafts.js::isDraftedThread`, read by both `agentsWithDrafts` + and `draftedLandingFor` — and a room and an archived thread are excluded on + both sides rather than on one. Several drafts resolve by newest `updatedAt`, + with the unsaved `new:` chat weighed on the same clock; a `new:` + winner lands on a new chat, which is where those words already live. Each + door passes the drafts map IN (`landOnAgent`, `resolveAgentLanding`), so the + rule stays pure and testable without the shell. - **Two doors RESOLVE, the rest ASSERT.** Only `landOnAgent` (the sidebar/rail row, back/forward, a typed URL) and `resolveAgentLanding` (`?agent=`) ask `agentLanding` where to land. New chat, ⌘J, the agent picker and switch-agent @@ -461,6 +475,15 @@ render** (ent#468 Option A), recorded on the issue. - `src/frontend/tests/unit/portalAgentsAtCentre.spec.js` — every pure rule above, the ent#524 drop/batch rules, and the source guards that no unit test can reach (which surface mounts what, no `[0]` left anywhere) +- `src/frontend/tests/unit/portalDraftLanding.spec.js` — ent#784's drafts arm as + a pure rule: each precedence arm, newest-edit-wins both ways round, the tie, + the room and archived exclusions, and the property that every row the sidebar + MARKS is a landing candidate (the shared-predicate guarantee) +- `src/frontend/tests/unit/portalDraftLanding.mount.spec.js` — the same two + flows `e2e/workspace-drafts.spec.js:92`/`:116` walk, at the shell's seam: the + shell must hand the rule its drafts map and then open the chat it names. Red + at `c8af3d70` on exactly the e2e's symptom (`sessionId: null` where the draft + was), which is why the pure specs alone were not enough - Re-pointed rather than deleted when their subject moved: `portalRatings`, `portalReportsRendering`, `portalAvailabilityChip`, `portalAskSingleSource`, `portalAgentPageUx`, `portalRail`, `portalLoadingTreatment`, diff --git a/src/frontend/src/components/portal/portalDrafts.js b/src/frontend/src/components/portal/portalDrafts.js index db61c3c52..bf14e4549 100644 --- a/src/frontend/src/components/portal/portalDrafts.js +++ b/src/frontend/src/components/portal/portalDrafts.js @@ -175,13 +175,84 @@ export function removeDraftsBucket(storage, identity) { export function agentsWithDrafts(threads, newChatDraftAgents) { const out = new Set() for (const t of Array.isArray(threads) ? threads : []) { - if (t && !t.is_room && t.hasDraft && t.agent_name) out.add(t.agent_name) + if (isDraftedThread(t)) out.add(t.agent_name) } const extra = newChatDraftAgents instanceof Set ? newChatDraftAgents : [] for (const name of extra) if (name) out.add(name) return out } +/** + * Does this thread row hold a draft that opening its agent should land on? + * + * The ONE predicate behind both halves of ent#784's promise, which is why it + * is a named export rather than a condition inside `agentsWithDrafts`: the + * agent row's draft mark (`agentsWithDrafts`, read by `PortalSidebar`) and the + * drafts-win landing arm (`portalUtils::agentLanding`) must light and land on + * the SAME rows, or the mark stops meaning "click here to continue". + * + * A ROOM is excluded for `agentsWithDrafts`' own reason (the room row is the + * draft's own door), and so is an ARCHIVED thread: nothing should invite the + * person to carry on writing in a chat that has been put away, and the mark + * must not promise a landing the rule refuses. Reset already MOVES the draft + * off the row it archives (`Portal.vue::onMainReset`), so this covers the + * archive paths that do not. + * + * `hasDraft` is the shell's `decorate()` stamp, read from the drafts store's + * key set — so the rows this answers for are exactly the rows with a stored + * draft, never a second opinion about where drafts live. + */ +export function isDraftedThread(t) { + return !!t && !t.is_room && !!t.hasDraft && !!t.agent_name && !t.archived_at +} + +/** + * ent#784 arm 3 — which of this agent's drafted chats to open, or null. + * + * The candidates are `isDraftedThread` rows of this agent plus the agent's + * unsaved `new:` chat, and the winner is the one edited most recently + * (`updatedAt` from the store's map). A `new:` winner answers `sessionId: + * null` — a new chat, which is where that draft already lives, so landing + * there restores it with no move. + * + * Deterministic on a tie: `threads` arrives most-recent-first, the comparison + * is strictly-greater, and the unsaved chat is weighed last — so equal + * timestamps resolve to the newest listed thread, never to a coin flip. An + * entry with no usable `updatedAt` counts as 0 (the storage codec already + * dropped anything else), which is the oldest, not a win by accident. + * + * One row can be a candidate here without carrying the sidebar's agent-row + * mark: the shell hands the mark `sidebarThreads`, which omits an unused Main + * (`portalInbox::inSidebar`). That direction is harmless — the promise is + * "a mark means clicking the row lands you on those words", and every marked + * row IS a candidate. The reverse only means an unmarked draft is still found. + */ +export function draftedLandingFor({ agentName, threads = [], drafts = null } = {}) { + if (!agentName || typeof agentName !== 'string') return null + const map = drafts && typeof drafts === 'object' && !Array.isArray(drafts) ? drafts : null + if (!map) return null + const at = (key) => { + const n = Number(map[key]?.updatedAt) + return Number.isFinite(n) ? n : 0 + } + const held = (key) => !!key && hasDraftText(map[key]?.text) + let best = null + for (const t of Array.isArray(threads) ? threads : []) { + if (!isDraftedThread(t) || t.agent_name !== agentName) continue + const sid = t.id || t.session_id + const key = threadKey(sid) + if (!held(key)) continue + const when = at(key) + if (!best || when > best.updatedAt) best = { sessionId: sid, updatedAt: when } + } + const fresh = newChatKey(agentName) + if (held(fresh)) { + const when = at(fresh) + if (!best || when > best.updatedAt) best = { sessionId: null, updatedAt: when } + } + return best ? { sessionId: best.sessionId } : null +} + /** * What the composer shows, and what is written, when its conversation changes * identity WITHOUT a remount. Four transitions exist in `PortalConversation`: diff --git a/src/frontend/src/components/portal/portalUtils.js b/src/frontend/src/components/portal/portalUtils.js index bf43cb956..5fcde19b2 100644 --- a/src/frontend/src/components/portal/portalUtils.js +++ b/src/frontend/src/components/portal/portalUtils.js @@ -1,5 +1,12 @@ // Shared helpers for the client-portal chat shell (#138). +// ent#784: the landing rule's drafts arm reads the drafts module's predicate +// rather than a second copy of it. `portalDrafts.js` imports nothing, so this +// edge adds no cycle — and it must stay that way: `portalInbox.js` already +// imports THIS file, so an import of `portalInbox` from either of these two +// would close one. +import { draftedLandingFor } from './portalDrafts' + // Deterministic per-agent color — used for the avatar tint and the small thread // color dots in the sidebar so a thread visually ties to its agent. export function agentColor(name) { @@ -565,17 +572,18 @@ function isTransportError(err) { || code === 'ECONNABORTED' || code === 'ERR_NETWORK' || code === 'ETIMEDOUT' } -export function resolveAgentLanding({ agent, forceNew = false, agents = [], threads = [] } = {}) { +export function resolveAgentLanding({ agent, forceNew = false, agents = [], threads = [], drafts = null } = {}) { if (!agent || typeof agent !== 'string') return null if (!Array.isArray(agents) || !agents.some((a) => a && a.name === agent)) return null // ent#784: the same rule the sidebar's agent row uses (`agentLanding`), not a // second one — a deep link and a sidebar click must land in the same place. - // `forceNew` is kept so existing `?new=1` links keep working, but it is now - // redundant: the default IS a new chat. This door passes no - // `lastOpenSessionId` (a deep link carries no session memory), so it always - // resolves to a fresh chat. + // `forceNew` is the one way past the whole precedence: `?new=1` says "a fresh + // chat" in so many words, so it outranks a waiting draft and an empty chat + // alike. Without it this door passes no `lastOpenSessionId` (a deep link + // carries no session memory) but DOES pass the drafts map, so a deep link and + // a sidebar click land on the same words. if (forceNew) return { agentName: agent, sessionId: null } - return agentLanding({ agentName: agent, threads }) + return agentLanding({ agentName: agent, threads, drafts }) } // ent#358: the Workspace is now the ONLY continuous-conversation surface, so a @@ -1751,18 +1759,36 @@ export function feedbackAcknowledgement(captureFeedback) { // deep link via `resolveAgentLanding`). The gesture doors — New chat, the agent // picker, the switch-agent key — do not call this: they already mean "fresh". // -// ent#784 reverses ent#523's rule. The default is a NEW, empty chat +// ent#784 reverses ent#523's rule: the default is a NEW, empty chat // (`sessionId: null`), because most visits to an agent start new work and -// resuming cost two actions every time. Nothing is minted server-side: the row -// is born on the first send (`newThread`, ent#451), so landing repeatedly -// accumulates no empty chats. +// resuming cost two actions every time. +// +// The operator's 2026-10-05 ruling on ent#784 settles the precedence, and +// these are the arms, in order: +// +// 1. a link that names a CHAT opens that chat — not this function's job, and +// unchanged (the `/workspace/c/:id` route never reaches here); +// 2. `lastOpenSessionId`, the seam ent#621's agent-switch keys pass. Honoured +// only when it still names a live, unarchived chat of THIS agent in +// `threads` — the principal's own list — so a stale or forged id falls +// through rather than landing somewhere it should not (the #3140 class); +// 3. the agent's chat holding an unsent DRAFT, newest edit first +// (`draftedLandingFor`). A draft mark on the agent's row has to mean +// "click here to continue", so this and the mark share one predicate +// rather than two rules that agree today; +// 4. the agent's existing EMPTY chat, reused rather than adding another one; +// 5. a new chat. +// +// Arms 3 and 5 mint nothing server-side: a drafted `new:` chat and a fresh one +// are both `sessionId: null`, and the row is born on the first send +// (`newThread`, ent#451). Arm 4 opens a row that already exists. So landing +// repeatedly, however often, accumulates no empty chats. // -// The ONE exception is `lastOpenSessionId` (the seam ent#621's agent-switch -// keys will pass). It is honoured only when it still names a live, unarchived -// chat of THIS agent in `threads` — the principal's own list — so a stale or -// forged id falls back to the default rather than landing somewhere it should -// not (the #3140 class). -export function agentLanding({ agentName, threads = [], lastOpenSessionId = null } = {}) { +// `drafts` is the drafts store's map (`{ key: { text, updatedAt } }`) passed in +// by the caller, never read from the store here: this stays a pure function +// every door can test without mounting the shell. Omitting it simply disables +// arm 3 — the old behaviour — rather than throwing. +export function agentLanding({ agentName, threads = [], lastOpenSessionId = null, drafts = null } = {}) { if (!agentName || typeof agentName !== 'string') return null if (lastOpenSessionId) { const row = (Array.isArray(threads) ? threads : []).find((t) => t && !t.is_room @@ -1770,6 +1796,8 @@ export function agentLanding({ agentName, threads = [], lastOpenSessionId = null && (t.id || t.session_id) === lastOpenSessionId) if (row) return { agentName, sessionId: lastOpenSessionId } } + const drafted = draftedLandingFor({ agentName, threads, drafts }) + if (drafted) return { agentName, sessionId: drafted.sessionId } return { agentName, sessionId: null } } diff --git a/src/frontend/src/views/Portal.vue b/src/frontend/src/views/Portal.vue index bd8f100fe..089c62ff4 100644 --- a/src/frontend/src/views/Portal.vue +++ b/src/frontend/src/views/Portal.vue @@ -237,9 +237,13 @@ agent — it resolves to the chat you were last in and renders the conversation, with the agent's numbers in the band above it and its context one click away in Agent details. The URL is kept (every - link to it still works); `landOnAgent` replaces it with the - thread's own URL as soon as the list is in hand. The skeleton below - covers that beat, so nothing renders here. + link to it still works). ent#784 then made the landing a + PRECEDENCE — a validated `lastOpenSessionId`, else the chat holding + an unsent draft (newest first), else the agent's existing empty + chat, else a new one — and only the two arms that name a THREAD + replace this URL with the thread's own (via `openThread`); a new + chat stays here, which is what makes it reloadable. The skeleton + below covers that beat, so nothing renders here. ent#360's reasoning is not reverted — an agent still has a home with its history, what it can do and a place to ask you something. It is @@ -1779,10 +1783,28 @@ function landOnAgent(name) { // first; back/forward and a typed `/workspace/a/:name` come through here, and // did not. Ask before touching anything. if (guardLeaveCall(() => landOnAgent(name))) return - const landing = agentLanding({ agentName: name, threads: threads.value }) + // ent#784: landing is not a gesture, so the composer focuses only where that + // cannot summon an on-screen keyboard (see `focusOnMount` below). Set BEFORE + // the branch, not only on the new-chat path: a landing on a drafted chat is + // the same kind of arrival, and `openThread` does not touch this ref — so + // leaving a gesture's `always` standing would make the next landing focus on + // a touch device (the T3(b) rule, one door at a time). + composerFocusMode.value = 'fine-pointer' + const landing = agentLanding({ + agentName: name, + threads: threads.value, + // The drafts arm's candidates. Passed in rather than read inside the rule, + // so `agentLanding` stays a pure function both doors can be tested through. + drafts: drafts.drafts, + }) if (!landing) return if (landing.sessionId) { const row = threads.value.find((t) => (t.id || t.session_id) === landing.sessionId) + // `openThread` remounts the conversation on the landed session, which is + // what restores the draft: the fresh instance's `useComposerDraft` reads + // `thread:` at setup and `PortalConversation`'s mount then puts the + // caret at its end, on a fine pointer only — the same rule as above, shared + // through `shouldAutoFocusComposer` rather than re-decided here. if (row) { openThread(row); return } } // Idempotent: the watcher fires on the route param AND on the thread list @@ -1793,9 +1815,6 @@ function landOnAgent(name) { unreachableAgent.value = null activeAgentName.value = name activeRoomId.value = null - // ent#784: landing is not a gesture, so the composer focuses only where that - // cannot summon an on-screen keyboard (see `focusOnMount` below). - composerFocusMode.value = 'fine-pointer' startingNewChat.value = true pendingSession.value = null; prefill.value = ''; convGen.value++ } @@ -2396,6 +2415,9 @@ function resolveAgentQuery() { forceNew, agents: store.agents, threads: threads.value, + // ent#784: the same drafts map `landOnAgent` passes — ONE rule for every + // door, so a `?agent=` link and a sidebar click land on the same chat. + drafts: drafts.drafts, }) if (!landing) { // The link named an agent this caller cannot reach (un-shared since the URL diff --git a/src/frontend/tests/unit/portalDraftLanding.mount.spec.js b/src/frontend/tests/unit/portalDraftLanding.mount.spec.js new file mode 100644 index 000000000..863515a43 --- /dev/null +++ b/src/frontend/tests/unit/portalDraftLanding.mount.spec.js @@ -0,0 +1,208 @@ +// @vitest-environment jsdom +/** + * ent#784 arm 3, mounted — the two flows `e2e/workspace-drafts.spec.js` lost. + * + * The pure arm cases are in `portalDraftLanding.spec.js`. Nothing there proves + * the SHELL asks the rule the right question: `landOnAgent` has to hand it the + * drafts map and then actually open the thread it names. That is what was + * broken — the rule answered "a new chat" for every landing, so the two e2e + * cases below went red at `c8af3d70` while every unit spec stayed green. + * + * Replayed here at the shell's seam (`shallowMount`, the harness of + * `portalAgentLandingRemount.mount.spec.js`), as the e2e's own two flows: + * + * `:92` type into A's thread → open B → open A again + * `:116` a reload on `/workspace/a/B` with `new:B` holding words + * + * `PortalConversation` is a stub, so the assertions stop at the props that + * decide what the composer binds — the session, the fresh-chat flag, and the + * `draftKeyFor` those two resolve to. The real textarea, its value and its + * focus are the e2e's half; what this file owns is that the shell lands on the + * chat whose key holds the words. + */ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { shallowMount, flushPromises } from '@vue/test-utils' +import { setActivePinia, createPinia } from 'pinia' +import { createRouter, createMemoryHistory } from 'vue-router' +import { draftKeyFor, draftStorageKey, persistDraft } from '@/components/portal/portalDrafts' + +vi.hoisted(() => { + window.matchMedia = window.matchMedia || ((q) => ({ + matches: false, media: q, addEventListener() {}, removeEventListener() {}, addListener() {}, removeListener() {}, + })) +}) + +vi.mock('@/stores/auth', () => ({ + useAuthStore: () => ({ isAuthenticated: true, authHeader: {}, userEmail: 'me@example.com', logout: vi.fn() }), +})) +vi.mock('axios', () => { + const mk = () => ({ + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), + interceptors: { request: { use: vi.fn() }, response: { use: vi.fn() } }, + defaults: { headers: { common: {} } }, + }) + return { + default: Object.assign( + { get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), create: mk }, + { interceptors: { request: { use: vi.fn() }, response: { use: vi.fn() } }, + defaults: { headers: { common: {} } } }, + ), + } +}) + +import { useClientPortalStore } from '@/stores/clientPortal' +import { usePortalDraftsStore } from '@/stores/portalDrafts' +import Portal from '@/views/Portal.vue' + +globalThis.ResizeObserver = globalThis.ResizeObserver || class { observe() {} unobserve() {} disconnect() {} } + +const WHO = 'me@example.com' +const Blank = { template: '
' } +function makeRouter() { + return createRouter({ + history: createMemoryHistory(), + routes: [ + { path: '/workspace', component: Blank }, + { path: '/workspace/inbox', component: Blank }, + { path: '/workspace/c/:sessionId', component: Blank }, + { path: '/workspace/r/:roomId', component: Blank }, + { path: '/workspace/a/:agentName', component: Blank }, + { path: '/login', component: Blank }, + ], + }) +} + +// The e2e's own fixture: two agents, two chats with A and one with B, every one +// of them already used (`message_count: 2`), so nothing here is an empty chat +// and the drafts arm is the only thing that can beat "a new chat". +const sess = (id, agent, when) => ({ + id, session_id: id, agent_name: agent, title: `${agent} — ${id}`, + created_at: when, last_message_at: when, message_count: 2, unread: 0, +}) +const THREADS = [ + sess('sess-a', 'drafts-a', '2026-09-03T10:00:00Z'), + sess('sess-a2', 'drafts-a', '2026-09-02T10:00:00Z'), + sess('sess-b', 'drafts-b', '2026-09-01T10:00:00Z'), +] + +let store +function arm() { + store = useClientPortalStore() + store.portalToken = 'tok' + store.fetchRoster = vi.fn(async () => { + store.agents = [{ name: 'drafts-a' }, { name: 'drafts-b' }] + // The drafts store keys its bucket on the server-reported principal, so + // without this every draft written below lands in the memory-only map and + // the reload flow could not be replayed at all. + store.clientEmail = WHO + store.error = null + store.rosterLoaded = true + }) + store.fetchAllSessions = vi.fn(async () => { store.sessionsFailed = false; return THREADS }) + store.fetchChatState = vi.fn(async (opts) => (opts ? { state: {}, previews: {} } : {})) + store.fetchSessions = vi.fn(async () => THREADS) + store.fetchAsks = vi.fn(async () => []) + store.seedAgentRecency = vi.fn() + store.ensureBriefing = vi.fn() + store.loadAgentSuggestions = vi.fn() + store.markChatRead = vi.fn(async () => {}) +} + +async function boot(path) { + const router = makeRouter() + await router.push(path) + await router.isReady() + const w = shallowMount(Portal, { global: { plugins: [router] } }) + await flushPromises(); await flushPromises() + return { w, router } +} + +/** Where the shell landed, as the composer will read it. */ +function landed(w) { + const c = w.findComponent({ name: 'PortalConversation' }) + if (!c.exists()) return { exists: false } + const sessionId = c.props('sessionId') + const newChat = c.props('newChat') + const agentName = c.props('agent')?.name + return { + exists: true, + sessionId, + newChat, + agentName, + draftKey: draftKeyFor({ sessionId, agentName, newChat }), + } +} + +/** A sidebar agent-row click: `openAgentPage` pushes this and the watcher lands. */ +async function openAgent(router, name) { + await router.push(`/workspace/a/${name}`) + await flushPromises(); await flushPromises() +} + +beforeEach(() => { + localStorage.clear() + setActivePinia(createPinia()) + arm() +}) + +describe('ent#784 — the shell lands on the chat holding the draft', () => { + it('A → B → A comes back to the words typed in A (e2e :92)', async () => { + const { w, router } = await boot('/workspace/c/sess-a') + expect(landed(w).sessionId).toBe('sess-a') + + // Typing into A's thread. The store write is what the composer's + // write-through binding does on every keystroke. + const drafts = usePortalDraftsStore() + drafts.set('thread:sess-a', 'half a thought for A') + await flushPromises() + + // B: a different agent, nothing typed with it — a fresh chat. + await openAgent(router, 'drafts-b') + expect(landed(w)).toMatchObject({ agentName: 'drafts-b', sessionId: null, newChat: true }) + + // Back to A. This is the assertion that was red: the landing resolved to a + // new chat (`sessionId: null`, key `new:drafts-a`) and the composer came up + // empty, while A's row still carried the draft mark. + await openAgent(router, 'drafts-a') + expect(landed(w)).toMatchObject({ + agentName: 'drafts-a', sessionId: 'sess-a', newChat: false, draftKey: 'thread:sess-a', + }) + expect(drafts.get(landed(w).draftKey)).toBe('half a thought for A') + }) + + it('a reload on an agent page restores its unsaved chat, then A still has its own (e2e :116)', async () => { + // What a reload sees: the bucket on disk, written by the session before it. + persistDraft(localStorage, WHO, 'thread:sess-a', { text: 'for A', updatedAt: 100 }) + persistDraft(localStorage, WHO, 'new:drafts-b', { text: 'for B', updatedAt: 200 }) + expect(localStorage.getItem(draftStorageKey(WHO))).toContain('for B') + + const { w, router } = await boot('/workspace/a/drafts-b') + // B's draft lives on the unsaved chat, so the landing IS a new chat — and + // its key is the one holding the words, which is what makes the composer + // come up with them in it. + expect(landed(w)).toMatchObject({ + agentName: 'drafts-b', sessionId: null, newChat: true, draftKey: 'new:drafts-b', + }) + const drafts = usePortalDraftsStore() + expect(drafts.get('new:drafts-b')).toBe('for B') + + // And clicking A goes to A's thread, not to a new chat (the red one). + await openAgent(router, 'drafts-a') + expect(landed(w)).toMatchObject({ + agentName: 'drafts-a', sessionId: 'sess-a', newChat: false, draftKey: 'thread:sess-a', + }) + expect(drafts.get('thread:sess-a')).toBe('for A') + }) + + it('the landing is not a gesture: the composer may focus only on a fine pointer', async () => { + // T3(b), on the drafted-thread path. `landOnAgent` sets this before it + // branches, so a landing cannot inherit a previous gesture's `always` and + // pop the soft keyboard on a phone. + const drafts = usePortalDraftsStore() + const { w, router } = await boot('/workspace/c/sess-a') + drafts.set('thread:sess-a', 'words') + await flushPromises() + await openAgent(router, 'drafts-a') + expect(w.findComponent({ name: 'PortalConversation' }).props('focusOnMount')).toBe('fine-pointer') + }) +}) diff --git a/src/frontend/tests/unit/portalDraftLanding.spec.js b/src/frontend/tests/unit/portalDraftLanding.spec.js new file mode 100644 index 000000000..f6e6d8651 --- /dev/null +++ b/src/frontend/tests/unit/portalDraftLanding.spec.js @@ -0,0 +1,164 @@ +/** + * ent#784 arm 3 — "drafts win": opening an agent lands on its newest draft. + * + * The operator's 2026-10-05 ruling settled the precedence a landing resolves + * by, and this file owns the pure half of the arm it added: a link to a chat, + * then `lastOpenSessionId` (ent#621), then the chat holding an unsent draft, + * then a new chat. Before this the rule always answered a new chat, so a draft + * typed into an existing chat was not where opening the agent took you — the + * draft mark on the agent's row pointed at words you could not get back to by + * clicking it (`e2e/workspace-drafts.spec.js:92` and `:116`). + * + * The candidate set is the one that lights the sidebar's mark. That is the + * point of `isDraftedThread` being shared rather than copied, and it is why + * the archived and room cases are asserted here: both sides have to agree, or + * "the mark means click here to continue" is only true by coincidence. + */ +import { describe, it, expect } from 'vitest' +import { agentLanding } from '@/components/portal/portalUtils' +import { + agentsWithDrafts, draftedLandingFor, isDraftedThread, +} from '@/components/portal/portalDrafts' + +// As `fetchAllSessions` returns them, after the shell's `decorate()` has +// stamped `hasDraft` from the drafts store's key set: most recent first. +const row = (id, agent, extra = {}) => ({ + id, session_id: id, agent_name: agent, last_message_at: '2026-09-27T10:00:00Z', message_count: 2, ...extra, +}) +const at = (text, updatedAt) => ({ text, updatedAt }) + +describe('agentLanding — arm 3, the chat holding a draft', () => { + it('lands on the drafted thread instead of a new chat', () => { + const threads = [row('s2', 'a'), row('s1', 'a', { hasDraft: true })] + const drafts = { 'thread:s1': at('half a thought for A', 100) } + expect(agentLanding({ agentName: 'a', threads, drafts })) + .toEqual({ agentName: 'a', sessionId: 's1' }) + }) + + it('lands on a new chat when the draft is the unsaved one', () => { + // `new:` IS the new chat's key, so `sessionId: null` is not a + // fallthrough here — it is the landing that restores those words. + const threads = [row('s1', 'a')] + expect(agentLanding({ agentName: 'a', threads, drafts: { 'new:a': at('for A', 100) } })) + .toEqual({ agentName: 'a', sessionId: null }) + }) + + it('picks the newest edit when several chats hold drafts', () => { + const threads = [row('s2', 'a', { hasDraft: true }), row('s1', 'a', { hasDraft: true })] + const drafts = { 'thread:s1': at('older', 100), 'thread:s2': at('newer', 200) } + expect(agentLanding({ agentName: 'a', threads, drafts }).sessionId).toBe('s2') + // …and the other way round, so this is reading `updatedAt` rather than + // list order (which would make the assertion above pass for free). + expect(agentLanding({ + agentName: 'a', + threads, + drafts: { 'thread:s1': at('newer', 300), 'thread:s2': at('older', 200) }, + }).sessionId).toBe('s1') + }) + + it('weighs the unsaved chat against the threads by the same clock', () => { + const threads = [row('s1', 'a', { hasDraft: true })] + expect(agentLanding({ + agentName: 'a', threads, drafts: { 'thread:s1': at('older', 100), 'new:a': at('newer', 200) }, + }).sessionId).toBeNull() + expect(agentLanding({ + agentName: 'a', threads, drafts: { 'thread:s1': at('newer', 300), 'new:a': at('older', 200) }, + }).sessionId).toBe('s1') + }) + + it('resolves a tie to the newest listed thread, never at random', () => { + const threads = [row('s2', 'a', { hasDraft: true }), row('s1', 'a', { hasDraft: true })] + const drafts = { 'thread:s1': at('same', 100), 'thread:s2': at('same', 100), 'new:a': at('same', 100) } + for (let i = 0; i < 5; i++) { + expect(agentLanding({ agentName: 'a', threads, drafts }).sessionId).toBe('s2') + } + }) + + it('never crosses agents', () => { + const threads = [row('s9', 'b', { hasDraft: true }), row('s1', 'a')] + const drafts = { 'thread:s9': at("b's words", 100) } + expect(agentLanding({ agentName: 'a', threads, drafts })) + .toEqual({ agentName: 'a', sessionId: null }) + }) + + it('ignores an archived drafted chat, and so does the mark', () => { + // Both halves, in one case, on purpose: a landing that refuses the row + // while the row still carries the mark is the drift this arm exists to + // prevent. Reset MOVES the draft off the row it archives, so nothing is + // lost by refusing to invite the person back into an archive. + const threads = [row('s1', 'a', { hasDraft: true, archived_at: '2026-09-28T10:00:00Z' })] + const drafts = { 'thread:s1': at('in the archive', 100) } + expect(agentLanding({ agentName: 'a', threads, drafts })) + .toEqual({ agentName: 'a', sessionId: null }) + expect(agentsWithDrafts(threads, new Set())).toEqual(new Set()) + }) + + it('ignores a room, and so does the mark', () => { + const threads = [{ id: 'r1', is_room: true, agent_name: 'a', hasDraft: true }] + const drafts = { 'room:r1': at('for the room', 100) } + expect(agentLanding({ agentName: 'a', threads, drafts })) + .toEqual({ agentName: 'a', sessionId: null }) + expect(agentsWithDrafts(threads, new Set())).toEqual(new Set()) + }) + + it('every marked row is a landing candidate (the shared predicate)', () => { + // The promise in words: a mark on the agent's row means clicking it lands + // you on those words. So for each agent the mark names, the rule must + // answer something other than "a new chat you have not typed in" — either + // the drafted thread, or the unsaved chat that holds the draft. + const threads = [ + row('s1', 'a', { hasDraft: true }), + row('s2', 'b'), + { id: 'r1', is_room: true, agent_name: 'a', hasDraft: true }, + row('s3', 'c', { hasDraft: true, archived_at: '2026-09-28T10:00:00Z' }), + ] + const drafts = { + 'thread:s1': at('for A', 100), 'new:b': at('for B', 90), + 'room:r1': at('room', 80), 'thread:s3': at('archived', 70), + } + const marked = agentsWithDrafts(threads, new Set(['b'])) + expect(marked).toEqual(new Set(['a', 'b'])) + for (const name of marked) { + expect(draftedLandingFor({ agentName: name, threads, drafts })).not.toBeNull() + } + expect(draftedLandingFor({ agentName: 'c', threads, drafts })).toBeNull() + }) + + it('arm 2 still outranks it — ent#621 returns you where you were', () => { + const threads = [row('s2', 'a'), row('s1', 'a', { hasDraft: true })] + const drafts = { 'thread:s1': at('for A', 100) } + expect(agentLanding({ agentName: 'a', threads, drafts, lastOpenSessionId: 's2' }).sessionId) + .toBe('s2') + // A stale id falls THROUGH to the drafts arm, not past it to a new chat. + expect(agentLanding({ agentName: 'a', threads, drafts, lastOpenSessionId: 'gone' }).sessionId) + .toBe('s1') + }) + + it('with no drafts the answer is still a new chat', () => { + const threads = [row('s1', 'a')] + expect(agentLanding({ agentName: 'a', threads, drafts: {} })) + .toEqual({ agentName: 'a', sessionId: null }) + // And a caller that passes no map at all keeps the old behaviour rather + // than throwing — the arm is disabled, not broken. + expect(agentLanding({ agentName: 'a', threads })) + .toEqual({ agentName: 'a', sessionId: null }) + }) + + it('does not trust a stale stamp over the stored text', () => { + // `hasDraft` is a projection; the map is the source. A row stamped after + // the draft was sent must not win an empty landing. + const threads = [row('s1', 'a', { hasDraft: true })] + expect(draftedLandingFor({ agentName: 'a', threads, drafts: { 'thread:s1': at(' ', 100) } })) + .toBeNull() + expect(draftedLandingFor({ agentName: 'a', threads, drafts: {} })).toBeNull() + }) + + it('isDraftedThread is the predicate both sides read', () => { + expect(isDraftedThread(row('s1', 'a', { hasDraft: true }))).toBe(true) + expect(isDraftedThread(row('s1', 'a'))).toBe(false) + expect(isDraftedThread(row('s1', 'a', { hasDraft: true, archived_at: 'x' }))).toBe(false) + expect(isDraftedThread({ id: 'r', is_room: true, hasDraft: true, agent_name: 'a' })).toBe(false) + expect(isDraftedThread({ id: 's', hasDraft: true })).toBe(false) + expect(isDraftedThread(null)).toBe(false) + }) +}) From dace0c22ee14c7f2eef02de125510825f23a3adc Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Mon, 5 Oct 2026 04:54:37 -0400 Subject: [PATCH 07/22] fix(portal): opening an agent reuses its empty chat (abilityai/trinity-enterprise#784) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- docs/memory/architecture/workspace.md | 10 ++- .../workspace-agents-at-the-centre.md | 12 +++ .../src/components/portal/portalUtils.js | 44 +++++++++++ src/frontend/src/views/Portal.vue | 11 ++- .../tests/unit/portalAgentsAtCentre.spec.js | 45 ++++++++--- .../unit/portalDraftLanding.mount.spec.js | 32 +++++++- .../tests/unit/portalDraftLanding.spec.js | 75 ++++++++++++++++++- .../tests/unit/workspaceAgentLanding.spec.js | 24 ++++-- .../tests/unit/workspaceNewChat.spec.js | 12 ++- 9 files changed, 238 insertions(+), 27 deletions(-) diff --git a/docs/memory/architecture/workspace.md b/docs/memory/architecture/workspace.md index f915c3c85..3bc298963 100644 --- a/docs/memory/architecture/workspace.md +++ b/docs/memory/architecture/workspace.md @@ -1005,7 +1005,15 @@ naming a CHAT wins (and never reaches the rule); then a validated `lastOpenSessi ent#621 seam, checked against the principal's own thread list; then the agent's chat holding an unsent **draft**, newest `updatedAt` first, where a `new:` winner lands on a new chat (that key IS where the words are) and a thread winner opens that thread; then -the agent's existing **empty** chat, reused; then a new, empty chat. The drafts arm reads +the agent's existing **empty** chat, reused (`agentEmptyChat` — an unarchived, +non-room thread of this agent with no message sent: no `last_message_at` AND +`message_count` 0 or absent, since the two fields come from different reads; +Main first, then newest `created_at`, so list order never decides); then a new, +empty chat. The write-side twin of that arm is `ensureMainListed`, a GET that +INSERTS: it now also returns early when the agent already has an empty chat, so +visiting an agent can never add a SECOND empty row ("at most one empty chat per +agent at any time"). An agent whose chats are all used still gets its Main on +the visit, which is the case #2579 was about. The drafts arm reads `portalDrafts.js::isDraftedThread` — the SAME predicate `agentsWithDrafts` lights the sidebar's agent-row mark from, shared 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 diff --git a/docs/memory/feature-flows/workspace-agents-at-the-centre.md b/docs/memory/feature-flows/workspace-agents-at-the-centre.md index 65f1f425b..8458977ba 100644 --- a/docs/memory/feature-flows/workspace-agents-at-the-centre.md +++ b/docs/memory/feature-flows/workspace-agents-at-the-centre.md @@ -205,6 +205,7 @@ rule is a plain function (the ent#392 precedent): | `agentChatTabs` | Main first, then recency; an archive stays a tab (ruled “becomes the newest tab”); Main is labelled by its **role** | | `agentLanding` | a **precedence** (ent#784, replacing ent#523's `landingThread`; operator ruling 2026-10-05): a validated `lastOpenSessionId` (live, unarchived, this agent's), then the chat holding an unsent **draft** (newest `updatedAt`; a `new:` winner = a new chat), then the agent's existing **empty** chat, then a new, empty chat | | `resolveAgentLanding` | the `?agent=` deep link, delegating to `agentLanding` so there is one answer | +| `agentEmptyChat` | arm 4's "empty": unarchived, non-room, no `last_message_at` and `message_count` 0/absent; Main first, then newest `created_at` | | `orderRosterAgents` | most recent collaboration, then name; `primaryName` is ent#491's seam | | `agentPreview` | the newest chat's **title** — the sidebar list carries no message content (#2198), so a body preview would reinstate the N+1 | | `composerAvailabilityNotice` | what the composer says for a stopped/unavailable agent | @@ -233,6 +234,17 @@ rule is a plain function (the ent#392 precedent): winner lands on a new chat, which is where those words already live. Each door passes the drafts map IN (`landOnAgent`, `resolveAgentLanding`), so the rule stays pure and testable without the shell. +- **The empty chat is reused, and only the frontend can hold that.** Arm 4 + (`agentEmptyChat`): an unarchived, non-room thread with no message sent — no + `last_message_at` AND `message_count` 0 or absent, because the cross-agent + batch omits the count while the per-agent read carries it, so either field + alone would misjudge a row. Main first, then newest `created_at`, so two + doors reading one list cannot reuse two different rows. The matching promise + on the WRITE side is `ensureMainListed`, which is a GET that INSERTS: it + returns early for an agent that already has an empty chat, or visiting one + would add a second empty row and then land on one of the two. #2579's "the + pinned tab has to be there" still holds for every agent whose chats are all + used, which is what it was about. - **Two doors RESOLVE, the rest ASSERT.** Only `landOnAgent` (the sidebar/rail row, back/forward, a typed URL) and `resolveAgentLanding` (`?agent=`) ask `agentLanding` where to land. New chat, ⌘J, the agent picker and switch-agent diff --git a/src/frontend/src/components/portal/portalUtils.js b/src/frontend/src/components/portal/portalUtils.js index 5fcde19b2..f0489615d 100644 --- a/src/frontend/src/components/portal/portalUtils.js +++ b/src/frontend/src/components/portal/portalUtils.js @@ -438,6 +438,48 @@ export function agentHasMain(threads, agentName) { .some((t) => t && !t.is_room && t.agent_name === agentName && !!t.is_main) } +/** + * ent#784 arm 4 — has this agent an EMPTY chat already? Its id, or null. + * + * The operator's 2026-10-05 ruling: opening an agent must not create a new + * chat every time, so an empty one is REUSED and at most one exists per agent + * at any moment. "Empty" is the same "unused" test ent#523 applied to an + * unused Main, widened to any row that fits it: an unarchived, non-room thread + * of this agent with no message sent — no `last_message_at`, and + * `message_count` 0 or absent. Both halves are required, because the two + * fields come from different reads: the cross-agent batch omits + * `message_count` entirely, so counting an absent count as "used" would make + * every row non-empty there, and treating a present count of 2 as empty + * because `last_message_at` is missing would land the person in a used chat. + * + * Deterministic when legacy data holds several: **Main first**, then the newest + * `created_at`, then the id — so the answer never depends on list order. Main + * first is not a tiebreak detail: it is the row the tab strip pins and the one + * every post-ent#523 pair already has, so reusing anything else would leave an + * unused Main sitting beside the chat being typed in. + * + * Pure, like the rest of the rule: it reads the list it is handed and mints + * nothing. The matching promise on the WRITE side is `Portal.vue`'s + * `ensureMainListed`, which must not add a Main to an agent that already has + * an empty chat. + */ +export function agentEmptyChat(threads, agentName) { + if (!agentName) return null + const empty = (Array.isArray(threads) ? threads : []).filter((t) => t + && !t.is_room && t.agent_name === agentName && !t.archived_at + && !t.last_message_at && !Number(t.message_count || 0)) + if (!empty.length) return null + const ts = (t) => { + const n = Date.parse(t.created_at || '') + return Number.isFinite(n) ? n : 0 + } + const id = (t) => t.id || t.session_id || '' + empty.sort((a, b) => (b.is_main ? 1 : 0) - (a.is_main ? 1 : 0) + || ts(b) - ts(a) + || String(id(a)).localeCompare(String(id(b)))) + return id(empty[0]) || null +} + // #2579: is this thread inside the window where a generated title may still // land? Two `touch_portal_session(added=1)` calls happen per exchange (the // user's message and the reply), and `_title_plan` gates on the PRE-turn @@ -1798,6 +1840,8 @@ export function agentLanding({ agentName, threads = [], lastOpenSessionId = null } const drafted = draftedLandingFor({ agentName, threads, drafts }) if (drafted) return { agentName, sessionId: drafted.sessionId } + const empty = agentEmptyChat(threads, agentName) + if (empty) return { agentName, sessionId: empty } return { agentName, sessionId: null } } diff --git a/src/frontend/src/views/Portal.vue b/src/frontend/src/views/Portal.vue index 089c62ff4..bc7c546ea 100644 --- a/src/frontend/src/views/Portal.vue +++ b/src/frontend/src/views/Portal.vue @@ -907,7 +907,7 @@ import { stageZone } from '@/components/portal/portalBriefingState' import { isNewChatHotkey, resolveAgentLanding, shouldMarkTurnRead, shouldEscapeStage, agentLanding, - agentHasMain, titleSettling, shouldFetchTitleHealth, titleGenerationNotice, + agentHasMain, agentEmptyChat, titleSettling, shouldFetchTitleHealth, titleGenerationNotice, TITLE_SETTLE_DELAYS_MS, // ent#557: the SAME sum the sidebar renders, so the tab and the rows cannot // disagree about the number. @@ -1845,6 +1845,15 @@ const MAIN_ENSURE_ATTEMPTS = 2 function ensureMainListed(name) { if (!name) return Promise.resolve() if (agentHasMain(threads.value, name)) return Promise.resolve() + // ent#784 arm 4, the write side of "at most one empty chat per agent at any + // time" (operator ruling 2026-10-05). This is a GET that INSERTS, so for an + // agent that already has an empty chat but no Main — legacy data, since + // nothing mints a non-Main empty row today — visiting it would add a SECOND + // empty chat and then land on one of them. The ruling outranks #2579's "the + // pinned tab has to be there": the empty chat that exists is the one arm 4 + // reuses, and the person is never shown two. An agent whose chats are all + // used still gets its Main on this visit, which is the case #2579 was about. + if (agentEmptyChat(threads.value, name)) return Promise.resolve() const inflight = mainEnsured.get(name) if (inflight) return inflight const spent = mainAttempts.get(name) || 0 diff --git a/src/frontend/tests/unit/portalAgentsAtCentre.spec.js b/src/frontend/tests/unit/portalAgentsAtCentre.spec.js index b0aed9dd3..429167c74 100644 --- a/src/frontend/tests/unit/portalAgentsAtCentre.spec.js +++ b/src/frontend/tests/unit/portalAgentsAtCentre.spec.js @@ -97,26 +97,36 @@ describe('ent#523 — Main is pinned, archives stay reachable', () => { // --------------------------------------------------------------------------- describe('ent#784 — landing', () => { - it('lands on a NEW chat even when recent chats exist', () => { + it('never resumes a USED chat, however recent', () => { // ent#784 reverses ent#523: most visits start new work, so resuming cost // two actions every time. Continuity is one click away in the tab strip. + // The 2026-10-05 ruling did not soften this — it reuses the EMPTY chat + // (arm 4 below), never a conversation that has been spoken in. const landed = agentLanding({ agentName: 'a', threads: [ - main(), - chat('c1', { last_message_at: '2026-09-07T09:00:00Z' }), - chat('c2', { last_message_at: '2026-09-07T12:00:00Z' }), + main({ last_message_at: '2026-09-07T08:00:00Z', message_count: 4 }), + chat('c1', { last_message_at: '2026-09-07T09:00:00Z', message_count: 2 }), + chat('c2', { last_message_at: '2026-09-07T12:00:00Z', message_count: 2 }), ], }) expect(landed).toEqual({ agentName: 'a', sessionId: null }) }) - it('lands a first-time visitor on a new chat, minting no row', () => { - // AC 4: the landing itself creates nothing server-side. The helper is pure - // and returns a null session; the row is born on the first send - // (`newThread`, ent#451), so repeated visits accumulate no empty chats. + it('reuses the agent\'s unused Main rather than adding a chat beside it', () => { + // Arm 4 (ruling 2026-10-05): "opening an agent must not create a new chat + // every time; at most one empty chat per agent exists at any time". A + // post-ent#523 pair always HAS an unused Main, so this is the ordinary + // answer, and it mints nothing — the row is already there. expect(agentLanding({ agentName: 'a', threads: [main()] })) - .toEqual({ agentName: 'a', sessionId: null }) + .toEqual({ agentName: 'a', sessionId: 'main' }) + }) + + it('lands a first-time visitor on a new chat, minting no row', () => { + // AC 4: the landing itself creates nothing server-side. With no row to + // reuse the helper returns a null session; the row is born on the first + // send (`newThread`, ent#451), so repeated visits accumulate no empty + // chats either way. expect(agentLanding({ agentName: 'a', threads: [] })) .toEqual({ agentName: 'a', sessionId: null }) }) @@ -141,12 +151,15 @@ describe('ent#784 — landing', () => { expect(landed.sessionId).toBe('s9') }) - it('falls back to the new-chat rule for an id it cannot vouch for', () => { + it('falls back through the rest of the rule for an id it cannot vouch for', () => { // Validated against the principal's OWN thread list, so an archived chat, // another agent's chat, a room, or an id that simply is not there all fall // back rather than landing somewhere they should not (the #3140 class). + // "Falls back" means into the arms below it, not past them to a new chat: + // with a used Main present there is nothing to reuse and the answer is + // null; the unused-Main case is the arm-4 test above. const threads = [ - main(), + main({ last_message_at: '2026-09-07T08:00:00Z', message_count: 2 }), chat('arch', { archived_at: 'x', last_message_at: '2026-09-07T23:00:00Z' }), chat('other', { agent_name: 'b', last_message_at: '2026-09-07T23:00:00Z' }), { id: 'room1', agent_name: 'a', is_room: true }, @@ -166,9 +179,17 @@ describe('ent#784 — landing', () => { it('is the SAME rule the ?agent= deep link uses', () => { // Two answers to "which chat do I land in" is how a deep link and a // sidebar click put a first-time visitor in different places. - const threads = [main(), chat('c2', { last_message_at: '2026-09-07T12:00:00Z' })] + const threads = [ + main({ last_message_at: '2026-09-07T08:00:00Z', message_count: 2 }), + chat('c2', { last_message_at: '2026-09-07T12:00:00Z', message_count: 2 }), + ] const agents = [{ name: 'a' }] expect(resolveAgentLanding({ agent: 'a', agents, threads }).sessionId).toBeNull() + // …including the arm that reuses an empty chat: one rule, both doors. + expect(resolveAgentLanding({ agent: 'a', agents, threads: [main()] }).sessionId).toBe('main') + // `?new=1` is the one way past the whole precedence. + expect(resolveAgentLanding({ agent: 'a', agents, threads: [main()], forceNew: true }).sessionId) + .toBeNull() }) }) diff --git a/src/frontend/tests/unit/portalDraftLanding.mount.spec.js b/src/frontend/tests/unit/portalDraftLanding.mount.spec.js index 863515a43..1c0749e3c 100644 --- a/src/frontend/tests/unit/portalDraftLanding.mount.spec.js +++ b/src/frontend/tests/unit/portalDraftLanding.mount.spec.js @@ -86,7 +86,7 @@ const THREADS = [ ] let store -function arm() { +function arm(threads = THREADS) { store = useClientPortalStore() store.portalToken = 'tok' store.fetchRoster = vi.fn(async () => { @@ -98,9 +98,9 @@ function arm() { store.error = null store.rosterLoaded = true }) - store.fetchAllSessions = vi.fn(async () => { store.sessionsFailed = false; return THREADS }) + store.fetchAllSessions = vi.fn(async () => { store.sessionsFailed = false; return threads }) store.fetchChatState = vi.fn(async (opts) => (opts ? { state: {}, previews: {} } : {})) - store.fetchSessions = vi.fn(async () => THREADS) + store.fetchSessions = vi.fn(async () => threads) store.fetchAsks = vi.fn(async () => []) store.seedAgentRecency = vi.fn() store.ensureBriefing = vi.fn() @@ -194,6 +194,32 @@ describe('ent#784 — the shell lands on the chat holding the draft', () => { expect(drafts.get('thread:sess-a')).toBe('for A') }) + it('an agent with an empty chat reuses it, and the GET that INSERTS is not called', async () => { + // F2 / arm 4, both halves. `ensureMainListed` is a GET that INSERTS + // (`list_sessions` → `ensure_main_session`), so for an agent that already + // has an empty chat but no Main it would add a SECOND empty row — which + // the 2026-10-05 ruling forbids ("at most one empty chat per agent at any + // time"). The frontend guard is the only place this can be held; the read + // must simply not happen. + setActivePinia(createPinia()) + arm([ + { id: 'spare', session_id: 'spare', agent_name: 'drafts-a', created_at: '2026-09-05T10:00:00Z' }, + sess('sess-b', 'drafts-b', '2026-09-01T10:00:00Z'), + ]) + const { w } = await boot('/workspace/a/drafts-a') + expect(landed(w)).toMatchObject({ agentName: 'drafts-a', sessionId: 'spare', newChat: false }) + expect(store.fetchSessions).not.toHaveBeenCalled() + }) + + it('an agent whose chats are all used still gets its Main on this visit (#2579)', async () => { + // The guard above must not swallow the case #2579 was about: with nothing + // empty to reuse, the pinned Main is still ensured on the first visit. + setActivePinia(createPinia()) + arm([sess('sess-a', 'drafts-a', '2026-09-03T10:00:00Z')]) + await boot('/workspace/a/drafts-a') + expect(store.fetchSessions).toHaveBeenCalledWith('drafts-a') + }) + it('the landing is not a gesture: the composer may focus only on a fine pointer', async () => { // T3(b), on the drafted-thread path. `landOnAgent` sets this before it // branches, so a landing cannot inherit a previous gesture's `always` and diff --git a/src/frontend/tests/unit/portalDraftLanding.spec.js b/src/frontend/tests/unit/portalDraftLanding.spec.js index f6e6d8651..b2da7b843 100644 --- a/src/frontend/tests/unit/portalDraftLanding.spec.js +++ b/src/frontend/tests/unit/portalDraftLanding.spec.js @@ -15,7 +15,7 @@ * "the mark means click here to continue" is only true by coincidence. */ import { describe, it, expect } from 'vitest' -import { agentLanding } from '@/components/portal/portalUtils' +import { agentEmptyChat, agentLanding } from '@/components/portal/portalUtils' import { agentsWithDrafts, draftedLandingFor, isDraftedThread, } from '@/components/portal/portalDrafts' @@ -162,3 +162,76 @@ describe('agentLanding — arm 3, the chat holding a draft', () => { expect(isDraftedThread(null)).toBe(false) }) }) + +/** + * ent#784 arm 4 — "reuse the empty chat" (operator ruling, 2026-10-05). + * + * Opening an agent must not create a new chat every time, and at most one + * empty chat per agent may exist at any moment. "Empty" is ent#523's own + * "unused" test (no message sent) applied to any row that fits it, not only to + * Main — and both fields are checked, because the cross-agent batch omits + * `message_count` while the per-agent read carries it. + */ +describe('agentEmptyChat / agentLanding — arm 4, the existing empty chat', () => { + const empty = (id, over = {}) => ({ id, agent_name: 'a', created_at: '2026-09-01T10:00:00Z', ...over }) + + it('reuses an unused Main instead of starting another chat', () => { + expect(agentEmptyChat([empty('main', { is_main: true })], 'a')).toBe('main') + expect(agentLanding({ agentName: 'a', threads: [empty('main', { is_main: true })], drafts: {} })) + .toEqual({ agentName: 'a', sessionId: 'main' }) + }) + + it('counts a row as used on EITHER field', () => { + // Different reads supply different fields, so one of them missing must not + // decide it: a count of 2 is used even with no `last_message_at`, and an + // arrival time is used even where the count never came down the wire. + expect(agentEmptyChat([empty('s1', { message_count: 2 })], 'a')).toBeNull() + expect(agentEmptyChat([empty('s1', { last_message_at: '2026-09-02T10:00:00Z' })], 'a')).toBeNull() + // …and absent/zero on both IS empty, which is what the batch read looks like. + expect(agentEmptyChat([empty('s1')], 'a')).toBe('s1') + expect(agentEmptyChat([empty('s1', { message_count: 0 })], 'a')).toBe('s1') + }) + + it('skips an archived empty chat, a room, and another agent', () => { + expect(agentEmptyChat([empty('s1', { archived_at: '2026-09-02T10:00:00Z' })], 'a')).toBeNull() + expect(agentEmptyChat([{ id: 'r1', agent_name: 'a', is_room: true }], 'a')).toBeNull() + expect(agentEmptyChat([empty('s1', { agent_name: 'b' })], 'a')).toBeNull() + expect(agentEmptyChat([], 'a')).toBeNull() + expect(agentEmptyChat([empty('s1')], '')).toBeNull() + }) + + it('picks Main first, then the newest, whatever order the list is in', () => { + // Legacy data can hold several. The answer must not depend on list order, + // or two doors reading the same list could reuse two different rows. + const rows = [ + empty('old', { created_at: '2026-08-01T10:00:00Z' }), + empty('main', { is_main: true, created_at: '2026-07-01T10:00:00Z' }), + empty('new', { created_at: '2026-09-01T10:00:00Z' }), + ] + expect(agentEmptyChat(rows, 'a')).toBe('main') + expect(agentEmptyChat([...rows].reverse(), 'a')).toBe('main') + const noMain = rows.filter((r) => !r.is_main) + expect(agentEmptyChat(noMain, 'a')).toBe('new') + expect(agentEmptyChat([...noMain].reverse(), 'a')).toBe('new') + }) + + it('a draft outranks it, and it outranks a new chat', () => { + // The whole precedence in one case, which is the part a per-arm test + // cannot show: arms 3, 4 and 5 over the same list. + const threads = [ + empty('main', { is_main: true }), + { id: 's1', agent_name: 'a', last_message_at: '2026-09-02T10:00:00Z', message_count: 2, hasDraft: true }, + ] + expect(agentLanding({ agentName: 'a', threads, drafts: { 'thread:s1': at('words', 100) } }).sessionId) + .toBe('s1') + expect(agentLanding({ agentName: 'a', threads, drafts: {} }).sessionId).toBe('main') + expect(agentLanding({ agentName: 'a', threads: threads.slice(1), drafts: {} }).sessionId).toBeNull() + }) + + it('reusing it is what keeps "at most one empty chat" true', () => { + // The write-side twin is `Portal.vue::ensureMainListed`, which is a GET + // that INSERTS: it must not add a Main to an agent that already has an + // empty chat. This is the predicate it asks (pinned in the mount spec). + expect(agentEmptyChat([empty('s1')], 'a')).toBe('s1') + }) +}) diff --git a/src/frontend/tests/unit/workspaceAgentLanding.spec.js b/src/frontend/tests/unit/workspaceAgentLanding.spec.js index 19b8f1cba..0b809ee4d 100644 --- a/src/frontend/tests/unit/workspaceAgentLanding.spec.js +++ b/src/frontend/tests/unit/workspaceAgentLanding.spec.js @@ -14,17 +14,22 @@ import { describe, it, expect } from 'vitest' import { resolveAgentLanding, deliveryFailureReason } from '@/components/portal/portalUtils' const agents = [{ name: 'scribe' }, { name: 'auditor' }] -// As `fetchAllSessions` returns them: most recent first. +// As `fetchAllSessions` returns them: most recent first. Every one of them has +// been spoken in (`last_message_at` + a count), which is what "threads with +// that agent exist" has meant here since ent#784's 2026-10-05 ruling gave an +// UNUSED chat its own arm — a row with neither field is an empty chat the rule +// now reuses, so leaving these bare would have tested the opposite case. const threads = [ - { id: 's3', agent_name: 'auditor' }, - { id: 's2', agent_name: 'scribe' }, - { id: 's1', agent_name: 'scribe' }, + { id: 's3', agent_name: 'auditor', last_message_at: '2026-09-07T12:00:00Z', message_count: 2 }, + { id: 's2', agent_name: 'scribe', last_message_at: '2026-09-07T11:00:00Z', message_count: 2 }, + { id: 's1', agent_name: 'scribe', last_message_at: '2026-09-07T10:00:00Z', message_count: 4 }, ] describe('resolveAgentLanding', () => { it('starts a new chat even when threads with that agent exist (ent#784)', () => { // ent#784 reversed ent#523's resume-by-default: opening an agent means - // starting new work. The deep link and the sidebar row share one rule. + // starting new work, and a chat that has been spoken in is never resumed. + // The deep link and the sidebar row share one rule. expect(resolveAgentLanding({ agent: 'scribe', agents, threads })) .toEqual({ agentName: 'scribe', sessionId: null }) }) @@ -52,10 +57,15 @@ describe('resolveAgentLanding', () => { }) it('tolerates a thread list that uses session_id instead of id', () => { - // ent#784: the shape no longer changes the answer for this door — a deep - // link carries no session memory, so it always resolves to a fresh chat. + // Either shape has to be readable, because the two reads disagree: the + // row below is an empty chat, and the arm that reuses one has to find its + // id under whichever key this list carries. const alt = [{ session_id: 'sX', agent_name: 'scribe' }] expect(resolveAgentLanding({ agent: 'scribe', agents, threads: alt })) + .toEqual({ agentName: 'scribe', sessionId: 'sX' }) + // …and a USED chat under that same key is still never resumed. + const used = [{ session_id: 'sX', agent_name: 'scribe', last_message_at: '2026-09-07T10:00:00Z', message_count: 2 }] + expect(resolveAgentLanding({ agent: 'scribe', agents, threads: used })) .toEqual({ agentName: 'scribe', sessionId: null }) }) diff --git a/src/frontend/tests/unit/workspaceNewChat.spec.js b/src/frontend/tests/unit/workspaceNewChat.spec.js index 673fcea06..d1df70501 100644 --- a/src/frontend/tests/unit/workspaceNewChat.spec.js +++ b/src/frontend/tests/unit/workspaceNewChat.spec.js @@ -41,9 +41,17 @@ describe('ent#451 — ?new=1 asks for a fresh thread', () => { it('the landing function already honoured it', () => { expect(resolveAgentLanding({ agent: 'sage', forceNew: true, agents, threads })) .toEqual({ agentName: 'sage', sessionId: null }) - // ent#784 made `forceNew` redundant rather than wrong: a plain landing now - // resolves to a fresh chat too, so the flag can never resume a thread. + // ent#784's first shape made `forceNew` redundant — every landing was a + // fresh chat. The operator's 2026-10-05 ruling gave it back its job: the + // precedence can now answer an existing chat (a draft, or the empty one + // being reused — `ps_old` here has no message), and `?new=1` is the one + // way past ALL of it. So the flag is load-bearing again, and the two + // answers must differ or nothing is checking it. expect(resolveAgentLanding({ agent: 'sage', forceNew: false, agents, threads }).sessionId) + .toBe('ps_old') + // A USED thread is still never resumed, with or without the flag. + const used = [{ id: 'ps_old', agent_name: 'sage', last_message_at: '2026-09-07T10:00:00Z', message_count: 2 }] + expect(resolveAgentLanding({ agent: 'sage', forceNew: false, agents, threads: used }).sessionId) .toBeNull() }) From 1db1dd5cb4461e0c0b0b69fdc2006319e40b5eeb Mon Sep 17 00:00:00 2001 From: trinity-ability <309458136+trinity-ability@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:22:43 +0100 Subject: [PATCH 08/22] fix(portal): a landed new chat keeps its rail, its new-thread intent 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 --- docs/memory/architecture/workspace.md | 10 +- .../src/components/portal/portalRail.js | 15 +- src/frontend/src/views/Portal.vue | 58 +++- .../portalAgentLandingStage.mount.spec.js | 276 ++++++++++++++++++ 4 files changed, 346 insertions(+), 13 deletions(-) create mode 100644 src/frontend/tests/unit/portalAgentLandingStage.mount.spec.js diff --git a/docs/memory/architecture/workspace.md b/docs/memory/architecture/workspace.md index 3bc298963..3b50627c8 100644 --- a/docs/memory/architecture/workspace.md +++ b/docs/memory/architecture/workspace.md @@ -1024,7 +1024,15 @@ arm opens a row that already exists. `landOnAgent` is therefore synchronous, keeps `/workspace/a/:name`, and guards the call before writing `activeAgentName`; the composer's mount focus is gated by a `focusOnMount` mode (`always` for a gesture, `fine-pointer` for a landing) so an -unasked-for landing cannot raise a soft keyboard. Main is named by its role in both the tab strip and the header and is +unasked-for landing cannot raise a soft keyboard. Because that URL is now where a new chat +RESTS, three things follow from it. It carries the **rail** like every other chat: the rail +rules read it as a rail-free page only until the landing has put the named agent on stage +(`unlandedAgentPage`), and its column is reserved mid-load like `/workspace/c/:id`. Clicking +the row of the agent whose new chat is **already on stage** is a no-op in `openAgentPage` +— the push would not re-fire the landing, and clearing `startingNewChat` there sent the +first message without `new_thread`, i.e. into Main. And a landing that **reuses a chat** +(the empty one, a remembered one) mounts as a thread, which focuses nothing, so the shell +hands the caret over under the same pointer rule (`focusLandedComposer`). Main is named by its role in both the tab strip and the header and is not renameable; an archived chat stays a tab (ruled “becomes the newest tab”), though you never LAND in one by default; an unused Main is filtered from the **sidebar** only (a projection, not a filter on `threads`, because the strip must show Main from the first visit). diff --git a/src/frontend/src/components/portal/portalRail.js b/src/frontend/src/components/portal/portalRail.js index 76d900f5a..28ad672b2 100644 --- a/src/frontend/src/components/portal/portalRail.js +++ b/src/frontend/src/components/portal/portalRail.js @@ -516,9 +516,11 @@ export function railEmptyCopy(tab, participants) { // ---------------------------------------------------------------- shell placement /** - * Whom the rail is scoped to. The agent page is a destination of its own with - * no conversation beside it, so it has no rail (#474: the rail renders beside - * `PortalConversation` and `PortalRoom`). A room's participants arrive with + * Whom the rail is scoped to. `agentPage` is a stage with no conversation + * beside it, so it has no rail (#474: the rail renders beside + * `PortalConversation` and `PortalRoom`) — since ent#784 that is an agent URL + * which has not LANDED yet, never one a new chat is resting on (the shell + * decides which, `Portal.vue::unlandedAgentPage`). A room's participants arrive with * the room's own fetch, so this can legitimately be empty for a beat — the * rail stays mounted on the ROUTE fact (`railVisibleFor`) and its tabs show * their empty states rather than the rail flickering in and out. @@ -539,7 +541,9 @@ export function railParticipantsFor({ * synchronous (the route) or verdicts (the stage state), never on data that is * still arriving — so a live update can never remount it. * - * agentPage `/workspace/a/:name` — a page, not a conversation + * agentPage a stage with no conversation: `/workspace/a/:name` before it + * has landed, or the Projects pages. NOT a landed agent URL — + * that is a new chat, and has the rail every chat has (ent#784) * stageState `stageZone(...).state`: only a `ready` stage holds a * conversation; loading / failed / empty stages show no rail * roomId `/workspace/r/:id` — the rail follows the room only where @@ -577,7 +581,8 @@ export function railVisibleFor({ * * Deliberately narrower than `railVisibleFor`'s route set: * - * - an agent page never carries a rail, so reserving there would invent a gap; + * - a Projects page never carries a rail, so reserving there would invent a + * gap (an agent URL DOES reserve: since ent#784 it is a 1:1 route itself); * - a ROOM route is excluded even though a ready room usually has a rail, * because that depends on `roomsAvailable`, which arrives ON the roster * payload (#2128) — reserving on a capability we have not been told about diff --git a/src/frontend/src/views/Portal.vue b/src/frontend/src/views/Portal.vue index bc7c546ea..b52cc8f04 100644 --- a/src/frontend/src/views/Portal.vue +++ b/src/frontend/src/views/Portal.vue @@ -848,7 +848,7 @@ import { useRoute, useRouter } from 'vue-router' import { useClientPortalStore, MULTI_AGENT_UNAVAILABLE, PLATFORM_LOGIN_ROUTE } from '@/stores/clientPortal' import { useAuthStore } from '@/stores/auth' import { usePortalDraftsStore } from '@/stores/portalDrafts' -import { threadKey } from '@/components/portal/portalDrafts' +import { threadKey, shouldAutoFocusComposer } from '@/components/portal/portalDrafts' import { safeStorage } from '@/utils/safeStorage' import PortalSidebar from '@/components/portal/PortalSidebar.vue' import PortalBrand from '@/components/portal/PortalBrand.vue' @@ -1059,9 +1059,20 @@ const inboxAgentLabels = computed(() => agentLabels(store.agents)) // ent#661: `/workspace/projects` and `/workspace/projects/:projectId`. const projectsRoute = computed(() => route.name === 'WorkspaceProjects' || route.name === 'WorkspaceProject') const activeProjectId = computed(() => route.params.projectId || null) -// A stage page that never carries a rail: the agent page (ent#360) and the -// Projects pages (ent#661). Both rail rules read it as their `agentPage`. -const railFreePage = computed(() => activeAgentPageName.value || (projectsRoute.value ? 'projects' : null)) +// ent#784 (review): `/workspace/a/:name` used to be left within a tick, so the +// rail rules could read it as "a page, not a conversation". It is now where a +// landed new chat RESTS — the same conversation `/workspace` and +// `/workspace/c/:id` show, one URL apart — so it is rail-free only until the +// landing has put the named agent on stage. Reading it as a page for longer +// gave a new chat no rail, and the first send (which moves the URL to the +// thread's own) then slid one in beside a reply already streaming. +const unlandedAgentPage = computed(() => ( + activeAgentPageName.value && activeAgentName.value !== activeAgentPageName.value + ? activeAgentPageName.value + : null)) +// A stage page that carries no rail: an agent URL that has not landed yet, and +// the Projects pages (ent#661). The rail rules read it as their `agentPage`. +const railFreePage = computed(() => unlandedAgentPage.value || (projectsRoute.value ? 'projects' : null)) const activeAgent = computed(() => { if (isInboxRoute.value) { return inboxSelectedAgent({ item: inboxSelection.value, threads: threads.value, asks: store.asks, agents: store.agents }) @@ -1221,7 +1232,7 @@ const roomParticipants = ref([]) const workSignal = ref(emptySignal()) const railParticipants = computed(() => railParticipantsFor({ - agentPage: activeAgentPageName.value, + agentPage: unlandedAgentPage.value, roomId: activeRoomIdFromRoute.value, roomParticipants: roomParticipants.value, activeAgent: activeAgent.value?.name, @@ -1262,9 +1273,13 @@ const railVisible = computed(() => railVisibleFor({ const RAIL_MOTION = 'transition-[width] duration-300 ease-out overflow-hidden ' + 'motion-reduce:transition-none motion-reduce:duration-0' +// ent#784 (review): NOT `railFreePage` here. That is true for an agent URL +// until it lands, which on a cold link is the whole of the loading stage — the +// only stage this rule answers for — so reading it would never reserve the +// column for a route that is now a 1:1 conversation like `/workspace/c/:id`. const railColumnReserved = computed(() => Boolean( railColumnReservedFor({ - agentPage: railFreePage.value, + agentPage: projectsRoute.value ? 'projects' : null, stageState: stage.value.state, roomId: activeRoomIdFromRoute.value, }) && !voiceCanvasHasColumn.value @@ -1406,6 +1421,17 @@ function openSuggestionSection(name) { function focusConversationComposer() { conversationRef.value?.focusComposer?.() } +// ent#784 (review): the caret for a LANDING, under the landing's own rule. A +// new chat focuses itself when it mounts (`focusOnMount`); a landing that +// reuses a chat — the agent's empty one, a remembered one — mounts as a thread, +// and a thread mount focuses nothing. So the shell hands the caret over for +// those, reading the same `composerFocusMode` the mount reads: a landing on a +// touch device still does not summon the keyboard. +function focusLandedComposer() { + if (composerFocusMode.value === 'fine-pointer' + && !shouldAutoFocusComposer(typeof window !== 'undefined' ? window.matchMedia?.bind(window) : null)) return + nextTick(focusConversationComposer) +} function onRoomParticipants(list) { roomParticipants.value = Array.isArray(list) ? list : [] } // A signal belongs to the chat that reported it. The conversation is keyed by @@ -1755,6 +1781,20 @@ function openAgentPage(name) { // ent#551 QA: the rail's agent row. It pushed a route with no guard, so the // conversation remounted and the call ended without a word. if (guardLeaveCall(() => openAgentPage(name))) return + // ent#784 (review): the row of the agent whose landed new chat is ALREADY on + // stage. The push below would be a no-op (same URL), so the route watcher + // would not re-fire and nothing would re-land — while the two writes above it + // would still spend `startingNewChat`. The composer then kept reading "New + // chat" and sent its first message without `new_thread`, which the server + // files in the agent's MAIN chat. There is nothing to open: the person is + // where the click leads, so it only hands the caret back to the field the + // click took it from. + if (activeAgentPageName.value === name && activeAgentName.value === name + && startingNewChat.value && !pendingSession.value) { + composerFocusMode.value = 'fine-pointer' + focusLandedComposer() + return + } unreachableAgent.value = null pendingSession.value = null startingNewChat.value = false @@ -1805,7 +1845,11 @@ function landOnAgent(name) { // `thread:` at setup and `PortalConversation`'s mount then puts the // caret at its end, on a fine pointer only — the same rule as above, shared // through `shouldAutoFocusComposer` rather than re-decided here. - if (row) { openThread(row); return } + // + // (review) A landing on a chat with NO draft — the agent's empty one — is + // restored by nothing, so that mount leaves focus on the sidebar row the + // click came from. `focusLandedComposer` covers it under the same rule. + if (row) { openThread(row); focusLandedComposer(); return } } // Idempotent: the watcher fires on the route param AND on the thread list // arriving, and this landing no longer navigates away — so without this the diff --git a/src/frontend/tests/unit/portalAgentLandingStage.mount.spec.js b/src/frontend/tests/unit/portalAgentLandingStage.mount.spec.js new file mode 100644 index 000000000..164a2ed94 --- /dev/null +++ b/src/frontend/tests/unit/portalAgentLandingStage.mount.spec.js @@ -0,0 +1,276 @@ +// @vitest-environment jsdom +/** + * ent#784 (review) — what the stage is once an agent has LANDED. + * + * ent#784 made `/workspace/a/:name` the URL a landed new chat RESTS on, where + * every earlier version left it within a tick. Three things had only ever been + * true because nobody stayed there, and each was reproduced in a browser: + * + * 1. Clicking the row of the agent you are already on cleared the fresh-chat + * intent (`openAgentPage`) and then pushed the URL it was already on — a + * no-op, so the landing never re-ran. The composer still read "New chat", + * and its first send went out without `new_thread`, which the server + * resolves to the agent's MAIN chat. + * 2. 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. + * 3. A landing that REUSES a chat (the empty one, or a remembered one) goes + * through `openThread`, and a thread mount focuses nothing — so "the cursor + * is in the message field" held for a new chat and not for the agent's + * empty one. + * + * `Portal.vue` is mounted for real (shallow — the harness of + * `portalAgentLandingRemount.mount.spec.js`), so the click door, the rail's + * conditions and the focus hand-off are the shell's own. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' +import { defineComponent, h } from 'vue' +import { shallowMount, flushPromises } from '@vue/test-utils' +import { setActivePinia, createPinia } from 'pinia' +import { createRouter, createMemoryHistory } from 'vue-router' + +// The landing's focus rule asks `(pointer: fine)`; jsdom answers every query +// `false`, i.e. a phone. Each case says which device it is on. +let finePointer = false +vi.hoisted(() => { + window.matchMedia = window.matchMedia || ((q) => ({ + matches: false, media: q, addEventListener() {}, removeEventListener() {}, addListener() {}, removeListener() {}, + })) +}) +function installMatchMedia() { + window.matchMedia = (q) => ({ + matches: q.includes('pointer: fine') ? finePointer : false, + media: q, + addEventListener() {}, removeEventListener() {}, addListener() {}, removeListener() {}, + }) +} + +vi.mock('@/stores/auth', () => ({ + useAuthStore: () => ({ isAuthenticated: true, authHeader: {}, userEmail: 'me@example.com', logout: vi.fn() }), +})) +vi.mock('axios', () => { + const mk = () => ({ + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), + interceptors: { request: { use: vi.fn() }, response: { use: vi.fn() } }, + defaults: { headers: { common: {} } }, + }) + return { + default: Object.assign( + { get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), create: mk }, + { interceptors: { request: { use: vi.fn() }, response: { use: vi.fn() } }, + defaults: { headers: { common: {} } } }, + ), + } +}) + +import { useClientPortalStore } from '@/stores/clientPortal' +import Portal from '@/views/Portal.vue' + +globalThis.ResizeObserver = globalThis.ResizeObserver || class { observe() {} unobserve() {} disconnect() {} } + +const Blank = { template: '
' } +function makeRouter() { + return createRouter({ + history: createMemoryHistory(), + routes: [ + { path: '/workspace', component: Blank }, + { path: '/workspace/inbox', component: Blank }, + { path: '/workspace/c/:sessionId', component: Blank }, + { path: '/workspace/r/:roomId', component: Blank }, + { path: '/workspace/a/:agentName', component: Blank }, + { path: '/login', component: Blank }, + ], + }) +} + +// scout's Main is USED, so opening scout is a new chat (arm 5). sage's Main is +// EMPTY, so opening sage reuses it (arm 4) — the landing that names a thread. +const SCOUT_MAIN = { id: 't1', session_id: 't1', agent_name: 'scout', is_main: true, last_message_at: '2026-09-27T10:00:00Z', unread: 0 } +const SAGE_EMPTY = { id: 'g1', session_id: 'g1', agent_name: 'sage', is_main: true, created_at: '2026-09-20T10:00:00Z', unread: 0 } +const THREADS = [SCOUT_MAIN, SAGE_EMPTY] + +let store +let focusComposer + +// Stubbed, but honest about the one thing the shell reaches through +// `conversationRef` here: the auto-stub exposes no `focusComposer`. +const ConversationStub = defineComponent({ + name: 'PortalConversation', + props: ['agent', 'sessionId', 'newChat', 'threads', 'focusOnMount', 'prefill'], + setup(_, { expose }) { + expose({ focusComposer, startVoiceCall: vi.fn(), endVoiceCall: vi.fn() }) + return () => h('div') + }, +}) + +function arm() { + store = useClientPortalStore() + store.portalToken = 'tok' + store.fetchRoster = vi.fn(async () => { + store.agents = [{ name: 'scout' }, { name: 'sage' }] + store.error = null + store.rosterLoaded = true + }) + store.fetchAllSessions = vi.fn(async () => { store.sessionsFailed = false; return THREADS }) + store.fetchChatState = vi.fn(async (opts) => (opts ? { state: {}, previews: {} } : {})) + store.fetchSessions = vi.fn(async () => THREADS) + store.fetchAsks = vi.fn(async () => []) + store.seedAgentRecency = vi.fn() + store.ensureBriefing = vi.fn() + store.loadAgentSuggestions = vi.fn() + store.markChatRead = vi.fn(async () => {}) +} + +let mounted = [] +async function boot(path) { + arm() + const router = makeRouter() + await router.push(path) + await router.isReady() + const w = shallowMount(Portal, { + global: { plugins: [router], stubs: { PortalConversation: ConversationStub } }, + }) + mounted.push(w) + await flushPromises(); await flushPromises() + return { w, router } +} + +const conversation = (w) => w.findComponent({ name: 'PortalConversation' }) +// `uid` is the instance identity: a `convGen` bump changes `:key`, so a remount +// is a new component instance with a new uid. (Not `c.vm` — with a hand-written +// stub the wrapper hands back a fresh proxy per read, which never compares equal.) +const showing = (w) => { + const c = conversation(w) + return { session: c.props('sessionId'), newChat: c.props('newChat'), agent: c.props('agent')?.name, uid: c.vm.$.uid } +} +// The desktop sidebar's agent row: the click door, exactly as the template binds it. +async function clickAgentRow(w, name) { + w.findAllComponents({ name: 'PortalSidebar' })[0].vm.$emit('open-agent', name) + await flushPromises() + await w.vm.$nextTick() +} +const rail = (w) => w.findComponent({ name: 'PortalRail' }) + +beforeEach(() => { + localStorage.clear() + setActivePinia(createPinia()) + finePointer = false + installMatchMedia() + focusComposer = vi.fn() +}) +afterEach(() => { + for (const w of mounted) w.unmount() + mounted = [] +}) + +describe('ent#784 — clicking the agent you are already on keeps the new chat', () => { + it('a second click on the same row does not spend the fresh-chat intent', async () => { + const { w, router } = await boot('/workspace/a/scout') + const landed = showing(w) + expect(landed.newChat).toBe(true) + expect(landed.session).toBe(null) + + await clickAgentRow(w, 'scout') + + // Still the SAME unsent chat, and still one that will ask for a new thread: + // `newChat` is what the composer turns into `new_thread` on its first send, + // and without it the server files the message in the agent's Main. + const after = showing(w) + expect(after.newChat).toBe(true) + expect(after.session).toBe(null) + expect(after.uid).toBe(landed.uid) // no remount + expect(router.currentRoute.value.path).toBe('/workspace/a/scout') + }) + + it('a click on a DIFFERENT agent still lands on that agent', async () => { + // The door must stay a door: the early return is for the row you are on. + const { w } = await boot('/workspace/a/scout') + await clickAgentRow(w, 'sage') + expect(showing(w).agent).toBe('sage') + }) + + it('the same click from a THREAD of that agent still lands fresh', async () => { + // Being "on scout" is not being on scout's NEW chat: from one of its + // threads the row means what it always meant. + const { w, router } = await boot('/workspace/c/t1') + expect(showing(w).session).toBe('t1') + await clickAgentRow(w, 'scout') + expect(router.currentRoute.value.path).toBe('/workspace/a/scout') + expect(showing(w).newChat).toBe(true) + expect(showing(w).session).toBe(null) + }) +}) + +describe('ent#784 — a landed new chat has the rail every other chat has', () => { + it('the rail is beside a thread (control)', async () => { + const { w } = await boot('/workspace/c/t1') + expect(rail(w).exists()).toBe(true) + }) + + it('and beside the new chat an agent click lands on', async () => { + // Same agent, same conversation component, one URL apart. Without the rail + // here the first send — which moves the URL to `/workspace/c/:id` — brings + // the column in beside a reply that is already streaming. + const { w, router } = await boot('/workspace/a/scout') + expect(router.currentRoute.value.path).toBe('/workspace/a/scout') + expect(showing(w).newChat).toBe(true) + expect(rail(w).exists()).toBe(true) + }) + + it('but not beside the refusal for an agent the link cannot reach', async () => { + const { w } = await boot('/workspace/a/ghost') + expect(rail(w).exists()).toBe(false) + }) + + it('and its column is held while an agent link is still loading (#2711)', async () => { + // The agent URL is a 1:1 conversation route now, so it reserves the rail's + // column mid-load like `/workspace/c/:id` does — or the conversation paints + // full width and gives the width back the moment the roster lands. + arm() + store.fetchRoster = vi.fn(() => new Promise(() => {})) // the roster never answers + const router = makeRouter() + await router.push('/workspace/a/scout') + await router.isReady() + const w = shallowMount(Portal, { + global: { plugins: [router], stubs: { PortalConversation: ConversationStub } }, + }) + mounted.push(w) + await flushPromises() + expect(w.find('[data-reserved="true"]').exists()).toBe(true) + }) +}) + +describe('ent#784 — a landing that reuses a chat puts the caret in the field too', () => { + it('opening an agent whose empty chat is reused focuses the composer (fine pointer)', async () => { + finePointer = true + const { w, router } = await boot('/workspace/c/t1') + focusComposer.mockClear() + + await clickAgentRow(w, 'sage') + + expect(router.currentRoute.value.path).toBe('/workspace/c/g1') // arm 4: the empty chat + expect(showing(w).session).toBe('g1') + expect(focusComposer).toHaveBeenCalled() + }) + + it('and does not on a touch device, where focus summons the keyboard', async () => { + finePointer = false + const { w, router } = await boot('/workspace/c/t1') + focusComposer.mockClear() + + await clickAgentRow(w, 'sage') + + expect(router.currentRoute.value.path).toBe('/workspace/c/g1') + expect(focusComposer).not.toHaveBeenCalled() + }) + + it('a second click on the row you are on hands the caret back', async () => { + // The click itself moved focus to the sidebar button; nothing remounts, so + // nothing else would return it. + finePointer = true + const { w } = await boot('/workspace/a/scout') + focusComposer.mockClear() + await clickAgentRow(w, 'scout') + expect(focusComposer).toHaveBeenCalled() + }) +}) From 27cfcc467c8d65048c6bda8a1ff771081f59b7f2 Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Mon, 5 Oct 2026 08:50:31 -0400 Subject: [PATCH 09/22] feat(portal): the Workspace key map and its pure helpers (abilityai/trinity-enterprise#621) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Checkpoint A of ent#621: the declaration every Workspace key will be dispatched from, with nothing wired to it yet (the shell dispatcher is B). `portalKeymap.js` declares all nine shell chords — ⌘J, ⌘., ⌘/, ⌥↑/↓, ⌥⇧↑/↓, ⌥. — plus the reserved ⌘K (ent#577) and the six component-owned protocol keys (Esc, the call's M, the preview's arrows, tab roving, a dialog's Tab cycle, the typeahead's bare arrows). The protocol entries are declared precisely because `keymapCollisions` is only worth anything if it can see the whole surface: a shell chord that shadows Esc is the 2026-09-07 class, and a map listing only its own keys cannot tell you. Matching is platform-free — `primary` is meta XOR ctrl everywhere (the shipped ⌘J semantics) and a chord matches the printed `key` OR the physical `code`. The two arms disagree about Shift on purpose: without the `key` arm DE's `Shift+7` → `/` is dead, without the `code` arm `⌥.` is dead on a US Mac (it types `≥`), and with Shift accepted on the `code` arm macOS `⌘?` Help search is swallowed. - `isNewChatHotkey` / `newChatHotkeyLabel` delegate to the map, keeping their signatures and their truth table, so "what ⌘J means" has one answer. The physical `KeyJ` arm only widens the set. - `resolveComposerKey` claims BARE arrows only. It used to claim an arrow whatever the modifiers were and the caller `preventDefault`s a `move-*`, so ⌥↓ / ⌥⇧↓ were silently dead whenever an @-popup happened to be open — a handler claiming a chord it never declared. - The reserved ⌘K resolves to nothing: the shell must not `preventDefault` a key it cannot answer, so the browser's Ctrl+K keeps working until Spotlight ships. It is hidden from the key list too — a dead row in a help dialog is worse than an undocumented key. - `keymapSuppressed` is a matrix: anything modal stops every shell key (⌘J included — today it remounts under an open file preview), while a call stops only the keys that would move you. ⌘J keeps its own "leave the call?" ask; ⌘/ is allowed, because opening a dialog leaves nothing. - `hasModalOpen` is a DOM probe, not a registry: it sees overlays that do not exist yet, and its exemption is per action (the rail keys do not see the rail's own sheet; ⌘/ can close its own list). - The stale `agentLanding` header comment is corrected — the switch-agent keys ARE a caller, passing arm 2's `lastOpenSessionId`; the gesture doors (New chat, the picker, ⌘J) still mean "fresh". `portalKeymap.js` reuses `isMacLike` / `nextActiveIndex` and `portalUtils.js` delegates back, which closes a module cycle. It is benign — neither module touches an import of the other at evaluation time — and both init orders are exercised: `workspaceKeymap.spec.js` imports the map first, `portalChatTabsAndTitles.spec.js` the utils. Tests: 190 pass across the four specs. Mutation battery, restored from a scratch copy: dropping the bare-arrow guard reddens the composer row; a chat chord without its Shift reddens four keymap cases including the collision check; Shift on the physical arm reddens the Decision 29 case; a reserved key that resolves reddens the ⌘K case. Co-Authored-By: Claude Opus 5 --- .../src/components/portal/portalKeymap.js | 463 ++++++++++++++++++ .../src/components/portal/portalUtils.js | 41 +- .../unit/portalChatTabsAndTitles.spec.js | 21 + .../unit/portalComposerTypeahead.spec.js | 17 + .../tests/unit/workspaceKeymap.dom.spec.js | 78 +++ .../tests/unit/workspaceKeymap.spec.js | 373 ++++++++++++++ 6 files changed, 982 insertions(+), 11 deletions(-) create mode 100644 src/frontend/src/components/portal/portalKeymap.js create mode 100644 src/frontend/tests/unit/workspaceKeymap.dom.spec.js create mode 100644 src/frontend/tests/unit/workspaceKeymap.spec.js diff --git a/src/frontend/src/components/portal/portalKeymap.js b/src/frontend/src/components/portal/portalKeymap.js new file mode 100644 index 000000000..696062908 --- /dev/null +++ b/src/frontend/src/components/portal/portalKeymap.js @@ -0,0 +1,463 @@ +import { isMacLike, nextActiveIndex } from './portalUtils' + +/** + * The Workspace key map (ent#621) — one declaration, one resolver. + * + * Every key the Workspace answers is declared HERE, including the ones this + * module never dispatches: the reserved `⌘K` (ent#577 Spotlight) and the + * component-owned protocol keys (Esc, the call's `M`, the preview's arrows, + * tab roving, a dialog's Tab cycle, the typeahead's bare arrows). Declaring + * them is what lets `keymapCollisions` see the WHOLE surface — a shell chord + * that shadows a protocol key is the 2026-09-07 class, and a map that lists + * only its own keys cannot tell you about it. + * + * Matching is deliberately platform-free: `primary` is meta XOR ctrl on every + * platform (the shipped ⌘J semantics, ent#451), and a chord matches by `key` + * OR by physical `code`. The `code` arm is the non-US fallback — on a layout + * whose `key` is non-Latin (Cyrillic, Greek, Hebrew, Arabic) the US position + * still works — and on a US Mac it is the ONLY arm for `⌥.`, which types `≥`. + * `platform` is therefore an argument of the LABELS (`chordLabel`, + * `ariaKeyshortcuts`), never of the matcher. + * + * Shift is accepted on a `key` match only (`shiftOnKey`, Decision 29): DE's + * `Shift+7` → `/` and FR's shifted `.` must work, while macOS `⌘?` (Help + * search, `key: '?'`) must stay the browser's. + */ + +/** Dispatched by the shell. */ +export const OWNER_SHELL = 'shell' +/** Owned by a component, declared here so collisions are visible. */ +export const OWNER_PROTOCOL = 'overlay-protocol' + +/** Keys that CHANGE what is on stage — suppressed during a voice call. */ +export const SCOPE_MOVING = 'moving' +/** Keys that DO something where you are. */ +export const SCOPE_DOING = 'doing' +/** Component-owned keys (Esc and friends). Never dispatched here. */ +export const SCOPE_PROTOCOL = 'protocol' + +const chord = (c) => Object.freeze({ + key: null, code: null, primary: false, alt: false, shift: false, shiftOnKey: false, ...c, +}) + +export const WORKSPACE_KEYMAP = Object.freeze([ + Object.freeze({ + id: 'find-anything', + action: 'find-anything', + surface: 'workspace', + owner: OWNER_SHELL, + scope: SCOPE_DOING, + // ent#577 owns the surface. Reserved means exactly this: nothing binds the + // chord, so the browser's own Ctrl+K keeps working until Spotlight ships. + // It is hidden from the key list rather than listed as "soon" — a dead row + // in a help dialog is worse than an undocumented key. + reserved: true, + label: 'Find anything', + note: 'Reserved for search (not yet available).', + chord: chord({ key: 'k', code: 'KeyK', primary: true }), + }), + Object.freeze({ + id: 'new-chat', + action: 'new-chat', + surface: 'workspace', + owner: OWNER_SHELL, + scope: SCOPE_DOING, + label: 'New chat with this agent', + chord: chord({ key: 'j', code: 'KeyJ', primary: true }), + }), + Object.freeze({ + id: 'rail-toggle', + action: 'rail-toggle', + surface: 'workspace', + owner: OWNER_SHELL, + scope: SCOPE_DOING, + // A call takes the rail's column for the voice canvas, so the key has + // nothing to show: silent no-op, like every moving key (T8). + callSuppressed: true, + needsRail: true, + label: 'Show or hide the rail', + chord: chord({ key: '.', code: 'Period', primary: true }), + }), + Object.freeze({ + id: 'key-list', + action: 'key-list', + surface: 'workspace', + owner: OWNER_SHELL, + scope: SCOPE_DOING, + label: 'Keyboard shortcuts', + chord: chord({ key: '/', code: 'Slash', primary: true, shiftOnKey: true }), + }), + Object.freeze({ + id: 'agent-next', + action: 'agent-next', + surface: 'workspace', + owner: OWNER_SHELL, + scope: SCOPE_MOVING, + callSuppressed: true, + label: 'Next agent', + note: 'One agent per press — a held key does not walk the list.', + chord: chord({ key: 'ArrowDown', alt: true }), + }), + Object.freeze({ + id: 'agent-prev', + action: 'agent-prev', + surface: 'workspace', + owner: OWNER_SHELL, + scope: SCOPE_MOVING, + callSuppressed: true, + label: 'Previous agent', + chord: chord({ key: 'ArrowUp', alt: true }), + }), + Object.freeze({ + id: 'chat-next', + action: 'chat-next', + surface: 'workspace', + owner: OWNER_SHELL, + scope: SCOPE_MOVING, + callSuppressed: true, + label: "Next chat with this agent", + chord: chord({ key: 'ArrowDown', alt: true, shift: true }), + }), + Object.freeze({ + id: 'chat-prev', + action: 'chat-prev', + surface: 'workspace', + owner: OWNER_SHELL, + scope: SCOPE_MOVING, + callSuppressed: true, + label: 'Previous chat with this agent', + chord: chord({ key: 'ArrowUp', alt: true, shift: true }), + }), + Object.freeze({ + id: 'rail-tab-next', + action: 'rail-tab-next', + surface: 'workspace', + owner: OWNER_SHELL, + scope: SCOPE_MOVING, + callSuppressed: true, + needsRail: true, + label: 'Next rail tab', + chord: chord({ key: '.', code: 'Period', alt: true }), + }), + // --- protocol: owned where the learnings put them, declared here ---------- + Object.freeze({ + id: 'close-top', + action: 'close-top', + surface: 'workspace', + owner: OWNER_PROTOCOL, + scope: SCOPE_PROTOCOL, + context: 'the innermost thing first — popup, reply chip, dialog, sheet, then a running turn', + label: 'Close or cancel', + chords: [chord({ key: 'Escape' })], + }), + Object.freeze({ + id: 'call-mute', + action: 'call-mute', + surface: 'workspace', + owner: OWNER_PROTOCOL, + scope: SCOPE_PROTOCOL, + context: 'during a voice call', + label: 'Mute or unmute', + chords: [chord({ key: 'm', code: 'KeyM' })], + }), + Object.freeze({ + id: 'preview-step', + action: 'preview-step', + surface: 'workspace', + owner: OWNER_PROTOCOL, + scope: SCOPE_PROTOCOL, + context: 'in the file preview', + label: 'Previous or next file', + chords: [chord({ key: 'ArrowLeft' }), chord({ key: 'ArrowRight' })], + }), + Object.freeze({ + id: 'tab-roving', + action: 'tab-roving', + surface: 'workspace', + owner: OWNER_PROTOCOL, + scope: SCOPE_PROTOCOL, + context: 'once a tab strip has focus', + label: 'Move between tabs', + chords: [ + chord({ key: 'ArrowLeft' }), chord({ key: 'ArrowRight' }), + chord({ key: 'Home' }), chord({ key: 'End' }), + ], + }), + Object.freeze({ + id: 'dialog-focus-cycle', + action: 'dialog-focus-cycle', + surface: 'workspace', + owner: OWNER_PROTOCOL, + scope: SCOPE_PROTOCOL, + context: 'inside an open dialog (the focus trap)', + label: 'Move focus', + chords: [chord({ key: 'Tab' }), chord({ key: 'Tab', shift: true })], + }), + Object.freeze({ + id: 'typeahead-step', + action: 'typeahead-step', + surface: 'workspace', + owner: OWNER_PROTOCOL, + scope: SCOPE_PROTOCOL, + // Decision 36: BARE arrows only. The popup claiming `⌥↓` was a handler + // claiming a chord it never declared. + context: 'while the @ or / popup is open', + label: 'Move through the suggestions', + chords: [chord({ key: 'ArrowUp' }), chord({ key: 'ArrowDown' })], + }), +]) + +/** Every chord an entry answers, single- or multi-chord. */ +export function entryChords(entry) { + if (!entry) return [] + if (Array.isArray(entry.chords)) return entry.chords.filter(Boolean) + return entry.chord ? [entry.chord] : [] +} + +/** The entry for an action id, or null. */ +export function findBinding(action, map = WORKSPACE_KEYMAP) { + const list = Array.isArray(map) ? map : [] + return list.find((e) => e && e.action === action) || null +} + +/** + * Does this keyboard event answer this chord? + * + * Tolerant of a plain object (the existing ⌘J truth table passes one), so a + * missing `code` simply disables the physical arm rather than throwing. + */ +export function matchesChord(e, c) { + if (!e || !c) return false + const meta = !!e.metaKey + const ctrl = !!e.ctrlKey + const primaryOk = c.primary ? (meta !== ctrl) : (!meta && !ctrl) + if (!primaryOk) return false + if (!!e.altKey !== !!c.alt) return false + const shiftOk = !!e.shiftKey === !!c.shift + const byKey = !!c.key && typeof e.key === 'string' + && e.key.toLowerCase() === String(c.key).toLowerCase() + && (c.shiftOnKey || shiftOk) + // Decision 29: shift is never accepted on the physical arm — macOS `⌘?` + // (Help search) sits on `Shift`+`Slash` and must stay the browser's. + const byCode = !!c.code && typeof e.code === 'string' && e.code === c.code && shiftOk + return byKey || byCode +} + +/** + * The action this event asks for, or null. + * + * Reserved and protocol entries resolve to NULL on purpose: the shell must not + * `preventDefault` a key it does not handle (⌘K stays the browser's until + * ent#577), and Esc is a capture-phase protocol the shell never dispatches. + */ +export function resolveWorkspaceKey(e, map = WORKSPACE_KEYMAP) { + if (!e) return null + // A held key must not remount the conversation N times a second. + if (e.repeat) return null + if (e.isComposing || e.keyCode === 229) return null + const list = Array.isArray(map) ? map : [] + for (const entry of list) { + if (!entry || entry.reserved || entry.owner !== OWNER_SHELL) continue + if (entryChords(entry).some((c) => matchesChord(e, c))) return entry.action + } + return null +} + +const chordSignatures = (c) => { + const mods = `|p:${!!c.primary}|a:${!!c.alt}` + const out = [] + if (c.key) { + const k = `k:${String(c.key).toLowerCase()}${mods}` + if (c.shiftOnKey) out.push(`${k}|s:true`, `${k}|s:false`) + else out.push(`${k}|s:${!!c.shift}`) + } + if (c.code) out.push(`c:${c.code}${mods}|s:${!!c.shift}`) + return out +} + +/** + * Pairs of entries that answer the same chord. + * + * Two protocol entries MAY share one (the preview's arrows and a tab strip's + * roving are the same keys in different places, each owned by the component + * that has focus) — what must never happen is a shell key shadowing another + * shell key, or shadowing a protocol key the learnings already assigned. + */ +export function keymapCollisions(map = WORKSPACE_KEYMAP) { + const list = (Array.isArray(map) ? map : []).filter(Boolean) + const found = [] + for (let i = 0; i < list.length; i += 1) { + for (let j = i + 1; j < list.length; j += 1) { + const a = list[i] + const b = list[j] + if (a.owner !== OWNER_SHELL && b.owner !== OWNER_SHELL) continue + const sigs = new Set(entryChords(a).flatMap(chordSignatures)) + const clash = entryChords(b).flatMap(chordSignatures).some((s) => sigs.has(s)) + if (clash) found.push([a.id, b.id]) + } + } + return found +} + +/** + * Should the shell stay out of the way? + * + * Anything modal — a dialog, the file preview, the agent picker, the mobile + * drawer — suppresses EVERY shell key, ⌘J included (today it remounts the + * conversation under an open file preview). A voice call suppresses the keys + * that would change what is on stage, plus the rail toggle; ⌘J keeps its own + * "leave the call?" ask (ent#534/551) and ⌘/ is allowed, because opening a + * dialog leaves nothing. + * + * Per-action exemptions are the CALLER's: it passes `hasModalOpen`'s `ignore` + * so rail keys do not see the rail's own sheet and ⌘/ can close its own list. + */ +export function keymapSuppressed({ + action, modalOpen = false, drawerOpen = false, callActive = false, defaultPrevented = false, +} = {}, map = WORKSPACE_KEYMAP) { + const entry = findBinding(action, map) + // Not ours to dispatch → suppressed, which fails closed for an action id + // that only exists in a future map. + if (!entry || entry.reserved || entry.owner !== OWNER_SHELL) return true + // A nearer owner already claimed this event (the Esc protocol, the + // typeahead's bare arrows): yielding is the protocol. + if (defaultPrevented) return true + if (modalOpen || drawerOpen) return true + return !!(callActive && entry.callSuppressed) +} + +/** + * Is something modal open right now? + * + * A DOM probe rather than a registry: it sees every overlay that is honest + * about being modal, including ones that do not exist yet, and an overlay that + * forgets to register is the whole 2026-09-07 class. The mobile drawer is NOT + * `aria-modal` (an `aria-modal` with no focus trap and no Esc is an AT trap, + * #1923) — the shell passes its own flag for that one. + */ +export function hasModalOpen(root, { ignore = [] } = {}) { + if (!root || typeof root.querySelector !== 'function') return false + const skip = (Array.isArray(ignore) ? ignore : [ignore]).filter(Boolean) + const selector = `[aria-modal="true"]${skip.map((s) => `:not(${s})`).join('')}` + return !!root.querySelector(selector) +} + +/** + * Wrapped index. The same arithmetic the typeahead's roving selection uses — + * one modulo in the codebase, not two. + */ +export function cycleIndex(current, delta, length) { + return nextActiveIndex(current, delta, length) +} + +/** + * The agent a switch key lands on, by name. + * + * `order` is the list the SIDEBAR shows (`orderRosterAgents`), so the keys walk + * what the eye sees. One agent (or none) is a silent no-op per the AC. + */ +export function nextAgent(order, activeName, delta) { + const list = (Array.isArray(order) ? order : []) + .map((a) => (typeof a === 'string' ? a : a && a.name)) + .filter((n) => typeof n === 'string' && n) + if (list.length < 2) return null + const i = list.indexOf(activeName) + const next = list[cycleIndex(i, delta, list.length)] + return next || null +} + +/** + * The "last open chat per agent" writer, as a rule rather than four call sites. + * + * `pendingSession` is a LOAD INSTRUCTION, not the chat on stage: clicking an + * agent row nulls it while the agent is unchanged, and recording that would + * forget the chat the person was just reading. So a null under an UNCHANGED + * agent is ignored; a null under a NEW agent is real (an unsent new chat), and + * recording it lets `agentLanding`'s own arms 3–5 return to the draft. + */ +export function recordLastOpen(map, { agentName, sessionId = null, keyChanged = false } = {}) { + if (!map || typeof map.set !== 'function') return false + if (!agentName || typeof agentName !== 'string') return false + if (!keyChanged && !sessionId) return false + map.set(agentName, sessionId || null) + return true +} + +/** + * The rail tab `⌥.` moves to. Walks only the tabs this session may SEE + * (`visibleTabs`), never `RAIL_TAB_ORDER` — a door-failed tab must stay + * unreachable by key as surely as by click. + */ +export function nextRailTab(tabs, currentId, delta = 1) { + const list = (Array.isArray(tabs) ? tabs : []).filter(Boolean) + if (!list.length) return null + const i = list.findIndex((t) => t && t.id === currentId) + const next = list[cycleIndex(i, delta, list.length)] + return (next && next.id) || null +} + +const GLYPHS = Object.freeze({ + ArrowUp: '↑', ArrowDown: '↓', ArrowLeft: '←', ArrowRight: '→', Escape: 'Esc', +}) + +const keyGlyph = (key) => { + if (!key) return '' + if (GLYPHS[key]) return GLYPHS[key] + return key.length === 1 ? key.toUpperCase() : key +} + +/** What a person reads: `⌥⇧↓` on a Mac, `Alt+Shift+↓` everywhere else. */ +export function chordLabel(c, platform) { + if (!c) return '' + const mac = isMacLike(platform) + const parts = [] + if (c.alt) parts.push(mac ? '⌥' : 'Alt') + if (c.shift) parts.push(mac ? '⇧' : 'Shift') + if (c.primary) parts.push(mac ? '⌘' : 'Ctrl') + parts.push(keyGlyph(c.key)) + return mac ? parts.join('') : parts.join('+') +} + +// WAI-ARIA 1.2: the PRINTED character for printable keys (`Alt+.`), key names +// only for non-printables (`Alt+ArrowDown`). Alternatives are space-separated. +const ariaKey = (key) => (key && key.length === 1 ? key.toUpperCase() : key || '') + +/** What assistive tech reads — `aria-keyshortcuts`, in ARIA's own spelling. */ +export function ariaKeyshortcuts(chordOrChords, platform) { + const list = Array.isArray(chordOrChords) ? chordOrChords : [chordOrChords] + return list.filter(Boolean).map((c) => { + const parts = [] + if (c.primary) parts.push(isMacLike(platform) ? 'Meta' : 'Control') + if (c.alt) parts.push('Alt') + if (c.shift) parts.push('Shift') + parts.push(ariaKey(c.key)) + return parts.join('+') + }).join(' ') +} + +/** + * The key list's rows, rendered FROM the map — nothing hand-typed, so a key + * that changes cannot leave a help dialog lying about it. + * + * The reserved entry is dropped (no dead rows). Where the page has no rail, + * the rail rows say so rather than disappearing: a key that is absent here and + * silent there is the one a person retries forever. + */ +export function keyListRows(map = WORKSPACE_KEYMAP, platform, { railAvailable = true } = {}) { + return (Array.isArray(map) ? map : []).filter((e) => e && !e.reserved).map((e) => { + const chords = entryChords(e) + const notes = [] + if (e.note) notes.push(e.note) + if (e.context) notes.push(e.context) + if (e.needsRail && !railAvailable) notes.push('This page has no rail.') + return { + action: e.action, + scope: e.scope, + owner: e.owner, + label: e.label, + keys: chords.map((c) => chordLabel(c, platform)).join(' / '), + ariaKeyshortcuts: ariaKeyshortcuts(chords, platform), + note: notes.join(' ') || null, + } + }) +} diff --git a/src/frontend/src/components/portal/portalUtils.js b/src/frontend/src/components/portal/portalUtils.js index f0489615d..8ea8340b1 100644 --- a/src/frontend/src/components/portal/portalUtils.js +++ b/src/frontend/src/components/portal/portalUtils.js @@ -7,6 +7,15 @@ // would close one. import { draftedLandingFor } from './portalDrafts' +// ent#621: the key map is the single declaration of every Workspace chord, so +// the two hotkey helpers below delegate to it rather than keeping a second +// definition of "⌘J". That closes a cycle with `portalKeymap.js` (which reuses +// `isMacLike` and `nextActiveIndex` from here) — benign, and it must STAY +// benign: neither module may touch an import of the other at module-evaluation +// time. Both init orders are exercised, by `workspaceKeymap.spec.js` (map +// first) and `portalChatTabsAndTitles.spec.js` (utils first). +import { resolveWorkspaceKey, findBinding, chordLabel } from './portalKeymap' + // Deterministic per-agent color — used for the avatar tint and the small thread // color dots in the sidebar so a thread visually ties to its agent. export function agentColor(name) { @@ -522,14 +531,16 @@ export function moreTabsLabel(n) { return `${n} more` } -// --- New chat hotkey (ent#451) ---------------------------------------------- +// --- New chat hotkey (ent#451, now one entry in the ent#621 map) ------------ // ⌘J on Mac, Ctrl+J elsewhere — ruled 2026-09-06 (⌘N is the browser's, ⌘⇧O // declined). Plain modifier only: Shift/Alt variants are someone else's. +// +// The predicate is kept (its callers and its truth table are the contract) but +// the RULE now lives in `portalKeymap.js` with the other eight chords, so +// "what ⌘J means" has one answer. The map adds the physical `KeyJ` arm, which +// only widens the set: a layout whose `key` is not `j` now works too. export function isNewChatHotkey(e) { - if (!e || typeof e.key !== 'string') return false - if (e.key.toLowerCase() !== 'j') return false - if (e.shiftKey || e.altKey) return false - return !!(e.metaKey || e.ctrlKey) && !(e.metaKey && e.ctrlKey) + return resolveWorkspaceKey(e) === 'new-chat' } export function isMacLike(platform) { @@ -564,7 +575,7 @@ export function titleGenerationNotice(health) { } export function newChatHotkeyLabel(platform) { - return isMacLike(platform) ? '⌘J' : 'Ctrl+J' + return chordLabel(findBinding('new-chat').chord, platform) } // #2101: bounded briefing hint grid. Order deterministically — a card with a @@ -1315,11 +1326,17 @@ export function resolveComposerKey({ // A faithful reproduction of Vue's `.exact`: any modifier falls through // unprevented and inserts a newline, exactly as today. const plainEnter = key === 'Enter' && !shiftKey && !ctrlKey && !metaKey && !altKey + // ent#621 (Decision 36): the popup owns BARE arrows only. Claiming `⌥↓` — + // and `preventDefault`ing it, which is what the caller does with `move-down` + // — was a handler claiming a chord it never declared, so the Workspace's + // switch-chat key died over an open @-popup. A modified arrow now falls + // through to `pass`, where the shell's declared binding can have it. + const bareArrow = !shiftKey && !ctrlKey && !metaKey && !altKey if (open) { if (key === 'Escape') return 'dismiss' - if (key === 'ArrowDown') return hasCandidates ? 'move-down' : 'close' - if (key === 'ArrowUp') return hasCandidates ? 'move-up' : 'close' + if (key === 'ArrowDown' && bareArrow) return hasCandidates ? 'move-down' : 'close' + if (key === 'ArrowUp' && bareArrow) return hasCandidates ? 'move-up' : 'close' if (key === 'Tab' && !shiftKey) return hasCandidates ? 'accept' : 'pass' if (plainEnter && hasCandidates && hasActive) return 'accept' if (CARET_KEYS.has(key)) return 'close' @@ -1797,9 +1814,11 @@ export function feedbackAcknowledgement(captureFeedback) { // --- Agents at the centre (ent#523, landing rule replaced by ent#784) -------- // Where opening an agent lands you. ONE rule, called by every door that has to -// RESOLVE a landing (the sidebar/rail row via `landOnAgent`, and the `?agent=` -// deep link via `resolveAgentLanding`). The gesture doors — New chat, the agent -// picker, the switch-agent key — do not call this: they already mean "fresh". +// RESOLVE a landing: the sidebar/rail row via `landOnAgent`, the `?agent=` deep +// link via `resolveAgentLanding`, and — since ent#621 — the switch-agent KEYS, +// which call `landOnAgent` with the `lastOpenSessionId` arm 2 reads. The gesture +// doors still do not call this, because they already mean "fresh": New chat, the +// agent picker, and ⌘J. // // ent#784 reverses ent#523's rule: the default is a NEW, empty chat // (`sessionId: null`), because most visits to an agent start new work and diff --git a/src/frontend/tests/unit/portalChatTabsAndTitles.spec.js b/src/frontend/tests/unit/portalChatTabsAndTitles.spec.js index e5743b791..9256623a4 100644 --- a/src/frontend/tests/unit/portalChatTabsAndTitles.spec.js +++ b/src/frontend/tests/unit/portalChatTabsAndTitles.spec.js @@ -31,6 +31,7 @@ import { NEW_CHAT_TAB_ID, NEW_CHAT_TAB_LABEL, MAIN_TAB_LABEL, agentHasMain, titleSettling, shouldFetchTitleHealth, TITLE_SETTLE_DELAYS_MS, } from '../../src/components/portal/portalUtils' +import { resolveWorkspaceKey } from '../../src/components/portal/portalKeymap' const src = (rel) => readFileSync(resolve(__dirname, '../../src', rel), 'utf8') @@ -226,6 +227,26 @@ describe('the New chat hotkey', () => { expect(newChatHotkeyLabel('Linux x86_64')).toBe('Ctrl+J') expect(newChatHotkeyLabel(undefined)).toBe('Ctrl+J') }) + + // ent#621: the rule moved into the key map and this predicate delegates, so + // the truth table above is now also the map's. Asserting the EQUIVALENCE is + // what makes that safe: the day the two disagree, the hotkey every portal + // session has learned is the one that changed. This spec reaches both modules + // through `portalUtils` FIRST, which is the other half of the two modules' + // import-cycle check (`workspaceKeymap.spec.js` imports the map first). + it('is the map\'s new-chat entry, by construction (ent#621)', () => { + const cases = [ + ev({ metaKey: true }), ev({ ctrlKey: true }), ev({ ctrlKey: true, key: 'J' }), + ev({}), ev({ metaKey: true, shiftKey: true }), ev({ ctrlKey: true, altKey: true }), + ev({ metaKey: true, ctrlKey: true }), ev({ metaKey: true, key: 'k' }), + ev({ metaKey: true, key: 'ArrowDown' }), ev({ altKey: true, key: 'ArrowDown' }), + ] + for (const e of cases) { + expect(isNewChatHotkey(e), JSON.stringify(e)).toBe(resolveWorkspaceKey(e) === 'new-chat') + } + // And it is still TRUE for the chord it has always been true for. + expect(isNewChatHotkey(ev({ metaKey: true }))).toBe(true) + }) }) describe('titleGenerationNotice', () => { diff --git a/src/frontend/tests/unit/portalComposerTypeahead.spec.js b/src/frontend/tests/unit/portalComposerTypeahead.spec.js index d2de5600f..e45d81b6a 100644 --- a/src/frontend/tests/unit/portalComposerTypeahead.spec.js +++ b/src/frontend/tests/unit/portalComposerTypeahead.spec.js @@ -443,6 +443,23 @@ describe('resolveComposerKey', () => { expect(resolveComposerKey({ key: 'ArrowDown', open: true, hasCandidates: false })).toBe('close') }) + // ent#621 (Decision 36): the popup owns BARE arrows. It used to claim the + // arrow whatever the modifiers were, and the caller `preventDefault`s a + // `move-*`, so the Workspace's ⌥⇧↓ (switch chat) and ⌥↓ (switch agent) were + // silently dead for as long as an @-popup happened to be open — a handler + // claiming a chord it never declared. A modified arrow now passes. + it('claims a bare arrow only, and passes a chord it does not own', () => { + const open = { open: true, hasCandidates: true } + for (const mod of ['altKey', 'metaKey', 'ctrlKey', 'shiftKey']) { + expect(resolveComposerKey({ key: 'ArrowDown', ...open, [mod]: true }), mod).toBe('pass') + expect(resolveComposerKey({ key: 'ArrowUp', ...open, [mod]: true }), mod).toBe('pass') + } + expect(resolveComposerKey({ key: 'ArrowDown', altKey: true, shiftKey: true, ...open })).toBe('pass') + // Without candidates the popup used to CLOSE on a modified arrow; that was + // the same over-claim (it also prevented the event), so it passes too. + expect(resolveComposerKey({ key: 'ArrowDown', open: true, hasCandidates: false, altKey: true })).toBe('pass') + }) + it('accepts the top row on Tab, and lets Shift+Tab leave the composer', () => { expect(resolveComposerKey({ key: 'Tab', open: true, hasCandidates: true })).toBe('accept') expect(resolveComposerKey({ key: 'Tab', open: true, hasCandidates: true, shiftKey: true })).toBe('pass') diff --git a/src/frontend/tests/unit/workspaceKeymap.dom.spec.js b/src/frontend/tests/unit/workspaceKeymap.dom.spec.js new file mode 100644 index 000000000..98105772b --- /dev/null +++ b/src/frontend/tests/unit/workspaceKeymap.dom.spec.js @@ -0,0 +1,78 @@ +// @vitest-environment jsdom +/** + * ent#621 — `hasModalOpen`, against a real DOM. + * + * The probe is the whole modal-suppression mechanism, so it is tested on + * nodes rather than on a mocked `querySelector`: a selector that is subtly + * wrong (`[aria-modal]` instead of `[aria-modal="true"]`, or a `:not()` on the + * wrong element) passes a mock and fails in a browser. + * + * Two properties matter and pull in opposite directions. It must see every + * overlay that says it is modal — including ones that do not exist yet, which + * is why this is a probe and not a registry — and it must NOT see the one the + * pressed key belongs to: the rail's own sheet for the rail keys, the key list + * itself for ⌘/. That exemption is per action, which is what keeps `⌥↓` from + * switching agent behind an open sheet. + */ +import { describe, it, expect, afterEach } from 'vitest' +import { hasModalOpen } from '../../src/components/portal/portalKeymap' + +const add = (html) => { + const host = document.createElement('div') + host.innerHTML = html + document.body.appendChild(host) + return host +} + +afterEach(() => { document.body.innerHTML = '' }) + +describe('hasModalOpen', () => { + it('is false on a page with nothing over it', () => { + add('
a non-modal popover (the theme switch)
') + expect(hasModalOpen(document)).toBe(false) + }) + + it('sees any overlay that declares itself modal', () => { + add('
…
') + expect(hasModalOpen(document)).toBe(true) + }) + + it('reads the attribute\'s value, not just its presence', () => { + // A dialog that is mounted-but-closed writes `aria-modal="false"`. + add('
closed
') + expect(hasModalOpen(document)).toBe(false) + }) + + it('ignores the overlay the pressed key belongs to — per action', () => { + add('') + // The rail keys: the sheet is not "something over the rail". + expect(hasModalOpen(document, { ignore: '[data-ws-rail-sheet]' })).toBe(false) + // Every other key: it is. + expect(hasModalOpen(document)).toBe(true) + }) + + it('still sees a second overlay above the exempted one', () => { + add('' + + '
') + expect(hasModalOpen(document, { ignore: '[data-ws-rail-sheet]' })).toBe(true) + expect(hasModalOpen(document, { ignore: ['[data-ws-rail-sheet]', '[data-ws-key-list]'] })).toBe(false) + }) + + it('lets ⌘/ close its own list while every other key stays suppressed', () => { + add('
') + expect(hasModalOpen(document, { ignore: '[data-ws-key-list]' })).toBe(false) + expect(hasModalOpen(document)).toBe(true) + }) + + it('answers false for a root it cannot query', () => { + expect(hasModalOpen(null)).toBe(false) + expect(hasModalOpen(undefined)).toBe(false) + expect(hasModalOpen({})).toBe(false) + }) + + it('scopes to the root it is given', () => { + const host = add('
') + expect(hasModalOpen(host)).toBe(true) + expect(hasModalOpen(document.createElement('section'))).toBe(false) + }) +}) diff --git a/src/frontend/tests/unit/workspaceKeymap.spec.js b/src/frontend/tests/unit/workspaceKeymap.spec.js new file mode 100644 index 000000000..748997908 --- /dev/null +++ b/src/frontend/tests/unit/workspaceKeymap.spec.js @@ -0,0 +1,373 @@ +/** + * ent#621 — the Workspace key map: the declaration and its pure helpers. + * + * This spec imports `portalKeymap.js` FIRST (nothing from `portalUtils` has + * been evaluated yet), which is half the cycle check the two modules' headers + * promise; `portalChatTabsAndTitles.spec.js` imports them the other way round. + * + * What is worth pinning is what an obvious implementation gets wrong: + * + * * a chord is matched by the printed `key` OR by the physical `code`, and + * the two arms disagree about Shift on purpose (Decision 29) — without the + * `key` arm, DE's `Shift+7` → `/` is dead; without the `code` arm, `⌥.` on + * a US Mac is dead (it types `≥`); with Shift accepted on the `code` arm, + * macOS `⌘?` Help search is swallowed; + * * the map's own entries must not shadow each other — asserting "no + * collisions" is only worth anything if the checker can SEE one, so a + * planted clash is asserted too; + * * the reserved `⌘K` resolves to NOTHING (the shell must not + * `preventDefault` a key it does not handle) and never reaches the list; + * * `keymapSuppressed` is a matrix, not a boolean: a modal stops every shell + * key including ⌘J, a call stops only the keys that would move you. + */ +import { describe, it, expect } from 'vitest' +import { + WORKSPACE_KEYMAP, OWNER_SHELL, OWNER_PROTOCOL, SCOPE_MOVING, SCOPE_DOING, SCOPE_PROTOCOL, + entryChords, findBinding, matchesChord, resolveWorkspaceKey, keymapCollisions, + keymapSuppressed, cycleIndex, nextAgent, recordLastOpen, nextRailTab, + chordLabel, ariaKeyshortcuts, keyListRows, +} from '../../src/components/portal/portalKeymap' +import { isNewChatHotkey, newChatHotkeyLabel } from '../../src/components/portal/portalUtils' + +// A keydown as the browser delivers it. Plain object on purpose: the existing +// ⌘J truth table passes one, and `isNewChatHotkey` keeps that signature. +const ev = (o = {}) => ({ + key: '', code: '', metaKey: false, ctrlKey: false, shiftKey: false, altKey: false, + repeat: false, isComposing: false, keyCode: 0, ...o, +}) + +const chordOf = (action) => { + const entry = findBinding(action) + return entryChords(entry)[0] +} + +describe('the map as a declaration', () => { + it('declares one Workspace surface, with an owner and a scope on every entry', () => { + for (const entry of WORKSPACE_KEYMAP) { + expect(entry.surface, entry.id).toBe('workspace') + expect([OWNER_SHELL, OWNER_PROTOCOL], entry.id).toContain(entry.owner) + expect([SCOPE_MOVING, SCOPE_DOING, SCOPE_PROTOCOL], entry.id).toContain(entry.scope) + expect(entryChords(entry).length, entry.id).toBeGreaterThan(0) + expect(typeof entry.label, entry.id).toBe('string') + } + }) + + it('gives every shell chord a modifier — a bare key belongs to the field', () => { + for (const entry of WORKSPACE_KEYMAP.filter((e) => e.owner === OWNER_SHELL)) { + for (const c of entryChords(entry)) { + expect(c.primary || c.alt, entry.id).toBe(true) + } + } + }) + + it('declares the nine chords the issue names, plus the protocol keys', () => { + const shell = WORKSPACE_KEYMAP.filter((e) => e.owner === OWNER_SHELL) + expect(shell.map((e) => e.action)).toEqual([ + 'find-anything', 'new-chat', 'rail-toggle', 'key-list', + 'agent-next', 'agent-prev', 'chat-next', 'chat-prev', 'rail-tab-next', + ]) + expect(shell.filter((e) => e.reserved).map((e) => e.action)).toEqual(['find-anything']) + // The component-owned keys are declared so the collision check sees the + // whole surface; the handlers stay where the Esc learnings put them. + expect(WORKSPACE_KEYMAP.filter((e) => e.owner === OWNER_PROTOCOL).map((e) => e.action)) + .toEqual(['close-top', 'call-mute', 'preview-step', 'tab-roving', 'dialog-focus-cycle', 'typeahead-step']) + }) + + it('has no collisions — and the checker can see one when there is', () => { + expect(keymapCollisions(WORKSPACE_KEYMAP)).toEqual([]) + const planted = [...WORKSPACE_KEYMAP, { + id: 'spotlight-lookalike', action: 'x', surface: 'workspace', owner: OWNER_SHELL, + scope: SCOPE_DOING, label: 'x', chord: { key: 'j', code: 'KeyJ', primary: true }, + }] + expect(keymapCollisions(planted)).toEqual([['new-chat', 'spotlight-lookalike']]) + // A shell key that shadows a protocol key is a collision too. + const shadow = [...WORKSPACE_KEYMAP, { + id: 'esc-stealer', action: 'y', surface: 'workspace', owner: OWNER_SHELL, + scope: SCOPE_DOING, label: 'y', chord: { key: 'Escape' }, + }] + expect(keymapCollisions(shadow)).toEqual([['close-top', 'esc-stealer']]) + // Two PROTOCOL entries may share one: the preview's arrows and a tab + // strip's roving are the same keys in different places. + expect(keymapCollisions(WORKSPACE_KEYMAP.filter((e) => e.owner === OWNER_PROTOCOL))).toEqual([]) + }) +}) + +describe('matchesChord', () => { + const railToggle = chordOf('rail-toggle') + const keyList = chordOf('key-list') + const agentNext = chordOf('agent-next') + const chatNext = chordOf('chat-next') + const railTab = chordOf('rail-tab-next') + + it('takes the primary modifier as meta XOR ctrl, on every platform', () => { + expect(matchesChord(ev({ key: '.', metaKey: true }), railToggle)).toBe(true) + expect(matchesChord(ev({ key: '.', ctrlKey: true }), railToggle)).toBe(true) + expect(matchesChord(ev({ key: '.', metaKey: true, ctrlKey: true }), railToggle)).toBe(false) + expect(matchesChord(ev({ key: '.' }), railToggle)).toBe(false) + }) + + it('matches the physical position when the printed key is something else', () => { + // ⌥. on a US Mac types `≥`; the Period position is the only arm left. + expect(matchesChord(ev({ key: '≥', code: 'Period', altKey: true }), railTab)).toBe(true) + // A Cyrillic layout: `key` is non-Latin, the US position still works. + expect(matchesChord(ev({ key: 'о', code: 'KeyJ', metaKey: true }), chordOf('new-chat'))).toBe(true) + // No `code` at all (a hand-built event) simply loses that arm. + expect(matchesChord({ key: 'j', metaKey: true }, chordOf('new-chat'))).toBe(true) + expect(matchesChord({ code: 'KeyJ', metaKey: true }, chordOf('new-chat'))).toBe(true) + }) + + it('accepts Shift on the printed key but never on the physical one (Decision 29)', () => { + // DE: `/` is Shift+7. FR: `.` is a shifted key. Both must work. + expect(matchesChord(ev({ key: '/', code: 'Digit7', metaKey: true, shiftKey: true }), keyList)).toBe(true) + expect(matchesChord(ev({ key: '/', code: 'Slash', metaKey: true }), keyList)).toBe(true) + // macOS ⌘? (Help search) is Shift on the Slash POSITION with `key: '?'`: + // neither arm may claim it. + expect(matchesChord(ev({ key: '?', code: 'Slash', metaKey: true, shiftKey: true }), keyList)).toBe(false) + // A chord without `shiftOnKey` rejects Shift outright. + expect(matchesChord(ev({ key: '.', metaKey: true, shiftKey: true }), railToggle)).toBe(false) + }) + + it('is exact about Alt and Shift, which is what keeps the four arrows apart', () => { + expect(matchesChord(ev({ key: 'ArrowDown', altKey: true }), agentNext)).toBe(true) + expect(matchesChord(ev({ key: 'ArrowDown', altKey: true, shiftKey: true }), agentNext)).toBe(false) + expect(matchesChord(ev({ key: 'ArrowDown', altKey: true, shiftKey: true }), chatNext)).toBe(true) + expect(matchesChord(ev({ key: 'ArrowDown', altKey: true }), chatNext)).toBe(false) + expect(matchesChord(ev({ key: 'ArrowDown' }), agentNext)).toBe(false) + expect(matchesChord(ev({ key: 'ArrowDown', altKey: true, metaKey: true }), agentNext)).toBe(false) + }) + + it('answers false rather than throwing for junk', () => { + expect(matchesChord(null, agentNext)).toBe(false) + expect(matchesChord(ev({ key: 'ArrowDown', altKey: true }), null)).toBe(false) + expect(matchesChord({ altKey: true }, agentNext)).toBe(false) + }) +}) + +describe('resolveWorkspaceKey', () => { + it('names the action for each of the eight live chords', () => { + expect(resolveWorkspaceKey(ev({ key: 'j', metaKey: true }))).toBe('new-chat') + expect(resolveWorkspaceKey(ev({ key: 'j', ctrlKey: true }))).toBe('new-chat') + expect(resolveWorkspaceKey(ev({ key: '.', metaKey: true }))).toBe('rail-toggle') + expect(resolveWorkspaceKey(ev({ key: '/', metaKey: true }))).toBe('key-list') + expect(resolveWorkspaceKey(ev({ key: 'ArrowDown', altKey: true }))).toBe('agent-next') + expect(resolveWorkspaceKey(ev({ key: 'ArrowUp', altKey: true }))).toBe('agent-prev') + expect(resolveWorkspaceKey(ev({ key: 'ArrowDown', altKey: true, shiftKey: true }))).toBe('chat-next') + expect(resolveWorkspaceKey(ev({ key: 'ArrowUp', altKey: true, shiftKey: true }))).toBe('chat-prev') + expect(resolveWorkspaceKey(ev({ key: '.', altKey: true }))).toBe('rail-tab-next') + }) + + it('resolves the reserved and the protocol keys to nothing', () => { + // ⌘K is ent#577's. Resolving it would make the dispatcher `preventDefault` + // a key it cannot answer, taking the browser's own Ctrl+K with it. + expect(resolveWorkspaceKey(ev({ key: 'k', code: 'KeyK', metaKey: true }))).toBeNull() + // Esc is a capture-phase protocol owned by whoever is innermost. + expect(resolveWorkspaceKey(ev({ key: 'Escape' }))).toBeNull() + expect(resolveWorkspaceKey(ev({ key: 'm' }))).toBeNull() + expect(resolveWorkspaceKey(ev({ key: 'Tab' }))).toBeNull() + }) + + it('ignores a held key and an IME composition', () => { + expect(resolveWorkspaceKey(ev({ key: 'ArrowDown', altKey: true, repeat: true }))).toBeNull() + expect(resolveWorkspaceKey(ev({ key: 'ArrowDown', altKey: true, isComposing: true }))).toBeNull() + expect(resolveWorkspaceKey(ev({ key: 'ArrowDown', altKey: true, keyCode: 229 }))).toBeNull() + expect(resolveWorkspaceKey(null)).toBeNull() + }) + + it('says nothing about plain typing', () => { + for (const key of ['a', 'Enter', '.', '/', 'j', 'ArrowDown', 'ArrowUp']) { + expect(resolveWorkspaceKey(ev({ key })), key).toBeNull() + } + }) + + it('still answers for ⌘J exactly as isNewChatHotkey does', () => { + const cases = [ + ev({ key: 'j', metaKey: true }), ev({ key: 'J', ctrlKey: true }), + ev({ key: 'j' }), ev({ key: 'j', metaKey: true, shiftKey: true }), + ev({ key: 'j', ctrlKey: true, altKey: true }), ev({ key: 'k', metaKey: true }), + ] + for (const e of cases) { + expect(isNewChatHotkey(e), e.key).toBe(resolveWorkspaceKey(e) === 'new-chat') + } + }) +}) + +describe('keymapSuppressed', () => { + const sup = (o) => keymapSuppressed(o) + + it('lets a shell key through when nothing is in the way', () => { + for (const action of ['new-chat', 'rail-toggle', 'key-list', 'agent-next', 'chat-prev', 'rail-tab-next']) { + expect(sup({ action }), action).toBe(false) + } + }) + + it('yields to a nearer owner that already claimed the event', () => { + expect(sup({ action: 'agent-next', defaultPrevented: true })).toBe(true) + expect(sup({ action: 'new-chat', defaultPrevented: true })).toBe(true) + }) + + it('stops every shell key under something modal — ⌘J included', () => { + for (const action of ['new-chat', 'rail-toggle', 'key-list', 'agent-next', 'chat-next', 'rail-tab-next']) { + expect(sup({ action, modalOpen: true }), action).toBe(true) + expect(sup({ action, drawerOpen: true }), action).toBe(true) + } + }) + + it('stops only the moving keys and the rail during a call', () => { + for (const action of ['agent-next', 'agent-prev', 'chat-next', 'chat-prev', 'rail-tab-next', 'rail-toggle']) { + expect(sup({ action, callActive: true }), action).toBe(true) + } + // ⌘J keeps its own "leave the call?" ask; ⌘/ opens a dialog, which leaves + // nothing. + expect(sup({ action: 'new-chat', callActive: true })).toBe(false) + expect(sup({ action: 'key-list', callActive: true })).toBe(false) + }) + + it('fails closed for anything it does not dispatch', () => { + expect(sup({ action: 'find-anything' })).toBe(true) + expect(sup({ action: 'close-top' })).toBe(true) + expect(sup({ action: 'a-future-action' })).toBe(true) + expect(sup({})).toBe(true) + }) +}) + +describe('the walk helpers', () => { + it('wraps an index at both ends and survives an empty list', () => { + expect(cycleIndex(0, 1, 3)).toBe(1) + expect(cycleIndex(2, 1, 3)).toBe(0) + expect(cycleIndex(0, -1, 3)).toBe(2) + expect(cycleIndex(-1, 1, 3)).toBe(0) + expect(cycleIndex(-1, -1, 3)).toBe(2) + expect(cycleIndex(0, 1, 1)).toBe(0) + expect(cycleIndex(0, 1, 0)).toBe(-1) + }) + + it('steps through the roster the sidebar shows, and wraps', () => { + const order = [{ name: 'scout' }, { name: 'sage' }, { name: 'rabbit' }] + expect(nextAgent(order, 'scout', 1)).toBe('sage') + expect(nextAgent(order, 'rabbit', 1)).toBe('scout') + expect(nextAgent(order, 'scout', -1)).toBe('rabbit') + expect(nextAgent(['scout', 'sage'], 'sage', 1)).toBe('scout') + // An agent that is not in the list (a room, the Inbox, a stale name) starts + // the walk at an end rather than doing nothing. + expect(nextAgent(order, null, 1)).toBe('scout') + expect(nextAgent(order, 'gone', -1)).toBe('rabbit') + }) + + it('does nothing when there is nowhere to go', () => { + expect(nextAgent([{ name: 'solo' }], 'solo', 1)).toBeNull() + expect(nextAgent([], 'solo', 1)).toBeNull() + expect(nextAgent(null, 'solo', 1)).toBeNull() + expect(nextAgent([{ name: '' }, { id: 1 }], null, 1)).toBeNull() + }) + + it('cycles only the rail tabs this session may see', () => { + const tabs = [{ id: 'work' }, { id: 'files' }, { id: 'info' }] + expect(nextRailTab(tabs, 'work')).toBe('files') + expect(nextRailTab(tabs, 'info')).toBe('work') + expect(nextRailTab(tabs, 'info', -1)).toBe('files') + // A remembered tab this session cannot see starts at the first visible one. + expect(nextRailTab(tabs, 'canvas')).toBe('work') + expect(nextRailTab([{ id: 'work' }], 'work')).toBe('work') + expect(nextRailTab([], 'work')).toBeNull() + expect(nextRailTab(null, 'work')).toBeNull() + }) +}) + +describe('recordLastOpen', () => { + it('records the chat on stage for the agent on stage', () => { + const m = new Map() + expect(recordLastOpen(m, { agentName: 'scout', sessionId: 't1', keyChanged: true })).toBe(true) + expect(m.get('scout')).toBe('t1') + expect(recordLastOpen(m, { agentName: 'scout', sessionId: 't2' })).toBe(true) + expect(m.get('scout')).toBe('t2') + }) + + it('ignores a cleared load instruction under the agent already on stage', () => { + // Clicking an agent row nulls `pendingSession` BEFORE the route changes: + // recording that would forget the chat the person was just reading. + const m = new Map([['scout', 't2']]) + expect(recordLastOpen(m, { agentName: 'scout', sessionId: null })).toBe(false) + expect(m.get('scout')).toBe('t2') + }) + + it('records a null for a NEW agent — an unsent new chat is a landing', () => { + // `agentLanding`'s arms 3–5 then return to the draft, the agent's existing + // empty chat, or a fresh one. No sentinel, no second rule. + const m = new Map([['scout', 't2']]) + expect(recordLastOpen(m, { agentName: 'sage', sessionId: null, keyChanged: true })).toBe(true) + expect(m.get('sage')).toBeNull() + expect(m.get('scout')).toBe('t2') + }) + + it('writes nothing without a map or an agent', () => { + expect(recordLastOpen(null, { agentName: 'scout', sessionId: 't1' })).toBe(false) + expect(recordLastOpen(new Map(), { sessionId: 't1', keyChanged: true })).toBe(false) + expect(recordLastOpen(new Map(), {})).toBe(false) + }) +}) + +describe('what people and screen readers read', () => { + it('labels a chord for the platform', () => { + expect(chordLabel(chordOf('new-chat'), 'MacIntel')).toBe('⌘J') + expect(chordLabel(chordOf('new-chat'), 'Win32')).toBe('Ctrl+J') + expect(chordLabel(chordOf('rail-toggle'), 'MacIntel')).toBe('⌘.') + expect(chordLabel(chordOf('key-list'), 'Linux x86_64')).toBe('Ctrl+/') + expect(chordLabel(chordOf('agent-next'), 'MacIntel')).toBe('⌥↓') + expect(chordLabel(chordOf('agent-prev'), 'Win32')).toBe('Alt+↑') + expect(chordLabel(chordOf('chat-next'), 'MacIntel')).toBe('⌥⇧↓') + expect(chordLabel(chordOf('chat-prev'), 'Win32')).toBe('Alt+Shift+↑') + expect(chordLabel(chordOf('rail-tab-next'), 'MacIntel')).toBe('⌥.') + expect(chordLabel(chordOf('close-top'), 'MacIntel')).toBe('Esc') + expect(chordLabel(null, 'MacIntel')).toBe('') + }) + + it('keeps the shipped ⌘J label, which now comes from the map', () => { + expect(newChatHotkeyLabel('MacIntel')).toBe('⌘J') + expect(newChatHotkeyLabel('Linux x86_64')).toBe('Ctrl+J') + expect(newChatHotkeyLabel(undefined)).toBe('Ctrl+J') + }) + + it('spells aria-keyshortcuts ARIA\'s way: the character for printables', () => { + expect(ariaKeyshortcuts(chordOf('agent-next'), 'MacIntel')).toBe('Alt+ArrowDown') + expect(ariaKeyshortcuts(chordOf('chat-prev'), 'Win32')).toBe('Alt+Shift+ArrowUp') + expect(ariaKeyshortcuts(chordOf('rail-tab-next'), 'Win32')).toBe('Alt+.') + expect(ariaKeyshortcuts(chordOf('rail-toggle'), 'MacIntel')).toBe('Meta+.') + expect(ariaKeyshortcuts(chordOf('rail-toggle'), 'Win32')).toBe('Control+.') + expect(ariaKeyshortcuts(chordOf('key-list'), 'MacIntel')).toBe('Meta+/') + expect(ariaKeyshortcuts(chordOf('new-chat'), 'MacIntel')).toBe('Meta+J') + // Alternatives are space-separated, which is the attribute's own grammar. + expect(ariaKeyshortcuts(entryChords(findBinding('tab-roving')), 'Win32')) + .toBe('ArrowLeft ArrowRight Home End') + }) +}) + +describe('keyListRows', () => { + const rows = keyListRows(WORKSPACE_KEYMAP, 'MacIntel') + + it('renders every live binding from the map, and the reserved one from none', () => { + expect(rows.map((r) => r.action)).not.toContain('find-anything') + expect(rows).toHaveLength(WORKSPACE_KEYMAP.length - 1) + expect(rows.find((r) => r.action === 'agent-next')).toMatchObject({ + label: 'Next agent', keys: '⌥↓', ariaKeyshortcuts: 'Alt+ArrowDown', scope: SCOPE_MOVING, + }) + expect(rows.find((r) => r.action === 'tab-roving').keys).toBe('← / → / Home / End') + }) + + it('says a key is one-per-press rather than letting someone hold it', () => { + expect(rows.find((r) => r.action === 'agent-next').note).toMatch(/one agent per press/i) + }) + + it('tells the truth on a page with no rail instead of hiding the rows', () => { + const noRail = keyListRows(WORKSPACE_KEYMAP, 'Win32', { railAvailable: false }) + for (const action of ['rail-toggle', 'rail-tab-next']) { + expect(noRail.find((r) => r.action === action).note, action).toMatch(/no rail/i) + } + expect(noRail.find((r) => r.action === 'agent-next').note || '').not.toMatch(/no rail/i) + expect(rows.find((r) => r.action === 'rail-toggle').note || '').not.toMatch(/no rail/i) + }) + + it('names where a protocol key applies, so Esc does not read as global', () => { + expect(rows.find((r) => r.action === 'close-top').note).toMatch(/innermost/i) + expect(rows.find((r) => r.action === 'call-mute').note).toMatch(/voice call/i) + }) +}) From 88be733e93724c41e9975fb63a5ede86c900dc95 Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Mon, 5 Oct 2026 09:25:23 -0400 Subject: [PATCH 10/22] feat(portal): the agent and chat keys, and the last-open memory (abilityai/trinity-enterprise#621) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Checkpoint B of the Workspace key map. The map and its pure rules landed in 58350ed7; this wires them to the shell. * `Portal.vue`'s `onGlobalKeydown` becomes the ONE dispatcher over `resolveWorkspaceKey` + `keymapSuppressed`, with the full ladder: an unclaimed chord returns without `preventDefault` (⌘K keeps reaching the browser until ent#577), ⌘J keeps its place at the top and its own "leave the call?" ask, a nearer owner's `defaultPrevented` wins, a focused `` bail → `keymapSuppressed` for modal / drawer / call / `defaultPrevented`) before it ever calls `preventDefault`. Two consequences for anyone adding a key: matching is **platform-free** (`primary` is meta XOR ctrl, and a chord matches by `key` OR physical `code`) with the platform confined to the labels, and `keymapCollisions()` must stay empty for mac AND win — a second listener added beside this one is the defect the map exists to prevent. The sidebar's roster order also lives in the shell (`orderedRoster`), handed to both sidebar instances, because the agent keys have to walk the order the eye reads and cannot ask a child what it decided. See [workspace-chat-tabs-and-titles.md](../feature-flows/workspace-chat-tabs-and-titles.md) § The key map. + **The Workspace Inbox (ent#610).** Route `/workspace/inbox` (`WorkspaceInbox`) is a stage of the same `views/Portal.vue` shell, not a new view. `components/portal/PortalInbox.vue` (container: header, `OverflowTabs` Action · Unread · All, list + pane, phone collapse), diff --git a/docs/memory/feature-flows/workspace-agents-at-the-centre.md b/docs/memory/feature-flows/workspace-agents-at-the-centre.md index 8458977ba..0dae3e55a 100644 --- a/docs/memory/feature-flows/workspace-agents-at-the-centre.md +++ b/docs/memory/feature-flows/workspace-agents-at-the-centre.md @@ -206,7 +206,7 @@ rule is a plain function (the ent#392 precedent): | `agentLanding` | a **precedence** (ent#784, replacing ent#523's `landingThread`; operator ruling 2026-10-05): a validated `lastOpenSessionId` (live, unarchived, this agent's), then the chat holding an unsent **draft** (newest `updatedAt`; a `new:` winner = a new chat), then the agent's existing **empty** chat, then a new, empty chat | | `resolveAgentLanding` | the `?agent=` deep link, delegating to `agentLanding` so there is one answer | | `agentEmptyChat` | arm 4's "empty": unarchived, non-room, no `last_message_at` and `message_count` 0/absent; Main first, then newest `created_at` | -| `orderRosterAgents` | most recent collaboration, then name; `primaryName` is ent#491's seam | +| `orderRosterAgents` | most recent collaboration, then name; `primaryName` is ent#491's seam. Called **once, in the shell** since ent#621 (`orderedRoster`, handed to both sidebar instances) — the switch-agent keys must walk the order the eye reads, and the sidebar no longer sorts what it is given | | `agentPreview` | the newest chat's **title** — the sidebar list carries no message content (#2198), so a body preview would reinstate the N+1 | | `composerAvailabilityNotice` | what the composer says for a stopped/unavailable agent | @@ -245,13 +245,19 @@ rule is a plain function (the ent#392 precedent): would add a second empty row and then land on one of the two. #2579's "the pinned tab has to be there" still holds for every agent whose chats are all used, which is what it was about. -- **Two doors RESOLVE, the rest ASSERT.** Only `landOnAgent` (the sidebar/rail - row, back/forward, a typed URL) and `resolveAgentLanding` (`?agent=`) ask - `agentLanding` where to land. New chat, ⌘J, the agent picker and switch-agent - do not call it at all — they already mean "fresh". That split is also the - focus rule: a gesture may focus the composer on any pointer (#2579 AC 2), a - landing only on a fine one (ent#784), because a landing nobody asked for in - those words must not slide a keyboard up over a phone. +- **The doors that RESOLVE, and the doors that ASSERT.** `landOnAgent` (the + sidebar/rail row, back/forward, a typed URL, and the switch-agent keys) and + `resolveAgentLanding` (`?agent=`) ask `agentLanding` where to land. New chat, + ⌘J and the agent picker do not call it at all — they already mean "fresh". + The switch-agent keys are on the resolving side on purpose: walking the + roster is navigation, not a request for a blank page, so each key press + passes the `lastOpenSessionId` the shell remembers for that agent and arm 2 + returns the person to the chat they were last in (see + `workspace-chat-tabs-and-titles.md` → Key map). That split is also the focus + rule: a gesture may focus the composer on any pointer (#2579 AC 2), a landing + only on a fine one (ent#784) — except a key landing, which focuses the + composer unconditionally, because the hands are already on the keyboard and + nothing about a chord can slide a keyboard up over a phone. - **An unused Main is filtered from the sidebar only.** It exists for every pair the moment an agent is opened; the tab strip must still show it from the first visit, so this is a projection for one consumer (`sidebarThreads`), not a diff --git a/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md b/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md index df3787700..db6e66c49 100644 --- a/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md +++ b/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md @@ -1,7 +1,7 @@ -# Feature: Workspace chats as tabs, New chat hotkey, and renameable titles +# Feature: Workspace chats as tabs, the key map, and renameable titles -> **Status**: ✅ Implemented (2026-09-06; the four tab-strip defects and the pre-turn title spawn, #2579, 2026-09-07) -> **Issues**: abilityai/trinity-enterprise#451 (remaining slice — #2430 shipped the fresh-thread half), abilityai/trinity-enterprise#473, [#2579](https://github.com/abilityai/trinity/issues/2579) +> **Status**: ✅ Implemented (2026-09-06; the four tab-strip defects and the pre-turn title spawn, #2579, 2026-09-07; the key map, ent#621, 2026-10-05) +> **Issues**: abilityai/trinity-enterprise#451 (remaining slice — #2430 shipped the fresh-thread half), abilityai/trinity-enterprise#473, [#2579](https://github.com/abilityai/trinity/issues/2579), abilityai/trinity-enterprise#621 (the key map) > **Requirement**: `docs/memory/requirements/core-agent.md` §5.21 > **Related**: [workspace-absorbs-session.md](workspace-absorbs-session.md) (the ent#451 `new_thread` half), [workspace-sidebar-ia.md](workspace-sidebar-ia.md) (the row this extends), [workspace-agent-page.md](workspace-agent-page.md) (the full chat list), [workspace-agents-at-the-centre.md](workspace-agents-at-the-centre.md) (the pinned Main chat #2579 ensures is listed) @@ -58,14 +58,100 @@ PortalConversation.vue (`emit('new-chat')` → `Portal.vue::newChatWithAgent(activeAgent.name)`); the sidebar's button remains the cross-agent picker. The label shows at `md`, the `` at `lg`; `title` and `aria-keyshortcuts` carry the chord always. -- `isNewChatHotkey(e)`: `j` with a **plain** ⌘ or Ctrl — never Shift/Alt, - never both modifiers (those are someone else's shortcuts). ⌘N is the - browser's; ⌘⇧O was declined. -- Armed on `window` in `Portal.vue`'s `onMounted` **above** `bootstrap()`'s - `await` (contract #23: handlers are armed at mount, never behind fetched - data), removed in `onBeforeUnmount`, inert until signed in. It resolves the - agent in front of the person — the agent page's, or the open conversation's - — and opens the picker in a room or on the roster root. +- The chord is `j` with a **plain** ⌘ or Ctrl — never Shift/Alt, never both + modifiers (those are someone else's shortcuts). ⌘N is the browser's; + ⌘⇧O was declined. `isNewChatHotkey(e)` survives as a thin delegate to the + key map below (`resolveWorkspaceKey(e) === 'new-chat'`), so the truth table + that shipped with ent#451 still describes it exactly. +- It resolves the agent in front of the person — the agent page's, or the open + conversation's — and opens the picker in a room or on the roster root. + +## The key map (ent#621) + +⌘J was the Workspace's only key, matched inline. ent#621 made it one entry in +a **declared map** with a **single dispatcher**, because the second and third +chord is where a keyboard surface usually goes wrong: two handlers claiming one +chord, a key that works everywhere except the message field, a chord that eats +a modal's Esc. + +**One declaration — `components/portal/portalKeymap.js`** (pure, no DOM, no +store). `WORKSPACE_KEYMAP` holds every key the Workspace answers *and* the keys +it deliberately does not dispatch, so collisions are visible across the whole +surface rather than only within one owner's share: + +| Keys | Does | Owner | +|---|---|---| +| ⌘J / Ctrl+J | New chat with this agent | shell | +| ⌥↓ / ⌥↑ (Alt+↓ / Alt+↑) | Next / previous agent | shell | +| ⌥⇧↓ / ⌥⇧↑ (Alt+Shift+↓ / ↑) | Next / previous chat with this agent | shell | +| ⌘. / Ctrl+. | Show or hide the rail | shell | +| ⌥. (Alt+.) | Next rail tab | shell | +| ⌘/ / Ctrl+/ | The keyboard-shortcuts list | shell | +| Esc, the call's `M`, the preview's ←/→, tab roving, a dialog's Tab cycle, the composer typeahead's bare arrows | — | the component that owns the overlay | +| ⌘K / Ctrl+K | nothing — **reserved** for search (ent#577) | nobody | + +Reserved means exactly that: no handler, no `preventDefault`, and no row in the +key list. The browser's own ⌘K keeps working until search ships, and a help +dialog never advertises a key that does nothing. + +**Matching is platform-free; only the LABELS are not.** `primary` is meta XOR +ctrl on every platform (the shipped ⌘J semantics), and a chord matches by +`key` **or** by physical `code`. The `code` arm is what makes the map work on a +non-Latin layout — and on a US Mac it is the *only* arm for `⌥.`, which types +`≥`. Shift is accepted on a `key` match only, so DE's `Shift+7` → `/` and +FR's shifted `.` reach ⌘/ while macOS's `⌘?` stays the browser's Help search. +`chordLabel` / `ariaKeyshortcuts` take the platform and render `⌥↓` vs +`Alt+↓` and `Alt+ArrowDown`; `keymapCollisions(map, platform)` is asserted +empty for mac **and** win, which is the test that would have caught a new chord +shadowing a protocol key. + +**One dispatcher — `Portal.vue::onGlobalKeydown`**, still the single `window` +listener armed above `bootstrap()`'s `await` (contract #23) and removed in +`onBeforeUnmount`. The ladder, in order, because the order is the design: + +1. `resolveWorkspaceKey(e)` — a chord nobody owns, a **key repeat**, or an IME + composition returns null and we return **without** `preventDefault`; +2. signed in (the OTP form answers no keys); +3. ⌘J keeps its place at the top and its own "leave the call?" ask — the one + key a live voice call answers rather than swallows; +4. an action with no dispatch entry (a protocol key, a reserved chord) is left + to the browser rather than silently eaten; +5. a focused `` and `defaultPrevented` bails, modal / drawer / + call suppression, and the composer focused after every moving key); + `portalChatCycle.mount.spec.js` (the conversation's chat walk emits exactly + what a tab click does); `portalKeyList.mount.spec.js` (rows, Esc, focus + return); `portalKeyHints.mount.spec.js` (the hints on the rows, the rail and + the tabs). `portalRosterRow.spec.js` mounts the sidebar for the bound and the + handed-down order — the lift's own regression. - `src/frontend/e2e/workspace-chat-tabs.spec.js` (**#2579**) — the geometry no node-env pin can execute: every visible tab exactly 160px (Main included), a long title clipped with its full text on `title=`, the counted "N more" diff --git a/docs/memory/requirements/core-agent.md b/docs/memory/requirements/core-agent.md index 1531d5a2a..36a226742 100644 --- a/docs/memory/requirements/core-agent.md +++ b/docs/memory/requirements/core-agent.md @@ -1813,6 +1813,85 @@ bounding the table. OSS-core (Workspace rule above). Flow: source pin can execute it). - **Flow**: `docs/memory/feature-flows/workspace-chat-tabs-and-titles.md` +### 5.21a Workspace key map (trinity-enterprise#621) + +- **Status**: ✅ Implemented (2026-10-05) · **ID**: `WORKSPACE_KEY_MAP` +- **Description**: §5.21's ⌘J was the Workspace's only key and was matched + inline. This makes the keyboard a **declared map with one dispatcher**: nine + working chords for moving between agents and chats and for the rail, a + reserved chord for search, and the component-owned protocol keys (Esc and + friends) declared alongside them so collisions are visible across the whole + surface rather than within one owner's share. +- **AC-1 — the keys**: `⌘J`/`Ctrl+J` new chat with this agent (§5.21, + unchanged); `⌥↓`/`⌥↑` next / previous **agent**; `⌥⇧↓`/`⌥⇧↑` next / + previous **chat with this agent**; `⌘.`/`Ctrl+.` show or hide the rail; + `⌥.` next rail tab; `⌘/`/`Ctrl+/` the keyboard-shortcuts list. +- **AC-2 — one declaration**: every chord lives in + `src/frontend/src/components/portal/portalKeymap.js` (pure — no DOM, no + store), with its scope, its owner, and its label. `keymapCollisions(map, + platform)` must be **empty on mac and on Windows/Linux**; a key that exists + without a map entry, or a shell chord that shadows a protocol key, is the + defect this prevents. +- **AC-3 — one dispatcher**: `Portal.vue::onGlobalKeydown` is the Workspace's + single `window` keydown listener, armed at mount above the bootstrap `await` + and removed on unmount. Its ladder: resolve → signed in → ⌘J's + leave-the-call ask → undispatched actions pass through → a focused + `` owns Alt+↓ natively on Firefox/Windows — the one // editable target the agent keys step aside for (the composer is a // textarea and must keep working). From 10bfd65bcd8d1813c46bfbb0c00eed23051f42ec Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Mon, 5 Oct 2026 11:16:25 -0400 Subject: [PATCH 15/22] fix(portal): remember a same-agent new chat for the agent keys (ent#621 review H1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last-open writer tested only "did the agent change", so a null `pendingSession` under an unchanged agent was always read as a load instruction. But ⌘J and the strip's New chat tab (both `newChatWithAgent`) null it on the agent already on stage — the commonest way to be sitting on an unsent chat — so ⌥↓ then ⌥↑ came back to the chat ⌘J had just left instead of the new one, against AC-8. The watcher now also observes `startingNewChat` and passes `keyChanged || (fresh && !sid)`: a deliberate fresh chat records `null` for the current agent, while every load-instruction clear (`openAgentPage`, `openRoom`, the unreachable arm, `openThread`) leaves the flag false and is still ignored, so the chat the person was reading survives those. Two mount cases: ⌘J on the agent on stage then ⌥↓/⌥↑ returns the new chat, and a load-instruction clear under an unchanged agent still returns the old one. Reverting only the watcher turns the first red and leaves the second green. Co-Authored-By: Claude Opus 5 --- src/frontend/src/views/Portal.vue | 17 +++++++- .../tests/unit/workspaceKeymap.mount.spec.js | 42 +++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/src/frontend/src/views/Portal.vue b/src/frontend/src/views/Portal.vue index 0df5afbd1..d33e3b6be 100644 --- a/src/frontend/src/views/Portal.vue +++ b/src/frontend/src/views/Portal.vue @@ -2346,13 +2346,26 @@ const lastOpen = new Map() // chat, or a fresh one. The rule is `recordLastOpen` (pure, ent#621 Decision 35); // this reads REFS rather than route computeds, because every writer here fires // before its own push has resolved. +// +// Review H1: "the agent changed" is not the only real null. ⌘J and the strip's +// New chat tab (both `newChatWithAgent`) null `pendingSession` on the agent +// ALREADY on stage, which is the commonest way to be sitting on an unsent chat +// — and under the agent-only test that read as an instruction, so ⌥↑ came back +// to the chat ⌘J had just left. `startingNewChat` is what separates the two: a +// deliberate fresh chat sets it, while every load-instruction clear +// (`openAgentPage`, `openRoom`, the unreachable arm, `openThread`) clears it, +// so the ignore rule still holds for them. let lastOpenAgent = null -watch([activeAgentName, pendingSession], ([agent, sid]) => { +watch([activeAgentName, pendingSession, startingNewChat], ([agent, sid, fresh]) => { const onStage = !!agent && !activeRoomId.value && !unreachableAgent.value const keyChanged = agent !== lastOpenAgent lastOpenAgent = agent if (!onStage) return - recordLastOpen(lastOpen, { agentName: agent, sessionId: sid, keyChanged }) + recordLastOpen(lastOpen, { + agentName: agent, + sessionId: sid, + keyChanged: keyChanged || (fresh && !sid), + }) }) // What assistive tech hears after a key move. The shell says it once; the band, diff --git a/src/frontend/tests/unit/workspaceKeymap.mount.spec.js b/src/frontend/tests/unit/workspaceKeymap.mount.spec.js index f92102c1a..5a637a134 100644 --- a/src/frontend/tests/unit/workspaceKeymap.mount.spec.js +++ b/src/frontend/tests/unit/workspaceKeymap.mount.spec.js @@ -289,6 +289,48 @@ describe('ent#621 — the last chat open with each agent', () => { expect(showing(w).session).toBe(null) }) + it('a new chat started on the agent already on stage is what ⌥↑ comes back to', async () => { + // Review H1. ⌘J on the agent you are already in nulls `pendingSession` + // under an UNCHANGED agent — the same shape as a load-instruction clear — + // so the writer needs `startingNewChat` to tell the two apart. Without it + // the memory still names the chat ⌘J just left, and ⌥↑ reopens it. + const { w } = await boot('/workspace/c/s-old') + expect(showing(w).session).toBe('s-old') + + await press(w, { key: 'j', meta: true }) // ⌘J — a fresh chat with scout + expect(showing(w).agent).toBe('scout') + expect(showing(w).newChat).toBe(true) + expect(showing(w).session).toBe(null) + + await press(w, { key: 'ArrowDown', alt: true }) // → sage + expect(showing(w).agent).toBe('sage') + await press(w, { key: 'ArrowUp', alt: true }) // ← scout + expect(showing(w).agent).toBe('scout') + expect(showing(w).newChat).toBe(true) // not 's-old' again + expect(showing(w).session).toBe(null) + }) + + it('a load-instruction clear under an unchanged agent is still ignored', async () => { + // The other half of H1: `startingNewChat` false means the null is an + // instruction, not an intent (`openAgentPage`, `openRoom` and the + // unreachable arm all clear it), and the chat the person was reading a + // moment ago must survive it. Written as the clear itself because every + // live door either leaves the stage or pushes a route that then lands a + // new chat — what is pinned here is the writer's rule. + const { w } = await boot('/workspace/c/s-old') + expect(showing(w).session).toBe('s-old') + + w.vm.pendingSession = null // agent unchanged, no new-chat intent + await w.vm.$nextTick() + expect(w.vm.startingNewChat).toBe(false) + + await press(w, { key: 'ArrowDown', alt: true }) // → sage + expect(showing(w).agent).toBe('sage') + await press(w, { key: 'ArrowUp', alt: true }) // ← scout + expect(showing(w).agent).toBe('scout') + expect(showing(w).session).toBe('s-old') + }) + it('a stale id is not honoured — the landing validates it', async () => { // The memory is in-session, so the only way to hold an id the list no longer // has is for the list to change under it. The rule lives in `agentLanding` From 61cdcf6eea8f64b6a1799f268eadb8d7b466051a Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Mon, 5 Oct 2026 11:18:11 -0400 Subject: [PATCH 16/22] docs(memory): key map ladder exception + platform-free collisions wording (ent#621 review I1-I3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I1 — the ladder sentence read as though nothing claims the event before the suppression rung, but ⌘J during an active voice call calls preventDefault first and routes to the leave-call guard. That is the pre-existing ent#534/551 ask, kept deliberately, so the exception is now stated in architecture/frontend.md and in AC-3 rather than the code being moved. I2 — the docs named a `platform` argument `keymapCollisions(map)` does not take. Matching is platform-free, so one call is the whole proof; the three mentions say that instead of implying a per-OS check. I3 — "one window listener" becomes "one chord dispatcher" in frontend.md (and AC-3), naming PortalRail.vue's pre-existing sheet-only Esc listener as the `close-top` protocol entry it is. Co-Authored-By: Claude Opus 5 --- docs/memory/architecture/frontend.md | 2 +- .../workspace-chat-tabs-and-titles.md | 11 +++++---- docs/memory/requirements/core-agent.md | 23 ++++++++++++------- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/docs/memory/architecture/frontend.md b/docs/memory/architecture/frontend.md index 2c63875c0..d55feda21 100644 --- a/docs/memory/architecture/frontend.md +++ b/docs/memory/architecture/frontend.md @@ -59,7 +59,7 @@ **Agent-to-agent collaboration data** (`stores/network.js`): the Vue Flow node-graph rendering of collaboration (the Dashboard's Graph mode + `AgentNode.vue`) was **decommissioned in #1689**; the underlying collaboration data still flows and feeds the Timeline replay. Detection: the backend chat endpoint accepts `X-Source-Agent` and broadcasts `agent_collaboration` WS events; `activity_service` broadcasts `agent_activity` (`activity_type`: chat_start/chat_end/tool_call/schedule_start/schedule_end/agent_collaboration; `activity_state`: started/completed/failed/cancelled — a user-cancelled terminal is recorded as `cancelled`, distinct from `failed`, #1332). -**The Workspace key map (ent#621).** The Workspace keyboard is ONE declaration and ONE listener: `components/portal/portalKeymap.js` holds every chord — including the reserved `⌘K` and the component-owned protocol keys (Esc and friends) it never dispatches, because a map that lists only its own keys cannot tell you a new chord shadows one of theirs — and `views/Portal.vue::onGlobalKeydown` is the single `window` keydown handler, armed at mount above the bootstrap `await` and gated by a fixed ladder (resolve → signed in → undispatched actions pass through → native-`` bail → `keymapSuppressed` for modal / drawer / call / `defaultPrevented`) before it ever calls `preventDefault`. "One dispatcher" is the claim, not "one `window` listener": `PortalRail.vue` has carried a sheet-only `window` keydown of its own since before this map (Esc → `emit('close')`), which is the `close-top` protocol entry the map declares and never dispatches — it resolves no chord and the dispatcher is still the only thing that acts on one. The ladder has ONE exception, stated rather than moved: `⌘J` during an active voice call calls `preventDefault` before the suppression ladder and routes to the leave-call guard — the pre-existing ent#534/551 ask, kept by ruling T8, so a modal open during a call sees a harmless re-arm rather than a swallowed key. Two consequences for anyone adding a key: matching is **platform-free** (`primary` is meta XOR ctrl, and a chord matches by `key` OR physical `code`) with the platform confined to the labels — so `keymapCollisions(map)` takes no platform argument and ONE call is the whole proof it stays empty, rather than one per OS — and a second chord dispatcher added beside this one is the defect the map exists to prevent. The sidebar's roster order also lives in the shell (`orderedRoster`), handed to both sidebar instances, because the agent keys have to walk the order the eye reads and cannot ask a child what it decided. See [workspace-chat-tabs-and-titles.md](../feature-flows/workspace-chat-tabs-and-titles.md) § The key map. **The Workspace Inbox (ent#610).** Route `/workspace/inbox` (`WorkspaceInbox`) is a stage of the same `views/Portal.vue` shell, not a new view. `components/portal/PortalInbox.vue` diff --git a/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md b/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md index db6e66c49..befe957bb 100644 --- a/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md +++ b/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md @@ -101,9 +101,10 @@ non-Latin layout — and on a US Mac it is the *only* arm for `⌥.`, which type `≥`. Shift is accepted on a `key` match only, so DE's `Shift+7` → `/` and FR's shifted `.` reach ⌘/ while macOS's `⌘?` stays the browser's Help search. `chordLabel` / `ariaKeyshortcuts` take the platform and render `⌥↓` vs -`Alt+↓` and `Alt+ArrowDown`; `keymapCollisions(map, platform)` is asserted -empty for mac **and** win, which is the test that would have caught a new chord -shadowing a protocol key. +`Alt+↓` and `Alt+ArrowDown`. `keymapCollisions(map)` does NOT take a platform +— matching is platform-free, so one call is the whole proof — and it is +asserted empty, which is the test that would have caught a new chord shadowing +a protocol key. **One dispatcher — `Portal.vue::onGlobalKeydown`**, still the single `window` listener armed above `bootstrap()`'s `await` (contract #23) and removed in @@ -543,8 +544,8 @@ never-opened chat count at all. capped/deduped ensure and its sign-out clear, and the settle cycle's bails and three clear sites. - **ent#621 — the key map.** `workspaceKeymap.spec.js` (pure: the chord table, - the `e.repeat` / IME bails, `keymapCollisions` empty on mac **and** win, the - label and `aria-keyshortcuts` spellings, `cycleIndex` / `nextRailTab` + the `e.repeat` / IME bails, `keymapCollisions` empty in its one + platform-free call, the label and `aria-keyshortcuts` spellings, `cycleIndex` / `nextRailTab` wrapping, `keyListRows` minus the reserved row, and ⌘J answering exactly as `isNewChatHotkey` does — the equivalence that makes the delegation safe); `workspaceKeymap.dom.spec.js` (the `[aria-modal]` probe and its per-action diff --git a/docs/memory/requirements/core-agent.md b/docs/memory/requirements/core-agent.md index 36a226742..a10449223 100644 --- a/docs/memory/requirements/core-agent.md +++ b/docs/memory/requirements/core-agent.md @@ -1828,15 +1828,22 @@ bounding the table. OSS-core (Workspace rule above). Flow: `⌥.` next rail tab; `⌘/`/`Ctrl+/` the keyboard-shortcuts list. - **AC-2 — one declaration**: every chord lives in `src/frontend/src/components/portal/portalKeymap.js` (pure — no DOM, no - store), with its scope, its owner, and its label. `keymapCollisions(map, - platform)` must be **empty on mac and on Windows/Linux**; a key that exists - without a map entry, or a shell chord that shadows a protocol key, is the - defect this prevents. + store), with its scope, its owner, and its label. `keymapCollisions(map)` + must be **empty**: matching is platform-free, so the checker takes no + platform argument and one call is the whole proof — not one per OS. A key + that exists without a map entry, or a shell chord that shadows a protocol + key, is the defect this prevents. - **AC-3 — one dispatcher**: `Portal.vue::onGlobalKeydown` is the Workspace's - single `window` keydown listener, armed at mount above the bootstrap `await` - and removed on unmount. Its ladder: resolve → signed in → ⌘J's - leave-the-call ask → undispatched actions pass through → a focused - `` keeps its native + `Alt+↓` → suppression → `preventDefault` → act. The ⌘J step is the ONE + exception to "the ladder runs before any `preventDefault`": during an + active voice call it claims the event and routes to the leave-call guard + above the suppression rung, which is the pre-existing ent#534/551 ask kept + deliberately (ruling T8) rather than an oversight. - **AC-4 — every key works from the message field**: the conventional "ignore an editable target" guard is deliberately **inverted**; the composer is a textarea and the keys must work while typing. A nearer owner still wins From 75c430acce198e49e16d41f5f511de43e1ef037c Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Mon, 5 Oct 2026 11:30:58 -0400 Subject: [PATCH 17/22] docs(memory): chat-tabs flow says one chord dispatcher, not one window listener (ent#621 review I3) Co-Authored-By: Claude Opus 5 --- docs/memory/feature-flows/workspace-chat-tabs-and-titles.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md b/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md index befe957bb..6c0bbbbc2 100644 --- a/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md +++ b/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md @@ -106,8 +106,9 @@ FR's shifted `.` reach ⌘/ while macOS's `⌘?` stays the browser's Help search asserted empty, which is the test that would have caught a new chord shadowing a protocol key. -**One dispatcher — `Portal.vue::onGlobalKeydown`**, still the single `window` -listener armed above `bootstrap()`'s `await` (contract #23) and removed in +**One dispatcher — `Portal.vue::onGlobalKeydown`**, still the single chord +dispatcher (`PortalRail.vue`'s sheet-only Esc `window` listener is the `close-top` +protocol entry the map declares, not a second dispatcher), armed above `bootstrap()`'s `await` (contract #23) and removed in `onBeforeUnmount`. The ladder, in order, because the order is the design: 1. `resolveWorkspaceKey(e)` — a chord nobody owns, a **key repeat**, or an IME From 7136e8cf09f859a2b5f8b9b0b1d5fab7cc31eff3 Mon Sep 17 00:00:00 2001 From: trinity-ability <309458136+trinity-ability@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:25:55 +0100 Subject: [PATCH 18/22] fix(portal): a held Workspace key no longer leaks its repeats to the browser (ent#621 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A key repeat resolved to nothing and the dispatcher returned before `preventDefault`, so the auto-repeat of a chord the shell had just claimed went to the browser. Reproduced against the rebased branch: a held ⌥. types `≥` into the message field on a Mac, and a held Ctrl+J opens Downloads on Chrome/Firefox for Windows and Linux (⌘J's repeats were swallowed before the map existed). The dispatcher now remembers the action of the press it claimed (`heldKey`) and swallows repeats of that chord, matched through the new pure `workspaceChord`. A repeat still never dispatches, the repeat of a press the shell did not claim stays the browser's, and any new press ends the hold. Also re-pins the "no rail" case after the ent#784 review fix: a landed agent URL is a chat with a rail, so ⌘. works there; the rail-less page in the test is now the unreachable-agent refusal. Co-Authored-By: Claude Fable 5.1 --- .../workspace-chat-tabs-and-titles.md | 8 +- docs/memory/requirements/core-agent.md | 4 + .../src/components/portal/portalKeymap.js | 19 ++++ src/frontend/src/views/Portal.vue | 28 +++++- .../tests/unit/portalKeyList.mount.spec.js | 3 +- .../tests/unit/workspaceKeymap.mount.spec.js | 94 ++++++++++++++++++- .../tests/unit/workspaceKeymap.spec.js | 17 +++- 7 files changed, 162 insertions(+), 11 deletions(-) diff --git a/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md b/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md index 6c0bbbbc2..b0721e56f 100644 --- a/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md +++ b/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md @@ -111,8 +111,12 @@ dispatcher (`PortalRail.vue`'s sheet-only Esc `window` listener is the `close-to protocol entry the map declares, not a second dispatcher), armed above `bootstrap()`'s `await` (contract #23) and removed in `onBeforeUnmount`. The ladder, in order, because the order is the design: -1. `resolveWorkspaceKey(e)` — a chord nobody owns, a **key repeat**, or an IME - composition returns null and we return **without** `preventDefault`; +0. a **key repeat** never dispatches (one press is one action), but the repeat + of the press the shell last claimed is `preventDefault`ed — `heldKey`, + matched with `workspaceChord(e)` — so a held `⌥.` cannot type `≥` into the + message field; the repeat of an unclaimed press is left to the browser; +1. `resolveWorkspaceKey(e)` — a chord nobody owns or an IME composition returns + null and we return **without** `preventDefault`; 2. signed in (the OTP form answers no keys); 3. ⌘J keeps its place at the top and its own "leave the call?" ask — the one key a live voice call answers rather than swallows; diff --git a/docs/memory/requirements/core-agent.md b/docs/memory/requirements/core-agent.md index a10449223..91398bc28 100644 --- a/docs/memory/requirements/core-agent.md +++ b/docs/memory/requirements/core-agent.md @@ -1855,6 +1855,10 @@ bounding the table. OSS-core (Workspace rule above). Flow: call suppress the moving and rail keys **silently**; `⌘J` keeps its own ask during a call, and `⌘/` stays available. A held key does not walk the list (`e.repeat` resolves to nothing) and an IME composition is never a chord. + The auto-repeat of a press the shell **claimed** is still swallowed + (`heldKey` + `workspaceChord`), so a held `⌥.` cannot type `≥` into the + message field and a held `Ctrl+J` cannot open the browser's Downloads; the + repeat of a press it did not claim stays the browser's. - **AC-6 — non-US layouts**: a chord matches by `key` **or** by physical `code`, so the US position works on a non-Latin layout; `Shift` is accepted on a `key` match only, so DE's `Shift+7` → `/` and FR's shifted `.` reach diff --git a/src/frontend/src/components/portal/portalKeymap.js b/src/frontend/src/components/portal/portalKeymap.js index 6803cdacb..ed1593c65 100644 --- a/src/frontend/src/components/portal/portalKeymap.js +++ b/src/frontend/src/components/portal/portalKeymap.js @@ -254,6 +254,25 @@ export function resolveWorkspaceKey(e, map = WORKSPACE_KEYMAP) { if (!e) return null // A held key must not remount the conversation N times a second. if (e.repeat) return null + return workspaceChord(e, map) +} + +/** + * The action whose CHORD this event is — a press or its auto-repeat alike. + * + * `resolveWorkspaceKey` answers "should something happen", and for a repeat + * the answer is no: one press is one action. But a repeat of a chord the shell + * claimed is still not the browser's, and the dispatcher cannot say so with a + * null: left alone, a held ⌥. types `≥` into the message field on a Mac and a + * held Ctrl+J opens the browser's Downloads. This is the half of the question + * the repeat can still answer — WHOSE chord it is — and the dispatcher pairs it + * with the press it claimed. + * + * Reserved and protocol entries are null here for the same reason they are + * null there, and an IME composition is never a chord. + */ +export function workspaceChord(e, map = WORKSPACE_KEYMAP) { + if (!e) return null if (e.isComposing || e.keyCode === 229) return null const list = Array.isArray(map) ? map : [] for (const entry of list) { diff --git a/src/frontend/src/views/Portal.vue b/src/frontend/src/views/Portal.vue index d33e3b6be..ce72bb7b3 100644 --- a/src/frontend/src/views/Portal.vue +++ b/src/frontend/src/views/Portal.vue @@ -947,7 +947,7 @@ import { // ent#621 — the Workspace key map. The rules are pure and live there; this view // owns the one listener and the dispatch table (T3). import { - resolveWorkspaceKey, keymapSuppressed, hasModalOpen, nextAgent, recordLastOpen, + resolveWorkspaceKey, workspaceChord, keymapSuppressed, hasModalOpen, nextAgent, recordLastOpen, nextRailTab, keyListRows, hostPlatform, WORKSPACE_KEYMAP, } from '@/components/portal/portalKeymap' // ent#557: the tab title's unread half. The router owns the label; this pushes @@ -2496,9 +2496,11 @@ const KEY_DISPATCH = Object.freeze({ // bootstrap's await (contract #23, guarded by `mountListenerOrdering.spec.js`), // and inert until signed in. In order, because the order IS the design: // -// 1. `resolveWorkspaceKey` — a chord we do not own, a key repeat or an IME -// composition returns null, and we return WITHOUT `preventDefault`: ⌘K -// must keep reaching the browser until ent#577 binds it. +// 0. A key REPEAT never dispatches — one press is one action — but the repeat +// of the press the shell just claimed is swallowed (`heldKey`, below). +// 1. `resolveWorkspaceKey` — a chord we do not own or an IME composition +// returns null, and we return WITHOUT `preventDefault`: ⌘K must keep +// reaching the browser until ent#577 binds it. // 2. Signed in. The OTP form answers no keys. // 3. ⌘J keeps its place at the top AND its own "leave the call?" ask // (ent#534/551) — the one key a call answers rather than swallows. @@ -2512,13 +2514,30 @@ const KEY_DISPATCH = Object.freeze({ // (`defaultPrevented` — the Esc protocol, the typeahead's bare arrows), // anything modal, the mobile drawer, or a live voice call. // Only then `preventDefault`, and only then the action. +// +// (review) `heldKey` is the action of the press the shell last CLAIMED. A held +// key auto-repeats, and those repeats used to fall out at step 1 unprevented — +// so the browser got them: a held ⌥. typed `≥` into the message field on a +// Mac, a held Ctrl+J opened Downloads. The repeat is matched to the claimed +// PRESS rather than re-run down the ladder, for two reasons: the action has +// usually changed what the ladder would say (⌘J has just opened the picker, +// which is modal), and the repeat of a press the shell did NOT claim must stay +// the browser's exactly as the press did (⌘. on a page with no rail). Any new +// press ends the hold, so a stale claim can never eat a later key. +let heldKey = null function onGlobalKeydown(e) { + if (e.repeat) { + if (heldKey && workspaceChord(e) === heldKey) e.preventDefault() + return + } + heldKey = null const action = resolveWorkspaceKey(e) if (!action) return if (!store.isClientSignedIn) return if (action === 'new-chat') { if (voiceCall.value.active) { // ent#534/ent#551: the call owns the stage — ask first e.preventDefault() + heldKey = action guardLeaveCall(() => onGlobalKeydown(e)) return } @@ -2535,6 +2554,7 @@ function onGlobalKeydown(e) { railAvailable: railKeysAvailable(), })) return e.preventDefault() + heldKey = action run() if (STAGE_KEYS.includes(action)) nextTick(focusConversationComposer) } diff --git a/src/frontend/tests/unit/portalKeyList.mount.spec.js b/src/frontend/tests/unit/portalKeyList.mount.spec.js index cdcd3eab7..e2de1f975 100644 --- a/src/frontend/tests/unit/portalKeyList.mount.spec.js +++ b/src/frontend/tests/unit/portalKeyList.mount.spec.js @@ -66,7 +66,8 @@ describe('ent#621 — the key list is the map, rendered', () => { expect(rowFor('rail-tab-next').textContent).not.toContain('no rail') w.unmount() - // On the agent page there is no rail. The row STAYS and says so: a key + // On a page with no rail (Projects, the Inbox with nothing selected) the + // row STAYS and says so: a key // that vanishes from the list and does nothing on the page is the one a // person retries forever. const bare = open({ rows: keyListRows(WORKSPACE_KEYMAP, MAC, { railAvailable: false }) }) diff --git a/src/frontend/tests/unit/workspaceKeymap.mount.spec.js b/src/frontend/tests/unit/workspaceKeymap.mount.spec.js index 5a637a134..a4be15e7a 100644 --- a/src/frontend/tests/unit/workspaceKeymap.mount.spec.js +++ b/src/frontend/tests/unit/workspaceKeymap.mount.spec.js @@ -453,6 +453,80 @@ describe('ent#621 — the suppression ladder', () => { }) }) +// A raw keydown with every field the cases below need (`press` has no `code`, +// `ctrlKey` or `repeat`). +async function fire(w, init) { + const e = new KeyboardEvent('keydown', { bubbles: true, cancelable: true, ...init }) + window.dispatchEvent(e) + await flushPromises() + await w.vm.$nextTick() + return e +} + +describe('ent#621 (review) — a held key stays the shell\'s', () => { + // The auto-repeat of a chord the shell CLAIMED was left to the browser: the + // resolver answers null for a repeat, and the dispatcher returned on null + // before `preventDefault`. Reproduced in a browser — a held ⌥. types `≥` + // into the message field on a Mac, a held Ctrl+J opens Downloads on + // Chrome/Firefox for Windows and Linux (⌘J's repeats were swallowed before + // this map existed). + it('swallows the repeat of a press it claimed, and does not act again', async () => { + const { w } = await boot('/workspace/c/s-main') + await fire(w, { key: 'ArrowDown', altKey: true }) + const moved = showing(w).agent + expect(moved).not.toBe('scout') + + const e = await fire(w, { key: 'ArrowDown', altKey: true, repeat: true }) + expect(e.defaultPrevented).toBe(true) // not the browser's + expect(showing(w).agent).toBe(moved) // and not a second move + }) + + it('a held ⌥. never types into the message field', async () => { + viewport(true) + const { w } = await boot('/workspace/c/s-main') + // A US Mac: ⌥. arrives as `key: '≥'`, matched on its physical `code`. + await fire(w, { key: '≥', code: 'Period', altKey: true }) + expect(w.vm.railState.open).toBe(true) + const tab = w.vm.railState.tab + + const e = await fire(w, { key: '≥', code: 'Period', altKey: true, repeat: true }) + expect(e.defaultPrevented).toBe(true) + expect(w.vm.railState.tab).toBe(tab) // one press, one tab + }) + + it('a held Ctrl+J makes one new chat and never reaches the browser', async () => { + const { w } = await boot('/workspace/c/s-main') + await fire(w, { key: 'j', code: 'KeyJ', ctrlKey: true }) + expect(showing(w).newChat).toBe(true) + const uid = conversation(w).vm.$.uid + + const e = await fire(w, { key: 'j', code: 'KeyJ', ctrlKey: true, repeat: true }) + expect(e.defaultPrevented).toBe(true) + expect(conversation(w).vm.$.uid).toBe(uid) // not remounted per repeat + }) + + it('leaves the repeat of a press it did NOT claim to the browser', async () => { + viewport(true) + // Nothing owns ⌘. where there is no rail, so neither the press nor its + // repeat may be taken from the browser (Safari's Stop). + const { w } = await boot('/workspace/a/ghost') + const first = await fire(w, { key: '.', code: 'Period', metaKey: true }) + expect(first.defaultPrevented).toBe(false) + const e = await fire(w, { key: '.', code: 'Period', metaKey: true, repeat: true }) + expect(e.defaultPrevented).toBe(false) + }) + + it('stops swallowing once another key has been pressed', async () => { + // The claim belongs to ONE physical press. A later repeat event that does + // not continue it must not be eaten on the strength of an old one. + const { w } = await boot('/workspace/c/s-main') + await fire(w, { key: 'ArrowDown', altKey: true }) + await fire(w, { key: 'a' }) + const e = await fire(w, { key: 'ArrowDown', altKey: true, repeat: true }) + expect(e.defaultPrevented).toBe(false) + }) +}) + describe('ent#621 [C] — ⌘. shows and hides the rail', () => { it('toggles the rail column, remembers the tab, and persists', async () => { viewport(true) @@ -496,11 +570,25 @@ describe('ent#621 [C] — ⌘. shows and hides the rail', () => { expect(w.vm.railSheetOpen).toBe(false) }) - it('is a silent no-op on a page with no rail', async () => { + it('works on the new chat an agent landing rests on', async () => { viewport(true) - // The agent page has no rail column at all. A key that toggled hidden - // state there would be a key that does nothing visible — twice. + // ent#784 (review): `/workspace/a/:name` is a chat like any other once it + // has landed, so it has the rail — and the rail's key. It used to read as + // a rail-free page, which left ⌘. dead on the commonest stage there is. const { w } = await boot('/workspace/a/nova') + expect(w.vm.railHasColumn).toBe(true) + const before = w.vm.railState.open + const e = await press(w, { key: '.', meta: true }) + expect(w.vm.railState.open).toBe(!before) + expect(e.defaultPrevented).toBe(true) + }) + + it('is a silent no-op on a page with no rail', async () => { + viewport(true) + // The refusal for an agent this link cannot reach holds no conversation, + // so no rail column at all. A key that toggled hidden state there would be + // a key that does nothing visible — twice. + const { w } = await boot('/workspace/a/ghost') expect(w.vm.railHasColumn).toBe(false) const before = w.vm.railState.open const e = await press(w, { key: '.', meta: true }) diff --git a/src/frontend/tests/unit/workspaceKeymap.spec.js b/src/frontend/tests/unit/workspaceKeymap.spec.js index 748997908..f9fe88b30 100644 --- a/src/frontend/tests/unit/workspaceKeymap.spec.js +++ b/src/frontend/tests/unit/workspaceKeymap.spec.js @@ -23,7 +23,7 @@ import { describe, it, expect } from 'vitest' import { WORKSPACE_KEYMAP, OWNER_SHELL, OWNER_PROTOCOL, SCOPE_MOVING, SCOPE_DOING, SCOPE_PROTOCOL, - entryChords, findBinding, matchesChord, resolveWorkspaceKey, keymapCollisions, + entryChords, findBinding, matchesChord, resolveWorkspaceKey, workspaceChord, keymapCollisions, keymapSuppressed, cycleIndex, nextAgent, recordLastOpen, nextRailTab, chordLabel, ariaKeyshortcuts, keyListRows, } from '../../src/components/portal/portalKeymap' @@ -173,6 +173,21 @@ describe('resolveWorkspaceKey', () => { expect(resolveWorkspaceKey(null)).toBeNull() }) + it('still knows whose chord a held key is, so its repeat can be kept from the browser', () => { + // One press is one action, so `resolveWorkspaceKey` answers null for a + // repeat — but the repeat of a chord the shell claimed is not the + // browser's either: a held ⌥. types `≥` into the message field on a Mac, + // and a held Ctrl+J opens the browser's Downloads. + expect(workspaceChord(ev({ key: '≥', code: 'Period', altKey: true, repeat: true }))).toBe('rail-tab-next') + expect(workspaceChord(ev({ key: 'j', code: 'KeyJ', ctrlKey: true, repeat: true }))).toBe('new-chat') + expect(workspaceChord(ev({ key: 'ArrowDown', altKey: true }))).toBe('agent-next') + // Never for a chord the shell does not dispatch, and never mid-composition. + expect(workspaceChord(ev({ key: 'k', code: 'KeyK', metaKey: true, repeat: true }))).toBeNull() + expect(workspaceChord(ev({ key: 'Escape', repeat: true }))).toBeNull() + expect(workspaceChord(ev({ key: 'ArrowDown', altKey: true, repeat: true, isComposing: true }))).toBeNull() + expect(workspaceChord(null)).toBeNull() + }) + it('says nothing about plain typing', () => { for (const key of ['a', 'Enter', '.', '/', 'j', 'ArrowDown', 'ArrowUp']) { expect(resolveWorkspaceKey(ev({ key })), key).toBeNull() From 1334f5b9c22a0c6ab61722efd736d2a09dd32972 Mon Sep 17 00:00:00 2001 From: trinity-ability <309458136+trinity-ability@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:56:52 +0100 Subject: [PATCH 19/22] =?UTF-8?q?feat(portal):=20=E2=8C=98/=20focuses=20th?= =?UTF-8?q?e=20sidebar=20search,=20and=20the=20rail=20shows=20shortcut=20t?= =?UTF-8?q?ips=20(ent#621=20follow-up)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `⌘/` (Ctrl+/) puts the cursor in the sidebar's "Search agents and chats" field and selects its text; pressed again from the field it hands the caret back to the message field. Below `sm` it opens the drawer and focuses that instance. The field shows the chord (key cap, title, aria-keyshortcuts). Chosen to echo the Dashboard's `/` filter key. - The shortcuts list moves to `⌥/` to free the chord, keeping the `⌘.` / `⌥.` pairing. - `PortalKeyTips`: a small panel pinned to the bottom of the open rail with four headline chords and an "All shortcuts" button; one icon at the foot of the collapsed strip; neither on the mobile sheet. Rows are derived from the key map (`KEY_TIPS` -> `keyTipRows`). Dismissible, remembered per browser in localStorage. - `PortalKeyCap`: the one `` recipe for the key list, the tips and the search field. Requirements: core-agent.md 5.21a AC-1/9 amended, AC-11 and AC-12 added. Co-Authored-By: Claude Fable 5.1 --- .../workspace-chat-tabs-and-titles.md | 21 +++- docs/memory/requirements/core-agent.md | 30 ++++- .../src/components/base/BaseModal.vue | 2 +- .../src/components/portal/PortalKeyCap.vue | 7 ++ .../src/components/portal/PortalKeyList.vue | 9 +- .../src/components/portal/PortalKeyTips.vue | 80 +++++++++++++ .../src/components/portal/PortalRail.vue | 23 +++- .../src/components/portal/PortalSidebar.vue | 41 ++++++- .../src/components/portal/portalKeymap.js | 73 +++++++++++- src/frontend/src/views/Portal.vue | 40 ++++++- .../tests/unit/portalKeyHints.mount.spec.js | 52 ++++++++ .../tests/unit/portalKeyTips.mount.spec.js | 111 ++++++++++++++++++ .../tests/unit/workspaceKeymap.mount.spec.js | 107 ++++++++++++++++- .../tests/unit/workspaceKeymap.spec.js | 84 +++++++++++-- 14 files changed, 647 insertions(+), 33 deletions(-) create mode 100644 src/frontend/src/components/portal/PortalKeyCap.vue create mode 100644 src/frontend/src/components/portal/PortalKeyTips.vue create mode 100644 src/frontend/tests/unit/portalKeyTips.mount.spec.js diff --git a/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md b/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md index b0721e56f..5d5fb5a94 100644 --- a/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md +++ b/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md @@ -86,7 +86,8 @@ surface rather than only within one owner's share: | ⌥⇧↓ / ⌥⇧↑ (Alt+Shift+↓ / ↑) | Next / previous chat with this agent | shell | | ⌘. / Ctrl+. | Show or hide the rail | shell | | ⌥. (Alt+.) | Next rail tab | shell | -| ⌘/ / Ctrl+/ | The keyboard-shortcuts list | shell | +| ⌘/ / Ctrl+/ | Cursor into the sidebar's search field; again from the field, back to the message field | shell | +| ⌥/ (Alt+/) | The keyboard-shortcuts list | shell | | Esc, the call's `M`, the preview's ←/→, tab roving, a dialog's Tab cycle, the composer typeahead's bare arrows | — | the component that owns the overlay | | ⌘K / Ctrl+K | nothing — **reserved** for search (ent#577) | nobody | @@ -127,7 +128,7 @@ protocol entry the map declares, not a second dispatcher), armed above `bootstra 6. `keymapSuppressed`: a nearer owner already claimed the event (`defaultPrevented`, which is how the Esc protocol and the typeahead's bare arrows win), anything modal (`[aria-modal="true"]`, with a per-action - exemption so ⌘/ can close its own dialog and the rail sheet does not count + exemption so ⌥/ can close its own dialog and the rail sheet does not count as something modal over the rail), the mobile drawer, or a voice call — which suppresses the moving and rail keys silently. @@ -152,7 +153,21 @@ inverted here, since the hands that want the next chat are already typing. rule's own arms rather than a second rule. Memory is in-memory and session-scoped, as the acceptance criteria asked: a reload starts fresh. -**Discoverability.** ⌘/ opens `PortalKeyList` — a `BaseModal` dialog built +**Search key.** ⌘/ runs `Portal.vue::focusSearch`: at and above `sm` it calls +the desktop `PortalSidebar`'s exposed `focusSearch()` (focus + select), or — +when that field already has focus (`searchHasFocus()`) — hands the caret back +through `focusConversationComposer`; below `sm` it opens the drawer and focuses +that instance's field on the next tick. The field shows the chord in a +`PortalKeyCap` while it is empty and unfocused. + +**Tips panel.** `PortalKeyTips.vue` is pinned to the bottom of the OPEN rail +(`PortalRail`'s `keyTips` prop; an icon button at the foot of the collapsed +strip is the same door; the sheet has neither). Its rows are +`keyTipRows(platform)` over `KEY_TIPS`, its button emits `open-keys`, and its +close emits `dismiss-key-tips`, which the shell persists with +`saveKeyTipsDismissed` (`localStorage['trinity-workspace-key-tips']`). + +**Discoverability.** ⌥/ opens `PortalKeyList` — a `BaseModal` dialog built from `keyListRows(WORKSPACE_KEYMAP, platform)`, so a key cannot exist without appearing in the list (minus the reserved row). The sidebar footer has a "Keyboard shortcuts" button carrying the same chord, and the surfaces a key diff --git a/docs/memory/requirements/core-agent.md b/docs/memory/requirements/core-agent.md index 91398bc28..f65eb84fb 100644 --- a/docs/memory/requirements/core-agent.md +++ b/docs/memory/requirements/core-agent.md @@ -1825,7 +1825,8 @@ bounding the table. OSS-core (Workspace rule above). Flow: - **AC-1 — the keys**: `⌘J`/`Ctrl+J` new chat with this agent (§5.21, unchanged); `⌥↓`/`⌥↑` next / previous **agent**; `⌥⇧↓`/`⌥⇧↑` next / previous **chat with this agent**; `⌘.`/`Ctrl+.` show or hide the rail; - `⌥.` next rail tab; `⌘/`/`Ctrl+/` the keyboard-shortcuts list. + `⌥.` next rail tab; `⌘/`/`Ctrl+/` the cursor into the sidebar's search + field (AC-11); `⌥/` the keyboard-shortcuts list. - **AC-2 — one declaration**: every chord lives in `src/frontend/src/components/portal/portalKeymap.js` (pure — no DOM, no store), with its scope, its owner, and its label. `keymapCollisions(map)` @@ -1850,10 +1851,10 @@ bounding the table. OSS-core (Workspace rule above). Flow: through `defaultPrevented` (the Esc protocol, the composer typeahead's bare arrows, which claim bare arrows only). - **AC-5 — suppression**: anything modal (`[aria-modal="true"]`, with a - per-action exemption so `⌘/` can close its own dialog and the rail sheet + per-action exemption so `⌥/` can close its own dialog and the rail sheet does not count as modal over the rail), the mobile drawer, and a live voice call suppress the moving and rail keys **silently**; `⌘J` keeps its own ask - during a call, and `⌘/` stays available. A held key does not walk the list + during a call, and `⌘/` and `⌥/` stay available. A held key does not walk the list (`e.repeat` resolves to nothing) and an IME composition is never a chord. The auto-repeat of a press the shell **claimed** is still swallowed (`heldKey` + `workspaceChord`), so a held `⌥.` cannot type `≥` into the @@ -1877,7 +1878,7 @@ bounding the table. OSS-core (Workspace rule above). Flow: that agent. An unsent new chat records nothing, so its draft is reached through the landing rule's own arms rather than a second rule. Every moving key focuses the composer afterwards. -- **AC-9 — discoverable**: `⌘/` opens a shortcuts dialog built **from the +- **AC-9 — discoverable**: `⌥/` opens a shortcuts dialog built **from the map**, so a key cannot ship without a row (the reserved chord is excluded — nothing binds it, and a dead row in a help dialog is worse than an undocumented key). The sidebar footer offers the same dialog, and every @@ -1886,6 +1887,26 @@ bounding the table. OSS-core (Workspace rule above). Flow: - **AC-10 — reserved, not bound**: `⌘K`/`Ctrl+K` is declared and **unhandled** — no `preventDefault`, no row in the list — so the browser's own behaviour stands until the search surface ships. +- **AC-11 — the search key**: `⌘/`/`Ctrl+/` puts the cursor in the sidebar's + "Search agents and chats" field and selects what is in it — the Dashboard's + `/` filter key (§ fleet filter), with the modifier a key needs to work from + inside a text field. Pressed again while the field has focus it hands the + caret back to the message field. Below `sm` the sidebar is the drawer, so the + key opens it and focuses that instance's field. The field shows the chord + (a key cap while it is empty and unfocused, plus `title` and + `aria-keyshortcuts`). It is NOT the reserved `⌘K` surface: it focuses the + search that exists and adds none. The shortcuts list moved to `⌥/` to free + the chord, keeping the `⌘.` / `⌥.` pairing (the primary key acts, the Option + key steps to its neighbour). +- **AC-12 — the tips panel**: the open rail carries a small panel pinned to its + bottom edge listing four headline chords (switch agent, switch chat, search, + new chat) and a button that opens the shortcuts dialog. Rows are derived from + the map (`KEY_TIPS` → `keyTipRows`), never typed. The collapsed strip carries + the same door as one icon button at its foot; the mobile sheet carries + neither. The panel is dismissible, and the dismissal is remembered per + browser (`localStorage['trinity-workspace-key-tips']`, a per-viewer + convenience — nothing is stored server-side); the sidebar footer button stays + as the way back to the full list. - **Not this slice**: a search/spotlight surface behind `⌘K`; remapping or disabling the chords in settings (they are modifier chords; nothing conflicts with typing); the mobile drawer's own Esc/focus-trap contract (#1923). @@ -1898,6 +1919,7 @@ bounding the table. OSS-core (Workspace rule above). Flow: `workspaceKeymap.mount.spec.js` (the shell: dispatch, the walk, the memory, the bails and suppression), `portalChatCycle.mount.spec.js`, `portalKeyList.mount.spec.js`, `portalKeyHints.mount.spec.js`, + `portalKeyTips.mount.spec.js` (the tips panel, both rail forms), `portalRosterRow.spec.js` (the bound and the handed-down order, mounted). - **Flow**: `docs/memory/feature-flows/workspace-chat-tabs-and-titles.md` (→ The key map); the landing rule it calls is diff --git a/src/frontend/src/components/base/BaseModal.vue b/src/frontend/src/components/base/BaseModal.vue index cd52d9077..1d428d72b 100644 --- a/src/frontend/src/components/base/BaseModal.vue +++ b/src/frontend/src/components/base/BaseModal.vue @@ -67,7 +67,7 @@ import { // Attributes land on the DIALOG, not on `Teleport`. A consumer that needs the // overlay element itself marked — `PortalKeyList`'s `data-ws-key-list`, which -// the Workspace key dispatcher excludes so `⌘/` can close the list it opened +// the Workspace key dispatcher excludes so `⌥/` can close the list it opened // — must put that attribute on the SAME element as `aria-modal`, because that // is the element the `:not(...)` exclusion matches. Without this the attribute // is dropped silently and the marker is a comment rather than a selector. diff --git a/src/frontend/src/components/portal/PortalKeyCap.vue b/src/frontend/src/components/portal/PortalKeyCap.vue new file mode 100644 index 000000000..9b8bd25e8 --- /dev/null +++ b/src/frontend/src/components/portal/PortalKeyCap.vue @@ -0,0 +1,7 @@ + diff --git a/src/frontend/src/components/portal/PortalKeyList.vue b/src/frontend/src/components/portal/PortalKeyList.vue index 95b9486e2..a8af25d4c 100644 --- a/src/frontend/src/components/portal/PortalKeyList.vue +++ b/src/frontend/src/components/portal/PortalKeyList.vue @@ -1,5 +1,5 @@ @@ -177,6 +181,18 @@
+ + +
@@ -188,6 +204,7 @@ import OverflowTabs from '../OverflowTabs.vue' import BaseButton from '@/components/base/BaseButton.vue' import BaseBadge from '@/components/base/BaseBadge.vue' import PortalAvatar from './PortalAvatar.vue' +import PortalKeyTips from './PortalKeyTips.vue' import { RAIL_SIGNAL_LIVE, RAIL_SIGNAL_UPDATED, @@ -221,9 +238,13 @@ const props = defineProps({ participants: { type: Array, default: () => [] }, // The mobile bottom-sheet form. A sheet is always "open". sheet: { type: Boolean, default: false }, + // ent#621 (follow-up): show the shortcut tips — the panel at the foot of the + // open column, the icon at the foot of the collapsed strip. The shell turns + // it off once the person has closed it; the rail holds no memory of its own. + keyTips: { type: Boolean, default: false }, }) -const emit = defineEmits(['update:open', 'update:activeTab', 'close', 'see-hints']) +const emit = defineEmits(['update:open', 'update:activeTab', 'close', 'see-hints', 'open-keys', 'dismiss-key-tips']) const mode = computed(() => (props.sheet ? 'sheet' : props.open ? 'open' : 'collapsed')) diff --git a/src/frontend/src/components/portal/PortalSidebar.vue b/src/frontend/src/components/portal/PortalSidebar.vue index 5138a4f59..4f5d1c975 100644 --- a/src/frontend/src/components/portal/PortalSidebar.vue +++ b/src/frontend/src/components/portal/PortalSidebar.vue @@ -42,13 +42,31 @@
+ + +
@@ -323,7 +341,7 @@ + the `⌥/` chord opens (the ent#261 `` precedent). --> -
- -
-

Shortcuts

+ never sits over the content. + + TWO ROWS, and nothing above or below them: the four tips flow by column + into a 2×2 (agent over chat, search over new chat), and the two controls + stack beside them. What gives when the rail is dragged narrow is the + LABEL (it shortens with an ellipsis, the full text on hover); a key cap + never wraps and a row never becomes two. The columns are `auto`, so + each is as wide as its own content asks before either gives anything + up — the first is the wider one (`Alt+Shift+↑↓` against `Ctrl+J`), and + an even split would shorten its labels with room to spare beside it. --> +
+ +
    +
  • + {{ row.label }} + {{ row.keys }} +
    • +
+
-
-
    -
  • - {{ row.label }} - {{ row.keys }} -
    • -
- All shortcuts + + +
@@ -64,7 +77,6 @@ * rail whether to mount this at all. */ import BaseCard from '@/components/base/BaseCard.vue' -import BaseButton from '@/components/base/BaseButton.vue' import PortalKeyCap from './PortalKeyCap.vue' import { keyTipRows, keyHint, keyShortcutsFor, hostPlatform } from './portalKeymap' @@ -73,6 +85,13 @@ const ROWS = keyTipRows(KEY_PLATFORM) const KEY_LIST_HINT = keyHint('key-list', KEY_PLATFORM) const KEY_LIST_SHORTCUTS = keyShortcutsFor('key-list', KEY_PLATFORM) +// The keyboard the sidebar footer's "Keyboard shortcuts" button draws: the same +// door, so the same picture — in both forms of this component. +const KEYBOARD_GLYPH = 'M3 8a2 2 0 012-2h14a2 2 0 012 2v8a2 2 0 01-2 2H5a2 2 0 01-2-2V8zm4 1v.01M11 9v.01M15 9v.01M7 13h10' +// The panel's two controls, ONE class string: 24px targets stacked beside the +// two rows, so they add no line of their own. +const TIP_BTN = 'p-1 rounded text-gray-400 hover:text-gray-700 dark:hover:text-gray-200 hover:bg-gray-100 dark:hover:bg-gray-700 transition' + defineProps({ compact: { type: Boolean, default: false }, }) diff --git a/src/frontend/src/components/portal/portalKeymap.js b/src/frontend/src/components/portal/portalKeymap.js index a26368c4e..285035a2a 100644 --- a/src/frontend/src/components/portal/portalKeymap.js +++ b/src/frontend/src/components/portal/portalKeymap.js @@ -532,6 +532,26 @@ export function keyHint(actions, platform, map = WORKSPACE_KEYMAP) { return bindingsFor(actions, map).map((c) => chordLabel(c, platform)).join(' / ') } +/** + * The same hint, PACKED: chords that share every modifier and differ only in a + * one-glyph key print the modifiers once — `⌥↑↓` for `⌥↑ / ⌥↓`, + * `Alt+Shift+↑↓` for the Windows pair. For a place too small for the long form + * (the rail's two-row tips); anything that cannot be packed without losing + * something — different modifiers, a key that is a word — falls back to + * `keyHint`, so the packed form is never a guess. + */ +export function keyHintCompact(actions, platform, map = WORKSPACE_KEYMAP) { + const chords = bindingsFor(actions, map) + if (chords.length < 2) return keyHint(actions, platform, map) + const [first] = chords + const glyphs = chords.map((c) => keyGlyph(c.key)) + const packable = glyphs.every((g) => g.length === 1) && chords.every((c) => + !!c.alt === !!first.alt && !!c.shift === !!first.shift && !!c.primary === !!first.primary) + if (!packable) return keyHint(actions, platform, map) + // `chordLabel` owns the modifier spelling and order; hand it the packed key. + return chordLabel({ ...first, key: null }, platform) + glyphs.join('') +} + /** The same binding as `aria-keyshortcuts` — ARIA's spelling, space-separated. */ export function keyShortcutsFor(actions, platform, map = WORKSPACE_KEYMAP) { return ariaKeyshortcuts(bindingsFor(actions, map), platform) @@ -557,7 +577,10 @@ export function keyTipRows(platform, map = WORKSPACE_KEYMAP, tips = KEY_TIPS) { return (Array.isArray(tips) ? tips : []).filter(Boolean).map((t) => ({ id: t.id, label: t.label, - keys: keyHint(t.actions, platform, map), + // Packed for the key cap (the panel is two rows of two); the long form is + // kept for the hover, where there is room to spell it out. + keys: keyHintCompact(t.actions, platform, map), + fullKeys: keyHint(t.actions, platform, map), ariaKeyshortcuts: keyShortcutsFor(t.actions, platform, map), })).filter((r) => r.keys) } diff --git a/src/frontend/tests/unit/portalKeyTips.mount.spec.js b/src/frontend/tests/unit/portalKeyTips.mount.spec.js index bed6efb1e..ebeafd998 100644 --- a/src/frontend/tests/unit/portalKeyTips.mount.spec.js +++ b/src/frontend/tests/unit/portalKeyTips.mount.spec.js @@ -3,7 +3,8 @@ * ent#621 (follow-up) — the rail's shortcut tips panel, mounted. * * A small panel pinned to the bottom of the OPEN rail: four headline chords - * and a button that opens the full shortcuts dialog. The collapsed strip + * packed as two rows of two, a button that opens the full shortcuts dialog and + * a close — all inside those two rows. The collapsed strip * carries the same door as one icon; the mobile sheet carries neither. It is * dismissible, and the rail only REPORTS the dismissal — the shell owns the * memory (`workspaceKeymap.mount.spec.js`). @@ -51,6 +52,27 @@ describe('ent#621 (follow-up) — the tips panel in the open rail', () => { w.unmount() }) + it('is two rows of two: agent over chat, search over new chat', () => { + // jsdom lays nothing out, so this pins the three things the packing rests + // on (the real geometry is checked in a browser): the list flows by COLUMN + // into two rows, a label may shorten but a key cap never wraps, and a + // shortened label is still readable on hover. + const w = mountRail() + const list = panel(w).find('ul') + expect(list.classes()).toEqual(expect.arrayContaining(['grid', 'grid-flow-col', 'grid-rows-2'])) + const rows = list.findAll('li') + expect(rows.map((r) => r.attributes('data-ws-key-tip'))).toEqual(['agent', 'chat', 'search', 'new-chat']) + for (const row of rows) { + expect(row.find('kbd').classes()).toContain('whitespace-nowrap') + const label = row.find('[data-ws-key-tip-label]') + expect(label.classes()).toContain('truncate') + expect(row.attributes('title')).toContain(label.text()) + } + // Nothing above or below the two rows: no heading line, no footer button. + expect(panel(w).find('h3').exists()).toBe(false) + w.unmount() + }) + it('sits AFTER the body, so it is the bottom of the rail and never over its content', () => { const w = mountRail() const body = w.find('[data-testid="portal-rail-body"]').element @@ -63,7 +85,11 @@ describe('ent#621 (follow-up) — the tips panel in the open rail', () => { it('its button asks the shell for the full list', async () => { const w = mountRail() - await panel(w).find('[data-testid="ws-key-tips-all"]').trigger('click') + const all = panel(w).find('[data-testid="ws-key-tips-all"]') + // Icon-only to fit the two rows, so the name has to be in the label. + expect(all.attributes('aria-label')).toContain('All shortcuts') + expect(all.attributes('title')).toContain(keyHint('key-list', P)) + await all.trigger('click') expect(w.emitted('open-keys')).toHaveLength(1) w.unmount() }) diff --git a/src/frontend/tests/unit/workspaceKeymap.spec.js b/src/frontend/tests/unit/workspaceKeymap.spec.js index 759930637..2da21bd21 100644 --- a/src/frontend/tests/unit/workspaceKeymap.spec.js +++ b/src/frontend/tests/unit/workspaceKeymap.spec.js @@ -26,7 +26,7 @@ import { entryChords, findBinding, matchesChord, resolveWorkspaceKey, workspaceChord, keymapCollisions, keymapSuppressed, cycleIndex, nextAgent, recordLastOpen, nextRailTab, chordLabel, ariaKeyshortcuts, keyListRows, - KEY_TIPS, keyTipRows, keyHint, KEY_TIPS_STORAGE_KEY, loadKeyTipsDismissed, saveKeyTipsDismissed, + KEY_TIPS, keyTipRows, keyHint, keyHintCompact, KEY_TIPS_STORAGE_KEY, loadKeyTipsDismissed, saveKeyTipsDismissed, } from '../../src/components/portal/portalKeymap' import { isNewChatHotkey, newChatHotkeyLabel } from '../../src/components/portal/portalUtils' @@ -421,13 +421,31 @@ describe('ent#621 — the search key and the tips panel (follow-up)', () => { for (const action of tip.actions) expect(findBinding(action), action).not.toBeNull() } const mac = keyTipRows('MacIntel') - expect(mac.map((r) => r.keys)).toEqual(['⌥↑ / ⌥↓', '⌥⇧↑ / ⌥⇧↓', '⌘/', '⌘J']) - expect(keyTipRows('Win32').map((r) => r.keys)).toEqual(['Alt+↑ / Alt+↓', 'Alt+Shift+↑ / Alt+Shift+↓', 'Ctrl+/', 'Ctrl+J']) - // DERIVED, not typed: the row is whatever `keyHint` says for its actions. - for (const [i, tip] of KEY_TIPS.entries()) expect(mac[i].keys).toBe(keyHint(tip.actions, 'MacIntel')) + // PACKED: the panel is two rows of two, so a pair of chords that differ + // only in their last key prints its modifiers once. + expect(mac.map((r) => r.keys)).toEqual(['⌥↑↓', '⌥⇧↑↓', '⌘/', '⌘J']) + expect(keyTipRows('Win32').map((r) => r.keys)).toEqual(['Alt+↑↓', 'Alt+Shift+↑↓', 'Ctrl+/', 'Ctrl+J']) + // DERIVED, not typed: the row is whatever the map says for its actions — + // packed for the cap, spelled out in full for the hover. + for (const [i, tip] of KEY_TIPS.entries()) { + expect(mac[i].keys).toBe(keyHintCompact(tip.actions, 'MacIntel')) + expect(mac[i].fullKeys).toBe(keyHint(tip.actions, 'MacIntel')) + } expect(mac.every((r) => r.label && r.ariaKeyshortcuts)).toBe(true) }) + it('packs a pair of chords only when nothing is lost by it', () => { + expect(keyHintCompact(['agent-prev', 'agent-next'], 'MacIntel')).toBe('⌥↑↓') + expect(keyHintCompact(['chat-prev', 'chat-next'], 'Linux x86_64')).toBe('Alt+Shift+↑↓') + // One chord is just that chord. + expect(keyHintCompact('search-focus', 'MacIntel')).toBe(keyHint('search-focus', 'MacIntel')) + // Different modifiers cannot share a prefix: spelled out, as `keyHint` does. + expect(keyHintCompact(['rail-toggle', 'rail-tab-next'], 'MacIntel')).toBe(keyHint(['rail-toggle', 'rail-tab-next'], 'MacIntel')) + // …and neither can a key that is a word rather than a glyph. + expect(keyHintCompact(['close-top', 'call-mute'], 'MacIntel')).toBe(keyHint(['close-top', 'call-mute'], 'MacIntel')) + expect(keyHintCompact(['no-such-action'], 'MacIntel')).toBe('') + }) + it('a tip whose actions the map no longer binds is dropped, not shown keyless', () => { const rows = keyTipRows('MacIntel', WORKSPACE_KEYMAP, [{ id: 'gone', label: 'Gone', actions: ['no-such-action'] }, KEY_TIPS[2]]) expect(rows.map((r) => r.id)).toEqual(['search']) From 54cd121123accde9ee131e54018faab6ccbd265c Mon Sep 17 00:00:00 2001 From: trinity-ability <309458136+trinity-ability@users.noreply.github.com> Date: Tue, 6 Oct 2026 16:00:37 +0100 Subject: [PATCH 21/22] =?UTF-8?q?fix(portal):=20a=20=E2=8C=98J=20parked=20?= =?UTF-8?q?behind=20the=20leave-call=20ask=20fires=20once=20the=20person?= =?UTF-8?q?=20confirms=20(ent#621=20merge-train)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The call branch `preventDefault`ed the event and then re-entered the ladder with that same event, so on the resumed pass the suppression rung read the shell's own mark as "a nearer owner claimed it" and bailed — ⌘J was dead exactly when the person had said "end the call and leave". The continuation now re-enters as the `resumed` pass after a `nextTick` (the confirm dialog is aria-modal and leaves the DOM on that flush), and `keymapSuppressed` waives only the `defaultPrevented` rung for it; modal, drawer and call still apply. Also from the finding: the physical-`code` arm answers only when the printed key is not a single printable ASCII character, so a Latin non-QWERTY layout keeps paste and undo (Dvorak's ⌘V arrives on `Period`, ⌘Z on `Slash`). The non-ASCII cases the arm exists for (`≥`, Cyrillic) are unchanged. Tests: pure cases for the resumed rung and the Dvorak shape in workspaceKeymap.spec.js; the shell mounted for real in workspaceKeymap.mount.spec.js — ⌘J during a call asks, confirm fires the new chat, cancel fires nothing; the two voice-mode source pins re-pinned to the new continuation. `isNewChatHotkey` stays: workspaceKeymap.spec.js and portalChatTabsAndTitles.spec.js (here and on the stacked ent#621 follow-up branch) import it as the ⌘J equivalence pin, and the chat-tabs flow documents it as the delegate kept on purpose. Co-Authored-By: Claude Fable 5.1 --- .../workspace-chat-tabs-and-titles.md | 16 ++++++++-- docs/memory/requirements/core-agent.md | 10 +++++-- .../src/components/portal/portalKeymap.js | 28 ++++++++++++++--- src/frontend/src/views/Portal.vue | 14 +++++++-- .../tests/unit/portalVoiceMode.spec.js | 11 +++++-- .../tests/unit/workspaceKeymap.mount.spec.js | 30 +++++++++++++++++++ .../tests/unit/workspaceKeymap.spec.js | 28 +++++++++++++++++ 7 files changed, 124 insertions(+), 13 deletions(-) diff --git a/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md b/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md index b0721e56f..acd8de7fb 100644 --- a/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md +++ b/docs/memory/feature-flows/workspace-chat-tabs-and-titles.md @@ -98,7 +98,12 @@ dialog never advertises a key that does nothing. ctrl on every platform (the shipped ⌘J semantics), and a chord matches by `key` **or** by physical `code`. The `code` arm is what makes the map work on a non-Latin layout — and on a US Mac it is the *only* arm for `⌥.`, which types -`≥`. Shift is accepted on a `key` match only, so DE's `Shift+7` → `/` and +`≥`. It answers **only when the printed key is not a single printable ASCII +character** (merge-train 2026-10-06): on a Latin non-QWERTY layout the position +and the character disagree on purpose — Dvorak's ⌘V arrives as `key: 'v'` on +`Period`, ⌘Z as `key: 'z'` on `Slash` — and letting the position out-vote a +key that names itself made the rail toggle eat paste and the key list eat +undo. Shift is accepted on a `key` match only, so DE's `Shift+7` → `/` and FR's shifted `.` reach ⌘/ while macOS's `⌘?` stays the browser's Help search. `chordLabel` / `ariaKeyshortcuts` take the platform and render `⌥↓` vs `Alt+↓` and `Alt+ArrowDown`. `keymapCollisions(map)` does NOT take a platform @@ -119,7 +124,14 @@ protocol entry the map declares, not a second dispatcher), armed above `bootstra null and we return **without** `preventDefault`; 2. signed in (the OTP form answers no keys); 3. ⌘J keeps its place at the top and its own "leave the call?" ask — the one - key a live voice call answers rather than swallows; + key a live voice call answers rather than swallows. The ask parks the SAME + event and, on confirm, re-enters the ladder after a `nextTick` as the + **resumed** pass (`onGlobalKeydown(e, { resumed: true })`): step 3 has + already `preventDefault`ed that event, so rung 6 is told the mark is the + shell's own and not a nearer owner's (merge-train 2026-10-06 — read as a + claim, ⌘J was dead exactly when the person had said "end the call and + leave"), and the tick is what lets the confirm dialog leave the DOM before + the modal probe runs; 4. an action with no dispatch entry (a protocol key, a reserved chord) is left to the browser rather than silently eaten; 5. a focused `