fix(conformance): preserve the pinned pnpm runtime - #95
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
BunsDev
added
bug
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The Unix pnpm runtime root validation is currently inconsistent with the updated test fixture and will reject valid layouts, likely breaking the Unix supervisor test/CI gate.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the Phase 1 conformance harness to preserve the full content-addressed pnpm runtime across the Unix broker→runner UID transition, install a fixed trusted pnpm wrapper, and ensure the pinned Node installation is present in the restricted Windows child PATH. It also repins the Phase 1 conformance lock/authority to the new immutable revision and updates the corresponding documentation and workflow assertions.
Changes:
- Copy and validate the entire pnpm runtime tree (not just the pnpm launcher) into the Unix trusted directory and invoke it via the trusted Node copy.
- Add Windows child PATH coverage for the pinned Node root and update workflow tests accordingly.
- Repin Phase 1 conformance authority hashes/revisions and refresh docs/workflow digests to match.
File summaries
| File | Description |
|---|---|
src/phase1-conformance-lock.test.ts |
Updates expected Phase 1 harness authority revision/tree and referenced blob/digests. |
src/client-v1-conformance-workflow.test.ts |
Adds assertions for Windows child PATH including the pinned Node root and for trusted pnpm runtime handling. |
scripts/unix-producer-supervisor.test.sh |
Updates the Unix supervisor fixture to model a cmd-shim pnpm launcher targeting pnpm.cjs in a runtime directory. |
scripts/unix-producer-supervisor.sh |
Implements pnpm launcher target parsing, validates/copies the full pnpm runtime tree, and installs a fixed trusted wrapper. |
phase1-conformance.lock.json |
Repins the harness/harnessAuthority revision/tree and updates file blob/digest entries. |
docs/phase1-conformance.md |
Documents the new pnpm runtime preservation approach and updates byte/sha256 tables. |
.github/workflows/client-v1-conformance.yml |
Prepends $nodeRoot to the restricted Windows child PATH and updates expected digest/size entries. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| pnpm_cli="$( | ||
| LC_ALL=C /usr/bin/sed -n 's/^# cmd-shim-target=//p' "$pnpm_executable" | ||
| )" | ||
| if [[ "$pnpm_cli" != /* || "$pnpm_cli" == *$'\n'* ]]; then | ||
| echo 'unix-producer-supervisor: pnpm launcher target is unsafe' >&2 | ||
| exit 1 | ||
| fi | ||
| pnpm_cli="$(canonical_file "$pnpm_cli")" || | ||
| { echo 'unix-producer-supervisor: pnpm CLI entrypoint is unsafe' >&2; exit 1; } | ||
| pnpm_bin_directory="$(canonical_directory "$(dirname "$pnpm_cli")")" || | ||
| { echo 'unix-producer-supervisor: pnpm bin directory is unsafe' >&2; exit 1; } | ||
| pnpm_runtime_root="$(canonical_directory "$(dirname "$pnpm_bin_directory")")" || | ||
| { echo 'unix-producer-supervisor: pnpm runtime root is unsafe' >&2; exit 1; } | ||
| if [[ "$(basename "$pnpm_runtime_root")" != pnpm || | ||
| "$pnpm_cli" != "$pnpm_runtime_root/bin/pnpm.cjs" ]]; then | ||
| echo 'unix-producer-supervisor: pnpm launcher target is unexpected' >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Resolved the concrete fixture mismatch in b3d9103: the privileged fixture now uses the real node_modules/pnpm/bin/pnpm.cjs package shape, and Linux CI passed that happy path. The basename check is intentional fail-closed binding to the exact pnpm package root produced by the pinned action rather than accepting an arbitrary parent tree. Commit 9198890 also rejects carriage returns explicitly, with workflow and privileged native regressions; 6c739f6 repins the resulting immutable authority.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Preserves the complete content-addressed pnpm runtime across the Unix broker-to-runner UID transition, installs a fixed trusted wrapper, and includes the pinned Node installation in the restricted Windows child PATH. Adds regressions and repins the immutable Phase 1 authority in a separate commit. Protected run 33918622169 is not release evidence for OpenCoven/sdk#38; publication remains disabled. Local lint, typecheck, unit/heavy tests, build, Rust/native gates, and Unix supervisor tests pass. Apply ci:full for Linux, macOS, and Windows acceptance.