Skip to content

Commit 6175d73

Browse files
committed
fix(data): close project governance consistency gaps
Filtered watch pushes must stay bounded by the changed batch, accepted reclassification previews must describe the exact write they authorize, and derived mirrors must retain the machine identity used by mapping rules. Without those guarantees, large archives regress over time and project governance can either apply unseen changes or report the wrong result across machines.
1 parent 9d32b50 commit 6175d73

9 files changed

Lines changed: 294 additions & 74 deletions

File tree

internal/db/worktree_reclassification.go

Lines changed: 65 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ import (
1414

1515
const worktreeReclassificationSampleLimit = 10
1616

17-
var ErrWorktreeMappingSetChanged = errors.New("worktree mapping set changed")
17+
var ErrWorktreeMappingSetChanged = errors.New(
18+
"worktree reclassification preview changed",
19+
)
1820

1921
type WorktreeReclassificationDraft struct {
2022
Machine string `json:"machine"`
@@ -49,10 +51,11 @@ type WorktreeReclassificationPreview struct {
4951
}
5052

5153
type worktreeReclassificationEvaluation struct {
52-
matched int
53-
updates []worktreeMappingSessionUpdate
54-
projects map[string]int
55-
sessions []WorktreeReclassificationSessionSample
54+
matched int
55+
updates []worktreeMappingSessionUpdate
56+
projects map[string]int
57+
sessions []WorktreeReclassificationSessionSample
58+
impactToken string
5659
}
5760

5861
func (db *DB) PreviewWorktreeReclassification(
@@ -120,8 +123,18 @@ func (db *DB) ApplyWorktreeReclassification(
120123
if err != nil {
121124
return WorktreeProjectMapping{}, WorktreeReclassificationPreview{}, err
122125
}
123-
currentToken := worktreeMappingSetToken(stored)
124126
collision := exactWorktreeMapping(stored, normalized.PathPrefix)
127+
effective := overlayWorktreeMapping(stored, normalized, collision)
128+
evaluation, err := evaluateWorktreeMappingsTx(
129+
ctx, tx, normalized.Machine, enabledWorktreeMappings(effective),
130+
&normalized, "",
131+
)
132+
if err != nil {
133+
return WorktreeProjectMapping{}, WorktreeReclassificationPreview{}, err
134+
}
135+
currentToken := worktreeReclassificationToken(
136+
stored, normalized, collision, evaluation,
137+
)
125138
if acceptedToken == "" || acceptedToken != currentToken ||
126139
!sameOptionalMappingID(existingMappingID, mappingIDPointer(collision)) {
127140
return WorktreeProjectMapping{}, WorktreeReclassificationPreview{},
@@ -134,18 +147,6 @@ func (db *DB) ApplyWorktreeReclassification(
134147
if err != nil {
135148
return WorktreeProjectMapping{}, WorktreeReclassificationPreview{}, err
136149
}
137-
active, err := loadActiveWorktreeMappingsTx(ctx, tx, normalized.Machine)
138-
if err != nil {
139-
return WorktreeProjectMapping{}, WorktreeReclassificationPreview{}, fmt.Errorf(
140-
"loading active worktree mappings: %w", err,
141-
)
142-
}
143-
evaluation, err := evaluateWorktreeMappingsTx(
144-
ctx, tx, normalized.Machine, active, &mapping, "",
145-
)
146-
if err != nil {
147-
return WorktreeProjectMapping{}, WorktreeReclassificationPreview{}, err
148-
}
149150
affected := map[string]struct{}{mapping.Project: {}}
150151
updated := 0
151152
for _, update := range evaluation.updates {
@@ -171,7 +172,7 @@ func (db *DB) ApplyWorktreeReclassification(
171172
}
172173

173174
preview := worktreeReclassificationPreviewFromEvaluation(
174-
worktreeMappingSetTokenWithReplacement(stored, mapping),
175+
acceptedToken,
175176
mapping.Project, mappingIDPointer(&mapping), evaluation,
176177
)
177178
preview.UpdatedSessions = updated
@@ -210,7 +211,8 @@ func previewWorktreeReclassificationTx(
210211
return WorktreeReclassificationPreview{}, err
211212
}
212213
return worktreeReclassificationPreviewFromEvaluation(
213-
worktreeMappingSetToken(stored), draft.Project,
214+
worktreeReclassificationToken(stored, draft, collision, evaluation),
215+
draft.Project,
214216
mappingIDPointer(collision), evaluation,
215217
), nil
216218
}
@@ -353,6 +355,8 @@ func evaluateWorktreeMappingsTx(
353355
},
354356
)
355357

358+
impactHash := sha256.New()
359+
writeWorktreeTokenFields(impactHash, "impact-v1")
356360
evaluation := worktreeReclassificationEvaluation{projects: map[string]int{}}
357361
for _, row := range sessions {
358362
if sessionID != "" && row.id != sessionID {
@@ -375,6 +379,18 @@ func evaluateWorktreeMappingsTx(
375379
continue
376380
}
377381
evaluation.matched++
382+
matchCwd := row.matchCwd
383+
if matchCwd == "" {
384+
matchCwd = row.cwd
385+
}
386+
nextProject := row.project
387+
if shouldUpdate {
388+
nextProject = update.nextProject
389+
}
390+
writeWorktreeTokenFields(
391+
impactHash, row.id, row.project, nextProject, row.cwd,
392+
matchCwd, row.filePath,
393+
)
378394
if !shouldUpdate {
379395
continue
380396
}
@@ -386,6 +402,7 @@ func evaluateWorktreeMappingsTx(
386402
NextProject: update.nextProject, Cwd: update.cwd,
387403
})
388404
}
405+
evaluation.impactToken = hex.EncodeToString(impactHash.Sum(nil))
389406
return evaluation, nil
390407
}
391408

@@ -428,22 +445,41 @@ func worktreeMappingSetToken(mappings []WorktreeProjectMapping) string {
428445
fields := []string{
429446
strconv.FormatInt(mapping.ID, 10), mapping.Machine,
430447
normalizedMappingPath(mapping.PathPrefix), mapping.Layout,
431-
mapping.Project, strconv.FormatBool(mapping.Enabled), mapping.UpdatedAt,
432-
}
433-
for _, field := range fields {
434-
_, _ = fmt.Fprintf(hash, "%d:%s", len(field), field)
448+
mapping.Project, mapping.OriginalProject,
449+
strconv.FormatBool(mapping.Enabled), mapping.UpdatedAt,
435450
}
451+
writeWorktreeTokenFields(hash, fields...)
436452
}
437453
return hex.EncodeToString(hash.Sum(nil))
438454
}
439455

440-
func worktreeMappingSetTokenWithReplacement(
456+
func worktreeReclassificationToken(
441457
stored []WorktreeProjectMapping,
442-
mapping WorktreeProjectMapping,
458+
draft WorktreeProjectMapping,
459+
collision *WorktreeProjectMapping,
460+
evaluation worktreeReclassificationEvaluation,
443461
) string {
444-
return worktreeMappingSetToken(overlayWorktreeMapping(
445-
stored, mapping, exactWorktreeMapping(stored, mapping.PathPrefix),
446-
))
462+
collisionID := ""
463+
if collision != nil {
464+
collisionID = strconv.FormatInt(collision.ID, 10)
465+
}
466+
hash := sha256.New()
467+
writeWorktreeTokenFields(
468+
hash, "reclassification-v2", worktreeMappingSetToken(stored),
469+
draft.Machine, normalizedMappingPath(draft.PathPrefix), draft.Layout,
470+
draft.Project, draft.OriginalProject, strconv.FormatBool(draft.Enabled),
471+
collisionID, evaluation.impactToken,
472+
)
473+
return hex.EncodeToString(hash.Sum(nil))
474+
}
475+
476+
func writeWorktreeTokenFields(
477+
hash interface{ Write([]byte) (int, error) },
478+
fields ...string,
479+
) {
480+
for _, field := range fields {
481+
_, _ = fmt.Fprintf(hash, "%d:%s", len(field), field)
482+
}
447483
}
448484

449485
func mappingIDPointer(mapping *WorktreeProjectMapping) *int64 {

internal/db/worktree_reclassification_test.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func TestWorktreeReclassificationPreviewHonorsSpecificRuleAndBoundsSamples(t *te
6868
"the specific mapping must remain authoritative")
6969
}
7070

71-
func TestWorktreeReclassificationTokenTracksMappingsButNotSessions(t *testing.T) {
71+
func TestWorktreeReclassificationTokenBindsDraftAndAffectedSessions(t *testing.T) {
7272
d := testDB(t)
7373
ctx := context.Background()
7474
seedReclassificationSession(t, d, "one", "archive.example", "/worktrees/service/one", "branch")
@@ -79,14 +79,29 @@ func TestWorktreeReclassificationTokenTracksMappingsButNotSessions(t *testing.T)
7979

8080
preview, err := d.PreviewWorktreeReclassification(ctx, draft)
8181
require.NoError(t, err)
82+
changedDraft := draft
83+
changedDraft.Project = "different-service"
84+
_, _, err = d.ApplyWorktreeReclassification(
85+
ctx, changedDraft, preview.MappingToken, preview.ExistingMappingID,
86+
)
87+
require.ErrorIs(t, err, ErrWorktreeMappingSetChanged,
88+
"a preview for one normalized draft must not authorize another")
89+
8290
seedReclassificationSession(t, d, "two", "archive.example", "/worktrees/service/two", "branch")
83-
mapping, applied, err := d.ApplyWorktreeReclassification(
91+
_, _, err = d.ApplyWorktreeReclassification(
8492
ctx, draft, preview.MappingToken, preview.ExistingMappingID,
8593
)
94+
require.ErrorIs(t, err, ErrWorktreeMappingSetChanged,
95+
"a newly affected session must invalidate the accepted preview")
96+
97+
current, err := d.PreviewWorktreeReclassification(ctx, draft)
98+
require.NoError(t, err)
99+
mapping, applied, err := d.ApplyWorktreeReclassification(
100+
ctx, draft, current.MappingToken, current.ExistingMappingID,
101+
)
86102
require.NoError(t, err)
87103
assert.Equal(t, "branch", mapping.OriginalProject)
88-
assert.Equal(t, 2, applied.UpdatedSessions,
89-
"sessions arriving after preview are included")
104+
assert.Equal(t, 2, applied.UpdatedSessions)
90105

91106
stalePreview, err := d.PreviewWorktreeReclassification(ctx, WorktreeReclassificationDraft{
92107
Machine: "other.example", PathPrefix: "/worktrees/service",

internal/duckdb/project_inventory_test.go

Lines changed: 59 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -44,30 +44,17 @@ func seedInventorySession(
4444
}}), "InsertMessages")
4545
}
4646

47-
// duckPushMachine is the fixed machine name the DuckDB sync harness stamps
48-
// onto every pushed session (internal/duckdb/sync.go's newTestSync passes
49-
// this literal to New). Unlike PostgreSQL's push (which preserves each
50-
// session's own Machine field via pushedSessionMachine, falling back only
51-
// for "local"/empty sentinels), DuckDB's push.go unconditionally writes
52-
// s.machine onto every mirrored session row -- worktree mapping rows are
53-
// not affected, since mapping publication mirrors the Machine field
54-
// verbatim. Fixtures pushed through the sync harness must therefore set
55-
// every session's Machine (and every governing mapping's Machine) to this
56-
// value, or governance evaluation will silently never match after push.
47+
// duckPushMachine is the fallback machine name used by the DuckDB sync
48+
// harness for sessions whose source machine is empty or the "local" sentinel.
5749
const duckPushMachine = "test-machine"
5850

5951
// buildInventoryFixture seeds the shared alpha/beta/gamma/misc aggregate
6052
// fixture used by the DuckDB inventory tests: distinct and empty cwds, a
6153
// trashed session excluded from every count, and three mapping rules that
6254
// together exercise every branch of annotateProjectInventoryRows. It
63-
// mirrors internal/postgres's buildInventoryFixture in spirit, but every
64-
// session and every governing mapping shares duckPushMachine (see above);
65-
// alpha-2 is excluded from governance by a non-matching cwd prefix instead
66-
// of a different machine, since DuckDB's push collapses per-session
67-
// machine identity and so cannot express a same-push cross-machine
68-
// exclusion the way PostgreSQL's fixture does (that scoping is instead
69-
// covered by TestDuckProjectInventoryCrossArchiveIsolation's hand-inserted
70-
// rows, which bypass the push collapse).
55+
// mirrors internal/postgres's buildInventoryFixture in spirit. Every session
56+
// and governing mapping happens to share duckPushMachine; multi-machine push
57+
// behavior is covered separately below.
7158
func buildInventoryFixture(t *testing.T, local *db.DB, ctx context.Context) {
7259
t.Helper()
7360
seedInventorySession(t, local, "alpha-1", "alpha", func(s *db.Session) {
@@ -297,16 +284,62 @@ func TestDuckProjectInventoryIgnoresUnattributedSessions(t *testing.T) {
297284
"alpha's session count is unchanged")
298285
}
299286

287+
func TestDuckPushPreservesSessionMachineForGovernance(t *testing.T) {
288+
ctx := context.Background()
289+
local := newLocalDB(t)
290+
for _, fixture := range []struct {
291+
id string
292+
project string
293+
machine string
294+
cwd string
295+
}{
296+
{id: "host-a-session", project: "alpha", machine: "host-a", cwd: "/repos/alpha"},
297+
{id: "host-b-session", project: "beta", machine: "host-b", cwd: "/repos/beta"},
298+
} {
299+
seedInventorySession(t, local, fixture.id, fixture.project, func(s *db.Session) {
300+
s.Machine = fixture.machine
301+
s.Cwd = fixture.cwd
302+
})
303+
_, err := local.CreateWorktreeProjectMapping(ctx, db.WorktreeProjectMapping{
304+
Machine: fixture.machine, PathPrefix: fixture.cwd,
305+
Layout: db.WorktreeMappingLayoutExplicit,
306+
Project: fixture.project, Enabled: true,
307+
})
308+
require.NoError(t, err)
309+
}
310+
311+
syncer := newInMemoryTestSync(t, local, SyncOptions{})
312+
pushDataReadMirror(t, ctx, syncer)
313+
314+
rows, err := syncer.DB().QueryContext(ctx,
315+
`SELECT id, machine FROM sessions ORDER BY id`)
316+
require.NoError(t, err)
317+
defer rows.Close()
318+
machines := map[string]string{}
319+
for rows.Next() {
320+
var id, machine string
321+
require.NoError(t, rows.Scan(&id, &machine))
322+
machines[id] = machine
323+
}
324+
require.NoError(t, rows.Err())
325+
assert.Equal(t, map[string]string{
326+
"host-a-session": "host-a",
327+
"host-b-session": "host-b",
328+
}, machines)
329+
330+
inventory, err := NewStoreFromDB(syncer.DB()).GetProjectInventory(ctx)
331+
require.NoError(t, err)
332+
assert.Equal(t, 2, inventory.GovernedSessions,
333+
"each mirrored session must join the mapping from its source machine")
334+
}
335+
300336
// TestDuckProjectInventoryCrossArchiveIsolation verifies that inventory
301337
// governance is scoped per source archive AND per machine when a DuckDB
302338
// mirror serves more than one archive. It pushes one archive through the
303339
// real sync path (archive A) and hand-inserts a second archive's mapping
304340
// and session rows directly into the DuckDB mirror (archive B), following
305-
// the pattern in TestDuckPushReplicatesWorktreeMappings. Archive A's
306-
// session ends up with machine=duckPushMachine after push (DuckDB's push
307-
// collapses every session's machine field to the syncing machine; see
308-
// duckPushMachine's doc comment), so archive B's hand-inserted rows
309-
// deliberately reuse that same machine name and path prefix, isolating
341+
// the pattern in TestDuckPushReplicatesWorktreeMappings. Both archives
342+
// deliberately use the same machine name and path prefix, isolating
310343
// source_archive_id as the only variable between archive A and B: a broken
311344
// (source_archive_id, machine) scope -- either in the candidate-row SQL or
312345
// in how projectInventoryMappings groups rules by archive -- would
@@ -324,6 +357,7 @@ func TestDuckProjectInventoryCrossArchiveIsolation(t *testing.T) {
324357
// Archive A (the local push archive) has no worktree mapping of its
325358
// own on duckPushMachine.
326359
seedInventorySession(t, local, "a-session", "proj-a", func(s *db.Session) {
360+
s.Machine = duckPushMachine
327361
s.Cwd = "/repos/shared"
328362
s.StartedAt = new("2024-01-01T00:00:00Z")
329363
})
@@ -336,10 +370,10 @@ func TestDuckProjectInventoryCrossArchiveIsolation(t *testing.T) {
336370
`SELECT machine FROM sessions WHERE id = 'a-session'`,
337371
).Scan(&aSessionMachine), "read back a-session machine")
338372
require.Equal(t, duckPushMachine, aSessionMachine,
339-
"push must stamp every session with the syncing machine name")
373+
"push must preserve an explicit source machine")
340374

341375
// Archive B: hand-inserted mirror rows for a second source archive,
342-
// reusing the same (collapsed) machine name and path prefix as archive
376+
// reusing the same machine name and path prefix as archive
343377
// A's session, plus its own governed session.
344378
const archiveB = "archive-b"
345379
_, err := syncer.DB().ExecContext(ctx, `

internal/duckdb/push.go

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -915,9 +915,15 @@ func (s *Sync) upsertSession(
915915
return nil
916916
}
917917

918-
func sessionInsertArgs(sess db.Session, machine, archiveID, fingerprint string) []any {
918+
func sessionInsertArgs(
919+
sess db.Session,
920+
fallbackMachine string,
921+
archiveID string,
922+
fingerprint string,
923+
) []any {
919924
return []any{
920-
sess.ID, sess.Project, machine, sess.Agent,
925+
sess.ID, sess.Project,
926+
mirroredSessionMachine(sess, fallbackMachine), sess.Agent,
921927
sess.AgentLabel, sess.Entrypoint,
922928
nilString(sess.FirstMessage), nilString(sess.DisplayName),
923929
nilString(sess.SessionName),
@@ -953,6 +959,16 @@ func sessionInsertArgs(sess db.Session, machine, archiveID, fingerprint string)
953959
}
954960
}
955961

962+
// mirroredSessionMachine preserves the source archive's machine identity.
963+
// "local" and empty are local-only sentinels, so only those use the machine
964+
// configured for this mirror push.
965+
func mirroredSessionMachine(sess db.Session, fallbackMachine string) string {
966+
if sess.Machine != "" && sess.Machine != "local" {
967+
return sess.Machine
968+
}
969+
return fallbackMachine
970+
}
971+
956972
func insertMessages(
957973
ctx context.Context, exec duckMutationExecutor, msgs []db.Message,
958974
) error {

internal/duckdb/sync.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1002,7 +1002,7 @@ func (s *Sync) sessionFingerprints(
10021002
// quality analytics stale until the next full rebuild.
10031003
func duckSessionFingerprintFields(sess db.Session, machine string) []any {
10041004
return []any{
1005-
sess.ID, sess.Project, machine, sess.Agent,
1005+
sess.ID, sess.Project, mirroredSessionMachine(sess, machine), sess.Agent,
10061006
sess.AgentLabel, sess.Entrypoint,
10071007
nilString(sess.FirstMessage), nilString(sess.DisplayName),
10081008
nilString(sess.SessionName),

0 commit comments

Comments
 (0)