Skip to content

fix(codex): log unattended Codex write service-home refusals - #6100

Merged
lidge-jun merged 3 commits into
devfrom
codex/t4-issue-triage-sync-refusal-log
Sep 27, 2026
Merged

lidge-jun merged 3 commits into
devfrom
codex/t4-issue-triage-sync-refusal-log

Conversation

@lidge-jun

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

Copy link
Copy Markdown
Owner

Summary

When an unattended proxy cannot prove it owns the installed service homes, syncModelsToCodex refuses Codex writes on the service-home authority. Until now that refusal left nothing behind. Startup marked /readyz failed with no log line, and ocx sync printed only "Codex sync did not complete". #5782 reports a nine-day catalog-sync stall that went unnoticed for this reason.

The sync now logs one line naming the authority, saying that catalog sync and Codex config injection were skipped, and pointing to POST /api/sync, which already returns the concrete reason in its 409 body. The admission message stays out of the log because it can name private home and service-definition paths. Callers that pass log = null, such as the periodic catalog auto-refresh, remain silent, so drift healing does not repeat the line. Behavior for other refusal authorities is unchanged. structure/config.md now describes this exception to the "concrete messages on stderr" rule.

This reimplements the diagnostic slice of #5782 by @tcflying without the raw message. The rest of #5782 (Windows durable stop and ChatGPT bridge) stays under review there.

Co-authored-by: tcflying 77509083+tcflying@users.noreply.github.com

