fix(lifecycle): bind in-place proxy restart to runtime identity - #1131
fix(lifecycle): bind in-place proxy restart to runtime identity#1131luvs01 wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe PR replaces stop-then-start proxy restarts with attested in-place replacement. It adds PID-bound HMAC authorization, bounded restart observation, same-port replacement checks, CLI and tray integration, tests, and multilingual documentation updates. ChangesProxy restart lifecycle
Sequence Diagram(s)sequenceDiagram
participant CLI
participant ProxyRestartCoordinator
participant ManagementAPI
participant ReplacementProxy
CLI->>ProxyRestartCoordinator: restart live proxy
ProxyRestartCoordinator->>ManagementAPI: attest and submit PID-bound capability
ManagementAPI->>ReplacementProxy: validate target and schedule drain
ProxyRestartCoordinator->>ReplacementProxy: observe healthy new PID on same port
ReplacementProxy-->>CLI: replacement readiness
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
github-actions
Bot
added
the
intake: hygiene-blocked
|
⏳ DRAFT
What to do
Review readiness checklist
0/4 boxes ticked. This PR stays in draft until every box above is ticked. |
|
@coderabbitai review |
|
@codex review |
✅ Action performedReview finished.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e45eb7f2c3
ℹ️ 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".
| const response = await fetchImpl(`${baseUrl}${SYSTEM_RESTART_PATH}`, { | ||
| method: SYSTEM_RESTART_METHOD, | ||
| headers: { | ||
| [SYSTEM_RESTART_EXPECTED_PID_HEADER]: String(target.pid), | ||
| [SYSTEM_RESTART_NONCE_HEADER]: challenge, | ||
| [SYSTEM_RESTART_CAPABILITY_HEADER]: capability, | ||
| }, |
There was a problem hiding this comment.
When the new CLI is installed while a proxy from the previous release is still running, this POST sends only the newly introduced capability headers. The previous server's requireManagementAuth does not recognize those headers and requires the management credential for every /api/* request, so it returns 401; line 133 treats that as a definite rejection, and both ocx restart and the tray refuse to observe or fall back, leaving users unable to restart the old process into the new version. Negotiate capability support or provide a safe compatibility path for an already-attested older runtime.
Useful? React with 👍 / 👎.
| else console.error("↩️ Restart aborted: the proxy was not stopped cleanly."); | ||
| // The running proxy owns its drain and replacement through /api/system/restart. | ||
| // If nothing is live, restart degrades to the documented `ensure` start behavior. | ||
| await handleProxyRestart(handleEnsure); |
There was a problem hiding this comment.
On Windows with the Task Scheduler backend, ocx stop ends the task but leaves its enabled registration installed, and this stopped-proxy fallback invokes handleEnsure, which starts a detached proxy with OCX_SERVICE=1. On the next ocx restart, isSupervisedServiceChild sees that marker and isServiceViable() reports an enabled scheduler registration as viable even though the direct process is not owned by a running task, so the restart path exits with code 1 instead of spawning a replacement; no supervisor observes that exit, leaving the proxy down and managed routing pointed at it. Start the installed service here or persist enough runtime ownership information to distinguish an actual supervised child from an ensure-spawned process.
Useful? React with 👍 / 👎.
| # handing off to an identity-verified replacement. The tray observes health/PID | ||
| # rather than the detached CLI exit, so keep a watchdog margin around that shared | ||
| # lifecycle budget. The CLI remains the lifecycle owner; the tray never kills. | ||
| Set-PendingAction "Restart Proxy" 160 |
There was a problem hiding this comment.
When __tray-restart fails immediately—for example because runtime attestation is missing, the target changed, or the POST is rejected—Start-OcxCommand has already disposed the child process and only reports whether it launched, while the old proxy remains online with the same PID. Consequently the pending predicate can never complete and this new 160-second deadline leaves the tray displaying “Restarting...” for nearly three minutes before surfacing the already-known failure. Retain and observe the detached command's exit status, or add an explicit completion/failure signal, while keeping the long watchdog only for commands that are still running.
Useful? React with 👍 / 👎.
| if (!ownershipBlocked) { | ||
| const r = await restoreNativeCodexAsync(); | ||
| if (r.success) console.log(`↩️ ${r.message}`); | ||
| else { | ||
| stopFailed = true; | ||
| console.error(`⚠️ ${r.message}`); | ||
| } | ||
| } | ||
| // revertSystemEnv is NOT gated: it carries its own ownership check and concerns launchctl | ||
| // user env, not CODEX_HOME. Safety net for when the daemon's syncCleanup didn't run (SIGKILL). | ||
| try { revertSystemEnv(); } catch { /* best-effort */ } | ||
| if (!ownershipBlocked) { | ||
| // Same safety net for the Grok Build managed block (marker-owned, idempotent). | ||
| try { | ||
| const g = stripGrokConfig(); | ||
| if (g.changed) console.log(`↩️ ${g.message}`); | ||
| // A refused strip (e.g. orphaned marker) leaves the fence pointing at a dead proxy — | ||
| // reporting success there hides a broken end state. | ||
| else if (!g.ok) { stopFailed = true; console.error(`⚠️ ${g.message}`); } | ||
| } catch { /* best-effort */ } | ||
| if (!await restoreSharedStateAfterStop()) stopFailed = true; |
There was a problem hiding this comment.
When stopServiceIfInstalled() reports an ownership mismatch, this gate now skips restoreSharedStateAfterStop() entirely, including revertSystemEnv(). Before this refactor, environment rollback deliberately ran outside the ownership gate because it has its own marker/ownership check and cleans the current home's launchctl variables and shell environment file independently of the foreign service's Codex/Grok state. On macOS, stopping the current proxy in this scenario can therefore leave ANTHROPIC_BASE_URL and related variables pointing at its dead port. Move revertSystemEnv() back outside this gate while continuing to gate native Codex and Grok teardown.
Useful? React with 👍 / 👎.
| if (!previous) { | ||
| try { | ||
| return await io.startWhenStopped() | ||
| ? { ok: true, mode: "started" } | ||
| : { ok: false, phase: "start" }; |
There was a problem hiding this comment.
A single findLiveProxy() transport timeout returns null here and is treated as proof that no proxy exists. Both fallback implementations immediately probe again, so if the same running proxy answers that second probe, handleEnsure or runTrayProxyStart returns true and the coordinator reports a successful “started” restart even though the PID never changed and no restart occurred; if the second probe also lands during a restart handoff gap, it can instead spawn a competing start. Require stable absence or retain the runtime/PID candidate and verify that it is gone before selecting the start-only path.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs-site/src/content/docs/reference/cli/lifecycle.md`:
- Around line 39-42: Update the lifecycle descriptions to document that a live
proxy lacking an attested runtime PID or having a non-runtime source fails with
phase "identity" and does not fall back to ensure or perform stop/start: add
this behavior to docs-site/src/content/docs/reference/cli/lifecycle.md lines
39-42, docs-site/src/content/docs/ko/reference/cli/lifecycle.md lines 39-42 in
Korean, and docs-site/src/content/docs/ru/reference/cli/lifecycle.md lines 43-46
in Russian.
In `@src/cli/index.ts`:
- Around line 407-412: `handleEnsure` currently returns `false` when
`codexAutoStartEnabled(config)` is disabled, which makes `runProxyRestart` and
`reportRestartFailure` treat a deliberate skip as a failed start. Update the
restart flow so the `startWhenStopped` path can distinguish “autostart disabled”
from an actual startup failure, using `handleEnsure` and the
`runProxyRestart`/`reportRestartFailure` wiring. Preserve the existing
disabled-autostart message, but make `ocx restart` return success and keep
`process.exitCode` at 0 when the proxy was intentionally not started.
- Around line 575-579: Update the catch block surrounding stripGrokConfig in the
stop flow so any thrown failure sets restored = false before continuing.
Preserve the existing best-effort behavior and error handling for returned
!grok.ok results, ensuring both failure paths prevent ocx stop from reporting
successful restoration.
- Around line 490-503: Update the tray start action’s pending timeout in
Set-PendingAction for “Start Proxy” within the tray start flow to cover the
CLI’s 20-second probe plus its 40-second follow-up wait, using a budget
consistent with the existing extended restart timeout. Preserve the existing
polling and completion behavior.
- Line 54: Remove the top-level MEMORY_DRAIN_RESTART_MS and
REPLACEMENT_READY_TIMEOUT_MS import from src/cli/index.ts, and make
PROXY_RESTART_OBSERVE_MS obtain these values only within the handleProxyRestart
restart path. Prefer a dependency-free shared constants module if reuse is
required; otherwise derive the observation window locally while preserving
existing timing behavior.
In `@tests/server-management-auth.test.ts`:
- Around line 165-178: Extend the existing system-restart capability tests with
two HTTP-level negative cases: send a DELETE request to SYSTEM_RESTART_PATH
using otherwise valid restart headers and assert it is rejected, then create
valid capability headers bound to a different port and assert a request against
the live server is rejected. Keep these cases adjacent to the
tampered-capability assertion and preserve the existing valid-request
expectations.
In `@tests/system-restart-client.test.ts`:
- Around line 85-94: Add focused tests beside “rejects missing runtime proof
state before fetching” that provide runtime records with valid attestationSecret
but mismatched pid and mismatched port relative to target. Assert
requestBoundSystemRestart returns accepted false and fetchImpl is never called
for each case, covering both runtime identity guard comparisons.
- Around line 26-70: Consolidate the duplicated healthz response fixtures by
removing the inline response construction from successfulDeps and reusing
successfulDepsResponse instead. Update the single successfulDepsResponse builder
to include target.port, preserving the response shape served by
src/server/index.ts and keeping existing request behavior unchanged.
In `@tests/tray-proxy.test.ts`:
- Around line 124-190: Add focused regression tests near the existing
runProxyRestart tests for both thrown-error branches: make findLive reject and
assert { ok: false, phase: "request", error } with no request or fallback start,
then make waitForReplacement reject and assert { ok: false, phase:
"replacement", error }. Use call tracking where needed to verify fail-closed
behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a0234fdd-4d17-40de-afde-0e03c7c9e1c2
📒 Files selected for processing (30)
docs-site/src/content/docs/guides/grok-build.mddocs-site/src/content/docs/ja/guides/grok-build.mddocs-site/src/content/docs/ja/reference/cli/lifecycle.mddocs-site/src/content/docs/ja/troubleshooting/windows-memory.mddocs-site/src/content/docs/ko/guides/grok-build.mddocs-site/src/content/docs/ko/reference/cli/lifecycle.mddocs-site/src/content/docs/ko/troubleshooting/windows-memory.mddocs-site/src/content/docs/reference/cli/lifecycle.mddocs-site/src/content/docs/ru/guides/grok-build.mddocs-site/src/content/docs/ru/reference/cli/lifecycle.mddocs-site/src/content/docs/ru/troubleshooting/windows-memory.mddocs-site/src/content/docs/troubleshooting/windows-memory.mddocs-site/src/content/docs/zh-cn/guides/grok-build.mddocs-site/src/content/docs/zh-cn/reference/cli/lifecycle.mddocs-site/src/content/docs/zh-cn/troubleshooting/windows-memory.mdsrc/cli/index.tssrc/cli/system-restart-client.tssrc/cli/tray-proxy.tssrc/lib/system-restart-contract.tssrc/server/index.tssrc/server/management-auth.tssrc/server/management/system-routes.tssrc/tray/windows-tray.ps1structure/05_gui-and-management-api.mdtests/grok-lifecycle.test.tstests/server-management-auth.test.tstests/system-restart-client.test.tstests/system-restart.test.tstests/tray-proxy.test.tstests/windows-tray.test.ts
| When a proxy is running, ask that exact attested PID and port to restart in place, wait for its | ||
| normal drain, and verify a different runtime PID on the same port. Managed routing and service | ||
| supervision stay installed throughout; an uncertain request is observed rather than replayed as a | ||
| separate stop/start. If no proxy is running, the command falls back to the normal `ensure` start. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document the rejected live-identity path in every lifecycle page.
A live proxy without a runtime PID or with a non-runtime source fails with phase: "identity"; it does not fall back to ensure. Add this behavior to all localized lifecycle descriptions.
docs-site/src/content/docs/reference/cli/lifecycle.md#L39-L42: state that restart fails without stop/start when the live proxy cannot be attested.docs-site/src/content/docs/ko/reference/cli/lifecycle.md#L39-L42: add the same identity-failure behavior in Korean.docs-site/src/content/docs/ru/reference/cli/lifecycle.md#L43-L46: add the same identity-failure behavior in Russian.
📍 Affects 3 files
docs-site/src/content/docs/reference/cli/lifecycle.md#L39-L42(this comment)docs-site/src/content/docs/ko/reference/cli/lifecycle.md#L39-L42docs-site/src/content/docs/ru/reference/cli/lifecycle.md#L43-L46
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs-site/src/content/docs/reference/cli/lifecycle.md` around lines 39 - 42,
Update the lifecycle descriptions to document that a live proxy lacking an
attested runtime PID or having a non-runtime source fails with phase "identity"
and does not fall back to ensure or perform stop/start: add this behavior to
docs-site/src/content/docs/reference/cli/lifecycle.md lines 39-42,
docs-site/src/content/docs/ko/reference/cli/lifecycle.md lines 39-42 in Korean,
and docs-site/src/content/docs/ru/reference/cli/lifecycle.md lines 43-46 in
Russian.
Source: Path instructions
| import { withProcessRuntimeProvenance } from "../lib/bun-runtime"; | ||
| import { initializeNodeLauncherContext } from "./launcher-context"; | ||
| import { createLocalAttestationSecret } from "../lib/local-management-attestation"; | ||
| import { MEMORY_DRAIN_RESTART_MS, REPLACEMENT_READY_TIMEOUT_MS } from "../server/management/system-restart"; |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Measure the import cost and side effects pulled in by the new static CLI import.
set -euo pipefail
fd -t f 'system-restart.ts' src/server/management -x cat -n | head -80
echo "--- top-level imports of the restart module ---"
fd -t f 'system-restart.ts' src/server/management -x rg -n '^import|^export const|^const '
echo "--- top-level statements that are not declarations (possible side effects) ---"
fd -t f 'system-restart.ts' src/server/management -x rg -n '^(?!import|export|const|let|function|class|type|interface|//|/\*|\s|$)' -P
echo "--- how src/cli/index.ts imports other server modules ---"
rg -n 'await import\("\.\./server' src/cli/index.tsRepository: lidge-jun/opencodex
Length of output: 5671
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- src/cli/index.ts outline/import context ---"
sed -n '1,70p;330,480p;490,530p' src/cli/index.ts
echo "--- find usages of imported constants in cli ---"
rg -n 'MEMORY_DRAIN_RESTART_MS|REPLACEMENT_READY_TIMEOUT_MS|PROXY_RESTART_OBSERVE_MS' src/cli/index.ts src/server src/lib src/config src/service | head -120
echo "--- lifecycle imports at src/server/management/system-restart.ts top imports ---"
fd -t f 'lifecycle.ts' src -x sh -c 'echo "--- $1"; sed -n "1,80p" "$1"' sh {}
echo "--- service imports/top at src/service.ts ---"
fd -t f 'service.ts' src -x sh -c 'echo "--- $1"; sed -n "1,80p" "$1"' sh {}
echo "--- config imports/top at src/config.ts ---"
fd -t f 'config.ts' src -x sh -c 'echo "--- $1"; sed -n "1,80p" "$1"' sh {}Repository: lidge-jun/opencodex
Length of output: 14970
🏁 Script executed:
#!/bin/bash
set -u
echo "--- constants and handleProxyRestart usage ---"
rg -n 'MEMORY_DRAIN_RESTART_MS|REPLACEMENT_READY_TIMEOUT_MS|PROXY_RESTART_OBSERVE_MS|function handleProxyRestart|async function handleProxyRestart' src || true
echo "--- handleProxyRestart context ---"
rg -n 'PROXY_RESTART_OBSERVE_MS|waitForProxyReplacement|reportRestartFailure' src/cli/index.ts || true
sed -n '500,565p' src/cli/index.ts
echo "--- restart module non-declaration top-level statements probe ---"
python3 - <<'PY'
from pathlib import Path
import re
root = Path('src/server/management/system-restart.ts')
text = root.read_text()
top = ''.join(text.splitlines(True)[:120])
print("length:", len(top), "lines:", top.count("\n")+1)
declarations = re.findall(r'(?m)^(/\*.*?\*/|//.*|import .*|export .*|const .*|let .*|function .*|class .*|interface .*|type .*|async function .*|export interface .*)$', top, re.S)
print("top declarations:", len(declarations))
for d in declarations[:80]:
s=' '.join(d.split())
if '<120' not in s: print(s[:140])
non_decl=[]
for i,line in enumerate(top.splitlines(),1):
s=line.strip()
if s.startswith(('import ','export ','const ','let ','function ','class ','type ','interface ','async function ','//','/*','')):
continue
non_decl.append((i,line))
print("non-declaration first 40 lines:")
for i,line in non_decl[:40]:
print(f"{i}: {line}")
PYRepository: lidge-jun/opencodex
Length of output: 4663
Avoid the top-level CLI import for handleProxyRestart’s constants.
src/cli/index.ts imports MEMORY_DRAIN_RESTART_MS and REPLACEMENT_READY_TIMEOUT_MS at the module top, but only PROXY_RESTART_OBSERVE_MS needs them, inside the restart handling path. This top-level import forces the whole src/server/management/system-restart graph to load before any CLI command dispatch. Move these constants to a small, dependency-free shared module, or derive the observed restart window inside the handleProxyRestart code path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cli/index.ts` at line 54, Remove the top-level MEMORY_DRAIN_RESTART_MS
and REPLACEMENT_READY_TIMEOUT_MS import from src/cli/index.ts, and make
PROXY_RESTART_OBSERVE_MS obtain these values only within the handleProxyRestart
restart path. Prefer a dependency-free shared constants module if reuse is
required; otherwise derive the observation window locally while preserving
existing timing behavior.
Source: Path instructions
| async function handleEnsure(): Promise<boolean> { | ||
| if (!currentExternalCodexModelProvider()) reconcileJournal(); | ||
| const config = loadConfig(); | ||
| if (!codexAutoStartEnabled(config)) { | ||
| console.log("Codex autostart is disabled."); | ||
| return; | ||
| return false; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
ocx restart now fails with exit code 1 when Codex autostart is disabled.
Line 412 returns false when codexAutoStartEnabled(config) is false. Line 1162 passes handleEnsure as the startWhenStopped callback. Trace the disabled-autostart path with no live proxy:
runProxyRestartfinds no live proxy and callsstartWhenStopped.handleEnsureprints "Codex autostart is disabled." and returnsfalse.runProxyRestartreturns{ ok: false, phase: "start" }.reportRestartFailureprints "❌ Proxy was not running and the fallback start did not become healthy." (line 533).- Line 550 sets
process.exitCode = 1.
Nothing failed. The operator deliberately disabled autostart. The command now emits a false error and a nonzero exit code, which breaks scripts and CI wrappers that treat ocx restart exit status as health.
Distinguish "deliberately not started" from "start attempt failed". One option is a tri-state return from the fallback so the coordinator can report a skip.
🐛 Proposed fix sketch
-async function handleEnsure(): Promise<boolean> {
+/** Returns false only for a real failure; "skipped" means autostart is off by choice. */
+async function handleEnsure(): Promise<boolean | "skipped"> {
if (!currentExternalCodexModelProvider()) reconcileJournal();
const config = loadConfig();
if (!codexAutoStartEnabled(config)) {
console.log("Codex autostart is disabled.");
- return false;
+ return "skipped";
}Then have the restart wiring treat "skipped" as success without printing a start failure, and keep process.exitCode at 0.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cli/index.ts` around lines 407 - 412, `handleEnsure` currently returns
`false` when `codexAutoStartEnabled(config)` is disabled, which makes
`runProxyRestart` and `reportRestartFailure` treat a deliberate skip as a failed
start. Update the restart flow so the `startWhenStopped` path can distinguish
“autostart disabled” from an actual startup failure, using `handleEnsure` and
the `runProxyRestart`/`reportRestartFailure` wiring. Preserve the existing
disabled-autostart message, but make `ocx restart` return success and keep
`process.exitCode` at 0 when the proxy was intentionally not started.
| // serviceCommand("start") already spends up to 20s confirming the supervised | ||
| // child. Slow Windows hosts can still be publishing native-main state after that | ||
| // first window, so keep one shared follow-up budget instead of returning a false | ||
| // failure while Task Scheduler is still starting the approved child. | ||
| waitForProxy: () => waitForProxy(40_000), | ||
| info: message => console.log(message), | ||
| error: message => console.error(message), | ||
| }); | ||
| if (!ok) process.exitCode = 1; | ||
| // serviceCommand("start") can set exitCode=1 after its own 20s probe, while | ||
| // the coordinator's bounded follow-up observes the same service become live. | ||
| // The final observed state, not the earlier probe, owns this command result. | ||
| process.exitCode = ok ? 0 : 1; | ||
| return ok; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The tray start watchdog is now shorter than the CLI start wait, so a slow start reports a false failure.
Line 494 raises the CLI follow-up budget to waitForProxy(40_000). The comment explains that serviceCommand("start") already spent up to 20 seconds, so the CLI can now observe a start for roughly 60 seconds total.
The tray side was not extended to match. src/tray/windows-tray.ps1 line 248 still uses Set-PendingAction "Start Proxy" 15. The tray polls health every 3 seconds and calls Complete-PendingAction $false once pendingDeadline passes (line 241).
Failure mode on a slow Windows host:
- The operator clicks "Start Proxy". The tray dispatches
__tray-startand arms a 15-second watchdog. - At 15 seconds the proxy is not yet healthy. The tray logs "Start Proxy failed to reach the expected state" and shows an error balloon.
- The CLI keeps waiting and the proxy becomes healthy at, for example, 35 seconds.
process.exitCodeis 0.
The operator sees a failure notification for a start that succeeded. This PR extended the restart budget to 160 seconds (line 262) for exactly this reason but left the start budget at the old value.
Raise the tray start budget to cover the CLI's own start observation window.
🐛 Proposed fix in `src/tray/windows-tray.ps1`
$startItem.add_Click({
$statusItem.Text = "Proxy: Starting..."
- Set-PendingAction "Start Proxy" 15
+ # serviceCommand("start") spends up to 20s, then handleTrayProxyStart waits a further
+ # 40s (src/cli/index.ts waitForProxy(40_000)). Keep a margin around that budget so the
+ # tray never reports a failure for a start the CLI is still confirming.
+ Set-PendingAction "Start Proxy" 75
if (-not (Start-OcxCommand @("__tray-start"))) { $script:pendingAction = $null }
})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cli/index.ts` around lines 490 - 503, Update the tray start action’s
pending timeout in Set-PendingAction for “Start Proxy” within the tray start
flow to cover the CLI’s 20-second probe plus its 40-second follow-up wait, using
a budget consistent with the existing extended restart timeout. Preserve the
existing polling and completion behavior.
| try { | ||
| const grok = stripGrokConfig(); | ||
| if (grok.changed) console.log(`↩️ ${grok.message}`); | ||
| else if (!grok.ok) { restored = false; console.error(`⚠️ ${grok.message}`); } | ||
| } catch { /* best-effort */ } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
A thrown stripGrokConfig failure leaves restored true and contradicts the stated invariant.
Line 578 sets restored = false when stripGrokConfig() returns !grok.ok. The bare catch { } at line 579 swallows a thrown failure and leaves restored at true.
The comment at lines 572-573 states the rule: "a refused Grok strip is actionable because it would point Grok at a dead proxy." A thrown failure has the identical consequence. The fence in ~/.grok/config.toml survives, the proxy is gone, and ocx stop reports success at line 660.
src/grok/inject.ts lines 460-507 currently wraps its body and returns errorResult("strip", error) instead of throwing, so this is latent today. The guard still contradicts its own stated intent, and a future refactor of stripGrokConfig would silently downgrade the stop result.
🛡️ Proposed fix
try {
const grok = stripGrokConfig();
if (grok.changed) console.log(`↩️ ${grok.message}`);
else if (!grok.ok) { restored = false; console.error(`⚠️ ${grok.message}`); }
- } catch { /* best-effort */ }
+ } catch (error) {
+ // Same consequence as a refused strip: the fence survives and points Grok at a
+ // dead proxy, so the stop must not report success.
+ restored = false;
+ console.error(`⚠️ Grok config cleanup failed: ${error instanceof Error ? error.message : String(error)}`);
+ }
return restored;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cli/index.ts` around lines 575 - 579, Update the catch block surrounding
stripGrokConfig in the stop flow so any thrown failure sets restored = false
before continuing. Preserve the existing best-effort behavior and error handling
for returned !grok.ok results, ensuring both failure paths prevent ocx stop from
reporting successful restoration.
| const tampered = await fetch(new URL(SYSTEM_RESTART_PATH, server.url), { | ||
| method: SYSTEM_RESTART_METHOD, | ||
| headers: { ...headers, [SYSTEM_RESTART_CAPABILITY_HEADER]: "C".repeat(43) }, | ||
| }); | ||
| expect(tampered.status).toBe(503); | ||
|
|
||
| const request = new Request(new URL(SYSTEM_RESTART_PATH, server.url), { | ||
| method: SYSTEM_RESTART_METHOD, | ||
| headers, | ||
| }); | ||
| const local = { attestationSecret: secret, pid: process.pid, port: server.port }; | ||
| expect(requireManagementAuth(request, unavailable, remoteConfig(), local)).toBeNull(); | ||
| expect(managementPrincipal(request, unavailable, remoteConfig(), local)) | ||
| .toBe("system-restart-capability"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add two negative cases: wrong method and wrong bound port.
The tampered-capability case at Lines 165-169 only corrupts the capability value. Two bindings enforced by restartCapabilityPayload in src/lib/system-restart-contract.ts (Lines 32-34) have no HTTP-level regression test:
- Method binding.
hasSystemRestartCapabilityrejects a non-POST request atsrc/server/management-auth.tsLine 271. Nothing asserts that aDELETEtoSYSTEM_RESTART_PATHwith valid restart headers is refused. A future refactor that widens the method guard would pass the whole suite. - Port binding. A capability minted for a different port must fail verification, because
local.portcomes from the live bound port atsrc/server/index.tsLine 680. This is the binding that stops a capability captured from one proxy instance from authorizing a different instance on another port.
Both cases are cheap to add next to the existing assertions and they pin the security properties the PR is built on.
💚 Proposed additional negative cases
const tampered = await fetch(new URL(SYSTEM_RESTART_PATH, server.url), {
method: SYSTEM_RESTART_METHOD,
headers: { ...headers, [SYSTEM_RESTART_CAPABILITY_HEADER]: "C".repeat(43) },
});
expect(tampered.status).toBe(503);
+
+ // The capability is bound to POST; another method on the same route must not pass.
+ const wrongMethod = await fetch(new URL(SYSTEM_RESTART_PATH, server.url), {
+ method: "DELETE",
+ headers,
+ });
+ expect(wrongMethod.status).not.toBe(202);
+
+ // A capability minted for a different port must not authorize this instance.
+ const foreignPortCapability = createSystemRestartCapability(
+ secret,
+ nonce,
+ SYSTEM_RESTART_METHOD,
+ SYSTEM_RESTART_PATH,
+ process.pid,
+ server.port === 65535 ? 65534 : server.port + 1,
+ );
+ const wrongPort = await fetch(new URL(SYSTEM_RESTART_PATH, server.url), {
+ method: SYSTEM_RESTART_METHOD,
+ headers: { ...headers, [SYSTEM_RESTART_CAPABILITY_HEADER]: foreignPortCapability! },
+ });
+ expect(wrongPort.status).toBe(503);
+ expect(scheduled).toBe(1);As per path instructions: "Tests are flat Bun tests under tests/. A behavior change in src/ should come with a focused regression test near the existing tests for that subsystem."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const tampered = await fetch(new URL(SYSTEM_RESTART_PATH, server.url), { | |
| method: SYSTEM_RESTART_METHOD, | |
| headers: { ...headers, [SYSTEM_RESTART_CAPABILITY_HEADER]: "C".repeat(43) }, | |
| }); | |
| expect(tampered.status).toBe(503); | |
| const request = new Request(new URL(SYSTEM_RESTART_PATH, server.url), { | |
| method: SYSTEM_RESTART_METHOD, | |
| headers, | |
| }); | |
| const local = { attestationSecret: secret, pid: process.pid, port: server.port }; | |
| expect(requireManagementAuth(request, unavailable, remoteConfig(), local)).toBeNull(); | |
| expect(managementPrincipal(request, unavailable, remoteConfig(), local)) | |
| .toBe("system-restart-capability"); | |
| const tampered = await fetch(new URL(SYSTEM_RESTART_PATH, server.url), { | |
| method: SYSTEM_RESTART_METHOD, | |
| headers: { ...headers, [SYSTEM_RESTART_CAPABILITY_HEADER]: "C".repeat(43) }, | |
| }); | |
| expect(tampered.status).toBe(503); | |
| // The capability is bound to POST; another method on the same route must not pass. | |
| const wrongMethod = await fetch(new URL(SYSTEM_RESTART_PATH, server.url), { | |
| method: "DELETE", | |
| headers, | |
| }); | |
| expect(wrongMethod.status).not.toBe(202); | |
| // A capability minted for a different port must not authorize this instance. | |
| const foreignPortCapability = createSystemRestartCapability( | |
| secret, | |
| nonce, | |
| SYSTEM_RESTART_METHOD, | |
| SYSTEM_RESTART_PATH, | |
| process.pid, | |
| server.port === 65535 ? 65534 : server.port + 1, | |
| ); | |
| const wrongPort = await fetch(new URL(SYSTEM_RESTART_PATH, server.url), { | |
| method: SYSTEM_RESTART_METHOD, | |
| headers: { ...headers, [SYSTEM_RESTART_CAPABILITY_HEADER]: foreignPortCapability! }, | |
| }); | |
| expect(wrongPort.status).toBe(503); | |
| expect(scheduled).toBe(1); | |
| const request = new Request(new URL(SYSTEM_RESTART_PATH, server.url), { | |
| method: SYSTEM_RESTART_METHOD, | |
| headers, | |
| }); | |
| const local = { attestationSecret: secret, pid: process.pid, port: server.port }; | |
| expect(requireManagementAuth(request, unavailable, remoteConfig(), local)).toBeNull(); | |
| expect(managementPrincipal(request, unavailable, remoteConfig(), local)) | |
| .toBe("system-restart-capability"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/server-management-auth.test.ts` around lines 165 - 178, Extend the
existing system-restart capability tests with two HTTP-level negative cases:
send a DELETE request to SYSTEM_RESTART_PATH using otherwise valid restart
headers and assert it is rejected, then create valid capability headers bound to
a different port and assert a request against the live server is rejected. Keep
these cases adjacent to the tampered-capability assertion and preserve the
existing valid-request expectations.
Source: Path instructions
| function successfulDeps() { | ||
| const secret = createLocalAttestationSecret(); | ||
| const challenge = "A".repeat(43); | ||
| const requests: Array<{ url: string; init?: RequestInit }> = []; | ||
| const fetchImpl = (async (input: string | URL | Request, init?: RequestInit) => { | ||
| const url = String(input); | ||
| requests.push({ url, init }); | ||
| if (url.endsWith("/healthz")) { | ||
| const proof = createLocalAttestationProof(secret, challenge, target.pid!, target.port); | ||
| return new Response(JSON.stringify({ | ||
| status: "ok", | ||
| service: "opencodex", | ||
| version: "test", | ||
| uptime: 1, | ||
| pid: target.pid, | ||
| port: target.port, | ||
| }), { | ||
| status: 200, | ||
| headers: { | ||
| "content-type": "application/json", | ||
| [LOCAL_ATTESTATION_PROOF_HEADER]: proof!, | ||
| }, | ||
| }); | ||
| } | ||
| return new Response(JSON.stringify({ success: true }), { status: 202 }); | ||
| }) as typeof fetch; | ||
|
|
||
| return { | ||
| secret, | ||
| challenge, | ||
| requests, | ||
| deps: { | ||
| fetchImpl, | ||
| readRuntime: () => ({ | ||
| pid: target.pid!, | ||
| port: target.port, | ||
| hostname: target.hostname, | ||
| attestationSecret: secret, | ||
| }), | ||
| findLive: async () => target, | ||
| createChallenge: () => challenge, | ||
| now: () => 1_000, | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Two healthz fixtures now exist and they have diverged. Build the inline one from the helper.
The inline healthz response at Lines 35-48 includes port: target.port. The successfulDepsResponse helper at Lines 197-209 builds the same response but omits port. Both currently pass, because requestBoundSystemRestart binds the port through verifyLocalAttestationProof(runtime.attestationSecret, challenge, target.pid, target.port, proof) and never reads body.port. The divergence is therefore latent, not broken.
The failure mode: if isOpencodexHealthz is later tightened to require port, the tests at Lines 163-184 (which use the helper) would start failing while the tests using successfulDeps keep passing. A reader would then have to diff two near-identical fixtures to find out why. Collapse them onto one builder.
♻️ Proposed dedup of the healthz fixture
if (url.endsWith("/healthz")) {
- const proof = createLocalAttestationProof(secret, challenge, target.pid!, target.port);
- return new Response(JSON.stringify({
- status: "ok",
- service: "opencodex",
- version: "test",
- uptime: 1,
- pid: target.pid,
- port: target.port,
- }), {
- status: 200,
- headers: {
- "content-type": "application/json",
- [LOCAL_ATTESTATION_PROOF_HEADER]: proof!,
- },
- });
+ return successfulDepsResponse(secret, challenge);
}Then add the port field to the single remaining builder so the fixture matches what src/server/index.ts Line 631 actually serves:
function successfulDepsResponse(secret: string, challenge: string): Response {
const proof = createLocalAttestationProof(secret, challenge, target.pid!, target.port);
return new Response(JSON.stringify({
status: "ok",
service: "opencodex",
version: "test",
uptime: 1,
pid: target.pid,
+ port: target.port,
}), {successfulDepsResponse is a hoisted function declaration, so calling it from successfulDeps above its definition is safe.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/system-restart-client.test.ts` around lines 26 - 70, Consolidate the
duplicated healthz response fixtures by removing the inline response
construction from successfulDeps and reusing successfulDepsResponse instead.
Update the single successfulDepsResponse builder to include target.port,
preserving the response shape served by src/server/index.ts and keeping existing
request behavior unchanged.
| test("rejects missing runtime proof state before fetching", async () => { | ||
| let fetches = 0; | ||
| const fetchImpl = (async () => { fetches += 1; return new Response(); }) as typeof fetch; | ||
| const noRuntime = await requestBoundSystemRestart(target, 10_000, { | ||
| fetchImpl, | ||
| readRuntime: () => null, | ||
| }); | ||
| expect(noRuntime.accepted).toBe(false); | ||
| expect(fetches).toBe(0); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Cover the runtime identity mismatch branch, not only the missing-runtime branch.
requestBoundSystemRestart rejects on a compound guard: !runtime?.attestationSecret || runtime.pid !== target.pid || runtime.port !== target.port (src/cli/system-restart-client.ts, Lines 72-75). This test only exercises readRuntime: () => null. The pid and port mismatch clauses are untested.
That matters because those two clauses are the check that stops the CLI from signing a capability for a stale runtime record after a PID or port change — the exact failure this PR exists to prevent. A refactor that dropped either comparison would leave the suite green.
💚 Proposed additional mismatch cases
expect(noRuntime.accepted).toBe(false);
expect(fetches).toBe(0);
});
+
+ test("rejects a stale runtime record before fetching", async () => {
+ const base = successfulDeps();
+ const stalePid = await requestBoundSystemRestart(target, 10_000, {
+ ...base.deps,
+ readRuntime: () => ({
+ pid: target.pid! + 1,
+ port: target.port,
+ hostname: target.hostname,
+ attestationSecret: base.secret,
+ }),
+ });
+ expect(stalePid.accepted).toBe(false);
+
+ const stalePort = successfulDeps();
+ const outcome = await requestBoundSystemRestart(target, 10_000, {
+ ...stalePort.deps,
+ readRuntime: () => ({
+ pid: target.pid!,
+ port: target.port + 1,
+ hostname: target.hostname,
+ attestationSecret: stalePort.secret,
+ }),
+ });
+ expect(outcome.accepted).toBe(false);
+ expect(stalePort.requests).toHaveLength(0);
+ });As per path instructions: "A behavior change in src/ should come with a focused regression test near the existing tests for that subsystem."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test("rejects missing runtime proof state before fetching", async () => { | |
| let fetches = 0; | |
| const fetchImpl = (async () => { fetches += 1; return new Response(); }) as typeof fetch; | |
| const noRuntime = await requestBoundSystemRestart(target, 10_000, { | |
| fetchImpl, | |
| readRuntime: () => null, | |
| }); | |
| expect(noRuntime.accepted).toBe(false); | |
| expect(fetches).toBe(0); | |
| }); | |
| test("rejects missing runtime proof state before fetching", async () => { | |
| let fetches = 0; | |
| const fetchImpl = (async () => { fetches += 1; return new Response(); }) as typeof fetch; | |
| const noRuntime = await requestBoundSystemRestart(target, 10_000, { | |
| fetchImpl, | |
| readRuntime: () => null, | |
| }); | |
| expect(noRuntime.accepted).toBe(false); | |
| expect(fetches).toBe(0); | |
| }); | |
| test("rejects a stale runtime record before fetching", async () => { | |
| const base = successfulDeps(); | |
| const stalePid = await requestBoundSystemRestart(target, 10_000, { | |
| ...base.deps, | |
| readRuntime: () => ({ | |
| pid: target.pid! + 1, | |
| port: target.port, | |
| hostname: target.hostname, | |
| attestationSecret: base.secret, | |
| }), | |
| }); | |
| expect(stalePid.accepted).toBe(false); | |
| const stalePort = successfulDeps(); | |
| const outcome = await requestBoundSystemRestart(target, 10_000, { | |
| ...stalePort.deps, | |
| readRuntime: () => ({ | |
| pid: target.pid!, | |
| port: target.port + 1, | |
| hostname: target.hostname, | |
| attestationSecret: stalePort.secret, | |
| }), | |
| }); | |
| expect(outcome.accepted).toBe(false); | |
| expect(stalePort.requests).toHaveLength(0); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/system-restart-client.test.ts` around lines 85 - 94, Add focused tests
beside “rejects missing runtime proof state before fetching” that provide
runtime records with valid attestationSecret but mismatched pid and mismatched
port relative to target. Assert requestBoundSystemRestart returns accepted false
and fetchImpl is never called for each case, covering both runtime identity
guard comparisons.
Source: Path instructions
| test("request uncertainty observes for a replacement and never falls back to stop/start", async () => { | ||
| const calls: string[] = []; | ||
| const error = new Error("response connection closed"); | ||
| const result = await runProxyRestart({ | ||
| findLive: async () => ({ pid: 10, port: 10100, source: "runtime" }), | ||
| startWhenStopped: async () => { calls.push("fallback-start"); return true; }, | ||
| requestInPlaceRestart: async () => { | ||
| calls.push("request"); | ||
| return { accepted: false, uncertain: true, error }; | ||
| }, | ||
| waitForReplacement: async () => { calls.push("wait"); return null; }, | ||
| }); | ||
| expect(result).toEqual({ ok: false, phase: "request", error }); | ||
| expect(calls).toEqual(["request", "wait"]); | ||
| }); | ||
|
|
||
| test("a replacement proves success even when the request response was lost", async () => { | ||
| const error = new Error("response connection closed"); | ||
| const replacement: ProxyRestartLive = { pid: 20, port: 10100, source: "runtime" }; | ||
| const result = await runProxyRestart({ | ||
| findLive: async () => ({ pid: 10, port: 10100, source: "runtime" }), | ||
| startWhenStopped: async () => true, | ||
| requestInPlaceRestart: async () => ({ accepted: false, uncertain: true, error }), | ||
| waitForReplacement: async () => replacement, | ||
| }); | ||
| expect(result).toEqual({ ok: true, mode: "restarted", live: replacement }); | ||
| }); | ||
|
|
||
| test("a definite request rejection does not wait or start another proxy", async () => { | ||
| const calls: string[] = []; | ||
| const error = new Error("target changed"); | ||
| const result = await runProxyRestart({ | ||
| findLive: async () => ({ pid: 10, port: 10100, source: "runtime" }), | ||
| startWhenStopped: async () => { calls.push("fallback-start"); return true; }, | ||
| requestInPlaceRestart: async () => { | ||
| calls.push("request"); | ||
| return { accepted: false, uncertain: false, error }; | ||
| }, | ||
| waitForReplacement: async () => { calls.push("wait"); return null; }, | ||
| }); | ||
| expect(result).toEqual({ ok: false, phase: "request", error }); | ||
| expect(calls).toEqual(["request"]); | ||
| }); | ||
|
|
||
| test("an accepted restart that never publishes a replacement fails closed", async () => { | ||
| const calls: string[] = []; | ||
| const result = await runProxyRestart({ | ||
| findLive: async () => ({ pid: 10, port: 10100, source: "runtime" }), | ||
| startWhenStopped: async () => { calls.push("fallback-start"); return true; }, | ||
| requestInPlaceRestart: async () => { calls.push("request"); return { accepted: true }; }, | ||
| waitForReplacement: async () => { calls.push("wait"); return null; }, | ||
| }); | ||
| expect(result).toEqual({ ok: false, phase: "replacement" }); | ||
| expect(calls).toEqual(["request", "wait"]); | ||
| }); | ||
|
|
||
| test("an unverified live target fails closed before the restart request", async () => { | ||
| const calls: string[] = []; | ||
| const result = await runProxyRestart({ | ||
| findLive: async () => ({ pid: null, port: 10100, source: "config" }), | ||
| startWhenStopped: async () => { calls.push("fallback-start"); return true; }, | ||
| requestInPlaceRestart: async () => { calls.push("request"); return { accepted: true }; }, | ||
| waitForReplacement: async () => { calls.push("wait"); return null; }, | ||
| }); | ||
| expect(result).toEqual({ ok: false, phase: "identity" }); | ||
| expect(calls).toEqual([]); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add regression tests for the two thrown-error branches of runProxyRestart.
The suite covers returned outcomes well. Two thrown-error branches in src/cli/tray-proxy.ts have no coverage, and both are reachable in production:
io.findLiverejects →src/cli/tray-proxy.tsline 98 maps it to{ ok: false, phase: "request" }.src/cli/index.tsline 542 throwsrestart_deadline_expiredfromfindLiveon purpose, so this is a wired production path.io.waitForReplacementrejects →src/cli/tray-proxy.tsline 135 maps it to{ ok: false, phase: "replacement" }.
Both phase values drive the operator message in reportRestartFailure at src/cli/index.ts lines 525-535. Without tests, a future change to the phase mapping would silently change what the operator is told.
💚 Proposed additional test cases
test("a discovery failure fails closed without requesting or starting", async () => {
const calls: string[] = [];
const error = new Error("restart_deadline_expired");
const result = await runProxyRestart({
findLive: async () => { throw error; },
startWhenStopped: async () => { calls.push("fallback-start"); return true; },
requestInPlaceRestart: async () => { calls.push("request"); return { accepted: true }; },
waitForReplacement: async () => { calls.push("wait"); return null; },
});
expect(result).toEqual({ ok: false, phase: "request", error });
expect(calls).toEqual([]);
});
test("a failed replacement observation reports the replacement phase", async () => {
const error = new Error("probe failed");
const result = await runProxyRestart({
findLive: async () => ({ pid: 10, port: 10100, source: "runtime" }),
startWhenStopped: async () => true,
requestInPlaceRestart: async () => ({ accepted: true }),
waitForReplacement: async () => { throw error; },
});
expect(result).toEqual({ ok: false, phase: "replacement", error });
});Based on path instructions: "A behavior change in src/ should come with a focused regression test near the existing tests for that subsystem."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/tray-proxy.test.ts` around lines 124 - 190, Add focused regression
tests near the existing runProxyRestart tests for both thrown-error branches:
make findLive reject and assert { ok: false, phase: "request", error } with no
request or fallback start, then make waitForReplacement reject and assert { ok:
false, phase: "replacement", error }. Use call tracking where needed to verify
fail-closed behavior.
Source: Path instructions
e45eb7f to
74c2a9c
Compare
1 participant
Summary
ocx restartand the Windows tray use the running proxy's existingPOST /api/system/restartlifecycle instead of a separate stop-then-start transaction.This builds on the restart surface introduced by #580 / #594. It is not a duplicate of #720 / #737 (readiness boundary) or #733 / #752 (tray socket inheritance).
Verification
bun run typecheckpassed on both Bun runtimes.bun run privacy:scan, PowerShell AST parsing, andgit diff --checkpassed.cd docs-site && bun run buildpassed, 221 pages generated.api-usagecache test and reproduced unchanged on the latestdevworktree. The affected restart surface remains green in the focused runs above.Checklist
This changes the management-auth boundary. Explicit maintainer security review and sponsorship are still required before merge.
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.
Summary by CodeRabbit
ocx restartnow performs an in-place proxy restart while preserving managed routing and service supervision.