fix(update): support native pnpm self-updates by oliver-mee · Pull Request #4203 · lidge-jun/opencodex

oliver-mee · 2026-09-10T13:05:59Z

Summary

The published updater treats every global installation as npm. In a pnpm-managed global installation, the host's pnpm guard receives npm-only flags and the update stops after proxy shutdown. This PR adds a manager-aware path that lets pnpm own the global group, dependency tree, shims, and rollback, while preserving npm's existing transactional path. It also performs registry integrity checks before shutdown and uses the verified active launcher for recovery.

Linked issue

Fixes #4202

Type of change

How to test

bun test tests/update/update-pnpm.test.ts passed with 26 tests and 0 failures.
The broader relevant update, launcher, install-script, and layout suite passed with 176 tests, 2 existing Bun-runtime skips, and 0 failures.
bun run typecheck, bun run privacy:scan, bun run skill:surface:check, changed ESM syntax checks, git diff --check, and the layout suite passed.
The Astro documentation build passed with 425 pages.
An isolated real-pnpm smoke updated OpenCodex from 2.48.0-preview.20260908 to 2.49.0 and confirmed the generated ocx shim reported 2.49.0.

Checklist

One concern only, scope limited to the summary above
I ran it and verified end to end (or explained below why that does not apply)

The end-to-end update check used isolated XDG and PNPM_HOME directories. The live installation was not modified.

Model used

GPT-5.6-luna via Codex, with GPT-5.6 Sol used for the independent review and findings pass.

🤖 Generated with Codex (GPT-5.6-luna)
🧑‍💻 Ideated, directed and reviewed by a human, @oliver-mee

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.

Bind pnpm updates to the global installation that owns OpenCodex, verify package trees and generated shims during recovery, and launch through the verified active package. Add manager-aware integrity preflight, focused tests, and documentation while keeping the npm transactional path intact.

github-actions · 2026-09-10T13:06:16Z

✅ Deterministic PR hygiene checks passed.

github-actions · 2026-09-10T13:06:32Z

⏳ DRAFT

review readiness checklist open (0/4 boxes ticked).

What to do

Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

⬜ All CI tests are green on my local testing.
⬜ I pushed my PR to the latest dev commit.
⬜ I resolved all correct Codex and CodeRabbit findings.
⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

Hygiene

✅ Deterministic PR hygiene checks passed.

coderabbitai · 2026-09-10T13:06:49Z

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

Ingwannu

The pnpm installation boundary is worth supporting, but the new shared verifier changes npm's existing staging/recovery contract as well.

In src/update/transactional-install.mjs, verifyInstallTree now delegates to dependencyPackageDir/createRequire.resolve. That resolution can find dependencies in ancestor node_modules outside the candidate package tree. The old npm verifier specifically checked the staged package's own node_modules. A candidate missing its own bundled Bun or sentinel dependency must not pass merely because an ancestor installation supplies one; otherwise staging/boot recovery can call a non-self-contained candidate healthy and discard or replace the known-good copy.

Please preserve the strict npm candidate-tree verifier and add an explicit pnpm-aware ownership/graph check for the separate pnpm layout. Cover a missing/truncated candidate Bun with an unrelated ancestor Bun present, a legitimate pnpm virtual-store layout, and the rollback/boot-restore decision. This is not a request to remove Windows/Bun workarounds or loosen the manifest check.

This is a blocking focused finding, not a complete approval of the rest of this large updater change. Package-manager selection, global-prefix ownership, lifecycle stop/recovery, and exact-head full CI still need their own completed review. The reported isolated pnpm smoke is useful but does not establish preservation of the npm recovery invariant.

lidge-jun · 2026-09-10T16:56:30Z

리뷰 · 우선순위 61 / 80

