Skip to content

fix(kiro): retry rebuilt requests at the rebuilt destination - #6374

Merged
lidge-jun merged 6 commits into
devfrom
fix/luvs01-687-retry-budget-20261001
Oct 1, 2026
Merged

lidge-jun merged 6 commits into
devfrom
fix/luvs01-687-retry-budget-20261001

Conversation

@luvs01

@luvs01 luvs01 commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

A Kiro connection-reset retry could combine rebuilt credentials and body with the old captured destination. Use the rebuilt request URL for both physical retries and send-budget admission, and derive legacy gateway fallback from the rebuilt region. Record an authorized request rebuild separately from the one permitted alternate-target transition so the new region retains its normal gateway fallback. The accounting still enforces physical-send limits.

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

Verification

Current upstream integration: a6114b62ed1b65dede802359ccd373d913979b24; published HEAD 2534809a7445f1d0f4be9fefccbc7bb08269400a. The source history through efa286fde6912a1dad6656fc349f6c0fd0c98358 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/lib/execution-budget-permits.test.ts: 23 passed, 0 failed, 138 assertions. This focused run verifies budget accounting, including rebuild and alternate-target admission. It does not claim a live Kiro endpoint or reset retry was exercised.
  • 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 and test:changed were not run under the focused-validation exception because of the resource and synthetic-test limits of this maintenance pass. The focused accounting suite ran; live Kiro transport and reset behavior were not exercised. 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 2534809a7445f1d0f4be9fefccbc7bb08269400a, 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
    • Requests rebuilt after a connection reset now retry using the updated destination, credentials, and body.
    • Regional gateway fallback now stays aligned with the rebuilt request, including when an earlier destination change has occurred.
    • Rebuilt requests can continue retrying within existing safety and total-send limits without incorrectly using up destination-change allowances.
  • Documentation
    • Clarified how reset-triggered retries and regional gateway fallback work.

luvs01 and others added 6 commits September 29, 2026 02:03
…dget

A reset-triggered rebuild retargets the request to a new region's canonical host. Treat that move as authorized rebasing, not a failover transition, so the region's own legacy gateway rotation still fits the one-transition budget. The legacy host is also resolved from the current request URL so rebuilt credentials are never sent to the original region's gateway.
@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 (2)
src/AGENTS.md — auto-discovered
structure/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: 99d6aedf-4f3e-451e-a9f1-8ab2d4261485

📥 Commits

Reviewing files that changed from the base of the PR and between a6114b6 and 2534809.

📒 Files selected for processing (6)
  • src/adapters/kiro-retry.ts
  • src/lib/request-execution-budget.ts
  • structure/transports/inventory.md
  • structure/transports/responses-spend.md
  • tests/lib/execution-budget-permits.test.ts
  • tests/providers/kiro/kiro-transport-parity.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

Kiro reset retries now use rebuilt request destinations and derive legacy gateway fallbacks from the current request URL. The request execution budget records validated rebases without charging target-transition or alternate-target allowances, while retaining replay-safety and total-send checks.

Changes

Kiro reset retry and budget handling

Layer / File(s) Summary
Rebased dispatch budget
src/lib/request-execution-budget.ts, tests/lib/execution-budget-permits.test.ts, structure/transports/responses-spend.md
DispatchIntent adds rebasedTarget. Admission, reservation, and release exclude rebased destinations from transition and alternate-target charges. Tests cover replay-safety checks, release refunds, subsequent rebases, and the total-send limit.
Kiro retry destinations and fallback
src/adapters/kiro-retry.ts, tests/providers/kiro/kiro-transport-parity.test.ts, structure/transports/inventory.md
Reset recovery tracks the current destination and its rebase status. Connection and HTTP endpoint failure fallbacks derive the legacy gateway from the current request URL. Tests check that retries use rebuilt request values and that a regional gateway fallback remains available within the execution budget.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant fetchWithResetRecovery
  participant createRequestExecutionBudget
  participant fetchKiroAttempt
  fetchWithResetRecovery->>createRequestExecutionBudget: Reserve dispatch for current destination
  fetchWithResetRecovery->>fetchKiroAttempt: Send request
  fetchKiroAttempt-->>fetchWithResetRecovery: Return connection reset
  Caller->>fetchWithResetRecovery: Rebuild request with updated URL and credentials
  fetchWithResetRecovery->>createRequestExecutionBudget: Reserve rebased destination
  fetchWithResetRecovery->>fetchKiroAttempt: Retry rebuilt request
  fetchKiroAttempt-->>fetchWithResetRecovery: Return endpoint failure
  fetchWithResetRecovery->>fetchKiroAttempt: Send to legacy gateway for current request URL
Loading

Merge Risk: ⚪ Minimal · up to 25348

Kiro reset retries now follow a rebuilt request's destination while replay-safety and total-send limits still apply. No concrete merge-blocking issue was identified, and focused tests plus CI are reported as passing.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 25348

The inspected recovery path keeps rebuilt credentials and request bodies aligned with their destination while retaining physical-send limits. No newly reachable attack path was established, but the rebuild exemption relies on trusted callers and concurrency coverage remains incomplete.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure is the selected Kiro credential and request body sent during one logical request's recovery. The shared exemption is also available to in-process dispatch callers, but the inspected evidence does not establish a network caller that can set it directly.

Trust Boundaries and Controls

  • observed — Account-bound dispatch checks selection revision, credential generation, expiration and account status. Rebuilding replaces both the request and its credential binding; the physical-send wrapper evaluates egress at the selected destination and disables automatic redirects.
  • observed — The budget trusts rebasedTarget as a caller assertion rather than independently verifying rebuild authority. It nevertheless retains physical-send ceilings, reserve eligibility, explicit replay refusal and observer admission checks; Kiro requires a single-use permit before dispatch.

Resilience and Maintainability Implications

  • observed — External settlement uses aggregate pending-send counts, and release restores a captured previous target. These mechanisms predate this PR. Inspected sequential tests do not establish correct ownership under concurrent external settlement or out-of-order release, so concurrency safety remains unresolved rather than an introduced finding.

Hardening Proposals

  • proposed — If additional callers adopt rebasing, consider an authority-bearing rebuild operation instead of a freely supplied boolean, so the exemption cannot silently spread to ordinary failover paths. This is control-drift prevention, not a demonstrated attack path.
🚥 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 3 functions across 4 files. (2 skipped: 2 … 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 and concisely describes the main change: Kiro retries now use the rebuilt request destination.
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.
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 3 functions across 4 files. (2 skipped: 2 unsupported.)

  • 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 commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 1, 2026
@luvs01
luvs01 marked this pull request as ready for review October 1, 2026 09:11
@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.

@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 5939b09 into dev Oct 1, 2026
42 checks passed
@lidge-jun
lidge-jun deleted the fix/luvs01-687-retry-budget-20261001 branch October 1, 2026 13:20
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