Skip to content

Commit c544f11

Browse files
Nickclaude
andcommitted
Resolve subagent parent only from an unambiguous edge (review fix)
roborev flagged that the bare LIMIT 1 subquery could re-parent a correctly parented subagent arbitrarily when a child is referenced by spawn edges from more than one distinct session (reachable via copied/forked history). This became reachable for already-'subagent' rows once the prior commit added the "OR parent differs" branch. Resolve the parent only from an unambiguous edge: GROUP BY subagent_session_id HAVING COUNT(DISTINCT tc.session_id) = 1, wrapped in COALESCE(..., parent_session_id) so conflicting edges preserve the current parent instead of picking one arbitrarily. This also removes the LIMIT 1 nondeterminism between the SET and WHERE evaluations. Add TestLinkSubagentSessionsPreservesParentOnConflictingEdges (mutation-verified: fails if the exactly-one-distinct-spawner guard is relaxed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9e58019 commit c544f11

2 files changed

Lines changed: 91 additions & 22 deletions

File tree

internal/db/link_subagent_nested_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,3 +182,60 @@ func TestLinkSubagentSessionsLinksNullParentSubagent(t *testing.T) {
182182
"orphan.parent_session_id")
183183
}
184184
}
185+
186+
// TestLinkSubagentSessionsPreservesParentOnConflictingEdges guards against
187+
// arbitrary re-parenting when a child is referenced by spawn edges from more
188+
// than one distinct session (reachable via copied/forked history). The parent
189+
// must be resolved only from an unambiguous edge; with conflicting edges the
190+
// current parent is preserved rather than picked arbitrarily by a bare LIMIT 1.
191+
func TestLinkSubagentSessionsPreservesParentOnConflictingEdges(t *testing.T) {
192+
d := testDB(t)
193+
194+
insertSession(t, d, "p1", "p", func(s *Session) {
195+
s.MessageCount = 1
196+
})
197+
insertSession(t, d, "p2", "p", func(s *Session) {
198+
s.MessageCount = 1
199+
})
200+
// "kid" is already a correctly parented subagent under p2.
201+
insertSession(t, d, "kid", "p", func(s *Session) {
202+
s.MessageCount = 1
203+
parent := "p2"
204+
s.ParentSessionID = &parent
205+
s.RelationshipType = "subagent"
206+
})
207+
208+
// Two DISTINCT sessions both carry a spawn edge to "kid". p1's edge is
209+
// inserted first (lower rowid), so a bare `LIMIT 1` would pick p1 and
210+
// wrongly re-parent kid away from its current p2. The unambiguous-edge
211+
// resolution must instead preserve p2.
212+
insertMessages(t, d,
213+
Message{
214+
SessionID: "p1", Ordinal: 0, Role: "assistant",
215+
Content: "spawn kid (copy A)", HasToolUse: true,
216+
ToolCalls: []ToolCall{{
217+
ToolName: "Agent", Category: "Task",
218+
SubagentSessionID: "kid",
219+
}},
220+
},
221+
Message{
222+
SessionID: "p2", Ordinal: 0, Role: "assistant",
223+
Content: "spawn kid (copy B)", HasToolUse: true,
224+
ToolCalls: []ToolCall{{
225+
ToolName: "Agent", Category: "Task",
226+
SubagentSessionID: "kid",
227+
}},
228+
},
229+
)
230+
231+
require.NoError(t, d.LinkSubagentSessions(), "LinkSubagentSessions")
232+
233+
kid, err := d.GetSession(context.Background(), "kid")
234+
requireNoError(t, err, "GetSession kid")
235+
assert.Equal(t, "subagent", kid.RelationshipType, "kid relationship_type")
236+
if assert.NotNil(t, kid.ParentSessionID, "kid parent") {
237+
assert.Equal(t, "p2", *kid.ParentSessionID,
238+
"conflicting edges must preserve the current parent, "+
239+
"not pick one arbitrarily")
240+
}
241+
}

internal/db/sessions.go

Lines changed: 34 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1566,37 +1566,49 @@ func (db *DB) LinkSubagentSessions() error {
15661566
// same pattern).
15671567
_, err := db.getWriter().Exec(`
15681568
UPDATE sessions
1569-
SET parent_session_id = (
1570-
SELECT tc.session_id
1571-
FROM tool_calls tc
1572-
WHERE tc.subagent_session_id = sessions.id
1573-
LIMIT 1
1569+
SET parent_session_id = COALESCE(
1570+
(
1571+
SELECT tc.session_id
1572+
FROM tool_calls tc
1573+
WHERE tc.subagent_session_id = sessions.id
1574+
GROUP BY tc.subagent_session_id
1575+
HAVING COUNT(DISTINCT tc.session_id) = 1
1576+
),
1577+
parent_session_id
15741578
),
15751579
relationship_type = 'subagent',
15761580
local_modified_at = strftime('%Y-%m-%dT%H:%M:%fZ','now')
1577-
-- The tool_calls edge (from toolUseResult.agentId) is the record of
1578-
-- the actual spawn, so it is authoritative over the path-derived
1579-
-- parent set at parse time. Nested subagents (depth >= 2) all live in
1580-
-- the same flat <main>/subagents/ dir, so path derivation pins them to
1581-
-- the main session AND tags them 'subagent'; the previous
1582-
-- relationship_type != subagent guard then skipped them, leaving
1583-
-- the hierarchy flat. Update the row when EITHER it is not yet tagged
1584-
-- 'subagent' (upgrades a continuation/fork/empty classification even
1585-
-- when its parent already matches the spawner) OR the authoritative
1586-
-- parent differs (the nested depth>=2 re-parent). Null-safe IS NOT.
1587-
-- Already-correct subagents match neither branch and are left as-is to
1588-
-- avoid needless local_modified_at churn.
1581+
-- The tool_calls edge (from toolUseResult.agentId) records the actual
1582+
-- spawn, authoritative over the path-derived parent set at parse time.
1583+
-- Nested subagents (depth >= 2) live flat in <main>/subagents/, so path
1584+
-- derivation pins them to the main session AND tags them 'subagent';
1585+
-- the old relationship_type != 'subagent' guard skipped them, leaving
1586+
-- the hierarchy flat.
1587+
--
1588+
-- Resolve the parent only from an UNAMBIGUOUS edge: the grouped
1589+
-- subquery yields a spawner only when exactly one distinct session
1590+
-- spawned this child (HAVING COUNT(DISTINCT tc.session_id) = 1). With
1591+
-- conflicting edges from copied/forked history it yields NULL and
1592+
-- COALESCE keeps the current parent, so a correctly parented subagent
1593+
-- is never re-parented arbitrarily. Update when EITHER the row is not
1594+
-- yet 'subagent' (upgrade continuation/fork/empty; parent preserved if
1595+
-- ambiguous) OR the unambiguous parent differs (null-safe IS NOT).
1596+
-- Already-correct subagents match neither branch (no churn).
15891597
WHERE EXISTS (
15901598
SELECT 1 FROM tool_calls tc
15911599
WHERE tc.subagent_session_id = sessions.id
15921600
)
15931601
AND (
15941602
relationship_type != 'subagent'
1595-
OR parent_session_id IS NOT (
1596-
SELECT tc.session_id
1597-
FROM tool_calls tc
1598-
WHERE tc.subagent_session_id = sessions.id
1599-
LIMIT 1
1603+
OR parent_session_id IS NOT COALESCE(
1604+
(
1605+
SELECT tc.session_id
1606+
FROM tool_calls tc
1607+
WHERE tc.subagent_session_id = sessions.id
1608+
GROUP BY tc.subagent_session_id
1609+
HAVING COUNT(DISTINCT tc.session_id) = 1
1610+
),
1611+
parent_session_id
16001612
)
16011613
)`)
16021614
if err != nil {

0 commit comments

Comments
 (0)