Skip to content

fix(#2557): download openshell binaries during watch mode - #2558

Merged
benoitf merged 3 commits into
mainfrom
agent/2557-openshell-watch-download
Jul 24, 2026
Merged

benoitf merged 3 commits into
mainfrom
agent/2557-openshell-watch-download

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

  • Adds the tsx ./scripts/download.ts step to the openshell extension's watch script so that OpenShell binaries are downloaded for the host platform before starting the vite watcher
  • This ensures that first-time developers who run pnpm installpnpm watch have the binaries available in extensions/openshell/assets/<platform>-<arch>/, which is where the extension looks for them in development mode (per PR fix(openshell): use extension assets folder for bundled binaries in dev mode #2486)
  • The download is idempotent — it checks a version marker file and skips if the correct version is already present

Test plan

  • Run pnpm install in a clean checkout, then pnpm watch
  • Verify that extensions/openshell/assets/<platform>-<arch>/ is populated with openshell binaries
  • Verify that the openshell extension discovers the bundled binaries in dev mode
  • Run pnpm watch a second time and verify the download step is skipped (idempotent)

Closes #2557

Post-script verification

  • Branch is not main/master (agent/2557-openshell-watch-download)
  • Secret scan passed (gitleaks — 06fa8ae0fd6b2fc5a9999b866b3bd703adaf10b8..HEAD)
  • PR body secret scan passed (gitleaks — no-git)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

The openshell extension's watch script only ran vite build --watch,
which does not download the openshell binaries needed for development.
First-time developers who ran pnpm install followed by pnpm watch
would be missing the binaries in the assets folder, causing the
extension to fall back to system PATH lookups.

Add tsx ./scripts/download.ts to the watch script so that binaries
are downloaded for the host platform before starting the vite
watcher. The download is idempotent and skips if binaries are
already present at the correct version.

Note: unit tests and linters could not be run because node_modules
are not installed in the sandbox environment. The change is a
single-line script modification in package.json with no production
code impact.

Closes #2557
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner July 23, 2026 07:01
@fullsend-ai-coder
fullsend-ai-coder Bot requested review from MarsKubeX and gastoner and removed request for a team July 23, 2026 07:01

@MarsKubeX MarsKubeX left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For watch command that developers run repeatedly during iteration, downloading both openshell and image-builder every invocation adds unnecessary startup latency. The download is idempotent (it checks a version marker), but it still resolves the GitHub release for both components each time.

Consider whether the watch script should scope the download to just the openshell component:

"watch": "tsx ./scripts/download.ts --component openshell && vite build --watch"

@fbricon

fbricon commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

The openshell binary should be considered as a dependency so it makes more sense to install it as part of the "pnpm install" process, as @benoitf mentioned in #2557 (comment)

@benoitf

benoitf commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

/fs-code

@deboer-tim

Copy link
Copy Markdown
Contributor

/fs-fix

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 1:05 PM UTC · Completed 1:11 PM UTC
Commit: 9592166 · View workflow run →

The watch script previously ran `tsx ./scripts/download.ts` without
the --component flag, which downloads both openshell and image-builder
binaries on every invocation. Since watch is run repeatedly during
development, this adds unnecessary startup latency from resolving the
GitHub release for image-builder.

Scope the download to `--component openshell` to match the dedicated
`download` script and avoid the extra network round-trip.

Addresses review feedback on #2558
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (human-triggered)

Scoped the watch script download to --component openshell only, addressing the review feedback about unnecessary startup latency from downloading both components.

Fixed (1):

  1. watch script downloads both openshell and image-builder, adding unnecessary latency (extensions/openshell/package.json): Added --component openshell flag to the watch script's download step so it only downloads the openshell binary, skipping the image-builder download. This matches the pattern used by the dedicated download script and avoids an extra GitHub release resolution on every watch invocation.

Tests: passed

Updated by fullsend fix agent

@benoitf

benoitf commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

add it as a post-install step, do not use --component

@benoitf

benoitf commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

/fs-fix

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 1:18 PM UTC · Completed 1:25 PM UTC
Commit: 3bd0c6c · View workflow run →

Move the binary download from the watch script to a postinstall hook
so openshell binaries are fetched during pnpm install, treating them
as a project dependency. Remove --component flag per reviewer request
so all components are downloaded.

Addresses review feedback on #2558
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 2 (human-triggered)

Addressed both review findings from benoitf: moved openshell binary download from the watch script to a postinstall hook in package.json, and removed the --component flag so all components are downloaded during pnpm install.

Fixed (2):

  1. move download to postinstall step instead of watch script (extensions/openshell/package.json): Moved the binary download from the watch script to a postinstall hook so openshell binaries are fetched during pnpm install, treating them as a project dependency rather than a dev-time watch concern.
  2. do not use --component flag (extensions/openshell/package.json): Removed the --component openshell flag from the download invocation per reviewer request. The postinstall script now runs tsx ./scripts/download.ts without --component, downloading all components (openshell and image-builder).

Tests: passed

Updated by fullsend fix agent

@benoitf
benoitf merged commit 85af848 into main Jul 24, 2026
53 of 54 checks passed
@benoitf
benoitf deleted the agent/2557-openshell-watch-download branch July 24, 2026 07:20
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.

Improve first time developer experience

5 participants