Skip to content

fix: keep fenced proxies restartable and failed Windows updates recoverable (#5496, #5624) - #5643

Merged
lidge-jun merged 8 commits into
devfrom
codex/260923-p2-update-continuity
Sep 23, 2026
Merged

lidge-jun merged 8 commits into
devfrom
codex/260923-p2-update-continuity

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 23, 2026 •

Copy link
Copy Markdown
Owner

Summary

Two continuity gaps for an installed proxy whose package is updated or replaced.

#5496: a package-tree fenced proxy stays discoverable. When the installed package tree is replaced under a live proxy, the guard answers /healthz, /readyz and /v1/* with 503 package_tree_changed. Liveness treated any non-200 /healthz as "no proxy", so ocx restart reported that no proxy was running and started one that could not bind, and ocx stop's orphan fallback missed it. The automatic drain and restart already on dev (src/server/index/package-tree-guard.ts) is unchanged. This PR closes the manual path:

  • The fenced /healthz withholds readiness but keeps identity. It answers the local attestation challenge and reports restartCapability, plus the installedVersion on disk once the replacement has held for the guard's full stability interval (a readable manifest alone does not mean npm finished extracting).
  • Liveness accepts the fenced 503 only for callers that opt in (ocx restart discovery, the ocx stop and uninstall orphan fallbacks, service stop) and only after this home's runtime record names the same pid and port and the listener proves that record's secret. The pid in the body is never trusted on its own. Ensure, update health waits and replacement waits stay opted out.
  • The bound restart client admits exactly that fenced body. It compares the CLI with installedVersion, because the in-place respawn runs the replaced files at the same path, and refuses with a clear message while that version is unreadable.
  • The invariant is recorded as INV-FENCE-01 (structure/overview.md) and in the package-tree fence section of structure/ops/docs-and-release.md.

#5624: a failed Windows update leaves nothing that blocks the next one and says what to do.

  • Staging directories are created exclusively under a unique name and carry an ownership marker. Cleanup removes the marker last, so a stage held by a locked bunx.exe stays provably owned, and a later update removes marked stages older than a floor set well above the install timeout. Unmarked stages from older updaters, npm's own .opencodex-<random> rename-asides, other packages' stages and links are reported by folder name (no account-bearing paths) and never deleted.
  • A post-swap verification failure moves the rejected tree into the owned stage instead of deleting it in place, so a locked file cannot turn a rollback into a double fault.
  • After a failed npm update the launcher prints the next step for that phase (retry; then ocx stop, reinstall, ocx service restart; or the recovery marker after a double fault) in place of the old "Try manually" line. The dashboard job points at the terminal.
  • New docs page: troubleshooting/update-failed (English; the sidebar label is translated, other locales fall back to the English page) for users on 2.64.0 and earlier whose update fails this way. reference/cli/lifecycle.md links to it.

Established vs not: our updater never creates @bitkyc08/.opencodex-* (that is npm's rename-aside during a direct global install), and the ENOTDIR on mkdir in the report is not explained by this code. The job log withholds the path, so which component was a file is unknown. This PR does not claim to fix that root cause. It makes the failure recoverable and non-blocking.

Plan and audit record: devlog/_plan/260923_p2_update_continuity/.

Closes #5496
Closes #5624

Verification

  • Local checks: NOT RUN. This lane does not run the local suite, focused tests, typecheck or build. Evidence is static review plus hosted CI on the exact head.
  • New tests:
    • tests/ci-workflows/package-tree-fenced-restart.test.ts: real startServer, in order: boot 200, replace the observed tree, fenced 503 without installedVersion until the replacement settles, automatic admission, manual discovery through the real findLiveProxy and real requestBoundSystemRestart joining the pending restart, exactly one handoff; a service child that lost its service home keeps the fence and never hands off.
    • tests/server/proxy-liveness-package-tree-fence.test.ts: opt-in only; wrong secret, unsigned listener, missing or mismatched record, expected-pid mismatch and non-fence 503 bodies are all refused.
    • tests/cli/system-restart-client-package-tree.test.ts: fenced restart accepted when the installed version matches; skew, unsettled tree, missing proof and other non-OK responses refused.
    • tests/update/update-transactional-leftovers.test.ts: an injected failure at each updater step (stage mkdir ENOTDIR, npm failure after partial write, staged verify, live move-aside EXDEV, stage placement, post-swap verify, double fault) keeps or restores the previous version, leads to service restore through planStoppedRuntimeRecovery, and names the next command; a locked leftover survives while recent and is removed once stale; unowned trees are left intact.
  • Hosted CI results on the exact head are reported in a comment below.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. (Identity for a fenced proxy is bound to the per-process attestation secret in the owned runtime record; no credential is logged or added to a response.)

…ested identity (#5496)

A proxy whose installed package tree was replaced fences /healthz, /readyz and
/v1/* with 503 package_tree_changed. Liveness treated any non-200 /healthz as
"no proxy", so ocx restart reported the proxy as not running and fell back to a
start that could not bind, and ocx stop's orphan fallback missed it too.

The fence now withholds readiness but not identity. The fenced /healthz answers
the local attestation challenge and reports restartCapability plus the
installedVersion now on disk. Liveness accepts the 503 only for callers that opt
in (restart discovery, ocx stop and uninstall orphan fallbacks, service stop)
and only after this home's runtime record names the same pid and port and the
listener proves that record's secret; the pid in the body alone is never
identity. The bound restart client admits exactly that fenced body, compares
the CLI with installedVersion because the in-place respawn runs the replaced
files at the same path, and refuses while that version is unreadable.

The invariant is recorded as INV-FENCE-01 in structure/overview.md and in the
package-tree fence section of structure/ops/docs-and-release.md.
…p on failure (#5624)

A Windows update that failed during the staging npm install kept the previous
version and restored the service, but its cleanup was a silent best-effort
rmSync: a locked bunx.exe left the staging tree behind with nothing marking it,
nothing ever swept it, and the dashboard job only recorded
"update command failed (1)".

Staging directories are now created exclusively under a unique name and carry
an ownership marker. Cleanup removes everything but the marker first, so a
partly locked stage stays provably owned, and the next update removes marked
stages older than an in-flight floor. Unmarked stages from older updaters,
npm's own .<name>-<random> rename-asides, other packages' stages and links are
reported and never deleted. A post-swap verification failure moves the
rejected tree into the owned stage instead of deleting it in place, so a locked
file there cannot turn a rollback into a double fault.

After a failed npm update the launcher prints a phase-specific next step (retry,
then stop + reinstall + service restart; or the recovery marker after a double
fault) in place of the old "Try manually" line, and the dashboard job points at
the terminal for it. The ENOTDIR root cause in the report is not established by
this change.
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 23, 2026 04:47
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-23T04:53:12.785050Z 9d80cc6 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

Next included review available in 3 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 8ba73131-1b2e-43c7-966a-535e90c34d00

📥 Commits

Reviewing files that changed from the base of the PR and between 0f9254b and 761b79c.

📒 Files selected for processing (29)
  • bin/ocx.mjs
  • devlog/_plan/260923_p2_update_continuity/000_roadmap.md
  • devlog/_plan/260923_p2_update_continuity/010_fenced_proxy_identity.md
  • devlog/_plan/260923_p2_update_continuity/020_updater_leftovers_and_guidance.md
  • docs-site/astro.config.mjs
  • docs-site/src/content/docs/reference/cli/lifecycle.md
  • docs-site/src/content/docs/troubleshooting/update-failed.md
  • scripts/test-layout/layout.json
  • src/cli/index.ts
  • src/cli/system-restart-client.ts
  • src/lib/package-tree-integrity.ts
  • src/server/index/package-tree-guard.ts
  • src/server/index/serve-options.ts
  • src/server/index/startup-warnings.ts
  • src/server/proxy-liveness.ts
  • src/service/orchestration.ts
  • src/update/job.ts
  • src/update/transactional-install.d.mts
  • src/update/transactional-install.mjs
  • src/update/update-failure-guidance.d.mts
  • src/update/update-failure-guidance.mjs
  • structure/ops/docs-and-release.md
  • structure/overview.md
  • tests/ci-workflows/package-tree-fenced-restart.test.ts
  • tests/cli/system-restart-client-package-tree.test.ts
  • tests/fixtures/test-layout-expected.json
  • tests/server/proxy-liveness-package-tree-fence.test.ts
  • tests/service/stale-state-purge.test.ts
  • tests/update/update-transactional-leftovers.test.ts
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

@github-actions github-actions Bot added bug Something isn't working intake: hygiene-blocked Deterministic PR hygiene checks failed labels Sep 23, 2026
@github-actions

github-actions Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot removed the intake: hygiene-blocked Deterministic PR hygiene checks failed label Sep 23, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

if (fenced && !isHealthzVersion(body.installedVersion)) {
return rejected("restart_package_tree_unsettled");
}
const proxyVersion = fenced
? body.installedVersion as string
: typeof body.version === "string" ? body.version : undefined;

P1 Badge Wait for package-tree stabilization before restarting

When an out-of-band npm install has written a valid package.json but is still extracting bin/, src/, or node_modules/, this check treats the replacement as settled and allows the fenced process to accept the restart request. npm does not make manifest readability an install-completion signal, so ocx restart during that window can terminate the working process and respawn from a partial tree. Gate restart on the guard's stable replacement observation/debounce or an explicit install-completion lock rather than a semver-shaped manifest value alone.


for (const path of result.notOwned) {
log("Not removing " + path + ": this updater did not create it. Delete it by hand once no OpenCodex process is running from it.");

P1 Badge Redact account names from leftover paths

For a user-scoped npm prefix, path commonly contains an account identifier, such as C:\Users\Alice\... or /home/alice/...; bin/ocx.mjs supplies console.log as this logger, so encountering an unowned npm rename-aside or old stage prints that identifier verbatim. Log a basename or home-redacted path instead; the adjacent removed/in-use messages have the same exposure.

AGENTS.md reference: AGENTS.md:L454-L455

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

… by position

The uninstall orphan fallback was outside the audited opt-in set for fenced
identity and is pinned by its own source test, so it keeps plain liveness.
The stop-path ordering test now locates the orphan probe by its call rather
than its exact empty argument list, which the fenced opt-in changed.
…cement settles; log leftover names only

A readable package.json is not an install-completion signal: npm can write the
manifest while it is still extracting the rest of the tree. The fenced
/healthz now reports installedVersion only after the guard's stability
debounce has seen the same replacement identity for the full interval, and
withdraws it whenever the tree moves again, so a manual restart cannot respawn
from a partial tree.

Leftover and staging log lines name the folder only; the full path under a
user-scoped npm prefix carries the account name.
@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 64 / 80

이 PR은 업데이트 중에 프록시가 끊기는 두 구멍을 막아요. 베이스는 dev예요.

패키지 파일이 돌아가는 프록시 밑에서 바뀌면, 프록시는 /healthz에 503을 줘요. 예전에는 이 답을 "프록시가 없다"로 봤어요. ocx restart는 새로 띄우다가 포트에 막히고, ocx stop도 그 프로세스를 놓쳤어요. 이제는 503이어도 이 집의 런타임 기록과 비밀번호 증명이 맞으면, 재시작과 중지는 그 프록시를 알아봐요. 요청을 받을 준비는 아직 아니에요. 디스크의 버전은 패키지 파일이 몇 초 동안 같은 채로 있을 때만 알려 줘요. 그 전이면 ocx restart는 설치가 끝날 때까지 기다리라고 해요.

윈도우에서 npm 업데이트가 실패해도 다음 시도가 막히지 않게 해요. 임시 폴더는 매번 새 이름이고, 우리가 만들었다는 표식을 넣어요. 지울 때는 표식을 맨 마지막에 지워요. 파일이 잠겨 있으면 다음 업데이트가 반시간보다 오래된 우리 폴더만 치워요. 남이 만든 폴더와 npm이 이름을 바꿔 둔 폴더는 이름만 알려 주고 지우지 않아요. 실패한 단계에 따라 다음에 칠 명령을 터미널에 찍어요. 대시보드는 터미널을 보라고만 해요. ENOTDIR의 원인은 이 PR이 고친 것이 아니에요.

src/cli/index.ts:1586 - ocx uninstall은 pid 파일이 없을 때 findLiveProxy()를 옵션 없이 호출해요. 펜스된 503은 여기서 없는 프록시예요. 이어서 하는 확인은 200이 아니면 모르는 상태라서, "프록시를 못 찾았고 꺼졌는지도 확인하지 못했다"에서 멈춰요. ocx stop은 이 경우를 알아보게 고쳤는데, 제거는 빠져 있어요. PR 설명은 제거도 알아본다고 적혀 있어요.

src/lib/package-tree-integrity.ts:66 - 설치가 끝났다는 신호는 package.json 하나의 파일 정보예요. 218행은 그 정보가 5초 동안 같을 때만 버전을 돌려줘요. npm이 package.json을 먼저 쓰고 나머지 파일을 5초보다 오래 풀면, ocx restart와 자동 재시작이 아직 덜 풀린 파일로 다시 뜰 수 있어요. 테스트는 파일 정보를 한 번에 바꾸는 가짜라서 이 경우를 보지 않아요.

src/update/update-failure-guidance.mjs:28 - 이전 버전이 남아 있으면 ocx stop 다음에 다시 설치하라고 해요. 파일을 되돌리다 두 번 실패한 경우에는 그 말이 없어요. 8행 주석은 재설치 전에 프록시를 멈추게 한다고 적혀 있어요.

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

ocx uninstall도 중지처럼 펜스된 프록시를 알아보게 할지 정해 주셔야 해요. 지금 커밋은 검사 범위를 넘어서 일부러 빼 두었어요.

package.json만 5초 보는 것으로 충분한지 정해 주셔야 해요. 글로벌 npm이 완성된 폴더를 한 번에 바꾸면 이 간격으로 충분해요. 살아 있는 폴더 안에 파일을 하나씩 풀면 부족해요.

우리 것이 아닌 임시 폴더는 업데이트할 때마다 다시 출력돼요. 사용자가 지우기 전까지 같은 줄이 반복돼요. 한 번만 알릴지 매번 알릴지 정해 주셔야 해요.

너의 추천

제거의 고아 프록시 찾기에 acceptPackageTreeFenced: true를 넣고, pid 파일이 없는 펜스 프록시를 멈추는 테스트를 추가하세요. 두 번 실패한 안내에도 ocx stop을 먼저 넣으세요. 테스트 샤드 CI가 아직 도는 중이에요. 그 결과가 통과한 다음에 머지하세요.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant