diff --git a/docs/superpowers/specs/2026-07-03-targeted-feedback-design.md b/docs/superpowers/specs/2026-07-03-targeted-feedback-design.md new file mode 100644 index 0000000..b9f4577 --- /dev/null +++ b/docs/superpowers/specs/2026-07-03-targeted-feedback-design.md @@ -0,0 +1,191 @@ +# Targeted feedback: pin console comments to panes and elements + +**Date:** 2026-07-03 +**Status:** Draft — awaiting review + +## Problem + +The experimental agent↔human console (issue #108) lets the human send free-text messages to the +listening agent, but a message is scoped to the whole board (`project/agent`). "The p95 line is +mislabeled" forces the agent to guess *which* chart, node, or section the human means. The human +should be able to pin a comment to a specific pane or UI element, and the agent should receive +enough structured context to resolve exactly what was meant — without a screenshot and without +guessing. + +## Goals + +- Pin a console message to a pane, flow node, component node (`props.id`), checklist item, or + markdown heading. +- Deliver the pin to the agent through the existing inbox channel, in terms the agent already + thinks in (spec ids, pane indexes, labels) — not DOM paths. +- Capture enough human-readable context at pin time (labels, pane title, text excerpt, board + version) that the agent understands the reference even if it can't or doesn't re-pull the board. +- Zero change to the security model: pinning rides the existing URL-gated `POST /inbox`. + +## Non-goals (explicitly out of scope for v1) + +- Persistent annotations or pin badges rendered on the board. The inbox stays a transient + conversation ring; a durable annotation layer is future work. +- Comment threading, replies, or resolution state. +- Vega-Lite mark-level, mermaid element-level, or calltree node targeting (no stable ids today). +- Flow *edge* targeting (thin hit targets, selector reliability unverified) — nodes only in v1. +- Multi-element selection or region/box selection. + +## Approaches considered + +1. **Extend the inbox event with a structured `target` + client-side context snapshot** + (**chosen**). Rides the entire existing console machinery — SSE echo, read receipts, unread + nudge, `termchart inbox` polling. Small server delta (validate one new optional field). The + agent gets spec-level identifiers it can resolve against `termchart pull`. +2. **DOM-path anchoring** (capture CSS path + bounding box for any element). Pins anything + immediately, but anchors are fragile across re-renders and the agent reasons in spec terms, + not DOM terms — poor fit for "the agent fully understands". +3. **Persistent annotation layer** (new store, pin badges, CRUD, new CLI verb). Durable and + threaded, but a large new surface for what is fundamentally a live feedback loop. YAGNI. + +Approach 1 wins: it extends a proven channel instead of building a parallel one. Approach 2's one +real advantage (pin anything) is folded in as a fallback — an unaddressable click resolves to its +enclosing pane plus a text excerpt of what was clicked. + +## Design + +### 1. Wire format: `InboxEvent.target` + +`InboxEvent` (packages/viewer/src/state.ts) gains one optional field, present only on `message` +events created with a pin: + +```ts +/** Where a console message is pinned. Captured client-side at pin time; agent-facing. */ +export interface InboxTarget { + kind: "pane" | "flow-node" | "component" | "checklist-item" | "heading"; + /** Pane indexes from the board root into nested `panes` arrays; [] on a non-panes board. */ + panePath?: number[]; + paneTitle?: string; + /** Stable spec id: flow node id, component props.id, "checklistId/itemId", heading slug. */ + id?: string; + /** Human-readable label captured at pin time (node label, heading text, item text). */ + label?: string; + /** Rendered text of/around the clicked element, for context when `id` is absent or stale. */ + excerpt?: string; + /** Board history seq at pin time, so the agent can tell the board has since changed. + * Reserved in the wire format; the v1 viewer omits it (the board version isn't tracked + * client-side) — `label`/`excerpt`/`paneTitle` carry the staleness resilience instead. */ + seq?: number; +} +``` + +Resolution contract for the agent: `panePath` indexes the (possibly nested) `panes` arrays of the +scope's current spec; `id` resolves within the pane it names (flow `nodes[].id`, component +`props.id`, checklist `/`, markdown heading slug). `kind: "pane"` with an +`excerpt` means "this pane, specifically the part reading ``" — the fallback for elements +with no stable id. + +### 2. Server: validation and caps (server.ts) + +`POST /w//inbox` accepts an optional `target` object on `kind: "message"` bodies. Validation +mirrors the existing inbox caps (reject with 400, never truncate silently): + +- `kind` must be one of the five values above; unknown keys stripped. +- `panePath`: array of ≤8 non-negative integers. +- `id`, `label`, `paneTitle`: strings ≤200 chars (same cap as `MAX_INBOX_REF`). +- `excerpt`: string ≤400 chars. +- `seq`: number. + +The validated target is stored on the event and flows unchanged through the ring, SSE `inbox` +events, and `GET /inbox`. No new endpoints; read receipts, long-poll, and the unread-nudge header +are untouched. + +### 3. Viewer client: gesture-triggered inline composer + +No mode toggle and no extra buttons — the gesture itself is the entry point. Three equivalent +triggers, all resolving the element under the pointer and opening an **inline composer popover** +anchored to it: + +- **Double-click / double-tap** on an element (primary, desktop + touch). +- **Long-press** (~500 ms, primary on touch where double-tap may zoom). +- **`c` while hovering** (desktop accelerator; ignored when focus is in an input). + +**Inline composer** (`client/comment-target.ts`, new): a small floating popover at the element — +target chip (e.g. `📌 pane "Latency" › node api-gw`) + a one-line textarea. `Enter` sends via the +existing `postInbox()` with `{kind:"message", text, target}`; `Esc` or clicking elsewhere cancels. +While open, the resolved element gets an outline highlight (reusing the pulse styling approach). +The console panel does **not** need to be open; if it is, the sent message appears in the +transcript with the same chip. Listeners are installed once, document-level, capture-phase; +double-click on an unresolvable area (outside any pane/board) does nothing. + +**Text-selection caveat**: double-click in prose (markdown/text panes) natively selects a word. +The composer opens anyway and collapses the accidental selection; users who are double-clicking +*to select/copy* can `Esc` (instant, no state), or use long-press/`c`-hover in text-heavy panes. +Acceptable for a beta-flagged feature; revisit if it proves annoying in practice. + +Resolution walks the gesture's target element upward, most specific first: + +1. `.react-flow__node[data-id]` → `flow-node` (label = trimmed node text). +2. `[data-item]` inside a checklist → `checklist-item` (id = `checklistId/itemId`). +3. Element with an `id` attribute inside a component renderer root → `component` + (component nodes already render `props.id` as the HTML id; chrome ids are excluded by + requiring the ancestor to be inside a renderer mount). +4. `h1–h4[id]` inside a markdown body → `heading` (requires the markdown renderer change below). +5. `.pane` → `pane`, with `excerpt` = trimmed text of the originally clicked element (≤400). + +`panePath` and `paneTitle` are computed for every kind by walking the DOM up through nested pane +containers (index among sibling panes at each level). On a non-panes board, `panePath` is `[]`. + +**Flow note**: React Flow selection stays disabled (`elementsSelectable: false`); the gesture +listener is a plain DOM capture handler and never involves React Flow's selection state. + +### 4. Markdown heading ids (`client/renderers/markdown.ts`) + +The markdown renderer adds slug `id`s to `h1–h4` (lowercase, alphanumeric + hyphens, `-2` suffix +on duplicates — GitHub-style). This is the one renderer change and is independently useful +(deep-linkable reports). + +### 5. Agent side: CLI rendering + skill docs + +`termchart inbox` (packages/cli/src/inbox.ts) renders the target as an indented context line: + +``` +[3] message: "the p95 line is mislabeled" + 📌 pane[1] "Latency" › flow-node api-gw ("API Gateway") · board v37 +``` + +`--json` passes `target` through verbatim. + +Docs updated so a listening agent knows the contract: +- `plugin/skills/diagram-recipes/status-state.md` — the inbox HTTP/CLI contract section gains the + `target` shape and the resolution rules above. +- `plugin/skills/inbox-watch/SKILL.md` — "when an event carries a target, resolve `id`/`panePath` + against `termchart pull` for that scope before acting; if `target.seq` is behind the current + board version, re-check that the element still exists." + +### 6. Error handling + +- Stale pins: the board may change between pin and read. The agent detects this via `target.seq` + vs current version; `label`/`excerpt`/`paneTitle` keep the comment intelligible even when the + `id` no longer resolves. The server does **not** validate targets against the live spec — + a pin is testimony about what the human saw, not a reference the server guarantees. +- Unresolvable gesture (outside any pane/board): ignored — no popover, no highlight. +- Oversized/malformed target: 400 from the server; the console surfaces the same send-failure + state it already has. + +### 7. Testing + +- Server unit tests: `POST /inbox` with valid target (round-trips through `GET /inbox` and SSE), + rejects bad kind / oversized fields / deep panePath; `message` without target unchanged. +- Client unit test (if the harness allows): target resolution from a synthetic DOM (flow node, + nested pane path, heading, fallback-to-pane). +- E2E: double-click a flow node in a nested pane, type into the inline composer, send, assert + `termchart inbox --json` returns the expected target and the console transcript (when open) + renders the chip. + +### 8. Rollout + +Independent release channels all touched: viewer Cloud Run deploy (server + client), npm CLI +publish (inbox target rendering), plugin marketplace bump (skill doc updates). Feature stays +behind the existing console-beta toggle — no default-experience change. + +## Future work (recorded, not planned) + +- Durable annotations with on-board pin badges and resolution state. +- Flow edge and vega mark targeting. +- Agent-initiated pins ("look here") via `status`/`focus` carrying a target. diff --git a/packages/cli/src/inbox.ts b/packages/cli/src/inbox.ts index 42e5515..d7662a6 100644 --- a/packages/cli/src/inbox.ts +++ b/packages/cli/src/inbox.ts @@ -37,12 +37,25 @@ function parse(argv: string[]): Args { return a; } -interface InboxEvent { seq: number; ts: number; kind: "message" | "action"; text?: string; ref?: string; } +interface InboxTarget { kind: string; panePath?: number[]; paneTitle?: string; id?: string; label?: string; excerpt?: string; seq?: number; } +interface InboxEvent { seq: number; ts: number; kind: "message" | "action"; text?: string; ref?: string; target?: InboxTarget; } interface InboxResponse { events: InboxEvent[]; cursor: number; } +/** One indented context line for a pinned message: where the human pinned it, in spec terms. */ +function fmtTarget(t: InboxTarget): string { + const parts: string[] = []; + if (t.panePath?.length || t.paneTitle) parts.push(`pane[${(t.panePath ?? []).join(",")}]${t.paneTitle ? ` "${t.paneTitle}"` : ""}`); + if (t.kind !== "pane") parts.push(`${t.kind}${t.id ? ` ${t.id}` : ""}${t.label ? ` ("${t.label}")` : ""}`); + let line = ` 📌 ${parts.join(" › ")}`; + if (t.excerpt) line += ` · "${t.excerpt}"`; + if (t.seq !== undefined) line += ` · board v${t.seq}`; + return line; +} + function fmt(e: InboxEvent): string { if (e.kind === "action") return `[${e.seq}] action ${e.ref ?? ""}`; - return `[${e.seq}] message ${e.text ?? ""}`; + const head = `[${e.seq}] message ${e.text ?? ""}`; + return e.target ? `${head}\n${fmtTarget(e.target)}` : head; } const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); diff --git a/packages/cli/test/inbox.test.ts b/packages/cli/test/inbox.test.ts index 9ea34df..de2ad6f 100644 --- a/packages/cli/test/inbox.test.ts +++ b/packages/cli/test/inbox.test.ts @@ -47,6 +47,40 @@ describe("inbox", () => { expect(errs.join("")).toContain("cursor: 2"); }); + it("renders a pinned target as an indented context line (targeted feedback)", async () => { + const { url } = await stub({ + events: [ + { seq: 3, ts: 3, kind: "message", text: "the p95 line is mislabeled", target: { kind: "flow-node", panePath: [1, 0], paneTitle: "Latency", id: "api-gw", label: "API Gateway", seq: 37 } }, + { seq: 4, ts: 4, kind: "message", text: "tighten this section", target: { kind: "pane", panePath: [0], excerpt: "Rollout risks: none identified" } }, + { seq: 5, ts: 5, kind: "message", text: "untargeted, no pin line" }, + ], + cursor: 5, + }); + const out: string[] = []; + const so = vi.spyOn(process.stdout, "write").mockImplementation((s) => (out.push(String(s)), true)); + const se = vi.spyOn(process.stderr, "write").mockImplementation(() => true); + const code = await inbox(["--project", "p", "--agent", "a"], { env: env(url) }); + so.mockRestore(); se.mockRestore(); + expect(code).toBe(0); + const text = out.join(""); + expect(text).toContain('📌 pane[1,0] "Latency" › flow-node api-gw ("API Gateway") · board v37'); + expect(text).toContain('📌 pane[0] · "Rollout risks: none identified"'); + // Exactly two pin lines — the untargeted message must not grow one. + expect(text.match(/📌/g)).toHaveLength(2); + }); + + it("--json passes a target through verbatim", async () => { + const target = { kind: "component", id: "risk-card", label: "Risks", seq: 2 }; + const { url } = await stub({ events: [{ seq: 1, ts: 1, kind: "message", text: "hi", target }], cursor: 1 }); + const out: string[] = []; + const so = vi.spyOn(process.stdout, "write").mockImplementation((s) => (out.push(String(s)), true)); + const se = vi.spyOn(process.stderr, "write").mockImplementation(() => true); + const code = await inbox(["--project", "p", "--agent", "a", "--json"], { env: env(url) }); + so.mockRestore(); se.mockRestore(); + expect(code).toBe(0); + expect(JSON.parse(out.join("")).events[0].target).toEqual(target); + }); + it("--json prints the raw { events, cursor }", async () => { const { url } = await stub(resp); const out: string[] = []; diff --git a/packages/viewer/bin/termchart-viewer.mjs b/packages/viewer/bin/termchart-viewer.mjs old mode 100644 new mode 100755 diff --git a/packages/viewer/e2e/rich.e2e.mjs b/packages/viewer/e2e/rich.e2e.mjs index 68fd3ac..a45a579 100644 --- a/packages/viewer/e2e/rich.e2e.mjs +++ b/packages/viewer/e2e/rich.e2e.mjs @@ -335,8 +335,8 @@ try { })); ok("agent-checked items show as checked from the spec", await p.evaluate(() => { - // The Mantine Checkbox forwards data-item onto its , so the attr IS the input. - const build = document.querySelector('#diagram [data-id="ci"] input[data-item="build"]'); + // data-item sits on the Checkbox ROOT row (wrapperProps); the input is its descendant. + const build = document.querySelector('#diagram [data-id="ci"] [data-item="build"] input'); return !!build && build.checked; })); @@ -346,15 +346,15 @@ try { await p2.waitForFunction(() => document.querySelector("#status .live-dot"), null, { timeout: 10000 }); await p2.locator(`#scopes button[title="e2e/checklist"]`).click(); await p2.waitForFunction( - () => document.querySelector('#diagram [data-id="approval"] input[data-item="changelog"]'), + () => document.querySelector('#diagram [data-id="approval"] [data-item="changelog"] input'), null, { timeout: 20000 }, ); // USER toggles an editable item on client 1 → it ticks immediately (optimistic). - await p.locator('#diagram [data-id="approval"] input[data-item="changelog"]').click(); + await p.locator('#diagram [data-id="approval"] [data-item="changelog"] input').click(); await p.waitForFunction( - () => document.querySelector('#diagram [data-id="approval"] input[data-item="changelog"]').checked, + () => document.querySelector('#diagram [data-id="approval"] [data-item="changelog"] input').checked, null, { timeout: 5000 }, ); @@ -371,7 +371,7 @@ try { // …and the SECOND client sees it via the SSE update. await p2.waitForFunction( - () => document.querySelector('#diagram [data-id="approval"] input[data-item="changelog"]').checked, + () => document.querySelector('#diagram [data-id="approval"] [data-item="changelog"] input').checked, null, { timeout: 8000 }, ); @@ -380,13 +380,13 @@ try { // AGENT side: a setChecked patch ticks a read-only (agent-driven) item live on both clients. await patchCheck("checklist", "ci", "e2e", true); await p.waitForFunction( - () => document.querySelector('#diagram [data-id="ci"] input[data-item="e2e"]').checked, + () => document.querySelector('#diagram [data-id="ci"] [data-item="e2e"] input').checked, null, { timeout: 8000 }, ); ok("agent setChecked patch ticks a read-only item live (client 1)", true); await p2.waitForFunction( - () => document.querySelector('#diagram [data-id="ci"] input[data-item="e2e"]').checked, + () => document.querySelector('#diagram [data-id="ci"] [data-item="e2e"] input').checked, null, { timeout: 8000 }, ); @@ -456,6 +456,28 @@ try { ok("clicking a chip posts an action to the inbox (agent reads the ref)", events.some((e) => e.kind === "action" && e.ref === "rerun")); } + // ---- targeted feedback: pin a comment to an element (dblclick → inline composer) ------------ + // While the console flag is on: double-click a checklist item → the inline composer opens with + // the target resolved in spec terms; Enter delivers it to the inbox with a structured `target`. + await p.locator("#diagram .tc-checklist [data-item]").first().dblclick(); + await p.waitForFunction(() => document.querySelector(".tc-pin-popover textarea"), null, { timeout: 5000 }); + ok("dblclick on a checklist item opens the pinned composer", true); + ok("composer chip names the element in spec terms", await p.evaluate(() => { + const chip = document.querySelector(".tc-pin-popover .tc-pin-chip"); + return !!chip && /checklist-item/.test(chip.textContent); + })); + await p.locator(".tc-pin-popover textarea").fill("this step is obsolete"); + await p.locator(".tc-pin-popover textarea").press("Enter"); + await p.waitForFunction(() => !document.querySelector(".tc-pin-popover"), null, { timeout: 5000 }); + ok("Enter sends the pinned comment and closes the composer", true); + await p.waitForTimeout(300); + { + const events = await readInbox("checklist"); + const ev = events.find((e) => e.kind === "message" && e.text === "this step is obsolete"); + ok("pinned comment lands in the inbox with its structured target (agent-resolvable)", + !!ev && !!ev.target && ev.target.kind === "checklist-item" && typeof ev.target.id === "string" && ev.target.id.includes("/")); + } + // Toggle it back off so the rest of the run sees the default (off) experience. await p.locator("#console-toggle").click(); await p.waitForFunction(() => { diff --git a/packages/viewer/src/client/comment-composer.ts b/packages/viewer/src/client/comment-composer.ts new file mode 100644 index 0000000..a80bffc --- /dev/null +++ b/packages/viewer/src/client/comment-composer.ts @@ -0,0 +1,147 @@ +// Targeted feedback: gesture-triggered inline composer. No mode toggle — double-click/double-tap, +// a ~500ms touch long-press, or `c` while hovering opens a small popover anchored to the resolved +// element (see comment-target.ts); Enter sends the comment pinned to that target, Esc cancels. +// Gated by the console beta flag via deps.isEnabled; installs nothing visible until a gesture. + +import { formatTarget, resolveTargetWithElement, type CommentTarget } from "./comment-target.js"; + +export interface TargetedFeedbackDeps { + /** Deliver the pinned comment (the console's inbox POST). Resolves false on failure. */ + send: (text: string, target: CommentTarget) => Promise; + /** Feature gate, checked per gesture (console beta flag). Default: always on. */ + isEnabled?: () => boolean; +} + +const LONG_PRESS_MS = 500; +const LONG_PRESS_SLOP_PX = 10; + +let deps: TargetedFeedbackDeps | null = null; +let pop: HTMLElement | null = null; +let pinned: Element | null = null; +let hovered: Element | null = null; +let press: { timer: ReturnType; x: number; y: number } | null = null; + +function close(): void { + pop?.remove(); + pop = null; + pinned?.classList.remove("tc-pin-highlight"); + pinned = null; +} + +function open(from: Element): void { + const hit = resolveTargetWithElement(from); + if (!hit) return; + close(); + // A double-click natively selected a word; the composer supersedes that selection. + window.getSelection?.()?.removeAllRanges(); + pinned = hit.el; + pinned.classList.add("tc-pin-highlight"); + + pop = document.createElement("div"); + pop.className = "tc-pin-popover"; + const chip = document.createElement("div"); + chip.className = "tc-pin-chip"; + chip.textContent = formatTarget(hit.target); + const ta = document.createElement("textarea"); + ta.rows = 2; + ta.maxLength = 2000; + ta.placeholder = "Comment on this… (Enter to send, Esc to cancel)"; + ta.setAttribute("aria-label", "Pinned comment"); + const status = document.createElement("div"); + status.className = "tc-pin-status"; + pop.append(chip, ta, status); + + ta.addEventListener("keydown", (e) => { + // Consume both keys: the viewer has window-level Escape cascade / letter hotkeys that must + // not also fire on a keystroke the composer already handled. + if (e.key === "Escape") { e.preventDefault(); e.stopPropagation(); close(); } + if (e.key === "Enter" && !e.shiftKey) { e.preventDefault(); e.stopPropagation(); void submit(ta, status, hit.target); } + }); + + // Anchor at the pinned element (fixed, viewport coords; clamped so it never leaves the screen). + const r = hit.el.getBoundingClientRect(); + pop.style.left = `${Math.max(8, Math.min(r.left, window.innerWidth - 340))}px`; + pop.style.top = `${Math.min(r.bottom + 6, window.innerHeight - 120)}px`; + document.body.appendChild(pop); + ta.focus(); +} + +async function submit(ta: HTMLTextAreaElement, status: HTMLElement, target: CommentTarget): Promise { + const text = ta.value.trim(); + if (!text || !deps) return; + ta.disabled = true; + status.textContent = "Sending…"; + const ok = await deps.send(text, target); + if (ok) { close(); return; } + ta.disabled = false; + status.textContent = "Failed to send — Enter to retry"; + ta.focus(); +} + +const gated = (): boolean => !!deps && (deps.isEnabled?.() ?? true); + +const isTyping = (t: EventTarget | null): boolean => + t instanceof HTMLElement && (t.tagName === "INPUT" || t.tagName === "TEXTAREA" || t.isContentEditable); + +function onDblClick(e: MouseEvent): void { + if (!gated() || !(e.target instanceof Element) || pop?.contains(e.target)) return; + open(e.target); +} + +function onMouseMove(e: MouseEvent): void { + if (e.target instanceof Element) hovered = e.target; +} + +function onKeyDown(e: KeyboardEvent): void { + if (e.key !== "c" || e.metaKey || e.ctrlKey || e.altKey) return; + if (!gated() || isTyping(e.target) || !hovered) return; + // Same guards as the viewer's `b` hotkey: the sidebar list owns letters (type-ahead), and an + // open modal overlay owns the keyboard. + if (e.target instanceof Element && e.target.closest("#scopes")) return; + if (document.querySelector(".tc-modal-overlay")) return; + open(hovered); +} + +function onPointerDown(e: PointerEvent): void { + // Click-away closes an open composer (clicks inside it are its own). + if (pop && e.target instanceof Element && !pop.contains(e.target)) close(); + if (!gated() || (e.pointerType !== "touch" && e.pointerType !== "pen")) return; + const from = e.target; + if (!(from instanceof Element)) return; + clearPress(); + press = { x: e.clientX ?? 0, y: e.clientY ?? 0, timer: setTimeout(() => { press = null; open(from); }, LONG_PRESS_MS) }; +} + +function onPointerMove(e: PointerEvent): void { + if (press && Math.hypot((e.clientX ?? 0) - press.x, (e.clientY ?? 0) - press.y) > LONG_PRESS_SLOP_PX) clearPress(); +} + +function clearPress(): void { + if (press) { clearTimeout(press.timer); press = null; } +} + +export function initTargetedFeedback(d: TargetedFeedbackDeps): void { + teardownTargetedFeedback(); + deps = d; + document.addEventListener("dblclick", onDblClick); + document.addEventListener("mousemove", onMouseMove); + document.addEventListener("keydown", onKeyDown); + document.addEventListener("pointerdown", onPointerDown); + document.addEventListener("pointermove", onPointerMove); + document.addEventListener("pointerup", clearPress); + document.addEventListener("pointercancel", clearPress); +} + +export function teardownTargetedFeedback(): void { + document.removeEventListener("dblclick", onDblClick); + document.removeEventListener("mousemove", onMouseMove); + document.removeEventListener("keydown", onKeyDown); + document.removeEventListener("pointerdown", onPointerDown); + document.removeEventListener("pointermove", onPointerMove); + document.removeEventListener("pointerup", clearPress); + document.removeEventListener("pointercancel", clearPress); + clearPress(); + close(); + deps = null; + hovered = null; +} diff --git a/packages/viewer/src/client/comment-target.ts b/packages/viewer/src/client/comment-target.ts new file mode 100644 index 0000000..3390478 --- /dev/null +++ b/packages/viewer/src/client/comment-target.ts @@ -0,0 +1,78 @@ +/** + * Targeted feedback: resolve the element under a gesture to a pinnable target in SPEC terms + * (flow node id, component props.id, checklist item, heading slug, or the enclosing pane) — + * never DOM paths. Pure DOM-in/target-out so it's unit-testable; the composer adds `seq` and + * posts it as `target` on an inbox message (server caps mirror MAX_LABEL/MAX_EXCERPT). + */ +export interface CommentTarget { + kind: "pane" | "flow-node" | "component" | "checklist-item" | "heading"; + panePath?: number[]; + paneTitle?: string; + id?: string; + label?: string; + excerpt?: string; + seq?: number; +} + +const MAX_LABEL = 120; +const MAX_EXCERPT = 400; + +const text = (el: Element, cap: number): string => (el.textContent ?? "").trim().slice(0, cap); + +/** Pane indexes from the board root down to the pane containing `el` ([] outside any pane). */ +function panePath(el: Element): { path: number[]; title?: string } { + const path: number[] = []; + let title: string | undefined; + for (let pane = el.closest(".pane"); pane; pane = pane.parentElement?.closest(".pane") ?? null) { + const container = pane.parentElement; + if (!container?.classList.contains("panes")) break; + path.unshift(Array.prototype.indexOf.call(container.children, pane)); + // Own bar only (not a nested pane's): the bar is a direct child of this .pane. + const bar = Array.prototype.find.call(pane.children, (c: Element) => c.classList.contains("pane-bar")) as Element | undefined; + title ??= bar?.querySelector(".pane-title")?.textContent?.trim() || undefined; + } + return { path, title }; +} + +function withPane(el: Element, t: CommentTarget): CommentTarget { + const { path, title } = panePath(el); + t.panePath = path; + if (title) t.paneTitle = title; + return t; +} + +/** Like resolveTarget, but also returns the matched element (for the composer's highlight). */ +export function resolveTargetWithElement(el: Element): { target: CommentTarget; el: Element } | null { + const flow = el.closest(".react-flow__node[data-id]"); + if (flow) return { target: withPane(flow, { kind: "flow-node", id: flow.getAttribute("data-id")!, label: text(flow, MAX_LABEL) }), el: flow }; + + const item = el.closest("[data-item]"); + const list = item?.closest(".tc-checklist[data-id]"); + if (item && list) + return { target: withPane(item, { kind: "checklist-item", id: `${list.getAttribute("data-id")}/${item.getAttribute("data-item")}`, label: text(item, MAX_LABEL) }), el: item }; + + const heading = el.closest("h1[id], h2[id], h3[id], h4[id]"); + if (heading?.closest(".markdown-body")) + return { target: withPane(heading, { kind: "heading", id: heading.id, label: text(heading, MAX_LABEL) }), el: heading }; + + const comp = el.closest(".tc-component [id]"); + if (comp) return { target: withPane(comp, { kind: "component", id: comp.id, label: text(comp, MAX_LABEL) }), el: comp }; + + const pane = el.closest(".pane"); + if (pane) return { target: withPane(pane, { kind: "pane", excerpt: text(el, MAX_EXCERPT) }), el: pane }; + + return null; +} + +export function resolveTarget(el: Element): CommentTarget | null { + return resolveTargetWithElement(el)?.target ?? null; +} + +/** Human-readable chip text for a pin — same vocabulary the CLI prints for the agent. */ +export function formatTarget(t: CommentTarget): string { + const parts: string[] = []; + if (t.paneTitle) parts.push(`pane "${t.paneTitle}"`); + else if (t.panePath?.length) parts.push(`pane[${t.panePath.join(",")}]`); + if (t.kind !== "pane") parts.push(`${t.kind}${t.id ? ` ${t.id}` : ""}`); + return `📌 ${parts.join(" › ") || "board"}`; +} diff --git a/packages/viewer/src/client/console.ts b/packages/viewer/src/client/console.ts index a400ebd..db8dd78 100644 --- a/packages/viewer/src/client/console.ts +++ b/packages/viewer/src/client/console.ts @@ -8,10 +8,12 @@ // OFF by default (localStorage flag `tc-console`). When off, this module touches nothing — the // default viewer experience is unchanged. Theme-aware (CSS vars) and reduced-motion-safe. +import { formatTarget, type CommentTarget } from "./comment-target.js"; + const wsid = location.pathname.split("/").filter(Boolean)[1] ?? ""; const STORAGE_KEY = "tc-console"; -interface InboxEvent { seq: number; ts: number; kind: "message" | "action"; text?: string; ref?: string; } +interface InboxEvent { seq: number; ts: number; kind: "message" | "action"; text?: string; ref?: string; target?: CommentTarget; } interface SuggestItem { id: string; label: string; } let enabled = false; @@ -38,6 +40,8 @@ const sig = (kind: string, key: string) => `${kind}|${key}`; function isOn(): boolean { try { return localStorage.getItem(STORAGE_KEY) === "1"; } catch { return false; } } +/** Gate for the targeted-feedback gestures: same beta flag as the console panel. */ +export const isConsoleOn = isOn; function setOn(on: boolean): void { try { localStorage.setItem(STORAGE_KEY, on ? "1" : "0"); } catch { /* ignore */ } } @@ -52,11 +56,17 @@ function escTextNode(parent: HTMLElement, cls: string, text: string): HTMLElemen /** Append a console row (message or action) to the log and scroll to it. Returns the row so the * caller can attach a delivery-status line (see deliver()). */ -function appendRow(kind: "message" | "action" | "sent", text: string): HTMLElement { +function appendRow(kind: "message" | "action" | "sent", text: string, target?: CommentTarget): HTMLElement { if (!logEl) return document.createElement("div"); const empty = logEl.querySelector(".tc-console-empty"); if (empty) empty.remove(); const row = escTextNode(logEl, `tc-console-row tc-console-row--${kind}`, text); + if (target) { + const pin = document.createElement("span"); + pin.className = "tc-console-pin"; + pin.textContent = formatTarget(target); // textContent only — pin fields are untrusted display + row.prepend(pin); + } row.scrollIntoView({ block: "nearest" }); return row; } @@ -118,7 +128,7 @@ function updateReadSummary(): void { * `seenSeqs` so the SSE `inbox` echo of OUR OWN event isn't drawn a second time (it was already * shown optimistically) — the reconciliation that keeps every screen in sync without a local dupe. */ -async function postInbox(body: { kind: "message" | "action"; text?: string; ref?: string }): Promise { +async function postInbox(body: { kind: "message" | "action"; text?: string; ref?: string; target?: CommentTarget }): Promise { if (!project || !agent) return null; // Mark this as a self-sent event before the request resolves, so an SSE echo that races ahead of // the response is suppressed (consumed in drawEvent). The seq dedupe below handles the common case. @@ -229,10 +239,25 @@ function drawEvent(e: InboxEvent): void { const s = sig(e.kind, e.kind === "message" ? (e.text ?? "") : (e.ref ?? "")); const idx = pendingSelf.indexOf(s); if (idx >= 0) { pendingSelf.splice(idx, 1); return; } - if (e.kind === "message") appendRow("message", e.text ?? ""); + if (e.kind === "message") appendRow("message", e.text ?? "", e.target); else appendRow("action", `→ ${labelForRef(e.ref ?? "")}`); } +/** + * Targeted feedback: deliver a comment pinned to a pane/element (from the inline composer, + * comment-composer.ts). Same path as a typed console message — optimistic row (when the panel is + * open), tokenless inbox POST, delivery receipt — plus the bounded `target`. Returns success so + * the composer can keep the text on failure. + */ +export async function sendPinnedMessage(text: string, target: CommentTarget): Promise { + const row = appendRow("sent", text, target); + setStatus(row, "Sending…", "pending"); + const ev = await postInbox({ kind: "message", text, target }); + if (ev) { markDelivered(row, ev.seq); return true; } + setStatus(row, "Failed to send", "failed"); + return false; +} + function labelForRef(ref: string): string { const found = curSuggestions.find((s) => s.id === ref); return found ? found.label : ref; diff --git a/packages/viewer/src/client/renderers/checklist.tsx b/packages/viewer/src/client/renderers/checklist.tsx index 2a39c69..d50b579 100644 --- a/packages/viewer/src/client/renderers/checklist.tsx +++ b/packages/viewer/src/client/renderers/checklist.tsx @@ -62,7 +62,10 @@ export function Checklist({ id, items, editable }: ChecklistProps) { return ( (); + +function slugify(html: string): string { + return html + .toLowerCase() + .replace(/<[^>]*>/g, "") + .replace(/[^\p{L}\p{N}\s_-]/gu, "") + .trim() + .replace(/\s+/g, "-"); +} + +marked.use({ + renderer: { + heading({ tokens, depth }) { + const inner = this.parser.parseInline(tokens); + const base = depth <= 4 ? slugify(inner) : ""; + if (!base) return `${inner}\n`; + const n = (slugCounts.get(base) ?? 0) + 1; + slugCounts.set(base, n); + return `${inner}\n`; + }, + }, +}); + /** * Markdown → HTML (GitHub-flavored), with LaTeX math via KaTeX→MathML. Full capability: raw HTML * passes through (no sanitizer), per the trust-the-token decision. Wrapped in `.markdown-body`. */ export function toHtml(content: string): string { + slugCounts = new Map(); // duplicate-slug counting is per document const html = marked.parse(content) as string; return `
${html}
`; } diff --git a/packages/viewer/src/client/style.css b/packages/viewer/src/client/style.css index 0f4c228..003e273 100644 --- a/packages/viewer/src/client/style.css +++ b/packages/viewer/src/client/style.css @@ -588,6 +588,31 @@ body.history-open #tc-history { display: flex; } .tc-console-send:hover { filter: brightness(1.08); } .tc-console-send:disabled { opacity: 0.5; cursor: not-allowed; filter: none; } .tc-console-send:focus-visible { outline: 2px solid var(--focus); outline-offset: 2px; } +/* Targeted feedback: the pin chip on console rows, the element highlight while composing, and the + gesture-opened inline composer (double-click / long-press / `c` — see comment-composer.ts). */ +.tc-console-pin { + display: block; font-size: 10.5px; line-height: 1.4; opacity: 0.85; margin-bottom: 2px; + overflow: hidden; text-overflow: ellipsis; white-space: nowrap; +} +.tc-pin-highlight { outline: 2px solid var(--focus); outline-offset: 2px; border-radius: 4px; } +.tc-pin-popover { + position: fixed; z-index: 60; width: 320px; max-width: calc(100vw - 16px); + display: flex; flex-direction: column; gap: 6px; padding: 10px; + background: var(--surface-2); border: 1px solid var(--border); border-radius: 10px; + box-shadow: 0 8px 24px rgba(0, 0, 0, 0.25); +} +.tc-pin-chip { + font-size: 11px; color: var(--muted); + overflow: hidden; text-overflow: ellipsis; white-space: nowrap; +} +.tc-pin-popover textarea { + resize: none; font: inherit; font-size: 16px; line-height: 1.4; /* 16px: iOS auto-zooms inputs <16px on focus */ + background: var(--bg); color: var(--fg); border: 1px solid var(--border); + border-radius: 8px; padding: 7px 9px; +} +.tc-pin-popover textarea:focus { outline: none; border-color: var(--focus); } +.tc-pin-status { font-size: 11px; color: #f87171; } +.tc-pin-status:empty { display: none; } /* Tablet / narrow desktop (e.g. iPad portrait ~834px): keep the whole toolbar on ONE row by going icon-only, instead of letting the labelled buttons wrap to an untidy second row. Labels return on wider desktops where there's room. Each tool stays a ≥44px square tap target. */ diff --git a/packages/viewer/src/client/viewer.ts b/packages/viewer/src/client/viewer.ts index 470087a..2033409 100644 --- a/packages/viewer/src/client/viewer.ts +++ b/packages/viewer/src/client/viewer.ts @@ -2,7 +2,8 @@ import { toPng } from "html-to-image"; import { renderInto, teardownTree, patchInto } from "./registry.js"; import { INTRO_PANES } from "./intro.js"; import { setActiveScope } from "./interact.js"; -import { initConsole, setConsoleScope, onInboxEvent, onInboxRead, onSuggest, resyncConsole, isConsoleOpen, closeConsole } from "./console.js"; +import { initConsole, setConsoleScope, onInboxEvent, onInboxRead, onSuggest, resyncConsole, sendPinnedMessage, isConsoleOn, isConsoleOpen, closeConsole } from "./console.js"; +import { initTargetedFeedback } from "./comment-composer.js"; import { nextScopeIndex, matchScope } from "./keynav.js"; import type { PatchOp } from "../flow-patch.js"; import { sortBoards, type BoardSort, type TemplateSchema } from "../state.js"; @@ -1268,6 +1269,9 @@ if (isShareMode) { if (templatesBtn) { templatesBtn.hidden = false; templatesBtn.onclick = () => void openTemplatesPopover(templatesBtn); } if (EXPERIMENTAL) { initConsole(); + // Targeted feedback: pin a comment to a pane/element via double-click, touch long-press, or + // `c` over the hovered element. Rides the console's inbox POST; gated by the same beta flag. + initTargetedFeedback({ send: sendPinnedMessage, isEnabled: isConsoleOn }); const ht = document.getElementById("history-toggle"); if (ht) ht.onclick = () => toggleHistory(!historyEnabled); } else { diff --git a/packages/viewer/src/server.ts b/packages/viewer/src/server.ts index 9a57efd..f2ccc9c 100644 --- a/packages/viewer/src/server.ts +++ b/packages/viewer/src/server.ts @@ -3,7 +3,7 @@ import { timingSafeEqual } from "node:crypto"; import { readFileSync } from "node:fs"; import { gzipSync, brotliCompressSync, constants as zlibConstants } from "node:zlib"; import { join, normalize, sep } from "node:path"; -import { Store, StatusStore, InboxStore, type Payload, type StatusUpdate, type PersistenceBackend, type SuggestItem } from "./state.js"; +import { Store, StatusStore, InboxStore, type Payload, type StatusUpdate, type PersistenceBackend, type SuggestItem, type InboxTarget } from "./state.js"; import { seedDemo, isReadOnlyWsid } from "./seed.js"; import { SSEHub, type SSEMessage } from "./sse.js"; import { ShareStore, SHARES_WSID } from "./share-store.js"; @@ -32,6 +32,42 @@ export interface ServerOpts { // so these caps keep the transient ring + suggestion set from growing unbounded. const MAX_INBOX_TEXT = 2000; // a console message / next-step note; longer ⇒ 400 const MAX_INBOX_REF = 200; // an action's chip id +const MAX_TARGET_STR = 200; // a pin's id / label / paneTitle +const MAX_TARGET_EXCERPT = 400; // a pin's captured element text +const MAX_TARGET_PANE_DEPTH = 8; // panePath entries (real boards nest 1-2 deep) +const TARGET_KINDS = new Set(["pane", "flow-node", "component", "checklist-item", "heading"]); + +/** + * Validate a pinned-comment target (targeted feedback). Returns the sanitized target (unknown + * keys stripped), or a string describing why it's invalid. Bounded like the other inbox fields — + * the route is tokenless, so every field is enumerated/capped and nothing is stored verbatim. + */ +function parseInboxTarget(raw: unknown): InboxTarget | string { + if (typeof raw !== "object" || raw === null || Array.isArray(raw)) return "inbox target must be an object"; + const t = raw as Record; + if (typeof t.kind !== "string" || !TARGET_KINDS.has(t.kind)) return `inbox target kind must be one of ${[...TARGET_KINDS].join("/")}`; + const out: InboxTarget = { kind: t.kind as InboxTarget["kind"] }; + if (t.panePath !== undefined) { + if (!Array.isArray(t.panePath) || t.panePath.length > MAX_TARGET_PANE_DEPTH || !t.panePath.every((n) => Number.isInteger(n) && (n as number) >= 0)) + return `inbox target panePath must be ≤${MAX_TARGET_PANE_DEPTH} non-negative integers`; + out.panePath = t.panePath as number[]; + } + for (const key of ["paneTitle", "id", "label"] as const) { + const v = t[key]; + if (v === undefined) continue; + if (typeof v !== "string" || v.length > MAX_TARGET_STR) return `inbox target ${key} must be a string ≤${MAX_TARGET_STR} chars`; + out[key] = v; + } + if (t.excerpt !== undefined) { + if (typeof t.excerpt !== "string" || t.excerpt.length > MAX_TARGET_EXCERPT) return `inbox target excerpt must be a string ≤${MAX_TARGET_EXCERPT} chars`; + out.excerpt = t.excerpt; + } + if (t.seq !== undefined) { + if (typeof t.seq !== "number" || !Number.isFinite(t.seq)) return "inbox target seq must be a number"; + out.seq = t.seq; + } + return out; +} const MAX_SUGGEST_ITEMS = 12; // chips an agent can push at once const MAX_SUGGEST_LABEL = 120; // a chip's label const MAX_INBOX_WAIT = 25000; // long-poll cap (ms) @@ -641,8 +677,16 @@ export function createViewerServer(opts: ServerOpts) { // A message must carry text; an action must carry a ref — otherwise the event is empty. if (b.kind === "message" && !text) return send(res, 400, "inbox message needs non-empty text"); if (b.kind === "action" && !ref) return send(res, 400, "inbox action needs a ref"); + // Targeted feedback: an optional pin on message events only (an action's context is its chip). + let target: InboxTarget | undefined; + if (b.target !== undefined) { + if (b.kind !== "message") return send(res, 400, "inbox target is only valid on a message"); + const parsed = parseInboxTarget(b.target); + if (typeof parsed === "string") return send(res, 400, parsed); + target = parsed; + } const scope = `${b.project}/${b.agent}`; - const event = inboxStore.append(wsid, scope, { kind: b.kind, text, ref }); + const event = inboxStore.append(wsid, scope, { kind: b.kind, text, ref, target }); // Echo to other screens so every console stays in sync (the originating client showed it // optimistically). This does NOT alter the diagram — it's a console-only event. fanout(wsid, { event: "inbox", data: { scope, event } }); diff --git a/packages/viewer/src/state.ts b/packages/viewer/src/state.ts index 7901f6a..dfdb8b8 100644 --- a/packages/viewer/src/state.ts +++ b/packages/viewer/src/state.ts @@ -139,12 +139,29 @@ export class StatusStore { * chip's `ref`). Each event gets a monotonic `seq` so a polling agent can read incrementally * with a `since` cursor. */ +/** + * Where a console message is pinned (targeted feedback). Captured client-side at pin time and + * passed through verbatim to the agent — testimony about what the human saw, never resolved or + * verified by the server. `panePath` indexes nested `panes` arrays; `id` is the spec-level id + * (flow node id, component props.id, "checklistId/itemId", heading slug) within that pane. + */ +export interface InboxTarget { + kind: "pane" | "flow-node" | "component" | "checklist-item" | "heading"; + panePath?: number[]; + paneTitle?: string; + id?: string; + label?: string; + excerpt?: string; + seq?: number; +} + export interface InboxEvent { seq: number; ts: number; kind: "message" | "action"; text?: string; ref?: string; + target?: InboxTarget; } /** A single agent → human suggestion chip the human can click in the console. */ diff --git a/packages/viewer/test/comment-composer.test.ts b/packages/viewer/test/comment-composer.test.ts new file mode 100644 index 0000000..eb4ab95 --- /dev/null +++ b/packages/viewer/test/comment-composer.test.ts @@ -0,0 +1,169 @@ +// @vitest-environment happy-dom +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { initTargetedFeedback, teardownTargetedFeedback } from "../src/client/comment-composer.js"; +import type { CommentTarget } from "../src/client/comment-target.js"; + +const BOARD = ` +
+
+
Flow
+
+
+
API Gateway
+
+
+
+
+ `; + +let sent: { text: string; target: CommentTarget }[]; +let sendOk: boolean; +let enabled: boolean; + +const send = vi.fn(async (text: string, target: CommentTarget) => { + sent.push({ text, target }); + return sendOk; +}); + +beforeEach(() => { + document.body.innerHTML = BOARD; + sent = []; + sendOk = true; + enabled = true; + send.mockClear(); + initTargetedFeedback({ send, isEnabled: () => enabled }); +}); +afterEach(() => teardownTargetedFeedback()); + +const node = () => document.querySelector(".node-label")!; +const popover = () => document.querySelector(".tc-pin-popover"); +const textarea = () => popover()?.querySelector("textarea") as HTMLTextAreaElement | undefined; + +const dblclick = (el: Element) => el.dispatchEvent(new MouseEvent("dblclick", { bubbles: true, cancelable: true })); +const key = (target: EventTarget, k: string) => + target.dispatchEvent(new KeyboardEvent("keydown", { key: k, bubbles: true, cancelable: true })); + +describe("targeted-feedback composer", () => { + it("opens an anchored composer on double-click with the resolved target chip", () => { + dblclick(node()); + expect(popover()).toBeTruthy(); + expect(popover()!.textContent).toContain("api-gw"); + expect(textarea()).toBeTruthy(); + // The pinned element is highlighted while composing. + expect(document.querySelector(".tc-pin-highlight")).toBeTruthy(); + }); + + it("does nothing on double-click outside any addressable element", () => { + const stray = document.createElement("div"); + document.body.appendChild(stray); + dblclick(stray); + expect(popover()).toBeNull(); + }); + + it("does nothing when disabled (console beta flag off)", () => { + enabled = false; + dblclick(node()); + expect(popover()).toBeNull(); + }); + + it("Escape closes the composer and clears the highlight", () => { + dblclick(node()); + key(textarea()!, "Escape"); + expect(popover()).toBeNull(); + expect(document.querySelector(".tc-pin-highlight")).toBeNull(); + }); + + it("Enter sends the text with the resolved target and closes on success", async () => { + dblclick(node()); + textarea()!.value = "this label is wrong"; + key(textarea()!, "Enter"); + await vi.waitFor(() => expect(popover()).toBeNull()); + expect(sent).toHaveLength(1); + expect(sent[0].text).toBe("this label is wrong"); + expect(sent[0].target).toMatchObject({ kind: "flow-node", id: "api-gw", panePath: [0], paneTitle: "Flow" }); + }); + + it("stays open and shows a failure note when the send fails", async () => { + sendOk = false; + dblclick(node()); + textarea()!.value = "hello"; + key(textarea()!, "Enter"); + await vi.waitFor(() => expect(popover()!.textContent).toContain("Failed")); + expect(textarea()!.value).toBe("hello"); // nothing lost + }); + + it("ignores Enter on an empty composer", () => { + dblclick(node()); + key(textarea()!, "Enter"); + expect(send).not.toHaveBeenCalled(); + expect(popover()).toBeTruthy(); + }); + + it("opens via the `c` hotkey over the hovered element", () => { + node().dispatchEvent(new MouseEvent("mousemove", { bubbles: true })); + key(document.body, "c"); + expect(popover()).toBeTruthy(); + expect(popover()!.textContent).toContain("api-gw"); + }); + + it("ignores `c` when the sidebar board list owns letters (type-ahead) or an overlay is open", () => { + node().dispatchEvent(new MouseEvent("mousemove", { bubbles: true })); + // Focus inside #scopes: letters belong to the sidebar type-ahead (same guard as the `b` hotkey). + const scopes = document.createElement("nav"); + scopes.id = "scopes"; + const item = document.createElement("button"); + scopes.appendChild(item); + document.body.appendChild(scopes); + key(item, "c"); + expect(popover()).toBeNull(); + scopes.remove(); + // A modal overlay owns the keyboard. + const overlay = document.createElement("div"); + overlay.className = "tc-modal-overlay"; + document.body.appendChild(overlay); + key(document.body, "c"); + expect(popover()).toBeNull(); + }); + + it("Escape inside the composer does not bubble to window-level Escape handlers", () => { + dblclick(node()); + let leaked = false; + const spy = () => { leaked = true; }; + window.addEventListener("keydown", spy); + key(textarea()!, "Escape"); + window.removeEventListener("keydown", spy); + expect(popover()).toBeNull(); // composer closed itself… + expect(leaked).toBe(false); // …and consumed the key (the viewer's Esc cascade must not also fire) + }); + + it("ignores `c` while typing in an input", () => { + node().dispatchEvent(new MouseEvent("mousemove", { bubbles: true })); + const input = document.getElementById("some-input") as HTMLInputElement; + key(input, "c"); + expect(popover()).toBeNull(); + }); + + it("opens on a ~500ms touch long-press, not on a quick tap", () => { + vi.useFakeTimers(); + const down = new Event("pointerdown", { bubbles: true }); + Object.assign(down, { pointerType: "touch" }); + node().dispatchEvent(down); + vi.advanceTimersByTime(200); + node().dispatchEvent(new Event("pointerup", { bubbles: true })); + vi.advanceTimersByTime(1000); + expect(popover()).toBeNull(); // quick tap — no composer + + const down2 = new Event("pointerdown", { bubbles: true }); + Object.assign(down2, { pointerType: "touch" }); + node().dispatchEvent(down2); + vi.advanceTimersByTime(600); + expect(popover()).toBeTruthy(); // held — composer opens + vi.useRealTimers(); + }); + + it("only one composer at a time — a second gesture replaces the first", () => { + dblclick(node()); + dblclick(node()); + expect(document.querySelectorAll(".tc-pin-popover")).toHaveLength(1); + }); +}); diff --git a/packages/viewer/test/comment-target.test.ts b/packages/viewer/test/comment-target.test.ts new file mode 100644 index 0000000..d617c34 --- /dev/null +++ b/packages/viewer/test/comment-target.test.ts @@ -0,0 +1,122 @@ +// @vitest-environment happy-dom +import { beforeEach, describe, expect, it } from "vitest"; +import { resolveTarget } from "../src/client/comment-target.js"; + +// A board mirroring the real renderer DOM: pane[0] = markdown report, pane[1] = nested rows +// (pane[1,0] = flow, pane[1,1] = component tree with a checklist and an id'd card). +const BOARD = ` +
+
+
Report
+
+
+

