fix(sync): draft-snapshot apply quarantines instead of aborting the pull batch (#133)#136
Merged
Merged
Conversation
…ull batch (#133) The draft.snapshot.created apply case hard-errored on an unknown project and the raw error aborted the whole ApplyEventsWithStats batch, freezing the pull cursor. Mirror the env pointer shape: a winning tombstone drops the pointer; an absent, non-tombstoned project quarantines as a replayable, cursor-consuming draft_pending_project conflict; the env replay generalizes to ReplayPendingProjectConflicts covering both kinds. Signed-but-malformed draft/env payloads (decode failure, unsafe path, bad blob ref) wrap state.ErrEventVerification and quarantine-as-consumed on both planes. The pending-pointer chain-successor hold is pinned as a bounded, recoverable interaction (Codex review), not a wedge. Fixes #133 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughDraft snapshot apply handling is changed to mirror env-profile behavior: missing non-tombstoned projects now quarantine the pointer event as a replayable conflict instead of aborting the batch, while tombstoned projects silently drop it. ChangesPending Project Conflict Generalization
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Sync as CLI sync/devices approve
participant Apply as ApplyEventsWithStats
participant Handler as applyEventTx
participant Store as State Store
participant Replay as ReplayPendingProjectConflicts
Sync->>Apply: apply pulled events (draft/env pointer)
Apply->>Handler: process EventDraftSnapshotCreated / EventEnvProfileUpdated
Handler->>Store: lookup project by path
alt project tombstoned
Handler-->>Apply: drop pointer silently
else project missing (not tombstoned)
Handler-->>Apply: pending-project error
Apply->>Store: insert pending conflict (env/draft_pending_project)
end
Sync->>Replay: replay pending project conflicts
Replay->>Store: fetch open pending conflicts
Replay->>Handler: re-apply stored pointer event
Handler-->>Replay: success
Replay->>Store: resolve conflict
Related issues: Suggested labels: sync, bug-fix, needs-tests-verified Suggested reviewers: Reederey87 🐰 A pointer once quarantined, no longer stuck in dread, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/sync/events.go`:
- Around line 1000-1011: The pending-project replay loop in
internal/sync/events.go is applying conflicts in the order returned by
OpenConflictsByType instead of event order, which can let a newer draft override
an older one when RecordDraftSnapshotTx updates current_snapshot_id. Collect the
pending-project conflicts first, sort them by the restored event’s order
metadata (or equivalent event sequence/time field available on the unmarshaled
state.Event/details), and then call ApplyEventsWithStats in that sorted order so
older drafts are replayed before newer ones.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 65c627bb-b78b-4807-a519-b969c6653cbb
📒 Files selected for processing (9)
internal/cli/devices.gointernal/cli/sync.gointernal/sync/apply_test.gointernal/sync/events.gospec/07_NAMESPACE_AND_SYNC_MODEL.mdspec/09_SECRETS_AND_ENVIRONMENT.mdspec/13_CLI_DAEMON_API.mdspec/16_TEST_PLAN.mdspec/18_WORK_LOG.md
Comment on lines
1000
to
1011
| for _, c := range conflicts { | ||
| var details eventVerificationConflictDetails | ||
| if json.Unmarshal([]byte(c.DetailsJSON), &details) != nil || details.Kind != EventConflictKindEnvPendingProject { | ||
| if json.Unmarshal([]byte(c.DetailsJSON), &details) != nil || !isPendingProjectConflictKind(details.Kind) { | ||
| continue | ||
| } | ||
| var restored state.Event | ||
| if err := json.Unmarshal([]byte(details.EventJSON), &restored); err != nil { | ||
| logging.Logger(ctx).Warn("env-pending replay: conflict carries unparseable event JSON", | ||
| "conflict_id", c.ID, "event_id", details.EventID, "err", err.Error()) | ||
| logging.Logger(ctx).Warn("pending-project replay: conflict carries unparseable event JSON", | ||
| "conflict_id", c.ID, "kind", details.Kind, "event_id", details.EventID, "err", err.Error()) | ||
| continue | ||
| } | ||
| _, stats, err := ApplyEventsWithStats(ctx, st, []state.Event{restored}, nil) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Replay pending pointers in event order before applying them.
Line 1000 currently replays conflicts in OpenConflictsByType order (details_json, id), not event order. For multiple draft_pending_project conflicts on the same project, this can apply a newer draft before an older one; RecordDraftSnapshotTx then unconditionally updates current_snapshot_id, so the older replay can become the current draft.
Proposed fix
- replayed := 0
+ type pendingProjectReplay struct {
+ conflict state.Conflict
+ details eventVerificationConflictDetails
+ event state.Event
+ }
+ var pending []pendingProjectReplay
for _, c := range conflicts {
var details eventVerificationConflictDetails
if json.Unmarshal([]byte(c.DetailsJSON), &details) != nil || !isPendingProjectConflictKind(details.Kind) {
continue
}
var restored state.Event
if err := json.Unmarshal([]byte(details.EventJSON), &restored); err != nil {
logging.Logger(ctx).Warn("pending-project replay: conflict carries unparseable event JSON",
"conflict_id", c.ID, "kind", details.Kind, "event_id", details.EventID, "err", err.Error())
continue
}
+ pending = append(pending, pendingProjectReplay{conflict: c, details: details, event: restored})
+ }
+ sort.Slice(pending, func(i, j int) bool {
+ if pending[i].event.HLC == pending[j].event.HLC {
+ if pending[i].event.DeviceID == pending[j].event.DeviceID {
+ return pending[i].event.ID < pending[j].event.ID
+ }
+ return pending[i].event.DeviceID < pending[j].event.DeviceID
+ }
+ return pending[i].event.HLC < pending[j].event.HLC
+ })
+ replayed := 0
+ for _, replay := range pending {
+ c := replay.conflict
+ details := replay.details
+ restored := replay.event
_, stats, err := ApplyEventsWithStats(ctx, st, []state.Event{restored}, nil)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for _, c := range conflicts { | |
| var details eventVerificationConflictDetails | |
| if json.Unmarshal([]byte(c.DetailsJSON), &details) != nil || details.Kind != EventConflictKindEnvPendingProject { | |
| if json.Unmarshal([]byte(c.DetailsJSON), &details) != nil || !isPendingProjectConflictKind(details.Kind) { | |
| continue | |
| } | |
| var restored state.Event | |
| if err := json.Unmarshal([]byte(details.EventJSON), &restored); err != nil { | |
| logging.Logger(ctx).Warn("env-pending replay: conflict carries unparseable event JSON", | |
| "conflict_id", c.ID, "event_id", details.EventID, "err", err.Error()) | |
| logging.Logger(ctx).Warn("pending-project replay: conflict carries unparseable event JSON", | |
| "conflict_id", c.ID, "kind", details.Kind, "event_id", details.EventID, "err", err.Error()) | |
| continue | |
| } | |
| _, stats, err := ApplyEventsWithStats(ctx, st, []state.Event{restored}, nil) | |
| type pendingProjectReplay struct { | |
| conflict state.Conflict | |
| details eventVerificationConflictDetails | |
| event state.Event | |
| } | |
| var pending []pendingProjectReplay | |
| for _, c := range conflicts { | |
| var details eventVerificationConflictDetails | |
| if json.Unmarshal([]byte(c.DetailsJSON), &details) != nil || !isPendingProjectConflictKind(details.Kind) { | |
| continue | |
| } | |
| var restored state.Event | |
| if err := json.Unmarshal([]byte(details.EventJSON), &restored); err != nil { | |
| logging.Logger(ctx).Warn("pending-project replay: conflict carries unparseable event JSON", | |
| "conflict_id", c.ID, "kind", details.Kind, "event_id", details.EventID, "err", err.Error()) | |
| continue | |
| } | |
| pending = append(pending, pendingProjectReplay{conflict: c, details: details, event: restored}) | |
| } | |
| sort.Slice(pending, func(i, j int) bool { | |
| if pending[i].event.HLC == pending[j].event.HLC { | |
| if pending[i].event.DeviceID == pending[j].event.DeviceID { | |
| return pending[i].event.ID < pending[j].event.ID | |
| } | |
| return pending[i].event.DeviceID < pending[j].event.DeviceID | |
| } | |
| return pending[i].event.HLC < pending[j].event.HLC | |
| }) | |
| replayed := 0 | |
| for _, replay := range pending { | |
| c := replay.conflict | |
| details := replay.details | |
| restored := replay.event | |
| _, stats, err := ApplyEventsWithStats(ctx, st, []state.Event{restored}, nil) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/sync/events.go` around lines 1000 - 1011, The pending-project replay
loop in internal/sync/events.go is applying conflicts in the order returned by
OpenConflictsByType instead of event order, which can let a newer draft override
an older one when RecordDraftSnapshotTx updates current_snapshot_id. Collect the
pending-project conflicts first, sort them by the restored event’s order
metadata (or equivalent event sequence/time field available on the unmarshaled
state.Event/details), and then call ApplyEventsWithStats in that sorted order so
older drafts are replayed before newer ones.
Reederey87
added a commit
that referenced
this pull request
Jul 6, 2026
spec/14 DIRECTION bullet marking the wave complete (draft-pending quarantine #133, self-healing WCK rotation #134, squash-merge worktree GC P4-GIT-04, devstrap service P4-PROD-04) with the post-wave backlog and the Pass-7 audit checkpoint; ledger intro trail extended. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #133.
What
The
draft.snapshot.createdapply case inapplyEventTxhard-errored on an unknown project, and that raw error aborted the entireApplyEventsWithStatsbatch and froze the pull cursor — one stale/early draft pointer (e.g. itsproject.addedsitting in a verification quarantine because the origin device is unpinned here) wedged the whole sync.How
Mirrors the env-pointer shape shipped in the ENV-SYNC-01 wave (#131/#132):
draft_pending_projectconflict (new kind besideenv_pending_project; sharedinsertPendingProjectConflict). The batch never aborts.ReplayPendingProjectConflictscovering both kinds — run after every pull apply (internal/cli/sync.go) and at approve-time replay (internal/cli/devices.go); draft re-apply re-runsRecordDraftSnapshotTxthrough the normal verified path and resolves the conflict only after a successful apply.pathkey.Cleanpath, or (opus review) a blob ref that can never passage_blob:validation — now wrapsstate.ErrEventVerificationat the apply layer, quarantining-as-consumed instead of aborting the batch or error-looping the pending replay (mirrors the PR feat(devices): synced device-trust propagation (TRUST-01) #132 trust-payload convention; only an APPROVED signer can reach these branches). The env-side wraps and blob-ref validation were coordinator review additions on top of the delegated draft-side fix.events, so the origin device's next chained event breaks onvalidatePrevEventHashand holds that device's cursor. Verified to be a bounded temporary hold with existing recovery (the real CLI can never emit a draft before its ownproject.added; once the project lands, replay inserts the pointer, the re-delivered successor applies, and its hash-chain conflict auto-resolves via the P6-SEC-03 path) — pinned end-to-end byTestApplyDraftSnapshotPendingChainSuccessorRecoversand documented in spec/07.Tests
TestApplyDraftSnapshotUnknownProjectQuarantinesWithoutAbort(batch continues, cursor advances, conflict kind pinned),TestApplyDraftSnapshotTombstonedProjectDrops,TestApplyDraftSnapshotMalformedPayloadQuarantinesWithoutAbort,TestApplyDraftSnapshotBadBlobRefQuarantinesWithoutAbort,TestApplyEnvProfileMalformedPayloadQuarantinesWithoutAbort,TestReplayPendingDraftSnapshotConflictRecovers,TestApplyDraftSnapshotPendingChainSuccessorRecovers.Specs
spec/07 (P6-SYNC-01 conflict-kind list + replay + chain-hold note), spec/09 (renamed replay), spec/13 (sync pending-project replay sentence), spec/16 (test inventory), spec/18 work log.
Validation
gofmtclean ·golangci-lint run0 issues ·go test -race ./...ok ·spec-drift --base origin/mainpass · dual review (coordinator line-by-line + gpt-5.5 implementation + opus reviewer + Codex review).🤖 Generated with Claude Code
Summary by CodeRabbit