pnpm migration: pinned pnpm 10.34.5, four authoritative locks, converted tooling/CI/cloud/sandbox/docs - #820
Merged
Merged
Conversation
…sults in pnpm migration plan
…record base waiver in checklist, sequence native matrix after push
…ed lock with distinct peer contexts
…ild-process tests
…ger through the shared helper
…manager through the shared helper
…m repair Replace the npm-ci + npm-lock-v3-parser sidecar installer with a readiness-first gate: exact SDK pin + entrypoint + platform native package checks, an md5 fingerprint of the sidecar manifest, workspace lock, and workspace config recorded in node_modules .claude-sidecar-ready.json, and a single frozen, sidecar-filtered pnpm install at the workspace root as the only repair. Current installations return a receipt without installing; failures fail before server spawn with no mutable recovery.
…ple image Sandbox dependency prep (plan §7.3): install the pinned pnpm@10.34.5 in the image via the npm bootstrap, and replace the one-shot .sandbox-npm-ci-done marker with node_modules/.sandbox-deps-state keyed by a sha256 fingerprint over manager identity, install policy flags, the root manifest, the selected manager's lock/config files, and the workspace member manifests. Manager selection mirrors scripts/lib/package-manager.ts: packageManager is authoritative; absent fields fall back to lock presence with package-lock.json first so legacy npm-era branches keep their npm ci path. A mismatched/missing state purges node_modules and re-runs the frozen install; an unchanged fingerprint reuses it; the old marker never satisfies the new state. sandbox-selftest grows Proof 6 with hermetic transition cases (pnpm tree + stale npm marker/volume, unchanged-fingerprint reuse, changed fingerprint reinstall, legacy npm selection) driven through the real entrypoint; Proof 3a now opens the mktemp mount modes that rootless docker would otherwise make unreadable to the non-root reader. examples/docker client-builder installs pnpm against the workspace lock (--ignore-scripts preserved) before building the client; the final runtime image stays minimal (Rust binary + static client only).
…ount-gated purge Proof runs against the real bind-mounted repo exposed two hard blockers for pnpm-as-sandbox installs, plus one safety gap: - pnpm's default store-path resolution (storePathRelativeToHome) probes the project root's writability by touching a _tmp_ file next to the root manifest; on the root-owned bind mount that EACCES killed every install. The entrypoint install and the image env now pin the store to the same canonical per-user location pnpm would pick (/home/sandbox/.local/share/pnpm/store). - pnpm workspace members with their own dependencies need writable member node_modules; the wrapper mounts named volumes there (like the existing root node_modules/target volumes), and the entrypoint chowns them only when the member dir exists and a volume is actually mounted. - The pre-install purge now deletes only mounted node_modules dirs: a direct docker run without the wrapper's volumes can no longer destroy the user's real node_modules through the bind mount; such runs fail closed on the install instead. The selftest's Proof 6 fixtures mirror the production topology: fixture trees bind-mounted at /workspace with throwaway named volumes at /workspace/node_modules, stale npm-era content planted into the volume, and assertions taken from the containers' own output.
An empty-deps fixture makes the frozen install a no-op, and pnpm's no-op headless path rewrites the wanted lockfile through a temp file in the project root — which the read-only bind mount cannot host (EACCES). The real repo flow links real packages and provably never writes the root, so the fixture now pins ms@2.1.3 to drive the same path.
The T15 whole-branch review found the §4.3 composition surfaces were never converted: package.json nested npm run calls, launch-rust.sh, the MCP stdio e2e helper, measure-bandwidth/amplifier-backfill usage strings, the perf-audit receipt label, and four standalone shell tests still dispatched through npm/npx. - package.json: all 11 nested calls now pnpm run; test:sequential drops npm's -- forwarding separator per the §4.1 mapping. - launch-rust.sh: typecheck:client, build:client, and the silent prepare:rust-runtime dispatch through pnpm run. - mcp-stdio-client.ts: ensureMcpServerBuilt resolves the project manager through the shared helper (test/setup/manager-command.ts), matching global-setup.ts. - measure-bandwidth.ts / amplifier-backfill-bundle.ts: npx tsx usage becomes pnpm exec tsx (file was never +x; shebang updated anyway). - run-visible-first-audit.ts receipt records the pnpm command label. - cloud-vitest-integration, cloud-e2e-retry-receipt, cloud-run-config, cloud-run-wrapper shell tests dispatch via pnpm exec / pnpm run with npm's -- separators dropped. - Dockerfile: playwright browser bootstrap uses pnpm dlx per §7.2.4 (m-5; exactly versioned, build-time only). - Text stragglers converted (checkout-free lane hint, electron report hint, fresh-agent control manual-run note, amplifier-launch-smoke manual-run note); demos.yml cites §3.3 item 5 (n-1). - electron-builder-config.test.ts: the text-only composition-string assertions are REMOVED per §9.3, not rewritten (m-4); behavior coverage lives in the real electron:build chain. Focused: typecheck green; electron-builder-config 9/9 under the electron config; all four shell tests green on the new spellings; ensureMcpServerBuilt ran a real build through the shared helper.
…m-3) verify-workspace-peers.mjs was the only structural assert of the load-bearing peer split (root zod 4.3.6 vs sidecar zod 4.4.3 plus the sidecar/MCP-runtime SDK pins) but nothing executed it. It now runs in the default unit lane via test/unit/scripts/verify-workspace-peers.test.ts: - zero-arg invocation enforces the real workspace lock on every run; - an explicit lock-path argument targets fixture locks, letting the test prove drift detection (tampered root zod -> exit 1 naming the observed version) and fail-closed structure checks without mutating the repo. Red/green: the fixture tests failed against the script before the argv path existed (it silently checked the real lock and exited 0).
…ser install The final gate's image build failed at 'RUN pnpm dlx playwright@1.58.2': exit 127 — pnpm was not on PATH yet in stage-3, because the pinned-pnpm bootstrap (npm install -g pnpm@10.34.5) came after the browser install. The old 'npx --yes' spelling worked only because npx ships with the node base image. Reorder: bootstrap pnpm first, then dlx playwright (exactly versioned, build-time only). Build receipt: Cloud Build bc2d0d5a failed pre-fix; post-fix image build verified green.
- amplifier-stub-adoption-contract: the opt-in run hint uses the repo-owned pnpm run test:vitest path without npm's -- separator (its sibling file was converted with M-1; this one was missed). - component-edge-cases: the file-header run hint says pnpm instead of npx vitest, matching the repo-owned direct path. - mcp-stdio-client: the fallback error message is manager-neutral, matching the primary 'manager run build:tools failed' wording. Comment/message-only; typecheck green, touched unit file 60/60. The dual-era analyzer fixture (rust-only-server-runtime) and the coordinator display fixture keep their intentional npm-era strings.
danshapiro
force-pushed
the
the-usual/pnpm-migration
branch
from
September 23, 2026 02:48
e0781a0 to
f9cb47a
Compare
…stic The PR's Windows CI exposed two test bugs written under WSL (the class the plan's step-9 native matrix was designed to catch, surfaced early): 1. prepare-electron-runtime: the test demanded the exec-bit chmod on every host, but the stager deliberately skips the operation for win32 targets (Windows executability comes from the .exe extension; NTFS does not retain POSIX bits). The assertion now expects the operation on POSIX hosts and its ABSENCE on a win32 host staging a win32 runtime. 2. verify-electron-artifact: the 'different platform' receipt fixture hardcoded 'win32', which only mismatches on POSIX hosts — on the Windows runner the receipt matched the host and the verifier correctly accepted it. The fixture now picks a platform that differs from the host on every runner. Local (POSIX) proof: 38/38 green under the electron config; the CI Windows job re-runs as the native proof.
pull Bot
pushed a commit
to HinchK/freshell
that referenced
this pull request
Sep 23, 2026
…sion point The pnpm-migration run deferred the Darwin-only fsevents install-script decision to the first native macOS matrix run. PR danshapiro#820's macOS CI jobs (macos-latest and macos-15-intel Electron builds) passed with fsevents NOT allow-listed — its script stays blocked with a warning and the builds complete. Record that acceptance next to the allowlist so the next reader knows it is deliberate, not an oversight.
pull Bot
pushed a commit
to HinchK/freshell
that referenced
this pull request
Sep 23, 2026
The T9 dual-manager hook rewrite (merged in danshapiro#820 as fb5d524) lost the '#!/usr/bin/env bash' interpreter line. Git executes hooks directly, and a shebang-less script is executed under sh/dash, whose parser rejects the hook's herestring at the manager-selection block: 'Syntax error: redirection unexpected' at line 180 — every push from every worktree failed. The regression was masked all run: core.hooksPath serves the MAIN checkout's working-tree hook, and local main was fast-forwarded to the danshapiro#820/danshapiro#819 era only after the migration branch had been pushed — so the first push after the fast-forward was the hook's real debut. The T9 hermetic tests invoked the hook via bash explicitly, which is why the gap survived review. Guard tests added to prepush-manager.test.ts: the hook must declare a bash interpreter (proven red against the shebang-less file) and stay parseable by bash -n. Pushed with --no-verify per the documented bypass: the broken hook is the artefact being fixed; the commit is the two-line shebang + guard tests, and CI validates the branch fully on the PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Execute the pnpm migration plan (
docs/plans/2026-09-19-pnpm-migration.md) in full: Freshell's first-party dependency installation, development, build, test, and release workflows now run on exactly pinned pnpm 10.34.5 instead of npm, with no application behavior change.pnpm-lock.yaml(root +crates/freshell-claude-sidecar+packages/freshell-mcp-runtime) plus three independent demo locks (examples/demo-projects/synth,dataviz/viz-a,dataviz/viz-b). The five npmpackage-lock.jsonfiles are removed; frozen installs fail closed on missing/mismatched locks (no mutable recovery). Import fidelity was proven byte-identical; the root zod 4.3.6 / sidecar zod 4.4.3 peer split is preserved and now enforced by a unit-lane check (scripts/verify-workspace-peers.mjs).pnpm deploy, stages receipt-verified, and runs checkout-free (real-SDK acceptance 3/3 against the packaged artifact);electron-builder.ymlpackages the receipt;npmRebuild: falseandnpm_*compatibility variables untouched.pnpm/action-setup..env.example.Verification
e0781a059(cloud vitest 4-shard, cloud e2e, Rust, electron, docker lanes; coordinator receiptfull-suite success exit=0, dirty:0).Deferred (plan step 9, by design)
Native Windows/macOS build proofs on their real runners, fsevents
allowBuildsdecision on the first native macOS run, pnpm-era base-gate branch (until main migrates), and the separately-authorized live-checkout transition for the self-hosted server.