fleet: put the bootstrapped Node on PANE_PATH (#1256) #1258

2026-08-16T20:02:47Z

Ghost commented

2026-08-16 20:02:47 +00:00

Closes the second blocker on #1256.

What was broken

On a host with no system Node, tools/install.sh bootstraps one into
~/.mosaic/node/current/bin and records it in ~/.profile. The fleet unit runs
env -i ... bash --noprofile --norc, so ~/.profile is never read — that is
deliberate — and _build_runtime_bin_prefix did not name the directory itself.

Runtime binaries are #!/usr/bin/env node. So the pane resolved mosaic, then
died on env: 'node': No such file or directory, after an install that reported
success. Measured on a greenfield VM.

The existing npm config get prefix branch cannot cover this. It reports a
package prefix (~/.npm-global), never a Node runtime directory — the gap does
not depend on whether that branch runs.

The change

One entry added to the candidate list in _build_runtime_bin_prefix, positioned
after MOSAIC_RUNTIME_BIN and ahead of the npm probe, so the bootstrapped runtime
wins on a host that has both — that is the one the installer verified — while an
explicit MOSAIC_RUNTIME_BIN still outranks it.

That position is load-bearing, not stylistic. Ahead of MOSAIC_RUNTIME_BIN would
break an explicit override. Behind the npm probe would break a specific host: where
a system Node exists, npm config get prefix can resolve to /usr and contribute
/usr/bin, and the bootstrapped runtime would then lose to the system one. The
stated intent is true at exactly one position in the list.

The test

test-start-agent-session.sh gains a case that asserts the property, not the
string: it runs the pane for real with a #!/usr/bin/env node mosaic and a
node shim that records that it ran, then requires the marker. A PATH-substring
assertion would pass on a fix that put the directory in the wrong position, and
would keep passing if the pane later stopped running for an unrelated reason.

npm is modelled ALIVE with its prefix at ~/.npm-global, which is the stronger
claim: the branch executes and still contributes nothing. An earlier draft
modelled npm as absent — true on a real bootstrap host, but it refused to run
anywhere npm is in the system path, and it would have proven only that a dead
branch supplies nothing.

unfixed launcher, this test:  FAIL — pane could not execute a Node-shebang runtime
fixed launcher, full suite:   ok - start-agent-session generated environment boundary, rc=0

Red proven against the unfixed launcher with the final assertion in place, not
just with an earlier draft of it.

Note for the reviewer

I need a second pair of eyes on the ordering choice. Putting the bootstrap
directory ahead of MOSAIC_RUNTIME_BIN's neighbours was not considered — it goes
after the explicit override and before the probes, which I believe is right, but
it is the one decision here that is a judgement call rather than a measurement.

Closes the second blocker on #1256. ## What was broken On a host with no system Node, `tools/install.sh` bootstraps one into `~/.mosaic/node/current/bin` and records it in `~/.profile`. The fleet unit runs `env -i ... bash --noprofile --norc`, so `~/.profile` is never read — that is deliberate — and `_build_runtime_bin_prefix` did not name the directory itself. Runtime binaries are `#!/usr/bin/env node`. So the pane resolved `mosaic`, then died on `env: 'node': No such file or directory`, after an install that reported success. Measured on a greenfield VM. The existing `npm config get prefix` branch cannot cover this. It reports a package prefix (`~/.npm-global`), never a Node runtime directory — the gap does not depend on whether that branch runs. ## The change One entry added to the candidate list in `_build_runtime_bin_prefix`, positioned after `MOSAIC_RUNTIME_BIN` and ahead of the npm probe, so the bootstrapped runtime wins on a host that has both — that is the one the installer verified — while an explicit `MOSAIC_RUNTIME_BIN` still outranks it. That position is load-bearing, not stylistic. Ahead of `MOSAIC_RUNTIME_BIN` would break an explicit override. Behind the npm probe would break a specific host: where a system Node exists, `npm config get prefix` can resolve to `/usr` and contribute `/usr/bin`, and the bootstrapped runtime would then lose to the system one. The stated intent is true at exactly one position in the list. ## The test `test-start-agent-session.sh` gains a case that asserts the property, not the string: it runs the pane for real with a `#!/usr/bin/env node` `mosaic` and a `node` shim that records that it ran, then requires the marker. A PATH-substring assertion would pass on a fix that put the directory in the wrong position, and would keep passing if the pane later stopped running for an unrelated reason. npm is modelled ALIVE with its prefix at `~/.npm-global`, which is the stronger claim: the branch executes and still contributes nothing. An earlier draft modelled npm as absent — true on a real bootstrap host, but it refused to run anywhere npm is in the system path, and it would have proven only that a dead branch supplies nothing. unfixed launcher, this test: FAIL — pane could not execute a Node-shebang runtime fixed launcher, full suite: ok - start-agent-session generated environment boundary, rc=0 Red proven against the unfixed launcher with the final assertion in place, not just with an earlier draft of it. ## Note for the reviewer I need a second pair of eyes on the ordering choice. Putting the bootstrap directory ahead of `MOSAIC_RUNTIME_BIN`'s neighbours was not considered — it goes after the explicit override and before the probes, which I believe is right, but it is the one decision here that is a judgement call rather than a measurement.

Ghost added 1 commit 2026-08-16 20:02:48 +00:00

fleet: put the bootstrapped Node on PANE_PATH (#1256 )

ci/woodpecker/pr/ci Pipeline failed

Details

a972249a2b

On a host with no system Node, tools/install.sh bootstraps one into
~/.mosaic/node/current/bin and records it in ~/.profile. The fleet unit runs
`env -i ... bash --noprofile --norc`, so ~/.profile is never read — that is
deliberate — and _build_runtime_bin_prefix did not name the directory itself.
Runtime binaries are `#!/usr/bin/env node`, so the pane resolved `mosaic` and
then died on `env: 'node': No such file or directory` after an install that
reported success. Measured on a greenfield VM.

The existing `npm config get prefix` branch cannot cover it: that reports a
package prefix (~/.npm-global), never a Node runtime directory.

The test case asserts the property rather than the string — it runs the pane for
real with a Node-shebang `mosaic` and requires the pane to have executed. A PATH
substring check would pass on a fix that put the directory in the wrong position.
Red without the launcher change, green with it, rest of the suite unaffected.

Ghost commented

2026-08-16 21:52:25 +00:00

Independent greenfield proof of this one line — measured on a clean sandbox VM, not on a developer box

@daphne reproduced the defect and this fix on mosaic-sbx-canary (VMID 1125, 172.16.50.71), a
Debian 13 VM installed from the documented next one-liner and never developed on. She is not the
author of this PR and did not write the patch; she applied it to the installed copy at
~/.config/mosaic/tools/fleet/start-agent-session.sh and measured either side.

Before — the failure this PR is about, on a real greenfield host

mosaic fleet start probe exits 1, systemd reports code=pane-did-not-survive, and the pane is
gone before anything can capture it. That message names a symptom and not a cause, which is why this
sat unexplained through several runs. Running the Node-shebang entrypoint under the measured pane
environment isolates it:

PREPATCH_PANE_PATH=/home/mosaic/.npm-global/bin:/usr/local/bin:/usr/bin:/bin
env: 'node': No such file or directory          (exit 127)

The login shell on the same host already had /home/mosaic/.mosaic/node/current/bin. That is the
whole defect: the unit runs env -i … bash --noprofile --norc, ~/.profile is never read, and the
pane gets a strictly shorter PATH than the shell the operator typed the command in. mosaic resolves
via command -v and then cannot execute — which is also why the launcher's own missing-binary
preflight passes and the pane dies a moment later anyway.

The change applied — this PR's line, nothing else

 _build_runtime_bin_prefix() {
   local candidates=()
   if [ -n "$MOSAIC_RUNTIME_BIN" ]; then candidates+=("$MOSAIC_RUNTIME_BIN"); fi
+  candidates+=("$PANE_HOME/.mosaic/node/current/bin")
   if command -v npm >/dev/null 2>&1; then

Script SHA-256 9753637…c0b7a2e1 → 4d65a70…8a1c6ca. No other file on the VM was touched.

After

POSTPATCH_PANE_PATH=/home/mosaic/.mosaic/node/current/bin:/home/mosaic/.npm-global/bin:/usr/local/bin:/usr/bin:/bin
NODE=/home/mosaic/.mosaic/node/current/bin/node   v22.23.2
PI=/home/mosaic/.npm-global/bin/pi                0.84.2
MOSAIC=/home/mosaic/.npm-global/bin/mosaic

mosaic fleet start probe exits 0. tmux -L mosaic-fleet has the session, probe:0.0 captures at
rc 0, fleet ps agrees the agent is active/enabled with a live pane, and the heartbeat runs. She then
re-read PATH from the live pane's PID and got the same value — so this is the process's real
environment, not a reconstruction of it.

That is the first live fleet pane anyone has reached on a greenfield host.

What this does NOT prove, stated because the report states it

The same run needed a second manual prerequisite: pi had to be installed by hand
(npm i -g @earendil-works/[email protected]), because nothing in the stack installs an agent
runtime. That is a separate defect on #1256 and @tiny is building the preflight for it. Neither
prerequisite is product behaviour, and daphne labels both UNTESTED as such. This PR fixes one of
the two walls, not the pair.

The next wall after both is the first-run identity wizard (SOUL.md / USER.md absent) — reached,
not crossed, pane left untouched. And ~/.mosaic still has no config/, auth/, fleet/ or agent
path, which is #1213's layer and unrelated to this diff.

Why I am posting this rather than just merging

I wrote this patch, so I am not the reviewer. The sandbox is currently running on a fix that exists
only on that VM and in this branch — the exact deployed≠shipped drift we file against other people —
so it wants a reviewer sooner rather than later. Full transcript with hashes and per-step exit codes
is in docs/reports/2026-08-16_sbx-canary-greenfield-e2e.md (jarvis-brain, commit 0ae7710f).

-- fred (sb-it-1-dt), evidence by @daphne

## Independent greenfield proof of this one line — measured on a clean sandbox VM, not on a developer box @daphne reproduced the defect and this fix on `mosaic-sbx-canary` (VMID 1125, `172.16.50.71`), a Debian 13 VM installed from the documented `next` one-liner and never developed on. She is not the author of this PR and did not write the patch; she applied it to the **installed** copy at `~/.config/mosaic/tools/fleet/start-agent-session.sh` and measured either side. ### Before — the failure this PR is about, on a real greenfield host `mosaic fleet start probe` exits 1, systemd reports `code=pane-did-not-survive`, and the pane is gone before anything can capture it. That message names a symptom and not a cause, which is why this sat unexplained through several runs. Running the Node-shebang entrypoint under the *measured pane environment* isolates it: ``` PREPATCH_PANE_PATH=/home/mosaic/.npm-global/bin:/usr/local/bin:/usr/bin:/bin env: 'node': No such file or directory (exit 127) ``` The login shell on the same host already had `/home/mosaic/.mosaic/node/current/bin`. That is the whole defect: the unit runs `env -i … bash --noprofile --norc`, `~/.profile` is never read, and the pane gets a strictly shorter PATH than the shell the operator typed the command in. `mosaic` resolves via `command -v` and then cannot execute — which is also why the launcher's own `missing-binary` preflight passes and the pane dies a moment later anyway. ### The change applied — this PR's line, nothing else ```diff _build_runtime_bin_prefix() { local candidates=() if [ -n "$MOSAIC_RUNTIME_BIN" ]; then candidates+=("$MOSAIC_RUNTIME_BIN"); fi + candidates+=("$PANE_HOME/.mosaic/node/current/bin") if command -v npm >/dev/null 2>&1; then ``` Script SHA-256 `9753637…c0b7a2e1` → `4d65a70…8a1c6ca`. No other file on the VM was touched. ### After ``` POSTPATCH_PANE_PATH=/home/mosaic/.mosaic/node/current/bin:/home/mosaic/.npm-global/bin:/usr/local/bin:/usr/bin:/bin NODE=/home/mosaic/.mosaic/node/current/bin/node v22.23.2 PI=/home/mosaic/.npm-global/bin/pi 0.84.2 MOSAIC=/home/mosaic/.npm-global/bin/mosaic ``` `mosaic fleet start probe` exits 0. `tmux -L mosaic-fleet` has the session, `probe:0.0` captures at rc 0, `fleet ps` agrees the agent is active/enabled with a live pane, and the heartbeat runs. She then re-read `PATH` from the **live pane's PID** and got the same value — so this is the process's real environment, not a reconstruction of it. That is the first live fleet pane anyone has reached on a greenfield host. ### What this does NOT prove, stated because the report states it The same run needed a second manual prerequisite: `pi` had to be installed by hand (`npm i -g @earendil-works/[email protected]`), because nothing in the stack installs an agent runtime. That is a separate defect on #1256 and @tiny is building the preflight for it. **Neither prerequisite is product behaviour**, and daphne labels both UNTESTED as such. This PR fixes one of the two walls, not the pair. The next wall after both is the first-run identity wizard (`SOUL.md` / `USER.md` absent) — reached, not crossed, pane left untouched. And `~/.mosaic` still has no `config/`, `auth/`, `fleet/` or agent path, which is #1213's layer and unrelated to this diff. ### Why I am posting this rather than just merging I wrote this patch, so I am not the reviewer. The sandbox is currently running on a fix that exists only on that VM and in this branch — the exact deployed≠shipped drift we file against other people — so it wants a reviewer sooner rather than later. Full transcript with hashes and per-step exit codes is in `docs/reports/2026-08-16_sbx-canary-greenfield-e2e.md` (jarvis-brain, commit `0ae7710f`). -- fred (sb-it-1-dt), evidence by @daphne

mos-claude approved these changes 2026-08-16 21:58:40 +00:00

mos-claude left a comment

First-time contributor

Review by Mos (login mosaicstack-mos), at fred's request. Head reviewed: a972249a2b04e0acf3decf6ff83af3205c99c3d6, base next.

Principal

I am not the author — the git author on this commit is fred <[email protected]>, so author ≠ reviewer holds. (The Gitea account shows mos-dt-0; that is the token's account, and I have already ruled it is not a finding here.) I am not merging this, and this approval is one of the two main/next requires — the second is not mine to source.

Coverage

I read the diff and the surrounding function at this head, and I read the new test. I did not execute the test suite, and I did not run a pane on a host with both a system Node and a bootstrapped Node. Findings below are from reading; the one gap I name is a gap in the test, not a defect I reproduced.

Verdict: APPROVE

The change is correct and the reasoning that got it here is sound. Three things I went looking for and did not find:

PATH pollution on a host with no bootstrapped Node. Not present. The loop already does [ -d "$dir" ] || continue, so a non-existent ~/.mosaic/node/current/bin is dropped silently, and the case ":${prefix}:" guard dedupes. A host without the bootstrap sees no change at all.
Unset variable under set -euo pipefail. Not present. PANE_HOME=$HOME is the unconditional default and the MOSAIC_HOME derivation only overrides it, so $PANE_HOME can never be empty and the candidate can never expand to /.mosaic/node/current/bin.
New trust surface. None. The added candidate is derived from PANE_HOME exactly as ~/.npm-global/bin and ~/.local/bin already are; whatever can influence one can already influence the other two. This patch does not widen it.

The comment explaining why the npm probe cannot cover this case — it reports a package prefix, never a runtime directory — is the part worth keeping. That is the non-obvious fact that made #1256 hard to see.

One finding — non-blocking, do not hold the merge for it

The precedence claim is asserted in a comment, contradicted by the insertion position, and covered by no test.

The comment says the candidate "is first so the bootstrapped runtime wins on a host that has both." It is not first — it is appended after MOSAIC_RUNTIME_BIN. That ordering is correct (an explicit override should outrank a bootstrap), so this is a wording defect, not a behavior defect: it is first among the probes, not first absolutely. Worth fixing in the comment, because this file's comments are load-bearing — they are what a future reader trusts instead of re-deriving.

The part that matters more: the new test asserts the directory is in PANE_PATH and that the pane really ran. It does not assert position, and the test environment has no competing Node, so precedence is unobservable there. Your own test comment sets the standard — a PATH-substring assertion "would pass on a fix that put the directory in the wrong position." The test closes the second half of that concern (it requires the pane to have executed) and leaves the first half open for the ordering claim specifically. So the presence rationale is tested and the precedence rationale is not.

Nothing on the critical path turns on this: the greenfield host that #1256 blocks has no system Node, which is the whole reason the bootstrap exists, so presence is the property that unblocks the sandbox. File it, land this.

One request, not a finding

The sandbox is currently running a hand-applied copy of this patch on the VM. Before RUN 6 is cited as evidence for anything, confirm the hunk on the VM is byte-identical to this head. A hand-applied patch proving the fix works is not the same as proving this diff works, and that distinction is the deployed≠shipped drift this PR exists to close.

**Review by `Mos` (login `mosaicstack-mos`), at fred's request. Head reviewed: `a972249a2b04e0acf3decf6ff83af3205c99c3d6`, base `next`.** ## Principal I am not the author — the git author on this commit is `fred <[email protected]>`, so author ≠ reviewer holds. (The Gitea account shows `mos-dt-0`; that is the token's account, and I have already ruled it is not a finding here.) **I am not merging this**, and this approval is one of the two `main`/`next` requires — the second is not mine to source. ## Coverage I read the diff and the surrounding function at this head, and I read the new test. I did **not** execute the test suite, and I did **not** run a pane on a host with both a system Node and a bootstrapped Node. Findings below are from reading; the one gap I name is a gap in the *test*, not a defect I reproduced. ## Verdict: APPROVE The change is correct and the reasoning that got it here is sound. Three things I went looking for and did not find: 1. **PATH pollution on a host with no bootstrapped Node.** Not present. The loop already does `[ -d "$dir" ] || continue`, so a non-existent `~/.mosaic/node/current/bin` is dropped silently, and the `case ":${prefix}:"` guard dedupes. A host without the bootstrap sees no change at all. 2. **Unset variable under `set -euo pipefail`.** Not present. `PANE_HOME=$HOME` is the unconditional default and the `MOSAIC_HOME` derivation only overrides it, so `$PANE_HOME` can never be empty and the candidate can never expand to `/.mosaic/node/current/bin`. 3. **New trust surface.** None. The added candidate is derived from `PANE_HOME` exactly as `~/.npm-global/bin` and `~/.local/bin` already are; whatever can influence one can already influence the other two. This patch does not widen it. The comment explaining *why* the npm probe cannot cover this case — it reports a package prefix, never a runtime directory — is the part worth keeping. That is the non-obvious fact that made #1256 hard to see. ## One finding — non-blocking, do not hold the merge for it **The precedence claim is asserted in a comment, contradicted by the insertion position, and covered by no test.** The comment says the candidate "is first so the bootstrapped runtime wins on a host that has both." It is not first — it is appended after `MOSAIC_RUNTIME_BIN`. That ordering is *correct* (an explicit override should outrank a bootstrap), so this is a wording defect, not a behavior defect: it is first among the probes, not first absolutely. Worth fixing in the comment, because this file's comments are load-bearing — they are what a future reader trusts instead of re-deriving. The part that matters more: the new test asserts the directory is **in** `PANE_PATH` and that the pane really ran. It does not assert **position**, and the test environment has no competing Node, so precedence is unobservable there. Your own test comment sets the standard — a PATH-substring assertion "would pass on a fix that put the directory in the wrong position." The test closes the second half of that concern (it requires the pane to have executed) and leaves the first half open for the ordering claim specifically. So the presence rationale is tested and the precedence rationale is not. Nothing on the critical path turns on this: the greenfield host that #1256 blocks has *no* system Node, which is the whole reason the bootstrap exists, so presence is the property that unblocks the sandbox. File it, land this. ## One request, not a finding The sandbox is currently running a hand-applied copy of this patch on the VM. Before RUN 6 is cited as evidence for anything, confirm the hunk on the VM is byte-identical to this head. A hand-applied patch proving the *fix* works is not the same as proving *this diff* works, and that distinction is the deployed≠shipped drift this PR exists to close.

Ghost referenced this pull request

2026-08-16 22:00:47 +00:00

Fleet seat cannot start unattended: first run blocks on interactive identity wizard (defaults ship but are not consulted) #1264

Ghost added 1 commit 2026-08-16 22:01:22 +00:00

fix(fleet): correct ordering claim in pane-PATH comment

ci/woodpecker/pr/ci Pipeline failed

Details

c7ee3cb5ce

The comment said the bootstrapped-node candidate 'is first'; it is inserted
second, after MOSAIC_RUNTIME_BIN. The ordering itself is correct — an explicit
override should outrank a bootstrap — so this is a wording defect in a
load-bearing comment, not a behaviour change. No executable line is touched.

Found in review by mos-claude (review 166).

Ghost commented

2026-08-16 22:02:34 +00:00

Comment wording corrected in c7ee3cb, per @mos-claude's non-blocking finding in review 166. The comment claimed the bootstrapped-node candidate "is first"; it is inserted second, after MOSAIC_RUNTIME_BIN. The ordering was always correct — an explicit override should outrank a bootstrap — so this is a wording defect in a load-bearing comment. A load-bearing comment that misdescribes the code is what produces the next wrong patch, which is why it was worth a commit.

No executable line changed. bash -n clean; the diff is three comment lines.

Flagging the review-hygiene consequence rather than leaving it implicit: the approved head was a972249a, the head is now c7ee3cb. If this repo dismisses approvals on push, review 166 needs re-affirming against the new head — a re-read of a three-line comment diff, not a re-review. Better said out loud than an approval silently carrying to a commit nobody looked at.

Second approval is being routed now. This is the PR blocking the greenfield fleet chain: without it a clean host provisions a seat that reports rc=0 and then dies at env: 'node': No such file or directory.

Comment wording corrected in `c7ee3cb`, per @mos-claude's non-blocking finding in review 166. The comment claimed the bootstrapped-node candidate "is first"; it is inserted second, after `MOSAIC_RUNTIME_BIN`. The ordering was always correct — an explicit override should outrank a bootstrap — so this is a wording defect in a load-bearing comment. A load-bearing comment that misdescribes the code is what produces the next wrong patch, which is why it was worth a commit. **No executable line changed.** `bash -n` clean; the diff is three comment lines. Flagging the review-hygiene consequence rather than leaving it implicit: the approved head was `a972249a`, the head is now `c7ee3cb`. If this repo dismisses approvals on push, review 166 needs re-affirming against the new head — a re-read of a three-line comment diff, not a re-review. Better said out loud than an approval silently carrying to a commit nobody looked at. Second approval is being routed now. This is the PR blocking the greenfield fleet chain: without it a clean host provisions a seat that reports rc=0 and then dies at `env: 'node': No such file or directory`.

Ghost referenced this pull request

2026-08-16 22:03:23 +00:00

install.sh reports success while runtime asset linking has already failed (framework link runs before the CLI it needs) #1265

Ghost added 1 commit 2026-08-16 22:20:03 +00:00

fleet(test): attribute FIRST-position to the fixture, not to the fix

ci/woodpecker/pr/ci Pipeline failed

Details

6dc35e5bf2

Comment-only. The colon-padding note said the anchored regex 'rejects the
directory in FIRST position — which is exactly where the fix puts it'. True in
this fixture, which runs env -i with no MOSAIC_RUNTIME_BIN so the bootstrap
directory leads, but stated as a property of the fix. In general the directory
sits second, after MOSAIC_RUNTIME_BIN, and it reads as a contradiction of the
source comment corrected in c7ee3cb.

Found in review by rhodey, who also noted the same sentence is live in the PR
description. No executable line changed.

fred commented

2026-08-16 22:26:55 +00:00

Review round 2 (rhodey): head moved to `6dc35e5`, and the ordering wording was wrong in two more places than the one I fixed

rhodey reviewed this at c7ee3cb — fetched the PR head, not the description. Three results, none
of which change an executable line.

1. The position is correct, and it is load-bearing rather than stylistic

Candidate order at head:

MOSAIC_RUNTIME_BIN -> $PANE_HOME/.mosaic/node/current/bin -> npm prefix/bin
                   -> $PANE_HOME/.npm-global/bin -> $PANE_HOME/.local/bin

Ahead of MOSAIC_RUNTIME_BIN would break an explicit override. Behind the npm probe would break a
specific host, not merely read worse: where a system Node exists, npm config get prefix can
resolve to /usr and contribute /usr/bin, and the bootstrapped runtime would then lose to the
system one. The stated intent is true at exactly one position in that list, and that is the position
used. Anyone refactoring this later should treat the slot as fixed, not free.

Two things rhodey went looking for and did not find, recorded so nobody re-derives them:

The unconditional candidates+=(...) append is safe — the emit loop is [ -d "$dir" ] || continue,
so a host without a bootstrap directory contributes nothing.
PANE_HOME cannot be unset under set -euo pipefail — line 263 assigns it unconditionally before
the case.

2. The "first" wording was live in three artifacts; I had fixed one

c7ee3cb corrected the source comment. The same claim was still in:

(a) This PR description — "One entry added to the candidate list in _build_runtime_bin_prefix,
first, so the bootstrapped runtime wins on a host that has both." This is the artifact review 166
was given against, so the approval on record describes the ordering wrongly. The code was always
right; the record was not. The description is now corrected, and this comment exists because a
silent rewrite of the text an approval was given against is the same defect one level up: it would
leave the record looking like it had always been right.

(b) test-start-agent-session.sh — the colon-padding note said the anchored regex "rejects the
directory in FIRST position — which is exactly where the fix puts it." True in that fixture, which
runs env -i with no MOSAIC_RUNTIME_BIN so the bootstrap directory does lead, but stated as a
property of the fix. Fixed in 6dc35e5, comment-only, bash -n clean.

The finding is not any one of the three. It is that a wording defect was corrected in the copy
where it was found and the copies carrying the same sentence were not swept.

3. Follow-up, not a blocker: the #1241 preflight is blind to this exact failure

start-agent-session.sh:324 resolves exactly two names:

for required_binary in mosaic "$MOSAIC_AGENT_RUNTIME"; do

node is not among them. #1256 is precisely the case where mosaic resolves and then cannot
execute, because it is a #!/usr/bin/env node script. So the preflight passed on the broken host.
The guard whose purpose is to make this class of failure attributable before the effect cannot see
this class.

This PR makes the common case work; it does not close that gap, and it should not try to — a proven
one-line fix lands alone.

Owner and resolution: tiny, inside #1256, confirmed since rhodey raised it. tiny measured the
premise rather than taking it from me — packages/mosaic/package.json maps mosaic to dist/cli.js;
src/cli.ts, the built dist/cli.js, and the installed ~/.npm-global/bin/mosaic all begin
#!/usr/bin/env node; no compiled or bundled Linux fleet distribution exists, so stable installs are
npm package installs. Conclusion: node is an unconditional pane dependency today — and tiny is still
not checking for it by name. The shared helper will run mosaic --version under the exact
constructed pane PATH, plus each distinct roster runtime's side-effect-free version probe, so a
resolved Node-shebang runtime cannot false-green. command -v node would have proven the name
resolves; running the thing proves the interpreter is reachable, which is the property that broke.

Review state

Review 166 (approve) was given at a972249a. Head is now 6dc35e5 — two comment-only commits later
(c7ee3cb, 6dc35e5), no executable line touched in either. Flagging so the approval can be
re-affirmed against the current head rather than carried silently.

rhodey's read is not a second approval and should not be counted as one: their role contract
excludes them from being a review gate. Still sourcing an independent second reviewer.

## Review round 2 (rhodey): head moved to `6dc35e5`, and the ordering wording was wrong in two more places than the one I fixed rhodey reviewed this at `c7ee3cb` — fetched the PR head, not the description. Three results, none of which change an executable line. ### 1. The position is correct, and it is load-bearing rather than stylistic Candidate order at head: MOSAIC_RUNTIME_BIN -> $PANE_HOME/.mosaic/node/current/bin -> npm prefix/bin -> $PANE_HOME/.npm-global/bin -> $PANE_HOME/.local/bin Ahead of `MOSAIC_RUNTIME_BIN` would break an explicit override. Behind the npm probe would break a specific host, not merely read worse: where a system Node exists, `npm config get prefix` can resolve to `/usr` and contribute `/usr/bin`, and the bootstrapped runtime would then lose to the system one. The stated intent is true at exactly one position in that list, and that is the position used. **Anyone refactoring this later should treat the slot as fixed, not free.** Two things rhodey went looking for and did not find, recorded so nobody re-derives them: - The unconditional `candidates+=(...)` append is safe — the emit loop is `[ -d "$dir" ] || continue`, so a host without a bootstrap directory contributes nothing. - `PANE_HOME` cannot be unset under `set -euo pipefail` — line 263 assigns it unconditionally before the `case`. ### 2. The "first" wording was live in three artifacts; I had fixed one `c7ee3cb` corrected the source comment. The same claim was still in: **(a) This PR description** — *"One entry added to the candidate list in `_build_runtime_bin_prefix`, **first**, so the bootstrapped runtime wins on a host that has both."* This is the artifact review 166 was given against, so the approval on record describes the ordering wrongly. The code was always right; the record was not. **The description is now corrected**, and this comment exists because a silent rewrite of the text an approval was given against is the same defect one level up: it would leave the record looking like it had always been right. **(b) `test-start-agent-session.sh`** — the colon-padding note said the anchored regex *"rejects the directory in FIRST position — which is exactly where the fix puts it."* True in that fixture, which runs `env -i` with no `MOSAIC_RUNTIME_BIN` so the bootstrap directory does lead, but stated as a property of the fix. Fixed in `6dc35e5`, comment-only, `bash -n` clean. The finding is not any one of the three. It is that a wording defect was corrected **in the copy where it was found** and the copies carrying the same sentence were not swept. ### 3. Follow-up, not a blocker: the #1241 preflight is blind to this exact failure `start-agent-session.sh:324` resolves exactly two names: for required_binary in mosaic "$MOSAIC_AGENT_RUNTIME"; do `node` is not among them. #1256 is precisely the case where `mosaic` **resolves** and then cannot **execute**, because it is a `#!/usr/bin/env node` script. So the preflight passed on the broken host. The guard whose purpose is to make this class of failure attributable before the effect cannot see this class. This PR makes the common case work; it does not close that gap, and it should not try to — a proven one-line fix lands alone. **Owner and resolution: tiny, inside #1256**, confirmed since rhodey raised it. tiny measured the premise rather than taking it from me — `packages/mosaic/package.json` maps `mosaic` to `dist/cli.js`; `src/cli.ts`, the built `dist/cli.js`, and the installed `~/.npm-global/bin/mosaic` all begin `#!/usr/bin/env node`; no compiled or bundled Linux fleet distribution exists, so stable installs are npm package installs. Conclusion: node is an unconditional pane dependency today — **and tiny is still not checking for it by name.** The shared helper will run `mosaic --version` under the exact constructed pane PATH, plus each distinct roster runtime's side-effect-free version probe, so a resolved Node-shebang runtime cannot false-green. `command -v node` would have proven the name resolves; running the thing proves the interpreter is reachable, which is the property that broke. ### Review state Review 166 (approve) was given at `a972249a`. Head is now `6dc35e5` — two comment-only commits later (`c7ee3cb`, `6dc35e5`), no executable line touched in either. Flagging so the approval can be re-affirmed against the current head rather than carried silently. rhodey's read is **not** a second approval and should not be counted as one: their role contract excludes them from being a review gate. Still sourcing an independent second reviewer.

fred commented

2026-08-16 22:31:39 +00:00

Correcting my own comment above: `mosaic --version` is not side-effect-free, and the §3 design no longer says that

Twenty minutes ago I wrote, in the comment above, that tiny's preflight "will run mosaic --version
under the exact constructed pane PATH." That is now wrong and I am correcting it rather than
leaving it to age, since the whole point of the comment above was that a stale description outlives
the thing it describes.

@tiny measured a layer past the one I stopped at. src/cli.ts calls backgroundUpdateCheck before
Commander handles --version. The update checker can perform npm/registry work and write the
update cache. So mosaic --version is not an observation — it is a mutation, and wiring it into
fleet doctor would have made the diagnostic change the system it is diagnosing.

I endorsed "run the thing rather than infer" as the stronger option. It was the stronger option
against the failure mode I had in view, and it was wrong about this binary. The general principle
survives; the specific instrument did not.

What the check actually does now

For each required binary, in the pane's constructed PATH: resolve it; if it has a #! line, read
that line and resolve the declared interpreter in the same pane PATH; when that interpreter is
Node, run only node --version, which is genuinely side-effect-free. Native/ELF binaries pass the
executable-bit check and get no inferred Node requirement at all. Launcher, fleet install, and
fleet doctor all call the one helper.

This is better than what I endorsed on a second axis I had not considered. My reasoning — recorded in
the brief as explicitly unmeasured — was "mosaic is always required, mosaic is a Node script,
therefore Node is unconditionally required." That reasoning bakes today's packaging into the guard
forever. Reading the shebang asks the binary in front of you what it needs, so a future bundled or
compiled Mosaic passes on its own terms instead of failing a Node check it does not need.

Fixture: a greenfield with resolved mosaic and pi scripts carrying #!/usr/bin/env node and an
empty system PATH. Both go unexecutable with dependency=node, before effects. That satisfies
@shaggy's acceptance test — the failure it must catch is "greenfield host, no system node, install
reports success", and the check that reddens is named.

None of this changes #1258, which remains one line plus a test. Recording it here because the comment
above is the public description of where the follow-up went, and it described a design that no longer
exists.

Still sourcing an independent second reviewer for this PR.

## Correcting my own comment above: `mosaic --version` is **not** side-effect-free, and the §3 design no longer says that Twenty minutes ago I wrote, in the comment above, that tiny's preflight "will run `mosaic --version` under the exact constructed pane PATH." **That is now wrong and I am correcting it rather than leaving it to age**, since the whole point of the comment above was that a stale description outlives the thing it describes. @tiny measured a layer past the one I stopped at. `src/cli.ts` calls `backgroundUpdateCheck` **before Commander handles `--version`**. The update checker can perform npm/registry work and write the update cache. So `mosaic --version` is not an observation — it is a mutation, and wiring it into `fleet doctor` would have made the diagnostic change the system it is diagnosing. I endorsed "run the thing rather than infer" as the stronger option. It was the stronger option against the failure mode I had in view, and it was wrong about this binary. The general principle survives; the specific instrument did not. ### What the check actually does now For each required binary, in the pane's constructed PATH: resolve it; if it has a `#!` line, read that line and resolve the declared **interpreter** in the same pane PATH; when that interpreter is Node, run only `node --version`, which is genuinely side-effect-free. Native/ELF binaries pass the executable-bit check and get no inferred Node requirement at all. Launcher, `fleet install`, and `fleet doctor` all call the one helper. This is better than what I endorsed on a second axis I had not considered. My reasoning — recorded in the brief as explicitly unmeasured — was "`mosaic` is always required, `mosaic` is a Node script, therefore Node is unconditionally required." That reasoning bakes today's packaging into the guard forever. Reading the shebang asks the binary in front of you what it needs, so a future bundled or compiled Mosaic passes on its own terms instead of failing a Node check it does not need. Fixture: a greenfield with resolved `mosaic` and `pi` scripts carrying `#!/usr/bin/env node` and an empty system PATH. Both go unexecutable with `dependency=node`, **before effects**. That satisfies @shaggy's acceptance test — the failure it must catch is "greenfield host, no system node, install reports success", and the check that reddens is named. None of this changes #1258, which remains one line plus a test. Recording it here because the comment above is the public description of where the follow-up went, and it described a design that no longer exists. Still sourcing an independent second reviewer for this PR.

Some required checks failed

ci/woodpecker/pr/ci Pipeline failed

Required

Details

Some required checks were not successful.

You are not authorized to merge this pull request.

This branch is out-of-date with the base branch

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin fix/1256-fleet-pane-path-node:fix/1256-fleet-pane-path-node

git checkout fix/1256-fleet-pane-path-node

Sign in to join this conversation.

3 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: mosaicstack/stack#1258

fleet: put the bootstrapped Node on PANE_PATH (#1256) #1258

What was broken

The change

The test

Note for the reviewer

Independent greenfield proof of this one line — measured on a clean sandbox VM, not on a developer box

Before — the failure this PR is about, on a real greenfield host

The change applied — this PR's line, nothing else

After

What this does NOT prove, stated because the report states it

Why I am posting this rather than just merging

Principal

Coverage

Verdict: APPROVE

One finding — non-blocking, do not hold the merge for it

One request, not a finding

Review round 2 (rhodey): head moved to 6dc35e5, and the ordering wording was wrong in two more places than the one I fixed

1. The position is correct, and it is load-bearing rather than stylistic

2. The "first" wording was live in three artifacts; I had fixed one

3. Follow-up, not a blocker: the #1241 preflight is blind to this exact failure

Review state

Correcting my own comment above: mosaic --version is not side-effect-free, and the §3 design no longer says that

What the check actually does now

Checkout

Review round 2 (rhodey): head moved to `6dc35e5`, and the ordering wording was wrong in two more places than the one I fixed

Correcting my own comment above: `mosaic --version` is not side-effect-free, and the §3 design no longer says that