이 PR은 이슈 #4202를 겨냥합니다. 지금 dev(cc871a876)의 src/update/index.ts에서 detectInstall()은 source / bun / npm 세 갈래만 있고, 경로에 node_modules가 있으면 bun이 아니면 전부 npm으로 봅니다. updateCommand와 bin/ocx.mjs·src/update/transactional-install.mjs는 npm 전용 플래그(--allow-scripts=bun, --no-audit, --no-fund)를 붙인 채 전역 업데이트를 돌립니다. pnpm 전역 설치에서는 그 플래그가 거절되어, 프록시를 이미 멈춘 뒤에 업데이트가 실패하는 #4202 재현과 맞습니다. 그래서 방향 자체는 현재 dev에 필요한 설치/패키징 버그 수정입니다.

다만 패치 표면이 매우 큽니다(+2500/−200줄대). src/update/pnpm-global-install.mjs, pnpm-invocation.mjs, install-detection.mjs, bin/ocx.mjs, job/badge/문서/대량 테스트까지 한 번에 들어갑니다. 드래프트·머지 상태도 아직 불명확하고, npm 트랜잭션 경로를 유지한다고 했지만 런처·무결성·복구·shim 검증이 한꺼번에 바뀌므로 회귀 비용이 큽니다. types/config 대형 분할 캠페인과는 무관하지만, 업데이트 서브시스템을 통째로 키우는 변경이라 랜딩 전에 범위를 한 번 더 잘라 보는 편이 안전합니다.

