From 7e7d6e8efa59ec89fbdf1caf181915fdf4c4a937 Mon Sep 17 00:00:00 2001 From: xarmian Date: Mon, 24 Aug 2026 16:15:44 +0000 Subject: [PATCH 01/17] feat(web): render agents' stamped names wherever agent actors display (TASK-2759) The input half has existed since BUG-2542: the CLI resolves an agent name (.pad.toml agent_name -> $PAD_AGENT -> detected runtime) and sends it as X-Pad-Agent, and the server stamps it into activity metadata as `agent`. Nothing rendered it. Every agent write displayed as an undifferentiated "agent", and on the console audit log it displayed under the name of the HUMAN whose credentials the write rode on. Recon's discriminator: metadata.agent is stamped only by agentMeta(), reached only from logActivityWithMetaReturningID, so workspace Activity rows are the only carrier in the data model. Comments, versions, items, structured note/decision entries and SSE events record the actor KIND and no name. That makes render-vs-exempt mechanical rather than per-surface judgement: does this surface hold an Activity? Rendering (5 sites, each already holding the metadata): - the activity page's Live view fold (activityEpisodes.ts) - the activity page's Audit rows (getSourceLabel) - the dashboard's Recent Activity rows - TimelineActivityCard on the item timeline - the console audit log's user column Exempt, name absent from the payload: comment authorship (TASK-2760 files the server half), version cards, structured note/decision cards, the SSE toast, ItemDetail's "Created by", and the console UserActivityTab (its row type omits metadata). Retires the GENERIC_AGENT_IDS shim on its own stated retirement condition (CONVE-2757 rule 4, PR #1192): it filtered a hardcoded list of one team's client ids out of the Live view, which made display quality depend on that team's naming habits. Names now render verbatim -- no allow-list, no normalization, no title-casing; any transform is a doorway for a workspace's vocabulary to re-enter product logic. Historical claude-code rows render as claude-code, which is honest: a reader learns every write came from one undifferentiated client, which the filter concealed. The audit log renders both facts rather than replacing one with the other ("wren (via Dave)") -- the agent acted, and that account is who it acted as, and an ops surface needs both. The shim's test inverted with it, and the file's header doc asserted that "every current seat sends the generic client id claude-code" -- falsified by this change, so rewritten rather than left (CONVE-23). Claude-Session: https://claude.ai/code/session_01Cpr3teiHHsgcTmg2xhHA86 --- .../timeline/TimelineActivityCard.svelte | 7 +- web/src/lib/utils/activityEpisodes.test.ts | 26 ++++++- web/src/lib/utils/activityEpisodes.ts | 36 +++------ web/src/lib/utils/agentActor.ts | 75 +++++++++++++++++++ .../[username]/[workspace]/+page.svelte | 3 +- .../[workspace]/activity/+page.svelte | 16 +++- .../console/admin/audit-log/+page.svelte | 11 +++ 7 files changed, 141 insertions(+), 33 deletions(-) create mode 100644 web/src/lib/utils/agentActor.ts diff --git a/web/src/lib/components/timeline/TimelineActivityCard.svelte b/web/src/lib/components/timeline/TimelineActivityCard.svelte index ef4b6907..7263d181 100644 --- a/web/src/lib/components/timeline/TimelineActivityCard.svelte +++ b/web/src/lib/components/timeline/TimelineActivityCard.svelte @@ -2,6 +2,7 @@ import type { Activity } from '$lib/types'; import { relativeTime } from '$lib/utils/markdown'; import { parseFieldChanges } from '$lib/utils/activityChanges'; + import { agentNameOf } from '$lib/utils/agentActor'; import Chip from '$lib/components/common/Chip.svelte'; let { activity }: { activity: Activity } = $props(); @@ -37,7 +38,11 @@ } function getActorLabel(a: Activity): string { - return a.actor === 'agent' ? 'Agent' : 'User'; + if (a.actor !== 'agent') return 'User'; + // The agent's own name when it sent one. `actor_name` below stays the + // human whose credentials the write rode on — the two are rendered + // side by side rather than merged, because they are different facts. + return agentNameOf(metadata) ?? 'Agent'; } function getSourceLabel(source: string): string { diff --git a/web/src/lib/utils/activityEpisodes.test.ts b/web/src/lib/utils/activityEpisodes.test.ts index 3ff66545..c677f4f0 100644 --- a/web/src/lib/utils/activityEpisodes.test.ts +++ b/web/src/lib/utils/activityEpisodes.test.ts @@ -83,8 +83,32 @@ describe('foldEpisodes', () => { expect(eps[0].itemTitle).toBe('One'); }); - it('never shows a generic client id as a seat name', () => { + // TASK-2759 inverted this case. It used to assert that `claude-code` was + // filtered back to the generic "agent" label, by a hardcoded list of one + // team's client ids that this unit deleted (CONVE-2757: Pad does not grade + // which names are "real"). A generic-looking id is still a name an agent + // chose to send, and reporting it at the resolution it arrived is the + // point — a reader learns "every write here is one undifferentiated + // client", which the old label actively concealed. + it('renders a generic-looking client id verbatim rather than filtering it', () => { const eps = foldEpisodes([act(1, { metadata: '{"agent":"claude-code"}' })], { now }); + expect(eps[0].actorLabel).toBe('claude-code'); + // The fold key follows the label, so a filtered id would also have + // collapsed distinct clients into one episode — assert the key, since + // that consequence outlives any change to the label's wording. + expect(eps[0].key.startsWith('agent:claude-code|')).toBe(true); + }); + + // Each stamp shape gets its own fold call: within one call these rows would + // merge into a single episode (same actor, same item, inside the gap), and + // the merged label would then prove nothing about the rows behind it. + it.each([ + ['no agent key', '{}'], + ['an empty name', '{"agent":""}'], + ['a non-string name', '{"agent":123}'], + ['unparseable metadata', 'not json'] + ])('falls back to the generic label for %s', (_case, metadata) => { + const eps = foldEpisodes([act(1, { metadata })], { now }); expect(eps[0].actorLabel).toBe('agent'); }); diff --git a/web/src/lib/utils/activityEpisodes.ts b/web/src/lib/utils/activityEpisodes.ts index c903d219..061d3d1c 100644 --- a/web/src/lib/utils/activityEpisodes.ts +++ b/web/src/lib/utils/activityEpisodes.ts @@ -1,4 +1,5 @@ import type { Activity } from '$lib/types'; +import { agentActorLabel } from '$lib/utils/agentActor'; /** * The episode fold behind the activity page's Live view (IDEA-2755). @@ -11,12 +12,12 @@ import type { Activity } from '$lib/types'; * * Identity today is (actor kind + name, item). The server stamps * `metadata.agent` from the client's X-Pad-Agent header - * (handlers_documents.go auditMeta) — every current seat sends the generic - * client id "claude-code", so agent rows collapse to one "agent" label and - * seats are distinguishable only by the disjoint items they work, which - * multi-seat turf discipline already guarantees. The moment a seat sends - * its own name in X-Pad-Agent, its label (and fold key) lights up here with - * no further change — concept B's lanes want exactly that. + * (handlers_documents.go agentMeta), and this fold renders whatever is + * there verbatim — see `agentActor.ts` for where the name comes from and + * what it does not claim. An agent that names itself gets its own label + * and its own fold key; one that sends a generic client id gets that id; + * one that sends nothing collapses into the shared "agent" label, and its + * writes are then distinguishable only by the items they touch. * * LIVENESS is claimed only from evidence: an episode is `live` when its * newest event is younger than `liveMinutes`. The view never says "active @@ -31,7 +32,7 @@ export interface Episode { itemTitle?: string; itemSlug?: string; collectionSlug?: string; - /** Display label for the actor ("Dave", "agent", a seat name when stamped). */ + /** Display label for the actor ("Dave", an agent's stamped name, or "agent"). */ actorLabel: string; /** "agent" | "user" — drives the row's border treatment like the audit view. */ actorKind: string; @@ -58,28 +59,9 @@ export interface FoldOptions { now?: () => number; } -/** Client ids that are tools, not seats — never shown as an actor name. - * SHIM with a retirement condition (CONVE-2757 rule 4): this list encodes - * one team's tooling and dies with IDEA-2750 part 1, when agents carry - * real display names via PAD_AGENT_NAME and the platform renders - * metadata.agent verbatim. */ -const GENERIC_AGENT_IDS = new Set(['claude-code', 'cli', 'agent']); - -function metaAgentName(metadata: string): string | undefined { - try { - const meta = JSON.parse(metadata) as Record; - const name = meta.agent; - if (typeof name !== 'string' || name.length === 0) return undefined; - return GENERIC_AGENT_IDS.has(name) ? undefined : name; - } catch { - return undefined; - } -} - function actorKeyOf(a: Activity): { key: string; label: string; kind: string } { - const seat = metaAgentName(a.metadata); if (a.actor === 'agent') { - const label = seat ?? 'agent'; + const label = agentActorLabel(a.metadata, 'agent'); return { key: `agent:${label}`, label, kind: 'agent' }; } const label = a.actor_name ?? (a.source === 'cli' ? 'cli' : 'web'); diff --git a/web/src/lib/utils/agentActor.ts b/web/src/lib/utils/agentActor.ts new file mode 100644 index 00000000..a201fe99 --- /dev/null +++ b/web/src/lib/utils/agentActor.ts @@ -0,0 +1,75 @@ +/** + * The agent display name carried on a workspace activity row. + * + * WHERE THE NAME COMES FROM. The CLI resolves it once per process + * (`internal/cli/agent_identity.go::ResolveAgentName`: `agent_name` in + * .pad.toml, then $PAD_AGENT, then a detected runtime) and sends it as + * `X-Pad-Agent`. The server stamps it into the activity's metadata as + * `agent` (`handlers_documents.go::agentMeta`), reached from + * `logActivityWithMetaReturningID` — so **workspace activity rows are the + * only thing in the data model that carries it**. Comments, versions, + * items, structured note/decision entries and SSE events all record the + * actor KIND ("agent" / "user") and no name; a surface holding one of + * those has nothing to render here and says "agent" as it always did. + * + * VERBATIM IS THE WHOLE CONTRACT. Whatever an agent sent is what a reader + * sees — no allow-list, no normalization, no title-casing. Pad does not + * know which client ids are "real names": a run of seats calling + * themselves `claude-code` and a run calling themselves `wren` are the + * same fact reported at different resolutions, and the display's job is to + * report it, not to grade it. This replaces a shim that filtered a + * hardcoded set of one team's client ids out of the Live view, which made + * the feature's display quality depend on that team's naming habits + * (CONVE-2757: no workspace's tool names in product logic, display filters + * included). Historical rows stamped `claude-code` render `claude-code`. + * + * WHAT IT DOES NOT CLAIM. The header is client-supplied and self-declared, + * so a name here records honesty, not identity — see the "WHAT THIS IS + * NOT" section of ResolveAgentName's doc comment for the two ways it can + * be wrong. Nothing rendered from this function is evidence about who + * acted; it is a label the actor chose. + */ + +/** + * The agent name stamped on an activity's metadata, or undefined when the + * row carries none (a human's write, a pre-BUG-2542 row, an agent that + * never sent the header) or the metadata is unparseable. + * + * `metadata` is the raw JSON string as the API returns it — callers that + * have already parsed it for other fields can use {@link agentNameOf} + * instead of parsing twice. + */ +export function agentNameFromMetadata(metadata: string | undefined | null): string | undefined { + if (!metadata) return undefined; + try { + return agentNameOf(JSON.parse(metadata) as Record); + } catch { + return undefined; + } +} + +/** {@link agentNameFromMetadata} for callers holding the parsed object. */ +export function agentNameOf(metadata: Record | undefined | null): string | undefined { + const name = metadata?.agent; + // A non-string cannot occur through agentMeta (it marshals map[string]string), + // but this parses untrusted JSON off the wire, so it is checked rather than + // assumed — an object here would otherwise render as "[object Object]". + if (typeof name !== 'string' || name.length === 0) return undefined; + return name; +} + +/** + * The label for an agent actor: its stamped name, or `fallback` when the + * row carries none. + * + * The fallback stays the caller's, because each surface already has its + * own vocabulary for the nameless case ("agent" in the feed's lowercase + * badges, "Agent" in the timeline's chips) and this change is not the + * place to unify them. + */ +export function agentActorLabel( + metadata: string | undefined | null, + fallback: string +): string { + return agentNameFromMetadata(metadata) ?? fallback; +} diff --git a/web/src/routes/[username]/[workspace]/+page.svelte b/web/src/routes/[username]/[workspace]/+page.svelte index 1f1184d4..e2f17e13 100644 --- a/web/src/routes/[username]/[workspace]/+page.svelte +++ b/web/src/routes/[username]/[workspace]/+page.svelte @@ -9,6 +9,7 @@ import { uiStore } from '$lib/stores/ui.svelte'; import { syncService } from '$lib/services/sync.svelte'; import { relativeTime } from '$lib/utils/markdown'; + import { agentActorLabel } from '$lib/utils/agentActor'; import OnboardingLaunchpad from '$lib/components/OnboardingLaunchpad.svelte'; import ConnectWorkspaceModal from '$lib/components/ConnectWorkspaceModal.svelte'; import Button from '$lib/components/common/Button.svelte'; @@ -632,7 +633,7 @@ {@const changes = parseActivityChanges(activity.metadata)}
{#if activity.actor === 'agent'} - agent + {agentActorLabel(activity.metadata, 'agent')} {:else if activity.actor_name} {activity.actor_name} {:else if activity.source === 'cli'} diff --git a/web/src/routes/[username]/[workspace]/activity/+page.svelte b/web/src/routes/[username]/[workspace]/activity/+page.svelte index 94228d6a..405bf284 100644 --- a/web/src/routes/[username]/[workspace]/activity/+page.svelte +++ b/web/src/routes/[username]/[workspace]/activity/+page.svelte @@ -7,6 +7,7 @@ import { titleStore } from '$lib/stores/title.svelte'; import { relativeTime } from '$lib/utils/markdown'; import { parseFieldChanges } from '$lib/utils/activityChanges'; + import { agentNameOf } from '$lib/utils/agentActor'; import { createScrollRestoration } from '$lib/scroll/restore.svelte'; import PageHeader from '$lib/components/common/PageHeader.svelte'; import EmptyState from '$lib/components/common/EmptyState.svelte'; @@ -240,8 +241,17 @@ } } - function getSourceLabel(source: string, actor: string, actorName?: string): { label: string; kind: string } { - if (actor === 'agent') return { label: 'agent', kind: 'agent' }; + function getSourceLabel( + source: string, + actor: string, + actorName?: string, + metadata?: Record + ): { label: string; kind: string } { + // An agent's own name when it sent one, else the generic badge. The + // name is never merged with `actorName` — that is the human whose + // credentials the write rode on, and conflating the two is the + // mis-attribution this renders to end. + if (actor === 'agent') return { label: agentNameOf(metadata) ?? 'agent', kind: 'agent' }; if (actorName) return { label: actorName, kind: source === 'cli' ? 'cli' : 'user' }; if (source === 'cli') return { label: 'cli', kind: 'cli' }; return { label: 'web', kind: 'web' }; @@ -370,7 +380,7 @@ {@const itemRef = activity.item_ref || meta.item_ref} {@const collSlug = activity.collection_slug || meta.collection_slug} {@const fieldChanges = parseFieldChanges(meta.changes)} - {@const src = getSourceLabel(activity.source, activity.actor, activity.actor_name)} + {@const src = getSourceLabel(activity.source, activity.actor, activity.actor_name, meta)}
import { onMount } from 'svelte'; import { adminFetch } from '$lib/stores/admin.svelte'; + import { agentNameFromMetadata } from '$lib/utils/agentActor'; import Chip from '$lib/components/common/Chip.svelte'; import EmptyState from '$lib/components/common/EmptyState.svelte'; @@ -162,7 +163,17 @@ } } + // An agent's write is authenticated with a HUMAN's credentials, so + // `actor_name` — joined from user_id — used to be the only thing this + // column showed for it: the agent's work rendered under the name of the + // person whose token it borrowed. Both facts are real and an audit + // surface needs both, so they render together rather than one replacing + // the other: the agent acted, that account is who it acted as. The name + // is self-declared (see agentActor.ts) and this column is the last place + // that should be implied otherwise, hence "via" rather than a merge. function displayUser(entry: Activity): string { + const agent = entry.actor === 'agent' ? agentNameFromMetadata(entry.metadata) : undefined; + if (agent) return entry.actor_name ? `${agent} (via ${entry.actor_name})` : agent; if (entry.actor_name) return entry.actor_name; if (entry.actor === 'system') return 'System'; if (entry.user_id) return entry.user_id.length > 12 ? entry.user_id.slice(0, 12) + '\u2026' : entry.user_id; From 07192869105406a4f19068d726f0de08cbad0afb Mon Sep 17 00:00:00 2001 From: xarmian Date: Mon, 24 Aug 2026 16:24:14 +0000 Subject: [PATCH 02/17] test(web): assert the agent-name binding from each consuming surface (TASK-2759) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five render sites, five consuming-side assertions (CONVE-19). The helper has its own unit tests, and a correct helper that a page never calls — or calls with the wrong argument — passes every one of them; the Audit view's defect was exactly that shape, with the metadata parsed three lines above the call that ignored it. Each file's load-bearing legs are the negative ones: the generic label a pre-fix build produced is asserted absent where a name is stamped, and asserted present for every stamp shape that carries no name (missing key, empty string, non-string, unparseable). Two also pin that a non-agent row never reads the stamp, since the metadata blob is shared and agentMeta merges into it by string splice. activityEpisodes.test.ts's shim case inverted with the shim it named. Claude-Session: https://claude.ai/code/session_01Cpr3teiHHsgcTmg2xhHA86 --- .../activity/EpisodeFeed.svelte.test.ts | 32 ++++ .../timelineActivityAgentName.svelte.test.ts | 92 +++++++++++ web/src/lib/utils/agentActor.test.ts | 79 ++++++++++ .../activityPageAgentName.svelte.test.ts | 147 ++++++++++++++++++ .../admin/auditLogAgentName.svelte.test.ts | 142 +++++++++++++++++ ...workspaceDashboardAgentName.svelte.test.ts | 145 +++++++++++++++++ 6 files changed, 637 insertions(+) create mode 100644 web/src/lib/components/timeline/timelineActivityAgentName.svelte.test.ts create mode 100644 web/src/lib/utils/agentActor.test.ts create mode 100644 web/src/routes/activityPageAgentName.svelte.test.ts create mode 100644 web/src/routes/console/admin/auditLogAgentName.svelte.test.ts create mode 100644 web/src/routes/workspaceDashboardAgentName.svelte.test.ts diff --git a/web/src/lib/components/activity/EpisodeFeed.svelte.test.ts b/web/src/lib/components/activity/EpisodeFeed.svelte.test.ts index a6ad6014..ccd73d0b 100644 --- a/web/src/lib/components/activity/EpisodeFeed.svelte.test.ts +++ b/web/src/lib/components/activity/EpisodeFeed.svelte.test.ts @@ -154,4 +154,36 @@ describe('EpisodeFeed', () => { expect(checkpoint!.textContent).toContain('latest: Checkpoint: wiring the fold'); expect(checkpoint!.textContent).not.toContain('second line stays hidden'); }); + + // TASK-2759. foldEpisodes computes the label; this asserts the FEED renders + // it (CONVE-19 — a correct fold the card never reads would pass every test + // in activityEpisodes.test.ts). Two named agents on separate items also + // prove the fold key follows the name: a filtered or blanked label would + // merge them into one card, so the count is the counterfactual. + it('renders each agent under its own stamped name', async () => { + const onOtherItem = { + document_id: 'item-other', + item_ref: 'BUG-7', + item_title: 'Other thing', + item_slug: 'other-thing', + collection_slug: 'bugs', + }; + mountFeed([ + act(1, { metadata: '{"agent":"wren"}' }), + act(2, { metadata: '{"agent":"rook"}', ...onOtherItem }), + ]); + await settle(); + + const labels = [...host.querySelectorAll('.ep-actor')].map((el) => el.textContent); + expect(labels.sort()).toEqual(['rook', 'wren']); + }); + + it('renders the generic label for an agent that stamped no name', async () => { + // `act`'s default metadata is '{}' — the pre-BUG-2542 shape, and the + // shape any agent that never sends the header still produces. + mountFeed([act(1)]); + await settle(); + + expect(host.querySelector('.ep-actor')!.textContent).toBe('agent'); + }); }); diff --git a/web/src/lib/components/timeline/timelineActivityAgentName.svelte.test.ts b/web/src/lib/components/timeline/timelineActivityAgentName.svelte.test.ts new file mode 100644 index 00000000..9f09f91a --- /dev/null +++ b/web/src/lib/components/timeline/timelineActivityAgentName.svelte.test.ts @@ -0,0 +1,92 @@ +import { describe, it, expect } from 'vitest'; +import { render } from '@testing-library/svelte'; +import TimelineActivityCard from './TimelineActivityCard.svelte'; +import type { Activity } from '$lib/types'; + +/** + * TASK-2759. The card's actor chip said "Agent" for every agent write, so a + * timeline could not tell you WHICH agent touched the item — the name has + * been stamped on the activity's metadata since BUG-2542 and nothing read it. + * + * These assert the BINDING, not the helper (CONVE-19): `agentActor.ts` has + * its own unit tests, and a correct helper that nothing calls here would pass + * every one of them. So each case checks what the CARD renders, and the + * load-bearing legs are the negative ones — a chip hardcoded back to "Agent" + * still satisfies a name-is-somewhere assertion if the name also appears in + * `actor_name`, which is why the two are asserted apart. + */ +function activity(overrides: Partial = {}): Activity { + return { + id: 'act-1', + workspace_id: 'ws-1', + document_id: 'item-1', + action: 'updated', + actor: 'agent', + source: 'cli', + metadata: JSON.stringify({ agent: 'wren' }), + created_at: new Date('2026-08-24T12:00:00Z').toISOString(), + ...overrides + } as Activity; +} + +describe('TimelineActivityCard agent name', () => { + it('renders the stamped name in place of the generic chip', () => { + const { getByText, queryByText } = render(TimelineActivityCard, { + activity: activity() + }); + + expect(getByText('wren')).toBeTruthy(); + // The counterfactual: the pre-TASK-2759 card rendered this, and would + // still render it if the chip were rebuilt without reading metadata. + expect(queryByText('Agent')).toBeNull(); + }); + + it('renders a generic-looking client id verbatim', () => { + // The retired GENERIC_AGENT_IDS shim swallowed exactly this value. A + // reader seeing `claude-code` learns the writes came from one + // undifferentiated client — a fact the filter hid behind "Agent". + const { getByText, queryByText } = render(TimelineActivityCard, { + activity: activity({ metadata: JSON.stringify({ agent: 'claude-code' }) }) + }); + + expect(getByText('claude-code')).toBeTruthy(); + expect(queryByText('Agent')).toBeNull(); + }); + + it('keeps the agent name and the human account separate', () => { + // An agent write authenticates with a person's credentials, so + // `actor_name` is that person. Both render; neither replaces nor + // absorbs the other, because they are different facts. + const { getByText } = render(TimelineActivityCard, { + activity: activity({ actor_name: 'Dave' }) + }); + + expect(getByText('wren')).toBeTruthy(); + expect(getByText('Dave')).toBeTruthy(); + }); + + it.each([ + ['no agent key', JSON.stringify({ changes: 'status: open → done' })], + ['an empty name', JSON.stringify({ agent: '' })], + ['a non-string name', JSON.stringify({ agent: 123 })], + ['unparseable metadata', 'not json'] + ])('falls back to the generic chip given %s', (_case, metadata) => { + const { getByText } = render(TimelineActivityCard, { + activity: activity({ metadata }) + }); + + expect(getByText('Agent')).toBeTruthy(); + }); + + it('never reads the stamp for a non-agent actor', () => { + // Guards against keying the chip on metadata alone. A human's write can + // carry an `agent` key — the merge in agentMeta is textual and this + // blob is shared — and it is not a claim that an agent acted. + const { getByText, queryByText } = render(TimelineActivityCard, { + activity: activity({ actor: 'user', metadata: JSON.stringify({ agent: 'wren' }) }) + }); + + expect(getByText('User')).toBeTruthy(); + expect(queryByText('wren')).toBeNull(); + }); +}); diff --git a/web/src/lib/utils/agentActor.test.ts b/web/src/lib/utils/agentActor.test.ts new file mode 100644 index 00000000..e0fb5105 --- /dev/null +++ b/web/src/lib/utils/agentActor.test.ts @@ -0,0 +1,79 @@ +import { describe, it, expect } from 'vitest'; +import { agentActorLabel, agentNameFromMetadata, agentNameOf } from './agentActor'; + +/** + * The helper's whole contract is "verbatim, or nothing" — so the cases that + * matter are the ones a filter or a normalizer would change. TASK-2759 + * replaced exactly such a filter (GENERIC_AGENT_IDS), and these assert the + * shapes it used to swallow. + */ +describe('agentNameFromMetadata', () => { + it('returns the stamped name verbatim', () => { + expect(agentNameFromMetadata('{"agent":"wren"}')).toBe('wren'); + }); + + it.each([ + // The three ids the retired shim filtered. They are here as named + // cases, not as a list to re-suppress: whichever way this function + // changes, someone should have to look at these on purpose. + 'claude-code', + 'cli', + 'agent', + // Shapes a normalizer would mangle: casing, inner spacing, unicode. + 'Claude-Code', + 'my agent', + 'wren-2', + 'ロボット' + ])('does not filter or transform %s', (name) => { + expect(agentNameFromMetadata(JSON.stringify({ agent: name }))).toBe(name); + }); + + it('preserves surrounding whitespace rather than trimming it', () => { + // Trimming is a normalization, and the only writer (the CLI's + // X-Pad-Agent header) is already OWS-stripped by Go's header parser + // before the server stamps it — so a trim here would only ever act on + // a value some other client deliberately sent. + expect(agentNameFromMetadata('{"agent":" wren "}')).toBe(' wren '); + }); + + it.each([ + ['no agent key', '{"changes":"status: open -> done"}'], + ['an empty name', '{"agent":""}'], + ['a null name', '{"agent":null}'], + ['a numeric name', '{"agent":123}'], + ['an object name', '{"agent":{"name":"wren"}}'], + ['unparseable metadata', 'not json'], + ['an empty string', ''], + ['undefined', undefined], + ['null', null] + ])('returns undefined for %s', (_case, metadata) => { + expect(agentNameFromMetadata(metadata as string | undefined | null)).toBeUndefined(); + }); + + it('reads the same name as the parsed-object form', () => { + const raw = '{"agent":"rook","changes":"status"}'; + expect(agentNameFromMetadata(raw)).toBe(agentNameOf(JSON.parse(raw))); + }); + + it('takes the last value when the stamp was spliced onto existing metadata', () => { + // agentMeta merges by string splice (handlers_documents.go:287), so a + // row whose metadata already carried an `agent` key arrives with a + // duplicate. JSON.parse resolves that last-wins; this pins the + // behaviour so a future switch to a hand-rolled parser cannot change + // which name a reader sees without a test failing. + expect(agentNameFromMetadata('{"agent":"spliced","agent":"original"}')).toBe('original'); + }); +}); + +describe('agentActorLabel', () => { + it('returns the stamped name when there is one', () => { + expect(agentActorLabel('{"agent":"wren"}', 'agent')).toBe('wren'); + }); + + it("returns the caller's own fallback when there is not", () => { + // The fallback is deliberately not shared: surfaces disagree on its + // casing ("agent" in the feed's badges, "Agent" in timeline chips). + expect(agentActorLabel('{}', 'agent')).toBe('agent'); + expect(agentActorLabel('{}', 'Agent')).toBe('Agent'); + }); +}); diff --git a/web/src/routes/activityPageAgentName.svelte.test.ts b/web/src/routes/activityPageAgentName.svelte.test.ts new file mode 100644 index 00000000..97b1892a --- /dev/null +++ b/web/src/routes/activityPageAgentName.svelte.test.ts @@ -0,0 +1,147 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { flushSync, mount, unmount, tick } from 'svelte'; +import { page } from '$app/state'; +import type { Activity } from '$lib/types'; + +/** + * TASK-2759, the activity page's two views. + * + * `getSourceLabel` returned a hardcoded 'agent' for every agent row, so the + * Audit view could not say which agent wrote. The Live view had the name but + * filtered a hardcoded set of client ids out of it (the retired + * GENERIC_AGENT_IDS shim). + * + * Both views are asserted through the PAGE (CONVE-19): the fold and the + * helper have their own tests, and either could be correct while this page + * passed the wrong arguments — which is the defect the Audit half actually + * was, since the metadata was parsed three lines above the call that ignored + * it. + */ +// `api.activity.list` resolves to a bare Activity[] — the page reads +// `result.length` and assigns it straight to state, so an envelope-shaped +// mock makes the page render empty instead of failing loudly. +const listActivity = vi.hoisted(() => + vi.fn<(slug: string, params: unknown) => Promise>() +); + +vi.mock('$lib/api/client', () => ({ + api: { + collections: { list: vi.fn().mockResolvedValue([]) }, + activity: { list: (slug: string, params: unknown) => listActivity(slug, params) }, + comments: { list: vi.fn().mockResolvedValue([]) } + }, + PadApiError: class extends Error {} +})); + +const { default: ActivityPage } = await import('./[username]/[workspace]/activity/+page.svelte'); + +let seq = 0; +function act(over: Partial = {}): Activity { + seq += 1; + return { + id: `a${seq}`, + workspace_id: 'ws', + action: 'updated', + actor: 'agent', + source: 'cli', + metadata: JSON.stringify({ agent: 'wren' }), + created_at: new Date().toISOString(), + document_id: 'item-1', + item_ref: 'TASK-1', + item_title: 'A thing', + item_slug: 'a-thing', + collection_slug: 'tasks', + ...over + } as Activity; +} + +let host: HTMLElement; +let app: Record | null = null; + +/** Mount the page in `view`, with `rows` as the feed. The view is read from + * localStorage in onMount (it cannot be a prop — SSR has no localStorage, + * so the component always starts on 'live' and restores after hydration). */ +async function mountPage(view: 'live' | 'audit', rows: Activity[]): Promise { + localStorage.setItem('pad-activity-view', view); + listActivity.mockResolvedValue(rows); + app = mount(ActivityPage, { target: host, props: {} }) as Record; + flushSync(); + for (let i = 0; i < 4; i++) { + await Promise.resolve(); + await tick(); + } + flushSync(); +} + +beforeEach(() => { + host = document.createElement('div'); + document.body.appendChild(host); + // The page derives its workspace from the route; the $app/state mock ships + // empty params, and an empty slug short-circuits the load effect entirely + // (every assertion here would then pass or fail on an empty page). + page.params.workspace = 'ws'; + page.params.username = 'alice'; + localStorage.clear(); + listActivity.mockReset(); +}); + +afterEach(() => { + if (app) unmount(app as never); + app = null; + host.remove(); + localStorage.clear(); +}); + +describe('activity page — Audit view', () => { + it('badges an agent row with the stamped name', async () => { + await mountPage('audit', [act()]); + + const badge = host.querySelector('.actor-badge.agent'); + expect(badge).not.toBeNull(); + expect(badge!.textContent!.trim()).toBe('wren'); + }); + + it('badges a generic-looking client id verbatim', async () => { + await mountPage('audit', [act({ metadata: JSON.stringify({ agent: 'claude-code' }) })]); + + expect(host.querySelector('.actor-badge.agent')!.textContent!.trim()).toBe('claude-code'); + }); + + it.each([ + ['no agent key', JSON.stringify({ changes: 'status' })], + ['an empty name', JSON.stringify({ agent: '' })], + ['unparseable metadata', 'not json'] + ])('falls back to the generic badge given %s', async (_case, metadata) => { + await mountPage('audit', [act({ metadata })]); + + expect(host.querySelector('.actor-badge.agent')!.textContent!.trim()).toBe('agent'); + }); + + it('never reads the stamp for a human row', async () => { + // The badge for a human is keyed on actor_name/source, and an `agent` + // key can ride along on any row's shared metadata blob. + await mountPage('audit', [ + act({ actor: 'user', actor_name: 'Dave', source: 'web' }) + ]); + + expect(host.querySelector('.actor-badge.agent')).toBeNull(); + expect(host.textContent).toContain('Dave'); + expect(host.textContent).not.toContain('wren'); + }); +}); + +describe('activity page — Live view', () => { + it('labels an episode with the stamped name', async () => { + await mountPage('live', [act()]); + + expect(host.querySelector('.ep-actor')!.textContent!.trim()).toBe('wren'); + }); + + it('labels a generic-looking client id verbatim', async () => { + // The counterfactual for the retired shim: it turned exactly this + // value back into 'agent', collapsing every such client into one card. + await mountPage('live', [act({ metadata: JSON.stringify({ agent: 'claude-code' }) })]); + + expect(host.querySelector('.ep-actor')!.textContent!.trim()).toBe('claude-code'); + }); +}); diff --git a/web/src/routes/console/admin/auditLogAgentName.svelte.test.ts b/web/src/routes/console/admin/auditLogAgentName.svelte.test.ts new file mode 100644 index 00000000..1a7d724f --- /dev/null +++ b/web/src/routes/console/admin/auditLogAgentName.svelte.test.ts @@ -0,0 +1,142 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { flushSync, mount, unmount, tick } from 'svelte'; + +/** + * TASK-2759, the worst mis-attribution the surface enumeration found. + * + * An agent authenticates with a person's credentials, so this column's + * `actor_name` — joined from `user_id` — is that person. It was checked + * FIRST, so an agent's write rendered under a human's name on the one + * surface that exists to answer "who did this", with nothing anywhere in the + * row to say an agent was involved. + * + * The fix renders both facts rather than swapping which one is hidden, and + * these assert the page's rendered cell (CONVE-19): `agentActor.ts` is unit + * tested separately, and a correct helper this page never calls would pass + * all of those. + */ +const adminFetchMock = vi.hoisted(() => vi.fn<(path: string) => Promise>()); + +vi.mock('$lib/stores/admin.svelte', () => ({ + adminFetch: (path: string) => adminFetchMock(path) +})); + +const { default: AuditLogPage } = await import('./audit-log/+page.svelte'); + +interface Row { + id: string; + action: string; + actor: string; + source: string; + created_at: string; + metadata?: string; + actor_name?: string; + user_id?: string; +} + +function row(over: Partial = {}): Row { + return { + id: 'a1', + action: 'updated', + actor: 'agent', + source: 'cli', + created_at: new Date('2026-08-24T12:00:00Z').toISOString(), + metadata: JSON.stringify({ agent: 'wren' }), + actor_name: 'Dave', + user_id: 'user-1', + ...over + }; +} + +let host: HTMLElement; +let app: Record | null = null; + +/** Mount the page with `rows` as the audit-log response and let its + * onMount fetch resolve into the table. */ +async function mountWith(rows: Row[]): Promise { + adminFetchMock.mockResolvedValue(rows); + app = mount(AuditLogPage, { target: host, props: {} }) as Record; + flushSync(); + await Promise.resolve(); + await Promise.resolve(); + await tick(); + flushSync(); +} + +/** The user column is the second cell of each body row. */ +function userCells(): string[] { + return [...host.querySelectorAll('tbody tr')].map( + (tr) => tr.querySelectorAll('td')[1]?.textContent?.trim() ?? '' + ); +} + +beforeEach(() => { + host = document.createElement('div'); + document.body.appendChild(host); + adminFetchMock.mockReset(); +}); + +afterEach(() => { + if (app) unmount(app as never); + app = null; + host.remove(); +}); + +describe('console audit log — agent attribution', () => { + it('names the agent and the account it acted as', async () => { + await mountWith([row()]); + + expect(userCells()).toEqual(['wren (via Dave)']); + }); + + it('does not render the human alone for an agent write', async () => { + // The counterfactual, stated as its own case: this is exactly what the + // pre-TASK-2759 cell produced, and it is what the cell produces again + // if the agent branch is dropped or ordered after the actor_name one. + await mountWith([row()]); + + expect(userCells()).not.toEqual(['Dave']); + }); + + it('renders the agent alone when no account name resolved', async () => { + await mountWith([row({ actor_name: undefined })]); + + expect(userCells()).toEqual(['wren']); + }); + + it.each([ + ['no agent key', JSON.stringify({ ip: '127.0.0.1' })], + ['an empty name', JSON.stringify({ agent: '' })], + ['unparseable metadata', 'not json'], + ['absent metadata', undefined] + ])('falls back to the account name given %s', async (_case, metadata) => { + await mountWith([row({ metadata })]); + + expect(userCells()).toEqual(['Dave']); + }); + + it('ignores a stamp on a row whose actor is not an agent', async () => { + // The metadata blob is shared, and agentMeta merges into it by string + // splice — an `agent` key on a human's row is not a claim that an + // agent acted, and reading it unconditionally would invent one. + await mountWith([row({ actor: 'user' })]); + + expect(userCells()).toEqual(['Dave']); + }); + + it('leaves the existing System and user-id fallbacks intact', async () => { + await mountWith([ + row({ id: 'a2', actor: 'system', actor_name: undefined, metadata: '{}' }), + row({ + id: 'a3', + actor: 'user', + actor_name: undefined, + user_id: 'user-abcdefghijklmnop', + metadata: '{}' + }) + ]); + + // 12 characters then an ellipsis, per the column's existing truncation. + expect(userCells()).toEqual(['System', 'user-abcdefg…']); + }); +}); diff --git a/web/src/routes/workspaceDashboardAgentName.svelte.test.ts b/web/src/routes/workspaceDashboardAgentName.svelte.test.ts new file mode 100644 index 00000000..196e292e --- /dev/null +++ b/web/src/routes/workspaceDashboardAgentName.svelte.test.ts @@ -0,0 +1,145 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { flushSync, mount, unmount, tick } from 'svelte'; +import { page } from '$app/state'; +import type { Activity } from '$lib/types'; + +/** + * TASK-2759, the workspace dashboard's Recent Activity rows. + * + * This is the first surface most people see, and it rendered a flat "agent" + * badge for every agent write. Asserted through the PAGE (CONVE-19): the row + * passes `activity.metadata` to the shared helper, and passing the wrong + * argument — or not calling it — is invisible to the helper's own tests. + */ +const dashboardGet = vi.hoisted(() => vi.fn<(slug: string) => Promise>()); + +vi.mock('$lib/services/sync.svelte', () => ({ + syncService: { onSync: () => () => {}, start: () => {}, stop: () => {} } +})); + +vi.mock('$lib/api/client', () => ({ + api: { + dashboard: { get: (slug: string) => dashboardGet(slug) }, + collections: { list: vi.fn().mockResolvedValue([]) }, + workspaces: { + get: vi.fn().mockResolvedValue({ id: 'w1', slug: 'ws', name: 'WS' }), + me: vi.fn().mockResolvedValue({ role: 'owner' }), + list: vi.fn().mockResolvedValue([]) + } + }, + PadApiError: class extends Error {} +})); + +const { default: DashboardPage } = await import('./[username]/[workspace]/+page.svelte'); + +let seq = 0; +function act(over: Partial = {}): Activity { + seq += 1; + return { + id: `a${seq}`, + workspace_id: 'ws', + action: 'updated', + actor: 'agent', + source: 'cli', + metadata: JSON.stringify({ agent: 'wren' }), + created_at: new Date().toISOString(), + item_ref: 'TASK-1', + item_title: 'A thing', + item_slug: 'a-thing', + collection_slug: 'tasks', + ...over + } as Activity; +} + +/** + * A complete DashboardResponse with only `recent_activity` populated. + * + * Every array is present because the page reads `.length` on them + * unconditionally in its section guards — a payload missing one throws + * during render and leaves the whole page blank, which reads exactly like + * "the badge did not render". + */ +function dashboard(recent: Activity[]) { + return { + summary: { total_items: recent.length, by_collection: {} }, + active_items: [], + starred_items: [], + active_plans: [], + attention: [], + recent_activity: recent, + suggested_next: [], + has_agent_activity: true, + needs_onboarding: false, + degraded: false, + degraded_sections: [] + }; +} + +let host: HTMLElement; +let app: Record | null = null; + +async function mountWith(recent: Activity[]): Promise { + dashboardGet.mockResolvedValue(dashboard(recent)); + app = mount(DashboardPage, { target: host, props: {} }) as Record; + flushSync(); + for (let i = 0; i < 6; i++) { + await Promise.resolve(); + await tick(); + } + flushSync(); +} + +/** Actor badges in the Recent Activity list, in render order. */ +function agentBadges(): string[] { + return [...host.querySelectorAll('.activity-row .actor-badge.agent')].map( + (el) => el.textContent?.trim() ?? '' + ); +} + +beforeEach(() => { + host = document.createElement('div'); + document.body.appendChild(host); + page.params.workspace = 'ws'; + page.params.username = 'alice'; + dashboardGet.mockReset(); +}); + +afterEach(() => { + if (app) unmount(app as never); + app = null; + host.remove(); +}); + +describe('workspace dashboard — Recent Activity agent badge', () => { + it('badges each agent row with its stamped name', async () => { + await mountWith([act(), act({ metadata: JSON.stringify({ agent: 'rook' }) })]); + + expect(agentBadges()).toEqual(['wren', 'rook']); + }); + + it('badges a generic-looking client id verbatim', async () => { + await mountWith([act({ metadata: JSON.stringify({ agent: 'claude-code' }) })]); + + expect(agentBadges()).toEqual(['claude-code']); + }); + + it.each([ + ['no agent key', JSON.stringify({ changes: 'status' })], + ['an empty name', JSON.stringify({ agent: '' })], + ['unparseable metadata', 'not json'] + ])('falls back to the generic badge given %s', async (_case, metadata) => { + await mountWith([act({ metadata })]); + + // The counterfactual for the whole change: this is what EVERY agent row + // showed before it, so a badge stuck on this string is the failure mode. + expect(agentBadges()).toEqual(['agent']); + }); + + it('leaves a human row on its own name and never reads the stamp', async () => { + await mountWith([act({ actor: 'user', actor_name: 'Dave', source: 'web' })]); + + expect(agentBadges()).toEqual([]); + expect(host.textContent).toContain('Dave'); + expect(host.textContent).not.toContain('wren'); + }); +}); From 39844197ce5aa3676d28f025f6d59082d218fd6c Mon Sep 17 00:00:00 2001 From: xarmian Date: Mon, 24 Aug 2026 16:26:41 +0000 Subject: [PATCH 03/17] test(web): close two coverage holes the mutation matrix found (TASK-2759) Per-file negative controls showed EpisodeFeed and the console audit-log tests staying GREEN when the retired GENERIC_AGENT_IDS filter was reinstated: neither file used a value the filter would have swallowed, so both measured 'a name reaches the surface' without measuring 'an unfiltered name does'. One claude-code fixture each. 7/7 mutations now detected per-file. Claude-Session: https://claude.ai/code/session_01Cpr3teiHHsgcTmg2xhHA86 --- .../lib/components/activity/EpisodeFeed.svelte.test.ts | 10 ++++++++++ .../console/admin/auditLogAgentName.svelte.test.ts | 9 +++++++++ 2 files changed, 19 insertions(+) diff --git a/web/src/lib/components/activity/EpisodeFeed.svelte.test.ts b/web/src/lib/components/activity/EpisodeFeed.svelte.test.ts index ccd73d0b..3d938e3f 100644 --- a/web/src/lib/components/activity/EpisodeFeed.svelte.test.ts +++ b/web/src/lib/components/activity/EpisodeFeed.svelte.test.ts @@ -178,6 +178,16 @@ describe('EpisodeFeed', () => { expect(labels.sort()).toEqual(['rook', 'wren']); }); + // Named agents alone do not discriminate: reinstating the retired + // GENERIC_AGENT_IDS filter left this file green until this case existed, + // because nothing here used a value the filter would have swallowed. + it('renders a generic-looking client id verbatim', async () => { + mountFeed([act(1, { metadata: '{"agent":"claude-code"}' })]); + await settle(); + + expect(host.querySelector('.ep-actor')!.textContent).toBe('claude-code'); + }); + it('renders the generic label for an agent that stamped no name', async () => { // `act`'s default metadata is '{}' — the pre-BUG-2542 shape, and the // shape any agent that never sends the header still produces. diff --git a/web/src/routes/console/admin/auditLogAgentName.svelte.test.ts b/web/src/routes/console/admin/auditLogAgentName.svelte.test.ts index 1a7d724f..dcfe0965 100644 --- a/web/src/routes/console/admin/auditLogAgentName.svelte.test.ts +++ b/web/src/routes/console/admin/auditLogAgentName.svelte.test.ts @@ -98,6 +98,15 @@ describe('console audit log — agent attribution', () => { expect(userCells()).not.toEqual(['Dave']); }); + it('renders a generic-looking client id verbatim', async () => { + //'wren' alone does not discriminate: reinstating the retired + // GENERIC_AGENT_IDS filter left this file green until this case + // existed, because no fixture used a value the filter would swallow. + await mountWith([row({ metadata: JSON.stringify({ agent: 'claude-code' }) })]); + + expect(userCells()).toEqual(['claude-code (via Dave)']); + }); + it('renders the agent alone when no account name resolved', async () => { await mountWith([row({ actor_name: undefined })]); From a3ba6eec6c0a99fb57d2cef1ea3c83a684a0a10f Mon Sep 17 00:00:00 2001 From: xarmian Date: Mon, 24 Aug 2026 16:32:19 +0000 Subject: [PATCH 04/17] docs: the "name your agents" story for agent attribution (TASK-2759) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The README's For AI Agents section promised that agent actions are attributed, and said nothing about naming the agent — which was fair while nothing rendered the name. Now that five surfaces do, the section carries the precedence (.pad.toml agent_name -> $PAD_AGENT -> detected runtime), where the name shows up, and that Pad renders it verbatim rather than keeping a list of approved names. The honesty framing is QUOTED from ResolveAgentName's own contract comment rather than restated: the header is self-declared, an agent that omits it is indistinguishable from the human whose credentials it uses, and a human running `! pad ...` in an agent's terminal inherits that attribution. It is a label an actor chose, not evidence about who acted — which is also why the admin audit log shows both the agent and the account. Both SKILL.md copies gain one clause: the name an agent sends is now DISPLAYED, so a specific name beats a generic client id. Their existing attribution principle was already accurate and is otherwise untouched. Claude-Session: https://claude.ai/code/session_01Cpr3teiHHsgcTmg2xhHA86 --- README.md | 29 ++++++++++++++++++++++++++++- plugin/skills/pad/SKILL.md | 2 +- skills/pad/SKILL.md | 2 +- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 2a3affe7..63287a5d 100644 --- a/README.md +++ b/README.md @@ -126,7 +126,34 @@ pad item create convention "Run tests before completing tasks" \ --field priority=must ``` -Agents load relevant conventions automatically. All agent actions are attributed in the activity feed, so you always know what the AI changed. +Agents load relevant conventions automatically, and every agent action is attributed in the activity feed — so you can see what the AI changed rather than finding it later in a diff. + +**Name your agents:** + +By default an agent's writes show up as a generic `agent`. Give it a name and that name appears instead — in the activity feed's Live and Audit views, on the dashboard's recent activity, on item timelines, and in the admin audit log. With more than one agent working a project, that is the difference between "something automated touched this" and knowing which one. + +Pad takes the first of these it finds: + +```bash +# 1. Per-workspace, committed with the project — the deliberate choice. +# In .pad.toml: +# agent_name = "reviewer" + +# 2. Per-process, runtime-agnostic. Any harness can set it. +export PAD_AGENT=reviewer + +# 3. Otherwise Pad detects the runtime it knows (Claude Code reports +# "claude-code"), and falls back to the generic "agent" if it can't. +``` + +The name is rendered exactly as sent — Pad does not keep a list of approved names or rewrite what you choose. + +**What this does not claim.** The name is supplied by the client and self-declared, so it records honesty, not identity. From `ResolveAgentName`'s own contract in `internal/cli/agent_identity.go`: + +> - an agent that omits it is indistinguishable from the human whose credentials it is using; +> - a human running `! pad ...` inside an agent's terminal inherits that terminal's environment and will be attributed to the agent. + +So it is not a basis for machine-verifiable provenance: treat it as a label an actor chose, useful for reading a trail, not as evidence about who acted. Because the credentials belong to a person either way, surfaces that exist for provenance show both — the admin audit log renders `reviewer (via Dana)` rather than picking one. **Onboard agents to a new codebase:** diff --git a/plugin/skills/pad/SKILL.md b/plugin/skills/pad/SKILL.md index 9a64718d..4890c9f1 100644 --- a/plugin/skills/pad/SKILL.md +++ b/plugin/skills/pad/SKILL.md @@ -315,7 +315,7 @@ See the **Onboarding** entry under Natural Language Routing above — it branche 5. **Be conversational.** You're not a command executor. You're a project partner. 6. **Reference existing items.** Use `[[Item Title]]` links in content to connect items. 7. **Keep it practical.** Size each item so it's a single meaningful unit of work — what "meaningful" means depends on the workspace (one branch/PR for code, one interview round for hiring, one research question for research). Ideas should be actionable. Docs should be concise. Check the workspace's conventions for domain-specific sizing rules. -8. **Attribution matters.** Items and comments you create are stamped `created_by: agent` and `source: cli` automatically — but the agent half only works if the CLI can tell it is being run by an agent. It detects Claude Code on its own; under any other harness, set `PAD_AGENT=` in the environment (or `agent_name` in `.pad.toml`) or your writes will be recorded as the human whose credentials you are using. Note this is self-declared, not proof: it makes the trail honest, it does not make it verifiable, so never treat `created_by` on a comment as evidence that a human said something. +8. **Attribution matters.** Items and comments you create are stamped `created_by: agent` and `source: cli` automatically — but the agent half only works if the CLI can tell it is being run by an agent. It detects Claude Code on its own; under any other harness, set `PAD_AGENT=` in the environment (or `agent_name` in `.pad.toml`) or your writes will be recorded as the human whose credentials you are using. Whatever you send is DISPLAYED verbatim wherever agent actors appear — the activity feed, the dashboard, item timelines — so a specific name (`reviewer`, `nightly-triage`) is more use to a reader than a generic client id. Note this is self-declared, not proof: it makes the trail honest, it does not make it verifiable, so never treat `created_by` on a comment as evidence that a human said something. 9. **Follow project conventions.** Always load and follow active conventions before performing work. They are project-specific rules that override your defaults. When a role is active, load both role-specific and global conventions. 10. **Learn and teach.** When the user corrects your behavior or teaches you a project-specific rule, offer to save it as a convention: "Should I save this as a project convention so future agents follow it too?" Use `pad item create convention "Title" --field trigger= --field scope= --field priority=should --stdin` with an appropriate trigger inferred from the context. If the correction is role-specific, add `--field role=`. 11. **Role context is per-conversation.** If roles exist, ask which role the user is working as on first invocation. Remember it for the session. Auto-filter queries and suggest assignments accordingly. Never block on role — if the user says "no role" or the workspace has no roles, work normally. diff --git a/skills/pad/SKILL.md b/skills/pad/SKILL.md index ed4e1f5e..c08cce09 100644 --- a/skills/pad/SKILL.md +++ b/skills/pad/SKILL.md @@ -325,7 +325,7 @@ Run the **onboard** invokable playbook — see the **Onboarding** entry under Na 5. **Be conversational.** You're not a command executor. You're a project partner. 6. **Reference existing items.** Use `[[Item Title]]` links in content to connect items. 7. **Keep it practical.** Size each item so it's a single meaningful unit of work — what "meaningful" means depends on the workspace (one branch/PR for code, one interview round for hiring, one research question for research). Ideas should be actionable. Docs should be concise. Check the workspace's conventions for domain-specific sizing rules. -8. **Attribution matters.** Items and comments you create are stamped `created_by: agent` and `source: cli` automatically — but the agent half only works if the CLI can tell it is being run by an agent. It detects Claude Code on its own; under any other harness, set `PAD_AGENT=` in the environment (or `agent_name` in `.pad.toml`) or your writes will be recorded as the human whose credentials you are using. Note this is self-declared, not proof: it makes the trail honest, it does not make it verifiable, so never treat `created_by` on a comment as evidence that a human said something. +8. **Attribution matters.** Items and comments you create are stamped `created_by: agent` and `source: cli` automatically — but the agent half only works if the CLI can tell it is being run by an agent. It detects Claude Code on its own; under any other harness, set `PAD_AGENT=` in the environment (or `agent_name` in `.pad.toml`) or your writes will be recorded as the human whose credentials you are using. Whatever you send is DISPLAYED verbatim wherever agent actors appear — the activity feed, the dashboard, item timelines — so a specific name (`reviewer`, `nightly-triage`) is more use to a reader than a generic client id. Note this is self-declared, not proof: it makes the trail honest, it does not make it verifiable, so never treat `created_by` on a comment as evidence that a human said something. 9. **Follow project conventions.** Always load and follow active conventions before performing work. They are project-specific rules that override your defaults. When a role is active, load both role-specific and global conventions. 10. **Learn and teach.** When the user corrects your behavior or teaches you a project-specific rule, offer to save it as a convention: "Should I save this as a project convention so future agents follow it too?" Use `pad item create convention "Title" --field trigger= --field scope= --field priority=should --stdin` with an appropriate trigger inferred from the context. If the correction is role-specific, add `--field role=`. 11. **Role context is per-conversation.** If roles exist, ask which role the user is working as on first invocation. Remember it for the session. Auto-filter queries and suggest assignments accordingly. Never block on role — if the user says "no role" or the workspace has no roles, work normally. From d1c5c3976e2be52090c0ec37646d29eda95ff17e Mon Sep 17 00:00:00 2001 From: xarmian Date: Mon, 24 Aug 2026 16:40:05 +0000 Subject: [PATCH 05/17] fix(web): stop the badge CSS upper-casing a stamped agent name (TASK-2759) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex round 1. `.actor-badge` sets text-transform: uppercase, so the activity page's audit rows and the dashboard's recent activity rendered `Wren` and `wren` as the same pixels — the verbatim contract broken in CSS rather than in code, and invisible to every textContent assertion in the suite. The codebase already draws this line: `.actor-badge.user` opts out of the transform, because a human's badge carries a NAME while "agent" / "cli" / "web" are CATEGORY words that read as chips. A stamped agent name is a name, so it follows the same rule via a `named` modifier; the generic fallback stays a chip. Swept the class rather than fixing the two reported sites (CONVE-18): the other three surfaces are unaffected — Chip has no transform, EpisodeFeed's uppercase rule is .section-label ("HAPPENING NOW"), and the audit log's cell is untransformed. Two sites, both fixed. Tests assert the class the markup applies, and say so: Svelte component styles are not injected under this vitest setup (0 style elements, so getComputedStyle resolves nothing), which leaves the adjacent CSS rule outside what the suite can observe. The class is the half a refactor drops. Claude-Session: https://claude.ai/code/session_01Cpr3teiHHsgcTmg2xhHA86 --- .../[username]/[workspace]/+page.svelte | 15 +++++++++-- .../[workspace]/activity/+page.svelte | 27 ++++++++++++++----- .../activityPageAgentName.svelte.test.ts | 24 +++++++++++++++++ ...workspaceDashboardAgentName.svelte.test.ts | 18 +++++++++++++ 4 files changed, 76 insertions(+), 8 deletions(-) diff --git a/web/src/routes/[username]/[workspace]/+page.svelte b/web/src/routes/[username]/[workspace]/+page.svelte index e2f17e13..39db3967 100644 --- a/web/src/routes/[username]/[workspace]/+page.svelte +++ b/web/src/routes/[username]/[workspace]/+page.svelte @@ -9,7 +9,7 @@ import { uiStore } from '$lib/stores/ui.svelte'; import { syncService } from '$lib/services/sync.svelte'; import { relativeTime } from '$lib/utils/markdown'; - import { agentActorLabel } from '$lib/utils/agentActor'; + import { agentNameFromMetadata } from '$lib/utils/agentActor'; import OnboardingLaunchpad from '$lib/components/OnboardingLaunchpad.svelte'; import ConnectWorkspaceModal from '$lib/components/ConnectWorkspaceModal.svelte'; import Button from '$lib/components/common/Button.svelte'; @@ -631,9 +631,15 @@
{#each dashboard.recent_activity.slice(0, 10) as activity, i (i)} {@const changes = parseActivityChanges(activity.metadata)} + {@const agentName = agentNameFromMetadata(activity.metadata)}
{#if activity.actor === 'agent'} - {agentActorLabel(activity.metadata, 'agent')} + + {agentName ?? 'agent'} {:else if activity.actor_name} {activity.actor_name} {:else if activity.source === 'cli'} @@ -1304,6 +1310,11 @@ background: color-mix(in srgb, var(--accent-purple) 15%, transparent); color: var(--accent-purple); } + /* A stamped agent name is a name, not a category word — see the markup. */ + .actor-badge.named { + text-transform: none; + letter-spacing: normal; + } .actor-badge.cli { background: color-mix(in srgb, var(--accent-blue) 15%, transparent); color: var(--accent-blue); diff --git a/web/src/routes/[username]/[workspace]/activity/+page.svelte b/web/src/routes/[username]/[workspace]/activity/+page.svelte index 405bf284..5e29ba9b 100644 --- a/web/src/routes/[username]/[workspace]/activity/+page.svelte +++ b/web/src/routes/[username]/[workspace]/activity/+page.svelte @@ -246,15 +246,24 @@ actor: string, actorName?: string, metadata?: Record - ): { label: string; kind: string } { + ): { label: string; kind: string; named: boolean } { // An agent's own name when it sent one, else the generic badge. The // name is never merged with `actorName` — that is the human whose // credentials the write rode on, and conflating the two is the // mis-attribution this renders to end. - if (actor === 'agent') return { label: agentNameOf(metadata) ?? 'agent', kind: 'agent' }; - if (actorName) return { label: actorName, kind: source === 'cli' ? 'cli' : 'user' }; - if (source === 'cli') return { label: 'cli', kind: 'cli' }; - return { label: 'web', kind: 'web' }; + // + // `named` drives the badge's uppercasing off. "agent" / "cli" / "web" + // are CATEGORY words and read as chips; a stamped name is a name, and + // upper-casing it would collapse `Wren` and `wren` into the same + // pixels — breaking the verbatim contract in CSS rather than in code. + // The `user` kind has always opted out for exactly this reason. + if (actor === 'agent') { + const name = agentNameOf(metadata); + return { label: name ?? 'agent', kind: 'agent', named: name !== undefined }; + } + if (actorName) return { label: actorName, kind: source === 'cli' ? 'cli' : 'user', named: true }; + if (source === 'cli') return { label: 'cli', kind: 'cli', named: false }; + return { label: 'web', kind: 'web', named: false }; } function borderClass(source: string, actor: string): string { @@ -422,7 +431,7 @@ {/if}
- {src.label} + {src.label} { expect(host.querySelector('.actor-badge.agent')!.textContent!.trim()).toBe('agent'); }); + // Codex round 1. The badge's base rule uppercases, so `Wren` and `wren` + // rendered identically — the verbatim contract broken in CSS rather than + // in code, and invisible to every textContent assertion above. + // + // BOUNDARY: this asserts the class the markup applies, not the pixels. + // Svelte component styles are not injected under this vitest setup + // (`document.querySelectorAll('style').length` is 0, so getComputedStyle + // resolves nothing), which leaves the one adjacent CSS rule — + // `.actor-badge.named { text-transform: none }` — outside what this suite + // can observe. The class is the half a refactor would actually drop. + it('marks a stamped name as a name so the badge stops upper-casing it', async () => { + await mountPage('audit', [act({ metadata: JSON.stringify({ agent: 'Wren' }) })]); + + const badge = host.querySelector('.actor-badge.agent')!; + expect(badge.textContent!.trim()).toBe('Wren'); + expect(badge.classList.contains('named')).toBe(true); + }); + + it('leaves the generic badge upper-cased as a category word', async () => { + await mountPage('audit', [act({ metadata: '{}' })]); + + expect(host.querySelector('.actor-badge.agent')!.classList.contains('named')).toBe(false); + }); + it('never reads the stamp for a human row', async () => { // The badge for a human is keyed on actor_name/source, and an `agent` // key can ride along on any row's shared metadata blob. diff --git a/web/src/routes/workspaceDashboardAgentName.svelte.test.ts b/web/src/routes/workspaceDashboardAgentName.svelte.test.ts index 196e292e..a91aa809 100644 --- a/web/src/routes/workspaceDashboardAgentName.svelte.test.ts +++ b/web/src/routes/workspaceDashboardAgentName.svelte.test.ts @@ -135,6 +135,24 @@ describe('workspace dashboard — Recent Activity agent badge', () => { expect(agentBadges()).toEqual(['agent']); }); + // Codex round 1 — see the twin case in activityPageAgentName for the + // reasoning and for the boundary this assertion does NOT cover (the CSS + // rule itself is unobservable under this vitest setup). + it('marks a stamped name as a name so the badge stops upper-casing it', async () => { + await mountWith([act({ metadata: JSON.stringify({ agent: 'Wren' }) })]); + + const badge = host.querySelector('.activity-row .actor-badge.agent')!; + expect(badge.textContent!.trim()).toBe('Wren'); + expect(badge.classList.contains('named')).toBe(true); + }); + + it('leaves the generic badge upper-cased as a category word', async () => { + await mountWith([act({ metadata: '{}' })]); + + const badge = host.querySelector('.activity-row .actor-badge.agent')!; + expect(badge.classList.contains('named')).toBe(false); + }); + it('leaves a human row on its own name and never reads the stamp', async () => { await mountWith([act({ actor: 'user', actor_name: 'Dave', source: 'web' })]); From fa22b6680e7cdb6eaaafcdcced8f3d68d59494ba Mon Sep 17 00:00:00 2001 From: xarmian Date: Mon, 24 Aug 2026 16:45:35 +0000 Subject: [PATCH 06/17] docs+test: correct two over-claims and pin name escaping (TASK-2759) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex round 2, fresh angles. P1, accepted — my own docs over-claimed. The README and both SKILL.md copies said the name appears wherever agent actors appear, including "item timelines". Comments, version snapshots and note/decision entries carry the actor KIND and no name (that is the exempt set the plan named, and TASK-2760 files the comment half), so on a timeline only ACTIVITY entries show it. Both now say which entries carry it and which read "Agent". P2, accepted — the README's fallback was wrong in a way that mattered. When nothing resolves a name, the CLI omits X-Pad-Agent entirely (client.go:1884), so actorFromRequest records the write as "user": it is attributed to the PERSON, not to a generic "agent". Verified both call sites rather than reasoning from the label. The generic "agent" rows that do exist come from pre-naming writes and from audit events logged without agentMeta. P2, accepted — the name is attacker-influenced text and every test used benign values, so a rewrite to {@html} would have passed. Added a markup payload at two surfaces that build their labels through different paths, asserting no element is created and the text survives intact. Claude-Session: https://claude.ai/code/session_01Cpr3teiHHsgcTmg2xhHA86 --- README.md | 12 ++++++++---- plugin/skills/pad/SKILL.md | 2 +- skills/pad/SKILL.md | 2 +- .../timelineActivityAgentName.svelte.test.ts | 16 ++++++++++++++++ .../routes/activityPageAgentName.svelte.test.ts | 12 ++++++++++++ 5 files changed, 38 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 63287a5d..5d160123 100644 --- a/README.md +++ b/README.md @@ -130,7 +130,7 @@ Agents load relevant conventions automatically, and every agent action is attrib **Name your agents:** -By default an agent's writes show up as a generic `agent`. Give it a name and that name appears instead — in the activity feed's Live and Audit views, on the dashboard's recent activity, on item timelines, and in the admin audit log. With more than one agent working a project, that is the difference between "something automated touched this" and knowing which one. +An agent that identifies itself gets its name shown on its writes — in the activity feed's Live and Audit views, on the dashboard's recent activity, on item timeline *activity* entries, and in the admin audit log. With more than one agent working a project, that is the difference between "something automated touched this" and knowing which one. Pad takes the first of these it finds: @@ -142,11 +142,15 @@ Pad takes the first of these it finds: # 2. Per-process, runtime-agnostic. Any harness can set it. export PAD_AGENT=reviewer -# 3. Otherwise Pad detects the runtime it knows (Claude Code reports -# "claude-code"), and falls back to the generic "agent" if it can't. +# 3. Otherwise Pad detects the runtimes it knows — Claude Code reports +# "claude-code" — and that detected id is used as the name. ``` -The name is rendered exactly as sent — Pad does not keep a list of approved names or rewrite what you choose. +**If none of the three produce a name, the write is not marked as an agent's at all** — it is recorded as the person whose credentials it used, which is the case the caveat below is about. The generic `agent` label you may see on older entries is a write that identified itself before Pad stored names, or an event type that records the actor without the name (workspace membership changes, sign-ins). + +The name is rendered exactly as sent — Pad keeps no list of approved names, and does not re-case or rewrite what you choose. + +Not every entry can show it. Comments, version snapshots, and implementation-note/decision entries record only *that* an agent acted, because the name is not stored on those rows — they still read `Agent`. Activity entries are the ones that carry it. **What this does not claim.** The name is supplied by the client and self-declared, so it records honesty, not identity. From `ResolveAgentName`'s own contract in `internal/cli/agent_identity.go`: diff --git a/plugin/skills/pad/SKILL.md b/plugin/skills/pad/SKILL.md index 4890c9f1..187c5106 100644 --- a/plugin/skills/pad/SKILL.md +++ b/plugin/skills/pad/SKILL.md @@ -315,7 +315,7 @@ See the **Onboarding** entry under Natural Language Routing above — it branche 5. **Be conversational.** You're not a command executor. You're a project partner. 6. **Reference existing items.** Use `[[Item Title]]` links in content to connect items. 7. **Keep it practical.** Size each item so it's a single meaningful unit of work — what "meaningful" means depends on the workspace (one branch/PR for code, one interview round for hiring, one research question for research). Ideas should be actionable. Docs should be concise. Check the workspace's conventions for domain-specific sizing rules. -8. **Attribution matters.** Items and comments you create are stamped `created_by: agent` and `source: cli` automatically — but the agent half only works if the CLI can tell it is being run by an agent. It detects Claude Code on its own; under any other harness, set `PAD_AGENT=` in the environment (or `agent_name` in `.pad.toml`) or your writes will be recorded as the human whose credentials you are using. Whatever you send is DISPLAYED verbatim wherever agent actors appear — the activity feed, the dashboard, item timelines — so a specific name (`reviewer`, `nightly-triage`) is more use to a reader than a generic client id. Note this is self-declared, not proof: it makes the trail honest, it does not make it verifiable, so never treat `created_by` on a comment as evidence that a human said something. +8. **Attribution matters.** Items and comments you create are stamped `created_by: agent` and `source: cli` automatically — but the agent half only works if the CLI can tell it is being run by an agent. It detects Claude Code on its own; under any other harness, set `PAD_AGENT=` in the environment (or `agent_name` in `.pad.toml`) or your writes will be recorded as the human whose credentials you are using. Whatever you send is DISPLAYED verbatim on the surfaces that store it — the activity feed, the dashboard's recent activity, activity entries on an item's timeline, the admin audit log — so a specific name (`reviewer`, `nightly-triage`) is more use to a reader than a generic client id. Comments, versions and note/decision entries record only that an agent acted, not which one. Note this is self-declared, not proof: it makes the trail honest, it does not make it verifiable, so never treat `created_by` on a comment as evidence that a human said something. 9. **Follow project conventions.** Always load and follow active conventions before performing work. They are project-specific rules that override your defaults. When a role is active, load both role-specific and global conventions. 10. **Learn and teach.** When the user corrects your behavior or teaches you a project-specific rule, offer to save it as a convention: "Should I save this as a project convention so future agents follow it too?" Use `pad item create convention "Title" --field trigger= --field scope= --field priority=should --stdin` with an appropriate trigger inferred from the context. If the correction is role-specific, add `--field role=`. 11. **Role context is per-conversation.** If roles exist, ask which role the user is working as on first invocation. Remember it for the session. Auto-filter queries and suggest assignments accordingly. Never block on role — if the user says "no role" or the workspace has no roles, work normally. diff --git a/skills/pad/SKILL.md b/skills/pad/SKILL.md index c08cce09..ae335562 100644 --- a/skills/pad/SKILL.md +++ b/skills/pad/SKILL.md @@ -325,7 +325,7 @@ Run the **onboard** invokable playbook — see the **Onboarding** entry under Na 5. **Be conversational.** You're not a command executor. You're a project partner. 6. **Reference existing items.** Use `[[Item Title]]` links in content to connect items. 7. **Keep it practical.** Size each item so it's a single meaningful unit of work — what "meaningful" means depends on the workspace (one branch/PR for code, one interview round for hiring, one research question for research). Ideas should be actionable. Docs should be concise. Check the workspace's conventions for domain-specific sizing rules. -8. **Attribution matters.** Items and comments you create are stamped `created_by: agent` and `source: cli` automatically — but the agent half only works if the CLI can tell it is being run by an agent. It detects Claude Code on its own; under any other harness, set `PAD_AGENT=` in the environment (or `agent_name` in `.pad.toml`) or your writes will be recorded as the human whose credentials you are using. Whatever you send is DISPLAYED verbatim wherever agent actors appear — the activity feed, the dashboard, item timelines — so a specific name (`reviewer`, `nightly-triage`) is more use to a reader than a generic client id. Note this is self-declared, not proof: it makes the trail honest, it does not make it verifiable, so never treat `created_by` on a comment as evidence that a human said something. +8. **Attribution matters.** Items and comments you create are stamped `created_by: agent` and `source: cli` automatically — but the agent half only works if the CLI can tell it is being run by an agent. It detects Claude Code on its own; under any other harness, set `PAD_AGENT=` in the environment (or `agent_name` in `.pad.toml`) or your writes will be recorded as the human whose credentials you are using. Whatever you send is DISPLAYED verbatim on the surfaces that store it — the activity feed, the dashboard's recent activity, activity entries on an item's timeline, the admin audit log — so a specific name (`reviewer`, `nightly-triage`) is more use to a reader than a generic client id. Comments, versions and note/decision entries record only that an agent acted, not which one. Note this is self-declared, not proof: it makes the trail honest, it does not make it verifiable, so never treat `created_by` on a comment as evidence that a human said something. 9. **Follow project conventions.** Always load and follow active conventions before performing work. They are project-specific rules that override your defaults. When a role is active, load both role-specific and global conventions. 10. **Learn and teach.** When the user corrects your behavior or teaches you a project-specific rule, offer to save it as a convention: "Should I save this as a project convention so future agents follow it too?" Use `pad item create convention "Title" --field trigger= --field scope= --field priority=should --stdin` with an appropriate trigger inferred from the context. If the correction is role-specific, add `--field role=`. 11. **Role context is per-conversation.** If roles exist, ask which role the user is working as on first invocation. Remember it for the session. Auto-filter queries and suggest assignments accordingly. Never block on role — if the user says "no role" or the workspace has no roles, work normally. diff --git a/web/src/lib/components/timeline/timelineActivityAgentName.svelte.test.ts b/web/src/lib/components/timeline/timelineActivityAgentName.svelte.test.ts index 9f09f91a..091c1a7d 100644 --- a/web/src/lib/components/timeline/timelineActivityAgentName.svelte.test.ts +++ b/web/src/lib/components/timeline/timelineActivityAgentName.svelte.test.ts @@ -78,6 +78,22 @@ describe('TimelineActivityCard agent name', () => { expect(getByText('Agent')).toBeTruthy(); }); + // Codex round 2. The name is attacker-influenced: it is whatever a client + // put in X-Pad-Agent, and the server stores it without inspection. Svelte + // text interpolation escapes it today, so this passes as written — the + // point is that it would STOP passing if any of these render paths were + // rewritten to `{@html}`, which is the plausible way an "allow rich agent + // labels" change would arrive. + it('renders a name containing markup as text, never as elements', () => { + const payload = ''; + const { container, getByText } = render(TimelineActivityCard, { + activity: activity({ metadata: JSON.stringify({ agent: payload }) }) + }); + + expect(container.querySelector('img')).toBeNull(); + expect(getByText(payload)).toBeTruthy(); + }); + it('never reads the stamp for a non-agent actor', () => { // Guards against keying the chip on metadata alone. A human's write can // carry an `agent` key — the merge in agentMeta is textual and this diff --git a/web/src/routes/activityPageAgentName.svelte.test.ts b/web/src/routes/activityPageAgentName.svelte.test.ts index 92e6cd6d..cdd9495f 100644 --- a/web/src/routes/activityPageAgentName.svelte.test.ts +++ b/web/src/routes/activityPageAgentName.svelte.test.ts @@ -141,6 +141,18 @@ describe('activity page — Audit view', () => { expect(host.querySelector('.actor-badge.agent')!.classList.contains('named')).toBe(false); }); + // Codex round 2 — the same escaping claim at a second surface, because the + // two views build their labels through different code paths and a future + // `{@html}` would land in one of them, not both. See the twin case in + // timelineActivityAgentName for why this passes today. + it('renders a name containing markup as text, never as elements', async () => { + const payload = ''; + await mountPage('audit', [act({ metadata: JSON.stringify({ agent: payload }) })]); + + expect(host.querySelector('img')).toBeNull(); + expect(host.querySelector('.actor-badge.agent')!.textContent!.trim()).toBe(payload); + }); + it('never reads the stamp for a human row', async () => { // The badge for a human is keyed on actor_name/source, and an `agent` // key can ride along on any row's shared metadata blob. From 6dc6499a6efb70b76303a0acdc798dfd207898ae Mon Sep 17 00:00:00 2001 From: xarmian Date: Mon, 24 Aug 2026 16:52:09 +0000 Subject: [PATCH 07/17] refactor(web): drop the one-use helper wrapper, bound name width (TASK-2759) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex round 3. P3 — `agentActorLabel` had a single caller. Every other site already holds parsed metadata and reaches for `agentNameOf`, and each supplies its own fallback anyway, so the wrapper saved one `?? 'agent'` and cost an inconsistency in how five call sites looked. Removed; the reasoning stays in the file so it is not re-added. P2 — names are unbounded text in fixed-layout rows. Before this unit the agent badge held one of four fixed words; it now holds whatever a client put in X-Pad-Agent, while still being `flex-shrink: 0`, so one long name pushes the timestamp off the row. Bounded with an ellipsis at the two badges and the episode actor label (which has the same exposure for people's names, and had it before this change), with the full value on the title attribute. The timeline chip and the audit-log cell both wrap, so they take the title only. P2 on presentation consistency — three surfaces show the (agent, human) pair three ways, and this unit invented one of the three. Declined as scope rather than as wrong, and filed as IDEA-2762 with what a decision has to cover. Codex also independently confirmed the exempt set: comments, versions and structured entries genuinely do not carry the name, and the linked comment activity that does is skipped when the card renders. Claude-Session: https://claude.ai/code/session_01Cpr3teiHHsgcTmg2xhHA86 --- .../components/activity/EpisodeFeed.svelte | 9 +++++++- .../timeline/TimelineActivityCard.svelte | 5 +++- web/src/lib/utils/activityEpisodes.ts | 4 ++-- web/src/lib/utils/agentActor.test.ts | 18 ++++----------- web/src/lib/utils/agentActor.ts | 23 ++++++++----------- .../[username]/[workspace]/+page.svelte | 15 ++++++++++-- .../[workspace]/activity/+page.svelte | 18 +++++++++++++-- 7 files changed, 57 insertions(+), 35 deletions(-) diff --git a/web/src/lib/components/activity/EpisodeFeed.svelte b/web/src/lib/components/activity/EpisodeFeed.svelte index 45b333d3..2cf396fd 100644 --- a/web/src/lib/components/activity/EpisodeFeed.svelte +++ b/web/src/lib/components/activity/EpisodeFeed.svelte @@ -68,7 +68,7 @@ {#snippet card(ep: Episode, live: boolean)}
- {ep.actorLabel} + {ep.actorLabel} {episodeVerb(ep.actions)} {#if ep.itemRef} {#if ep.itemSlug && ep.collectionSlug} @@ -199,6 +199,13 @@ font-weight: 700; color: var(--text-primary); white-space: nowrap; + /* An actor label is now arbitrary client-supplied text for agents + (whatever went in X-Pad-Agent) as well as for people. `nowrap` + without a bound lets one long name push the rest of the card's + line out; the full value stays in the title attribute. */ + max-width: 20ch; + overflow: hidden; + text-overflow: ellipsis; } .ep-verb { font-size: 12.5px; diff --git a/web/src/lib/components/timeline/TimelineActivityCard.svelte b/web/src/lib/components/timeline/TimelineActivityCard.svelte index 7263d181..ab87af25 100644 --- a/web/src/lib/components/timeline/TimelineActivityCard.svelte +++ b/web/src/lib/components/timeline/TimelineActivityCard.svelte @@ -57,10 +57,13 @@
+ {getActorLabel(activity)}{getActorLabel(activity)} {#if activity.actor_name} {activity.actor_name} diff --git a/web/src/lib/utils/activityEpisodes.ts b/web/src/lib/utils/activityEpisodes.ts index 061d3d1c..debd4af8 100644 --- a/web/src/lib/utils/activityEpisodes.ts +++ b/web/src/lib/utils/activityEpisodes.ts @@ -1,5 +1,5 @@ import type { Activity } from '$lib/types'; -import { agentActorLabel } from '$lib/utils/agentActor'; +import { agentNameFromMetadata } from '$lib/utils/agentActor'; /** * The episode fold behind the activity page's Live view (IDEA-2755). @@ -61,7 +61,7 @@ export interface FoldOptions { function actorKeyOf(a: Activity): { key: string; label: string; kind: string } { if (a.actor === 'agent') { - const label = agentActorLabel(a.metadata, 'agent'); + const label = agentNameFromMetadata(a.metadata) ?? 'agent'; return { key: `agent:${label}`, label, kind: 'agent' }; } const label = a.actor_name ?? (a.source === 'cli' ? 'cli' : 'web'); diff --git a/web/src/lib/utils/agentActor.test.ts b/web/src/lib/utils/agentActor.test.ts index e0fb5105..c37da2e2 100644 --- a/web/src/lib/utils/agentActor.test.ts +++ b/web/src/lib/utils/agentActor.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { agentActorLabel, agentNameFromMetadata, agentNameOf } from './agentActor'; +import { agentNameFromMetadata, agentNameOf } from './agentActor'; /** * The helper's whole contract is "verbatim, or nothing" — so the cases that @@ -65,15 +65,7 @@ describe('agentNameFromMetadata', () => { }); }); -describe('agentActorLabel', () => { - it('returns the stamped name when there is one', () => { - expect(agentActorLabel('{"agent":"wren"}', 'agent')).toBe('wren'); - }); - - it("returns the caller's own fallback when there is not", () => { - // The fallback is deliberately not shared: surfaces disagree on its - // casing ("agent" in the feed's badges, "Agent" in timeline chips). - expect(agentActorLabel('{}', 'agent')).toBe('agent'); - expect(agentActorLabel('{}', 'Agent')).toBe('Agent'); - }); -}); +// The `agentActorLabel(metadata, fallback)` wrapper these last cases covered +// was removed in codex round 3: one caller, and it hid the `?? fallback` that +// every other site wrote inline. Each surface's own fallback is asserted where +// that surface is tested, which is where the casing difference actually lives. diff --git a/web/src/lib/utils/agentActor.ts b/web/src/lib/utils/agentActor.ts index a201fe99..2315536e 100644 --- a/web/src/lib/utils/agentActor.ts +++ b/web/src/lib/utils/agentActor.ts @@ -58,18 +58,13 @@ export function agentNameOf(metadata: Record | undefined | null return name; } -/** - * The label for an agent actor: its stamped name, or `fallback` when the - * row carries none. - * - * The fallback stays the caller's, because each surface already has its - * own vocabulary for the nameless case ("agent" in the feed's lowercase - * badges, "Agent" in the timeline's chips) and this change is not the - * place to unify them. +/* + * There is deliberately no `label(metadata, fallback)` wrapper. One existed + * and had a single caller: every other site already holds the parsed + * metadata and reaches for `agentNameOf`, and each supplies its own + * fallback anyway — surfaces disagree on the nameless case ("agent" in the + * feed's lowercase badges, "Agent" in the timeline's chips), so the wrapper + * saved a `?? 'agent'` and cost an inconsistency in how five call sites + * looked. Two functions, one difference between them: do you have the JSON + * string or the parsed object. */ -export function agentActorLabel( - metadata: string | undefined | null, - fallback: string -): string { - return agentNameFromMetadata(metadata) ?? fallback; -} diff --git a/web/src/routes/[username]/[workspace]/+page.svelte b/web/src/routes/[username]/[workspace]/+page.svelte index 39db3967..9a377b71 100644 --- a/web/src/routes/[username]/[workspace]/+page.svelte +++ b/web/src/routes/[username]/[workspace]/+page.svelte @@ -639,7 +639,11 @@ and upper-casing it would make `Wren` and `wren` identical — breaking the verbatim contract in CSS. Humans already get this treatment (the .actor-name span below is never transformed). --> - {agentName ?? 'agent'} + {agentName ?? 'agent'} {:else if activity.actor_name} {activity.actor_name} {:else if activity.source === 'cli'} @@ -1310,10 +1314,17 @@ background: color-mix(in srgb, var(--accent-purple) 15%, transparent); color: var(--accent-purple); } - /* A stamped agent name is a name, not a category word — see the markup. */ + /* A stamped agent name is a name, not a category word — see the markup. + Bounded because the badge is `flex-shrink: 0` and the name is arbitrary + client-supplied text, not one of the fixed words this badge used to + hold; the full value stays reachable via the title attribute. */ .actor-badge.named { text-transform: none; letter-spacing: normal; + max-width: 16ch; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; } .actor-badge.cli { background: color-mix(in srgb, var(--accent-blue) 15%, transparent); diff --git a/web/src/routes/[username]/[workspace]/activity/+page.svelte b/web/src/routes/[username]/[workspace]/activity/+page.svelte index 5e29ba9b..765d1aa2 100644 --- a/web/src/routes/[username]/[workspace]/activity/+page.svelte +++ b/web/src/routes/[username]/[workspace]/activity/+page.svelte @@ -431,7 +431,11 @@ {/if}
- {src.label} + {src.label} Date: Mon, 24 Aug 2026 17:01:33 +0000 Subject: [PATCH 08/17] fix(web): keep named actors out of the anonymous fold key; type the dashboard fixture (TASK-2759) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex round 4. P2 — the fold key was built from the DISPLAY LABEL, so an agent that sends `agent` in X-Pad-Agent folded together with every agent that sent no name at all: two different claims ("this actor" and "we have no name for this actor") sharing one key, which also contradicted the file's own comment about a named agent getting its own key. Named and anonymous are now separate namespaces. Swept the sibling rather than the reported half (CONVE-18): the user branch had the identical defect for a person whose display name is `cli` or `web`. Both fixed, both asserted. P2 test gap — the dashboard route fixture was typed `Activity`, but `recent_activity` is a REDUCED DTO with no id/workspace_id/document_id and an OPTIONAL metadata. A fixture richer than the real payload cannot fail when the payload changes, and it hid a reachable case: rows logged by the audit helpers never call agentMeta, so absent metadata is a shape the server really sends. Fixture now derives from DashboardResponse and both route suites cover absent metadata. P2 on activity debounce — real, verified against the store rather than taken on report, and outside this unit's web-only boundary. CreateActivityDebounced matches on (document, action, user) without actor, and its UPDATE leaves the original `actor` in place while mergeActivityMeta overlays the newer `agent` name, so a coalesced row can name the wrong writer in either direction. Filed as BUG-2763 with both orderings worked through. This unit did not cause it; it is the first thing to make it visible. Codex also reported SSR/hydration CLEAN and verified this diff's claims about the Go side against the Go code. Claude-Session: https://claude.ai/code/session_01Cpr3teiHHsgcTmg2xhHA86 --- web/src/lib/utils/activityEpisodes.test.ts | 39 ++++++++++++++++++- web/src/lib/utils/activityEpisodes.ts | 21 ++++++++-- .../activityPageAgentName.svelte.test.ts | 6 ++- ...workspaceDashboardAgentName.svelte.test.ts | 28 ++++++++----- 4 files changed, 77 insertions(+), 17 deletions(-) diff --git a/web/src/lib/utils/activityEpisodes.test.ts b/web/src/lib/utils/activityEpisodes.test.ts index c677f4f0..3f32cb77 100644 --- a/web/src/lib/utils/activityEpisodes.test.ts +++ b/web/src/lib/utils/activityEpisodes.test.ts @@ -93,10 +93,45 @@ describe('foldEpisodes', () => { it('renders a generic-looking client id verbatim rather than filtering it', () => { const eps = foldEpisodes([act(1, { metadata: '{"agent":"claude-code"}' })], { now }); expect(eps[0].actorLabel).toBe('claude-code'); - // The fold key follows the label, so a filtered id would also have + // The fold key follows the name, so a filtered id would also have // collapsed distinct clients into one episode — assert the key, since // that consequence outlives any change to the label's wording. - expect(eps[0].key.startsWith('agent:claude-code|')).toBe(true); + expect(eps[0].key.startsWith('agent:named:claude-code|')).toBe(true); + }); + + // Codex round 4. `agent` is a legal value for X-Pad-Agent, and a person + // may be named `cli`. A key built from the display label alone folds + // either one together with the actors we have NO name for — two + // different claims ("this actor" vs "unattributed") sharing one key. + it('separates an actor literally named like the generic label from unnamed ones', () => { + const onOther = { document_id: 'item-2', item_ref: 'BUG-2' }; + const eps = foldEpisodes( + [ + act(1, { metadata: '{"agent":"agent"}' }), + act(2, { metadata: '{}', ...onOther }) + ], + { now } + ); + + expect(eps).toHaveLength(2); + // Same rendered label, deliberately different identity. + expect(eps.map((e) => e.actorLabel)).toEqual(['agent', 'agent']); + expect(new Set(eps.map((e) => e.key.split('|')[0])).size).toBe(2); + }); + + it('separates a person named like a source from an unattributed one', () => { + const onOther = { document_id: 'item-2', item_ref: 'BUG-2' }; + const eps = foldEpisodes( + [ + act(1, { actor: 'user', actor_name: 'cli', source: 'cli' }), + act(2, { actor: 'user', actor_name: undefined, source: 'cli', ...onOther }) + ], + { now } + ); + + expect(eps).toHaveLength(2); + expect(eps.map((e) => e.actorLabel)).toEqual(['cli', 'cli']); + expect(new Set(eps.map((e) => e.key.split('|')[0])).size).toBe(2); }); // Each stamp shape gets its own fold call: within one call these rows would diff --git a/web/src/lib/utils/activityEpisodes.ts b/web/src/lib/utils/activityEpisodes.ts index debd4af8..a7885ca0 100644 --- a/web/src/lib/utils/activityEpisodes.ts +++ b/web/src/lib/utils/activityEpisodes.ts @@ -59,13 +59,26 @@ export interface FoldOptions { now?: () => number; } +/** + * The fold key must distinguish "this actor" from "an actor we have no name + * for", and the LABEL cannot do that job: an agent may legitimately send + * `agent` in X-Pad-Agent, and a person's display name may be `cli`, so a key + * built from the label alone silently merges a named actor with the nameless + * ones. The name is client-supplied text, so treating it as a distinct + * namespace rather than a value in the same one is the only version that + * cannot collide. (Both halves matter: the user branch had the same defect + * for a person named `cli` or `web` — CONVE-18.) + */ function actorKeyOf(a: Activity): { key: string; label: string; kind: string } { if (a.actor === 'agent') { - const label = agentNameFromMetadata(a.metadata) ?? 'agent'; - return { key: `agent:${label}`, label, kind: 'agent' }; + const name = agentNameFromMetadata(a.metadata); + return { key: name ? `agent:named:${name}` : 'agent:anon', label: name ?? 'agent', kind: 'agent' }; } - const label = a.actor_name ?? (a.source === 'cli' ? 'cli' : 'web'); - return { key: `user:${label}`, label, kind: 'user' }; + if (a.actor_name) { + return { key: `user:named:${a.actor_name}`, label: a.actor_name, kind: 'user' }; + } + const label = a.source === 'cli' ? 'cli' : 'web'; + return { key: `user:anon:${label}`, label, kind: 'user' }; } /** diff --git a/web/src/routes/activityPageAgentName.svelte.test.ts b/web/src/routes/activityPageAgentName.svelte.test.ts index cdd9495f..d3ca3820 100644 --- a/web/src/routes/activityPageAgentName.svelte.test.ts +++ b/web/src/routes/activityPageAgentName.svelte.test.ts @@ -110,7 +110,11 @@ describe('activity page — Audit view', () => { it.each([ ['no agent key', JSON.stringify({ changes: 'status' })], ['an empty name', JSON.stringify({ agent: '' })], - ['unparseable metadata', 'not json'] + ['unparseable metadata', 'not json'], + // A row can reach the client with no metadata at all — the audit + // helpers that log workspace-membership and auth events never call + // agentMeta, so they produce actor=agent with nothing stamped. + ['absent metadata', undefined as unknown as string] ])('falls back to the generic badge given %s', async (_case, metadata) => { await mountPage('audit', [act({ metadata })]); diff --git a/web/src/routes/workspaceDashboardAgentName.svelte.test.ts b/web/src/routes/workspaceDashboardAgentName.svelte.test.ts index a91aa809..db2cc859 100644 --- a/web/src/routes/workspaceDashboardAgentName.svelte.test.ts +++ b/web/src/routes/workspaceDashboardAgentName.svelte.test.ts @@ -1,7 +1,16 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { flushSync, mount, unmount, tick } from 'svelte'; import { page } from '$app/state'; -import type { Activity } from '$lib/types'; +import type { DashboardResponse } from '$lib/types'; + +/** + * The dashboard's rows are NOT `Activity` — `recent_activity` is a reduced + * shape with no id/workspace_id/document_id and an OPTIONAL metadata. Typing + * the fixture as the real DTO is what keeps this suite honest if that payload + * changes, and it is why the omitted-metadata case below is reachable at all + * (codex round 4). + */ +type RecentActivity = DashboardResponse['recent_activity'][number]; /** * TASK-2759, the workspace dashboard's Recent Activity rows. @@ -32,12 +41,8 @@ vi.mock('$lib/api/client', () => ({ const { default: DashboardPage } = await import('./[username]/[workspace]/+page.svelte'); -let seq = 0; -function act(over: Partial = {}): Activity { - seq += 1; +function act(over: Partial = {}): RecentActivity { return { - id: `a${seq}`, - workspace_id: 'ws', action: 'updated', actor: 'agent', source: 'cli', @@ -48,7 +53,7 @@ function act(over: Partial = {}): Activity { item_slug: 'a-thing', collection_slug: 'tasks', ...over - } as Activity; + }; } /** @@ -59,7 +64,7 @@ function act(over: Partial = {}): Activity { * during render and leaves the whole page blank, which reads exactly like * "the badge did not render". */ -function dashboard(recent: Activity[]) { +function dashboard(recent: RecentActivity[]) { return { summary: { total_items: recent.length, by_collection: {} }, active_items: [], @@ -78,7 +83,7 @@ function dashboard(recent: Activity[]) { let host: HTMLElement; let app: Record | null = null; -async function mountWith(recent: Activity[]): Promise { +async function mountWith(recent: RecentActivity[]): Promise { dashboardGet.mockResolvedValue(dashboard(recent)); app = mount(DashboardPage, { target: host, props: {} }) as Record; flushSync(); @@ -126,7 +131,10 @@ describe('workspace dashboard — Recent Activity agent badge', () => { it.each([ ['no agent key', JSON.stringify({ changes: 'status' })], ['an empty name', JSON.stringify({ agent: '' })], - ['unparseable metadata', 'not json'] + ['unparseable metadata', 'not json'], + // metadata is OPTIONAL on this DTO, so absent is a shape the server + // really sends — not just a defensive case (codex round 4). + ['absent metadata', undefined] ])('falls back to the generic badge given %s', async (_case, metadata) => { await mountWith([act({ metadata })]); From 454afe573a0c03ca9ad91528267ce6857098171d Mon Sep 17 00:00:00 2001 From: xarmian Date: Mon, 24 Aug 2026 17:07:45 +0000 Subject: [PATCH 09/17] perf+a11y(web): one metadata parse per audit row, drop a redundant tooltip (TASK-2759) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex round 5. P2 — the admin audit log parsed each row's metadata twice once this unit added a second reader (displayUser alongside formatMetadata), on a table that grows through "Load more". Hoisted to one `parseMetadata` per row, passed to both. formatMetadata now takes the parsed object, which also removes the try/catch it no longer needs. Left alone, with the reason: the dashboard also reads metadata twice per row, but it renders at most ten and its other reader (parseActivityChanges) has callers outside this diff whose signature I am not changing for ten rows. P3 — the title I put on the timeline actor chip duplicated text that is always fully visible: that row wraps and the chip never clips, so it added no information and gives assistive technology the same string twice. Removed. The titles on the two badges and the episode label stay — those DO clip, and there the attribute is the only way to the full value. Codex reported clean on reachability (no code-reachable wrong-name case beyond the documented self-declared limitation and the excluded BUG-2763) and on history coherence. Claude-Session: https://claude.ai/code/session_01Cpr3teiHHsgcTmg2xhHA86 --- .../timeline/TimelineActivityCard.svelte | 9 +++--- .../console/admin/audit-log/+page.svelte | 31 +++++++++++++------ 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/web/src/lib/components/timeline/TimelineActivityCard.svelte b/web/src/lib/components/timeline/TimelineActivityCard.svelte index ab87af25..27250155 100644 --- a/web/src/lib/components/timeline/TimelineActivityCard.svelte +++ b/web/src/lib/components/timeline/TimelineActivityCard.svelte @@ -57,13 +57,14 @@
- + {getActorLabel(activity)}{getActorLabel(activity)} {#if activity.actor_name} {activity.actor_name} diff --git a/web/src/routes/console/admin/audit-log/+page.svelte b/web/src/routes/console/admin/audit-log/+page.svelte index 54be5332..05a975b7 100644 --- a/web/src/routes/console/admin/audit-log/+page.svelte +++ b/web/src/routes/console/admin/audit-log/+page.svelte @@ -1,7 +1,7 @@