Skip to content

fix(kiro): keep Advisor requests cloneable for the retry snapshot - #7981

Merged
lidge-jun merged 1 commit into
devfrom
fix/kiro-advisor-clone-7974
Oct 30, 2026
Merged

lidge-jun merged 1 commit into
devfrom
fix/kiro-advisor-clone-7974

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Oct 30, 2026 •

Copy link
Copy Markdown
Owner

Summary

With Advisor enabled, every Kiro request failed with invalid_request_error: The object can not be cloned. (#7974). Advisor attaches a function-valued guard to the parsed request (parsed._advisorGuard), and the Kiro adapter took its completion-retry snapshot with structuredClone(parsed), which throws DataCloneError on functions.

The snapshot now copies the request without top-level function values, while the original request keeps its guard so streaming delivery still applies it. The completion retry rebuilds from the snapshot as before. No other adapter clones the parsed request this way. structure/providers/kiro.md records the snapshot contract.

Closes #7974

Verification

Bun 1.4.4; focused because the shared host is under load (AGENTS.md resource exception; breadth left to PR CI).

  • Before the fix: bun test tests/providers/kiro/kiro-single-final.test.ts -t 'advisor guard survives request building and completion retry' → 0 pass / 1 fail, DataCloneError: The object can not be cloned. from buildRequest's structuredClone.
  • After: bun test tests/providers/kiro/kiro-single-final.test.ts tests/providers/kiro/kiro-adapter.test.ts tests/providers/kiro/kiro-completion-with-real-tools.test.ts tests/providers/kiro/kiro-fallback-error-body.test.ts → 141 pass / 0 fail.
  • bun run typecheck (2,243 files, 0 errors) and bun run structure:check pass.
  • NOT RUN locally: bun run test:changed and the full suite (host at critical load); PR CI test changed covers them.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed (structure contract; no user-facing change beyond the fix).
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults (no credential or auth change).

Summary by CodeRabbit

  • Bug Fixes
    • Improved Kiro’s completion retry behavior for requests containing runtime callbacks, helping the retry complete without altering the original request.

@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner October 30, 2026 11:24
@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 3a70dd4 and ad83825.

Files selected for processing: 3
  • src/adapters/kiro/adapter.ts
  • structure/providers/kiro.md
  • tests/providers/kiro/kiro-single-final.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

Walkthrough

The Kiro adapter now excludes top-level function-valued properties from its retry snapshot. A regression test checks that an Advisor-like guard remains on the original request and that the completion retry produces a completed turn.

Changes: Kiro retry snapshot

Layer / File(s) Summary
Filter callbacks and verify retry behavior
src/adapters/kiro/adapter.ts, tests/providers/kiro/kiro-single-final.test.ts, structure/providers/kiro.md
At adapter.ts:300–303, buildRequest filters function-valued fields before cloning the snapshot. The test at kiro-single-final.test.ts:41–82 checks that the guard stays on the input, two requests occur, the retry body includes the prompt and model, and the final event marks the turn done. The provider documentation at kiro.md:162 describes the filtered snapshot.

Priority: Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested labels: bug

Suggested reviewers: ingwannu

Merge Risk: Minimal · up to ad838

The reported Advisor retry failure is addressed, with no concrete unresolved merge-blocking defect identified.

Pre-merge checks: Passed 4 · Failed 1

Failed 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 with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed 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: keeping Kiro Advisor requests cloneable for the retry snapshot.
Linked Issues check Passed Issue #7974 requires Kiro requests to work with Advisor-like function metadata and regression coverage. In src/adapters/kiro/adapter.ts, buildRequest creates requestSnapshot from `Object.entries…
Out of Scope Changes check Passed The reviewed changes are limited to the Kiro snapshot implementation in src/adapters/kiro/adapter.ts, focused regression coverage in tests/providers/kiro/kiro-single-final.test.ts, and the related…
Full details: Linked Issues check

Explanation

Issue #7974 requires Kiro requests to work with Advisor-like function metadata and regression coverage. In src/adapters/kiro/adapter.ts, buildRequest creates requestSnapshot from Object.entries(parsed) after filtering top-level function values. The original parsed object remains unchanged, so _advisorGuard remains available for streaming. The completion retry clones the sanitized snapshot in fallbackFactory and rebuilds the request. tests/providers/kiro/kiro-single-final.test.ts adds advisor guard survives request building and completion retry. The test supplies an async-generator _advisorGuard, completes request building, verifies the same guard remains on the input, verifies two physical requests, checks retry body content, and verifies a completed final event. structure/providers/kiro.md documents the snapshot contract.

Full details: Out of Scope Changes check

Explanation

The reviewed changes are limited to the Kiro snapshot implementation in src/adapters/kiro/adapter.ts, focused regression coverage in tests/providers/kiro/kiro-single-final.test.ts, and the related contract note in structure/providers/kiro.md. Each change supports issue #7974. No unrelated behavior or cleanup is demonstrated by the supplied evidence.

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 with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

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
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 30, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer integration into dev (MAINTAINERS.md maintainer-integration exception; not a self-approval or independent review). Closes #7974.

  • Exact head: ad8382594c46b76855ea5ad527e5b49afd5900dc
  • Exact-head CI: Cross-platform CI run 41047914771 — gates, test changed, structure gate and the ci aggregate success (full shards skipped by the fast-lane selector); hygiene, labeler, enforce-target, CodeRabbit success. No failing or pending check on this head.
  • Independent review: a fresh reviewer (not the builder) read the exact head diff against the brief — PASS. Accepted note for follow-up: the regression drives the adapter directly; delivery-level coverage with an observable Advisor guard would add confidence but is not required for this defect.
  • Local (Bun 1.4.4, focused under the shared-host load exception): red before the fix (DataCloneError from structuredClone), 141 pass / 0 fail across the four Kiro test files after; typecheck and structure:check pass.
  • Union check vs current origin/dev 0cc5a5958d: clean merge; three files, none capped or near the 2,000-line default; no new test file, union or count change.

@lidge-jun
lidge-jun merged commit 4fc8700 into dev Oct 30, 2026
31 checks passed
@lidge-jun
lidge-jun deleted the fix/kiro-advisor-clone-7974 branch October 30, 2026 11:39
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.

1 participant