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 (published && !restored) | ||
| throw new AssistantStorageCommitUnconfirmedError(); |
There was a problem hiding this comment.
🟡 Cleanup masks unconfirmed commits
When temporary-file removal fails after uncertain publication, writeAssistantJson replaces AssistantStorageCommitUnconfirmedError with the cleanup error. Callers cannot identify and reconcile the possibly committed operation.
Learn more
A rejection from an awaited finally block replaces the error already thrown by its try or catch. Here, storage has already published the destination and failed to confirm rollback. Another filesystem failure during fs.rm therefore removes the only signal that the commit is uncertain. This is especially reachable during the same degraded-storage condition that caused publication or rollback to fail.
Example: The destination rename succeeds, directory sync fails with EIO, rollback also fails, and removing .rollback then fails with EIO. The caller receives the removal error without ASSISTANT_STORAGE_COMMIT_UNCONFIRMED, although the new generation remains readable.
Recommended fix: Make temporary cleanup best-effort when an earlier error is pending, or capture cleanup failures without replacing AssistantStorageCommitUnconfirmedError. Preserve cleanup failures only when no primary operation error exists.
Was this helpful? React with 👍 or 👎 to provide feedback.
Primary change type
Problem and motivation
A failed metadata acknowledgement could be mistaken for a completed rollback, and private Resume replay accepted a changed execution argument.
Summary and scope
Report an explicit unconfirmed commit when publication succeeded but durability/rollback did not. Retain both valid generations for reconciliation. Reject original-base Resume replay with different execution, while exact lost-ACK proof still returns the committed lifecycle.
Related work
Related issue: https://linear.app/sapiom/issue/SAP-3292
Depends on #1023. Native stack target: main.
Validation
45 focused tests passed across Resume, Resume-store and metadata files. Full Harness server TypeScript, scoped ESLint and diff checks passed.
Tests and documentation
Inject both post-publication durability failure and rollback failure, then restart and reconcile the exact Resume UUID/revision. Verify prior generation retention and execution-mismatch rejection. Updated assistant-history.md. Increment:69 changed lines including tests/docs/changeset; SDK pr:size is absent.
Compatibility and release impact
Security
AI assistance
Checklist