mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-11 13:28:57 +00:00
Merge pull request #1194 from PerpetualSoftware/feat/TASK-2759-agent-name-surfacing
feat(web): surface agent display names wherever agent actors render (TASK-2759)
This commit is contained in:
@@ -126,7 +126,40 @@ 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:**
|
||||
|
||||
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 console's audit log and per-user activity views. 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 runtimes it knows — Claude Code reports
|
||||
# "claude-code" — and that detected id is used as the name.
|
||||
```
|
||||
|
||||
**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`:
|
||||
|
||||
> - 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.
|
||||
|
||||
Since the name is chosen by whoever is writing, it is displayed as an isolated unit: it is shown as sent, but it cannot re-order or restyle the text around it, and the account half of `name (via account)` is rendered separately so a chosen name cannot forge it.
|
||||
|
||||
**Onboard agents to a new codebase:**
|
||||
|
||||
|
||||
@@ -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=<name>` 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=<name>` 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 console's audit and per-user activity views — 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=<inferred> --field scope=<inferred> --field priority=should --stdin` with an appropriate trigger inferred from the context. If the correction is role-specific, add `--field role=<slug>`.
|
||||
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.
|
||||
|
||||
+1
-1
@@ -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=<name>` 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=<name>` 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 console's audit and per-user activity views — 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=<inferred> --field scope=<inferred> --field priority=should --stdin` with an appropriate trigger inferred from the context. If the correction is role-specific, add `--field role=<slug>`.
|
||||
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.
|
||||
|
||||
@@ -68,7 +68,7 @@
|
||||
{#snippet card(ep: Episode, live: boolean)}
|
||||
<div class="episode-card" class:agent={ep.actorKind === 'agent'} class:earlier={!live}>
|
||||
<div class="ep-main">
|
||||
<span class="ep-actor">{ep.actorLabel}</span>
|
||||
<bdi class="ep-actor" title={ep.actorLabel}>{ep.actorLabel}</bdi>
|
||||
<span class="ep-verb">{episodeVerb(ep.actions)}</span>
|
||||
{#if ep.itemRef}
|
||||
{#if ep.itemSlug && ep.collectionSlug}
|
||||
@@ -199,6 +199,15 @@
|
||||
font-weight: 700;
|
||||
color: var(--text-primary);
|
||||
white-space: nowrap;
|
||||
/* An actor label is arbitrary 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. 24ch matches the activity page's
|
||||
badge, where the number's reasoning and its limits are written
|
||||
out. */
|
||||
max-width: 24ch;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
}
|
||||
.ep-verb {
|
||||
font-size: 12.5px;
|
||||
|
||||
@@ -154,4 +154,57 @@ 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).
|
||||
//
|
||||
// Both events are on the SAME item, deliberately: that is what makes the
|
||||
// card count discriminating. Two different items would produce two cards
|
||||
// however the actors were keyed, so the earlier version of this test proved
|
||||
// only that labels render (codex round 10). Same item, same window, two
|
||||
// names — a fold that ignored the name would yield ONE card.
|
||||
it('renders each agent under its own stamped name', async () => {
|
||||
mountFeed([
|
||||
act(1, { metadata: '{"agent":"wren"}' }),
|
||||
act(2, { metadata: '{"agent":"rook"}' }),
|
||||
]);
|
||||
await settle();
|
||||
|
||||
expect(host.querySelectorAll('.episode-card')).toHaveLength(2);
|
||||
const labels = [...host.querySelectorAll('.ep-actor')].map((el) => el.textContent);
|
||||
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');
|
||||
});
|
||||
|
||||
// Codex round 11. The label element must stay a <bdi>: an agent name is
|
||||
// self-declared text and a bidi control inside it would otherwise reorder
|
||||
// the verb and item title that follow it on the card's line. Swapping the
|
||||
// element back to a <span> passes every text assertion above.
|
||||
it('isolates the actor label so a bidi control cannot reorder the card', async () => {
|
||||
mountFeed([act(1, { metadata: '{"agent":"wren\u202egnimalb"}' })]);
|
||||
await settle();
|
||||
|
||||
const el = host.querySelector('.ep-actor')!;
|
||||
expect(el.tagName).toBe('BDI');
|
||||
expect(el.textContent).toBe('wren\u202egnimalb');
|
||||
});
|
||||
|
||||
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');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -13,6 +13,7 @@
|
||||
-->
|
||||
<script lang="ts">
|
||||
import { adminFetch, type AdminUser } from '$lib/stores/admin.svelte';
|
||||
import { agentNameFromMetadata } from '$lib/utils/agentActor';
|
||||
|
||||
interface Props {
|
||||
user: AdminUser;
|
||||
@@ -31,6 +32,10 @@
|
||||
user_id?: string;
|
||||
created_at: string;
|
||||
actor_name?: string;
|
||||
// Carried so a named agent can be shown (TASK-2759). The endpoint
|
||||
// serializes whole models.Activity rows, so this was always present
|
||||
// on the wire — the type, not the payload, was the omission.
|
||||
metadata?: string;
|
||||
ip_address?: string;
|
||||
user_agent?: string;
|
||||
}
|
||||
@@ -301,7 +306,18 @@
|
||||
<div class="activity-main">
|
||||
<div class="activity-summary">
|
||||
<span class="activity-action">{describe(ev)}</span>
|
||||
<span class="activity-source">via {ev.source}</span>
|
||||
<!-- TASK-2759. The endpoint returns whole Activity rows, so the
|
||||
stamped agent name is on the wire here; only this local type
|
||||
was dropping it. Without it an admin reading a user's
|
||||
activity sees "via cli" and cannot tell WHICH agent acted —
|
||||
the same gap the audit log had, on the same rows. -->
|
||||
{#if ev.actor === 'agent'}
|
||||
{@const agentName = agentNameFromMetadata(ev.metadata)}
|
||||
{#if agentName}
|
||||
<bdi class="activity-agent" title={agentName}>{agentName}</bdi>
|
||||
{/if}
|
||||
{/if}
|
||||
<span class="activity-source">via {ev.source}</span>
|
||||
</div>
|
||||
{#if ev.workspace_id}
|
||||
<div class="activity-meta">workspace: {ev.workspace_id.slice(0, 8)}…</div>
|
||||
@@ -392,6 +408,16 @@
|
||||
font-size: 0.85rem;
|
||||
color: var(--text-primary);
|
||||
}
|
||||
/* An agent name is arbitrary client-supplied text: <bdi> keeps a bidi
|
||||
control inside it from reordering the row, and the bound matches the
|
||||
activity page's badge, where the number's reasoning is written out. */
|
||||
.activity-agent {
|
||||
font-weight: 600;
|
||||
max-width: 24ch;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
white-space: nowrap;
|
||||
}
|
||||
.activity-source {
|
||||
font-size: 0.7rem;
|
||||
color: var(--text-muted);
|
||||
|
||||
@@ -13,6 +13,7 @@
|
||||
-->
|
||||
<script lang="ts">
|
||||
import { adminFetch, type AdminUser } from '$lib/stores/admin.svelte';
|
||||
import { agentNameFromMetadata } from '$lib/utils/agentActor';
|
||||
import Chip from '$lib/components/common/Chip.svelte';
|
||||
|
||||
interface Props {
|
||||
@@ -39,6 +40,8 @@
|
||||
user_id?: string;
|
||||
created_at: string;
|
||||
actor_name?: string;
|
||||
// See UserActivityTab — same rows, same omission (TASK-2759).
|
||||
metadata?: string;
|
||||
}
|
||||
|
||||
let metrics = $state<Metrics | null>(null);
|
||||
@@ -227,6 +230,12 @@
|
||||
{#each recentItems as ev (ev.id)}
|
||||
<li class="recent-row">
|
||||
<span class="recent-action">{ev.action}</span>
|
||||
{#if ev.actor === 'agent'}
|
||||
{@const agentName = agentNameFromMetadata(ev.metadata)}
|
||||
{#if agentName}
|
||||
<bdi class="recent-agent" title={agentName}>{agentName}</bdi>
|
||||
{/if}
|
||||
{/if}
|
||||
<span class="recent-source">via {ev.source}</span>
|
||||
<span class="recent-time" title={ev.created_at}>{relativeTime(ev.created_at)}</span>
|
||||
</li>
|
||||
@@ -332,6 +341,16 @@
|
||||
font-weight: 500;
|
||||
color: var(--text-primary);
|
||||
}
|
||||
/* An agent name is arbitrary client-supplied text: <bdi> keeps a bidi
|
||||
control inside it from reordering the row, and the bound matches the
|
||||
activity page's badge, where the number's reasoning is written out. */
|
||||
.recent-agent {
|
||||
font-weight: 600;
|
||||
max-width: 24ch;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
white-space: nowrap;
|
||||
}
|
||||
.recent-source {
|
||||
color: var(--text-muted);
|
||||
font-size: 0.75rem;
|
||||
|
||||
@@ -0,0 +1,169 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { flushSync, mount, unmount, tick } from 'svelte';
|
||||
|
||||
/**
|
||||
* TASK-2759, codex round 12 — and a correction to my own exemption.
|
||||
*
|
||||
* The plan listed this tab as exempt because its local row type omitted
|
||||
* `metadata`. That was true and it was the wrong reason: the endpoint
|
||||
* serializes whole `models.Activity` rows, so the stamped name was on the
|
||||
* wire the whole time and the surface DOES meet the unit's discriminator
|
||||
* ("does this surface hold an Activity?"). An admin reading a user's
|
||||
* activity saw "via cli" with no way to tell which agent acted — the same
|
||||
* gap the audit log had, on the same rows.
|
||||
*/
|
||||
const adminFetchMock = vi.hoisted(() => vi.fn<(path: string) => Promise<any>>());
|
||||
|
||||
vi.mock('$lib/stores/admin.svelte', () => ({
|
||||
adminFetch: (path: string) => adminFetchMock(path)
|
||||
}));
|
||||
|
||||
const { default: UserActivityTab } = await import('./UserActivityTab.svelte');
|
||||
const { default: UserOverviewTab } = await import('./UserOverviewTab.svelte');
|
||||
|
||||
function event(over: Record<string, unknown> = {}) {
|
||||
return {
|
||||
id: 'e1',
|
||||
action: 'updated',
|
||||
actor: 'agent',
|
||||
source: 'cli',
|
||||
created_at: new Date('2026-08-24T12:00:00Z').toISOString(),
|
||||
metadata: JSON.stringify({ agent: 'wren' }),
|
||||
document_id: 'item-1',
|
||||
...over
|
||||
};
|
||||
}
|
||||
|
||||
let host: HTMLElement;
|
||||
let app: Record<string, unknown> | null = null;
|
||||
|
||||
async function mountWith(events: Record<string, unknown>[]): Promise<void> {
|
||||
adminFetchMock.mockResolvedValue({ events, next_offset: null });
|
||||
app = mount(UserActivityTab, {
|
||||
target: host,
|
||||
props: { user: { id: 'u1' } as never, active: true }
|
||||
}) as Record<string, unknown>;
|
||||
flushSync();
|
||||
for (let i = 0; i < 6; i++) {
|
||||
await Promise.resolve();
|
||||
await tick();
|
||||
}
|
||||
flushSync();
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
host = document.createElement('div');
|
||||
document.body.appendChild(host);
|
||||
adminFetchMock.mockReset();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (app) unmount(app as never);
|
||||
app = null;
|
||||
host.remove();
|
||||
});
|
||||
|
||||
describe('admin user activity — agent name', () => {
|
||||
it('names the agent behind an agent-sourced row', async () => {
|
||||
await mountWith([event()]);
|
||||
|
||||
const el = host.querySelector('.activity-agent')!;
|
||||
expect(el).not.toBeNull();
|
||||
expect(el.textContent).toBe('wren');
|
||||
});
|
||||
|
||||
it('renders nothing extra when the row carries no name', async () => {
|
||||
// The counterfactual for the whole surface: this is what every row
|
||||
// looked like before, and an unconditional element would show an empty
|
||||
// one here rather than falling back cleanly.
|
||||
await mountWith([event({ metadata: '{}' })]);
|
||||
|
||||
expect(host.querySelector('.activity-agent')).toBeNull();
|
||||
expect(host.textContent).toContain('via cli');
|
||||
});
|
||||
|
||||
it('never reads the stamp for a non-agent row', async () => {
|
||||
await mountWith([event({ actor: 'user' })]);
|
||||
|
||||
expect(host.querySelector('.activity-agent')).toBeNull();
|
||||
});
|
||||
|
||||
// A named fixture alone does not discriminate: reinstating the retired
|
||||
// GENERIC_AGENT_IDS filter in the shared helper left this whole file green
|
||||
// until this case existed, because nothing here used a value the filter
|
||||
// would have swallowed. Same omission the feed and audit-log suites had —
|
||||
// I repeated it when adding this file later.
|
||||
it('renders a generic-looking client id verbatim', async () => {
|
||||
await mountWith([event({ metadata: JSON.stringify({ agent: 'claude-code' }) })]);
|
||||
|
||||
expect(host.querySelector('.activity-agent')!.textContent).toBe('claude-code');
|
||||
});
|
||||
|
||||
it('isolates the name so a bidi control cannot reorder the row', async () => {
|
||||
await mountWith([event({ metadata: JSON.stringify({ agent: 'wrengnimalb' }) })]);
|
||||
|
||||
const el = host.querySelector('.activity-agent')!;
|
||||
expect(el.tagName).toBe('BDI');
|
||||
expect(el.textContent).toBe('wrengnimalb');
|
||||
});
|
||||
|
||||
it('survives unparseable metadata without throwing', async () => {
|
||||
await mountWith([event({ metadata: 'not json' })]);
|
||||
|
||||
expect(host.querySelector('.activity-agent')).toBeNull();
|
||||
expect(host.textContent).toContain('via cli');
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The overview tab renders the same rows through its own markup and its own
|
||||
* filter (writes only), so it is a second binding, not a second view of the
|
||||
* first — CONVE-19. It reads the same endpoint, plus a metrics call.
|
||||
*/
|
||||
async function mountOverview(events: Record<string, unknown>[]): Promise<void> {
|
||||
adminFetchMock.mockImplementation((path: string) =>
|
||||
path.includes('/metrics')
|
||||
? Promise.resolve({})
|
||||
: Promise.resolve({ events, next_offset: null })
|
||||
);
|
||||
app = mount(UserOverviewTab, {
|
||||
target: host,
|
||||
props: { user: { id: 'u1' } as never, active: true }
|
||||
}) as Record<string, unknown>;
|
||||
flushSync();
|
||||
for (let i = 0; i < 6; i++) {
|
||||
await Promise.resolve();
|
||||
await tick();
|
||||
}
|
||||
flushSync();
|
||||
}
|
||||
|
||||
describe('admin user overview — agent name', () => {
|
||||
it('names the agent behind a write', async () => {
|
||||
await mountOverview([event()]);
|
||||
|
||||
const el = host.querySelector('.recent-agent')!;
|
||||
expect(el).not.toBeNull();
|
||||
expect(el.tagName).toBe('BDI');
|
||||
expect(el.textContent).toBe('wren');
|
||||
});
|
||||
|
||||
it('renders nothing extra when the row carries no name', async () => {
|
||||
await mountOverview([event({ metadata: '{}' })]);
|
||||
|
||||
expect(host.querySelector('.recent-agent')).toBeNull();
|
||||
expect(host.textContent).toContain('via cli');
|
||||
});
|
||||
|
||||
it('never reads the stamp for a non-agent row', async () => {
|
||||
await mountOverview([event({ actor: 'user' })]);
|
||||
|
||||
expect(host.querySelector('.recent-agent')).toBeNull();
|
||||
});
|
||||
|
||||
it('renders a generic-looking client id verbatim', async () => {
|
||||
await mountOverview([event({ metadata: JSON.stringify({ agent: 'claude-code' }) })]);
|
||||
|
||||
expect(host.querySelector('.recent-agent')!.textContent).toBe('claude-code');
|
||||
});
|
||||
});
|
||||
@@ -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 {
|
||||
@@ -52,13 +57,19 @@
|
||||
|
||||
<div class="card">
|
||||
<div class="row">
|
||||
<!-- The label DOES clip (see .actor-label), so it carries its full value
|
||||
in a title. An earlier round removed this attribute on the grounds
|
||||
that the chip never clipped; a later round added the width bound and
|
||||
made that premise false. -->
|
||||
<Chip
|
||||
size="sm"
|
||||
color={activity.actor === 'agent' ? 'var(--accent-purple)' : 'var(--status-blue)'}
|
||||
>{getActorLabel(activity)}</Chip
|
||||
><bdi class="actor-label" title={getActorLabel(activity)}
|
||||
>{getActorLabel(activity)}</bdi
|
||||
></Chip
|
||||
>
|
||||
{#if activity.actor_name}
|
||||
<span class="actor-name">{activity.actor_name}</span>
|
||||
<bdi class="actor-name">{activity.actor_name}</bdi>
|
||||
{/if}
|
||||
<span class="action-label {getActionClass(activity.action)}">{getActionLabel(activity.action)}</span>
|
||||
{#if activity.action === 'moved' && metadata.from_collection && metadata.to_collection}
|
||||
@@ -123,6 +134,20 @@
|
||||
font-weight: 500;
|
||||
}
|
||||
|
||||
/* An agent's label is arbitrary client-supplied text. <bdi> keeps a bidi
|
||||
control character inside it from reordering the rest of the row, and the
|
||||
bound stops one very long name from widening a card that lives in a pane
|
||||
whose width is not the card's to negotiate. 24ch matches the activity
|
||||
page's badge, where the number's reasoning and its limits are written
|
||||
out. */
|
||||
.actor-label {
|
||||
display: inline-block;
|
||||
max-width: 24ch;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
vertical-align: bottom;
|
||||
}
|
||||
|
||||
.action-label {
|
||||
font-size: 0.85em;
|
||||
color: var(--text-muted);
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
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> = {}): 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();
|
||||
});
|
||||
|
||||
// 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 = '<img src=x onerror="alert(1)">';
|
||||
const { container, getByText } = render(TimelineActivityCard, {
|
||||
activity: activity({ metadata: JSON.stringify({ agent: payload }) })
|
||||
});
|
||||
|
||||
expect(container.querySelector('img')).toBeNull();
|
||||
expect(getByText(payload)).toBeTruthy();
|
||||
});
|
||||
|
||||
// Codex round 11 — the element, not just the text. The chip's label sits
|
||||
// inline before the action label and timestamp, so a bidi control in a
|
||||
// self-declared name would reorder them if the label were not isolated.
|
||||
it('isolates the actor label so a bidi control cannot reorder the row', () => {
|
||||
const { container } = render(TimelineActivityCard, {
|
||||
activity: activity({ metadata: JSON.stringify({ agent: 'wren\u202egnimalb' }) })
|
||||
});
|
||||
|
||||
const el = container.querySelector('.actor-label')!;
|
||||
expect(el.tagName).toBe('BDI');
|
||||
expect(el.textContent).toBe('wren\u202egnimalb');
|
||||
});
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
@@ -83,8 +83,67 @@ 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 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: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
|
||||
// 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');
|
||||
});
|
||||
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { Activity } from '$lib/types';
|
||||
import { agentNameFromMetadata } 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,32 +59,26 @@ 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<string, unknown>;
|
||||
const name = meta.agent;
|
||||
if (typeof name !== 'string' || name.length === 0) return undefined;
|
||||
return GENERIC_AGENT_IDS.has(name) ? undefined : name;
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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 } {
|
||||
const seat = metaAgentName(a.metadata);
|
||||
if (a.actor === 'agent') {
|
||||
const label = seat ?? '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' };
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -0,0 +1,99 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { 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)));
|
||||
});
|
||||
});
|
||||
|
||||
// Four of the five surfaces call this one, not the string form, because they
|
||||
// already hold the parsed metadata. Equivalence with agentNameFromMetadata was
|
||||
// the only thing pinning it, which covers the shared path and not this
|
||||
// function's own edges — most of them only existed incidentally inside
|
||||
// component tests (codex round 14).
|
||||
describe('agentNameOf', () => {
|
||||
it('returns the name from a parsed object', () => {
|
||||
expect(agentNameOf({ agent: 'wren', changes: 'status' })).toBe('wren');
|
||||
});
|
||||
|
||||
it.each([
|
||||
['no agent key', { changes: 'status' }],
|
||||
['an empty name', { agent: '' }],
|
||||
['a null name', { agent: null }],
|
||||
['a numeric name', { agent: 123 }],
|
||||
['an object name', { agent: { name: 'wren' } }],
|
||||
['an empty object', {}],
|
||||
['undefined', undefined],
|
||||
['null', null]
|
||||
])('returns undefined for %s', (_case, meta) => {
|
||||
expect(agentNameOf(meta as Record<string, unknown> | undefined | null)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('does not filter a generic-looking id', () => {
|
||||
expect(agentNameOf({ agent: 'claude-code' })).toBe('claude-code');
|
||||
});
|
||||
|
||||
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');
|
||||
});
|
||||
});
|
||||
|
||||
// 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.
|
||||
@@ -0,0 +1,81 @@
|
||||
/**
|
||||
* 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.
|
||||
*
|
||||
* WHICH MAKES IT A LEAF, NEVER A FRAGMENT. Because the actor authors this
|
||||
* value, any string a surface BUILDS around it hands the author influence
|
||||
* over the parts they did not write — a name can spell out the connective
|
||||
* text ("admin (via root)") or carry a bidi control that reorders whatever
|
||||
* was appended after it. So every surface renders it as its own isolated
|
||||
* element (<bdi>) and composes with siblings in markup rather than by
|
||||
* concatenation; the console audit log's `displayUser` is the worked
|
||||
* example, and returns parts for exactly this reason. That is not a
|
||||
* softening of the verbatim rule above: isolation alters no characters and
|
||||
* rejects no names, it just refuses to let one value redraw another.
|
||||
*/
|
||||
|
||||
/**
|
||||
* 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<string, unknown>);
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
/** {@link agentNameFromMetadata} for callers holding the parsed object. */
|
||||
export function agentNameOf(metadata: Record<string, unknown> | 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;
|
||||
}
|
||||
|
||||
/*
|
||||
* 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.
|
||||
*/
|
||||
@@ -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 { 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';
|
||||
@@ -630,11 +631,21 @@
|
||||
<div class="activity-list">
|
||||
{#each dashboard.recent_activity.slice(0, 10) as activity, i (i)}
|
||||
{@const changes = parseActivityChanges(activity.metadata)}
|
||||
{@const agentName = agentNameFromMetadata(activity.metadata)}
|
||||
<div class="activity-row">
|
||||
{#if activity.actor === 'agent'}
|
||||
<span class="actor-badge agent">agent</span>
|
||||
<!-- `named` drops the badge's uppercasing: the generic "agent" is a
|
||||
CATEGORY word and reads as a chip, but a stamped name is a name,
|
||||
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). -->
|
||||
<bdi
|
||||
class="actor-badge agent"
|
||||
class:named={agentName}
|
||||
title={agentName}>{agentName ?? 'agent'}</bdi
|
||||
>
|
||||
{:else if activity.actor_name}
|
||||
<span class="actor-name">{activity.actor_name}</span>
|
||||
<bdi class="actor-name">{activity.actor_name}</bdi>
|
||||
{:else if activity.source === 'cli'}
|
||||
<span class="actor-badge cli">cli</span>
|
||||
{/if}
|
||||
@@ -1303,6 +1314,19 @@
|
||||
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.
|
||||
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. 24ch matches the activity page's badge, where the reasoning for
|
||||
the number and what it does not cover is written out. */
|
||||
.actor-badge.named {
|
||||
text-transform: none;
|
||||
letter-spacing: normal;
|
||||
max-width: 24ch;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
white-space: nowrap;
|
||||
}
|
||||
.actor-badge.cli {
|
||||
background: color-mix(in srgb, var(--accent-blue) 15%, transparent);
|
||||
color: var(--accent-blue);
|
||||
|
||||
@@ -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,11 +241,29 @@
|
||||
}
|
||||
}
|
||||
|
||||
function getSourceLabel(source: string, actor: string, actorName?: string): { label: string; kind: string } {
|
||||
if (actor === 'agent') return { label: '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' };
|
||||
function getSourceLabel(
|
||||
source: string,
|
||||
actor: string,
|
||||
actorName?: string,
|
||||
metadata?: Record<string, unknown>
|
||||
): { 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.
|
||||
//
|
||||
// `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 {
|
||||
@@ -370,7 +389,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)}
|
||||
<div class="entry {borderClass(activity.source, activity.actor)}">
|
||||
<span
|
||||
class="entry-icon"
|
||||
@@ -412,7 +431,11 @@
|
||||
{/if}
|
||||
</div>
|
||||
<div class="entry-meta">
|
||||
<span class="actor-badge {src.kind}">{src.label}</span>
|
||||
<bdi
|
||||
class="actor-badge {src.kind}"
|
||||
class:named={src.named}
|
||||
title={src.named ? src.label : undefined}>{src.label}</bdi
|
||||
>
|
||||
<span
|
||||
class="entry-time"
|
||||
title={new Date(activity.created_at).toLocaleString()}
|
||||
@@ -707,6 +730,31 @@
|
||||
background: color-mix(in srgb, var(--accent-purple) 15%, transparent);
|
||||
color: var(--accent-purple);
|
||||
}
|
||||
/* A name, not a category word — same reasoning as `.actor-badge.user`
|
||||
below, which has always opted out. See getSourceLabel's `named`.
|
||||
|
||||
Bounded because the badge is `flex-shrink: 0` and a name is arbitrary
|
||||
text — an agent's is whatever went in X-Pad-Agent, a person's is
|
||||
whatever they set — not one of the four fixed words this badge used to
|
||||
hold. Unbounded, one long name pushes the timestamp off the row.
|
||||
|
||||
24ch is a judgement, not a measurement: it comfortably fits ordinary
|
||||
full names ("Alexandra Whitfield" is 19) while still bounding the
|
||||
pathological case. It is a LAYOUT bound and says nothing about which
|
||||
names are valid — nothing is rejected or altered, and the full value is
|
||||
on the title attribute. What it does NOT cover: `title` is not reachable
|
||||
by touch and awkward by keyboard, so a clipped name is effectively
|
||||
unreadable on a phone. Raising the bound trades that against the
|
||||
overflow it exists to prevent; a real disclosure affordance would be the
|
||||
actual fix and is not this unit's. */
|
||||
.actor-badge.named {
|
||||
text-transform: none;
|
||||
letter-spacing: normal;
|
||||
max-width: 24ch;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
white-space: nowrap;
|
||||
}
|
||||
.actor-badge.cli {
|
||||
background: color-mix(in srgb, var(--accent-blue) 15%, transparent);
|
||||
color: var(--accent-blue);
|
||||
|
||||
@@ -0,0 +1,197 @@
|
||||
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<Activity[]>>()
|
||||
);
|
||||
|
||||
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> = {}): 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<string, unknown> | 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<void> {
|
||||
localStorage.setItem('pad-activity-view', view);
|
||||
listActivity.mockResolvedValue(rows);
|
||||
app = mount(ActivityPage, { target: host, props: {} }) as Record<string, unknown>;
|
||||
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'],
|
||||
// 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 })]);
|
||||
|
||||
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);
|
||||
});
|
||||
|
||||
// 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 = '<img src=x onerror="alert(1)">';
|
||||
await mountPage('audit', [act({ metadata: JSON.stringify({ agent: payload }) })]);
|
||||
|
||||
expect(host.querySelector('img')).toBeNull();
|
||||
expect(host.querySelector('.actor-badge.agent')!.textContent!.trim()).toBe(payload);
|
||||
});
|
||||
|
||||
// Codex round 11 — the badge element itself carries the isolation, since
|
||||
// it sits inline with the verb, item ref and timestamp on the row.
|
||||
it('isolates the badge so a bidi control cannot reorder the row', async () => {
|
||||
await mountPage('audit', [act({ metadata: JSON.stringify({ agent: 'wren\u202egnimalb' }) })]);
|
||||
|
||||
const badge = host.querySelector('.actor-badge.agent')!;
|
||||
expect(badge.tagName).toBe('BDI');
|
||||
expect(badge.textContent!.trim()).toBe('wren\u202egnimalb');
|
||||
});
|
||||
|
||||
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');
|
||||
});
|
||||
});
|
||||
@@ -1,6 +1,7 @@
|
||||
<script lang="ts">
|
||||
import { onMount } from 'svelte';
|
||||
import { adminFetch } from '$lib/stores/admin.svelte';
|
||||
import { agentNameOf } from '$lib/utils/agentActor';
|
||||
import Chip from '$lib/components/common/Chip.svelte';
|
||||
import EmptyState from '$lib/components/common/EmptyState.svelte';
|
||||
|
||||
@@ -102,10 +103,28 @@
|
||||
});
|
||||
}
|
||||
|
||||
function formatMetadata(metadata: string | undefined, action: string): string {
|
||||
if (!metadata) return '\u2014';
|
||||
/** One parse per row, shared by every consumer below — the row's metadata
|
||||
* is read by both the User and Details columns, and parsing it twice on a
|
||||
* table that grows through "Load more" is pure waste. Returns null when
|
||||
* there is nothing parseable, which both callers treat as "no data". */
|
||||
function parseMetadata(metadata: string | undefined): Record<string, any> | null {
|
||||
if (!metadata) return null;
|
||||
try {
|
||||
return JSON.parse(metadata);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function formatMetadata(data: Record<string, any> | null, action: string): string {
|
||||
if (!data) return '\u2014';
|
||||
// The try still wraps the FORMATTERS, not just the parse it used to
|
||||
// share with. Hoisting the parse out narrowed this guard to nothing,
|
||||
// and the branches below can genuinely throw on well-formed JSON —
|
||||
// `String(data.keys)` on `{"keys":{"toString":null}}` cannot convert
|
||||
// to a primitive. That used to render an em dash; it must not take
|
||||
// the whole audit page down instead (codex round 6).
|
||||
try {
|
||||
const data = JSON.parse(metadata);
|
||||
switch (action) {
|
||||
case 'role_changed':
|
||||
if (data.old_role && data.new_role) return `${data.old_role} \u2192 ${data.new_role}`;
|
||||
@@ -162,11 +181,48 @@
|
||||
}
|
||||
}
|
||||
|
||||
function displayUser(entry: Activity): string {
|
||||
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;
|
||||
return 'Unknown';
|
||||
// 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.
|
||||
//
|
||||
// Returns the PARTS, never a joined string. The agent half is
|
||||
// attacker-chosen text and the human half is not, so building
|
||||
// `${agent} (via ${human})` would let a writer pick a name that forges the
|
||||
// construction (`admin (via root)`), or that carries a bidi control
|
||||
// character and visually reorders the suffix it was appended to — the
|
||||
// audited party editing how the audit reads. Kept apart, the template can
|
||||
// isolate each one, which bounds a hostile name to its own element instead
|
||||
// of letting it rewrite its neighbours (codex round 8).
|
||||
//
|
||||
// This is not a rendering quirk, it is ResolveAgentName's documented
|
||||
// attribution-honesty problem arriving through the renderer. That contract
|
||||
// (internal/cli/agent_identity.go, "WHAT THIS IS NOT") says the header
|
||||
// records honesty rather than identity, because the actor authors it. A
|
||||
// surface that COMPOSES with an authored value inherits that: it hands the
|
||||
// author influence over the parts they did not write. So the rule for any
|
||||
// future edit here — including a new column, a tooltip, an export or a
|
||||
// search-result summary — is that a self-declared value is a leaf, never a
|
||||
// fragment something else is built around.
|
||||
function displayUser(
|
||||
entry: Activity,
|
||||
meta: Record<string, any> | null
|
||||
): { agent?: string; account?: string } {
|
||||
const agent = entry.actor === 'agent' ? agentNameOf(meta) : undefined;
|
||||
if (agent) return { agent, account: entry.actor_name || undefined };
|
||||
if (entry.actor_name) return { account: entry.actor_name };
|
||||
if (entry.actor === 'system') return { account: 'System' };
|
||||
if (entry.user_id) {
|
||||
return {
|
||||
account:
|
||||
entry.user_id.length > 12 ? entry.user_id.slice(0, 12) + '\u2026' : entry.user_id
|
||||
};
|
||||
}
|
||||
return { account: 'Unknown' };
|
||||
}
|
||||
|
||||
async function loadEntries(append = false) {
|
||||
@@ -270,17 +326,29 @@
|
||||
</thead>
|
||||
<tbody>
|
||||
{#each entries as entry (entry.id)}
|
||||
{@const meta = parseMetadata(entry.metadata)}
|
||||
{@const who = displayUser(entry, meta)}
|
||||
<tr>
|
||||
<td class="time-cell" title={new Date(entry.created_at).toISOString()}>
|
||||
{relativeTime(entry.created_at)}
|
||||
</td>
|
||||
<td>{displayUser(entry)}</td>
|
||||
<td class="user-cell">
|
||||
<!-- <bdi> per name: an agent's is self-declared text and may
|
||||
contain bidi controls; isolation stops it reordering the
|
||||
" (via " literal or the account name beside it. -->
|
||||
{#if who.agent}
|
||||
<bdi class="agent-name" title={who.agent}>{who.agent}</bdi>
|
||||
{#if who.account}<span class="via"
|
||||
>(via <bdi>{who.account}</bdi>)</span
|
||||
>{/if}
|
||||
{:else}<bdi>{who.account}</bdi>{/if}
|
||||
</td>
|
||||
<td>
|
||||
<Chip size="sm" color={actionColor(entry.action)}>
|
||||
{formatAction(entry.action)}
|
||||
</Chip>
|
||||
</td>
|
||||
<td class="details-cell">{formatMetadata(entry.metadata, entry.action)}</td>
|
||||
<td class="details-cell">{formatMetadata(meta, entry.action)}</td>
|
||||
<td class="ip-cell">{entry.ip_address || '\u2014'}</td>
|
||||
</tr>
|
||||
{/each}
|
||||
@@ -374,6 +442,25 @@
|
||||
.time-cell {
|
||||
white-space: nowrap;
|
||||
}
|
||||
/* Bounded for the same reason .details-cell is: an agent name is arbitrary
|
||||
client-supplied text, and one very long or combining-heavy name must not
|
||||
be able to widen the table for everyone reading it. The full value stays
|
||||
on the title attribute. */
|
||||
.user-cell {
|
||||
max-width: 260px;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
white-space: nowrap;
|
||||
}
|
||||
.agent-name {
|
||||
font-weight: 600;
|
||||
}
|
||||
/* Visually distinct from the self-declared half, so a name that spells out
|
||||
"(via someone)" cannot pass itself off as this part of the cell. */
|
||||
.via {
|
||||
color: var(--text-muted);
|
||||
font-size: 0.85em;
|
||||
}
|
||||
.details-cell {
|
||||
max-width: 300px;
|
||||
overflow: hidden;
|
||||
|
||||
@@ -0,0 +1,222 @@
|
||||
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<unknown>>());
|
||||
|
||||
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> = {}): 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<string, unknown> | 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<void> {
|
||||
adminFetchMock.mockResolvedValue(rows);
|
||||
app = mount(AuditLogPage, { target: host, props: {} }) as Record<string, unknown>;
|
||||
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() ?? ''
|
||||
);
|
||||
}
|
||||
|
||||
/** The details column — fourth cell, after time / user / action. */
|
||||
function detailCells(): string[] {
|
||||
return [...host.querySelectorAll('tbody tr')].map(
|
||||
(tr) => tr.querySelectorAll('td')[3]?.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']);
|
||||
});
|
||||
|
||||
// Codex round 8. The agent half of this cell is attacker-chosen text and
|
||||
// the account half is not, so the two must not be concatenated into one
|
||||
// string: a writer could pick a name that forges the "(via …)" suffix, or
|
||||
// that carries U+202E and visually reorders the suffix appended after it.
|
||||
// The audited party would then be editing how the audit reads.
|
||||
it('keeps a forged "(via …)" inside the self-declared half', async () => {
|
||||
await mountWith([row({ metadata: JSON.stringify({ agent: 'admin (via root)' }) })]);
|
||||
|
||||
// Exactly one real "via" element, and it names the actual account.
|
||||
const via = host.querySelectorAll('.user-cell .via');
|
||||
expect(via).toHaveLength(1);
|
||||
expect(via[0].textContent!.trim()).toBe('(via Dave)');
|
||||
// The forgery is text inside the agent's own element, not structure.
|
||||
expect(host.querySelector('.user-cell .agent-name')!.textContent).toBe('admin (via root)');
|
||||
});
|
||||
|
||||
it('isolates each name so a bidi control cannot reorder its neighbours', async () => {
|
||||
// U+202E RIGHT-TO-LEFT OVERRIDE reorders everything after it until the
|
||||
// end of its isolate. <bdi> is what makes "until the end" mean "this
|
||||
// name", rather than the rest of the cell.
|
||||
await mountWith([row({ metadata: JSON.stringify({ agent: 'wrengnimalb' }) })]);
|
||||
|
||||
const agentEl = host.querySelector('.user-cell .agent-name')!;
|
||||
expect(agentEl.tagName).toBe('BDI');
|
||||
expect(host.querySelector('.user-cell .via bdi')!.tagName).toBe('BDI');
|
||||
// The account name is in its own isolate, so it is still a whole,
|
||||
// separately-ordered value no matter what the agent name contains.
|
||||
expect(host.querySelector('.user-cell .via bdi')!.textContent).toBe('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 })]);
|
||||
|
||||
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']);
|
||||
});
|
||||
|
||||
// Codex round 6. Hoisting the row's parse out of formatMetadata left its
|
||||
// try/catch wrapped around nothing, so a formatter that throws on
|
||||
// well-formed JSON — `String(data.keys)` cannot convert `{"toString":null}`
|
||||
// to a primitive — would take the whole page down instead of rendering an
|
||||
// em dash. These drive the Details column, which no test here touched, and
|
||||
// they fail against the narrowed guard.
|
||||
it('renders an em dash when a formatter throws on well-formed metadata', async () => {
|
||||
await mountWith([
|
||||
row({
|
||||
actor: 'user',
|
||||
action: 'settings_changed',
|
||||
metadata: '{"keys":{"toString":null}}'
|
||||
})
|
||||
]);
|
||||
|
||||
expect(detailCells()).toEqual(['—']);
|
||||
// The row still rendered at all — the counterfactual is a blank table.
|
||||
expect(userCells()).toEqual(['Dave']);
|
||||
});
|
||||
|
||||
it('formats a known action from the shared parse', async () => {
|
||||
// Proves the hoisted object actually reaches formatMetadata, not just
|
||||
// displayUser: a hoist that passed the wrong value would em-dash here.
|
||||
await mountWith([
|
||||
row({
|
||||
actor: 'user',
|
||||
action: 'role_changed',
|
||||
metadata: JSON.stringify({ old_role: 'editor', new_role: 'admin' })
|
||||
})
|
||||
]);
|
||||
|
||||
expect(detailCells()).toEqual(['editor → admin']);
|
||||
});
|
||||
|
||||
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…']);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,181 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { flushSync, mount, unmount, tick } from 'svelte';
|
||||
import { page } from '$app/state';
|
||||
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.
|
||||
*
|
||||
* 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<unknown>>());
|
||||
|
||||
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');
|
||||
|
||||
function act(over: Partial<RecentActivity> = {}): RecentActivity {
|
||||
return {
|
||||
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
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* 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: RecentActivity[]) {
|
||||
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<string, unknown> | null = null;
|
||||
|
||||
async function mountWith(recent: RecentActivity[]): Promise<void> {
|
||||
dashboardGet.mockResolvedValue(dashboard(recent));
|
||||
app = mount(DashboardPage, { target: host, props: {} }) as Record<string, unknown>;
|
||||
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'],
|
||||
// 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 })]);
|
||||
|
||||
// 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']);
|
||||
});
|
||||
|
||||
// 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);
|
||||
});
|
||||
|
||||
// Codex round 11 — same claim at this binding: the badge element is the
|
||||
// isolate, and a <span> would satisfy every text assertion above.
|
||||
it('isolates the badge so a bidi control cannot reorder the row', async () => {
|
||||
await mountWith([act({ metadata: JSON.stringify({ agent: 'wren\u202egnimalb' }) })]);
|
||||
|
||||
const badge = host.querySelector('.activity-row .actor-badge.agent')!;
|
||||
expect(badge.tagName).toBe('BDI');
|
||||
expect(badge.textContent!.trim()).toBe('wren\u202egnimalb');
|
||||
});
|
||||
|
||||
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');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user