Skip to content

Commit 980f5e0

Browse files
committed
refactor(session): extract Session.isDescendantOf from auto-reply walk (F13)
The lineage walk in SessionAutoReply (formerly run-events) was ad hoc. ACP and TUI will eventually need the same operation. Move the walk to Session.Interface.isDescendantOf with the lookup cache as a caller-provided argument so AutoReply retains dedup across events. - Add Session.isDescendantOf(sid, root, opts?: { maxDepth?, cache? }). Default maxDepth = 64 (generic). Cache, when provided, is mutated with all confirmed-descendant intermediate nodes, turning repeated calls against the same lineage into O(1) after the first walk. NotFoundError defects are caught and treated as a non-match. - AutoReply now delegates via a 2-line shim that passes its own MAX_LINEAGE_DEPTH (32) and its descendants-Set as the cache. Drops the inline 23-line Effect.fn plus Option/NotFoundError imports. - 7 new tests on Session.isDescendantOf: identity, direct child, grandchild, unrelated, non-existent, maxDepth boundary, cache accumulation. AutoReply's existing 14 auto-reply.test tests and the subagent-hang regression continue to pass unchanged. Diamond review: codex-5.3 spec APPROVE, Opus quality APPROVE. Refs: F13 in docs/superpowers/plans/2026-04-23-audit-remediation.md
1 parent 347ecbe commit 980f5e0

3 files changed

Lines changed: 172 additions & 25 deletions

File tree

packages/opencode/src/session/auto-reply/auto-reply.ts

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
import { Cause, Effect, Fiber, Option } from "effect"
1+
import { Cause, Effect, Fiber } from "effect"
22
import { Bus } from "@/bus"
33
import { Permission } from "@/permission"
44
import { Question } from "@/question"
55
import { Session } from "@/session"
6-
import { NotFoundError } from "@/storage"
76
import { SessionID } from "@/session/schema"
87
import { Log } from "@/util"
98
import type { Sink } from "./sink"
@@ -49,28 +48,8 @@ export const make = Effect.fn("SessionAutoReply.make")(function* (config: Config
4948

5049
const descendants = new Set<SessionID>([config.rootSessionID])
5150

52-
const isDescendant = Effect.fn("SessionAutoReply.isDescendant")(function* (sid: SessionID) {
53-
if (descendants.has(sid)) return true
54-
let cur: SessionID | undefined = sid
55-
const chain: SessionID[] = []
56-
let depth = 0
57-
while (cur !== undefined && !descendants.has(cur) && depth < MAX_LINEAGE_DEPTH) {
58-
chain.push(cur)
59-
depth++
60-
const lookup: Option.Option<Session.Info> = yield* session.get(cur).pipe(
61-
Effect.option,
62-
Effect.catchDefect((defect) => {
63-
if (!NotFoundError.isInstance(defect)) return Effect.die(defect)
64-
return Effect.succeed(Option.none<Session.Info>())
65-
}),
66-
)
67-
if (Option.isNone(lookup)) break
68-
cur = lookup.value.parentID ?? undefined
69-
}
70-
if (cur === undefined || !descendants.has(cur)) return false
71-
chain.forEach((item) => descendants.add(item))
72-
return true
73-
})
51+
const isDescendant = (sid: SessionID) =>
52+
session.isDescendantOf(sid, config.rootSessionID, { maxDepth: MAX_LINEAGE_DEPTH, cache: descendants })
7453

7554
const bump = (kind: "question" | "permission", sid: SessionID) => {
7655
if (kind === "question") stats.autoRejectedQuestions++

packages/opencode/src/session/session.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,22 @@ export interface Interface {
375375
sessionID: SessionID,
376376
predicate: (msg: MessageV2.WithParts) => boolean,
377377
) => Effect.Effect<Option.Option<MessageV2.WithParts>>
378+
/**
379+
* Returns true if `sid` is `root` or transitively descends from `root` via
380+
* `parentID`. Walks the parent chain up to `opts.maxDepth` (default 64) and
381+
* stops at any `NotFoundError` (returns false). When `opts.cache` is
382+
* provided, it is used both as a positive-hit short-circuit and as an
383+
* accumulator: every confirmed descendant in the walked chain is added to
384+
* the set. Callers that issue many `isDescendantOf` calls against the same
385+
* root (e.g. SessionAutoReply) should seed the cache with `new Set([root])`
386+
* and reuse it across calls so the parent chain is traversed at most once
387+
* per node.
388+
*/
389+
readonly isDescendantOf: (
390+
sid: SessionID,
391+
root: SessionID,
392+
opts?: { maxDepth?: number; cache?: Set<SessionID> },
393+
) => Effect.Effect<boolean>
378394
}
379395

380396
export class Service extends Context.Service<Service, Interface>()("@opencode/Session") {}
@@ -671,6 +687,39 @@ export const layer: Layer.Layer<Service, never, Bus.Service | Storage.Service> =
671687
return Option.none<MessageV2.WithParts>()
672688
})
673689