Latency Overview

+

p95 crept up after the cache change.

+
+
+
+
+
Arch
+
+
+
+
Flow
+
+
+
API Gateway
+
+
+
+
+
Tasks
+
+
+
+
Run tests
+
+
Risks: none identified
+
+
+
+
+
+
+
`; + +beforeEach(() => { + document.body.innerHTML = BOARD; +}); + +const q = (sel: string): Element => document.querySelector(sel)!; + +describe("resolveTarget (targeted feedback)", () => { + it("resolves a flow node by data-id, with its pane path and title", () => { + expect(resolveTarget(q(".node-label"))).toEqual({ + kind: "flow-node", + id: "api-gw", + label: "API Gateway", + panePath: [1, 0], + paneTitle: "Flow", + }); + }); + + it("resolves a checklist item as checklistId/itemId", () => { + expect(resolveTarget(q(".item-label"))).toEqual({ + kind: "checklist-item", + id: "deploy-steps/run-tests", + label: "Run tests", + panePath: [1, 1], + paneTitle: "Tasks", + }); + }); + + it("resolves an id'd node in a component tree", () => { + expect(resolveTarget(q(".card-text"))).toEqual({ + kind: "component", + id: "risk-card", + label: "Risks: none identified", + panePath: [1, 1], + paneTitle: "Tasks", + }); + }); + + it("resolves a markdown heading by its slug id", () => { + expect(resolveTarget(q("#latency-overview"))).toEqual({ + kind: "heading", + id: "latency-overview", + label: "Latency Overview", + panePath: [0], + paneTitle: "Report", + }); + }); + + it("falls back to the enclosing pane with an excerpt of the clicked element", () => { + expect(resolveTarget(q(".prose"))).toEqual({ + kind: "pane", + panePath: [0], + paneTitle: "Report", + excerpt: "p95 crept up after the cache change.", + }); + }); + + it("returns null outside any pane or addressable element", () => { + const stray = document.createElement("div"); + document.body.appendChild(stray); + expect(resolveTarget(stray)).toBeNull(); + }); + + it("resolves on a single-board view (no panes): empty panePath, no title", () => { + document.body.innerHTML = `
Solo
`; + expect(resolveTarget(q(".s"))).toEqual({ kind: "flow-node", id: "solo", label: "Solo", panePath: [] }); + }); + + it("caps excerpt at 400 chars and label at 120", () => { + document.body.innerHTML = ` +
T
+

