v1.91.11.0: CHANGELOG, TODOS, docs and conventions for the refactor wave

This commit is contained in:
garrytan committed 2026-09-30 06:52:10 +00:00
1 parent 0778c70b91
commit c688d98ee5
8 files changed
+83 -49

No files matched your search

+5 -1
View File
@@ -270,5 +270,9 @@ bun run skill:check # health dashboard for all skills
- Run `bun run gen:skill-docs --host codex` to regenerate Codex-specific output.
- Browser steps in skills are `aside repl` scripts per `scripts/resolvers/aside.ts` (Aside first), each with a `$B` equivalent for the fallback engine — `$B <command>` is the browse binary and is a legitimate tool when the Aside probe does not print `READY`. Local HTML renders through `bin/gstack-render.ts`, which picks the same way.
- Safety skills (careful, freeze, guard) use inline advisory prose — always confirm before destructive operations.
- State paths resolve via `bin/gstack-paths` (sourced via `eval "$(...)"`). Honors `GSTACK_HOME`, `CLAUDE_PLUGIN_DATA`, `CLAUDE_PLANS_DIR`.
- State paths resolve through one chain owned by `lib/state-root.ts` and its sourced bash twin `bin/gstack-state-root.sh` (`GSTACK_STATE_ROOT` → `GSTACK_HOME` → `GSTACK_STATE_DIR` → gstack's `CLAUDE_PLUGIN_DATA` → `~/.gstack`; see docs/state-root.md). Skill prose uses `eval "$(bin/gstack-paths)"` with its `${GSTACK_STATE_ROOT:?…}` guard; `test/state-root-ratchet.test.ts` rejects hand-rolled chains.
- Browse daemon HTTP routes are entries in `browse/src/routes/table.ts` (its header shows how to add one); never dispatch on `url.pathname` in `server.ts`.
- Both test lanes run shards through `scripts/lib/shard-engine.ts`; the free and paid runners hold lane policy only. PTY harness code lives in `test/helpers/pty/*` behind the `claude-pty-runner.ts` barrel.
- Outside-voice failure prose (auth, timeout, empty, fallback) comes only from `outsideVoiceFailurePolicy()` in `scripts/resolvers/outside-voice.ts`.
- `test/module-size-ratchet.test.ts` keeps refactored owner modules at or under 800 lines (150 per function) and residual files from growing.
- The `claude` CLI binary resolves via `lib/claude-bin.ts` (re-exported from `browse/src/claude-bin.ts` for browse internals; `Bun.which()` + `GSTACK_CLAUDE_BIN` override). Set `GSTACK_CLAUDE_BIN=wsl` plus `GSTACK_CLAUDE_BIN_ARGS='["claude"]'` to run Claude through WSL on Windows.
+7 -7
View File
@@ -67,7 +67,7 @@ Node.js would work. Bun is better here for three reasons:
3. **Native TypeScript.** The server runs as `bun run server.ts` during development. No compilation step, no `ts-node`, no source maps to debug. The compiled binary is for deployment; source files are for development.
4. **Built-in HTTP server.** `Bun.serve()` is fast, simple, and doesn't need Express or Fastify. The server handles ~10 routes total. A framework would be overhead.
4. **Built-in HTTP server.** `Bun.serve()` is fast, simple, and doesn't need Express or Fastify. The server handles ~30 routes, declared in one route table (`browse/src/routes/table.ts`; its header shows how to add one). A framework would be overhead.
The bottleneck is always Chromium, not the CLI or server. Bun's startup speed (~1ms for the compiled binary vs ~100ms for Node) is nice but not the reason we chose it. The compiled binary and native SQLite are.
@@ -215,14 +215,14 @@ Page content harvested by CDP can contain lone UTF-16 surrogate halves (orphaned
| Egress path | Module | Sanitization point |
|---|---|---|
| `POST /command` (HTTP) | `browse/src/server.ts` | `handleCommandInternal` wrapper (sanitizes the result of `handleCommandInternalImpl`) |
| `POST /command/batch` | `browse/src/server.ts` | Same wrapper — batch consumers inherit it |
| `GET /activity/stream` (SSE) | `browse/src/server.ts` | `sanitizeReplacer` passed to `JSON.stringify` |
| `GET /inspector/events` (SSE) | `browse/src/server.ts` | `sanitizeReplacer` passed to `JSON.stringify` |
| `POST /command` (HTTP) | `browse/src/routes/commands.ts` (wrapper in `browse/src/server.ts`) | `handleCommandInternal` wrapper (sanitizes the result of `handleCommandInternalImpl`) |
| `POST /batch` | `browse/src/routes/commands.ts` | Same wrapper — batch consumers inherit it |
| `GET /activity/stream` (SSE) | `browse/src/routes/activity.ts` | `sanitizeReplacer` applied inside `createSseEndpoint` |
| `GET /inspector/events` (SSE) | `browse/src/routes/inspector.ts` | `sanitizeReplacer` applied inside `createSseEndpoint` |
`sanitizeReplacer` is a `JSON.stringify` replacer function that cleans every string value during encoding. Post-stringify regex doesn't work here — `JSON.stringify` has already converted `\uD800` into the literal escape sequence `"\\ud800"` before the regex could match, so the replacer must run inside the encoding pipeline. The pure-string helper `sanitizeLoneSurrogates` is used directly for `text/plain` responses.
**Architectural invariant.** Every new SSE/WebSocket writer or HTTP response that ships page-content-derived strings MUST go through one of two paths: `JSON.stringify(payload, sanitizeReplacer)` for object payloads, or `sanitizeLoneSurrogates(body)` for text bodies. New surfaces that bypass both will desync the system. Inline comments at both SSE producers in `server.ts` say so; `browse/test/server-sanitize-surrogates.test.ts` pins wiring with bug-repro + invariant tests (`handleCommandInternalImpl` rename, central sanitization line, replacer existence, SSE producers stringify with replacer).
**Architectural invariant.** Every new SSE/WebSocket writer or HTTP response that ships page-content-derived strings MUST go through one of two paths: `JSON.stringify(payload, sanitizeReplacer)` for object payloads, or `sanitizeLoneSurrogates(body)` for text bodies. New surfaces that bypass both will desync the system. Inline comments at both SSE producers (`routes/activity.ts`, `routes/inspector.ts`) say so; `browse/test/server-sanitize-surrogates.test.ts` pins wiring with bug-repro + invariant tests (`handleCommandInternalImpl` rename, central sanitization line, replacer existence, SSE producers stringify with replacer).
### Prompt injection defense (sidebar agent)
@@ -350,7 +350,7 @@ Templates contain the workflows, tips, and examples that require human judgment.
| `{{TEST_VALUE_BAR:<mode>}}` | `resolvers/test-value.ts` | Shared test value bar (authoring gate, value card, X/Y coverage, red-first proof, low-value catalog) for /qa and /qa-only (`qa`) and /test-audit (`audit`); /plan-eng-review and /ship embed it through the coverage audit |
| `{{TEST_VALUE_MESSAGE:<key>}}` | `resolvers/test-value.ts` | One degraded-mode message (problem, consequence, fix, docs anchor) from the shared constants |
| `{{TEST_BOOTSTRAP}}` | `resolvers/testing.ts` | Test framework detection, bootstrap, CI/CD setup for /ship and /design-review |
| `{{CODEX_PLAN_REVIEW}}` | `resolvers/review.ts` | Optional outside plan review for /plan-ceo-review and /plan-eng-review: Claude Code on Codex, Codex on other supported harnesses, with the caller's native subagent fallback |
| `{{CODEX_PLAN_REVIEW}}` | `resolvers/outside-voice-steps.ts` | Optional outside plan review for /plan-ceo-review and /plan-eng-review: Claude Code on Codex, Codex on other supported harnesses, with the caller's native subagent fallback |
| `{{DESIGN_SETUP}}` | `resolvers/design.ts` | Discovery pattern for `$D` design binary, mirrors `{{BROWSE_SETUP}}` |
| `{{DESIGN_DETECTOR}}` | `resolvers/design.ts` | Probe block + sentinel reading for the user-installed impeccable engine (`bin/gstack-design-detect.ts`); `:phase0` renders design-review's mechanical scan, `:gate` design-html's bounded slop gate |
| `{{DESIGN_MD_CHECK}}` | `resolvers/design.ts` | Open DESIGN.md format check through `bin/gstack-design-md.ts`, with the one-time conversion offer persisted in the file; `:calibrate` renders the tokens-as-calibration form for /design-review |
+35
View File
@@ -1,5 +1,40 @@
# Changelog
## [1.91.11.0] - 2026-09-30
gstack now looks up its state folder one way everywhere, and the five most copy-pasted or oversized parts of the codebase each have a single owner. Before, about 50 scripts, hooks and libraries each resolved the state folder with their own rule, and the rules disagreed. If you set `GSTACK_HOME`, `GSTACK_STATE_DIR` or `GSTACK_STATE_ROOT`, telemetry, analytics, update-check snoozes, the egress ledger and hook logs now all land in the folder you chose. Nothing is moved for you. Run `~/.claude/skills/gstack/bin/gstack-paths --explain` to see the active folder and whether `~/.gstack` still holds older state; [docs/state-root.md](docs/state-root.md) has the move recipe.
| Hotspot | Before | After |
| --- | ---: | ---: |
| Code files with a hand-rolled `${GSTACK_*:-…}` chain | 48 | 2 (the bash owner and one allowlisted partial-upgrade fallback) |
| `browse/src/server.ts` lines (`buildFetchHandler` alone) | 3,464 (1,560) | 2,224 (~350) |
| `test/helpers/claude-pty-runner.ts` lines | 5,047 | 30 (barrel over `test/helpers/pty/*`) |
| `scripts/resolvers/review.ts` lines | 1,921 | removed (5 modules, largest 771) |
| Shard spawn/kill/sandbox implementations | 2 | 1 (`scripts/lib/shard-engine.ts`) |
### Changed
- **One state-root rule.** Every script, hook and skill resolves state as `GSTACK_STATE_ROOT` → `GSTACK_HOME` → `GSTACK_STATE_DIR` → `CLAUDE_PLUGIN_DATA` (only for the gstack plugin) → `~/.gstack`. Skill bash blocks stop with a reinstall message if the resolver is missing, instead of writing under `/`.
- **Privacy opt-outs never get looser.** `telemetry`, `memorable_recall`, `codex_reviews`, `update_check` and trust-policy denies take the most restrictive value across the active folder and `~/.gstack`. `gstack-config set` says when another folder still overrides you and prints the command that fixes it, and `gstack-config list` shows which folder each merged key came from.
- **Uninstall deletes state only at `~/.gstack`.** `gstack-uninstall` refuses (exit 2) when that path resolves to `/`, your home, the gstack checkout or the current repository. For a relocated folder it leaves the folder in place and prints the exact removal command.
- **Outside-voice fallbacks read the same in every skill.** `/plan-devex-review` now also treats an "API key" error as an authentication failure, `/office-hours` names its fallback subagent like the other skills, and the `/review` and `/ship` adversarial pass says "timed out after 9 minutes" (a timed-out pass is still missing coverage).
- After a revert of this release, state written to a non-default folder while it was live stays in that folder.
### Fixed
- A pair-agent setup key that had not been exchanged yet was accepted as a bearer token on the browse daemon's `/command`, `/batch` and `/file`. A setup key now authenticates only the `/connect` exchange.
### For contributors
- **State root:** `lib/state-root.ts` (`resolveStateRoot`, `readConfigKey`) and its bash twin `bin/gstack-state-root.sh` (builtins only) own the chain. A parity table runs every row through both with `PATH` empty, Windows rows included. `test/state-root-ratchet.test.ts` rejects new hand-rolled chains, and `test-setup.ts` strips inherited `GSTACK_STATE_ROOT` / `GSTACK_STATE_DIR` so ambient variables cannot leak into tests. `hosts/claude/hooks/hook-log.ts` is the five hooks' one error-log writer (0600).
- **Browse routes:** the daemon's HTTP routes are one declared table (`browse/src/routes/table.ts`: method, path, auth kind, listener surface), with one auth gate, one denial per auth kind, and handlers in `browse/src/routes/*.ts`. A black-box matrix over every route, both listeners and five credential types passed on the old server and passes unchanged on the new one. The route tests now send real requests instead of grepping `server.ts`, and `browse/test/server-route-dispatch-ratchet.test.ts` keeps dispatch inside the table.
- **Shard engine:** `scripts/test-strict-output.ts` grew into `scripts/lib/shard-engine.ts` (process-group spawn, wall timeout and group kill, strict Bun verdicts, per-shard tmp and Chromium sandbox, logs, duration seeds, flag loop), and both runners use it. Lane policy stays per lane. A seven-outcome fixture corpus recorded from the old runners pins identical verdicts, and paid `--list` output is byte-identical. A timed-out free shard now stops reading at its deadline, as the paid lane already did.
- **PTY harness:** `test/helpers/claude-pty-runner.ts` is a barrel over `test/helpers/pty/*` (screen, launch, classify, auq, plan-native, boundaries, judge). One `runPtySession` loop drives the observation, counting and floor runners. A scripted fake PTY driver (`pty/fake-session.ts`) with an injectable clock runs each runner deterministically, and the unit test is split along the same modules. Tests keep importing the barrel.
- **Review resolvers:** `scripts/resolvers/review.ts` is split into `review-dashboard.ts`, `plan-gates.ts`, `spec-review.ts`, `outside-voice-steps.ts` and `review-scope.ts`. Generated output is byte-identical except the fallback wording above, which now comes from `outsideVoiceFailurePolicy()` in `outside-voice.ts`; `test/outside-voice-failure-policy.test.ts` rejects hand-written copies.
- **Ratchets:** `test/module-size-ratchet.test.ts` keeps the new owner modules at or under 800 lines and 150 lines per function, and stops the residual files (`server.ts`, both runners) from growing. `test/touchfiles.test.ts` checks that every moved module still selects the paid evals its source file selected (goldens in `test/fixtures/touchfile-moved-code/`, recorded before the move).
- **Budgets:** the guarded `gstack-paths` line in always-loaded preambles moved a few budgets to their measured values, each with its derivation recorded: carve-guards for ship (1.397 → 1.402), plan-ceo-review skeleton (80,150 → 80,700 bytes), plan-eng-review (1.169 → 1.174), design-consultation (1.08 → 1.083) and qa (1.095 → 1.102), and the `unfreeze` eager ceiling (393 → 448 tokens).
- **Deferred:** `TODOS.md` "P3: next refactor wave" lists the hotspots this wave did not touch and the behavior bugs it found and left alone.
## [1.91.9.0] - 2026-09-29
Every gstack workflow that proposes, writes, reviews or ships tests now applies one test value bar: a test earns its place by protecting behavior a real regression would break, and test count is not a goal. `/ship`'s coverage gate counts only tests that clear that bar, and the new `/test-audit` sweeps existing tests for ones that cost more than they protect.
+24 -33
View File
@@ -641,41 +641,32 @@ identity-based answer.
**Effort:** S. **Priority:** P3. **Depends on:** none.
### P3: one state-root rule for the bridge's four stores
### P3: next refactor wave (from the 2026-09 refactor wave survey)
**What:** The hook, `bin/gstack-config` and `bin/gstack-memorable` resolve
their root as `GSTACK_STATE_ROOT` > `GSTACK_HOME` > `GSTACK_STATE_DIR`; the
egress ledger (`lib/egress-receipt.ts`) honors `GSTACK_HOME` >
`GSTACK_STATE_DIR`; the trust-policy store (`lib/gbrain-repo-policy-client.ts`)
only `GSTACK_HOME`; `bin/gstack-uninstall` deletes only
`${GSTACK_STATE_DIR:-$HOME/.gstack}`. Extract one shared rule (a
`lib/state-root.ts` plus its bash twin) and use it everywhere.
**What:** Hotspots the 1.91.11.0 wave surveyed but did not refactor, plus
bugs it found and left alone because fixing them changes behavior:
- `bin/gstack-memory-ingest.ts` (2,674 lines; `ingestPass` is ~600 lines).
- `scripts/resolvers/design.ts` `generateDesignMethodology` (503 lines).
- `browse/src/browser-manager.ts` (2,143 lines) and `browse/src/cli.ts` (2,018 lines).
- `lib/cso/*` dense one-line style.
- Browse root-token denials disagree: some routes answer 401 `Unauthorized`,
others 403 `Root token required`. The route table's per-kind denial map
(`browse/src/routes/table.ts`) pins today's split.
- The sidebar's inspector live updates never arrive: `/inspector/events` is
`root-bearer` (it sat behind the old blanket root check), but
`extension/sidepanel.js` opens it with a cookie-only EventSource, and it
listens for `inspectResult` while the server emits `state` / `inspector`.
Fix both together, then flip the cookie rows in
`browse/test/server-route-auth-blackbox.test.ts`.
- Claude Code plugin-mode state (a pointer from `~/.gstack` to
`CLAUDE_PLUGIN_DATA`, plus merge, `--explain` and uninstall participation),
deferred by the W1 evidence gate: no official plugin distribution exists.
- The CSO native launchers (`lib/cso/launcher*.c`) pass only `HOME`,
`GSTACK_HOME` and `CLAUDE_PLUGIN_*` to the core, so `/cso` ignores an
exported `GSTACK_STATE_ROOT` / `GSTACK_STATE_DIR`. Needs a native rebuild.
- TODOS.md itself (4,500+ lines) needs restructuring.
**Why:** With `GSTACK_STATE_ROOT` set, the gate lives under one directory and
the receipts under another; the tests pin all three variables to one temp dir,
so the drift is invisible to them. Found by the /ship red team.
**Context:** Uninstall already flips `memorable_recall` off whenever it reads
`on`, kept state or not (through gstack-config's own resolution), so no config
can say `on` after the hook is gone; the remaining drift is observability, not
consent.
**Effort:** S (human ~3 h / CC+gstack ~20 min). **Priority:** P3.
**Depends on:** none.
### P3: shared hook logging helper
**What:** `stateRoot()` and the `hook-errors.log` appender now exist in five
hooks (`question-log`, `question-preference`, `auq-error-fallback`,
`timeline-stop`, `memorable-user-prompt`), with drifting env-var precedence.
Extract `hosts/claude/hooks/hook-log.ts` (root resolution, 0600 append, the
rate limiter the memorable hook added) and migrate the five.
**Why:** One place to fix precedence and file modes; the memorable hook's
rate limiter belongs to every hook that can fail on every prompt.
**Effort:** S (human ~2 h / CC+gstack ~15 min). **Priority:** P3.
**Depends on:** the state-root rule above.
**Effort:** M per item. **Priority:** P3.
## Aside integration follow-ups (filed via /plan-ceo-review + /plan-eng-review on the third-party-actions Aside plan)
+1 -1
View File
@@ -1 +1 @@
1.91.9.0
1.91.11.0
+1 -1
View File
@@ -1,4 +1,4 @@
# gstack digest v1.91.9.0 — regenerate/re-copy after upgrading gstack
# gstack digest v1.91.11.0 — regenerate/re-copy after upgrading gstack
Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed
for agent hosts without a full skill install. The full skills add workflows,
+9 -5
View File
@@ -10,12 +10,13 @@ gstack/
│ ├── SKILL.md.tmpl # /browse: Aside first ({{ASIDE_SETUP}} + cookbook), $B fallback
│ ├── src/ # CLI + server + commands
│ │ ├── commands.ts # Command registry (single source of truth)
│ │ ├── routes/ # Daemon HTTP route table (table.ts: entry shape, auth kinds, denials, dispatch) + handlers per area
│ │ └── snapshot.ts # SNAPSHOT_FLAGS metadata array
│ ├── test/ # Integration tests + fixtures
│ └── dist/ # Compiled binary
├── hosts/ # Typed host configs (one per AI agent)
│ ├── claude.ts # Primary host config
│ ├── claude/hooks/ # Claude Code lifecycle hooks (AUQ capture + enforcement, spawned-session directive, timeline stop, Memorable recall bridge (opt-in))
│ ├── claude/hooks/ # Claude Code lifecycle hooks (AUQ capture + enforcement, spawned-session directive, timeline stop, Memorable recall bridge (opt-in)); hook-log.ts is their one error-log writer
│ ├── codex.ts, factory.ts, kiro.ts # Existing hosts
│ ├── opencode.ts, slate.ts, cursor.ts, openclaw.ts # IDE hosts
│ ├── hermes.ts, gbrain.ts # Agent runtime hosts
@@ -25,15 +26,18 @@ gstack/
│ ├── gen-agents-digest.ts # Generates the budget-capped instruction-tier digest (agents-digest/)
│ ├── host-config.ts # HostConfig interface + validator
│ ├── host-config-export.ts # Shell bridge for setup script
│ ├── resolvers/ # Template resolver modules (preamble, aside = the Aside driver contract + research, browse = $B fallback setup + command reference, qa = surface-aware QA/exploration, sections = lazy loading, design, design-checklist = renders review/design-checklist.md from lib/design-catalog.ts, review, gbrain, etc.)
│ ├── resolvers/ # Template resolver modules (preamble, aside = the Aside driver contract + research, browse = $B fallback setup + command reference, qa = surface-aware QA/exploration, sections = lazy loading, design, design-checklist = renders review/design-checklist.md from lib/design-catalog.ts, review-dashboard / plan-gates / spec-review / review-scope, outside-voice = outside-voice primitives + the one failure policy, outside-voice-steps = second opinion / adversarial / plan and doc review, gbrain, etc.)
│ ├── lib/shard-engine.ts # Shared shard engine for both test lanes: spawn, group kill, strict verdicts, per-shard sandbox, logs, duration seeds, flags
│ ├── skill-check.ts # Health dashboard
│ ├── test-free-shards.ts # Strict parallel free-suite runner (GSTACK_FREE_JOBS, opt-in flaky retry)
│ ├── test-paid-shards.ts # Sharded paid-tier runner (one Bun process per shard)
│ ├── test-free-shards.ts # Free-lane policy on the shard engine (GSTACK_FREE_JOBS, opt-in flaky retry)
│ ├── test-paid-shards.ts # Paid-lane policy on the shard engine (one Bun process per shard)
│ ├── test-strict-output.ts # Compatibility re-export of lib/shard-engine.ts
│ ├── eval-flake-rank.ts # Flake-telemetry dial: ranks tests by retried passes across eval runs + the free-lane ledger
│ ├── sandbox-doctor.sh # One-command cloud-sandbox fixer: makes the free suite run green
│ └── dev-skill.ts # Watch mode
├── lib/state-root.ts # State-root owner (resolveStateRoot, readConfigKey); bin/gstack-state-root.sh is its sourced bash twin; docs/state-root.md
├── test/ # Skill validation + eval tests
│ ├── helpers/ # skill-parser.ts, session-runner.ts, llm-judge.ts, eval-store.ts, aside-available.ts (Aside self-skip probe)
│ ├── helpers/ # skill-parser.ts, session-runner.ts, llm-judge.ts, eval-store.ts, aside-available.ts (Aside self-skip probe); pty/ = the PTY harness (session.ts owns the runner loop, fake-session.ts the scripted test driver), imported through the claude-pty-runner.ts barrel
│ ├── fixtures/ # Ground truth JSON, planted-bug fixtures, eval baselines, impeccable engine captures (impeccable-*.json, the dumped slop page, fake-impeccable.ts shim)
│ ├── aside-driver.test.ts # Tier 1: pins the {{ASIDE_SETUP}} contract sentences + the fallback hand-off
│ ├── aside-render.test.ts # Tier 1 pins + fake-executable runs on both engines + a live Aside render (self-skips without Aside)
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "gstack",
"version": "1.91.9",
"version": "1.91.11",
"description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.",
"license": "MIT",
"type": "module",