diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index 601e3ebab..79d159a95 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -47,6 +47,6 @@ jobs: path: results.sarif retention-days: 5 - - uses: github/codeql-action/upload-sarif@24c54180a607b1449ed407dd24f251e4e9147c8d # v4.38.3 + - uses: github/codeql-action/upload-sarif@6f2ac81f5cd81fe351a197ba0b00d91d03cd2aee # v4.39.0 with: sarif_file: results.sarif diff --git a/AGENTS.md b/AGENTS.md index 4299662d0..ac84585d1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -303,4 +303,4 @@ Agents poll `eval:bg:*` logs for the `### gstack-detach EXIT= ###` sentine - Every Codex run's verdict goes through `lib/outside-review-result.ts`; its reason codes live in `lib/gate-outcomes.ts`, each with a `docs/troubleshooting.md` anchor. - `test/module-size-ratchet.test.ts` keeps refactored owner modules at or under 800 lines (150 per function) and residual files from growing. - The `claude` CLI binary resolves via `lib/claude-bin.ts` (re-exported from `browse/src/claude-bin.ts` for browse internals; `Bun.which()` + `GSTACK_CLAUDE_BIN` override). Set `GSTACK_CLAUDE_BIN=wsl` plus `GSTACK_CLAUDE_BIN_ARGS='["claude"]'` to run Claude through WSL on Windows. -- Every Bun gstack starts ignores the working directory's `.env` files and `bunfig.toml`: bun shebangs are `#!/usr/bin/env -S bun --no-env-file --config=/dev/null`, shell and skill callers pass `bun --no-env-file --config=/dev/null`, TypeScript spawns use `bunIsolationArgs()` from `lib/bun-isolation.ts`, and `bun build --compile` adds `--no-compile-autoload-dotenv --no-compile-autoload-bunfig`. `test/bun-entry-isolation.test.ts` enforces all four. +- Every Bun gstack starts ignores the working directory's `.env` files and `bunfig.toml`: bun shebangs are `#!/usr/bin/env -S bun --no-env-file --config=/dev/null`, skill prose passes `bun --no-env-file --config=/dev/null`, shipped shell files define `GSTACK_BUN_CONFIG` with the line in `test/bun-entry-isolation.test.ts` (`NUL` when `OSTYPE` is `msys` or `cygwin` and `cygpath` is on `PATH`, where native Bun cannot open `/dev/null`, #3415) and pass `bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}"`, TypeScript spawns use `bunIsolationArgs()` from `lib/bun-isolation.ts`, and `bun build --compile` adds `--no-compile-autoload-dotenv --no-compile-autoload-bunfig`. `test/bun-entry-isolation.test.ts` enforces all four. diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c4d48b05..16e635d98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## [Unreleased] +## [1.91.230.0] - 2026-10-26 ### Changed - **The archetype the user picks on the /design-shotgun board now reaches /design-html.** Captions name the archetype id (`classic-hero`, `long-form-story`, `minimal-conversion`, `comparison`). On approval, approved.json also gets the chosen `archetype` and its `sections` outline, added with the Edit tool so no free text reaches a shell command. /design-html reads both in Case A, and the craft section uses them as given instead of choosing again or asking. @@ -13,6 +13,86 @@ - `design-html-slop-gate` is now selected by changes to `lib/design-craft.ts` and `bin/gstack-design-craft` (`test/helpers/touchfiles-data.ts`), which it already exercised but did not list. - `test/design-craft.test.ts` also covers accepted exceptions: count coverage and overflow, duplicate ids carried forward (the highest count wins), a bad exceptions file still checked in full, the finalized.json block in design-html/SKILL.md parsed by the same reader the CLI uses, rejected malformed lists, CLI exit codes with `--accepted`, and the archetype handoff contract (the same four ids in /design-shotgun and the craft section, the approved.json step, Case A and the craft section's no-re-ask rule). +## [1.91.229.0] - 2026-10-26 + +**The OpenSSF Scorecard workflow uploads its results with `github/codeql-action/upload-sarif` 4.39.0 (from 4.38.3), recreated from Dependabot's #3307.** Dependabot's branch can't take a version stamp at merge time, so the same one-line change ships from a crew branch. The new pin `6f2ac81f5cd81fe351a197ba0b00d91d03cd2aee` is the commit that the annotated tag `v4.39.0` points to in github/codeql-action, and GitHub shows that commit as verified. 4.39.0's one breaking change raises the minimum CodeQL bundle to 2.20.7, which applies to `init` and `analyze`. This workflow uses only `upload-sarif` with Scorecard's own SARIF file. + +### Changed +- **`.github/workflows/scorecard.yml`**: `upload-sarif` pinned to `6f2ac81f…` (`# v4.39.0`), byte-for-byte the change in #3307. + +## [1.91.228.0] - 2026-10-26 + +**Skills start at once on Windows again: the workflow lease lock no longer depends on symlinks (#3330).** `bin/gstack-lease` took its lock with `ln -s "" .lock`. In Git for Windows' default symlink mode (`MSYS` unset), `ln -s` cannot link to a target that does not exist, so the lock was never taken. Every skill start then waited out all 300 tries (0.1 s apart, plus process start costs) before `gstack-skill-start` printed anything, and still wrote no lease. One report measured 499 s per skill start. The error also blamed a "live process" with an empty pid. + +### Fixed +- **`bin/gstack-lease`**: the lock is now a `.lock` directory taken with `mkdir`, which is atomic on every host gstack runs on, with an `owner` file naming the holder's pid and start identity. A taker writes a claim (`.lock.claim.`) before its `mkdir` and renames it in as the owner after, so a lock left ownerless by a taker killed between the two steps is broken once no claimant lives and the lock is at least 2 s old and unchanged while it was checked (a live taker between its two steps is never mistaken for a crash, and a new lock made meanwhile is never removed). A claim that records no start identity (Git Bash has none) is abandoned after 2 minutes, so a reused pid cannot keep an orphan forever. An owner that names no valid pid (0, zero-padded or not a number) counts as a dead holder, an owner file that exists but cannot be read keeps the lock, and an empty owner, a stray file at `.lock`, or an ownerless lock whose mtime is a minute or more in the future (clock skew) counts as ownerless instead of wedging every skill start. A lock is removed by renaming it aside first, so nobody sees it half-removed; once renamed it is released even if the renamed copy cannot be deleted, and `touch` prunes leftover copies and claims over 2 minutes old whose claimant is gone or recorded no identity. A late taker never overwrites an owner already in the lock it made. A waiter no longer writes a claim or runs `mkdir` while the lock is visibly held, and a live holder that never lets go bounds the wait at 300 tries or 30 s, whichever comes first (a non-numeric `GSTACK_LEASE_LOCK_TRIES` falls back to 300 instead of looping forever). Stale-holder semantics are unchanged: a lock is broken only when its holder is dead or its pid now belongs to another process, breakers still serialize on `.lock.break` and reread before removing, and unlock removes only the caller's own lock. A symlink `.lock` left by an older gstack is still read and broken the same way, and it excludes the new lock at the same path. The reverse does not hold: an older gstack's `ln -s` onto the directory lock links inside it, so a process that started before an upgrade of the same checkout can briefly share the lock (documented as a known limit, with a taker stalled over 2 minutes mid-take on a host without start identities). +- A dead holder's lock that cannot be renamed aside (a Windows file in use) is removed in place under `.lock.break`, and when that fails too the try is counted, so `touch` ends with exit 3 instead of spinning without sleeping. The message no longer calls a dead holder live. +- The 120-minute prune in `touch` no longer reaches into the lock: it removed the `owner` file of a holder that resumed past the window, which then left its own lock behind. +- When the lock cannot be taken although nobody holds it, `touch` now fails after ten quick retries with `gstack-lease: cannot take the lease directory lock ` instead of waiting out every try and naming an empty pid. `gstack-skill-start` stays fail-open, as before. +- `docs/capy-install.md` describes the directory lock. + +### Tests +- `test/gstack-lease-git-bash.test.ts` (new, in the curated windows-free-tests lane, launched through `lib/script-invocation.ts`, so Git Bash on `windows-latest`): `touch` with the default 300 tries takes the lock on its first try (no sleep between tries), writes its lease and leaves no lock. On `windows-latest` the old bin fails this case; a host whose `ln -s` fails like Git Bash's still takes the lock (with a control that the shim really fails `ln -s`); a lock held by a live Git Bash process is kept and named, and a dead holder's lock is broken. The last two fail against the old bin. +- `test/gstack-lease.test.ts`: the dead-holder, live-holder and dead-breaker cases run against both the directory lock and the legacy symlink lock. The race cases inject at `mkdir` instead of `ln`. New cases: a lock held by a running gstack-lease excludes a second taker, which names it; 16 parallel touches all land and leave nothing behind; an ownerless lock is kept while a claimant lives and broken once none does; a lock that cannot be taken while nobody holds it fails without sleeping between tries. Seven of these fail against the old bin. +- From /ship's coverage audit and review: the 16 parallel touches now also prove no two are inside the lock at once (a mutant whose `mkdir` always succeeds fails it); the exclusion case holds the lock until the test releases it instead of for a 3 s sleep; a young ownerless lock is kept and malformed locks (empty owner, pid 0 or 00, a non-numeric owner, a stray file, an mtime an hour ahead) are broken; an identity-less claim older than 2 minutes neither keeps an ownerless lock nor survives `touch` while an old claim with a live identity is honored; an unreadable owner file keeps the lock; `--help` prints the whole usage again; leftover renamed-aside copies are pruned; a live holder bounds `touch` by the 30 s deadline with the default and with a non-numeric tries value, without one `mkdir` on the held lock; the prune keeps a resumed holder's lock; a renamed-aside lock that cannot be deleted never takes a new taker's lock with it; a dead holder's lock that cannot be renamed aside ends `touch` instead of spinning. Each case that pins a fix fails against the bin before that fix. Also covered: a live or ownerless `.lock.break`, a failed owner rename, a lock released between a failed take and its check, and unlock's fallback. The Git Bash case now also checks that the live holder still owns the lock after a blocked `touch`. + +### Gate integrity +- No threshold, budget, timeout, trial count, case kind or assertion was weakened. Three existing race tests moved their fault injection from `ln` to `mkdir`, because the lock no longer calls `ln`. Their assertions are unchanged. Under /ship review, the exclusion case's wait for the holder's owner file rose from 2 s to 10 s and its fixed 3 s hold became a release file (no assertion changed), the ownerless-lock cases backdate their planted lock by a minute because an ownerless lock is now broken only once 2 s old (their assertions are unchanged; the new young-lock case pins the guard), and the exclusion and parallel cases got an explicit 30 s test timeout instead of Bun's 5 s default. + +## [1.91.227.0] - 2026-10-26 + +**Windows: gstack's record bins stop losing review, question, learning and timeline records when Git Bash path conversion is off (#3415).** Since GSTA-56 every shell bin started Bun with `--config=/dev/null`. Native `bun.exe` cannot open `/dev/null`: it exits 1 with `EINVAL` on Bun 1.3.10, 1.4.2 and 1.4.4 (measured on windows-latest). Git Bash hides this by rewriting the argument to `nul`, but only while MSYS path conversion is on. With it off (`MSYS_NO_PATHCONV` set to any value, even empty; `MSYS2_ARG_CONV_EXCL=*`; or Cygwin), `gstack-review-log`, `gstack-question-log` and `gstack-learnings-log` exited 1 with nothing written, and `gstack-timeline-log` exited 0 with nothing written. + +### Fixed +- **40 shipped shell files** (the bash bins, the Claude hook wrappers, `setup`, `scripts/build-cso.sh` and one migration) define `GSTACK_BUN_CONFIG` once and start Bun with `--config="${GSTACK_BUN_CONFIG:-/dev/null}"`. The value is `NUL` only when `OSTYPE` is `msys` or `cygwin` (Git Bash, MSYS2 and Cygwin, the check `gstack-doctor` already uses) and `cygpath` is on `PATH`; everywhere else it is `/dev/null`. The `cygpath` check is a builtin `command -v`, so it forks nothing. It means an `OSTYPE=msys` inherited by a Linux or macOS bash keeps `/dev/null`, instead of passing a relative `NUL` that Bun there resolves to a file in the repository. Bun 1.4.2 and later, the version gstack tests against, read `NUL` as an empty config whether or not conversion is on. Bun 1.3.10 on Windows exits 1 on `--config=NUL` and on the lowercase `nul` that Git Bash used to produce alike (measured from PowerShell on windows-latest; the #3085 entry recorded a panic). Versions between 1.3.3 and 1.4.1, which setup and `gstack-doctor` only warn about, were not measured. The `:-/dev/null` default covers a call that loses its definition: it still passes `/dev/null`, so POSIX is unchanged and Windows fails loudly as before, and never reads the repository's `bunfig.toml`. `bin/gstack-codex-probe` is sourced into the caller's shell, so it defines the variable inside the one function that starts Bun and still runs nothing at source time. +- **`bin/gstack-timeline-log`** and **`gstack-developer-profile --log-session`**: invalid input still exits 0 with nothing logged (the hook-path contract). Their validators now print `valid` or `invalid`, and any non-zero exit, which is Bun itself failing, is reported on stderr and returned instead of being skipped as invalid. The verdict is not an exit code because a Bun crash on Windows exits 3. When stamping a missing `ts` fails, `gstack-timeline-log` keeps the original event instead of appending an empty line. Their production callers (`gstack-skill-start`, `gstack-skill-end` and the `/office-hours` templates) still discard stderr and the exit status, so the report shows only when the bin is run directly. +- Unchanged: bun shebangs keep `--config=/dev/null` because a shebang cannot name a per-platform path, and skill prose (templates and resolvers) keeps it because `test/bun-entry-isolation.test.ts` holds skill sources to the literal spelling. TypeScript spawns keep `bunIsolationArgs()`, and `scripts/ubicloud/` keeps `/dev/null` because it runs on Linux. +- Not covered: callers that hand `bun.exe` a POSIX script path built from `pwd` or `$HOME` (the Claude hook wrappers, `autoplan/bin/phase-publication-hook`, `plan-ceo-review/bin/mode-handoff-hook`, and some `gstack-doctor`, `gstack-codex-probe`, `setup` and migration calls) only got the config spelling. With path conversion off they may still fail on the `/c/...` script path itself. The record bins convert their own paths with `cygpath -m`; the others were not verified on Windows with conversion off. Bun shebang entry points and Bun calls in skill prose still pass the literal `--config=/dev/null`, so with conversion off they fail with `EINVAL` as before. One example is the outside-voice probe in `scripts/resolvers/outside-voice.ts`, which then reports Claude as not installed. + +### Tests +- `test/bun-config-shell-bins.test.ts` (new; force-included in the Windows lane): + - On every platform, the four record bins run through `lib/script-invocation.ts` with conversion off in a repository whose `bunfig.toml` preload and `.env` are hostile. Each writes exactly one record, and neither the preload nor the `.env` state root takes effect. Invalid timeline input still exits 0 silently. + - On POSIX hosts a simulated Windows bash (`OSTYPE=msys`, a `cygpath`, and a `bun` stand-in that rejects `/dev/null` with EINVAL as `bun.exe` does) reproduces the bug: on main the four bins write nothing and the timeline control exits 0 (5 failures); with the fix they pass. The controls, without `OSTYPE=msys`, show each bin exits non-zero with nothing written. + - An inherited `OSTYPE=msys` with no `cygpath`, in a repository that commits a file named `NUL` with a preload, leaves each bin on `/dev/null`: the record is written and the preload never runs. This fails on the first version of this fix (4 failures). + - `gstack-developer-profile --log-session` reports a rejected config on stderr and exits non-zero, and still skips invalid input silently. This fails on main. + - A `bun` that exits 3 with no output, the way a Bun crash exits on Windows, makes both bins report and exit non-zero instead of skipping the event as invalid input. This fails with an exit-code verdict. +- `test/bun-isolation-launch.test.ts`: the shell-file spelling stays isolated with conversion off, and a win32-only control shows that the literal `--config=/dev/null` fails there with EINVAL. +- `test/bun-entry-isolation.test.ts`: shipped shell files must use the fail-closed spelling and define `GSTACK_BUN_CONFIG` with the canonical line before first use. A `local` definition covers only the rest of its function. Skill sources keep the literal spelling. +- `test/learnings-injection.test.ts`, `browse/test/learnings-injection.test.ts` and `test/setup-hook-canonical-paths.test.ts` now anchor on the new spelling. The two learnings-injection files also check that the anchored `bun -e` block exists. Before, a missing anchor sliced the script's last character and the negative assertions passed without checking anything. + +### Gate integrity +- `test/bun-entry-isolation.test.ts` is strengthened. On POSIX the required shell spelling still resolves to `/dev/null`. + - Now offenders: the literal `--config=/dev/null` in a shipped shell file, and an undefined or redefined `GSTACK_BUN_CONFIG`. + - The shell-caller detector also skips a `--config` value before deciding whether a call starts a program, so `bun --no-env-file --config= -e` no longer passes unchecked. The tree has no such call. + - Negative controls, each of which fails the check: the old spelling, a variable without the default, an empty `--config=`, a missing, late or altered definition, a `local` definition used from another function, and a reverted bin. + - No threshold, budget, timeout, trial count or case kind changed. + +## [1.91.226.0] - 2026-10-26 + +**/review can re-review just its own fixes after a fixing pass, under the same rules as /ship (#3339).** Before, every /review fix cycle reran the whole review over the whole branch, so a large branch could hit the three-cycle stop on findings in code the earlier passes had already read. Now the pass after an edited pass starts with `gstack-review-log --start review --fix-range` and its checklist, specialists and QA read only what changed since that pass. The Step 4.8 adversarial review still reads the whole branch on every pass, because its rows carry their own freshness. A narrowed pass is never the final review: when it makes no edits, /review runs one full pass before writing the final record, and a fix-range record is never CURRENT. Garry's "do it" on 2026-10-24 for a fix-range re-review with a final full pass covers it. + +### Changed +- **`review/SKILL.md.tmpl`** Step 3: the first pass always starts full. The pass right after an edited pass starts with `--start review --fix-range`, and wherever that pass sets DIFF_BASE outside Step 4.8 it uses the printed `FIX_BASE=` instead. Step 4.8 and every other `` use (state-matrix detection, the gate, banned-terms and slop scans, the queue check) keep the base branch. State-matrix detection builds `origin/` from its argument, and the slop scan diffs `...HEAD`, which is empty while /review's fixes are uncommitted. If the logger refuses (for example, the first pass read uncommitted work, so no commit marks what it read), the pass starts full. +- **Step 5.8**: an edited pass now saves its own record with `converged:false` before the repeat, because the fix-range start takes its base from that record. Before, only the final pass was logged. Step 5.0 never treats this invocation's own records as prior reviews, so a defect the user skipped this invocation is not suppressed on the next pass and still counts as unresolved. "No edits" fills the final record only after a full pass; after a fix-range pass it repeats Steps 3–5 with a full start. Each logged pass finishes its own REVIEW_START. +- Nearby prose was tightened to pay for the growth (the slop-scan note, the Step 4.7 probe-reuse rule, the search-before-recommending list, the version-compare note, the final-report and Greptile lines). Every host's review SKILL.md shrank by 36 characters, and its size pin was lowered to match. The Claude skeleton is 80,581 bytes against its 80,900 cap. + +### Tests +- `test/review-fix-range.test.ts` adds five cases. Two check the rendered rules: the first pass's start block has no `--fix-range`, only the pass after an edited pass narrows, FIX_BASE replaces only DIFF_BASE outside Step 4.8, a refusal falls back to a full start, the edited pass is logged before the repeat and kept out of Step 5.0, and a fix-range pass is never final. Three run /review's real order on every pass against the real logger: core start, then the Step 4.8 attempt's start and finish, then fixes in the working tree, then the core finish with the pass's own token, and never a commit. A committed first pass narrows the next repeat to the uncommitted fixes, and the following repeat is refused back to a full pass that is CURRENT. A zero-edit fix-range row is UNVERIFIED even if it were logged. A first pass over uncommitted work can't be narrowed. +- `test/review-workflow-clarity.test.ts`: three expectations follow the new contract: the "No edits" marker, the edited pass saving its record with `converged:false`, and each pass finishing its own token instead of only the final one. The test formerly named "finish only the final core token without log-time capture" is now "each logged pass finishes its own core token without log-time capture". It still requires one `--finish REVIEW_START`, no `--start review` in Step 5.8 and no log-time token capture. + +### Gate integrity +- One gate change to review: /review may narrow a repeat pass. Verdict: this does not weaken what the final review covers, because a narrowed pass can't be the final record. The logger never marks a fix-range row CURRENT, Step 5.8 requires a full pass after a zero-edit fix-range pass, Step 4.8 stays full, and Step 5.0 can't suppress this invocation's skipped defects through its own intermediate records. Mutation checks on the rendered skill, each caught by the suite: + - letting a fix-range pass be final; + - dropping the full-pass condition on "No edits"; + - narrowing every repeat; + - dropping the refusal fallback; + - narrowing the first pass; + - not logging the edited pass; + - logging after the repeat; + - narrowing Step 4.8 and other base uses; + - letting Step 5.0 read this invocation's records. +- No threshold, budget, timeout, trial count or case kind changed. Size pins only went down. + ## [1.91.225.0] - 2026-10-26 **/design-html builds landing pages around one visitor action and pages that look finished, without inventing proof; adapted from Elaya's design skills (GSTA-92).** Freeform /design-html asked for purpose, audience and feel, then built. It never asked for the one action the page exists for or where the traffic comes from, and nothing caught a page that shipped with invented customers and numbers, a dead `href="#"`, a scroll-listener reveal, `100vh`, or no description, favicon or working skip link. Now every run reads a craft section at Step 1, and a free checker counts the tells at Step 4. Adapted from Elaya's [ai-design-skills](https://github.com/elayadesign/ai-design-skills) (landing-page-design, design-to-motion) and [redesign-skill](https://github.com/elayadesign/redesign-skill), MIT, © 2026 Elaya. Thank you, Elaya. diff --git a/VERSION b/VERSION index c779e60ef..2b6fc7540 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.225.0 +1.91.230.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index b8de4afe8..e920a894c 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.225.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.230.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/autoplan/bin/phase-publication-hook b/autoplan/bin/phase-publication-hook index 2de8c92d9..c88b95e29 100755 --- a/autoplan/bin/phase-publication-hook +++ b/autoplan/bin/phase-publication-hook @@ -1,6 +1,7 @@ #!/usr/bin/env bash # Native Read barrier. A broken install must return deny JSON, never silence. set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac _AUTOPLAN_DECIDED='' _autoplan_backstop() { if [ -z "$_AUTOPLAN_DECIDED" ]; then @@ -9,7 +10,7 @@ _autoplan_backstop() { } trap _autoplan_backstop EXIT _AUTOPLAN_HOOK_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd -P)" -_AUTOPLAN_RESULT="$(bun --no-env-file --config=/dev/null "$_AUTOPLAN_HOOK_DIR/phase-publication-hook.ts")" +_AUTOPLAN_RESULT="$(bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" "$_AUTOPLAN_HOOK_DIR/phase-publication-hook.ts")" test -n "$_AUTOPLAN_RESULT" printf '%s\n' "$_AUTOPLAN_RESULT" _AUTOPLAN_DECIDED=1 diff --git a/bin/gstack-brain-sync b/bin/gstack-brain-sync index 50f377b0b..0c5d07afb 100755 --- a/bin/gstack-brain-sync +++ b/bin/gstack-brain-sync @@ -77,6 +77,7 @@ BASH_COMPAT=50 set -uo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac . "$(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 @@ -576,7 +577,7 @@ PYEOF CONSENT_TS="$SCRIPT_DIR/../lib/transcript-consent.ts" transcript_policy() { command -v bun >/dev/null 2>&1 || return 2 - GSTACK_STATE_ROOT="$GSTACK_HOME" bun --no-env-file --config=/dev/null run "$CONSENT_TS" "$@" + GSTACK_STATE_ROOT="$GSTACK_HOME" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" run "$CONSENT_TS" "$@" } # 0 when staged transcript pages may sync at all. diff --git a/bin/gstack-browser-ensure b/bin/gstack-browser-ensure index bda9718f5..764995fbb 100755 --- a/bin/gstack-browser-ensure +++ b/bin/gstack-browser-ensure @@ -24,6 +24,7 @@ # GSTACK_CHROMIUM_NO_SANDBOX=1 for the probe. Bash 3.2 plus POSIX tools. BASH_COMPAT=50 set -u +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" || { echo "gstack-browser-ensure: cannot resolve the install root" >&2; exit 1; } DOCS="https://github.com/garrytan/gstack/blob/main/docs/troubleshooting.md#browser-ensure-failed" @@ -58,7 +59,7 @@ probe() { else PROBE_OUT=$(cd "$ROOT" && node -e "$PROBE" &1); PROBE_RC=$?; fi elif command -v bun >/dev/null 2>&1; then ENGINE=bun - PROBE_OUT=$(cd "$ROOT" && bun --no-env-file --config=/dev/null -e "$PROBE" &1); PROBE_RC=$? + PROBE_OUT=$(cd "$ROOT" && bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e "$PROBE" &1); PROBE_RC=$? else fail "neither node nor bun is on PATH to launch Chromium" "install Node.js (https://nodejs.org) or Bun, then re-run $0" fi diff --git a/bin/gstack-codex-probe b/bin/gstack-codex-probe index a089b8d62..0292e5674 100755 --- a/bin/gstack-codex-probe +++ b/bin/gstack-codex-probe @@ -157,6 +157,7 @@ _gstack_codex_select_model() { # working-directory context forwarded to the shared role resolver. # The TOML is read by the TypeScript resolver; bash never parses it. local _kind="${1:-exec}" _explicit="${2:-}" _role="${3:-}" _cwd="${4:-}" _line _rc _rest + local GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac case "$_kind" in exec|review) ;; *) echo "ERROR: _gstack_codex_select_model takes exec or review" >&2; return 2 ;; esac case "$_role" in ''|plan-review) ;; *) echo "ERROR: _gstack_codex_select_model role must be plan-review" >&2; return 2 ;; esac if ! command -v bun >/dev/null 2>&1; then @@ -168,7 +169,7 @@ _gstack_codex_select_model() { [ -n "$_role" ] && set -- "$@" --role "$_role" [ -n "$_explicit" ] && set -- "$@" --explicit "$_explicit" [ -n "$_cwd" ] && set -- "$@" --cwd "$_cwd" - _line=$(bun --no-env-file --config=/dev/null run "$_GSTACK_CODEX_BIN/../scripts/resolve-codex-generation-model.ts" "$@") + _line=$(bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" run "$_GSTACK_CODEX_BIN/../scripts/resolve-codex-generation-model.ts" "$@") _rc=$? _GSTACK_CODEX_SEL="${_line%%$'\t'*}" _rest="${_line#*$'\t'}" diff --git a/bin/gstack-codex-session-import b/bin/gstack-codex-session-import index 782ae0550..73543e799 100755 --- a/bin/gstack-codex-session-import +++ b/bin/gstack-codex-session-import @@ -20,6 +20,7 @@ # Writes via bin/gstack-question-log so source tagging, dedup, and async # derive all apply uniformly. set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac SCRIPT_DIR="$(cd "$(dirname "$0")" && 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 @@ -101,7 +102,7 @@ IMPORTED=0 SKIPPED_NO_ANSWER=0 for SESSION_FILE in "${SESSION_FILES[@]}"; do - COUNT_LINE=$(SESSION_FILE_PATH="$SESSION_FILE" QLOG_BIN="$SCRIPT_DIR/gstack-question-log" bun --no-env-file --config=/dev/null -e ' + COUNT_LINE=$(SESSION_FILE_PATH="$SESSION_FILE" QLOG_BIN="$SCRIPT_DIR/gstack-question-log" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e ' const fs = require("fs"); const path = require("path"); const { spawnSync } = require("child_process"); diff --git a/bin/gstack-developer-profile b/bin/gstack-developer-profile index f5bfd5391..5e004d453 100755 --- a/bin/gstack-developer-profile +++ b/bin/gstack-developer-profile @@ -28,6 +28,7 @@ # docs/designs/PLAN_TUNING_V0.md). Event file: ~/.gstack/projects/{SLUG}/ # question-events.jsonl. set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" ROOT_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" @@ -70,7 +71,7 @@ do_migrate() { TMPOUT=$(mktemp "$GSTACK_HOME/developer-profile.json.tmp.XXXXXX") trap 'rm -f "$TMPOUT"' EXIT - cat "$LEGACY_FILE" | bun --no-env-file --config=/dev/null -e " + 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); const sessions = []; const signalsAcc = {}; @@ -127,7 +128,7 @@ do_migrate() { mv "$LEGACY_FILE" "$LEGACY_FILE.migrated-$TS" local COUNT - COUNT=$(PROFILE_FILE_PATH="$PROFILE_FILE" bun --no-env-file --config=/dev/null -e "console.log(JSON.parse(require('fs').readFileSync(process.env.PROFILE_FILE_PATH,'utf-8')).sessions.length)" 2>/dev/null || echo "?") + COUNT=$(PROFILE_FILE_PATH="$PROFILE_FILE" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e "console.log(JSON.parse(require('fs').readFileSync(process.env.PROFILE_FILE_PATH,'utf-8')).sessions.length)" 2>/dev/null || echo "?") echo "MIGRATE: ok — migrated $COUNT sessions from builder-profile.jsonl" } @@ -175,7 +176,8 @@ EOF # and update aggregates (signals_accumulated, resources_shown, topics). # Fix for #1671: the writer side of the v1.0.0.0 migration. Reader and # writer now share the same file. -# Silent skip on invalid input (matches gstack-timeline-log:22-26 pattern). +# Invalid input is skipped silently; a Bun failure is reported and returned, +# as in gstack-timeline-log (#3415). # ----------------------------------------------------------------------- do_log_session() { local INPUT="${1:-}" @@ -184,12 +186,23 @@ do_log_session() { fi # Validate: input must be parseable JSON with required fields (date, mode). - if ! printf '%s' "$INPUT" | bun --no-env-file --config=/dev/null -e " - const j = JSON.parse(await Bun.stdin.text()); - if (!j.date || !j.mode) process.exit(1); - " 2>/dev/null; then - return 0 + # The validator prints its verdict; a non-zero exit means Bun itself failed. + local VALIDATE_RC=0 VALIDATE_OUT + VALIDATE_OUT=$(printf '%s' "$INPUT" | bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " + const text = await Bun.stdin.text(); + let j; + try { j = JSON.parse(text); } catch {} + console.log(j && j.date && j.mode ? 'valid' : 'invalid'); + ") || VALIDATE_RC=$? + if [ "$VALIDATE_RC" -ne 0 ]; then + echo "gstack-developer-profile: bun exited $VALIDATE_RC before the session was validated; nothing was logged" >&2 + return "$VALIDATE_RC" fi + case "$VALIDATE_OUT" in + valid) ;; + invalid) return 0 ;; + *) echo "gstack-developer-profile: bun printed no verdict while validating the session; nothing was logged" >&2; return 1 ;; + esac ensure_profile @@ -197,7 +210,7 @@ do_log_session() { TMPOUT=$(mktemp "$GSTACK_HOME/developer-profile.json.tmp.XXXXXX") trap 'rm -f "$TMPOUT"' EXIT - PROFILE_FILE_PATH="$PROFILE_FILE" RECORD_INPUT="$INPUT" TMPOUT_PATH="$TMPOUT" bun --no-env-file --config=/dev/null -e " + PROFILE_FILE_PATH="$PROFILE_FILE" RECORD_INPUT="$INPUT" TMPOUT_PATH="$TMPOUT" 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(); @@ -244,7 +257,7 @@ do_reconcile() { TMPOUT=$(mktemp "$GSTACK_HOME/developer-profile.json.tmp.XXXXXX") trap 'rm -f "$TMPOUT"' EXIT - PROFILE_FILE_PATH="$PROFILE_FILE" PROJECTS_DIR="$GSTACK_HOME/projects" TMPOUT_PATH="$TMPOUT" bun --no-env-file --config=/dev/null -e " + PROFILE_FILE_PATH="$PROFILE_FILE" PROJECTS_DIR="$GSTACK_HOME/projects" TMPOUT_PATH="$TMPOUT" 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')); @@ -314,7 +327,7 @@ do_reconcile() { # ----------------------------------------------------------------------- do_read() { ensure_profile - cat "$PROFILE_FILE" | bun --no-env-file --config=/dev/null -e " + cat "$PROFILE_FILE" | bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " const p = JSON.parse(await Bun.stdin.text()); const sessions = p.sessions || []; @@ -388,7 +401,7 @@ do_profile() { # ----------------------------------------------------------------------- do_gap() { ensure_profile - cat "$PROFILE_FILE" | bun --no-env-file --config=/dev/null -e " + cat "$PROFILE_FILE" | bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " const p = JSON.parse(await Bun.stdin.text()); const declared = p.declared || {}; const inferred = (p.inferred && p.inferred.values) || {}; @@ -417,7 +430,7 @@ do_derive() { fi cd "$ROOT_DIR" - PROFILE_FILE_PATH="$PROFILE_FILE" EVENTS_PATH="$EVENTS" bun --no-env-file --config=/dev/null -e " + PROFILE_FILE_PATH="$PROFILE_FILE" EVENTS_PATH="$EVENTS" 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'); @@ -504,7 +517,7 @@ do_trace() { return 0 fi cd "$ROOT_DIR" - EVENTS_PATH="$EVENTS" TRACE_DIM="$DIM" bun --no-env-file --config=/dev/null -e " + EVENTS_PATH="$EVENTS" TRACE_DIM="$DIM" 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'); @@ -542,7 +555,7 @@ do_trace() { # ----------------------------------------------------------------------- do_check_mismatch() { ensure_profile - cat "$PROFILE_FILE" | bun --no-env-file --config=/dev/null -e " + cat "$PROFILE_FILE" | bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " const p = JSON.parse(await Bun.stdin.text()); const declared = p.declared || {}; const inferred = (p.inferred && p.inferred.values) || {}; @@ -586,7 +599,7 @@ do_narrative() { do_vibe() { ensure_profile cd "$ROOT_DIR" - cat "$PROFILE_FILE" | PROFILE_DATA="$(cat "$PROFILE_FILE")" bun --no-env-file --config=/dev/null -e " + cat "$PROFILE_FILE" | PROFILE_DATA="$(cat "$PROFILE_FILE")" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " import('./scripts/archetypes.ts').then(async (mod) => { const p = JSON.parse(process.env.PROFILE_DATA); const dims = (p.inferred && p.inferred.values) || { diff --git a/bin/gstack-distill-apply b/bin/gstack-distill-apply index 9a0a804b0..769445d1c 100755 --- a/bin/gstack-distill-apply +++ b/bin/gstack-distill-apply @@ -23,6 +23,7 @@ # gstack-distill-apply --proposal --gbrain-published true # gstack-distill-apply --list # show pending proposals set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac SCRIPT_DIR="$(cd "$(dirname "$0")" && 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 @@ -57,7 +58,7 @@ if [ ! -f "$PROPOSAL_FILE" ]; then fi if [ "$ACTION" = "list" ]; then - PROPOSAL_FILE_PATH="$PROPOSAL_FILE" bun --no-env-file --config=/dev/null -e ' + PROPOSAL_FILE_PATH="$PROPOSAL_FILE" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e ' const fs = require("fs"); const p = JSON.parse(fs.readFileSync(process.env.PROPOSAL_FILE_PATH, "utf-8")); const proposals = p.proposals || []; @@ -100,7 +101,7 @@ MEMORY_FILE_PATH="$MEMORY_FILE" \ PROFILE_FILE_PATH="$PROFILE_FILE" \ PREF_BIN="$SCRIPT_DIR/gstack-question-preference" \ GBRAIN_PUBLISHED="$GBRAIN_PUBLISHED" \ -bun --no-env-file --config=/dev/null -e ' +bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e ' const fs = require("fs"); const { spawnSync } = require("child_process"); const idx = parseInt(process.env.PROPOSAL_IDX, 10); diff --git a/bin/gstack-distill-free-text b/bin/gstack-distill-free-text index 97e3e1291..2c4f50f67 100755 --- a/bin/gstack-distill-free-text +++ b/bin/gstack-distill-free-text @@ -20,6 +20,7 @@ # auditability via --status when you want it. # Per D6: Anthropic SDK direct call, fail-loud on missing ANTHROPIC_API_KEY. set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac # Heredoc delivery guard. bash 5.2+ writes a heredoc body <=64KiB through a # pipe in the forked child before exec, with no reader on the other end. On @@ -59,7 +60,7 @@ esac # --- Status subcommand -------------------------------------------------- if [ "$MODE" = "status" ]; then - COST_LOG_PATH="$COST_LOG" SLUG_PATH="$SLUG" bun --no-env-file --config=/dev/null -e ' + COST_LOG_PATH="$COST_LOG" SLUG_PATH="$SLUG" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e ' const fs = require("fs"); const slug = process.env.SLUG_PATH; const path = process.env.COST_LOG_PATH; @@ -98,7 +99,7 @@ if [ ! -f "$LOG_FILE" ]; then exit 0 fi -EVENTS_JSON=$(LOG_FILE_PATH="$LOG_FILE" bun --no-env-file --config=/dev/null -e ' +EVENTS_JSON=$(LOG_FILE_PATH="$LOG_FILE" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e ' const fs = require("fs"); const lines = fs.readFileSync(process.env.LOG_FILE_PATH, "utf-8").trim().split("\n").filter(Boolean); const out = []; @@ -119,7 +120,7 @@ EVENTS_JSON=$(LOG_FILE_PATH="$LOG_FILE" bun --no-env-file --config=/dev/null -e process.stdout.write(JSON.stringify(out)); ') -EVENT_COUNT=$(printf '%s' "$EVENTS_JSON" | bun --no-env-file --config=/dev/null -e 'const a = JSON.parse(await Bun.stdin.text()); console.log(a.length);') +EVENT_COUNT=$(printf '%s' "$EVENTS_JSON" | bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e 'const a = JSON.parse(await Bun.stdin.text()); console.log(a.length);') if [ "$EVENT_COUNT" -eq 0 ]; then echo "NO_FREE_TEXT: nothing to distill" exit 0 @@ -172,7 +173,7 @@ if [ "$MODE" = "dry-run" ]; then echo "$DISTILL_PROMPT" echo echo "=== EVENTS ($EVENT_COUNT) ===" - echo "$EVENTS_JSON" | bun --no-env-file --config=/dev/null -e 'console.log(JSON.stringify(JSON.parse(await Bun.stdin.text()), null, 2));' + echo "$EVENTS_JSON" | bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e 'console.log(JSON.stringify(JSON.parse(await Bun.stdin.text()), null, 2));' exit 0 fi @@ -196,7 +197,7 @@ fi RESULT=$(EVENTS_JSON="$EVENTS_JSON" DISTILL_PROMPT="$DISTILL_PROMPT" \ PROPOSAL_FILE_PATH="$PROPOSAL_FILE" LOG_FILE_PATH="$LOG_FILE" \ ANTHROPIC_API_KEY="$ANTHROPIC_API_KEY" \ - bun --no-env-file --config=/dev/null --cwd "$ROOT_DIR" -e ' + bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" --cwd "$ROOT_DIR" -e ' const fs = require("fs"); const Anthropic = require("@anthropic-ai/sdk").default; const client = new Anthropic({ apiKey: process.env.ANTHROPIC_API_KEY }); diff --git a/bin/gstack-doctor b/bin/gstack-doctor index d088098c6..9e18a5a38 100755 --- a/bin/gstack-doctor +++ b/bin/gstack-doctor @@ -40,6 +40,7 @@ BASH_COMPAT=50 set -u +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac usage() { sed -n '3,13p' "$0" | sed 's/^# \{0,1\}//' @@ -512,7 +513,7 @@ check_windows_bash() { row warn "windows bash" "not checked: bun not found" "install Bun (see the bun row), then rerun $BIN/gstack-doctor" return fi - out=$(bun --no-env-file --config=/dev/null "$BIN/gstack-windows-bash.ts" ${GSTACK_DOCTOR_BASH_PROBE:+--probe "$GSTACK_DOCTOR_BASH_PROBE"} &1) + out=$(bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" "$BIN/gstack-windows-bash.ts" ${GSTACK_DOCTOR_BASH_PROBE:+--probe "$GSTACK_DOCTOR_BASH_PROBE"} &1) rc=$? out=$(printf '%s\n' "$out" | tr -d '\000-\010\013-\037') bash=$(printf '%s\n' "$out" | sed -n 's/^WINDOWS_BASH: //p' | head -1) diff --git a/bin/gstack-doctor-check.sh b/bin/gstack-doctor-check.sh index 12fb50a90..8cc9e2e34 100644 --- a/bin/gstack-doctor-check.sh +++ b/bin/gstack-doctor-check.sh @@ -19,6 +19,7 @@ # remote's capy/support.env, `unknown` when its fetch failed, or ""), LIVE. # GSTACK_CAPY_WRAPPER_PATH (optional) names the wrapper a below-minimum fix runs. +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac . "$BIN/gstack-doctor-components.sh" || { echo "gstack-doctor: $BIN/gstack-doctor-components.sh is missing; reinstall with cd $SRC && ./setup" >&2; exit 1; } . "$BIN/gstack-install-registry.sh" 2>/dev/null || { echo "gstack-doctor: $BIN/gstack-install-registry.sh is missing; reinstall with cd $SRC && ./setup" >&2; exit 1; } . "$BIN/gstack-bun-version.sh" 2>/dev/null || { echo "gstack-doctor: $BIN/gstack-bun-version.sh is missing; reinstall with cd $SRC && ./setup" >&2; exit 1; } @@ -213,7 +214,7 @@ _dc_check_browser() { out=$(cd "$ROOT" && { command -v timeout >/dev/null 2>&1 && timeout 90 node -e "$script" &1 || node -e "$script" &1; }); rc=$? elif command -v bun >/dev/null 2>&1; then engine=bun - out=$(cd "$ROOT" && bun --no-env-file --config=/dev/null -e "$script" &1); rc=$? + out=$(cd "$ROOT" && bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e "$script" &1); rc=$? else _dc_row FAIL browser "neither node nor bun is on PATH to launch Chromium" "install Node.js (https://nodejs.org) or Bun, then $fix" BROWSER_UNAVAILABLE return @@ -329,7 +330,7 @@ _dc_check_pins() { return fi node=$(node --version /dev/null | clamp 'v0-9.' 20); node="${node#v}" - out=$(bun --no-env-file --config=/dev/null "$BIN/gstack-runtime-pins.ts" --project "$project" ${_dc_bun:+--bun "$_dc_bun"} ${node:+--node "$node"} &1); rc=$? + out=$(bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" "$BIN/gstack-runtime-pins.ts" --project "$project" ${_dc_bun:+--bun "$_dc_bun"} ${node:+--node "$node"} &1); rc=$? verdict=$(printf '%s\n' "$out" | sed -n 's/^PINS_VERDICT: //p' | tail -1 | clamp 'a-z' 8) summary=$(printf '%s\n' "$out" | sed -n 's/^PIN_SUMMARY: //p' | tr '\n' '|' | sed 's/|$//; s/|/; /g') case "$verdict" in @@ -357,7 +358,7 @@ _dc_check_pins() { fi fi if [ -n "$_dc_ln_dir" ] && [ "$_dc_ln_ver" != "$_dc_bun" ] \ - && bun --no-env-file --config=/dev/null "$BIN/gstack-runtime-pins.ts" --project "$project" --bun "$_dc_ln_ver" ${node:+--node "$node"} /dev/null | grep -q '^PINS_VERDICT: pass'; then + && bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" "$BIN/gstack-runtime-pins.ts" --project "$project" --bun "$_dc_ln_ver" ${node:+--node "$node"} /dev/null | grep -q '^PINS_VERDICT: pass'; then note="; bun $_dc_ln_ver found at $_dc_ln_dir (not on PATH) satisfies them" fix="$_dc_ln_fix" fi _dc_row FAIL pins "$failline$note" "$fix" PROJECT_PIN_MISMATCH ;; diff --git a/bin/gstack-gbrain-install b/bin/gstack-gbrain-install index 9ecdcb4e0..4b67a12c6 100755 --- a/bin/gstack-gbrain-install +++ b/bin/gstack-gbrain-install @@ -36,6 +36,7 @@ # 2 — prerequisite missing or invalid argument # 3 — post-install validation failed (PATH shadow, broken binary, etc.) set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac # --- defaults --- # No version pin by default — install the latest default-branch HEAD (#1744). @@ -100,7 +101,7 @@ fi # bun, which this installer already requires. It used to need jq, which it # never checked for, so a successful install reported failure (#1675). pkg_field() { - GSTACK_PKG_JSON="$1" GSTACK_PKG_FIELD="$2" bun --no-env-file --config=/dev/null -e ' + GSTACK_PKG_JSON="$1" GSTACK_PKG_FIELD="$2" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e ' try { const pkg = JSON.parse(require("fs").readFileSync(process.env.GSTACK_PKG_JSON, "utf8")); const v = process.env.GSTACK_PKG_FIELD.split(".").reduce((o, k) => (o == null ? o : o[k]), pkg); @@ -198,7 +199,7 @@ _bun_exe_hint() { [ "$IS_WINDOWS" -eq 1 ] || return 0 command -v bun.exe >/dev/null 2>&1 && return 0 local real_bun - real_bun=$(bun --no-env-file --config=/dev/null -e 'console.log(process.execPath)' 2>/dev/null | tr -d '\r' || true) + real_bun=$(bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e 'console.log(process.execPath)' 2>/dev/null | tr -d '\r' || true) echo " detected: bun was installed via npm — bun.exe is NOT on %PATH%, and the gbrain.exe shim needs it." >&2 if [ -n "$real_bun" ]; then echo " fix: add bun.exe's directory to PATH (persist it in your shell profile):" >&2 diff --git a/bin/gstack-gbrain-source-wireup b/bin/gstack-gbrain-source-wireup index c5f23738c..97cbb1b4a 100755 --- a/bin/gstack-gbrain-source-wireup +++ b/bin/gstack-gbrain-source-wireup @@ -39,6 +39,7 @@ # Depends on: bun (already a gstack dependency), gbrain, git. set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" CONFIG_BIN="$SCRIPT_DIR/gstack-config" @@ -222,7 +223,7 @@ ensure_worktree() { # is not JSON. source_local_path() { gbrain sources list --json 2>/dev/null \ - | GB_SOURCE_ID="$1" bun --no-env-file --config=/dev/null -e ' + | GB_SOURCE_ID="$1" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e ' const raw = await Bun.stdin.text(); try { const s = (JSON.parse(raw).sources || []).find((x) => x.id === process.env.GB_SOURCE_ID); diff --git a/bin/gstack-hook-check b/bin/gstack-hook-check index e6cc2a73c..a399d2ba6 100755 --- a/bin/gstack-hook-check +++ b/bin/gstack-hook-check @@ -37,6 +37,7 @@ # (needed when a hook runs TypeScript) is not found. Set GSTACK_HOOK_CHECK_BUN to # the bun to run (setup passes its own). set -u +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac LIST_ONLY=0 SETUP_FILE="" while [ $# -gt 1 ]; do @@ -173,7 +174,7 @@ if [ -n "$ENTRIES" ]; then # up below (lib/path-spelling.ts shellPath; test/path-spelling.test.ts keeps # the two equal). # shellcheck disable=SC2086 # newline-split list (IFS is set to a newline below) - (cd "$ROOT" && IFS=$NL && "$BUN" --no-env-file --config=/dev/null -e ' + (cd "$ROOT" && IFS=$NL && "$BUN" --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e ' const shellPath = (value) => process.platform === "win32" ? String(value).replace(/\\/g, "/").replace(/^([A-Za-z]):(?=\/|$)/, (_, drive) => `/${drive.toLowerCase()}`) : String(value); diff --git a/bin/gstack-install-verdict.sh b/bin/gstack-install-verdict.sh index e500344a8..3e244e830 100644 --- a/bin/gstack-install-verdict.sh +++ b/bin/gstack-install-verdict.sh @@ -31,6 +31,8 @@ # spawned sessions (ambient OpenClaw) only warn. GSTACK_ALLOW_UNSUPPORTED_RUNTIME=1 # admits anyway and says so. +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac + # gstack_source_root BIN_DIR — the source checkout behind an install: an # installed runtime root's recorded .source-path (setup _link_runtime_dists), # else BIN_DIR/.. resolved. Prints the realpath; returns 1 when unresolvable. @@ -159,7 +161,7 @@ gstack_install_admission() { v_bun=$(gstack_install_verdict_field "$file" bun_path) if [ -z "$detail" ] && [ -n "$v_bun" ]; then running=$(gstack_realpath "$where" 2>/dev/null) - _gstack_bun_is_shim "$where" && running=$(bun --no-env-file --config=/dev/null -p process.execPath /dev/null | head -1) + _gstack_bun_is_shim "$where" && running=$(bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -p process.execPath /dev/null | head -1) if [ -n "$running" ] && [ "$running" != "$v_bun" ]; then _ia_status=runtime code=BUN_RESOLUTION_DRIFT detail="bun $found at $running is running, but the install verdict recorded bun $(gstack_install_verdict_field "$file" bun_version) at $v_bun" diff --git a/bin/gstack-ios-qa-daemon b/bin/gstack-ios-qa-daemon index 391dd225d..0a2f713c7 100755 --- a/bin/gstack-ios-qa-daemon +++ b/bin/gstack-ios-qa-daemon @@ -21,6 +21,7 @@ # session tokens are outstanding. set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac # The --help heredoc is over 512 bytes; keep it off a pipe (test/heredoc-pipe-deadlock.test.ts). BASH_COMPAT=50 @@ -67,4 +68,4 @@ if ! command -v bun >/dev/null 2>&1; then exit 1 fi -exec bun --no-env-file --config=/dev/null run "$ENTRY" "$@" +exec bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" run "$ENTRY" "$@" diff --git a/bin/gstack-ios-qa-mint b/bin/gstack-ios-qa-mint index 8270ffbda..44cbb8ebf 100755 --- a/bin/gstack-ios-qa-mint +++ b/bin/gstack-ios-qa-mint @@ -10,6 +10,7 @@ # Allowlist file: ~/.gstack/ios-qa-allowlist.json (mode 0600). set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" GSTACK_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" @@ -25,4 +26,4 @@ if ! command -v bun >/dev/null 2>&1; then exit 1 fi -exec bun --no-env-file --config=/dev/null run "$ENTRY" "$@" +exec bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" run "$ENTRY" "$@" diff --git a/bin/gstack-ios-qa-regen b/bin/gstack-ios-qa-regen index b14a86251..33e6c95b9 100755 --- a/bin/gstack-ios-qa-regen +++ b/bin/gstack-ios-qa-regen @@ -3,6 +3,7 @@ # package and the app-owned typed state accessors. set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac usage() { cat <<'EOF' @@ -144,7 +145,7 @@ do fi done -bun --no-env-file --config=/dev/null run "$GENERATOR" --input "$APP_SOURCE" --output "$GENERATED_DIR" +bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" run "$GENERATOR" --input "$APP_SOURCE" --output "$GENERATED_DIR" # Stamp only after successful accessor generation. ios-sync uses this marker # to distinguish a complete current install from an interrupted regeneration. diff --git a/bin/gstack-learnings-log b/bin/gstack-learnings-log index ca8bfeb05..f5fa21713 100755 --- a/bin/gstack-learnings-log +++ b/bin/gstack-learnings-log @@ -6,6 +6,7 @@ # Append-only storage. Duplicates (same key+type) are resolved at read time # by gstack-learnings-search ("latest winner" per key+type). set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" # Windows git-bash (#1950): pwd yields a POSIX path (/c/Users/...), which Bun # on Windows cannot resolve as an ES module specifier in the import below. @@ -27,7 +28,7 @@ INPUT="$1" TMPERR=$(mktemp "${TMPDIR:-/tmp}/gstack-learnings-log.XXXXXX") || { echo "gstack-learnings-log: could not create a temp file in ${TMPDIR:-/tmp}, so the learning was not recorded. Fix: point TMPDIR at a writable directory." >&2; exit 1; } trap 'rm -f "$TMPERR"' EXIT set +e -VALIDATED=$(printf '%s' "$INPUT" | GSTACK_LIB_DIR="$SCRIPT_DIR/../lib" bun --no-env-file --config=/dev/null -e " +VALIDATED=$(printf '%s' "$INPUT" | GSTACK_LIB_DIR="$SCRIPT_DIR/../lib" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " const { hasInjection } = await import(process.env.GSTACK_LIB_DIR + '/jsonl-store.ts'); const raw = await Bun.stdin.text(); let j; diff --git a/bin/gstack-learnings-search b/bin/gstack-learnings-search index 6f58737a4..8e19da27c 100755 --- a/bin/gstack-learnings-search +++ b/bin/gstack-learnings-search @@ -8,6 +8,7 @@ # exits 127 and a failed read exits non-zero with its stderr (#2790): "could # not read" must never look like "nothing recorded". set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" eval "$("$SCRIPT_DIR/gstack-slug" 2>/dev/null)" . "$(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; } @@ -69,7 +70,7 @@ emit_tagged_file() { emit_tagged_file cross "$f" done fi -} | GSTACK_SEARCH_TYPE="$TYPE" GSTACK_SEARCH_QUERY="$QUERY" GSTACK_SEARCH_LIMIT="$LIMIT" GSTACK_SEARCH_CROSS="$CROSS_PROJECT" bun --no-env-file --config=/dev/null -e " +} | GSTACK_SEARCH_TYPE="$TYPE" GSTACK_SEARCH_QUERY="$QUERY" GSTACK_SEARCH_LIMIT="$LIMIT" GSTACK_SEARCH_CROSS="$CROSS_PROJECT" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " const lines = (await Bun.stdin.text()).trim().split('\n').filter(Boolean); const now = Date.now(); const type = process.env.GSTACK_SEARCH_TYPE || ''; diff --git a/bin/gstack-lease b/bin/gstack-lease index ded79313e..2073af92d 100755 --- a/bin/gstack-lease +++ b/bin/gstack-lease @@ -14,7 +14,8 @@ # (bin/gstack-harness-pid.sh, the same derivation the session marker # uses), or behind --from (a caller whose own parent is short-lived # passes its parent pid); leases older than 120 min are pruned. Exits 3 -# and writes nothing when a live process holds the directory lock +# and writes nothing when a live process holds the directory lock or +# the lock cannot be taken # gstack-lease live [--root ] [--exclude-ancestry-of ] # print ` m ` per lease held by a live process that # is neither the given pid nor one of its ancestors (the caller's own @@ -31,7 +32,7 @@ # runtime root's .source-path, else ../). A root that is not a git checkout # exits 2 and writes nothing. GSTACK_LEASE_DIR overrides the lease directory # (hermetic tests; a dev checkout's own tests must not lease it). Bash 3.2 -# plus git, ps, find, ln, readlink. +# plus git, ps, find, stat, date, head, mkdir, mv, readlink. set -u BIN="$(cd "$(dirname "$0")" && pwd -P)" # shellcheck source=bin/gstack-install-verdict.sh @@ -53,7 +54,7 @@ while [ $# -gt 0 ]; do done case "$VERB" in touch|live|list) ;; - -h|--help|'') sed -n '2,24p' "$0" | sed 's/^# \{0,1\}//'; exit 0 ;; + -h|--help|'') sed -n '2,25p' "$0" | sed 's/^# \{0,1\}//'; exit 0 ;; *) echo "gstack-lease: unknown verb '$VERB'" >&2; exit 2 ;; esac [ -n "$ROOT" ] || ROOT=$(gstack_source_root "$BIN") @@ -99,43 +100,139 @@ state() { } # The lease directory's lock: touch's publish and prune, and reclaim's # reread-and-delete, run under it, so a reclaim never deletes a renewal and a -# lease never disappears while it is live. The lock is a symlink whose target -# names its holder (` `): ln -s creates it and its owner -# in one atomic step. It is broken only when that holder is dead or its pid -# now belongs to another process, never by age, and unlock removes only the -# caller's own lock. A live holder makes lock() give up after -# GSTACK_LEASE_LOCK_TRIES tries 0.1 s apart (default 300). +# lease never disappears while it is live. The lock is a directory, .lock, +# whose owner file names its holder (` `); mkdir is +# atomic everywhere gstack runs, Git Bash on Windows included, where ln -s to a +# target that does not exist always fails (#3330). A taker publishes its token +# as a claim (.lock.claim.) before its mkdir and renames the claim into +# the lock as its owner after, so a lock without an owner is either being +# taken by a live claimant or orphaned by a dead one. A lock is broken only +# when its holder is dead or its pid now belongs to another process, never by +# age, and unlock removes only the caller's own lock. An ownerless lock is +# broken once no claimant lives and it is at least 2 s old (and unchanged +# while it was checked), so a taker between its mkdir and its owner rename is +# never taken for a crash, even when its claim landed after the claims were +# read. A claim is honored while its claimant lives; one that records no +# start identity (so a reused pid cannot be told apart) is abandoned after 2 +# minutes, since a live taker holds one for milliseconds. Known limit: on such +# a host a taker stalled over 2 minutes between its mkdir and its owner rename +# can be broken and then publish into its successor's lock. An owner whose +# pid is 0, zero-padded or not a number counts as a dead holder, and a lock +# whose owner file exists but cannot be read is never treated as ownerless. A symlink .lock left by an older +# gstack names its holder the same way and excludes a directory lock at the +# same path. Known limit: the reverse does not hold, since an older gstack's +# ln -s onto a directory lock links inside it and succeeds; only a process +# that started before an upgrade of this checkout can run that code. A live holder +# makes lock() give up after GSTACK_LEASE_LOCK_TRIES tries 0.1 s apart +# (default 300) or 30 s, whichever comes first. LOCK="$DIR/.lock" TOKEN="" +# take PATH — create the lock PATH owned by TOKEN; 1 when it exists. +take() { + if [ -e "$1" ] || [ -L "$1" ]; then return 1; fi + { printf '%s\n' "$TOKEN" > "$1.claim.$$"; } 2>/dev/null || { rm -f "$1.claim.$$"; return 1; } + if mkdir "$1" 2>/dev/null; then + [ ! -e "$1/owner" ] && mv -f "$1.claim.$$" "$1/owner" && return 0 + [ -e "$1/owner" ] || rm -rf "$1" + fi + rm -f "$1.claim.$$" + return 1 +} +# owner_of PATH — the token the lock PATH names, empty when it names none. +owner_of() { if [ -L "$1" ]; then readlink "$1" 2>/dev/null; else head -1 "$1/owner" 2>/dev/null; fi; } +mtime() { stat -c %Y "$1" 2>/dev/null || stat -f %m "$1" 2>/dev/null; } +# orphaned PATH — PATH is a lock (not a legacy symlink) whose owner file is +# missing or readable and empty, has no live claimant, is at least 2 s old and kept +# its mtime throughout. The clock is read before the first mtime, so a pause +# only makes the lock look younger, and a lock made since (a new generation) +# has a newer mtime. Claims are read before the owner, so a taker that renames +# its claim in between is seen as the owner. An empty claim is a taker that +# has not reached its mkdir, so it never made this lock. An mtime a minute or +# more in the future (clock skew) counts as old. +orphaned() { + _oclock=$(date +%s) + [ -e "$1" ] && [ ! -L "$1" ] || return 1 + _om=$(mtime "$1") || return 1 + for _c in "$1".claim.*; do + _t=$(head -1 "$_c" 2>/dev/null) + abandoned "$_c" "$_t" && continue + [ -n "$_t" ] && ! holder_gone "$_t" && return 1 + done + if [ -d "$1" ] && [ -e "$1/owner" ]; then + _o=$(head -1 "$1/owner" 2>/dev/null) || return 1 + [ -z "$_o" ] || return 1 + fi + [ "$(mtime "$1")" = "$_om" ] || return 1 + _oage=$((_oclock - _om)) + [ "$_oage" -ge 2 ] || [ "$_oage" -le -60 ] +} +# drop PATH — remove a lock in one step: a directory is renamed aside first, +# so nobody sees it ownerless on the way out. Once renamed the lock is +# released, whether or not the renamed copy can be deleted (touch prunes it). +drop() { + if [ -L "$1" ]; then rm -f "$1"; return; fi + mv "$1" "$1.gone.$$" 2>/dev/null || return 1 + rm -rf "$1.gone.$$" 2>/dev/null + return 0 +} lock() { [ -n "$TOKEN" ] || TOKEN="$$ $(identity $$)" - _tries=0 - until ln -s "$TOKEN" "$LOCK" 2>/dev/null; do - _tries=$((_tries + 1)) - LOCK_OWNER=$(readlink "$LOCK" 2>/dev/null) - [ -n "$LOCK_OWNER" ] && holder_gone "$LOCK_OWNER" && break_lock "$LOCK_OWNER" && continue - [ "$_tries" -ge "${GSTACK_LEASE_LOCK_TRIES:-300}" ] && return 1 + _tries=0 _gone=0 _max=${GSTACK_LEASE_LOCK_TRIES:-300} _start=$SECONDS + case "$_max" in ''|*[!0-9]*) _max=300 ;; esac + until take "$LOCK"; do + LOCK_OWNER=$(owner_of "$LOCK") + if [ ! -e "$LOCK" ] && [ ! -L "$LOCK" ]; then + # Released since take, or take itself cannot work here: never wait out + # the tries for a lock nobody holds. + _gone=$((_gone + 1)) + [ "$_gone" -ge 10 ] && return 1 + continue + fi + _gone=0 _tries=$((_tries + 1)) + if [ -n "$LOCK_OWNER" ]; then holder_gone "$LOCK_OWNER" && break_lock "$LOCK_OWNER" && continue + elif orphaned "$LOCK"; then break_lock "" && continue + fi + [ "$_tries" -ge "$_max" ] && return 1 + [ $((SECONDS - _start)) -ge 30 ] && return 1 sleep 0.1 done } -holder_gone() { case "$(state "${1%% *}" "${1#* }" 0)" in dead|stale) return 0 ;; esac; return 1; } -# break_lock OWNER — remove the lock only if it still names OWNER. Breakers -# take a second symlink, .lock.break, and reread the lock under it, so a lock -# taken since the verdict is never removed; a .lock.break whose holder is gone -# is cleared. Returns 1 when another breaker holds .lock.break. Known limit: -# that clear is not atomic, so two gstack-lease processes killed in a row (one -# holding .lock, the next inside this function) can let a reclaim delete a -# renewal; one crash is always safe. +# abandoned CLAIM TOKEN — the claim records no start identity and is over 2 +# minutes old. +abandoned() { [ -z "${2#* }" ] && [ -z "$(find "$1" -mmin -2 2>/dev/null)" ]; } +holder_gone() { + case "${1%% *}" in ''|*[!0-9]*|0*) return 0 ;; esac + case "$(state "${1%% *}" "${1#* }" 0)" in dead|stale) return 0 ;; esac; return 1 +} +# break_lock OWNER — remove the lock only if it still names OWNER (empty: is +# still orphaned). Breakers take a second lock, .lock.break, and reread the +# lock under it, so a lock taken since the verdict is never removed; a +# .lock.break whose holder is gone is cleared. A lock that cannot be renamed +# aside is removed in place (other breakers wait on .lock.break meanwhile). +# Returns 1 when another breaker holds .lock.break or the lock could not be +# removed, so lock() counts the try instead of spinning. Known limit: that +# clear is check-then-remove, so a breaker killed while holding .lock.break, +# followed by two live breakers clearing it at once, can let both break and a +# reclaim delete a renewal. break_lock() { - if ! ln -s "$TOKEN" "$LOCK.break" 2>/dev/null; then - _b=$(readlink "$LOCK.break" 2>/dev/null) - [ -n "$_b" ] && holder_gone "$_b" && [ "$(readlink "$LOCK.break" 2>/dev/null)" = "$_b" ] && rm -f "$LOCK.break" + if ! take "$LOCK.break"; then + _b=$(owner_of "$LOCK.break") + if [ -n "$_b" ]; then + holder_gone "$_b" && [ "$(owner_of "$LOCK.break")" = "$_b" ] && drop "$LOCK.break" + else + orphaned "$LOCK.break" && drop "$LOCK.break" + fi return 1 fi - [ "$(readlink "$LOCK" 2>/dev/null)" = "$1" ] && rm -f "$LOCK" - rm -f "$LOCK.break" - return 0 + _kept=0 + if [ -n "$1" ]; then + [ "$(owner_of "$LOCK")" = "$1" ] && { drop "$LOCK" || rm -rf "$LOCK" || _kept=1; } + else + orphaned "$LOCK" && { drop "$LOCK" || rm -rf "$LOCK" || _kept=1; } + fi + drop "$LOCK.break" + return $_kept } -unlock() { [ "$(readlink "$LOCK" 2>/dev/null)" = "$TOKEN" ] && rm -f "$LOCK"; return 0; } +unlock() { [ "$(owner_of "$LOCK")" = "$TOKEN" ] && { drop "$LOCK" || rm -rf "$LOCK"; }; return 0; } # reclaim FILE RECORD — delete a stale lease only if, under the lock, it still # holds the record judged stale; a renewal since then keeps it. reclaim() { @@ -153,10 +250,21 @@ case "$VERB" in touch) [ -n "$PID" ] || PID=$(gstack_harness_pid "${FROM:-$PPID}") mkdir -p "$DIR" || exit 2 - lock || { echo "gstack-lease: the lease directory is locked by live process ${LOCK_OWNER%% *}; lease $PID not renewed" >&2; exit 3; } + lock || { + if [ -n "${LOCK_OWNER:-}" ] && ! holder_gone "$LOCK_OWNER"; then echo "gstack-lease: the lease directory is locked by live process ${LOCK_OWNER%% *}; lease $PID not renewed" >&2 + else echo "gstack-lease: cannot take the lease directory lock $LOCK; lease $PID not renewed" >&2 + fi + exit 3 + } _tmp="$DIR/.$PID.$$" identity "$PID" > "$_tmp" && mv -f "$_tmp" "$DIR/$PID" || { rm -f "$_tmp"; unlock; exit 2; } - find "$DIR" -mmin +120 -type f -exec rm -f {} + 2>/dev/null + find "$DIR" -maxdepth 1 -mmin +120 -type f ! -name '.lock*' -exec rm -f {} + 2>/dev/null + for _c in "$DIR"/.lock*.claim.*; do + [ -f "$_c" ] || continue + _t=$(head -1 "$_c" 2>/dev/null) + [ -n "$(find "$_c" -mmin +2 2>/dev/null)" ] && { abandoned "$_c" "$_t" || holder_gone "$_t"; } && rm -f "$_c" + done + rm -rf "$DIR"/.lock*.gone.* 2>/dev/null unlock exit 0 ;; list) diff --git a/bin/gstack-memorable b/bin/gstack-memorable index 629040674..558767b14 100755 --- a/bin/gstack-memorable +++ b/bin/gstack-memorable @@ -32,6 +32,7 @@ # Heredoc delivery guard (see bin/gstack-settings-hook for the rationale). BASH_COMPAT=50 set -uo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" ROOT_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" @@ -374,7 +375,7 @@ status_bridge() { # JSON; a failed query is reported as unknown, never as an empty history. local egress_json if egress_json="$("$EGRESS_BIN" list --sink "$SINK" --json 2>/dev/null)"; then - n="$(printf '%s' "$egress_json" | bun --no-env-file --config=/dev/null -e 'const a=JSON.parse(require("fs").readFileSync(0,"utf8")||"[]");console.log(Array.isArray(a)?a.length:0)' 2>/dev/null)" + n="$(printf '%s' "$egress_json" | bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e 'const a=JSON.parse(require("fs").readFileSync(0,"utf8")||"[]");console.log(Array.isArray(a)?a.length:0)' 2>/dev/null)" case "$n" in *[!0-9]*|"") n="unknown (could not parse gstack-egress output)" ;; esac else n="unknown (gstack-egress list failed; run it yourself)" diff --git a/bin/gstack-question-log b/bin/gstack-question-log index 7d7fd7b00..6661ba8a8 100755 --- a/bin/gstack-question-log +++ b/bin/gstack-question-log @@ -31,6 +31,7 @@ # # Append-only JSONL. Dedup is at read time in gstack-question-sensitivity --read-log. set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" # Windows git-bash (#1950): pwd yields a POSIX path (/c/Users/...), which Bun # on Windows cannot resolve as an ES module specifier in bun -e imports. @@ -51,7 +52,7 @@ INPUT="$1" TMPERR=$(mktemp "${TMPDIR:-/tmp}/gstack-question-log.XXXXXX") || { echo "gstack-question-log: could not create a temp file in ${TMPDIR:-/tmp}, so the question was not logged. Fix: point TMPDIR at a writable directory." >&2; exit 1; } trap 'rm -f "$TMPERR"' EXIT set +e -VALIDATED=$(printf '%s' "$INPUT" | GSTACK_LIB_DIR="$SCRIPT_DIR/../lib" GSTACK_SCRIPTS_DIR="$SCRIPT_DIR/../scripts" bun --no-env-file --config=/dev/null -e " +VALIDATED=$(printf '%s' "$INPUT" | GSTACK_LIB_DIR="$SCRIPT_DIR/../lib" GSTACK_SCRIPTS_DIR="$SCRIPT_DIR/../scripts" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " const { hasInjection } = await import(process.env.GSTACK_LIB_DIR + '/jsonl-store.ts'); const { resolveOptionKey } = await import(process.env.GSTACK_SCRIPTS_DIR + '/psychographic-signals.ts'); const path = require('path'); @@ -241,7 +242,7 @@ LOG_FILE="$GSTACK_HOME/projects/$SLUG/question-log.jsonl" # Lookup is bounded so the bin stays cheap on hot paths. DEDUP_SKIP="" if [ -f "$LOG_FILE" ]; then - DEDUP_SKIP=$(VALIDATED_JSON="$VALIDATED" LOG_FILE_PATH="$LOG_FILE" bun --no-env-file --config=/dev/null -e ' + DEDUP_SKIP=$(VALIDATED_JSON="$VALIDATED" LOG_FILE_PATH="$LOG_FILE" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e ' const fs = require("fs"); const j = JSON.parse(process.env.VALIDATED_JSON); if (!j.tool_use_id) { console.log(""); process.exit(0); } @@ -261,7 +262,7 @@ if [ -f "$LOG_FILE" ]; then fi if [ "$DEDUP_SKIP" = "dup" ]; then - echo "DEDUP: skipped (source=$(echo "$VALIDATED" | bun --no-env-file --config=/dev/null -e 'const j=JSON.parse(await Bun.stdin.text()); console.log(j.source);'), tool_use_id duplicate)" + echo "DEDUP: skipped (source=$(echo "$VALIDATED" | bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e 'const j=JSON.parse(await Bun.stdin.text()); console.log(j.source);'), tool_use_id duplicate)" exit 0 fi diff --git a/bin/gstack-question-preference b/bin/gstack-question-preference index 2cc2e81fc..5d0d1af40 100755 --- a/bin/gstack-question-preference +++ b/bin/gstack-question-preference @@ -35,6 +35,7 @@ # --check already ignores those prefs; storing them made --stats lie. # always-ask on a one-way id is fine (it agrees with the safety override). set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" ROOT_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" @@ -105,7 +106,7 @@ do_check() { fi ensure_file cd "$ROOT_DIR" - PREF_FILE_PATH="$PREF_FILE" GLOBAL_PREF_FILE_PATH="$GLOBAL_PREF_FILE" QID="$QID" QSUMMARY="$QSUMMARY" bun --no-env-file --config=/dev/null -e " + PREF_FILE_PATH="$PREF_FILE" GLOBAL_PREF_FILE_PATH="$GLOBAL_PREF_FILE" QID="$QID" QSUMMARY="$QSUMMARY" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " import('./scripts/one-way-doors.ts').then((oneway) => { const fs = require('fs'); const qid = process.env.QID; @@ -201,7 +202,7 @@ do_write() { set +e local RESULT - RESULT=$(cd "$ROOT_DIR" && printf '%s' "$INPUT" | PREF_FILE_PATH="$PREF_FILE" EVENT_FILE_PATH="$EVENT_FILE" FREE_TEXT="$FREE_TEXT" HAS_FREE_TEXT="$HAS_FREE_TEXT" bun --no-env-file --config=/dev/null -e " + RESULT=$(cd "$ROOT_DIR" && printf '%s' "$INPUT" | PREF_FILE_PATH="$PREF_FILE" EVENT_FILE_PATH="$EVENT_FILE" FREE_TEXT="$FREE_TEXT" HAS_FREE_TEXT="$HAS_FREE_TEXT" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " const fs = require('fs'); const raw = await Bun.stdin.text(); let j; @@ -320,7 +321,7 @@ do_clear() { echo '{}' > "$PREF_FILE" echo "OK: cleared all preferences" else - PREF_FILE_PATH="$PREF_FILE" QID="$QID" bun --no-env-file --config=/dev/null -e " + PREF_FILE_PATH="$PREF_FILE" QID="$QID" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " const fs = require('fs'); const prefs = JSON.parse(fs.readFileSync(process.env.PREF_FILE_PATH, 'utf-8')); if (prefs[process.env.QID] !== undefined) { @@ -339,7 +340,7 @@ do_clear() { # ----------------------------------------------------------------------- do_stats() { ensure_file - (cd "$ROOT_DIR" && PREF_FILE_PATH="$PREF_FILE" bun --no-env-file --config=/dev/null -e " + (cd "$ROOT_DIR" && PREF_FILE_PATH="$PREF_FILE" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " import('./scripts/one-way-doors.ts').then((oneway) => { const fs = require('fs'); const prefs = JSON.parse(fs.readFileSync(process.env.PREF_FILE_PATH, 'utf-8')); diff --git a/bin/gstack-relink b/bin/gstack-relink index f280390c2..f7443deba 100755 --- a/bin/gstack-relink +++ b/bin/gstack-relink @@ -9,6 +9,7 @@ # GSTACK_INSTALL_DIR — override gstack install directory # GSTACK_SKILLS_DIR — override target skills directory set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" GSTACK_CONFIG="${SCRIPT_DIR}/gstack-config" @@ -474,7 +475,7 @@ if { [ "$DISABLED" != " " ] || grep -q '^Disabled on this install' "$_ROUTER" 2> _ROUTER_TMP="$(mktemp -d "${TMPDIR:-/tmp}/gstack-router.XXXXXX")" || _ROUTER_TMP="" _DISABLED_CSV="$(printf '%s' "$DISABLED" | xargs | tr ' ' ',')" if [ -n "$_ROUTER_TMP" ] && command -v bun >/dev/null 2>&1 \ - && (cd "$_GEN_SRC" && bun --no-env-file --config=/dev/null run scripts/gen-skill-docs.ts --host claude --out-dir "$_ROUTER_TMP" ${_DISABLED_CSV:+"--disabled-skills=$_DISABLED_CSV"} ${_PLAN_MODE_ARG:+"$_PLAN_MODE_ARG"} >/dev/null 2>&1) \ + && (cd "$_GEN_SRC" && bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" run scripts/gen-skill-docs.ts --host claude --out-dir "$_ROUTER_TMP" ${_DISABLED_CSV:+"--disabled-skills=$_DISABLED_CSV"} ${_PLAN_MODE_ARG:+"$_PLAN_MODE_ARG"} >/dev/null 2>&1) \ && [ -s "$_ROUTER_TMP/SKILL.md" ] && cp "$_ROUTER_TMP/SKILL.md" "$_ROUTER.tmp.$$" && mv "$_ROUTER.tmp.$$" "$_ROUTER"; then _link_root_skill_alias [ -n "$_DISABLED_CSV" ] && echo "Router: disabled skills left out of $_ROUTER" diff --git a/bin/gstack-review-log b/bin/gstack-review-log index 102fb1dc1..b60832497 100755 --- a/bin/gstack-review-log +++ b/bin/gstack-review-log @@ -17,6 +17,7 @@ # a model read the code. Caller-supplied binding fields are always discarded. # Plan-tier rows retain their legacy binding behavior. set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac export GIT_OPTIONAL_LOCKS=0 export GIT_CONFIG_PARAMETERS="${GIT_CONFIG_PARAMETERS:+$GIT_CONFIG_PARAMETERS }'core.fsmonitor=false' 'core.untrackedCache=false'" SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" @@ -77,7 +78,7 @@ SESSION_KIND=$("$SCRIPT_DIR/gstack-session-kind" 2>/dev/null || echo interactive export GSTACK_STAMP_COMMIT_FULL="$COMMIT_FULL" GSTACK_STAMP_TREE="$TREE" GSTACK_STAMP_WTREE="$WTREE" GSTACK_STAMP_DIRTY="$DIRTY" GSTACK_STAMP_SESSION_KIND="$SESSION_KIND" if [ "$INPUT" = --start ]; then case "${3:-}" in ''|--fix-range) ;; *) echo 'Usage: gstack-review-log --start SKILL [--fix-range]' >&2; exit 2 ;; esac - GSTACK_REVIEW_SKILL="${2:-}" GSTACK_REVIEW_FIX_RANGE="${3:-}" bun --no-env-file --config=/dev/null -e ' + GSTACK_REVIEW_SKILL="${2:-}" GSTACK_REVIEW_FIX_RANGE="${3:-}" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e ' const { captureReviewStart } = await import(process.env.GSTACK_REVIEW_LIB); try { console.log(captureReviewStart(process.env.GSTACK_REVIEW_SKILL, process.env, process.env.GSTACK_REVIEW_FIX_RANGE === "--fix-range")); } catch (e) { console.error(e.message); process.exit(1); } @@ -85,7 +86,7 @@ if [ "$INPUT" = --start ]; then exit $? fi if [ "$INPUT" = --check-shared-libs ] && [ "$#" -eq 2 ]; then - GSTACK_REVIEW_TOKEN="$2" bun --no-env-file --config=/dev/null -e ' + GSTACK_REVIEW_TOKEN="$2" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e ' const { checkSharedLibsReuse } = await import(process.env.GSTACK_REVIEW_LIB); console.log(JSON.stringify(checkSharedLibsReuse(JSON.parse(await Bun.stdin.text()), process.env.GSTACK_REVIEW_TOKEN))); ' @@ -113,7 +114,7 @@ case "$INPUT" in --*) usage ;; esac # Caller values for the binding keys are dropped before stamping. A binding or # validation failure reaches stderr as its own line (never folded into # "invalid JSON"), and exits 1 with nothing written. -STAMPED=$(printf '%s' "$INPUT" | GSTACK_REVIEW_TOKEN="$TOKEN" GSTACK_REVIEW_FINDINGS="$FINDINGS" bun --no-env-file --config=/dev/null -e " +STAMPED=$(printf '%s' "$INPUT" | GSTACK_REVIEW_TOKEN="$TOKEN" GSTACK_REVIEW_FINDINGS="$FINDINGS" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " const { bindReview } = await import(process.env.GSTACK_REVIEW_LIB); let rec; try { rec = JSON.parse(await Bun.stdin.text()); } catch (e) { console.error('gstack-review-log: invalid JSON, skipping (' + e.message + ')'); process.exit(1); } diff --git a/bin/gstack-review-read b/bin/gstack-review-read index 8233e2076..6be765749 100755 --- a/bin/gstack-review-read +++ b/bin/gstack-review-read @@ -9,6 +9,7 @@ # rows using review_freshness; legacy or uncaptured diff evidence cannot fall # back to HEAD equality. Plan rows are unchanged. set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" case "$(uname -s)" in MINGW*|MSYS*|CYGWIN*) command -v cygpath >/dev/null 2>&1 && SCRIPT_DIR="$(cygpath -m "$SCRIPT_DIR")" ;; @@ -19,7 +20,7 @@ gstack_state_root_select GSTACK_HOME="$_gstack_sr_root" WTREE=$("$SCRIPT_DIR/gstack-wtree" 2>/dev/null || echo "unknown") if [ -f "$GSTACK_HOME/projects/$SLUG/$BRANCH-reviews.jsonl" ]; then - GSTACK_REVIEW_LIB="$SCRIPT_DIR/../lib/review-evidence.ts" GSTACK_REVIEW_WTREE="$WTREE" bun --no-env-file --config=/dev/null -e ' + GSTACK_REVIEW_LIB="$SCRIPT_DIR/../lib/review-evidence.ts" GSTACK_REVIEW_WTREE="$WTREE" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e ' const { reviewFreshness } = await import(process.env.GSTACK_REVIEW_LIB); for (const line of (await Bun.stdin.text()).split("\n").filter(Boolean)) { try { diff --git a/bin/gstack-settings-hook b/bin/gstack-settings-hook index 9068f57bd..239be557f 100755 --- a/bin/gstack-settings-hook +++ b/bin/gstack-settings-hook @@ -69,6 +69,7 @@ BASH_COMPAT=50 set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac ACTION="${1:-}" SETTINGS_FILE="${GSTACK_SETTINGS_FILE:-${CLAUDE_CONFIG_DIR:-$HOME/.claude}/settings.json}" @@ -307,7 +308,7 @@ function gsWriteIfChanged(path, beforeText, settings, existed) { GSTACK_SETTINGS_INPUT="$SETTINGS_FILE" export GSTACK_SETTINGS_INPUT -SETTINGS_FILE=$(bun --no-env-file --config=/dev/null -e "$_HOOK_JS_PRELUDE"'gsMain(function () { +SETTINGS_FILE=$(bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e "$_HOOK_JS_PRELUDE"'gsMain(function () { process.stdout.write(gsResolveSettingsPath(process.env.GSTACK_SETTINGS_INPUT)); });') || exit $? @@ -405,7 +406,7 @@ case "$ACTION" in fi _acquire_lock || exit 5 _mutation_env - GSTACK_SETTINGS_PATH="$SETTINGS_FILE" GSTACK_HOOK_CMD="$HOOK_CMD" bun --no-env-file --config=/dev/null -e "$_HOOK_JS_PRELUDE"'gsMain(function () { + GSTACK_SETTINGS_PATH="$SETTINGS_FILE" GSTACK_HOOK_CMD="$HOOK_CMD" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e "$_HOOK_JS_PRELUDE"'gsMain(function () { const settingsPath = process.env.GSTACK_SETTINGS_PATH; const hookCmd = process.env.GSTACK_HOOK_CMD; const loaded = gsLoadSettings(settingsPath); @@ -435,7 +436,7 @@ case "$ACTION" in [ -f "$SETTINGS_FILE" ] || exit 1 _acquire_lock || exit 5 _mutation_env - GSTACK_SETTINGS_PATH="$SETTINGS_FILE" bun --no-env-file --config=/dev/null -e "$_HOOK_JS_PRELUDE"'gsMain(function () { + GSTACK_SETTINGS_PATH="$SETTINGS_FILE" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e "$_HOOK_JS_PRELUDE"'gsMain(function () { const settingsPath = process.env.GSTACK_SETTINGS_PATH; const loaded = gsLoadSettings(settingsPath); const settings = loaded.settings; @@ -505,7 +506,7 @@ case "$ACTION" in GSTACK_TIMEOUT="$TIMEOUT" \ GSTACK_DIFF_ONLY="$DIFF_ONLY" \ GSTACK_ENSURE="$ENSURE" \ - bun --no-env-file --config=/dev/null -e "$_HOOK_JS_PRELUDE"'gsMain(function () { + bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e "$_HOOK_JS_PRELUDE"'gsMain(function () { const settingsPath = process.env.GSTACK_SETTINGS_PATH; const event = process.env.GSTACK_EVENT; const cmd = process.env.GSTACK_COMMAND; @@ -660,7 +661,7 @@ case "$ACTION" in [ -f "$SETTINGS_FILE" ] || exit 0 _acquire_lock || exit 5 _mutation_env - GSTACK_SETTINGS_PATH="$SETTINGS_FILE" GSTACK_SOURCE="$SOURCE" GSTACK_RS_EVENT="$RS_EVENT" bun --no-env-file --config=/dev/null -e "$_HOOK_JS_PRELUDE"'gsMain(function () { + GSTACK_SETTINGS_PATH="$SETTINGS_FILE" GSTACK_SOURCE="$SOURCE" GSTACK_RS_EVENT="$RS_EVENT" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e "$_HOOK_JS_PRELUDE"'gsMain(function () { const settingsPath = process.env.GSTACK_SETTINGS_PATH; const source = process.env.GSTACK_SOURCE; const onlyEvent = process.env.GSTACK_RS_EVENT || ""; @@ -763,7 +764,7 @@ case "$ACTION" in GSTACK_PRUNE_ALL="$PRUNE_ALL" \ GSTACK_PT_OPTOUT="$GSTACK_PT_OPTOUT" \ GSTACK_SWEEP_EXCLUDE_SOURCES="${GSTACK_SWEEP_EXCLUDE_SOURCES:-}" \ - bun --no-env-file --config=/dev/null -e "$_HOOK_JS_PRELUDE"'gsMain(function () { + bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e "$_HOOK_JS_PRELUDE"'gsMain(function () { const settingsPath = process.env.GSTACK_SETTINGS_PATH; const root = (process.env.GSTACK_REPOINT_ROOT || "").replace(/\/+$/, ""); const all = process.env.GSTACK_PRUNE_ALL === "1"; @@ -902,7 +903,7 @@ case "$ACTION" in fi _mutation_env GSTACK_SETTINGS_PATH="$SETTINGS_FILE" GSTACK_RESTORE_PATH="$LATEST" \ - bun --no-env-file --config=/dev/null -e "$_HOOK_JS_PRELUDE"'gsMain(function () { + bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e "$_HOOK_JS_PRELUDE"'gsMain(function () { var fs = require("fs"); var target = process.env.GSTACK_SETTINGS_PATH; var backup = process.env.GSTACK_RESTORE_PATH; @@ -944,7 +945,7 @@ case "$ACTION" in exit 1 fi [ -f "$SETTINGS_FILE" ] || exit 0 - GSTACK_SETTINGS_PATH="$SETTINGS_FILE" GSTACK_LI_EVENT="$LI_EVENT" GSTACK_LI_OWNED_BY="$LI_OWNED_BY" GSTACK_LI_CMD_RE="$LI_CMD_RE" bun --no-env-file --config=/dev/null -e "$_HOOK_JS_PRELUDE"'gsMain(function () { + GSTACK_SETTINGS_PATH="$SETTINGS_FILE" GSTACK_LI_EVENT="$LI_EVENT" GSTACK_LI_OWNED_BY="$LI_OWNED_BY" GSTACK_LI_CMD_RE="$LI_CMD_RE" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e "$_HOOK_JS_PRELUDE"'gsMain(function () { const event = process.env.GSTACK_LI_EVENT; const ownedBy = process.env.GSTACK_LI_OWNED_BY || ""; const reSrc = process.env.GSTACK_LI_CMD_RE || ""; @@ -982,7 +983,7 @@ case "$ACTION" in list-sources) [ -f "$SETTINGS_FILE" ] || { echo "(no settings file)"; exit 0; } - GSTACK_SETTINGS_PATH="$SETTINGS_FILE" bun --no-env-file --config=/dev/null -e "$_HOOK_JS_PRELUDE"'gsMain(function () { + GSTACK_SETTINGS_PATH="$SETTINGS_FILE" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e "$_HOOK_JS_PRELUDE"'gsMain(function () { const fs = require("fs"); let settings = {}; try { settings = JSON.parse(fs.readFileSync(process.env.GSTACK_SETTINGS_PATH, "utf8")); } diff --git a/bin/gstack-specialist-stats b/bin/gstack-specialist-stats index af0b3d15e..0c5c66532 100755 --- a/bin/gstack-specialist-stats +++ b/bin/gstack-specialist-stats @@ -8,6 +8,7 @@ # Fix-range passes (scope "fix-range", #3339) are skipped: a narrowed dispatch # says nothing about a specialist's hit rate on a whole branch. set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" eval "$("$SCRIPT_DIR/gstack-slug" 2>/dev/null)" . "$(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; } @@ -33,7 +34,7 @@ if [ -z "$COMBINED" ]; then exit 0 fi -printf '%s' "$COMBINED" | bun --no-env-file --config=/dev/null -e " +printf '%s' "$COMBINED" | bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " const lines = (await Bun.stdin.text()).trim().split('\n').filter(Boolean); const NEVER_GATE = new Set(['security', 'data-migration', 'state-machines']); const stats = {}; diff --git a/bin/gstack-team-init b/bin/gstack-team-init index 901f5999b..d284ad04c 100755 --- a/bin/gstack-team-init +++ b/bin/gstack-team-init @@ -18,6 +18,7 @@ BASH_COMPAT=50 set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac MODE="${1:-}" @@ -192,7 +193,7 @@ HOOK_EOF if ! command -v bun >/dev/null 2>&1; then echo " ! bun not found — manually add this PreToolUse hook (matcher Skill|skill, \"onFailure\": \"block\") to .claude/settings.json:" echo " $HOOK_COMMAND" - elif GSTACK_SETTINGS_PATH="$SETTINGS" GSTACK_HOOK_COMMAND="$HOOK_COMMAND" bun --no-env-file --config=/dev/null -e " + elif GSTACK_SETTINGS_PATH="$SETTINGS" GSTACK_HOOK_COMMAND="$HOOK_COMMAND" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " const fs = require('fs'); const settingsPath = process.env.GSTACK_SETTINGS_PATH; const handler = { type: 'command', command: process.env.GSTACK_HOOK_COMMAND, onFailure: 'block' }; diff --git a/bin/gstack-telemetry-log b/bin/gstack-telemetry-log index b5eac29b9..a662e350f 100755 --- a/bin/gstack-telemetry-log +++ b/bin/gstack-telemetry-log @@ -21,6 +21,7 @@ # # NOTE: Uses set -uo pipefail (no -e) — telemetry must never exit non-zero set -uo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac GSTACK_DIR="${GSTACK_DIR:-$(cd "$(dirname "$0")/.." && pwd)}" SCRIPT_DIR="$GSTACK_DIR/bin" @@ -205,7 +206,7 @@ ERR_FIELD="null" # look like a JSON string, the whole message becomes null — never raw. ERR_MSG_FIELD="null" if [ -n "$ERROR_MESSAGE" ]; then - ERR_MSG_FIELD="$(printf '%s' "$ERROR_MESSAGE" | GSTACK_LIB_DIR="$SCRIPT_DIR/../lib" bun --no-env-file --config=/dev/null -e " + ERR_MSG_FIELD="$(printf '%s' "$ERROR_MESSAGE" | GSTACK_LIB_DIR="$SCRIPT_DIR/../lib" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " const { redactFindingSpans } = await import(process.env.GSTACK_LIB_DIR + '/redact-engine.ts'); const input = await Bun.stdin.text(); const out = redactFindingSpans(input, { repoVisibility: 'private' }); diff --git a/bin/gstack-timeline-log b/bin/gstack-timeline-log index 2efd343f0..ab6eabff1 100755 --- a/bin/gstack-timeline-log +++ b/bin/gstack-timeline-log @@ -10,6 +10,7 @@ # Optional: branch, outcome, duration_s, session, ts. # Validation failure → skip silently (non-blocking). set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" eval "$("$SCRIPT_DIR/gstack-slug" 2>/dev/null)" . "$(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; } @@ -19,21 +20,33 @@ mkdir -p "$GSTACK_HOME/projects/$SLUG" INPUT="$1" -# Validate: input must be parseable JSON with required fields -if ! printf '%s' "$INPUT" | bun --no-env-file --config=/dev/null -e " - const j = JSON.parse(await Bun.stdin.text()); - if (!j.skill || !j.event) process.exit(1); -" 2>/dev/null; then - exit 0 # skip silently, non-blocking +# Validate: input must be parseable JSON with required fields. The validator +# prints its verdict; a non-zero exit means Bun itself failed (#3415). An exit +# code cannot carry the verdict: a Bun crash on Windows exits 3. +VALIDATE_RC=0 +VALIDATE_OUT=$(printf '%s' "$INPUT" | bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " + const text = await Bun.stdin.text(); + let j; + try { j = JSON.parse(text); } catch {} + console.log(j && j.skill && j.event ? 'valid' : 'invalid'); +") || VALIDATE_RC=$? +if [ "$VALIDATE_RC" -ne 0 ]; then + echo "gstack-timeline-log: bun exited $VALIDATE_RC before the event was validated; nothing was logged" >&2 + exit "$VALIDATE_RC" fi +case "$VALIDATE_OUT" in + valid) ;; + invalid) exit 0 ;; # skip silently, non-blocking + *) echo "gstack-timeline-log: bun printed no verdict while validating the event; nothing was logged" >&2; exit 1 ;; +esac # Inject timestamp if not present -if ! printf '%s' "$INPUT" | bun --no-env-file --config=/dev/null -e "const j=JSON.parse(await Bun.stdin.text()); if(!j.ts) process.exit(1)" 2>/dev/null; then - INPUT=$(printf '%s' "$INPUT" | bun --no-env-file --config=/dev/null -e " +if ! printf '%s' "$INPUT" | bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e "const j=JSON.parse(await Bun.stdin.text()); if(!j.ts) process.exit(1)" 2>/dev/null; then + STAMPED=$(printf '%s' "$INPUT" | bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " const j = JSON.parse(await Bun.stdin.text()); j.ts = new Date().toISOString(); console.log(JSON.stringify(j)); - " 2>/dev/null) || true + " 2>/dev/null) && [ -n "$STAMPED" ] && INPUT=$STAMPED || true fi echo "$INPUT" >> "$GSTACK_HOME/projects/$SLUG/timeline.jsonl" diff --git a/bin/gstack-timeline-read b/bin/gstack-timeline-read index c4dd93cfe..35db5fb46 100755 --- a/bin/gstack-timeline-read +++ b/bin/gstack-timeline-read @@ -10,6 +10,7 @@ # --branch, or an unknown argument exits 2 before any read (#1723); --help # prints the usage line and exits 0. set -euo pipefail +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac USAGE='Usage: gstack-timeline-read [--since "7 days ago"|7d|YYYY-MM-DD] [--limit N] [--branch NAME]' SINCE_FORMS='accepted: " minute|hour|day|week|month[s] ago", "h|d|w", or an ISO date YYYY-MM-DD or YYYY-MM-DDTHH:MM:SSZ' SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" @@ -108,7 +109,7 @@ command -v bun >/dev/null 2>&1 || { exit 127 } -cat "$TIMELINE_FILE" | GSTACK_TIMELINE_SINCE_MS="$SINCE_MS" GSTACK_TIMELINE_BRANCH="$BRANCH" GSTACK_TIMELINE_LIMIT="$LIMIT" bun --no-env-file --config=/dev/null -e " +cat "$TIMELINE_FILE" | GSTACK_TIMELINE_SINCE_MS="$SINCE_MS" GSTACK_TIMELINE_BRANCH="$BRANCH" GSTACK_TIMELINE_LIMIT="$LIMIT" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " const lines = (await Bun.stdin.text()).trim().split('\n').filter(Boolean); const branch = process.env.GSTACK_TIMELINE_BRANCH || ''; const limit = Number(process.env.GSTACK_TIMELINE_LIMIT); diff --git a/browse/test/learnings-injection.test.ts b/browse/test/learnings-injection.test.ts index de3b09538..2d202dca1 100644 --- a/browse/test/learnings-injection.test.ts +++ b/browse/test/learnings-injection.test.ts @@ -6,17 +6,22 @@ import { spawnSync } from 'child_process'; const SCRIPT_PATH = path.join(import.meta.dir, '../../bin/gstack-learnings-search'); const SCRIPT = fs.readFileSync(SCRIPT_PATH, 'utf-8'); const BIN_DIR = path.join(import.meta.dir, '../../bin'); +const BUN_BLOCK_AT = SCRIPT.indexOf('bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e'); describe('gstack-learnings-search injection safety', () => { + it('finds the bun -e block the checks below read', () => { + expect(BUN_BLOCK_AT).toBeGreaterThanOrEqual(0); + }); + it('must not interpolate variables into JS string literals', () => { - const jsBlock = SCRIPT.slice(SCRIPT.indexOf('bun --no-env-file --config=/dev/null -e')); + const jsBlock = SCRIPT.slice(BUN_BLOCK_AT); expect(jsBlock).not.toMatch(/const \w+ = '\$\{/); expect(jsBlock).not.toMatch(/= \$\{[A-Z_]+\};/); expect(jsBlock).not.toMatch(/'\$\{CROSS_PROJECT\}'/); }); it('must use process.env for parameters', () => { - const jsBlock = SCRIPT.slice(SCRIPT.indexOf('bun --no-env-file --config=/dev/null -e')); + const jsBlock = SCRIPT.slice(BUN_BLOCK_AT); expect(jsBlock).toContain('process.env'); }); }); diff --git a/docs/capy-install.md b/docs/capy-install.md index c2d0ce97b..dd272cb18 100644 --- a/docs/capy-install.md +++ b/docs/capy-install.md @@ -120,15 +120,30 @@ lease …`). Stale needs proof: two well-formed identities from the same backend that differ (`ps` is read under `LC_ALL=C TZ=UTC0`). An identity that cannot be read, a malformed record or a backend change keeps the lease. `touch` (publish by rename, prune) and reclaim (reread, then delete only the record judged -stale) run under one lock in the lease directory, a symlink naming its holder's -pid and start identity, so a renewal is never deleted and a live lease never -disappears from a concurrent scan. The lock is broken only when its holder is -dead or its pid was reused, never by age, and breakers serialize on a second -symlink and reread the lock before removing it. One killed gstack-lease process -is always safe; two killed in a row, one holding the lock and the next while -breaking it, can still let a reclaim delete a renewal. While a live process holds it, -`touch` exits 3 without writing (`gstack-skill-start` reports that the -session holds no lease) and a reclaim keeps the lease. `bin/gstack-lease list` shows `alive|dead|stale`, and +stale) run under one lock in the lease directory, a `.lock` directory whose +`owner` file names its holder's pid and start identity, so a renewal is never +deleted and a live lease never disappears from a concurrent scan. The lock is a +directory because `mkdir` is atomic on every host, while Git Bash on Windows +cannot `ln -s` to a target that does not exist (#3330). A taker publishes a +claim before its `mkdir` and renames it in as the owner after, so an ownerless +lock is broken only once no claimant lives and the lock is at least 2 s old +and unchanged while checked, which keeps a taker between its two steps safe. +A claim that records no start identity is abandoned after 2 minutes, so a +reused pid cannot keep an orphan forever; an owner that names no valid pid +counts as a dead holder, and an owner file that cannot be read keeps the +lock. A live holder that never lets go bounds the wait at 300 tries or 30 s. A lock that names a holder is broken only when that holder is dead or +its pid was reused, never by age, and breakers serialize on a second lock and +reread the lock before removing it. A symlink `.lock` left by an older gstack +is honored the same way. Three known limits remain. An older gstack's `ln -s` +onto a directory lock links inside it and succeeds, so a process that started +before an upgrade of this checkout can share the lock with a newer one. And a +breaker killed while holding the second lock, followed by two live breakers +clearing it at once, can let a reclaim delete a renewal. On a host without start +identities, a taker stalled over 2 minutes between its `mkdir` and its owner +rename can be broken and then publish into its successor's lock. While a live process +holds the lock, or it cannot be taken at all, `touch` exits 3 without writing +(`gstack-skill-start` reports that the session holds no lease) and a reclaim +keeps the lease. `bin/gstack-lease list` shows `alive|dead|stale`, and `GSTACK_UPGRADE_FORCE=1` overrides. ## What the installer does diff --git a/gstack-upgrade/migrations/v1.86.0.0.sh b/gstack-upgrade/migrations/v1.86.0.0.sh index fcdbf83cc..ceb589c23 100755 --- a/gstack-upgrade/migrations/v1.86.0.0.sh +++ b/gstack-upgrade/migrations/v1.86.0.0.sh @@ -5,8 +5,9 @@ # shared old renders are retired. This post-setup pass repairs missed upgrades. # Idempotent and non-fatal: foreign entries and failed replacements stay intact. set -u +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac INSTALL_DIR="${GSTACK_INSTALL_DIR:-$HOME/.claude/skills/gstack}" if [ -f "$INSTALL_DIR/bin/gstack-migrate-claude-code" ] && command -v bun >/dev/null 2>&1; then - bun --no-env-file --config=/dev/null "$INSTALL_DIR/bin/gstack-migrate-claude-code" --install-dir "$INSTALL_DIR" || true + bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" "$INSTALL_DIR/bin/gstack-migrate-claude-code" --install-dir "$INSTALL_DIR" || true fi exit 0 diff --git a/hosts/claude/hooks/auq-error-fallback-hook b/hosts/claude/hooks/auq-error-fallback-hook index 4db80b738..7c351fd9b 100755 --- a/hosts/claude/hooks/auq-error-fallback-hook +++ b/hosts/claude/hooks/auq-error-fallback-hook @@ -3,5 +3,6 @@ # wrapper makes the TypeScript hook executable via bun. Settings.json # references this file directly. set -e +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac HERE="$(cd "$(dirname "$0")" && pwd)" -exec bun --no-env-file --config=/dev/null "$HERE/auq-error-fallback-hook.ts" +exec bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" "$HERE/auq-error-fallback-hook.ts" diff --git a/hosts/claude/hooks/memorable-user-prompt-hook b/hosts/claude/hooks/memorable-user-prompt-hook index f476ac7f4..9a7d07069 100755 --- a/hosts/claude/hooks/memorable-user-prompt-hook +++ b/hosts/claude/hooks/memorable-user-prompt-hook @@ -12,8 +12,9 @@ # vendor process running with the prompt on its stdin. Forwarded, the signal # reaches the .ts, which kills the vendor's process group on its way out. # `<&0` keeps stdin: bash hands background jobs /dev/null otherwise. +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac HERE="$(cd "$(dirname "$0")" && pwd)" || exit 0 -bun --no-env-file --config=/dev/null "$HERE/memorable-user-prompt-hook.ts" <&0 & +bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" "$HERE/memorable-user-prompt-hook.ts" <&0 & _child=$! trap 'kill -TERM "$_child" 2>/dev/null' TERM INT HUP wait "$_child" 2>/dev/null || wait "$_child" 2>/dev/null || true diff --git a/hosts/claude/hooks/question-log-hook b/hosts/claude/hooks/question-log-hook index 5b6012cec..c70484a90 100755 --- a/hosts/claude/hooks/question-log-hook +++ b/hosts/claude/hooks/question-log-hook @@ -3,5 +3,6 @@ # wrapper makes the TypeScript hook executable via bun. Settings.json # references this file directly. set -e +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac HERE="$(cd "$(dirname "$0")" && pwd)" -exec bun --no-env-file --config=/dev/null "$HERE/question-log-hook.ts" +exec bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" "$HERE/question-log-hook.ts" diff --git a/hosts/claude/hooks/question-preference-hook b/hosts/claude/hooks/question-preference-hook index 8dac5ff3c..a910c3cf4 100755 --- a/hosts/claude/hooks/question-preference-hook +++ b/hosts/claude/hooks/question-preference-hook @@ -3,5 +3,6 @@ # wrapper makes the TypeScript hook executable via bun. Settings.json # references this file directly. set -e +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac HERE="$(cd "$(dirname "$0")" && pwd)" -exec bun --no-env-file --config=/dev/null "$HERE/question-preference-hook.ts" +exec bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" "$HERE/question-preference-hook.ts" diff --git a/hosts/claude/hooks/timeline-stop-hook b/hosts/claude/hooks/timeline-stop-hook index 74c716649..9cddf1fe5 100755 --- a/hosts/claude/hooks/timeline-stop-hook +++ b/hosts/claude/hooks/timeline-stop-hook @@ -5,6 +5,7 @@ # # FAIL-OPEN (F5): a Stop-event telemetry repair must never block the session. # Every failure path — bun missing, script crash — still exits 0. +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac HERE="$(cd "$(dirname "$0")" && pwd)" || exit 0 -bun --no-env-file --config=/dev/null "$HERE/timeline-stop-hook.ts" || true +bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" "$HERE/timeline-stop-hook.ts" || true exit 0 diff --git a/lib/bun-isolation.ts b/lib/bun-isolation.ts index 63c9d09ef..426b6dd3f 100644 --- a/lib/bun-isolation.ts +++ b/lib/bun-isolation.ts @@ -4,10 +4,12 @@ * variables, and a `bunfig.toml` preload runs code before the entry point. * gstack runs inside the user's repository, so that directory belongs to the * code under review. Every Bun gstack starts passes --no-env-file and an empty - * --config: shebangs and shell callers spell it - * `bun --no-env-file --config=/dev/null`, TypeScript spawns use - * bunIsolationArgs(). test/bun-entry-isolation.test.ts holds every entry point - * to that form. + * --config: shebangs and skill prose spell it + * `bun --no-env-file --config=/dev/null`, shipped shell files spell + * `--config="${GSTACK_BUN_CONFIG:-/dev/null}"` (NUL when OSTYPE is msys or + * cygwin and cygpath is on PATH, #3415), TypeScript spawns use + * bunIsolationArgs(). + * test/bun-entry-isolation.test.ts holds every entry point to that form. * * POSIX passes /dev/null. Native Windows has no /dev/null and `--config=NUL` * panicked Bun 1.3.10 while parsing the flag (#3085), so a TypeScript spawn on diff --git a/package.json b/package.json index 88c1a1100..7c5eb415d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.91.225", + "version": "1.91.230", "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/plan-ceo-review/bin/mode-handoff-hook b/plan-ceo-review/bin/mode-handoff-hook index b0461686f..58ad95c08 100755 --- a/plan-ceo-review/bin/mode-handoff-hook +++ b/plan-ceo-review/bin/mode-handoff-hook @@ -1,5 +1,6 @@ #!/usr/bin/env bash # Bash shim — Claude Code runs hook commands through /bin/sh; this runs the # TypeScript hook with bun. +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac HERE="$(cd "$(dirname "$0")" && pwd)" -exec bun --no-env-file --config=/dev/null "$HERE/mode-handoff-hook.ts" +exec bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" "$HERE/mode-handoff-hook.ts" diff --git a/review/SKILL.md b/review/SKILL.md index f319f1669..edb01a2c8 100644 --- a/review/SKILL.md +++ b/review/SKILL.md @@ -505,8 +505,11 @@ git diff "$DIFF_BASE" ``` 1. Save the printed REVIEW_START for this core candidate before reading its diff. -2. Each re-review captures a new token before reading, never at log time. Earlier - core tokens remain unused; Step 5.8 finishes only the final core token. +2. Each re-review captures a new token before reading, never at log time. After an + edited pass, start the next with `--start review --fix-range`: it prints + REVIEW_START, then `FIX_BASE=`. Wherever this pass sets DIFF_BASE outside + Step 4.8, use FIX_BASE instead; Step 4.8 and every other `` use stay full. + On refusal, start a full pass. 3. Native/outside reviewer attempts own separate PASS_START tokens, not REVIEW_START. 4. Read non-ignored untracked source too (`git ls-files --others --exclude-standard`); the captured candidate includes it. @@ -540,13 +543,13 @@ OFFLINE=$(echo "$QUEUE_JSON" | jq -r '.offline // false') - `OFFLINE=true`: skip this section. - Otherwise, include ONE line in the review output: `Version claimed: v. Queue: PR(s) ahead. ` where VERDICT is either `Slot free` (if `BRANCH_VERSION >= NEXT_SLOT`) or `⚠ queue moved — rerun /ship to reconcile v → v`. -Compare dotted version components as integers from left to right; missing trailing components count as zero. +Compare versions numerically per dotted component; missing ones count as zero. --- ## Step 3.5: Diff scans -Run the gate scan first (local, seconds; every diff size), then the slop scan. +Run the gate scan first (local; every diff size), then the slop scan. ### Gate scan @@ -589,14 +592,11 @@ category, Step 5.8's record and the final report. ### Slop scan (advisory) -Scan changed files for empty catches, redundant `return await` and needless abstractions: - ```bash bun run slop:diff origin/ 2>/dev/null || true ``` -Include findings as non-blocking informational diagnostics. If slop:diff is -unavailable, skip silently. +Findings are non-blocking informational diagnostics; skip if unavailable. --- @@ -719,10 +719,8 @@ a clean result. Gate findings are ASK in Step 5 regardless of how mechanical the revert looks. A test-only flake fix (timeout, retry, looser assertion, skip) states `P(fail | regression)` before and after, from a forced probe; without both numbers it is RH-15. **Search-before-recommending:** Research proposed fixes through Aside, especially -concurrency, caching, auth and framework behavior: -- Check current best practice for the installed framework version. -- Look for a newer built-in before proposing a workaround. -- Verify API signatures against current docs. +concurrency, caching, auth and framework behavior: current best practice for the +installed version, newer built-ins that replace a workaround, and current API signatures. Prompt file text (create, write and send it with the Web research runs in Aside blocks above): `{framework} {version} {pattern} current best practice and whether a built-in replaces it. Reply with up to 5 bullets, each with its source URL.` @@ -1052,7 +1050,7 @@ Verify claims or flag them as unknown; "this looks fine" is not evidence. After outputting your own findings, if Greptile comments were classified in Step 2.5: -**Include a Greptile summary in your output header:** `+ N Greptile comments (X valid, Y fixed, Z FP)` +**Greptile summary in your output header:** `+ N Greptile comments (X valid, Y fixed, Z FP)` Before replying to any comment, run greptile-triage.md's **Escalation Detection** to choose Tier 1 (friendly) or Tier 2 (firm) reply templates. @@ -1067,7 +1065,7 @@ Before replying to any comment, run greptile-triage.md's **Escalation Detection* 3. **VALID BUT ALREADY FIXED comments:** Reply with greptile-triage.md's **Already Fixed reply template**, no AskUserQuestion: what was done and the fixing commit SHA; save to both per-project and global greptile-history. -4. **SUPPRESSED comments:** Skip silently (known false positives from previous triage). +4. **SUPPRESSED comments:** Skip silently. --- @@ -1076,16 +1074,17 @@ Before replying to any comment, run greptile-triage.md's **Escalation Detection* ### 1. Re-review after edits 1. A pass covers Steps 3–5, including all reviewers before fixes. Allow at most 3 fix cycles: - - Edited: increment CYCLES once. Below 3, repeat Steps 3–5 with a new - REVIEW_START. At 3, persist `converged:false` and remaining findings by filling - and saving the record below. Report nonconvergence and coverage gaps, then STOP - this invocation, without a clean summary or a fourth pass. - - No edits: fill the record below. + - Edited: increment CYCLES once, then fill and save the record below with + `converged:false`. Step 5.0 never treats this invocation's records as prior + reviews. Below 3, repeat Steps 3–5 with a new REVIEW_START. At 3, report + nonconvergence and coverage gaps, then STOP this invocation, without a clean + summary or a fourth pass. + - No edits: after a full pass, fill the record below. A fix-range pass is never + final: repeat Steps 3–5 with a full start. 2. On a repeat, execute Steps 3–5 in order. At Step 4.7, reuse only this invocation's - unchanged-input QA evidence; rerun affected probes after source, test, contract, - command or fixture changes. Reusing a probe never skips a review step. - A probe is affected when its entrypoint, dependencies, contract or replay inputs - change; if uncertain, rerun it. + unchanged-input QA evidence; rerun a probe if its source, tests, entrypoint, + dependencies, contract, command, fixture or replay inputs changed, or if uncertain. + Reusing a probe never skips a review step. 3. **Verify completed actions.** On the final zero-edit pass, reconcile this invocation's actions with current findings. Deduplicate by structural identity and advisory/defect kind. For a completed extraction, retain `fixed` and the @@ -1121,7 +1120,7 @@ pass: report it with its reason. - Use Step 4.6's `specialists` object unchanged, including its empty small-diff map. If this host omits Review Army, use `specialists: {}` without claiming specialist coverage. -- Build `findings` from Step 5's combined final-pass findings (core, specialist, +- Build `findings` from Step 5's combined findings (core, specialist, adversarial, actionable Greptile, verified exploratory QA findings) and invocation actions. Retain `fingerprint`, `severity` (`CRITICAL|INFORMATIONAL`), `action`, and any `advisory`, `evidence_paths`, `helper_target`. Recheck source after fixes. The logger uses `sharedLibsFingerprint`, @@ -1149,11 +1148,11 @@ were skipped or this host omits Review Army). This default is not completion evi unresolved non-advisory core defects still count in `issues_found`, `critical`, `informational`. The logger builds trusted `review_binding` from the validated captured branch digest, discarding caller bindings. Never invent a binding -or replace REVIEW_START at log time; finish only the final core token. +or replace REVIEW_START at log time; finish only this pass's own token. ### Report the final review -Emit one final report, merging all reviewers rather than concatenating them: +Emit one final report merging all reviewers: 1. `Pre-Landing Review: N issues (X critical, Y informational)` counts final unresolved non-advisory defects. State INCOMPLETE if `COMPLETED` is false, even when N=0. 2. Use the checklist's action groups with confidence-tagged finding lines. Keep fixed, diff --git a/review/SKILL.md.tmpl b/review/SKILL.md.tmpl index 1eb1711e7..667c2aa9f 100644 --- a/review/SKILL.md.tmpl +++ b/review/SKILL.md.tmpl @@ -87,8 +87,11 @@ git diff "$DIFF_BASE" ``` 1. Save the printed REVIEW_START for this core candidate before reading its diff. -2. Each re-review captures a new token before reading, never at log time. Earlier - core tokens remain unused; Step 5.8 finishes only the final core token. +2. Each re-review captures a new token before reading, never at log time. After an + edited pass, start the next with `--start review --fix-range`: it prints + REVIEW_START, then `FIX_BASE=`. Wherever this pass sets DIFF_BASE outside + Step 4.8, use FIX_BASE instead; Step 4.8 and every other `` use stay full. + On refusal, start a full pass. 3. Native/outside reviewer attempts own separate PASS_START tokens, not REVIEW_START. 4. Read non-ignored untracked source too (`git ls-files --others --exclude-standard`); the captured candidate includes it. @@ -122,13 +125,13 @@ OFFLINE=$(echo "$QUEUE_JSON" | jq -r '.offline // false') - `OFFLINE=true`: skip this section. - Otherwise, include ONE line in the review output: `Version claimed: v. Queue: PR(s) ahead. ` where VERDICT is either `Slot free` (if `BRANCH_VERSION >= NEXT_SLOT`) or `⚠ queue moved — rerun /ship to reconcile v → v`. -Compare dotted version components as integers from left to right; missing trailing components count as zero. +Compare versions numerically per dotted component; missing ones count as zero. --- ## Step 3.5: Diff scans -Run the gate scan first (local, seconds; every diff size), then the slop scan. +Run the gate scan first (local; every diff size), then the slop scan. ### Gate scan @@ -143,14 +146,11 @@ category, Step 5.8's record and the final report. ### Slop scan (advisory) -Scan changed files for empty catches, redundant `return await` and needless abstractions: - ```bash bun run slop:diff origin/ 2>/dev/null || true ``` -Include findings as non-blocking informational diagnostics. If slop:diff is -unavailable, skip silently. +Findings are non-blocking informational diagnostics; skip if unavailable. --- @@ -180,10 +180,8 @@ a clean result. Gate findings are ASK in Step 5 regardless of how mechanical the revert looks. {{FLAKE_FIX_NUMBER:review}} **Search-before-recommending:** Research proposed fixes through Aside, especially -concurrency, caching, auth and framework behavior: -- Check current best practice for the installed framework version. -- Look for a newer built-in before proposing a workaround. -- Verify API signatures against current docs. +concurrency, caching, auth and framework behavior: current best practice for the +installed version, newer built-ins that replace a workaround, and current API signatures. Prompt file text (create, write and send it with the Web research runs in Aside blocks above): `{framework} {version} {pattern} current best practice and whether a built-in replaces it. Reply with up to 5 bullets, each with its source URL.` @@ -352,7 +350,7 @@ Verify claims or flag them as unknown; "this looks fine" is not evidence. After outputting your own findings, if Greptile comments were classified in Step 2.5: -**Include a Greptile summary in your output header:** `+ N Greptile comments (X valid, Y fixed, Z FP)` +**Greptile summary in your output header:** `+ N Greptile comments (X valid, Y fixed, Z FP)` Before replying to any comment, run greptile-triage.md's **Escalation Detection** to choose Tier 1 (friendly) or Tier 2 (firm) reply templates. @@ -367,7 +365,7 @@ Before replying to any comment, run greptile-triage.md's **Escalation Detection* 3. **VALID BUT ALREADY FIXED comments:** Reply with greptile-triage.md's **Already Fixed reply template**, no AskUserQuestion: what was done and the fixing commit SHA; save to both per-project and global greptile-history. -4. **SUPPRESSED comments:** Skip silently (known false positives from previous triage). +4. **SUPPRESSED comments:** Skip silently. --- @@ -376,16 +374,17 @@ Before replying to any comment, run greptile-triage.md's **Escalation Detection* ### 1. Re-review after edits 1. A pass covers Steps 3–5, including all reviewers before fixes. Allow at most 3 fix cycles: - - Edited: increment CYCLES once. Below 3, repeat Steps 3–5 with a new - REVIEW_START. At 3, persist `converged:false` and remaining findings by filling - and saving the record below. Report nonconvergence and coverage gaps, then STOP - this invocation, without a clean summary or a fourth pass. - - No edits: fill the record below. + - Edited: increment CYCLES once, then fill and save the record below with + `converged:false`. Step 5.0 never treats this invocation's records as prior + reviews. Below 3, repeat Steps 3–5 with a new REVIEW_START. At 3, report + nonconvergence and coverage gaps, then STOP this invocation, without a clean + summary or a fourth pass. + - No edits: after a full pass, fill the record below. A fix-range pass is never + final: repeat Steps 3–5 with a full start. 2. On a repeat, execute Steps 3–5 in order. At Step 4.7, reuse only this invocation's - unchanged-input QA evidence; rerun affected probes after source, test, contract, - command or fixture changes. Reusing a probe never skips a review step. - A probe is affected when its entrypoint, dependencies, contract or replay inputs - change; if uncertain, rerun it. + unchanged-input QA evidence; rerun a probe if its source, tests, entrypoint, + dependencies, contract, command, fixture or replay inputs changed, or if uncertain. + Reusing a probe never skips a review step. 3. **Verify completed actions.** On the final zero-edit pass, reconcile this invocation's actions with current findings. Deduplicate by structural identity and advisory/defect kind. For a completed extraction, retain `fixed` and the @@ -421,7 +420,7 @@ pass: report it with its reason. - Use Step 4.6's `specialists` object unchanged, including its empty small-diff map. If this host omits Review Army, use `specialists: {}` without claiming specialist coverage. -- Build `findings` from Step 5's combined final-pass findings (core, specialist, +- Build `findings` from Step 5's combined findings (core, specialist, adversarial, actionable Greptile, verified exploratory QA findings) and invocation actions. Retain `fingerprint`, `severity` (`CRITICAL|INFORMATIONAL`), `action`, and any `advisory`, `evidence_paths`, `helper_target`. Recheck source after fixes. The logger uses `sharedLibsFingerprint`, @@ -449,11 +448,11 @@ were skipped or this host omits Review Army). This default is not completion evi unresolved non-advisory core defects still count in `issues_found`, `critical`, `informational`. The logger builds trusted `review_binding` from the validated captured branch digest, discarding caller bindings. Never invent a binding -or replace REVIEW_START at log time; finish only the final core token. +or replace REVIEW_START at log time; finish only this pass's own token. ### Report the final review -Emit one final report, merging all reviewers rather than concatenating them: +Emit one final report merging all reviewers: 1. `Pre-Landing Review: N issues (X critical, Y informational)` counts final unresolved non-advisory defects. State INCOMPLETE if `COMPLETED` is false, even when N=0. 2. Use the checklist's action groups with confidence-tagged finding lines. Keep fixed, diff --git a/scripts/build-cso.sh b/scripts/build-cso.sh index 62be650c4..303d03e6a 100644 --- a/scripts/build-cso.sh +++ b/scripts/build-cso.sh @@ -1,5 +1,6 @@ #!/usr/bin/env bash set -eu +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac CSO_BUILD_ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" cd "$CSO_BUILD_ROOT" @@ -227,7 +228,7 @@ cso_build_evaluation_candidate() { cp -R lib scripts "$source_root/" cp -R test/helpers "$source_root/test/" cp "$candidate_catalog" "$source_root/lib/cso/runtime-catalog.json" - (cd "$source_root" && "$BUN_CMD" --no-env-file --config=/dev/null scripts/cso-runtime-promotion.ts check-evaluation lib/cso/runtime-catalog.json) + (cd "$source_root" && "$BUN_CMD" --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" scripts/cso-runtime-promotion.ts check-evaluation lib/cso/runtime-catalog.json) (cd "$source_root" && cso_compile_bun lib/cso/cli.ts "$CSO_EVALUATION_UNIT/gstack-cso-core") (cd "$source_root" && cso_compile_bun scripts/cso-eval-producer.ts "$CSO_EVALUATION_UNIT/cso-eval-producer") cso_compile_watchdog "$CSO_EVALUATION_UNIT/gstack-cso-watchdog" diff --git a/scripts/lib/windows-curation.ts b/scripts/lib/windows-curation.ts index 8d153facf..36dae63ac 100644 --- a/scripts/lib/windows-curation.ts +++ b/scripts/lib/windows-curation.ts @@ -314,6 +314,7 @@ export const KNOWN_WINDOWS_SAFE: Array<{ file: string; reason: string }> = [ { file: 'test/ship-hook-windows-paths.test.ts', reason: 'runs bin/ helpers through explicit bash and Bun argv with forward-slash paths; never executes a shebang; the path-spelling simulation is skipIf win32' }, { file: 'test/gstack-wtree-windows-paths.test.ts', reason: '#3101: runs bin/gstack-wtree through explicit bash in an os.tmpdir linked worktree with forward-slash paths; never executes a shebang; the drive-letter simulation is skipIf win32' }, { file: 'test/setup-hook-syntax-gate.test.ts', reason: '#3088/#3100: runs bin/gstack-hook-check and the setup hook sections through explicit bash with forward-slash paths; never executes a shebang. Its Bun drive-path spelling is the Windows bug it guards' }, + { file: 'test/bun-config-shell-bins.test.ts', reason: '#3415: runs the record bins through lib/script-invocation.ts (Git Bash on Windows) with MSYS path conversion off, the case it guards; its bun and cygpath stand-ins and simulated-OSTYPE cases are skipIf win32' }, { file: 'test/path-spelling.test.ts', reason: 'sources bin/gstack-path-spelling.sh through explicit bash and checks lib/path-spelling.ts; on Windows it exercises the real Git Bash spellings' }, { file: 'test/state-root-parity.test.ts', reason: 'runs the bash twin and lib/state-root.ts over an env table with PATH empty; no shebang execution, raw-string comparison is platform-neutral' }, { file: 'test/generator-eexist.test.ts', diff --git a/setup b/setup index 10c0ae56c..50225c258 100755 --- a/setup +++ b/setup @@ -1,6 +1,7 @@ #!/usr/bin/env bash # gstack setup — build browser binary + register skills with Claude Code / Codex set -e +GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac umask 077 # Restrict new files to owner-only (0o600 files, 0o700 dirs) # Heredoc delivery guard. bash 5.2+ writes a heredoc body <=64KiB through a @@ -362,7 +363,7 @@ _runtime_root_refused() { # --claude-model is validated before anything is written, and persisted once # Claude is known to be selected (below, and in the --refresh-registered parent). if [ "$CLAUDE_MODEL_SET" -eq 1 ]; then - if ! _CLAUDE_MODEL_RESOLVED="$(cd "$SOURCE_GSTACK_DIR" && bun --no-env-file --config=/dev/null run scripts/models.ts claude-overlay "$CLAUDE_MODEL_ARG" 2>&1)"; then + if ! _CLAUDE_MODEL_RESOLVED="$(cd "$SOURCE_GSTACK_DIR" && bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" run scripts/models.ts claude-overlay "$CLAUDE_MODEL_ARG" 2>&1)"; then printf 'Error: --claude-model %s\n' "$_CLAUDE_MODEL_RESOLVED" | sed '2,$s/^/ /' >&2 echo " Nothing was installed or changed." >&2 exit 1 @@ -1317,7 +1318,7 @@ if [ "$LOCAL_INSTALL" -eq 0 ]; then _CODEX_PREFLIGHT_SELECTED=1 fi fi -bun --no-env-file --config=/dev/null "$SOURCE_GSTACK_DIR/scripts/preflight-codex-overlap.ts" \ +bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" "$SOURCE_GSTACK_DIR/scripts/preflight-codex-overlap.ts" \ --source "$SOURCE_GSTACK_DIR" --namespace "$CODEX_SKILLS" \ --selected "$_CODEX_PREFLIGHT_SELECTED" --local "$CODEX_REPO_LOCAL" \ --windows "$IS_WINDOWS" --relocation "$HOME/.gstack/repos/gstack" || exit 1 @@ -1584,7 +1585,7 @@ ensure_playwright_browser() { echo "gstack setup failed: Node.js is required on Windows" >&2 return 1 else - probe_cmd="bun --no-env-file --config=/dev/null --eval 'import { chromium } from \"playwright\"; const browser = await chromium.launch(); await browser.close();'" + probe_cmd="bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" --eval 'import { chromium } from \"playwright\"; const browser = await chromium.launch(); await browser.close();'" fi ( cd "$SOURCE_GSTACK_DIR" diff --git a/test/bun-config-shell-bins.test.ts b/test/bun-config-shell-bins.test.ts new file mode 100644 index 000000000..9eefc12eb --- /dev/null +++ b/test/bun-config-shell-bins.test.ts @@ -0,0 +1,204 @@ +/** + * #3415: gstack's shell bins start Bun with + * --config="${GSTACK_BUN_CONFIG:-/dev/null}", and each defines + * GSTACK_BUN_CONFIG as NUL when OSTYPE is msys or cygwin and cygpath is on PATH + * (Git Bash, MSYS2, Cygwin). Native bun.exe cannot + * open /dev/null (EINVAL), so a Windows bash with MSYS path conversion off + * (MSYS_NO_PATHCONV=1, MSYS2_ARG_CONV_EXCL=*) made the literal spelling fail: + * the record bins wrote nothing, and gstack-timeline-log still exited 0. + * + * On every platform the free lanes run, windows-latest included, the record + * bins run through lib/script-invocation.ts with conversion off in a hostile + * repository: each writes its record, and the repository's bunfig.toml preload + * and .env never load. On POSIX a simulated Windows bash (OSTYPE=msys, a + * cygpath, a bun that rejects /dev/null the way bun.exe does) reproduces the + * bug; its controls, without OSTYPE=msys, show each bin fails loudly with + * nothing written instead of exiting 0. An OSTYPE=msys inherited by a POSIX + * bash with no cygpath keeps /dev/null, so a file named NUL in the repository + * is never read as Bun's config. + */ +import { afterAll, beforeAll, 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 { runScriptSync } from '../lib/script-invocation'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const IS_WINDOWS = process.platform === 'win32'; + +let tmp = ''; +let repo = ''; +let stateRoot = ''; +let evilRoot = ''; +let preloadMarker = ''; +let fakeBun = ''; +let crashBun = ''; + +const BINS: Array<{ bin: string; input: string; log: string }> = [ + { bin: 'gstack-review-log', log: 'main-reviews.jsonl', input: JSON.stringify({ skill: 'plan-eng-review', timestamp: '2026-10-25T00:00:00Z', status: 'clean', unresolved: 0, critical_gaps: 0, issues_found: 0, mode: 'FULL_REVIEW', commit: 'abc1234' }) }, + { bin: 'gstack-question-log', log: 'question-log.jsonl', input: JSON.stringify({ skill: 'ship', question_id: 'ship-test-failure-triage', question_summary: 'Tests failed', options_count: 3, user_choice: 'fix-now', recommended: 'fix-now', session_id: 's1' }) }, + { bin: 'gstack-learnings-log', log: 'learnings.jsonl', input: JSON.stringify({ skill: 'review', type: 'pitfall', key: 'n-plus-one', insight: 'fixture insight', confidence: 8, source: 'observed' }) }, + { bin: 'gstack-timeline-log', log: 'timeline.jsonl', input: JSON.stringify({ skill: 'review', event: 'started', branch: 'main' }) }, +]; + +function env(pathPrefix?: string, extra: Record = {}): NodeJS.ProcessEnv { + const out: NodeJS.ProcessEnv = {}; + for (const [k, v] of Object.entries(process.env)) if (!/^(GSTACK_|MSYS|OSTYPE$)/.test(k)) out[k] = v; + const PATH = Object.keys(out).find(k => k.toUpperCase() === 'PATH') ?? 'PATH'; + if (pathPrefix) out[PATH] = pathPrefix + path.delimiter + (out[PATH] ?? ''); + return { ...out, HOME: path.join(tmp, 'home'), GSTACK_STATE_ROOT: stateRoot, MSYS_NO_PATHCONV: '1', MSYS2_ARG_CONV_EXCL: '*', ...extra }; +} + +function run(bin: string, input: string, pathPrefix?: string, extra: Record = {}, rest: string[] = []) { + return runScriptSync(path.join(ROOT, 'bin', bin), [input, ...rest], { cwd: repo, env: env(pathPrefix, extra), encoding: 'utf8', timeout: 60_000 }); +} + +function records(log: string): string[] { + const projects = path.join(stateRoot, 'projects'); + if (!fs.existsSync(projects)) return []; + return fs.readdirSync(projects).flatMap(slug => { + const file = path.join(projects, slug, log); + return fs.existsSync(file) ? fs.readFileSync(file, 'utf8').split('\n').filter(Boolean) : []; + }); +} + +function resetState(): void { + fs.rmSync(stateRoot, { recursive: true, force: true }); + fs.rmSync(preloadMarker, { force: true }); + fs.mkdirSync(stateRoot, { recursive: true }); +} + +function writeExec(file: string, text: string): void { + fs.writeFileSync(file, text); + fs.chmodSync(file, 0o755); +} + +beforeAll(() => { + tmp = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-bunconfig-'))); + repo = path.join(tmp, 'repo'); + stateRoot = path.join(tmp, 'state'); + evilRoot = path.join(tmp, 'evil'); + preloadMarker = path.join(tmp, 'preload-ran'); + for (const d of [repo, stateRoot, path.join(tmp, 'home')]) fs.mkdirSync(d, { recursive: true }); + for (const args of [['init', '-q', '-b', 'main'], ['-c', 'user.email=t@example.com', '-c', 'user.name=t', 'commit', '-q', '--allow-empty', '-m', 'init']]) { + expect(spawnSync('git', args, { cwd: repo, timeout: 30_000 }).status).toBe(0); + } + fs.writeFileSync(path.join(repo, '.env'), `GSTACK_STATE_ROOT=${evilRoot}\nGSTACK_HOME=${evilRoot}\n`); + fs.writeFileSync(path.join(repo, 'preload.ts'), `require('fs').writeFileSync(${JSON.stringify(preloadMarker)}, 'ran');\n`); + fs.writeFileSync(path.join(repo, 'bunfig.toml'), 'preload = ["./preload.ts"]\n'); + if (IS_WINDOWS) return; + fakeBun = path.join(tmp, 'fake-bun'); + fs.mkdirSync(fakeBun); + writeExec(path.join(fakeBun, 'bun'), [ + '#!/usr/bin/env bash', + 'a=()', + 'for x in "$@"; do', + ' case "$x" in', + ' --config=/dev/null) echo "EINVAL: Invalid argument (open())" >&2; echo "while reading config \\"/dev/null\\"" >&2; exit 1 ;;', + ' --config=NUL) a+=(--config=/dev/null) ;;', + ' *) a+=("$x") ;;', + ' esac', + 'done', + `exec ${JSON.stringify(process.execPath)} "\${a[@]}"`, + '', + ].join('\n')); + crashBun = path.join(tmp, 'crash-bun'); + fs.mkdirSync(crashBun); + writeExec(path.join(crashBun, 'bun'), '#!/usr/bin/env bash\nexit 3\n'); + writeExec(path.join(fakeBun, 'cygpath'), '#!/usr/bin/env bash\nprintf \'%s\\n\' "${@: -1}"\n'); +}); + +afterAll(() => { if (tmp) fs.rmSync(tmp, { recursive: true, force: true }); }); + +describe('#3415: record bins start Bun on a Windows bash with path conversion off', () => { + test('fixture control: a plain bun in the repository runs its bunfig preload', () => { + const r = spawnSync(process.execPath, ['-e', '0'], { cwd: repo, encoding: 'utf8', timeout: 60_000 }); + expect(r.status).toBe(0); + expect(fs.existsSync(preloadMarker)).toBe(true); + fs.rmSync(preloadMarker); + }); + + for (const { bin, input, log } of BINS) { + test(`${bin} writes its record and ignores the repository bunfig.toml and .env`, () => { + resetState(); + const r = run(bin, input); + expect(r.stderr).not.toContain('EINVAL'); + expect(r.status).toBe(0); + expect(records(log).length).toBe(1); + expect(fs.existsSync(preloadMarker)).toBe(false); + expect(fs.existsSync(evilRoot)).toBe(false); + }); + } + + test('gstack-timeline-log still skips invalid input silently with exit 0', () => { + resetState(); + for (const bad of ['not json', '{"skill":"review"}', 'null']) { + const r = run('gstack-timeline-log', bad); + expect(r.status).toBe(0); + expect(r.stderr).toBe(''); + } + expect(records('timeline.jsonl')).toEqual([]); + }); +}); + +describe.skipIf(IS_WINDOWS)('#3415: simulated Windows bash (POSIX hosts)', () => { + test('gstack-developer-profile --log-session reports a Bun failure instead of skipping it, and still skips invalid input silently', () => { + resetState(); + const session = JSON.stringify({ date: '2026-10-25', mode: 'startup' }); + const failed = run('gstack-developer-profile', '--log-session', fakeBun, {}, [session]); + expect(failed.stderr).toContain('EINVAL'); + expect(failed.stderr).toContain('nothing was logged'); + expect(failed.status).not.toBe(0); + for (const bad of ['not json', '{"date":"2026-10-25"}', 'null']) { + const r = run('gstack-developer-profile', '--log-session', undefined, {}, [bad]); + expect(r.status).toBe(0); + expect(r.stderr).toBe(''); + } + }); + + test('a Bun that dies with exit 3 (how a Bun crash exits on Windows) is reported, never read as invalid input', () => { + resetState(); + const timeline = run('gstack-timeline-log', JSON.stringify({ skill: 'review', event: 'started' }), crashBun); + expect(timeline.status).not.toBe(0); + expect(timeline.stderr).toContain('bun exited 3'); + expect(records('timeline.jsonl')).toEqual([]); + const profile = run('gstack-developer-profile', '--log-session', crashBun, {}, [JSON.stringify({ date: '2026-10-25', mode: 'startup' })]); + expect(profile.status).not.toBe(0); + expect(profile.stderr).toContain('bun exited 3'); + }); + + for (const { bin, input, log } of BINS) { + test(`${bin} passes NUL and writes its record`, () => { + resetState(); + const r = run(bin, input, fakeBun, { OSTYPE: 'msys' }); + expect(r.stderr).not.toContain('EINVAL'); + expect(r.status).toBe(0); + expect(records(log).length).toBe(1); + expect(fs.existsSync(preloadMarker)).toBe(false); + }); + + test(`an inherited OSTYPE=msys without cygpath keeps /dev/null: ${bin} never reads a repository file named NUL`, () => { + resetState(); + const nul = path.join(repo, 'NUL'); + fs.writeFileSync(nul, 'preload = ["./preload.ts"]\n'); + try { + const r = run(bin, input, undefined, { OSTYPE: 'msys' }); + expect(r.status).toBe(0); + expect(records(log).length).toBe(1); + expect(fs.existsSync(preloadMarker)).toBe(false); + } finally { + fs.rmSync(nul, { force: true }); + fs.rmSync(preloadMarker, { force: true }); + } + }); + + test(`control: when Bun rejects the config, ${bin} fails loudly and writes nothing`, () => { + resetState(); + const r = run(bin, input, fakeBun); + expect(r.stderr).toContain('EINVAL'); + expect(r.status).not.toBe(0); + expect(records(log)).toEqual([]); + }); + } +}); diff --git a/test/bun-entry-isolation.test.ts b/test/bun-entry-isolation.test.ts index 71b0eb338..5b53d30e9 100644 --- a/test/bun-entry-isolation.test.ts +++ b/test/bun-entry-isolation.test.ts @@ -6,9 +6,11 @@ * kinds are held to the isolated form: * 1. bun shebangs: exactly ISOLATED_SHEBANG (`bun run ` ignores the * shebang, so callers carry the flags too); - * 2. shell callers in shipped shell files, skill templates, generated skill - * docs and resolvers: `bun --no-env-file --config=/dev/null` before - * `run`, `-e`/`-p` or a script path; + * 2. shell callers: skill templates, generated skill docs and resolvers + * spell `bun --no-env-file --config=/dev/null` before `run`, `-e`/`-p` + * or a script path; shipped shell files spell SHELL_FLAGS after + * CONFIG_DEF, so a Windows bash with MSYS path conversion off passes + * NUL instead of a /dev/null native Bun cannot open (#3415); * 3. TypeScript spawns of a Bun: bunIsolationArgs() (or browse's * bunChildFlags()) in the argument list; * 4. scriptInvocation, which launches gstack's own bun bins; @@ -27,6 +29,8 @@ import { emptyBunConfig as browseEmptyBunConfig } from '../browse/src/bun-child- const ROOT = path.resolve(import.meta.dir, '..'); const FLAGS = '--no-env-file --config=/dev/null'; const ISOLATED_SHEBANG = `#!/usr/bin/env -S bun ${FLAGS}`; +const SHELL_FLAGS = '--no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}"'; +const CONFIG_DEF = 'GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac'; const TRACKED = spawnSync('git', ['ls-files', '-z'], { cwd: ROOT, encoding: 'utf8', timeout: 30_000 }).stdout.split('\0').filter(Boolean) .filter(f => !/(^|\/)test\//.test(f) && fs.existsSync(path.join(ROOT, f)) && fs.statSync(path.join(ROOT, f)).isFile()); @@ -43,7 +47,7 @@ const BUN_WORD = /(^|[\s;&|({`"']|\$\()(bun|"\$\{?BUN(?:_[A-Z]+)?\}?"|\$\{?BUN(? /** Whether the text after a `bun` word starts a program: -e/-p, a script path, `run ` or a bin path. A package script (`bun run build`) is not one. */ function runsCode(rest: string): boolean { - const r = rest.replace(/^\s+/, '').replace(/^(?:--(?:no-env-file|cwd[= ]\S+)\s+)*/, ''); + const r = rest.replace(/^\s+/, '').replace(/^(?:--(?:no-env-file|cwd[= ]\S+|config(?:=\S*|\s+\S+))\s+)*/, ''); return /^(?:run\s+)?(?:-e|-p|--eval|--print)(?:\s|=|$)/.test(r) || /^(?:run\s+)?["']?[^\s"'|;&)]*\.(?:ts|js|mjs|cjs|tsx)(?:["'\s)|;&\\`]|$)/.test(r) || /^run\s+["']?\$\{?[A-Za-z_]/.test(r) @@ -51,14 +55,14 @@ function runsCode(rest: string): boolean { } /** Bun invocations on one shell line that start a program without the isolation flags. */ -function unisolatedBunCalls(line: string): string[] { +function unisolatedBunCalls(line: string, flags = FLAGS): string[] { if (/^\s*(#|\/\/|\*)/.test(line)) return []; const out: string[] = []; for (const m of line.matchAll(BUN_WORD)) { const at = m.index! + m[1]!.length; if (/\bln\s+-\w+\s+$/.test(line.slice(0, at))) continue; const rest = line.slice(at + m[2]!.length); - if (!rest.startsWith(` ${FLAGS}`) && runsCode(rest)) out.push(line.slice(at, at + 80)); + if (!rest.startsWith(` ${flags}`) && runsCode(rest)) out.push(line.slice(at, at + 80)); } return out; } @@ -69,6 +73,26 @@ const SHELL_EXEMPT: Record = { 'skillify/SKILL.md': 'generated from skillify/SKILL.md.tmpl', }; +/** Run only on Linux (the Ubicloud VM or its Linux-only local fallback), where /dev/null is the spelling; remote command strings expand there. */ +const SHELL_LINUX_ONLY = (f: string) => f.startsWith('scripts/ubicloud/'); + +/** Lines of shell text that use GSTACK_BUN_CONFIG with no CONFIG_DEF line before them, or define it any other way. A `local` definition covers only the rest of its function. */ +function configDefinitionGaps(text: string): string[] { + const gaps: string[] = []; + let defined = false; + let definedInFunction = false; + text.split('\n').forEach((line, i) => { + if (/^\s*(?:local\s+)?GSTACK_BUN_CONFIG=/.test(line)) { + const local = /^\s*local\s/.test(line); + if (line.trim().replace(/^local\s+/, '') !== CONFIG_DEF) gaps.push(`${i + 1}: redefined: ${line.trim()}`); + else if (local) definedInFunction = true; + else defined = true; + } else if (/^}/.test(line)) definedInFunction = false; + else if (!defined && !definedInFunction && line.includes('${GSTACK_BUN_CONFIG')) gaps.push(`${i + 1}: used before CONFIG_DEF`); + }); + return gaps; +} + const isShell = (f: string) => f.endsWith('.sh') || /^#!.*\b(?:ba|z)?sh\b/.test(firstLine(f)); const isSkillSource = (f: string) => f.endsWith('.tmpl') || /^scripts\/resolvers\/.+\.ts$/.test(f) || /(^|\/)SKILL\.md$/.test(f) || /\/sections\/[^/]+\.md$/.test(f); @@ -116,13 +140,41 @@ describe('GSTA-56: gstack never takes configuration from the audited repository' ]) expect(unisolatedBunCalls(ok)).toEqual([]); }); + test('in shipped shell files the detector requires the Windows-safe, fail-closed config spelling (#3415)', () => { + for (const bad of [ + `printf x | bun ${FLAGS} -e "console.log(1)"`, + 'bun --no-env-file --config="$GSTACK_BUN_CONFIG" -e 1', + 'bun --no-env-file --config= -e 1', + 'bun --no-env-file --config=NUL run "$X/bin/gstack-design-detect.ts"', + ]) expect(unisolatedBunCalls(bad, SHELL_FLAGS)).not.toEqual([]); + for (const ok of [ + `printf x | bun ${SHELL_FLAGS} -e "console.log(1)"`, + `exec bun ${SHELL_FLAGS} "$HERE/question-log-hook.ts"`, + `probe_cmd="bun ${SHELL_FLAGS} --eval 'import x from \\"y\\"'"`, + ]) expect(unisolatedBunCalls(ok, SHELL_FLAGS)).toEqual([]); + }); + + test('the config-definition check flags a missing, late or altered GSTACK_BUN_CONFIG definition', () => { + const use = `bun ${SHELL_FLAGS} -e 1`; + expect(configDefinitionGaps(`#!/usr/bin/env bash\n${CONFIG_DEF}\n${use}\n`)).toEqual([]); + expect(configDefinitionGaps(`f() {\n local ${CONFIG_DEF}\n ${use}\n}\n`)).toEqual([]); + expect(configDefinitionGaps(`f() {\n local ${CONFIG_DEF}\n}\ng() {\n ${use}\n}\n`)).toEqual(['5: used before CONFIG_DEF']); + expect(configDefinitionGaps(`#!/usr/bin/env bash\n${use}\n`)).toEqual(['2: used before CONFIG_DEF']); + expect(configDefinitionGaps(`${use}\n${CONFIG_DEF}\n`)).toEqual(['1: used before CONFIG_DEF']); + expect(configDefinitionGaps(`GSTACK_BUN_CONFIG=./bunfig.toml\n${use}\n`)).toEqual(['1: redefined: GSTACK_BUN_CONFIG=./bunfig.toml', '2: used before CONFIG_DEF']); + expect(configDefinitionGaps(`${CONFIG_DEF}\nGSTACK_BUN_CONFIG=\n${use}\n`)).toEqual(['2: redefined: GSTACK_BUN_CONFIG=']); + }); + test('shell callers, skill templates, generated skill docs and resolvers pass the isolation flags', () => { const offenders: string[] = []; for (const f of TRACKED) { if (f.startsWith('.github/') || SHELL_EXEMPT[f] || !(isSkillSource(f) || isShell(f))) continue; - fs.readFileSync(path.join(ROOT, f), 'utf8').split('\n').forEach((line, i) => { - for (const call of unisolatedBunCalls(line)) offenders.push(`${f}:${i + 1}: ${call}`); + const flags = isSkillSource(f) || SHELL_LINUX_ONLY(f) ? FLAGS : SHELL_FLAGS; + const text = fs.readFileSync(path.join(ROOT, f), 'utf8'); + text.split('\n').forEach((line, i) => { + for (const call of unisolatedBunCalls(line, flags)) offenders.push(`${f}:${i + 1}: ${call}`); }); + if (flags === SHELL_FLAGS) for (const gap of configDefinitionGaps(text)) offenders.push(`${f}:${gap}`); } expect(offenders).toEqual([]); }); diff --git a/test/bun-isolation-launch.test.ts b/test/bun-isolation-launch.test.ts index eba3d4862..d56a943bf 100644 --- a/test/bun-isolation-launch.test.ts +++ b/test/bun-isolation-launch.test.ts @@ -5,6 +5,9 @@ * --config=nul for the native bun.exe, which Bun 1.4.2 reads as an empty * config; a native Windows spawn has no /dev/null, so TypeScript callers use * bunIsolationArgs() (lib/bun-isolation.ts) instead of spelling the flag. + * With conversion off (MSYS_NO_PATHCONV=1) bun.exe gets /dev/null itself and + * fails with EINVAL, so gstack's shell files resolve GSTACK_BUN_CONFIG to NUL + * there (#3415). */ import { afterAll, beforeAll, describe, expect, test } from 'bun:test'; import { spawnSync } from 'node:child_process'; @@ -15,6 +18,8 @@ import { bunIsolationArgs } from '../lib/bun-isolation'; import { bashCommandInvocation } from '../lib/script-invocation'; const PRINT = 'console.log("env=" + (process.env.GSTACK_LAUNCH_PROBE ?? "unset"))'; +const CONFIG_DEF = 'GSTACK_BUN_CONFIG=/dev/null; case "${OSTYPE:-}" in msys*|cygwin*) if command -v cygpath >/dev/null 2>&1; then GSTACK_BUN_CONFIG=NUL; fi ;; esac'; +const NO_PATHCONV = { MSYS_NO_PATHCONV: '1', MSYS2_ARG_CONV_EXCL: '*' }; let project = ''; let preloadMarker = ''; @@ -61,6 +66,18 @@ describe('GSTA-56: isolated Bun launches on this platform', () => { expectIsolated(bash('bun --no-env-file --config=/dev/null run probe.ts')); }); + test('a shipped shell file spelling, with MSYS path conversion off', () => { + expectIsolated(bash(`${CONFIG_DEF}; bun --no-env-file --config="\${GSTACK_BUN_CONFIG:-/dev/null}" -e '${PRINT}'`, NO_PATHCONV)); + expectIsolated(bash(`${CONFIG_DEF}; bun --no-env-file --config="\${GSTACK_BUN_CONFIG:-/dev/null}" run probe.ts`, NO_PATHCONV)); + }); + + test.if(process.platform === 'win32')('control: with MSYS path conversion off, bun.exe cannot open --config=/dev/null', () => { + const r = bash(`bun --no-env-file --config=/dev/null -e '${PRINT}'`, NO_PATHCONV); + expect(r.status).not.toBe(0); + expect(r.stderr).toContain('EINVAL'); + expect(fs.existsSync(preloadMarker)).toBe(false); + }); + test('a TypeScript spawn with bunIsolationArgs()', () => { expectIsolated(spawnSync(process.execPath, [...bunIsolationArgs(), 'probe.ts'], { cwd: project, encoding: 'utf8', timeout: 60_000 })); }); diff --git a/test/fixtures/skill-md-size-allowlist.json b/test/fixtures/skill-md-size-allowlist.json index 444c0d79b..ebb862fd6 100644 --- a/test/fixtures/skill-md-size-allowlist.json +++ b/test/fixtures/skill-md-size-allowlist.json @@ -9,7 +9,7 @@ "agy/gstack-plan-design-review": 140013, "agy/gstack-plan-devex-review": 135760, "agy/gstack-plan-eng-review": 152582, - "agy/gstack-review": 124218, + "agy/gstack-review": 124182, "claude/design-review": 136531, "codex/gstack-autoplan": 130628, "codex/gstack-design-consultation": 109452, @@ -19,7 +19,7 @@ "codex/gstack-plan-design-review": 142466, "codex/gstack-plan-devex-review": 138649, "codex/gstack-plan-eng-review": 155128, - "codex/gstack-review": 125122, + "codex/gstack-review": 125086, "copilot/gstack-autoplan": 131637, "copilot/gstack-codex": 100235, "copilot/gstack-design-consultation": 109536, @@ -29,7 +29,7 @@ "copilot/gstack-plan-design-review": 140884, "copilot/gstack-plan-devex-review": 136912, "copilot/gstack-plan-eng-review": 153307, - "copilot/gstack-review": 125489, + "copilot/gstack-review": 125453, "cursor/gstack-autoplan": 129278, "cursor/gstack-design-consultation": 107222, "cursor/gstack-design-review": 142507, @@ -37,7 +37,7 @@ "cursor/gstack-plan-design-review": 138968, "cursor/gstack-plan-devex-review": 134591, "cursor/gstack-plan-eng-review": 151436, - "cursor/gstack-review": 140750, + "cursor/gstack-review": 140714, "factory/gstack-autoplan": 131562, "factory/gstack-codex": 100655, "factory/gstack-design-consultation": 108670, @@ -47,7 +47,7 @@ "factory/gstack-plan-design-review": 141749, "factory/gstack-plan-devex-review": 137960, "factory/gstack-plan-eng-review": 154550, - "factory/gstack-review": 144529, + "factory/gstack-review": 144493, "gbrain/gstack-autoplan": 129815, "gbrain/gstack-design-review": 132842, "gbrain/gstack-land-and-deploy": 117167, @@ -67,7 +67,7 @@ "kiro/gstack-plan-design-review": 138808, "kiro/gstack-plan-devex-review": 134431, "kiro/gstack-plan-eng-review": 151286, - "kiro/gstack-review": 140562, + "kiro/gstack-review": 140526, "openclaw/gstack-autoplan": 129520, "openclaw/gstack-design-review": 131647, "openclaw/gstack-land-and-deploy": 116757, @@ -82,7 +82,7 @@ "opencode/gstack-plan-design-review": 140738, "opencode/gstack-plan-devex-review": 136812, "opencode/gstack-plan-eng-review": 153188, - "opencode/gstack-review": 142985, + "opencode/gstack-review": 142949, "slate/gstack-autoplan": 129159, "slate/gstack-design-consultation": 107143, "slate/gstack-design-review": 142414, @@ -90,6 +90,6 @@ "slate/gstack-plan-design-review": 138888, "slate/gstack-plan-devex-review": 134511, "slate/gstack-plan-eng-review": 151361, - "slate/gstack-review": 140656 + "slate/gstack-review": 140620 } } diff --git a/test/gstack-lease-git-bash.test.ts b/test/gstack-lease-git-bash.test.ts new file mode 100644 index 000000000..61cc68c18 --- /dev/null +++ b/test/gstack-lease-git-bash.test.ts @@ -0,0 +1,94 @@ +/** + * #3330: gstack-lease took its lock with `ln -s .lock`. Git + * for Windows' default symlink mode (MSYS unset) cannot link to a target that + * does not exist, so the lock was never taken and every skill start waited out + * 300 tries (one report: 499 s) and still wrote no lease. The lock is now a + * directory (mkdir) with an owner file. test/gstack-lease.test.ts pins the full + * contract but is POSIX-bound (mode bits, sleep children), so this file is the + * one the curated windows-free-tests lane runs: it launches the bin through + * lib/script-invocation.ts, which is Git Bash on windows-latest. + */ +import { afterEach, 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 { BIN_DIR, runBashSync, runScriptSync } from '../lib/script-invocation'; + +const LEASE = path.join(BIN_DIR, 'gstack-lease'); +const fwd = (p: string) => p.replace(/\\/g, '/'); +const dirs: string[] = []; +afterEach(() => { + for (const d of dirs.splice(0)) fs.rmSync(d, { recursive: true, force: true }); +}); + +// No inherited lease dir from the test preload, and Git Bash's default symlink mode (no winsymlinks). +const { GSTACK_LEASE_DIR: _preload, MSYS: _msys, CYGWIN: _cygwin, GSTACK_LEASE_LOCK_TRIES: _tries, ...baseEnv } = process.env as Record; + +function checkout() { + const dir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-lease-gb-'))); + dirs.push(dir); + const r = spawnSync('git', ['init', '-q', dir], { encoding: 'utf8', timeout: 30_000 }); + if (r.status !== 0) throw new Error(r.stderr); + return { dir, leases: path.join(dir, '.git', 'gstack-leases') }; +} + +/** A BASH_ENV file in which `ln -s` behaves as Git Bash's default: linking to a missing target fails. */ +function gitBashLn(dir: string): string { + const file = path.join(dir, 'git-bash-ln.sh'); + fs.writeFileSync(file, 'ln() { if [ "$1" = -s ] && [ ! -e "$2" ]; then echo "ln: failed to create symbolic link \'$3\': No such file or directory" >&2; return 1; fi; command ln "$@"; }\n'); + return file; +} + +/** A BASH_ENV file that logs every `sleep` (one per failed lock try) to $WAITS. */ +function sleepLog(dir: string): string { + const file = path.join(dir, 'sleep-log.sh'); + fs.writeFileSync(file, 'sleep() { echo "$*" >> "$WAITS"; command sleep "$@"; }\n'); + return file; +} + +describe('gstack-lease under Git Bash semantics (#3330)', () => { + test('touch takes the lock on its first try with the default tries and writes the lease, leaving no lock behind', () => { + const { dir, leases } = checkout(); + const waits = path.join(dir, 'waits.log'); + const r = runScriptSync(LEASE, ['touch', '--root', fwd(dir), '--pid', '4242'], { encoding: 'utf8', env: { ...baseEnv, BASH_ENV: fwd(sleepLog(dir)), WAITS: fwd(waits) }, timeout: 120_000 }); + expect(r.status, `${r.error ?? ''}${r.stdout}${r.stderr}`).toBe(0); + expect(fs.existsSync(waits), 'lock() slept between tries').toBe(false); + expect(fs.readdirSync(leases)).toEqual(['4242']); + }); + + test('a host whose ln -s cannot link to a missing target still takes the lock (negative control: the shim really fails ln -s)', () => { + const { dir, leases } = checkout(); + const shim = gitBashLn(dir); + const probe = runBashSync('ln -s "1 2" "$PROBE"', { encoding: 'utf8', env: { ...baseEnv, BASH_ENV: fwd(shim), PROBE: fwd(path.join(dir, 'probe')) }, timeout: 30_000 }); + expect(probe.status, `${probe.error ?? ''}${probe.stderr}`).not.toBe(0); + expect(probe.stderr).toContain('failed to create symbolic link'); + const r = runScriptSync(LEASE, ['touch', '--root', fwd(dir), '--pid', '4243'], { encoding: 'utf8', env: { ...baseEnv, BASH_ENV: fwd(shim), GSTACK_LEASE_LOCK_TRIES: '3' }, timeout: 120_000 }); + expect(r.status, `${r.error ?? ''}${r.stdout}${r.stderr}`).toBe(0); + expect(fs.readdirSync(leases)).toEqual(['4243']); + }); + + test('a lock held by a live Git Bash process is kept (touch exits 3 naming it); a dead holder\'s lock is broken', () => { + const { dir, leases } = checkout(); + const script = [ + 'set -u', + 'mkdir -p "$LEASES/.lock" && printf "%s \\n" "$$" > "$LEASES/.lock/owner"', + 'GSTACK_LEASE_LOCK_TRIES=2 bash "$LEASE" touch --root "$ROOT" --pid 4244; echo "held=$? holder=$$ owner=$(head -1 "$LEASES/.lock/owner")"', + '( exit 0 ) & dead=$!; wait "$dead"', + 'printf "%s \\n" "$dead" > "$LEASES/.lock/owner"', + 'GSTACK_LEASE_LOCK_TRIES=2 bash "$LEASE" touch --root "$ROOT" --pid 4245; echo "dead=$?"', + ].join('\n'); + const r = runBashSync(script, { + encoding: 'utf8', timeout: 120_000, + env: { ...baseEnv, LEASE: fwd(LEASE), ROOT: fwd(dir), LEASES: fwd(leases) }, + }); + const out = `${r.error ?? ''}${r.stdout}${r.stderr}`; + expect(r.status, out).toBe(0); + const holder = /held=(\d+) holder=(\d+) owner=(\d+)/.exec(r.stdout); + expect(holder?.[1], out).toBe('3'); + expect(holder?.[3], out).toBe(holder?.[2]); + expect(r.stderr).toContain(`locked by live process ${holder?.[2]}`); + expect(r.stdout, out).toContain('dead=0'); + expect(fs.readdirSync(leases).sort()).toEqual(['4245']); + }); +}); diff --git a/test/gstack-lease.test.ts b/test/gstack-lease.test.ts index f2b6585d2..05cb4945d 100644 --- a/test/gstack-lease.test.ts +++ b/test/gstack-lease.test.ts @@ -59,6 +59,24 @@ function otherStart(recorded: string): string { return recorded.startsWith('linux ') ? recorded.replace(/\d+$/, (n) => String(Number(n) + 1)) : 'ps Thu Jan 1 00:00:00 1970'; } +/** A directory lock as gstack-lease takes it: /owner holds the holder's token. */ +function plantLock(lockPath: string, token: string) { + fs.mkdirSync(lockPath, { recursive: true }); + fs.writeFileSync(path.join(lockPath, 'owner'), token + '\n'); +} +const ownerOf = (lockPath: string) => fs.readFileSync(path.join(lockPath, 'owner'), 'utf8').trim(); +/** Backdate a lock a minute: an ownerless lock is broken only once it is 2 s old. */ +function age(lockPath: string) { + const minuteAgo = new Date(Date.now() - 60_000); + fs.utimesSync(lockPath, minuteAgo, minuteAgo); +} + +/** Each lock layout a holder can leave: the directory lock, and the symlink lock an older gstack took. */ +const LAYOUTS: Array<[string, (lockPath: string, token: string) => void]> = [ + ['directory lock', plantLock], + ['legacy symlink lock', (lockPath, token) => fs.symlinkSync(token, lockPath)], +]; + /** A BASH_ENV file whose functions shadow commands inside the lease script (fault injection). */ function bashEnv(dir: string, body: string): string { const file = path.join(dir, 'bash-env.sh'); @@ -72,6 +90,8 @@ describe('gstack-lease', () => { expect(fs.statSync(LEASE).mode & 0o111).not.toBe(0); const r = spawnSync('bash', ['-n', LEASE], { encoding: 'utf8', timeout: 30_000 }); expect(r.status, r.stderr).toBe(0); + const help = spawnSync('bash', [LEASE, '--help'], { encoding: 'utf8', timeout: 30_000 }); + expect(help.stdout).toContain('print ` m alive|dead|stale` for every lease'); }); test('touch writes /gstack-leases/ for --pid and prunes leases older than 120 minutes', () => { @@ -160,7 +180,7 @@ describe('gstack-lease', () => { const renewed = path.join(dir, 'renewed'); fs.writeFileSync(renewed, fresh); // The holder renews just before reclaim takes the lock, as if touch ran after the stale verdict. - const racing = bashEnv(dir, `ln() { case "$*" in *"/.lock") command cp "${renewed}" "${file}" ;; esac; command ln "$@"; }\n`); + const racing = bashEnv(dir, `mkdir() { case "$*" in *"/.lock") command cp "${renewed}" "${file}" ;; esac; command mkdir "$@"; }\n`); const r = run(['live', '--root', dir], { BASH_ENV: racing }); expect(r.status, r.stdout + r.stderr).toBe(0); expect(r.stderr).not.toContain('reclaimed'); @@ -168,23 +188,24 @@ describe('gstack-lease', () => { expect(fs.readdirSync(leases).filter((n) => n.startsWith('.'))).toEqual([]); }); - test('a lock whose holder died, or whose pid now belongs to another process, is broken at once', () => { + test.each(LAYOUTS)('a %s whose holder died, or whose pid now belongs to another process, is broken at once', (_layout, plant) => { const { dir, leases } = checkout(); const holder = sleeper(); fs.mkdirSync(leases, { recursive: true }); - fs.symlinkSync(`${deadPid()} `, path.join(leases, '.lock')); + plant(path.join(leases, '.lock'), `${deadPid()} `); const r = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); expect(r.status, r.stdout + r.stderr).toBe(0); expect(fs.existsSync(path.join(leases, '.lock'))).toBe(false); const identity = fs.readFileSync(path.join(leases, String(holder)), 'utf8').trim(); expect(identity).toMatch(/^(linux|ps) /); - fs.symlinkSync(`${holder} ${otherStart(identity)}`, path.join(leases, '.lock')); + plant(path.join(leases, '.lock'), `${holder} ${otherStart(identity)}`); const again = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); expect(again.status, again.stdout + again.stderr).toBe(0); expect(fs.existsSync(path.join(leases, '.lock'))).toBe(false); + expect(fs.readdirSync(leases).filter((n) => n.startsWith('.'))).toEqual([]); }); - test('a live holder\'s lock is never broken, however old: touch exits 3 without writing and a reclaim keeps the lease', () => { + test.each(LAYOUTS)('a live holder\'s %s is never broken, however old: touch exits 3 without writing and a reclaim keeps the lease', (layout, plant) => { const { dir, leases } = checkout(); const owner = sleeper(); const holder = sleeper(); @@ -192,9 +213,10 @@ describe('gstack-lease', () => { expect(run(['touch', '--root', dir, '--pid', String(holder)]).status).toBe(0); const ownerToken = `${owner} ${fs.readFileSync(path.join(leases, String(owner)), 'utf8').trim()}`; const lockPath = path.join(leases, '.lock'); - fs.symlinkSync(ownerToken, lockPath); + plant(lockPath, ownerToken); const old = new Date(Date.now() - 5 * 60_000); fs.lutimesSync(lockPath, old, old); + if (layout === 'directory lock') fs.utimesSync(path.join(lockPath, 'owner'), old, old); const holderFile = path.join(leases, String(holder)); const stale = otherStart(fs.readFileSync(holderFile, 'utf8').trim()) + '\n'; fs.writeFileSync(holderFile, stale); @@ -207,7 +229,7 @@ describe('gstack-lease', () => { expect(l.stderr).not.toContain('reclaimed'); expect(l.stdout).toContain(`${holder} `); expect(fs.existsSync(holderFile)).toBe(true); - expect(fs.readlinkSync(lockPath)).toBe(ownerToken); + expect(layout === 'directory lock' ? ownerOf(lockPath) : fs.readlinkSync(lockPath)).toBe(ownerToken); }); test('two breakers of one dead lock: the second rereads under .lock.break and leaves the lock the first one took', () => { @@ -217,21 +239,21 @@ describe('gstack-lease', () => { expect(run(['touch', '--root', dir, '--pid', String(owner)]).status).toBe(0); const ownerToken = `${owner} ${fs.readFileSync(path.join(leases, String(owner)), 'utf8').trim()}`; const lockPath = path.join(leases, '.lock'); - fs.symlinkSync(`${deadPid()} `, lockPath); + plantLock(lockPath, `${deadPid()} `); // Between this breaker's dead verdict and its .lock.break, another breaker removes the dead lock and takes it. - const first = bashEnv(dir, `ln() { case "$*" in *"/.lock.break") command rm -f "${lockPath}"; command ln -s "${ownerToken}" "${lockPath}" ;; esac; command ln "$@"; }\n`); + const first = bashEnv(dir, `mkdir() { case "$*" in *"/.lock.break") command rm -rf "${lockPath}"; command mkdir "${lockPath}"; printf '%s\\n' "${ownerToken}" > "${lockPath}/owner" ;; esac; command mkdir "$@"; }\n`); const t = run(['touch', '--root', dir, '--pid', String(holder)], { BASH_ENV: first, GSTACK_LEASE_LOCK_TRIES: '3' }); expect(t.status, t.stdout + t.stderr).toBe(3); - expect(fs.readlinkSync(lockPath)).toBe(ownerToken); - expect(fs.existsSync(path.join(leases, '.lock.break'))).toBe(false); + expect(ownerOf(lockPath)).toBe(ownerToken); + expect(fs.readdirSync(leases).filter((n) => n.startsWith('.')).sort()).toEqual(['.lock']); }); - test('a .lock.break left by a dead breaker is cleared and the dead lock is still broken', () => { + test.each(LAYOUTS)('a .lock.break (%s) left by a dead breaker is cleared and the dead lock is still broken', (_layout, plant) => { const { dir, leases } = checkout(); const holder = sleeper(); fs.mkdirSync(leases, { recursive: true }); - fs.symlinkSync(`${deadPid()} `, path.join(leases, '.lock')); - fs.symlinkSync(`${deadPid()} `, path.join(leases, '.lock.break')); + plant(path.join(leases, '.lock'), `${deadPid()} `); + plant(path.join(leases, '.lock.break'), `${deadPid()} `); const r = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '5' }); expect(r.status, r.stdout + r.stderr).toBe(0); expect(fs.existsSync(path.join(leases, String(holder)))).toBe(true); @@ -249,12 +271,355 @@ describe('gstack-lease', () => { fs.writeFileSync(holderFile, otherStart(fs.readFileSync(holderFile, 'utf8').trim()) + '\n'); const lockPath = path.join(leases, '.lock'); // While reclaim deletes the stale lease under its lock, the lock changes hands. - const swap = bashEnv(dir, `rm() { if [ "$*" = "-f ${holderFile}" ]; then command rm -f "${lockPath}"; command ln -s "${ownerToken}" "${lockPath}"; fi; command rm "$@"; }\n`); + const swap = bashEnv(dir, `rm() { if [ "$*" = "-f ${holderFile}" ]; then command rm -rf "${lockPath}"; command mkdir "${lockPath}"; printf '%s\\n' "${ownerToken}" > "${lockPath}/owner"; fi; command rm "$@"; }\n`); const r = run(['live', '--root', dir, '--exclude-ancestry-of', String(owner)], { BASH_ENV: swap }); expect(r.stderr).toContain(`reclaimed stale lease ${holder}`); - expect(fs.readlinkSync(lockPath)).toBe(ownerToken); + expect(ownerOf(lockPath)).toBe(ownerToken); }); + test('a lock held by a running gstack-lease excludes a second taker, which names it; both leases land once it is released', async () => { + const { dir, leases } = checkout(); + const [first, second] = [sleeper(), sleeper()]; + // The first touch holds the lock across its prune (find runs under the lock) until the test releases it. + const release = path.join(dir, 'release'); + const slow = bashEnv(dir, `find() { case "$*" in *'+120'*) until [ -e "${release}" ]; do sleep 0.05; done ;; esac; command find "$@"; }\n`); + const holder = spawn('bash', [LEASE, 'touch', '--root', dir, '--pid', String(first)], { env: { ...baseEnv, BASH_ENV: slow }, stdio: 'ignore' }); + children.push(holder); + const exited = new Promise((resolve) => holder.on('exit', resolve)); + const owner = path.join(leases, '.lock', 'owner'); + for (let i = 0; i < 500 && !fs.existsSync(owner); i++) await Bun.sleep(20); + expect(fs.readFileSync(owner, 'utf8')).toMatch(new RegExp(`^${holder.pid} `)); + const blocked = run(['touch', '--root', dir, '--pid', String(second)], { GSTACK_LEASE_LOCK_TRIES: '3' }); + fs.writeFileSync(release, ''); + expect(blocked.status, blocked.stdout + blocked.stderr).toBe(3); + expect(blocked.stderr).toContain(`locked by live process ${holder.pid};`); + expect(await exited).toBe(0); + expect(run(['touch', '--root', dir, '--pid', String(second)]).status).toBe(0); + expect(fs.readdirSync(leases).sort()).toEqual([String(first), String(second)].sort()); + }, 30_000); + + test('parallel touches all take the lock in turn and leave no lock, claim or temp file behind', async () => { + const { dir, leases } = checkout(); + const pids = Array.from({ length: 16 }, (_, i) => 4300 + i); + // The prune runs under the lock: a second process inside it at the same time logs an overlap. + const inside = path.join(dir, 'inside'); + const overlaps = path.join(dir, 'overlaps.log'); + const probe = bashEnv(dir, `find() { case "$*" in *'+120'*) command mkdir "${inside}" 2>/dev/null || echo overlap >> "${overlaps}"; sleep 0.05; command rmdir "${inside}" 2>/dev/null ;; esac; command find "$@"; }\n`); + const codes = await Promise.all(pids.map((p) => new Promise((resolve) => { + const c = spawn('bash', [LEASE, 'touch', '--root', dir, '--pid', String(p)], { env: { ...baseEnv, BASH_ENV: probe }, stdio: 'ignore' }); + children.push(c); + c.on('exit', resolve); + }))); + expect(codes).toEqual(pids.map(() => 0)); + expect(fs.existsSync(overlaps), 'two touches were inside the lock at once').toBe(false); + expect(fs.readdirSync(leases).sort()).toEqual(pids.map(String).sort()); + }, 30_000); + + // #3330: a taker killed between its mkdir and its owner rename leaves an ownerless lock. + test('an ownerless lock is broken once every claimant is gone, and kept while a live claimant may still own it', () => { + const { dir, leases } = checkout(); + const holder = sleeper(); + const lockPath = path.join(leases, '.lock'); + fs.mkdirSync(lockPath, { recursive: true }); + age(lockPath); + fs.writeFileSync(path.join(leases, `.lock.claim.${holder}`), `${holder} \n`); + const kept = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); + expect(kept.status, kept.stdout + kept.stderr).toBe(3); + expect(kept.stderr).toContain('cannot take the lease directory lock'); + expect(fs.existsSync(path.join(leases, String(holder)))).toBe(false); + expect(fs.readdirSync(lockPath)).toEqual([]); + + const dead = deadPid(); + fs.renameSync(path.join(leases, `.lock.claim.${holder}`), path.join(leases, `.lock.claim.${dead}`)); + fs.writeFileSync(path.join(leases, `.lock.claim.${dead}`), `${dead} \n`); + fs.writeFileSync(path.join(leases, '.lock.claim.1'), ''); + age(lockPath); + const broken = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); + expect(broken.status, broken.stdout + broken.stderr).toBe(0); + expect(fs.existsSync(path.join(leases, String(holder)))).toBe(true); + expect(fs.existsSync(lockPath)).toBe(false); + + fs.rmSync(path.join(leases, `.lock.claim.${dead}`)); + fs.mkdirSync(lockPath); + age(lockPath); + const unclaimed = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); + expect(unclaimed.status, unclaimed.stdout + unclaimed.stderr).toBe(0); + expect(fs.existsSync(lockPath)).toBe(false); + }); + + // Value: protects=an ownerless lock younger than 2 s is never broken, so a live taker whose claim landed after the breaker read the claims keeps + // the lock it is about to publish; a malformed lock (empty owner, pid 0, or a stray file) is broken once old instead of wedging every touch; + // fails_when=orphaned() drops its age check, or treats only a missing owner file (not an empty or invalid one) as ownerless; + // why_new=the ownerless cases above all backdate the lock; nothing pins the young-lock guard or the malformed shapes; + // seam=none; no_claim=the claim-read race itself (a young unclaimed lock stands in for a taker whose claim the breaker missed) + test('an ownerless lock is broken only once it is 2 s old, and a malformed lock is treated as ownerless', () => { + const { dir, leases } = checkout(); + const holder = sleeper(); + const lockPath = path.join(leases, '.lock'); + fs.mkdirSync(lockPath, { recursive: true }); + // The clock reads the lock's own mtime, so the lock stays 0 s old however slow the runner is. + const frozen = bashEnv(dir, `date() { case "$1" in +%s) command stat -c %Y "${lockPath}" 2>/dev/null || command stat -f %m "${lockPath}" ;; *) command date "$@" ;; esac; }\n`); + const young = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3', BASH_ENV: frozen }); + expect(young.status, young.stdout + young.stderr).toBe(3); + expect(fs.existsSync(lockPath)).toBe(true); + expect(fs.existsSync(path.join(leases, String(holder)))).toBe(false); + + const shapes: Array<[string, () => void]> = [ + ['an empty owner file', () => plantLock(lockPath, '')], + ['owner pid 0', () => plantLock(lockPath, '0 ')], + ['a zero-padded owner pid', () => plantLock(lockPath, '00 ')], + ['a non-numeric owner', () => plantLock(lockPath, 'garbage x')], + ['an ownerless lock an hour ahead (clock skew)', () => fs.mkdirSync(lockPath)], + ['a stray file', () => { fs.rmSync(lockPath, { recursive: true, force: true }); fs.writeFileSync(lockPath, ''); }], + ]; + for (const [shape, plant] of shapes) { + fs.rmSync(lockPath, { recursive: true, force: true }); + fs.rmSync(path.join(leases, String(holder)), { force: true }); + plant(); + age(lockPath); + if (shape.includes('clock skew')) { + const hourAhead = new Date(Date.now() + 3_600_000); + fs.utimesSync(lockPath, hourAhead, hourAhead); + } + const r = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); + expect(r.status, `${shape}: ${r.stdout}${r.stderr}`).toBe(0); + expect(fs.readdirSync(leases), shape).toEqual([String(holder)]); + } + }); + + // Value: protects=a claim without a start identity left by a killed taker neither blocks recovery of an ownerless lock (even when its pid is + // alive again) nor lingers once 2 minutes old, while a claim whose recorded identity is still live is honored however old; + // fails_when=orphaned() honors identity-less claims of any age, expires live identified claims, or touch's prune skips claim files; + // why_new=the ownerless cases plant fresh claims only; nothing ages one or checks the prune for claims; + // seam=none; no_claim=real pid reuse (a live sleeper stands in for the reused pid) + test('an identity-less claim older than 2 minutes is abandoned and pruned; an old claim with a live identity is honored', () => { + const { dir, leases } = checkout(); + const [holder, reused] = [sleeper(), sleeper()]; + const lockPath = path.join(leases, '.lock'); + fs.mkdirSync(lockPath, { recursive: true }); + age(lockPath); + const claim = path.join(leases, `.lock.claim.${reused}`); + fs.writeFileSync(claim, `${reused} \n`); + const fiveMinutesAgo = new Date(Date.now() - 300_000); + fs.utimesSync(claim, fiveMinutesAgo, fiveMinutesAgo); + const r = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); + expect(r.status, r.stdout + r.stderr).toBe(0); + expect(fs.readdirSync(leases)).toEqual([String(holder)]); + + // A claim that records its claimant's start identity is honored however old, so a stalled live taker keeps its lock. + expect(run(['touch', '--root', dir, '--pid', String(reused)]).status).toBe(0); + const identity = fs.readFileSync(path.join(leases, String(reused)), 'utf8').trim(); + expect(identity, 'this host records start identities').not.toBe(''); + fs.rmSync(path.join(leases, String(reused))); + fs.mkdirSync(lockPath); + age(lockPath); + fs.writeFileSync(claim, `${reused} ${identity}\n`); + fs.utimesSync(claim, fiveMinutesAgo, fiveMinutesAgo); + const kept = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); + expect(kept.status, kept.stdout + kept.stderr).toBe(3); + expect(fs.existsSync(lockPath)).toBe(true); + expect(fs.existsSync(claim)).toBe(true); + }); + + // Value: protects=a live holder's lock is never broken because its owner file could not be read (a Windows sharing violation, a permission error); + // fails_when=orphaned() treats an unreadable owner (empty owner_of output) as no owner; + // why_new=every ownerless case removes or empties the owner file; none makes a present owner unreadable; + // seam=none; no_claim=a real sharing violation (a head shim fails reads of the owner file) + test('a lock whose owner file cannot be read is kept, not taken for ownerless', () => { + const { dir, leases } = checkout(); + const [holder, owner] = [sleeper(), sleeper()]; + const lockPath = path.join(leases, '.lock'); + plantLock(lockPath, `${owner} `); + age(lockPath); + const shim = bashEnv(dir, 'head() { case "$*" in *"/.lock/owner") return 1 ;; esac; command head "$@"; }\n'); + const r = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3', BASH_ENV: shim }); + expect(r.status, r.stdout + r.stderr).toBe(3); + expect(ownerOf(lockPath)).toBe(String(owner)); + expect(fs.existsSync(path.join(leases, String(holder)))).toBe(false); + }); + + // Value: protects=a live holder that never lets go bounds touch at 30 s even with the default 300 tries, a non-numeric + // GSTACK_LEASE_LOCK_TRIES falls back to the default instead of looping forever, and a visibly held lock is never mkdir'd; + // fails_when=lock() loses its SECONDS deadline or its tries validation, or take() stops checking the path first; + // why_new=every live-holder case lowers the tries; none runs the default budget, a bad value or counts mkdir calls; + // seam=none; no_claim=real elapsed time (a sleep shim advances SECONDS 10 s per wait) + test('a live holder bounds touch by a 30 s deadline, and a non-numeric tries value cannot loop forever', () => { + const { dir, leases } = checkout(); + const [holder, owner] = [sleeper(), sleeper()]; + plantLock(path.join(leases, '.lock'), `${owner} `); + const waits = path.join(dir, 'waits.log'); + const mkdirs = path.join(dir, 'mkdirs.log'); + const shim = bashEnv(dir, `sleep() { echo "$*" >> "${waits}"; SECONDS=$((SECONDS + 10)); }\nmkdir() { case "$*" in *"/.lock") echo "$*" >> "${mkdirs}" ;; esac; command mkdir "$@"; }\n`); + for (const tries of [undefined, 'abc']) { + fs.rmSync(waits, { force: true }); + const env: Record = { BASH_ENV: shim }; + if (tries) env.GSTACK_LEASE_LOCK_TRIES = tries; + const r = spawnSync('bash', [LEASE, 'touch', '--root', dir, '--pid', String(holder)], { encoding: 'utf8', env: { ...baseEnv, ...env }, timeout: 20_000 }); + expect(r.error?.message ?? '', `tries=${tries}: touch did not end`).not.toContain('ETIMEDOUT'); + expect(r.status, `tries=${tries}: ${r.stdout}${r.stderr}`).toBe(3); + expect(r.stderr).toContain(`locked by live process ${owner};`); + expect(r.stderr).not.toContain('integer expression'); + expect(fs.readFileSync(waits, 'utf8').trim().split('\n').length, `tries=${tries}`).toBeLessThanOrEqual(3); + } + expect(fs.existsSync(mkdirs), 'take() ran mkdir on a visibly held lock').toBe(false); + expect(ownerOf(path.join(leases, '.lock'))).toBe(String(owner)); + }, 30_000); + + // Value: protects=a holder that resumes past the 120-minute prune window keeps its own lock metadata, so unlock still recognizes and removes it; + // fails_when=touch's prune recurses into .lock (deleting owner) or matches lock files at the top level; + // why_new=the prune test plants only lease files; none ages the lock's owner file before the prune runs; + // seam=none; no_claim=a real suspension (a BASH_ENV find shim backdates the owner file just before the prune) + test('the 120-minute prune never deletes the lock it runs under', () => { + const { dir, leases } = checkout(); + const holder = sleeper(); + const shim = bashEnv(dir, 'find() { command touch -d "3 hours ago" "$1/.lock/owner" 2>/dev/null || command touch -t 197001010000 "$1/.lock/owner"; command find "$@"; }\n'); + // Renamed-aside copies a failed delete left behind are pruned too. + fs.mkdirSync(path.join(leases, '.lock.gone.99999'), { recursive: true }); + fs.writeFileSync(path.join(leases, '.lock.gone.99999', 'owner'), '99999 \n'); + fs.mkdirSync(path.join(leases, '.lock.break.gone.99998')); + const r = run(['touch', '--root', dir, '--pid', String(holder)], { BASH_ENV: shim }); + expect(r.status, r.stdout + r.stderr).toBe(0); + expect(fs.readdirSync(leases)).toEqual([String(holder)]); + }); + + // Value: protects=once a lock is renamed aside it is released: a failed delete of the renamed copy never removes a lock a new taker made at .lock; + // fails_when=drop() reports failure after a successful rename, so unlock's in-place rm -rf fallback runs on the new taker's lock; + // why_new=the rename-aside tests fail the rename itself; none lets the rename succeed and the delete fail; + // seam=none; no_claim=a real new taker in that window (the rm shim plants one, then fails like a Windows file in use) + test('a renamed-aside lock that cannot be deleted never takes a new taker\'s lock with it', () => { + const { dir, leases } = checkout(); + const [holder, taker] = [sleeper(), sleeper()]; + const lockPath = path.join(leases, '.lock'); + const shim = bashEnv(dir, `rm() { case "$*" in *".lock.gone."*) command mkdir "${lockPath}" && printf '%s \\n' "${taker}" > "${lockPath}/owner"; return 1 ;; esac; command rm "$@"; }\n`); + const r = run(['touch', '--root', dir, '--pid', String(holder)], { BASH_ENV: shim }); + expect(r.status, r.stdout + r.stderr).toBe(0); + expect(ownerOf(lockPath)).toBe(String(taker)); + expect(fs.existsSync(path.join(leases, String(holder)))).toBe(true); + }); + + test('a lock that cannot be taken while nobody holds it fails at once instead of waiting out the tries', () => { + const { dir, leases } = checkout(); + const holder = sleeper(); + const waits = path.join(dir, 'waits.log'); + const broken = bashEnv(dir, `mkdir() { case "$*" in *"/.lock") return 1 ;; esac; command mkdir "$@"; }\nsleep() { echo "$*" >> "${waits}"; command sleep "$@"; }\n`); + const r = run(['touch', '--root', dir, '--pid', String(holder)], { BASH_ENV: broken }); + expect(r.status, r.stdout + r.stderr).toBe(3); + expect(fs.existsSync(waits), 'lock() slept between tries').toBe(false); + expect(r.stderr).toContain('cannot take the lease directory lock'); + expect(r.stderr).not.toContain('locked by live process'); + expect(fs.readdirSync(leases)).toEqual([]); + }); + + // Value: protects=a .lock.break held by a live breaker (owner, or claim on an ownerless dir) is never cleared; an ownerless one with no claimant is; + // fails_when=break_lock's liveness check on .lock.break is dropped, or orphaned() ignores a live claim; + // why_new=the existing .lock.break test plants only a dead breaker's owner, never a live breaker or an ownerless one; + // seam=none; no_claim=two live breakers racing for real (sleeping children stand in as the live holders) + test('a .lock.break whose breaker lives (owner or claim) is never cleared; an ownerless one with no live claimant is', () => { + const { dir, leases } = checkout(); + const holder = sleeper(); + const breaker = sleeper(); + const lockPath = path.join(leases, '.lock'); + const breakPath = path.join(leases, '.lock.break'); + plantLock(lockPath, `${deadPid()} `); + plantLock(breakPath, `${breaker} `); + const owned = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); + expect(owned.status, owned.stdout + owned.stderr).toBe(3); + expect(ownerOf(breakPath)).toBe(String(breaker)); + expect(fs.existsSync(lockPath)).toBe(true); + expect(fs.existsSync(path.join(leases, String(holder)))).toBe(false); + + fs.rmSync(breakPath, { recursive: true }); + fs.mkdirSync(breakPath); + age(breakPath); + fs.writeFileSync(path.join(leases, `.lock.break.claim.${breaker}`), `${breaker} \n`); + const claimed = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); + expect(claimed.status, claimed.stdout + claimed.stderr).toBe(3); + expect(fs.readdirSync(breakPath)).toEqual([]); + expect(fs.existsSync(lockPath)).toBe(true); + expect(fs.existsSync(path.join(leases, String(holder)))).toBe(false); + + fs.rmSync(path.join(leases, `.lock.break.claim.${breaker}`)); + age(breakPath); + const orphaned = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '5' }); + expect(orphaned.status, orphaned.stdout + orphaned.stderr).toBe(0); + expect(fs.readdirSync(leases)).toEqual([String(holder)]); + }); + + // Value: protects=a lock dir whose owner rename failed is removed, never left ownerless; touch exits 3 saying it cannot take the lock; + // fails_when=take() stops removing the lock dir after a failed owner rename; + // why_new=the ownerless-lock test plants the dir by hand; none fails take() between its mkdir and its owner rename; + // seam=none; no_claim=real mv failure modes (a BASH_ENV shim fails the claim-to-owner rename) + test('a lock whose owner cannot be published is removed, not left ownerless, and touch fails at once', () => { + const { dir, leases } = checkout(); + const holder = sleeper(); + const shim = bashEnv(dir, 'mv() { case "$*" in *".lock.claim."*) return 1 ;; esac; command mv "$@"; }\n'); + const r = run(['touch', '--root', dir, '--pid', String(holder)], { BASH_ENV: shim }); + expect(r.status, r.stdout + r.stderr).toBe(3); + expect(r.stderr).toContain('cannot take the lease directory lock'); + expect(r.stderr).not.toContain('locked by live process'); + expect(fs.readdirSync(leases)).toEqual([]); + }); + + // Value: protects=a lock that vanishes between a failed take and its check is retaken at once: touch writes its lease, no sleep, no lock left; + // fails_when=the vanished-lock retry limit drops to 1, so one release race exits 3; + // why_new=the fails-at-once test pins only the upper bound (every take fails); no test fails a single take; + // seam=none; no_claim=a real release race (a BASH_ENV shim fails the first mkdir of the lock) + test('a lock released between a failed take and its check is retaken without waiting', () => { + const { dir, leases } = checkout(); + const holder = sleeper(); + const waits = path.join(dir, 'waits.log'); + const once = path.join(dir, 'failed-once'); + const shim = bashEnv(dir, `mkdir() { case "$*" in *"/.lock") if [ ! -e "${once}" ]; then : > "${once}"; return 1; fi ;; esac; command mkdir "$@"; }\nsleep() { echo "$*" >> "${waits}"; command sleep "$@"; }\n`); + const r = run(['touch', '--root', dir, '--pid', String(holder)], { BASH_ENV: shim }); + expect(r.status, r.stdout + r.stderr).toBe(0); + expect(fs.existsSync(once), 'the shim never failed a take').toBe(true); + expect(fs.existsSync(waits), 'lock() slept between tries').toBe(false); + expect(fs.readdirSync(leases)).toEqual([String(holder)]); + }); + + // Value: protects=unlock removes the caller's own lock even when renaming it aside fails: touch exits 0, lease written, no .lock left behind; + // fails_when=the rm -rf fallback after drop in unlock() is removed; + // why_new=no test fails the rename-aside step of drop(), so unlock's fallback never runs; + // seam=none; no_claim=real mv failure modes such as a Windows file in use (a BASH_ENV shim fails the .lock.gone rename) + test('unlock still removes its own lock when renaming it aside fails', () => { + const { dir, leases } = checkout(); + const holder = sleeper(); + const shim = bashEnv(dir, 'mv() { case "$*" in *".lock.gone."*) return 1 ;; esac; command mv "$@"; }\n'); + const r = run(['touch', '--root', dir, '--pid', String(holder)], { BASH_ENV: shim }); + expect(r.status, r.stdout + r.stderr).toBe(0); + expect(fs.readdirSync(leases)).toEqual([String(holder)]); + }); + + // Value: protects=a dead holder's lock that cannot be renamed aside is removed in place, and one that cannot be removed at all ends touch + // with exit 3 within the tries limit instead of spinning, without calling the dead holder live; + // fails_when=break_lock reports success after a failed drop and lock() loops without sleeping or counting tries; + // why_new=the dead-holder tests always let the rename-aside succeed; the tries limit is only reached through the live-holder path; + // seam=none; no_claim=real mv/rm failure modes such as a Windows file in use (a BASH_ENV shim fails the .lock.gone rename and the rm) + test('a dead holder\'s lock that cannot be renamed aside ends touch with a verdict instead of spinning', () => { + const { dir, leases } = checkout(); + const holder = sleeper(); + const dead = deadPid(); + const touch = (shim: string) => spawnSync('bash', [LEASE, 'touch', '--root', dir, '--pid', String(holder)], { + encoding: 'utf8', env: { ...baseEnv, BASH_ENV: shim, GSTACK_LEASE_LOCK_TRIES: '3' }, timeout: 8_000, + }); + const noRename = 'mv() { case "$*" in *".lock.gone."*) return 1 ;; esac; command mv "$@"; }\n'; + plantLock(path.join(leases, '.lock'), `${dead} `); + const removed = touch(bashEnv(dir, noRename)); + expect(removed.error?.message ?? '', 'touch did not end within 8 s').not.toContain('ETIMEDOUT'); + expect(removed.status, removed.stdout + removed.stderr).toBe(0); + expect(fs.readdirSync(leases)).toEqual([String(holder)]); + + fs.rmSync(path.join(leases, String(holder))); + plantLock(path.join(leases, '.lock'), `${dead} `); + const stuck = touch(bashEnv(dir, noRename + 'rm() { case "$*" in *"/.lock") return 1 ;; esac; command rm "$@"; }\n')); + expect(stuck.error?.message ?? '', 'touch did not end within 8 s').not.toContain('ETIMEDOUT'); + expect(stuck.status, stuck.stdout + stuck.stderr).toBe(3); + expect(stuck.stderr).toContain('cannot take the lease directory lock'); + expect(stuck.stderr).not.toContain('locked by live process'); + expect(ownerOf(path.join(leases, '.lock'))).toBe(String(dead)); + expect(fs.existsSync(path.join(leases, String(holder)))).toBe(false); + }, 20_000); + test('a lease without a recorded identity (older gstack) counts while fresh and is stale past the 120-minute window', () => { const { dir, leases } = checkout(); const holder = sleeper(); @@ -333,7 +698,7 @@ describe('gstack-lease', () => { fs.writeFileSync(path.join(state, 'config.yaml'), 'update_check: false\n'); const leaseDir = path.join(state, 'leases'); fs.mkdirSync(leaseDir); - fs.symlinkSync(`${sleeper()} `, path.join(leaseDir, '.lock')); + plantLock(path.join(leaseDir, '.lock'), `${sleeper()} `); const r = spawnSync(START, ['--skill', 'testskill', '--parent-pid', '424244'], { encoding: 'utf8', cwd: home, env: { PATH: process.env.PATH!, HOME: home, GSTACK_HOME: state, GSTACK_LEASE_DIR: leaseDir, GSTACK_LEASE_LOCK_TRIES: '2' }, timeout: 30_000, }); diff --git a/test/learnings-injection.test.ts b/test/learnings-injection.test.ts index df387f248..89515fe79 100644 --- a/test/learnings-injection.test.ts +++ b/test/learnings-injection.test.ts @@ -6,10 +6,15 @@ const SCRIPT = path.join(import.meta.dir, "..", "bin", "gstack-learnings-search" describe("gstack-learnings-search injection prevention", () => { const script = readFileSync(SCRIPT, "utf-8"); + const bunBlockAt = script.indexOf('bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e "'); + + test("the bun -e block the checks below read is present", () => { + expect(bunBlockAt).toBeGreaterThanOrEqual(0); + }); test("no shell interpolation inside bun -e string", () => { // Extract the bun -e block (everything between `bun -e "` and the closing `"`) - const bunBlock = script.slice(script.indexOf('bun --no-env-file --config=/dev/null -e "')); + const bunBlock = script.slice(bunBlockAt); // Should NOT contain ${VAR} patterns (shell interpolation) // These are RCE vectors: a malicious learnings entry with '; rm -rf / ;' in the @@ -27,7 +32,7 @@ describe("gstack-learnings-search injection prevention", () => { }); test("uses process.env for all user-controlled values", () => { - const bunBlock = script.slice(script.indexOf('bun --no-env-file --config=/dev/null -e "')); + const bunBlock = script.slice(bunBlockAt); // Must use process.env for TYPE, QUERY, LIMIT. // SLUG and CROSS are no longer threaded as env vars inside the bun @@ -52,7 +57,7 @@ describe("gstack-learnings-search injection prevention", () => { }); test("current vs cross-project rows distinguished by inline tags, not SLUG env (#1619)", () => { - const bunBlock = script.slice(script.indexOf('bun --no-env-file --config=/dev/null -e "')); + const bunBlock = script.slice(bunBlockAt); // The bun block must inspect the per-line tag to mark cross-project rows. // The current shape emits `current\t` or `cross\t` from the // upstream pipe (via emit_tagged_file). Inside the bun block, the script diff --git a/test/plan-mode-preamble-render.test.ts b/test/plan-mode-preamble-render.test.ts index 78a476c0f..24bedde3f 100644 --- a/test/plan-mode-preamble-render.test.ts +++ b/test/plan-mode-preamble-render.test.ts @@ -16,7 +16,7 @@ import * as path from 'path'; const ROOT = path.resolve(import.meta.dir, '..'); const PLAN_ARG = '${_PLAN_MODE_ARG:+"$_PLAN_MODE_ARG"}'; -const GENERATOR = /(?:\bbun(?: --no-env-file --config=\/dev\/null)?|\bbun_cmd|_gstack_render_bun) run (?:gen:skill-docs(?::user)?|scripts\/gen-skill-docs\.ts) --[a-z]/; +const GENERATOR = /(?:\bbun(?: --no-env-file --config=(?:\/dev\/null|"\$\{GSTACK_BUN_CONFIG:-\/dev\/null\}"))?|\bbun_cmd|_gstack_render_bun) run (?:gen:skill-docs(?::user)?|scripts\/gen-skill-docs\.ts) --[a-z]/; const CONFIG_READ = '[ "$("$GSTACK_CONFIG" get plan_mode_preamble 2>/dev/null || true)" != off ] || _PLAN_MODE_ARG="--plan-mode=off"'; /** Generator invocations in a file: not comments, and not echoed or logged instructions. */ diff --git a/test/review-fix-range.test.ts b/test/review-fix-range.test.ts index defb04c03..757b6be1c 100644 --- a/test/review-fix-range.test.ts +++ b/test/review-fix-range.test.ts @@ -1,5 +1,5 @@ /** - * #3339: /ship Step 9 may re-review only the fix range, but a fix-range pass + * #3339: /ship Step 9 and /review may re-review only the fix range, but a fix-range pass * can never be the last review. The fix base comes from the review log (never * the working tree or the caller) and is the commit whose tree the fixing pass * started from, a narrowed row is never CURRENT and carries no @@ -214,3 +214,75 @@ describe('ship Step 9 instructions', () => { expect(army).toContain('continue to Step 10, and only from a full pass: after a fix-range pass, run one full Step 9 first.'); }); }); + +describe('review Step 3 and Step 5.8 instructions', () => { + const skill = fs.readFileSync(path.join(ROOT, 'review/SKILL.md'), 'utf8'); + const step3 = skill.slice(skill.indexOf('## Step 3: Get the diff'), skill.indexOf('## Step 3.4:')); + const step58 = skill.slice(skill.indexOf('## Step 5.8: Persist Eng Review result'), skill.indexOf('### Report the final review')); + const flat3 = step3.replace(/\s+/g, ' '), flat58 = step58.replace(/\s+/g, ' '); + + test('the first pass starts full and only the pass after an edited pass narrows its DIFF_BASE, never Step 4.8 or other base uses', () => { + const block = step3.slice(step3.indexOf('```bash'), step3.indexOf('```', step3.indexOf('```bash') + 7)); + expect(block).toContain('gstack-review-log --start review\ngit diff "$DIFF_BASE"'); + expect(block).not.toContain('--fix-range'); + expect(flat3).toContain('After an edited pass, start the next with `--start review --fix-range`: it prints REVIEW_START, then `FIX_BASE=`. Wherever this pass sets DIFF_BASE outside Step 4.8, use FIX_BASE instead; Step 4.8 and every other `` use stay full. On refusal, start a full pass.'); + expect(skill.match(/--fix-range/g)).toHaveLength(1); + }); + + test('an edited pass is logged before the repeat, kept out of Step 5.0, and a fix-range pass is never the final review', () => { + const edited = flat58.indexOf("Edited: increment CYCLES once, then fill and save the record below with `converged:false`. Step 5.0 never treats this invocation's records as prior reviews."); + const repeat = flat58.indexOf('Below 3, repeat Steps 3–5 with a new REVIEW_START.'); + const none = flat58.indexOf('No edits: after a full pass, fill the record below. A fix-range pass is never final: repeat Steps 3–5 with a full start.'); + expect(edited).toBeGreaterThan(-1); + expect(repeat).toBeGreaterThan(edited); + expect(none).toBeGreaterThan(repeat); + expect(flat58).toContain("finish only this pass's own token"); + }); +}); + +describe('review Step 3/5.8 order against the real logger', () => { + // /review's order on every pass: core start, Step 4.8 attempt start and finish, fixes in the working tree, core finish; never a commit. + const reviewRow = (fields: Record) => JSON.stringify({ skill: 'review', status: 'clean', issues_found: 0, critical: 0, informational: 0, ...fields }); + function reviewPass(fields: Record, start: string[], fixes?: string) { + const started = bin('gstack-review-log', start); + expect(started.code).toBe(0); + const [token, base] = started.out.split('\n'); + // Step 4.8: each pass's adversarial attempt saves its row before any fix and before the core row. + const attempt = bin('gstack-review-log', ['--start', 'adversarial-review']); + expect(attempt.code).toBe(0); + expect(bin('gstack-review-log', [JSON.stringify({ skill: 'adversarial-review', status: 'clean', source: 'claude', ...clean }), '--finish', attempt.out]).code).toBe(0); + if (fixes) edit(fixes); + expect(bin('gstack-review-log', [reviewRow(fields), '--finish', token!]).code).toBe(0); + return base; + } + + test('a committed first pass narrows the next repeat to the uncommitted fixes, then the chain returns to a full pass', () => { + const reviewed = head(); + reviewPass(fixing, ['--start', 'review'], 'export const a = 2;\n'); + expect(reviewPass(fixing, ['--start', 'review', '--fix-range'], 'export const a = 3;\n')).toBe(`FIX_BASE=${reviewed}`); + expect(gitIn(repo, `diff --name-only ${reviewed}`).trim()).toBe('a.ts'); + const third = narrowStart(); + expect(third.code).toBe(1); + expect(third.err).toContain('started from uncommitted content'); + reviewPass(clean, ['--start', 'review']); + const last = rows().at(-1)!; + expect(last.scope).toBeUndefined(); + expect(reviewFreshness(last, last.wtree)?.status).toBe('CURRENT'); + }, 30_000); + + test('a zero-edit fix-range pass is never CURRENT even if it were logged, so the final record needs a full pass', () => { + reviewPass(fixing, ['--start', 'review'], 'export const a = 2;\n'); + reviewPass(clean, ['--start', 'review', '--fix-range']); + const narrowed = rows().at(-1)!; + expect(narrowed.scope).toBe('fix-range'); + expect(reviewFreshness(narrowed, narrowed.review_binding.range_end_wtree)?.status).toBe('UNVERIFIED'); + }, 30_000); + + test('a first pass over uncommitted work cannot be narrowed', () => { + edit('export const a = 1; // uncommitted\n'); + reviewPass(fixing, ['--start', 'review'], 'export const a = 2;\n'); + const r = narrowStart(); + expect(r.code).toBe(1); + expect(r.err).toContain('started from uncommitted content'); + }, 30_000); +}); diff --git a/test/review-workflow-clarity.test.ts b/test/review-workflow-clarity.test.ts index c68b71d1a..4d9bcdd40 100644 --- a/test/review-workflow-clarity.test.ts +++ b/test/review-workflow-clarity.test.ts @@ -229,7 +229,8 @@ test('review finalization ownership: initialize invocation state and capture the expect(flat).toMatch(/REVIEW_START for this core candidate before reading its diff/i); expectMentions(flat, [['never', 'reading', 'before']], 'flat'); expect(flat).toMatch(/separate PASS_START tokens, not REVIEW_START/); - expectMentions(flat, [['only', 'finishes', 'final']], 'flat'); + expect(flat).toContain('After an edited pass, start the next with `--start review --fix-range`'); + expect(flat).toContain('On refusal, start a full pass.'); }); test('review finalization ownership: late findings use Fix-First before the bounded parent transition', () => { @@ -240,7 +241,7 @@ test('review finalization ownership: late findings use Fix-First before the boun 'Structured approval does not waive advisory/test_stub ASK gates', '## Step 5.8: Persist Eng Review result', 'Edited: increment CYCLES once', - 'No edits: fill the record below', + 'No edits: after a full pass, fill the record below', '### 2. Fill the record', '--finish REVIEW_START', ]; @@ -248,7 +249,7 @@ test('review finalization ownership: late findings use Fix-First before the boun expect(positions.every(position => position >= 0)).toBe(true); expect(positions).toEqual([...positions].sort((a, b) => a - b)); expect(flat).toContain('at most 3 fix cycles'); - expect(flat).toContain('At 3, persist `converged:false`'); + expect(flat).toContain('Edited: increment CYCLES once, then fill and save the record below with `converged:false`.'); const limit = flat.slice(flat.indexOf('At 3,'), flat.indexOf('No edits:')); expect(limit).toMatch(/then stop this invocation/i); expectMentions(limit, [['without', 'summary', 'fourth']], 'limit'); @@ -276,14 +277,14 @@ test('review finalization ownership: required native completion and optional out expect(flat).toContain('`CONVERGED`: true only for a completed zero-edit pass'); }); -test('review finalization ownership: finish only the final core token without log-time capture', () => { +test('review finalization ownership: each logged pass finishes its own core token without log-time capture', () => { const step = skill.slice(skill.indexOf('## Step 5.8: Persist Eng Review result')); const flat = step.replace(/\s+/g, ' '); expect(step.match(/--finish REVIEW_START/g)).toHaveLength(1); expect(step).not.toContain('--start review'); expect(step).not.toContain('--finish PASS_START'); expect(flat).toMatch(/never invent a binding or replace REVIEW_START at log time/i); - expectMentions(flat, [['only', 'finish', 'final']], 'flat'); + expect(flat).toContain("finish only this pass's own token"); expect(step).toContain('"completed":COMPLETED,"converged":CONVERGED,"cycles":CYCLES'); }); diff --git a/test/setup-hook-canonical-paths.test.ts b/test/setup-hook-canonical-paths.test.ts index 8d238185c..0dc955da9 100644 --- a/test/setup-hook-canonical-paths.test.ts +++ b/test/setup-hook-canonical-paths.test.ts @@ -210,8 +210,8 @@ describe('setup: heal-first ordering + visibility', () => { describe('gstack-settings-hook: shared prelude (dedupe key == prune predicate)', () => { test('every bun script call site uses the shared JS prelude concatenation', () => { const codeLines = hookBinSrc.split('\n').filter((l) => !l.trim().startsWith('#')); - const bunCalls = codeLines.filter((l) => l.includes('bun --no-env-file --config=/dev/null -e ')); - const preludeCalls = codeLines.filter((l) => l.includes(`bun --no-env-file --config=/dev/null -e "$_HOOK_JS_PRELUDE"'`)); + const bunCalls = codeLines.filter((l) => l.includes('bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e ')); + const preludeCalls = codeLines.filter((l) => l.includes(`bun --no-env-file --config="\${GSTACK_BUN_CONFIG:-/dev/null}" -e "$_HOOK_JS_PRELUDE"'`)); expect(bunCalls.length).toBeGreaterThanOrEqual(6); expect(preludeCalls.length).toBe(bunCalls.length); });