690+
const isDescendantOf = Effect.fn("Session.isDescendantOf")(function* (
691+
sid: SessionID,
692+
root: SessionID,
693+
opts?: { maxDepth?: number; cache?: Set<SessionID> },
694+
) {
695+
const maxDepth = opts?.maxDepth ?? 64
696+
const known = opts?.cache ?? new Set<SessionID>([root])
697+
if (sid === root) return true
698+
if (known.has(sid)) return true
699+
// Walk parent chain. Track the chain so we can promote every visited
700+
// node into the cache once we hit a known descendant — turns repeated
701+
// calls against the same lineage into O(1) after the first walk.
702+
const chain: SessionID[] = []
703+
let cur: SessionID | undefined = sid
704+
let depth = 0
705+
while (cur !== undefined && !known.has(cur) && depth < maxDepth) {
706+
chain.push(cur)
707+
depth++
708+
const lookup: Option.Option<Info> = yield* get(cur).pipe(
709+
Effect.option,
710+
Effect.catchDefect((defect) => {
711+
if (!NotFoundError.isInstance(defect)) return Effect.die(defect)
712+
return Effect.succeed(Option.none<Info>())
713+
}),
714+
)
715+
if (Option.isNone(lookup)) break
716+
cur = lookup.value.parentID ?? undefined
717+
}
718+
if (cur === undefined || !known.has(cur)) return false
719+
chain.forEach((item) => known.add(item))
720+
return true
721+
})
722+
674723
return Service.of({
675724
create,
676725
fork,
@@ -693,6 +742,7 @@ export const layer: Layer.Layer<Service, never, Bus.Service | Storage.Service> =
693742
getPart,
694743
updatePartDelta,
695744
findMessage,
745+
isDescendantOf,
696746
})
697747
}),
698748
)

packages/opencode/test/session/session.test.ts

Lines changed: 119 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { Bus } from "../../src/bus"
55
import { Log } from "../../src/util"
66
import { Instance } from "../../src/project/instance"
77
import { MessageV2 } from "../../src/session/message-v2"
8-
import { MessageID, PartID, type SessionID } from "../../src/session/schema"
8+
import { MessageID, PartID, SessionID } from "../../src/session/schema"
99
import { AppRuntime } from "../../src/effect/app-runtime"
1010
import { tmpdir } from "../fixture/fixture"
1111

@@ -179,3 +179,121 @@ describe("Session", () => {
179179
expect(missing).toBe(true)
180180
})
181181
})
182+
183+
function isDescendantOf(
184+
sid: SessionID,
185+
root: SessionID,
186+
opts?: { maxDepth?: number; cache?: Set<SessionID> },
187+
) {
188+
return AppRuntime.runPromise(SessionNs.Service.use((svc) => svc.isDescendantOf(sid, root, opts)))
189+
}
190+
191+
describe("Session.isDescendantOf", () => {
192+
test("identity: a session is its own descendant", async () => {
193+
await using tmp = await tmpdir({ git: true })
194+
await Instance.provide({
195+
directory: tmp.path,
196+
fn: async () => {
197+
const root = await create({ title: "root" })
198+
expect(await isDescendantOf(root.id, root.id)).toBe(true)
199+
await remove(root.id)
200+
},
201+
})
202+
})
203+
204+
test("direct child is a descendant", async () => {
205+
await using tmp = await tmpdir({ git: true })
206+
await Instance.provide({
207+
directory: tmp.path,
208+
fn: async () => {
209+
const root = await create({ title: "root" })
210+
const child = await create({ title: "child", parentID: root.id })
211+
expect(await isDescendantOf(child.id, root.id)).toBe(true)
212+
await remove(root.id)
213+
},
214+
})
215+
})
216+
217+
test("grandchild is a descendant", async () => {
218+
await using tmp = await tmpdir({ git: true })
219+
await Instance.provide({
220+
directory: tmp.path,
221+
fn: async () => {
222+
const root = await create({ title: "root" })
223+
const child = await create({ title: "child", parentID: root.id })
224+
const grandchild = await create({ title: "grandchild", parentID: child.id })
225+
expect(await isDescendantOf(grandchild.id, root.id)).toBe(true)
226+
await remove(root.id)
227+
},
228+
})
229+
})
230+
231+
test("unrelated session is not a descendant", async () => {
232+
await using tmp = await tmpdir({ git: true })
233+
await Instance.provide({
234+
directory: tmp.path,
235+
fn: async () => {
236+
const rootA = await create({ title: "rootA" })
237+
const rootB = await create({ title: "rootB" })
238+
expect(await isDescendantOf(rootB.id, rootA.id)).toBe(false)
239+
await remove(rootA.id)
240+
await remove(rootB.id)
241+
},
242+
})
243+
})
244+
245+
test("non-existent session id is not a descendant", async () => {
246+
await using tmp = await tmpdir({ git: true })
247+
await Instance.provide({
248+
directory: tmp.path,
249+
fn: async () => {
250+
const root = await create({ title: "root" })
251+
const ghost = SessionID.make("ses_ghost_does_not_exist_00000")
252+
expect(await isDescendantOf(ghost, root.id)).toBe(false)
253+
await remove(root.id)
254+
},
255+
})
256+
})
257+
258+
test("respects maxDepth: returns false when chain is deeper than the limit", async () => {
259+
await using tmp = await tmpdir({ git: true })
260+
await Instance.provide({
261+
directory: tmp.path,
262+
fn: async () => {
263+
const root = await create({ title: "root" })
264+
// Build a chain root → c1 → c2 → c3.
265+
const c1 = await create({ title: "c1", parentID: root.id })
266+
const c2 = await create({ title: "c2", parentID: c1.id })
267+
const c3 = await create({ title: "c3", parentID: c2.id })
268+
// maxDepth: 1 means we walk at most one parent edge from c3, which
269+
// reaches c2 (not yet known), so we cannot confirm root and must
270+
// return false. With the default depth (64), c3 is reachable.
271+
expect(await isDescendantOf(c3.id, root.id, { maxDepth: 1 })).toBe(false)
272+
expect(await isDescendantOf(c3.id, root.id)).toBe(true)
273+
await remove(root.id)
274+
},
275+
})
276+
})
277+
278+
test("cache accumulates confirmed descendants across calls", async () => {
279+
await using tmp = await tmpdir({ git: true })
280+
await Instance.provide({
281+
directory: tmp.path,
282+
fn: async () => {
283+
const root = await create({ title: "root" })
284+
const c1 = await create({ title: "c1", parentID: root.id })
285+
const c2 = await create({ title: "c2", parentID: c1.id })
286+
const cache = new Set<SessionID>([root.id])
287+
// First call walks the full chain c2 → c1 → root and promotes both
288+
// intermediate nodes into the cache.
289+
expect(await isDescendantOf(c2.id, root.id, { cache })).toBe(true)
290+
expect(cache.has(c1.id)).toBe(true)
291+
expect(cache.has(c2.id)).toBe(true)
292+
// A subsequent call for c1 short-circuits via the cache (depth: 0
293+
// forbids any walk; cache hit alone must satisfy the call).
294+
expect(await isDescendantOf(c1.id, root.id, { cache, maxDepth: 0 })).toBe(true)
295+
await remove(root.id)
296+
},
297+
})
298+
})
299+
})

0 commit comments

Comments
 (0)