Repository navigation
docs(codex-home): say which refusals still compensate after #4531 - #4552
Conversation
Three leftovers from scoping the history preflight, found by re-reading the owned docs against the shipped behaviour. `codex-home.md` still said a detected migration always restores all three preimages before refusing. That is now direction- and reason-dependent: on apply the paginated refusal retires the relabel unit and the write stands, because it is permanent and compensating it produced a home with no OpenCodex models at all. Every other reason there still compensates, and restore and removal compensate on all of them, because retiring a provider definition its thread rows still name would orphan them. The unattended-sync test stubbed the injector returning a paginated refusal, which it can no longer produce, so the test guarded an unreachable shape while still passing. It now stubs an operational reason, which is still a hard refusal. One preflight test claimed to assert that provider definitions are preserved; it asserts which target sets reach a paginated row. Renamed to match. Co-authored-by: Cursor <cursoragent@cursor.com>
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
added
the
documentation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe change updates Codex-home migration-compensation documentation and aligns two integration test descriptions with current refusal reasons. Test logic and assertions remain unchanged. ChangesHistory migration behavior
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Other Merge Risk: ⚪ Minimal · up to This documentation and test-alignment change introduces no established production behavior risk and is ready to merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
리뷰 · 우선순위 66 / 80이 PR은 바로 전에 머지된 #4531의 문서·테스트 정확도 뒤처리입니다. #4531이 한 일은 이렇습니다. 지금 Codex home이 페이지네이션된 history를 쓰면, 예전에는 지금 이 PR의 diff는 점수 66인 이유입니다. #4531(77)만큼 급한 사고 수정은 아니고, types.ts/config.ts 분할과도 무관합니다. 다만 structure는 이 저장소의 SSOT이고, stub이 닿을 수 없는 모양을 지키면 CI가 초록이어도 회귀를 못 잡습니다. 그래서 “문서만”이라 닫지 말고, tip CI 초록이면 바로 머지하는 편이 맞습니다. 지금 checks는 hygiene/label/changes/keyring 등은 이미 통과했고, gates·test shards·macos·CodeRabbit은 아직 pending입니다. 라인 250 근처 (structure/codex-home.md) - 옛 문장 “Detected migration restores all three preimages before returning a structured refusal”을 지우고, apply에서 paginated 거절만 쓰기를 남기고 다른 이유·restore/removal은 보상한다는 쪽으로 고쳤다. 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
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. |
…#4531 (lidge-jun#4552) Three leftovers from scoping the history preflight, found by re-reading the owned docs against the shipped behaviour. `codex-home.md` still said a detected migration always restores all three preimages before refusing. That is now direction- and reason-dependent: on apply the paginated refusal retires the relabel unit and the write stands, because it is permanent and compensating it produced a home with no OpenCodex models at all. Every other reason there still compensates, and restore and removal compensate on all of them, because retiring a provider definition its thread rows still name would orphan them. The unattended-sync test stubbed the injector returning a paginated refusal, which it can no longer produce, so the test guarded an unreachable shape while still passing. It now stubs an operational reason, which is still a hard refusal. One preflight test claimed to assert that provider definitions are preserved; it asserts which target sets reach a paginated row. Renamed to match. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary
Three accuracy leftovers from #4531, found by re-reading the owned docs and the stub-driven tests against what actually shipped.
structure/codex-home.mdstill said a detected migration "restores all three preimages before returning a structured refusal". That is now direction- and reason-dependent. On apply,history_paginated_requires_native_writerretires the relabel unit and the config/profile/journal write stands, because it is permanent and compensating it is exactly what left every current Codex home with no OpenCodex models. Every other reason on apply still compensates, and restore and removal compensate on all of them, because retiring a provider definition its thread rows still name would orphan them. The section also now records that an already-paginated home cannot yet be uninstalled through the product, and that apply keeps a[model_providers.opencodex]table the home already published.tests/codex-integration/codex-sync-api.test.ts— the unattended-sync test stubbedinjectCodexConfigreturningsuccess: falsewithhistory_paginated_requires_native_writer. The injector no longer produces that combination, so the test guarded an unreachable shape while still passing. It now stubshistory_injection_preflight_unavailable, which is still a hard refusal, so the assertion means something again.tests/codex-integration/codex-history-provider.test.ts— one test was named for preserving provider definitions, which is no longer what the apply direction does. It asserts which(providerTableMode, resumeHistory)target sets reach a paginated row; renamed to say that.No source behaviour changes.
src/is untouched.Verification
bun run typecheck— passbun run structure:check— pass (structure/ SSOT checks passed)bun run privacy:scan— pass (Privacy scan passed)Checklist
Summary by CodeRabbit
Documentation
Tests