From 19ed2ac5bbe82f681627fbb097b4992a66929c4c Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 11 Nov 2026 09:59:26 -0800 Subject: [PATCH] Let browser checks and question preferences work from non-Claude hosts' gstack folders (GST-2442) (#4292) * Let browser checks and question preferences work from non-Claude hosts' gstack folders (GST-2442) gstack-browser-ensure looked for Playwright only in the host's gstack folder, which has no package.json or node_modules, so every browser skill on Codex, Cursor and the other non-Claude hosts reported Playwright missing. It now uses the checkout recorded in .source-path, but only when the folder has no package.json of its own and the pointer is a regular file naming an absolute path whose real directory holds gstack's package.json and this helper, neither a link. Anything else keeps the folder, as before, and --check never writes. Setup also copies scripts/one-way-doors.ts and scripts/psychographic-signals.ts into those folders: the question-preference helper loads them at run time, so its --check and --signal-keys failed with "Cannot find module". Co-authored-by: SatriaSuriaAriffin <57977752+SatriaSuriaAriffin@users.noreply.github.com> Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Split a run-on sentence in the copied-installs changelog entry Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: SatriaSuriaAriffin <57977752+SatriaSuriaAriffin@users.noreply.github.com> Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-browser-ensure | 17 ++++ ...stalls-find-browser-and-question-helper.md | 4 + setup | 5 +- test/gstack-browser-ensure.test.ts | 91 +++++++++++++++++++ test/runtime-root-assets.test.ts | 23 +++++ 5 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 changelog.d/copied-installs-find-browser-and-question-helper.md diff --git a/bin/gstack-browser-ensure b/bin/gstack-browser-ensure index 5f8605f4e..0f87e3274 100755 --- a/bin/gstack-browser-ensure +++ b/bin/gstack-browser-ensure @@ -27,6 +27,23 @@ 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; } +# A host runtime root (~/.codex/skills/gstack, ...) has no package.json or +# node_modules; setup records the gstack checkout it came from in .source-path. +# Use that checkout's dependencies only when the pointer is a regular file +# holding an absolute path whose real directory has gstack's own package.json +# and this helper, neither a link. Anything else keeps the root, as before. +if [ ! -e "$ROOT/package.json" ] && [ -f "$ROOT/.source-path" ] && [ ! -L "$ROOT/.source-path" ]; then + _src=$(head -1 "$ROOT/.source-path" | tr -d '\r') + case "$_src" in + /*|[A-Za-z]:/*) _src=$(cd "$_src" 2>/dev/null && pwd -P) || _src="" ;; + *) _src="" ;; + esac + if [ -n "$_src" ] && [ -f "$_src/package.json" ] && [ ! -L "$_src/package.json" ] \ + && [ -f "$_src/bin/gstack-browser-ensure" ] && [ ! -L "$_src/bin/gstack-browser-ensure" ] \ + && grep -Eq '^[[:space:]]*"name":[[:space:]]*"gstack",?[[:space:]]*$' "$_src/package.json"; then + ROOT="$_src" + fi +fi DOCS="https://github.com/garrytan/gstack/blob/main/docs/troubleshooting.md#browser-ensure-failed" CHECK=0 QUIET=0 WITH_DEPS="" for _arg in "$@"; do diff --git a/changelog.d/copied-installs-find-browser-and-question-helper.md b/changelog.d/copied-installs-find-browser-and-question-helper.md new file mode 100644 index 000000000..4bb064216 --- /dev/null +++ b/changelog.d/copied-installs-find-browser-and-question-helper.md @@ -0,0 +1,4 @@ +--- +important: blocker +--- +- **Browser skills and question preferences work again from Codex, Cursor and the other non-Claude hosts' gstack folders.** Setup gives each of those hosts its own gstack folder (`~/.codex/skills/gstack` and the like) with no `package.json` or `node_modules`, and records the gstack checkout it came from in `.source-path`. `gstack-browser-ensure`, which every browser skill runs first, looked for Playwright only in that folder, so it reported `BROWSER_UNAVAILABLE playwright is not installed` even when the checkout had it. It now uses the checkout's dependencies, but only when the folder has no `package.json` of its own and `.source-path` is a regular file naming an absolute path whose real directory holds gstack's own `package.json` and `bin/gstack-browser-ensure`, neither one a link. An empty, stale, relative or linked pointer, or a directory that isn't gstack, is ignored as before, and `--check` still never writes. Setup also now puts `scripts/one-way-doors.ts` and `scripts/psychographic-signals.ts` in those folders. The question-preference helper loads them at run time, so its `--check` and `--signal-keys` stopped with `Cannot find module './scripts/one-way-doors.ts'`. Thanks to @SatriaSuriaAriffin, who found and reported both (#4290). diff --git a/setup b/setup index c969f3d7d..f47cfec69 100755 --- a/setup +++ b/setup @@ -793,7 +793,9 @@ _link_or_copy() { # And crew/lib, which bin and lib import (the crew/ move, GST-1833), plus the # scripts/ modules they import: a root whose bin and lib are copies (Windows) # needs them beside it to start (/review and /ship's review planner imports -# fix-check.ts). +# fix-check.ts), and the two classifiers bin/gstack-question-preference +# imports inside its bun -e (one-way-doors.ts, psychographic-signals.ts), +# which the import walk cannot see. # Pinned by test/runtime-root-assets.test.ts, which also follows those imports. A destination that already exists # as a real entry is left alone: in the repo-local sidecar review/ is a link to # the source, so its files already resolve and must never be linked over. @@ -816,6 +818,7 @@ _link_runtime_dists() { done for f in VERSION scripts/jargon-list.json scripts/question-registry.ts \ scripts/resolvers/fix-check.ts scripts/discover-skills.ts scripts/lib/ci-history.ts \ + scripts/one-way-doors.ts scripts/psychographic-signals.ts \ docs/askuserquestion-split.md docs/askuserquestion-cjk.md docs/test-value-bar.md \ review/checklist.md review/design-checklist.md review/greptile-triage.md review/TODOS-format.md \ plan-devex-review/dx-hall-of-fame.md; do diff --git a/test/gstack-browser-ensure.test.ts b/test/gstack-browser-ensure.test.ts index 31f2a684e..4e4d9a71c 100644 --- a/test/gstack-browser-ensure.test.ts +++ b/test/gstack-browser-ensure.test.ts @@ -57,6 +57,97 @@ function fixture(opts: { playwright?: boolean } = {}) { const lastLine = (s: string) => s.trim().split('\n').at(-1); +// Value: protects=a runtime root with no package.json (~/.codex/skills/gstack and +// the other host roots) probes the dependencies of the gstack checkout its +// .source-path names, and only of a real gstack checkout; fails_when=the +// helper ignores the pointer, or follows an empty, stale, relative, linked or +// non-gstack one, or lets a pointer override a root's own package.json, or +// --check writes anything; why_new=no fixture had a .source-path; +// seam=none; no_claim=a real Chromium launch (Playwright is a stub). +describe('gstack-browser-ensure in a runtime root without package.json', () => { + const playwrightStub = (version: string) => + `module.exports.chromium = { launch: async () => ({ version: () => ${JSON.stringify(version)}, close: async () => {}, newPage: async () => ({ setContent: async () => {}, title: async () => "gstack smoke test" }) }) };\n`; + function runtime(checkoutName = 'checkout') { + const f = fixture({ playwright: false }); + const checkout = path.join(f.base, checkoutName); + write(path.join(checkout, 'package.json'), '{\n "name": "gstack",\n "version": "0.0.0"\n}\n'); + write(path.join(checkout, 'bin/gstack-browser-ensure'), '#!/usr/bin/env bash\n', 0o755); + write(path.join(checkout, 'node_modules/playwright/index.js'), playwrightStub('141.1.0')); + const point = (text: string) => write(path.join(f.root, '.source-path'), text); + const tree = () => walkFiles(f.base).filter(p => !p.startsWith(path.join(f.base, 'stub'))).sort(); + return { ...f, checkout, point, tree }; + } + const walkFiles = (dir: string): string[] => fs.readdirSync(dir, { withFileTypes: true }) + .flatMap(e => e.isDirectory() ? walkFiles(path.join(dir, e.name)) : [path.join(dir, e.name)]); + const unavailable = (root: string) => `BROWSER_UNAVAILABLE playwright is not installed under ${root}/node_modules`; + + for (const [label, name, eol] of [ + ['the recorded checkout', 'checkout', '\n'], + ['a checkout path with spaces', 'gstack checkout', '\n'], + ['a CRLF pointer', 'checkout', '\r\n'], + ] as const) { + test(`--check probes ${label} and writes nothing`, () => { + const f = runtime(name); + f.point(f.checkout + eol); + const files = f.tree(); + const r = f.run(['--check']); + expect(r.status, r.stdout + r.stderr).toBe(0); + expect(lastLine(r.stdout)).toBe('BROWSER_OK node 141.1.0'); + expect(f.tree()).toEqual(files); + expect(f.calls()).toBe(''); + }); + } + + test('without --check a valid pointer is a probe only, and a link to the checkout resolves to it', () => { + const f = runtime(); + const link = path.join(f.base, 'checkout-link'); + fs.symlinkSync(f.checkout, link); + f.point(link + '\n'); + const r = f.run(); + expect(r.status, r.stdout + r.stderr).toBe(0); + expect(lastLine(r.stdout)).toBe('BROWSER_OK node 141.1.0'); + expect(fs.existsSync(path.join(f.root, 'node_modules'))).toBe(false); + expect(f.calls()).toBe(''); + }); + + const refused: Array<[string, (f: ReturnType) => void]> = [ + ['an empty pointer', f => f.point('\n')], + ['a stale pointer', f => f.point(path.join(f.base, 'gone') + '\n')], + ['a relative pointer', f => f.point(path.relative(process.cwd(), f.checkout) + '\n')], + ['a pointer file that is a link', f => { write(path.join(f.base, 'elsewhere'), f.checkout + '\n'); fs.symlinkSync(path.join(f.base, 'elsewhere'), path.join(f.root, '.source-path')); }], + ['a directory whose package.json is not gstack\'s', f => { write(path.join(f.checkout, 'package.json'), '{\n "name": "left-pad"\n}\n'); f.point(f.checkout + '\n'); }], + ['a directory whose package.json is a link', f => { + write(path.join(f.base, 'real-package.json'), fs.readFileSync(path.join(f.checkout, 'package.json'), 'utf8')); + fs.rmSync(path.join(f.checkout, 'package.json')); + fs.symlinkSync(path.join(f.base, 'real-package.json'), path.join(f.checkout, 'package.json')); + f.point(f.checkout + '\n'); + }], + ['a directory without gstack-browser-ensure', f => { fs.rmSync(path.join(f.checkout, 'bin'), { recursive: true }); f.point(f.checkout + '\n'); }], + ]; + for (const [label, setUp] of refused) { + test(`--check ignores ${label} and reports the runtime root, as before`, () => { + const f = runtime(); + setUp(f); + const files = f.tree(); + const r = f.run(['--check']); + expect(r.status, r.stdout + r.stderr).toBe(1); + expect(lastLine(r.stdout)).toBe(unavailable(f.root)); + expect(f.tree()).toEqual(files); + expect(f.calls()).toBe(''); + }); + } + + test('a root with its own package.json keeps its own dependencies over a valid pointer', () => { + const f = runtime(); + write(path.join(f.root, 'package.json'), '{\n "name": "gstack"\n}\n'); + write(path.join(f.root, 'node_modules/playwright/index.js'), playwrightStub('142.2.0')); + f.point(f.checkout + '\n'); + const r = f.run(['--check']); + expect(r.status, r.stdout + r.stderr).toBe(0); + expect(lastLine(r.stdout)).toBe('BROWSER_OK node 142.2.0'); + }); +}); + describe('gstack-browser-ensure', () => { test('--check never installs: unavailable Chromium is BROWSER_UNAVAILABLE, exit 1, with the fix line and code', () => { const f = fixture(); diff --git a/test/runtime-root-assets.test.ts b/test/runtime-root-assets.test.ts index 5e5cb3635..be5647d47 100644 --- a/test/runtime-root-assets.test.ts +++ b/test/runtime-root-assets.test.ts @@ -197,6 +197,29 @@ describe.skipIf(process.platform === 'win32')('runtime roots carry every $GSTACK expect(r.stdout).toContain('--fix-check-prompt'); }); + // Value: protects=the question helper's --check and --signal-keys start from a + // runtime root, linked or copied; fails_when=setup's asset list drops + // scripts/one-way-doors.ts or scripts/psychographic-signals.ts; + // why_new=the helper imports them inside a bash-quoted bun -e, which the + // static import walk above cannot see; seam=none; no_claim=native Windows. + for (const windows of [false, true]) { + test(`the question helper's checks start from a ${windows ? 'copied (Windows)' : 'linked'} runtime root`, () => { + const { root, stderr, status } = buildRoot('codex', windows); + expect(stderr).toBe(''); + expect(status).toBe(0); + const home = path.join(tmp, windows ? 'home-windows' : 'home'); + for (const [args, expected] of [[['--check', 'land-and-deploy-merge'], 'ASK_NORMALLY'], [['--signal-keys'], 'scope-appetite']] as const) { + const r = spawnSync('bash', [path.join(root, 'bin', 'gstack-question-preference'), ...args], { + cwd: tmp, encoding: 'utf8', timeout: 30_000, + env: { PATH: process.env.PATH ?? '/usr/bin:/bin', HOME: home, GSTACK_STATE_ROOT: path.join(home, 'question-state') }, + }); + expect(r.stderr).not.toContain('Cannot find module'); + expect(r.status, r.stdout + r.stderr).toBe(0); + expect(r.stdout).toContain(expected); + } + }); + } + test('a runtime root never gains a whole skill directory beside the rendered copies', () => { const { root } = buildRoot('codex'); for (const dir of ['plan-design-review', 'office-hours']) {