From 68a6c923a86421d5ab9eb5f58175cdeacaa37a20 Mon Sep 17 00:00:00 2001 From: DeyangChan Date: Wed, 5 Aug 2026 11:58:59 +0800 Subject: [PATCH 1/4] Linked means a session is listening, and the cog says which version The workspace's link state now rests on evidence a session can only produce by being there: - A stream watcher opens with a HANDSHAKE line naming a command. Its `watching` heartbeat starts when `ack` answers it, and it exits 3 after two minutes (`--handshake-timeout `) if nothing does. - Presence also requires the rounds to move: a queued round unclaimed past 90 seconds drops the link, and `check` names it on every call. - The cog menu shows the version the workspace loaded with and the one the server is on, and offers a reload when they differ. An answer written on a comment carried in from an earlier version is kept. `postReply` and the composer's note edit copy the comment into the current version first, so the reply lands in the file the workspace reads. Stop is withdrawn to docs/review-wishlist.md, with interrupting the agent's turn as its first acceptance criterion. The `cancel` sentinel, `/api/cancel`, the `cancelled` command and the CANCELLED event go with it; `check` always exits 0. Rounds still on disk with status `cancelled` stay terminal. Host profiles move to plugins/vstack/host-profiles/, leaving `hosts/` to the adapter markdown that maps ops to a product's tools. A fitted page follows its canvas: the refit watches #stageArea, whose box no scrollbar of its own can change. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 2 +- docs/review-wishlist.md | 25 ++ plugins/vstack/contracts/README.md | 8 +- plugins/vstack/contracts/host.md | 12 +- plugins/vstack/contracts/review-loop.md | 19 +- .../phase-build/assets/build-board.html | 47 +++- .../experimental/spec/assets/spec-tree.html | 47 +++- .../experimental/start/assets/chooser.html | 47 +++- .../{hosts => host-profiles}/claude.json | 0 .../{hosts => host-profiles}/codex.json | 0 .../vstack/{hosts => host-profiles}/grok.json | 0 plugins/vstack/lib/host.mjs | 4 +- plugins/vstack/lib/json-bridge.mjs | 4 +- plugins/vstack/lib/shell/shell.css | 8 + plugins/vstack/lib/shell/shell.js | 28 ++- plugins/vstack/lib/shell/topbar.html | 11 + plugins/vstack/lib/update-check.mjs | 12 + plugins/vstack/skills/review/SKILL.md | 73 +++--- .../skills/review/assets/review-server.mjs | 215 +++++++++-------- .../skills/review/assets/workspace.html | 223 ++++++++++-------- plugins/vstack/skills/review/hosts/claude.md | 13 +- plugins/vstack/skills/review/hosts/codex.md | 13 +- plugins/vstack/skills/review/hosts/grok.md | 17 +- .../skills/review/references/workflow.md | 45 ++-- .../skills/review/tests/host-profiles.mjs | 2 +- .../skills/review/tests/review-lifecycle.mjs | 78 +++++- .../assets/story-map-template.html | 47 +++- 27 files changed, 704 insertions(+), 296 deletions(-) create mode 100644 docs/review-wishlist.md rename plugins/vstack/{hosts => host-profiles}/claude.json (100%) rename plugins/vstack/{hosts => host-profiles}/codex.json (100%) rename plugins/vstack/{hosts => host-profiles}/grok.json (100%) diff --git a/CLAUDE.md b/CLAUDE.md index 4c1dc2c..64989f7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -83,7 +83,7 @@ The layering rule that everything else follows (`plugins/vstack/contracts/README - **Adapters speak hosts.** Only `skills/review/hosts/*.md` may mention host-specific tools (Monitor, Artifact, etc.). A SKILL.md references Host ops (`background`, `watch_stream`, `share`, …); the adapter maps them to tools. -- **Profiles are data.** `hosts/.json` carries UI labels, install steps, and +- **Profiles are data.** `host-profiles/.json` carries UI labels, install steps, and capability flags; servers inject it as `window.__VSTACK_HOST__`, selected by `--host` / `VSTACK_HOST` (default `claude`). Loaded via `lib/host.mjs`. - **On-disk roles are stable:** review threads use `by: "agent" | "reviewer"`. diff --git a/docs/review-wishlist.md b/docs/review-wishlist.md new file mode 100644 index 0000000..b0a8e69 --- /dev/null +++ b/docs/review-wishlist.md @@ -0,0 +1,25 @@ +# Wishlist — the review tool + +Features that have been considered for the review tool and are not being built yet. Each entry says +what it has to do to ship. An entry stays here until its acceptance criteria can be met. + +## Stop a round in flight + +**Status: withdrawn on 5 August 2026**, after an implementation that could not meet criterion 1. + +**What it should do.** The reviewer presses Stop and the agent stops working on that round. + +**Acceptance criteria.** + +1. Stop interrupts the agent's current turn, the way Esc does in the reviewer's own session. + A request the agent has to notice for itself does not qualify. +2. The interruption holds without the agent calling a protocol command. An agent that never calls + `check` still stops. +3. The workspace shows the round as ended once it has ended. + +**What it needs.** A Host op that interrupts the running turn, exposed by every host the plugin +supports — or a host-specific adapter path, with a fallback that says plainly what happens on a host +without the op. `contracts/host.md` has no such op today. + +**Until then.** The reviewer sends again. The brief is the state of the review rather than a diff, +so the next send supersedes the last one. diff --git a/plugins/vstack/contracts/README.md b/plugins/vstack/contracts/README.md index 65ca541..c042e77 100644 --- a/plugins/vstack/contracts/README.md +++ b/plugins/vstack/contracts/README.md @@ -16,7 +16,7 @@ particular agent product. ``` plugins/vstack/ contracts/ ← this directory (the specs) - hosts/ ← profiles that implement Host (claude.json, codex.json, grok.json) + host-profiles/ ← profiles that implement Host (claude.json, codex.json, grok.json) lib/host.mjs ← loads a profile; used by servers skills/review/ SKILL.md ← loop in contract terms (no host-specific tools) @@ -29,10 +29,10 @@ plugins/vstack/ 1. **Engine speaks contracts.** `review-server.mjs`, the workspace, and shared shell never name a product except as data from a Host profile. -2. **Adapters speak hosts.** Only `hosts/*.md` (and the Host profile JSON) may - mention Monitor, Artifact, `monitor`, etc. +2. **Adapters speak hosts.** Only `skills/review/hosts/*.md` (and the Host + profile JSON) may mention Monitor, Artifact, `monitor`, etc. 3. **Profiles are data.** UI labels, install steps, and capability flags come - from `hosts/.json`, injected as `window.__VSTACK_HOST__` and selected by + from `host-profiles/.json`, injected as `window.__VSTACK_HOST__` and selected by `VSTACK_HOST` / `--host`. 4. **On-disk roles are stable.** Review threads use `by: "agent" | "reviewer"`. Older files may still say `"claude"`; readers treat that as `"agent"`. diff --git a/plugins/vstack/contracts/host.md b/plugins/vstack/contracts/host.md index adb9d82..5454219 100644 --- a/plugins/vstack/contracts/host.md +++ b/plugins/vstack/contracts/host.md @@ -4,7 +4,7 @@ A **Host** is the coding-agent product that runs the skill (Claude Code, Codex, Build, …). The review engine does not call into a host. The *agent session* running under a host fulfills these operations by using that host’s tools. -Every Host is described by a **profile** (`hosts/.json`, schema +Every Host is described by a **profile** (`host-profiles/.json`, schema [`host.schema.json`](host.schema.json)). Servers load it; the workspace reads `window.__VSTACK_HOST__`. @@ -46,8 +46,12 @@ node review-server.mjs watch --all --stream - Process must not exit after the first event. - Lines are UTF-8 text, one event per line (see [review-loop.md](review-loop.md)). -- While this process runs, the engine’s `watching` heartbeat is live and the UI - shows **Linked**. +- **The agent must be able to act on a line as it arrives.** +- The engine tests that: the stream opens with a `HANDSHAKE` line naming a + command the agent must run. The `watching` heartbeat starts once it is + answered, and the watcher exits `3` if two minutes pass first. Answering + proves a session is receiving the stream, which is the only claim the UI's + **Linked** state is allowed to make. ### `stop(handle)` — **required** @@ -90,7 +94,7 @@ user provides or skips harvest. When serving a workspace, the server: 1. Resolves Host via `--host ` or env `VSTACK_HOST` (default `claude`). -2. Loads `plugins/vstack/hosts/.json`. +2. Loads `plugins/vstack/host-profiles/.json`. 3. Injects into the page: ```html diff --git a/plugins/vstack/contracts/review-loop.md b/plugins/vstack/contracts/review-loop.md index 56e92e9..7648827 100644 --- a/plugins/vstack/contracts/review-loop.md +++ b/plugins/vstack/contracts/review-loop.md @@ -13,7 +13,7 @@ Host-independent: any Host that fulfills [host.md](host.md) can drive this loop. | --- | --- | | **Engine** | Serves workspace, stores state, freezes versions, emits events | | **Agent** | Applies feedback, publishes versions, replies, fulfills Host ops | -| **Reviewer** | Comments in the browser; Send / Stop / Approve / Share | +| **Reviewer** | Comments in the browser; Send / Approve / Share | --- @@ -47,8 +47,8 @@ a caller never has to pick between the two itself. | `reviews/v/feedback.md` | Markdown brief for the agent | | `reviews/v/feedback.json` | Same, structured | | `rounds/r.json` | Durable membership, revisions, outcomes, and completion record | +| `handshake` | A stream watcher waiting to be told its events are being read | | `pending` | Notification only: review sent, agent must `claim` it | -| `cancel` | Sentinel: reviewer asked to stop the in-flight round | | `approved` | Sentinel: design signed off; engine shutting down | | `share` | Sentinel: reviewer wants a shareable link | | `url` | Present only while `serve` is running | @@ -83,12 +83,12 @@ Host selection: `--host ` or `VSTACK_HOST=` (affects UI injection only). | Command | Contract | | --- | --- | | `serve --file …` / `serve --app …` | Long-lived via Host `background`. Binds `127.0.0.1`. | +| `ack --file/name … --token ` \| `ack --all --token ` | Answer a stream watcher's handshake. Only this arms the `watching` heartbeat | | `claim --file/name … --round r` | Acknowledge delivery while preserving the durable round ledger | | `publish --file/name … --round r --label … [--addressed ids]` | Validate full round coverage, freeze next version, and mark comments addressed | | `reply --file/name … --round r --comment --text "…"` | Append `{ by: "agent", text, at }`; status → `question` | -| `cancelled --file/name … --round r` | Acknowledge Stop, leave comments open, and close the active round | | `share --file/name … --url ` | Record public URL; clear `share` sentinel | -| `check --file/name …` | Exit `0` continue, `2` stop requested | +| `check --file/name …` | Always exits `0`. Names a queued round nobody has claimed | | `status --file/name …` | Human/debug snapshot | | `watch [--all] [--file …] --stream` | Event stream via Host `watch_stream` | @@ -101,9 +101,11 @@ One line of stdout per event (from `watch --stream`): | Prefix | Meaning | Agent action | | --- | --- | --- | | `WATCHING` | Stream armed | — | +| `HANDSHAKE` | The watcher asking whether anyone receives it | Run the `ack` command it prints, immediately | +| `LINKED` | The handshake was answered | — | +| `UNWIRED` | The handshake went unanswered; the watcher exits `3` | Start it again via `watch_stream` | | `REVIEW` | `pending` written; round id and path to `feedback.md` | `claim` the round, apply brief, publish/reply | | `REPLIED` | Reviewer answered a question | Continue that comment’s thread | -| `CANCELLED` | Stop requested | Do not publish half-work; run `cancelled --round …`; report | | `SHARE` | Link requested | Host `share` if capable; then `share --url` | | `APPROVED` | Sign-off; server exiting | Confirm; next pipeline stage as skill says | | `OPENED` | Another live store joined `--all` | — | @@ -125,7 +127,6 @@ reviewer comments ──Send──► round record + pending + feedback.md │ │ │◄──── version ready ─────┘ │ - Stop ──► cancel ──► CANCELLED (agent must check during long rounds) Approve ──► approved ──► APPROVED + server exit Share ──► share ──► SHARE ──► share --url ``` @@ -134,10 +135,12 @@ Rules: 1. Only a validated `publish --round … --addressed …` closes comments (reviewer has no resolve). 2. The engine rejects publication unless every round member is addressed, dismissed, or waiting on the reviewer. -3. The engine rejects unknown IDs, changed comment revisions, unclaimed rounds, stale round IDs, and any publish after Stop. -4. Agent must `check` at checkpoints; `cancelled --round …` acknowledges Stop. Do not delete protocol files manually. +3. The engine rejects unknown IDs, changed comment revisions, unclaimed rounds, and stale round IDs. +4. A round in flight cannot be called off. The reviewer's only correction is to send again, which supersedes the brief. Do not delete protocol files manually. 5. Retrying an already completed `publish --round …` is idempotent and creates no extra version. 6. One `watch_stream` per session is enough with `--all`. +7. Presence is proven. A stream watcher writes its `watching` heartbeat from the moment its handshake is answered, so **Linked** means a session is receiving the stream. Default window 120 s (`--handshake-timeout `). +8. Presence is also claim-backed. The engine reports the agent present (workspace **Linked**) only while the `watching` heartbeat is fresh **and** no queued round has sat unclaimed past the claim window (90 s). A stalled round drops presence — a watcher whose events nobody reads must look the same to the reviewer as no watcher at all. --- diff --git a/plugins/vstack/experimental/phase-build/assets/build-board.html b/plugins/vstack/experimental/phase-build/assets/build-board.html index 452197b..8cf4a27 100644 --- a/plugins/vstack/experimental/phase-build/assets/build-board.html +++ b/plugins/vstack/experimental/phase-build/assets/build-board.html @@ -186,6 +186,14 @@ .cogmenu[hidden]{display:none} .cogmenu .row{display:flex;align-items:center;gap:10px;justify-content:space-between} .cogmenu .lbl{font-size:12px;color:var(--ink-2);white-space:nowrap} +.cogmenu .about{align-items:flex-start} +.cogmenu .abouts{display:grid;gap:3px;justify-items:end} +.cogmenu .abouts .one{display:flex;gap:8px;align-items:baseline} +.cogmenu .abouts em{font-style:normal;font-size:10.5px;letter-spacing:.03em;color:var(--ink-3)} +.cogmenu .abouts b{font:600 11px/1.3 var(--mono);color:var(--ink);font-variant-numeric:tabular-nums} +.cogmenu .stale{font-size:11px;line-height:1.4;color:var(--brand);justify-content:flex-start} +.cogmenu .stale[hidden]{display:none} +.cogmenu .about[hidden]{display:none} /* the live link, stated rather than implied */ .linkdot{display:inline-flex;align-items:center;gap:6px;font:600 10.5px/1 var(--mono); @@ -425,6 +433,17 @@ + + +
@@ -605,6 +624,30 @@ } const hideLink = () => { const el = $('#linkDot'); if (el) el.hidden = true }; + /* ── which version is running ── + The page reports what served it, held from load; the server reports what it + is on now. A tab open across an update shows both and offers the reload. */ + let pageVersion = null, serverVersion = null; + function paintVersions () { + const row = $('#cogAbout'); + if (!row) return; + row.hidden = !pageVersion && !serverVersion; + const put = (id, value) => { const el = $(id); if (el) el.textContent = value || '—' }; + put('#cogVersionPage', pageVersion || serverVersion); + put('#cogVersionServer', serverVersion); + const line = $('#cogServerLine'); + if (line) line.hidden = !serverVersion; + const stale = $('#cogStale'); + if (stale) stale.hidden = !(pageVersion && serverVersion && pageVersion !== serverVersion); + } + /** What the server is on right now, which a page learns from its own payload. */ + function setServerVersion (version) { + const next = version || null; + if (serverVersion === next) return; + serverVersion = next; + paintVersions(); + } + /* ── one live-link client, instead of one per page ── Wires the dot to a server: SSE when the page has an event stream, a plain poll for a server that only answers /ping. Either way the shell owns the @@ -778,6 +821,8 @@ if (opts.wip) wip(true, typeof opts.wip === 'string' ? opts.wip : undefined); name(opts.name, opts.eyebrow); wireSettings(); + pageVersion = (window.__VSTACK_BUILD__ || {}).version || null; + paintVersions(); applyTheme(); applyLang(); updateNotice(); @@ -785,7 +830,7 @@ } const api = { - init, setTheme, setLang, setLink, setWatching, hideLink, name, wip, + init, setTheme, setLang, setLink, setWatching, setServerVersion, hideLink, name, wip, connect, toast, armConfirm, esc, get theme () { return theme }, get lang () { return lang }, diff --git a/plugins/vstack/experimental/spec/assets/spec-tree.html b/plugins/vstack/experimental/spec/assets/spec-tree.html index 263ab91..96973c2 100644 --- a/plugins/vstack/experimental/spec/assets/spec-tree.html +++ b/plugins/vstack/experimental/spec/assets/spec-tree.html @@ -190,6 +190,14 @@ .cogmenu[hidden]{display:none} .cogmenu .row{display:flex;align-items:center;gap:10px;justify-content:space-between} .cogmenu .lbl{font-size:12px;color:var(--ink-2);white-space:nowrap} +.cogmenu .about{align-items:flex-start} +.cogmenu .abouts{display:grid;gap:3px;justify-items:end} +.cogmenu .abouts .one{display:flex;gap:8px;align-items:baseline} +.cogmenu .abouts em{font-style:normal;font-size:10.5px;letter-spacing:.03em;color:var(--ink-3)} +.cogmenu .abouts b{font:600 11px/1.3 var(--mono);color:var(--ink);font-variant-numeric:tabular-nums} +.cogmenu .stale{font-size:11px;line-height:1.4;color:var(--brand);justify-content:flex-start} +.cogmenu .stale[hidden]{display:none} +.cogmenu .about[hidden]{display:none} /* the live link, stated rather than implied */ .linkdot{display:inline-flex;align-items:center;gap:6px;font:600 10.5px/1 var(--mono); @@ -542,6 +550,17 @@ + + +
@@ -834,6 +853,30 @@

} const hideLink = () => { const el = $('#linkDot'); if (el) el.hidden = true }; + /* ── which version is running ── + The page reports what served it, held from load; the server reports what it + is on now. A tab open across an update shows both and offers the reload. */ + let pageVersion = null, serverVersion = null; + function paintVersions () { + const row = $('#cogAbout'); + if (!row) return; + row.hidden = !pageVersion && !serverVersion; + const put = (id, value) => { const el = $(id); if (el) el.textContent = value || '—' }; + put('#cogVersionPage', pageVersion || serverVersion); + put('#cogVersionServer', serverVersion); + const line = $('#cogServerLine'); + if (line) line.hidden = !serverVersion; + const stale = $('#cogStale'); + if (stale) stale.hidden = !(pageVersion && serverVersion && pageVersion !== serverVersion); + } + /** What the server is on right now, which a page learns from its own payload. */ + function setServerVersion (version) { + const next = version || null; + if (serverVersion === next) return; + serverVersion = next; + paintVersions(); + } + /* ── one live-link client, instead of one per page ── Wires the dot to a server: SSE when the page has an event stream, a plain poll for a server that only answers /ping. Either way the shell owns the @@ -1007,6 +1050,8 @@

if (opts.wip) wip(true, typeof opts.wip === 'string' ? opts.wip : undefined); name(opts.name, opts.eyebrow); wireSettings(); + pageVersion = (window.__VSTACK_BUILD__ || {}).version || null; + paintVersions(); applyTheme(); applyLang(); updateNotice(); @@ -1014,7 +1059,7 @@

} const api = { - init, setTheme, setLang, setLink, setWatching, hideLink, name, wip, + init, setTheme, setLang, setLink, setWatching, setServerVersion, hideLink, name, wip, connect, toast, armConfirm, esc, get theme () { return theme }, get lang () { return lang }, diff --git a/plugins/vstack/experimental/start/assets/chooser.html b/plugins/vstack/experimental/start/assets/chooser.html index e304db4..2fcac24 100644 --- a/plugins/vstack/experimental/start/assets/chooser.html +++ b/plugins/vstack/experimental/start/assets/chooser.html @@ -179,6 +179,14 @@ .cogmenu[hidden]{display:none} .cogmenu .row{display:flex;align-items:center;gap:10px;justify-content:space-between} .cogmenu .lbl{font-size:12px;color:var(--ink-2);white-space:nowrap} +.cogmenu .about{align-items:flex-start} +.cogmenu .abouts{display:grid;gap:3px;justify-items:end} +.cogmenu .abouts .one{display:flex;gap:8px;align-items:baseline} +.cogmenu .abouts em{font-style:normal;font-size:10.5px;letter-spacing:.03em;color:var(--ink-3)} +.cogmenu .abouts b{font:600 11px/1.3 var(--mono);color:var(--ink);font-variant-numeric:tabular-nums} +.cogmenu .stale{font-size:11px;line-height:1.4;color:var(--brand);justify-content:flex-start} +.cogmenu .stale[hidden]{display:none} +.cogmenu .about[hidden]{display:none} /* the live link, stated rather than implied */ .linkdot{display:inline-flex;align-items:center;gap:6px;font:600 10.5px/1 var(--mono); @@ -467,6 +475,17 @@ + + +
@@ -702,6 +721,30 @@

} const hideLink = () => { const el = $('#linkDot'); if (el) el.hidden = true }; + /* ── which version is running ── + The page reports what served it, held from load; the server reports what it + is on now. A tab open across an update shows both and offers the reload. */ + let pageVersion = null, serverVersion = null; + function paintVersions () { + const row = $('#cogAbout'); + if (!row) return; + row.hidden = !pageVersion && !serverVersion; + const put = (id, value) => { const el = $(id); if (el) el.textContent = value || '—' }; + put('#cogVersionPage', pageVersion || serverVersion); + put('#cogVersionServer', serverVersion); + const line = $('#cogServerLine'); + if (line) line.hidden = !serverVersion; + const stale = $('#cogStale'); + if (stale) stale.hidden = !(pageVersion && serverVersion && pageVersion !== serverVersion); + } + /** What the server is on right now, which a page learns from its own payload. */ + function setServerVersion (version) { + const next = version || null; + if (serverVersion === next) return; + serverVersion = next; + paintVersions(); + } + /* ── one live-link client, instead of one per page ── Wires the dot to a server: SSE when the page has an event stream, a plain poll for a server that only answers /ping. Either way the shell owns the @@ -875,6 +918,8 @@

if (opts.wip) wip(true, typeof opts.wip === 'string' ? opts.wip : undefined); name(opts.name, opts.eyebrow); wireSettings(); + pageVersion = (window.__VSTACK_BUILD__ || {}).version || null; + paintVersions(); applyTheme(); applyLang(); updateNotice(); @@ -882,7 +927,7 @@

} const api = { - init, setTheme, setLang, setLink, setWatching, hideLink, name, wip, + init, setTheme, setLang, setLink, setWatching, setServerVersion, hideLink, name, wip, connect, toast, armConfirm, esc, get theme () { return theme }, get lang () { return lang }, diff --git a/plugins/vstack/hosts/claude.json b/plugins/vstack/host-profiles/claude.json similarity index 100% rename from plugins/vstack/hosts/claude.json rename to plugins/vstack/host-profiles/claude.json diff --git a/plugins/vstack/hosts/codex.json b/plugins/vstack/host-profiles/codex.json similarity index 100% rename from plugins/vstack/hosts/codex.json rename to plugins/vstack/host-profiles/codex.json diff --git a/plugins/vstack/hosts/grok.json b/plugins/vstack/host-profiles/grok.json similarity index 100% rename from plugins/vstack/hosts/grok.json rename to plugins/vstack/host-profiles/grok.json diff --git a/plugins/vstack/lib/host.mjs b/plugins/vstack/lib/host.mjs index 80c97b0..1650f06 100644 --- a/plugins/vstack/lib/host.mjs +++ b/plugins/vstack/lib/host.mjs @@ -1,7 +1,7 @@ /** * host.mjs — load a Host profile (contracts/host.md). * - * Profiles live in plugins/vstack/hosts/.json. Servers inject the profile + * Profiles live in plugins/vstack/host-profiles/.json. Servers inject the profile * into pages as window.__VSTACK_HOST__. Skills never hardcode product names * in the engine; they pass --host / VSTACK_HOST and read the adapter markdown. */ @@ -12,7 +12,7 @@ import { fileURLToPath } from 'node:url' import { injectHead } from './live-link.mjs' const HERE = path.dirname(fileURLToPath(import.meta.url)) -const HOSTS_DIR = path.join(HERE, '..', 'hosts') +const HOSTS_DIR = path.join(HERE, '..', 'host-profiles') const DEFAULT_ID = 'claude' diff --git a/plugins/vstack/lib/json-bridge.mjs b/plugins/vstack/lib/json-bridge.mjs index 5cb0c44..212c745 100644 --- a/plugins/vstack/lib/json-bridge.mjs +++ b/plugins/vstack/lib/json-bridge.mjs @@ -57,7 +57,7 @@ import path from 'node:path' import http from 'node:http' import crypto from 'node:crypto' import { fileURLToPath } from 'node:url' -import { checkForUpdate, dismissUpdate, withUpdate } from './update-check.mjs' +import { checkForUpdate, dismissUpdate, withUpdate, withVersion } from './update-check.mjs' import { loadHost, resolveHostId, withHost } from './host.mjs' import { workDir, findWorkDir, TOOL } from './workdir.mjs' import { writeAtomic, watchingRecently, startHeartbeat, startPresence, openInBrowser } from './live-link.mjs' @@ -237,7 +237,7 @@ function page () { const doc = /^\s*]*>)/i, `$1\n${handle}`) : `\n\n\n${handle}${body}` - return withUpdate(withHost(doc, HOST), update) + return withUpdate(withVersion(withHost(doc, HOST)), update) } const send = (res, code, type, body) => { diff --git a/plugins/vstack/lib/shell/shell.css b/plugins/vstack/lib/shell/shell.css index 29baddd..d4493c9 100644 --- a/plugins/vstack/lib/shell/shell.css +++ b/plugins/vstack/lib/shell/shell.css @@ -106,6 +106,14 @@ .cogmenu[hidden]{display:none} .cogmenu .row{display:flex;align-items:center;gap:10px;justify-content:space-between} .cogmenu .lbl{font-size:12px;color:var(--ink-2);white-space:nowrap} +.cogmenu .about{align-items:flex-start} +.cogmenu .abouts{display:grid;gap:3px;justify-items:end} +.cogmenu .abouts .one{display:flex;gap:8px;align-items:baseline} +.cogmenu .abouts em{font-style:normal;font-size:10.5px;letter-spacing:.03em;color:var(--ink-3)} +.cogmenu .abouts b{font:600 11px/1.3 var(--mono);color:var(--ink);font-variant-numeric:tabular-nums} +.cogmenu .stale{font-size:11px;line-height:1.4;color:var(--brand);justify-content:flex-start} +.cogmenu .stale[hidden]{display:none} +.cogmenu .about[hidden]{display:none} /* the live link, stated rather than implied */ .linkdot{display:inline-flex;align-items:center;gap:6px;font:600 10.5px/1 var(--mono); diff --git a/plugins/vstack/lib/shell/shell.js b/plugins/vstack/lib/shell/shell.js index 6dacad7..1d46b94 100644 --- a/plugins/vstack/lib/shell/shell.js +++ b/plugins/vstack/lib/shell/shell.js @@ -103,6 +103,30 @@ window.VSShell = (function () { } const hideLink = () => { const el = $('#linkDot'); if (el) el.hidden = true }; + /* ── which version is running ── + The page reports what served it, held from load; the server reports what it + is on now. A tab open across an update shows both and offers the reload. */ + let pageVersion = null, serverVersion = null; + function paintVersions () { + const row = $('#cogAbout'); + if (!row) return; + row.hidden = !pageVersion && !serverVersion; + const put = (id, value) => { const el = $(id); if (el) el.textContent = value || '—' }; + put('#cogVersionPage', pageVersion || serverVersion); + put('#cogVersionServer', serverVersion); + const line = $('#cogServerLine'); + if (line) line.hidden = !serverVersion; + const stale = $('#cogStale'); + if (stale) stale.hidden = !(pageVersion && serverVersion && pageVersion !== serverVersion); + } + /** What the server is on right now, which a page learns from its own payload. */ + function setServerVersion (version) { + const next = version || null; + if (serverVersion === next) return; + serverVersion = next; + paintVersions(); + } + /* ── one live-link client, instead of one per page ── Wires the dot to a server: SSE when the page has an event stream, a plain poll for a server that only answers /ping. Either way the shell owns the @@ -276,6 +300,8 @@ window.VSShell = (function () { if (opts.wip) wip(true, typeof opts.wip === 'string' ? opts.wip : undefined); name(opts.name, opts.eyebrow); wireSettings(); + pageVersion = (window.__VSTACK_BUILD__ || {}).version || null; + paintVersions(); applyTheme(); applyLang(); updateNotice(); @@ -283,7 +309,7 @@ window.VSShell = (function () { } const api = { - init, setTheme, setLang, setLink, setWatching, hideLink, name, wip, + init, setTheme, setLang, setLink, setWatching, setServerVersion, hideLink, name, wip, connect, toast, armConfirm, esc, get theme () { return theme }, get lang () { return lang }, diff --git a/plugins/vstack/lib/shell/topbar.html b/plugins/vstack/lib/shell/topbar.html index 0039c30..ce1bde4 100644 --- a/plugins/vstack/lib/shell/topbar.html +++ b/plugins/vstack/lib/shell/topbar.html @@ -57,6 +57,17 @@ + + +
diff --git a/plugins/vstack/lib/update-check.mjs b/plugins/vstack/lib/update-check.mjs index 2cd9788..4272bbe 100644 --- a/plugins/vstack/lib/update-check.mjs +++ b/plugins/vstack/lib/update-check.mjs @@ -216,6 +216,18 @@ export async function checkForUpdate (hostProfile = null) { } } +/** The version of the copy that is running. */ +export const currentVersion = () => manifestVersion() + +/** Tell a served page which version served it, so the page can say so without + reaching for anything. A page kept open across an update still reports the + version it loaded with, which is the point. */ +export function withVersion (html) { + const version = manifestVersion() + if (!version) return html + return injectHead(html, `\n`) +} + /** Put the handle into a served page. No-op when there is nothing to say, so callers can apply it unconditionally. */ export function withUpdate (html, info) { diff --git a/plugins/vstack/skills/review/SKILL.md b/plugins/vstack/skills/review/SKILL.md index 39c90b2..4e3f4e9 100644 --- a/plugins/vstack/skills/review/SKILL.md +++ b/plugins/vstack/skills/review/SKILL.md @@ -12,9 +12,13 @@ this file** — they live only in the Host adapter. | You are running under… | Load adapter | Set | | --- | --- | --- | -| **Codex** | `hosts/codex.md` | `VSTACK_HOST=codex` (or `--host codex` on `serve`) | -| **Grok** Build / Grok CLI | `hosts/grok.md` | `VSTACK_HOST=grok` (or `--host grok` on `serve`) | -| **Claude Code** | `hosts/claude.md` | `VSTACK_HOST=claude` (default if unset) | +| **Codex** | `skills/review/hosts/codex.md` | `VSTACK_HOST=codex` (or `--host codex` on `serve`) | +| **Grok** Build / Grok CLI | `skills/review/hosts/grok.md` | `VSTACK_HOST=grok` (or `--host grok` on `serve`) | +| **Claude Code** | `skills/review/hosts/claude.md` | `VSTACK_HOST=claude` (default if unset) | + +Adapters live in the `hosts/` directory beside this SKILL.md. Do not read +`plugins/vstack/host-profiles/.json` instead: that JSON is UI data with no +tool mapping. **Read the adapter before §3.** Every `background`, `watch_stream`, `stop`, `share`, and `browser_capture` step is fulfilled exactly as that file says. @@ -138,7 +142,6 @@ The page opens in **its own browser window** on the canvas — own viewport, own | **Addressed** | comments you closed stay in the list in their own section, each offering **Revert** or **Refine** | | **Publish a link to this wireframe** (the ▾ beside Send) | only when Host `capabilities.share` is `artifact`. Asks you to publish **the wireframe** (Host op `share`) and hand the URL back. Hidden on hosts without public share, and in a live review | | **Approve & finish** (the ▾ beside Send) | sign-off. Ends the review, closes the server, and tells you the design is settled — behind a confirm that warns how many comments are being left unapplied | -| **Cancel** | stops the round you're working on. Not a kill: finish what you were mid-way through, then say what you'd already changed | There is no per-comment resolve button: **you** close comments out by addressing them. Approve is the whole-page verdict, not an item-by-item one — one click that means *the design is done*, which is the @@ -158,56 +161,56 @@ accepts a status going backwards when the reviewer deliberately sent it back. ## 5 · Catch the review, and hold up your end of the conversation -After starting the server, start the watcher with Host op **`watch_stream`** (adapter -names the tool). Prefer the streaming form so nothing has to be re-armed: +After starting the server, start the watcher with Host op **`watch_stream`**. Look up the tool for +that op in your adapter — `watch_stream` delivers each line of output to you as it arrives, and it +is a different op from the `background` you used in §3. ```bash node "$SKILL/assets/review-server.mjs" watch --all --stream # or --file ``` -It never exits. Each line of its output is one event, delivered to you as it happens, and the -process keeps running — so there is nothing to re-arm after a round, which is the step that gets -forgotten and leaves a review nobody is reading. `--all` covers every review open in the project, -so a session with a wireframe and a story map up needs one watcher, not one each. +Each line of its output is one event, delivered to you as it happens, and the process keeps running, +so one watcher covers the whole session — `--all` takes in every review open in the project, +including ones opened later. + +**It opens with a `HANDSHAKE` line naming a command. Run that command straight away.** The watcher +goes live once you answer, the workspace says **Linked**, and the round events start reaching you. +Answering proves the op was fulfilled, since only a session that can run commands can answer. +Answer within two minutes; after that the watcher prints `UNWIRED` and exits, and you start it again +with the tool your adapter names for `watch_stream`. -While it runs the page says **Linked**; with no watcher it says **Unlinked**, in amber, so the -reviewer can see that what they send will sit there. +The page says **Linked** for as long as the watcher is answered and the rounds are being claimed, +and **Unlinked** in amber the rest of the time, so the reviewer always knows which one they have. Each event is one line (full table: `contracts/review-loop.md`): | | What it means | What you do | |---|---|---| +| **`HANDSHAKE`** | the watcher asking whether you can hear it | run the command it prints, now — it is what brings the watcher live | +| **`UNWIRED`** | the handshake went unanswered and the watcher exited | start it again with the tool your adapter names for `watch_stream` | | **`REVIEW`** | a review landed; the line names its round and brief | `claim` the round, then apply it — the steps below | | **`REPLIED`** | they answered a question you asked | read the thread and carry on with that comment. Nothing else announces this — a reply writes no sentinel | -| **`CANCELLED`** | the reviewer pressed **Stop** | don't publish what you had half-done. Say what you had already changed and what you hadn't, then run `cancelled --round …` | | **`SHARE`** | they want a link to send someone | Host op `share` if capable, then §6; if the Host cannot share publicly, say so and offer a file/bundle instead | | **`APPROVED`** | the design is signed off; the server has closed itself | say it's approved, note any `openComments` deliberately left, and carry on with whatever comes next | | **`CLOSED`** | that review's tab went away | the watcher drops it and keeps watching the rest; it only stops when none are left | -**Use the protocol commands rather than deleting state files.** `claim --round …` consumes `pending`, -`cancelled --round …` acknowledges Stop, and `share --url` clears `share`. The durable round record -remains available for validation, recovery, and idempotent retries. +**Use the protocol commands rather than deleting state files.** `claim --round …` consumes `pending` +and `share --url` clears `share`. The durable round record remains available for validation, +recovery, and idempotent retries. -### Stopping a round in flight +### Checking during a round -While you work, **no waiter is armed** — you are the only thing that can notice the reviewer changing -their mind. The workspace's **Stop** button writes the `cancel` sentinel; nothing in it can interrupt -a turn already running, so a round that never looks is a round that cannot be stopped. +While you work, **no waiter is armed** — nothing will interrupt you, and a round in flight cannot be +called off. If the reviewer changes their mind they send again, and that brief supersedes. -**Check at every checkpoint of a round** — after reading the feedback, between batches of edits, and -always immediately before `publish`: +**Check at the checkpoints of a long round** — after reading the feedback, and before `publish`: ```bash -node "$SKILL/assets/review-server.mjs" check --file "$FILE" || STOP=1 +node "$SKILL/assets/review-server.mjs" check --file "$FILE" ``` -Exit 2 means stop. Then: - -1. **Don't publish.** A half-applied version published as a new one is the worst outcome — the - reviewer now has to review your interrupted work. -2. Leave the file as it is. Say plainly what you had already changed and what you hadn't. -3. Run `node "$SKILL/assets/review-server.mjs" cancelled --file "$FILE" --round `. - The review is still open; only this round ended, and the watcher is still running. +It always exits 0. It exists to name a round sitting in the queue that nobody has claimed: if it does, +claim that round before anything else, because comments are sitting unread. The longer the round, the more it matters: a check costs nothing, and one that never runs makes the button a lie. @@ -233,7 +236,7 @@ On a review landing: --round r17 --label "Filters collapsed, overdue sorts first" --addressed c1f3k2,c9dk1 ``` Only `--addressed` marks a comment done. Publish fails before creating a version if the round was - not claimed, an id is unknown or stale, Stop is outstanding, or any open comment is unaccounted for. + not claimed, an id is unknown or stale, or any open comment is unaccounted for. 7. Leave **`watch_stream` running** and say what changed in a few lines. Then wait — don't ask "shall I continue?", the loop is the point. (Only re-arm if you used one-shot `watch` without `--stream`.) **Closing the browser tab closes the review.** The workspace holds an SSE @@ -303,7 +306,7 @@ node "$SKILL/assets/bundle-artifact.mjs" --file "$FILE" --out review.html ## 7 · Reviewing a UI that already exists Point the same workspace at a running app and everything above still holds — the -modes, the marks, the threads, the timeline, Stop, Approve. Three things differ, +modes, the marks, the threads, the timeline, Approve. Three things differ, and they all follow from the same fact: **what is under review is code, not a file you own.** @@ -406,12 +409,12 @@ route. - State lives in `/.vstack/local/review//` beside the file — versions, reviews, threads, and the sentinels. The page itself stays clean. A live review has nothing to sit beside, so it lands in `.vstack/local/review//` under the directory you started it from. - **Every vstack tool writes under `.vstack/local//`**, so a project grows one dot-directory, not one per engine. `lib/workdir.mjs` resolves it — use that rather than joining the path by hand. One gitignore line covers the lot (`**/.vstack/local/`); the rest of `.vstack/` is the pipeline and belongs in the repo. - The server binds to `127.0.0.1` only. Port 7788 busy usually means a review server is already running — pass `--port`. -- `node "$SKILL/assets/review-server.mjs" status --file "$FILE"` prints the current version, whether a review is waiting, and any stop / sign-off / share request outstanding. -- `check --file "$FILE"` is the same question reduced to an exit code — 0 carry on, 2 stop. Use it inside a round, where `status` is too much output to read repeatedly. +- `node "$SKILL/assets/review-server.mjs" status --file "$FILE"` prints the current version, whether a review is waiting, and any sign-off / share request outstanding. +- `check --file "$FILE"` is the same question in one line, and always exits 0. Use it inside a round, where `status` is too much output to read repeatedly. If it names a round waiting unclaimed, claim that round before anything else — comments are sitting unread. - **Every command takes `--name ` in place of `--file` for a live review** — `publish`, `reply`, `share`, `status`, `check`. The brief tells you which name to use. - Full command reference and troubleshooting: `references/workflow.md`. - Contracts: `plugins/vstack/contracts/` — Host ops and review-loop protocol. -- Host adapters: `hosts/claude.md`, `hosts/codex.md`, `hosts/grok.md`. +- Host adapters: `skills/review/hosts/claude.md`, `skills/review/hosts/codex.md`, `skills/review/hosts/grok.md`. ## State & handoff diff --git a/plugins/vstack/skills/review/assets/review-server.mjs b/plugins/vstack/skills/review/assets/review-server.mjs index e2acb1b..6129e78 100644 --- a/plugins/vstack/skills/review/assets/review-server.mjs +++ b/plugins/vstack/skills/review/assets/review-server.mjs @@ -21,10 +21,10 @@ * node review-server.mjs claim --file --round r1 * node review-server.mjs publish --file --round r1 --label "…" [--addressed c1,c3] * node review-server.mjs reply --file --round r1 --comment --text "…" - * node review-server.mjs cancelled --file --round r1 + * node review-server.mjs ack --file --token * node review-server.mjs share --file --url * node review-server.mjs status --file - * node review-server.mjs check --file (exit 2 = stop asked) + * node review-server.mjs check --file (names a round nobody has claimed) * node review-server.mjs watch --file (blocks until there is something to do) * * Every command takes `--app ` or `--name ` in place of `--file` when @@ -38,8 +38,8 @@ * versions/v.meta.json label, date, what it addressed * reviews/v/ annotations.json · feedback.json · feedback.md * pending sentinel written on send, watched by the agent + * handshake a stream watcher waiting to be told its events land * rounds/r.json durable round membership and completion record - * cancel sentinel written when the reviewer calls a round off * approved sentinel written on sign-off — the review is over * share sentinel — they want a shareable public link * url the live URL — present only while the server runs @@ -63,9 +63,9 @@ import https from 'node:https' import zlib from 'node:zlib' import fs from 'node:fs' import path from 'node:path' -import { createHash } from 'node:crypto' +import { createHash, randomBytes } from 'node:crypto' import { fileURLToPath } from 'node:url' -import { checkForUpdate, dismissUpdate, withUpdate } from '../../../lib/update-check.mjs' +import { checkForUpdate, currentVersion, dismissUpdate, withUpdate, withVersion } from '../../../lib/update-check.mjs' import { resolveHostId, loadHost, withHost, AGENT_ROLE, REVIEWER_ROLE } from '../../../lib/host.mjs' import { workDir, subjectDir, toolNames, LOCAL, TOOL } from '../../../lib/workdir.mjs' import { writeAtomic, watchingRecently, startHeartbeat, startPresence, openInBrowser } from '../../../lib/live-link.mjs' @@ -127,7 +127,7 @@ if (LIVE) { console.error(' are rewritten to stay inside the proxy, but bot protection, a login wall or a') console.error(' strict CSRF check can still refuse it. If the site misbehaves, say so.') } -} else if (args._ === 'watch' && (args.all === true || args.all === 'true')) { +} else if (['watch', 'ack'].includes(args._) && (args.all === true || args.all === 'true')) { /* `watch --all` names no subject on purpose — it finds the live ones itself, so a session with several pages open arms one waiter instead of one each. */ DIR = process.cwd(); NAME = 'all'; STORE = workDir(DIR, TOOL.review) @@ -160,7 +160,7 @@ const P = { round: id => path.join(STORE, 'rounds', `${id}.json`), lock: () => path.join(STORE, 'transition.lock'), pending: () => path.join(STORE, 'pending'), - cancel: () => path.join(STORE, 'cancel'), + handshake: () => path.join(STORE, 'handshake'), approved: () => path.join(STORE, 'approved'), share: () => path.join(STORE, 'share'), url: () => path.join(STORE, 'url'), @@ -300,6 +300,9 @@ function commentRevision (comment) { return createHash('sha256').update(JSON.stringify(value)).digest('hex').slice(0, 16) } +/* `cancelled` is still terminal here although nothing writes it any more: a + store filled before the Stop control was withdrawn can hold one, and reading + it as live would hand the agent a round the reviewer called off. */ function loadActiveRound (state = loadState()) { if (!state.activeRound) return null const round = readJSON(P.round(state.activeRound)) @@ -344,7 +347,6 @@ function nextRound (version, comments, feedback) { round.comments = [...members.values()] round.feedback = feedback round.updatedAt = new Date().toISOString() - if (fs.existsSync(P.cancel())) round.status = 'queued' return saveActiveRound(round) } @@ -375,12 +377,26 @@ function migrateLegacyPending () { return round } +/* "Linked" must mean someone will act on what the reviewer sends, not that a + watch process is alive. The heartbeat proves the process; a round nobody + claims within this window proves its events go unread — a watcher started + with the wrong host op, a dead session, and a killed watcher all look the + same from here. 90s gives an agent mid-turn time to reach the claim. */ +/* A round leaves the queue only by being claimed, so its wait is measured from + when it was created. Nothing else about the round decides this: a heartbeat + with a round nobody has picked up is the state this exists to catch. */ +const CLAIM_STALL_MS = 90_000 +const roundStalled = round => !!round && round.status === 'queued' && + Date.now() - Date.parse(round.createdAt || '') > CLAIM_STALL_MS +const agentListening = () => someoneWatching() && !roundStalled(loadActiveRound()) + function roundSummary (round) { if (!round) return null return { id: round.id, status: round.status, baseVersion: round.baseVersion, comments: (round.comments || []).map(comment => comment.id), createdAt: round.createdAt, claimedAt: round.claimedAt || null, + stalled: roundStalled(round), } } @@ -438,7 +454,6 @@ function cmdPublish (quiet) { else if (requestedRound !== active.id) errors.push(`active round is ${active.id}, not ${requestedRound}`) if (active.status !== 'active') errors.push(`claim ${active.id} before publishing it`) if (args.replace === true || args.replace === 'true') errors.push('--replace cannot complete an active review round') - if (fs.existsSync(P.cancel())) errors.push('the reviewer asked to stop this round') const members = new Map((active.comments || []).map(comment => [comment.id, comment])) for (const id of addressed) if (!members.has(id)) errors.push(`${id} does not belong to ${active.id}`) @@ -467,9 +482,6 @@ function cmdPublish (quiet) { } else if (addressed.length) { console.error('Cannot mark comments addressed without an active review round') process.exit(2) - } else if (fs.existsSync(P.cancel())) { - console.error('Cannot publish: the reviewer asked to stop') - process.exit(2) } // Validation is complete. Nothing above this line mutates a version or a @@ -591,25 +603,6 @@ function cmdClaim () { touch() } -/** Acknowledge that a requested stop was honored. Publish remains blocked until - * this explicit transition closes the active round and clears the request. */ -function cmdCancelled () { - const request = readJSON(P.cancel()) - if (!request) { console.error('No cancel request to acknowledge'); process.exit(2) } - const round = loadActiveRound() - const requested = args.round && args.round !== true ? String(args.round) : null - if (round && (!requested || requested !== round.id)) { - if (!requested) console.error(`Include --round ${round.id}`) - else console.error(`Active round is ${round.id}, not ${requested}`) - process.exit(2) - } - if (round) finishActiveRound(round, 'cancelled', { reason: request.reason || null }) - fs.rmSync(P.pending(), { force: true }) - fs.rmSync(P.cancel(), { force: true }) - console.log(`Cancelled ${round?.id || 'the current review round'} — open comments were left open`) - touch() -} - /** * Hand the published Artifact's URL back to the workspace. It appears under the * ▾ beside Send, tagged with the version it was published from — so a link that @@ -631,28 +624,18 @@ function cmdShare () { } /** - * "Should I still be doing this?" — one cheap call, made between steps of a - * round. Exit 2 means the reviewer pressed Stop while you were working. - * - * This is the whole mechanism behind Stop, and it only works if it is actually - * called: nothing here can interrupt a turn that is already running, so a round - * that never checks cannot be stopped until it ends. + * "Is anything waiting on me?" — one cheap call, made between steps of a round. + * A round sitting in the queue is named here and nothing suppresses it: an agent + * asking and being told nothing, twice, while six comments sat queued is exactly + * how a broken watcher stays broken. Exit is always 0. */ function cmdCheck () { - const req = fs.existsSync(P.cancel()) ? readJSON(P.cancel(), {}) : null - if (!req) { - if (!args.quiet) console.log('carry on') - process.exit(0) - } - console.log('STOP — the reviewer asked you to stop this round.') - console.log(` asked at ${req.at || 'unknown'}`) - if (req.comments?.length) console.log(` in flight ${req.comments.join(', ')}`) - console.log(` reason ${req.reason || '(none given)'}`) - console.log('\nStop where you are. Do not publish a half-applied version. Tell the') - console.log('reviewer what you had already changed and what you left alone, then acknowledge it:') - const round = loadActiveRound() - console.log(` node review-server.mjs cancelled ${SUBJECT}${round ? ` --round ${round.id}` : ''}`) - process.exit(2) + const waiting = loadActiveRound() + if (waiting?.status === 'queued') { + console.log(`carry on — but ${waiting.id} (${(waiting.comments || []).length} comment(s), sent ${waiting.createdAt}) is waiting unclaimed.`) + console.log(`Claim it: node review-server.mjs claim ${SUBJECT} --round ${waiting.id}`) + } else console.log('carry on') + process.exit(0) } /** @@ -718,6 +701,34 @@ function liveStores (from = process.cwd(), depth = 5) { return found } +/* How long a stream watcher waits to be told its events are being read. Long + enough that a session which started it mid-turn still gets there; + `--handshake-timeout ` for a host that needs longer. */ +const HANDSHAKE_MS = Math.max(1, Number(args['handshake-timeout']) || 120) * 1000 +/* Live only once a stream watcher has been answered, or straight away for the + one-shot watch, which proves itself by exiting. */ +let heartbeat = null +const stopBeating = () => { heartbeat?.stop(); heartbeat = null } + +/** + * Answer a stream watcher's handshake. Only a session that can run commands can + * do this, which is exactly what the watcher needs to know about itself. + */ +function cmdAck () { + const waiting = readJSON(P.handshake()) + if (!waiting) { + console.log('Nothing to answer — no watcher is waiting on a handshake for this review.') + return + } + const token = args.token && args.token !== true ? String(args.token) : null + if (token !== waiting.token) { + console.error('That is not the token the waiting watcher printed — read its HANDSHAKE line again.') + process.exit(2) + } + fs.rmSync(P.handshake(), { force: true }) + console.log('Answered — the watcher is wired to this session, and the workspace says Linked.') +} + /** * `watch --stream` — the same watch, as an event stream that never ends. * @@ -731,12 +742,40 @@ function liveStores (from = process.cwd(), depth = 5) { * * node review-server.mjs watch --all --stream */ -async function cmdStream (stores, beatAll, stop, label, all) { +async function cmdStream (stores, label, all, subjectFlags) { const seen = new Map(stores.map(s => [s, { sent: null, flags: new Set(), replies: repliesIn(s) }])) const say = line => { process.stdout.write(line + '\n') } say(`WATCHING ${stores.length} review(s): ${stores.map(label).join(', ')}`) + /* Presence is proven before it is claimed. Nothing here can tell which tool + started this process — every host spawns children the same way — so ask for + the one thing only a live session can do, and run a command back. The + heartbeat starts when that lands, which is what makes the page's Linked + mean a session is receiving this stream. */ + const token = randomBytes(4).toString('hex') + fs.mkdirSync(STORE, { recursive: true }) + writeJSON(P.handshake(), { token, at: new Date().toISOString(), pid: process.pid }) + say(`HANDSHAKE this stream is not live until you answer it. Run now:`) + say(` node "${process.argv[1]}" ack ${subjectFlags} --token ${token}`) + const askedAt = Date.now() + while (true) { + if (!heartbeat) { + if (!fs.existsSync(P.handshake())) { + heartbeat = startHeartbeat(() => stores.map(store => inStore(store, 'watching'))) + say('LINKED handshake answered — the workspace says Linked from here') + } else if (Date.now() - askedAt > HANDSHAKE_MS) { + /* Exiting is the point: on a host where a finished background command + re-invokes the session, this delivers itself to whoever started the + watcher. */ + fs.rmSync(P.handshake(), { force: true }) + say('UNWIRED the handshake went unanswered, so these events reach no one.') + say(' Start this again with the Host op watch_stream, using the tool') + say(' your Host adapter names for it.') + return process.exit(3) + } + } + for (const store of [...stores]) { const at = n => inStore(store, n) const was = seen.get(store) @@ -750,7 +789,7 @@ async function cmdStream (stores, beatAll, stop, label, all) { } // Each sentinel is announced once per appearance, not once per poll. - for (const [file, what] of [['approved', 'APPROVED '], ['cancel', 'CANCELLED'], ['share', 'SHARE ']]) { + for (const [file, what] of [['approved', 'APPROVED '], ['share', 'SHARE ']]) { if (fs.existsSync(at(file))) { if (!was.flags.has(file)) { say(`${what} ${label(store)} · read ${at(file)}`); was.flags.add(file) } } else was.flags.delete(file) @@ -789,9 +828,9 @@ async function cmdStream (stores, beatAll, stop, label, all) { // stream and the heartbeat path alive so a later serve can OPENED in. // Without --all, empty means the only subject closed: done. if (!stores.length) { - if (!all) { stop(); say('CLOSED nothing left to watch'); return process.exit(0) } + if (!all) { stopBeating(); say('CLOSED nothing left to watch'); return process.exit(0) } } - beatAll() + heartbeat?.beat() await new Promise(r => setTimeout(r, 1000)) } } @@ -831,12 +870,8 @@ async function cmdWatch () { } const label = store => path.basename(store) - // `stores` shrinks as reviews close; the heartbeat re-reads it every beat. - const hb = startHeartbeat(() => stores.map(store => inStore(store, 'watching'))) - const beatAll = hb.beat - const stop = hb.stop - process.on('SIGINT', () => { stop(); process.exit(130) }) - process.on('SIGTERM', () => { stop(); process.exit(143) }) + process.on('SIGINT', () => { stopBeating(); process.exit(130) }) + process.on('SIGTERM', () => { stopBeating(); process.exit(143) }) touch() // the page hears about it straight away if (args.stream === true || args.stream === 'true') { @@ -845,9 +880,14 @@ async function cmdWatch () { if (!stores.length && all) { process.stdout.write('WATCHING 0 review(s): waiting for a live serve…\n') } - return cmdStream(stores, beatAll, stop, label, all) + // The stream arms its heartbeat only once its handshake is answered. + return cmdStream(stores, label, all, all ? '--all' : SUBJECT) } + /* The one-shot form proves itself by exiting, which is what delivers its + event, so it needs no handshake. `stores` shrinks as reviews close, and the + heartbeat re-reads it every beat. */ + heartbeat = startHeartbeat(() => stores.map(store => inStore(store, 'watching'))) console.log(`watching ${stores.length} review(s): ${stores.map(label).join(', ')}`) /* Exiting IS the wake-up — a running process cannot interrupt an idle agent session, so the only way to be called is to finish. That makes re-arming @@ -856,7 +896,7 @@ async function cmdWatch () { puts it back. Prefer `watch --stream` via Host op watch_stream. */ const rearm = `node "${process.argv[1]}" ${process.argv.slice(2).join(' ')}` const done = (what, store, file) => { - stop() + stopBeating() console.log(`${what} ${label(store)}`) if (file) { try { console.log(fs.readFileSync(file, 'utf8')) } catch {} } console.log(`\nThis one-shot watch has now ended. Either restart it:\n ${rearm}`) @@ -871,7 +911,6 @@ async function cmdWatch () { for (const store of [...stores]) { const at = n => inStore(store, n) if (fs.existsSync(at('approved'))) return done('APPROVED', store, at('approved')) - if (fs.existsSync(at('cancel'))) return done('CANCELLED', store, at('cancel')) if (fs.existsSync(at('share'))) return done('SHARE', store, at('share')) if (fs.existsSync(at('pending'))) return done('REVIEW', store, at('pending')) if (!fs.existsSync(at('url'))) { @@ -883,7 +922,7 @@ async function cmdWatch () { if (!stores.length) break await new Promise(r => setTimeout(r, 1000)) } - stop() + stopBeating() console.log('CLOSED — nothing left to watch. Nothing to re-arm.') process.exit(0) } @@ -904,7 +943,6 @@ function cmdStatus () { versions: listVersions().map(v => `v${v.n}: ${v.label}`), activeRound: roundSummary(loadActiveRound(state)), pendingReview: fs.existsSync(P.pending()) ? readJSON(P.pending(), {}) : null, - cancelRequest: fs.existsSync(P.cancel()) ? readJSON(P.cancel(), {}) : null, approved: fs.existsSync(P.approved()) ? readJSON(P.approved(), {}) : null, shareRequest: fs.existsSync(P.share()) ? readJSON(P.share(), {}) : null, shareUrl: loadState().shareUrl || null, @@ -917,7 +955,7 @@ const clients = new Set() let reloadTimer = null /* Only when it changes — a heartbeat file ticking every two seconds is not worth a message every two seconds. */ -startPresence(clients, someoneWatching).unref?.() +startPresence(clients, agentListening).unref?.() /* Set once the server is listening, so a request handler can end the review. */ let closeServer = null /* Live-page bookkeeping, so the server can close itself when the tab does. */ @@ -965,6 +1003,9 @@ function payload () { } return { mode: LIVE ? 'live' : 'local', + // What this server is on now. A tab opened before an update still holds the + // version that served it, so the workspace can show both. + version: currentVersion(), name: pageName(), fileName: LIVE ? (APP ? APP.host : state.app || '') : path.basename(FILE), app: appOrigin(), @@ -983,12 +1024,12 @@ function payload () { or a second tab, showed a review where nothing was happening. */ pendingReview: fs.existsSync(P.pending()) ? readJSON(P.pending(), {}) : null, activeReview: roundSummary(activeRound), - cancelRequest: fs.existsSync(P.cancel()) ? readJSON(P.cancel(), {}) : null, /* Whether an agent session is actually waiting on this review. The link dot used to say "Linked" whenever the page could reach this server, which is a fact about the browser and the file server — not about anyone being - there to read what you send. */ - watching: someoneWatching(), + there to read what you send. A live heartbeat with a round sitting + unclaimed is the same lie one layer up, so that drops it too. */ + watching: agentListening(), } } @@ -1197,7 +1238,7 @@ function proxyUpgrade (req, socket, head) { function serveWorkspace (res) { let html = fs.readFileSync(path.join(HERE, 'workspace.html'), 'utf8') if (BASE) html = html.replace(//i, `\n`) - html = withHost(html, HOST_PROFILE) + html = withVersion(withHost(html, HOST_PROFILE)) send(res, 200, withUpdate(html, update), MIME['.html']) } @@ -1225,7 +1266,7 @@ async function handle (req, res) { idleSince = null // Presence rides the same stream: a waiter starting or stopping is news the // page needs, and it is the one change no file write announces. - try { res.write(`event: presence\ndata: ${JSON.stringify({ watching: someoneWatching() })}\n\n`) } catch {} + try { res.write(`event: presence\ndata: ${JSON.stringify({ watching: agentListening() })}\n\n`) } catch {} const ping = setInterval(() => { try { res.write(': ping\n\n') } catch {} }, 25000) req.on('close', () => { clearInterval(ping) @@ -1329,8 +1370,6 @@ font:14px/1.6 ui-sans-serif,system-ui,-apple-system,sans-serif;color:#667;backgr feedback: path.join(dir, 'feedback.md'), sentAt: stillOut ? prev.sentAt : new Date().toISOString(), }) - // A new review supersedes any earlier "stop" — they have moved on. - fs.rmSync(P.cancel(), { force: true }) console.log(`\n● ${round.id} sent for v${n} — ${round.comments.length} comment(s) → ${path.join(dir, 'feedback.md')}`) return sendJSON(res, 200, { ok: true, roundId: round.id }) }) @@ -1363,24 +1402,6 @@ font:14px/1.6 ui-sans-serif,system-ui,-apple-system,sans-serif;color:#667;backgr touch() return sendJSON(res, 200, { ok: true }) } - if (p === '/api/cancel' && req.method === 'POST') { - const body = JSON.parse(await readBody(req) || '{}') - const n = Number(body.version) || loadState().version - return withStoreLock(() => { - writeJSON(P.cancel(), { - page: FILE || appOrigin(), app: appOrigin(), - name: pageName(), - version: n, - comments: body.comments || [], - reason: body.reason || 'The reviewer cancelled this round.', - at: new Date().toISOString(), - }) - fs.rmSync(P.pending(), { force: true }) - console.log(`\n■ Cancel requested on v${n} — stop, then tell the reviewer what you had already changed`) - touch() - return sendJSON(res, 200, { ok: true }) - }) - } /** * Sign-off. The review is over: write the verdict and close the server, which * removes `url` and ends the waiter — so the same exit that means "tab closed" @@ -1413,7 +1434,6 @@ font:14px/1.6 ui-sans-serif,system-ui,-apple-system,sans-serif;color:#667;backgr outcomes: Object.fromEntries((active.comments || []).map(comment => [comment.id, 'left_open_on_approval'])), }) fs.rmSync(P.pending(), { force: true }) - fs.rmSync(P.cancel(), { force: true }) const left = openComments.length console.log(`\n✓ Approved at v${n}${left ? ` — ${left} comment(s) left unapplied` : ''} — the review is closed`) sendJSON(res, 200, { ok: true }) @@ -1465,11 +1485,8 @@ async function cmdServe () { saveState(state) } // Terminal signals belong to the review that raised them. A new one starts - // clean, or the first waiter it arms fires on last week's verdict. An active - // round is recovery, not a new review: preserve its Stop request across a - // server restart so publication cannot slip past it. + // clean, or the first waiter it arms fires on last week's verdict. fs.rmSync(P.approved(), { force: true }) - if (!loadActiveRound()) fs.rmSync(P.cancel(), { force: true }) fs.rmSync(P.share(), { force: true }) const port = Number(args.port || 7788) const server = http.createServer((req, res) => { @@ -1563,13 +1580,13 @@ switch (args._) { case 'publish': withStoreLock(() => cmdPublish()); break case 'claim': withStoreLock(cmdClaim); break case 'reply': withStoreLock(cmdReply); break - case 'cancelled': withStoreLock(cmdCancelled); break + case 'ack': withStoreLock(cmdAck); break case 'share': withStoreLock(cmdShare); break case 'status': cmdStatus(); break case 'check': cmdCheck(); break case 'watch': cmdWatch(); break case 'serve': cmdServe(); break default: - console.error(`Unknown command "${args._}". Use: serve | claim | publish | reply | cancelled | share | status | check | watch`) + console.error(`Unknown command "${args._}". Use: serve | claim | publish | reply | ack | share | status | check | watch`) process.exit(1) } diff --git a/plugins/vstack/skills/review/assets/workspace.html b/plugins/vstack/skills/review/assets/workspace.html index aa66cb6..e9a158c 100644 --- a/plugins/vstack/skills/review/assets/workspace.html +++ b/plugins/vstack/skills/review/assets/workspace.html @@ -81,8 +81,13 @@ /* The column is what animates: the track collapses to nothing while the panel slides out through it, so the page under review grows into the space instead of the panel simply vanishing from it. */ +/* The clip is load-bearing, not tidiness. A closed track still holds the panel + at its full width (see below), and Safari counts that overhang as page + content it has to fit: it scales the whole workspace to about 85% and leaves + the window's own background down the right and along the bottom. Clipping + here keeps the overhang inside the box that the track belongs to. */ #main{display:grid;grid-template-columns:minmax(0,1fr) var(--pw,var(--panelw));min-height:0; - transition:grid-template-columns .18s ease-out} + overflow:hidden;transition:grid-template-columns .18s ease-out} #canvasArea{grid-column:1;grid-row:1;overflow:hidden} #panel{grid-column:2;grid-row:1;position:relative;z-index:40} @@ -264,6 +269,14 @@ .cogmenu[hidden]{display:none} .cogmenu .row{display:flex;align-items:center;gap:10px;justify-content:space-between} .cogmenu .lbl{font-size:12px;color:var(--ink-2);white-space:nowrap} +.cogmenu .about{align-items:flex-start} +.cogmenu .abouts{display:grid;gap:3px;justify-items:end} +.cogmenu .abouts .one{display:flex;gap:8px;align-items:baseline} +.cogmenu .abouts em{font-style:normal;font-size:10.5px;letter-spacing:.03em;color:var(--ink-3)} +.cogmenu .abouts b{font:600 11px/1.3 var(--mono);color:var(--ink);font-variant-numeric:tabular-nums} +.cogmenu .stale{font-size:11px;line-height:1.4;color:var(--brand);justify-content:flex-start} +.cogmenu .stale[hidden]{display:none} +.cogmenu .about[hidden]{display:none} /* the live link, stated rather than implied */ .linkdot{display:inline-flex;align-items:center;gap:6px;font:600 10.5px/1 var(--mono); @@ -375,12 +388,6 @@ two things. */ #hiNew{display:none} body.phase #hiNew{display:inline-flex} -/* Send becomes Stop while a round is out. Not red-on-red: a stop is a quiet - correction, not a second call to action competing with the one it replaces. */ -#btnSend.stop{background:var(--surface);border-color:var(--brand-line);color:var(--brand)} -#btnSend.stop:hover{background:var(--brand);border-color:var(--brand);color:#fff;filter:none} -#btnSend.stop[disabled]{opacity:1;color:var(--ink-3);border-color:var(--line-2)} -.split #btnSend.stop + .caret{border-left-color:var(--brand-line)} .btn .kbd{font:500 10px/1 var(--mono);opacity:.6} .seg .kbd{font:500 10px/1 var(--mono);opacity:.65} /* Icon-only sizes — wider cells, nothing else. Selected is the shell's selected @@ -598,16 +605,13 @@ /* ── work in flight ── Progress lives on the comments themselves, so it is obvious *which* ones are - being worked on — and cancel sits next to them, where the doubt is. */ + being worked on. */ #pwork{display:flex;align-items:center;gap:8px;padding:8px 10px;border-bottom:1px solid var(--line); background:var(--brand-soft)} #pwork[hidden]{display:none} #pwork .spin{width:12px;height:12px;border-radius:50%;border:2px solid var(--brand-line); border-top-color:var(--brand);animation:spin .7s linear infinite;flex:none} #pwork .ptxt{flex:1;min-width:0;font-size:11.5px;color:var(--ink-2);line-height:1.35} -#pwork .pcancel{font-size:11.5px;font-weight:600;color:var(--brand);white-space:nowrap;flex:none; - border:1px solid var(--brand-line);border-radius:6px;padding:3px 8px;background:var(--surface)} -#pwork .pcancel:hover{background:var(--brand);border-color:var(--brand);color:#fff} @keyframes spin{to{transform:rotate(360deg)}} @keyframes sweep{0%{left:-38%}55%{left:100%}100%{left:100%}} #pfoot{border-top:1px solid var(--line);padding:9px 10px;display:flex;gap:7px;align-items:center} @@ -902,6 +906,17 @@ + + +
@@ -1018,7 +1033,6 @@
@@ -1170,6 +1184,30 @@

} const hideLink = () => { const el = $('#linkDot'); if (el) el.hidden = true }; + /* ── which version is running ── + The page reports what served it, held from load; the server reports what it + is on now. A tab open across an update shows both and offers the reload. */ + let pageVersion = null, serverVersion = null; + function paintVersions () { + const row = $('#cogAbout'); + if (!row) return; + row.hidden = !pageVersion && !serverVersion; + const put = (id, value) => { const el = $(id); if (el) el.textContent = value || '—' }; + put('#cogVersionPage', pageVersion || serverVersion); + put('#cogVersionServer', serverVersion); + const line = $('#cogServerLine'); + if (line) line.hidden = !serverVersion; + const stale = $('#cogStale'); + if (stale) stale.hidden = !(pageVersion && serverVersion && pageVersion !== serverVersion); + } + /** What the server is on right now, which a page learns from its own payload. */ + function setServerVersion (version) { + const next = version || null; + if (serverVersion === next) return; + serverVersion = next; + paintVersions(); + } + /* ── one live-link client, instead of one per page ── Wires the dot to a server: SSE when the page has an event stream, a plain poll for a server that only answers /ping. Either way the shell owns the @@ -1343,6 +1381,8 @@

if (opts.wip) wip(true, typeof opts.wip === 'string' ? opts.wip : undefined); name(opts.name, opts.eyebrow); wireSettings(); + pageVersion = (window.__VSTACK_BUILD__ || {}).version || null; + paintVersions(); applyTheme(); applyLang(); updateNotice(); @@ -1350,7 +1390,7 @@

} const api = { - init, setTheme, setLang, setLink, setWatching, hideLink, name, wip, + init, setTheme, setLang, setLink, setWatching, setServerVersion, hideLink, name, wip, connect, toast, armConfirm, esc, get theme () { return theme }, get lang () { return lang }, @@ -1543,10 +1583,7 @@

working: n => `Sent ${n} comment${n > 1 ? 's' : ''} — {agent} is working…`, hiNew: 'Highlight new', hiNewTitle: 'Outline what this phase adds over the one before it', phaseCur: 'the full design', phaseOf: n => `Phase ${n}`, - stop: 'Stop', stopping: 'Stopping…', stopTitle: 'Ask {agent} to stop this round at its next checkpoint', - stopAsked: 'Asked {agent} to stop — it stops at its next checkpoint', - stopped: 'Round called off — send it again whenever you are ready', editing: '{agent} is editing the page…', ready: v => `v${v} is ready`, changed: 'The page changed — reload to see it', reviewChanges: 'Review changes', later: 'Later', backToCurrent: 'Back to current', viewingOld: v => `Viewing v${v} — read only`, readOnly: v => `read-only · v${v}`, @@ -1558,12 +1595,12 @@

save: 'Save', newlineHint: 'Shift+Enter for a new line', workingOn: n => `{agent} is working on ${n} comment${n > 1 ? 's' : ''}`, queuedOn: n => `${n} comment${n > 1 ? 's' : ''} waiting to be picked up`, + notPickedUp: n => `${n} comment${n > 1 ? 's' : ''} sent — not picked up yet`, workingAndQueued: (w, q) => `{agent} is working on ${w} · ${q} waiting`, queuedTag: 'queued', heldOn: n => `${n} comment${n > 1 ? 's' : ''} queued — sends when this round ends`, sendNow: 'Send the queue now', sendNowDesc: n => `${n} queued comment${n > 1 ? 's' : ''} — don't wait for this round to finish`, - cancel: 'Cancel', cancelled: 'Asked {agent} to pause', cancelFail: 'Could not ask {agent} to stop', addressedGroup: n => `Addressed (${n})`, earlierGroup: n => `Earlier (${n})`, revert: 'Revert', refine: 'Refine', revertTitle: 'Ask {agent} to put this back the way it was', @@ -1646,10 +1683,7 @@

working: n => `已发送 ${n} 条批注 — {agent} 处理中…`, hiNew: '高亮新增', hiNewTitle: '标出本阶段相对上一阶段新增的部分', phaseCur: '完整设计', phaseOf: n => `阶段 ${n}`, - stop: '停止', stopping: '正在停止…', stopTitle: '请求 {agent} 在下一个检查点停止本轮', - stopAsked: '已请求 {agent} 停止 — 将在下一个检查点停下', - stopped: '本轮已取消 — 随时可以重新发送', editing: '{agent} 正在修改页面…', ready: v => `v${v} 已就绪`, changed: '页面已更新 — 重新加载查看', reviewChanges: '查看更新', later: '稍后', backToCurrent: '回到最新版本', viewingOld: v => `正在查看 v${v} — 只读`, readOnly: v => `只读 · v${v}`, @@ -1661,12 +1695,12 @@

save: '保存', newlineHint: 'Shift+Enter 换行', workingOn: n => `{agent} 正在处理 ${n} 条批注`, queuedOn: n => `${n} 条批注等待接收`, + notPickedUp: n => `${n} 条批注已发送,尚未被接收`, workingAndQueued: (w, q) => `{agent} 正在处理 ${w} 条 · ${q} 条等待中`, queuedTag: '等待中', heldOn: n => `${n} 条已排队 — 本轮结束后发送`, sendNow: '立即发送队列', sendNowDesc: n => `${n} 条排队中 — 不等本轮结束`, - cancel: '取消', cancelled: '已请求 {agent} 暂停', cancelFail: '无法通知 {agent} 停止', addressedGroup: n => `已处理(${n})`, earlierGroup: n => `更早(${n})`, revert: '撤回', refine: '继续完善', revertTitle: '让 {agent} 把这处改回原样', @@ -1751,7 +1785,9 @@

historyClearedAt: null, clearingHistory: false, mode: 'annotate', size: SIZES[1], - zoom: 1, + // `fitted` says the zoom is the one that fills the canvas rather than one the + // reviewer chose, so it is free to follow the canvas when the canvas changes. + zoom: 1, fitted: true, ann: [], carried: [], sel: null, // Comments whose element is not on the page as it stands right now. @@ -1771,7 +1807,6 @@

is what survives a reload — then go out as one batch when the round ends. */ held: new Set(), // A stop has been asked for and the round has not ended yet. - stopping: false, linked: null, // null until the connection has actually said one way or other // The shareable Artifact: asked for here, published by Claude, link comes back. share: { url: null, version: null, pending: false }, @@ -1785,7 +1820,7 @@

const isHistory = () => S.viewing !== S.version; /** The frame holds a running app, not a file we published. */ const live = () => S.runtime === 'live'; -/** There is a session on the other end: it can be sent to, replied to, stopped. */ +/** There is a session on the other end: it can be sent to and replied to. */ const served = () => S.runtime === 'local' || S.runtime === 'live'; /* ─────────────────────────── boot ─────────────────────────── */ @@ -1813,6 +1848,8 @@

if (live()) S.route = S.startPath; adoptShare(data); VSShell.setWatching(data.watching); + S.watching = data.watching; + VSShell.setServerVersion(data.version); const phase = S.runtime === 'phase'; loadAnnotations(); @@ -1918,7 +1955,18 @@

VSShell.connect({ url: API + '/events', onLink: up => { S.linked = up }, - on: { reload: onReloadEvent }, + on: { + reload: onReloadEvent, + // The strip needs the same fact as the dot: a queued round with nobody + // listening reads "not picked up yet", and no reload event carries that. + presence: ev => { + let watching; + try { watching = JSON.parse(ev.data).watching } catch { return } + if (S.watching === watching) return; + S.watching = watching; + renderWork(); + }, + }, }); } async function onReloadEvent () { @@ -1927,20 +1975,17 @@

// The link can land on its own, without the page or the review moving. adoptShare(data, true); VSShell.setWatching(data.watching); + S.watching = data.watching; + VSShell.setServerVersion(data.version); if (data.historyClearedAt && data.historyClearedAt !== S.historyClearedAt) { applyHistoryClear(data); toast(T('historyCleared')); return; } - /* A stop that has been taken: the server drops the brief when it records the - request, so a brief that is gone while we are stopping means the round is - over whether or not anyone is still listening — which is the whole reason - Stop used to hang forever. - - ONLY while stopping. A missing brief otherwise means Claude picked it up - and deleted it, which is the *start* of the work, not the end of it — - reading that as "round over" is what took the progress bars off comments - the moment anything else touched the store. */ + /* A missing brief means Claude picked it up and deleted it, which is the + *start* of the work, not the end of it — reading that as "round over" is + what took the progress bars off comments the moment anything else touched + the store. */ const finishedRound = readFlight(data); // collected, active, or complete? const bumped = (data.currentVersion || 1) !== S.version; if (!bumped && data.html === S.html) { @@ -1950,11 +1995,7 @@

for (const comment of data.reviews?.[version]?.annotations || []) latest.set(comment.id, comment); } const merged = mergeThreads([...latest.values()]); - if (finishedRound) { - const stopped = S.stopping; - roundEnded(); - if (stopped) toast(T('stopped')); - } + if (finishedRound) roundEnded(); if (merged.changed) { render(); if (S.sel) openComposer(S.sel, false); @@ -2037,57 +2078,22 @@

for (const id of [...S.queued]) if (!S.working.has(id)) S.queued.delete(id); // Held comments are waiting too — just here rather than in a brief on disk. const n = S.working.size, q = S.queued.size, h = S.held.size; - // Claude answering or addressing the last comment in flight ends the round - // just as surely as publishing does — the button has to come back. - if (!n && S.stopping) { S.stopping = false; S.awaiting = false } $('#pwork').hidden = !n; - if (n) $('#pworkText').textContent = S.stopping ? T('stopping') - : q === n && !h ? T('queuedOn')(q) + if (n) $('#pworkText').textContent = + q === n && !h ? (S.watching === false ? T('notPickedUp')(q) : T('queuedOn')(q)) : q + h ? T('workingAndQueued')(n - q, q + h) : T('workingOn')(n); - $('#btnCancel').disabled = S.stopping; composer.classList.toggle('working', !!S.sel && S.working.has(S.sel)); } -/** Stop, rather than let Claude finish something the reviewer no longer wants. - This raises a request; it cannot reach into a turn already running. Claude - checks for it at each checkpoint in the round and stops there — so the - button says "Stopping…" until the round actually ends, rather than - pretending the work died the instant it was pressed. */ -async function cancelWork () { - if (S.stopping || !S.working.size) return; - const ids = [...S.working]; - if (!served()) { clearWorking(); S.awaiting = false; return } - S.stopping = true; - renderCounts(); renderWork(); - try { - await fetch(API + '/cancel', { - method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ version: S.version, comments: ids, reason: 'The reviewer stopped this round from the workspace.' }), - }); - // Sending the same review again has to be possible after calling it off. - S.sentSig = null; - toast(T('stopAsked')); - } catch { - S.stopping = false; - renderCounts(); renderWork(); - toast(T('cancelFail')); - } -} - /** The round is over — however it ended. Claude publishing, replying or closing the review all land here, and the button goes back to Send. */ function roundEnded () { - const stopped = S.stopping; - S.stopping = false; S.awaiting = false; clearWorking(); renderCounts(); - /* The queue fires as the round ends — one round out at a time is the deal. - Not after a Stop: the reviewer just said "not now", and firing the next - batch on the heels of that would be the opposite of stopping. What is held - goes out with their next Send instead. */ - if (!stopped) flushHeld(); + // The queue fires as the round ends — one round out at a time is the deal. + flushHeld(); } /** Everything held back during the round, out as one send. The set is cleared @@ -2227,7 +2233,6 @@

$('#btnCopyShare').textContent = T('copyLink'); $('#btnDismissShare').textContent = T('later'); if ($('#shareBanner').classList.contains('on')) showShareReady(); - $('#btnCancel').textContent = T('cancel'); renderApprove(); renderShare(); composer.querySelector('.csave').innerHTML = esc(T('save')) + ' '; composer.querySelector('.hint').textContent = T('newlineHint'); @@ -2286,7 +2291,6 @@

$('#btnDismissWork').onclick = hideWork; $('#btnCopyShare').onclick = () => { if (S.share.url) copy(S.share.url); hideShare() }; $('#btnDismissShare').onclick = hideShare; - $('#btnCancel').onclick = cancelWork; // One button, on the panel's edge: it pushes the comments away and pulls them // back, so there is never a second control saying the same thing elsewhere. $('#panelHandle').onclick = () => setPanel(!document.body.classList.contains('panelopen')); @@ -2585,14 +2589,38 @@

} function fitZoom () { const wrap = $('#canvasWrap'); + // The refit is debounced, so it can land after the canvas has gone — a phase + // page, or a bundle that swapped the body out. + if (!wrap) return; setZoom(Math.min(1, (wrap.clientWidth - 52) / S.size.width, (wrap.clientHeight - 52) / (S.size.height + 36))); + S.fitted = true; } $$('#zoombar button').forEach(b => b.onclick = () => { - if (b.dataset.z === 'in') setZoom(S.zoom + 0.1); - else if (b.dataset.z === 'out') setZoom(S.zoom - 0.1); + if (b.dataset.z === 'in') { setZoom(S.zoom + 0.1); S.fitted = false } + else if (b.dataset.z === 'out') { setZoom(S.zoom - 0.1); S.fitted = false } else fitZoom(); }); -addEventListener('resize', () => { if (S.zoom < 1) fitZoom() }); +/* A fitted page follows the room it has, whatever changed that — the window, + the comments coming or going, a notice appearing under the bar. Watching the + canvas rather than the window is what makes putting the comments away grow + the page into the space they left, instead of leaving empty canvas beside it. + A zoom the reviewer set by hand stays where they put it. + Watch #stageArea, measure #canvasWrap inside it. The wrap is the box that + scrolls, and the frame is as tall as the page it holds, so the wrap can carry + a scrollbar. Watching a box that its own scrollbar narrows would feed itself: + a fit that clears the scrollbar widens the box, and the wider box fits back to + a zoom that brings the scrollbar in again. The stage is the wrap's own frame — + it moves with the panel, the window and the row beneath it, and never with the + scrollbar inside it. + It waits for the canvas to stop moving first. Every new zoom re-rasterises + the framed page at a new scale, and doing that on each frame of the panel's + slide, or of a window drag, is work the reviewer paid for and cannot see. */ +let refitTimer = 0; +new ResizeObserver(() => { + if (!S.fitted) return; + clearTimeout(refitTimer); + refitTimer = setTimeout(fitZoom, 90); +}).observe($('#stageArea') || document.body); /* ───────────────────────── two modes ──────────────────────── */ @@ -3146,12 +3174,14 @@

} function closeComposer () { if (!composerOpen()) return; - const a = annById(S.sel); + let a = annById(S.sel); if (a) { const note = composer.querySelector('.cnote').value.trim(); const reply = composer.querySelector('.creply').value.trim(); - if (reply) postReply(a, reply); - if (note !== (a.note || '')) { a.note = note; save() } + // Both of these write, so both work on this version's copy — an edit to a + // comment carried in from an earlier version is saved nowhere otherwise. + if (reply) a = postReply(a, reply); + if (note !== (a.note || '')) { a = adopt(a); a.note = note; save() } // No words and no thread is not feedback — clearing a comment removes it, // which is also how you take one back. if (!hasSubstance(a)) S.ann = S.ann.filter(x => x.id !== a.id); @@ -3166,13 +3196,20 @@

is no separate button for it, because a reply nobody reads is worse than no button at all. */ function postReply (a, text) { - a.replies = (a.replies || []).concat({ by: 'reviewer', text, at: new Date().toISOString() }); - if (a.status === 'question') a.status = 'open'; - if (a.status === 'addressed') { a.status = 'open'; a.reopenedAt = new Date().toISOString() } + /* Onto this version's copy, always. Answering is the one thing a carried + comment invites — it is on the list because Claude asked something — and a + reply written onto the ghost was saved nowhere, since `save()` posts only + `S.ann`. The answer was on screen until the next reload took it away, and + the question came back unanswered with nobody able to tell it had been. */ + const live = adopt(a); + live.replies = (live.replies || []).concat({ by: 'reviewer', text, at: new Date().toISOString() }); + if (live.status === 'question') live.status = 'open'; + else if (live.status === 'addressed') { live.status = 'open'; live.reopenedAt = new Date().toISOString() } // A reply is new review input regardless of which reply surface wrote it. // Reset this here so both the canvas composer and panel thread enable Send. S.sentSig = null; save(); + return live; } /** @@ -3808,11 +3845,9 @@

/* Send stays Send, even with a round out. Noticing something else while Claude works is the normal case, not an interruption to be blocked — the comment - goes out and joins the round. Stopping is a different intent, and it has its - own button on the strip that names what would be stopped. */ + goes out and joins the round. */ function renderSendButton (open) { const btn = $('#btnSend'); - btn.classList.remove('stop'); // Both labels, and the bar's width picks one — the long form while there is // room to name the agent, the bare verb when there is not. const artifact = S.runtime === 'artifact'; @@ -4314,8 +4349,8 @@

/* ─────────────── the shareable copy ─────────────── The workspace cannot publish an Artifact — only Claude can. So this asks, the - same way cancel and approve ask, and the link comes back down the same wire - the page reloads on. */ + same way approve asks, and the link comes back down the same wire the page + reloads on. */ function adoptShare (data, announce) { const had = S.share.url; S.share = { diff --git a/plugins/vstack/skills/review/hosts/claude.md b/plugins/vstack/skills/review/hosts/claude.md index 9cf5f8b..0001eed 100644 --- a/plugins/vstack/skills/review/hosts/claude.md +++ b/plugins/vstack/skills/review/hosts/claude.md @@ -1,7 +1,8 @@ # Host adapter: Claude Code Implements [contracts/host.md](../../../contracts/host.md) for **Claude Code**. -Profile: `plugins/vstack/hosts/claude.json` (`id: claude`). +Profile: `plugins/vstack/host-profiles/claude.json` (`id: claude`). That JSON is +UI data only — the op-to-tool map is this file. Pass on every server command (or export once per shell): @@ -19,9 +20,9 @@ Default when unset is `claude`, so existing installs keep working without this. | Host op | Claude Code tool | How | | --- | --- | --- | | `background(cmd)` | Bash / shell with `run_in_background: true` | `node …/review-server.mjs serve …` must outlive the turn | -| `watch_stream(cmd)` | **Monitor** tool, `persistent: true` | `node …/review-server.mjs watch --all --stream` | +| `watch_stream(cmd)` | **Monitor** tool, `persistent: true` | `node …/review-server.mjs watch --all --stream` — Monitor delivers each line to the session as it arrives | | `stop(handle)` | TaskStop / stop the background task | After approve or when ending the session | -| `run(cmd)` | Bash (foreground) | `publish`, `reply`, `share`, `check`, `status` | +| `run(cmd)` | Bash (foreground) | `publish`, `reply`, `ack`, `share`, `check`, `status` | | `edit` | Edit / Write tools | Change the HTML file or app source | | `share(file)` | **Artifact** tool (favicon 🎨) | Publish the wireframe file; then `share --url ` | | `browser_capture` | Claude-in-Chrome / browser tools | Navigate, screenshot, run `harvest-reference.js` | @@ -41,8 +42,14 @@ node "$SKILL/assets/review-server.mjs" serve --file "$FILE" --port 7788 --host c # watch_stream (Monitor, persistent: true): node "$SKILL/assets/review-server.mjs" watch --all --stream + +# then answer the HANDSHAKE line it prints, with Bash (foreground): +node "$SKILL/assets/review-server.mjs" ack --all --token ``` +The `HANDSHAKE` line arrives in the session as soon as Monitor has it. Answer it +with `ack`, and the watcher is live from then on. + Tell the user **http://localhost:7788/**. --- diff --git a/plugins/vstack/skills/review/hosts/codex.md b/plugins/vstack/skills/review/hosts/codex.md index b0969ed..89834f8 100644 --- a/plugins/vstack/skills/review/hosts/codex.md +++ b/plugins/vstack/skills/review/hosts/codex.md @@ -1,7 +1,8 @@ # Host adapter: Codex Implements [contracts/host.md](../../../contracts/host.md) for **Codex**. -Profile: `plugins/vstack/hosts/codex.json` (`id: codex`). +Profile: `plugins/vstack/host-profiles/codex.json` (`id: codex`). That JSON is +UI data only — the op-to-tool map is this file. Pass the host explicitly when starting a server. Codex shell calls do not necessarily share exported environment variables: @@ -17,7 +18,7 @@ node "$SKILL/assets/review-server.mjs" serve --file "$FILE" --port 7788 --host c | `background(cmd)` | persistent shell execution (`exec_command`) | Start with a short yield and retain the returned session id. The review server must stay alive. | | `watch_stream(cmd)` | a second persistent `exec_command`, then `write_stdin` | Run `watch --all --stream`; poll the session with an empty write, normally for 30 seconds at a time, until it emits an event. Keep polling while reviews remain open. | | `stop(handle)` | `write_stdin` | Send Ctrl-C (`\u0003`) to the retained server or watcher session. | -| `run(cmd)` | foreground `exec_command` | Use for `publish`, `claim`, `reply`, `check`, `cancelled`, `share`, and `status`. | +| `run(cmd)` | foreground `exec_command` | Use for `publish`, `claim`, `reply`, `ack`, `check`, `share`, and `status`. | | `edit` | `apply_patch` | Change the wireframe or application source without overwriting unrelated work. | | `share(file)` | no generic public Artifact publisher | Profile uses `capabilities.share: copy`; offer the HTML file or an offline bundle instead of inventing a URL. | | `browser_capture` | Codex Browser controls, when installed | Navigate, resize, screenshot, and run `harvest-reference.js`. If Browser is unavailable, use screenshots supplied by the user. | @@ -35,6 +36,9 @@ node "$SKILL/assets/review-server.mjs" serve --file "$FILE" --port 7788 --host c # second persistent exec session; retain and poll this session id node "$SKILL/assets/review-server.mjs" watch --all --stream + +# then answer the HANDSHAKE line it prints, in the foreground +node "$SKILL/assets/review-server.mjs" ack --all --token ``` Tell the user **http://localhost:7788/** (or `/__review/` for live `--app`). @@ -47,9 +51,10 @@ normal persistent command session: 1. Start it with `exec_command` and keep the returned session id. 2. Poll it with an empty `write_stdin`, using a bounded wait so the user keeps receiving progress updates. -3. On `REVIEW`, `REPLIED`, `CANCELLED`, `SHARE`, `APPROVED`, or `CLOSED`, follow +3. Answer the `HANDSHAKE` line the stream opens with, using `run`: `ack --all --token `. The watcher goes live once you do; answer within two minutes. +4. On `REVIEW`, `REPLIED`, `SHARE`, `APPROVED`, or `CLOSED`, follow the core skill and review-loop contract. -4. Resume polling after each published round. Do not send the final response +5. Resume polling after each published round. Do not send the final response while the review is still active; keep the Codex turn open until approval, closure, or an explicit request from the user to stop. diff --git a/plugins/vstack/skills/review/hosts/grok.md b/plugins/vstack/skills/review/hosts/grok.md index e872fa4..961f814 100644 --- a/plugins/vstack/skills/review/hosts/grok.md +++ b/plugins/vstack/skills/review/hosts/grok.md @@ -1,7 +1,8 @@ # Host adapter: Grok Build Implements [contracts/host.md](../../../contracts/host.md) for **Grok Build** -(and the Grok coding TUI). Profile: `plugins/vstack/hosts/grok.json` (`id: grok`). +(and the Grok coding TUI). Profile: `plugins/vstack/host-profiles/grok.json` +(`id: grok`). That JSON is UI data only — the op-to-tool map is this file. **Always** set the host so the workspace says “Grok”, not the default: @@ -19,7 +20,7 @@ export VSTACK_HOST=grok | `background(cmd)` | `run_terminal_command` with `background: true` | `serve` must outlive the turn | | `watch_stream(cmd)` | **`monitor`** tool, `persistent: true` | `watch --all --stream` — each stdout line is a chat event | | `stop(handle)` | `kill_command_or_subagent` with the task id | After approve or when ending the review | -| `run(cmd)` | `run_terminal_command` (foreground) | `publish`, `claim`, `reply`, `check`, `cancelled`, `status` | +| `run(cmd)` | `run_terminal_command` (foreground) | `publish`, `claim`, `reply`, `ack`, `check`, `status` | | `edit` | file edit tools (`search_replace`, `write`, …) | HTML wireframe or app source | | `share(file)` | **Not available** as a public Artifact | Profile `capabilities.share: copy` — do not run the share-URL flow; UI hides “Publish a link” | | `browser_capture` | Browser MCP / chrome-devtools when connected | Otherwise use user screenshots per skill §2 | @@ -39,8 +40,14 @@ node "$SKILL/assets/review-server.mjs" serve --file "$FILE" --port 7788 --host g # monitor, persistent: true node "$SKILL/assets/review-server.mjs" watch --all --stream + +# then answer the HANDSHAKE line it prints, with run_terminal_command +node "$SKILL/assets/review-server.mjs" ack --all --token ``` +The `HANDSHAKE` line arrives as soon as `monitor` has it. Answer it with `ack`, +and the watcher is live from then on. + Tell the user **http://localhost:7788/** (or `/__review/` for live `--app`). --- @@ -51,9 +58,9 @@ When `monitor` delivers a line: | Line prefix | Action (same as [review-loop.md](../../../contracts/review-loop.md)) | | --- | --- | +| `HANDSHAKE` | Run the `ack` command it prints, immediately — the watcher goes live once you do | | `REVIEW` | `claim` the round, read `feedback.md`, apply, `publish` / `reply` — never delete protocol files | | `REPLIED` | Continue that comment’s thread | -| `CANCELLED` | Do not publish half-work; run `cancelled --round `; report | | `SHARE` | Host has no artifact share — tell the user to copy/export the HTML, or use `bundle-artifact.mjs` for a file they can send | | `APPROVED` | Confirm; offer next pipeline stage if applicable | | `CLOSED` | Note the review ended | @@ -61,10 +68,10 @@ When `monitor` delivers a line: During a long round, `check` before publish: ```bash -node "$SKILL/assets/review-server.mjs" check --file "$FILE" || echo STOP +node "$SKILL/assets/review-server.mjs" check --file "$FILE" ``` -Exit 2 means stop. +It always exits 0. If it names a round waiting unclaimed, claim that round first. --- diff --git a/plugins/vstack/skills/review/references/workflow.md b/plugins/vstack/skills/review/references/workflow.md index 7698798..0052edd 100644 --- a/plugins/vstack/skills/review/references/workflow.md +++ b/plugins/vstack/skills/review/references/workflow.md @@ -23,7 +23,7 @@ wireframes/ feedback.json the same, structured rounds/r1.json durable membership, revisions and outcomes pending notification — written on send, cleared by claim - cancel sentinel — the reviewer called this round off + handshake a stream watcher waiting to be told its events land approved sentinel — signed off; the review is over share sentinel — they want a shareable Artifact link url the live URL — exists only while serving @@ -42,13 +42,10 @@ commenting on when they sent round *n* — the timeline scrubs to it, and it is what `share` publishes. A round nobody sent a review from has no capture, which the workspace says in the frame rather than showing an error. -`pending`, `cancel`, `approved` and `share` are the four ways the workspace -reaches you, and only one is ever meaningful at a time: sending clears `cancel`, -cancelling clears `pending`, approving clears both, publishing clears `cancel`, -and `serve` clears `approved` and `share` at startup. It clears `cancel` only -when no active round needs to recover, so a restart cannot bypass Stop. Do not delete protocol files manually: `claim` -clears `pending`, `cancelled` clears `cancel`, and `share --url` clears `share`. -The round record remains as the validation and recovery ledger. +`pending`, `approved` and `share` are the three ways the workspace reaches you. +Approving clears `pending`, and `serve` clears `approved` and `share` at startup. +Do not delete protocol files manually: `claim` clears `pending` and `share --url` +clears `share`. The round record remains as the validation and recovery ledger. ## Commands @@ -63,19 +60,19 @@ node "$SKILL/assets/review-server.mjs" publish --file "$FILE" \ node "$SKILL/assets/review-server.mjs" reply --file "$FILE" \ --round r1 --comment c7f2a1 --text "Every overdue row, or only the ones assigned to you?" -# acknowledge Stop without publishing the partial round -node "$SKILL/assets/review-server.mjs" cancelled --file "$FILE" --round r1 - # serve (Host op background) — opens the workspace in the browser, closes itself 90s after the tab does # --host / VSTACK_HOST selects UI labels (claude | codex | grok); see contracts/host.md node "$SKILL/assets/review-server.mjs" serve --file "$FILE" --port 7788 --host "$VSTACK_HOST" node "$SKILL/assets/review-server.mjs" serve --file "$FILE" --idle-timeout 0 --host "$VSTACK_HOST" # stay up until stopped node "$SKILL/assets/review-server.mjs" serve --file "$FILE" --no-open # leave the browser alone +# answer a stream watcher's handshake — until this lands it claims no presence +node "$SKILL/assets/review-server.mjs" ack --all --token 7f3a91 + # hand back a public URL when Host capabilities.share is artifact node "$SKILL/assets/review-server.mjs" share --file "$FILE" --url "https://example.com/…" -# where are we — current version, a waiting review, a cancel, a sign-off, a share request +# where are we — current version, a waiting review, a sign-off, a share request node "$SKILL/assets/review-server.mjs" status --file "$FILE" # shareable single file @@ -133,9 +130,11 @@ between rounds: ```text WATCHING 2 review(s): wireframe, spec-tree +HANDSHAKE this stream is not live until you answer it. Run now: + node …/review-server.mjs ack --all --token 7f3a91 +LINKED handshake answered — the workspace says Linked from here REVIEW wireframe · r17 · 3 comment(s) · …/reviews/v12/feedback.md REPLIED wireframe · v12/c7h0zh0 · "let's align it to the bottom" -CANCELLED wireframe · read …/cancel OPENED story-map-template · now watching 3 review(s) CLOSED spec-tree · the tab went away ``` @@ -145,18 +144,24 @@ opened after the watcher started; `--file` can be repeated, and combines with `--all` for a page living outside the project. While it runs each page shows **Linked**; with no watcher they show **Unlinked**, in amber. +**Linked** also needs the rounds to move: a round left unclaimed for 90 seconds +flips the page back to **Unlinked** and marks the sent comments "not picked up +yet". If that happens while your watcher is running, its events are not reaching +you — check how it was started against the Host adapter, then claim the round. + First thing after a `REVIEW`: run `claim --round ` using the id in the event. -It clears the notification but preserves the durable ledger. Use -`cancelled --round ` after honoring Stop, and `share --url` after publishing a link. +It clears the notification but preserves the durable ledger. Use `share --url` +after publishing a link. A one-shot form (`watch` without `--stream`) still exists: it exits on the first event and prints the command to restart itself. Nothing points at it any more — it is there for scripting, not for the loop. -**Cancel is a request, not a kill.** Nothing can reach into a turn you are -already running — the sentinel is how the reviewer says *stop*, and you answer -for whatever you had already changed. If you are working a long round, check for -`$STORE/cancel` before you publish. +**Answer the handshake.** The stream opens by asking whether anyone receives it, +because nothing in the process can tell which tool started it. The heartbeat +starts when `ack` lands and the page says Linked from then on. Answer within two +minutes (`--handshake-timeout `), or the watcher prints `UNWIRED` and +exits `3`. **Arm exactly one waiter per review.** Re-arming without stopping the previous one leaves loops polling paths that no longer exist. @@ -213,7 +218,7 @@ the desktop one. The reviewer has no resolve button — a validated `publish --round --addressed ` is the only thing that closes a comment out. The command fails without creating a version when any round member is left open, an id or -revision is stale, the round was not claimed, or Stop is outstanding. +revision is stale, or the round was not claimed. They can delete a comment or clear the lot, but they cannot mark one done. Emptying a comment's text deletes it, so an empty comment never reaches you. diff --git a/plugins/vstack/skills/review/tests/host-profiles.mjs b/plugins/vstack/skills/review/tests/host-profiles.mjs index 725406e..e0accdf 100644 --- a/plugins/vstack/skills/review/tests/host-profiles.mjs +++ b/plugins/vstack/skills/review/tests/host-profiles.mjs @@ -33,7 +33,7 @@ assert.match(html, /"name":"Codex"/) runtime (update-check gates on "none" alone). */ for (const id of listHosts()) { const p = loadHost(id) - const where = `hosts/${id}.json` + const where = `host-profiles/${id}.json` assert.deepEqual(Object.keys(p).filter(k => !['id', 'name', 'capabilities', 'install'].includes(k)), [], `${where}: unknown top-level keys`) assert.match(p.id, /^[a-z][a-z0-9-]*$/, `${where}: id pattern`) diff --git a/plugins/vstack/skills/review/tests/review-lifecycle.mjs b/plugins/vstack/skills/review/tests/review-lifecycle.mjs index 57b29b2..0e65cbe 100644 --- a/plugins/vstack/skills/review/tests/review-lifecycle.mjs +++ b/plugins/vstack/skills/review/tests/review-lifecycle.mjs @@ -76,12 +76,75 @@ try { assert.equal(first.response.status, 200) assert.equal(first.body.roundId, 'r1') - let result = cli('publish', '--round', 'r1', '--label', 'Too soon', '--addressed', 'c1,c2') + // "carry on" alone is how queued comments go unread: an unclaimed round is + // named on every check, and the exit code still says continue. + let result = cli('check') + assert.equal(result.status, 0, 'check always carries on') + assert.match(result.stdout, /r1 .* waiting unclaimed/) + assert.match(result.stdout, /claim .*--round r1/) + + // A fresh watcher heartbeat with the round still young reads as linked … + fs.writeFileSync(path.join(store, 'watching'), String(Date.now())) + let project = await request('/api/project') + assert.equal(project.body.watching, true) + assert.equal(project.body.activeReview.stalled, false) + + // … but past the claim window the heartbeat no longer counts: a watcher + // nobody reads and no watcher at all must look the same to the reviewer. + const roundFile = path.join(store, 'rounds', 'r1.json') + const backdated = JSON.parse(fs.readFileSync(roundFile)) + backdated.createdAt = new Date(Date.now() - 120_000).toISOString() + fs.writeFileSync(roundFile, JSON.stringify(backdated)) + project = await request('/api/project') + assert.equal(project.body.watching, false, 'a round unclaimed past the window must drop the linked state') + assert.equal(project.body.activeReview.stalled, true) + + result = cli('publish', '--round', 'r1', '--label', 'Too soon', '--addressed', 'c1,c2') assert.equal(result.status, 2, 'an unclaimed round must not publish') assert.match(result.stderr, /claim r1/i) assert.equal(JSON.parse(fs.readFileSync(path.join(store, 'state.json'))).version, 1) assert.equal(cli('claim', '--round', 'r1').status, 0) + project = await request('/api/project') + assert.equal(project.body.watching, true, 'claiming the round restores the linked state') + assert.match(cli('check').stdout, /^carry on\s*$/, 'a claimed round needs no warning') + + /* A stream watcher asks for the one thing only a live session can do, because + nothing in the process can tell which tool started it. Presence begins when + the handshake is answered; unanswered, the watcher exits saying so, which on + hosts that re-invoke on exit delivers itself to whoever started it. */ + const watcher = (...extra) => { + const child = spawn(process.execPath, [SERVER, 'watch', '--file', page, '--stream', ...extra], { + cwd: temp, stdio: ['ignore', 'pipe', 'pipe'], + }) + let out = '' + child.stdout.on('data', chunk => { out += chunk }) + return { child, read: () => out, ended: new Promise(resolve => child.once('exit', resolve)) } + } + fs.rmSync(path.join(store, 'watching'), { force: true }) + const ignored = watcher('--handshake-timeout', '2') + assert.equal(await ignored.ended, 3, 'an unanswered watcher must exit non-zero') + assert.match(ignored.read(), /HANDSHAKE/) + assert.match(ignored.read(), /UNWIRED/) + assert.equal(fs.existsSync(path.join(store, 'watching')), false, + 'a watcher nobody answered must never claim presence') + + const wired = watcher('--handshake-timeout', '30') + for (let i = 0; i < 50 && !fs.existsSync(path.join(store, 'handshake')); i++) { + await new Promise(resolve => setTimeout(resolve, 100)) + } + const token = JSON.parse(fs.readFileSync(path.join(store, 'handshake'), 'utf8')).token + assert.equal(cli('ack', '--token', 'wrong').status, 2, 'a wrong token must not answer the handshake') + assert.equal(cli('ack', '--token', token).status, 0) + for (let i = 0; i < 50 && !fs.existsSync(path.join(store, 'watching')); i++) { + await new Promise(resolve => setTimeout(resolve, 100)) + } + assert.ok(fs.existsSync(path.join(store, 'watching')), 'an answered watcher starts beating') + assert.match(wired.read(), /LINKED/) + wired.child.kill('SIGTERM') + await wired.ended + assert.equal(cli('ack', '--token', token).status, 0, 'answering twice is not an error') + fs.writeFileSync(path.join(store, 'watching'), String(Date.now())) result = cli('publish', '--round', 'r1', '--label', 'Incomplete', '--addressed', 'c1') assert.equal(result.status, 2, 'an unresolved comment must block publication') assert.match(result.stderr, /c2 is still open/) @@ -128,17 +191,13 @@ try { })]) assert.equal(second.body.roundId, 'r2') assert.equal(cli('claim', '--round', 'r2').status, 0) - await request('/api/cancel', { - method: 'POST', headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ version: 2, comments: ['c2'], reason: 'Stop now' }), - }) + // A restart is recovery, not a new review: the claimed round has to survive it. server.kill('SIGTERM') await new Promise(resolve => server.once('exit', resolve)) await startServer() - result = cli('publish', '--round', 'r2', '--label', 'Must not land', '--addressed', 'c2') - assert.equal(result.status, 2, 'Stop must remain a hard publication gate after restart') - assert.match(result.stderr, /asked to stop/) - assert.equal(cli('cancelled', '--round', 'r2').status, 0) + const recovered = await request('/api/project') + assert.equal(recovered.body.activeReview.id, 'r2', 'an active round must survive a restart') + assert.equal(recovered.body.activeReview.status, 'active') let approval = await request('/api/approve', { method: 'POST', headers: { 'content-type': 'application/json' }, @@ -188,6 +247,7 @@ try { assert.equal(cli('reply', '--comment', 'c404', '--text', 'Nobody home').status, 1, 'a comment in no version at all must fail loudly') + console.log('review lifecycle integration: ok') } finally { server?.kill('SIGTERM') diff --git a/plugins/vstack/skills/user-story-map/assets/story-map-template.html b/plugins/vstack/skills/user-story-map/assets/story-map-template.html index 5d976d3..f1f1488 100644 --- a/plugins/vstack/skills/user-story-map/assets/story-map-template.html +++ b/plugins/vstack/skills/user-story-map/assets/story-map-template.html @@ -209,6 +209,14 @@ .cogmenu[hidden]{display:none} .cogmenu .row{display:flex;align-items:center;gap:10px;justify-content:space-between} .cogmenu .lbl{font-size:12px;color:var(--ink-2);white-space:nowrap} +.cogmenu .about{align-items:flex-start} +.cogmenu .abouts{display:grid;gap:3px;justify-items:end} +.cogmenu .abouts .one{display:flex;gap:8px;align-items:baseline} +.cogmenu .abouts em{font-style:normal;font-size:10.5px;letter-spacing:.03em;color:var(--ink-3)} +.cogmenu .abouts b{font:600 11px/1.3 var(--mono);color:var(--ink);font-variant-numeric:tabular-nums} +.cogmenu .stale{font-size:11px;line-height:1.4;color:var(--brand);justify-content:flex-start} +.cogmenu .stale[hidden]{display:none} +.cogmenu .about[hidden]{display:none} /* the live link, stated rather than implied */ .linkdot{display:inline-flex;align-items:center;gap:6px;font:600 10.5px/1 var(--mono); @@ -499,6 +507,17 @@
+ + +
@@ -721,6 +740,30 @@

} const hideLink = () => { const el = $('#linkDot'); if (el) el.hidden = true }; + /* ── which version is running ── + The page reports what served it, held from load; the server reports what it + is on now. A tab open across an update shows both and offers the reload. */ + let pageVersion = null, serverVersion = null; + function paintVersions () { + const row = $('#cogAbout'); + if (!row) return; + row.hidden = !pageVersion && !serverVersion; + const put = (id, value) => { const el = $(id); if (el) el.textContent = value || '—' }; + put('#cogVersionPage', pageVersion || serverVersion); + put('#cogVersionServer', serverVersion); + const line = $('#cogServerLine'); + if (line) line.hidden = !serverVersion; + const stale = $('#cogStale'); + if (stale) stale.hidden = !(pageVersion && serverVersion && pageVersion !== serverVersion); + } + /** What the server is on right now, which a page learns from its own payload. */ + function setServerVersion (version) { + const next = version || null; + if (serverVersion === next) return; + serverVersion = next; + paintVersions(); + } + /* ── one live-link client, instead of one per page ── Wires the dot to a server: SSE when the page has an event stream, a plain poll for a server that only answers /ping. Either way the shell owns the @@ -894,6 +937,8 @@

if (opts.wip) wip(true, typeof opts.wip === 'string' ? opts.wip : undefined); name(opts.name, opts.eyebrow); wireSettings(); + pageVersion = (window.__VSTACK_BUILD__ || {}).version || null; + paintVersions(); applyTheme(); applyLang(); updateNotice(); @@ -901,7 +946,7 @@

} const api = { - init, setTheme, setLang, setLink, setWatching, hideLink, name, wip, + init, setTheme, setLang, setLink, setWatching, setServerVersion, hideLink, name, wip, connect, toast, armConfirm, esc, get theme () { return theme }, get lang () { return lang }, From 2bcb6cfc79a11a7d15f1ff50135ec345e9f5fb02 Mon Sep 17 00:00:00 2001 From: DeyangChan Date: Wed, 5 Aug 2026 17:15:42 +0800 Subject: [PATCH 2/4] A watcher finds a review whose page lives outside the directory it was started from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A review's store sits beside the page under review, and `watch --all` found stores by walking the directory it was run from. A page written to a temp directory — where an agent puts a file it has just generated — takes its store with it, so the watcher walked straight past a running review. It then heartbeated into nothing while the workspace said Unlinked. `serve` now leaves a pointer under the directory it was run from, `.vstack/local/review/.serving/`, naming the store it is serving. The watcher follows those pointers as well as walking. A pointer whose store has no `url` behind it belongs to a server that was killed, and the reader deletes it. A handshake now carries the token it printed, so only the watcher that asked acts on the answer — a second watcher's handshake is not an answer to the first. A watcher that is answered but covers no review reports UNLINKED rather than LINKED, because nothing is listening to any workspace at that point whatever the handshake proved. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 2 +- README.md | 12 ++ docs/assets/live-link.svg | 65 ++++++++++ docs/assets/review-lifecycle.svg | 82 +++++++++++++ plugins/vstack/contracts/review-loop.md | 17 ++- plugins/vstack/skills/review/SKILL.md | 5 +- .../skills/review/assets/review-server.mjs | 111 ++++++++++++++++-- .../skills/review/references/workflow.md | 13 +- .../skills/review/tests/review-lifecycle.mjs | 75 +++++++++++- 9 files changed, 366 insertions(+), 16 deletions(-) create mode 100644 docs/assets/live-link.svg create mode 100644 docs/assets/review-lifecycle.svg diff --git a/CLAUDE.md b/CLAUDE.md index 64989f7..6166d01 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -95,7 +95,7 @@ The layering rule that everything else follows (`plugins/vstack/contracts/README a self-contained HTML page inside the workspace, or reverse-proxies a running app (`--app`) so the workspace shares an origin with what it annotates (that origin-sharing is why comments can attach to elements, not coordinates). CLI - subcommands (`publish`, `claim`, `reply`, `cancelled`, `share`, `status`, + subcommands (`publish`, `claim`, `reply`, `ack`, `share`, `status`, `check`, `watch`) drive the protocol; sentinels and round records live on disk. - `lib/json-bridge.mjs` — the live link for JSON-document pages (user-story-map, plus the experimental spec and phase-build tools): the page POSTs saves and diff --git a/README.md b/README.md index a268d97..02b565e 100644 --- a/README.md +++ b/README.md @@ -80,6 +80,18 @@ No archaeology through 200 messages. No screenshot named `final-final-v2-actuall - A local web browser - At least one strong opinion about border radius +## Technical Details + +### Live Link + +Each workspace is linked to one agent session. The link holds while that session is active, its heartbeat is less than 15 seconds old, and every submitted review round has been claimed. + +![The workspace page in a browser tab talks over http and SSE to the review server on 127.0.0.1. The server reads and writes a store on disk holding the state, the versions, the comments, the rounds, and the files that carry the link. The agent session watches and writes the same store.](docs/assets/live-link.svg) + +### Review Lifecycle + +![Your comments are submitted as one review round. The agent claims the round and reads its brief, asking for clarification when a comment is unclear. Comments sent while the round is in progress join it. Publishing is blocked until every comment has been applied, answered, or dismissed, and the published version appears in the same workspace.](docs/assets/review-lifecycle.svg) + ## Contribute Visual Stack is open source and under active development. Expect rough edges, breaking changes, and occasional moments of character development. diff --git a/docs/assets/live-link.svg b/docs/assets/live-link.svg new file mode 100644 index 0000000..42550ea --- /dev/null +++ b/docs/assets/live-link.svg @@ -0,0 +1,65 @@ + + + + LIVE LINK + How the workspace, review server, and agent session connect + + + + BROWSER TAB + Workspace page + The comments, the versions, and the page or app being reviewed, all on one origin. + + + + + http + SSE + + + + + REVIEW ENGINE + + + ONE LOCAL PROCESS, ON 127.0.0.1 + Review server + Serves the workspace, proxies your app, and pushes every change to the tab. + + + + + reads and writes + + + Store on disk + .vstack/local/review/<name>/ + + state.json + the current version and the round in flight + + versions/ + every version, frozen as it was published + + reviews/ + the comments and the brief they became + + rounds/ + what was sent, and what closed it + + handshake · watching · pending + the link, and what waits on the agent + + + + + watches and writes + + + + YOUR AGENT + Agent session + A watcher that turns the store into events, and the commands that answer them. + + The workspace and agent session communicate through the shared store. This allows a review + round to continue after the browser tab closes or the agent session restarts. + diff --git a/docs/assets/review-lifecycle.svg b/docs/assets/review-lifecycle.svg new file mode 100644 index 0000000..7d51393 --- /dev/null +++ b/docs/assets/review-lifecycle.svg @@ -0,0 +1,82 @@ + + + + REVIEW LIFECYCLE + A review round, from submission to the next version + + + + You, in the workspace + + Review server + + Agent session + + + + + + + + + EVERY ROUND + + + + + Send + Your comments are submitted as one review round. + + + + Review + The agent claims the round and reads its brief. + + + + + + + IF A COMMENT IS UNCLEAR + + Question + The agent asks for clarification on that comment. + + + + Reply + Your answer is added to the comment. + + + + + + + IF YOU COMMENT MID-ROUND + + Send again + New comments join the current round. + + + + Review again + The agent reads them at the next checkpoint. + + + + Publish + Blocked until every comment has been + applied, answered, or dismissed. + + + + New version + The published version appears in the same workspace. + + + + Approve + The review server closes after approval. + + + diff --git a/plugins/vstack/contracts/review-loop.md b/plugins/vstack/contracts/review-loop.md index 7648827..418f429 100644 --- a/plugins/vstack/contracts/review-loop.md +++ b/plugins/vstack/contracts/review-loop.md @@ -47,13 +47,22 @@ a caller never has to pick between the two itself. | `reviews/v/feedback.md` | Markdown brief for the agent | | `reviews/v/feedback.json` | Same, structured | | `rounds/r.json` | Durable membership, revisions, outcomes, and completion record | -| `handshake` | A stream watcher waiting to be told its events are being read | +| `handshake` | A stream watcher waiting to be told its events are being read. Carries the token it printed; `ack` marks the record answered rather than deleting it, and only the watcher whose token it holds acts on it and clears it | | `pending` | Notification only: review sent, agent must `claim` it | | `approved` | Sentinel: design signed off; engine shutting down | | `share` | Sentinel: reviewer wants a shareable link | | `url` | Present only while `serve` is running | | `watching` | Heartbeat while Host op `watch_stream` is active | +`serve` also records the store it is serving under the directory it was run +from: `/.vstack/local/review/.serving/`, one file per live review, +holding that review's store path. It is written after `url` and removed with it. + +`watch --all` finds a review by walking the directory it was run from **and** by +following those pointers. The pointer is what covers a page that lives outside +that directory, whose store lives outside it too. A pointer whose store has no +`url` is stale, and the reader deletes it. + Every vstack tool keeps its per-machine working files under `.vstack/local//`, resolved by `lib/workdir.mjs`: the enclosing `.vstack` when the artifact already sits in one, otherwise the one beside it. Engines must @@ -102,7 +111,8 @@ One line of stdout per event (from `watch --stream`): | --- | --- | --- | | `WATCHING` | Stream armed | — | | `HANDSHAKE` | The watcher asking whether anyone receives it | Run the `ack` command it prints, immediately | -| `LINKED` | The handshake was answered | — | +| `LINKED` | The handshake was answered and at least one review is covered | — | +| `UNLINKED` | The handshake was answered and no review turned up to cover, so no workspace goes Linked | Start it again via `watch_stream` with `--file` if a review is running elsewhere; a later serve in the same directory is picked up without it | | `UNWIRED` | The handshake went unanswered; the watcher exits `3` | Start it again via `watch_stream` | | `REVIEW` | `pending` written; round id and path to `feedback.md` | `claim` the round, apply brief, publish/reply | | `REPLIED` | Reviewer answered a question | Continue that comment’s thread | @@ -140,7 +150,8 @@ Rules: 5. Retrying an already completed `publish --round …` is idempotent and creates no extra version. 6. One `watch_stream` per session is enough with `--all`. 7. Presence is proven. A stream watcher writes its `watching` heartbeat from the moment its handshake is answered, so **Linked** means a session is receiving the stream. Default window 120 s (`--handshake-timeout `). -8. Presence is also claim-backed. The engine reports the agent present (workspace **Linked**) only while the `watching` heartbeat is fresh **and** no queued round has sat unclaimed past the claim window (90 s). A stalled round drops presence — a watcher whose events nobody reads must look the same to the reviewer as no watcher at all. +8. Presence is per review, and per watcher. A watcher heartbeats only the stores it covers, and goes live only on an answer carrying its own token — a second watcher's handshake is not an answer to the first. It reports `LINKED` once it covers a review, and `UNLINKED` when none has turned up. +9. Presence is also claim-backed. The engine reports the agent present (workspace **Linked**) only while the `watching` heartbeat is fresh **and** no queued round has sat unclaimed past the claim window (90 s). A stalled round drops presence — a watcher whose events nobody reads must look the same to the reviewer as no watcher at all. --- diff --git a/plugins/vstack/skills/review/SKILL.md b/plugins/vstack/skills/review/SKILL.md index 4e3f4e9..f733686 100644 --- a/plugins/vstack/skills/review/SKILL.md +++ b/plugins/vstack/skills/review/SKILL.md @@ -171,7 +171,8 @@ node "$SKILL/assets/review-server.mjs" watch --all --stream # or --file ` if a review is already running for a page outside this directory. A serve started here after it needs nothing | | **`UNWIRED`** | the handshake went unanswered and the watcher exited | start it again with the tool your adapter names for `watch_stream` | | **`REVIEW`** | a review landed; the line names its round and brief | `claim` the round, then apply it — the steps below | | **`REPLIED`** | they answered a question you asked | read the thread and carry on with that comment. Nothing else announces this — a reply writes no sentinel | diff --git a/plugins/vstack/skills/review/assets/review-server.mjs b/plugins/vstack/skills/review/assets/review-server.mjs index 6129e78..0c29e07 100644 --- a/plugins/vstack/skills/review/assets/review-server.mjs +++ b/plugins/vstack/skills/review/assets/review-server.mjs @@ -45,6 +45,10 @@ * url the live URL — present only while the server runs * watching heartbeat — an agent session is waiting on this review * + * A serve also leaves a pointer to that store under the directory it was run + * from — `/.vstack/local/review/.serving/` — so `watch --all`, run + * from the same place, finds a review whose page lives somewhere else entirely. + * * Serving opens the workspace in the machine's default browser as soon as it is * up — `--no-open`, or VSTACK_NO_OPEN=1, for a run that should not. * @@ -660,6 +664,44 @@ const storeFor = f => { } const inStore = (store, name) => path.join(store, name) +/* Where a server records the store it is serving, for the benefit of a watcher + that cannot walk to it. + + A review's store sits beside the page under review, and `watch --all` finds + stores by walking the directory it was run from. A page written outside that + directory — a temp directory is the usual one, since that is where an agent + puts a file it just generated — takes its store with it, and the watcher + walks right past a review that is running. It then heartbeats into nothing + while the workspace says Unlinked, which is the one failure this protocol + must not have. + The directory both processes share is the one the session ran them from, so + the server leaves a pointer there naming its real store. Keyed by the store + path, so a second serve from the same place adds a pointer rather than + overwriting one. */ +const servingDir = from => path.join(workDir(from, TOOL.review), '.serving') +const servingFile = (from, store) => + path.join(servingDir(from), createHash('sha1').update(store).digest('hex').slice(0, 12)) + +/** Stores pointed at from `from`, minus any whose server is gone. */ +function pointedStores (from) { + const found = [] + for (const tool of toolNames(TOOL.review)) { + const dir = path.join(workDir(from, tool), '.serving') + let entries = [] + try { entries = fs.readdirSync(dir) } catch { continue } + for (const name of entries) { + const pointer = path.join(dir, name) + let store = '' + try { store = fs.readFileSync(pointer, 'utf8').trim() } catch { continue } + // The pointer is written after `url` and removed with it, so a pointer + // with no `url` behind it belongs to a server that was killed outright. + if (store && fs.existsSync(path.join(store, 'url'))) found.push(store) + else fs.rmSync(pointer, { force: true }) + } + } + return found +} + /** Every store with a server behind it — `url` exists only while one runs. */ function liveStores (from = process.cwd(), depth = 5) { const found = [] @@ -698,7 +740,8 @@ function liveStores (from = process.cwd(), depth = 5) { } } walk(from, depth) - return found + // A store found both ways is one review, so compare resolved paths. + return [...new Set([...found, ...pointedStores(from)].map(store => path.resolve(store)))] } /* How long a stream watcher waits to be told its events are being read. Long @@ -717,7 +760,12 @@ const stopBeating = () => { heartbeat?.stop(); heartbeat = null } function cmdAck () { const waiting = readJSON(P.handshake()) if (!waiting) { + // Naming the directory it looked in, because the usual reason to find + // nothing is being somewhere else: `--all` resolves the handshake from the + // working directory, and an ack run from a different one reads a file that + // was never there rather than the one the watcher wrote. console.log('Nothing to answer — no watcher is waiting on a handshake for this review.') + console.log(`Looked in ${STORE}`) return } const token = args.token && args.token !== true ? String(args.token) : null @@ -725,8 +773,15 @@ function cmdAck () { console.error('That is not the token the waiting watcher printed — read its HANDSHAKE line again.') process.exit(2) } - fs.rmSync(P.handshake(), { force: true }) - console.log('Answered — the watcher is wired to this session, and the workspace says Linked.') + /* Answered, not gone. A second watcher overwrites the first one's handshake, + so a watcher that read "the file I wrote is missing" as "someone answered + me" would go live on an answer addressed to another process — and the one + nobody answered would heartbeat forever. The token stays on the record, and + only the watcher that owns it clears it. */ + writeJSON(P.handshake(), { ...waiting, answeredAt: new Date().toISOString() }) + // Whether the workspace goes Linked is the watcher's to report: it knows + // which reviews it covers, and this command does not. + console.log('Answered — the watcher is wired to this session. Read its next line.') } /** @@ -759,22 +814,56 @@ async function cmdStream (stores, label, all, subjectFlags) { say(` node "${process.argv[1]}" ack ${subjectFlags} --token ${token}`) const askedAt = Date.now() + /* Answered means answered *here*. A second watcher on the same review + overwrites this record, so an answer carrying someone else's token is not + this watcher's to act on — and only the watcher that owns the record clears + it. Without that, the watcher nobody answered goes live too, and heartbeats + long after the answered one has stopped. */ + const mine = () => readJSON(P.handshake())?.token === token + const answered = () => { const record = readJSON(P.handshake()); return record?.token === token && !!record.answeredAt } + + /* The handshake proves a session is reading this stream. It says nothing + about whether the stream reaches the review the reviewer is looking at, and + a watcher covering no store heartbeats into nothing — so LINKED waits for a + store to be under it, and the gap is named rather than papered over. */ + let saidLinked = false, saidUnlinked = false, answeredAt = 0 + const sayLink = () => { + if (saidLinked || !stores.length) return + saidLinked = true + say('LINKED handshake answered — the workspace says Linked from here') + } + /* Arming the watcher before the serve is a supported order, and a review that + turns up a moment later needs no explaining — so the empty case is only + worth reporting once it has had time to stop being empty. */ + const EMPTY_LINK_MS = 15_000 + const sayNoLink = () => { + if (saidLinked || saidUnlinked || Date.now() - answeredAt < EMPTY_LINK_MS) return + saidUnlinked = true + say(`UNLINKED handshake answered, but no live review is visible from ${process.cwd()},`) + say(' so no workspace says Linked. A serve started here is picked up on its') + say(' own; one already running for a page outside this directory is not —') + say(' for that, start this again with the tool your adapter names for') + say(` watch_stream: node "${process.argv[1]}" watch --file --stream`) + } + while (true) { if (!heartbeat) { - if (!fs.existsSync(P.handshake())) { + if (answered()) { + fs.rmSync(P.handshake(), { force: true }) + answeredAt = Date.now() heartbeat = startHeartbeat(() => stores.map(store => inStore(store, 'watching'))) - say('LINKED handshake answered — the workspace says Linked from here') + sayLink() } else if (Date.now() - askedAt > HANDSHAKE_MS) { /* Exiting is the point: on a host where a finished background command re-invokes the session, this delivers itself to whoever started the watcher. */ - fs.rmSync(P.handshake(), { force: true }) + if (mine()) fs.rmSync(P.handshake(), { force: true }) say('UNWIRED the handshake went unanswered, so these events reach no one.') say(' Start this again with the Host op watch_stream, using the tool') say(' your Host adapter names for it.') return process.exit(3) } - } + } else sayNoLink() for (const store of [...stores]) { const at = n => inStore(store, n) @@ -822,6 +911,8 @@ async function cmdStream (stores, label, all, subjectFlags) { seen.set(store, { sent: null, flags: new Set(), replies: repliesIn(store) }) say(`OPENED ${label(store)} · now watching ${stores.length} review(s)`) } + // A review that arrives after the handshake is what makes the link real. + if (heartbeat) sayLink() } // With --all, an empty set means "no tab open right now" — keep the @@ -1523,6 +1614,7 @@ async function cmdServe () { stops waiting instead of hanging until its timeout. */ const close = why => { try { fs.rmSync(P.url(), { force: true }) } catch {} + try { fs.rmSync(servingFile(process.cwd(), STORE), { force: true }) } catch {} console.log(`closed (${why})`) process.exit(0) } @@ -1565,6 +1657,11 @@ async function cmdServe () { server.listen(port, '127.0.0.1', () => { fs.mkdirSync(STORE, { recursive: true }) fs.writeFileSync(P.url(), url + '\n') + // So `watch --all`, run from here, finds this review wherever the page lives. + try { + fs.mkdirSync(servingDir(process.cwd()), { recursive: true }) + writeAtomic(servingFile(process.cwd(), STORE), STORE + '\n') + } catch {} console.log(`${LIVE ? 'live review' : 'wireframe'} · ${pageName()} · v${loadState().version}`) console.log(` workspace ${url}`) console.log(LIVE ? ` app ${APP.origin} (proxied)` : ` page ${FILE}`) diff --git a/plugins/vstack/skills/review/references/workflow.md b/plugins/vstack/skills/review/references/workflow.md index 0052edd..0611fc4 100644 --- a/plugins/vstack/skills/review/references/workflow.md +++ b/plugins/vstack/skills/review/references/workflow.md @@ -140,9 +140,16 @@ CLOSED spec-tree · the tab went away ``` `--all` covers every review with a live server under the project, including ones -opened after the watcher started; `--file` can be repeated, and combines with -`--all` for a page living outside the project. While it runs each page shows -**Linked**; with no watcher they show **Unlinked**, in amber. +opened after the watcher started, and any review whose server you started from +this directory — a page written to a temp directory keeps its store beside +itself, and the server leaves a pointer here so the watcher still finds it. +`--file` can be repeated, and combines with `--all` for a server started +somewhere else entirely. While it runs each page shows **Linked**; with no +watcher they show **Unlinked**, in amber. + +A watcher that covers no review at all says `UNLINKED` in place of `LINKED` once +its handshake is answered. Nothing is listening to any workspace at that point, +whatever the handshake proved: start it again with `--file `. **Linked** also needs the rounds to move: a round left unclaimed for 90 seconds flips the page back to **Unlinked** and marks the sent comments "not picked up diff --git a/plugins/vstack/skills/review/tests/review-lifecycle.mjs b/plugins/vstack/skills/review/tests/review-lifecycle.mjs index 0e65cbe..619f2b1 100644 --- a/plugins/vstack/skills/review/tests/review-lifecycle.mjs +++ b/plugins/vstack/skills/review/tests/review-lifecycle.mjs @@ -65,7 +65,7 @@ async function sendRound (version, comments) { }) } -let server +let server, awayServer try { fs.writeFileSync(page, 'Round test

Initial

') assert.equal(cli('publish', '--label', 'Initial').status, 0) @@ -144,6 +144,78 @@ try { wired.child.kill('SIGTERM') await wired.ended assert.equal(cli('ack', '--token', token).status, 0, 'answering twice is not an error') + + /* Starting a second watcher overwrites the first one's handshake, so an + answer names which one it is for. A watcher that read a missing handshake + as its own answer would go live on someone else's — and keep beating after + the answered one stopped, which is presence claiming exactly what it cannot + see. */ + fs.rmSync(path.join(store, 'watching'), { force: true }) + const ignoredWatcher = watcher('--handshake-timeout', '4') + for (let i = 0; i < 50 && !fs.existsSync(path.join(store, 'handshake')); i++) { + await new Promise(resolve => setTimeout(resolve, 100)) + } + const firstToken = JSON.parse(fs.readFileSync(path.join(store, 'handshake'), 'utf8')).token + const answeredWatcher = watcher('--handshake-timeout', '30') + for (let i = 0; i < 50 && JSON.parse(fs.readFileSync(path.join(store, 'handshake'), 'utf8')).token === firstToken; i++) { + await new Promise(resolve => setTimeout(resolve, 100)) + } + const secondToken = JSON.parse(fs.readFileSync(path.join(store, 'handshake'), 'utf8')).token + assert.notEqual(secondToken, firstToken, 'the second watcher asks in its own name') + assert.equal(cli('ack', '--token', secondToken).status, 0) + assert.equal(await ignoredWatcher.ended, 3, 'a watcher answered in another name must still time out') + assert.match(ignoredWatcher.read(), /UNWIRED/) + assert.doesNotMatch(ignoredWatcher.read(), /LINKED/, 'only the watcher that was answered may claim presence') + assert.match(answeredWatcher.read(), /LINKED/) + assert.ok(fs.existsSync(path.join(store, 'watching')), + 'the answered watcher keeps beating through the other one exiting') + answeredWatcher.child.kill('SIGTERM') + await answeredWatcher.ended + + /* A page an agent generates lands in a temp directory, and its store lands + beside it — outside the directory the session runs `watch --all` from. The + walk cannot reach it, so the serve leaves a pointer in the directory both + processes do share, and the watcher heartbeats the review it names. Without + that, the handshake is answered, the stream says Linked, and the workspace + sits on Unlinked with nobody able to see why. */ + const away = fs.mkdtempSync(path.join(os.tmpdir(), 'vstack-away-test-')) + const awayPage = path.join(away, 'elsewhere.html') + const awayStore = path.join(away, '.vstack', 'local', 'review', 'elsewhere') + fs.writeFileSync(awayPage, 'Elsewhere

Away

') + awayServer = spawn(process.execPath, [SERVER, 'serve', '--file', awayPage, + '--port', String(port + 1), '--idle-timeout', '0', '--no-open'], { cwd: temp, stdio: 'ignore' }) + for (let i = 0; i < 60 && !fs.existsSync(path.join(awayStore, 'url')); i++) { + await new Promise(resolve => setTimeout(resolve, 100)) + } + assert.ok(fs.existsSync(path.join(awayStore, 'url')), 'the second review must come up') + + const everywhere = spawn(process.execPath, [SERVER, 'watch', '--all', '--stream', '--handshake-timeout', '30'], + { cwd: temp, stdio: ['ignore', 'pipe', 'pipe'] }) + let heard = '' + everywhere.stdout.on('data', chunk => { heard += chunk }) + const allHandshake = path.join(temp, '.vstack', 'local', 'review', 'handshake') + for (let i = 0; i < 50 && !fs.existsSync(allHandshake); i++) { + await new Promise(resolve => setTimeout(resolve, 100)) + } + assert.equal(spawnSync(process.execPath, [SERVER, 'ack', '--all', + '--token', JSON.parse(fs.readFileSync(allHandshake, 'utf8')).token], { cwd: temp, encoding: 'utf8' }).status, 0) + for (let i = 0; i < 50 && !fs.existsSync(path.join(awayStore, 'watching')); i++) { + await new Promise(resolve => setTimeout(resolve, 100)) + } + assert.ok(fs.existsSync(path.join(awayStore, 'watching')), + 'a review outside the watcher\'s directory must still be heartbeaten') + assert.match(heard, /LINKED/) + everywhere.kill('SIGTERM') + await new Promise(resolve => everywhere.once('exit', resolve)) + + const pointer = path.join(temp, '.vstack', 'local', 'review', '.serving') + assert.equal(fs.readdirSync(pointer).length, 2, 'each live serve points at its own store') + awayServer.kill('SIGTERM') + await new Promise(resolve => awayServer.once('exit', resolve)) + awayServer = null + assert.equal(fs.readdirSync(pointer).length, 1, 'a serve that ends takes its pointer with it') + fs.rmSync(away, { recursive: true, force: true }) + fs.writeFileSync(path.join(store, 'watching'), String(Date.now())) result = cli('publish', '--round', 'r1', '--label', 'Incomplete', '--addressed', 'c1') assert.equal(result.status, 2, 'an unresolved comment must block publication') @@ -251,5 +323,6 @@ try { console.log('review lifecycle integration: ok') } finally { server?.kill('SIGTERM') + awayServer?.kill('SIGTERM') fs.rmSync(temp, { recursive: true, force: true }) } From 9d8b08a9fc5d28b3412b3f331f5b341159091b59 Mon Sep 17 00:00:00 2001 From: DeyangChan Date: Wed, 5 Aug 2026 17:57:49 +0800 Subject: [PATCH 3/4] Drag a thing to where it should go, and strike out what should be removed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two annotation tools beside Comment, on a toolbar down the left of the canvas. The bar still decides whether the pointer clicks through the page or annotates it; the toolbar decides what annotating leaves behind. It takes its own strip rather than floating over the canvas, because a toolbar in the top-left corner covers the part of a design people look at first, and it stays in both modes so switching to View and back does not move the page. Each button is an icon, so hovering one says what it is, what key picks it, and what gesture it takes: a picture of a pen does not say that clicking leaves a point and dragging leaves a box. The same three parts go to a screen reader as one sentence. Both new tools are optional-note: the mark is the instruction, and anything typed adds to it rather than supplying it. Both outline whatever the pointer is over, so the element the gesture will take is settled before the press rather than after it. Comment does not — it works on the place you press, and a box following the pointer everywhere is noise. Move draws an arrow from a thing to where it should go. A pixel delta alone stops meaning anything the first time the page reflows, so a move also captures the element it was dropped on and which side of it — inside, before, or after. The brief leads with that and keeps the delta for the case where nothing was under the drop and direction is all the reviewer gave. Delete strikes out what should go. Dragging across text takes exactly those words, through the page's own caret positions, so the agent is told the phrase to remove rather than the paragraph it sat in; the words are found again by their text when the page is rebuilt, so the strike stays on them. Clicking takes a whole element, and strikes every line written inside it — a box drawn round a paragraph leaves the words standing, which is not what was asked for. Also carries a pending one-line fix in shell.css out to every page: the [hidden] rule for the cog's version row. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 1 + plugins/vstack/contracts/review-loop.md | 10 +- .../phase-build/assets/build-board.html | 4 + .../experimental/spec/assets/spec-tree.html | 4 + .../experimental/start/assets/chooser.html | 4 + plugins/vstack/lib/shell/shell.css | 4 + plugins/vstack/skills/review/SKILL.md | 6 +- .../skills/review/assets/workspace.html | 639 +++++++++++++++++- .../skills/review/references/workflow.md | 21 +- .../assets/story-map-template.html | 4 + 10 files changed, 666 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 02b565e..d972f94 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,7 @@ $vstack:review Wireframe a desktop personal task manager with minimal aesthetics - Work in a familiar, Figma-like interface. - Click any element and leave feedback exactly where the problem is hiding. +- Drag a thing to where it belongs, or strike out what should go — no note required. - Stay in the workspace as your agent publishes each update. - Preview desktop, tablet, and mobile layouts before production does it for you. - Compare revisions and identify the exact moment things went wrong. diff --git a/plugins/vstack/contracts/review-loop.md b/plugins/vstack/contracts/review-loop.md index 418f429..d92c4a3 100644 --- a/plugins/vstack/contracts/review-loop.md +++ b/plugins/vstack/contracts/review-loop.md @@ -162,14 +162,22 @@ Rules: | Field | Meaning | | --- | --- | | `id` | Pass to `--addressed` | -| `note` | Requirement text | +| `kind` | `comment` · `area` · `general` · `move` · `strike` | +| `note` | Requirement text. Empty is valid on `move` and `strike` | | `anchor` | Element identity (tag, id, classes, text, region, selector) | +| `move` | `move` only — `{ target: { …anchor identity, where }, delta }`, `where` is `inside` · `before` · `after` | +| `strike` | `strike` only — `{ scope: 'text' \| 'element', text }` | | `screenSize` | Layout the comment was made at | | `route` | Live only — app path | | `status` | `open` · `question` · `addressed` | | `replies` | `{ by, text, at }[]` | | `reopened` / `wantsRevert` | Returned from Refine / Revert | +A comment carries its requirement in `note`. A `move` and a `strike` carry it in +their own fields instead, so a reader must not treat an empty `note` as an +incomplete comment. `move.target` outranks `move.delta`: the element and side +survive a reflow and the pixel distance does not. + --- ## Share diff --git a/plugins/vstack/experimental/phase-build/assets/build-board.html b/plugins/vstack/experimental/phase-build/assets/build-board.html index 8cf4a27..f44b6d6 100644 --- a/plugins/vstack/experimental/phase-build/assets/build-board.html +++ b/plugins/vstack/experimental/phase-build/assets/build-board.html @@ -189,6 +189,10 @@ .cogmenu .about{align-items:flex-start} .cogmenu .abouts{display:grid;gap:3px;justify-items:end} .cogmenu .abouts .one{display:flex;gap:8px;align-items:baseline} +/* An author `display` beats the UA rule for [hidden], so every row that is + hidden from script needs its own. A page that never hears a server version + showed "server —" for good without this. */ +.cogmenu .abouts .one[hidden]{display:none} .cogmenu .abouts em{font-style:normal;font-size:10.5px;letter-spacing:.03em;color:var(--ink-3)} .cogmenu .abouts b{font:600 11px/1.3 var(--mono);color:var(--ink);font-variant-numeric:tabular-nums} .cogmenu .stale{font-size:11px;line-height:1.4;color:var(--brand);justify-content:flex-start} diff --git a/plugins/vstack/experimental/spec/assets/spec-tree.html b/plugins/vstack/experimental/spec/assets/spec-tree.html index 96973c2..d2d0092 100644 --- a/plugins/vstack/experimental/spec/assets/spec-tree.html +++ b/plugins/vstack/experimental/spec/assets/spec-tree.html @@ -193,6 +193,10 @@ .cogmenu .about{align-items:flex-start} .cogmenu .abouts{display:grid;gap:3px;justify-items:end} .cogmenu .abouts .one{display:flex;gap:8px;align-items:baseline} +/* An author `display` beats the UA rule for [hidden], so every row that is + hidden from script needs its own. A page that never hears a server version + showed "server —" for good without this. */ +.cogmenu .abouts .one[hidden]{display:none} .cogmenu .abouts em{font-style:normal;font-size:10.5px;letter-spacing:.03em;color:var(--ink-3)} .cogmenu .abouts b{font:600 11px/1.3 var(--mono);color:var(--ink);font-variant-numeric:tabular-nums} .cogmenu .stale{font-size:11px;line-height:1.4;color:var(--brand);justify-content:flex-start} diff --git a/plugins/vstack/experimental/start/assets/chooser.html b/plugins/vstack/experimental/start/assets/chooser.html index 2fcac24..c64c6da 100644 --- a/plugins/vstack/experimental/start/assets/chooser.html +++ b/plugins/vstack/experimental/start/assets/chooser.html @@ -182,6 +182,10 @@ .cogmenu .about{align-items:flex-start} .cogmenu .abouts{display:grid;gap:3px;justify-items:end} .cogmenu .abouts .one{display:flex;gap:8px;align-items:baseline} +/* An author `display` beats the UA rule for [hidden], so every row that is + hidden from script needs its own. A page that never hears a server version + showed "server —" for good without this. */ +.cogmenu .abouts .one[hidden]{display:none} .cogmenu .abouts em{font-style:normal;font-size:10.5px;letter-spacing:.03em;color:var(--ink-3)} .cogmenu .abouts b{font:600 11px/1.3 var(--mono);color:var(--ink);font-variant-numeric:tabular-nums} .cogmenu .stale{font-size:11px;line-height:1.4;color:var(--brand);justify-content:flex-start} diff --git a/plugins/vstack/lib/shell/shell.css b/plugins/vstack/lib/shell/shell.css index d4493c9..9aa50f4 100644 --- a/plugins/vstack/lib/shell/shell.css +++ b/plugins/vstack/lib/shell/shell.css @@ -109,6 +109,10 @@ .cogmenu .about{align-items:flex-start} .cogmenu .abouts{display:grid;gap:3px;justify-items:end} .cogmenu .abouts .one{display:flex;gap:8px;align-items:baseline} +/* An author `display` beats the UA rule for [hidden], so every row that is + hidden from script needs its own. A page that never hears a server version + showed "server —" for good without this. */ +.cogmenu .abouts .one[hidden]{display:none} .cogmenu .abouts em{font-style:normal;font-size:10.5px;letter-spacing:.03em;color:var(--ink-3)} .cogmenu .abouts b{font:600 11px/1.3 var(--mono);color:var(--ink);font-variant-numeric:tabular-nums} .cogmenu .stale{font-size:11px;line-height:1.4;color:var(--brand);justify-content:flex-start} diff --git a/plugins/vstack/skills/review/SKILL.md b/plugins/vstack/skills/review/SKILL.md index f733686..0a4bebd 100644 --- a/plugins/vstack/skills/review/SKILL.md +++ b/plugins/vstack/skills/review/SKILL.md @@ -122,10 +122,14 @@ The page opens in **its own browser window** on the canvas — own viewport, own | | | |---|---| -| **View / Annotate** | two modes, **space** toggles. **View hides every annotation** so the page is judged as it really is; Annotate brings them back | +| **View / Annotate** | two modes, **esc** toggles. **View hides every annotation** so the page is judged as it really is; Annotate brings them back | | **Click** | a comment pin at that spot | | **Drag** | an area comment over that region — it carries the element that contains the box *and* everything named inside it | | Either way | the note opens **on the canvas** where the mark is. A comment with nothing typed in it is discarded on dismiss | +| **Toolbar** (left of the canvas) | what Annotate draws with: Comment · Move · Delete, keys **c** · **m** · **d**. Picking one in View mode returns to Annotate with it | +| **Move** and **Delete** | both are optional-note: the mark is the instruction, and anything typed adds to it. Both outline whatever the pointer is over, so it is clear which element the gesture will take | +| **Move** | drag a thing to where it should go. Arrives as `kind: move` — the element to move, the element it was dropped on, and which side of it | +| **Delete** | drag across text to strike exactly those words, or click an element to strike everything written inside it. Arrives as `kind: strike` | | **Target** | the note says which element the comment attached to | | **Attached to an element** | a comment belongs to the thing it was made on, not to a coordinate. The mark rides it when the layout moves, and **goes off the page with it** — a comment made inside a modal, tab or step is not drawn while that thing is closed. It stays in the list tagged *not on screen*, and it still reaches you | | Captions | stay hidden — a mark shows its note when it's open, or on hover in Annotate | diff --git a/plugins/vstack/skills/review/assets/workspace.html b/plugins/vstack/skills/review/assets/workspace.html index e9a158c..3b23108 100644 --- a/plugins/vstack/skills/review/assets/workspace.html +++ b/plugins/vstack/skills/review/assets/workspace.html @@ -272,6 +272,10 @@ .cogmenu .about{align-items:flex-start} .cogmenu .abouts{display:grid;gap:3px;justify-items:end} .cogmenu .abouts .one{display:flex;gap:8px;align-items:baseline} +/* An author `display` beats the UA rule for [hidden], so every row that is + hidden from script needs its own. A page that never hears a server version + showed "server —" for good without this. */ +.cogmenu .abouts .one[hidden]{display:none} .cogmenu .abouts em{font-style:normal;font-size:10.5px;letter-spacing:.03em;color:var(--ink-3)} .cogmenu .abouts b{font:600 11px/1.3 var(--mono);color:var(--ink);font-variant-numeric:tabular-nums} .cogmenu .stale{font-size:11px;line-height:1.4;color:var(--brand);justify-content:flex-start} @@ -457,7 +461,8 @@ #stageArea{position:relative;overflow:hidden;background:var(--surface-2); background-image:radial-gradient(circle at 1px 1px,var(--line-2) 1px,transparent 0); background-size:22px 22px} -#canvasWrap{position:absolute;inset:0;overflow:auto} +/* The left inset is the toolbar's strip — see #toolbar below. */ +#canvasWrap{position:absolute;inset:0 0 0 54px;overflow:auto} #canvas{padding:26px;display:flex;justify-content:center;min-height:100%;align-items:flex-start} /* transform-origin is top-left so the negative margins in setZoom() shrink the @@ -524,6 +529,51 @@ #drag{position:absolute;border:2px dashed var(--brand);border-radius:3px;display:none; background:color-mix(in srgb,var(--brand) 8%,transparent);pointer-events:none} +/* ── move: an arrow from a thing to where it should go ── + The mark itself has no size. It sits at the start of the drag and hangs its + parts off that point, so the number and the note stay upright while the shaft + between the two ends turns. */ +.mark.move,.mark.strike{width:0;height:0} +.mark.move .shaft{position:absolute;left:0;top:-1px;height:2px;background:var(--mk); + transform-origin:0 50%;pointer-events:auto;cursor:pointer} +/* Two pixels is nothing to aim at, so the line takes clicks from a band around + itself. */ +.mark.move .shaft::before{content:'';position:absolute;inset:-7px 0} +.mark.move .shaft::after{content:'';position:absolute;right:-1px;top:50%;width:0;height:0; + border:5px solid transparent;border-left:9px solid var(--mk);transform:translateY(-50%)} +.mark.move .tail{position:absolute;left:-3.5px;top:-3.5px;width:7px;height:7px; + border-radius:50%;background:var(--mk)} +/* Where the thing should end up, at the size it is now — the arrow says the + direction and this says what will be sitting there. */ +.mark.move .drop{position:absolute;border:1.5px dashed var(--mk);border-radius:3px;opacity:.75; + background:color-mix(in srgb,var(--mk) 6%,transparent)} + +/* ── delete: struck through and marked for removal ── + One bar per line of struck text. Taking a whole element strikes every line + written in it and draws the element's edge round the lot. */ +.mark.strike .bar{position:absolute;border-radius:2px;pointer-events:auto;cursor:pointer; + background:color-mix(in srgb,var(--mk) 13%,transparent)} +.mark.strike .bar::after{content:'';position:absolute;left:0;right:0;top:50%;height:2px; + margin-top:-1px;background:var(--mk)} +.mark.strike .box{position:absolute;border:1.5px solid var(--mk);border-radius:3px; + background:color-mix(in srgb,var(--mk) 6%,transparent);pointer-events:auto;cursor:pointer} +.mark.strike .box[hidden]{display:none} + +.mark.move .lbl,.mark.strike .lbl{position:absolute;left:9px;top:7px;width:max-content; + max-width:280px;background:var(--mk);color:#fff;padding:3px 7px;border-radius:4px; + font:500 11.5px/1.35 var(--font);white-space:pre-wrap;box-shadow:0 1px 4px rgba(0,0,0,.2)} +/* What Move would pick up if the pointer went down here. Move is the one tool + whose gesture starts on a specific thing rather than at a place, so it says + which thing before the drag rather than after it. */ +#hoverBox{position:absolute;display:none;pointer-events:none;border:1.5px solid var(--brand); + border-radius:3px;background:color-mix(in srgb,var(--brand) 7%,transparent)} +/* The preview drawn while the arrow is being dragged, before there is a mark. */ +#dragArrow{position:absolute;display:none;pointer-events:none} +#dragArrow .shaft{position:absolute;left:0;top:-1px;height:2px;background:var(--brand); + transform-origin:0 50%} +#dragArrow .shaft::after{content:'';position:absolute;right:-1px;top:50%;width:0;height:0; + border:5px solid transparent;border-left:9px solid var(--brand);transform:translateY(-50%)} + /* ── on-canvas composer ── */ #composer{position:absolute;z-index:30;width:280px;background:var(--surface);border:1px solid var(--line-2); border-radius:10px;box-shadow:var(--shadow-pop);display:none;overflow:hidden} @@ -797,6 +847,40 @@ .banner .btn{height:25px;font-size:11.5px} #workBanner .txt,#shareBanner .txt{min-width:0;overflow:hidden;text-overflow:ellipsis} +/* The tools, down the left edge of the canvas. Same shell as the zoom controls + in the corner below it — both are things you reach for while working on the + page, rather than decisions about the review. + It takes its own strip rather than floating over the canvas: a toolbar that + covers the top-left corner of the page covers the part of a design people + look at first. The strip is there in both modes, so switching to View and + back does not move the page. */ +#toolbar{position:absolute;left:12px;top:12px;display:grid;gap:2px; + background:var(--surface);border:1px solid var(--line-2);border-radius:8px;padding:3px; + box-shadow:var(--shadow-pop);z-index:20} +#toolbar[hidden]{display:none} +#toolbar button{position:relative;width:30px;height:30px;border-radius:5px;color:var(--ink-2); + display:grid;place-items:center} +#toolbar button:hover{background:var(--surface-2);color:var(--ink)} +#toolbar button[aria-pressed=true]{background:var(--ink);color:var(--surface)} +/* In View the pointer is driving the page, so no tool is the one in use. + Picking one here is how you come back to annotating with it. */ +body.viewing #toolbar button[aria-pressed=true]{background:var(--surface-2);color:var(--ink)} +#toolbar svg{width:17px;height:17px;fill:none;stroke:currentColor;stroke-width:1.6; + stroke-linecap:round;stroke-linejoin:round} +/* Icons alone need their names said. The bubble sits off the right edge, clear + of the page underneath, and answers the keyboard as well as the pointer. */ +#toolbar .tip{position:absolute;left:calc(100% + 8px);top:50%;transform:translateY(-50%); + width:max-content;max-width:196px;text-align:left; + background:var(--ink);color:var(--surface);font:550 11.5px/1.3 var(--font); + padding:6px 9px;border-radius:6px;box-shadow:var(--shadow-pop);pointer-events:none; + opacity:0;visibility:hidden;transition:opacity .1s} +#toolbar button:hover .tip,#toolbar button:focus-visible .tip{opacity:1;visibility:visible} +#toolbar .tip b{font-weight:600} +#toolbar .tip kbd{margin-left:6px;font:600 10px/1 var(--mono);opacity:.55} +/* The gesture, under the name that has already been read. */ +#toolbar .tip em{display:block;margin-top:3px;font-style:normal;font-weight:450; + font-size:11px;line-height:1.4;opacity:.72} + #zoombar{position:absolute;left:12px;bottom:12px;display:flex;gap:2px;align-items:center;background:var(--surface); border:1px solid var(--line-2);border-radius:8px;padding:3px;box-shadow:var(--shadow-pop);z-index:20} #zoombar button{width:26px;height:24px;border-radius:5px;color:var(--ink-2);display:grid;place-items:center;font-size:14px} @@ -957,13 +1041,44 @@
-
+
+
+
+
+ +
+ + + + + +
+