${"x".repeat(500)}

`; + const t = resolveTarget(q(".long"))!; + expect(t.excerpt).toHaveLength(400); + document.body.innerHTML = `
${"y".repeat(200)}
`; + expect(resolveTarget(q(".s"))!.label).toHaveLength(120); + }); +}); diff --git a/packages/viewer/test/console.test.ts b/packages/viewer/test/console.test.ts new file mode 100644 index 0000000..fff974d --- /dev/null +++ b/packages/viewer/test/console.test.ts @@ -0,0 +1,58 @@ +// @vitest-environment happy-dom +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { initConsole, onInboxEvent, sendPinnedMessage, setConsoleScope } from "../src/client/console.js"; + +// The console module keeps per-scope state at module level; give each test a distinct scope so +// seq-dedupe sets from an earlier test can't swallow this test's events. +let scopeN = 0; +let project: string; + +const okJson = (body: unknown) => ({ ok: true, json: async () => body }) as Response; + +let posted: { url: string; body: Record }[]; + +beforeEach(() => { + document.body.innerHTML = ""; + posted = []; + project = `p${++scopeN}`; + vi.stubGlobal("fetch", vi.fn(async (url: string, init?: RequestInit) => { + if (init?.method === "POST") { + posted.push({ url: String(url), body: JSON.parse(String(init.body)) }); + return okJson({ seq: 1, ts: 1, kind: "message", text: "x" }); + } + return okJson({ items: [], events: [], acked: 0 }); + })); +}); +afterEach(() => vi.unstubAllGlobals()); + +describe("console targeted feedback", () => { + it("sendPinnedMessage POSTs the message with its target and reports success", async () => { + setConsoleScope(project, "a"); + const ok = await sendPinnedMessage("the p95 label is wrong", { kind: "flow-node", id: "api-gw", panePath: [1, 0] }); + expect(ok).toBe(true); + const post = posted.find((p) => p.url.endsWith("/inbox"))!; + expect(post.body).toMatchObject({ + project, + agent: "a", + kind: "message", + text: "the p95 label is wrong", + target: { kind: "flow-node", id: "api-gw", panePath: [1, 0] }, + }); + }); + + it("sendPinnedMessage reports failure when the POST fails", async () => { + vi.stubGlobal("fetch", vi.fn(async () => ({ ok: false }) as Response)); + setConsoleScope(project, "a"); + expect(await sendPinnedMessage("hi", { kind: "pane", panePath: [0] })).toBe(false); + }); + + it("renders a pin chip on transcript rows whose event carries a target", () => { + localStorage.setItem("tc-console", "1"); + initConsole(); // builds + opens the panel (flag is on) + setConsoleScope(project, "a"); + onInboxEvent(`${project}/a`, { seq: 5, ts: 1, kind: "message", text: "seen from another screen", target: { kind: "flow-node", id: "api-gw", paneTitle: "Flow" } }); + const row = document.querySelector("#tc-console .tc-console-row--message"); + expect(row?.textContent).toContain("seen from another screen"); + expect(row?.querySelector(".tc-console-pin")?.textContent).toContain("api-gw"); + }); +}); diff --git a/packages/viewer/test/markdown.test.ts b/packages/viewer/test/markdown.test.ts index 641d545..9b6c5ab 100644 --- a/packages/viewer/test/markdown.test.ts +++ b/packages/viewer/test/markdown.test.ts @@ -9,6 +9,40 @@ describe("markdown renderer", () => { expect(html).toContain("
  • a
    • "); }); + // Heading slug ids (targeted feedback): h1–h4 get GitHub-style ids so a pinned comment (and a + // deep link) can address a markdown section. + describe("heading slug ids", () => { + it("gives h1–h4 a lowercase-hyphen slug id", () => { + const html = toHtml("## Latency Overview"); + expect(html).toContain('

    '); + }); + + it("strips punctuation and inline formatting from the slug", () => { + expect(toHtml("# Hello, World!")).toContain('

    '); + expect(toHtml("## The `api` layer")).toContain('

    '); + }); + + it("suffixes duplicate slugs -2, -3 within one document", () => { + const html = toHtml("## Setup\n\ntext\n\n## Setup\n\nmore\n\n## Setup"); + expect(html).toContain('id="setup"'); + expect(html).toContain('id="setup-2"'); + expect(html).toContain('id="setup-3"'); + }); + + it("resets duplicate counting per document (no -2 leak across renders)", () => { + toHtml("## Setup"); + expect(toHtml("## Setup")).toContain('

    '); + }); + + it("leaves h5+ without ids (not addressable targets)", () => { + expect(toHtml("##### deep note")).toMatch(/]*id=)/); + }); + + it("omits the id when the slug would be empty", () => { + expect(toHtml("## !!!")).toMatch(/]*id=)/); + }); + }); + it("renders gfm tables", () => { const html = toHtml("| A | B |\n|---|---|\n| 1 | 2 |"); expect(html).toContain(" { expect((got.body as { events: { kind: string; ref: string }[] }).events[0]).toMatchObject({ kind: "action", ref: "rerun-tests" }); }); + // Targeted feedback: a message may pin itself to a pane/element via a bounded `target` + // (docs/superpowers/specs/2026-07-03-targeted-feedback-design.md). Testimony, not a + // server-verified reference — the server caps shapes/sizes but never resolves it. + describe("pinned targets", () => { + const target = { + kind: "flow-node", + panePath: [1, 0], + paneTitle: "Latency", + id: "api-gw", + label: "API Gateway", + excerpt: "p95 420ms", + seq: 37, + }; + + it("round-trips a full target on a message (POST echo + GET /inbox)", async () => { + const r = await inboxReq({ project: "p", agent: "a", kind: "message", text: "label is wrong", target }); + expect(r.status).toBe(200); + expect((await r.json()).target).toEqual(target); + const got = await getJson("inbox?project=p&agent=a"); + expect((got.body as { events: { target?: unknown }[] }).events[0]).toMatchObject({ text: "label is wrong", target }); + }); + + it("accepts a minimal pane target (kind + panePath only)", async () => { + const r = await inboxReq({ project: "p", agent: "a", kind: "message", text: "this pane", target: { kind: "pane", panePath: [0] } }); + expect(r.status).toBe(200); + expect((await r.json()).target).toEqual({ kind: "pane", panePath: [0] }); + }); + + it("strips unknown target keys instead of storing them", async () => { + const r = await inboxReq({ project: "p", agent: "a", kind: "message", text: "hi", target: { kind: "pane", panePath: [0], junk: "x" } }); + expect(r.status).toBe(200); + expect((await r.json()).target).toEqual({ kind: "pane", panePath: [0] }); + }); + + it("400s malformed targets (kind, shapes, caps)", async () => { + const t = (target: unknown) => inboxReq({ project: "p", agent: "a", kind: "message", text: "hi", target }); + expect((await t("api-gw")).status).toBe(400); // not an object + expect((await t({ panePath: [0] })).status).toBe(400); // no kind + expect((await t({ kind: "vega-mark" })).status).toBe(400); // unknown kind + expect((await t({ kind: "pane", panePath: [0, 1, 2, 3, 4, 5, 6, 7, 8] })).status).toBe(400); // >8 deep + expect((await t({ kind: "pane", panePath: [-1] })).status).toBe(400); // negative index + expect((await t({ kind: "pane", panePath: [1.5] })).status).toBe(400); // non-integer + expect((await t({ kind: "flow-node", id: "x".repeat(201) })).status).toBe(400); // id cap + expect((await t({ kind: "flow-node", label: "x".repeat(201) })).status).toBe(400); // label cap + expect((await t({ kind: "pane", paneTitle: "x".repeat(201) })).status).toBe(400); // title cap + expect((await t({ kind: "pane", excerpt: "x".repeat(401) })).status).toBe(400); // excerpt cap + expect((await t({ kind: "pane", seq: "37" })).status).toBe(400); // seq must be a number + }); + + it("400s a target on an action event — pins belong to messages", async () => { + const r = await inboxReq({ project: "p", agent: "a", kind: "action", ref: "chip", target: { kind: "pane" } }); + expect(r.status).toBe(400); + }); + }); + it("400s malformed inbox posts", async () => { expect((await inboxReq({ project: "p", agent: "a" })).status).toBe(400); // no kind expect((await inboxReq({ project: "p", agent: "a", kind: "nope", text: "x" })).status).toBe(400); diff --git a/plugin/skills/diagram-recipes/status-state.md b/plugin/skills/diagram-recipes/status-state.md index 88be4d7..8c3df99 100644 --- a/plugin/skills/diagram-recipes/status-state.md +++ b/plugin/skills/diagram-recipes/status-state.md @@ -138,9 +138,33 @@ Two directions: ``` Read is open (URL only, like `list`/`pull`); the cursor (latest `seq`) prints to stderr so you can - pass it as the next `--since`. Each event is `{ seq, ts, kind:"message"|"action", text?, ref? }`: + pass it as the next `--since`. Each event is `{ seq, ts, kind:"message"|"action", text?, ref?, target? }`: a `message` carries the human's `text`; an `action` carries the clicked chip's `id` in `ref`. + **Pinned comments (`target`).** The human can pin a message to a specific pane or element + (double-click / long-press / `c` in the viewer). Such messages carry a `target`: + + ``` + { kind: "pane"|"flow-node"|"component"|"checklist-item"|"heading", + panePath?: number[], paneTitle?: string, id?: string, label?: string, excerpt?: string, seq?: number } + ``` + + The CLI prints it as an indented context line under the message: + + ``` + [3] message the p95 line is mislabeled + 📌 pane[1,0] "Latency" › flow-node api-gw ("API Gateway") + ``` + + How to resolve it: `panePath` indexes the (possibly nested) `panes` arrays of the scope's current + spec (`termchart pull`); `id` is spec-level **within that pane** — a flow `nodes[].id`, a component + `props.id`, a checklist `"/"`, or a markdown heading slug. `kind:"pane"` with + an `excerpt` means "this pane, specifically the part reading ``" (the fallback for + elements without a stable id). A target is **testimony about what the human saw**, not + server-verified: the board may have changed since the pin, so if `id` no longer resolves, use + `label` / `paneTitle` / `excerpt` to locate what was meant. Act on the pinned element — patch that + node/pane, answer about that section — not the board at large. + **The agent waits on input** — push a view, advertise choices, then block: ```bash @@ -166,8 +190,9 @@ new sidebar entry) in reaction to a chat message unless the human explicitly ask confirmed it with them first. The human is looking at the current scope; keep the conversation and its results there, rather than scattering drive-by views across their screen. -Raw HTTP: `POST /inbox` (tokenless: `{project,agent,kind,text?/ref?}` — append-only, **never** touches -the stored spec), `GET /inbox?project=&agent=&since=&wait=` (reading marks the scope read), +Raw HTTP: `POST /inbox` (tokenless: `{project,agent,kind,text?/ref?,target?}` — append-only, **never** +touches the stored spec; `target` only on `message`, bounded: strings ≤200 chars, excerpt ≤400, +panePath ≤8 indexes), `GET /inbox?project=&agent=&since=&wait=` (reading marks the scope read), `POST /suggest` (token-gated: `{project,agent,items:[{id,label}]}`), `GET /suggest?project=&agent=`. ## Clearing scopes diff --git a/plugin/skills/inbox-watch/SKILL.md b/plugin/skills/inbox-watch/SKILL.md index d157dc3..81ddcec 100644 --- a/plugin/skills/inbox-watch/SKILL.md +++ b/plugin/skills/inbox-watch/SKILL.md @@ -76,6 +76,24 @@ cancels jobs (for Claude Code, `CronDelete `). | You asked a question / pushed chips, waiting on the answer | **`--wait`** (3) | | Idle between bursts; harness can schedule | **cron + `--since`** (4) | +## Pinned messages (`target`) — comments about a SPECIFIC element + +The human can pin a message to a pane or element (double-click / long-press / `c` in the viewer). +The event then carries a `target` and the CLI prints an indented `📌` context line: + +``` +[3] message the p95 line is mislabeled + 📌 pane[1,0] "Latency" › flow-node api-gw ("API Gateway") +``` + +Treat the pin as the subject of the message. To resolve it: `termchart pull` the same scope, follow +`panePath` into the nested `panes` arrays, then find `id` **within that pane** (flow `nodes[].id`, +component `props.id`, checklist `"/"`, or a markdown heading slug). +`kind:"pane"` + `excerpt` means "this pane, the part reading ``". The board may have +changed since the pin — if `id` doesn't resolve, fall back to `label`/`paneTitle`/`excerpt` to work +out what was meant, and say so if it's gone. Act on the pinned element (patch that node, rewrite +that section), not the board at large. Full shape: diagram-recipes `status-state.md`. + ## Two rules that always apply - **Stay in the scope the message came from.** Act on a message in the *same* `project/agent` it