src/update/index.ts detectInstall - HEAD는 pnpm을 구분하지 않습니다. PR이 Installer에 pnpm을 넣는다면 경로 판별(전역 store vs npm prefix) 오탐이 없는지, Windows에서 특히 확인이 필요합니다.
src/update/transactional-install.mjs / bin/ocx.mjs - 지금도 npm 플래그를 하드코딩합니다. pnpm 분기에서 그 플래그를 절대 전달하지 않는지, 실패 시 프록시를 멈추기 전에 거절하는지(#4202 기대)가 핵심입니다.
src/update/pnpm-global-install.mjs - 신규 대용량 모듈입니다. 패키지 트리·shim·active package 검증이 실제 전역 pnpm 레이아웃과 어긋나면 복구 경로가 더 위험해질 수 있습니다.
tests/update/update-pnpm.test.ts - 픽스처 경로 익명화 커밋이 있는 점은 좋습니다. 전체 CI/크로스플랫폼 그린과 드래프트 체크리스트가 비어 있으면 ready로 보지 마세요.

메인테이너의 판단이 필요한 지점

한 방에 pnpm-native 전체 경로를 넣을지, 1차로 pnpm 감지 시 프록시 정지 전 안전 에러만 랜딩할지.
npm transactional 경로와의 듀얼 유지 비용과, Installer 타입 확산이 job/badge/notify까지 퍼지는 범위.
[Feature]: atomic ocx update command with daemon drain, schema validation, and multi-client sync #4173(atomic update) / fix(update): retire exited pinned-start children before cleanup #4185 등과 업데이트 오케스트레이션이 겹치는지.

너의 추천
#4202를 막으려면 최소 슬라이스(감지 + 정지 전 실패 메시지, 또는 pnpm 네이티브 add -g 한 경로)로 쪼개 ready 체크리스트를 채운 뒤 머지하세요. 지금 크기 그대로면 드래프트를 유지하고 크로스플랫폼 업데이트 회귀가 그린일 때만 해제하세요. 이슈 #4202는 PR 번호로 연결만 유지하면 됩니다.

이 댓글은 grok-bot이 작성했습니다

@oliver-mee

Global pnpm installations cannot self-update: the updater treats every node_modules installation as npm and forwards npm-only flags (--allow-scripts=bun, --no-audit, --no-fund) to pnpm's global add, which rejects them. The failure lands after the proxy has already been stopped. Carried from #4203 by @oliver-mee, restricted to the update, launcher, service and CLI surface. The install detector now recognises pnpm's isolated, store-link, preserved-symlink and hoisted-group layouts; pnpm gets a native global update path that owns its own group, shims and rollback; and registry integrity is checked before the proxy is stopped rather than after. The shared install-tree verifier is split rather than shared, which answers the blocking review on #4203. verifyInstallTree stays confined to the candidate's own tree: Node's resolver walks the ancestor directory chain, so a global npm candidate at <prefix>/lib/node_modules/@scope/pkg could otherwise satisfy its bundled-Bun requirement from <prefix>/lib/node_modules/bun, which belongs to a different package. Three decisions read that verdict - accepting the stage before the swap, rolling back after it, and reaping the only backup at boot - so a non-self-contained candidate called healthy costs the known-good copy. verifyPnpmInstallTree keeps out-of-package resolution, because pnpm legitimately exposes dependencies through a virtual store, a package-root symlink or a hoisted group, but bounds it: the dependency must be reachable through a root this package instance owns, and an enclosing node_modules counts only when pnpm's own bookkeeping (.pnpm or .modules.yaml) claims it. Ownership is probed lexically rather than filtered from require.resolve output, because the resolver reports the realpath of the resolved file and a dependency reached through pnpm's own symlink comes back as a virtual-store path that no lexical ownership test can recognise. Refs #4203 Closes #4202 Co-authored-by: Oliver Mee <102673257+oliver-mee@users.noreply.github.com>

lidge-jun · 2026-09-11T00:43:27Z

Landed via #4235 at 55c74ac

lidge-jun · 2026-09-11T00:43:29Z

Superseded by #4235 (already on dev at 55c74ac).

oliver-mee added 2 commits September 10, 2026 20:21

test(update): anonymise pnpm fixture paths

e74c6e5

github-actions Bot added the bug Something isn't working label Sep 10, 2026

Ingwannu requested changes Sep 10, 2026

View reviewed changes

Ingwannu mentioned this pull request Sep 10, 2026

[Bug]: global pnpm installations cannot self-update #4202

Closed

2 tasks

lidge-jun mentioned this pull request Sep 10, 2026

docs(devlog): plan the 260911 lane dispatch round #4217

Merged

3 tasks

lidge-jun mentioned this pull request Sep 10, 2026

Experimental: preserve parent prompt-cache prefixes for Desktop side chats #4222

Draft

7 tasks

lidge-jun mentioned this pull request Sep 10, 2026

fix(update): support native pnpm self-updates #4235

Merged

3 tasks

lidge-jun closed this Sep 11, 2026

lidge-jun added the landed-via-maintainer Original PR closed after landing via a maintainer merge train label Sep 11, 2026

Navigation Menu

fix(update): support native pnpm self-updates - #4203

fix(update): support native pnpm self-updates#4203
oliver-mee wants to merge 2 commits into
lidge-jun:devfrom
oliver-mee:feat/pnpm-native-self-update

oliver-mee commented Sep 10, 2026 •

edited by github-actions Bot

Loading

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Sep 10, 2026

Draft PR not reviewed

Uh oh!

Ingwannu left a comment

Uh oh!

lidge-jun commented Sep 10, 2026

Uh oh!

lidge-jun commented Sep 11, 2026

Uh oh!

lidge-jun commented Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

oliver-mee commented Sep 10, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Linked issue

Type of change

How to test

Checklist

Model used

Review readiness checklist

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⏳ DRAFT

What to do

Review readiness checklist

Hygiene

Uh oh!

coderabbitai Bot commented Sep 10, 2026

Draft PR not reviewed

Uh oh!

Ingwannu left a comment

Choose a reason for hiding this comment

Uh oh!

lidge-jun commented Sep 10, 2026

리뷰 · 우선순위 61 / 80

Uh oh!

lidge-jun commented Sep 11, 2026

Uh oh!

lidge-jun commented Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

oliver-mee commented Sep 10, 2026 •

edited by github-actions Bot

Loading

github-actions Bot commented Sep 10, 2026 •

edited

Loading