From 39e0148bddc584f7bd9f6311ae77763990115c39 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 2 Nov 2026 01:16:58 -0800 Subject: [PATCH 1/5] v1.92.67.0 fix(setup): read skill names without a process per skill (#2120) (#3752) * fix(setup): read skill names without a process per skill (#2120) On Windows on ARM64, x64-emulated Git Bash intermittently deadlocks in MSYS2's fork() emulation, and ./setup hung in the loops that read every SKILL.md name through grep | sed | tr plus a basename (about four process starts per skill). A sourced helper, bin/gstack-skill-name.sh, reads the name with a bash read loop and no subshell; gstack-patch-names, setup's link and cleanup loops and gstack-uninstall's inventory use it and parameter expansion instead of basename. Codex links keep their directory names. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: fixtures that copy gstack-patch-names also copy its sourced helper Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: linker harnesses source the skill-name helper Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * changelog: fragment for fork-free skill names (#2120) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * changelog: name the host linkers that still run basename Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test(qa-lazy-sections): the setup-install harness sources the skill-name helper Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 8 ++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- bin/gstack-patch-names | 6 +- bin/gstack-skill-name.sh | 21 +++ bin/gstack-uninstall | 7 +- package.json | 2 +- setup | 13 +- test/plan-mode-preamble-render.test.ts | 2 +- test/qa-lazy-sections.test.ts | 3 +- test/relink-disabled-router.test.ts | 2 +- test/relink.test.ts | 1 + test/render-sections-provenance.test.ts | 4 +- test/setup-alias-name-uniqueness.test.ts | 1 + test/setup-claude-code-migration.test.ts | 2 +- test/setup-claude-skill-assets.test.ts | 1 + test/setup-fork-free-skill-names.test.ts | 150 ++++++++++++++++++++++ test/setup-link-ownership.test.ts | 1 + test/setup-playwright-best-effort.test.ts | 1 + test/team-mode.test.ts | 2 +- test/user-render-out-dir-install.test.ts | 1 + 21 files changed, 212 insertions(+), 20 deletions(-) create mode 100644 bin/gstack-skill-name.sh create mode 100644 test/setup-fork-free-skill-names.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b6a4528ba..6cff9b4b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## [1.92.67.0] - 2026-11-02 + +**`./setup` no longer starts a handful of processes per skill just to read its name (#2120).** On Windows 11 on ARM64, Git Bash runs under x64 emulation, and its `fork()` can deadlock when a script starts hundreds of short-lived processes in a row. Setup did exactly that: `gstack-patch-names` and setup's skill-linking loops ran `grep | sed | tr` and `basename` for every skill. The reporter counted about 220 process starts on a full install. Thanks to @habassa5, who traced the hang to those loops. + +### Fixed +- **Skill names are read with bash builtins.** A new sourced helper, `bin/gstack-skill-name.sh`, reads the `name:` line with a `read` loop. It gives the same result as the old pipeline on CRLF files, files with no final newline, and files with no `name:` line. `gstack-patch-names`, setup's Claude skill linker, its two cleanup loops and `gstack-uninstall` use it. They use parameter expansion instead of `basename`. `sed` and `mv` run only for a skill that is actually renamed. +- **What is not fixed yet.** Setup still starts `ln`, `mkdir` and `cp` per skill, and the linkers for Cursor, OpenCode, Copilot, Factory, Agy and Kiro still run `basename` per skill. Nobody has confirmed on a Windows ARM64 machine that the hang is gone. + ## [1.92.66.0] - 2026-11-02 **The secrets check reads Python and Dart string context from what git diffed, and a quote inside an interpolation no longer ends a string (GST-738).** env.kv's source-syntax exemption treats a `password:` line as code only when it sits outside every string. Two inputs could make the scanner place a committed string's contents outside a string. One was a working copy that no longer matched what git diffed. The other was a quote inside an f-string or Dart `${…}` expression. A scoped review pass now applies an explicit `--diff-base` to its Red Team as well. diff --git a/VERSION b/VERSION index b0d5ebd0b..61c63e5b8 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.92.66.0 +1.92.67.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 01116c9ae..4bcbb97c3 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.92.66.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.92.67.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, diff --git a/bin/gstack-patch-names b/bin/gstack-patch-names index bef02aae4..d86884176 100755 --- a/bin/gstack-patch-names +++ b/bin/gstack-patch-names @@ -8,13 +8,15 @@ DO_PREFIX="$2" # Normalize prefix arg case "$DO_PREFIX" in true|1) DO_PREFIX=1 ;; *) DO_PREFIX=0 ;; esac +# Names are read with a bash builtin loop: no process per skill (#2120). +. "$(dirname "$0")/gstack-skill-name.sh" PATCHED=0 for skill_dir in "$GSTACK_DIR"/*/; do [ -f "$skill_dir/SKILL.md" ] || continue - dir_name="$(basename "$skill_dir")" + dir_name=${skill_dir%/}; dir_name=${dir_name##*/} [ "$dir_name" = "node_modules" ] && continue - cur=$(grep -m1 '^name:' "$skill_dir/SKILL.md" 2>/dev/null | sed 's/^name:[[:space:]]*//' | tr -d '[:space:]' || true) + gstack_skill_md_name "$skill_dir/SKILL.md"; cur=$GSTACK_SKILL_NAME [ -z "$cur" ] && continue [ "$cur" = "gstack" ] && continue # never prefix root skill if [ "$DO_PREFIX" -eq 1 ]; then diff --git a/bin/gstack-skill-name.sh b/bin/gstack-skill-name.sh new file mode 100644 index 000000000..01ca61d23 --- /dev/null +++ b/bin/gstack-skill-name.sh @@ -0,0 +1,21 @@ +# shellcheck shell=bash +# gstack-skill-name.sh: sourced helper (bash 3.2). gstack_skill_md_name FILE +# sets GSTACK_SKILL_NAME to the first column-0 "name:" line of FILE with every +# whitespace character removed (CR included), or to "" when there is none or +# FILE cannot be read. Always returns 0. +# +# It starts no process: no subshell, pipe or command substitution, and callers +# must not wrap it in $( ). Setup reads ~55 names per run, and the +# grep | sed | tr pipeline it replaces deadlocked MSYS2's fork() under x64 +# emulation on Windows ARM64 (#2120). +gstack_skill_md_name() { + GSTACK_SKILL_NAME= + [ -f "$1" ] && [ -r "$1" ] || return 0 + local _gsn_line + while IFS= read -r _gsn_line || [ -n "$_gsn_line" ]; do + case "$_gsn_line" in + name:*) _gsn_line=${_gsn_line#name:}; GSTACK_SKILL_NAME=${_gsn_line//[[:space:]]/}; return 0 ;; + esac + done < "$1" + return 0 +} diff --git a/bin/gstack-uninstall b/bin/gstack-uninstall index c88a53568..6e561cf1d 100755 --- a/bin/gstack-uninstall +++ b/bin/gstack-uninstall @@ -60,6 +60,9 @@ GSTACK_DIR="${GSTACK_DIR:-$(cd "$(dirname "$0")/.." && pwd)}" STATE_DIR="$HOME/.gstack" _STATE_DOC="https://github.com/garrytan/gstack/blob/main/docs/state-root.md" . "$(dirname "$0")/gstack-state-root.sh" 2>/dev/null || { echo "$0: cannot resolve the gstack state root: $(dirname "$0")/gstack-state-root.sh is missing. fix: reinstall with ./setup or /gstack-upgrade ($_STATE_DOC)" >&2; exit 1; } +# Skill names without a process per skill (#2120); an older copy lacking the helper keeps the pipeline. +. "$(dirname "$0")/gstack-skill-name.sh" 2>/dev/null \ + || gstack_skill_md_name() { GSTACK_SKILL_NAME=$(grep -m1 '^name:' "$1" 2>/dev/null | sed 's/^name:[[:space:]]*//' | tr -d '[:space:]' || true); } gstack_state_root_select RESOLVED_ROOT="$_gstack_sr_root" RESOLVED_VAR="$_gstack_sr_var" @@ -290,8 +293,8 @@ _INVENTORY=" _gstack-command connect-chrome gstack-connect-chrome " if [ -d "$GSTACK_DIR" ]; then for _SRC in "$GSTACK_DIR"/*/; do [ -f "$_SRC/SKILL.md" ] || continue - _SRC_NAME="$(basename "$_SRC")" - _FM_NAME=$(grep -m1 '^name:' "$_SRC/SKILL.md" 2>/dev/null | sed 's/^name:[[:space:]]*//' | tr -d '[:space:]' || true) + _SRC_NAME=${_SRC%/}; _SRC_NAME=${_SRC_NAME##*/} + gstack_skill_md_name "$_SRC/SKILL.md"; _FM_NAME=$GSTACK_SKILL_NAME for _N in "$_SRC_NAME" "$_FM_NAME"; do [ -n "$_N" ] || continue case "$_INVENTORY" in *" $_N "*) ;; *) _INVENTORY="$_INVENTORY$_N gstack-$_N " ;; esac diff --git a/package.json b/package.json index 48bb0444d..ba562a6cd 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.92.66", + "version": "1.92.67", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/setup b/setup index f4d247792..649c6fd5e 100755 --- a/setup +++ b/setup @@ -298,6 +298,7 @@ SOURCE_GSTACK_DIR="$(cd "$(dirname "$0")" && pwd -P)" . "$SOURCE_GSTACK_DIR/bin/gstack-state-root.sh" 2>/dev/null || { echo "$0: cannot resolve the gstack state root: $SOURCE_GSTACK_DIR/bin/gstack-state-root.sh is missing. fix: reinstall with ./setup or /gstack-upgrade (docs/state-root.md)" >&2; exit 1; } gstack_state_root_select; GSTACK_STATE_ROOT="$_gstack_sr_root" . "$SOURCE_GSTACK_DIR/bin/gstack-path-spelling.sh" || { echo "$0: $SOURCE_GSTACK_DIR/bin/gstack-path-spelling.sh is missing. fix: reinstall from a fresh clone" >&2; exit 1; } +. "$SOURCE_GSTACK_DIR/bin/gstack-skill-name.sh" || { echo "$0: $SOURCE_GSTACK_DIR/bin/gstack-skill-name.sh is missing. fix: reinstall from a fresh clone" >&2; exit 1; } . "$SOURCE_GSTACK_DIR/bin/gstack-claude-root.sh" || { echo "$0: $SOURCE_GSTACK_DIR/bin/gstack-claude-root.sh is missing. fix: reinstall from a fresh clone" >&2; exit 1; } . "$SOURCE_GSTACK_DIR/bin/gstack-install-registry.sh" || { echo "$0: $SOURCE_GSTACK_DIR/bin/gstack-install-registry.sh is missing. fix: reinstall from a fresh clone (docs/ADDING_A_HOST.md)" >&2; exit 1; } . "$SOURCE_GSTACK_DIR/bin/gstack-render-claude.sh" || { echo "$0: $SOURCE_GSTACK_DIR/bin/gstack-render-claude.sh is missing. fix: reinstall from a fresh clone" >&2; exit 1; } @@ -2578,11 +2579,11 @@ link_claude_skill_dirs() { local linked=() for skill_dir in "$gstack_dir"/*/; do if [ -f "$skill_dir/SKILL.md" ]; then - dir_name="$(basename "$skill_dir")" + dir_name=${skill_dir%/}; dir_name=${dir_name##*/} # Skip node_modules [ "$dir_name" = "node_modules" ] && continue # Use frontmatter name: if present (e.g., run-tests/ with name: test → symlink as "test") - skill_name=$(grep -m1 '^name:' "$skill_dir/SKILL.md" 2>/dev/null | sed 's/^name:[[:space:]]*//' | tr -d '[:space:]') + gstack_skill_md_name "$skill_dir/SKILL.md"; skill_name=$GSTACK_SKILL_NAME [ -z "$skill_name" ] && skill_name="$dir_name" case "${_DISABLED_SKILLS:- }" in *" $skill_name "*|*" $dir_name "*) continue ;; esac # Apply gstack- prefix unless --no-prefix or already prefixed @@ -2748,7 +2749,7 @@ cleanup_old_claude_symlinks() { # literal (empty skills_dir) is rejected by the same guard. for old_target in "$skills_dir"/*; do [ -e "$old_target" ] || [ -L "$old_target" ] || continue - skill_name="$(basename "$old_target")" + skill_name=${old_target##*/} [ "$skill_name" = "node_modules" ] && continue [ "$skill_name" = "gstack" ] && continue # Skip already-prefixed dirs (gstack-upgrade) — no old symlink to clean @@ -2788,7 +2789,7 @@ cleanup_old_claude_symlinks() { if [ "${IS_WINDOWS:-0}" -eq 1 ] && [ -d "$gstack_dir" ]; then for skill_dir in "$gstack_dir"/*/; do if [ -f "$skill_dir/SKILL.md" ]; then - skill_name="$(basename "$skill_dir")" + skill_name=${skill_dir%/}; skill_name=${skill_name##*/} [ "$skill_name" = "node_modules" ] && continue case "$skill_name" in gstack-*) continue ;; esac old_target="$skills_dir/$skill_name" @@ -2819,7 +2820,7 @@ cleanup_prefixed_claude_symlinks() { local removed=() for skill_dir in "$gstack_dir"/*/; do if [ -f "$skill_dir/SKILL.md" ]; then - skill_name="$(basename "$skill_dir")" + skill_name=${skill_dir%/}; skill_name=${skill_name##*/} [ "$skill_name" = "node_modules" ] && continue # Only clean up prefixed entries for dirs that AREN'T already prefixed # (e.g., remove gstack-qa but NOT gstack-upgrade which is the real dir name) @@ -2884,7 +2885,7 @@ link_codex_skill_dirs() { for skill_dir in "$agents_dir"/gstack*/; do if [ -f "$skill_dir/SKILL.md" ]; then - skill_name="$(basename "$skill_dir")" + skill_name=${skill_dir%/}; skill_name=${skill_name##*/} case "${_DISABLED_SKILLS:- }" in *" ${skill_name#gstack-} "*) _remove_disabled_host_entry "$skills_dir/$skill_name"; continue ;; esac # Skip the sidecar directory — it contains runtime asset symlinks (bin/, # browse/), not a skill. Linking it would overwrite the root gstack diff --git a/test/plan-mode-preamble-render.test.ts b/test/plan-mode-preamble-render.test.ts index 60d10c400..101926721 100644 --- a/test/plan-mode-preamble-render.test.ts +++ b/test/plan-mode-preamble-render.test.ts @@ -62,7 +62,7 @@ describe('every installed-skill render carries the plan-mode arg', () => { }); describe('gstack-relink router re-render', () => { - const BINS = ['gstack-config', 'gstack-relink', 'gstack-patch-names', 'gstack-state-root.sh', 'gstack-install-registry.sh']; + const BINS = ['gstack-config', 'gstack-relink', 'gstack-patch-names', 'gstack-skill-name.sh', 'gstack-state-root.sh', 'gstack-install-registry.sh']; const SAFE = '## Plan Mode Safe Operations'; function relinkFixture() { diff --git a/test/qa-lazy-sections.test.ts b/test/qa-lazy-sections.test.ts index e914dbac5..cc57bcfd2 100644 --- a/test/qa-lazy-sections.test.ts +++ b/test/qa-lazy-sections.test.ts @@ -570,7 +570,8 @@ describe('installed QA pointers', () => { 'set -e', `IS_WINDOWS=${copy ? 1 : 0}`, `SKILL_PREFIX=${prefix}`, 'QUIET=1', '_FOREIGN_SKIPPED_ENTRIES=()', '_BACKED_UP_SKILL_MDS=()', '_SKILL_BACKUP_ROOT="$HOME/backups"', 'GSTACK_USER_RENDER_DIR="$HOME/absent-render"', - 'GSTACK_STATE_ROOT="$HOME/.gstack"', '_SKILL_COPIES_FILE="$GSTACK_STATE_ROOT/skill-copies.tsv"', helpers, + 'GSTACK_STATE_ROOT="$HOME/.gstack"', '_SKILL_COPIES_FILE="$GSTACK_STATE_ROOT/skill-copies.tsv"', + `. "${path.join(ROOT, 'bin/gstack-skill-name.sh')}"`, helpers, 'log() { :; }', '_browser_hint() { :; }', 'bun_cmd() { :; }', install, ].join('\n'); const runInstall = () => runBashScript(script, { diff --git a/test/relink-disabled-router.test.ts b/test/relink-disabled-router.test.ts index 4ff263cd7..4268c9a6a 100644 --- a/test/relink-disabled-router.test.ts +++ b/test/relink-disabled-router.test.ts @@ -12,7 +12,7 @@ import * as os from 'os'; import * as path from 'path'; const ROOT = path.resolve(import.meta.dir, '..'); -const BINS = ['gstack-config', 'gstack-relink', 'gstack-patch-names', 'gstack-state-root.sh', 'gstack-install-registry.sh']; +const BINS = ['gstack-config', 'gstack-relink', 'gstack-patch-names', 'gstack-skill-name.sh', 'gstack-state-root.sh', 'gstack-install-registry.sh']; let tmp: string; let install: string; diff --git a/test/relink.test.ts b/test/relink.test.ts index 87dfa51d9..b054cd274 100644 --- a/test/relink.test.ts +++ b/test/relink.test.ts @@ -58,6 +58,7 @@ function setupMockInstall(skills: string[]): void { fs.chmodSync(path.join(mockBin, 'gstack-config'), 0o755); // A real install always ships the state-root twin beside the bins that source it. fs.copyFileSync(path.join(BIN, 'gstack-state-root.sh'), path.join(mockBin, 'gstack-state-root.sh')); + fs.copyFileSync(path.join(BIN, 'gstack-skill-name.sh'), path.join(mockBin, 'gstack-skill-name.sh')); if (fs.existsSync(path.join(BIN, 'gstack-relink'))) { fs.copyFileSync(path.join(BIN, 'gstack-relink'), path.join(mockBin, 'gstack-relink')); fs.chmodSync(path.join(mockBin, 'gstack-relink'), 0o755); diff --git a/test/render-sections-provenance.test.ts b/test/render-sections-provenance.test.ts index 2573c0bcd..ee2c4e4d1 100644 --- a/test/render-sections-provenance.test.ts +++ b/test/render-sections-provenance.test.ts @@ -77,7 +77,7 @@ describe('installed skills serve rendered sections (F5, #2706)', () => { test('gstack-relink (the gbrain-refresh path) serves the render, and the checkout again once the render is gone', () => { const fx = fixture(); fs.mkdirSync(path.join(fx.install, 'bin'), { recursive: true }); - for (const f of ['gstack-relink', 'gstack-config', 'gstack-state-root.sh', 'gstack-install-registry.sh', 'gstack-patch-names']) { + for (const f of ['gstack-relink', 'gstack-config', 'gstack-state-root.sh', 'gstack-install-registry.sh', 'gstack-patch-names', 'gstack-skill-name.sh']) { fs.copyFileSync(path.join(BIN, f), path.join(fx.install, 'bin', f)); fs.chmodSync(path.join(fx.install, 'bin', f), 0o755); } @@ -106,7 +106,7 @@ describe('installed skills serve rendered sections (F5, #2706)', () => { test('a sections/ directory the user owns is left alone by relink', () => { const fx = fixture(); fs.mkdirSync(path.join(fx.install, 'bin'), { recursive: true }); - for (const f of ['gstack-relink', 'gstack-config', 'gstack-state-root.sh', 'gstack-install-registry.sh', 'gstack-patch-names']) { + for (const f of ['gstack-relink', 'gstack-config', 'gstack-state-root.sh', 'gstack-install-registry.sh', 'gstack-patch-names', 'gstack-skill-name.sh']) { fs.copyFileSync(path.join(BIN, f), path.join(fx.install, 'bin', f)); fs.chmodSync(path.join(fx.install, 'bin', f), 0o755); } diff --git a/test/setup-alias-name-uniqueness.test.ts b/test/setup-alias-name-uniqueness.test.ts index eeca36123..b2a772ba4 100644 --- a/test/setup-alias-name-uniqueness.test.ts +++ b/test/setup-alias-name-uniqueness.test.ts @@ -80,6 +80,7 @@ beforeAll(() => { extractFn('_write_owned_marker'), extractFn('_print_windows_copy_note_once'), extractFn('_link_skill_runtime_assets'), + `. "${path.join(ROOT, 'bin/gstack-skill-name.sh')}"`, extractFn('link_claude_skill_dirs'), extractFn('_install_alias_skill_md'), extractFn('link_claude_root_skill_alias'), diff --git a/test/setup-claude-code-migration.test.ts b/test/setup-claude-code-migration.test.ts index cbc4ad113..85ebe5e9f 100644 --- a/test/setup-claude-code-migration.test.ts +++ b/test/setup-claude-code-migration.test.ts @@ -31,7 +31,7 @@ function fixture(copy: boolean) { put(path.join(root, 'VERSION'), '2.0.0.0\n'); put(path.join(root, 'ETHOS.md'), 'Fixture ethos\n'); put(path.join(root, 'package.json'), JSON.stringify({ type: 'module', scripts: { 'gen:skill-docs': 'bun run scripts/gen-skill-docs.ts' } })); - for (const bin of ['gstack-config', 'gstack-patch-names', 'gstack-relink', 'gstack-migrate-claude-code', 'gstack-claude-code', 'gstack-models', 'gstack-state-root.sh', 'gstack-bun-version.sh', 'gstack-path-spelling.sh', 'gstack-claude-root.sh', 'gstack-install-registry.sh', 'gstack-render-claude.sh', 'gstack-host-renders.sh']) { + for (const bin of ['gstack-config', 'gstack-patch-names', 'gstack-relink', 'gstack-migrate-claude-code', 'gstack-claude-code', 'gstack-models', 'gstack-state-root.sh', 'gstack-bun-version.sh', 'gstack-path-spelling.sh', 'gstack-skill-name.sh', 'gstack-claude-root.sh', 'gstack-install-registry.sh', 'gstack-render-claude.sh', 'gstack-host-renders.sh']) { const target = path.join(root, 'bin', bin); fs.mkdirSync(path.dirname(target), { recursive: true }); fs.copyFileSync(path.join(ROOT, 'bin', bin), target); diff --git a/test/setup-claude-skill-assets.test.ts b/test/setup-claude-skill-assets.test.ts index d6cffb8a1..ca95ddc9d 100644 --- a/test/setup-claude-skill-assets.test.ts +++ b/test/setup-claude-skill-assets.test.ts @@ -82,6 +82,7 @@ beforeAll(() => { '_FOREIGN_SKIPPED_ENTRIES=()', '_BACKED_UP_SKILL_MDS=()', `_SKILL_BACKUP_ROOT="${os.tmpdir()}/gstack-harness-backups-${process.pid}"`, + `. "${path.join(ROOT, 'bin/gstack-skill-name.sh')}"`, extractFn('link_claude_skill_dirs'), `link_claude_skill_dirs "${ROOT}" "${installDir}"`, ].join('\n'); diff --git a/test/setup-fork-free-skill-names.test.ts b/test/setup-fork-free-skill-names.test.ts new file mode 100644 index 000000000..a42168f6d --- /dev/null +++ b/test/setup-fork-free-skill-names.test.ts @@ -0,0 +1,150 @@ +/** + * #2120: on Windows on ARM64, x64-emulated Git Bash intermittently deadlocks + * in MSYS2's fork() emulation, and ./setup hung in the per-skill loops that + * read each SKILL.md name with a grep | sed | tr pipeline plus a basename + * (about four process starts per skill, ~55 skills a run). The deadlock + * itself needs that host; these cases pin the cause they can see anywhere: + * the loops start none of those processes per skill. Every grep, sed, tr and + * basename the code under test starts goes through a logging shim on PATH. + */ +import { describe, expect, test } from 'bun:test'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { runBashScript } from './helpers/bash-script'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const SETUP_SRC = fs.readFileSync(path.join(ROOT, 'setup'), 'utf-8'); +const HELPER = path.join(ROOT, 'bin/gstack-skill-name.sh'); +const SHIMMED = ['grep', 'sed', 'tr', 'basename']; +const SKILLS = ['qa', 'review', 'ship', 'office-hours', 'investigate', 'land-and-deploy', 'retro', 'cso']; + +function extractFn(name: string): string { + const start = SETUP_SRC.indexOf(`\n${name}() {`); + const end = SETUP_SRC.indexOf('\n}\n', start); + if (start < 0 || end < 0) throw new Error(`function not found: ${name}`); + return SETUP_SRC.slice(start + 1, end + 2); +} + +/** A temp tree, a PATH whose grep/sed/tr/basename log each start to calls.log, and a reader for that log. */ +function sandbox() { + const tmp = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-fork-free-'))); + const shim = path.join(tmp, 'shim'); + const log = path.join(tmp, 'calls.log'); + fs.mkdirSync(shim); + for (const name of SHIMMED) { + const real = spawnSync('bash', ['-c', `type -P ${name}`], { encoding: 'utf8', timeout: 10_000 }).stdout.trim(); + fs.writeFileSync(path.join(shim, name), `#!/bin/sh\necho ${name} >> "${log}"\nexec "${real}" "$@"\n`, { mode: 0o755 }); + } + return { + tmp, env: { PATH: `${shim}${path.delimiter}${process.env.PATH ?? ''}`, HOME: tmp, GSTACK_USER_RENDER_DIR: path.join(tmp, 'no-render') }, + calls: () => fs.existsSync(log) ? fs.readFileSync(log, 'utf8').split('\n').filter(Boolean) : [], + reset: () => fs.rmSync(log, { force: true }), + cleanup: () => fs.rmSync(tmp, { recursive: true, force: true }), + }; +} + +const skillMd = (name: string, eol = '\n') => ['---', `name: ${name}`, 'description: fixture', '---', `# ${name}`, ''].join(eol); + +/** A payload of SKILLS (qa in CRLF, office-hours named "oh"), then "zz-nameless" with no name: line after them. */ +function payload(dir: string) { + for (const name of SKILLS) { + fs.mkdirSync(path.join(dir, name), { recursive: true }); + fs.writeFileSync(path.join(dir, name, 'SKILL.md'), skillMd(name === 'office-hours' ? 'oh' : name, name === 'qa' ? '\r\n' : '\n')); + } + fs.mkdirSync(path.join(dir, 'zz-nameless'), { recursive: true }); + fs.writeFileSync(path.join(dir, 'zz-nameless', 'SKILL.md'), '---\ndescription: no name\n---\n# nameless\n'); +} + +describe.skipIf(process.platform === 'win32')('setup name reads start no process per skill (#2120)', () => { + // Value: protects=the helper returns what the old grep -m1 | sed | tr pipeline returned, and never a stale name; fails_when=it keeps the + // previous value, reads a non-column-0 name, keeps a CR or space, or returns non-zero; why_new=the helper is new; seam=none + test('gstack_skill_md_name matches the old pipeline on odd inputs, resets between files and always returns 0', () => { + const s = sandbox(); + try { + const cases: Record = { + plain: '---\nname: qa\n---\n', crlf: '---\r\nname: qa\r\n---\r\n', spaced: 'name: my skill \t\n', noeol: 'name: last', + indented: ' name: no\nname: yes\n', nameless: '---\ndescription: x\n---\n', empty: '', later: '# t\nbody\nname: deep\nname: second\n', + }; + const files = Object.entries(cases).map(([k, v]) => { const f = path.join(s.tmp, k); fs.writeFileSync(f, v); return f; }); + files.push(path.join(s.tmp, 'missing')); + const old = (f: string) => spawnSync('bash', ['-c', "grep -m1 '^name:' \"$1\" 2>/dev/null | sed 's/^name:[[:space:]]*//' | tr -d '[:space:]'", '_', f], { encoding: 'utf8', timeout: 10_000 }).stdout; + const script = [`set -euo pipefail; . "${HELPER}"; GSTACK_SKILL_NAME=stale`, + ...files.map(f => `gstack_skill_md_name "${f}"; printf '%s\n' "[$GSTACK_SKILL_NAME]"`)].join('\n'); + const r = runBashScript(script, { timeout: 10_000, env: s.env }); + expect(r.status, r.stderr ?? '').toBe(0); + expect(r.stdout.trim().split('\n')).toEqual(files.map(f => `[${old(f)}]`)); + expect(s.calls()).toEqual([]); + } finally { + s.cleanup(); + } + }); + + // Value: protects=no caller re-adds a fork per skill by wrapping the helper in $( ), and the helper itself stays fork-free; + // fails_when=a call site uses $(gstack_skill_md_name ...) or the helper body gains $(, a pipe or a backquote; why_new=the shims cannot see a subshell fork; seam=none + test('the helper starts no subshell and no caller wraps it in $( )', () => { + const body = fs.readFileSync(HELPER, 'utf8').split('\n').filter(l => !/^\s*#/.test(l)).join('\n'); + expect(body).not.toMatch(/\$\(|`|(^|[^|])\|([^|]|$)/m); + for (const rel of ['setup', 'bin/gstack-patch-names', 'bin/gstack-uninstall']) { + expect(fs.readFileSync(path.join(ROOT, rel), 'utf8'), rel).not.toMatch(/\$\(\s*gstack_skill_md_name/); + } + }); + + // Value: protects=gstack-patch-names reads every name without a per-skill grep|sed|tr|basename (the first loop #2120 hung in); + // fails_when=the loop goes back to a process pipeline per SKILL.md; why_new=no case counted process starts; seam=PATH shims + test('gstack-patch-names: no process per skill when nothing needs a rename; a rename starts only its own sed', () => { + const s = sandbox(); + try { + const dir = path.join(s.tmp, 'gstack'); + payload(dir); + const off = spawnSync('bash', [path.join(ROOT, 'bin/gstack-patch-names'), dir, '0'], { encoding: 'utf8', env: s.env, timeout: 30_000 }); + expect(off.status, off.stderr).toBe(0); + expect(off.stdout).toBe(''); + expect(s.calls()).toEqual([]); + + const on = spawnSync('bash', [path.join(ROOT, 'bin/gstack-patch-names'), dir, '1'], { encoding: 'utf8', env: s.env, timeout: 30_000 }); + expect(on.status, on.stderr).toBe(0); + expect(on.stdout).toContain(`patched name: field in ${SKILLS.length} skills`); + for (const name of SKILLS) expect(fs.readFileSync(path.join(dir, name, 'SKILL.md'), 'utf8')).toContain(`name: gstack-${name === 'office-hours' ? 'oh' : name}`); + expect(fs.readFileSync(path.join(dir, 'zz-nameless', 'SKILL.md'), 'utf8')).not.toContain('name:'); + expect(s.calls()).toEqual(Array(SKILLS.length).fill('sed')); + } finally { + s.cleanup(); + } + }); + + // Value: protects=link_claude_skill_dirs and both cleanup loops run with no per-skill grep, sed, tr or basename, and a nameless SKILL.md links + // under its directory name, never the previous skill's name (#2120); fails_when=a loop goes back to a pipeline or basename, or the name leaks; + // why_new=link tests checked links, not process starts; seam=PATH shims + test('link_claude_skill_dirs and the two cleanup loops: no per-skill grep, sed, tr or basename; names come from frontmatter', () => { + const s = sandbox(); + try { + const pl = path.join(s.tmp, 'skills', 'gstack'); + payload(pl); + const script = [ + 'set -e', 'IS_WINDOWS=0', 'SKILL_PREFIX=0', '_FOREIGN_SKIPPED_ENTRIES=()', '_BACKED_UP_SKILL_MDS=()', + '_link_skill_runtime_assets() { :; }', '_print_windows_copy_note_once() { :; }', + ...['_link_or_copy', '_gstack_link_target_abs', '_gstack_target_is_ours', '_claude_entry_is_ours', '_write_owned_marker', + '_gstack_generated_header', '_claude_entry_owned_strongly', '_backup_skill_md', '_cleanup_weak_dir', '_gstack_dir_only_links', + '_cleanup_linked_dir'].map(extractFn), + `. "${HELPER}"`, + ...['link_claude_skill_dirs', 'cleanup_old_claude_symlinks', 'cleanup_prefixed_claude_symlinks'].map(extractFn), + `cleanup_old_claude_symlinks "${pl}" "${path.dirname(pl)}"`, + `cleanup_prefixed_claude_symlinks "${pl}" "${path.dirname(pl)}"`, + `link_claude_skill_dirs "${pl}" "${path.dirname(pl)}"`, + ].join('\n'); + const r = runBashScript(script, { timeout: 30_000, env: s.env }); + expect(r.stderr ?? '').not.toContain('command not found'); + expect(r.status, r.stderr ?? '').toBe(0); + expect(r.stdout).toContain('linked skills:'); + for (const name of [...SKILLS.map(n => n === 'office-hours' ? 'oh' : n), 'zz-nameless']) { + expect(fs.existsSync(path.join(s.tmp, 'skills', name, 'SKILL.md')), name).toBe(true); + } + expect(fs.existsSync(path.join(s.tmp, 'skills', 'office-hours'))).toBe(false); + expect(s.calls()).toEqual([]); + } finally { + s.cleanup(); + } + }); +}); diff --git a/test/setup-link-ownership.test.ts b/test/setup-link-ownership.test.ts index ed8f5c465..745c44b39 100644 --- a/test/setup-link-ownership.test.ts +++ b/test/setup-link-ownership.test.ts @@ -28,6 +28,7 @@ const HELPERS = [ '_link_skill_runtime_assets() { :; }', '_print_windows_copy_note_once() { :; }', extractFn('_link_or_copy'), + `. "${path.join(ROOT, 'bin/gstack-skill-name.sh')}"`, extractFn('_gstack_link_target_abs'), extractFn('_gstack_target_is_ours'), extractFn('_claude_entry_is_ours'), diff --git a/test/setup-playwright-best-effort.test.ts b/test/setup-playwright-best-effort.test.ts index 689e2b67c..826bc6897 100644 --- a/test/setup-playwright-best-effort.test.ts +++ b/test/setup-playwright-best-effort.test.ts @@ -653,6 +653,7 @@ function runLinker(opts: { '_print_windows_copy_note_once() { :; }', '_FOREIGN_SKIPPED_ENTRIES=()', extractFn('_link_or_copy'), + `. "${path.join(ROOT, 'bin/gstack-skill-name.sh')}"`, extractFn('_gstack_link_target_abs'), extractFn('_gstack_target_is_ours'), extractFn('_claude_entry_is_ours'), diff --git a/test/team-mode.test.ts b/test/team-mode.test.ts index ad36946a2..4bca76887 100644 --- a/test/team-mode.test.ts +++ b/test/team-mode.test.ts @@ -455,7 +455,7 @@ describe('setup --team / --no-team / -q', () => { try { for (const rel of [ 'setup', 'VERSION', 'SKILL.md', 'qa/SKILL.md', 'bin/gstack-config', 'bin/gstack-patch-names', - 'bin/gstack-state-root.sh', 'bin/gstack-bun-version.sh', 'bin/gstack-path-spelling.sh', 'bin/gstack-claude-root.sh', 'bin/gstack-install-registry.sh', 'bin/gstack-render-claude.sh', 'bin/gstack-host-renders.sh', + 'bin/gstack-state-root.sh', 'bin/gstack-bun-version.sh', 'bin/gstack-path-spelling.sh', 'bin/gstack-skill-name.sh', 'bin/gstack-claude-root.sh', 'bin/gstack-install-registry.sh', 'bin/gstack-render-claude.sh', 'bin/gstack-host-renders.sh', 'scripts/resolve-codex-generation-model.ts', 'scripts/models.ts', 'scripts/preflight-codex-overlap.ts', 'scripts/discover-skills.ts', 'scripts/external-skill-names.ts', 'scripts/host-config.ts', 'lib/claude-code-migration.ts', 'lib/model-policy.ts', 'lib/model-catalog.ts', diff --git a/test/user-render-out-dir-install.test.ts b/test/user-render-out-dir-install.test.ts index 07dd014e7..3a73176cd 100644 --- a/test/user-render-out-dir-install.test.ts +++ b/test/user-render-out-dir-install.test.ts @@ -190,6 +190,7 @@ describe('link_claude_skill_dirs prefers rendered SKILL.md (behavior)', () => { '_FOREIGN_SKIPPED_ENTRIES=()', '_BACKED_UP_SKILL_MDS=()', `_SKILL_BACKUP_ROOT="${os.tmpdir()}/gstack-harness-backups-${process.pid}"`, + `. "${path.join(ROOT, 'bin/gstack-skill-name.sh')}"`, extractFn(SETUP_SRC, 'link_claude_skill_dirs'), `link_claude_skill_dirs "${src}" "${skills}"`, ].join('\n'); From 1a22ae96520b99d3119ce9d3e5c76b43e60ccc3c Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 2 Nov 2026 01:26:46 -0800 Subject: [PATCH 2/5] v1.92.68.0 feat(health): Step 1 detects tools with gstack-checks, no check lost (GST-116 PR 2) (#3691) * feat(health): Step 1 detects tools with gstack-checks detect --for health, no check lost (GST-116 PR 2) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: register the frozen /health detection fixture as free Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: Health Stack normalization and --for refusals (review pass 1) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(health): keep Health Stack command bytes, honor an empty section, detect from the working directory (review pass 1) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(health): a multi-line test script is reported, not printed into the line protocol Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(health): Health Stack rows stay markdown like /verify reads them; an empty section names why (fix-check) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: the PATH-stub cases of the /health no-loss test run on Linux and macOS (windows-latest cannot launch sh stubs) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 10 ++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- bin/gstack-checks | 14 +- docs/skills.md | 2 +- health/SKILL.md | 41 ++---- health/SKILL.md.tmpl | 41 ++---- lib/checks.ts | 56 +++++++- package.json | 2 +- test/fixtures/health-detect-legacy.sh | 31 +++++ test/health-detect-no-loss.test.ts | 183 ++++++++++++++++++++++++++ test/helpers/free-fixtures-data.ts | 1 + 12 files changed, 312 insertions(+), 73 deletions(-) create mode 100644 test/fixtures/health-detect-legacy.sh create mode 100644 test/health-detect-no-loss.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cff9b4b5..654ab2b52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## [1.92.68.0] - 2026-11-02 + +**`/health` now finds its tools with gstack's detector, the same one `/verify` uses, and a test proves it still finds every check it found before.** /health Step 1 used to carry its own shell block for detecting a type checker, linter, test runner, knip, shellcheck and gbrain, a second copy of the list in `lib/checks.ts`. It now runs `gstack-checks detect --for health`. Scoring, the gbrain sub-score and history are unchanged. GST-116, PR 2 of 2. + +### Changed +- **`gstack-checks detect --for health`** prints one `DIMENSION ()` line per tool, or `DIMENSION none ()` so Step 2 can record the category as skipped with its reason. A `## Health Stack` section in CLAUDE.md still wins row for row, test rows included. Otherwise it applies /health's own rules, including a knip or shellcheck found on PATH and gbrain when `~/.gbrain/config.json` exists. It reads files and PATH and runs nothing. + +### Tests +- `test/health-detect-no-loss.test.ts` runs the old Step 1 block, frozen verbatim in `test/fixtures/health-detect-legacy.sh`, beside the new detector on 16 fixture repos and two PATH and HOME setups. Every check the old block printed must still be found. Dropping the knip or gbrain rule fails the test. + ## [1.92.67.0] - 2026-11-02 **`./setup` no longer starts a handful of processes per skill just to read its name (#2120).** On Windows 11 on ARM64, Git Bash runs under x64 emulation, and its `fork()` can deadlock when a script starts hundreds of short-lived processes in a row. Setup did exactly that: `gstack-patch-names` and setup's skill-linking loops ran `grep | sed | tr` and `basename` for every skill. The reporter counted about 220 process starts on a full install. Thanks to @habassa5, who traced the hang to those loops. diff --git a/VERSION b/VERSION index 61c63e5b8..75a21384c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.92.67.0 +1.92.68.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 4bcbb97c3..f898c2f84 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.92.67.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.92.68.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, diff --git a/bin/gstack-checks b/bin/gstack-checks index ffe6ff6c4..574743a6c 100755 --- a/bin/gstack-checks +++ b/bin/gstack-checks @@ -1,14 +1,16 @@ #!/usr/bin/env -S bun --no-env-file --config=/dev/null /** - * gstack-checks — the build-and-verify loop's checks for /verify. Logic: lib/checks.ts. + * gstack-checks — the build-and-verify loop's checks for /verify, and /health's tool detection. Logic: lib/checks.ts. */ import { spawnSync } from 'node:child_process'; import { cancelActive } from '../lib/pregate/supervise'; -import { MAX_FAIL_LINES, TESTS, isTrusted, ledgerPath, load, nothingToRun, readLedger, recordTrust, runChecks, status, type Loaded } from '../lib/checks'; +import { MAX_FAIL_LINES, TESTS, healthDimensions, isTrusted, ledgerPath, load, nothingToRun, readLedger, recordTrust, runChecks, status, type Loaded } from '../lib/checks'; const USAGE = `Usage: gstack-checks [options] detect list the checks run would execute and whether they are trusted (reads files only) + detect --for health /health's dimensions: DIMENSION (), or DIMENSION none + (); a CLAUDE.md Health Stack section wins (reads files and PATH only) trust trust the current command set for this repo (checks, the pregate runner and its selection manifest commands); any change to one of them needs trust again run [--only a,b] [--allow-repo-commands] [--timeout ] @@ -43,6 +45,14 @@ async function main(argv: string[]): Promise { const [verb, ...args] = argv; if (!verb || verb === '--help' || verb === '-h') { process.stdout.write(USAGE); return verb ? 0 : 64; } if (!['detect', 'trust', 'run', 'status'].includes(verb)) { process.stderr.write(USAGE); return 64; } + const mode = flag(args, '--for'); + if (args.includes('--for')) { + if (verb !== 'detect' || mode !== 'health') { process.stderr.write(USAGE); return 64; } + const d = healthDimensions(process.cwd()); + for (const c of d.checks) process.stdout.write(`DIMENSION ${c.dimension} ${c.command} (${c.source})\n`); + for (const m of d.missing) process.stdout.write(`DIMENSION ${m.dimension} none (${m.reason})\n`); + return 0; + } const root = repoRoot(); if (!root) { process.stderr.write('gstack-checks: not inside a git repository\n'); return 3; } const timeoutS = Number(flag(args, '--timeout') ?? 600); diff --git a/docs/skills.md b/docs/skills.md index c121fc89d..335da1795 100644 --- a/docs/skills.md +++ b/docs/skills.md @@ -662,7 +662,7 @@ Instead of guessing and patching, it traces data flow, matches against known bug ## `/verify` -`/verify` is the loop you run while you build: change something, run the repo's checks, fix what broke, run again. `gstack-checks detect` finds the checks the repo already declares (a `gstack:verify:` line or a `## Health Stack` section in CLAUDE.md, else package.json scripts, tsconfig, eslint or biome, pyproject, Cargo.toml, go.mod) and never makes one up. `gstack-checks run` runs them plus the tests `/ship`'s pre-gate selects for your change, using pregate's own selector, so a green `/verify` predicts a green pre-gate. +`/verify` is the loop you run while you build: change something, run the repo's checks, fix what broke, run again. `gstack-checks detect` finds the checks the repo already declares (a `gstack:verify:` line or a `## Health Stack` section in CLAUDE.md, else package.json scripts, tsconfig, eslint or biome, pyproject, Cargo.toml, go.mod) and never makes one up. `gstack-checks run` runs them plus the tests `/ship`'s pre-gate selects for your change, using pregate's own selector, so a green `/verify` predicts a green pre-gate. `/health` finds its tools with the same detector (`gstack-checks detect --for health`), which adds its test, knip, shellcheck and gbrain dimensions. It asks once before running any repo command (`gstack-checks trust`) and asks again when a command changes. A touched file no test covers is `UNMAPPED`, a selection that ran zero tests is `ZERO_RUN`, and neither counts as green. A missing tool or service is reported, not worked around. It stops after five runs or when the same failure comes back twice, and sends a stubborn red to `/investigate`. When everything passes, `gstack-checks status` prints `READY_FOR_SHIP tree=` without rerunning anything, and you run `/ship`. It never reviews, bumps versions, pushes or opens a PR. diff --git a/health/SKILL.md b/health/SKILL.md index b6c84d87d..3b1ee34ca 100644 --- a/health/SKILL.md +++ b/health/SKILL.md @@ -115,45 +115,22 @@ When the user types `/health`, run this skill. ## Step 1: Detect Health Stack -Read CLAUDE.md and look for a `## Health Stack` section. If found, parse the tools -listed there and skip auto-detection. - -If no `## Health Stack` section exists, auto-detect available tools: +Detect the tools with gstack's detector: ```bash -# Type checker -[ -f tsconfig.json ] && echo "TYPECHECK: tsc --noEmit" - -# Linter -[ -f biome.json ] || [ -f biome.jsonc ] && echo "LINT: biome check ." -setopt +o nomatch 2>/dev/null || true -ls eslint.config.* .eslintrc.* .eslintrc 2>/dev/null | head -1 | xargs -I{} echo "LINT: eslint ." -[ -f .pylintrc ] || [ -f pyproject.toml ] && grep -q "pylint\|ruff" pyproject.toml 2>/dev/null && echo "LINT: ruff check ." - -# Test runner -[ -f package.json ] && grep -q '"test"' package.json 2>/dev/null && echo "TEST: $(node -e "console.log(JSON.parse(require('fs').readFileSync('package.json','utf8')).scripts.test)" 2>/dev/null)" -[ -f pyproject.toml ] && grep -q "pytest" pyproject.toml 2>/dev/null && echo "TEST: pytest" -[ -f Cargo.toml ] && echo "TEST: cargo test" -[ -f go.mod ] && echo "TEST: go test ./..." - -# Dead code -command -v knip >/dev/null 2>&1 && echo "DEADCODE: knip" -[ -f package.json ] && grep -q '"knip"' package.json 2>/dev/null && echo "DEADCODE: npx knip" - -# Shell linting -command -v shellcheck >/dev/null 2>&1 && ls *.sh scripts/*.sh bin/*.sh 2>/dev/null | head -1 | xargs -I{} echo "SHELL: shellcheck" - -# GBrain presence — only report as a dimension if gbrain is actually -# set up; otherwise skip so machines without gbrain aren't penalized. -if command -v gbrain >/dev/null 2>&1 && [ -f "$HOME/.gbrain/config.json" ]; then - echo "GBRAIN: gbrain doctor --json (wrapped in timeout 5s)" -fi +C="$HOME/.claude/skills/gstack/bin/gstack-checks"; "$C" detect --for health ``` +It reads files and PATH only and runs nothing. A `## Health Stack` section in +CLAUDE.md wins: its rows come back as they are, and auto-detection is skipped. +Each `DIMENSION ()` line is a tool to run in Step 2. +`DIMENSION none ()` means that category is unavailable: record it +as `SKIPPED` with the reason. A `gbrain` command keeps its `timeout 5s`. + Use Glob to search for shell scripts: - `**/*.sh` (shell scripts in the repo) -After auto-detection, present the detected tools via AskUserQuestion: +After auto-detection (no line's source is `CLAUDE.md Health Stack`), present the detected tools via AskUserQuestion: "I detected these health check tools for this project: diff --git a/health/SKILL.md.tmpl b/health/SKILL.md.tmpl index 47f83551e..da8c9c700 100644 --- a/health/SKILL.md.tmpl +++ b/health/SKILL.md.tmpl @@ -42,45 +42,22 @@ When the user types `/health`, run this skill. ## Step 1: Detect Health Stack -Read CLAUDE.md and look for a `## Health Stack` section. If found, parse the tools -listed there and skip auto-detection. - -If no `## Health Stack` section exists, auto-detect available tools: +Detect the tools with gstack's detector: ```bash -# Type checker -[ -f tsconfig.json ] && echo "TYPECHECK: tsc --noEmit" - -# Linter -[ -f biome.json ] || [ -f biome.jsonc ] && echo "LINT: biome check ." -setopt +o nomatch 2>/dev/null || true -ls eslint.config.* .eslintrc.* .eslintrc 2>/dev/null | head -1 | xargs -I{} echo "LINT: eslint ." -[ -f .pylintrc ] || [ -f pyproject.toml ] && grep -q "pylint\|ruff" pyproject.toml 2>/dev/null && echo "LINT: ruff check ." - -# Test runner -[ -f package.json ] && grep -q '"test"' package.json 2>/dev/null && echo "TEST: $(node -e "console.log(JSON.parse(require('fs').readFileSync('package.json','utf8')).scripts.test)" 2>/dev/null)" -[ -f pyproject.toml ] && grep -q "pytest" pyproject.toml 2>/dev/null && echo "TEST: pytest" -[ -f Cargo.toml ] && echo "TEST: cargo test" -[ -f go.mod ] && echo "TEST: go test ./..." - -# Dead code -command -v knip >/dev/null 2>&1 && echo "DEADCODE: knip" -[ -f package.json ] && grep -q '"knip"' package.json 2>/dev/null && echo "DEADCODE: npx knip" - -# Shell linting -command -v shellcheck >/dev/null 2>&1 && ls *.sh scripts/*.sh bin/*.sh 2>/dev/null | head -1 | xargs -I{} echo "SHELL: shellcheck" - -# GBrain presence — only report as a dimension if gbrain is actually -# set up; otherwise skip so machines without gbrain aren't penalized. -if command -v gbrain >/dev/null 2>&1 && [ -f "$HOME/.gbrain/config.json" ]; then - echo "GBRAIN: gbrain doctor --json (wrapped in timeout 5s)" -fi +C="{{RUNTIME_ROOT}}/bin/gstack-checks"; "$C" detect --for health ``` +It reads files and PATH only and runs nothing. A `## Health Stack` section in +CLAUDE.md wins: its rows come back as they are, and auto-detection is skipped. +Each `DIMENSION ()` line is a tool to run in Step 2. +`DIMENSION none ()` means that category is unavailable: record it +as `SKIPPED` with the reason. A `gbrain` command keeps its `timeout 5s`. + Use Glob to search for shell scripts: - `**/*.sh` (shell scripts in the repo) -After auto-detection, present the detected tools via AskUserQuestion: +After auto-detection (no line's source is `CLAUDE.md Health Stack`), present the detected tools via AskUserQuestion: "I detected these health check tools for this project: diff --git a/lib/checks.ts b/lib/checks.ts index a73c192d8..5396ba09e 100644 --- a/lib/checks.ts +++ b/lib/checks.ts @@ -53,8 +53,8 @@ export function declaredVerify(claudeMd: string): string | null { return null; } -/** The '## Health Stack' section (/health's own format): '- key: command' rows. 'test' and 'tests' are left to pregate; a repeated key gets -2, -3 so every check has its own name. */ -export function healthStack(claudeMd: string): Check[] { +/** The '## Health Stack' section (/health's own format): '- key: command' rows. 'test' and 'tests' are left to pregate unless keepTests (/health scores them); a repeated key gets -2, -3 so every check has its own name. */ +export function healthStack(claudeMd: string, keepTests = false): Check[] { const lines = claudeMd.split('\n'); const start = lines.findIndex(l => /^##\s+Health Stack\s*$/.test(l)); if (start < 0) return []; @@ -64,7 +64,7 @@ export function healthStack(claudeMd: string): Check[] { for (const line of lines.slice(start + 1)) { if (/^#{1,6}\s/.test(line)) break; const m = /^\s*[-*]\s*([A-Za-z0-9_-]+):\s*(.+?)\s*$/.exec(line); - if (!m || ['test', TESTS].includes(m[1].toLowerCase())) continue; + if (!m || (!keepTests && ['test', TESTS].includes(m[1].toLowerCase()))) continue; const key = m[1].toLowerCase(); let n = seen.get(key) ?? 0; let name = key; @@ -127,6 +127,56 @@ export function detectChecks(root: string): Check[] { }); } +export const HEALTH_DIMENSIONS = ['typecheck', 'lint', 'test', 'deadcode', 'shell', 'gbrain'] as const; +export interface HealthDimensions { checks: Array<{ dimension: string; command: string; source: string }>; missing: Array<{ dimension: string; reason: string }> } + +/** + * /health Step 1, in the directory /health runs from: a Health Stack section row for row (test rows kept; a section with no + * rows still turns auto-detection off), else /health's own presence rules, + * which may use a tool found on PATH (unlike /verify's declared-only manifestChecks). Reads files and PATH; runs nothing. + * test/health-detect-no-loss.test.ts holds these rules to the shell block they replaced. + */ +export function healthDimensions(root: string, env: NodeJS.ProcessEnv = process.env): HealthDimensions { + const claude = read(path.join(root, 'CLAUDE.md')) ?? ''; + const stack = healthStack(claude, true); + if (/^##\s+Health Stack\s*$/m.test(claude)) { + const keys = new Set(stack.map(c => c.name.replace(/-\d+$/, '').replace(/^tests$/, 'test'))); + return { checks: stack.map(c => ({ dimension: c.name, command: c.command, source: c.source })), missing: HEALTH_DIMENSIONS.filter(d => !keys.has(d)).map(dimension => ({ dimension, reason: stack.length ? 'not in the CLAUDE.md Health Stack section' : 'the CLAUDE.md Health Stack section has no "- key: command" rows' })) }; + } + const has = (file: string) => fs.existsSync(path.join(root, file)); + const which = (bin: string) => Bun.which(bin, { PATH: env.PATH ?? '' }) !== null; + const pkgText = read(path.join(root, 'package.json')); + const pyproject = read(path.join(root, 'pyproject.toml')); + let testScript: unknown; + try { testScript = pkgText?.includes('"test"') ? JSON.parse(pkgText).scripts?.test : undefined; } catch { /* an unreadable package.json declares nothing */ } + const listDir = (dir: string) => { try { return fs.readdirSync(dir); } catch { return []; } }; + const shellFiles = ['.', 'scripts', 'bin'].some(dir => listDir(path.join(root, dir)).some(f => f.endsWith('.sh'))); + const found: Array<[string, string, string] | false> = [ + has('tsconfig.json') && ['typecheck', 'tsc --noEmit', 'tsconfig.json'], + (has('biome.json') || has('biome.jsonc')) && ['lint', 'biome check .', 'biome.json'], + listDir(root).some(f => /^(eslint\.config\..+|\.eslintrc(\..+)?)$/.test(f)) && ['lint', 'eslint .', 'eslint config'], + !!pyproject && /pylint|ruff/.test(pyproject) && ['lint', 'ruff check .', 'pyproject.toml'], + typeof testScript === 'string' && testScript !== '' && !/[\r\n]/.test(testScript) && ['test', testScript, 'package.json'], + !!pyproject && pyproject.includes('pytest') && ['test', 'pytest', 'pyproject.toml'], + has('Cargo.toml') && ['test', 'cargo test', 'Cargo.toml'], + has('go.mod') && ['test', 'go test ./...', 'go.mod'], + which('knip') && ['deadcode', 'knip', 'PATH'], + !!pkgText?.includes('"knip"') && ['deadcode', 'npx knip', 'package.json'], + which('shellcheck') && shellFiles && ['shell', 'shellcheck', 'PATH, *.sh in the root, scripts/ or bin/'], + which('gbrain') && !!env.HOME && fs.existsSync(path.join(env.HOME, '.gbrain', 'config.json')) && ['gbrain', 'timeout 5s gbrain doctor --json', 'PATH, ~/.gbrain/config.json'], + ]; + const checks = found.filter((c): c is [string, string, string] => !!c).map(([dimension, command, source]) => ({ dimension, command, source })); + const reasons: Record = { + typecheck: 'no tsconfig.json', + lint: 'no biome.json, eslint config, or pyproject.toml naming ruff or pylint', + test: typeof testScript === 'string' && /[\r\n]/.test(testScript) ? 'the package.json test script spans lines; list it in a CLAUDE.md Health Stack section' : 'no package.json test script, pytest in pyproject.toml, Cargo.toml or go.mod', + deadcode: 'knip is not on PATH or in package.json', + shell: which('shellcheck') ? 'no *.sh file in the root, scripts/ or bin/' : 'shellcheck is not on PATH', + gbrain: 'gbrain is not on PATH or ~/.gbrain/config.json is absent', + }; + return { checks, missing: HEALTH_DIMENSIONS.filter(d => !checks.some(c => c.dimension === d)).map(dimension => ({ dimension, reason: reasons[dimension] })) }; +} + /** Every command a run could execute, read without executing any: the checks, the pregate runner and each selection manifest command. */ export function commandSet(root: string, checks: readonly Check[], config: Pick): string { return sha256(JSON.stringify({ checks: checks.map(c => [c.name, c.command, c.body ?? '']), scripts: packageScripts(root), runner: config.runner ?? detectedRunnerInputs(root), selection: Object.entries(config.selection ?? {}).sort(([a], [b]) => a.localeCompare(b)) })); diff --git a/package.json b/package.json index ba562a6cd..883d131b3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.92.67", + "version": "1.92.68", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/test/fixtures/health-detect-legacy.sh b/test/fixtures/health-detect-legacy.sh new file mode 100644 index 000000000..6d925861b --- /dev/null +++ b/test/fixtures/health-detect-legacy.sh @@ -0,0 +1,31 @@ +# /health Step 1 auto-detection, frozen verbatim from health/SKILL.md.tmpl at a305302 +# (v1.91.365.0) before /health moved onto gstack-checks detect --for health (GST-116 PR 2). +# test/health-detect-no-loss.test.ts runs it beside the new detector: every line it prints +# must still be found. Do not edit; it is the baseline, not live code. +# Type checker +[ -f tsconfig.json ] && echo "TYPECHECK: tsc --noEmit" + +# Linter +[ -f biome.json ] || [ -f biome.jsonc ] && echo "LINT: biome check ." +setopt +o nomatch 2>/dev/null || true +ls eslint.config.* .eslintrc.* .eslintrc 2>/dev/null | head -1 | xargs -I{} echo "LINT: eslint ." +[ -f .pylintrc ] || [ -f pyproject.toml ] && grep -q "pylint\|ruff" pyproject.toml 2>/dev/null && echo "LINT: ruff check ." + +# Test runner +[ -f package.json ] && grep -q '"test"' package.json 2>/dev/null && echo "TEST: $(node -e "console.log(JSON.parse(require('fs').readFileSync('package.json','utf8')).scripts.test)" 2>/dev/null)" +[ -f pyproject.toml ] && grep -q "pytest" pyproject.toml 2>/dev/null && echo "TEST: pytest" +[ -f Cargo.toml ] && echo "TEST: cargo test" +[ -f go.mod ] && echo "TEST: go test ./..." + +# Dead code +command -v knip >/dev/null 2>&1 && echo "DEADCODE: knip" +[ -f package.json ] && grep -q '"knip"' package.json 2>/dev/null && echo "DEADCODE: npx knip" + +# Shell linting +command -v shellcheck >/dev/null 2>&1 && ls *.sh scripts/*.sh bin/*.sh 2>/dev/null | head -1 | xargs -I{} echo "SHELL: shellcheck" + +# GBrain presence — only report as a dimension if gbrain is actually +# set up; otherwise skip so machines without gbrain aren't penalized. +if command -v gbrain >/dev/null 2>&1 && [ -f "$HOME/.gbrain/config.json" ]; then + echo "GBRAIN: gbrain doctor --json (wrapped in timeout 5s)" +fi diff --git a/test/health-detect-no-loss.test.ts b/test/health-detect-no-loss.test.ts new file mode 100644 index 000000000..9e6f11c59 --- /dev/null +++ b/test/health-detect-no-loss.test.ts @@ -0,0 +1,183 @@ +/** + * /health Step 1 moved onto gstack-checks detect --for health (GST-116 PR 2). The acceptance bar: every check the old + * Step 1 found is still found. The old shell block is frozen verbatim in test/fixtures/health-detect-legacy.sh; each + * fixture repo runs both under the same PATH and HOME, and every legacy (category, command) pair must appear in the + * detector's DIMENSION lines. + */ +import { afterAll, describe, expect, test } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { HEALTH_DIMENSIONS, healthDimensions } from '../lib/checks'; +import { runScriptSync } from '../lib/script-invocation'; +import { BIN } from './helpers/pregate-fixture'; + +const LEGACY = path.join(import.meta.dir, 'fixtures', 'health-detect-legacy.sh'); +const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'health-detect-')); +afterAll(() => fs.rmSync(tmp, { recursive: true, force: true })); + +const stubs = path.join(tmp, 'stubs'); +fs.mkdirSync(stubs); +for (const tool of ['knip', 'shellcheck', 'gbrain']) { + fs.writeFileSync(path.join(stubs, tool), '#!/bin/sh\nexit 0\n'); + fs.chmodSync(path.join(stubs, tool), 0o755); +} +const gbrainHome = path.join(tmp, 'home-gbrain'); +fs.mkdirSync(path.join(gbrainHome, '.gbrain'), { recursive: true }); +fs.writeFileSync(path.join(gbrainHome, '.gbrain', 'config.json'), '{}\n'); +const bareHome = path.join(tmp, 'home-bare'); +fs.mkdirSync(bareHome); +// The knip, shellcheck and gbrain stubs are extensionless #!/bin/sh files, which windows-latest cannot find or launch +// (windows-free-shard 3, run 41204425554); the cases that need them run on Linux and macOS. +const stubbed = test.skipIf(process.platform === 'win32'); + +const PKG = JSON.stringify({ name: 'fx', scripts: { test: 'vitest run', lint: 'eslint .' }, devDependencies: { knip: '^5.0.0' } }); +const FIXTURES: Record> = { + empty: {}, + tsconfig: { 'tsconfig.json': '{}' }, + 'biome-json': { 'biome.json': '{}' }, + 'biome-jsonc': { 'biome.jsonc': '{}' }, + 'eslint-flat': { 'eslint.config.mjs': 'export default [];' }, + 'eslintrc-json': { '.eslintrc.json': '{}' }, + 'eslintrc-bare': { '.eslintrc': '{}' }, + 'pyproject-ruff-pytest': { 'pyproject.toml': '[tool.ruff]\n[tool.pytest.ini_options]\n' }, + 'pyproject-pylint': { 'pyproject.toml': '[tool.pylint]\n' }, + 'pylintrc-only': { '.pylintrc': '[MASTER]\n' }, + 'package-test-knip': { 'package.json': PKG }, + cargo: { 'Cargo.toml': '[package]\nname = "fx"\n' }, + go: { 'go.mod': 'module fx\n' }, + 'shell-scripts-dir': { 'scripts/build.sh': 'echo hi\n' }, + 'shell-root-and-bin': { 'run.sh': 'echo hi\n', 'bin/tool.sh': 'echo hi\n' }, + everything: { 'tsconfig.json': '{}', 'biome.json': '{}', 'eslint.config.js': '', 'pyproject.toml': 'ruff pytest\n', 'package.json': PKG, 'Cargo.toml': '', 'go.mod': 'module fx\n', 'scripts/a.sh': '' }, +}; + +function repo(name: string, files: Record): string { + const dir = path.join(tmp, 'repos', name); + for (const [file, body] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(dir, file)), { recursive: true }); + fs.writeFileSync(path.join(dir, file), body); + } + fs.mkdirSync(dir, { recursive: true }); + return dir; +} + +const NORMALIZE: Record = { 'gbrain doctor --json (wrapped in timeout 5s)': 'timeout 5s gbrain doctor --json' }; +const pairs = (lines: Array<[string, string]>) => lines.map(([d, c]) => `${d}: ${c}`).sort(); + +function legacy(dir: string, env: NodeJS.ProcessEnv): string[] { + const r = spawnSync('bash', [LEGACY], { cwd: dir, env, encoding: 'utf8', timeout: 30_000 }); + expect(r.status).toBe(0); + return pairs(r.stdout.split('\n').flatMap(line => { + const m = /^(TYPECHECK|LINT|TEST|DEADCODE|SHELL|GBRAIN): (.+)$/.exec(line); + return m ? [[m[1].toLowerCase(), NORMALIZE[m[2]] ?? m[2]] as [string, string]] : []; + })); +} + +function detector(dir: string, env: NodeJS.ProcessEnv): { found: string[]; none: string[] } { + const r = runScriptSync(BIN('gstack-checks'), ['detect', '--for', 'health'], { cwd: dir, env, encoding: 'utf8', timeout: 60_000 }); + expect(r.status).toBe(0); + const lines = (r.stdout ?? '').split('\n').filter(Boolean).map(line => /^DIMENSION (\S+) (.+?) \((.*)\)$/.exec(line)!); + expect(lines.every(Boolean)).toBe(true); + return { found: pairs(lines.filter(m => m[2] !== 'none').map(m => [m[1], m[2]])), none: lines.filter(m => m[2] === 'none').map(m => m[1]) }; +} + +const ENVS = { + 'tools on PATH, gbrain configured': { PATH: `${stubs}${path.delimiter}${process.env.PATH}`, HOME: gbrainHome }, + 'machine PATH, no gbrain config': { PATH: process.env.PATH, HOME: bareHome }, +}; + +describe('/health detection: no check lost (GST-116 PR 2)', () => { + // Value: protects=/health still finds every tool its old Step 1 shell block found; fails_when=a ported rule drops a + // category or command (knip, shellcheck, gbrain, a lint or test rule) for any fixture; why_new=/health moved onto the + // detector in this change; seam=the frozen legacy block run beside the bin; no_claim=that the found tools succeed + for (const [envName, envVars] of Object.entries(ENVS)) { + (envName.startsWith('tools on PATH') ? stubbed : test)(`every legacy line is found: ${envName}`, () => { + const env = { ...process.env, ...envVars }; + let legacyTotal = 0; + for (const [name, files] of Object.entries(FIXTURES)) { + const dir = repo(name, files); + const old = legacy(dir, env); + const now = detector(dir, env); + legacyTotal += old.length; + expect({ name, lost: old.filter(p => !now.found.includes(p)) }).toEqual({ name, lost: [] }); + expect(new Set([...now.found.map(p => p.split(':')[0]), ...now.none])).toEqual(new Set(HEALTH_DIMENSIONS)); + } + expect(legacyTotal).toBeGreaterThan(20); + }); + } + + // Value: protects=the matrix exercises knip, shellcheck and gbrain, not just the manifest rules; fails_when=a stub stops + // being found so the no-loss comparison above runs with those categories empty; why_new=new fixture; seam=none + stubbed('the stubbed run covers every category on both sides', () => { + const env = { ...process.env, ...ENVS['tools on PATH, gbrain configured'] }; + const dir = repo('everything', FIXTURES.everything); + expect(new Set(legacy(dir, env).map(p => p.split(':')[0]))).toEqual(new Set(HEALTH_DIMENSIONS)); + expect(detector(dir, env).none).toEqual([]); + }); + + // Value: protects=a Health Stack section wins row for row, test rows included, and nothing is auto-detected beside it; + // fails_when=health mode drops the test row /verify leaves to pregate, or mixes manifest detection in; why_new=keepTests + // is new; seam=none + test('a Health Stack section is returned as written', () => { + const dir = repo('health-stack', { 'tsconfig.json': '{}', 'CLAUDE.md': '# x\n\n## Health Stack\n\n- typecheck: tsc --noEmit -p app\n- test: bun test\n- deadcode: knip --production\n- shell: shellcheck *.sh scripts/*.sh\n\n## Other\n' }); + const d = healthDimensions(dir, { PATH: stubs, HOME: gbrainHome }); + expect(d.checks.map(c => [c.dimension, c.command])).toEqual([['typecheck', 'tsc --noEmit -p app'], ['test', 'bun test'], ['deadcode', 'knip --production'], ['shell', 'shellcheck *.sh scripts/*.sh']]); + expect(d.missing).toEqual([{ dimension: 'lint', reason: 'not in the CLAUDE.md Health Stack section' }, { dimension: 'gbrain', reason: 'not in the CLAUDE.md Health Stack section' }]); + }); + + // Value: protects=a 'tests' row and repeated keys count as present, so Step 2 never skips a category it runs; + // fails_when=the -N suffix or tests→test normalization regresses and a listed category also prints none; why_new=new; seam=none + test('a tests row and repeated keys are not reported missing', () => { + const dir = repo('health-stack-dupes', { 'CLAUDE.md': '## Health Stack\n\n- tests: bun test\n- lint: eslint .\n- lint: biome check .\n' }); + const d = healthDimensions(dir, { PATH: '', HOME: bareHome }); + expect(d.checks.map(c => c.dimension)).toEqual(['tests', 'lint', 'lint-2']); + expect(d.missing.map(m => m.dimension)).toEqual(['typecheck', 'deadcode', 'shell', 'gbrain']); + }); + + // Value: protects=--for is accepted only as detect --for health; fails_when=run --for health falls through into the run + // path, or a missing or unknown mode is accepted; why_new=new flag; seam=the bin + test('--for is refused outside detect --for health', () => { + for (const args of [['run', '--for', 'health'], ['detect', '--for'], ['detect', '--for', 'verify']]) { + const r = runScriptSync(BIN('gstack-checks'), args, { cwd: tmp, encoding: 'utf8', timeout: 30_000 }); + expect(r.status).toBe(64); + expect(r.stderr ?? '').toContain('Usage: gstack-checks'); + } + }); + + // Value: protects=Health Stack rows read as markdown like gstack:verify: (code-span backticks dropped), an empty section + // still turns auto-detection off and says so, and detection runs where /health runs (a monorepo package); + // fails_when=a section with no rows falls through to auto-detection, or the bin detects from the git top-level; + // why_new=review pass 1 voices; seam=the bin from a subdirectory + test('Health Stack spans, an empty section, and the working directory', () => { + const tick = String.fromCharCode(96); + const spans = healthDimensions(repo('hs-spans', { 'CLAUDE.md': `## Health Stack\n\n- lint: ${tick}eslint .${tick} && ${tick}prettier --check .${tick}\n` }), { PATH: '', HOME: bareHome }); + expect(spans.checks.map(c => c.command)).toEqual(['eslint . && prettier --check .']); + const empty = healthDimensions(repo('hs-empty', { 'tsconfig.json': '{}', 'CLAUDE.md': '## Health Stack\n\nNo health tools are configured.\n' }), { PATH: stubs, HOME: gbrainHome }); + expect(empty.checks).toEqual([]); + expect(empty.missing.map(m => m.dimension)).toEqual([...HEALTH_DIMENSIONS]); + expect(empty.missing[0].reason).toBe('the CLAUDE.md Health Stack section has no "- key: command" rows'); + const mono = repo('monorepo', { 'tsconfig.json': '{}', 'packages/app/package.json': JSON.stringify({ scripts: { test: 'vitest run' } }) }); + expect(spawnSync('git', ['init', '-q'], { cwd: mono, timeout: 30_000 }).status).toBe(0); + const env = { ...process.env, PATH: process.env.PATH, HOME: bareHome }; + const pkg = detector(path.join(mono, 'packages', 'app'), env); + expect(pkg.found).toContain('test: vitest run'); + expect(pkg.none).toContain('typecheck'); + const multi = healthDimensions(repo('multiline-test', { 'package.json': JSON.stringify({ scripts: { test: 'vitest\nDIMENSION lint x (CLAUDE.md Health Stack)' } }) }), { PATH: '', HOME: bareHome }); + expect(multi.checks).toEqual([]); + expect(multi.missing.find(m => m.dimension === 'test')?.reason).toBe('the package.json test script spans lines; list it in a CLAUDE.md Health Stack section'); + const noHome = repo('no-home', { '.gbrain/config.json': '{}' }); + expect(healthDimensions(noHome, { PATH: stubs, HOME: '' }).checks.map(c => c.dimension)).not.toContain('gbrain'); + }); + + // Value: protects=an unavailable category says why, so Step 2 records SKIPPED with a reason; fails_when=a none line + // loses its reason or shellcheck's two causes collapse; why_new=new output; seam=none + stubbed('none lines name the reason', () => { + const empty = healthDimensions(repo('empty', {}), { PATH: '', HOME: bareHome }); + expect(empty.checks).toEqual([]); + expect(empty.missing.find(m => m.dimension === 'shell')?.reason).toBe('shellcheck is not on PATH'); + const noScripts = healthDimensions(repo('tsconfig', FIXTURES.tsconfig), { PATH: stubs, HOME: bareHome }); + expect(noScripts.missing.find(m => m.dimension === 'shell')?.reason).toBe('no *.sh file in the root, scripts/ or bin/'); + expect(noScripts.missing.find(m => m.dimension === 'gbrain')?.reason).toBe('gbrain is not on PATH or ~/.gbrain/config.json is absent'); + }); +}); diff --git a/test/helpers/free-fixtures-data.ts b/test/helpers/free-fixtures-data.ts index 7f7f14f1b..9d9131d47 100644 --- a/test/helpers/free-fixtures-data.ts +++ b/test/helpers/free-fixtures-data.ts @@ -150,6 +150,7 @@ export const FREE_FIXTURES: Readonly> = { 'test/fixtures/factory-scorecard/**': ['test/factory-scorecard.test.ts'], 'test/fixtures/free-shard-capture.ts': ['test/test-free-shards-capture.test.ts'], 'test/fixtures/golden/**': ['test/host-config.test.ts', 'test/ship-review-loop.test.ts'], + 'test/fixtures/health-detect-legacy.sh': ['test/health-detect-no-loss.test.ts'], 'test/fixtures/impeccable-antipatterns.json': ['test/design-catalog.test.ts', 'test/impeccable-fixtures.test.ts'], 'test/fixtures/impeccable-captures.meta.json': ['test/design-detect-contract.test.ts', 'test/impeccable-fixtures.test.ts'], 'test/fixtures/impeccable-detect-dom-sample.json': ['test/impeccable-fixtures.test.ts'], From 7762c39a4ecdba8268c1ffc2b0384bccc04b3fa6 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 2 Nov 2026 01:37:00 -0800 Subject: [PATCH 3/5] v1.92.69.0 fix: lock developer-profile writers so overlapping derives keep every session (#3437, #3435) (#3738) * fix(plan-tune): lock developer-profile writes and give derive a unique temp file (#3437) Concurrent background --derive runs shared developer-profile.json.tmp and rewrote the whole profile unlocked, so renames failed with ENOENT and a concurrent --log-session was dropped. Every read-modify-write now holds developer-profile.json.lock and writes a mktemp file. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * chore(plan-tune): remove applySignal and newDimensionTotals (#3435) Both lost their last production caller when --derive moved onto accumulateSignals; only three unit tests referenced them. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Use the PID in the lock token, anchor a relative state root once so do_derive's cd cannot leak the lock, and note the stale-takeover tradeoff. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes (pass 2) Keep Windows drive-letter state roots absolute when anchoring a relative root, and say which writers do not take the lock yet. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: take over a lock only from a dead holder, one taker at a time Step 11 voices: a late stale-lock mv could evict a fresh owner, a failed mv spun past the timeout, and the 10 s give-up sat under the 30 s stale age. Takeover now checks the holder's pid, re-reads the owner under a takeover lock, falls through to the give-up on failure, and reports an unwritable state directory at once. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: take over an owned lock older than 5 minutes (pid reuse) A crashed holder whose pid now belongs to a live process kept its lock forever. An owned lock past 5 minutes now counts as abandoned, and cleanup removes a takeover lock this writer holds. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: changelog fragment for the profile lock (#3437) and dead-helper removal (#3435) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: say the 5-minute takeover applies whatever the pid Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: keep the give-up test inside the type ratchet Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 15 +++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- bin/gstack-developer-profile | 146 ++++++++++++++++++++----- package.json | 2 +- scripts/psychographic-signals.ts | 34 ------ test/gstack-developer-profile.test.ts | 149 ++++++++++++++++++++++++++ test/plan-tune.test.ts | 22 ---- 8 files changed, 288 insertions(+), 84 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 654ab2b52..07112e9ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ # Changelog +## [1.92.69.0] - 2026-11-02 + +**Background `--derive` runs no longer drop /office-hours sessions or fail with ENOENT when they overlap (#3437).** `gstack-question-log` starts a background `gstack-developer-profile --derive` after every append, and a multi-question AskUserQuestion logs several rows at once. Every derive wrote the same `developer-profile.json.tmp` and rewrote the whole profile without a lock. On main at 3bfe566, 30 rounds of 8 overlapping derives plus one `--log-session` saw 9 of 270 runs fail with `ENOENT` on rename, and 2 of the 30 sessions disappeared. With this change the same run has 0 failures and keeps 30 of 30 sessions. + +### Fixed +- **`bin/gstack-developer-profile`**: `--derive`, `--log-session`, `--reconcile` and `--migrate` hold `developer-profile.json.lock` from before they read the profile until after they rename it, and each writes its own `mktemp` file. The owner file names the holder's process ID, and the lock is ownership-checked on release. A lock is taken over when its holder process is gone, or once it is 5 minutes old even if a process with that ID is running, since no writer takes that long and the ID may have been reused. Takeovers go one at a time under `developer-profile.json.lock.takeover`, so two waiters can't both take over. A writer that can't get the lock within 10 seconds writes nothing and exits 5. An unwritable state directory is reported right away instead of after the wait. A relative state root is anchored once, so the lock is released under the same path it was taken. Windows drive-letter roots stay as they are. +### Removed +- **`scripts/psychographic-signals.ts`**: `applySignal` and `newDimensionTotals`, which lost their last production caller when `--derive` moved onto `accumulateSignals` (#3435), and their three unit tests. + +### Not covered yet +- `gstack-distill-apply` and /plan-tune's declared edits still write the profile without the lock. + +### Tests +- `test/gstack-developer-profile.test.ts`: a directory at the old fixed temp path; a held lock blocking `--derive` and `--log-session`; takeover of an ownerless stale lock, of a dead holder's lock, and of a 5-minute-old lock whose process ID is alive; a takeover already in progress; a takeover that can't remove the lock still giving up; a relative state root; and 6 rounds of 6 derives plus a `--log-session` keeping every session. Each test failed on the code before its fix. + ## [1.92.68.0] - 2026-11-02 **`/health` now finds its tools with gstack's detector, the same one `/verify` uses, and a test proves it still finds every check it found before.** /health Step 1 used to carry its own shell block for detecting a type checker, linter, test runner, knip, shellcheck and gbrain, a second copy of the list in `lib/checks.ts`. It now runs `gstack-checks detect --for health`. Scoring, the gbrain sub-score and history are unchanged. GST-116, PR 2 of 2. diff --git a/VERSION b/VERSION index 75a21384c..d44ae96df 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.92.68.0 +1.92.69.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index f898c2f84..72187e385 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.92.68.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.92.69.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, diff --git a/bin/gstack-developer-profile b/bin/gstack-developer-profile index f64178e2b..bc489c5c1 100755 --- a/bin/gstack-developer-profile +++ b/bin/gstack-developer-profile @@ -25,6 +25,10 @@ # `completed` events in projects/*/timeline.jsonl. # Idempotent; backfilled rows count for tenure only. # +# Writers (--derive, --log-session, --reconcile, --migrate) hold +# developer-profile.json.lock for their read-modify-write and exit 5 when +# another writer keeps it past the give-up window (#3437). +# # Profile file: ~/.gstack/developer-profile.json (unified schema — see # docs/designs/PLAN_TUNING_V0.md). Event file: ~/.gstack/projects/{SLUG}/ # question-events.jsonl. @@ -37,6 +41,10 @@ ROOT_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" . "$(dirname "$0")/gstack-state-root.sh" 2>/dev/null || { echo "$0: cannot resolve the gstack state root: $(dirname "$0")/gstack-state-root.sh is missing. fix: reinstall with ./setup or /gstack-upgrade (docs/state-root.md)" >&2; exit 1; } gstack_state_root_select GSTACK_HOME="$_gstack_sr_root" +# A relative state root is anchored to the starting directory once, here: +# do_derive changes directory, and the profile lock must name the same path +# when it is taken and when it is released. +case "$GSTACK_HOME" in /*|[A-Za-z]:[\\/]*) ;; *) GSTACK_HOME="$PWD/$GSTACK_HOME" ;; esac # Windows git-bash: GSTACK_HOME resolves to an MSYS path (/c/Users/...), which # Bun on Windows cannot open as a filesystem path (bites --derive). Normalize # once, here, before PROFILE_FILE/LEGACY_FILE are derived from it below — @@ -52,10 +60,100 @@ SLUG="${SLUG:-unknown}" CMD="${1:---read}" shift || true +# ----------------------------------------------------------------------- +# Profile lock (#3437). Every subcommand that rewrites the profile reads, +# changes and renames the whole file, and gstack-question-log starts a +# background --derive after every append, so writers overlap: a shared temp +# name made renames fail and an unlocked rewrite dropped a concurrent +# --log-session. Writers hold a mkdir lock (.lock/, whose owner file +# names the holder's pid and a random token; release is ownership-checked) +# from before their read until after their rename, and write a unique temp +# file. A lock whose holder process is gone is taken over at once; one older +# than 5 minutes is taken over even when its pid is alive, since no writer +# runs that long and the pid may have been reused. Takeovers are serialized by a second mkdir lock +# (.lock.takeover/) and re-read the owner under it, so two waiters +# cannot both take over or remove a lock a third writer just took. A lock or +# takeover directory older than 30 s with no owner belongs to a writer that +# died before writing one; cleanup removes a takeover lock this writer holds. A writer that cannot get the lock within the +# give-up window writes nothing and exits 5. Writers outside this binary +# (gstack-distill-apply, /plan-tune's declared edits) do not take it yet. +# ----------------------------------------------------------------------- +LOCK_DIR="$PROFILE_FILE.lock" +TAKEOVER_DIR="$LOCK_DIR.takeover" +LOCK_TOKEN="" +HOLDS_TAKEOVER="" +PROFILE_TMP="" + +cleanup() { + if [ -n "$PROFILE_TMP" ]; then rm -f "$PROFILE_TMP"; fi + if [ -n "$HOLDS_TAKEOVER" ]; then rmdir "$TAKEOVER_DIR" 2>/dev/null || true; fi + if [ -n "$LOCK_TOKEN" ] && [ "$(cat "$LOCK_DIR/owner" 2>/dev/null || true)" = "$LOCK_TOKEN" ]; then + rm -rf "$LOCK_DIR" + fi +} +trap cleanup EXIT +trap 'exit 130' INT +trap 'exit 143' TERM + +dir_age_s() { + local mtime + # GNU stat first: BSD-style stat -f on Linux prints a filesystem block. + mtime=$(stat -c %Y "$1" 2>/dev/null || stat -f %m "$1" 2>/dev/null || echo "") + case "$mtime" in ""|*[!0-9]*) echo 0 ;; *) echo $(( $(date +%s) - mtime )) ;; esac +} + +lock_abandoned() { + if [ -n "$1" ]; then + ! kill -0 "${1%%-*}" 2>/dev/null || [ "$(dir_age_s "$LOCK_DIR")" -gt 300 ] + else + [ "$(dir_age_s "$LOCK_DIR")" -gt 30 ] + fi +} + +acquire_lock() { + if [ -n "$LOCK_TOKEN" ]; then return 0; fi + mkdir -p "$GSTACK_HOME" + # GSTACK_PROFILE_LOCK_TIMEOUT_MS: test-only override for the give-up. + local give_up_ms="${GSTACK_PROFILE_LOCK_TIMEOUT_MS:-10000}" waited_ms=0 started=$SECONDS vanished=0 owner + local token="$$-$RANDOM$RANDOM" + until mkdir "$LOCK_DIR" 2>/dev/null; do + # mkdir failed with no lock to wait for: retry once in case a holder just + # released it, then report the directory instead of waiting out the window. + if [ ! -e "$LOCK_DIR" ]; then + if [ "$vanished" -eq 0 ]; then vanished=1; continue; fi + echo "gstack-developer-profile: cannot create $LOCK_DIR (is $GSTACK_HOME writable?), so nothing was written (exit 5)." >&2 + exit 5 + fi + vanished=0 + owner=$(cat "$LOCK_DIR/owner" 2>/dev/null || true) + if [ -d "$LOCK_DIR" ] && lock_abandoned "$owner"; then + if [ -d "$TAKEOVER_DIR" ] && [ "$(dir_age_s "$TAKEOVER_DIR")" -gt 30 ]; then rmdir "$TAKEOVER_DIR" 2>/dev/null || true; fi + if mkdir "$TAKEOVER_DIR" 2>/dev/null; then + HOLDS_TAKEOVER=1 + if [ "$(cat "$LOCK_DIR/owner" 2>/dev/null || true)" = "$owner" ] && lock_abandoned "$owner"; then + rm -rf "$LOCK_DIR" 2>/dev/null || true + fi + rmdir "$TAKEOVER_DIR" 2>/dev/null || true + HOLDS_TAKEOVER="" + if [ ! -e "$LOCK_DIR" ]; then continue; fi + fi + fi + if [ "$waited_ms" -ge "$give_up_ms" ] || [ $(( SECONDS - started )) -gt $(( give_up_ms / 1000 )) ]; then + echo "gstack-developer-profile: another writer held $LOCK_DIR for ${give_up_ms} ms, so nothing was written (exit 5)." >&2 + exit 5 + fi + sleep 0.05 + waited_ms=$(( waited_ms + 50 )) + done + printf '%s\n' "$token" > "$LOCK_DIR/owner" + LOCK_TOKEN="$token" +} + # ----------------------------------------------------------------------- # Migration: builder-profile.jsonl → developer-profile.json # ----------------------------------------------------------------------- do_migrate() { + acquire_lock if [ ! -f "$LEGACY_FILE" ]; then echo "MIGRATE: no legacy file to migrate" return 0 @@ -68,9 +166,7 @@ do_migrate() { fi # Run migration in a temp file, then atomic rename. - local TMPOUT - TMPOUT=$(mktemp "$GSTACK_HOME/developer-profile.json.tmp.XXXXXX") - trap 'rm -f "$TMPOUT"' EXIT + PROFILE_TMP=$(mktemp "$GSTACK_HOME/developer-profile.json.tmp.XXXXXX") cat "$LEGACY_FILE" | bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " const lines = (await Bun.stdin.text()).trim().split('\n').filter(Boolean); @@ -113,11 +209,11 @@ do_migrate() { schema_version: 1, }; console.log(JSON.stringify(profile, null, 2)); - " > "$TMPOUT" + " > "$PROFILE_TMP" # Atomic rename. - mv "$TMPOUT" "$PROFILE_FILE" - trap - EXIT + mv "$PROFILE_TMP" "$PROFILE_FILE" + PROFILE_TMP="" # gbrain-sync: enqueue the migrated file for cross-machine sync (no-op if off). SCRIPT_DIR_E="$(cd "$(dirname "$0")" && pwd)" @@ -205,13 +301,12 @@ do_log_session() { *) echo "gstack-developer-profile: bun printed no verdict while validating the session; nothing was logged" >&2; return 1 ;; esac + acquire_lock ensure_profile - local TMPOUT - TMPOUT=$(mktemp "$GSTACK_HOME/developer-profile.json.tmp.XXXXXX") - trap 'rm -f "$TMPOUT"' EXIT + PROFILE_TMP=$(mktemp "$GSTACK_HOME/developer-profile.json.tmp.XXXXXX") - PROFILE_FILE_PATH="$PROFILE_FILE" RECORD_INPUT="$INPUT" TMPOUT_PATH="$TMPOUT" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " + PROFILE_FILE_PATH="$PROFILE_FILE" RECORD_INPUT="$INPUT" TMPOUT_PATH="$PROFILE_TMP" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " const fs = require('fs'); const entry = JSON.parse(process.env.RECORD_INPUT); if (!entry.ts) entry.ts = new Date().toISOString(); @@ -238,8 +333,8 @@ do_log_session() { fs.writeFileSync(process.env.TMPOUT_PATH, JSON.stringify(profile, null, 2)); " - mv "$TMPOUT" "$PROFILE_FILE" - trap - EXIT + mv "$PROFILE_TMP" "$PROFILE_FILE" + PROFILE_TMP="" "$SCRIPT_DIR/gstack-brain-enqueue" "developer-profile.json" 2>/dev/null & } @@ -252,13 +347,12 @@ do_log_session() { # rewritten only when something was added. # ----------------------------------------------------------------------- do_reconcile() { + acquire_lock ensure_profile - local TMPOUT - TMPOUT=$(mktemp "$GSTACK_HOME/developer-profile.json.tmp.XXXXXX") - trap 'rm -f "$TMPOUT"' EXIT + PROFILE_TMP=$(mktemp "$GSTACK_HOME/developer-profile.json.tmp.XXXXXX") - PROFILE_FILE_PATH="$PROFILE_FILE" PROJECTS_DIR="$GSTACK_HOME/projects" TMPOUT_PATH="$TMPOUT" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " + PROFILE_FILE_PATH="$PROFILE_FILE" PROJECTS_DIR="$GSTACK_HOME/projects" TMPOUT_PATH="$PROFILE_TMP" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " const fs = require('fs'); const path = require('path'); const profile = JSON.parse(fs.readFileSync(process.env.PROFILE_FILE_PATH, 'utf-8')); @@ -313,13 +407,13 @@ do_reconcile() { console.log('RECONCILE: ' + added + ' backfilled from ' + runs.length + ' office-hours run(s)'); " - if [ -s "$TMPOUT" ]; then - mv "$TMPOUT" "$PROFILE_FILE" - trap - EXIT + if [ -s "$PROFILE_TMP" ]; then + mv "$PROFILE_TMP" "$PROFILE_FILE" + PROFILE_TMP="" "$SCRIPT_DIR/gstack-brain-enqueue" "developer-profile.json" 2>/dev/null & else - rm -f "$TMPOUT" - trap - EXIT + rm -f "$PROFILE_TMP" + PROFILE_TMP="" fi } @@ -506,6 +600,7 @@ function accumulateSignals(lines, QUESTIONS, SIGNAL_MAP, dims) { # Derive: recompute inferred dimensions from question-events.jsonl # ----------------------------------------------------------------------- do_derive() { + acquire_lock ensure_profile local EVENTS="$GSTACK_HOME/projects/$SLUG/question-log.jsonl" local REGISTRY="$ROOT_DIR/scripts/question-registry.ts" @@ -515,8 +610,9 @@ do_derive() { exit 1 fi + PROFILE_TMP=$(mktemp "$GSTACK_HOME/developer-profile.json.tmp.XXXXXX") cd "$ROOT_DIR" - PROFILE_FILE_PATH="$PROFILE_FILE" EVENTS_PATH="$EVENTS" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " + PROFILE_FILE_PATH="$PROFILE_FILE" EVENTS_PATH="$EVENTS" TMPOUT_PATH="$PROFILE_TMP" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " import('./scripts/question-registry.ts').then(async (regmod) => { const sigmod = await import('./scripts/psychographic-signals.ts'); const fs = require('fs'); @@ -568,12 +664,12 @@ do_derive() { profile.gap = gap; profile.derived_at = new Date().toISOString(); - const tmp = profilePath + '.tmp'; - fs.writeFileSync(tmp, JSON.stringify(profile, null, 2)); - fs.renameSync(tmp, profilePath); + fs.writeFileSync(process.env.TMPOUT_PATH, JSON.stringify(profile, null, 2)); console.log('DERIVE: ok — ' + count + ' events, ' + skills.size + ' skills, ' + qids.size + ' questions'); }).catch(err => { console.error('DERIVE:', err.message); process.exit(1); }); " + mv "$PROFILE_TMP" "$PROFILE_FILE" + PROFILE_TMP="" } # ----------------------------------------------------------------------- diff --git a/package.json b/package.json index 883d131b3..766188b87 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.92.68", + "version": "1.92.69", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/scripts/psychographic-signals.ts b/scripts/psychographic-signals.ts index 43d4a89ab..d6a479497 100644 --- a/scripts/psychographic-signals.ts +++ b/scripts/psychographic-signals.ts @@ -306,29 +306,6 @@ export function resolveOptionKey(question_id: string, choice: string): string | return tie ? undefined : best; } -/** - * Apply a user choice for a question to the running dimension totals. - * - * @param dims - running total of dimension nudges (mutated) - * @param signal_key - from the question registry entry - * @param user_choice - the option key the user selected - * @returns list of dimension deltas applied (empty if no mapping) - */ -export function applySignal( - dims: Record, - signal_key: string, - user_choice: string, -): DimensionDelta[] { - const subMap = SIGNAL_MAP[signal_key]; - if (!subMap) return []; - const deltas = subMap[user_choice]; - if (!deltas) return []; - for (const { dim, delta } of deltas) { - dims[dim] = (dims[dim] ?? 0) + delta; - } - return deltas; -} - /** * Validate that every signal_key referenced in the registry has a matching * entry in SIGNAL_MAP. Called by tests to catch drift. @@ -353,17 +330,6 @@ export function validateRegistrySignalKeys(): { return { missing, extra }; } -/** Empty dimension totals — starting point for derivation. */ -export function newDimensionTotals(): Record { - return { - scope_appetite: 0, - risk_tolerance: 0, - detail_preference: 0, - autonomy: 0, - architecture_care: 0, - }; -} - /** Sigmoid clamp: map accumulated delta total to [0, 1]. */ export function normalizeToDimensionValue(total: number): number { // Simple sigmoid: each 1.0 of accumulated delta approaches saturation. diff --git a/test/gstack-developer-profile.test.ts b/test/gstack-developer-profile.test.ts index 46a945508..df23a9c3b 100644 --- a/test/gstack-developer-profile.test.ts +++ b/test/gstack-developer-profile.test.ts @@ -770,6 +770,155 @@ describe('gstack-developer-profile --log-session (#1671 fix)', () => { }); }); +// ----------------------------------------------------------------------- +// Concurrent writers (#3437). gstack-question-log starts a background +// --derive after every append, and a multi-question AskUserQuestion logs +// several rows at once, so derives overlap each other and /office-hours' +// --log-session. Each subcommand rewrites the whole profile, so every +// read-modify-write holds the profile lock and writes a unique temp file. +// ----------------------------------------------------------------------- + +async function runDevAsync(...args: string[]): Promise<{ stdout: string; stderr: string; status: number }> { + const proc = Bun.spawn([BIN_DEV, ...args], { + env: { ...process.env, GSTACK_HOME: tmpHome }, + cwd: ROOT, + stdout: 'pipe', + stderr: 'pipe', + }); + const timer = setTimeout(() => proc.kill(), 60_000); + const [stdout, stderr, status] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + clearTimeout(timer); + return { stdout, stderr, status }; +} + +describe('gstack-developer-profile concurrent writers (#3437)', () => { + const lockDir = () => path.join(tmpHome, 'developer-profile.json.lock'); + + test('--derive does not write through a fixed temp path', () => { + fs.mkdirSync(path.join(tmpHome, 'developer-profile.json.tmp')); + const r = runDev('--derive'); + expect(r.stderr).toBe(''); + expect(r.status).toBe(0); + expect(typeof (readProfile() as { derived_at?: string }).derived_at).toBe('string'); + }); + + test('a held profile lock keeps --derive and --log-session from writing', () => { + expect(runDev('--derive').status).toBe(0); + const before = fs.readFileSync(path.join(tmpHome, 'developer-profile.json'), 'utf-8'); + fs.mkdirSync(lockDir()); + fs.writeFileSync(path.join(lockDir(), 'owner'), process.pid + '-held\n'); + const env = { ...process.env, GSTACK_HOME: tmpHome, GSTACK_PROFILE_LOCK_TIMEOUT_MS: '300' }; + for (const args of [['--derive'], ['--log-session', JSON.stringify({ date: '2026-10-31', mode: 'builder' })]]) { + const res = spawnSync(BIN_DEV, args, { env, encoding: 'utf-8', cwd: ROOT, timeout: 30_000 }); + expect(res.status).toBe(5); + expect(res.stderr).toContain('developer-profile.json.lock'); + } + expect(fs.readFileSync(path.join(tmpHome, 'developer-profile.json'), 'utf-8')).toBe(before); + expect(fs.readFileSync(path.join(lockDir(), 'owner'), 'utf-8')).toBe(process.pid + '-held\n'); + }); + + test('a stale lock left by a crashed writer is taken over', () => { + fs.mkdirSync(lockDir()); + const old = new Date(Date.now() - 120_000); + fs.utimesSync(lockDir(), old, old); + const r = runDev('--log-session', JSON.stringify({ date: '2026-10-31', mode: 'builder', project_slug: 'p' })); + expect(r.status).toBe(0); + expect((readProfile() as { sessions: unknown[] }).sessions.length).toBe(1); + expect(fs.existsSync(lockDir())).toBe(false); + }); + + test('a lock whose holder process is gone is taken over at once', () => { + const gone = spawnSync('true', [], { timeout: 10_000 }).pid; + fs.mkdirSync(lockDir()); + fs.writeFileSync(path.join(lockDir(), 'owner'), gone + '-crashed\n'); + const env = { ...process.env, GSTACK_HOME: tmpHome, GSTACK_PROFILE_LOCK_TIMEOUT_MS: '300' }; + const res = spawnSync(BIN_DEV, ['--log-session', JSON.stringify({ date: '2026-10-31', mode: 'builder', project_slug: 'p' })], { env, encoding: 'utf-8', cwd: ROOT, timeout: 30_000 }); + expect(res.stderr).toBe(''); + expect(res.status).toBe(0); + expect((readProfile() as { sessions: unknown[] }).sessions.length).toBe(1); + expect(fs.existsSync(lockDir())).toBe(false); + }); + + test('a lock older than 5 minutes is taken over even when its pid is alive (pid reuse)', () => { + fs.mkdirSync(lockDir()); + fs.writeFileSync(path.join(lockDir(), 'owner'), process.pid + '-reused\n'); + const old = new Date(Date.now() - 10 * 60_000); + fs.utimesSync(lockDir(), old, old); + const env = { ...process.env, GSTACK_HOME: tmpHome, GSTACK_PROFILE_LOCK_TIMEOUT_MS: '300' }; + const res = spawnSync(BIN_DEV, ['--log-session', JSON.stringify({ date: '2026-10-31', mode: 'builder', project_slug: 'p' })], { env, encoding: 'utf-8', cwd: ROOT, timeout: 30_000 }); + expect(res.stderr).toBe(''); + expect(res.status).toBe(0); + expect((readProfile() as { sessions: unknown[] }).sessions.length).toBe(1); + expect(fs.existsSync(lockDir())).toBe(false); + }); + + test('a takeover already in progress is not repeated', () => { + const gone = spawnSync('true', [], { timeout: 10_000 }).pid; + fs.mkdirSync(lockDir()); + fs.writeFileSync(path.join(lockDir(), 'owner'), gone + '-crashed\n'); + fs.mkdirSync(lockDir() + '.takeover'); + const env = { ...process.env, GSTACK_HOME: tmpHome, GSTACK_PROFILE_LOCK_TIMEOUT_MS: '300' }; + const res = spawnSync(BIN_DEV, ['--log-session', JSON.stringify({ date: '2026-10-31', mode: 'builder' })], { env, encoding: 'utf-8', cwd: ROOT, timeout: 30_000 }); + expect(res.status).toBe(5); + expect(fs.readFileSync(path.join(lockDir(), 'owner'), 'utf-8')).toBe(gone + '-crashed\n'); + }); + + test('a takeover that cannot remove the lock still gives up', () => { + expect(runDev('--derive').status).toBe(0); + fs.mkdirSync(lockDir()); + const old = new Date(Date.now() - 120_000); + fs.utimesSync(lockDir(), old, old); + fs.chmodSync(tmpHome, 0o555); + try { + const env = { ...process.env, GSTACK_HOME: tmpHome, GSTACK_PROFILE_LOCK_TIMEOUT_MS: '300' }; + const res = spawnSync(BIN_DEV, ['--derive'], { env, encoding: 'utf-8', cwd: ROOT, timeout: 10_000 }); + expect(res.signal).toBeNull(); + expect([0, 5]).toContain(res.status ?? -1); + } finally { + fs.chmodSync(tmpHome, 0o755); + } + }, 20_000); + + test('a relative state root does not leave the lock behind', () => { + const env = { ...process.env, GSTACK_HOME: 'state' }; + for (const args of [['--derive'], ['--log-session', JSON.stringify({ date: '2026-10-31', mode: 'builder', project_slug: 'p' })]]) { + const res = spawnSync(BIN_DEV, args, { env, encoding: 'utf-8', cwd: tmpHome, timeout: 30_000 }); + expect(res.stderr).toBe(''); + expect(res.status).toBe(0); + } + const p = JSON.parse(fs.readFileSync(path.join(tmpHome, 'state', 'developer-profile.json'), 'utf-8')); + expect(p.sessions.length).toBe(1); + expect(typeof p.derived_at).toBe('string'); + expect(fs.existsSync(path.join(tmpHome, 'state', 'developer-profile.json.lock'))).toBe(false); + }); + + test('overlapping derives and log-sessions keep every session and leave valid JSON', async () => { + for (let i = 0; i < 5; i++) { + logQuestion({ skill: 'plan-ceo-review', question_id: 'plan-ceo-review-mode', question_summary: 'mode', user_choice: 'expand', session_id: 's' + i }); + } + expect(runDev('--derive').status).toBe(0); + const rounds = 6; + for (let round = 0; round < rounds; round++) { + const runs = await Promise.all([ + ...Array.from({ length: 6 }, () => runDevAsync('--derive')), + runDevAsync('--log-session', JSON.stringify({ date: '2026-10-31', mode: 'builder', project_slug: 'round-' + round })), + ]); + for (const r of runs) { + expect(r.stderr).toBe(''); + expect(r.status).toBe(0); + } + } + const p = readProfile() as { sessions: Array<{ project_slug: string }>; inferred: { sample_size: number } }; + expect(p.sessions.map((s) => s.project_slug).sort()).toEqual(Array.from({ length: rounds }, (_, i) => 'round-' + i).sort()); + expect(p.inferred.sample_size).toBe(5); + expect(fs.readdirSync(tmpHome).filter((f) => f.startsWith('developer-profile.json.'))).toEqual([]); + }, 120_000); +}); + // ----------------------------------------------------------------------- // SESSION_COUNT / TIER / NUDGE_ELIGIBLE must ignore mode:resources entries. // diff --git a/test/plan-tune.test.ts b/test/plan-tune.test.ts index 413ed07f9..467ec8b91 100644 --- a/test/plan-tune.test.ts +++ b/test/plan-tune.test.ts @@ -29,9 +29,7 @@ import { } from '../scripts/one-way-doors'; import { SIGNAL_MAP, - applySignal, validateRegistrySignalKeys, - newDimensionTotals, normalizeToDimensionValue, resolveOptionKey, ALL_DIMENSIONS, @@ -246,26 +244,6 @@ describe('psychographic signal map', () => { expect(missing).toEqual([]); }); - test('applySignal mutates dimension totals per mapping', () => { - const dims = newDimensionTotals(); - const applied = applySignal(dims, 'scope-appetite', 'expand'); - expect(applied.length).toBeGreaterThan(0); - expect(dims.scope_appetite).toBeCloseTo(0.06, 5); - }); - - test('applySignal returns [] for unknown signal_key', () => { - const dims = newDimensionTotals(); - const applied = applySignal(dims, 'no-such-signal', 'anything'); - expect(applied).toEqual([]); - expect(dims.scope_appetite).toBe(0); - }); - - test('applySignal returns [] for unknown user_choice', () => { - const dims = newDimensionTotals(); - const applied = applySignal(dims, 'scope-appetite', 'definitely-not-a-real-choice'); - expect(applied).toEqual([]); - }); - test('normalizeToDimensionValue maps 0 → 0.5 (neutral)', () => { expect(normalizeToDimensionValue(0)).toBeCloseTo(0.5, 5); }); From cd788faec718b60fefe38e1694fccc8b17775a43 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 2 Nov 2026 01:46:47 -0800 Subject: [PATCH 4/5] v1.92.70.0 test(free): build git fixtures once per file and copy them per test (GSTA-151 C) (#3586) * test(free): build git fixtures once per file and copy them per test test/helpers/git-fixture-template.ts builds a template once, copies it per test, rebinds every path-bearing file and fails on a leaked template path. Adopted by release-cut, train-backstop, restamp-mentions, pregate-user-policy and held-install-recovery; restamp-fixture's origin turns off auto-maintenance. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test(free): baseline the git fixture template regression test in the test-of-test ratchet Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(changelog): unreleased entry for the git fixture templates (GSTA-151 C) --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 10 +++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- package.json | 2 +- test/git-fixture-template.test.ts | 97 +++++++++++++++++++++++++ test/held-install-recovery.test.ts | 33 ++++++++- test/helpers/git-fixture-template.ts | 105 +++++++++++++++++++++++++++ test/helpers/pregate-fixture.ts | 36 +++++++-- test/helpers/restamp-fixture.ts | 40 ++++++++-- test/pregate-user-policy.test.ts | 8 +- test/release-cut.test.ts | 59 ++++++++++----- test/restamp-mentions.test.ts | 14 +++- test/test-of-test-ratchet.test.ts | 1 + test/train-backstop.test.ts | 22 +++++- 14 files changed, 385 insertions(+), 46 deletions(-) create mode 100644 test/git-fixture-template.test.ts create mode 100644 test/helpers/git-fixture-template.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 07112e9ea..09fff87eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## [1.92.70.0] - 2026-11-02 + +**Five free-test files stop rebuilding the same git repositories for every test: they spend 17–24 s less in summed per-file runtime, with every assertion intact.** Each test still gets its own private copy of the fixture. + +### Tests +- `test/helpers/git-fixture-template.ts` (new): builds a git fixture once per test file and copies it per test with `fs.cpSync`. The adopter rewrites every path-bearing file (remote URLs, alternates, state roots), and the copy is then scanned for any remaining reference to the template, so a test can never push into or read from the shared template. `test/git-fixture-template.test.ts` pins two-copy isolation and the leak scan. +- `test/release-cut.test.ts` (~10 s to ~6 s), `test/train-backstop.test.ts` (~4 s to ~1.6 s), `test/restamp-mentions.test.ts` (via `test/helpers/restamp-fixture.ts`), `test/pregate-user-policy.test.ts` (via `test/helpers/pregate-fixture.ts`) and `test/held-install-recovery.test.ts` (~33 s to ~22 s) use the template. Test and `expect()` counts are unchanged in every file. +- `test/helpers/restamp-fixture.ts` turns off auto-maintenance (`maintenance.auto=false`, `gc.auto=0`) in the bare origin the ship-queue train tests push to. +- `test/test-of-test-ratchet.test.ts`: one BASELINE row for the new helper test, which tests a test helper by design. + ## [1.92.69.0] - 2026-11-02 **Background `--derive` runs no longer drop /office-hours sessions or fail with ENOENT when they overlap (#3437).** `gstack-question-log` starts a background `gstack-developer-profile --derive` after every append, and a multi-question AskUserQuestion logs several rows at once. Every derive wrote the same `developer-profile.json.tmp` and rewrote the whole profile without a lock. On main at 3bfe566, 30 rounds of 8 overlapping derives plus one `--log-session` saw 9 of 270 runs fail with `ENOENT` on rename, and 2 of the 30 sessions disappeared. With this change the same run has 0 failures and keeps 30 of 30 sessions. diff --git a/VERSION b/VERSION index d44ae96df..9dc23d34a 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.92.69.0 +1.92.70.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 72187e385..1fe114a87 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.92.69.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.92.70.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, diff --git a/package.json b/package.json index 766188b87..15a0b5f64 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.92.69", + "version": "1.92.70", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/test/git-fixture-template.test.ts b/test/git-fixture-template.test.ts new file mode 100644 index 000000000..76375466c --- /dev/null +++ b/test/git-fixture-template.test.ts @@ -0,0 +1,97 @@ +/** + * test/helpers/git-fixture-template.ts: copies made from one template are + * independent repositories (a push in one never reaches the other or the + * template), and a copy whose rebind misses a path that names the template + * root, in any of its spellings, throws instead of reaching a test. + */ +import { afterAll, afterEach, describe, expect, test } from 'bun:test'; +import { execFileSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { gitFixtureTemplate, pathSpellings, rebindFile, rebindGitDir, templateReferences } from './helpers/git-fixture-template'; + +const roots: string[] = []; +afterEach(() => { for (const r of roots.splice(0)) fs.rmSync(r, { recursive: true, force: true }); }); + +function git(cwd: string, ...args: string[]): string { + return execFileSync('git', ['-c', 'user.name=t', '-c', 'user.email=t@t', '-c', 'commit.gpgsign=false', ...args], { cwd, encoding: 'utf8', timeout: 30_000 }).trim(); +} + +function build(root: string): { head: string } { + const origin = path.join(root, 'origin.git'); + const work = path.join(root, 'work'); + execFileSync('git', ['init', '-q', '--bare', '-b', 'main', origin], { timeout: 30_000 }); + fs.mkdirSync(work); + git(work, 'init', '-q', '-b', 'main'); + git(work, 'remote', 'add', 'origin', origin); + fs.writeFileSync(path.join(work, 'a.txt'), 'base\n'); + git(work, 'add', '-A'); + git(work, 'commit', '-q', '-m', 'base'); + git(work, 'push', '-q', 'origin', 'main'); + return { head: git(work, 'rev-parse', 'HEAD') }; +} + +const template = gitFixtureTemplate({ prefix: 'gstack-fixture-template-', build, rebind: (copy, tpl) => rebindGitDir(path.join(copy, 'work', '.git'), tpl, copy) }); +const leaky = gitFixtureTemplate({ prefix: 'gstack-fixture-template-leak-', build, rebind: () => {} }); +afterAll(() => { template.dispose(); leaky.dispose(); }); + +function copy() { + const c = template.copy(); + roots.push(c.root); + return { ...c, work: path.join(c.root, 'work'), origin: path.join(c.root, 'origin.git') }; +} +function pushChange(c: ReturnType, text: string): string { + fs.writeFileSync(path.join(c.work, 'a.txt'), text); + git(c.work, 'commit', '-q', '-am', text); + git(c.work, 'push', '-q', 'origin', 'main'); + return git(c.work, 'rev-parse', 'HEAD'); +} + +describe('git fixture template', () => { + test('two copies from one template mutated independently stay isolated from each other and from the template', () => { + const a = copy(); + const b = copy(); + expect(git(a.work, 'remote', 'get-url', 'origin')).toBe(a.origin); + expect(git(b.work, 'remote', 'get-url', 'origin')).toBe(b.origin); + const fromA = pushChange(a, 'from a\n'); + expect(git(a.origin, 'rev-parse', 'main')).toBe(fromA); + expect(git(b.origin, 'rev-parse', 'main')).toBe(b.meta.head); + const fromB = pushChange(b, 'from b\n'); + expect(fromB).not.toBe(fromA); + expect(git(a.origin, 'rev-parse', 'main')).toBe(fromA); + expect(git(b.origin, 'rev-parse', 'main')).toBe(fromB); + expect(fs.readFileSync(path.join(a.work, 'a.txt'), 'utf8')).toBe('from a\n'); + const c = copy(); + expect(git(c.origin, 'rev-parse', 'main')).toBe(c.meta.head); + expect(fs.readFileSync(path.join(c.work, 'a.txt'), 'utf8')).toBe('base\n'); + }); + + test('a copy whose rebind leaves the remote URL on the template throws and names the file', () => { + expect(() => leaky.copy()).toThrow(/still references its template [^\n]*\n.*config/); + }); + + test('native, forward-slash, backslash-escaped and Git Bash spellings are each rewritten, and each one left behind is found', () => { + const to = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-fixture-spellings-')); + roots.push(to); + const from = 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\gstack-fixture-tpl-AbC123'; + const spellings = pathSpellings(from); + expect(spellings).toEqual([ + 'C:\\\\Users\\\\RUNNER~1\\\\AppData\\\\Local\\\\Temp\\\\gstack-fixture-tpl-AbC123', + from, + 'C:/Users/RUNNER~1/AppData/Local/Temp/gstack-fixture-tpl-AbC123', + '/c/Users/RUNNER~1/AppData/Local/Temp/gstack-fixture-tpl-AbC123', + ]); + const file = path.join(to, 'config'); + fs.writeFileSync(file, spellings.map(s => 'url = ' + s + '/origin.git\n').join('')); + expect(templateReferences(to, from)).toEqual([file]); + rebindFile(file, from, to); + expect(fs.readFileSync(file, 'utf8')).toBe(spellings.map((_, i) => 'url = ' + (pathSpellings(to)[i] ?? to) + '/origin.git\n').join('')); + expect(templateReferences(to, from)).toEqual([]); + for (const s of spellings) { + fs.writeFileSync(file, 'url = ' + s + '\n'); + expect(templateReferences(to, from)).toEqual([file]); + } + expect(templateReferences(to, from, { origin: from + '\\origin.git' })).toEqual([file, '(template meta)']); + }); +}); diff --git a/test/held-install-recovery.test.ts b/test/held-install-recovery.test.ts index 3604370e2..5e16f97be 100644 --- a/test/held-install-recovery.test.ts +++ b/test/held-install-recovery.test.ts @@ -8,11 +8,12 @@ * already-unmerged), all in prefix mode; the PowerShell forms run under * powershell.exe in .github/workflows/windows-setup-e2e.yml. */ -import { afterEach, describe, expect, test } from 'bun:test'; +import { afterAll, afterEach, describe, expect, test } from 'bun:test'; import { execFileSync, spawn, spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; +import { gitFixtureTemplate } from './helpers/git-fixture-template'; const ROOT = path.resolve(import.meta.dir, '..'); const DOC = fs.readFileSync(path.join(ROOT, 'docs', 'troubleshooting.md'), 'utf8'); @@ -37,8 +38,33 @@ const CONFLICT_LINE = 'THROTTLE_SECONDS=3600 # 1 hour'; * bin/gstack-ff-update (an install that predates it), with VERSION 1.91.101.0. */ function makeFixture(source = ROOT) { - const base = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-held-'))); + if (source !== ROOT) { + const base = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-held-'))); + bases.push(base); + const { src, ...fx } = buildFixture(base, source); + expect(src).toContain(CONFLICT_LINE); + return { base, ...fx }; + } + const { root: base, meta: { src, ...fx } } = rootTemplate.copy(); bases.push(base); + expect(src).toContain(CONFLICT_LINE); + return { base, origin: path.join(base, 'origin.git'), ...fx }; +} + +/** makeFixture's origin.git for this checkout, built once per file; the per-test source checkouts build their own. */ +const rootTemplate = gitFixtureTemplate({ + prefix: 'gstack-held-', + realpath: true, + build: (base) => { + const { origin, ...meta } = buildFixture(base, ROOT); + fs.rmSync(path.join(base, 'work'), { recursive: true, force: true }); + return meta; + }, + rebind: () => {}, +}); +afterAll(() => rootTemplate.dispose()); + +function buildFixture(base: string, source: string) { const origin = path.join(base, 'origin.git'); const work = path.join(base, 'work'); execFileSync('git', ['clone', '-q', '--shared', source, work], { timeout: 60_000 }); @@ -53,7 +79,6 @@ function makeFixture(source = ROOT) { const held = git(work, 'rev-parse', 'HEAD'); fs.copyFileSync(path.join(ROOT, 'bin', 'gstack-ff-update'), path.join(work, 'bin', 'gstack-ff-update')); const src = fs.readFileSync(path.join(work, EDITED), 'utf8'); - expect(src).toContain(CONFLICT_LINE); fs.writeFileSync(path.join(work, EDITED), src.replace(CONFLICT_LINE, 'THROTTLE_SECONDS=3600 # 1 hour (upstream)')); fs.writeFileSync(path.join(work, 'VERSION'), '1.91.102.0\n'); git(work, 'add', '-A'); @@ -61,7 +86,7 @@ function makeFixture(source = ROOT) { const fixed = git(work, 'rev-parse', 'HEAD'); execFileSync('git', ['init', '-q', '--bare', '-b', 'main', origin], { timeout: 60_000 }); git(work, 'push', '-q', origin, 'HEAD:refs/heads/main'); - return { base, origin, held, fixed }; + return { origin, held, fixed, src }; } type Fx = ReturnType; diff --git a/test/helpers/git-fixture-template.ts b/test/helpers/git-fixture-template.ts new file mode 100644 index 000000000..a646747ab --- /dev/null +++ b/test/helpers/git-fixture-template.ts @@ -0,0 +1,105 @@ +/** + * Build a git fixture once per test file and hand each test its own copy. + * + * build(root) runs once, on the first copy(), into a template directory under + * os.tmpdir(). Every copy() makes a fresh sibling directory, copies the + * template into it with fs.cpSync (symlinks kept verbatim) and calls + * rebind(copyRoot, templateRoot), which must rewrite every path-bearing file + * the fixture wrote: remote URLs and pushurls in .git/config, reflog messages, + * FETCH_HEAD, objects/info/alternates, state roots, install records. The copy + * is then scanned for any remaining reference to the template root (native, + * forward-slash and backslash-escaped spellings, plus the realpath and Git + * Bash forms where they differ); a hit throws, so a test can never push into, + * fetch from or read state out of the shared template. dispose() removes the + * template; call it from afterAll. + */ +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; + +export interface GitFixtureCopy { root: string; meta: T } +export interface GitFixtureTemplate { copy: (prefix?: string) => GitFixtureCopy; dispose: () => void } + +/** The spellings a path takes inside files git and gstack write, escaped form first so a rewrite never splits it. */ +export function pathSpellings(root: string): string[] { + const forms = (p: string) => { + const forward = p.replace(/\\/g, '/'); + const drive = /^([A-Za-z]):\//.exec(forward); + return [p.replace(/\\/g, '\\\\'), p, forward, ...(drive ? ['/' + drive[1]!.toLowerCase() + '/' + forward.slice(3)] : [])]; + }; + let real = root; + try { real = fs.realpathSync.native(root); } catch {} + return [...new Set([...forms(root), ...forms(real)])]; +} + +/** Rewrite every spelling of from to the same spelling of to in a text file, when the file exists. */ +export function rebindFile(file: string, from: string, to: string): void { + if (!fs.existsSync(file)) return; + const targets = pathSpellings(to); + let text = fs.readFileSync(file, 'utf8'); + const before = text; + pathSpellings(from).forEach((spelling, i) => { text = text.split(spelling).join(targets[i] ?? to); }); + if (text !== before) fs.writeFileSync(file, text); +} + +/** Rebind the path-bearing files git writes in a git directory: config, FETCH_HEAD, alternates and every reflog. */ +export function rebindGitDir(gitDir: string, from: string, to: string): void { + for (const rel of ['config', 'FETCH_HEAD', path.join('objects', 'info', 'alternates')]) rebindFile(path.join(gitDir, rel), from, to); + const logs = path.join(gitDir, 'logs'); + if (!fs.existsSync(logs)) return; + for (const entry of fs.readdirSync(logs, { recursive: true, withFileTypes: true })) { + if (entry.isFile()) rebindFile(path.join(entry.parentPath, entry.name), from, to); + } +} + +/** Files and symlinks under dir (and the template's meta value) that still name templateRoot; zlib-compressed object storage is skipped. */ +export function templateReferences(dir: string, templateRoot: string, meta?: unknown): string[] { + const needles = pathSpellings(templateRoot).map(s => Buffer.from(s)); + const names = (buf: Buffer) => needles.some(n => buf.includes(n)); + const hits: string[] = []; + const walk = (d: string) => { + for (const entry of fs.readdirSync(d, { withFileTypes: true })) { + const p = path.join(d, entry.name); + if (entry.isSymbolicLink()) { if (names(Buffer.from(fs.readlinkSync(p)))) hits.push(p); } + else if (entry.isDirectory()) { if (!(path.basename(d) === 'objects' && /^(pack|[0-9a-f]{2})$/.test(entry.name))) walk(p); } + else if (entry.isFile() && names(fs.readFileSync(p))) hits.push(p); + } + }; + walk(dir); + if (meta !== undefined && names(Buffer.from(JSON.stringify(meta)))) hits.push('(template meta)'); + return hits; +} + +export function gitFixtureTemplate(opts: { + prefix: string; + build: (root: string) => T; + rebind: (copyRoot: string, templateRoot: string) => void; + realpath?: boolean; +}): GitFixtureTemplate { + let template: GitFixtureCopy | null = null; + const mkroot = (prefix: string) => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + return opts.realpath ? fs.realpathSync(dir) : dir; + }; + return { + copy(prefix = opts.prefix) { + if (!template) { + const root = mkroot(opts.prefix + 'tpl-'); + template = { root, meta: opts.build(root) }; + } + const { root: from, meta } = template; + const root = mkroot(prefix); + if (pathSpellings(root).some(s => pathSpellings(from).some(t => s.startsWith(t)))) throw new Error('git fixture copy ' + root + ' lies under its template ' + from); + fs.cpSync(from, root, { recursive: true, verbatimSymlinks: true }); + opts.rebind(root, from); + const leaks = templateReferences(root, from, meta); + if (leaks.length) fs.rmSync(root, { recursive: true, force: true }); + if (leaks.length) throw new Error('git fixture copy still references its template ' + from + ' (rebind missed them):\n' + leaks.join('\n')); + return { root, meta }; + }, + dispose() { + if (template) fs.rmSync(template.root, { recursive: true, force: true }); + template = null; + }, + }; +} diff --git a/test/helpers/pregate-fixture.ts b/test/helpers/pregate-fixture.ts index 94d0b3ad3..baef6855e 100644 --- a/test/helpers/pregate-fixture.ts +++ b/test/helpers/pregate-fixture.ts @@ -14,6 +14,7 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { runScriptSync } from '../../lib/script-invocation'; +import { gitFixtureTemplate, rebindGitDir, type GitFixtureTemplate } from './git-fixture-template'; import { gitArgvIn } from './scratch-repo'; export const ROOT = path.resolve(import.meta.dir, '..', '..'); @@ -80,20 +81,46 @@ jobs: run: node --test \${{ matrix.files }} `; -export function makeFixtureRepo(opts: { registry?: Record | null; pregate?: Record | null; prefix?: string; baseFiles?: Record } = {}): FixtureRepo { +export type FixtureOptions = { registry?: Record | null; pregate?: Record | null; prefix?: string; baseFiles?: Record }; + +export function makeFixtureRepo(opts: FixtureOptions = {}): FixtureRepo { const root = fs.mkdtempSync(path.join(os.tmpdir(), opts.prefix ?? 'gstack-pregate-fx-')); + seedFixtureRepo(root, opts); + return fixtureRepoAt(root); +} + +/** A template of makeFixtureRepo(opts), built once and copied per test (helpers/git-fixture-template.ts). */ +export function fixtureTemplate(opts: FixtureOptions = {}): GitFixtureTemplate { + return gitFixtureTemplate({ + prefix: opts.prefix ?? 'gstack-pregate-fx-', + build: root => { seedFixtureRepo(root, opts); return null; }, + rebind: (copy, tpl) => rebindGitDir(path.join(copy, 'work', '.git'), tpl, copy), + }); +} + +/** A fresh copy of a fixtureTemplate as its own FixtureRepo (cleanup() removes it like any other). */ +export function cloneFixture(template: GitFixtureTemplate, prefix?: string): FixtureRepo { + return fixtureRepoAt(template.copy(prefix).root); +} + +function fixtureRepoAt(root: string): FixtureRepo { const origin = path.join(root, 'origin.git'); const work = path.join(root, 'work'); const home = path.join(root, 'home'); - fs.mkdirSync(home, { recursive: true }); - fs.mkdirSync(work, { recursive: true }); const env = { GSTACK_HOME: home, GSTACK_STATE_DIR: home, HOME: home, GIT_CONFIG_GLOBAL: path.join(root, 'gitconfig'), GIT_CONFIG_NOSYSTEM: '1' }; - fs.writeFileSync(env.GIT_CONFIG_GLOBAL, '[user]\n\tname = t\n\temail = t@test\n[commit]\n\tgpgsign = false\n[init]\n\tdefaultBranch = main\n'); const git = (...args: string[]): string => { const r = gitArgvIn(work, args, 30_000, { ...process.env, ...env }); if (r.status !== 0) throw new Error(`git ${args.join(' ')} failed: status=${r.status} signal=${r.signal} error=${r.error?.message ?? ''} stderr=${r.stderr?.toString() ?? ''}`); return r.stdout?.toString().trim() ?? ''; }; + return { root, work, origin, home, git, env }; +} + +function seedFixtureRepo(root: string, opts: FixtureOptions): void { + const { origin, work, home, env, git } = fixtureRepoAt(root); + fs.mkdirSync(home, { recursive: true }); + fs.mkdirSync(work, { recursive: true }); + fs.writeFileSync(env.GIT_CONFIG_GLOBAL, '[user]\n\tname = t\n\temail = t@test\n[commit]\n\tgpgsign = false\n[init]\n\tdefaultBranch = main\n'); spawnSync('git', ['init', '-q', '--bare', origin], { timeout: 30_000, env: { ...process.env, ...env } }); git('init', '-q', '-b', 'main'); for (const dir of ['src', 'lib', 'bin', 'test', 'test/golden', 'test/nightly', 'generated', 'docs', 'scripts', '.gstack', '.github/workflows']) fs.mkdirSync(path.join(work, dir), { recursive: true }); @@ -130,7 +157,6 @@ export function makeFixtureRepo(opts: { registry?: Record | nul git('push', '-q', 'origin', 'main'); git('remote', 'set-head', 'origin', 'main'); git('checkout', '-q', '-b', 'feature'); - return { root, work, origin, home, git, env }; } export function write(work: string, rel: string, text: string): void { diff --git a/test/helpers/restamp-fixture.ts b/test/helpers/restamp-fixture.ts index 2b6366c99..6ef5b188f 100644 --- a/test/helpers/restamp-fixture.ts +++ b/test/helpers/restamp-fixture.ts @@ -11,6 +11,7 @@ import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; +import { gitFixtureTemplate, rebindGitDir, type GitFixtureTemplate } from './git-fixture-template'; import { gitArgvIn } from './scratch-repo'; import { mergeChangelogs } from '../../lib/changelog-check'; import { runScriptSync } from '../../lib/script-invocation'; @@ -32,21 +33,51 @@ export const BASE_POLICY = { stamp_sensitive_gates: ['digest'], }; -export function makeFixtureRepo(opts: { policy?: Record | null; version?: string; prefix?: string } = {}): FixtureRepo { +export type FixtureOptions = { policy?: Record | null; version?: string; prefix?: string }; + +export function makeFixtureRepo(opts: FixtureOptions = {}): FixtureRepo { const root = fs.mkdtempSync(path.join(os.tmpdir(), opts.prefix ?? 'gstack-restamp-fx-')); + seedFixtureRepo(root, opts); + return fixtureRepoAt(root); +} + +/** The FixtureRepo handle for a root seedFixtureRepo filled, directly or through a template copy. */ +export function fixtureRepoAt(root: string): FixtureRepo { const origin = path.join(root, 'origin.git'); const work = path.join(root, 'work'); const home = path.join(root, 'home'); - fs.mkdirSync(home, { recursive: true }); - fs.mkdirSync(work, { recursive: true }); const env = { GSTACK_HOME: home, GSTACK_STATE_DIR: home, HOME: home, GIT_CONFIG_GLOBAL: path.join(root, 'gitconfig'), GIT_CONFIG_NOSYSTEM: '1' }; - fs.writeFileSync(env.GIT_CONFIG_GLOBAL, '[user]\n\tname = t\n\temail = t@test\n[commit]\n\tgpgsign = false\n[init]\n\tdefaultBranch = main\n'); const git = (...args: string[]): string => { const r = gitArgvIn(work, args, 30_000, { ...process.env, ...env }); if (r.status !== 0) throw new Error(`git ${args.join(' ')} failed: ${r.stderr?.toString()}`); return r.stdout?.toString().trim() ?? ''; }; + return { root, work, origin, home, git, env }; +} + +/** A template of makeFixtureRepo(opts), built once and copied per test (helpers/git-fixture-template.ts). */ +export function fixtureTemplate(opts: FixtureOptions = {}): GitFixtureTemplate { + return gitFixtureTemplate({ + prefix: opts.prefix ?? 'gstack-restamp-fx-', + build: root => { seedFixtureRepo(root, opts); return null; }, + rebind: (copy, tpl) => rebindGitDir(path.join(copy, 'work', '.git'), tpl, copy), + }); +} + +/** A fresh copy of a fixtureTemplate as its own FixtureRepo (cleanup() removes it like any other). */ +export function cloneFixture(template: GitFixtureTemplate, prefix?: string): FixtureRepo { + return fixtureRepoAt(template.copy(prefix).root); +} + +/** makeFixtureRepo's repository, written into an existing root. */ +export function seedFixtureRepo(root: string, opts: FixtureOptions): void { + const { origin, work, home, env, git } = fixtureRepoAt(root); + fs.mkdirSync(home, { recursive: true }); + fs.mkdirSync(work, { recursive: true }); + fs.writeFileSync(env.GIT_CONFIG_GLOBAL, '[user]\n\tname = t\n\temail = t@test\n[commit]\n\tgpgsign = false\n[init]\n\tdefaultBranch = main\n'); spawnSync('git', ['init', '-q', '--bare', origin], { timeout: 30_000, env: { ...process.env, ...env } }); + git('--git-dir', origin, 'config', 'maintenance.auto', 'false'); + git('--git-dir', origin, 'config', 'gc.auto', '0'); git('init', '-q', '-b', 'main'); const version = opts.version ?? '1.2.3.0'; const npm = version.split('.').slice(0, 3).join('.'); @@ -66,7 +97,6 @@ export function makeFixtureRepo(opts: { policy?: Record | null; git('remote', 'add', 'origin', origin); git('push', '-q', 'origin', 'main'); git('remote', 'set-head', 'origin', 'main'); - return { root, work, origin, home, git, env }; } /** A PR branch from origin/main with an `[Unreleased]` CHANGELOG entry and a doc mentioning the version it expects. */ diff --git a/test/pregate-user-policy.test.ts b/test/pregate-user-policy.test.ts index aad41a5be..6cb2b2667 100644 --- a/test/pregate-user-policy.test.ts +++ b/test/pregate-user-policy.test.ts @@ -6,7 +6,7 @@ * requires-remote. Every assertion holds on any host platform: each lane the * fixture selects is either named in `local` or asserted only by label. */ -import { afterEach, describe, expect, test } from 'bun:test'; +import { afterAll, afterEach, describe, expect, test } from 'bun:test'; import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as os from 'node:os'; @@ -15,18 +15,20 @@ import { EXIT } from '../lib/headless-artifacts'; import { loadConfigFromUser, validateConfig } from '../lib/pregate/config'; import { buildContext, sha256, userConfigRejection } from '../lib/pregate/context'; import { runPregate, verifyPregate } from '../lib/pregate/index'; -import { BASE_PREGATE, BIN, cleanup, commit, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/pregate-fixture'; +import { BASE_PREGATE, BIN, cleanup, cloneFixture, commit, fixtureTemplate, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/pregate-fixture'; import { runScriptSync } from '../lib/script-invocation'; const repos: FixtureRepo[] = []; afterEach(() => { while (repos.length) cleanup(repos.pop()!); }); +const noBasePolicy = fixtureTemplate({ pregate: null }); +afterAll(() => noBasePolicy.dispose()); const SLUG = 'userpolicy'; const LOCAL_ALL = ['free-tests.yml/unit', 'windows.yml/windows-shard']; /** A fixture whose base carries no `.gstack/pregate.json`, a feature commit touching lib/util.js, and the user-file path for its slug. */ function setup(): { repo: FixtureRepo; userPolicy: string; run: (extra?: string[]) => ReturnType } { - const repo = makeFixtureRepo({ pregate: null }); + const repo = cloneFixture(noBasePolicy); repos.push(repo); commit(repo, { 'lib/util.js': 'exports.add = (a, b) => b + a;\n' }); const userPolicy = path.join(repo.home, 'projects', SLUG, 'pregate.json'); diff --git a/test/release-cut.test.ts b/test/release-cut.test.ts index f9b45efeb..6986b5630 100644 --- a/test/release-cut.test.ts +++ b/test/release-cut.test.ts @@ -6,12 +6,12 @@ * check run or a red one keeps a commit out; a cut that stopped half-way is * finished by the next run (docs/releases.md). */ -import { afterEach, describe, expect, test } from 'bun:test'; +import { afterAll, afterEach, describe, expect, test } from 'bun:test'; import { execFileSync, spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; -import * as os from 'node:os'; import * as path from 'node:path'; import { bunIsolationArgs } from '../lib/bun-isolation'; +import { gitFixtureTemplate, rebindGitDir } from './helpers/git-fixture-template'; import { BACKSTOP_CHECK, FLOOR_HOURS, NOTES_CAP, NOTES_REF, REQUIRED_CHECK, SCHEDULED_GRACE_HOURS, changelogEntries, compareVersions, cutRelease, describePlan, floorStatus, gateVerdict, gitIn, planRelease, readTrainNotes, releaseNotes, sanitizeReason, stableMovedAt, verifyRegression, verifySecurity, type Gh, type Git, @@ -55,19 +55,10 @@ function fakeGh(state: State): Gh { }; } -function fixture() { - const base = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-release-cut-')); - bases.push(base); - const origin = path.join(base, 'origin.git'); - const seed = path.join(base, 'seed'); - const work = path.join(base, 'work'); - execFileSync('git', ['init', '-q', '--bare', '-b', 'main', origin], { timeout: 30_000 }); - fs.mkdirSync(seed); - git(seed, 'init', '-q', '-b', 'main'); - git(seed, 'remote', 'add', 'origin', origin); - const shas: Record = {}; - let changelog = '# Changelog\n'; - const release = (version: string, headline: string, o: { message?: string; files?: Record } = {}) => { +/** Commits a release (VERSION, a CHANGELOG headline, optional files) in seed and pushes it to origin. */ +function releaser(seed: string, shas: Record) { + let changelog = fs.existsSync(path.join(seed, 'CHANGELOG.md')) ? fs.readFileSync(path.join(seed, 'CHANGELOG.md'), 'utf8') : '# Changelog\n'; + return (version: string, headline: string, o: { message?: string; files?: Record } = {}) => { changelog = changelog.replace('# Changelog\n', '# Changelog\n\n## [' + version + '] - 2026-10-2' + version.slice(-1) + '\n\n**' + headline + '**\n\n### Fixed\n- a thing\n'); fs.writeFileSync(path.join(seed, 'VERSION'), version + '\n'); fs.writeFileSync(path.join(seed, 'CHANGELOG.md'), changelog); @@ -78,11 +69,39 @@ function fixture() { shas[version] = git(seed, 'rev-parse', 'HEAD'); return shas[version]!; }; - release('1.0.0', 'First one.'); - release('1.0.1', 'Second: the updater no longer skips.'); - release('1.0.2', 'Third fixes Windows.'); - release('1.0.3', 'Fourth trims the preamble.'); - execFileSync('git', ['clone', '-q', origin, work], { timeout: 30_000 }); +} + +/** origin with releases 1.0.0..1.0.3 pushed from seed, and work cloned from it; built once for this file, copied per test. */ +const template = gitFixtureTemplate({ + prefix: 'gstack-release-cut-', + build: (base) => { + const origin = path.join(base, 'origin.git'); + const seed = path.join(base, 'seed'); + execFileSync('git', ['init', '-q', '--bare', '-b', 'main', origin], { timeout: 30_000 }); + fs.mkdirSync(seed); + git(seed, 'init', '-q', '-b', 'main'); + git(seed, 'remote', 'add', 'origin', origin); + const shas: Record = {}; + const release = releaser(seed, shas); + release('1.0.0', 'First one.'); + release('1.0.1', 'Second: the updater no longer skips.'); + release('1.0.2', 'Third fixes Windows.'); + release('1.0.3', 'Fourth trims the preamble.'); + execFileSync('git', ['clone', '-q', origin, path.join(base, 'work')], { timeout: 30_000 }); + return shas; + }, + rebind: (copy, tpl) => { for (const repo of ['seed', 'work']) rebindGitDir(path.join(copy, repo, '.git'), tpl, copy); }, +}); +afterAll(() => template.dispose()); + +function fixture() { + const { root: base, meta } = template.copy(); + bases.push(base); + const origin = path.join(base, 'origin.git'); + const seed = path.join(base, 'seed'); + const work = path.join(base, 'work'); + const shas: Record = { ...meta }; + const release = releaser(seed, shas); const refresh = () => { git(work, 'fetch', '-q', 'origin', '+refs/heads/*:refs/remotes/origin/*'); git(work, 'fetch', '-q', '--tags', 'origin'); }; const state: State = { checks: {}, released: new Set(), calls: [] }; return { base, origin, seed, work, shas, release, refresh, state, gh: fakeGh(state), git: gitIn(work), repo: 'o/r' }; diff --git a/test/restamp-mentions.test.ts b/test/restamp-mentions.test.ts index 0fc839989..a59c0e9c4 100644 --- a/test/restamp-mentions.test.ts +++ b/test/restamp-mentions.test.ts @@ -9,16 +9,26 @@ * version moves only on lines the branch added), and the negative control * (a base-side quoted row carrying that version stays). */ -import { afterEach, describe, expect, test } from 'bun:test'; +import { afterAll, afterEach, describe, expect, test } from 'bun:test'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { EXIT } from '../lib/headless-artifacts'; import { rewriteStamps, tentativeMentions } from '../lib/restamp'; -import { BASE_POLICY, cleanup, makeFixtureRepo, makePr, mergeMain, mergeToMain, readFile, restamp, type FixtureRepo } from './helpers/restamp-fixture'; +import type { GitFixtureTemplate } from './helpers/git-fixture-template'; +import { BASE_POLICY, cleanup, cloneFixture, fixtureTemplate, makePr, mergeMain, mergeToMain, readFile, restamp, type FixtureRepo } from './helpers/restamp-fixture'; const repos: FixtureRepo[] = []; afterEach(() => { while (repos.length) cleanup(repos.pop()!); }); +/** The helper's makeFixtureRepo, built once per policy for this file and copied per test. */ +const templates = new Map>(); +afterAll(() => { for (const t of templates.values()) t.dispose(); }); +function makeFixtureRepo(opts: { prefix: string; policy: Record }): FixtureRepo { + const key = JSON.stringify(opts.policy); + if (!templates.has(key)) templates.set(key, fixtureTemplate({ policy: opts.policy, prefix: 'restamp-mentions-' })); + return cloneFixture(templates.get(key)!, opts.prefix); +} + const stageDetail = (out: string, name: string) => new RegExp(`^stage ${name}: (\\w+)(?: \\((.*)\\))?$`, 'm').exec(out); describe('quoted stamp forms (pure)', () => { diff --git a/test/test-of-test-ratchet.test.ts b/test/test-of-test-ratchet.test.ts index cad251bda..4a0b48977 100644 --- a/test/test-of-test-ratchet.test.ts +++ b/test/test-of-test-ratchet.test.ts @@ -104,6 +104,7 @@ const BASELINE = [ 'test/extension-pty-inject-invariant.test.ts', 'test/gbrain-source-worktree-advance.test.ts', 'test/gbrain-sync-voyage-code-3-integration.test.ts', + 'test/git-fixture-template.test.ts', // GSTA-151: pins the copy isolation and leak scan of test/helpers/git-fixture-template.ts, which the templated git fixtures in release-cut, train-backstop, restamp-mentions, pregate-user-policy and held-install-recovery rely on 'test/gstack-gbrain-source-wireup.test.ts', 'test/gstack-home-module-scope.test.ts', 'test/gstack-upgrade-migration-v1_17_0_0.test.ts', diff --git a/test/train-backstop.test.ts b/test/train-backstop.test.ts index ecdf6aeed..2c2ca79ca 100644 --- a/test/train-backstop.test.ts +++ b/test/train-backstop.test.ts @@ -17,7 +17,8 @@ import type { GhRunner } from '../scripts/recover-receipts'; import { runBackstop } from '../scripts/train-backstop'; import { gitIn, trainPrefixes, type TrainRecord } from '../scripts/train-gate'; import { E2E_TOUCHFILES } from './helpers/touchfiles-data'; -import { BASE_POLICY, cleanup, makeFixtureRepo, type FixtureRepo } from './helpers/restamp-fixture'; +import { gitFixtureTemplate, rebindGitDir, type GitFixtureTemplate } from './helpers/git-fixture-template'; +import { BASE_POLICY, cleanup, fixtureRepoAt, seedFixtureRepo, type FixtureRepo } from './helpers/restamp-fixture'; import { TRAIN_BLOCK, TRAIN_POLICY } from './helpers/ship-queue-train-fixture'; const REPO = 'fixture/repo'; @@ -30,11 +31,24 @@ afterEach(() => { if (savedStateRoot === undefined) delete process.env.GSTACK_ST interface Fixture { repo: FixtureRepo; main0: string; prs: string[]; train: string[]; record: TrainRecord } -/** Five PR branches from main and the train T1..T5 gstack-ship-queue would build from them (lib/ship-queue-train.ts buildChain). */ +/** Five PR branches from main and the train T1..T5 gstack-ship-queue would build from them, built once per policy and copied per test. */ +const templates = new Map>>(); +afterAll(() => { for (const t of templates.values()) t.dispose(); }); function trainFixture(policy: Record = TRAIN_POLICY): Fixture { - const repo = makeFixtureRepo({ policy, prefix: 'train-backstop-fx-' }); + const key = JSON.stringify(policy); + if (!templates.has(key)) templates.set(key, gitFixtureTemplate({ prefix: 'train-backstop-fx-', build: root => buildTrain(root, policy), rebind: (copy, tpl) => rebindGitDir(path.join(copy, 'work', '.git'), tpl, copy) })); + const { root, meta } = templates.get(key)!.copy(); + const repo = fixtureRepoAt(root); repos.push(repo); process.env.GSTACK_STATE_ROOT = path.join(repo.root, 'state'); + return { repo, ...structuredClone(meta) }; +} + +/** lib/ship-queue-train.ts buildChain's T1..T5 over five PR branches from main. */ +function buildTrain(root: string, policy: Record): Omit { + seedFixtureRepo(root, { policy }); + const repo = fixtureRepoAt(root); + process.env.GSTACK_STATE_ROOT = path.join(repo.root, 'state'); const main0 = repo.git('rev-parse', 'HEAD'); const prs = [1, 2, 3, 4, 5].map(i => { repo.git('checkout', '-q', '-B', `pr${i}`, main0); @@ -56,7 +70,7 @@ function trainFixture(policy: Record = TRAIN_POLICY): Fixture { const record: TrainRecord = { schema: 1, kind: 'train-gate', nonce: TRAIN_NONCE, head_sha: parent, tree: repo.git('rev-parse', `${parent}^{tree}`), verdict: 'GREEN', census: { complete: true, planned: { rule: 108, judge: 24 }, executed: { rule: 108, judge: 24 }, unbound: 0, partial_shards: 0, cancelled_shards: 0, killed_before_upload: 0 }, panels: [], run: { id: 900, attempt: 1, url: 'https://example.invalid/runs/900' }, prefixes }; - return { repo, main0, prs, train, record }; + return { main0, prs, train, record }; } /** The coordinator's squash merge of PR i onto main, stamped at merge (VERSION, package.json, the release output). */ From 69c14b17d57d5eef537dacbd533ee88865a0a00a Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 2 Nov 2026 01:57:10 -0800 Subject: [PATCH 5/5] v1.92.71.0 test(free): end four non-subject waits early and drop a duplicated test block (GSTA-151 D) (#3582) * test(free): end four non-subject waits early and drop a duplicated test block setup's _wait_with_deadline polls every 0.1 s on the SECONDS clock; pregate-supervise and gstack-detach wait on pids and go-files instead of fixed sleeps; qa-probe-gates loses its duplicated C2 block; context-identity runs one restore. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test(setup): mark the sub-second wait bound as the contract (gstack-timing-ok) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(changelog): unreleased entry for the free-suite wait pass (GSTA-151 D) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 14 ++++++++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- package.json | 2 +- setup | 8 ++--- test/context-identity.test.ts | 5 +-- test/gstack-detach.test.ts | 14 +++++--- test/pregate-supervise.test.ts | 6 ++-- test/qa-probe-gates.test.ts | 39 ----------------------- test/setup-playwright-best-effort.test.ts | 12 +++++++ 10 files changed, 50 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09fff87eb..2946d21bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # Changelog +## [1.92.71.0] - 2026-11-02 + +**Four free-suite waits now end when the thing they wait for happens: the six files involved spend 18–29 s less in summed per-file runtime, with every assertion intact.** `./setup` returns from a finished child up to ~0.9 s sooner too. + +### Changed +- **`setup` `_wait_with_deadline`**: polls every 0.1 s and tracks the deadline on bash's `SECONDS` clock instead of sleeping a whole second per poll. A child that exits at once no longer costs a second. The kill still lands no earlier than the deadline, the exit codes (the child's status, or 124 after the kill) are unchanged, and it runs under bash 3.2. + +### Tests +- `test/setup-playwright-best-effort.test.ts`: a child that exits with 3 after 50 ms returns 3 in under 500 ms (the old loop took 1,008 ms); the deadline and kill cases are unchanged. The file drops from ~20.8 s to ~5.9 s on a CI-mirror VM. +- `test/pregate-supervise.test.ts`: the leftover-child case waits for that child's pid to exit (bounded at 3 s) instead of a fixed 2.5 s sleep. +- `test/gstack-detach.test.ts`: the detach and lock bodies wait on files the test writes (bounded at ~10 s, also written in `finally`) instead of `sleep 2` and `sleep 3`. +- `test/qa-probe-gates.test.ts`: the duplicated "judge-named QA workflow gaps (C2)" block is gone (3 tests that repeated the first block). +- `test/context-identity.test.ts`: one `restore(mono)` call serves both assertions. + ## [1.92.70.0] - 2026-11-02 **Five free-test files stop rebuilding the same git repositories for every test: they spend 17–24 s less in summed per-file runtime, with every assertion intact.** Each test still gets its own private copy of the fixture. diff --git a/VERSION b/VERSION index 9dc23d34a..2d527513b 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.92.70.0 +1.92.71.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 1fe114a87..181ad1acd 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.92.70.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.92.71.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, diff --git a/package.json b/package.json index 15a0b5f64..4f5009fa6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.92.70", + "version": "1.92.71", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/setup b/setup index 649c6fd5e..1eb823634 100755 --- a/setup +++ b/setup @@ -1539,15 +1539,15 @@ _kill_tree() { # poll the PID and SIGKILL the whole probe tree past the deadline. Returns # the probe's exit code, or 124 on timeout. _wait_with_deadline() { - local pid="$1" deadline_s="$2" waited=0 + local pid="$1" deadline_s="$2" end + end=$((SECONDS + deadline_s)) while kill -0 "$pid" 2>/dev/null; do - if [ "$waited" -ge "$deadline_s" ]; then + if [ "$SECONDS" -gt "$end" ]; then _kill_tree "$pid" wait "$pid" 2>/dev/null || true return 124 fi - sleep 1 - waited=$((waited + 1)) + sleep 0.1 done wait "$pid" } diff --git a/test/context-identity.test.ts b/test/context-identity.test.ts index 1ce221d29..a73f7b11a 100644 --- a/test/context-identity.test.ts +++ b/test/context-identity.test.ts @@ -353,8 +353,9 @@ describe('context-restore offers newer checkpoints it used to skip (#3065)', () const sibling = repo('elsewhere', 'git@github.com:acme/elsewhere.git'); const outside = save(sibling, 'sibling work', '20261002-100000'); const below = save(pkg, 'pkg feature', '20261001-100000'); - expect(restore(mono).out).not.toContain(outside); - expect(restore(mono).lines).toContain(`BELOW_CWD ${below}`); + const r = restore(mono); + expect(r.out).not.toContain(outside); + expect(r.lines).toContain(`BELOW_CWD ${below}`); // 200 newer checkpoints of another project push the below-cwd one out of the read window. const dir = path.dirname(outside); diff --git a/test/gstack-detach.test.ts b/test/gstack-detach.test.ts index 1b8e8295b..914c9a605 100644 --- a/test/gstack-detach.test.ts +++ b/test/gstack-detach.test.ts @@ -25,6 +25,7 @@ function waitFor(pred: () => boolean, ms: number): boolean { } return pred(); } +const GO_FILE_WAIT = 'i=0; while [ ! -e "$1" ] && [ "$i" -lt 100 ]; do sleep 0.1; i=$((i + 1)); done; [ -e "$1" ]'; function logHas(p: string, needle: string): boolean { try { return fs.readFileSync(p, 'utf-8').includes(needle); } catch { return false; } } @@ -33,19 +34,21 @@ describe('gstack-detach', () => { test('detaches (different pgid), returns immediately, completes, writes EXIT sentinel', () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gd-')); const log = path.join(dir, 'run.log'); + const go = path.join(dir, 'go'); try { const t0 = Date.now(); - const r = spawnSync(DETACH, ['--log', log, '--', 'bash', '-c', 'sleep 2; echo body-ran'], { encoding: 'utf-8', timeout: 10000 }); + const r = spawnSync(DETACH, ['--log', log, '--', 'bash', '-c', GO_FILE_WAIT + ' && echo body-ran', 'body', go], { encoding: 'utf-8', timeout: 10000 }); const elapsed = Date.now() - t0; expect(r.status).toBe(0); expect(r.stdout).toContain(`gstack-detach LOG ${log}`); expect(elapsed).toBeLessThan(1500); // non-blocking + fs.writeFileSync(go, ''); expect(waitFor(() => logHas(log, '### gstack-detach EXIT=0 ###'), 8000)).toBe(true); expect(logHas(log, 'body-ran')).toBe(true); // ran to completion after launcher returned const m = fs.readFileSync(log, 'utf-8').match(/pgid=(\d+)/); expect(m).not.toBeNull(); expect(m![1]).not.toBe(ownPgid()); // detached into its own group - } finally { fs.rmSync(dir, { recursive: true, force: true }); } + } finally { fs.writeFileSync(go, ''); fs.rmSync(dir, { recursive: true, force: true }); } }, 15000); test('default log is run-scoped under ~/.gstack-dev/eval-runs (no shared /tmp)', () => { @@ -127,17 +130,20 @@ describe('gstack-detach', () => { const lock = `gstack-detach-test-${process.pid}`; const logA = path.join(dir, 'a.log'); const logB = path.join(dir, 'b.log'); + const release = path.join(dir, 'release'); try { - // First holds the lock for ~3s; second must wait then acquire. - spawnSync(DETACH, ['--log', logA, '--lock', lock, '--', 'sleep', '3'], { encoding: 'utf-8', timeout: 10000, env: { ...process.env, HOME: dir } }); + // First holds the lock until the test writes the release file; second must wait then acquire. + spawnSync(DETACH, ['--log', logA, '--lock', lock, '--', 'bash', '-c', GO_FILE_WAIT, 'holder', release], { encoding: 'utf-8', timeout: 10000, env: { ...process.env, HOME: dir } }); waitFor(() => logHas(logA, "ACQUIRED"), 4000); spawnSync(DETACH, ['--log', logB, '--lock', lock, '--', 'echo', 'second-ran'], { encoding: 'utf-8', timeout: 10000, env: { ...process.env, HOME: dir } }); // Second should report WAITING (first still holds it) then ACQUIRE after release. expect(waitFor(() => logHas(logB, 'WAITING for lock'), 4000)).toBe(true); + fs.writeFileSync(release, ''); expect(waitFor(() => logHas(logB, '### gstack-detach EXIT=0 ###'), 12000)).toBe(true); expect(logHas(logB, 'second-ran')).toBe(true); expect(fs.existsSync(path.join(dir, '.gstack', 'locks', `${lock}.lock`))).toBe(true); } finally { + fs.writeFileSync(release, ''); fs.rmSync(dir, { recursive: true, force: true }); } }, 20000); diff --git a/test/pregate-supervise.test.ts b/test/pregate-supervise.test.ts index 70921184a..900f4f890 100644 --- a/test/pregate-supervise.test.ts +++ b/test/pregate-supervise.test.ts @@ -226,9 +226,11 @@ describe('gstack-pregate: the test stage never hangs silently', () => { expect(pregate(during, []).status).toBe(EXIT.fail); expect(lanes(during).detail).toContain('working tree changed during the check'); const after = makeFixtureRepo(); repos.push(after); - commit(after, { 'test/leftover.test.js': "const test = require('node:test');\nconst { spawn } = require('node:child_process');\ntest('leaves a child', () => { spawn('sh', ['-c', 'sleep 2; echo changed >> README.md'], { stdio: 'ignore' }).unref(); });\n" }); + commit(after, { 'test/leftover.test.js': "const test = require('node:test');\nconst { spawn } = require('node:child_process');\ntest('leaves a child', () => { const c = spawn('sh', ['-c', 'sleep 2; echo changed >> README.md'], { stdio: 'ignore' }); require('node:fs').writeFileSync(require('node:path').join(__dirname, '..', '..', 'leftover.pid'), String(c.pid)); c.unref(); });\n" }); expect(pregate(after, []).status).toBe(EXIT.ok); - await Bun.sleep(2500); + const until = Date.now() + 3000; + const leftover = await pidFrom(path.join(after.root, 'leftover.pid'), 3000).catch(() => 0); + if (leftover) await goneSoon(leftover, Math.max(0, until - Date.now())); expect(fs.readFileSync(path.join(after.work, 'README.md'), 'utf8')).toBe('# fixture\n'); }, 90_000); diff --git a/test/qa-probe-gates.test.ts b/test/qa-probe-gates.test.ts index 1478b8d84..2fbd20f75 100644 --- a/test/qa-probe-gates.test.ts +++ b/test/qa-probe-gates.test.ts @@ -344,42 +344,3 @@ describe('judge-named QA workflow gaps (C2)', () => { } }); }); - -// Lane S C2: gaps the qa-only/review/ship workflow judges named in their rationales. -describe('judge-named QA workflow gaps (C2)', () => { - const ctx = (skillName: string) => ({ host: 'claude' as const, skillName, tmplPath: '', paths: HOST_PATHS.claude }); - - test('Browser Quick leaves time for its six page probes, and the method states the same budget', () => { - for (const skillName of ['qa', 'qa-only']) { - const seconds = Number(/Browser Quick: SECONDS=(\d+)\./.exec(generateQAExploratory(ctx(skillName)))?.[1]); - // homepage + top 5 targets, each a checkpoint write plus one guarded command: 30 s left ~5 s per probe. - expect(seconds / 6).toBeGreaterThanOrEqual(20); - const patterns = fs.readFileSync(path.join(import.meta.dir, '..', 'qa', 'sections', 'qa-patterns.md'), 'utf8'); - expect(patterns).toContain(`${seconds / 60} minutes: homepage + top 5 navigation targets`); - } - }); - - test('PROBE_DIR names its source line, and stopping goes to the final report steps', () => { - for (const skillName of ['qa', 'qa-only']) { - const text = generateQAExploratory(ctx(skillName)); - const layout = text.indexOf('mixed standalone runs use REPORT_DIR/browser and REPORT_DIR/functional'); - const probeDir = text.indexOf("PROBE_DIR: this surface's owned probe directory per the line above."); - expect(layout).toBeGreaterThan(-1); - expect(probeDir).toBeGreaterThan(layout); - expect(text.slice(layout, probeDir).split('\n')).toHaveLength(3); - expect(text).toContain('write the report (§4), not a checkpoint.'); - const final = text.indexOf('## 4. Final report'); - expect(final).toBeGreaterThan(-1); - expect(text.indexOf('annotations.json', final)).toBeLessThan(text.indexOf('materialize PROBE_DIR', final)); - } - }); - - test('/review and /ship: smoke rechecks after smoke expiry are not-run, before step 4c revalidation', () => { - for (const skillName of ['review', 'ship']) { - const text = generateQAReview(ctx(skillName)); - const rule = text.indexOf('Post-expiry smoke rechecks are not-run.'); - expect(rule).toBeGreaterThan(-1); - expect(rule).toBeLessThan(text.indexOf('c. Re-review changed or uncertain coverage and repeat step 3 for affected checks.')); - } - }); -}); diff --git a/test/setup-playwright-best-effort.test.ts b/test/setup-playwright-best-effort.test.ts index 826bc6897..22bf15a19 100644 --- a/test/setup-playwright-best-effort.test.ts +++ b/test/setup-playwright-best-effort.test.ts @@ -271,6 +271,18 @@ describe('setup: Chromium bootstrap block executes best-effort', () => { expect(fs.readFileSync(path.join(r.tmp, 'mark'), 'utf-8')).toContain('killed'); }, 15_000); + test('_wait_with_deadline returns a quickly exiting child\'s status without waiting out a whole second', () => { + const t0 = Date.now(); + const r = runBashScript([ + extractFn('_kill_tree'), extractFn('_wait_with_deadline'), + '(sleep 0.05; exit 3) &', + '_wait_with_deadline $! 90 || echo "RC=$?"', + ].join('\n'), { timeout: 10_000 }); + expect(r.stdout).toBe('RC=3\n'); + // gstack-timing-ok: a sub-second return is the contract; the old 1 s poll took 1,008 ms, this path takes ~0.2 s + expect(Date.now() - t0).toBeLessThan(500); + }); + test('non-numeric timeout knob falls back to the default instead of erroring', () => { const r = runBlock({ probe: 'fail', bunx: 'exit 3', env: { GSTACK_PLAYWRIGHT_INSTALL_TIMEOUT: 'soon' } }); expect(r.status).toBe(0);