Repository navigation
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
| if (request && isAssistantLifecycleConflict(error)) | ||
| setRejected({ | ||
| request, | ||
| record: record ?? null, | ||
| refreshRequested: false, | ||
| }); |
There was a problem hiding this comment.
🟡 Lifecycle events suppress conflict recovery
When a lifecycle event precedes the 409 response, pending aborts and setRejected never records the conflict. Review latest record stays hidden, leaving the rejected operation on the retry-only path.
Learn more
The component aborts its pending request whenever entry.lifecycle.revision changes in the lifecycle-reset effect. A lifecycle transition that causes the server's 409 can also publish the new revision before the POST response reaches the browser. The cleanup then aborts the fetch, and the catch block skips all handling because abort.signal.aborted is true. The confirmed conflict therefore never enters the new recovery state.
Example: Continue starts at revision 2. End advances the source to revision 3 and its lifecycle event renders first. The effect aborts the POST before its lifecycle_changed response is processed, so the user sees only Retry Continue instead of Review latest record.
Recommended fix: Preserve enough operation state to classify the matching 409 despite a source lifecycle render, or stop aborting an in-flight Continue solely because its displayed lifecycle revision changed. Keep navigation, authority, scope, and boot-token abort fences intact, and add a test that publishes the conflicting lifecycle before fulfilling the 409 response.
Was this helpful? React with 👍 or 👎 to provide feedback.
Primary change type
Problem and motivation
A confirmed Continue lifecycle conflict left history review offering only retries of a request that the server had definitively rejected.
Summary and scope
Offer Review latest record only after a strictly parsed HTTP 409 lifecycle_changed. Fetch and display the saved record without creating work, then offer a separate Start a new continuation action. Replace the original operation only when its complete saved tuple still matches under the Web Lock. Uncertain failures and reloads preserve the original operation.
Related work
Related issue: https://linear.app/sapiom/issue/SAP-3292
Depends on #1037. Native stack target: main.
Validation
61/61 browser cases passed across assistant-continuation.spec.ts and assistant-history.spec.ts. Two deliberate regressions (loosening the definite-conflict check and removing the locked tuple comparison) failed as expected, were restored, and were followed by the clean broader run. Frontend TypeScript, explicit browser ESLint, scoped formatting and git diff --check passed. These are browser fixtures; actual server/native validation follows at the final stack.
Tests and documentation
Added seven conflict, new-tab receipt, missing-record and reload regressions; captured and inspected before/after action states. Updated assistant-history.md. Increment: 258 changed lines including tests/docs/changeset; SDK pr:size is absent.
Compatibility and release impact
Security
AI assistance
Checklist
Review evidence
Disposable browser fixture states before and after Review latest record; no historical baseline or native-execution claim. The refreshed text is visible before the separate Start a new continuation action becomes available.
Review disposition
A separate browser probe covered the lifecycle event arriving before the first 409 response. That event cancels adoption; the next explicit Retry Continue sends the identical saved tuple, observes a trustworthy 409 and enables Review latest record. Only the subsequent explicit new-operation action replaces the UUID. The child opened paused with zero prompt/final-response calls (1 probe passed). The extra retry is intentional when the first rejection could not be observed; no source change was required.