From 5a663776df89ee19501285863b140eb6499ce1bb Mon Sep 17 00:00:00 2001 From: Oleksii Dolhov Date: Fri, 28 Aug 2026 11:53:17 +0300 Subject: [PATCH] fix(workspace): the ask badge said 2 and gave you no way to find them (#2424) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sidebar advertised "2 asks are waiting on your answer" and then stranded you: the agent that raised them carried no badge, its tooltip did not mention them, and it could be collapsed out of the roster entirely. The only way to locate a blocked agent was to open agents one at a time. Observed on a 12-agent roster with two asks on ws-sage (11th of 12), so on a fresh load the one row that mattered was behind the "show more" toggle. Three failures, fixed together because separately each is a half-measure — a badge with no destination, or a destination nobody can see. 1. The unit. `askCount` is `openAsks.length`, and the tooltip said "agents": two asks on ONE agent rendered as "2 agents are waiting on your answer". The number was right, the noun was wrong, and they only diverge when a single agent raises more than one ask — which is why it went unnoticed. Resolved toward ASKS rather than agents, because the row badges added here now answer "which agent", leaving the header to answer "how many decisions". 2. The row. `PortalSidebar.vue:139` renders a per-agent badge from `unreadByAgent` — unread REPLIES. Keeping asks out of that count is deliberate and documented at line 9 ("one is waiting on you to decide, the other on you to read"), and is preserved: the ask gets the *own badge* that comment promised, in `status-urgent` — the token the operator NavBar's pending-operator-queue badge already uses, so the two surfaces agree — and visually distinct from the indigo unread pill beside it. `agentRowTitle` had the same hole, so this is an accessibility fix too: a blocked agent's accessible name was the bare "Open ws-sage". 3. The collapse. #2159 capped the roster at five for a good reason (a long fleet pushed chats below the fold), but the slice is plain roster order with no ask weighting. Ask-bearing agents are now never hidden — appended, NOT floated to the top, because re-sorting on a transient count moves rows under the cursor between refreshes, the same reason the roster is not re-sorted by availability. Not a regression: every piece shipped in its intended form; the gap was between them. Everything decidable moved into `portalUtils` (`asksByAgent`, `askBadgeTitle`, `agentRowTitle`, `visibleAgentRows`, `AGENT_COLLAPSE_LIMIT`) because vitest runs `environment: 'node'` with no mount harness — a rule inside the SFC is one no test can reach, which is how all three of these shipped. Mutation-checked: reverting the noun, dropping asks from the title, and restoring the plain slice each turn the suite red. `bg-amber-500` -> `bg-status-urgent-500` is required, not drive-by: new code must be at zero raw palette classes, so the new badge needed a token, and the header had to match it or the two ask indicators would differ. Amber maps to `state-autonomous` (an operating mode), which is the wrong claim. PortalSidebar is now at zero non-gray raw classes. Two pre-existing guards asserted the moved expressions as source strings and are rewritten to assert the properties behaviourally — strictly stronger, since they now fail on a broken bound or a dropped chip title, not only on a reworded one: - portalRosterRow #2159 "shows a fixed number by default" - portalAvailabilityChip #2196 "row title carries the state" Verification: 1518/1518 frontend unit tests, raw-color ratchet exit 0, production build clean. Closes #2424 Co-Authored-By: Claude Opus 5 (1M context) --- .../src/components/portal/PortalSidebar.vue | 64 +++++-- .../src/components/portal/portalUtils.js | 74 +++++++ .../unit/portalAskDiscoverability.spec.js | 180 ++++++++++++++++++ .../tests/unit/portalAvailabilityChip.spec.js | 12 +- .../tests/unit/portalRosterRow.spec.js | 13 +- 5 files changed, 324 insertions(+), 19 deletions(-) create mode 100644 src/frontend/tests/unit/portalAskDiscoverability.spec.js diff --git a/src/frontend/src/components/portal/PortalSidebar.vue b/src/frontend/src/components/portal/PortalSidebar.vue index 2207e9138..f7d814391 100644 --- a/src/frontend/src/components/portal/PortalSidebar.vue +++ b/src/frontend/src/components/portal/PortalSidebar.vue @@ -8,13 +8,20 @@ Workspace + rather than being summed into that one. Before the unread count, + because a blocked agent outranks unread chatter. + + #2424: `status-urgent` (not amber) — the token the operator NavBar's + pending-operator-queue badge already uses for "waiting on you", so the + two surfaces agree. Amber maps to `state-autonomous`, an operating + mode, which is a different claim. The wording comes from + `askBadgeTitle` because the inline literal said "agents" while + `askCount` counted asks. --> {{ askCount > 99 ? '99+' : askCount }} {{ chipFor(a).label }} + + {{ askCountFor(a.name) > 99 ? '99+' : askCountFor(a.name) }} totalUnread(props.threads)) // would be a second path to the same fact, free to disagree with it. const asksStore = useClientPortalStore() const askCount = computed(() => asksStore.askCount) +// #2424: the ask twin of `waiting`. Kept a separate map on purpose — see the +// brand-badge comment in the template. +const asksPerAgent = computed(() => asksByAgent(asksStore.openAsks)) +const askCountFor = (name) => asksPerAgent.value[name] || 0 // A row key has to include the kind: thread ids and room ids are independent // spaces, so two chats of different kinds could collide on a bare id. @@ -300,25 +324,33 @@ const agentLabel = (a) => (a.display_label || '').trim() || a.name // deleted or lost the agent, and neither is actionable for a client. const chipFor = (a) => availabilityChip(a, { detailed: props.isPlatformSession }) +// The state must be reachable without relying on colour — which is why #2424 +// is an accessibility fix too: a blocked agent's title was the bare +// "Open ws-sage" while two asks waited on it. function agentRowTitle(a) { - const n = waitingFor(a.name) - const label = agentLabel(a) - const who = label === a.name ? label : `${label} (${a.name})` - const base = n - ? `${who} — ${n} unread ${n === 1 ? 'reply' : 'replies'}` - : `Open ${who}` - // The state must be reachable without relying on colour. const chip = chipFor(a) - return chip ? `${base} — ${chip.title}` : base + return buildAgentRowTitle({ + label: agentLabel(a), + name: a.name, + unread: waitingFor(a.name), + askCount: askCountFor(a.name), + chipTitle: chip ? chip.title : '', + }) } // #2159: a long fleet made the agents block the whole sidebar, pushing chats // below the fold. Top 5, expandable in place. -const AGENT_COLLAPSE_LIMIT = 5 +// +// #2424: an agent with an open ask is never collapsed out. The slice was plain +// roster order, so the one row a person needed could be the hidden one — the +// reported case had it 11th of 12 while the header advertised its two asks. The +// rule is in portalUtils (`AGENT_COLLAPSE_LIMIT` now lives there too, one +// definition for the slice and the toggle). const agentsExpanded = ref(false) -const shownAgents = computed(() => ( - agentsExpanded.value ? props.roster : props.roster.slice(0, AGENT_COLLAPSE_LIMIT) -)) +const shownAgents = computed(() => visibleAgentRows(props.roster, { + expanded: agentsExpanded.value, + askCounts: asksPerAgent.value, +})) // ent#186: history + search rows show the conversation's agent avatar instead of // a bare color dot. The URL is resolved from the roster already loaded at sign-in diff --git a/src/frontend/src/components/portal/portalUtils.js b/src/frontend/src/components/portal/portalUtils.js index 7abf7efae..d9629c238 100644 --- a/src/frontend/src/components/portal/portalUtils.js +++ b/src/frontend/src/components/portal/portalUtils.js @@ -65,6 +65,80 @@ export function unreadByAgent(threads) { return out } +// ent#364 / #2424: asks per agent — the ask twin of `unreadByAgent`. +// +// Deliberately a SEPARATE map, never summed into the unread count. The two are +// different facts about different obligations: an ask is waiting on you to +// DECIDE, an unread reply on you to READ. PortalSidebar has said so since +// ent#364; what it lacked was this half. +export function asksByAgent(asks) { + const out = {} + for (const a of Array.isArray(asks) ? asks : []) { + const name = a?.agent_name + if (!name) continue + out[name] = (out[name] || 0) + 1 + } + return out +} + +// The aggregate badge's accessible name. +// +// #2424: it counted ASKS and said "agents" — two asks raised by one agent read +// as "2 agents are waiting on your answer". The number was right and the noun +// was wrong, and the two only diverge when a single agent raises more than one +// ask, which is why nobody caught it. +// +// Resolved toward asks rather than agents, because the row badges added +// alongside this now answer "which agent" — so the header's job is "how many +// decisions", and that is a count of asks. +export function askBadgeTitle(count) { + const n = Number(count) || 0 + if (n <= 0) return '' + return `${n} ${n === 1 ? 'ask is' : 'asks are'} waiting on your answer` +} + +// The agent row's accessible name. +// +// #2424: this composed unread replies and the availability chip and never +// mentioned asks, so a blocked agent's title was the bare "Open ws-sage" — the +// pending decision was unreachable for a screen-reader user as well as +// invisible. Asks lead: a decision outranks unread chatter. +export function agentRowTitle({ label, name, unread = 0, askCount = 0, chipTitle = '' } = {}) { + const who = label && label !== name ? `${label} (${name})` : (label || name || '') + const asks = Number(askCount) || 0 + const reads = Number(unread) || 0 + + const parts = [] + if (asks > 0) parts.push(`${asks} ${asks === 1 ? 'ask' : 'asks'} waiting on you`) + if (reads > 0) parts.push(`${reads} unread ${reads === 1 ? 'reply' : 'replies'}`) + + const base = parts.length ? `${who} — ${parts.join(', ')}` : `Open ${who}` + return chipTitle ? `${base} — ${chipTitle}` : base +} + +// #2159 capped the roster at five so a long fleet could not push chats below +// the fold. #2424: the cap is a plain roster-order slice, so on any fleet larger +// than five the agent WAITING ON YOU is as likely as not to be behind the +// toggle — observed with an agent 11th of 12 while the header advertised its +// two asks. +// +// Ask-bearing agents are appended, not floated to the top: re-sorting on a +// transient count moves rows under the cursor between refreshes, which is the +// same reason the roster is not re-sorted by availability. So the first N stay +// exactly where they were and the visible list simply grows. +export const AGENT_COLLAPSE_LIMIT = 5 + +export function visibleAgentRows(roster, { expanded = false, askCounts = {}, limit = AGENT_COLLAPSE_LIMIT } = {}) { + const list = Array.isArray(roster) ? roster : [] + if (expanded) return list + + const head = list.slice(0, limit) + const shown = new Set(head.map((a) => a?.name)) + const counts = askCounts || {} + const waiting = list.filter((a) => a?.name && !shown.has(a.name) && (Number(counts[a.name]) || 0) > 0) + return waiting.length ? [...head, ...waiting] : head +} + export function totalUnread(threads) { return (Array.isArray(threads) ? threads : []) .reduce((sum, t) => sum + (Number(t?.unread) || 0), 0) diff --git a/src/frontend/tests/unit/portalAskDiscoverability.spec.js b/src/frontend/tests/unit/portalAskDiscoverability.spec.js new file mode 100644 index 000000000..20d0ec274 --- /dev/null +++ b/src/frontend/tests/unit/portalAskDiscoverability.spec.js @@ -0,0 +1,180 @@ +/** + * #2424 — the Workspace said "2 asks are waiting" and gave you no way to find them. + * + * Three failures compounded, and they are only worth fixing together: the header + * badge reported the wrong unit, the agent row carried no ask indicator at all, + * and the agent could be collapsed out of the roster entirely. A person saw a + * count, had no row to click, and the agent was not on screen. + * + * Everything decidable lives in `portalUtils` because vitest runs + * `environment: 'node'` with no component-mount harness — a rule that lived + * inside the SFC would be one no test could reach, which is how all three of + * these shipped. + */ +import { describe, it, expect } from 'vitest' +import fs from 'fs' +import path from 'path' +import { + asksByAgent, + askBadgeTitle, + agentRowTitle, + visibleAgentRows, + AGENT_COLLAPSE_LIMIT, +} from '../../src/components/portal/portalUtils.js' + +const SIDEBAR = path.resolve(__dirname, '../../src/components/portal/PortalSidebar.vue') +const sidebarSource = () => fs.readFileSync(SIDEBAR, 'utf8') + +// --------------------------------------------------------------------------- +// 1. The unit the badge reports +// --------------------------------------------------------------------------- +describe('#2424 part 1 — the badge counts asks, so it must say asks', () => { + it('says "asks" for many, never "agents"', () => { + const title = askBadgeTitle(2) + expect(title).toContain('2') + expect(title).toMatch(/asks/i) + // The reported bug verbatim: two asks on ONE agent rendered as + // "2 agents are waiting on your answer". + expect(title).not.toMatch(/agents/i) + }) + + it('is singular for one', () => { + expect(askBadgeTitle(1)).toMatch(/\b1 ask\b/i) + expect(askBadgeTitle(1)).not.toMatch(/asks/i) + }) + + it('says nothing at zero — an empty title is worse than no attribute', () => { + expect(askBadgeTitle(0)).toBe('') + }) + + it('the SFC no longer builds that sentence inline', () => { + // The old bug was a template literal in the template. If it comes back, the + // pure function above stops being the single source of the wording. + expect(sidebarSource()).not.toMatch(/agents are.*waiting on your answer/) + }) +}) + +// --------------------------------------------------------------------------- +// 2. Per-agent counts, and the row that shows them +// --------------------------------------------------------------------------- +describe('#2424 part 2 — asks are attributable to an agent', () => { + it('groups by agent', () => { + expect(asksByAgent([ + { agent_name: 'ws-sage' }, + { agent_name: 'ws-sage' }, + { agent_name: 'scout' }, + ])).toEqual({ 'ws-sage': 2, scout: 1 }) + }) + + it('survives junk without throwing — the sidebar must not blank on a bad row', () => { + expect(asksByAgent(null)).toEqual({}) + expect(asksByAgent(undefined)).toEqual({}) + expect(asksByAgent([null, {}, { agent_name: '' }, { agent_name: 'a' }])).toEqual({ a: 1 }) + }) + + it('the row title names the pending decision', () => { + const t = agentRowTitle({ label: 'ws-sage', name: 'ws-sage', askCount: 2 }) + expect(t).toMatch(/2 asks/i) + // The observed title was the bare "Open ws-sage" while two asks waited. + expect(t).not.toBe('Open ws-sage') + }) + + it('is singular for one ask', () => { + expect(agentRowTitle({ label: 'a', name: 'a', askCount: 1 })).toMatch(/\b1 ask\b/i) + }) + + it('keeps unread replies as a SEPARATE fact, never summed', () => { + const t = agentRowTitle({ label: 'a', name: 'a', askCount: 2, unread: 3 }) + expect(t).toMatch(/2 asks/i) + expect(t).toMatch(/3 unread/i) + // "5" would mean the two counts were added — the exact conflation + // PortalSidebar.vue's own comment forbids. + expect(t).not.toMatch(/\b5\b/) + }) + + it('still renders the display label with its slug, and the availability chip', () => { + const t = agentRowTitle({ + label: 'Sage', name: 'ws-sage', askCount: 0, unread: 0, chipTitle: 'This agent is stopped', + }) + expect(t).toContain('Sage') + expect(t).toContain('ws-sage') + expect(t).toContain('This agent is stopped') + }) + + it('falls back to "Open " when nothing is pending', () => { + expect(agentRowTitle({ label: 'a', name: 'a' })).toBe('Open a') + }) + + it('the row renders an ask badge, tokenised and distinct from the unread pill', () => { + const src = sidebarSource() + expect(src).toMatch(/askCountFor\(/) + // status-urgent is the platform's "waiting on you" token — the same one the + // operator NavBar's pending-operator-queue badge uses. Not amber: that maps + // to `state-autonomous`, which is an operating mode, not a pending decision. + expect(src).toContain('bg-status-urgent-500') + // Raw palette classes are ratcheted to zero for new code (design contract). + expect(src).not.toContain('bg-amber-500') + }) +}) + +// --------------------------------------------------------------------------- +// 3. A blocked agent is never collapsed out of view +// --------------------------------------------------------------------------- +describe('#2424 part 3 — the agent you need is on screen', () => { + const roster = Array.from({ length: 12 }, (_, i) => ({ name: `a${String(i).padStart(2, '0')}` })) + + it('collapsed, shows the first N in roster order when nothing is pending', () => { + const out = visibleAgentRows(roster, { expanded: false, askCounts: {} }) + expect(out).toHaveLength(AGENT_COLLAPSE_LIMIT) + expect(out.map((a) => a.name)).toEqual(['a00', 'a01', 'a02', 'a03', 'a04']) + }) + + it('lifts an ask-bearing agent that would otherwise be hidden', () => { + // The reported case: ws-sage was 11th of 12, so on a fresh load the one row + // the person needed was behind the "show more" toggle. + const out = visibleAgentRows(roster, { expanded: false, askCounts: { a10: 2 } }) + expect(out.map((a) => a.name)).toContain('a10') + }) + + it('keeps roster order rather than floating the ask to the top', () => { + // Re-sorting on a transient count makes rows move under the cursor between + // refreshes — the layout-stability rule the availability chip already obeys. + const out = visibleAgentRows(roster, { expanded: false, askCounts: { a10: 2 } }) + const names = out.map((a) => a.name) + expect(names.indexOf('a10')).toBe(names.length - 1) + expect(names.slice(0, 5)).toEqual(['a00', 'a01', 'a02', 'a03', 'a04']) + }) + + it('does not duplicate an ask-bearing agent already inside the slice', () => { + const out = visibleAgentRows(roster, { expanded: false, askCounts: { a02: 1 } }) + expect(out.filter((a) => a.name === 'a02')).toHaveLength(1) + expect(out).toHaveLength(AGENT_COLLAPSE_LIMIT) + }) + + it('expanded, shows everything', () => { + expect(visibleAgentRows(roster, { expanded: true, askCounts: { a10: 2 } })).toHaveLength(12) + }) + + it('short rosters are untouched', () => { + const three = roster.slice(0, 3) + expect(visibleAgentRows(three, { expanded: false, askCounts: {} })).toHaveLength(3) + }) + + it('tolerates a missing roster', () => { + expect(visibleAgentRows(null, { expanded: false, askCounts: {} })).toEqual([]) + expect(visibleAgentRows(roster, {})).toHaveLength(AGENT_COLLAPSE_LIMIT) + }) + + it('the SFC drives its rows through the shared rule', () => { + const src = sidebarSource() + expect(src).toMatch(/visibleAgentRows\(/) + // The raw slice was the bug; it must not survive alongside the fix. + expect(src).not.toMatch(/roster\.slice\(0,\s*AGENT_COLLAPSE_LIMIT\)/) + }) + + it('the "show more" affordance still keys off the full roster', () => { + // With ask-lifting, the visible count can exceed the limit — so a toggle + // gated on `shown.length` would vanish exactly when an ask is pending. + expect(sidebarSource()).toMatch(/roster\.length > AGENT_COLLAPSE_LIMIT/) + }) +}) diff --git a/src/frontend/tests/unit/portalAvailabilityChip.spec.js b/src/frontend/tests/unit/portalAvailabilityChip.spec.js index c1c8dbf87..8ef901467 100644 --- a/src/frontend/tests/unit/portalAvailabilityChip.spec.js +++ b/src/frontend/tests/unit/portalAvailabilityChip.spec.js @@ -17,6 +17,7 @@ * helper, and that nothing became disabled. */ import { describe, it, expect } from 'vitest' +import { agentRowTitle } from '../../src/components/portal/portalUtils.js' import { readFileSync } from 'fs' import { fileURLToPath } from 'url' @@ -164,6 +165,15 @@ describe('#2196 the surfaces consume the shared rule', () => { }) it('the row title carries the state, so it is reachable without colour', () => { - expect(SIDEBAR).toMatch(/chip \? `\$\{base\} — \$\{chip\.title\}` : base/) + // #2424 moved the composition into portalUtils::agentRowTitle, so this + // asserts the property instead of the old inline ternary. Stronger: it now + // catches a chip title that is dropped as well as one that is reworded. + const withChip = agentRowTitle({ + label: 'a', name: 'a', chipTitle: 'This agent is stopped — ask admin to start it.', + }) + expect(withChip).toContain('This agent is stopped') + expect(agentRowTitle({ label: 'a', name: 'a' })).not.toMatch(/—/) + // #2424 additionally requires a pending ask to be reachable the same way. + expect(agentRowTitle({ label: 'a', name: 'a', askCount: 2 })).toMatch(/2 asks/i) }) }) diff --git a/src/frontend/tests/unit/portalRosterRow.spec.js b/src/frontend/tests/unit/portalRosterRow.spec.js index 73463fc12..4e08943b5 100644 --- a/src/frontend/tests/unit/portalRosterRow.spec.js +++ b/src/frontend/tests/unit/portalRosterRow.spec.js @@ -13,6 +13,7 @@ * agent would render as blank. */ import { describe, it, expect } from 'vitest' +import { visibleAgentRows, AGENT_COLLAPSE_LIMIT } from '../../src/components/portal/portalUtils.js' import { readFileSync } from 'fs' import { fileURLToPath } from 'url' @@ -69,8 +70,16 @@ describe('#2159 the row no longer shows the description', () => { describe('#2159 the roster is bounded and expandable', () => { it('shows a fixed number by default rather than the whole fleet', () => { - expect(source).toMatch(/const AGENT_COLLAPSE_LIMIT = \d+/) - expect(source).toMatch(/agentsExpanded\.value \? props\.roster : props\.roster\.slice\(0, AGENT_COLLAPSE_LIMIT\)/) + // #2424 moved the rule into portalUtils, so this asserts the PROPERTY + // rather than the old inline slice expression. Same guarantee, and now it + // fails on a broken bound instead of only on a reworded one. + const roster = Array.from({ length: 12 }, (_, i) => ({ name: `a${i}` })) + const collapsed = visibleAgentRows(roster, { expanded: false, askCounts: {} }) + expect(collapsed).toHaveLength(AGENT_COLLAPSE_LIMIT) + expect(AGENT_COLLAPSE_LIMIT).toBeLessThan(roster.length) + expect(visibleAgentRows(roster, { expanded: true, askCounts: {} })).toHaveLength(12) + // ...and the component still routes its rows through it. + expect(source).toMatch(/visibleAgentRows\(props\.roster/) }) it('uses ONE persistent toggle, not two v-if-alternated buttons', () => {