diff --git a/internal/cli/devices.go b/internal/cli/devices.go index 84b657a..d4f41f2 100644 --- a/internal/cli/devices.go +++ b/internal/cli/devices.go @@ -578,13 +578,13 @@ func replayQuarantinedEvents(ctx context.Context, stderr io.Writer, opts *option if replayed > 0 { _, _ = fmt.Fprintf(stderr, "Replayed %d quarantined event(s) from device %s\n", replayed, deviceID) } - // ENV-SYNC-01 review: the replayed events may include the project.added an - // env_pending_project quarantine was waiting on — re-attempt those now so - // approving the project's origin device recovers the profile in one step. - if n, err := dssync.ReplayPendingEnvProfileConflicts(ctx, store); err != nil { - _, _ = fmt.Fprintf(stderr, "warning: could not replay pending env profile conflicts: %v\n", err) + // The replayed events may include the project.added a pending-project + // pointer quarantine was waiting on — re-attempt those now so approving the + // project's origin device recovers env and draft pointers in one step. + if n, err := dssync.ReplayPendingProjectConflicts(ctx, store); err != nil { + _, _ = fmt.Fprintf(stderr, "warning: could not replay pending project conflicts: %v\n", err) } else if n > 0 { - _, _ = fmt.Fprintf(stderr, "Recovered %d pending env profile(s)\n", n) + _, _ = fmt.Fprintf(stderr, "Recovered %d pending project pointer(s)\n", n) } } diff --git a/internal/cli/sync.go b/internal/cli/sync.go index 1ce2651..7f2c50e 100644 --- a/internal/cli/sync.go +++ b/internal/cli/sync.go @@ -249,11 +249,10 @@ func pullAndApplyEvents(ctx context.Context, store *state.Store, hub dssync.Hub, } } } - // ENV-SYNC-01 review: an env.profile.updated quarantined earlier because - // its project had not applied yet may be recoverable now that this batch - // applied (the project could have arrived in it). Replay AFTER apply so - // recovery is one-cycle. - if _, err := dssync.ReplayPendingEnvProfileConflicts(ctx, store); err != nil { + // Pointer events quarantined earlier because their project had not applied + // yet may be recoverable now that this batch applied (the project could + // have arrived in it). Replay AFTER apply so recovery is one-cycle. + if _, err := dssync.ReplayPendingProjectConflicts(ctx, store); err != nil { return pullApplyOutcome{}, err } return pullApplyOutcome{events: remoteEvents, stats: stats}, nil diff --git a/internal/sync/apply_test.go b/internal/sync/apply_test.go index d8c2976..3039277 100644 --- a/internal/sync/apply_test.go +++ b/internal/sync/apply_test.go @@ -467,6 +467,34 @@ func signedEnvProfileEvent(t *testing.T, signing devicekeys.SigningIdentity, id, return ev } +func draftSnapshotEvent(t *testing.T, id, dev string, seq, hlc int64, payload DraftSnapshotPayload) state.Event { + t.Helper() + raw, err := json.Marshal(payload) + if err != nil { + t.Fatal(err) + } + return state.Event{ + ID: id, + DeviceID: dev, + Seq: seq, + HLC: hlc << hlcLogicalBits, + Type: EventDraftSnapshotCreated, + PayloadJSON: string(raw), + ContentHash: state.ContentHash(string(raw)), + } +} + +func signedDraftSnapshotEvent(t *testing.T, signing devicekeys.SigningIdentity, id, dev string, seq, hlc int64, payload DraftSnapshotPayload) state.Event { + t.Helper() + ev := draftSnapshotEvent(t, id, dev, seq, hlc, payload) + sig, err := devicekeys.Sign(signing.Private, "devstrap:event:v2", state.EventSignaturePayloadV2(ev)) + if err != nil { + t.Fatal(err) + } + ev.DeviceSig = sig + return ev +} + func TestApplyEnvProfileEventCreatesProfileAndBindings(t *testing.T) { ctx := context.Background() st, device := newSyncStore(t) @@ -533,7 +561,7 @@ func TestApplyEnvProfileEventDuplicateIdempotent(t *testing.T) { // TestApplyEnvProfileEventUnknownProjectQuarantinesWithoutAbort: an env event // for an absent (NOT tombstoned) project must not abort the batch, must not be // silently consumed, and must leave a replayable env_pending_project -// quarantine; ReplayPendingEnvProfileConflicts recovers it once the project +// quarantine; ReplayPendingProjectConflicts recovers it once the project // applies (Codex review P2 on the original soft-skip). func TestApplyEnvProfileEventUnknownProjectQuarantinesWithoutAbort(t *testing.T) { ctx := context.Background() @@ -579,7 +607,7 @@ func TestApplyEnvProfileEventUnknownProjectQuarantinesWithoutAbort(t *testing.T) } // Replay before the project exists: row stays open, nothing recovered. - if n, err := ReplayPendingEnvProfileConflicts(ctx, st); err != nil || n != 0 { + if n, err := ReplayPendingProjectConflicts(ctx, st); err != nil || n != 0 { t.Fatalf("premature replay: n=%d err=%v", n, err) } @@ -590,7 +618,7 @@ func TestApplyEnvProfileEventUnknownProjectQuarantinesWithoutAbort(t *testing.T) if _, err := ApplyEvents(ctx, st, []state.Event{addMissing}); err != nil { t.Fatal(err) } - if n, err := ReplayPendingEnvProfileConflicts(ctx, st); err != nil || n != 1 { + if n, err := ReplayPendingProjectConflicts(ctx, st); err != nil || n != 1 { t.Fatalf("replay after project applied: n=%d err=%v", n, err) } project, err := st.ProjectByPath(ctx, "work/acme/missing") @@ -656,6 +684,318 @@ func TestApplyEnvProfileEventTombstonedProjectDrops(t *testing.T) { } } +func TestApplyDraftSnapshotUnknownProjectQuarantinesWithoutAbort(t *testing.T) { + ctx := context.Background() + st, device := newSyncStore(t) + signing := addRemoteDeviceForApplyTest(t, st, "device-draft", "approved") + now := time.Now().UnixMilli() + draft := signedDraftSnapshotEvent(t, signing, "evt_draft_missing", "device-draft", 1, now, DraftSnapshotPayload{ + Path: "work/acme/missing-draft", + BlobRef: "age_blob:deadbeef", + ByteSize: 42, + FileCount: 3, + }) + add := projEvent(t, device.ID, EventProjectAdded, now+1, "work/acme/valid-draft", "github.com/acme/valid-draft") + add.Seq = 1 + safe, stats, err := ApplyEventsWithStats(ctx, st, []state.Event{draft, add}, nil) + if err != nil { + t.Fatal(err) + } + if stats.Quarantined != 1 || stats.CursorHeld { + t.Fatalf("stats=%+v, want exactly the draft event quarantined and no held cursor", stats) + } + if safe.After("device-draft") != 1 || safe.After(device.ID) != 1 { + t.Fatalf("safe cursor=%v, want both devices advanced (quarantine consumes the slot)", safe) + } + if _, err := st.ProjectByPath(ctx, "work/acme/valid-draft"); err != nil { + t.Fatal(err) + } + conflicts, err := st.OpenConflictsByType(ctx, ConflictEventVerification) + if err != nil { + t.Fatal(err) + } + found := false + for _, c := range conflicts { + var d eventVerificationConflictDetails + if json.Unmarshal([]byte(c.DetailsJSON), &d) == nil && d.Kind == EventConflictKindDraftPendingProject && d.EventID == "evt_draft_missing" { + found = true + } + } + if !found { + t.Fatalf("want a draft_pending_project quarantine for evt_draft_missing, got %#v", conflicts) + } +} + +func TestApplyDraftSnapshotTombstonedProjectDrops(t *testing.T) { + ctx := context.Background() + st, device := newSyncStore(t) + signing := addRemoteDeviceForApplyTest(t, st, "device-draft", "approved") + now := time.Now().UnixMilli() + add := projEvent(t, device.ID, EventProjectAdded, now, "work/acme/gone-draft", "github.com/acme/gone-draft") + del := projEvent(t, device.ID, EventProjectDeleted, now+1, "work/acme/gone-draft", "github.com/acme/gone-draft") + if _, err := ApplyEvents(ctx, st, []state.Event{add, del}); err != nil { + t.Fatal(err) + } + draft := signedDraftSnapshotEvent(t, signing, "evt_draft_gone", "device-draft", 1, now+2, DraftSnapshotPayload{ + Path: "work/acme/gone-draft", + BlobRef: "age_blob:deadbeef", + ByteSize: 42, + FileCount: 3, + }) + _, stats, err := ApplyEventsWithStats(ctx, st, []state.Event{draft}, nil) + if err != nil { + t.Fatal(err) + } + if stats.Quarantined != 0 { + t.Fatalf("stats=%+v, want the tombstoned draft pointer dropped without quarantine", stats) + } + conflicts, err := st.OpenConflictsByType(ctx, ConflictEventVerification) + if err != nil { + t.Fatal(err) + } + for _, c := range conflicts { + var d eventVerificationConflictDetails + if json.Unmarshal([]byte(c.DetailsJSON), &d) == nil && d.Kind == EventConflictKindDraftPendingProject { + t.Fatalf("tombstoned draft drop must not quarantine: %#v", c) + } + } +} + +// TestApplyDraftSnapshotBadBlobRefQuarantinesWithoutAbort (review finding): a +// signed draft event whose project EXISTS but whose blob ref can never pass +// RecordDraftSnapshotTx's validation must quarantine-as-consumed at the apply +// layer — a raw store error would abort the batch, or error-loop the pending +// replay once the project lands. +func TestApplyDraftSnapshotBadBlobRefQuarantinesWithoutAbort(t *testing.T) { + ctx := context.Background() + st, device := newSyncStore(t) + signing := addRemoteDeviceForApplyTest(t, st, "device-draft", "approved") + now := time.Now().UnixMilli() + add := projEvent(t, device.ID, EventProjectAdded, now, "work/acme/badref", "github.com/acme/badref") + add.Seq = 1 + bad := signedDraftSnapshotEvent(t, signing, "evt_badref_draft", "device-draft", 1, now+1, DraftSnapshotPayload{ + Path: "work/acme/badref", + BlobRef: "s3://not-an-age-blob", + ByteSize: 1, + FileCount: 1, + }) + good := projEvent(t, device.ID, EventProjectAdded, now+2, "work/acme/after-badref", "github.com/acme/after-badref") + good.Seq = 2 + _, stats, err := ApplyEventsWithStats(ctx, st, []state.Event{add, bad, good}, nil) + if err != nil { + t.Fatal(err) + } + if stats.Quarantined != 1 || stats.CursorHeld { + t.Fatalf("stats=%+v, want bad blob ref quarantined as consumed", stats) + } + if _, err := st.ProjectByPath(ctx, "work/acme/after-badref"); err != nil { + t.Fatalf("batch must continue past the bad blob ref: %v", err) + } +} + +// TestApplyDraftSnapshotPendingChainSuccessorRecovers (Codex review): a +// pending-quarantined pointer is consumed for the cursor but never inserted +// into events, so an approved device's NEXT chained event breaks on +// validatePrevEventHash and HOLDS that device's cursor. This pins that the +// hold is temporary, not a wedge: once the project lands, the pending replay +// inserts the pointer, the re-delivered successor applies, and its hash-chain +// conflict auto-resolves (the P6-SEC-03 resolve-by-event-id path). +func TestApplyDraftSnapshotPendingChainSuccessorRecovers(t *testing.T) { + ctx := context.Background() + st, _ := newSyncStore(t) + signingB := addRemoteDeviceForApplyTest(t, st, "device-b", "approved") + signingC := addRemoteDeviceForApplyTest(t, st, "device-c", "approved") + now := time.Now().UnixMilli() + + draft := signedDraftSnapshotEvent(t, signingB, "evt_chain_draft", "device-b", 1, now, DraftSnapshotPayload{ + Path: "work/acme/chain", + BlobRef: "age_blob:feedface", + ByteSize: 5, + FileCount: 1, + }) + successor := projEvent(t, "device-b", EventProjectAdded, now+1, "work/acme/other-b", "github.com/acme/other-b") + successor.ID = "evt_chain_successor" + successor.Seq = 2 + successor.PrevEventHash = draft.ContentHash + sig, err := devicekeys.Sign(signingB.Private, "devstrap:event:v2", state.EventSignaturePayloadV2(successor)) + if err != nil { + t.Fatal(err) + } + successor.DeviceSig = sig + + safe, stats, err := ApplyEventsWithStats(ctx, st, []state.Event{draft, successor}, nil) + if err != nil { + t.Fatal(err) + } + // The pending draft is consumed; the chained successor breaks and HOLDS + // device-b's cursor below seq 2 for re-delivery. + if stats.Quarantined != 2 { + t.Fatalf("stats=%+v, want pending draft + chain break both quarantined", stats) + } + if got := safe.After("device-b"); got != 1 { + t.Fatalf("safe cursor for device-b = %d, want 1 (held below the chained successor)", got) + } + + // The project lands from another device; the pending replay inserts the + // draft pointer, restoring the chain anchor. + add := signedProjEvent(t, signingC, "evt_chain_add", "device-c", 1, now+2, EventProjectAdded, "work/acme/chain", "github.com/acme/chain") + if _, err := ApplyEvents(ctx, st, []state.Event{add}); err != nil { + t.Fatal(err) + } + if n, err := ReplayPendingProjectConflicts(ctx, st); err != nil || n != 1 { + t.Fatalf("pending replay: n=%d err=%v", n, err) + } + + // The re-delivered successor now applies and auto-resolves its + // hash-chain conflict. + if _, stats, err := ApplyEventsWithStats(ctx, st, []state.Event{successor}, nil); err != nil || stats.Quarantined != 0 { + t.Fatalf("re-delivered successor: stats=%+v err=%v, want clean apply", stats, err) + } + if _, err := st.ProjectByPath(ctx, "work/acme/other-b"); err != nil { + t.Fatalf("successor project missing after recovery: %v", err) + } + open, err := st.OpenConflictsByType(ctx, ConflictEventHashChain) + if err != nil { + t.Fatal(err) + } + if len(open) != 0 { + t.Fatalf("hash-chain conflict not auto-resolved after recovery: %#v", open) + } +} + +// TestApplyEnvProfileMalformedPayloadQuarantinesWithoutAbort pins the same +// malformed-payload convention on the ENV pointer (#133 residual): a verified +// event whose payload can never decode quarantines as consumed instead of +// aborting the pull batch. +func TestApplyEnvProfileMalformedPayloadQuarantinesWithoutAbort(t *testing.T) { + ctx := context.Background() + st, device := newSyncStore(t) + signing := addRemoteDeviceForApplyTest(t, st, "device-env", "approved") + now := time.Now().UnixMilli() + bad := state.Event{ + ID: "evt_bad_env", + DeviceID: "device-env", + Seq: 1, + HLC: now << hlcLogicalBits, + Type: EventEnvProfileUpdated, + PayloadJSON: `{"path":`, + } + bad.ContentHash = state.ContentHash(bad.PayloadJSON) + sig, err := devicekeys.Sign(signing.Private, "devstrap:event:v2", state.EventSignaturePayloadV2(bad)) + if err != nil { + t.Fatal(err) + } + bad.DeviceSig = sig + good := projEvent(t, device.ID, EventProjectAdded, now+1, "work/acme/after-env", "github.com/acme/after-env") + good.Seq = 1 + _, stats, err := ApplyEventsWithStats(ctx, st, []state.Event{bad, good}, nil) + if err != nil { + t.Fatal(err) + } + if stats.Quarantined != 1 || stats.CursorHeld { + t.Fatalf("stats=%+v, want malformed env quarantined as consumed", stats) + } + if _, err := st.ProjectByPath(ctx, "work/acme/after-env"); err != nil { + t.Fatalf("batch must continue past malformed env event: %v", err) + } +} + +func TestApplyDraftSnapshotMalformedPayloadQuarantinesWithoutAbort(t *testing.T) { + ctx := context.Background() + st, device := newSyncStore(t) + signing := addRemoteDeviceForApplyTest(t, st, "device-draft", "approved") + now := time.Now().UnixMilli() + bad := state.Event{ + ID: "evt_bad_draft", + DeviceID: "device-draft", + Seq: 1, + HLC: now << hlcLogicalBits, + Type: EventDraftSnapshotCreated, + PayloadJSON: `{"path":`, + } + bad.ContentHash = state.ContentHash(bad.PayloadJSON) + sig, err := devicekeys.Sign(signing.Private, "devstrap:event:v2", state.EventSignaturePayloadV2(bad)) + if err != nil { + t.Fatal(err) + } + bad.DeviceSig = sig + good := projEvent(t, device.ID, EventProjectAdded, now+1, "work/acme/after-draft", "github.com/acme/after-draft") + good.Seq = 1 + _, stats, err := ApplyEventsWithStats(ctx, st, []state.Event{bad, good}, nil) + if err != nil { + t.Fatal(err) + } + if stats.Quarantined != 1 || stats.CursorHeld { + t.Fatalf("stats=%+v, want malformed draft quarantined as consumed", stats) + } + if _, err := st.ProjectByPath(ctx, "work/acme/after-draft"); err != nil { + t.Fatalf("batch must continue past malformed draft event: %v", err) + } + conflicts, err := st.OpenConflictsByType(ctx, ConflictEventVerification) + if err != nil { + t.Fatal(err) + } + found := false + for _, c := range conflicts { + var d eventVerificationConflictDetails + if json.Unmarshal([]byte(c.DetailsJSON), &d) == nil && d.Kind == EventConflictKindVerification && d.EventID == "evt_bad_draft" { + found = true + } + } + if !found { + t.Fatalf("want a verification quarantine for malformed draft payload, got %#v", conflicts) + } +} + +func TestReplayPendingDraftSnapshotConflictRecovers(t *testing.T) { + ctx := context.Background() + st, _ := newSyncStore(t) + signing := addRemoteDeviceForApplyTest(t, st, "device-draft", "approved") + now := time.Now().UnixMilli() + draft := signedDraftSnapshotEvent(t, signing, "evt_draft_replay", "device-draft", 1, now, DraftSnapshotPayload{ + Path: "work/acme/replay-draft", + BlobRef: "age_blob:cafebabe", + ByteSize: 99, + FileCount: 7, + }) + if _, stats, err := ApplyEventsWithStats(ctx, st, []state.Event{draft}, nil); err != nil { + t.Fatal(err) + } else if stats.Quarantined != 1 { + t.Fatalf("stats=%+v, want initial draft quarantine", stats) + } + if n, err := ReplayPendingProjectConflicts(ctx, st); err != nil || n != 0 { + t.Fatalf("premature replay: n=%d err=%v", n, err) + } + add := signedProjEvent(t, signing, "evt_add_replay_draft", "device-draft", 2, now+1, EventProjectAdded, "work/acme/replay-draft", "github.com/acme/replay-draft") + if _, err := ApplyEvents(ctx, st, []state.Event{add}); err != nil { + t.Fatal(err) + } + if n, err := ReplayPendingProjectConflicts(ctx, st); err != nil || n != 1 { + t.Fatalf("replay after project applied: n=%d err=%v", n, err) + } + project, err := st.ProjectByPath(ctx, "work/acme/replay-draft") + if err != nil { + t.Fatal(err) + } + snap, err := st.LatestDraftSnapshot(ctx, project.ID) + if err != nil { + t.Fatal(err) + } + if snap == nil || snap.BlobRef != "age_blob:cafebabe" || snap.ByteSize != 99 || snap.FileCount != 7 { + t.Fatalf("recovered draft snapshot=%#v", snap) + } + open, err := st.OpenConflictsByType(ctx, ConflictEventVerification) + if err != nil { + t.Fatal(err) + } + for _, c := range open { + var d eventVerificationConflictDetails + if json.Unmarshal([]byte(c.DetailsJSON), &d) == nil && d.Kind == EventConflictKindDraftPendingProject { + t.Fatalf("draft_pending_project conflict should be resolved after replay: %#v", c) + } + } +} + func TestApplyEnvProfileEventLWWConvergesBothOrders(t *testing.T) { ctx := context.Background() for _, order := range []string{"low-then-high", "high-then-low"} { diff --git a/internal/sync/events.go b/internal/sync/events.go index 8a77aab..b8a60b5 100644 --- a/internal/sync/events.go +++ b/internal/sync/events.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "sort" + "strings" "time" "github.com/Reederey87/DevStrap/internal/id" @@ -209,8 +210,13 @@ const ( // EventConflictKindEnvPendingProject quarantines a verified // env.profile.updated whose project has not applied yet (ENV-SYNC-01 // review): recoverable ordering, e.g. the project's origin device is not - // pinned here yet. Replayed by ReplayPendingEnvProfileConflicts. + // pinned here yet. Replayed by ReplayPendingProjectConflicts. EventConflictKindEnvPendingProject = "env_pending_project" + // EventConflictKindDraftPendingProject quarantines a verified + // draft.snapshot.created whose project has not applied yet. This mirrors + // env_pending_project so one stale/early draft pointer cannot wedge the + // pull cursor. + EventConflictKindDraftPendingProject = "draft_pending_project" ) type eventVerificationConflictDetails struct { @@ -593,15 +599,19 @@ func ApplyEventsWithStats(ctx context.Context, st *state.Store, events []state.E record(event, true) continue } - if errors.Is(err, errEnvProjectPending) { - if conflictErr := insertEnvPendingProjectConflict(ctx, st, event, err); conflictErr != nil { + if errors.Is(err, errEnvProjectPending) || errors.Is(err, errDraftProjectPending) { + kind := EventConflictKindEnvPendingProject + if errors.Is(err, errDraftProjectPending) { + kind = EventConflictKindDraftPendingProject + } + if conflictErr := insertPendingProjectConflict(ctx, st, event, kind, err); conflictErr != nil { return nil, stats, errors.Join(err, conflictErr) } stats.Quarantined++ // Consumed for the cursor like verification quarantines: // re-delivery would hit the same missing project, and the full // event is preserved in the conflict for the per-cycle replay - // (ReplayPendingEnvProfileConflicts) once the project applies. + // (ReplayPendingProjectConflicts) once the project applies. record(event, true) continue } @@ -686,18 +696,21 @@ func insertEventHashChainConflict(ctx context.Context, st *state.Store, event st return st.InsertConflict(ctx, "", ConflictEventHashChain, string(raw)) } -// errEnvProjectPending marks a verified env.profile.updated event whose -// project row is absent WITHOUT a tombstone (ENV-SYNC-01 review, Codex P2): -// the ordering is recoverable — most commonly the project.added author is not -// pinned on this device yet, so its event sits in a verification quarantine — -// and consuming the env event silently would lose the profile even after the -// project later replays. The apply loop quarantines it for replay instead. -var errEnvProjectPending = errors.New("env profile project not yet applied") +// Pending-project sentinels mark verified pointer events whose project row is +// absent WITHOUT a tombstone: the ordering is recoverable — most commonly the +// project.added author is not pinned on this device yet, so its event sits in a +// verification quarantine — and consuming the pointer silently would lose data +// even after the project later replays. The apply loop quarantines these for +// replay instead. +var ( + errEnvProjectPending = errors.New("env profile project not yet applied") + errDraftProjectPending = errors.New("draft snapshot project not yet applied") +) -// insertEnvPendingProjectConflict preserves the full env event for replay once -// its project applies (ReplayPendingEnvProfileConflicts). -func insertEnvPendingProjectConflict(ctx context.Context, st *state.Store, event state.Event, cause error) error { - return insertEventConflictOnce(ctx, st, event, EventConflictKindEnvPendingProject, cause.Error()) +// insertPendingProjectConflict preserves the full pointer event for replay once +// its project applies (ReplayPendingProjectConflicts). +func insertPendingProjectConflict(ctx context.Context, st *state.Store, event state.Event, kind string, cause error) error { + return insertEventConflictOnce(ctx, st, event, kind, cause.Error()) } func insertEventVerificationConflict(ctx context.Context, st *state.Store, event state.Event, cause error) error { @@ -820,28 +833,51 @@ func applyEventTx(ctx context.Context, tx *state.Tx, event state.Event) error { case EventDraftSnapshotCreated: var payload DraftSnapshotPayload if err := json.Unmarshal([]byte(event.PayloadJSON), &payload); err != nil { - return fmt.Errorf("decode event %s: %w", event.ID, err) + return fmt.Errorf("%w: decode draft snapshot event %s: %w", state.ErrEventVerification, event.ID, err) + } + // A blob ref that can never pass RecordDraftSnapshotTx's validation is + // the same malformed-payload class (review finding): validate HERE so + // it quarantines-as-consumed instead of surfacing as a raw store error + // that aborts the batch — or, worse, error-loops the pending replay + // once the project lands. + if !strings.HasPrefix(payload.BlobRef, "age_blob:") { + return fmt.Errorf("%w: draft snapshot blob ref %q must use age_blob: prefix", state.ErrEventVerification, payload.BlobRef) } // DRAFT-02: record the content-addressed blob reference against the // project. The blob content is fetched from the hub during sync and // extracted during materialization. pk, err := pathkey.Clean(payload.Path) if err != nil { - return fmt.Errorf("draft snapshot path %q: %w", payload.Path, err) + // A verified event whose payload can never apply (unsafe path) is + // the same malformed-payload class as a decode failure: quarantine + // as consumed instead of aborting the batch (#133). + return fmt.Errorf("%w: draft snapshot path %q: %w", state.ErrEventVerification, payload.Path, err) } project, err := tx.ProjectByPath(ctx, pk.Display) if err != nil { - return fmt.Errorf("draft snapshot for unknown project %q: %w", payload.Path, err) + // Mirror env.profile.updated: a winning delete drops the pointer, + // while a missing, non-tombstoned project is recoverable ordering + // and must be quarantined as consumed so the pull cursor advances. + if _, ok, terr := tx.TombstoneHLC(ctx, pk.Display); terr != nil { + return terr + } else if ok { + return nil + } + return fmt.Errorf("%w: %s", errDraftProjectPending, payload.Path) } return tx.RecordDraftSnapshotTx(ctx, project.ID, payload.BlobRef, payload.ByteSize, payload.FileCount, event) case EventEnvProfileUpdated: var payload EnvProfilePayload if err := json.Unmarshal([]byte(event.PayloadJSON), &payload); err != nil { - return fmt.Errorf("decode event %s: %w", event.ID, err) + // Same malformed-payload convention as the draft case above (#133): + // only an APPROVED signer can reach here (mustVerify), and a + // payload that can never decode must quarantine as consumed, not + // abort the pull batch. + return fmt.Errorf("%w: decode env profile event %s: %w", state.ErrEventVerification, event.ID, err) } pk, err := pathkey.Clean(payload.Path) if err != nil { - return fmt.Errorf("env profile path %q: %w", payload.Path, err) + return fmt.Errorf("%w: env profile path %q: %w", state.ErrEventVerification, payload.Path, err) } project, err := tx.ProjectByPath(ctx, pk.Display) if err != nil { @@ -946,16 +982,16 @@ func envCoordLess(hlc int64, deviceID, eventID string, event state.Event) bool { ) } -// ReplayPendingEnvProfileConflicts re-attempts every open env_pending_project -// quarantine (ENV-SYNC-01 review, Codex P2): a verified env.profile.updated -// whose project had not applied yet was preserved here instead of being -// consumed. Once the project lands — a later pull window, or a quarantined -// project.added replayed by `devices approve` — the stored event applies -// through the normal verified path and the conflict resolves. A still-missing -// project re-quarantines as a dedup no-op and the row stays open (visible, -// bounded). The conflict resolves only AFTER a successful apply, mirroring -// ReplayUndecryptableConflicts' resolve-after-apply rule. -func ReplayPendingEnvProfileConflicts(ctx context.Context, st *state.Store) (int, error) { +// ReplayPendingProjectConflicts re-attempts every open env_pending_project and +// draft_pending_project quarantine: a verified env.profile.updated or +// draft.snapshot.created whose project had not applied yet was preserved here +// instead of being consumed. Once the project lands — a later pull window, or a +// quarantined project.added replayed by `devices approve` — the stored event +// applies through the normal verified path and the conflict resolves. A +// still-missing project re-quarantines as a dedup no-op and the row stays open +// (visible, bounded). The conflict resolves only AFTER a successful apply, +// mirroring ReplayUndecryptableConflicts' resolve-after-apply rule. +func ReplayPendingProjectConflicts(ctx context.Context, st *state.Store) (int, error) { conflicts, err := st.OpenConflictsByType(ctx, ConflictEventVerification) if err != nil { return 0, err @@ -963,13 +999,13 @@ func ReplayPendingEnvProfileConflicts(ctx context.Context, st *state.Store) (int replayed := 0 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) @@ -979,16 +1015,20 @@ func ReplayPendingEnvProfileConflicts(ctx context.Context, st *state.Store) (int if stats.Quarantined > 0 { continue // project still missing (or a fresh verification failure); row stays open } - if err := st.ResolveConflict(ctx, c.ID, `{"action":"auto","reason":"project applied after env-pending hold (ENV-SYNC-01 replay)"}`); err != nil { + if err := st.ResolveConflict(ctx, c.ID, `{"action":"auto","reason":"project applied after pending-project hold replay"}`); err != nil { return replayed, err } - logging.Logger(ctx).Info("env-pending replay: quarantined env profile recovered", - "event_id", details.EventID, "device_id", details.DeviceID) + logging.Logger(ctx).Info("pending-project replay: quarantined pointer recovered", + "kind", details.Kind, "event_id", details.EventID, "device_id", details.DeviceID) replayed++ } return replayed, nil } +func isPendingProjectConflictKind(kind string) bool { + return kind == EventConflictKindEnvPendingProject || kind == EventConflictKindDraftPendingProject +} + // loadNamespaceProjection loads the projection slice Decide needs for a // namespace-convergence event (project.added/updated/deleted): the single row // (active or tombstoned) at the event's path, keyed by path_key. An absent path diff --git a/spec/07_NAMESPACE_AND_SYNC_MODEL.md b/spec/07_NAMESPACE_AND_SYNC_MODEL.md index 5777a0e..f295da6 100644 --- a/spec/07_NAMESPACE_AND_SYNC_MODEL.md +++ b/spec/07_NAMESPACE_AND_SYNC_MODEL.md @@ -719,7 +719,7 @@ Pinned by `internal/sync` grace tests (within-grace truncates, expired quarantin ### P6-SYNC-01 — Signature/trust failures in `ApplyEvents` no longer abort the whole batch -**Status.** Steps 1-2 are shipped: verification failures wrap `state.ErrEventVerification`, and `ApplyEvents` records `event_verification_failure` conflicts for signature/trust/content-hash failures and `ErrDivergentEvent`, then continues applying the rest of the batch. Quarantined events are counted as *consumed* for the cursor (a batch ending in one must not be re-delivered forever by the inclusive pull boundary). `insertEvent` verifies signature/trust **before** the prev-hash chain check — otherwise a revoked device's second chained event would surface as a transient `ErrEventHashChain` (its quarantined predecessor is never inserted) and permanently hold the cursor, reintroducing the wedge. Conflict details carry a machine-readable `kind` (`verification` vs `divergent`) plus the full marshaled `state.Event`, and dedup by event ID (the error string is volatile across trust-state changes). `devices approve` and `devices enroll --approve` replay matching `verification`-kind quarantined events and resolve those that now apply; a replayed `device.key.granted` additionally **ingests its WCK into the keyring** (post-#33 review, gpt-5.5) — `EncryptedHub.Pull` is the only other ingest path and it already advanced past the quarantined carrier, so without replay-time ingestion the granted `(epoch, kid)` would be permanently lost and every fleet event sealed under it would defer forever. `divergent`-kind rows are data-integrity disputes and are never auto-resolved by approval. The `device.revoked`/`device.lost` apply path is now shipped (TRUST-01) — `devices revoke`/`lost` emit the synced trust event and other devices flip the target on their next pull, so revoked traffic is rejected by synced trust state fleet-wide. Remaining: a still-pushing revoked device grows one open conflict row per distinct poisoned event (bounded aggregation is a follow-up). +**Status.** Steps 1-2 are shipped: verification failures wrap `state.ErrEventVerification`, and `ApplyEvents` records `event_verification_failure` conflicts for signature/trust/content-hash failures and `ErrDivergentEvent`, then continues applying the rest of the batch. Quarantined events are counted as *consumed* for the cursor (a batch ending in one must not be re-delivered forever by the inclusive pull boundary). `insertEvent` verifies signature/trust **before** the prev-hash chain check — otherwise a revoked device's second chained event would surface as a transient `ErrEventHashChain` (its quarantined predecessor is never inserted) and permanently hold the cursor, reintroducing the wedge. Conflict details carry a machine-readable `kind` (`verification`, `divergent`, `env_pending_project`, or `draft_pending_project`) plus the full marshaled `state.Event`, and dedup by event ID (the error string is volatile across trust-state changes). `env_pending_project` and `draft_pending_project` preserve verified pointer events whose project row is absent without a winning tombstone; they are cursor-consuming, replayable holds recovered by `ReplayPendingProjectConflicts` after pull apply and after device-approval replay. Because a pending-quarantined pointer is consumed for the cursor but never inserted into `events`, the origin device's NEXT chained event breaks on `validatePrevEventHash` and holds that device's cursor — a bounded, temporary hold, not a wedge: once the project lands, the replay inserts the pointer, the re-delivered successor applies, and its hash-chain conflict auto-resolves via the existing resolve-by-event-id path (Codex review; pinned by `TestApplyDraftSnapshotPendingChainSuccessorRecovers`). A signed-but-malformed pointer payload (JSON decode failure, unsafe path, or a blob ref that can never validate) wraps `state.ErrEventVerification` and quarantines-as-consumed instead of aborting the batch, on BOTH the env and draft planes. `devices approve` and `devices enroll --approve` replay matching `verification`-kind quarantined events and resolve those that now apply; a replayed `device.key.granted` additionally **ingests its WCK into the keyring** (post-#33 review, gpt-5.5) — `EncryptedHub.Pull` is the only other ingest path and it already advanced past the quarantined carrier, so without replay-time ingestion the granted `(epoch, kid)` would be permanently lost and every fleet event sealed under it would defer forever. `divergent`-kind rows are data-integrity disputes and are never auto-resolved by approval. The `device.revoked`/`device.lost` apply path is now shipped (TRUST-01) — `devices revoke`/`lost` emit the synced trust event and other devices flip the target on their next pull, so revoked traffic is rejected by synced trust state fleet-wide. Remaining: a still-pushing revoked device grows one open conflict row per distinct poisoned event (bounded aggregation is a follow-up). **Remaining actionable step.** 1. ~~Ship a real `device.revoked` apply path~~ — SHIPPED (TRUST-01, 2026-07-05): revoked events are rejected by synced trust state fleet-wide, not only by the verifier that made the local revoke decision. diff --git a/spec/09_SECRETS_AND_ENVIRONMENT.md b/spec/09_SECRETS_AND_ENVIRONMENT.md index 70fd630..48c9b2f 100644 --- a/spec/09_SECRETS_AND_ENVIRONMENT.md +++ b/spec/09_SECRETS_AND_ENVIRONMENT.md @@ -58,7 +58,7 @@ Env values are age-encrypted into content-addressed `age_blob:` blobs un - **Plane A — the signed, HLC-ordered event log** carries `env.profile.updated`: project path, profile name, provider/mode, encrypted `blob_ref` plus variable names for `devstrap_encrypted`, or provider refs for runtime-only profiles. The payload rides the `enc.v2` envelope, so var names and refs are not visible to the hub, and the event is in `mustVerifyEvent` because hydrated files and lifecycle-script environments are trust-affecting. - **Plane B — the content-addressed encrypted blob store** carries only the ciphertext. `devstrap sync` uploads any referenced local env blob the hub lacks and downloads referenced env blobs the local device lacks, keyed by SHA-256. Hydrate before the blob arrives preserves the file-not-found error class and adds the remedy `run 'devstrap sync' to pull it`. -Env profile replay is LWW by the profile row's source-event coordinate: the highest `(HLC, device_id, event_id)` wins, matching namespace reconciliation. Apply distinguishes a winning tombstone from an unapplied project (dual-review fix): a tombstoned path drops the pointer (a re-add + re-capture re-emits it), while an absent path without a tombstone quarantines the event as a replayable `env_pending_project` conflict — the cursor advances, the batch never aborts, and `ReplayPendingEnvProfileConflicts` (run after every pull apply and after `devices approve` replay) recovers the profile once the project lands. +Env profile replay is LWW by the profile row's source-event coordinate: the highest `(HLC, device_id, event_id)` wins, matching namespace reconciliation. Apply distinguishes a winning tombstone from an unapplied project (dual-review fix): a tombstoned path drops the pointer (a re-add + re-capture re-emits it), while an absent path without a tombstone quarantines the event as a replayable `env_pending_project` conflict — the cursor advances, the batch never aborts, and `ReplayPendingProjectConflicts` (run after every pull apply and after `devices approve` replay) recovers the profile once the project lands. Because blobs are encrypted client-side to the enrolled device recipient set before upload (see *Encryption*), the hub stays zero-knowledge: it sees only opaque event carriers and `age_blob:` ciphertext. Repo content never uses this path — it rides git's own blobless (`--filter=blob:none`) clone/fetch transport from each repo's existing remote — and `.git`, `node_modules`, and build artifacts are never placed in the blob store. diff --git a/spec/13_CLI_DAEMON_API.md b/spec/13_CLI_DAEMON_API.md index faf32c9..d77a12c 100644 --- a/spec/13_CLI_DAEMON_API.md +++ b/spec/13_CLI_DAEMON_API.md @@ -205,7 +205,7 @@ keys.rotate_max_age # config: age-triggered periodic WCK rotation deadline (P4 The file-backed test hub uses `--hub-file` (or `hub: file:`); the zero-infrastructure git carrier — the documented quickstart default since the `AD-1` swap (2026-07-04) — is selected via `hub: git+ssh://…` / `git+https://…` / `git+file://…` / scp-like `git@host:path.git` with optional `?branch=` (`GitCarrierHub` in `internal/hub`, local clone cache under `~/.devstrap/hub-git/`, hub id `git:`; the carrier design is canonical in `03_SYSTEM_ARCHITECTURE.md`); the local-folder / cloud-drive-folder carrier (`AD-1` final slice, 2026-07-05) is selected via `hub: folder:` (a Dropbox/iCloud/Drive folder or network mount; the path must be absolute and carries no `?`-parameters — `FolderHub` in `internal/hub`, hub id `folder:`, per-device lock + observation cache under `~/.devstrap/hub-folder//` while only ciphertext objects live in the shared folder; `hub init` remains git-only, so the folder scheme is set in `config.yaml`/`DEVSTRAP_HUB` directly); the R2/S3 scale-up backend is selected via `hub: r2://` (or `s3://`). Git-carrier auth is the user's existing git credentials, running non-interactively (a missing/denied key fails fast with the auth exit class and git's own stderr instead of prompting, followed by a second stderr line `hint: git authentication failed — check ssh key / repo access (load your key: ssh-add ~/.ssh/)` — the single error sink prints it for every auth-class failure, including ones wrapped in an app exit code, shipped 2026-07-05); R2/S3 credentials resolve most-explicit-first (`P6-HUB-02`): `DEVSTRAP_HUB_S3_ACCESS_KEY_ID`/`DEVSTRAP_HUB_S3_SECRET_ACCESS_KEY` env/config — where either value may be a 1Password `op://` reference resolved via `op read` at sync time — then `AWS_ACCESS_KEY_ID`/`AWS_SECRET_ACCESS_KEY` literals, then the per-workspace OS-keychain slot written by `devstrap hub login` (0600 file fallback under `DEVSTRAP_NO_KEYCHAIN`); `hub_s3_endpoint` and `hub_s3_region` (default `auto`) stay env/config. Plaintext env remains the CI/override fallback; the keychain/op:// path is the recommended custody on developer machines. Both backends push local events past the push cursor, pull hub events from the pull cursor, apply namespace events idempotently, and support `--namespace-only` and `--dry-run`. -Shipped (`EAGER-*`/`HUB-*`, audit `docs/audits/AUDIT_RECOMMENDATIONS_2026-06-28.md`): `sync` is the materialization entrypoint. A single `devstrap sync` eagerly blobless/partial-clones every mapped repo (`git clone --filter=blob:none`) from its existing remote, hydrates env profiles, extracts draft bundles, and (opt-in via `DEVSTRAP_REBUILD_DEPS`) rebuilds `node_modules`/build artifacts rather than syncing them — the rebuild runs BEFORE env hydrate (`P6-GIT-03`: lifecycle scripts are arbitrary repo-controlled code and must never execute with a freshly decrypted `.env` on disk) and tees its output to a `0600` log under `~/.devstrap/logs/rebuilds/.log`, named in the failure message. The hub pull is cursor-based (per-origin-device Seq cursors via `hub_device_cursors`, per-device contiguous-run safe cursor `SYNC-01`/`P5-SYNC-01` — an exact boundary, the `HUB-13` overlap is retired; per-device retention floor `410 -> snapshot`), and the command prints a real materialize summary. `materialize` returns non-zero when any project fails (`ErrPartialMaterialize`, `QUAL-03`) while still completing the batch, so CI/cron gates and `&&` chains detect partial failure. Repo content always rides git's own transport and never traverses the hub; only the signed namespace map (event log) and ciphertext blobs do. `--namespace-only` opts out of materialization. Per `P6-SEC-02`, `sync` now **pulls before it pushes** and runs the push behind a founder/join gate: a founder's first sync to an empty hub mints the workspace key (epoch 1) then pushes; a device that has no key and sees a non-empty hub (a joiner awaiting approval) DEFERS the push and prints `Awaiting workspace key grant: N local event(s) queued …`, leaving its events queued (push cursor unadvanced) until it is approved and ingests the fleet key on a later cycle. `--namespace-only` output reports the deferred count when the push is held. Per `P6-SEC-03`, the pull-side wait for a missing workspace key is **grace-bounded**: within `sync.key_grant_grace` the pull defers (truncates) at the first event it cannot decrypt, and past it those events are quarantined as recoverable `undecryptable` conflicts so the cursor advances and sync is never wedged forever; the quarantined carriers replay automatically once the grant arrives (see `07_NAMESPACE_AND_SYNC_MODEL.md`). Per `P6-SYNC-02`, the same window grace-bounds an **unknown envelope version** (a newer client's events defer per origin device until this binary upgrades, then quarantine), malformed envelopes forward straight to that quarantine, and retired-v1/anti-downgrade drops leave durable `sync_skipped_events` records — surfaced as `status` "Skipped hub events: N" and a graded `doctor` "skipped hub events" check with per-reason remedies, and a `hub gc` sweep refusal while any record is open. Records clear automatically when their event finally applies; there is deliberately no `sync --replay-skipped` flag (held classes retry at the per-device seq gap; quarantined classes ride the existing replay). Per `P4-SYNC-06`, after a **fully-clean** cycle (push not deferred; no truncated/skipped/undecryptable pull; no quarantined/cursor-held apply; no open `sync_skipped_events`) `sync` best-effort publishes a signed **ack marker** to `meta/acks/.json` recording the consumed transport cursor, push watermark, and current HLC — the tombstone-safety clock a compactor mins over. Writing never fails the sync (a `PutAck` error only delays a compactor's tombstone GC), and an unchanged cycle skips the redundant write. The hub is resolved through one selection seam (`hubFromOptions`, `P5-HUB-01`/`ARCH-03`): `--hub-file` (or a `hub: file:` config value) selects the file-backed test backend, and `hub: r2://` (or `s3://`) selects the Cloudflare R2 / S3 zero-knowledge backend — the production `aws-sdk-go-v2` S3 adapter (`internal/hub`, with `NopRetryer` so `R2Hub.Retry` is the single retry layer) is wired in, its keying/retry/conditional-put/`ListBlobs`/retention-floor logic is unit-tested, and the same conformance contract is proven against MinIO via an env-gated integration test. No FUSE/placeholder/lazy-VFS layer is part of this design — StrapFS stays deferred. Per `P4-SYNC-02`, when a device's pull cursor has fallen below the hub's retention floor the pull returns `ErrSnapshotRequired`; `sync` no longer dead-ends on it — it prints `Recovering from hub snapshot (retention floor passed our cursor)…` and runs one full-state snapshot exchange (get + fail-closed-verify the signed retention manifest → pull the tail so an in-batch grant is ingested → fetch + sha-check + unseal → import → advance cursors → pull imported draft blobs), then re-runs the incremental pull, which now succeeds. A **trust refusal** (the snapshot producer is not a locally approved device, a bad signature, an object sha256 mismatch, or an AEAD failure on every held key) exits `invalid-config` (2) with a pin/enroll remedy, distinct from a hub/fetch failure, which exits `network` (8). A **keyless joiner** (the snapshot is sealed under an epoch this device does not hold yet) prints the awaiting-grant defer and exits 0 — the next sync retries once the grant lands, importing nothing in the meantime. The hub is resolved through one selection seam (`hubFromOptions`, `P5-HUB-01`/`ARCH-03`): `--hub-file` (or a `hub: file:` config value) selects the file-backed test backend, and `hub: r2://` (or `s3://`) selects the Cloudflare R2 / S3 zero-knowledge backend — the production `aws-sdk-go-v2` S3 adapter (`internal/hub`, with `NopRetryer` so `R2Hub.Retry` is the single retry layer) is wired in, its keying/retry/conditional-put/`ListBlobs`/retention-floor logic is unit-tested, and the same conformance contract is proven against MinIO via an env-gated integration test. No FUSE/placeholder/lazy-VFS layer is part of this design — StrapFS stays deferred. +Shipped (`EAGER-*`/`HUB-*`, audit `docs/audits/AUDIT_RECOMMENDATIONS_2026-06-28.md`): `sync` is the materialization entrypoint. A single `devstrap sync` eagerly blobless/partial-clones every mapped repo (`git clone --filter=blob:none`) from its existing remote, hydrates env profiles, extracts draft bundles, and (opt-in via `DEVSTRAP_REBUILD_DEPS`) rebuilds `node_modules`/build artifacts rather than syncing them — the rebuild runs BEFORE env hydrate (`P6-GIT-03`: lifecycle scripts are arbitrary repo-controlled code and must never execute with a freshly decrypted `.env` on disk) and tees its output to a `0600` log under `~/.devstrap/logs/rebuilds/.log`, named in the failure message. The hub pull is cursor-based (per-origin-device Seq cursors via `hub_device_cursors`, per-device contiguous-run safe cursor `SYNC-01`/`P5-SYNC-01` — an exact boundary, the `HUB-13` overlap is retired; per-device retention floor `410 -> snapshot`), and the command prints a real materialize summary. `materialize` returns non-zero when any project fails (`ErrPartialMaterialize`, `QUAL-03`) while still completing the batch, so CI/cron gates and `&&` chains detect partial failure. Repo content always rides git's own transport and never traverses the hub; only the signed namespace map (event log) and ciphertext blobs do. `--namespace-only` opts out of materialization. Per `P6-SEC-02`, `sync` now **pulls before it pushes** and runs the push behind a founder/join gate: a founder's first sync to an empty hub mints the workspace key (epoch 1) then pushes; a device that has no key and sees a non-empty hub (a joiner awaiting approval) DEFERS the push and prints `Awaiting workspace key grant: N local event(s) queued …`, leaving its events queued (push cursor unadvanced) until it is approved and ingests the fleet key on a later cycle. `--namespace-only` output reports the deferred count when the push is held. Per `P6-SEC-03`, the pull-side wait for a missing workspace key is **grace-bounded**: within `sync.key_grant_grace` the pull defers (truncates) at the first event it cannot decrypt, and past it those events are quarantined as recoverable `undecryptable` conflicts so the cursor advances and sync is never wedged forever; the quarantined carriers replay automatically once the grant arrives (see `07_NAMESPACE_AND_SYNC_MODEL.md`). Per `P6-SYNC-02`, the same window grace-bounds an **unknown envelope version** (a newer client's events defer per origin device until this binary upgrades, then quarantine), malformed envelopes forward straight to that quarantine, and retired-v1/anti-downgrade drops leave durable `sync_skipped_events` records — surfaced as `status` "Skipped hub events: N" and a graded `doctor` "skipped hub events" check with per-reason remedies, and a `hub gc` sweep refusal while any record is open. Records clear automatically when their event finally applies; there is deliberately no `sync --replay-skipped` flag (held classes retry at the per-device seq gap; quarantined classes ride the existing replay). After every pull apply, `sync` also replays open **pending-project pointer** quarantines (`env_pending_project`/`draft_pending_project` — a verified `env.profile.updated` or `draft.snapshot.created` that arrived before its project, issue #133) via `ReplayPendingProjectConflicts`, so a pointer recovered in the same cycle its `project.added` applies; `devices approve` runs the same replay after re-applying a newly-approved device's quarantined events. Per `P4-SYNC-06`, after a **fully-clean** cycle (push not deferred; no truncated/skipped/undecryptable pull; no quarantined/cursor-held apply; no open `sync_skipped_events`) `sync` best-effort publishes a signed **ack marker** to `meta/acks/.json` recording the consumed transport cursor, push watermark, and current HLC — the tombstone-safety clock a compactor mins over. Writing never fails the sync (a `PutAck` error only delays a compactor's tombstone GC), and an unchanged cycle skips the redundant write. The hub is resolved through one selection seam (`hubFromOptions`, `P5-HUB-01`/`ARCH-03`): `--hub-file` (or a `hub: file:` config value) selects the file-backed test backend, and `hub: r2://` (or `s3://`) selects the Cloudflare R2 / S3 zero-knowledge backend — the production `aws-sdk-go-v2` S3 adapter (`internal/hub`, with `NopRetryer` so `R2Hub.Retry` is the single retry layer) is wired in, its keying/retry/conditional-put/`ListBlobs`/retention-floor logic is unit-tested, and the same conformance contract is proven against MinIO via an env-gated integration test. No FUSE/placeholder/lazy-VFS layer is part of this design — StrapFS stays deferred. Per `P4-SYNC-02`, when a device's pull cursor has fallen below the hub's retention floor the pull returns `ErrSnapshotRequired`; `sync` no longer dead-ends on it — it prints `Recovering from hub snapshot (retention floor passed our cursor)…` and runs one full-state snapshot exchange (get + fail-closed-verify the signed retention manifest → pull the tail so an in-batch grant is ingested → fetch + sha-check + unseal → import → advance cursors → pull imported draft blobs), then re-runs the incremental pull, which now succeeds. A **trust refusal** (the snapshot producer is not a locally approved device, a bad signature, an object sha256 mismatch, or an AEAD failure on every held key) exits `invalid-config` (2) with a pin/enroll remedy, distinct from a hub/fetch failure, which exits `network` (8). A **keyless joiner** (the snapshot is sealed under an epoch this device does not hold yet) prints the awaiting-grant defer and exits 0 — the next sync retries once the grant lands, importing nothing in the meantime. The hub is resolved through one selection seam (`hubFromOptions`, `P5-HUB-01`/`ARCH-03`): `--hub-file` (or a `hub: file:` config value) selects the file-backed test backend, and `hub: r2://` (or `s3://`) selects the Cloudflare R2 / S3 zero-knowledge backend — the production `aws-sdk-go-v2` S3 adapter (`internal/hub`, with `NopRetryer` so `R2Hub.Retry` is the single retry layer) is wired in, its keying/retry/conditional-put/`ListBlobs`/retention-floor logic is unit-tested, and the same conformance contract is proven against MinIO via an env-gated integration test. No FUSE/placeholder/lazy-VFS layer is part of this design — StrapFS stays deferred. ### hub diff --git a/spec/16_TEST_PLAN.md b/spec/16_TEST_PLAN.md index 4fa7fbb..1024b9d 100644 --- a/spec/16_TEST_PLAN.md +++ b/spec/16_TEST_PLAN.md @@ -45,7 +45,7 @@ The Phase 0 suite must cover: - repo operation locks reject active concurrent operations and reclaim stale same-host owners before hydrate/worktree mutation; - fresh worktree creation from an advanced remote SHA while local clone state is stale, collision-resistant worktree branch naming with retry, `worktree status` reporting stale after the remote base advances again, `worktree finalize` refusing stale bases unless `--allow-stale-base`, LFS-policy warning/pull branching for agent worktrees, and cleanup of DB-invisible checkouts plus `agent/...` branches when post-add LFS or SQLite insert failures occur; - forge detection/routing for `agent pr` across GitHub/GitLab/Gitea/Forgejo/Bitbucket/Azure-style remotes, forge-specific token env allowlists, Azure remote-key folding, hermetic SSH host-alias resolution tests using a PATH-shimmed `ssh -G`, and graceful unknown-forge compare-URL fallback; -- HLC monotonic send/receive, max-skew rejection, logical-counter overflow behavior, persisted local event HLC/sequence stamping and previous-hash linking across reopen, transactional idempotent event apply, divergent duplicate event quarantine, incoming `prev_event_hash` chain-break rejection with conflict recording, per-event `event_verification_failure` quarantine (a revoked device's event mid-batch: valid neighbors still apply, the safe cursor advances past it, exactly one deduped conflict row survives repeated pulls; `errors.Is` sentinel coverage for every `verifyEventSignature` failure path while infrastructure errors stay non-matching; approve-time replay applies the quarantined event and resolves its conflict; e2e `sync_revoked_quarantine.txtar` proves a revoked device's push cannot wedge other devices — `P6-SYNC-01`), grant-carrier verification before WCK ingestion (`EncryptedHub.Pull` refuses/ingests/nil-verifier back-compat; `VerifyRemoteEvent` matches the `insertEvent` trust regime across {local, approved+valid, forged sig, revoked, unknown} × {enrolled, not}; and the malicious-hub acceptance test `TestSyncRejectsForgedGrantBeforeWCKIngest` — a forged grant wrapping an attacker WCK to the victim's own recipient at epoch 2^40 leaves `CurrentKeyEpoch`, the key store, and the keyring untouched and lands as one quarantine conflict — `P6-SEC-01`), founder/join workspace-key bootstrap (`init` writes `role: founder`/`joiner` and mints no key; `init --join` prints approval-first next steps; e2e `sync_join_flow.txtar` proves a `--join` device that adds a project and syncs BEFORE approval defers its push — `pushed 0`, `Awaiting workspace key grant`, nothing leaked to the hub — and after approval its pre-approval project pushes and materializes on the founder, hub ciphertext throughout — `P6-SEC-02`), HLC-gated delete tombstone restore/ignore behavior, order-independent same-path/different-remote conflict protection with stable conflict details, and origin-atomic draft-snapshot recording (`TestInsertLocalEventTxMatchesInsertLocalEvent` pins `InsertLocalEventTx` parity with `InsertLocalEvent`; `TestDraftSnapshotCreateRecordsOriginSnapshotRow` and `TestRewrapDraftBlobRecordsOriginSupersedingSnapshot` prove `draft snapshot create` and the revoke rewrap write the origin's `draft_snapshots` row in one transaction with the event; e2e `draft_snapshot_gc_retains_origin.txtar` proves the origin's bundle blob survives `sync` + `hub gc` — `P6-DATA-01`), and sticky fail-closed enrollment (`TestHasEnrolledDevicesStickyAfterRevoke` pins the predicate — pending placeholders don't close the window, approved does, revoked/lost keep it closed; `TestApplyEventsRevokedLastDeviceStaysFailClosed` proves that with only a revoked device on record a validly-signed event from it, an unknown-device event, a signed pending-device event, and an unsigned no-key-device event all quarantine instead of applying — `P6-SYNC-03`). +- HLC monotonic send/receive, max-skew rejection, logical-counter overflow behavior, persisted local event HLC/sequence stamping and previous-hash linking across reopen, transactional idempotent event apply, divergent duplicate event quarantine, incoming `prev_event_hash` chain-break rejection with conflict recording, per-event `event_verification_failure` quarantine (a revoked device's event mid-batch: valid neighbors still apply, the safe cursor advances past it, exactly one deduped conflict row survives repeated pulls; `errors.Is` sentinel coverage for every `verifyEventSignature` failure path while infrastructure errors stay non-matching; approve-time replay applies the quarantined event and resolves its conflict; e2e `sync_revoked_quarantine.txtar` proves a revoked device's push cannot wedge other devices — `P6-SYNC-01`), grant-carrier verification before WCK ingestion (`EncryptedHub.Pull` refuses/ingests/nil-verifier back-compat; `VerifyRemoteEvent` matches the `insertEvent` trust regime across {local, approved+valid, forged sig, revoked, unknown} × {enrolled, not}; and the malicious-hub acceptance test `TestSyncRejectsForgedGrantBeforeWCKIngest` — a forged grant wrapping an attacker WCK to the victim's own recipient at epoch 2^40 leaves `CurrentKeyEpoch`, the key store, and the keyring untouched and lands as one quarantine conflict — `P6-SEC-01`), founder/join workspace-key bootstrap (`init` writes `role: founder`/`joiner` and mints no key; `init --join` prints approval-first next steps; e2e `sync_join_flow.txtar` proves a `--join` device that adds a project and syncs BEFORE approval defers its push — `pushed 0`, `Awaiting workspace key grant`, nothing leaked to the hub — and after approval its pre-approval project pushes and materializes on the founder, hub ciphertext throughout — `P6-SEC-02`), HLC-gated delete tombstone restore/ignore behavior, order-independent same-path/different-remote conflict protection with stable conflict details, and origin-atomic draft-snapshot recording (`TestInsertLocalEventTxMatchesInsertLocalEvent` pins `InsertLocalEventTx` parity with `InsertLocalEvent`; `TestDraftSnapshotCreateRecordsOriginSnapshotRow` and `TestRewrapDraftBlobRecordsOriginSupersedingSnapshot` prove `draft snapshot create` and the revoke rewrap write the origin's `draft_snapshots` row in one transaction with the event; `TestApplyDraftSnapshotUnknownProjectQuarantinesWithoutAbort`, `TestApplyDraftSnapshotTombstonedProjectDrops`, `TestApplyDraftSnapshotMalformedPayloadQuarantinesWithoutAbort`, and `TestReplayPendingDraftSnapshotConflictRecovers` pin the draft pending-project quarantine/replay path; e2e `draft_snapshot_gc_retains_origin.txtar` proves the origin's bundle blob survives `sync` + `hub gc` — `P6-DATA-01`), and sticky fail-closed enrollment (`TestHasEnrolledDevicesStickyAfterRevoke` pins the predicate — pending placeholders don't close the window, approved does, revoked/lost keep it closed; `TestApplyEventsRevokedLastDeviceStaysFailClosed` proves that with only a revoked device on record a validly-signed event from it, an unknown-device event, a signed pending-device event, and an unsigned no-key-device event all quarantine instead of applying — `P6-SYNC-03`). Grace-bounded missing-key quarantine (`P6-SEC-03`): `internal/sync/encryptedhub_test.go` grace cases (within-grace still truncates and records the sighting through the `MissingKeyWait` seam; expired grace forwards the carrier for quarantine at BOTH truncate sites — missing epoch and unheld kid at a held epoch — while later held-epoch events in the batch still decrypt; a nil seam keeps the legacy truncate-forever contract), `internal/state/key_grant_waits_test.go` (stable first-seen across re-sightings, kid-churn shares the epoch clock so hostile relabeling cannot restart the window, `RecordKeyEpoch` clears satisfied waits — epoch-level on any key, kid-specific only on the matching kid), `internal/cli/sync_never_granted_epoch_test.go` (`TestSyncQuarantinesNeverGrantedEpochThenRecovers`: full pull/apply cycle — quarantine advances the cursor and opens a wait, a later verified grant recovers the carrier IN THE SAME CYCLE because the replay runs before the batch applies, conflict auto-resolves, wait clears), `internal/cli/devices_epoch_guard_test.go` (held-epoch gap refusal with no DB write, open-wait refusal, keyless pass-through, `--allow-epoch-gap` override on both `approve` and `enroll --approve`), and e2e `sync_never_granted_epoch_wedge.txtar` (three-device fleet: a revoke-triggered rotation on the founder never grants epoch 2 to a device it does not know; at `key_grant_grace=0` that device quarantines instead of wedging, `doctor` warns `awaiting key grants`, the contiguity guard refuses its approvals until `--allow-epoch-gap`, and a re-approve from a complete device recovers the quarantined project end-to-end). @@ -280,7 +280,7 @@ This is the most important test. ```text 1. state: UpsertEnvProfileTx stamps source-event coordinates, switches encrypted/provider shapes, legacy wrappers leave NULL coords, and EnvProfilesForBlobRef returns affected profiles/vars -2. sync: apply env.profile.updated creates bindings, duplicates are idempotent, a tombstoned project drops the pointer without quarantine, an absent project quarantines as env_pending_project without aborting the batch and replays to recovery once the project applies, LWW converges in both delivery orders, and rewrapHubCleanup uploads the new blob before pushing the superseding event +2. sync: apply env.profile.updated creates bindings, duplicates are idempotent, a tombstoned project drops the pointer without quarantine, an absent project quarantines as env_pending_project without aborting the batch and replays to recovery once the project applies, draft.snapshot.created mirrors the same tombstone/drop and absent-project quarantine behavior as draft_pending_project (including malformed-payload verification quarantine), LWW converges in both delivery orders, and rewrapHubCleanup uploads the new blob before pushing the superseding event 3. cli: blobRefFromEvent extracts env blob refs for encrypted profiles but not provider profiles 4. txtar: env_exchange.txtar enrolls two homes, syncs through --hub-file, and hydrates captured values on the second device with no conflicts 5. `TestSnapshotRoundTripsEnvProfile` proves BuildSnapshot -> ImportSnapshot carries the env pointer into a fresh store idempotently; `TestImportSnapshotEnvLWW` proves the pointer merges by its own coordinate (an older pointer never regresses, a newer pointer wins even on a losing entry row) diff --git a/spec/18_WORK_LOG.md b/spec/18_WORK_LOG.md index 6cf41ff..7d7020a 100644 --- a/spec/18_WORK_LOG.md +++ b/spec/18_WORK_LOG.md @@ -31,6 +31,21 @@ Follow-ups: Entries are newest-first: each code-modifying cycle prepends ONE dated entry at the top. +## 2026-07-06 — fix(sync): draft-snapshot apply quarantines instead of aborting the pull batch (#133) + +Changed: +- `internal/sync/events.go`: the `draft.snapshot.created` apply case mirrors the env pointer shape (issue #133) — a winning tombstone drops the pointer; an absent, non-tombstoned project returns the new `errDraftProjectPending` sentinel, quarantined by the batch loop as a cursor-consuming, replayable `draft_pending_project` conflict (new kind beside `env_pending_project`, shared `insertPendingProjectConflict`). The env replay generalizes to `ReplayPendingProjectConflicts` covering both kinds (call sites: post-pull apply in `internal/cli/sync.go`, approve-time replay in `internal/cli/devices.go`); draft re-apply re-runs `RecordDraftSnapshotTx` through the normal verified path and resolves the conflict only after a successful apply. +- Malformed-payload convention (the issue's same-class residual, both planes): a signed-but-malformed **draft or env** payload — JSON decode failure, an unsafe `pathkey.Clean` path, or (opus review) a blob ref that can never pass `RecordDraftSnapshotTx`'s `age_blob:` validation — now wraps `state.ErrEventVerification` at the APPLY layer, so it quarantines-as-consumed instead of aborting the batch or error-looping the pending replay once the project lands (only an APPROVED signer can reach these branches; mirrors the PR #132 trust-payload convention). The env decode/path wraps and the blob-ref validation were coordinator review additions on top of the delegated draft-side fix. +- Post-review (Codex, dual-review): confirmed-and-pinned rather than fixed — a pending-quarantined pointer is consumed for the cursor but never inserted into `events`, so the origin device's next CHAINED event breaks on `validatePrevEventHash` and holds that device's cursor. Verified this is a bounded temporary hold with existing recovery, not a wedge (the same shape the shipped env/undecryptable designs carry): the real CLI can never emit a draft before its own project.added (draft creation requires a local project), so the pending case is cross-device — and once the project lands, the replay inserts the pointer, the re-delivered successor applies, and its `event_hash_chain_break` conflict auto-resolves through the P6-SEC-03 resolve-by-event-id path. Pinned end-to-end by `TestApplyDraftSnapshotPendingChainSuccessorRecovers`; documented in spec/07. +- Specs: spec/07 (P6-SYNC-01 status: conflict-kind list, `ReplayPendingProjectConflicts`, the chain-hold note), spec/09 (renamed replay), spec/13 (sync pending-project replay), spec/16 (test inventory). + +Validated: +- New: `TestApplyDraftSnapshotUnknownProjectQuarantinesWithoutAbort` (batch continues, cursor advances, `draft_pending_project` row), `TestApplyDraftSnapshotTombstonedProjectDrops`, `TestApplyDraftSnapshotMalformedPayloadQuarantinesWithoutAbort`, `TestApplyDraftSnapshotBadBlobRefQuarantinesWithoutAbort`, `TestApplyEnvProfileMalformedPayloadQuarantinesWithoutAbort`, `TestReplayPendingDraftSnapshotConflictRecovers`, `TestApplyDraftSnapshotPendingChainSuccessorRecovers`. +- gofmt clean; `go test -race ./...` all ok; golangci-lint; spec-drift vs origin/main. + +Follow-ups: +- None (closes #133). + ## 2026-07-05 — docs(spec/19): §F.3 multi-device completeness dogfood runbook Changed: