Conversation
Starting a second proxy with its own OPENCODEX_HOME while another proxy was running (for example the default home on 10100) rewrote the shared ~/.grok/config.toml opencodex base_url, and the same class of Codex and Claude startup sync, to the second proxy's port, then stripped or left it on exit. Owner discovery only read the second home's own records and configured port, so the existing one-way sibling mark was never set. ocx start now runs a cross-home owner check when same-home discovery finds no owner, before journal recovery and before any client write, and again under the bind lease. It reads the default home's runtime-port.json (only when the resolved home differs), the managed Grok base_url and the managed Codex provider base_url, keeps loopback URLs with an explicit port, and identity probes each port. A live OpenCodex whose reported PID is a positive integer other than this process sets the existing sibling mark, so every writer that honors it stays off the shared files. A foreign, stale, remote or pid-less answer grants nothing, so a lone custom-home instance still syncs. Journal recovery now runs only after that decision and never for a sibling. The Claude agent roster startup sync and the ensure-time Grok fence, which did not consult the mark, now skip for a sibling. Explicit ocx sync and ocx grok apply are unchanged.
|
Warning Review limit reachedNext included review available in 1 minute. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (18)
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 4bfae6f055
ℹ️ 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 owner = await findProxyOwnerBeforeJournalRecovery({ probeConfiguredPort: true }); | ||
| if (!owner.live) reconcileStartupJournal(); |
There was a problem hiding this comment.
When ocx ensure runs under a second OPENCODEX_HOME, this path never calls findCrossHomeOwner before recovering the journal. Although the spawned start child detects the live owner and marks itself as a sibling, that mark is process-local; after waitForProxy, the unmarked ensure parent still calls reconcileEnsureDesiredIntegrations, syncModelsToCodex, and syncClaudeAgentDefsAtProxyStartup. It can therefore replay the second home's journal and repoint or remove the live owner's shared Codex, Grok, and Claude configuration—the exact failure this change is meant to prevent. Apply the cross-home ownership decision in handleEnsure and suppress all parent-side shared-client writes when an owner is found.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the batch commit ca24aaf (#6113). handleEnsure now calls markCrossHomeSibling() before journal recovery, so the ensure parent marks itself, and every parent-side shared-client call honors the mark: Codex sync through the desired-state gate, the Grok fence, Claude agent sync, system-env injection, and the shell-hook and ensure-integration call sites. A repeated ensure that finds this home's sibling already live also defers to the other owner. The new regression in tests/cli/sibling-home-client-sync.test.ts reproduced the roster deletion before the fix.
| const bytes = Buffer.alloc(MAX_HINT_BYTES + 1); | ||
| const count = readSync(fd, bytes, 0, bytes.length, 0); | ||
| return count > MAX_HINT_BYTES ? null : bytes.toString("utf8", 0, count); |
There was a problem hiding this comment.
When the live owner uses another custom home and discovery depends on the shared Codex or Grok hint, any valid config.toml larger than 256 KiB is returned as null here, even if its managed provider block is present near the beginning. With no default-home runtime record to fall back to, the later start is treated as an ordinary owner and rewrites the shared client configuration. Large Codex configs are otherwise supported by the ownership paths, so the bounded discovery should retain enough evidence to identify a managed loopback target, or fail safely rather than equating an oversized file with an absent hint.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ca24aaf (#6113). Hints are now read the way their writers read them: Codex through readBoundedCodexConfig (1 MiB, the same cap the Codex injector enforces, so a larger config is refused by the writer too), and Grok in full, as the Grok injector does. Tests cover a Grok config over 256 KiB and a Codex config between 256 KiB and 1 MiB.
|
✅ Deterministic PR hygiene checks passed. |
리뷰 · 우선순위 66 / 80이 PR은 프록시를 하나 더 켜도, 먼저 켜 둔 쪽의 클라이언트 설정을 건드리지 않게 해요. 베이스는 기본 홈이 10100에서 이미 돌아가는데, 다른 폴더를 주인 찾기가 나중 홈의 기록과 포트만 봤어요. 먼저 켜 둔 쪽을 못 찾아서, 공유 파일을 건너뛰는 표시가 안 켜졌어요. 그 표시는 원래 같은 홈의 형제에게만 있었어요. 이제는 표시가 켜지면 Codex 동기화, Grok을 시작할 때 맞추는 일, 끌 때 지우는 일, 카탈로그, launchd, 셸 훅은 공유 파일을 안 건드려요. 이번에 그 표시를 새로 보는 곳은 Claude 에이전트 명단과, PR이 적어 둔 한계도 코드와 같아요. 둘 다 기록이나 주소를 남기기 전에 켜지면 둘 다 쓸 수 있어요. 본체가 재시작으로 잠깐 내려간 사이에 형제가 켜져도 그래요. src/cli/index.ts:751 - src/cli/cross-home-owner.ts:24 - 힌트 파일이 256KiB보다 크면 내용을 버리고 힌트가 없는 것으로 봐요. 관리 블록이 파일 앞쪽에 있어도요. 기본 홈의 메인테이너의 판단이 필요한 지점
256KiB를 넘는 관리 설정은 힌트가 없는 것으로 둘지, 그 경우 쓰기를 멈출지예요. 혼자 쓰는 커스텀 홈은 계속 동기화해야 하고, 이미 떠 있는 주인은 놓치면 안 돼요. 너의 추천
이 댓글은 grok-bot이 작성했습니다 |
|
This slice is integrated through the batch PR #6113 on current dev (6d64ea2), one commit per slice, so the three fixes need one CI cycle instead of three serial rebase cycles. The commit is tree-identical to this PR's head, plus the two Codex review fixes for #6108. I will close this PR once #6113 merges. |
|
Superseded by #6113, merged into |
1 participant
Summary
Starting a second proxy with its own
OPENCODEX_HOMEwhile another proxy was already running rewrote the other proxy's client routing. Example: the default home runs on 10100, andOPENCODEX_HOME=/tmp/x ocx start --port 10200changes the managedbase_urlin~/.grok/config.tomlto 10200 (and the same class of Codex and Claude startup sync). When the second proxy exits, it strips the block or leaves it pointing at a dead port. The cause is that owner discovery (findProxyOwnerBeforeJournalRecovery) only reads the second home's records and configured port. It never finds the primary, so the existing one-way sibling mark (src/codex/sibling-start.ts), which already keeps a same-home sibling off shared client files, is never set.After this change,
ocx startruns a cross-home owner check (src/cli/cross-home-owner.ts) when same-home discovery finds no owner. It runs before journal recovery and before any client write, and again under the bind lease.runtime-port.json(read only when the resolvedOPENCODEX_HOMEdiffers from<homedir>/.opencodex), the managed Grok[model_providers.opencodex].base_url, and the managed Codex providerbase_url. Reads are bounded, and only loopback URLs with an explicit port count.probePortOwner. A hit requires an OpenCodex health identity whose PID is a positive integer different from this process. A foreign service, a stale port, a remote or malformed URL, or a legacypid: nullanswer grants nothing, so a lone custom-home install keeps its automatic sync.ocx syncandocx grok applyare unchanged.Docs:
structure/runtime.md,structure/codex-home.md,structure/clients/integrations.md, and the CLI lifecycle reference (en, ko, ja, zh-cn, zh-tw) describe the cross-home condition.src/cli/index.tsstays at 1,999 lines (ratchet threshold 2,000).Known limits: two homes that start before either has published a record or a client URL can still both sync, and so can a sibling that starts while the primary is briefly down during its own restart. This change fixes the observed case (a running primary and a later sibling) without adding a cross-home lock. A hint that points only at the data-only listener, which has no
/healthz, is treated as unknown. The Claude intercept settings migration insrc/claude/intercept/runtime.ts(run duringstartServerwhen intercept is enabled) does not consult the mark yet. It is owned by a different area and is tracked separately.Verification
bun test tests/cli/sibling-home-client-sync.test.ts tests/cli/cli-start-journal-order.test.ts tests/cli/hub-gated-local-clients.test.ts tests/providers/xai/grok-lifecycle.test.ts tests/claude-integration/claude-agent-startup-sync.test.ts tests/codex-integration/codex-desired-state.test.ts tests/cli/cli-dispatch.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts: 174 pass, 0 fail.tests/cli/sibling-home-client-sync.test.ts(registered inscripts/test-layout/layout.jsonandtests/fixtures/test-layout-expected.json) uses fake loopback/healthzidentity servers for: another PID (sibling), the same PID,pid: null, a foreign service, a closed port, and remote/malformed URLs. It also spawns a real secondaryocx startwith isolatedHOME,OPENCODEX_HOME,CODEX_HOME,GROK_HOMEandCLAUDE_CONFIG_DIR, and checks that shared client files are left byte-identical while a lone custom home still injects.bun run typecheck,bun run structure:check,git diff --check: pass.bun run test:changedandprivacy:scanfrom a same-commit throwaway checkout: added below when complete. The local full suite was not run because seven release lanes share this machine and its test lock. Hosted CI shards are the full-suite evidence.Security review
Assets: user client routing files (
~/.grok/config.toml,CODEX_HOME/config.toml, the Claude agent roster) and the Codex journal. Entrypoint: a second localocx start/ocx ensure. Trust boundary: one isolated proxy home reaching user-global client state. The change only removes writes. Client-file URLs are treated as untrusted hints: loopback only, explicit port, bounded reads, and an OpenCodex identity plus a different positive PID required. No credential, token or request content is read or logged, and no auth check changes.Checklist