Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.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; 4 remain after this review. 📝 WalkthroughWalkthroughThe usage log now enforces entry and file-size limits. Full logs rotate to a single ChangesUsage log storage
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Contributor
|
✅ Deterministic PR hygiene checks passed. |
Owner
리뷰 · 우선순위 64 / 80이 PR은 지금 동작은 이렇게 바뀐다. 숫자는 이미 있던 management/recent 읽기 한도와 같다. 라인 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
Contributor
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. Automatic ready-for-review conversion failed; please mark the pull request ready manually if it is still a draft. |
3 tasks
7 tasks
3 tasks
Owner
|
Closing without a carry; the decision is recorded in #5608. The unconditional 64 MiB rotation and legacy-ledger deletion conflict with the opt-in |
lidge-jun
added a commit
that referenced
this pull request
Sep 23, 2026
…y work (#5608) * docs(devlog): plan the L5 luvs01 Responses and usage bundle Record the disposition, provenance and ordered carry recipe for #5474, #5305, #5434, #5560, #5542, #5553, #5562 and #5556. * fix(usage): verify OAuth rotation before withheld attribution (cherry picked from commit 1ac1ba0) * test(oauth): cover the eligible-target positive case and gate assertion (cherry picked from commit f6778bf) * fix(oauth): require a live two-account roster before withheld attribution The eligibility probe added for #5434 could report an alternate when the failed account had been removed and one stored account remained, because it skipped the roster-size guard that rotation applies. Read the roster once, apply the same guard, and describe the probe as applying no cooldown rather than as non-mutating. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(adapters): close Grok continuation and normalized tool catalog gaps Keep the existing request shape, scope external Cursor continuations to the current user request, repair ctc_ ids for xAI, and rebuild a full replay when a checkpoint no longer fits the envelope. Invalid or unreadable inputs fail closed to the previous safe default with debug diagnostics. This is the net change of two source commits. Their empty-catalog selector edit to src/adapters/xai-web-search.ts is omitted because dev already carries that rule (b20acc7, #5376), and the temporary test additions to capped files are replaced by the final extracted test files. (cherry picked from commit 71a9fe5) (cherry picked from commit ffd50f4) Refs #5350, #5560 * fix(cursor): preserve continuation scope and avoid false repetition recovery (cherry picked from commit 5a99d4d) (cherry picked from commit 31f21f0) * fix(responses): lower undeclared historical custom tools when the destination denies them Routed lowering collected only current custom declarations, so a compacted or replayed custom_tool_call leaked to xAI-like gateways as the native item type and came back as a misleading 422 missing id. Convert protocol-history items from the top-level input without expanding the live catalog, request full replay for orphan results, and fail closed before serializing leftovers. Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit 5da2883) (cherry picked from commit a326b67) * test(responses): split historical custom-tool replay coverage off the passthrough ratchet cap openai-responses-passthrough.test.ts is already at its 4809-line ceiling. Keep the new wire fixtures in a responses-prefixed file so the layout seed resolves it without raising a cap. Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit 6f43794) (cherry picked from commit 7e8fb09) * fix(responses): reject malformed historical custom calls (cherry picked from commit 52f7448) (cherry picked from commit 57407be) * fix(responses): bind historical outputs to custom calls (cherry picked from commit 5654b41) (cherry picked from commit c7781bf) * fix(responses): reject duplicate historical call ids (cherry picked from commit dc948dc) (cherry picked from commit fbecefa) * fix(xai): preserve stateful tool output continuations (cherry picked from commit 4edc411) (cherry picked from commit 82a5f6d) * fix(xai): keep replay-miss reasoning cleanup independent of output repair Co-Authored-By: Epinephrine <luvs01@hanmail.net> (cherry picked from commit 67ccd8d) (cherry picked from commit aac783f) * test(responses): verify combined continuation boundaries Exercise stateful output-only deltas, independent replay-miss reasoning cleanup, capability-driven historical lowering, placeholder ordering, native item-ID repair and preservation of the existing empty-catalog denial. Record the combined history contract and register the carried and new regression files. The layout-marker cleanup from e8e179f was completed while resolving its preceding source commit onto the current map. The existing dev selector normalization and role-fixture corrections remain authoritative and are not replaced by weaker or duplicate source changes. Co-authored-by: Yeonwoo Choi <32544727+twoimo@users.noreply.github.com> Co-authored-by: maosisheng <maosisheng@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Epinephrine <luvs01@hanmail.net> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> (cherry picked from commit 2ec0cd1) * fix(responses): normalize wrapped MCP tool names (cherry picked from commit b4c839b) (cherry picked from commit f68b32646069d3e385748fc6c36848765c68e02e) (cherry picked from commit e555e73) * fix(responses): unwrap default apply patch aliases (cherry picked from commit 453df76) (cherry picked from commit d63ff542415895fa25f5599aad9dc632c80de92d) (cherry picked from commit 7cbbf44) * docs(structure): record default.-prefixed alias recovery in the freeform contract The responses owners still claimed only bare exec/apply_patch calls accept alternate-field or outer-fence recovery; compileCodeModeHelperInput now also unwraps provider-invented default. aliases. Update runtime.md, transports/responses.md, providers/{chat-compat,kiro,xai-grok}.md. Co-Authored-By: Epinephrine <luvs01@hanmail.net> (cherry picked from commit 165ddd0) (cherry picked from commit d0776e703681729e2ea447775e77397faab16f29) (cherry picked from commit 19a2005) * fix(responses): compile structured shell payloads sent to code-mode exec (cherry picked from commit 9430bbd) (cherry picked from commit 59bc70bc30f954eea5a63f8a6fcf4de4dcbfd865) (cherry picked from commit 9662528) * test(responses): verify combined tool normalization boundaries Verify that default.apply_patch names and patch/content wrappers emit the same executable input through JSON and fragmented SSE. Move the carried decision record to unused ADR-0099 because current dev already owns ADR-0097; preserve both the structured code-mode shell and existing schema-bound flat-shell contracts. Co-authored-by: kosta <kosta963@gmail.com> Co-authored-by: Flowershangfromthebranches <152056395+Flowershangfromthebranches@users.noreply.github.com> (cherry picked from commit 3da6366a60ac5964e8a8de43486125d72729f8c8) (cherry picked from commit b57d7c5) * fix(responses): refuse ambiguous OpenCode Go resets (cherry picked from commit 62ac159) (cherry picked from commit b8f9a45) * test(responses): prove OpenCode Go pre-answer reset refusal by execution Per review on #5446: replace the replaySafe source-string count with an execution test that drops the connection before the answer on an opencode.ai/zen/go destination and asserts the 429 upstream_reset_replay_refused with exactly one send. (cherry picked from commit 92b74ec) (cherry picked from commit 808dd85) * fix(routing): isolate policy retry body snapshot (cherry picked from commit 8e2a0fe) (cherry picked from commit db854bf) * test(routing): pin the retry snapshot against nested input mutation (cherry picked from commit a240fc9) (cherry picked from commit b037810) * fix(responses): isolate policy compaction state (cherry picked from commit 714119e) (cherry picked from commit e6f9339) * fix(tests): bound the cold-spawn warm-up child on a live event loop The warm-up child was waited on through Bun.spawnSync, which made the spawn's own timeout the only bound it could honour — and no bound at all when the child or the primitive wedged: while a synchronous spawn blocks, the event loop is dead, so the hook budget and the per-test timeout freeze inside the same wait and nothing reports anything. Run 35511743422's macos 2/2 leg held that shape for eighteen silent minutes inside client-connect.test.ts before the job ceiling cut it and reported cancelled, which the ci gate reads as failure. The bound now lives on the parent's live loop: an asynchronous spawn, SIGKILL at the existing derived deadline, a short reap grace, and the call settles with or without the child's exit or EOF — so a descendant holding the pipes or a child that outlives its kill cannot turn a warm-up into an unbounded wait. A timed-out child now fails the warm-up by name instead of hanging the job. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> (cherry picked from commit aa9b889) (cherry picked from commit f86a534) * fix(responses): fail closed on synthetic or stale compaction source selectors compactionRoutingKeepsProviderIdentity evaluated override.sourceModel as the raw client selector, so a synthetic --fast/--effort form of a policy or combo selector (ocx/primary--fast) missed resolvePolicyProfileId/resolveComboId and fell through routeConcreteModel to the default provider. The same fallthrough swallowed policy aliases renamed or deleted mid-conversation, since config.routingProfiles is mutated in place. Both cases could wrongly report identity match and let provider-private compaction state or caller credentials cross a backend boundary. Strip synthetic-row suffixes via parseSyntheticRowId before the identity checks, and treat a source that only routes through the default provider as unproven: it can never match a concrete identity. Co-Authored-By: Epinephrine <luvs01@hanmail.net> (cherry picked from commit 7e59315) (cherry picked from commit 76b40f9) * fix(codex): bind scoped quota suppression to alternate (cherry picked from commit 23a3694) (cherry picked from commit 385f338) * fix(ci): restore core.ts to file-size ratchet cap The scoped-quota re-export grew src/server/responses/core.ts past its committed 210-line cap (213). Collapse the two-name re-export back to one line; the file's export list already carries longer single-line statements. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> (cherry picked from commit 559db16) (cherry picked from commit feb0c16) * fix(codex): record wrapped quota on suppressed moves and bind caller main - record the normalized 429/402 outcome before returning no-alternate on a suppressed same-workspace move, so a 5xx-wrapped quota refusal still cools the refused account instead of reading as transient - bind a request-owned `main` alternate by the caller credential's own workspace id (chatgpt-account-id header, else the bearer token's account claim) via callerCodexWorkspaceAccountId - apply the same scoped-quota workspace gate to the single bounded alternate send in the native /responses/compact path - cover all three in tests and update the transport doc Co-Authored-By: Epinephrine <luvs01@hanmail.net> (cherry picked from commit 5267593) (cherry picked from commit 466c75c) * test(server): move scoped-quota auth cases into a sibling file under the size cap server-auth.test.ts grew to 4684 against a 4589 baseline cap, so the file-size ratchet failed shard 3/4. The three scoped-quota suppression cases move byte-for-byte into server-auth-scoped-quota.test.ts, and the pool-retry harness they share is extracted to tests/helpers/pool-retry-harness.ts (per-run OPENCODEX_HOME dir, so each importing file keeps its own module state under bun test --isolate). Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> (cherry picked from commit b391c99) (cherry picked from commit 9050722) * fix(codex): re-check abort after the scoped-quota body read The workspace classification in shouldRetryCodexScopedQuotaOnAlternate reads the first response body asynchronously, so a client disconnect can land after the earlier abort check but before the branch records the first account, cancels its body, and sends the alternate. Re-check the abort signal immediately after the await in both paths: compact returns the 499 client_cancelled response after releasing the alternate lease, and the regular path releases its permit and lease and returns no-alternate while still recording the first account's real outcome. Co-Authored-By: Epinephrine <luvs01@hanmail.net> (cherry picked from commit c52b64b) (cherry picked from commit b2eda92) * fix(codex): release the discarded compact rejection body on abort The 499 exits around the scoped-quota classification return a fresh response while the first rejection's body is still open; cancel it so the abandoned upstream connection and tee resources are released. Co-Authored-By: Epinephrine <luvs01@hanmail.net> (cherry picked from commit 813efcc) (cherry picked from commit 1069b54) * fix(codex): keep compact abort cleanup off the return path upstream.body.cancel() can wait on a custom or stalled source; awaiting it at the abort checkpoints would park the 499 reply on cleanup. Fire it with the request's abort reason and swallow rejection, the same best-effort shape bufferCompactResponse already uses. Co-Authored-By: Epinephrine <luvs01@hanmail.net> (cherry picked from commit 2d1ee69) (cherry picked from commit 37a006e) * test(responses): preserve terminal refusal across recovery boundaries Exercise both operator-granted and default-denied reset paths through policy fallback and the alternate-account eligibility gate. Clarify that retries serialize the original body snapshot while identity metadata is established per attempt, and document synthetic compaction identity and transient replacement refusal. (cherry picked from commit be1fee9) * test(cursor): pin exact host-wrapper classification in continuation scope The wire carries no provenance for compaction summaries or ambient browser state, and the Codex client detects stored summaries by the same exact prefix. Pin that a user message that is itself an exact wrapper stays in history while the preceding real request remains the labeled one, and document the rule next to the continuation contract. Co-authored-by: Yeonwoo Choi <32544727+twoimo@users.noreply.github.com> * test(server): prove a suppressed same-workspace alternate is never sent Both credentials in the 5xx-wrapped scoped-quota case carry the same workspace header, so a negative check on the other account id could not fail. Assert the exact one-send sequence, and narrow the transport contract: suppression covers the in-request move, while later requests still select by per-account health. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * docs(devlog): record the L5 responses sequence outcome * docs(devlog): amend the L5 search and usage plan after review * fix(responses): isolate Cursor combo shadow calls (cherry picked from commit 3e5a4dd) (cherry picked from commit 7f45883) * fix(responses): repair terminal-less bridged search legs (cherry picked from commit 1fb6005) (cherry picked from commit c4fa8c8) * fix(responses): repair continuation legs and prove behavior in tests (cherry picked from commit 3948970) (cherry picked from commit 8d46989) * fix(responses): close shadow combo intersection and continuation coverage Apply the existing source-target non-intersection rule before early combo interception, and add production-path behavioral coverage for terminal-less continuation repair. (cherry picked from commit 4bfcc0a) (cherry picked from commit 3f3fdf1) * test(web-search): preserve complete but open bridge-leg coverage Carry the end-to-end handleResponses regression and transport contracts for repairing both the first and continuation search legs. The corresponding production changes are already preserved by the earlier terminal-repair carries; keep this broader integration coverage without applying that implementation twice. Source commit: b1044e7 Co-authored-by: Epinephrine <luvs01@hanmail.net> (cherry picked from commit 973a4ac) * test(web-search): keep repaired replay within caller and serving scope (cherry picked from commit bb49c9f) * test(server): reap fixture ACL workers before removing failover homes (cherry picked from commit ae52669) * fix(responses): keep combo shadow interception on the dispatch pick The carried early intersection check sampled a combo target with routeModel before dispatch. The combo loop then picked again, so the interception decision could follow a different target than the one sent, and a random or round-robin combo advanced its selection state for a probe. Restore the existing combo rule (a shadow call rewritten to a combo enters the combo) and its regression test, and keep the combo-child isolation marker. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * test(web-search): bind repaired-leg replay to a keyed caller principal The repaired-leg replay assertion expected a shared keyless loopback principal, which the replay cache refuses by design: keyless loopback callers get no retained replay. Configure an inbound API key, derive the expected principal with the production resolver, pass the full loopback admission, and assert that a keyless caller cannot form a scope. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix(usage): restrict legacy -main provider collapse to Codex labels A -main suffix was stripped from ANY provider name, so a configured provider like openrouter-main silently merged into openrouter in usage summaries and pricing fallback. Restrict the compatibility mapping to the known Codex provider labels (openai/chatgpt/openai-multi-main), matching codexAccountProviders. (cherry picked from commit 89a9272) (cherry picked from commit 0f0ef96) * fix(cli): mark access-key usage unavailable when attribution is absent Without attributionSince the server is reporting an empty or unreadable usage log, but the table still rendered 0 totals and never-used markers that are indistinguishable from real data. Show an unavailable marker spanning the usage columns instead, matching the ambiguous-union precedent. (cherry picked from commit 3c3f14b) (cherry picked from commit 83514c3) * fix(cli): treat a malformed attributionSince as unavailable typeof === 'string' accepted any value, so a malformed attributionSince made usageAvailable true and printed usage cells plus an 'attribution since' footer. Parse once, derive availability from the validated result, and reuse it for the footer. Covers the malformed-string regression. (cherry picked from commit e7584c3) (cherry picked from commit 1380693) * fix(gui): keep the app-server status read alive across Models tab switches (cherry picked from commit 9e6870d) (cherry picked from commit 5563577) * fix(usage): bound persisted requestedModel selectors Selectors are not length-bound at admission, so persist over-long values as a bounded prefix + sha256 digest of the full selector and encode the request-history requested_model filter input identically. Two selectors sharing the bounded prefix stay distinguishable instead of colliding on a truncated prefix. (cherry picked from commit e3d0f9b) (cherry picked from commit c8a9d1a) * fix(spend): enforce ceilings on native chat sends (cherry picked from commit 347204f) (cherry picked from commit 823a7d2) * fix(usage): preserve observation and native reservation boundaries (cherry picked from commit 22ee516) * test(usage): isolate native spend regressions within file caps (cherry picked from commit 1a8d5f7) * test(usage): drain fixture owners before removing Windows homes (cherry picked from commit 2241d03) * test(usage): batch canonical pagination fixture writes (cherry picked from commit ddfef13) * fix(cli): accept only an ISO-8601 UTC attributionSince Date.parse accepts strings such as "0" and rolls impossible dates over, so a malformed attributionSince could still switch the key table to numeric usage and never-used cells. Require the ISO-8601 UTC shape the server emits and an instant that round-trips to the same second. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * docs(usage): state the aliasing limit of the idempotent selector encoding Encoded requestedModel values must map to themselves because rows are normalized again on read, so a literal selector equal to another selector's persisted form shares its identity. Say so where the encoding is defined, and pin the behavior in the request-history test so the limit is deliberate rather than silent. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * docs(devlog): record the L5 search and usage outcome * docs(devlog): record the L5 pull request * docs(structure): keep runtime.md within its 600-line budget The default.-prefixed freeform wording wrapped one extra line and pushed structure/runtime.md over the structure SSOT budget. Reflow the paragraph; the wording is unchanged. * refactor(cursor): move current-request selection out of protobuf-request The carried continuation-scope helpers brought src/adapters/cursor/protobuf-request.ts to 2,000 lines, the file-size threshold for files without a recorded cap. Move the current-request selection and its host-wrapper classification to a sibling module unchanged; the caller passes its text extractor. Co-authored-by: Yeonwoo Choi <32544727+twoimo@users.noreply.github.com> * docs(devlog): record the #5553 re-pin in the L5 plan * docs(devlog): record the L5 original closures * fix(usage): keep the native Chat spend estimate out of recorded usage The native Chat spend reservation reused usageLogInputTokens for its input estimate. That field also feeds attempt usage, so native Chat attempts started recording an estimate-inflated input instead of the provider-reported counts (hosted CI: server-xai-oauth-401-replay and server-key-failover-e2e). Reserve the estimate through a spend-only spendInputEstimateTokens field; the reservation still charges it before each physical send. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * test(server): identify the suppressed alternate by its credential Hosted CI showed the send layer retrying the refused account's transient 502 on the same credential, so an exact one-dispatch count was the wrong oracle. Both accounts share a workspace header; record the credential each physical send presents and assert the suppressed alternate's credential never appears. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> * fix: close L5 retry, continuation, and history review gaps Classify spend refusals on transient retry legs, keep Cursor retry provenance in checkpoint actions, and rebuild legacy history selectors into the bounded projection. Scope policy snapshots and correct warm-up deadline handling, with focused regressions and documentation. * fix: retain policy fallback for decorated selectors Normalize supported Fast and effort policy selectors before snapshot gating, restore a real policy-hop fixture, and align Cursor wire assertions with retry provenance guidance. --------- Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> Co-authored-by: Yeonwoo Choi <32544727+twoimo@users.noreply.github.com> Co-authored-by: twoimo <twoimo@twoimoui-MacBookPro.local> Co-authored-by: maosisheng <maosisheng@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Epinephrine <luvs01@hanmail.net> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: kosta <kosta963@gmail.com> Co-authored-by: Flowershangfromthebranches <152056395+Flowershangfromthebranches@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
usage.jsonlgrew without bound.appendUsageEntrynow refuses single rows over 64 KiB and rotates the active ledger to a loneusage.jsonl.1archive once the next append would exceed 64 MiB, bounding the on-disk footprint to roughly twice the cap.usage.jsonl.1is registered inINITIAL_OWNED_PATHSandensureUsageLogDirso config ownership and uninstall track the archive.Verification
bun test tests/usage/usage-log.test.ts tests/config/config-ownership-uninstall.test.ts— 56 pass, 1 pre-existing platform skip.bun x tsc --noEmit— clean.Checklist
Summary by CodeRabbit
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.