Skip to content

fix(setup): link shared runtime libraries#2262

Open
fedster99 wants to merge 2 commits into
garrytan:mainfrom
fedster99:agent/link-gstack-runtime-lib
Open

fix(setup): link shared runtime libraries#2262
fedster99 wants to merge 2 commits into
garrytan:mainfrom
fedster99:agent/link-gstack-runtime-lib

Conversation

@fedster99

Copy link
Copy Markdown

What changed

  • Link lib/ beside bin/ in the shared agent sidecar and the Codex, Factory, OpenCode, and Kiro runtime roots.
  • Add setup validation coverage for every affected runtime root.

Why

Several scripts in bin/ import shared modules through ../lib. Minimal runtime installs exposed bin/ without lib/, so commands such as gstack-learnings-log failed after a fresh Codex setup with Cannot find module .../lib/jsonl-store.ts.

Keeping the two directories together makes the generated runtime self-contained and prevents a later gstack upgrade from restoring the broken layout.

Impact

Fresh and upgraded installs can run logging, memory, redaction, and other shared-library-backed commands from their host-specific runtime root. Unix symlink and Windows copy behavior both continue to use _link_or_copy.

Validation

  • bash -n setup
  • Generated Codex fixtures, then ran the five affected setup assertions on Bun 1.3.14: 5 passed
  • bun test test/setup-windows-fallback.test.ts: 8 passed
  • Fresh temporary-HOME install with ./setup --host codex --no-prefix
  • End-to-end gstack-learnings-log and gstack-learnings-search smoke passed from that installed runtime

The broader free-test command reached an unrelated environment-dependent timeout in global-discover.test.ts; no changed setup/runtime test failed.

@trunk-io

trunk-io Bot commented Jul 14, 2026

Copy link
Copy Markdown

Merging to main in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@fedster99
fedster99 marked this pull request as ready for review July 14, 2026 21:45
@time-attack

time-attack commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

@fedster99 Add tests that run a command requiring the installed sibling library across every affected host root, including Windows copy behavior.

Address review: end-to-end coverage that gstack-learnings-log (which
imports ../lib/jsonl-store.ts) actually executes from each installed
runtime root — agents sidecar, Codex, Factory, OpenCode, and Kiro —
built by the real setup shell code, in both symlink (IS_WINDOWS=0) and
Windows copy (IS_WINDOWS=1) modes. A negative control proves the probe
fails on the pre-fix bin-without-lib layout.

Verified: all 10 positive cells fail against upstream/main's setup and
pass with the fix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@fedster99

Copy link
Copy Markdown
Author

Done in f412cf1 — added test/setup-runtime-lib-command.test.ts, which goes beyond static validation and executes a lib-importing command from every affected host root:

  • Builds each runtime root (agents sidecar, Codex, Factory, OpenCode, Kiro) in a sandbox using the real setup shell code (the extracted _link_or_copy + each root-builder function/block).
  • Runs gstack-learnings-log — the exact command from the bug report, which imports $SCRIPT_DIR/../lib/jsonl-store.ts via bun — from the installed root and asserts it exits 0 and appends the learning.
  • Covers Windows copy behavior: every root is exercised twice, IS_WINDOWS=0 (symlink install, asserts lib is a symlink) and IS_WINDOWS=1 (copy install, asserts lib is a real directory copy), following the setup-windows-fallback.test.ts precedent (skipped on actual win32 runners where ln -snf silently degrades).
  • Includes a negative control: a root with bin/ but no lib/ (the pre-fix layout) must fail on the ../lib import — so the positive cells can't pass vacuously.

Verified the tests detect the regression: against upstream/main's setup (pre-fix), all 10 positive cells fail; with this PR's fix, 11/11 pass in ~3s.

@marcusLewi

marcusLewi commented Jul 17, 2026 via email

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants