Skip to content

fix(kiro): rebind continuation ownership after refusal failover - #6376

Merged
lidge-jun merged 6 commits into
devfrom
fix/luvs01-686-refusal-ownership-20261001
Oct 1, 2026
Merged

lidge-jun merged 6 commits into
devfrom
fix/luvs01-686-refusal-ownership-20261001

Conversation

@luvs01

@luvs01 luvs01 commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Kiro refusal failover replaced the OAuth account snapshot without rebinding the request continuation owner. Bind the continuation and reasoning replay scope after selecting the replacement account and before rebuilding or sending the request. Foreign account continuation state is removed, and response state belongs to the admitted replacement account. Synthetic composition regressions cover both buffered and streamed 400/403 refusal recovery.

Source history is preserved through commit bb11c93e5ec797f7ee4b8e81380e7fd0baaff347. The local validation below used the integration with dev at 0328373fb88fe0d019b29ae278e153d7fed4bcc7.

Verification

Current upstream integration: a6114b62ed1b65dede802359ccd373d913979b24; published HEAD ea76b2a0776430385d3e7e96e868e1587e4740ff. The source history through 71138522313cdac41ea016ed5c66a9d074f9f84d is retained without rewriting authors or commits. The PR-specific change was checked again after this integration; typecheck, structure, privacy and file-size gates passed. Cross-platform CI succeeded for this exact HEAD; its checkout tree matches the published HEAD tree. Skipped jobs remain skipped.

Windows, Bun 1.4.0, isolated test temporary directory:

  • bun test tests/oauth/generic-oauth-refusal-composition.test.ts: 4 passed, 0 failed, 116 assertions. Synthetic credentials, adapter/transport boundaries, and catalog behavior are isolated; the cases exercise continuation ownership through buffered and streamed refusal recovery without a provider connection.
  • bun node_modules/typescript/bin/tsc --noEmit: passed.
  • bun scripts/structure-ssot.ts: passed.
  • bun scripts/privacy-scan.ts: passed.
  • bun scripts/file-size-ratchet.ts: passed.
  • git diff --cached --check 0328373fb88fe0d019b29ae278e153d7fed4bcc7: passed.

The full local suite, test:changed, and broad Kiro transport suites were not run under the focused-validation exception. The narrow composition tests use controlled in-process responses and block real provider transport/credentials. Live transport coverage remains outstanding. Exact-HEAD CI succeeded; results from another HEAD are not its verification.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Relevant contracts and regression coverage are recorded with the change.
  • Security-sensitive changes have independent maintainer review; sponsorship is required before merge where the sponsored-surface gate applies.
  • Author validation is documented for ea76b2a0776430385d3e7e96e868e1587e4740ff, known correct findings are addressed, and exact-HEAD CI is complete; author review readiness is confirmed.

Independent maintainer approval, explicit security review, and any required maintainer-sponsored decision remain pending before merge. These are separate from author review readiness; no approval or sponsorship is claimed.

Summary by CodeRabbit

  • Bug Fixes
    • Improved recovery when generic OAuth requests are refused due to usage limits or temporary account restrictions. Retries now use the rotated account’s credentials and refresh reasoning replay state to match that account, helping buffered and streamed responses continue with the correct account context. This also improves credential handling during account failover for both Kiro and non-Kiro OAuth requests.

luvs01 and others added 5 commits September 29, 2026 02:02
…cope state

Replace the source-order-only coverage with a behavioral check: bind the reasoning replay scope under one account snapshot, commit it as served, rebind under the rotated account, and assert the new credential identity wins and the strip/drop flags retire the previous store's state.
@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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

🧰 Additional context used
📚 Code guidelines (1)
src/AGENTS.md — auto-discovered

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: e09e2e34-d729-44f3-a825-1ee03a26479d

📥 Commits

Reviewing files that changed from the base of the PR and between ea76b2a and 02f32ae.

📒 Files selected for processing (2)
  • src/server/responses/adapter-dispatch.ts
  • tests/oauth/generic-oauth-failover.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

The Kiro and non-Kiro generic OAuth failover paths now bind reasoning replay scope to the replay OAuth credential snapshot. Tests cover replay identity changes and refusal retries in buffered and streamed response modes.

