[Bug] Post-merge #7851: completed-drain restart refusal and Grok bind-scope regression #7871

Activity

1. coderabbitai commented on Oct 28, 2026

   

   coderabbitaiBot

   on Oct 28, 2026 – with coderabbitai

   Last edited by coderabbitai

   Contributor

   More actions

   🔗 Related PRs

   #7747 - fix(update): keep Codex routing for the restarted proxy during an update (#7423) [open]

   ---

   📝 Issue Planner

   _{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

   • Create Plan

   ---

   🧪 Issue enrichment is currently in open beta.

   To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

   issue_enrichment:
     auto_enrich:
       enabled: false

   💬 Have feedback or questions? Drop into our discord!
2. luvs01 commented on Oct 28, 2026

   

   luvs01

   on Oct 28, 2026 – with ChatGPT Codex Connector

   Last edited by luvs01

   CollaboratorAuthor

   More actions

   Verified candidate published as Draft PR #7874: #7874

   Head 889dc0f980d934af199b1f23b6493ee2297def39, based on dev@f9e7d939. Its remote tree exactly matches the locally validated index. Both issue items are implemented: completed-drain retirement of only the parent's own ownership records, with the ordinary child live/unknown-owner refusal retained; and separate bind hostname versus responding probe address for Grok policy.

   The actual parent/child and loopback-listener fixtures passed, along with the real Grok sync/inject matrix. Broader regression selection: 287 passed / 0 failed; layout/tooling/ratchet: 38 passed / 0 failed. Typecheck, structure and privacy checks passed. The full server's native lease shutdown order was source-reviewed, not exercised as a live provider/dashboard E2E.

   Current-head hosted CI and explicit maintainer/security acceptance remain pending. Native Windows/macOS replacement is untested. The issue remains open until integration and the stated acceptance/disposition; no security-only completion claim is made. The candidate also merges cleanly in a read-only integration check with the newer dev@492c1a15 after #7870.

   Review follow-up published at head 0fe66b2b16bce2e93eee5dc47104a584b7cfd075 (tree 945277083196bccc8494d60ffaf70788a8dc347d) by a normal fast-forward update. Missing/blank default binds are now explicit loopback in both discovery paths. The new actual reconciler/Grok matrix reproduced 18 old failures and passes all 48 cases after the correction; 10-file regression: 285 passed / 0 failed. The preceding 889dc0f CI completed successfully, but it is not current-head evidence after this correction. New CI/re-review and maintainer/native conditions remain.
3. luvs01

   added

   bugSomething isn't workingSomething isn't working

   on Oct 28, 2026
4. github-actions

   added

   serviceService lifecycle (WinSW/launchd/scheduler)Service lifecycle (WinSW/launchd/scheduler)

   on Oct 28, 2026
5. coderabbitai commented on Oct 28, 2026

   

   coderabbitaiBot

   on Oct 28, 2026 – with coderabbitai

   Contributor

   More actions

   Coding Plan

   Summary

   • P1 adds a child-only, consumed marker named after the parent PID. The parent sets it only after a completed drain and a successful listener stop. The child honors it only when the PID matches the validated actual parent, the records belong to that parent, every endpoint candidate returns recognized connection refusal, and no live owner exists.
   • The recovery gate accepts a distinct restart-parent-stopped observation kind only in handleStart. Arbitrary live or unknown owners stay refused. The child never waits for parent exit.
   • P2 keeps LiveProxy.hostname as bind scope and adds probeAddress for the address that answered. Wildcard scope therefore reaches Grok and produces the expected non-loopback refusal.
   • Focused regressions cover both defects and their controls: the completed-drain composition, the unmarked case, the exited parent, the missed-early-probe wildcard case, and the normal loopback case. Real process-replacement E2E is verified or recorded as untested.

   Design Choices

   Design Choice 1: Fix revision baseline

   Options Considered:

   1. Implement against the local HEAD 88aaa754.
   2. Rebase or check out current dev, which contains 7a1565bb, and implement there.

   Chosen Option: 2

   Rationale: HEAD does not contain either defect, so a fix there cannot address the reported composition.

   Design Choice 2: Parent signal for a stopped listener while waiting for child health

   Options Considered:

   1. Treat any valid OCX_RESTART_PARENT_PID as permission to recover past a live recorded owner.
   2. Add a durable field to the PID or runtime record that marks the record "stopped, awaiting replacement".
   3. Add a child-only, consumed environment marker. It names the parent PID. The parent sets it only on the completed-drain, listener-stopped path. The child also requires refused endpoints before it honors the marker.

   Chosen Option: 3

   Rationale: Option 1 would also authorize deadline, failed and rejected handoffs where the parent can still serve. Option 2 adds persistent state that needs owner-bound publication and stale-record cleanup. Option 3 needs no persistent state and reuses the existing actual-parent validation in takeRestartParentMarker.

   Design Choice 3: Observation representation for the handoff

   Options Considered:

   1. Return kind: "absent" when the handoff conditions hold.
   2. Return a distinct kind, for example { kind: "restart-parent-stopped", live: null, parentPid }. Only the start recovery gate accepts this kind, and only when the caller passed the validated marker.

   Chosen Option: 2

   Rationale: "Absent" would be false while the parent process is alive. A distinct kind keeps handleEnsure and other callers fail-closed and makes the exception auditable.

   Design Choice 4: Representation of bind scope and probe address

   Options Considered:

   1. Change hostname to mean "probe address" everywhere and add bindHostname.
   2. Keep LiveProxy.hostname as bind scope, which is its documented meaning. Add probeAddress for the address that answered. Fix every producer that writes the responding address into hostname.

   Chosen Option: 2

   Rationale: Consumers already use hostname as bind scope and apply probeHostname when they dial: runtime-api.ts:95, ready.ts:209, status.ts:939–942, index.ts:1300–1304 and Grok. Option 2 limits churn to the faulty producers.

   Design Choice 5: Scope of spawned-ensure metadata loss at `src/cli/index.ts:879–890`

   Options Considered:

   1. Fix it in this ticket.
   2. Record it as a separate observation and leave it unchanged.

   Chosen Option: 2

   Rationale: That branch is not the reported substitution. Changing it would widen scope beyond the ticket.

   💡 User Tips

   Regenerate the plan with different choices with @coderabbitai <feedback>.

   Implementation Steps

   Phase 1: P1 — Known-parent completed-drain handoff

   Goal. The replacement must start when all of these conditions hold:

   • The real parent finished a drain.
   • The parent stopped its listener.
   • The parent is waiting for replacement health.

   The child must distinguish the known waiting parent from arbitrary live or unknown owners.

   Failure mode at 7a1565bb.

   1. The parent's listener is closed, so findLiveProxy returns null.
   2. probeOwnerPastRestartParent returns at once on live: null (src/cli/restart-handoff.ts:79–110). This step is correct.
   3. Inside the lease, observeProxyLiveness sees an alive recorded PID with an OpenCodex command line. It returns unknown. 7a1565bb:tests/server/proxy-liveness-recovery.test.ts:35–40 asserts this result, even with ECONNREFUSED.
   4. The gate at 7a1565bb:src/cli/start-owner-fence.ts:6–21 throws.
   5. The child exits. spawnReplacementStart exhausts early-exit retries. The parent exits 1 with no serving replacement.

   Fix. Add an explicit, parent-verified, refusal-gated exception. The exception must not wait for parent exit. It must not convert a generic unknown result into permission to start.

   Task 1: Establish the revision baseline and port the reported reproductions

   Confirm that the fix base contains 7a1565bb. Convert the post-merge reproduction cases into failing tests before any product change.

   • Rebase the working branch onto current dev. Confirm that git merge-base --is-ancestor 7a1565bb968b343f68eddd6ea266222fcfd95ff1 HEAD exits 0. The local checkout is shallow. Fetch enough history first.
   • The local HEAD 88aaa754 lacks the unknown recovery gate. The ticket reports that dev equals 7a1565bb for all affected files.
   • Re-read these locations at the new base. Record their actual line numbers in the PR:
     • observeProxyLiveness and the ProxyLivenessObservation type in src/server/proxy-liveness.ts.
     • recoverStartJournalAfterOwnerRecheck and recoverStartStateUnderOwnershipLease in src/cli/start-owner-fence.ts.
     • The handleStart and handleEnsure call sites in src/cli/index.ts.
   • Obtain the four product-module cases from the PR #7851 post-merge validation comment. That comment reports 23 assertions with Bun 1.4.4, the repository preload, and isolated temporary homes.
   • Port the P1 cases into tests/cli/cli-start-owner-absence.test.ts. Port the P2 cases into the files named in Phase 2.
   • Before changing product code, confirm that each case fails for the reported reason.

   Task 2: Define the stopped-parent handoff marker contract

   Add a consumed, parent-bound marker next to OCX_RESTART_PARENT_PID.

   • In src/lib/system-restart-contract.ts (lines 48–75, next to withRestartParentMarker and takeRestartParentMarker):
     • Add a new environment constant, for example OCX_RESTART_PARENT_STOPPED_PID.
     • Add a with… helper that writes the parent PID.
     • Add a take… helper.
   • Apply these rules to the take… helper:
     • Always delete the variable, including when the value is invalid. This matches takeRestartParentMarker.
     • Accept the value only when it uses the same ^[1-9]\d{0,9}$ syntax.
     • Accept the value only when it equals the restart-parent PID already validated by takeRestartParentMarker against process.ppid.
     • In all other cases, return null.
   • The marker carries no authority by itself. It only narrows the meaning of an already-validated actual parent.
   • Extend takeRestartHandoffMarkers in src/cli/restart-handoff.ts to return the validated stopped-parent PID next to the existing restartParent value. Keep existing call sites source-compatible.

   Task 3: Emit the marker only on the unsupervised completed-drain, listener-stopped path

   The parent must assert "I no longer serve and I wait for your health" only when that statement is true.

   • In src/server/management/system-restart.ts, apply the stopped-parent marker when waitForHealth is true. Do this in spawnDetachedStart (lines 265–276) or in replacementStartEnvironment (lines 250–263).
   • Today, acceptSystemRestart passes true only from the completed-drain branch, after await of listener stop (lines 442–468).
   • Verify that the listener-stop fallback, deadline, failed drain, and rejected drain branches all pass false. If any branch passes true after an unsuccessful listener stop, require a successful stop before passing true.
   • Do not change the desktop-supervised or service-supervised branches. They recycle or exit without a replacement spawn. See tests/clients/desktop-supervised-restart.test.ts:51–95.
   • Confirm that src/server/restart-replacement.ts (lines 347–385) forwards the marker unchanged into every early-exit retry attempt. withRestartParentMarker at line 363 already applies to request.env. The new marker uses the same env object.
   • Do not change readiness rules. waitForReplacementReady must keep rejecting the parent PID as replacement health.

   Task 4: Add a refusal-gated handoff observation in proxy-liveness

   Let observeProxyLiveness recognize only the known stopped parent.

   • In src/server/proxy-liveness.ts, extend the ProxyLivenessObservation union (7a1565bb:474–494) with a distinct handoff kind. For example: { kind: "restart-parent-stopped"; live: null; parentPid: number }.
   • Add an optional observation input, for example stoppedRestartParentPid?: number. Keep behavior unchanged for callers that omit it.
   • Return the handoff kind only when all of these conditions hold. In every other case, keep the current result:
     • No candidate produced a live OpenCodex identity. Any live owner still returns kind: "live". Never bypass a different owner that appears.
     • The recorded PID snapshot equals stoppedRestartParentPid.
     • The runtime record belongs to that same PID and the same home.
     • Every loopback candidate for the runtime-record port fails with recognized connection refusal (isConnectionRefused).
     • If the configured port is probed and differs from the runtime-record port, every loopback candidate for the configured port also fails with recognized connection refusal.
   • These results do not qualify. Keep returning unknown for them:
     • Timeout, reset, a non-200 response, an unreadable body, or mixed-address uncertainty.
     • A clean foreign HTTP 200. probeEndpointLiveness counts that response as dead, but it does not prove that the parent's endpoint is closed.
   • Build a refusal-only check on the existing classifyHealthz and loopbackProbeHosts primitives. Do not reuse the aggregate dead state.
   • If the recorded PID is not alive, keep the existing absent logic. The exited-parent case must not depend on the marker.

   Task 5: Accept the handoff kind only in the start recovery gate

   Pass the exception narrowly through handleStart.

   • In src/cli/start-owner-fence.ts, let recoverStartJournalAfterOwnerRecheck accept an optional stoppedRestartParentPid. Pass it to observeProxyLiveness.
   • Change the gate condition to accept either of these:
     • kind === "absent".
     • kind === "restart-parent-stopped" with parentPid equal to the passed value.
   • For every other kind, keep throwing the existing "Proxy ownership is unverified; startup refused…" error unchanged.
   • On the handoff kind, do not delete the parent's PID or runtime record while the parent is alive. The replacement publishes its own records at src/cli/index.ts:608–617. The parent's exit cleanup at src/cli/index.ts:664–665 removes only its own records.
     • Verify that this exit cleanup is conditional on owner PID. If it is not conditional, make it compare the PID first. Without that check, the parent's exit after readiness could delete the replacement's records.
   • Run cross-home checks and reconcileStartupJournal unchanged on the handoff kind. Before #7851, this path reached recovery through live: null. This change restores the pre-fix(cli): recheck live ownership before journal recovery #7851 behavior only for the verified case.
   • In src/cli/index.ts, in handleStart around lines 441–483, pass the validated stopped-parent PID from takeRestartHandoffMarkers into recoverStartStateUnderOwnershipLease.
   • Do not pass the value from handleEnsure. Ensure is never a restart replacement and must stay fail-closed.
   • Do not add any wait for parent exit on this branch. probeOwnerPastRestartParent already returns at once on live: null. Keep that behavior.
   • Confirm that the running parent does not hold the ownership lease that 7a1565bb now takes for ordinary starts. If the parent holds the lease, the child would block until the replacement deadline. If such a hold exists, document it and release the lease after listener stop on the completed-drain path.
   • Keep the later pre-bind fence at src/cli/index.ts:527–556 unchanged. It must still refuse any live owner.

   Task 6: Add P1 regressions and controls

   Cover the composition and every negative case.

   • In tests/lib/system-restart-contract-security.test.ts, test the stopped-parent marker:
     • It is consumed when invalid.
     • It is rejected when the value differs from the validated parent.
     • It is rejected when no valid restart-parent marker exists.
     • It is rejected when the value equals the own PID.
   • In tests/server/proxy-liveness-recovery.test.ts, cover:
     • Alive OpenCodex parent, matching marker, and all endpoints ECONNREFUSED → restart-parent-stopped.
     • The same setup without the marker → unknown. Keep the existing test at lines 35–40 unchanged.
     • Marker for a different PID → unknown.
     • ETIMEDOUT or reset on any candidate → unknown.
     • Foreign HTTP 200 → unknown.
     • Live identity on any candidate → live.
   • In tests/cli/cli-start-owner-absence.test.ts, test the completed-drain composition. Use the live test-parent PID, retained PID and runtime records, refused endpoints, and both markers.
     • Use the real probeOwnerPastRestartParent. The existing restart-parent case at lines 90–91 replaces it with probe => probe(). Do not copy that replacement.
     • Assert that recovery runs and start proceeds.
     • Assert that no wait for parent exit occurs. Use the fake clock with zero advance.
     • Assert that the parent's records are not deleted before the replacement publishes its own.
   • In the same file, add an unmarked control. The same retained live parent with refused endpoints and only OCX_RESTART_PARENT_PID must still refuse. It must preserve the PID and journal.
   • In the same file, add an exited-parent control. Extend lines 133–180 with a variant that carries both markers while the recorded parent has exited and the port is refused. The result must be ordinary absent recovery.
   • In tests/server/system-restart.test.ts (near lines 716–776) and tests/server/restart-replacement.test.ts, verify:
     • The marker appears only when waitForHealth is true, for a completed drain with a successful listener stop.
     • The marker is absent for the deadline, failed drain, rejected drain, listener-stop fallback, and supervised branches.
     • The marker survives early-exit retries.
     • Readiness still rejects parent health as replacement readiness.

   🤖 Prompt for AI agents

   Implement the verified stopped-parent handoff for the unsupervised
   completed-drain restart path. Allow recovery without waiting for parent exit,
   but keep arbitrary live and unknown owners refused.
   
   - Establish the baseline before product changes:
   - Fetch enough shallow history, rebase onto current `dev`, and verify that
   `7a1565bb968b343f68eddd6ea266222fcfd95ff1` is an ancestor of HEAD.
   - Re-read the affected recovery types, functions, and
   `handleStart`/`handleEnsure` call sites. Record actual line numbers in the PR.
   - Obtain the four post-merge product-module reproductions from PR #7851. Port P1
   cases to `tests/cli/cli-start-owner-absence.test.ts` and P2 cases to the Phase 2
   test files. Confirm the reported failures before changing product code.
   - Define a consumed parent-bound marker in `src/lib/system-restart-contract.ts`
   next to `OCX_RESTART_PARENT_PID`:
   - Add the environment constant and `with…`/`take…` helpers.
   `OCX_RESTART_PARENT_STOPPED_PID` is the proposed name.
   - Always consume the marker. Require `^[1-9]\d{0,9}$` syntax and equality with
   the restart-parent PID already validated against `process.ppid`. Otherwise
   return `null`.
   - Return the validated stopped-parent PID from `takeRestartHandoffMarkers` in
   `src/cli/restart-handoff.ts`. Keep existing call sites source-compatible.
   - Emit the marker in `src/server/management/system-restart.ts` only for
   `waitForHealth === true` after a completed drain and successful listener stop:
     - Use `spawnDetachedStart` or `replacementStartEnvironment`.
   - Verify that deadline, failed drain, rejected drain, and listener-stop fallback
   branches pass `false`. Tighten any incorrect successful-stop condition.
     - Do not change supervised branches.
   - Verify that `src/server/restart-replacement.ts` preserves the marker in every
   early-exit retry. Keep readiness rejection of the parent PID unchanged.
   - Extend `ProxyLivenessObservation` in `src/server/proxy-liveness.ts` with a
   distinct `restart-parent-stopped` kind containing `live: null` and `parentPid`:
   - Add optional `stoppedRestartParentPid` input. Preserve behavior when the input
   is omitted.
   - Require no live OpenCodex identity, a matching recorded PID, and a runtime
   record for the same PID and home.
   - Require recognized connection refusal on every loopback candidate for the
   runtime-record port. If a different configured port is probed, require refusal
   on every candidate for that port too.
   - Build the refusal-only check with `classifyHealthz` and `loopbackProbeHosts`.
   Do not use aggregate `dead` as proof of refusal.
   - Keep timeout, reset, non-200, unreadable-body, mixed-address uncertainty, and
   clean foreign HTTP 200 cases `unknown`. Any live identity must still return
   `live`.
   - Preserve ordinary `absent` recovery for an exited recorded PID, regardless of
   the marker.
   - Pass the validated PID only through `handleStart` and
   `recoverStartStateUnderOwnershipLease` into
   `recoverStartJournalAfterOwnerRecheck`:
   - In `src/cli/start-owner-fence.ts`, accept `absent` or a
   `restart-parent-stopped` observation whose `parentPid` matches the passed PID.
   Keep the existing refusal error unchanged for every other result.
   - Preserve the live parent's PID and runtime records until the replacement
   publishes its records. Verify owner-conditional exit cleanup at
   `src/cli/index.ts:664–665`; add a PID comparison if missing.
   - Keep cross-home checks, `reconcileStartupJournal`, the immediate `live: null`
   return from `probeOwnerPastRestartParent`, and the later pre-bind fence
   unchanged.
     - Do not pass this input from `handleEnsure`. Do not wait for parent exit.
   - Verify that the running parent does not hold the ordinary-start ownership
   lease. If it does, document the hold and release the lease after listener stop
   on the completed-drain path.
   - Add the specified security, liveness, start-composition, restart, and retry
   regressions:
   - Cover invalid and mismatched markers, no validated parent, and own-PID
   rejection.
   - Cover refused endpoints with and without the marker, different PID,
   timeout/reset, foreign HTTP 200, and live identity.
   - Use the real `probeOwnerPastRestartParent` and a zero-advance fake clock for
   the completed-drain composition. Verify start progress and preservation of
   parent records.
   - Verify the unmarked refusal control and ordinary exited-parent recovery with
   both markers.
   - Verify marker absence on all excluded branches, marker survival across
   retries, and continued rejection of parent health.
   

   Phase 2: P2 — Separate bind scope from probe address

   Goal. Integration policy must use the scope the listener bound. It must not use the loopback address that answered the health probe.

   Failure mode.

   1. handleEnsure's early findProxyOwnerBeforeJournalRecovery misses a live 0.0.0.0 proxy (src/cli/index.ts:816–826).
   2. The fresh observation under the lease finds the proxy through 127.0.0.1. In 7a1565bb, recoverStartJournalAfterOwnerRecheck sets owner.live = observation.live. The observation returns the responding address as hostname.
   3. handleEnsure takes the live branch and forwards live.hostname (src/cli/index.ts:853–860). The value passes through ensureGrokFenceMatchesDesired (src/cli/ensure-desired-integrations.ts:128–134) and syncGrokConfig (src/grok/sync.ts:51–60).
   4. isLoopbackHostname("127.0.0.1") passes in src/grok/inject.ts:1142–1165. The injector writes api_key = "opencodex-loopback" (lines 1032–1040). That listener requires an admission token.

   This defect causes incorrect client configuration. It is not a server authentication bypass.

   Scope exclusion. The spawned-ensure branch at src/cli/index.ts:879–890 keeps only .port and reconciles with config hostname. It never receives a probe address. Record this metadata loss separately. Do not change that branch in this ticket.

   Task 1: Add `probeAddress` and keep `hostname` as bind scope

   Represent both values without changing the meaning of hostname for existing consumers.

   • In src/server/proxy-liveness.ts, add optional probeAddress?: string to LiveProxy (lines 152–158). It holds the bracketed or normalized address that answered.
   • Update the doc comment. hostname is bind scope. It comes from either the runtime record or config. State that configured-port fallback is configured scope, not listener-attested scope.
   • When no bind metadata exists, keep hostname undefined. Never fill it from the responding address. Downstream code (opts.hostname ?? config.hostname) keeps its current fallback.
   • Correct the comment at src/cli/index.ts:853–856. It currently claims that the value is what the proxy "ACTUALLY bound". State the real provenance instead.

   Task 2: Fix every producer that writes the responding address into `hostname`

   Correct the observation path. Correct probePortOwner for consistency.

   • In observeProxyLiveness and its live branch at 7a1565bb:src/server/proxy-liveness.ts, locate the code that returns the successful loopback candidate as hostname. This is the confirmed P2 site. Task 1.1 records its exact line.
     • Return hostname from the runtime record or config. Use the same provenance as the three findLiveProxy branches at lines 491–553.
     • Return the successful candidate as probeAddress.
   • Confirm that the findLiveProxy branches already preserve runtime.hostname, record.hostname, and config.hostname. Add probeAddress there only if the dial candidate is available without restructuring.
   • In probePortOwner (lines 591–601), change return { ...identity, hostname }. Return the candidate as probeAddress. Keep hostname as the input bind scope (opts.hostname).
     • The only production caller, chooseListenPort at src/cli/index.ts:318–339, does not read holder.hostname. Its behavior does not change.
     • Update tests/server/proxy-liveness.test.ts:301–313. Change the expectation from hostname: "[::1]" to probeAddress: "[::1]".
   • Do not change loopbackProbeHosts, alternate-family probing, wildcard normalization in probeHostname, or cross-home discovery in src/cli/cross-home-owner.ts:217–245.

   Task 3: Add P2 regressions and controls

   Prove that the missed-early-probe composition preserves wildcard scope through the real Grok writer.

   • In tests/server/proxy-liveness-recovery.test.ts, cover:
     • A runtime record with hostname: "0.0.0.0" that answers on 127.0.0.1 → observation live with hostname: "0.0.0.0" and probeAddress: "127.0.0.1".
     • Loopback control: a record with 127.0.0.1 or ::1 keeps that hostname.
   • Add an ensure composition test. Prefer extending the extracted-command harness in tests/cli/cli-start-owner-absence.test.ts or tests/clients/sync-client-integrations.test.ts:930–975. If a new file is created under tests/cli/, update the layout map as required by AGENTS.md:14–28.
     • Make the first ensure probe fail transiently.
     • Make the recovery observation find the wildcard-bound proxy through loopback.
     • Run the real reconcileEnsureDesiredIntegrations, the real syncGrokConfig, and the real injector against an isolated temporary grokHome.
     • Do not stub syncGrokConfig. tests/cli/ensure-desired-integrations-race.test.ts stubs it. Do not copy that setup.
     • Assert skippedReason: "non-loopback". Assert that no config.toml exists. Reuse the assertion shape from tests/providers/xai/grok-sync.test.ts:226–249.
   • In the same harness, add a normal loopback control. A proxy bound to loopback and recovered through the same path must write a Grok config with the loopback base_url.
   • Keep these existing tests unchanged and passing:
     • tests/providers/xai/grok-sync.test.ts:226–249: wildcard skip and IPv6 http://[::1]:10100/v1.
     • tests/providers/xai/grok-sync.test.ts:251–303: the explicit unauthenticated loopback-listener exception, which writes http://127.0.0.1:10102/v1, and the companion-listener case.

   🤖 Prompt for AI agents

   Separate listener bind scope from the address that answered a health probe.
   Preserve wildcard scope through ensure recovery so the real Grok writer refuses
   non-loopback configuration.
   
   - Update `LiveProxy` in `src/server/proxy-liveness.ts:152–158`:
   - Add optional `probeAddress?: string` for the bracketed or normalized
   responding address.
   - Keep `hostname` as bind scope from the runtime record or config. Document that
   configured-port fallback is configured scope, not listener-attested scope.
   - Leave `hostname` undefined when bind metadata is absent. Preserve the
   downstream `opts.hostname ?? config.hostname` fallback.
   - Correct live-result producers:
   - At the `observeProxyLiveness` site identified in Task 1.1, return
   runtime-record or config scope as `hostname`. Return the successful candidate as
   `probeAddress`. Follow the provenance of the three `findLiveProxy` branches at
   lines 491–553.
   - Verify that `findLiveProxy` preserves `runtime.hostname`, `record.hostname`,
   and `config.hostname`. Add `probeAddress` only where the dial candidate is
   available without restructuring.
   - In `probePortOwner` at lines 591–601, use `opts.hostname` for bind scope and
   the responding candidate for `probeAddress`.
   - Update `tests/server/proxy-liveness.test.ts:301–313` to expect `probeAddress:
   "[::1]"` instead of `hostname: "[::1]"`. The production caller
   `chooseListenPort` does not read `holder.hostname`.
   - Correct the provenance comment at `src/cli/index.ts:853–856`; do not claim
   listener-attested scope.
   - Preserve existing dialing and discovery behavior:
   - Do not change `loopbackProbeHosts`, alternate-family probing, `probeHostname`
   wildcard normalization, or `src/cli/cross-home-owner.ts:217–245`.
   - Leave spawned-ensure metadata loss at `src/cli/index.ts:879–890` unchanged.
   Record it as a separate observation.
   - Add liveness regressions in `tests/server/proxy-liveness-recovery.test.ts`:
   - A `0.0.0.0` runtime record recovered through `127.0.0.1` must retain
   `hostname: "0.0.0.0"` and report `probeAddress: "127.0.0.1"`.
   - A loopback runtime record must retain its `127.0.0.1` or `::1` bind hostname.
   - Add the ensure-to-Grok composition and loopback control:
   - Prefer the extracted-command harness in
   `tests/cli/cli-start-owner-absence.test.ts` or
   `tests/clients/sync-client-integrations.test.ts:930–975`. If adding a new
   `tests/cli/` file, update the layout map required by `AGENTS.md:14–28`.
   - Make the early ensure probe miss transiently, then recover the wildcard-bound
   proxy through loopback under the lease.
   - Use real `reconcileEnsureDesiredIntegrations`, `syncGrokConfig`, and the
   injector with an isolated temporary `grokHome`. Do not copy the stubbed
   `syncGrokConfig` setup from
   `tests/cli/ensure-desired-integrations-race.test.ts`.
   - Assert `skippedReason: "non-loopback"` and no `config.toml`, using the
   assertion shape in `tests/providers/xai/grok-sync.test.ts:226–249`.
   - Through the same recovery path, verify that a normal loopback proxy writes the
   loopback `base_url`.
   - Keep the existing wildcard, IPv6, explicit unauthenticated loopback-listener
   exception, and companion-listener tests at
   `tests/providers/xai/grok-sync.test.ts:226–303` unchanged and passing.
   

   Phase 3: Verification and evidence disposition

   Goal. Produce the closure evidence required by the ticket. Link the fix and focused regression evidence for each item. Provide an explicit E2E record.

   Task 1: Run focused validation under the pinned toolchain

   • Run the changed and affected suites with Bun 1.4.4, the repository preload, and isolated temporary homes:
     • tests/cli/cli-restart-handoff.test.ts
     • tests/cli/cli-start-owner-absence.test.ts
     • tests/cli/cli-start-journal-order.test.ts
     • tests/server/proxy-liveness*.test.ts
     • tests/server/system-restart*.test.ts
     • tests/server/restart-replacement.test.ts
     • tests/lib/system-restart-contract-security.test.ts
     • tests/cli/ensure-desired-integrations-race.test.ts
     • tests/providers/xai/grok-sync.test.ts
   • Run the focused typecheck and changed-test coverage required by src/AGENTS.md:22–29.
   • Confirm that the ported post-merge reproductions from Task 1.1 now assert the fixed behavior. Confirm that their controls still pass.

   Task 2: Verify or record real process-replacement E2E

   • Attempt a real dashboard-restart E2E in an isolated home:
     1. Start an unsupervised proxy.
     2. Trigger the system restart endpoint.
     3. Confirm a completed drain.
     4. Confirm that the replacement publishes new PID and runtime records.
     5. Confirm that the parent exits 0.
     6. Confirm that the replacement serves /healthz.
   • If the existing tests/ci-workflows/package-tree-*restart*.test.ts harness can run this composition, extend it. Otherwise, state in the PR that real process-replacement and dashboard E2E is untested. List the module tests that cover each step.
   • In the issue, link each item, P1 and P2, to its PR or PRs and its regression evidence. Separate PRs are acceptable.
   • Note the spawned-ensure metadata loss (src/cli/index.ts:879–890) as a separate observation. State that this ticket does not change it.

   🤖 Prompt for AI agents

   Validate both fixes under the pinned toolchain and document the closure
   evidence. Attempt real process-replacement E2E and state its status explicitly.
   
   - Use Bun 1.4.4, the repository preload, and isolated temporary homes to run:
     - `tests/cli/cli-restart-handoff.test.ts`
     - `tests/cli/cli-start-owner-absence.test.ts`
     - `tests/cli/cli-start-journal-order.test.ts`
     - `tests/server/proxy-liveness*.test.ts`
     - `tests/server/system-restart*.test.ts`
     - `tests/server/restart-replacement.test.ts`
     - `tests/lib/system-restart-contract-security.test.ts`
     - `tests/cli/ensure-desired-integrations-race.test.ts`
     - `tests/providers/xai/grok-sync.test.ts`
   - Run the focused typecheck and changed-test coverage required by
   `src/AGENTS.md:22–29`. Confirm that the ported post-merge reproductions assert
   the fixed behavior and that their controls pass.
   - Attempt dashboard-restart E2E in an isolated home:
     - Start an unsupervised proxy and trigger the system restart endpoint.
   - Confirm completed drain, publication of new PID and runtime records, parent
   exit 0, and replacement `/healthz` service.
   - If `tests/ci-workflows/package-tree-*restart*.test.ts` can run this
   composition, extend that harness.
   - Otherwise, state in the PR that real process-replacement and dashboard E2E is
   untested. Map each step to the module tests that cover it.
   - Link P1 and P2 in the issue to their PR or PRs and regression evidence.
   Separate PRs are acceptable.
   - Record spawned-ensure metadata loss at `src/cli/index.ts:879–890` as a
   separate observation that this ticket does not change.
   

   Research

   Ticket Summary

   This plan fixes two follow-up defects from #7851. That change merged as 7a1565bb968b343f68eddd6ea266222fcfd95ff1.

   • P1: On the unsupervised completed-drain restart path, the replacement's fresh recovery observation returns unknown because the waiting parent's PID record is still live. The replacement refuses startup. The parent then exits 1.
   • P2: After an early ensure probe misses a live wildcard-bound proxy, the recovered owner carries the responding loopback address as hostname. Grok then writes a tokenless loopback configuration instead of refusing non-loopback scope.

   The fix requires:

   • A narrow, parent-verified handoff for P1.
   • Separate bind-scope and probe-address values for P2.
   • Focused regressions for both defects, including the exited-parent control and the loopback control.
   • An explicit disposition for real process-replacement E2E testing.

   Codebase Summary

   OpenCodex is a Bun-native TypeScript CLI and proxy.

   Restart path

   • src/server/management/system-restart.ts runs dashboard restarts. acceptSystemRestart (lines 420–483) drains, stops the listener, and calls spawnStart(restartPort, drainOutcome === "completed").
   • src/server/restart-replacement.ts spawns the child. spawnReplacementStart (lines 347–374) adds OCX_RESTART_PARENT_PID for every replacement at line 363. It then waits for replacement readiness. Readiness rejects the parent's own health (lines 64–110).
   • The parent keeps ocx.pid and runtime-port.json until exit cleanup (src/cli/index.ts:664–665).

   Replacement start path

   • handleStart (src/cli/index.ts:437–483) consumes the markers through takeRestartHandoffMarkers.
   • It calls probeOwnerPastRestartParent (src/cli/restart-handoff.ts:79–110) and decideStartWithLiveOwner (src/cli/dispatch.ts:1110–1125).
   • It then calls recoverStartStateUnderOwnershipLease (src/cli/start-owner-fence.ts).

   Revision caveat

   • The checked-out HEAD 88aaa754 does not contain commit 7a1565bb in its available shallow ancestry.
   • The ticket states that dev blobs match 7a1565bb.
   • In 7a1565bb, the following code exists:
     • ProxyLivenessObservation (live | absent | unknown) and observeProxyLiveness, at 7a1565bb:src/server/proxy-liveness.ts:474–494.
     • The recovery gate that throws "Proxy ownership is unverified; startup refused…", at 7a1565bb:src/cli/start-owner-fence.ts:6–21.
     • A lease for ordinary starts, at 7a1565bb:src/cli/start-owner-fence.ts:23–49.

   Hostname and Grok path

   • LiveProxy.hostname is documented as the raw bind hostname (src/server/proxy-liveness.ts:152–158).
   • probeHostname normalizes wildcard binds to 127.0.0.1 only for dialing (lines 218–223).
   • probePortOwner already overloads hostname as the responding address (lines 591–601).
   • The live ensure branch forwards live.hostname (src/cli/index.ts:853–860).
   • That value goes to reconcileEnsureDesiredIntegrations, then to syncGrokConfig, then to the Grok injector.
   • The injector refuses non-loopback scope (src/grok/inject.ts:1142–1165).

   Tests

   Tests live in subsystem directories, as defined in AGENTS.md:14–28. New test files need layout-map coverage.

   Relevant suites:

   • tests/cli/cli-restart-handoff.test.ts
   • tests/cli/cli-start-owner-absence.test.ts
   • tests/server/proxy-liveness-recovery.test.ts
   • tests/server/system-restart.test.ts
   • tests/server/restart-replacement.test.ts
   • tests/lib/system-restart-contract-security.test.ts
   • tests/providers/xai/grok-sync.test.ts

   File-Level Change Summary and Follow-Up Agent Instructions

   Product files

   File	Change	Item
   src/lib/system-restart-contract.ts	Add the stopped-parent marker constant and with/take helpers. The marker must equal the validated parent PID and is always consumed.	P1
   src/cli/restart-handoff.ts	takeRestartHandoffMarkers also returns the validated stopped-parent PID. Do not change probeOwnerPastRestartParent.	P1
   src/server/management/system-restart.ts	Emit the marker only when waitForHealth is true, for a completed drain after a successful listener stop. Verify that all fallback branches pass false.	P1
   src/server/restart-replacement.ts	No logic change is expected. Verify that the marker survives retries and that readiness still rejects the parent.	P1
   src/server/proxy-liveness.ts	Add the restart-parent-stopped observation kind and refusal-only gating. Add LiveProxy.probeAddress. Return bind scope in hostname from observeProxyLiveness and probePortOwner.	P1, P2
   src/cli/start-owner-fence.ts	Accept the handoff kind only with a matching passed parent PID. Do not delete the live parent's records.	P1
   src/cli/index.ts	Pass the stopped-parent PID from handleStart, but not from handleEnsure. Verify that exit cleanup is owner-conditional. Correct the provenance comment at lines 853–856.	P1, P2

   Test files

   • tests/lib/system-restart-contract-security.test.ts
   • tests/server/proxy-liveness-recovery.test.ts
   • tests/server/proxy-liveness.test.ts (lines 301–313, the expectation update)
   • tests/cli/cli-start-owner-absence.test.ts
   • tests/server/system-restart.test.ts
   • tests/server/restart-replacement.test.ts
   • An ensure-to-Grok composition test under tests/cli/ or tests/clients/

   Instructions for follow-up agents

   1. Work on dev with 7a1565bb in the ancestry. Do not implement on 88aaa754.
   2. Never convert unknown to absent. The only new start permission is restart-parent-stopped. Require the validated actual parent, the matching marker, matching records, and recognized connection refusal on every candidate.
   3. Do not add any wait for parent exit on the completed-drain path.
   4. Keep LiveProxy.hostname as bind scope. Put the responding address only in probeAddress.
   5. Use the real syncGrokConfig and the real injector in the P2 composition test.
   6. State the E2E status explicitly in the PR.

   ---

   🚀 Next Steps

   🤖 All AI agent prompts combined

   Task: 1
   
   Implement the verified stopped-parent handoff for the unsupervised
   completed-drain restart path. Allow recovery without waiting for parent exit,
   but keep arbitrary live and unknown owners refused.
   
   - Establish the baseline before product changes:
   - Fetch enough shallow history, rebase onto current `dev`, and verify that
   `7a1565bb968b343f68eddd6ea266222fcfd95ff1` is an ancestor of HEAD.
   - Re-read the affected recovery types, functions, and
   `handleStart`/`handleEnsure` call sites. Record actual line numbers in the PR.
   - Obtain the four post-merge product-module reproductions from PR #7851. Port P1
   cases to `tests/cli/cli-start-owner-absence.test.ts` and P2 cases to the Phase 2
   test files. Confirm the reported failures before changing product code.
   - Define a consumed parent-bound marker in `src/lib/system-restart-contract.ts`
   next to `OCX_RESTART_PARENT_PID`:
   - Add the environment constant and `with…`/`take…` helpers.
   `OCX_RESTART_PARENT_STOPPED_PID` is the proposed name.
   - Always consume the marker. Require `^[1-9]\d{0,9}$` syntax and equality with
   the restart-parent PID already validated against `process.ppid`. Otherwise
   return `null`.
   - Return the validated stopped-parent PID from `takeRestartHandoffMarkers` in
   `src/cli/restart-handoff.ts`. Keep existing call sites source-compatible.
   - Emit the marker in `src/server/management/system-restart.ts` only for
   `waitForHealth === true` after a completed drain and successful listener stop:
     - Use `spawnDetachedStart` or `replacementStartEnvironment`.
   - Verify that deadline, failed drain, rejected drain, and listener-stop fallback
   branches pass `false`. Tighten any incorrect successful-stop condition.
     - Do not change supervised branches.
   - Verify that `src/server/restart-replacement.ts` preserves the marker in every
   early-exit retry. Keep readiness rejection of the parent PID unchanged.
   - Extend `ProxyLivenessObservation` in `src/server/proxy-liveness.ts` with a
   distinct `restart-parent-stopped` kind containing `live: null` and `parentPid`:
   - Add optional `stoppedRestartParentPid` input. Preserve behavior when the input
   is omitted.
   - Require no live OpenCodex identity, a matching recorded PID, and a runtime
   record for the same PID and home.
   - Require recognized connection refusal on every loopback candidate for the
   runtime-record port. If a different configured port is probed, require refusal
   on every candidate for that port too.
   - Build the refusal-only check with `classifyHealthz` and `loopbackProbeHosts`.
   Do not use aggregate `dead` as proof of refusal.
   - Keep timeout, reset, non-200, unreadable-body, mixed-address uncertainty, and
   clean foreign HTTP 200 cases `unknown`. Any live identity must still return
   `live`.
   - Preserve ordinary `absent` recovery for an exited recorded PID, regardless of
   the marker.
   - Pass the validated PID only through `handleStart` and
   `recoverStartStateUnderOwnershipLease` into
   `recoverStartJournalAfterOwnerRecheck`:
   - In `src/cli/start-owner-fence.ts`, accept `absent` or a
   `restart-parent-stopped` observation whose `parentPid` matches the passed PID.
   Keep the existing refusal error unchanged for every other result.
   - Preserve the live parent's PID and runtime records until the replacement
   publishes its records. Verify owner-conditional exit cleanup at
   `src/cli/index.ts:664–665`; add a PID comparison if missing.
   - Keep cross-home checks, `reconcileStartupJournal`, the immediate `live: null`
   return from `probeOwnerPastRestartParent`, and the later pre-bind fence
   unchanged.
     - Do not pass this input from `handleEnsure`. Do not wait for parent exit.
   - Verify that the running parent does not hold the ordinary-start ownership
   lease. If it does, document the hold and release the lease after listener stop
   on the completed-drain path.
   - Add the specified security, liveness, start-composition, restart, and retry
   regressions:
   - Cover invalid and mismatched markers, no validated parent, and own-PID
   rejection.
   - Cover refused endpoints with and without the marker, different PID,
   timeout/reset, foreign HTTP 200, and live identity.
   - Use the real `probeOwnerPastRestartParent` and a zero-advance fake clock for
   the completed-drain composition. Verify start progress and preservation of
   parent records.
   - Verify the unmarked refusal control and ordinary exited-parent recovery with
   both markers.
   - Verify marker absence on all excluded branches, marker survival across
   retries, and continued rejection of parent health.
   ===============================================================================
   
   Task: 2
   
   Separate listener bind scope from the address that answered a health probe.
   Preserve wildcard scope through ensure recovery so the real Grok writer refuses
   non-loopback configuration.
   
   - Update `LiveProxy` in `src/server/proxy-liveness.ts:152–158`:
   - Add optional `probeAddress?: string` for the bracketed or normalized
   responding address.
   - Keep `hostname` as bind scope from the runtime record or config. Document that
   configured-port fallback is configured scope, not listener-attested scope.
   - Leave `hostname` undefined when bind metadata is absent. Preserve the
   downstream `opts.hostname ?? config.hostname` fallback.
   - Correct live-result producers:
   - At the `observeProxyLiveness` site identified in Task 1.1, return
   runtime-record or config scope as `hostname`. Return the successful candidate as
   `probeAddress`. Follow the provenance of the three `findLiveProxy` branches at
   lines 491–553.
   - Verify that `findLiveProxy` preserves `runtime.hostname`, `record.hostname`,
   and `config.hostname`. Add `probeAddress` only where the dial candidate is
   available without restructuring.
   - In `probePortOwner` at lines 591–601, use `opts.hostname` for bind scope and
   the responding candidate for `probeAddress`.
   - Update `tests/server/proxy-liveness.test.ts:301–313` to expect `probeAddress:
   "[::1]"` instead of `hostname: "[::1]"`. The production caller
   `chooseListenPort` does not read `holder.hostname`.
   - Correct the provenance comment at `src/cli/index.ts:853–856`; do not claim
   listener-attested scope.
   - Preserve existing dialing and discovery behavior:
   - Do not change `loopbackProbeHosts`, alternate-family probing, `probeHostname`
   wildcard normalization, or `src/cli/cross-home-owner.ts:217–245`.
   - Leave spawned-ensure metadata loss at `src/cli/index.ts:879–890` unchanged.
   Record it as a separate observation.
   - Add liveness regressions in `tests/server/proxy-liveness-recovery.test.ts`:
   - A `0.0.0.0` runtime record recovered through `127.0.0.1` must retain
   `hostname: "0.0.0.0"` and report `probeAddress: "127.0.0.1"`.
   - A loopback runtime record must retain its `127.0.0.1` or `::1` bind hostname.
   - Add the ensure-to-Grok composition and loopback control:
   - Prefer the extracted-command harness in
   `tests/cli/cli-start-owner-absence.test.ts` or
   `tests/clients/sync-client-integrations.test.ts:930–975`. If adding a new
   `tests/cli/` file, update the layout map required by `AGENTS.md:14–28`.
   - Make the early ensure probe miss transiently, then recover the wildcard-bound
   proxy through loopback under the lease.
   - Use real `reconcileEnsureDesiredIntegrations`, `syncGrokConfig`, and the
   injector with an isolated temporary `grokHome`. Do not copy the stubbed
   `syncGrokConfig` setup from
   `tests/cli/ensure-desired-integrations-race.test.ts`.
   - Assert `skippedReason: "non-loopback"` and no `config.toml`, using the
   assertion shape in `tests/providers/xai/grok-sync.test.ts:226–249`.
   - Through the same recovery path, verify that a normal loopback proxy writes the
   loopback `base_url`.
   - Keep the existing wildcard, IPv6, explicit unauthenticated loopback-listener
   exception, and companion-listener tests at
   `tests/providers/xai/grok-sync.test.ts:226–303` unchanged and passing.
   ===============================================================================
   
   Task: 3
   
   Validate both fixes under the pinned toolchain and document the closure
   evidence. Attempt real process-replacement E2E and state its status explicitly.
   
   - Use Bun 1.4.4, the repository preload, and isolated temporary homes to run:
     - `tests/cli/cli-restart-handoff.test.ts`
     - `tests/cli/cli-start-owner-absence.test.ts`
     - `tests/cli/cli-start-journal-order.test.ts`
     - `tests/server/proxy-liveness*.test.ts`
     - `tests/server/system-restart*.test.ts`
     - `tests/server/restart-replacement.test.ts`
     - `tests/lib/system-restart-contract-security.test.ts`
     - `tests/cli/ensure-desired-integrations-race.test.ts`
     - `tests/providers/xai/grok-sync.test.ts`
   - Run the focused typecheck and changed-test coverage required by
   `src/AGENTS.md:22–29`. Confirm that the ported post-merge reproductions assert
   the fixed behavior and that their controls pass.
   - Attempt dashboard-restart E2E in an isolated home:
     - Start an unsupervised proxy and trigger the system restart endpoint.
   - Confirm completed drain, publication of new PID and runtime records, parent
   exit 0, and replacement `/healthz` service.
   - If `tests/ci-workflows/package-tree-*restart*.test.ts` can run this
   composition, extend that harness.
   - Otherwise, state in the PR that real process-replacement and dashboard E2E is
   untested. Map each step to the module tests that cover it.
   - Link P1 and P2 in the issue to their PR or PRs and regression evidence.
   Separate PRs are acceptable.
   - Record spawned-ensure metadata loss at `src/cli/index.ts:879–890` as a
   separate observation that this ticket does not change.
   

   💡 Iterate on the plan with: @coderabbitai <feedback>

   Example Feedback
   - @coderabbitai You can skip phase 3. Add a simple unit test case for phase 2.
   - @coderabbitai For design choice 1 go ahead with option 3 and replan.
   

   ---

   Start Coding →
6. lidge-jun

   added a commit that references this issue on Oct 28, 2026

   fix(restart): retire drained parent ownership and preserve bind scope (…

   ...

   f29091a
7. lidge-jun commented on Oct 28, 2026

   

   lidge-jun

   on Oct 28, 2026

   Owner

   More actions

   Fixed on dev by #7874 (merge f29091a5).

   • P1 completed-drain restart handoff: the parent retires its own PID record under the ownership lease only after a completed drain and a successful listener stop, checking its own PID and runtime port; mismatched or malformed records are preserved, and live/unknown owners stay fenced. Verified with a real Windows parent/child handoff covering completed drain, still-live parent, refused endpoints and the exited-parent control; disabling retirement reproduces the original parent exit 1.
   • P2 bind scope vs discovery address: recovery now carries the bind scope (hostname, admission policy) and the responding address (probeHostname, destination only) separately through ensure, reconciliation and Grok sync; regression in tests/server/proxy-liveness-bind-scope.test.ts.
   • Independent security review: PASS, 0 blocking. Exact-head PR CI: 33/33.

   Recorded as untested: a full dashboard HTTP/auth/provider-boot process-replacement E2E, and native POSIX/macOS execution of the handoff. The hosted macOS/Windows matrix runs in the next lane=all dev batch.
8. lidge-jun

   closed this as completedon Oct 28, 2026
9. github-actions commented on Oct 28, 2026

   

   github-actionsBot

   on Oct 28, 2026 – with GitHub Actions

   Contributor

   More actions

   _{Automated translation bookkeeping — detected language: English.}