mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
fix(setup): keep a contributor checkout's dev packages on bun.lock's versions (GSTA-97)
/ship Step 11: a production install over a full install reports "no changes" and leaves devDependencies at old versions after bun.lock moves, so the session updater's ./setup left contributors on stale TypeScript, Prettier and Agent SDK. A checkout with node_modules/typescript gets the full frozen install. The import-closure scan now follows shipped code into test/helpers (gstack-model-benchmark loads the providers). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
b5b8820df8
commit
9773f6844f
4 files changed
+33
-8
No files matched your search
+3
-3
@@ -7,13 +7,13 @@
|
||||
### Changed
|
||||
- **`setup`**: every dependency install (before the build, the Codex render and each host render) goes through one `install_runtime_deps` function: `bun install --production --frozen-lockfile`. Bun treats `--production` as frozen, so when `bun.lock` is behind `package.json` (a fork or a local edit) it refreshes the lockfile with `bun install --lockfile-only` and installs production-only again. **`bin/gstack-browser-ensure`** installs the same way when `node_modules` is missing, and its failure line suggests the production install rather than a full `bun install`.
|
||||
- **`package.json`**: `@anthropic-ai/sdk` (loaded by `gstack-distill-free-text`, /plan-tune's dream cycle) and `xterm` and `xterm-addon-fit` (copied into the browser extension by `bun run build`) move to `dependencies`, because a user install runs them. The Claude Agent SDK, TypeScript, Prettier and `@types/bun` stay dev-only.
|
||||
- **Contributors and CI keep the full install.** Bun never removes packages that are already installed, so a full `bun install` survives any later `./setup`. `bin/dev-setup` now always runs `bun install` (it skipped it when `node_modules` existed, which would have left a checkout first set up by `./setup` without the dev toolchain). CI workflows already run `bun install --frozen-lockfile` themselves. CONTRIBUTING.md says which command installs what.
|
||||
- **Contributors and CI keep the full install.** `./setup` treats a checkout that already has the dev toolchain (`node_modules/typescript`) as a contributor's and runs the full `bun install --frozen-lockfile` there, so dev packages keep following `bun.lock` after a pull (a production install would report "no changes" and leave them at old versions). `bin/dev-setup` now always runs `bun install` (it skipped it when `node_modules` existed, which would have left a checkout first set up by `./setup` without the dev toolchain). CI workflows already run `bun install --frozen-lockfile` themselves. CONTRIBUTING.md says which command installs what.
|
||||
|
||||
### If you already have gstack installed
|
||||
- An existing install keeps its dev dependencies, because `./setup` never removes packages. To get the space back: `cd ~/.claude/skills/gstack && rm -rf node_modules && bun install --production --frozen-lockfile`. Don't do this in a checkout you develop gstack in.
|
||||
- An existing install keeps its dev dependencies and keeps getting the full install, because it already has the dev toolchain. To get the space back: `cd ~/.claude/skills/gstack && rm -rf node_modules && bun install --production --frozen-lockfile`. Don't do this in a checkout you develop gstack in.
|
||||
|
||||
### Tests
|
||||
- `test/setup-production-deps.test.ts`: `install_runtime_deps`, sliced out of `setup` and run against a recording `bun` that models Bun's frozen `--production`, passes `--production` on the frozen-lockfile call, refreshes a stale lockfile before retrying, and stops setup when the refresh fails. Run with the real Bun on local `file:` packages: a fresh install leaves out devDependencies, a later production install keeps the ones a full install added, and a stale lockfile is refreshed while the install stays production-only. No other line in `setup` calls `bun install` (the pre-change line is the negative control). `bin/dev-setup`'s install block, run over an existing `node_modules`, still calls plain `bun install`. A scan of every shipped file (`bin`, `lib`, `browse/src`, `design/src`, `make-pdf/src`, `hosts`, `extension`, the resolvers, and the scripts `setup` and the build call by path) finds no import or `require` of a package outside `dependencies`. Modules the shipped code reaches by relative import (gen-skill-docs' `scripts/` helpers) are scanned too. Negative controls: a planted Agent SDK import is caught, including one split over several lines and a re-export; a type-only import is not; and moving `@anthropic-ai/sdk` back to dev flags `gstack-distill-free-text`. The packages `bun run build` copies out of `node_modules` are production dependencies.
|
||||
- `test/setup-production-deps.test.ts`: `install_runtime_deps`, sliced out of `setup` and run against a recording `bun` that models Bun's frozen `--production`, passes `--production` on the frozen-lockfile call, refreshes a stale lockfile before retrying, and stops setup when the refresh fails. A checkout with the dev toolchain gets the full frozen install. Run with the real Bun on local `file:` packages: a fresh install leaves out devDependencies, a later production install keeps the ones a full install added, and a stale lockfile is refreshed while the install stays production-only. No other line in `setup` calls `bun install` (the pre-change line is the negative control). `bin/dev-setup`'s install block, run over an existing `node_modules`, still calls plain `bun install`. A scan of every shipped file (`bin`, `lib`, `browse/src`, `design/src`, `make-pdf/src`, `hosts`, `extension`, the resolvers, and the scripts `setup` and the build call by path) finds no import or `require` of a package outside `dependencies`. Modules the shipped code reaches by relative import (gen-skill-docs' `scripts/` helpers, and the `test/helpers/providers` modules `gstack-model-benchmark` loads) are scanned too. Negative controls: a planted Agent SDK import is caught, including one split over several lines and a re-export; a type-only import is not; and moving `@anthropic-ai/sdk` back to dev flags `gstack-distill-free-text`. The packages `bun run build` copies out of `node_modules` are production dependencies.
|
||||
- `test/gstack-browser-ensure.test.ts`: with `node_modules` missing, the preflight runs the frozen production install, refreshes a stale lockfile before retrying, and ends a failed install in `BROWSER_UNAVAILABLE` with a production-install fix line.
|
||||
- The install fixtures' stub `bun` (`test/helpers/install-fixture.ts`, `test/helpers/claude-overlay-fixture.ts`, `test/team-mode.test.ts`) now matches `install --production --frozen-lockfile`, so a setup that dropped the flag would reach the real `bun` instead of the stub.
|
||||
|
||||
|
||||
+1
-1
@@ -14,7 +14,7 @@ bun install # install dependencies
|
||||
bin/dev-setup # activate dev mode
|
||||
```
|
||||
|
||||
`./setup` on its own installs production dependencies only. `bun install` (and `bin/dev-setup`, which runs it) adds the dev toolchain the tests and evals need: the Claude Agent SDK, TypeScript and Prettier. A later `./setup` never removes them.
|
||||
`./setup` on its own installs production dependencies only. `bun install` (and `bin/dev-setup`, which runs it) adds the dev toolchain the tests and evals need: the Claude Agent SDK, TypeScript and Prettier. Once they are installed, a later `./setup` runs the full install too, so they stay at `bun.lock`'s versions.
|
||||
|
||||
> **Full clone vs shallow.** The README's user-facing install uses `--depth 1` for speed. As a contributor, use a full clone (no `--depth` flag) — you'll need history for `git log`, `git blame`, `git bisect`, and reviewing PRs against earlier versions. If you already have a `--depth 1` clone from following the README, promote it to a full clone with `git fetch --unshallow`.
|
||||
|
||||
|
||||
@@ -1718,7 +1718,14 @@ bun_cmd() {
|
||||
# `bun install` (CONTRIBUTING.md, bin/dev-setup, CI) survives a later ./setup.
|
||||
# Bun's --production implies a frozen lockfile, so when bun.lock is out of date
|
||||
# (a fork or a local edit) refresh it first, then install production-only again.
|
||||
# A checkout that already has the dev toolchain (node_modules/typescript) is a
|
||||
# contributor's: a production install would leave its dev packages at old
|
||||
# versions after bun.lock moves, so it gets the full install instead.
|
||||
install_runtime_deps() {
|
||||
if [ -d node_modules/typescript ]; then
|
||||
bun_cmd install --frozen-lockfile 2>/dev/null || bun_cmd install
|
||||
return
|
||||
fi
|
||||
bun_cmd install --production --frozen-lockfile 2>/dev/null || { bun_cmd install --lockfile-only && bun_cmd install --production --frozen-lockfile; }
|
||||
}
|
||||
|
||||
|
||||
@@ -54,12 +54,12 @@ describe.skipIf(process.platform === 'win32')('setup installs production depende
|
||||
test('install_runtime_deps passes --production with the frozen lockfile, and refreshes a stale lockfile before installing production-only again', () => {
|
||||
const base = tempBase();
|
||||
const { bun, calls } = stubBun(base);
|
||||
expect(runBashScript(script(bun, 'install_runtime_deps'), { env: { ...process.env, FROZEN_RC: '0' } }).status).toBe(0);
|
||||
expect(runBashScript(script(bun, 'install_runtime_deps'), { cwd: base, env: { ...process.env, FROZEN_RC: '0' } }).status).toBe(0);
|
||||
expect(calls()).toEqual(['install --production --frozen-lockfile']);
|
||||
|
||||
const base2 = tempBase();
|
||||
const stub2 = stubBun(base2);
|
||||
expect(runBashScript(script(stub2.bun, 'install_runtime_deps'), { env: { ...process.env, FROZEN_RC: '1' } }).status).toBe(0);
|
||||
expect(runBashScript(script(stub2.bun, 'install_runtime_deps'), { cwd: base2, env: { ...process.env, FROZEN_RC: '1' } }).status).toBe(0);
|
||||
expect(stub2.calls()).toEqual(['install --production --frozen-lockfile', 'install --lockfile-only', 'install --production --frozen-lockfile']);
|
||||
|
||||
// Generated by /ship Step 7 coverage audit (GSTA-97).
|
||||
@@ -68,12 +68,29 @@ describe.skipIf(process.platform === 'win32')('setup installs production depende
|
||||
// why_new=both rows above end in a successful install; seam=none; no_claim=setup's own subshell wiring around the call
|
||||
const base3 = tempBase();
|
||||
const stub3 = stubBun(base3);
|
||||
const failed = runBashScript(script(stub3.bun, 'install_runtime_deps\necho AFTER'), { env: { ...process.env, FROZEN_RC: '1', FALLBACK_RC: '3' } });
|
||||
const failed = runBashScript(script(stub3.bun, 'install_runtime_deps\necho AFTER'), { cwd: base3, env: { ...process.env, FROZEN_RC: '1', FALLBACK_RC: '3' } });
|
||||
expect(failed.status).toBe(3);
|
||||
expect(failed.stdout).not.toContain('AFTER');
|
||||
expect(stub3.calls()).toEqual(['install --production --frozen-lockfile', 'install --lockfile-only']);
|
||||
});
|
||||
|
||||
// Value: protects=a contributor checkout (dev toolchain present) keeps its dev packages at bun.lock's versions after ./setup;
|
||||
// fails_when=setup runs the production install there, which Bun reports as "no changes" and leaves old dev versions;
|
||||
// why_new=every other row starts without node_modules; seam=none; no_claim=Bun's own update behavior (stub bun)
|
||||
test('a checkout with the dev toolchain gets the full frozen install, falling back to a plain install', () => {
|
||||
const base = tempBase();
|
||||
fs.mkdirSync(path.join(base, 'node_modules', 'typescript'), { recursive: true });
|
||||
const { bun, calls } = stubBun(base);
|
||||
expect(runBashScript(script(bun, 'install_runtime_deps'), { cwd: base, env: { ...process.env, FROZEN_RC: '0' } }).status).toBe(0);
|
||||
expect(calls()).toEqual(['install --frozen-lockfile']);
|
||||
|
||||
const base2 = tempBase();
|
||||
fs.mkdirSync(path.join(base2, 'node_modules', 'typescript'), { recursive: true });
|
||||
const stub2 = stubBun(base2);
|
||||
expect(runBashScript(script(stub2.bun, 'install_runtime_deps'), { cwd: base2, env: { ...process.env, FROZEN_RC: '1' } }).status).toBe(0);
|
||||
expect(stub2.calls()).toEqual(['install --frozen-lockfile', 'install']);
|
||||
});
|
||||
|
||||
test('every bun install in setup goes through install_runtime_deps', () => {
|
||||
const body = setup.replace(sliceFunction(setup, 'install_runtime_deps'), '');
|
||||
const directInstall = /(?:^|[\s;&|(])(?:bun_cmd|"?\$\{?BUN_CMD\}?"?|bun)\s+install\b/;
|
||||
@@ -266,12 +283,13 @@ describe('setup and runtime need only production dependencies', () => {
|
||||
for (const m of text.matchAll(relSpec)) {
|
||||
const spec = path.posix.normalize(path.posix.join(path.posix.dirname(rel), m[1] ?? m[2]));
|
||||
const target = [spec, `${spec}.ts`, `${spec}.js`, `${spec}/index.ts`].find((c) => fs.existsSync(path.join(ROOT, c)) && fs.statSync(path.join(ROOT, c)).isFile());
|
||||
if (!target || target.startsWith('..') || target in files || target in reached || /(^|\/)(test|node_modules)\//.test(target) || !/\.(ts|js|mjs)$/.test(target)) continue;
|
||||
if (!target || target.startsWith('..') || target in files || target in reached || /(^|\/)node_modules\//.test(target) || (/(^|\/)test\//.test(target) && !target.startsWith('test/helpers/')) || !/\.(ts|js|mjs)$/.test(target)) continue;
|
||||
reached[target] = fs.readFileSync(path.join(ROOT, target), 'utf-8');
|
||||
queue.push(target);
|
||||
}
|
||||
}
|
||||
expect(Object.keys(reached)).toContain('scripts/discover-skills.ts');
|
||||
expect(Object.keys(reached)).toContain('test/helpers/providers/claude.ts');
|
||||
expect(nonProductionImports(PKG, reached)).toEqual([]);
|
||||
});
|
||||
|
||||
|
||||
Reference in new issue
Block a user