Repository navigation
fix(cli): guard standalone update restart with exact replacement verification #6556
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
53c9e33
fix(cli): guard standalone update restart ownership and replacement
lidge-jun 7463bae
test(cli): exercise busy lease with matching host configuration
lidge-jun 917a03f
fix(cli): revalidate update configuration before stop and child admis…
lidge-jun cfbbbec
fix(cli): wait for a complete runtime around the update restart handoff
lidge-jun 8a8efef
Merge remote-tracking branch 'origin/dev' into codex/next-release-261…
lidge-jun 0a364f0
Merge remote-tracking branch 'origin/dev' into codex/next-release-261…
lidge-jun a7a26d3
Merge remote-tracking branch 'origin/dev' into codex/next-release-261…
lidge-jun 1e52cf9
Merge remote-tracking branch 'origin/dev' into codex/next-release-261…
lidge-jun 00c8504
test(cli): keep the proxied environment of the transport case in a child
lidge-jun 87d8123
Merge remote-tracking branch 'origin/dev' into codex/next-release-261…
lidge-jun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| # Guarded update restart | ||
|
|
||
| A newer CLI should be able to replace an older standalone proxy while preserving target ownership and confirming the exact launched replacement. This unit refines the public proposal in #6548; supervised cases remain explicit limitations until their replacement identity can be established. | ||
|
|
||
| - Archetype: satisfy-spec, one cohesive implementation cycle (wp1). | ||
| - Trigger: next-release replacement of public PR #6548 at fe3cf0aa823afd4144ab240191939ec6145b1c45. | ||
| - Goal: attributed draft PR targeting dev with focused lifecycle tests, independent review and current-head CI evidence. | ||
| - Non-goals: merge, source closure, release, installed-runtime operations, account changes, provider calls, version changes or native stacks. | ||
| - Verifiers: focused Bun tests, typecheck, structure/privacy/layout/ratchet gates; docs build for changed public prose. Full/changed local suites are impractical alongside concurrent release/lane work; hosted CI supplies wider evidence. | ||
| - Stop: publish reviewable focused change, account for source coverage, fix applicable defects or bound missing native/hosted evidence in draft. | ||
| - Evidence: ignored scratch holds investigation, review and test receipts. Public prose records only the already-public feature and published outcomes. | ||
| - Outcomes: DONE means this delivery scope is met, not merged or all native environments validated. Unresolved ownership or required scope expansion remains explicit. | ||
| - Escalation: changes outside CLI orchestration/tests/docs require scope review. No token/cost/time limit was specified; no paid upstream probes are allowed. | ||
|
|
||
| Base: dev 0818ea1812a028e1c14cd0b0511b44863407bc52. Source author: agentHits, 140916359+agentHits@users.noreply.github.com. Source consists of one commit and three files, with no native stack membership observed. | ||
|
|
||
| Baseline: 44 tests pass across system-restart-client and cli-version-skew after frozen dependency installation. Original public review requests handling failed stop, missing replacement version and behavioral regressions. | ||
|
|
||
| Current owner map: CLI index composes system-restart-client and tray-proxy; process-state owns runtime records; ownership-mutation-lease owns cooperative lifecycle serialization; proxy-liveness owns attestation/health; stop-approval owns settlement patterns; structure/runtime.md and public reference/cli/lifecycle.md describe lifecycle behavior. | ||
|
|
||
| The no-code alternatives do not deliver the requested newer-install restart. Existing unrestricted stop/start orchestration cannot satisfy the new target contract. Reuse the existing owner primitives and place the narrow orchestration beside the CLI instead of enlarging its index. | ||
|
|
||
| Architect consultation: Zeno (01a10493-8767-7422-8098-e28643decd60) proposed D1–D6. Main accepted eligibility, preserved snapshot, confirmed stop and exact replacement requirements; amended stop transport and child admission. Same architect reflected on the concrete scratch plan and returned ALIGNED, with transport/runtime evidence remaining implementation acceptance obligations. Independent audit follows. Family-level model independence is not claimed; the requested leaf model is gpt-6.1-sol. | ||
| The independent plan audit identified two lifecycle gaps; main amended the handoff and terminal result requirements. The architect rechecked D5/D6 and returned ALIGNED. Security analysis and detailed review remain in scratch. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| # wp1 — Publish the CLI restart replacement | ||
|
|
||
| One implementation cycle produces an ordinary draft PR against dev. Preserve the newer-CLI upgrade intent and proxy-newer/incomparable refusal from #6548. Add focused lifecycle regression coverage, keep index composition minimal, and synchronize runtime and user lifecycle documentation. | ||
|
|
||
| The scoped implementation and security audit remain in ignored scratch until the patch is public, per repository policy. Public delivery records source coverage, attribution, verification commands and native/live limits. No source PR is closed by this lane. | ||
|
|
||
| Expected changed areas: src/cli restart siblings and index wiring; tests/cli lifecycle regressions; both test-layout registries; structure/runtime.md; docs-site lifecycle reference. No integration branch or version file changes. |
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| import type { RuntimePortState } from "../config/process-state"; | ||
| import type { LiveProxy } from "../server/proxy-liveness"; | ||
|
|
||
| import type { UpdateRestartHome } from "./update-restart-home"; | ||
|
|
||
| export interface UpdateRestartCandidate { | ||
| home: UpdateRestartHome; | ||
| target: LiveProxy & { pid: number }; | ||
| runtime: RuntimePortState & { attestationSecret: string }; | ||
| cliVersion: string; | ||
| } | ||
|
|
||
| /** Private in-process evidence; diagnostics and JSON never expose the proof key. */ | ||
| export class UpdateRestartRequired extends Error { | ||
| #candidate: UpdateRestartCandidate; | ||
| constructor(candidate: UpdateRestartCandidate) { | ||
| super("restart_version_skew_cli_newer"); | ||
| this.#candidate = candidate; | ||
| } | ||
| candidate(): UpdateRestartCandidate { return this.#candidate; } | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| import { existsSync } from "node:fs"; | ||
| import { getRuntimePortPath, readRuntimePort } from "../config/process-state"; | ||
| import { packageVersion } from "../lib/package-version"; | ||
| import { parseStrictSemver } from "../lib/strict-semver"; | ||
| import { acquireOwnershipMutationLease, OWNERSHIP_MUTATION_LEASE_TOKEN_ENV } from "../service/ownership-mutation-lease.mjs"; | ||
| import { serviceStatePaths } from "../service/state"; | ||
| import { assertUpdateRestartConfiguration, assertUpdateRestartHome, type UpdateRestartHome } from "./update-restart-home"; | ||
|
|
||
| export const UPDATE_RESTART_CHILD_ENV = "OCX_UPDATE_RESTART_CHILD"; | ||
| export interface UpdateRestartChildMarker { home: UpdateRestartHome; version: string; port: number; hostname: string; deadlineAt: number } | ||
| export interface UpdateRestartChildGuard { check(port?: number, hostname?: string): void; complete(): void } | ||
|
|
||
| function parseMarker(raw: string): UpdateRestartChildMarker { | ||
| if (raw.length > 8192) throw new Error("update_restart_child_marker_invalid"); | ||
| const value = JSON.parse(raw) as UpdateRestartChildMarker; | ||
| if (!value || !Number.isSafeInteger(value.deadlineAt) || !Number.isInteger(value.port) | ||
| || typeof value.hostname !== "string" || value.port < 1 || value.port > 65535 || typeof value.version !== "string" | ||
| || value.version === "0.0.0" || !parseStrictSemver(value.version) || !value.home | ||
| || !Number.isSafeInteger(value.home.revision)) throw new Error("update_restart_child_marker_invalid"); | ||
| for (const directory of [value.home.config, value.home.codex]) { | ||
| if (!directory || typeof directory.path !== "string" || !directory.path | ||
| || !Number.isSafeInteger(directory.dev) || !Number.isSafeInteger(directory.ino)) { | ||
| throw new Error("update_restart_child_marker_invalid"); | ||
| } | ||
| } | ||
| return value; | ||
| } | ||
|
|
||
| export interface UpdateRestartChildIo { | ||
| env?: NodeJS.ProcessEnv; | ||
| now?: () => number; | ||
| version?: () => string; | ||
| checkHome?: (home: UpdateRestartHome) => void; | ||
| acquire?: (waitMs: number) => { release(): void }; | ||
| onExit?: (release: () => void) => void; | ||
| checkState?: (marker: UpdateRestartChildMarker) => void; | ||
| armDeadline?: (remainingMs: number) => () => void; | ||
| } | ||
|
|
||
| /** Admit the replacement before CLI preflight; custody belongs to this child PID. */ | ||
| export function admitUpdateRestartChild(argv: string[], io: UpdateRestartChildIo = {}): UpdateRestartChildGuard | undefined { | ||
| const env = io.env ?? process.env; | ||
| const raw = env[UPDATE_RESTART_CHILD_ENV]; | ||
| if (raw === undefined) return undefined; | ||
| delete env[UPDATE_RESTART_CHILD_ENV]; | ||
| delete env[OWNERSHIP_MUTATION_LEASE_TOKEN_ENV]; | ||
| const marker = parseMarker(raw); | ||
| if (argv.length !== 3 || argv[0] !== "start" || argv[1] !== "--port" || argv[2] !== String(marker.port)) { | ||
| throw new Error("update_restart_child_command_changed"); | ||
| } | ||
| const now = io.now ?? Date.now; | ||
| const check = (port = marker.port, hostname = marker.hostname) => { | ||
| if (now() >= marker.deadlineAt) throw new Error("update_restart_deadline_expired"); | ||
| const bindHost = (host: string) => !host.trim() || /^localhost\.?$/i.test(host.trim()) ? "127.0.0.1" : host.trim(); | ||
| if (port !== marker.port || bindHost(hostname) !== bindHost(marker.hostname) || (io.version ?? packageVersion)() !== marker.version) throw new Error("update_restart_child_identity_changed"); | ||
| (io.checkHome ?? assertUpdateRestartHome)(marker.home); | ||
| (io.checkState ?? (expected => { | ||
| assertUpdateRestartConfiguration(expected.hostname); | ||
| const current = readRuntimePort(); | ||
| if (existsSync(getRuntimePortPath()) && (!current || current.pid !== process.pid | ||
| || current.port !== expected.port || current.siblingOfPort !== undefined)) { | ||
| throw new Error("update_restart_competing_runtime"); | ||
| } | ||
| }))(marker); | ||
| }; | ||
| check(); | ||
| const lease = (io.acquire ?? (waitMs => acquireOwnershipMutationLease(serviceStatePaths(), { waitMs })))(Math.min(2000, marker.deadlineAt - now())); | ||
| let released = false; | ||
| let cancelDeadline: (() => void) | undefined; | ||
| const release = () => { if (!released) { released = true; cancelDeadline?.(); lease.release(); } }; | ||
| try { | ||
| check(); | ||
| cancelDeadline = (io.armDeadline ?? (remainingMs => { | ||
| const timer = setTimeout(() => process.exit(1), remainingMs); | ||
| timer.unref(); | ||
| return () => clearTimeout(timer); | ||
| }))(marker.deadlineAt - now()); | ||
| (io.onExit ?? (callback => { process.once("exit", callback); }))(release); | ||
| return { check, complete: () => { check(); release(); } }; | ||
| } catch (error) { release(); throw error; } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 5824
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 35085
Keep the version-skew error for ineligible update homes.
When
readUpdateRestartHome()cannot provide an eligible home,requestBoundSystemRestartreturnsupdate_restart_home_unverifiedfor a newer CLI (src/cli/system-restart-client.ts:162-163).reportRestartFailuredoes not map that code, so users can see only generic failure text instead of the existing recovery instructions to runocx stopand thenocx startfrom this installation (src/cli/index.ts:930-936).Suggested fix
if (skew.relation === "cli-newer") { - if (!updateHome) return rejected("update_restart_home_unverified"); + // Ineligible homes keep the existing skew diagnosis and recovery advice. + if (!updateHome) return rejected("restart_version_skew");📝 Committable suggestion
🤖 Prompt for AI Agents