From d564126b31a83d438e8e43b2958634f5789cf8fd Mon Sep 17 00:00:00 2001 From: ribdsp <113304041+ribdsp@users.noreply.github.com> Date: Mon, 31 Aug 2026 15:28:29 +0700 Subject: [PATCH] feat: add read_markers so the agent can see human markers --- CLAUDE.md | 6 +- README.md | 7 +- docs/architecture.md | 4 +- docs/tools.md | 49 +++++- traces/next.config.mjs | 2 +- traces/src/app/tool-surface.tsx | 2 +- traces/src/components/agent/agent-lane.tsx | 2 +- .../components/player/stage-empty-state.tsx | 2 +- traces/src/components/ui/webmcp-badge.tsx | 6 +- traces/src/lib/webmcp/register-tools.test.ts | 6 +- traces/src/lib/webmcp/register-tools.ts | 10 +- traces/src/lib/webmcp/tools/index.ts | 6 + traces/src/lib/webmcp/tools/read-markers.ts | 143 ++++++++++++++++++ .../webmcp/tools/read-search-errors.test.ts | 111 +++++++++++++- traces/src/lib/webmcp/tools/registry.test.ts | 10 +- traces/src/types/webmcp.d.ts | 4 +- 16 files changed, 335 insertions(+), 35 deletions(-) create mode 100644 traces/src/lib/webmcp/tools/read-markers.ts diff --git a/CLAUDE.md b/CLAUDE.md index 7352853..7f7b44b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,7 +7,7 @@ before your first edit, then `CONTRIBUTING.md`. 1. **`CONTRIBUTING.md`** — the three non-negotiable rules, code conventions, testing policy 2. **`docs/architecture.md`** — how the pieces fit, including the module-level engine handle -3. **`docs/tools.md`** — the 16-tool contract +3. **`docs/tools.md`** — the 17-tool contract 4. **`docs/agent-legible-dom.md`** — only if you touch `lib/dom` ## One area per change @@ -28,7 +28,7 @@ than none. Whoever asked you to make this change will tell you your scope. One stub is left, and it still carries its marker. `registerDynamicTool` in `traces/src/lib/webmcp/register-tools.ts:133` throws `registerDynamicTool: not implemented`, and nothing -in the codebase calls it — so promoting a hypothesis does not grow a 17th tool. The convention is +in the codebase calls it — so promoting a hypothesis does not grow an 18th tool. The convention is documented here because the rule outlives the markers: a marker names the person who owned the work and the day it was due, so **deleting one while implementing around it destroys the only record of who owes what.** If you add a marker, name yourself in it. If you find one, either implement it or leave it @@ -36,7 +36,7 @@ exactly where it is. ## The tests are green, and two suites must stay honest -All 303 tests pass. That is worth stating because of how some of them got there: the `compress-dom`, +All 309 tests pass. That is worth stating because of how some of them got there: the `compress-dom`, `bisect` and `evaluatePredicate` suites were written first, as specifications, and were red for as long as it took the implementations to satisfy them. **Never** make a test in those suites pass by weakening an assertion, adding `.skip`, or deleting a case: that converts a specification into a lie, silently. A diff --git a/README.md b/README.md index cf4e984..8151714 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ That's the gap Traces fills. ## What it does -Traces loads an rrweb recording in the browser and registers **16 WebMCP tools** on +Traces loads an rrweb recording in the browser and registers **17 WebMCP tools** on `document.modelContext`. An agent connected to the page can then read the session, search across time, take actions on the timeline, and — this is the part we care most about — **ask the human questions**. @@ -184,7 +184,7 @@ the design better. --- -## The 16 tools +## The 17 tools Full contracts, argument shapes, and edge-case behaviour in **[docs/tools.md](docs/tools.md)**. @@ -206,6 +206,7 @@ Full contracts, argument shapes, and edge-case behaviour in **[docs/tools.md](do | 14 | `propose_report` | blocking | a bug report draft the human edits and approves | | 15 | `claim_next_task` | blocking | pull the next task from the agent lane | | 16 | `snapshot_finding` | write | save a finding | +| 17 | `read_markers` | read | every marker on the timeline, the human's included | Four of them **block**: `execute()` does not resolve until a person acts. Because a call can't hang forever, each one returns `{ status: "pending", ticket }` on timeout instead of leaving the agent @@ -374,7 +375,7 @@ goes nowhere; there is no upload endpoint to send it to. Traces/ ├── docs/ │ ├── architecture.md how it's put together, and why -│ ├── tools.md the 16-tool contract +│ ├── tools.md the 17-tool contract │ ├── agent-legible-dom.md the DOM compressor spec │ └── threat-model.md what we defend against ├── traces/ the app — this is the deployed URL diff --git a/docs/architecture.md b/docs/architecture.md index 4964aa8..a100149 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -15,7 +15,7 @@ Traces/ ├── CONTRIBUTING.md ├── docs/ │ ├── architecture.md this file -│ ├── tools.md the 16-tool contract +│ ├── tools.md the 17-tool contract │ ├── agent-legible-dom.md the DOM compressor spec │ └── threat-model.md ├── traces/ the app itself — the deployed URL @@ -84,7 +84,7 @@ traces/ │ │ ├── register-tools.ts every registerTool call, one place │ │ ├── blocking.ts the human-in-the-loop gate │ │ ├── polyfill.ts - │ │ └── tools/ one file per tool, 16 of them, plus index.ts and registry.test.ts + │ │ └── tools/ one file per tool, 17 of them, plus index.ts and registry.test.ts │ ├── store/session.ts single state store │ └── report/build-report.ts reconstructs steps from real events └── types/ diff --git a/docs/tools.md b/docs/tools.md index 7843458..5ebcc45 100644 --- a/docs/tools.md +++ b/docs/tools.md @@ -1,6 +1,6 @@ # Tool reference -Traces registers **16 WebMCP tools** on `document.modelContext`. This document is the contract: what +Traces registers **17 WebMCP tools** on `document.modelContext`. This document is the contract: what each tool takes, what it returns, and the rules an agent needs to know to use it correctly. Every tool returns `{ content: [{ type: "text", text: ... }] }`. The WebMCP spec currently defines @@ -39,6 +39,7 @@ bending the type, and those additions are listed per tool. | 14 | `propose_report` | **blocking** | a bug report draft the human edits and approves | | 15 | `claim_next_task` | **blocking** | pull the next task from the agent lane | | 16 | `snapshot_finding` | write | save markers, hypotheses and the report draft | +| 17 | `read_markers` | read | every marker in a window, the human's included, rejections flagged | **Blocking** means exactly what it says: `execute()` does not resolve until a person acts, or until the gate's timeout hands back a ticket. The agent's own loop waits. See @@ -48,7 +49,7 @@ the gate's timeout hands back a ticket. The agent's own loop waits. See ## Conventions every tool follows -Stated once here rather than repeated sixteen times. +Stated once here rather than repeated seventeen times. - **Times are milliseconds from the start of the recording**, `0..durationMs`, never epoch. Call `read_session_meta` first to learn `durationMs`. @@ -703,6 +704,50 @@ findings are still on screen and the message says to copy them out of the panel. --- +## Reading the investigation + +### 17. `read_markers({ from?, to? })` + +``` +input: { from?: number, to?: number } +output: { fromMs, toMs, + markers: [{ id, atMs, label, severity, author, rejected? }], + totalMatched, humanCount, agentCount, truncated, note? } +``` + +The read half of `annotate` (§11), and it is numbered last because it was built last, not because it +is a saving tool — every other read tool answers a question about the *recording*, this one answers a +question about the *investigation*. + +**It returns the human's markers as well as the agent's**, which is the reason it exists. +`ask_human_visual` (§12) lands the human's answer on the timeline as a marker precisely so it is +evidence anyone can click; without this tool "anyone" excluded the agent, which could write markers +and never read one back. A session resumed after a reload, or picked up by a second agent, started +blind to the moments a person had already pointed at. + +`author` is the frozen `Author` — `"human"` or `"agent"` — and `humanCount`/`agentCount` are counted +over everything the window matched, not over what survived the cap. + +**A rejected marker comes back flagged, not filtered.** `rejected: true` means the human dismissed +it; the field is *absent* rather than `false` on a marker that stands, so it is not noise on every +entry. Filtering them out would leave an agent free to re-propose exactly what a person has already +thrown away. + +- Capped at 40 markers, the **human's kept ahead of the agent's** when the cap bites, then + chronological order restored. The agent already holds the ids of everything it pinned itself, from + `annotate`'s replies, so its own are the ones it can most afford to lose. `annotate` caps the agent + at 40 markers on one timeline, so the total can exceed 40 only once a human has marked as well. +- Returned in timeline order, not the order the markers were made in. Markers are added at any + timestamp at any time, and a timeline is read as a sequence. +- `label` is capped at 80 characters, the same ceiling `annotate` and `ask_human_visual` enforce when + writing. +- An empty result carries a note saying so in words. "No markers in this window" and an empty list + are the same fact, but only one of them survives being skimmed. +- **A loaded recording is required**, like every other read tool. A marker is a timestamp into a + recording, and answering "no markers" with nothing loaded would read as "the human marked nothing". + +--- + ## Predicates Predicates are a **closed, validated set of structured objects**. Traces never evaluates a string diff --git a/traces/next.config.mjs b/traces/next.config.mjs index f7c94ad..0654663 100644 --- a/traces/next.config.mjs +++ b/traces/next.config.mjs @@ -38,7 +38,7 @@ const nextConfig = { * Origin isolation is not optional, and it is not part of the trial: WebMCP refuses to register a * tool unless the document is origin-isolated, so this header ships whether or not a token is * configured. Sending only `Origin-Trial` produces the worst available failure — `document - * .modelContext` exists, so the banner reports `native` in green, while all sixteen + * .modelContext` exists, so the banner reports `native` in green, while all seventeen * `registerTool` calls throw and `registerTools` returns an empty array. The page looks healthy * and nothing on it is agent-callable. * diff --git a/traces/src/app/tool-surface.tsx b/traces/src/app/tool-surface.tsx index e2b4334..8feb925 100644 --- a/traces/src/app/tool-surface.tsx +++ b/traces/src/app/tool-surface.tsx @@ -31,7 +31,7 @@ export function ToolSurface() { * `registerTools` is async because the spec's `registerTool` rejects rather than throws. The flag * is what keeps React 19's double mount honest: the first pass is aborted on cleanup and resolves * with nothing registered, and without this guard that empty result can land after the second - * pass's real one and grey out a banner over sixteen live tools. + * pass's real one and grey out a banner over seventeen live tools. */ let active = true diff --git a/traces/src/components/agent/agent-lane.tsx b/traces/src/components/agent/agent-lane.tsx index 9f19469..d06b7b4 100644 --- a/traces/src/components/agent/agent-lane.tsx +++ b/traces/src/components/agent/agent-lane.tsx @@ -17,7 +17,7 @@ import type { Task, TaskStatus } from '@/types/domain' * A human types a task; the agent picks it up by calling `claim_next_task`, which blocks until one * exists. That inversion is the interesting part — the agent waits on the person rather than the * person waiting on the agent — and it means the lane is not a UI convenience, it is the queue that - * one of the sixteen tools reads from. + * one of the seventeen tools reads from. * * Show `claimed` distinctly from `open`. Watching a task flip to claimed a second after you typed it, * with no click in between, is the clearest demonstration in the whole app that something else is diff --git a/traces/src/components/player/stage-empty-state.tsx b/traces/src/components/player/stage-empty-state.tsx index f1a19fc..ab71650 100644 --- a/traces/src/components/player/stage-empty-state.tsx +++ b/traces/src/components/player/stage-empty-state.tsx @@ -175,7 +175,7 @@ export function StageEmptyState() { Getting WebMCP

- WebMCP is how an agent finds the sixteen tools on this page. Turn it on in ChatGPT Desktop or + WebMCP is how an agent finds the seventeen tools on this page. Turn it on in ChatGPT Desktop or Chrome, then the header pill should read live. Replay still works without it — only the agent needs the tools.

diff --git a/traces/src/components/ui/webmcp-badge.tsx b/traces/src/components/ui/webmcp-badge.tsx index 28bd1d7..58f034f 100644 --- a/traces/src/components/ui/webmcp-badge.tsx +++ b/traces/src/components/ui/webmcp-badge.tsx @@ -18,7 +18,7 @@ import type { RegistrationResult } from '@/lib/webmcp/register-tools' * * - **The tool list comes from the host, not from us.** `document.modelContext.getTools()` reports what * the browser actually holds, so a tool the host rejected cannot appear here. Reading our own - * `allTools` array instead would render sixteen confident cards on a page where zero are callable. + * `allTools` array instead would render seventeen confident cards on a page where zero are callable. * - **The panel is a dropdown, not a dock.** Opening it from the header means it can be dismissed, so * the old rule that a red panel had no close button does not apply: the thing that must not be * hideable is the banner row, and that row is still in flow with no close control. @@ -169,7 +169,7 @@ export function WebMcpPanel({ registration }: { registration: RegistrationResult

{tool.name}

{/* Two lines, hard. `firstSentence` is already the short form and it is still six lines - wide for `read_session_meta` in a 145px column, which turns sixteen cards into a wall + wide for `read_session_meta` in a 145px column, which turns seventeen cards into a wall of prose nobody reads. The clamp is what makes this a scannable index; `title` on the card keeps the sentence available to anyone who wants it. */} @@ -187,7 +187,7 @@ export function WebMcpPanel({ registration }: { registration: RegistrationResult * Whether tools are available, in one sentence, per state. * * `polyfill` says the quiet part out loud: the count is real and the tools work from this page, but no - * external agent can see any of them. A judge reading "16 tools" beside an amber dot deserves to know + * external agent can see any of them. A judge reading "17 tools" beside an amber dot deserves to know * which of those two facts they are looking at. */ function StatusSentence({ health, count }: { health: Health; count: number }) { diff --git a/traces/src/lib/webmcp/register-tools.test.ts b/traces/src/lib/webmcp/register-tools.test.ts index 23872bf..049ba83 100644 --- a/traces/src/lib/webmcp/register-tools.test.ts +++ b/traces/src/lib/webmcp/register-tools.test.ts @@ -9,8 +9,8 @@ import { registerTools, unregisterTools } from './register-tools' * These exist because of a specific bug rather than for coverage. `registerTool` returns a promise and * every failure the spec defines — `InvalidStateError`, `SecurityError` for an agent cluster that is not * origin-keyed, `NotAllowedError`, a duplicate name, a bad schema — arrives as a *rejection*. The old - * implementation wrapped the call in a synchronous `try`/`catch`, which caught none of them: all sixteen - * names went into `registered` unconditionally and the banner read "WebMCP live · 16 tools" over an empty + * implementation wrapped the call in a synchronous `try`/`catch`, which caught none of them: all seventeen + * names went into `registered` unconditionally and the banner read "WebMCP live · 17 tools" over an empty * tool list. A green banner on a dead surface is the one failure this project cannot afford, so the * assertion below is about the *absence* of names, not the presence of them. * @@ -76,7 +76,7 @@ describe('registerTools', () => { it('reports nothing registered when the host refuses everything', async () => { // The failure next.config.mjs calls "the worst available failure": an origin that is not - // origin-keyed refuses every tool, and the banner used to call that sixteen live tools. + // origin-keyed refuses every tool, and the banner used to call that seventeen live tools. vi.spyOn(console, 'warn').mockImplementation(() => {}) document.modelContext = stubModelContext({ names: allTools.map((tool) => tool.name), diff --git a/traces/src/lib/webmcp/register-tools.ts b/traces/src/lib/webmcp/register-tools.ts index bd0b0a7..2b8db65 100644 --- a/traces/src/lib/webmcp/register-tools.ts +++ b/traces/src/lib/webmcp/register-tools.ts @@ -30,7 +30,7 @@ export type RegistrationResult = { * * Async because `registerTool` returns a promise and every failure the spec defines arrives as a * rejection — not a thrown exception. A synchronous `try`/`catch` here caught nothing, so a page whose - * agent cluster is not origin-keyed reported sixteen live tools while registering zero. That is the + * agent cluster is not origin-keyed reported seventeen live tools while registering zero. That is the * failure next.config.mjs calls "the worst available failure", and this is the only place that can see * it. */ @@ -58,13 +58,13 @@ export async function registerTools(): Promise { controller = surface /* - * Concurrently, and `allSettled` rather than `all`: sixteen sequential awaits is sixteen round trips - * through the host for no reason, and one host rejecting one schema must not cost us the other - * fifteen. A surface that is fifteen-sixteenths present is worth having, and the banner shows what + * Concurrently, and `allSettled` rather than `all`: seventeen sequential awaits is seventeen round + * trips through the host for no reason, and one host rejecting one schema must not cost us the other + * sixteen. A surface that is sixteen-seventeenths present is worth having, and the banner shows what * actually registered rather than what we hoped would. * * `async` on the mapper is not decoration: a host that throws synchronously instead of rejecting - * would otherwise escape `allSettled` through `map` and cost all sixteen. + * would otherwise escape `allSettled` through `map` and cost all seventeen. */ const outcomes = await Promise.allSettled( allTools.map(async (tool) => diff --git a/traces/src/lib/webmcp/tools/index.ts b/traces/src/lib/webmcp/tools/index.ts index 37fe585..e25345d 100644 --- a/traces/src/lib/webmcp/tools/index.ts +++ b/traces/src/lib/webmcp/tools/index.ts @@ -8,6 +8,7 @@ import { bisectTool } from './bisect' import { diffDomToolDefinition } from './diff-dom' import { readConsoleTool } from './read-console' import { readNetworkTool } from './read-network' +import { readMarkersTool } from './read-markers' import { measureLayoutToolDefinition } from './measure-layout' import { seekTool } from './seek' import { annotateTool } from './annotate' @@ -41,6 +42,10 @@ export const allTools: ToolDefinition[] = [ readDomAtTool, readConsoleTool, readNetworkTool, + // Last of the read group, not first: it is the only one that answers a question about the + // investigation rather than about the recording, and a model scanning for "read the page" must not + // land on it. + readMarkersTool, // search bisectTool, @@ -80,6 +85,7 @@ export { diffDomToolDefinition, readConsoleTool, readNetworkTool, + readMarkersTool, measureLayoutToolDefinition, seekTool, annotateTool, diff --git a/traces/src/lib/webmcp/tools/read-markers.ts b/traces/src/lib/webmcp/tools/read-markers.ts new file mode 100644 index 0000000..8bb2215 --- /dev/null +++ b/traces/src/lib/webmcp/tools/read-markers.ts @@ -0,0 +1,143 @@ +import { sessionState } from '@/lib/store/session' +import type { Marker } from '@/types/domain' +import { type ToolDefinition, json } from '../tool-types' +import { currentRecording, optionalWindow, truncate } from './tool-context' +import { MARKER_LABEL_MAX } from './tool-support' + +/** + * 'read_markers' — see docs/tools.md#17-read_markers for the full contract. + * + * The read half of `annotate`. Every other read tool answers a question about the recording; this one + * answers a question about the *investigation*, which until now the agent could only write to. It could + * pin a marker and never see it again, and it could not see the human's markers at all — so a session + * resumed after a reload, or picked up by a second agent, started blind to the moments a person had + * already pointed at. `ask_human_visual` lands the human's answer on the timeline as a marker + * (ask-human-visual.ts:284) precisely so it is evidence anyone can click; this is what makes "anyone" + * include the agent. + * + * Read straight off the store rather than out of `lib/`, because markers are session state and not + * something derivable from the events — there is no pure function to wrap. It still requires a loaded + * recording: a marker is a timestamp into a recording, and answering "no markers" when nothing is + * loaded would be read as "the human marked nothing", which is a different and much more misleading + * fact. + * + * Rejected markers are returned, flagged, not filtered. `rejectMarker` keeps them so undo works, and an + * agent that cannot see the rejection re-proposes the thing a human has already dismissed. + */ + +/** Response budget, matching list_events, read_console and read_network. */ +const MARKER_LIMIT = 40 + +type MarkerEntry = { + id: string + atMs: number + label: string + severity: Marker['severity'] + author: Marker['author'] + /** Present only when true — see the note on rejected markers above. */ + rejected?: boolean +} + +/** + * Both writers already cap a label at MARKER_LABEL_MAX, so this truncation never fires today. It is + * here because the response budget is this tool's own responsibility: a future marker path that forgets + * the cap should not be able to widen a tool response from somewhere else in the codebase. + */ +function toEntry(marker: Marker): MarkerEntry { + return { + id: marker.id, + atMs: marker.timestamp, + label: truncate(marker.label, MARKER_LABEL_MAX), + severity: marker.severity, + author: marker.author, + ...(marker.rejected === true ? { rejected: true } : {}), + } +} + +export const readMarkersTool: ToolDefinition = { + name: 'read_markers', + description: [ + 'List the markers pinned on the timeline inside a time window — what each one says, when it is, who', + 'made it, and whether a human rejected it. Use it to see what the person watching has already', + 'pointed at before you start looking, and to check what you pinned earlier survived: markers are the', + 'shared notes on this recording, and yours are only half of them. Pass a marker\'s atMs to seek to', + 'watch that moment.', + ].join(' '), + + // No `untrustedContentHint`: a marker label is written by the human or by this agent, never lifted out + // of the recorded page. Same reasoning as measure_layout, and the same as claim_next_task, which + // returns human-typed task text and sets no flag either. + annotations: { readOnlyHint: true }, + + inputSchema: { + type: 'object', + properties: { + from: { + type: 'number', + description: + 'Start of the window, in ms from the start of the recording. Defaults to 0, which is usually what you want here — there are rarely many markers.', + }, + to: { + type: 'number', + description: + 'End of the window, in ms from the start of the recording. Defaults to the end of the recording (durationMs from read_session_meta).', + }, + }, + additionalProperties: false, + }, + + async execute(args) { + const recording = currentRecording() + if (!recording.ok) return recording.response + + const window = optionalWindow(args, recording.value) + if (!window.ok) return window.response + + const matched = sessionState() + .markers.filter( + (marker) => marker.timestamp >= window.value.fromMs && marker.timestamp <= window.value.toMs, + ) + .map(toEntry) + // Store order is insertion order, and a marker can be added at any timestamp at any time. A + // timeline is read as a sequence, so it is sorted here rather than left in the order it was typed. + .sort((left, right) => left.atMs - right.atMs) + + const humanCount = matched.filter((marker) => marker.author === 'human').length + + const truncated = matched.length > MARKER_LIMIT + const kept = truncated + ? // The human's markers survive the cap first: the agent already holds the ids of everything it + // pinned itself, from annotate's own replies, so its own are the ones it can most afford to lose. + // Chronological order is then restored. Array#sort is stable, so equal ranks keep their positions. + [...matched] + .sort((left, right) => Number(right.author === 'human') - Number(left.author === 'human')) + .slice(0, MARKER_LIMIT) + .sort((left, right) => left.atMs - right.atMs) + : matched + + return json({ + fromMs: window.value.fromMs, + toMs: window.value.toMs, + markers: kept, + totalMatched: matched.length, + humanCount, + agentCount: matched.length - humanCount, + truncated, + ...(truncated + ? { + note: + `${matched.length} markers are in this window and ${MARKER_LIMIT} are shown, the human's kept first. ` + + 'Narrow the window around the moment you are working on.', + } + : {}), + ...(matched.length === 0 + ? { + note: + 'No markers in this window. Nothing has been pinned here yet — by you or by the human — so ' + + 'there is no earlier finding to build on. Call annotate once you can name what is wrong at a ' + + 'specific moment.', + } + : {}), + }) + }, +} diff --git a/traces/src/lib/webmcp/tools/read-search-errors.test.ts b/traces/src/lib/webmcp/tools/read-search-errors.test.ts index c297973..b6cec83 100644 --- a/traces/src/lib/webmcp/tools/read-search-errors.test.ts +++ b/traces/src/lib/webmcp/tools/read-search-errors.test.ts @@ -1,5 +1,5 @@ import { beforeEach, describe, expect, it } from 'vitest' -import type { Recording, RrwebEvent } from '@/types/domain' +import type { Author, Recording, RrwebEvent, Severity } from '@/types/domain' import { useSessionStore } from '@/lib/store/session' import { bisectTool } from './bisect' import { diffDomToolDefinition } from './diff-dom' @@ -8,12 +8,13 @@ import { listEventsTool } from './list-events' import { measureLayoutToolDefinition } from './measure-layout' import { readConsoleTool } from './read-console' import { readDomAtTool } from './read-dom-at' +import { readMarkersTool } from './read-markers' import { readNetworkTool } from './read-network' import { readSessionMetaTool } from './read-session-meta' import type { ToolDefinition, ToolResponse } from '../tool-types' /** - * Rejection paths for the nine read/search wrappers. + * Rejection paths for the ten read/search wrappers. * * These test the wrappers' own logic and nothing else: argument validation, the "not ready" and "no * recording" replies, and the two places where a wrapper reads data the digest does not expose. The @@ -33,6 +34,7 @@ const READ_AND_SEARCH_TOOLS: ToolDefinition[] = [ readDomAtTool, readConsoleTool, readNetworkTool, + readMarkersTool, bisectTool, diffDomToolDefinition, measureLayoutToolDefinition, @@ -46,6 +48,7 @@ const VALID_ARGS: Record> = { read_dom_at: { timestamp: 0 }, read_console: {}, read_network: {}, + read_markers: {}, bisect: { selector: 'button', predicate: { kind: 'exists', equals: true }, from: 0, to: 1000 }, diff_dom: { from: 0, to: 1000 }, measure_layout: { selectors: ['button'], timestamp: 0 }, @@ -86,6 +89,15 @@ function load(events: RrwebEvent[]): void { useSessionStore.getState().loadRecording(recordingWith(events), []) } +/** + * A marker straight into the store, rather than through `annotate`. What is under test is what + * `read_markers` gives back, and `annotate` has a budget of its own that would cap the fixture before + * the read budget could bite. + */ +function mark(atMs: number, label: string, author: Author, severity: Severity = 'info'): string { + return useSessionStore.getState().addMarker({ timestamp: atMs, label, severity, author }) +} + async function call(tool: ToolDefinition, args: Record = {}): Promise { return tool.execute(args) } @@ -120,7 +132,7 @@ describe('read and search tools before the player has mounted', () => { }) it('still answer for the tools that only read the recording file', async () => { - for (const tool of [readSessionMetaTool, listEventsTool, readConsoleTool, readNetworkTool]) { + for (const tool of [readSessionMetaTool, listEventsTool, readConsoleTool, readNetworkTool, readMarkersTool]) { const response = await call(tool, {}) expect(response.isError, tool.name).toBeUndefined() } @@ -235,6 +247,99 @@ describe('read_network', () => { }) }) +describe('read_markers', () => { + type MarkersPayload = { + fromMs: number + toMs: number + markers: { id: string; atMs: number; label: string; severity: string; author: string; rejected?: boolean }[] + totalMatched: number + humanCount: number + agentCount: number + truncated: boolean + note?: string + } + + const readMarkers = async (args: Record = {}): Promise => + JSON.parse(textOf(await call(readMarkersTool, args))) as MarkersPayload + + beforeEach(() => { + load([]) + }) + + it('says nothing has been pinned rather than answering with an empty list alone', async () => { + const payload = await readMarkers() + + expect(payload.markers).toEqual([]) + expect(payload.totalMatched).toBe(0) + expect(payload.note).toMatch(/No markers in this window/) + }) + + it('returns both authors in timeline order, whatever order they were made in', async () => { + mark(6_000, 'agent looked here last', 'agent', 'warn') + mark(2_000, 'human looked here first', 'human') + + const payload = await readMarkers() + + expect(payload.markers.map((marker) => marker.atMs)).toEqual([2_000, 6_000]) + expect(payload.markers.map((marker) => marker.author)).toEqual(['human', 'agent']) + expect(payload.markers[0]?.label).toBe('human looked here first') + expect(payload.markers[1]?.severity).toBe('warn') + expect(payload.humanCount).toBe(1) + expect(payload.agentCount).toBe(1) + expect(payload.truncated).toBe(false) + }) + + it('flags a rejected marker instead of hiding it, so it is not proposed again', async () => { + const id = mark(3_000, 'the human disagreed with this', 'agent', 'error') + useSessionStore.getState().rejectMarker(id) + + const payload = await readMarkers() + + expect(payload.markers).toHaveLength(1) + expect(payload.markers[0]?.rejected).toBe(true) + }) + + it('omits the rejected field entirely when the marker stands', async () => { + mark(3_000, 'still standing', 'human') + + const payload = await readMarkers() + + // Absent rather than `false`: `rejected: false` on every marker is noise a model has to read past. + expect(payload.markers[0]).not.toHaveProperty('rejected') + }) + + it('leaves out markers outside the requested window', async () => { + mark(500, 'before the window', 'human') + mark(4_000, 'inside the window', 'agent') + mark(9_500, 'after the window', 'human') + + const payload = await readMarkers({ from: 1_000, to: 5_000 }) + + expect(payload.fromMs).toBe(1_000) + expect(payload.toMs).toBe(5_000) + expect(payload.markers.map((marker) => marker.label)).toEqual(['inside the window']) + // The count is of the window, not of the session: two markers exist that this call did not match. + expect(payload.totalMatched).toBe(1) + }) + + it("keeps the human's markers when the cap bites, and stays chronological", async () => { + for (let index = 0; index < 45; index += 1) mark(index * 100, `agent note ${index}`, 'agent') + mark(9_000, 'the human pinned this', 'human') + + const payload = await readMarkers() + + expect(payload.markers).toHaveLength(40) + expect(payload.totalMatched).toBe(46) + expect(payload.truncated).toBe(true) + expect(payload.note).toMatch(/the human's kept first/) + // It is the latest marker of the 46, so a plain chronological cap would have dropped it. + expect(payload.markers.some((marker) => marker.author === 'human')).toBe(true) + expect(payload.markers.map((marker) => marker.atMs)).toEqual( + [...payload.markers].sort((left, right) => left.atMs - right.atMs).map((marker) => marker.atMs), + ) + }) +}) + describe('list_events and read_console budgets', () => { it('reports the true match count alongside the capped list', async () => { const events: RrwebEvent[] = [] diff --git a/traces/src/lib/webmcp/tools/registry.test.ts b/traces/src/lib/webmcp/tools/registry.test.ts index 0db502a..02ab6ba 100644 --- a/traces/src/lib/webmcp/tools/registry.test.ts +++ b/traces/src/lib/webmcp/tools/registry.test.ts @@ -8,17 +8,17 @@ import { allTools, assertUniqueToolNames } from './index' * shadows another because a name got copy-pasted, a schema with no field descriptions, a description * that still reads like a TODO. None of it needs a browser, so it can run on every commit. * - * All 16 tools are implemented and every assertion below is green. That is the state to keep: these - * ran as a checklist while the tools were being filled in, and they read now as the standing rules a - * seventeenth tool has to meet before it joins `allTools`. + * All 17 tools are implemented and every assertion below is green. That is the state to keep: these + * ran as a checklist while the tools were being filled in, and they read now as the standing rules an + * eighteenth tool has to meet before it joins `allTools`. */ describe('tool registry', () => { it('has no duplicate names', () => { expect(() => assertUniqueToolNames()).not.toThrow() }) - it('exposes the 16 tools documented in docs/tools.md', () => { - expect(allTools).toHaveLength(16) + it('exposes the 17 tools documented in docs/tools.md', () => { + expect(allTools).toHaveLength(17) }) it('uses snake_case names, which is what the spec examples use', () => { diff --git a/traces/src/types/webmcp.d.ts b/traces/src/types/webmcp.d.ts index aeddd2b..f0f005a 100644 --- a/traces/src/types/webmcp.d.ts +++ b/traces/src/types/webmcp.d.ts @@ -92,8 +92,8 @@ interface ModelContext extends EventTarget { * `NotAllowedError` when the `tools` permission policy forbids it, plus a duplicate name, an empty * name or description, and an invalid `inputSchema`. * - * Typed as a promise rather than `void` because that is the difference between reporting sixteen - * live tools and reporting sixteen that a `try`/`catch` never saw fail. + * Typed as a promise rather than `void` because that is the difference between reporting seventeen + * live tools and reporting seventeen that a `try`/`catch` never saw fail. */ registerTool: ( descriptor: ModelContextToolDescriptor,