fix(mux): persist install under $HOME and reuse it when the version matches - #1095
fix(mux): persist install under $HOME and reuse it when the version matches#1095ibetitsmike wants to merge 3 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2a495e74a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…versions through the package manager Round 2 of review on #1095: - resolve install_version through npm/pnpm/bun view so registry auth applies - default log_path into the module root logs/ directory - split run.sh into scripts/install.sh.tftpl and scripts/start.sh.tftpl run by coder-utils - keep a pre-1.6.0 copy at /tmp/mux working for install=false and use_cached - keep a single install layout so the installed version is read from the active one - normalize an empty install_version to latest before resolving it - apply before asserting on the token-bearing start script
|
@codex review
|
There was a problem hiding this comment.
💡 Codex Review
Move the bootstrapped Node runtime into the module root
On any workspace without node, this lifecycle still downloads persistent, module-managed runtime data into the ad-hoc $HOME/.local/share/coder-mux directory. Because this PR reworks the existing module's lifecycle and otherwise establishes the module root, leaving the runtime here keeps troubleshooting and cleanup split across locations; store it beneath $HOME/.coder-modules/coder/mux and update the start-side lookup accordingly.
AGENTS.md reference: AGENTS.md:L44-L47
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
bun pm view only exists since Bun 1.2.15. Older releases print their usage to stdout and exit 1, so the warm-start check compared the installed version against 'Learn more about these at https://bun.sh/docs/cli/pm' and reinstalled on every start. Check the exit status and fall back to npm view (shares .npmrc credentials) and then to the plain registry lookup. Adds a container test on oven/bun:1.2.14-alpine covering the reuse with and without npm present.
|
Follow-ups from the second review round are filed as #1096 (base64-encode
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Keep the bootstrapped runtime inside the module root
When the workspace image lacks Node.js, this stores a module-managed persistent runtime under ~/.local/share/coder-mux, leaving runtime data outside the module's new $HOME/.coder-modules/coder/mux root. Because the module chooses and manages this location, it is not tool-defined state; place it beneath the module root and have both lifecycle scripts resolve the same path.
AGENTS.md reference: AGENTS.md:L43-L47
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
reviewing this. |
## Description The module installs `mux@<install_version>`, but the npm `mux` package is only a compatibility shim (`packages/mux-compat` in coder/xum). Its single dependency is an exact-pinned `@coder/xum` of the same version string, and the two packages are published independently. `mux` usually lands on the registry several minutes before its own `@coder/xum` dependency, so any workspace that starts inside that window resolves `mux@next` and then fails with: ```text No version matching "<ver>" found for specifier "@coder/xum" ❌ Failed to install mux via bun ``` The `coder_script` exits 1, the workspace reports a start error, and the Mux app goes unhealthy. This PR installs `@coder/xum` directly. It is the package that actually contains Mux, so there is no cross-package version coupling left to race against: whatever version or dist-tag resolves is fully installable on its own. `@coder/xum` ships both the `mux` and `xum` bins (`node_modules/.bin/mux -> ../@coder/xum/dist/cli/index.js`), so the existing binary probe, the `$INSTALL_PREFIX/mux` symlink, and everything that calls `mux server` are unchanged. No CI change on the publishing side is needed. Changes in `registry/coder/modules/mux`: - `run.sh`: `PKG="@coder/xum"` for the npm/pnpm/bun path; the no-package-manager tarball fallback now fetches metadata from `<registry>/@coder%2Fxum/<version>` (scoped names are URL-encoded in registry metadata paths) and constructs `<registry>/@coder/xum/-/xum-<version>.tgz`, which matches the registry's `dist.tarball` exactly. Error and progress messages name the package that is being installed. - `main.tf`: `install_version` and `registry_url` descriptions name `@coder/xum`. Default `install_prefix` moves from `/tmp/mux` to `$HOME/.coder-modules/coder/mux` and default `log_path` from `/tmp/mux.log` to `$HOME/.coder-modules/coder/mux/logs/mux.log`, the per-module root from AGENTS.md (Module Data Layout), so restarts that clear `/tmp` no longer discard the install or the diagnostics. `run.sh` already creates both directories with `mkdir -p` before writing; no other path in it assumed `/tmp`. The Node.js runtime that `run.sh` bootstraps on images without `node` also moves from `~/.local/share/coder-mux` to the same root, so all module data shares one directory. - `README.md`: documents the package switch and the new default paths; fixes the "Pin Version" example, which claimed the default `install_version` is `latest` while `main.tf` defaults to `next`, and pins a version that exists on `@coder/xum`; links the renamed upstream repository (coder/mux is now coder/xum). - `mux.tftest.hcl`: new `installs_coder_xum_package` run asserting the rendered script installs `@coder/xum` and uses the scoped metadata/tarball URLs (verified red/green by flipping `PKG` back to `mux`); new `default_paths_under_module_root` and `custom_install_prefix_and_log_path` runs assert the default and overridden paths (red/green by reverting the default to `/tmp/mux`); the trailing-slash registry assertion follows the new metadata URL. - `main.test.ts`: expected lines and the fake-`mux` fixtures follow the new default paths (`/root/.coder-modules/coder/mux` inside the test containers). The two install tests now remove their containers inside the test instead of leaving them to the global `afterAll` hook in `setup.ts`: that hook has a 5 second timeout, and deleting a container holding a full `@coder/xum` install (about 800 MB, 48k files) took longer than that on the CI runner under bun 1.4.1, failing the run after every test had passed. ### Breaking change `@coder/xum` only exists on npm from `0.28.2-next.24` onward (stable: `0.28.3`, `0.28.4`); older releases were published under `mux` only. The default (`next`), `latest`, and any pin of `0.28.3` or newer are unaffected. Templates that pin `install_version` to an older version will fail to install and need to move to `0.28.3` or newer. Private mirrors configured via `registry_url` must serve the scoped `@coder/xum` package. The default `install_prefix` and `log_path` also move out of `/tmp` (see above), and a bootstrapped Node.js runtime is downloaded once more into the new location: templates that relied on the old defaults, for example `install = false` with a binary pre-installed at `/tmp/mux/mux` or tooling that reads `/tmp/mux.log`, must set `install_prefix` and `log_path` explicitly or move to the new locations. Because existing valid configurations can stop working, this is released as a major bump. Note: this overlaps with #1095, which also reworks the install path of this module. Whichever merges second needs a rebase; the package switch is a small, self-contained change to port. ## Type of Change - [ ] New module - [ ] New template - [x] Bug fix - [ ] Feature/enhancement - [ ] Documentation - [ ] Other ## Module Information **Path:** `registry/coder/modules/mux` **New version:** `v2.0.0` (from `v1.5.0`, bumped with `.github/scripts/version-bump.sh major`) **Breaking change:** [x] Yes [ ] No ## Testing & Validation - [x] Tests pass (`bun test`) - [x] Code formatted (`bun fmt`) - [x] Changes tested locally - `terraform test -verbose` in the module: 24 passed, 0 failed. - `bun test main.test.ts` in the module (Docker, bun 1.2.15 and 1.4.1): 8 passed, 0 failed. `runs with npm present` performs a real `npm install @coder/xum@next --ignore-scripts` in `node:20-alpine`; `runs with default` exercises the scoped tarball fallback in `alpine/curl`. - `scripts/terraform_validate.sh`, `scripts/shellcheck_validate.sh`, `scripts/validate_set_u_order.sh`, `bun fmt:ci`, and `cmd/readmevalidation` all clean. - Manual end-to-end check in a scratch directory: `bun add --ignore-scripts @coder/xum@next` resolved `0.28.4-next.3.g2a1b5b267`; `node_modules/.bin/mux --version` printed `v0.28.5-nightly.5-3-g2a1b5b267-dirty (2a1b5b267)`; `mux server --port 4321` answered `GET /health` with 200 within 3 seconds. Skipping `@coder/xum`'s `postinstall` (`sh scripts/postinstall.sh`) is safe here: the script only rebuilds node-pty/duckdb for Electron and exits early in server mode or when installed as a dependency. - Registry check: `GET https://registry.npmjs.org/@coder%2Fxum/next` returns 200 and its `dist.tarball` equals the URL the fallback constructs. ## Related Issues None > Xum acted on behalf of @ibetitsmike for this PR. Generated with Xum using Claude.
2 participants
Summary
Every workspace start reinstalled Mux from npm into
/tmp/mux, because/tmpis wiped between starts and the rendered start script always took the reinstall branch (use_cacheddefaults tofalse, so[ "false" != true ]forcedbun add mux@nexteven when a copy was present). On the dogfood template this cost about 50 s per start and left a 502 window right after a restart (workspace build 663: container up 18:06:01Z, agent ready 18:06:53Z, withbun addalone reporting 17 s).This changes the
coder/muxmodule so the install is persistent and version-aware, and moves its lifecycle ontocoder-utils:install_prefixdefaults to$HOME/.coder-modules/coder/muxandlog_pathto$HOME/.coder-modules/coder/mux/logs/mux.log(the per-module root from AGENTS.md) instead of/tmp.muxpackage version and resolvesinstall_versionthrough the selected package manager (npm view/pnpm view/bun pm view ... --registry <registry_url>), so private registry credentials from npm/pnpm/bun config apply.bun pm viewonly exists since Bun 1.2.15 (older releases print their usage to stdout and exit 1), so on older bun the lookup falls back tonpm viewwhen npm is present and then to the plain registry lookup. When the versions match it skips the install; when they differ, cannot be resolved, or nothing is installed, it installs as before. The tarball fallback (no package manager) keeps its unauthenticatedcurllookup, matching how it downloads the tarball.npm/or.mux-package/), so the installed version is always read from the package behind the activemuxsymlink.install_versionis normalized tolatestonce in Terraform and feeds the lookup, the package-manager install and the tarball path alike.run.shis split intoscripts/install.sh.tftplandscripts/start.sh.tftpl, rendered at plan time and orchestrated bycoder-utils(module_directory = "$HOME/.coder-modules/coder/<slug>", so several instances on one agent do not share scripts, sync names or logs). The module now exposes thescriptsoutput. This raises the provider constraint to>= 2.13and, like the other coder-utils modules, needs acoderCLI withcoder exp syncin the workspace.install_prefix, a copy pre-installed at the pre-1.6.0 default/tmp/muxis linked into the new prefix wheninstall = falseoruse_cached = true, so existing templates keep starting.install = false(offline) otherwise behaves as before. Version bumped 1.5.0 to 1.6.0.Verification
Rendered the install script with Terraform and ran it in a dogfood workspace (
package_manager = "bun",install_version = "next", empty bun cache for the cold run):bun addinstalls 653 packagesmux@0.28.4-next.3.g3c06630a7 is already installed ...; skipping install(viabun pm view)npm viewfallback)For comparison, the previous head (935ff33) on bun 1.2.14 printed
does not match mux@next (Learn more about these at https://bun.sh/docs/cli/pm); reinstallingand reinstalled on every run.The warm path is dominated by the registry lookup (about 1 s for
bun pm viewornpm view). Also checked: an older pinnedinstall_versionfollowed by anextrun printsdoes not match mux@next (...); reinstallingand updates; an unreachable registry reports(unresolved)and falls through to the install path, which fails the same way it does today; the start script launches the binary from the prefix and logs intologs/mux.log.terraform test: 26 passed (new: default prefix persists, custom prefix, legacy/tmp/muxfallback rendered only for the default prefix, empty version means latest, default log path, coder-utils run order). Runs asserting on the token-bearing start script usecommand = apply.bun test main.test.ts: 10 passed. Both install tests (node:20-alpinevia npm,alpine/curlvia tarball) run the install script twice and assert the second run skips; a new test runs a pre-1.6.0 copy from/tmp/muxwithinstall = false; launcher tests read the default log path andlogs/install.log. A new test onoven/bun:1.2.14-alpine(forcedpackage_manager = "bun") installs once and asserts the next two starts skip, first without npm and then with npm added; it fails against 935ff33.terraform fmt, README validation clean; shellcheck on the rendered scripts reports only SC2050 (constant expressions inherent to rendered templates).Generated with Xum using Claude.