CalvinBackup/gstack
1
0
Fork 0
mirror of https://github.com/garrytan/gstack.git synced 2026-09-15 01:15:29 +02:00
Code Issues Packages Projects Releases Wiki Activity

docs: sync project docs with the v1.81.0.0 review fixes

Browse Source 
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 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-09-06 07:42:57 +00:00
co-authored by Claude Fable 5.1
parent 01dbd56b09
commit 97fc880869
7 changed files with 138 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CONTRIBUTING.md
+26 -16
View File
@@ -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 <path>` 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 <N>` 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 <path>` 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 <path>` 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/<ts>/` 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=<seconds>` (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/<ts>/` 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=<seconds>` (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