Verification

  • New regression in tests/codex-integration/codex-sync-api.test.ts: failed before the change (no log line), passes after. It also asserts that nothing is refreshed or injected and that the logged line does not contain the refusal's path.
  • bun test tests/codex-integration/codex-sync-api.test.ts tests/codex-integration/codex-admission.test.ts: 30 pass / 0 fail. bun test tests/codex-integration/codex-composed-acceptance.test.ts: 8 pass / 0 fail.
  • bun run typecheck, bun run privacy:scan, bun run structure:check, git diff --check origin/dev...HEAD: exit 0.
  • Independent review found one documentation mismatch in structure/config.md, which is fixed in this PR.
  • Rebased onto dev 6d64ea26 (two Swift tray files), then onto 468b954c (fix(images): use managed Pool with proxy admission bearer, scope first #6097, image Pool admission). Neither touches this diff. After the rebase, codex-sync-api.test.ts (18 pass) and structure:check were rerun. A local test:changed attempt was stopped externally (SIGTERM) while it waited for another lane's test lock, so it produced no result.
  • Full local suite and test:changed not run for this PR: seven release lanes share this machine, and broad coverage is left to CI.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 27, 2026 15:49
@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: accdbdfc-f38d-4d0f-97aa-3653bc40707b

📥 Commits

Reviewing files that changed from the base of the PR and between f04da55 and 75f863c.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: ee044a50-d330-4a3a-892f-909b579e7616

📥 Commits

Reviewing files that changed from the base of the PR and between 24b2f39 and f04da55.

📒 Files selected for processing (3)
  • src/codex/sync.ts
  • structure/config.md
  • tests/codex-integration/codex-sync-api.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

When Codex write admission is refused because service-home ownership cannot be established, sync logs a fixed explanation without exposing the private path. The refusal result and return behavior remain unchanged. A regression test and documentation cover the refusal.

Changes

Service-home refusal logging

Layer / File(s) Summary
Refusal logging and regression coverage
src/codex/sync.ts, tests/codex-integration/codex-sync-api.test.ts, structure/config.md
On service-home admission refusal, sync logs a fixed explanation that catalog sync and config injection were skipped. The refusal result retains its path-bearing reason. The test checks the result, skipped operations, and path-free log line. The documentation describes the log and API response.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to f04da

The refusal remains available through POST /api/sync, while the new log avoids exposing private paths. No merge-blocking issue is identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to f04da

The new refusal log does not include private paths, and the service-home check still blocks catalog and configuration writes. The API continues to return the detailed reason; its access controls were not independently verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The added output is confined to callers that supply a logger on this refusal path. The inspected API caller supplies no logger, and the changed test does not create a new runtime entrypoint.

Trust Boundaries and Controls

  • observed — The service-home ownership check remains ahead of both write operations. The detailed refusal reason remains in the existing API response rather than the new log line; this review did not establish the API's upstream authentication controls.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: logging unattended Codex write refusals caused by service-home ownership checks.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the bug Something isn't working label Sep 27, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

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

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-27T15:51:35.664532Z f04da55 PR opened
ℹ️ About Codex in GitHub

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

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

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

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 44 / 80

이 PR은 사람이 지켜보지 않을 때 Codex 쓰기가 거절되면, 그 사실을 로그에 한 줄 남겨요. 베이스는 dev예요.

프록시가 설치해 둔 서비스 홈의 주인이라고 증명하지 못하면, syncModelsToCodex는 카탈로그를 고치지 않고 Codex 설정도 넣지 않아요. 결과는 status: "refused"예요. 거절 문장에는 집 경로가 들어가요. 예전에는 그 결과가 /readyz만 실패로 바꾸고, ocx sync는 "Codex sync did not complete"만 찍었어요. #5782에서 9일 동안 카탈로그 동기화가 멈춘 채로 아무도 못 본 이유가 이거예요.

이제는 거절 권한이 service-home일 때만 log.error로 고정 문장을 찍어요. 문장에는 권한 이름, 카탈로그와 설정 주입을 건너뛰었다는 말, POST /api/sync를 보라는 말이 있어요. 집 경로는 로그에 안 넣어요. 경로가 있는 이유는 반환값 message에 그대로 남아요. POST /api/sync는 로그를 null로 호출해서 이 줄을 다시 찍지 않고, 409 본문에 그 이유를 넣어요. 앱 서버를 다시 띄울 때의 카탈로그 갱신도 로그가 null이라 조용해요. 다른 권한의 거절은 이 분기를 타지 않아요. 테스트는 경로가 로그에 없고, 갱신과 주입이 안 일어나는지 봐요. #5782의 Windows 종료와 ChatGPT 브리지는 이 PR에 없어요.

structure/config.md:273 - "경로 없는 로그 한 줄"이 항상 남는 것처럼 읽혀요. 로그를 넘긴 호출만 찍어요. ocx start와 ocx sync는 기본 로그가 콘솔이라 찍혀요. POST /api/sync와 앱 서버 재시작 쪽 카탈로그 갱신은 null이라 안 찍혀요.

src/cli/dispatch.ts:490 - ocx sync는 새 줄 다음에 "Fix the reported Codex config issue"를 그대로 찍어요. 새 줄의 원인은 서비스 홈 소유권이에요. 이 문장은 설정 파일을 고치라고 해요. synced.message에 구체적 이유가 있는데 CLI는 그 문장을 화면에 안 내요. 프록시가 꺼져 있으면 로그가 가리키는 POST /api/sync도 없어요.

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

로그를 비우는 주기 갱신이, 시작 때 한 번 찍은 뒤에 같은 거부를 다시 만나도 조용한 채로 둘지예요. 시작 로그를 못 본 프로세스는 다시 조용해져요.

ocx sync가 이미 가진 message를 경로만 지워서 보여줄지, 지금처럼 API로만 보낼지예요.

너의 추천

거절을 유지하고 경로는 로그에서 빼는 방향은 맞아요. 머지해도 돼요. 그 전에 structure/config.md에 로그가 나오는 호출과 안 나오는 호출을 한 문장으로 나눠 주세요. ocx sync의 다음 문장은 서비스 홈 거부일 때 설정 파일을 고치라고 하지 않게 해주세요.

#5782는 닫지 마세요. 진단 한 조각만 여기 있어요. Windows 종료와 ChatGPT 브리지는 그 이슈에 남겨 두세요. types.ts와 config.ts를 나누는 중복 PR은 이 변경과 겹치지 않아요. 베이스는 dev로 두세요.

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

@lidge-jun
lidge-jun force-pushed the codex/t4-issue-triage-sync-refusal-log branch 2 times, most recently from 9147a29 to 8f52270 Compare September 27, 2026 18:19
lidge-jun and others added 3 commits September 28, 2026 03:45
A service-home admission refusal in syncModelsToCodex returned without any
log line. Startup then marked /readyz failed with no evidence and `ocx sync`
only said the sync did not complete. Log one line naming the authority and
pointing to POST /api/sync for the specific reason. The admission message
can contain private home and service-definition paths, so it stays out of
the log.

Reimplements the diagnostic slice of #5782 without the raw message.

Co-authored-by: tcflying <77509083+tcflying@users.noreply.github.com>
@lidge-jun
lidge-jun force-pushed the codex/t4-issue-triage-sync-refusal-log branch from 8f52270 to 75f863c Compare September 27, 2026 18:45
@lidge-jun
lidge-jun merged commit 7b83ded into dev Sep 27, 2026
31 checks passed
@lidge-jun
lidge-jun deleted the codex/t4-issue-triage-sync-refusal-log branch September 27, 2026 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant