Skip to content

Commit d2d93de

Browse files
committed
fix(sync): preserve hierarchy repair intent
Generic relink seeds do not prove that a missing parent was deleted, so treating every queued session as cleanup work could erase valid parser-derived hierarchy while ingestion was incomplete. Persist destructive cleanup intent separately from ordinary edge reconciliation while retaining the row-keyed queue cost shape and legacy queue semantics. Archive rebuilds must also consume copied repair state after orphan restoration. Abort before swap when that transactional repair fails so a rebuilt archive cannot install stale hierarchy.
1 parent eac1897 commit d2d93de

7 files changed

Lines changed: 267 additions & 28 deletions

File tree

internal/db/db_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5632,6 +5632,9 @@ func TestCopySyncStateFrom_OnlyCopiesDurableKeys(t *testing.T) {
56325632
"seed source finished")
56335633
require.NoError(t, srcDB.QueueSubagentParentRepairs([]string{"queued-child"}),
56345634
"seed durable hierarchy repair")
5635+
require.NoError(t, srcDB.QueueSubagentParentCleanupRepairs(
5636+
[]string{"queued-former-child"},
5637+
), "seed durable hierarchy cleanup")
56355638
require.NoError(t, srcDB.UpsertSession(Session{
56365639
ID: "queued-session", Project: "p", Machine: "local", Agent: "claude",
56375640
}), "seed source queued session")
@@ -5667,6 +5670,13 @@ func TestCopySyncStateFrom_OnlyCopiesDurableKeys(t *testing.T) {
56675670
).Scan(&queuedRepairs), "query copied subagent repair queue")
56685671
assert.Equal(t, 1, queuedRepairs,
56695672
"pending hierarchy repairs must survive an archive rebuild")
5673+
var queuedCleanups int
5674+
require.NoError(t, dstDB.Reader().QueryRow(`
5675+
SELECT count(*) FROM subagent_parent_cleanup_queue
5676+
WHERE session_id = 'queued-former-child'`,
5677+
).Scan(&queuedCleanups), "query copied subagent cleanup queue")
5678+
assert.Equal(t, 1, queuedCleanups,
5679+
"pending destructive cleanup intent must survive an archive rebuild")
56705680

56715681
gotStarted, err := dstDB.GetSyncState("last_sync_started_at")
56725682
require.NoError(t, err, "GetSyncState last_sync_started_at")

internal/db/link_subagent_nested_test.go

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -775,8 +775,9 @@ func TestLinkSubagentSessionsForSessionsClearsDanglingParent(t *testing.T) {
775775
require.Equal(t, 1, n, "spawner must be deleted")
776776

777777
// The engine captures the kid as a pre-write child of the deleted
778-
// spawner and carries it into the scoped batch.
779-
require.NoError(t, d.LinkSubagentSessionsForSessions([]string{"kid"}))
778+
// spawner and persists cleanup intent before the destructive write.
779+
require.NoError(t, d.QueueSubagentParentCleanupRepairs([]string{"kid"}))
780+
require.NoError(t, d.RepairQueuedSubagentParents())
780781

781782
kid, err := d.GetSession(context.Background(), "kid")
782783
requireNoError(t, err, "GetSession kid")
@@ -788,6 +789,39 @@ func TestLinkSubagentSessionsForSessionsClearsDanglingParent(t *testing.T) {
788789
"re-links the child")
789790
}
790791

792+
// TestLinkSubagentSessionsForSessionsKeepsUnresolvedPathParent proves that a
793+
// normal changed-session seed is not evidence that its parent was deleted.
794+
// Providers can ingest a path-derived child before its parent, with no spawn
795+
// edge yet available; scoped edge linking must preserve that parser claim so
796+
// the later parent row completes the hierarchy instead of leaving the child
797+
// permanently un-parented.
798+
func TestLinkSubagentSessionsForSessionsKeepsUnresolvedPathParent(t *testing.T) {
799+
d := testDB(t)
800+
801+
insertSession(t, d, "kid", "p", func(s *Session) {
802+
s.MessageCount = 1
803+
s.ParentSessionID = Ptr("parent-not-ingested-yet")
804+
s.RelationshipType = "subagent"
805+
})
806+
807+
require.NoError(t, d.LinkSubagentSessionsForSessions([]string{"kid"}))
808+
assert.Equal(t, "parent-not-ingested-yet", parentOfSession(t, d, "kid"),
809+
"a generic changed-session seed must preserve parser-derived parentage")
810+
require.NoError(t, d.QueueSubagentParentRepairs([]string{"kid"}))
811+
require.NoError(t, d.RepairQueuedSubagentParents())
812+
assert.Equal(t, "parent-not-ingested-yet", parentOfSession(t, d, "kid"),
813+
"a durable generic repair must remain relink-only")
814+
815+
insertSession(t, d, "parent-not-ingested-yet", "p", func(s *Session) {
816+
s.MessageCount = 1
817+
})
818+
require.NoError(t, d.LinkSubagentSessionsForSessions(
819+
[]string{"parent-not-ingested-yet"},
820+
))
821+
assert.Equal(t, "parent-not-ingested-yet", parentOfSession(t, d, "kid"),
822+
"ingesting the parent later must leave the valid path hierarchy intact")
823+
}
824+
791825
// TestLinkSubagentSessionsForSessionsKeepsParentWhenSpawnerRemains pins the
792826
// deliberate limit of the dangling-parent repair: when only the EDGE is
793827
// gone but the stored parent session still exists, nothing distinguishes an
@@ -904,6 +938,32 @@ func TestRepairQueuedSubagentParentsMigratesLegacyJSONQueue(t *testing.T) {
904938
assert.Zero(t, queued, "successful repair must clear migrated rows")
905939
}
906940

941+
func TestRepairQueuedSubagentParentsMigratesLegacyCleanupIntent(t *testing.T) {
942+
d := testDB(t)
943+
insertSession(t, d, "spawner", "p", func(s *Session) {
944+
s.MessageCount = 1
945+
})
946+
insertSession(t, d, "kid", "p", func(s *Session) {
947+
s.MessageCount = 1
948+
s.ParentSessionID = Ptr("spawner")
949+
s.RelationshipType = "subagent"
950+
})
951+
insertMessages(t, d, spawnEdgeTo("spawner", "kid", "spawn"))
952+
require.NoError(t, d.SetSyncState(
953+
subagentParentRepairQueueStateKey, `["kid"]`,
954+
))
955+
_, err := d.DeleteParserExcludedSessions([]string{"spawner"})
956+
require.NoError(t, err)
957+
958+
require.NoError(t, d.RepairQueuedSubagentParents())
959+
960+
kid, err := d.GetSession(context.Background(), "kid")
961+
require.NoError(t, err)
962+
require.NotNil(t, kid)
963+
assert.Nil(t, kid.ParentSessionID,
964+
"the legacy queue contained pre-write children and must retain cleanup intent")
965+
}
966+
907967
// TestLinkSubagentSessionsForSessionsPlanIsBatchBounded pins the cost shape
908968
// of the scoped statement, mirroring TestLinkSubagentSessionsPlanScalesWith-
909969
// SpawnEdges: the watcher calls this once per changed file, so neither

internal/db/orphaned.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,13 @@ func (d *DB) CopySyncStateFrom(sourcePath string) error {
374374
return fmt.Errorf("copying subagent parent repair queue: %w", err)
375375
}
376376
}
377+
if oldDBHasTable(ctx, tx, "subagent_parent_cleanup_queue") {
378+
if _, err := tx.ExecContext(ctx, `
379+
INSERT OR IGNORE INTO main.subagent_parent_cleanup_queue (session_id)
380+
SELECT session_id FROM old_db.subagent_parent_cleanup_queue`); err != nil {
381+
return fmt.Errorf("copying subagent parent cleanup queue: %w", err)
382+
}
383+
}
377384

378385
headRevisionExpr := "0"
379386
if oldDBHasColumn(ctx, tx, "artifact_checkpoint_heads", "publication_revision") {

internal/db/schema.sql

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,6 +1081,13 @@ CREATE TABLE IF NOT EXISTS subagent_parent_repair_queue (
10811081
session_id TEXT PRIMARY KEY
10821082
) WITHOUT ROWID;
10831083

1084+
-- Subset of queued hierarchy repairs whose pre-write spawn edge may have been
1085+
-- removed. Only these captured former children are eligible for destructive
1086+
-- dangling-parent cleanup; ordinary changed-session seeds are relink-only.
1087+
CREATE TABLE IF NOT EXISTS subagent_parent_cleanup_queue (
1088+
session_id TEXT PRIMARY KEY
1089+
) WITHOUT ROWID;
1090+
10841091
-- Model pricing for cost calculation
10851092
CREATE TABLE IF NOT EXISTS model_pricing (
10861093
model_pattern TEXT PRIMARY KEY,

internal/db/sessions.go

Lines changed: 77 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1823,7 +1823,7 @@ func linkSubagentSessionsForSessionsQuery(ph string) string {
18231823
)`
18241824
}
18251825

1826-
// clearDanglingSubagentParentQuery repairs a captured child whose LAST spawn
1826+
// clearDanglingSubagentParentQuery repairs a captured former child whose LAST spawn
18271827
// edge was removed together with its spawner: both UNION branches of the
18281828
// linking statement select from remaining tool_calls, so an edge-less child
18291829
// can never be re-resolved there, and its parent now points at a session
@@ -1853,8 +1853,11 @@ func clearDanglingSubagentParentQuery(ph string) string {
18531853
// LinkSubagentSessionsForSessions is LinkSubagentSessions scoped to the
18541854
// sessions written by one sync batch: only children reachable from a batch
18551855
// member's spawn edges (or batch members that are themselves children) are
1856-
// re-resolved. Children in ids whose edges are all gone and whose parent no
1857-
// longer exists are un-parented (see clearDanglingSubagentParentQuery).
1856+
// re-resolved. Generic changed-session IDs are deliberately ineligible for
1857+
// dangling-parent cleanup: a parser-derived parent may simply not have been
1858+
// ingested yet. Destructive cleanup is reserved for former children captured
1859+
// before a write that can remove their spawn edges and persisted through
1860+
// QueueSubagentParentCleanupRepairs.
18581861
// Per-event paths — the session watcher re-syncs a single file
18591862
// on every change — must use this form so their linking cost tracks the
18601863
// changed batch; bulk paths (full sync, reconciliation, resync) keep the
@@ -1880,25 +1883,27 @@ func (db *DB) LinkSubagentSessionsForSessions(ids []string) error {
18801883
len(chunk), err,
18811884
)
18821885
}
1883-
_, err = db.getWriter().Exec(
1884-
clearDanglingSubagentParentQuery(ph), args...,
1885-
)
1886-
if err != nil {
1887-
return fmt.Errorf(
1888-
"clearing dangling subagent parents for %d changed "+
1889-
"sessions: %w",
1890-
len(chunk), err,
1891-
)
1892-
}
18931886
return nil
18941887
})
18951888
}
18961889

1897-
// QueueSubagentParentRepairs durably records sessions whose hierarchy may be
1898-
// changed by an upcoming write. Callers must queue the IDs before deleting or
1899-
// replacing messages because those writes can cascade away the only spawn
1900-
// edge that identifies an affected child.
1890+
// QueueSubagentParentRepairs durably records sessions whose hierarchy must be
1891+
// re-evaluated from surviving spawn edges. These generic seeds are never used
1892+
// for destructive dangling-parent cleanup; callers that captured a former
1893+
// child before removing edges use QueueSubagentParentCleanupRepairs instead.
19011894
func (db *DB) QueueSubagentParentRepairs(ids []string) error {
1895+
return db.queueSubagentParentRepairs(ids, false)
1896+
}
1897+
1898+
// QueueSubagentParentCleanupRepairs durably records former children captured
1899+
// before an exclusion or message replacement can remove their spawn edges.
1900+
// Cleanup intent is separate from ordinary relink work so a newly parsed child
1901+
// whose parent has not arrived yet never loses valid parser-derived parentage.
1902+
func (db *DB) QueueSubagentParentCleanupRepairs(ids []string) error {
1903+
return db.queueSubagentParentRepairs(ids, true)
1904+
}
1905+
1906+
func (db *DB) queueSubagentParentRepairs(ids []string, cleanup bool) error {
19021907
if len(ids) == 0 {
19031908
return nil
19041909
}
@@ -1911,20 +1916,39 @@ func (db *DB) QueueSubagentParentRepairs(ids []string) error {
19111916
}
19121917
defer func() { _ = tx.Rollback() }()
19131918

1914-
stmt, err := tx.Prepare(`
1919+
repairStmt, err := tx.Prepare(`
19151920
INSERT INTO subagent_parent_repair_queue (session_id) VALUES (?)
19161921
ON CONFLICT(session_id) DO NOTHING`)
19171922
if err != nil {
19181923
return fmt.Errorf("preparing subagent parent repair queue insert: %w", err)
19191924
}
1920-
defer stmt.Close()
1925+
defer repairStmt.Close()
1926+
var cleanupStmt *sql.Stmt
1927+
if cleanup {
1928+
cleanupStmt, err = tx.Prepare(`
1929+
INSERT INTO subagent_parent_cleanup_queue (session_id) VALUES (?)
1930+
ON CONFLICT(session_id) DO NOTHING`)
1931+
if err != nil {
1932+
return fmt.Errorf(
1933+
"preparing subagent parent cleanup queue insert: %w", err,
1934+
)
1935+
}
1936+
defer cleanupStmt.Close()
1937+
}
19211938
for _, id := range ids {
19221939
if id == "" {
19231940
continue
19241941
}
1925-
if _, err := stmt.Exec(id); err != nil {
1942+
if _, err := repairStmt.Exec(id); err != nil {
19261943
return fmt.Errorf("queueing subagent parent repair for %s: %w", id, err)
19271944
}
1945+
if cleanupStmt != nil {
1946+
if _, err := cleanupStmt.Exec(id); err != nil {
1947+
return fmt.Errorf(
1948+
"queueing subagent parent cleanup for %s: %w", id, err,
1949+
)
1950+
}
1951+
}
19281952
}
19291953
if err := tx.Commit(); err != nil {
19301954
return fmt.Errorf("committing subagent parent repair queue: %w", err)
@@ -1943,6 +1967,7 @@ func (db *DB) RepairQueuedSubagentParents() error {
19431967
var pending int
19441968
err := db.getWriter().QueryRow(`
19451969
SELECT EXISTS(SELECT 1 FROM subagent_parent_repair_queue)
1970+
OR EXISTS(SELECT 1 FROM subagent_parent_cleanup_queue)
19461971
OR EXISTS(SELECT 1 FROM pg_sync_state WHERE key = ?)`,
19471972
subagentParentRepairQueueStateKey,
19481973
).Scan(&pending)
@@ -1963,6 +1988,8 @@ func (db *DB) RepairQueuedSubagentParents() error {
19631988
for {
19641989
rows, err := tx.Query(`
19651990
SELECT session_id FROM subagent_parent_repair_queue
1991+
UNION
1992+
SELECT session_id FROM subagent_parent_cleanup_queue
19661993
ORDER BY session_id LIMIT ?`, maxSQLVars/2)
19671994
if err != nil {
19681995
return fmt.Errorf("listing queued subagent parent repairs: %w", err)
@@ -1996,15 +2023,26 @@ func (db *DB) RepairQueuedSubagentParents() error {
19962023
len(chunk), err,
19972024
)
19982025
}
2026+
cleanupSeeds := `(SELECT session_id
2027+
FROM subagent_parent_cleanup_queue WHERE session_id IN ` + ph + `)`
19992028
if _, err := tx.Exec(
2000-
clearDanglingSubagentParentQuery(ph), args...,
2029+
clearDanglingSubagentParentQuery(cleanupSeeds), args...,
20012030
); err != nil {
20022031
return fmt.Errorf(
20032032
"clearing queued dangling subagent parents for %d "+
20042033
"sessions: %w",
20052034
len(chunk), err,
20062035
)
20072036
}
2037+
if _, err := tx.Exec(
2038+
"DELETE FROM subagent_parent_cleanup_queue WHERE session_id IN "+ph,
2039+
args...,
2040+
); err != nil {
2041+
return fmt.Errorf(
2042+
"clearing %d queued subagent parent cleanups: %w",
2043+
len(chunk), err,
2044+
)
2045+
}
20082046
if _, err := tx.Exec(
20092047
"DELETE FROM subagent_parent_repair_queue WHERE session_id IN "+ph,
20102048
args...,
@@ -2037,20 +2075,35 @@ func migrateLegacySubagentParentRepairQueueTx(tx *sql.Tx) error {
20372075
if err := json.Unmarshal([]byte(encoded), &ids); err != nil {
20382076
return fmt.Errorf("decoding legacy subagent parent repair queue: %w", err)
20392077
}
2040-
stmt, err := tx.Prepare(`
2078+
repairStmt, err := tx.Prepare(`
20412079
INSERT INTO subagent_parent_repair_queue (session_id) VALUES (?)
20422080
ON CONFLICT(session_id) DO NOTHING`)
20432081
if err != nil {
20442082
return fmt.Errorf("preparing legacy subagent parent repair migration: %w", err)
20452083
}
2046-
defer stmt.Close()
2084+
defer repairStmt.Close()
2085+
cleanupStmt, err := tx.Prepare(`
2086+
INSERT INTO subagent_parent_cleanup_queue (session_id) VALUES (?)
2087+
ON CONFLICT(session_id) DO NOTHING`)
2088+
if err != nil {
2089+
return fmt.Errorf("preparing legacy subagent parent cleanup migration: %w", err)
2090+
}
2091+
defer cleanupStmt.Close()
20472092
for _, id := range ids {
20482093
if id == "" {
20492094
continue
20502095
}
2051-
if _, err := stmt.Exec(id); err != nil {
2096+
if _, err := repairStmt.Exec(id); err != nil {
20522097
return fmt.Errorf("migrating legacy subagent parent repair for %s: %w", id, err)
20532098
}
2099+
// The JSON queue predates generic post-write and attempted-session
2100+
// seeds; every legacy ID was captured before a destructive write and
2101+
// therefore carries cleanup intent.
2102+
if _, err := cleanupStmt.Exec(id); err != nil {
2103+
return fmt.Errorf(
2104+
"migrating legacy subagent parent cleanup for %s: %w", id, err,
2105+
)
2106+
}
20542107
}
20552108
if _, err := tx.Exec(
20562109
"DELETE FROM pg_sync_state WHERE key = ?",

internal/sync/engine.go

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2343,6 +2343,26 @@ func (e *Engine) resyncBuildLocked(
23432343
}
23442344
}
23452345

2346+
// CopySyncStateFrom runs after the fresh archive's normal linking pass so
2347+
// pending hierarchy work from the original must be consumed explicitly.
2348+
// Wait until orphan restoration is complete so every queued session and
2349+
// copied spawn edge is present. A failed repair leaves hierarchy state
2350+
// uncertain and must abort before the replacement can be installed.
2351+
if err := newDB.RepairQueuedSubagentParents(); err != nil {
2352+
log.Printf("resync: repair copied subagent parents: %v", err)
2353+
stats.Aborted = true
2354+
stats.Warnings = append(stats.Warnings,
2355+
"hierarchy repair failed, aborting swap: "+err.Error(),
2356+
)
2357+
newDB.Close()
2358+
removeTempDB(tempPath)
2359+
restoreSkipCache()
2360+
e.mu.Lock()
2361+
e.lastSyncStats = stats
2362+
e.mu.Unlock()
2363+
return stats, err
2364+
}
2365+
23462366
// Copy recall entries and their evidence from the quiesced old DB.
23472367
// The fresh DB is built from source files, which never contain
23482368
// recall entries, so without this every accepted entry is lost on
@@ -6710,7 +6730,7 @@ func (e *Engine) collectAndBatch(
67106730
// Persist affected IDs before any exclusion or replacement can
67116731
// cascade their only spawn edge away. The queue is cleared only in
67126732
// the same transaction that successfully repairs the hierarchy.
6713-
if err := e.db.QueueSubagentParentRepairs(children); err != nil {
6733+
if err := e.db.QueueSubagentParentCleanupRepairs(children); err != nil {
67146734
log.Printf("queue subagent parent repairs: %v", err)
67156735
stats.RecordFailed()
67166736
e.noteSQLiteContainerResult(r.path, false)
@@ -13374,7 +13394,7 @@ func (e *Engine) SyncSingleSessionContext(
1337413394
if err := e.db.RepairQueuedSubagentParents(); err != nil {
1337513395
return fmt.Errorf("repair queued subagent parents: %w", err)
1337613396
}
13377-
if err := e.db.QueueSubagentParentRepairs(priorChildren); err != nil {
13397+
if err := e.db.QueueSubagentParentCleanupRepairs(priorChildren); err != nil {
1337813398
return fmt.Errorf("queue subagent parent repairs: %w", err)
1337913399
}
1338013400
// Always attempt queued work after mutations begin, including when a later

0 commit comments

Comments
 (0)