Repository navigation
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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 |
|
✅ Deterministic PR hygiene checks passed. |
a76f8d0 to
f61b00d
Compare
Bound unresolved OAuth refresh consumers while preserving stale-flight ownership evidence across refused replacement admission. Report retryable capacity separately from authentication failures, and preserve cancellation priority and lease-release assertions.
f61b00d to
b722d74
Compare
|
Superseded by #7136, which carries this work with Co-authored-by credit. Thank you! |
|
Thank you, @luvs01. I've carried this onto an upstream branch in #7136, so the repository's full CI (including the macOS and Windows matrix) can run on it. Your commits are kept and credited with a |
) (#7136) * fix(oauth): bound refresh consumers and preserve retryable capacity Bound unresolved OAuth refresh consumers while preserving stale-flight ownership evidence across refused replacement admission. Report retryable capacity separately from authentication failures, and preserve cancellation priority and lease-release assertions. (cherry picked from commit b722d74) * fix(oauth): retire stale owners before admitting replacements Keep stale registry ownership through definitive-result cleanup and durable retirement regardless of spare consumer capacity. Recheck the installed flight when concurrent replacements resume. Co-authored-by: Epinephrine <luvs01@hanmail.net> * fix(responses): preserve cancellation at refresh capacity Observe both independent cancellation signals at initial refresh refusal and document retry behavior for OAuth capacity. Co-authored-by: Epinephrine <luvs01@hanmail.net> * fix(responses): link both client signals through runTurn recovery Address the hosted Codex P2: request-only cancellation takes priority during refresh refusal, including committed SSE. Remove both listeners on cleanup. Co-authored-by: Epinephrine <luvs01@hanmail.net> * fix(oauth): report deferred refresh retirement without identity Address the CodeRabbit observability finding with one bounded diagnostic per retained flight, a closed failure reason, and no account or credential fields. Co-authored-by: Epinephrine <luvs01@hanmail.net> * refactor(oauth): move refresh-flight retirement into its own module Pure move of retireSettledOAuthRefreshFlight, the refresh-waiter gate and OAuthRefreshFlightEvidence out of src/oauth/index.ts, so this branch merged with current dev stays under the 2000-line file-size ratchet. The registry map is passed explicitly; behavior is unchanged. --------- Co-authored-by: Epinephrine <luvs01@hanmail.net>
Summary
Cancelled requests release their active-turn slots while shared OAuth refresh work can remain pending. This change bounds those unresolved consumers independently, preserves stale-owner cleanup, and reports temporary saturation as retryable capacity instead of an authentication failure.
This remains a Draft. It bounds the count of unresolved consumers, not retained bytes, provider refresh duration or capacity across multiple processes.
Exact revision
7640a942b35044c974e534b84d5a8049f4a8ae9a.b722d746f10dafb7eeea2aabcf8b94aec6a6a5db.e27b603e952dae196fe6689632fbc8e8e5613b97.7640a942b35044c974e534b84d5a8049f4a8ae9a. The earlier three-commit history is preserved privately for traceability.77b5db2322f0c8efb7a4a4194c395b5c8f5c49da8cb027efa43b226ee4d0e0c1using full-index, no-renames, unified-3 Git diff.Contributions and commit identity
This revision builds on luvs01/opencodex#735; the Kiro cancellation follow-up corrects regression-test assertions without changing production code.
Verification
Linux, existing Bun 1.4.3 and matching locked dependencies. The earlier pre-correction tree a76f8d0 passed 250 tests / 1,977 assertions across 10 files, with 18 new follow-up regressions. Earlier and repeated runs are not added to this count.
All listed root/static gates passed. Commands used a scrubbed environment, isolated HOME/TMPDIR and the repository test-home preload. GUARD additionally rejected unmocked fetch calls; STATIC_GUARD rejected network fetches during static checks. No unmocked fetch was attempted. This is synthetic/application-level isolation, not an OS network-sandbox or live-provider claim.
The two reported follow-up failures were reproduced before correction: saturated stale ownership failed both pre/post-dispatch cases, and upstream-401 saturation/cancellation failed the original 12 transport cases. The final tests cover those paths plus late result settlement, retirement IO retry and committed SSE. Existing terminal-401, pause, 429 and account-failover cases remain passing. Follow-up code review found both reported P2s resolved and no new confirmed P1/P2; this does not substitute for required upstream approval or explicit security review.
Kiro cancellation CI follow-up
Hosted CI on the previous head exposed one stale 401 assertion after cancellation during credential recovery. It was reproduced locally, then corrected to the intended 499/client_cancelled contract with no production change. Both zero-lease assertions remain before body consumption; additional assertions check the cancellation type/code/message, absence of Retry-After, one refresh and no second inference dispatch.
The current tree passed 69 tests / 477 assertions across four files under the same offline/home-isolated guards, plus source typecheck, structure, privacy, filesize and diff checks. This is a separate run overlapping the earlier 250 tests; the totals must not be added. The new server-signed commit's hosted CI must be checked separately.
Scope exception and remaining evidence
The full application suite was not run: the parsed import graph reaches approximately 1,735 test files, disproportionate to this bounded private/offline verification. The focused regressions, adjacent transport/failover cases and layout checks above are the documented resource exception. Keep the PR in Draft until the required broader scope is accepted. Do not treat the unrun suite or skipped/older-head checks as passes.
scripts/test-temp-lock.ts:78: TS2339, missingSyncSubprocess.signalin the installed typings. Neither new regression file has a remaining diagnostic. An earlier broader selected-test compiler run had 33 diagnostics on unchanged lines; a same-command original-tree compiler comparison was not performed, so these are not claimed to be conclusively baseline-only.tests/responses/ws-endpoint.test.ts:56; basic layout membership/tooling passes, but the stricter scan is not claimed newly clean.Checklist
Review readiness checklist