fix(framework): install deps on worktree bootstrap + legible deps-preflight at gate seam (#856) #858

Merged

jason.woltje merged 1 commits from fix/856-worktree-deps-preflight into main 2026-07-20 09:38:13 +00:00

Conversation 2 Commits 1 Files Changed 4 +143 -2

jason.woltje commented 2026-07-20 09:17:51 +00:00

Owner

Copy Link

Defect (#856 — top park-window priority; 131/147 fleet tool-errors)

Worker git-worktrees under a fresh git worktree add run the QA gate suite (pnpm test/lint/typecheck/format:check) BEFORE node_modules is installed. pnpm workspaces do NOT share node_modules across separate git worktrees, so each fresh worktree has an empty node_modules/.bin and every gate binary (tsc/eslint/prettier/vitest) fails sh: 1: <tool>: not found. That failure is indistinguishable from a real test/lint failure -> false-red gates + wasted cycles + inflated fleet error signal.

Fix (both approaches implemented, defense-in-depth)

Approach 1 (primary correctness fix) — plugins/mosaic-framework/src/index.ts: this is the actual code that mechanically injects the "Git Worktree Requirement — MANDATORY" instruction block into every agent session and ACP worker spawn (native OC sessions via before_agent_start, and Codex/Claude ACP spawns via subagent_spawning). It is the concrete, executing worktree-add path in this repo — worktree creation itself is carried out by the worker following this injected instruction sequence, so fixing the instruction is the real fix. Added pnpm install --frozen-lockfile --prefer-offline (matching the CI install pattern in .woodpecker/ci.yml) immediately after git worktree add/cd, before "... all work happens here ...", plus an explanatory paragraph telling the worker never to skip/reorder it ahead of any gate.

Approach 2 (defense-in-depth, legible failure) — packages/mosaic/framework/tools/qa/qa-hook-stdin.sh: this is the single common gate-entry seam named in the issue itself — it's registered as the PostToolUse hook for every Edit|MultiEdit|Write in packages/mosaic/framework/runtime/claude/settings.json, i.e. the one wrapper the delivery cycle invokes on every code edit across the fleet, regardless of runtime/worker. Added a preflight, scoped to JS/TS files only (matching the existing skip logic already in the script), that checks for node_modules/.bin presence/non-emptiness and, if absent, emits the explicit sentinel deps not installed — run pnpm install (to stderr and the QA log) and exits nonzero — instead of silently proceeding (previous behavior: exit 0, write a QA report as if nothing were wrong) into a downstream raw not-found. No per-command checks were scattered into individual gates; the check lives in exactly one place.

Red-first evidence (throwaway fixture — no live worktree/worker touched)

1. RED: with a fresh fixture repo and PATH containing no node_modules/.bin, invoking a gate binary directly (sh -c 'tsc --noEmit') produces the raw sh: 1: tsc: not found (exit 127) — the defect. The pre-fix qa-hook-stdin.sh (checked out at main) does not intercept this at all — it exits 0 and writes a QA report as if the edit were clean, letting the raw not-found reach the operator unannotated whenever a gate is later invoked.
2. GREEN — approach 2: post-fix qa-hook-stdin.sh run against the same empty/missing node_modules/.bin fixture now exits 1 and prints exactly deps not installed — run pnpm install instead of silently passing.
3. GREEN — approach 1: after simulating the worktree-bootstrap install (populating node_modules/.bin with a stub tsc), the same gate-entry seam proceeds normally (exit 0) and the gate binary itself now resolves and runs successfully instead of hitting not-found.

This is codified as a permanent regression test, packages/mosaic/framework/tools/qa/test-deps-preflight.sh, following the existing test-*.sh bash convention in this tree (see packages/mosaic/framework/tools/git/test-help-exit-code.sh, packages/mosaic/framework/tools/codex/test-pr-diff-context.sh), covering: missing node_modules/.bin, present-but-empty node_modules/.bin, populated node_modules/.bin (no false positive), and non-JS/TS files (existing skip preserved). It is wired into the existing test:framework-shell script in packages/mosaic/package.json, run as part of pnpm test.

Gates (all run, verbatim below)

• pnpm typecheck — 42/42 tasks successful.
• pnpm lint — 23/23 tasks successful.
• pnpm format:check — "All matched files use Prettier code style!"
• pnpm test — 43/43 tasks successful (turbo), including the new deps-preflight regression passed (5/5 scenarios) line inside test:framework-shell, and the full @mosaicstack/mosaic vitest suite (1434 tests passed), @mosaicstack/gateway (628 passed / 12 skipped), @mosaicstack/db (19 passed / 7 skipped).
• shellcheck -x on both changed/added shell files (qa-hook-stdin.sh, test-deps-preflight.sh) — clean, no findings.

No pre-existing unrelated failures were observed in this run; no new failures were introduced by this change.

Scope

Framework tooling only (plugins/mosaic-framework/src/index.ts, packages/mosaic/framework/tools/qa/, packages/mosaic/package.json test wiring). No production application code touched.

Closes #856. NO self-merge — awaiting named-executor merge clearance; merge to be timed by coordinator to a no-critical-lane-mid-bootstrap seam. Author lane: ms-856-build.

## Defect (#856 — top park-window priority; 131/147 fleet tool-errors) Worker git-worktrees under a fresh `git worktree add` run the QA gate suite (`pnpm test`/`lint`/`typecheck`/`format:check`) BEFORE `node_modules` is installed. pnpm workspaces do NOT share `node_modules` across separate git worktrees, so each fresh worktree has an empty `node_modules/.bin` and every gate binary (`tsc`/`eslint`/`prettier`/`vitest`) fails `sh: 1: <tool>: not found`. That failure is indistinguishable from a real test/lint failure -> false-red gates + wasted cycles + inflated fleet error signal. ## Fix (both approaches implemented, defense-in-depth) **Approach 1 (primary correctness fix)** — `plugins/mosaic-framework/src/index.ts`: this is the actual code that mechanically injects the "Git Worktree Requirement — MANDATORY" instruction block into every agent session and ACP worker spawn (native OC sessions via `before_agent_start`, and Codex/Claude ACP spawns via `subagent_spawning`). It is the concrete, executing worktree-add path in this repo — worktree creation itself is carried out by the worker following this injected instruction sequence, so fixing the instruction is the real fix. Added `pnpm install --frozen-lockfile --prefer-offline` (matching the CI install pattern in `.woodpecker/ci.yml`) immediately after `git worktree add`/`cd`, before "... all work happens here ...", plus an explanatory paragraph telling the worker never to skip/reorder it ahead of any gate. **Approach 2 (defense-in-depth, legible failure)** — `packages/mosaic/framework/tools/qa/qa-hook-stdin.sh`: this is the single common gate-entry seam named in the issue itself — it's registered as the `PostToolUse` hook for every `Edit|MultiEdit|Write` in `packages/mosaic/framework/runtime/claude/settings.json`, i.e. the one wrapper the delivery cycle invokes on every code edit across the fleet, regardless of runtime/worker. Added a preflight, scoped to JS/TS files only (matching the existing skip logic already in the script), that checks for `node_modules/.bin` presence/non-emptiness and, if absent, emits the explicit sentinel `deps not installed — run pnpm install` (to stderr and the QA log) and exits nonzero — instead of silently proceeding (previous behavior: exit 0, write a QA report as if nothing were wrong) into a downstream raw not-found. No per-command checks were scattered into individual gates; the check lives in exactly one place. ## Red-first evidence (throwaway fixture — no live worktree/worker touched) 1. **RED**: with a fresh fixture repo and `PATH` containing no `node_modules/.bin`, invoking a gate binary directly (`sh -c 'tsc --noEmit'`) produces the raw `sh: 1: tsc: not found` (exit 127) — the defect. The pre-fix `qa-hook-stdin.sh` (checked out at `main`) does not intercept this at all — it exits 0 and writes a QA report as if the edit were clean, letting the raw not-found reach the operator unannotated whenever a gate is later invoked. 2. **GREEN — approach 2**: post-fix `qa-hook-stdin.sh` run against the same empty/missing `node_modules/.bin` fixture now exits 1 and prints exactly `deps not installed — run pnpm install` instead of silently passing. 3. **GREEN — approach 1**: after simulating the worktree-bootstrap install (populating `node_modules/.bin` with a stub `tsc`), the same gate-entry seam proceeds normally (exit 0) and the gate binary itself now resolves and runs successfully instead of hitting not-found. This is codified as a permanent regression test, `packages/mosaic/framework/tools/qa/test-deps-preflight.sh`, following the existing `test-*.sh` bash convention in this tree (see `packages/mosaic/framework/tools/git/test-help-exit-code.sh`, `packages/mosaic/framework/tools/codex/test-pr-diff-context.sh`), covering: missing `node_modules/.bin`, present-but-empty `node_modules/.bin`, populated `node_modules/.bin` (no false positive), and non-JS/TS files (existing skip preserved). It is wired into the existing `test:framework-shell` script in `packages/mosaic/package.json`, run as part of `pnpm test`. ## Gates (all run, verbatim below) - `pnpm typecheck` — 42/42 tasks successful. - `pnpm lint` — 23/23 tasks successful. - `pnpm format:check` — "All matched files use Prettier code style!" - `pnpm test` — 43/43 tasks successful (turbo), including the new `deps-preflight regression passed (5/5 scenarios)` line inside `test:framework-shell`, and the full `@mosaicstack/mosaic` vitest suite (1434 tests passed), `@mosaicstack/gateway` (628 passed / 12 skipped), `@mosaicstack/db` (19 passed / 7 skipped). - `shellcheck -x` on both changed/added shell files (`qa-hook-stdin.sh`, `test-deps-preflight.sh`) — clean, no findings. No pre-existing unrelated failures were observed in this run; no new failures were introduced by this change. ## Scope Framework tooling only (`plugins/mosaic-framework/src/index.ts`, `packages/mosaic/framework/tools/qa/`, `packages/mosaic/package.json` test wiring). No production application code touched. Closes #856. NO self-merge — awaiting named-executor merge clearance; merge to be timed by coordinator to a no-critical-lane-mid-bootstrap seam. Author lane: ms-856-build.

jason.woltje added 1 commit 2026-07-20 09:17:52 +00:00

fix(framework): install deps on worktree bootstrap + legible deps-preflight at gate seam (#856) ... 1e6937c488

Worker git-worktrees under a fresh `git worktree add` have empty
node_modules/.bin (pnpm workspaces do not share node_modules across
worktrees), so every gate binary (tsc/eslint/prettier/vitest) fails with
a raw `sh: 1: <tool>: not found` before the QA gate suite ever runs a
real check. That failure is indistinguishable from a genuine test/lint
failure -- a false-red gate that was responsible for 131/147 fleet
tool-errors.

- plugins/mosaic-framework/src/index.ts: the injected "Git Worktree
  Requirement" instructions now direct every worker to run
  `pnpm install --frozen-lockfile --prefer-offline` immediately after
  `git worktree add`/`cd`, before any gate is invoked.
- packages/mosaic/framework/tools/qa/qa-hook-stdin.sh: the common
  gate-entry seam (registered as the PostToolUse hook for every
  Edit/Write/MultiEdit in runtime/claude/settings.json) now preflights
  node_modules/.bin presence for JS/TS files and fails with an explicit
  "deps not installed -- run pnpm install" sentinel instead of silently
  letting a raw not-found surface downstream.
- packages/mosaic/framework/tools/qa/test-deps-preflight.sh: new
  regression harness (wired into test:framework-shell) covering missing
  node_modules/.bin, empty node_modules/.bin, populated node_modules/.bin,
  and non-JS/TS skip behavior.

Closes #856.

jason.woltje commented 2026-07-20 09:23:51 +00:00

Author

Owner

Copy Link

Durable exact-head Reviewer-of-Record — APPROVE (do not merge without coordinator named-executor clearance)

Exact head bound: 1e6937c488d98145d44ff486f6c56db5ceec533d (confirmed unchanged at review start AND end — no drift).

Independent CODE review — author lane ms-856-build != reviewer lane ms-858-review (distinct network-capable lane; strictly READ-ONLY — only Gitea API GETs + a throwaway clone under /home/hermes/ms-858-review, since removed; no commits/pushes/approvals/comments via any tool). Persisted here as a durable comment (the tea pr approve durable path is only repaired once #857 deploys). VERDICT: APPROVE.

• Scope (verified /pulls/858/files + git diff --stat): exactly 4 files, all framework-tooling — plugins/mosaic-framework/src/index.ts (+11/-1), packages/mosaic/framework/tools/qa/qa-hook-stdin.sh (+15/-0), packages/mosaic/framework/tools/qa/test-deps-preflight.sh (+116/-0, new), packages/mosaic/package.json (+1/-1). Zero production application code (apps/, packages/db, packages/gateway) touched.
• Approach 1 (index.ts): pnpm install --frozen-lockfile --prefer-offline injected immediately after cd into the new worktree, before any work/gate — correct cwd, sane flags. It is instructional/prompt text injected into agent context (advisory, matching the surrounding section), so it cannot itself fail-loud and block a spawn.
• Approach 2 (qa-hook-stdin.sh preflight, L53-66): sits AFTER the existing unchanged JS/TS regex gate (L48-51), so it fires only for .ts/.tsx/.js/.jsx/.mjs/.cjs. BIN_DIR=$PROJECT_ROOT/node_modules/.bin via git rev-parse --show-toplevel resolves to the current worktree's own root (verified live with a real git worktree add — returns the worktree path, not the main checkout), so the check is genuinely per-worktree. Exits nonzero only when that dir is absent/empty. Single check at the one common seam — no scattered per-command checks, no auto-install (matches the seam-only constraint).
• No #812 regression: merge-base→head diff on qa-hook-stdin.sh is a pure 15-line insertion at one point; the pre-existing JSON parse, jq/fallback extraction, logging, and qa-hook-handler.sh dispatch are byte-identical to merge-base.
• Red-first genuine (independently reproduced): the NEW test-deps-preflight.sh harness run against the PRE-FIX hook (merge-base 11d281845376eb5910a74403300956d8edcc93a8) FAILS (missing node_modules/.bin: expected nonzero exit, got 0; empty node_modules/.bin: ... got 0; exit 1) — i.e. pre-fix the hook silently exits 0 into the handler with deps entirely missing (the genuine false-red root cause). Against the post-fix head hook: deps-preflight regression passed (5/5 scenarios), exit 0. Also reproduced outside the harness with a real fresh git worktree add (no node_modules) → deps not installed — run pnpm install, exit 1. No scenario is tautological (each pipes real JSON through the real hook, asserts on live exit code + stderr).
• Integrity: no --no-verify/skip, no .only/.skip, no weakened/removed assertions, no commented-out logic, no mocking around a real failure.
• Shell quality: shellcheck -x on both shell files = 0 findings.
• package.json: the +1/-1 appends && bash framework/tools/qa/test-deps-preflight.sh to the existing test:framework-shell chain — nothing else changed; runs green.

Non-blocking notes (not defects): (1) a plain non-pnpm JS repo with no node_modules would also hard-stop at the preflight — but pre-fix that case degraded into an eventual raw not found gate failure anyway, so not a regression; outside the stated pnpm-worktree scope. (2) test scenario 1's assertion depends on tsc not being globally resolvable on a scoped PATH — an environmental assumption worth noting for CI portability.

Author != reviewer preserved (author ms-856-build, reviewer ms-858-review). Merge authority is the coordinator's (Mos) — NO self-merge; awaiting a NAMED-EXECUTOR clearance.

### Durable exact-head Reviewer-of-Record — APPROVE (do not merge without coordinator named-executor clearance) **Exact head bound:** `1e6937c488d98145d44ff486f6c56db5ceec533d` (confirmed unchanged at review start AND end — no drift). **Independent CODE review** — author lane `ms-856-build` != reviewer lane `ms-858-review` (distinct network-capable lane; strictly READ-ONLY — only Gitea API GETs + a throwaway clone under `/home/hermes/ms-858-review`, since removed; no commits/pushes/approvals/comments via any tool). Persisted here as a durable comment (the `tea pr approve` durable path is only repaired once #857 deploys). **VERDICT: APPROVE.** - **Scope (verified `/pulls/858/files` + `git diff --stat`):** exactly 4 files, all framework-tooling — `plugins/mosaic-framework/src/index.ts` (+11/-1), `packages/mosaic/framework/tools/qa/qa-hook-stdin.sh` (+15/-0), `packages/mosaic/framework/tools/qa/test-deps-preflight.sh` (+116/-0, new), `packages/mosaic/package.json` (+1/-1). Zero production application code (`apps/`, `packages/db`, `packages/gateway`) touched. - **Approach 1 (index.ts):** `pnpm install --frozen-lockfile --prefer-offline` injected immediately after `cd` into the new worktree, before any work/gate — correct cwd, sane flags. It is instructional/prompt text injected into agent context (advisory, matching the surrounding section), so it cannot itself fail-loud and block a spawn. - **Approach 2 (qa-hook-stdin.sh preflight, L53-66):** sits AFTER the existing unchanged JS/TS regex gate (L48-51), so it fires only for `.ts/.tsx/.js/.jsx/.mjs/.cjs`. `BIN_DIR=$PROJECT_ROOT/node_modules/.bin` via `git rev-parse --show-toplevel` resolves to the current worktree's own root (verified live with a real `git worktree add` — returns the worktree path, not the main checkout), so the check is genuinely per-worktree. Exits nonzero only when that dir is absent/empty. Single check at the one common seam — no scattered per-command checks, no auto-install (matches the seam-only constraint). - **No #812 regression:** merge-base→head diff on qa-hook-stdin.sh is a pure 15-line insertion at one point; the pre-existing JSON parse, jq/fallback extraction, logging, and `qa-hook-handler.sh` dispatch are byte-identical to merge-base. - **Red-first genuine (independently reproduced):** the NEW `test-deps-preflight.sh` harness run against the PRE-FIX hook (merge-base `11d281845376eb5910a74403300956d8edcc93a8`) FAILS (`missing node_modules/.bin: expected nonzero exit, got 0`; `empty node_modules/.bin: ... got 0`; exit 1) — i.e. pre-fix the hook silently exits 0 into the handler with deps entirely missing (the genuine false-red root cause). Against the post-fix head hook: `deps-preflight regression passed (5/5 scenarios)`, exit 0. Also reproduced outside the harness with a real fresh `git worktree add` (no node_modules) → `deps not installed — run pnpm install`, exit 1. No scenario is tautological (each pipes real JSON through the real hook, asserts on live exit code + stderr). - **Integrity:** no `--no-verify`/skip, no `.only`/`.skip`, no weakened/removed assertions, no commented-out logic, no mocking around a real failure. - **Shell quality:** `shellcheck -x` on both shell files = 0 findings. - **package.json:** the +1/-1 appends `&& bash framework/tools/qa/test-deps-preflight.sh` to the existing `test:framework-shell` chain — nothing else changed; runs green. **Non-blocking notes (not defects):** (1) a plain non-pnpm JS repo with no node_modules would also hard-stop at the preflight — but pre-fix that case degraded into an eventual raw `not found` gate failure anyway, so not a regression; outside the stated pnpm-worktree scope. (2) test scenario 1's assertion depends on `tsc` not being globally resolvable on a scoped `PATH` — an environmental assumption worth noting for CI portability. Author != reviewer preserved (author `ms-856-build`, reviewer `ms-858-review`). Merge authority is the coordinator's (Mos) — NO self-merge; awaiting a NAMED-EXECUTOR clearance.

jason.woltje commented 2026-07-20 09:38:10 +00:00

Author

Owner

Copy Link

Named-executor merge — PROVENANCE STAMP (pre-mutation)

Executor: Mos (coordinator / merge-authority). Host web1 · tmux pane mos-claude:0.0 · PID 3935025 · UTC 2026-07-20T09:38:10Z.

Clearance — all 6 checks verified primary-source by the executor, plus dual independent lane verification (MS-LEAD read-back 093108Z + homelab independent 092722Z/093207Z):

1. PR #858 open / unmerged / mergeable / base main, 4 framework-tooling files, no production code.
2. CI pipeline #1943 (pull_request) terminal success at exact head 1e6937c488d98145d44ff486f6c56db5ceec533d (all 9 steps green, poller-confirmed).
3. Durable Reviewer-of-Record comment #18384 = APPROVE (7/7 checks), author-lane ms-856-build ≠ reviewer-lane ms-858-review, bound to 1e6937c4, no REQUEST-CHANGES (2 non-blocking notes only).
4. reviewed-SHA == CI-SHA == PR head, all full-40 1e6937c488d98145d44ff486f6c56db5ceec533d.
5. Body carries Closes #856.
6. CI queue-guard clear (exit 0). #858 disjoint from #857 → clean pure-forward on main 3b70c66c (prospective tree 6999c5c1), no rebase.

Deployment note: this squash merge to main does NOT auto-deploy the framework to the fleet; the worktree-bootstrap install change activates only at the separate framework re-install step, which the executor gates to a no-worker-mid-bootstrap seam. Merging now is non-disruptive to in-flight lanes (e.g. ms-850-build's existing worktree is unaffected).

Merging now via squash. Written BEFORE the merge mutation per the named-executor provenance rule (xref #855).

### Named-executor merge — PROVENANCE STAMP (pre-mutation) **Executor:** Mos (coordinator / merge-authority). Host `web1` · tmux pane `mos-claude:0.0` · PID `3935025` · UTC 2026-07-20T09:38:10Z. **Clearance — all 6 checks verified primary-source by the executor, plus dual independent lane verification** (MS-LEAD read-back 093108Z + homelab independent 092722Z/093207Z): 1. PR #858 open / unmerged / mergeable / base `main`, 4 framework-tooling files, no production code. 2. CI pipeline #1943 (pull_request) terminal **success** at exact head `1e6937c488d98145d44ff486f6c56db5ceec533d` (all 9 steps green, poller-confirmed). 3. Durable Reviewer-of-Record comment #18384 = **APPROVE** (7/7 checks), author-lane `ms-856-build` ≠ reviewer-lane `ms-858-review`, bound to `1e6937c4`, no REQUEST-CHANGES (2 non-blocking notes only). 4. reviewed-SHA == CI-SHA == PR head, all full-40 `1e6937c488d98145d44ff486f6c56db5ceec533d`. 5. Body carries `Closes #856`. 6. CI queue-guard clear (exit 0). #858 disjoint from #857 → clean pure-forward on main `3b70c66c` (prospective tree `6999c5c1`), no rebase. **Deployment note:** this squash merge to `main` does NOT auto-deploy the framework to the fleet; the worktree-bootstrap install change activates only at the separate framework re-install step, which the executor gates to a no-worker-mid-bootstrap seam. Merging now is non-disruptive to in-flight lanes (e.g. ms-850-build's existing worktree is unaffected). Merging now via **squash**. Written BEFORE the merge mutation per the named-executor provenance rule (xref #855).

jason.woltje merged commit acd7d380f6 into main 2026-07-20 09:38:13 +00:00

jason.woltje referenced this issue from a commit 2026-07-20 09:38:14 +00:00

fix(framework): install deps on worktree bootstrap + legible deps-preflight at gate seam (#856) (#858)

jason.woltje referenced this issue from a commit 2026-07-20 09:39:03 +00:00

mos: #858 MERGED (squash acd7d380 on main), executor=Mos (prov 18387), #856 CLOSED; #857 terminal-complete (#1944 green); #850 in flight

jason.woltje referenced this pull request 2026-07-20 10:03:44 +00:00

fix(#812 follow-up): normalize detect-platform.sh host-match port comparison by scheme #859

jason.woltje referenced this pull request 2026-07-20 10:05:15 +00:00

flaky test: mosaic "whole mutator-class lease gate > observer revocation and monotonic TTL expiry deny the next mutator" (timing flake) blocks #856/#858 terminal completion #860

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

fleet-enhancement

Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

jason.woltje

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: mosaicstack/stack#858