Changes

OAuth refusal failover

Layer / File(s) Summary
Bind replay scope during failover
src/server/responses/adapter-dispatch.ts, tests/oauth/generic-oauth-failover.test.ts
Both failover paths pass the replay OAuth credential snapshot when binding reasoning replay scope. Tests check that rebinding occurs before request rebuilding or replay, and that changing account identity flags encrypted reasoning content and foreign item IDs for removal.
Exercise refusal retry compositions
tests/oauth/generic-oauth-refusal-composition.test.ts, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json
Tests cover MONTHLY_REQUEST_COUNT and TEMPORARILY_SUSPENDED refusals in buffered and streamed modes. They check account rotation, continuation and replay state, persisted response state, and account log labels. The test-layout mapping assigns the suite to the OAuth group.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 02f32

Retries now use replay and continuation state bound to the replacement OAuth account, with refusal-retry coverage across response modes. No actionable merge-blocking risk was identified.

Architecture Summary

Architecture risk: 🔵 Low · up to 02f32

The change affects 3 systems.

Changed systems: src, tests, scripts

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 1 changed file maps to changed impact.
  • observed — tests (service) was modified; 3 changed files map to changed impact.
  • observed — scripts (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in scripts/test-layout/layout.json: Added the explicit generic-oauth-refusal-composition.test.ts mapping to the oauth category.
  • observed — Modified behavior in tests/fixtures/test-layout-expected.json: Added the generic-oauth-refusal-composition.test.ts test-to-group mapping with group oauth.
  • observed — Modified behavior in tests/oauth/generic-oauth-refusal-composition.test.ts: Adds imports and test lifecycle setup that isolates each run in a temporary home, resets relevant caches, stubs unexpected network access, flushes pending state writers, restores spies and environment, and removes the temporary directory.
  • observed — Modified behavior in tests/oauth/generic-oauth-refusal-composition.test.ts: Adds request and completion helpers for buffered or streamed responses, plus a local observation type for captured request state and provider access.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the primary change: rebinding continuation ownership after Kiro OAuth refusal failover. This matches the updates in src/server/responses/adapter-dispatch.ts and the related…
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.
  • Fix all pre-merge checks with AI
✨ 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
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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 Oct 1, 2026
@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@luvs01
luvs01 marked this pull request as ready for review October 1, 2026 09:18
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

…ation

The non-Kiro 429 arm rotated through applyFailoverSnapshot but replayed
without rebinding the reasoning replay scope, so the retry could carry the
429'd account's continuation and encrypted reasoning. Bind it to the
replacement snapshot before replay, as the Kiro refusal arm now does.
@lidge-jun

Copy link
Copy Markdown
Owner

Maintainer follow-up (release lane review), commit 02f32ae1f9:

An independent review of ea76b2a077 found the same gap this PR closes for Kiro in the non-Kiro generic OAuth 429 arm of src/server/responses/adapter-dispatch.ts: it rotated through applyFailoverSnapshot and then replayed via rebuildAndRefetch("oauth-account-429") without bindRouteReasoningReplayScope, so the retry could carry the 429'd account's continuation and encrypted reasoning. The commit adds the same rebind (replacement snapshot, after adapter re-resolution, before seal/replay) and a source-order regression in tests/oauth/generic-oauth-failover.test.ts.

Security re-attestation at 02f32ae1f9: PASS. Local suites were not run on the project owner's instruction; hosted CI at this head is the execution evidence. Noted, not changed here: the Anthropic pool 429 arm updates the serving snapshot but not the replay snapshot (pre-existing on dev, no concrete leak found) — tracked as a follow-up.

@lidge-jun

Copy link
Copy Markdown
Owner

Maintainer integration record (MAINTAINERS.md, dev-only) — draft by release lane L2

@lidge-jun
lidge-jun merged commit 6256cb4 into dev Oct 1, 2026
35 of 37 checks passed
@lidge-jun
lidge-jun deleted the fix/luvs01-686-refusal-ownership-20261001 branch October 1, 2026 15:59
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.

2 participants