Skip to content

fix(harness): recover from confirmed Continue conflicts [SAP-3292] - #1038

Closed
ynadge wants to merge 1 commit into
assistant-ui-operation-retriesfrom
assistant-continue-conflict-recovery
Closed

ynadge wants to merge 1 commit into
assistant-ui-operation-retriesfrom
assistant-continue-conflict-recovery

Conversation

@ynadge

@ynadge ynadge commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Primary change type

  • Bug fix

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

  • Uses the existing history pane and buttons. A new operation requires both fresh displayed history and an explicit click; no automatic model or Terminal work. No change to native activation or runtime pins.
  • Changeset: @sapiom/harness patch.
  • Fix-forward: Preserve the saved request tuple and correct conflict classification or the locked comparison before allowing replacement.

Security

  • No secrets, credentials, private data, or unsanitized logs included.
  • No suspected vulnerability disclosed publicly.

AI assistance

  • AI assistance used: Codex implemented and reviewed the change with the validation described above.

Checklist

  • CONTRIBUTING.md read; maintainer-authorized issue work.
  • One focused increment, checks, documentation and release impact addressed.
  • I can explain and maintain the submitted changes.

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.

Before review Refreshed record
Before review Refreshed record

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.

@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 468d88ea-8fba-444f-b5bf-dbe344414cdf

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Devin Review

Comment on lines +95 to +100
if (request && isAssistantLifecycleConflict(error))
setRejected({
request,
record: record ?? null,
refreshRequested: false,
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant