Skip to content

fix(sync): validate autosync accepted seqs#375

Merged
Alan-TheGentleman merged 1 commit into
mainfrom
fix/autosync-ack-cardinality
May 14, 2026
Merged

fix(sync): validate autosync accepted seqs#375
Alan-TheGentleman merged 1 commit into
mainfrom
fix/autosync-ack-cardinality

Conversation

@Alan-TheGentleman
Copy link
Copy Markdown
Collaborator

Summary

  • validate cloud accepted_seqs cardinality before autosync acks local mutations
  • keep acking original local mutation seqs, not cloud-assigned seq values
  • add regression coverage for nil/empty/short/long accepted seq responses

Fixes #238
Fixes #298

Tests

  • go test ./internal/cloud/autosync ./cmd/engram
  • go test ./...

Review

  • Fresh reviewer approved: no blockers.

Copilot AI review requested due to automatic review settings May 14, 2026 11:16
@Alan-TheGentleman Alan-TheGentleman added the type:bug Bug fix label May 14, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens autosync’s safety checks when pushing local mutations to Engram Cloud by requiring the server to return an accepted_seqs list whose cardinality matches the pushed batch before acknowledging (acking) local mutation seqs. This prevents autosync from marking local mutations as acked when the server response is missing or structurally inconsistent, addressing reported silent-ack failure modes.

Changes:

  • Validate that PushMutations returns a non-nil result and that accepted_seqs count matches the pushed batch size before acking local mutation seqs.
  • Ensure autosync continues to ack the original local mutation Seq values (not cloud-assigned seq values).
  • Add regression tests covering nil/empty/short/long accepted_seqs responses.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
internal/cloud/autosync/manager.go Adds result validation and accepted_seqs cardinality checks before acking local mutation seqs.
internal/cloud/autosync/manager_test.go Adds/updates tests to ensure local seqs are acked only when accepted_seqs cardinality matches, and never acked on mismatch.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Alan-TheGentleman Alan-TheGentleman merged commit 9200d99 into main May 14, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Bug fix

Projects

None yet

2 participants