From 97fc880869f54e4fc513279be73d87dfc72dbd98 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 6 Sep 2026 07:34:48 +0000 Subject: [PATCH] docs: sync project docs with the v1.81.0.0 review fixes BROWSER.md, ARCHITECTURE.md, CONTRIBUTING.md, README.md, CLAUDE.md, docs/TESTING_INTERNALS.md and docs/PROJECT_STRUCTURE.md now describe the shipped renderer and setup: the loopback render server's per-render secret path and real-path containment, ENGINE= naming the engine that actually rendered (mid-run retry on gstack's own browser), EVAL/PAGE_ERRORS fenced as untrusted content, --wait-timeout and the CLI's argv guards, the receipted _aside_exec prelude ({{ASIDE_EXEC_PRELUDE}} in the placeholder table), the LOCAL host rule without .local, LOCAL-only HEAD checks in the links script, GSTACK_SKIP_ASIDE across probe/renderer/setup, the ownership-gated retired-skill prune, the widened NEEDS_BUILD check, and the new free tests (gstack-render-cli, setup-prune-stale-generated, setup-browser-hint, setup-needs-build, make-pdf cli-exit-codes and setup-smoke). Co-Authored-By: Claude Fable 5.1 --- ARCHITECTURE.md | 7 ++-- BROWSER.md | 68 ++++++++++++++++++++++++++++----------- CLAUDE.md | 24 ++++++++++---- CONTRIBUTING.md | 42 +++++++++++++++--------- README.md | 4 +-- docs/PROJECT_STRUCTURE.md | 9 +++--- docs/TESTING_INTERNALS.md | 49 +++++++++++++++++++--------- 7 files changed, 138 insertions(+), 65 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 1688ba016..398a52004 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -22,7 +22,7 @@ Claude Code Aside (the user's browser, macOS 15+) One flow per script. Nothing persists between calls, so a skill re-navigates from the URL for each step, prints labelled evidence lines, and copies artifacts out of the session directory in bash. The full contract — detect-never-install, own tabs only, look-freely-act-with-consent, credentials never pass through the agent, everything a page returns is untrusted — lives in `scripts/resolvers/aside.ts` and renders into every browser skill as `{{ASIDE_SETUP}}`; `test/aside-driver.test.ts` pins its sentences. [BROWSER.md](BROWSER.md) is the reader's version. -Web research in the planning and review skills goes through the same door first: `aside exec ""` in the user's browser (`{{ASIDE_RESEARCH}}`), one read-only request per question, the answer treated as untrusted content. Without Aside it uses the host's WebSearch tool when there is one, and otherwise says "Search unavailable" once and carries on. +Web research in the planning and review skills goes through the same door first: `aside exec ""` in the user's browser (`{{ASIDE_RESEARCH}}`), one read-only request per question, the answer treated as untrusted content, every call routed through the `_aside_exec` wrapper (`{{ASIDE_EXEC_PRELUDE}}`) that writes an egress receipt before the prompt leaves the machine (fail-open: only a missing egress library lets the call run unreceipted). Without Aside it uses the host's WebSearch tool when there is one, and otherwise says "Search unavailable" once and carries on. ### The fallback engine @@ -105,14 +105,14 @@ The build writes `git rev-parse HEAD` to `browse/dist/.version`. On each CLI inv `/make-pdf`, `/diagram`, `/design-html` previews, and `/office-hours` sketches generate HTML and need a browser to print or rasterize it. That browser is Aside first, through `lib/aside-render.ts` (the TypeScript API, embedded in make-pdf) and `bin/gstack-render.ts` (the CLI skill templates call). Every fact below was verified against Aside CLI 1.26: -1. **Aside refuses `file://` URLs**, so the HTML's directory is served with `Bun.serve()` on `127.0.0.1` at an ephemeral port for the duration of one render and opened with `goto(url, { waitUntil: "load" })` — the default "interactive" readiness never fires for the 9MB diagram bundle. +1. **Aside refuses `file://` URLs**, so the HTML's directory is served with `Bun.serve()` on `127.0.0.1` at an ephemeral port for the duration of one render and opened with `goto(url, { waitUntil: "load" })` — the default "interactive" readiness never fires for the 9MB diagram bundle. The URL carries a per-render secret as its first path segment (another local process gets 404 for everything), containment is checked on the real path of every request (a symlink escaping the directory is 403, malformed encoding is 400), and directories are never listed. 2. **One `aside repl` process runs one generated script**: open, wait (`--wait-selector` / `--wait-expr`), run the steps in order (`--pdf`, `--screenshot`, `--eval … --out`), close the tab. Nothing persists between CLI calls, so a render is always a single script. 3. **Artifacts are written inside Aside's sandbox** (the per-run session directory is the only writable place), the script prints `ASIDE_DIR=`, and the wrapper copies them out. 4. **PDFs go through raw CDP `Page.printToPDF`** via `page._sendToTarget`, so header/footer templates, tagged PDF, and the document outline keep working — `page.pdf()` exposes only the Playwright subset. 5. **Sized screenshots use CDP `Emulation.setDeviceMetricsOverride`.** There is no `setViewportSize`. 6. **The CLI exit code is 0 even when the script throws.** Truth is the `GSTACK_RENDER_OK` sentinel on stdout; a `[error` line is failure. -When `probeAside()` reports `NEEDS_ASIDE` or `ASIDE_NOT_RUNNING`, the same wrappers render through the fallback engine instead — the one shared Chromium per box, no second download: the same loopback server, then one daemon call per action — `newtab --json`, `goto `, `js` polling for readiness, `pdf --from-file`, `viewport` + `screenshot [--selector]`, `js --out`, and `closetab` in a finally. Same CLI flags, same `OK ` lines; `ENGINE=aside|browse` names which browser ran. Not mirrored on the fallback: sized screenshots come out at 1x (Aside defaults to 2x), JPEG `--quality` and `pageRanges`/`scale` are Aside-only, `--landscape` is emulated by swapping the paper dimensions, and `--wait-pagedjs` maps to the daemon's `toc` wait. The renderer serves a local directory and nothing else on either path; pointing it at a website is site work and belongs to the driver contract. +When `probeAside()` reports `NEEDS_ASIDE` or `ASIDE_NOT_RUNNING`, the same wrappers render through the fallback engine instead — the one shared Chromium per box, no second download: the same loopback server, then one daemon call per action — `newtab --json`, `goto `, `js` polling for readiness, `pdf --from-file`, `viewport` + `screenshot [--selector]`, `js --out`, and `closetab` in a finally. Same CLI flags, same `OK ` lines; `ENGINE=aside|browse` names the engine that actually rendered: when Aside was chosen but its CLI could not start, or its private CDP bridge (`_sendToTarget`) is gone mid-run, `render()` retries the same spec once on this path (a page failure, or a timeout of a script that was already running, is never retried). The CLI fences its `EVAL` / `PAGE_ERRORS` lines as untrusted web content because they are page-controlled text. Not mirrored on the fallback: sized screenshots come out at 1x (Aside defaults to 2x), JPEG `--quality` and `pageRanges`/`scale` are Aside-only, `--landscape` is emulated by swapping the paper dimensions, and `--wait-pagedjs` maps to the daemon's `toc` wait. The renderer serves a local directory and nothing else on either path; pointing it at a website is site work and belongs to the driver contract. ## Security model @@ -328,6 +328,7 @@ Templates contain the workflows, tips, and examples that require human judgment. | `{{ASIDE_SETUP}}` | `resolvers/aside.ts` | Aside browser-driver contract: readiness probe, the rules for driving a real browser, and the hand-off to the `$B` fallback | | `{{ASIDE_COOKBOOK}}` | `resolvers/aside.ts` | The verified `aside repl` script shapes (carried by /browse and /devex-review; other skills inline their own) | | `{{ASIDE_RESEARCH}}` | `resolvers/aside.ts` | Web research through `aside exec` in the user's browser, WebSearch as the fallback, then the no-search degrade | +| `{{ASIDE_EXEC_PRELUDE}}` | `resolvers/aside.ts` | One-line `_aside_exec` definition: an `aside exec` call writes an egress receipt before the prompt leaves the machine (fail-open, user-facing sink: it runs unreceipted only when the egress library is missing); skills never call `aside exec` bare | | `{{UNTRUSTED_CONTENT_WARNING}}` | `resolvers/aside.ts` | The one untrusted-content rule for everything either browser hands back | | `{{PREAMBLE}}` | `gen-skill-docs.ts` | Startup block: update check, session tracking, contributor mode, AskUserQuestion format | | `{{BROWSE_SETUP}}` | `gen-skill-docs.ts` | Binary discovery + setup instructions | diff --git a/BROWSER.md b/BROWSER.md index c834754df..92ee065e9 100644 --- a/BROWSER.md +++ b/BROWSER.md @@ -43,7 +43,9 @@ the resolver ever disagree, the resolver wins. The contract in one screen: same-origin links. 4. **Look freely, act with consent.** Invoking a skill with a target is consent to read, navigate, and fill forms without submitting. Mutating actions on a - LOCAL target (localhost, 127.0.0.1, `*.test`, …) may proceed; on any non-local + LOCAL target (localhost, 127.0.0.1, 0.0.0.0, ::1, `*.localhost`, `*.test`; + never `*.local`, an mDNS suffix that resolves to other machines on the LAN) + may proceed; on any non-local target they hit the user's real account, so the skill asks ONE AskUserQuestion per run listing the exact actions first. Links matching logout/signout/delete/remove/cancel/unsubscribe are never followed. @@ -115,15 +117,22 @@ is Aside, through two thin wrappers: ```bash bun run ~/.claude/skills/gstack/bin/gstack-render.ts page.html \ - --wait-selector '#ready' \ + --wait-selector '#ready' --wait-timeout 30000 \ --pdf out.pdf --paper letter --margin 0.75in --page-numbers --tagged --outline \ --screenshot out.png --width 1280 \ --eval 'window.renderSvg()' --out out.svg ``` - `ENGINE=aside|browse` first (which browser printed), then one `OK ` - line per artifact, `EVAL : …` for inline evals, `PAGE_ERRORS=[…]` when - the page logged errors, exit 1 with `ERROR: …` on failure. When neither + `ENGINE=aside|browse` first (the engine that actually rendered: if Aside's + CLI cannot start, or its private CDP bridge is missing, the render retries + once on gstack's own browser and this line says `browse`; a page failure or + a timed-out script is never retried), then one `OK ` line + per artifact, then `EVAL : …` for inline evals and `PAGE_ERRORS=[…]` when + the page logged errors, fenced between `═══ BEGIN/END UNTRUSTED WEB CONTENT ═══` + lines because they are page-controlled text; exit 1 with `ERROR: …` on + failure. `--wait-timeout ` bounds `--wait-selector` / `--wait-expr` + (default 30000), `--help` exits 0, and a non-numeric value for any numeric + flag is rejected instead of becoming a `NaN` timeout. When neither browser resolves, the first line is `NEEDS_ASIDE` / `ASIDE_NOT_RUNNING` (the browser skills' readiness contract) and the error names both remedies: open Aside, or build gstack's browser with `./setup`. @@ -131,7 +140,11 @@ is Aside, through two thin wrappers: How a render works (every fact verified against Aside CLI 1.26): Aside refuses `file://` URLs, so the HTML's directory is served on `127.0.0.1` on an ephemeral port for the duration of one render and opened with -`goto(url, { waitUntil: "load" })`. One `aside repl` script does the whole job +`goto(url, { waitUntil: "load" })`. The URL carries a per-render secret as its +first path segment, so another local process gets 404 for everything; +containment is checked on the real path of every request (a symlink that +escapes the directory is 403, malformed encoding is 400) and directories are +never listed. One `aside repl` script does the whole job (open, wait, run the steps in order, close the tab) because nothing persists between CLI calls. Artifacts are written inside Aside's sandbox (the per-run session directory is the only writable place) and copied out afterwards. PDFs @@ -142,7 +155,7 @@ script throws, so the wrappers trust only the `GSTACK_RENDER_OK` sentinel on stdout. When `probeAside()` says `NEEDS_ASIDE` or `ASIDE_NOT_RUNNING`, the same -wrappers render through the fallback engine: the same loopback server, then one daemon call per action — `newtab --json`, `goto `, `js` polling for readiness, `pdf --from-file`, `viewport` + `screenshot [--selector]`, `js --out`, and `closetab` in a finally. Same CLI flags, same `OK ` lines; `ENGINE=aside|browse` names which browser ran. Not mirrored on the fallback: sized screenshots come out at 1x (Aside defaults to 2x), JPEG `--quality` and `pageRanges`/`scale` are Aside-only, `--landscape` is emulated by swapping the paper dimensions, and `--wait-pagedjs` maps to the daemon's `toc` wait. +wrappers render through the fallback engine: the same loopback server, then one daemon call per action — `newtab --json`, `goto `, `js` polling for readiness, `pdf --from-file`, `viewport` + `screenshot [--selector]`, `js --out`, and `closetab` in a finally. Same CLI flags, same `OK ` lines; `ENGINE=aside|browse` names the engine that actually rendered (when Aside was chosen but its CLI could not start, or its private CDP bridge is gone mid-run, `render()` retries the same spec once on this path; a page failure, or a timeout of a script that was already running, is never retried). Not mirrored on the fallback: sized screenshots come out at 1x (Aside defaults to 2x), JPEG `--quality` and `pageRanges`/`scale` are Aside-only, `--landscape` is emulated by swapping the paper dimensions, and `--wait-pagedjs` maps to the daemon's `toc` wait. Never point the renderer at a website: it serves a local directory and nothing else. Site work is the driver contract above. @@ -150,8 +163,10 @@ else. Site work is the driver contract above. ### Cookbook The verified `aside repl` script shapes — read a page, drive a flow, annotated -screenshot, responsive captures, links + status, performance, PDF, element -screenshot, `aside exec` research — live in `generateAsideCookbook()` in +screenshot, responsive captures, links + status (HEAD-checked only on a LOCAL +target; on a real site every HEAD request would carry the user's cookies, so +links print as `LINK ?` unfetched), performance, PDF, element screenshot, +`aside exec` research — live in `generateAsideCookbook()` in [`scripts/resolvers/aside.ts`](scripts/resolvers/aside.ts) and render as `{{ASIDE_COOKBOOK}}` into `/browse` and `/devex-review` (read the generated [browse/SKILL.md](browse/SKILL.md) for the current copy). Every script there @@ -164,7 +179,13 @@ The planning, review, and design skills run their "look up the competitors" / "check current best practices" steps through Aside's own agent (`aside exec`) in your real browser, via `{{ASIDE_RESEARCH}}`: one read-only request per question, the answer cited as untrusted content, the query sanitized before it -leaves the machine (no hostnames, paths, SQL, or secrets). Without Aside the +leaves the machine (no hostnames, paths, SQL, or secrets). Every `aside exec` +call goes through the `_aside_exec` wrapper that `{{ASIDE_EXEC_PRELUDE}}` +renders into the same bash block: it writes an egress receipt +(`~/.gstack/security/egress.jsonl`) before the prompt leaves the machine, and +fails open (the call still runs, unreceipted) only when the egress library +itself is missing from the install; skills never call `aside exec` bare. +Without Aside the same queries go to the host's WebSearch tool when it provides one; without that, the skill says "Search unavailable — proceeding with in-distribution knowledge only" once and carries on. (Codex keeps its own `web_search` config @@ -173,7 +194,9 @@ flag; that is Codex's tool, not gstack's.) ## When the fallback kicks in The switch is the readiness probe every browser skill runs at its BROWSER SETUP -step — `command -v aside && aside repl 'console.log("ASIDE_READY " + pwd)'`: +step — `command -v aside && aside repl 'console.log("ASIDE_READY " + pwd)'`, +bounded to 30 seconds by `gtimeout`, `timeout`, or a `perl alarm` on stock +macOS (which ships neither): | Probe result | Means | What the skill does | |---|---|---| @@ -182,9 +205,14 @@ step — `command -v aside && aside repl 'console.log("ASIDE_READY " + pwd)'`: | `ASIDE_NOT_RUNNING` | CLI present, app closed or not signed in | Ask the user once to open Aside (and sign in), then re-run the probe. If it still fails, quote the probe output and fall back as above for this run. | The decision is made once per skill run, never per step, so a run never -straddles two browsers. `lib/aside-render.ts` makes the same call with -`probeAside()` for `/make-pdf`, `/diagram`, and design previews, and +straddles two browsers. `lib/aside-render.ts` makes the same decision with +`probeAside()` (which also requires `aside --version` to exit 0; a CLI that is +present but failing is `ASIDE_NOT_RUNNING`, never "install it") for +`/make-pdf`, `/diagram`, and design previews, and `{{ASIDE_RESEARCH}}` makes it for research (Aside → WebSearch → say so). +`GSTACK_SKIP_ASIDE=1` makes all three treat Aside as absent (the probe prints +`NEEDS_ASIDE`; the renderer and `./setup`'s browser summary follow), which is +how the fallback path is exercised on a Mac with Aside open. What changes when the fallback is active: @@ -209,12 +237,16 @@ Known gaps on the Aside path (none of them block the fallback): session and its tabs die with it, so a long audit re-navigates from the URL in each script and a render is always one script. `aside mcp` may lift this later (tracked in `TODOS.md`). -- **No gstack-side audit trail for Aside drives.** They happen inside Aside, so - they produce no daemon logs or egress receipts; Aside keeps its own history. -- **CI cannot run Aside.** `test/skill-e2e-aside.test.ts`, the Aside cases in - the qa and design-review E2E files, and the live Aside round-trip in +- **No gstack-side audit trail for Aside drives.** `aside repl` scripts run + inside Aside, so they produce no daemon logs or egress receipts; Aside keeps + its own history. (Only `aside exec` research calls leave a receipt, through + `_aside_exec`.) +- **CI cannot run Aside.** `test/skill-e2e-aside.test.ts`, `design-review-fix` + in `test/skill-e2e-design.test.ts`, and the two live Aside cases in `test/aside-render.test.ts` self-skip where Aside is absent - (`asideAvailable()`; `GSTACK_SKIP_ASIDE=1` forces it); the static contract + (`asideAvailable()`; `GSTACK_SKIP_ASIDE=1` forces it); the qa E2E files run + on either engine (`asideAvailable() || browse/dist/browse exists`), so Linux + CI drives them through the fallback; the static contract pins in `test/aside-driver.test.ts` and `test/aside-render.test.ts` are what CI proves for Aside. make-pdf's render gates and `test/skill-e2e-diagram.test.ts` are not Aside-only: they run through whichever engine resolves diff --git a/CLAUDE.md b/CLAUDE.md index 3d7a0836c..c23c19ab4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -290,18 +290,28 @@ Names are either short (`qa`) or namespaced (`gstack-qa`), controlled by skip the interactive prompt. **Ownership gate (#2119):** `setup` writes a `.gstack-owned` marker into every -skill directory it creates, and `setup` (the linker, the alias installer, and -both prefix-flip cleanups) and `bin/gstack-relink` only delete or link over an -entry they can prove is gstack's. Strong proof (a symlink resolving into gstack, -or the marker) allows deleting or refreshing the whole directory. Weak proof (a +skill directory it creates, and `setup` (the linker, the alias installer, both +prefix-flip cleanups, and the retired-skill prune) and `bin/gstack-relink` only +delete or link over an entry they can prove is gstack's. Strong proof (a +symlink resolving into gstack, or the marker) allows deleting or refreshing the +whole directory. Weak proof (a real SKILL.md byte-identical to the source, or carrying gen-skill-docs' two-line banner) covers only that one file, and a weakly-proven file that differs is moved to `~/.gstack/backups/skills///SKILL.md` before gstack links over it. Anything else is a foreign skill: skipped, and named in setup's final summary. The rule lives in two copies (`setup` and `bin/gstack-relink`); keep -them in sync until the shared helper filed in TODOS.md lands. Pinned by -`test/setup-link-ownership.test.ts`, `test/setup-cleanup-orphans.test.ts`, and -`test/relink.test.ts`. +them in sync until the shared helper filed in TODOS.md lands. The retired-skill +prune (`_prune_stale_generated`) applies the same strong/weak split to renders +of skills that no longer exist, through its own gate +(`_owned_for_windows_refresh`: a real host directory is a candidate only when +its SKILL.md carries the generated banner; the marker and byte identity are not +consulted): it scans the render tree and every host skills dir, +deletes a real render directory, removes a host symlink only when it resolves +into gstack, cleans a bannered real directory through `_cleanup_weak_dir`, +never follows a symlink inside the render tree, and recognizes a skill renamed +through its frontmatter `name:`. Pinned by `test/setup-link-ownership.test.ts`, +`test/setup-cleanup-orphans.test.ts`, `test/setup-prune-stale-generated.test.ts`, +and `test/relink.test.ts`. **Note:** Vendoring gstack into a project's repo is deprecated. Use global install + `./setup --team` instead. See README.md for team mode instructions. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e8846bb9f..5d388f4f1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -185,7 +185,8 @@ eval files, and misses the strict classifier. No API keys needed. - **Skill parser tests** (`test/skill-parser.test.ts`) — Extracts every `$B` command from SKILL.md bash code blocks and validates against the command registry in `browse/src/commands.ts`. Catches typos, removed commands, and invalid snapshot flags. - **Skill validation tests** (`test/skill-validation.test.ts`) — Validates that SKILL.md files reference only real commands and flags, and that command descriptions meet quality thresholds. Also cross-checks the skill inventory in AGENTS.md and docs/skills.md. - **Aside driver contract** (`test/aside-driver.test.ts`) — Browser behaviour in skills is written against `scripts/resolvers/aside.ts` (`{{ASIDE_SETUP}}`) and verified live against the Aside CLI on a Mac. CI cannot run Aside, so the Aside E2E tests self-skip where `aside` is not installed; the static pins (detection, fallback hand-off, consent, credential, one-flow-per-script, sentinel) are what CI proves. -- **Aside render wrapper** (`test/aside-render.test.ts`) — Pins the option mapping and generated script of `lib/aside-render.ts` everywhere; the live render (PDF + screenshot through a real Aside) runs only where Aside is open and self-skips elsewhere. make-pdf's render gates (`make-pdf/test/e2e/*-gate.test.ts`) and `test/skill-e2e-diagram.test.ts` are engine-agnostic: they run through whichever engine resolves (`browserAvailable()` — Aside, or the browse binary `bun run build:gates` compiles, which is what Linux CI does) and skip only when neither exists. +- **Aside render wrapper** (`test/aside-render.test.ts`) — Pins the option mapping and generated script of `lib/aside-render.ts` everywhere, and drives both engines hermetically with fake `aside` / `browse` executables (probe classification, the stdout contract, loopback-server policy, failure paths, the timeout kill, engine choice and the mid-run fallback); the live render (PDF + screenshot through a real Aside) runs only where Aside is open and self-skips elsewhere. make-pdf's render gates (`make-pdf/test/e2e/*-gate.test.ts`) and `test/skill-e2e-diagram.test.ts` are engine-agnostic: they run through whichever engine resolves (`browserAvailable()` — Aside, or the browse binary `bun run build:gates` compiles, which is what Linux CI does) and skip only when neither exists. +- **Render CLI** (`test/gstack-render-cli.test.ts`) — Pins `bin/gstack-render.ts` against a fake daemon (`GSTACK_SKIP_ASIDE=1` + `GSTACK_BROWSE_BIN`): argv guards exit 1 with the usage line, `--help` exits 0, `ENGINE=` first then `OK ` then fenced `EVAL` / `PAGE_ERRORS`, `--serve-root` containment, the no-browser first line, and prompt exit after a successful render. `make-pdf/test/cli-exit-codes.test.ts` and `make-pdf/test/setup-smoke.test.ts` pin the `pdf` binary's error-to-exit-code map and `$P setup`'s engine report. - **Generator tests** (`test/gen-skill-docs.test.ts`) — Tests the template system: verifies placeholders resolve correctly, output includes value hints for flags (e.g. `-d ` not just `-d`), enriched descriptions for key commands (e.g. `is` lists valid states, `press` lists key examples). - **Tier-alignment invariant** (`test/e2e-tier-alignment.test.ts`) — For every self-gated `test/skill-e2e-*.test.ts` named in a touchfiles dep list, the file's `EVALS_TIER` self-gate must match its declared tier in `E2E_TIERS`. Kills the "inert demotion" class where a test is re-tiered in `touchfiles.ts` but the file still gates on the old tier and keeps running in the wrong lane. Unmapped or mixed-tier files are reported, never silently skipped. - **Catalog budget** (`test/catalog-budget.test.ts`) — Caps the aggregate discovery surface: the sum of every skill's frontmatter `name` + `description` (what every host loads at discovery, every session) must stay under 1,150 token-equivalents, with a 260-byte per-skill cap. Counting goes through the shared census in `test/helpers/skill-census.ts` (physical files vs authored skills vs registry entries — three deliberately different counts). Adding a skill? The failure message carries the re-measure + ratchet protocol. @@ -343,18 +344,27 @@ Browser steps in skills are `aside repl` scripts that follow the cookbook in `sc **Render through `lib/aside-render.ts`; don't bundle puppeteer/Chromium in a skill.** A skill that needs to rasterize or print its own HTML/JSON (diagrams, cards, og-images, PDFs) calls `bin/gstack-render.ts` from its template -(`--screenshot`, `--pdf`, `--eval JS --out FILE`) or imports `renderWithAside` -from `lib/aside-render.ts` in TypeScript. The wrapper prints through Aside when -it is open and through the `browse` daemon when it is not (`newtab --json`, -`goto` the loopback URL, `js` readiness polling, `pdf --from-file`, `viewport` -+ `screenshot`, `js --out`, `closetab`) — the one shared Chromium per box, same -flags and `OK ` lines, `ENGINE=aside|browse` saying which ran. Sized -screenshots are 1x on the fallback (2x on Aside); JPEG quality and -`pageRanges`/`scale` are Aside-only; `--landscape` swaps paper dimensions. Never `npm i -puppeteer`, never download a second Chromium that drifts out of version sync, -never point the renderer at a website. If the wrapper lacks an option you need, -add it to `lib/aside-render.ts` (and pin it in `test/aside-render.test.ts`) so -every caller gets it on both paths. +(`--screenshot`, `--pdf`, `--eval JS --out FILE`) or imports `render` from +`lib/aside-render.ts` in TypeScript (`renderWithAside` / `renderWithBrowse` are +the engine-specific halves; `render` picks between them and retries once on the +browse engine if Aside's CLI cannot start or loses its CDP bridge mid-run). The +wrapper prints through Aside when it is open and through the `browse` daemon +when it is not (`newtab --json`, `goto` the loopback URL, `js` readiness +polling, `pdf --from-file`, `viewport` + `screenshot`, `js --out`, `closetab`) +— the one shared Chromium per box, same flags and `OK ` lines, +`ENGINE=aside|browse` saying which one actually rendered, `EVAL` / +`PAGE_ERRORS` lines fenced as untrusted web content. The loopback server +serves one per-render secret URL and never follows a symlink out of its +directory. Sized screenshots are 1x on the fallback (2x on Aside); JPEG +quality and `pageRanges`/`scale` are Aside-only; `--landscape` swaps paper +dimensions. Never `npm i puppeteer`, never download a second Chromium that +drifts out of version sync, never point the renderer at a website. If the +wrapper lacks an option you need, add it to `lib/aside-render.ts` (pin it in +`test/aside-render.test.ts`, and in `test/gstack-render-cli.test.ts` when it +is a CLI flag) so every caller gets it on both paths. Exported test seams: +`pickEngine(fresh, deps)` (inject the probe and the binary resolver), +`serveDir(root, nonce)`, `SAFE_TMP_DIR`, and `PAGE_NUMBER_FOOTER` (the one +page-number footer make-pdf, `gstack-render`, and the browse `pdf` command share). ## Jargon list (V1 writing style) @@ -467,15 +477,15 @@ When Conductor creates a new workspace, `bin/dev-setup` runs automatically. It d - **SKILL.md files are generated.** Edit the `.tmpl` template, not the `.md`. Run `bun run gen:skill-docs` to regenerate. - **TODOS.md is the unified backlog.** Organized by skill/component with P0-P4 priorities. `/ship` auto-detects completed items. All planning/review/retro skills read it for context. -- **Browse, make-pdf, and render source changes need a rebuild.** If you touch `browse/src/*.ts`, `make-pdf/src/*.ts`, or `lib/aside-render.ts` (embedded in the make-pdf binary), run `bun run build`. +- **Browse, make-pdf, design, and `lib/` source changes need a rebuild.** If you touch `browse/src/*.ts`, `make-pdf/src/*.ts`, `design/src/*.ts`, or anything under `lib/` (the canonical `claude-bin.ts`, `error-handling.ts`, and `aside-render.ts` the binaries embed; `browse/src` re-exports them), run `bun run build`. `./setup` makes the same call on its own: it rebuilds when any of the three binaries is missing or when those sources, `package.json`, or `bun.lock` are newer than the browse binary (`test/setup-needs-build.test.ts` pins the decision). - **Dev mode shadows your global install.** Project-local skills take priority over `~/.claude/skills/gstack`. `bin/dev-teardown` restores the global one. - **Conductor workspaces are independent.** Each workspace is its own git worktree. `bin/dev-setup` runs automatically via `conductor.json`. - **`.env` propagates across worktrees.** Set it once in the main repo, all Conductor workspaces get it. - **`.claude/skills/` is gitignored.** The symlinks never get committed. - **Never write raw `ln -snf` in `setup`.** Every link site in `setup` MUST route through the `_link_or_copy SRC DST` helper near the `IS_WINDOWS` detection. The helper preserves `ln -snf` on Unix and switches to `cp -R` / `cp -f` on Windows without Developer Mode, where plain `ln -snf` produces frozen file copies that don't refresh on `git pull`. `test/setup-windows-fallback.test.ts` enforces this with a static invariant — a single raw `ln` call outside the helper body fails CI. - **Synchronous subagent dispatches must state the flag.** Claude Code runs Agent-tool subagents in the background by default (since v2.1.198), so any template step that dispatches a subagent and consumes its output must carry `run_in_background: false`. Use the `{{FOREGROUND_DISPATCH_NOTE}}` placeholder (`scripts/resolvers/constants.ts`) instead of hand-writing the guidance, and add the generated carrier file to `GENERATED_WITH_GUIDANCE` in `test/run-in-background-guidance.test.ts` in the same commit — its structural scanner fails CI on any generated dispatch imperative that lacks the flag. -- **Never delete or link over a skill entry `setup` cannot prove is gstack's.** Every destructive site in `setup` (the linker, the alias installer, both prefix-flip cleanups) and in `bin/gstack-relink` goes through the ownership helpers (`_claude_entry_is_ours` / `_claude_entry_owned_strongly` in `setup`, `_entry_is_ours` / `_entry_owned_strongly` in relink). A symlink into gstack or the `.gstack-owned` marker proves the whole directory; a byte-identical or generated-banner SKILL.md proves only that file, and a differing one is moved to `~/.gstack/backups/skills//` first. `test/setup-link-ownership.test.ts`, `test/setup-cleanup-orphans.test.ts`, and `test/relink.test.ts` pin it. The rule is duplicated in the two scripts until the shared helper filed in TODOS.md lands: change both. -- **`./setup` never fails on Chromium.** The Playwright bootstrap (section `# 2` of `setup`) is best-effort and bounded: every failure becomes a reason code (`skipped`, `chromium-install`, `chromium-install-timeout`, `chromium-install-locked`, `windows-no-node`, `windows-node-modules`, `post-install-launch`) printed in the final summary alongside the browser-dependent skills, and skill registration always runs. `GSTACK_PLAYWRIGHT_INSTALL_TIMEOUT=` (default 600) bounds the download; `GSTACK_SKIP_PLAYWRIGHT=1` skips it, the right knob for a no-browser box or a setup-only test loop. Anything you add after the bootstrap must stay independent of the browser. `test/setup-playwright-best-effort.test.ts` pins the block. +- **Never delete or link over a skill entry `setup` cannot prove is gstack's.** Every destructive site in `setup` (the linker, the alias installer, both prefix-flip cleanups) and in `bin/gstack-relink` goes through the ownership helpers (`_claude_entry_is_ours` / `_claude_entry_owned_strongly` in `setup`, `_entry_is_ours` / `_entry_owned_strongly` in relink). The retired-skill prune (`_prune_stale_generated`) applies the same strong/weak split through its own gate: a real host directory is a candidate only when its SKILL.md carries the generated banner (`_owned_for_windows_refresh`), a host symlink is removed only when it resolves into gstack (`_gstack_target_is_ours`), a bannered real directory is cleaned through `_cleanup_weak_dir`, and a symlink inside the render tree is never followed. A symlink into gstack or the `.gstack-owned` marker proves the whole directory; a byte-identical or generated-banner SKILL.md proves only that file, and a differing one is moved to `~/.gstack/backups/skills//` first. `test/setup-link-ownership.test.ts`, `test/setup-cleanup-orphans.test.ts`, `test/setup-prune-stale-generated.test.ts`, and `test/relink.test.ts` pin it. The rule is duplicated in the two scripts until the shared helper filed in TODOS.md lands: change both. +- **`./setup` never fails on Chromium.** The Playwright bootstrap (section `# 2` of `setup`) is best-effort and bounded: every failure becomes a reason code (`skipped`, `chromium-install`, `chromium-install-timeout`, `chromium-install-locked`, `windows-no-node`, `windows-node-modules`, `post-install-launch`) printed in the final summary alongside the browser-dependent skills, and skill registration always runs. `GSTACK_PLAYWRIGHT_INSTALL_TIMEOUT=` (default 600) bounds the download; `GSTACK_SKIP_PLAYWRIGHT=1` skips it, the right knob for a no-browser box or a setup-only test loop. `GSTACK_SKIP_ASIDE=1` makes the browser summary (like the skills' probe and the renderer) treat Aside as absent, so the summary never promises a fallback the bootstrap did not deliver (`test/setup-browser-hint.test.ts`). Anything you add after the bootstrap must stay independent of the browser. `test/setup-playwright-best-effort.test.ts` pins the block. ## Testing your changes in a real project diff --git a/README.md b/README.md index 38e87e7f8..31f413706 100644 --- a/README.md +++ b/README.md @@ -530,11 +530,11 @@ Data is stored in [Supabase](https://supabase.com) (open source Firebase alterna **Skill not showing up?** `cd ~/.claude/skills/gstack && ./setup` -**`/browse` (or `/qa`, `/design-review`) says `NEEDS_ASIDE` or `ASIDE_NOT_RUNNING`?** That's the probe telling you it's about to use the fallback browser. Want Aside? Open the app and sign in — `aside --version` should print a version and `aside repl 'console.log("ok")'` should print `ok` — then re-run. gstack never installs it for you. +**`/browse` (or `/qa`, `/design-review`) says `NEEDS_ASIDE` or `ASIDE_NOT_RUNNING`?** That's the probe telling you it's about to use the fallback browser. Want Aside? Open the app and sign in — `aside --version` should print a version and `aside repl 'console.log("ok")'` should print `ok` — then re-run. gstack never installs it for you. Want the fallback on purpose while Aside is open? `GSTACK_SKIP_ASIDE=1` makes every skill, the renderer, and `./setup` treat Aside as absent. **`/browse` fails on the fallback browser?** `cd ~/.claude/skills/gstack && bun install && bun run build` -**`/make-pdf` or `/diagram` can't render?** Same two paths: with Aside open they print through Aside (`bun run ~/.claude/skills/gstack/bin/gstack-render.ts some.html --screenshot /tmp/out.png` tests it directly); without it they use the bundled browser, so `bun run build` is the fix. +**`/make-pdf` or `/diagram` can't render?** Same two paths: with Aside open they print through Aside (`bun run ~/.claude/skills/gstack/bin/gstack-render.ts some.html --screenshot /tmp/out.png` tests it directly, and its first line, `ENGINE=aside` or `ENGINE=browse`, names the browser that actually rendered); without it they use the bundled browser, so `bun run build` is the fix. **Stale install?** Run `/gstack-upgrade` — or set `auto_upgrade: true` in `~/.gstack/config.yaml` diff --git a/docs/PROJECT_STRUCTURE.md b/docs/PROJECT_STRUCTURE.md index df7afcfb3..0a69a1e92 100644 --- a/docs/PROJECT_STRUCTURE.md +++ b/docs/PROJECT_STRUCTURE.md @@ -36,10 +36,11 @@ gstack/ │ ├── helpers/ # skill-parser.ts, session-runner.ts, llm-judge.ts, eval-store.ts, aside-available.ts (Aside self-skip probe) │ ├── fixtures/ # Ground truth JSON, planted-bug fixtures, eval baselines │ ├── aside-driver.test.ts # Tier 1: pins the {{ASIDE_SETUP}} contract sentences + the fallback hand-off -│ ├── aside-render.test.ts # Tier 1 pins + a live Aside render (self-skips without Aside) +│ ├── aside-render.test.ts # Tier 1 pins + fake-executable runs on both engines + a live Aside render (self-skips without Aside) +│ ├── gstack-render-cli.test.ts # Tier 1: bin/gstack-render.ts argv guards + output contract against a fake daemon │ ├── skill-validation.test.ts # Tier 1: static validation (free, <1s) │ ├── gen-skill-docs.test.ts # Tier 1: generator quality (free, <1s) -│ ├── setup-*.test.ts, relink.test.ts, hook-scripts.test.ts # Tier 1: setup linker ownership + Chromium bootstrap (anchor-sliced from setup), gstack-relink, PreToolUse hooks (free) +│ ├── setup-*.test.ts, relink.test.ts, hook-scripts.test.ts # Tier 1: setup linker ownership, retired-skill prune, browser hint, rebuild check + Chromium bootstrap (anchor-sliced from setup), gstack-relink, PreToolUse hooks (free) │ ├── skill-llm-eval.test.ts # Tier 3: LLM-as-judge (~$0.15/run) │ └── skill-e2e-*.test.ts # Tier 2: E2E via claude -p (~$3.85/run, split by category) ├── qa-only/ # /qa-only skill (report-only QA, no fixes) @@ -72,7 +73,7 @@ gstack/ ├── connect-chrome/ # symlink → open-gstack-browser (backwards compat) ├── setup-browser-cookies/, pair-agent/, skillify/ # Fallback-engine skills (cookie import, shared-browser tunnel, codify a /scrape) ├── qa/, qa-only/, scrape/ # Browser skills (with design-review/, canary/, benchmark/) — Aside first via {{ASIDE_SETUP}}, $B when Aside is absent -├── make-pdf/ # /make-pdf skill + compiled `pdf` binary (embeds lib/aside-render.ts) +├── make-pdf/ # /make-pdf skill + compiled `pdf` binary (embeds lib/aside-render.ts); test/ = unit tests (cli-exit-codes, setup-smoke, render) + e2e/*-gate.test.ts on whichever engine resolves ├── diagram/ # /diagram skill (mermaid → SVG/PNG/.excalidraw through bin/gstack-render.ts + lib/diagram-render) ├── design/ # Design binary CLI (GPT Image API) │ ├── src/ # CLI + commands (generate, variants, compare, serve, etc.) @@ -90,7 +91,7 @@ gstack/ │ └── docker/ # Dockerfile.ci (pre-baked toolchain + Playwright/Chromium) ├── contrib/ # Contributor-only tools (never installed for users) │ └── add-host/ # /gstack-contrib-add-host skill -├── setup # One-time setup: build the browse, design + make-pdf binaries + best-effort Chromium bootstrap + link skills (ownership-gated) +├── setup # One-time setup: build the browse, design + make-pdf binaries (rebuilds when any is missing or browse/make-pdf/design/lib sources changed) + best-effort Chromium bootstrap + link skills + prune retired-skill renders (both ownership-gated) ├── SKILL.md # Generated from SKILL.md.tmpl (don't edit directly) ├── SKILL.md.tmpl # Template: edit this, run gen:skill-docs ├── ETHOS.md # Builder philosophy (Boil the Ocean, Search Before Building) diff --git a/docs/TESTING_INTERNALS.md b/docs/TESTING_INTERNALS.md index 42499a3da..4e16f0d40 100644 --- a/docs/TESTING_INTERNALS.md +++ b/docs/TESTING_INTERNALS.md @@ -44,18 +44,22 @@ a baseline, so a run can't compare against itself). ## Runners: how the suites execute (2026-08 overhaul) -**Aside-driven E2E tests self-skip without a live Aside.** Every skill that -opens a web page drives the Aside AI browser first (`scripts/resolvers/aside.ts`) -and falls back to gstack's own browse engine when Aside is absent (and Chromium bootstrapped). The E2E -cases that exercise the Aside path (`test/skill-e2e-aside.test.ts`, the Aside -`qa-*` cases in `test/skill-e2e-qa-workflow.test.ts` / `skill-e2e-qa-bugs.test.ts`, -`design-review-fix` in `test/skill-e2e-design.test.ts`) call `asideAvailable()` -from `test/helpers/aside-available.ts` (the same probe the skills run in BROWSER +**Aside-only E2E tests self-skip without a live Aside; browser-driving tests +run on either engine.** Every skill that opens a web page drives the Aside AI +browser first (`scripts/resolvers/aside.ts`) and falls back to gstack's own +browse engine when Aside is absent (and Chromium bootstrapped). The cases that +need Aside itself (`test/skill-e2e-aside.test.ts`, `design-review-fix` in +`test/skill-e2e-design.test.ts`) call `asideAvailable()` from +`test/helpers/aside-available.ts` (the same probe the skills run in BROWSER SETUP) and skip when the `aside` CLI or the Aside app is absent. CI runners have no Aside, so those run only on macOS dev machines and sit in the periodic tier; set `GSTACK_SKIP_ASIDE=1` to force the skip locally (which also exercises the -fallback hand-off). The `$B`-driven E2E cases and `browse/test/` run on every -platform as before, so Linux CI proves the fallback engine live. +fallback hand-off). The qa E2E files (`test/skill-e2e-qa-workflow.test.ts`, +`test/skill-e2e-qa-bugs.test.ts`) gate on `asideAvailable() || +fs.existsSync(browseBin)`: the skill's own BROWSER SETUP picks the engine, so +on a Mac they drive Aside and on Linux CI they drive the built browse binary, +skipping only when neither exists. The `$B`-driven E2E cases and `browse/test/` +run on every platform as before, so Linux CI proves the fallback engine live. **The renderer picks the same way, so the render gates are engine-agnostic.** `/make-pdf`, `/diagram`, and design previews print and screenshot their local @@ -66,9 +70,16 @@ paid) gate on `browserAvailable()` (`make-pdf/test/e2e/browser-available.ts`: `asideAvailable() || resolveBrowseBin() !== null`) — on a Mac they print through Aside, on Linux CI through the browse binary `bun run build:gates` compiles, and they skip only when neither exists. Only -`test/aside-render.test.ts`'s single live round-trip is Aside-only (its option -mapping and generated-script pins run everywhere). A green gate on Linux proves -the fallback engine, not Aside; the Mac run is the Aside evidence. +`test/aside-render.test.ts`'s two live Aside cases (a full round-trip and a +late-readiness `--wait-expr` poll) are Aside-only: its option +mapping and generated-script pins run everywhere, and its fake-executable cases +drive both engines hermetically (fake `aside` / `browse` scripts on PATH pin +probe classification, the stdout contract, loopback-server policy, the timeout +kill, engine choice and the mid-run fallback). `test/gstack-render-cli.test.ts` +does the same for `bin/gstack-render.ts` with `GSTACK_SKIP_ASIDE=1` and +`GSTACK_BROWSE_BIN` pointed at a fake daemon that logs every argv line. A green +gate on Linux proves the fallback engine, not Aside; the Mac run is the Aside +evidence. The browse-binary leg presumes Chromium bootstrapped: `resolveBrowseBin()` only checks that the binary (or the `find-browse` shim) exists, never that Chromium can launch, so on an install where the best-effort Chromium step @@ -155,8 +166,9 @@ in the test trees must carry a `timeout`, enforced by ratchet. **Anchor-sliced `setup` harnesses.** `setup` is one large bash script, so the -free tests that pin its linker, cleanup, and Chromium-bootstrap behavior never -run the whole thing. They slice the source by anchor (`extractFn(name)` takes +free tests that pin its linker, cleanup, retired-skill prune, browser hint, +rebuild decision, and Chromium-bootstrap behavior never run the whole thing. +They slice the source by anchor (`extractFn(name)` takes `name() {` through the next `\n}\n`; `test/setup-playwright-best-effort.test.ts` slices the `# 2. Ensure Playwright's Chromium is available` block up to `# 2b.`), join the extracted functions with stubbed collaborators, and execute @@ -167,7 +179,14 @@ testing nothing, and `test/setup-link-ownership.test.ts` and `test/setup-playwright-best-effort.test.ts` throw on any `command not found` on stderr as harness drift (a helper the test forgot to extract) rather than letting it degrade into a pass. Files: `test/setup-link-ownership.test.ts`, -`test/setup-cleanup-orphans.test.ts`, `test/setup-playwright-best-effort.test.ts`. +`test/setup-cleanup-orphans.test.ts`, `test/setup-playwright-best-effort.test.ts`, +`test/setup-prune-stale-generated.test.ts` (`_prune_stale_generated` against a +temp render tree plus host dirs: host cleanup after the generator already +pruned, symlink targets survive, frontmatter-renamed skills, foreign links), +`test/setup-browser-hint.test.ts` (`_browser_hint` and the bootstrap summary +across Aside present/absent, bootstrap ok/failed/skipped, `GSTACK_SKIP_ASIDE`), +and `test/setup-needs-build.test.ts` (the `NEEDS_BUILD` block sliced between +two anchors: every binary and source set flips it, Windows `.exe` suffixes). `test/relink.test.ts` shells out to a copy of the real `bin/gstack-relink` against a temp `GSTACK_INSTALL_DIR` / `GSTACK_SKILLS_DIR`, and `test/hook-scripts.test.ts` runs the real `careful/bin/check-careful.sh` and