diff --git a/packages/coding-agent/src/modes/daemon/daemon-supervisor.ts b/packages/coding-agent/src/modes/daemon/daemon-supervisor.ts index c972d1a86d..3fedd166a4 100644 --- a/packages/coding-agent/src/modes/daemon/daemon-supervisor.ts +++ b/packages/coding-agent/src/modes/daemon/daemon-supervisor.ts @@ -338,6 +338,12 @@ function throwIfAdmissionCancelled(admission: SupervisorPromptAdmission | undefi if (admission?.status === "cancelled") throw new PromptAdmissionCancelledError(); } +/** Match catalog sibling topology: relative parents are rooted at each child session file. */ +function canonicalSavedSiblingParentPath(session: Pick): string | undefined { + if (!session.parentSessionPath) return undefined; + return canonicalSessionPath(resolve(dirname(session.path), session.parentSessionPath)); +} + class SupervisorRecoveryCancelledError extends Error { readonly code = "supervisor_recovery_cancelled" as const; } @@ -2083,7 +2089,9 @@ export class DaemonSupervisor { (session) => canonicalSessionPath(session.path) === canonicalSessionPath(createCommand.sessionPath!), ); const targetSummary = target ? summaryForInactiveSession(target) : { sessionId: "new-root", rlmDepth: 0 }; - const reservation = this.summaryNameReservationInput(targetSummary, createCommand.name); + const reservation = target + ? this.savedSiblingNameReservationInput(target, savedSiblings, createCommand.name) + : this.summaryNameReservationInput(targetSummary, createCommand.name); return this.withSessionNameReservation(reservation, async () => { if (target?.parentSessionPath && (target.rlmDepth ?? 0) > 0) { this.assertSavedSiblingNameAvailable(savedSiblings, target, createCommand.name!); @@ -3180,10 +3188,19 @@ export class DaemonSupervisor { const siblings = await this.catalog.siblings(sessionPath, sessionDir ?? this.defaultSessionConfig.sessionDir); const saved = siblings.find((info) => canonicalSessionPath(info.path) === targetPath); if (!saved) throw new Error(`Session not found: ${sessionPath}`); + return this.savedSiblingNameReservationInput(saved, siblings, name); + } + + private savedSiblingNameReservationInput( + saved: SessionInfo, + siblings: readonly SessionInfo[], + name: string, + ): { name: string; depth: number; parentSessionId?: string; parentSessionPath?: string } { + const parentSessionPath = canonicalSavedSiblingParentPath(saved); return { name, depth: saved.rlmDepth ?? siblings.find((sibling) => sibling.rlmDepth !== undefined)?.rlmDepth ?? 0, - parentSessionPath: saved.parentSessionPath, + ...(parentSessionPath ? { parentSessionPath } : {}), }; } @@ -3222,23 +3239,64 @@ export class DaemonSupervisor { private assertSavedSiblingNameAvailable(siblings: SessionInfo[], target: SessionInfo, name: string): void { const setDepth = target.rlmDepth ?? siblings.find((sibling) => sibling.rlmDepth !== undefined)?.rlmDepth ?? 0; + const targetPath = canonicalSessionPath(target.path); + const parentSessionPath = canonicalSavedSiblingParentPath(target); + if (setDepth <= 0 || !parentSessionPath) { + throw new Error("Saved sibling catalog has no direct parent"); + } + + // catalog.siblings() is deliberately bounded: it returns the child set, not + // its parent. Preserve that boundary while supplying a local structural + // anchor to Core03's immutable exact-one-parent catalog validation. Reject + // ambiguous persisted rows rather than letting a malformed saved catalog + // weaken a name reservation. + const parentId = `saved-sibling-parent:${parentSessionPath}`; + const ids = new Set(); + const paths = new Set(); + let targetCount = 0; + for (const sibling of siblings) { + const siblingPath = canonicalSessionPath(sibling.path); + const siblingParentPath = canonicalSavedSiblingParentPath(sibling); + if ( + ids.has(sibling.id) || + paths.has(siblingPath) || + sibling.id === parentId || + siblingParentPath !== parentSessionPath || + (sibling.rlmDepth !== undefined && sibling.rlmDepth !== setDepth) + ) { + throw new Error("Saved sibling catalog is structurally ambiguous"); + } + ids.add(sibling.id); + paths.add(siblingPath); + if (sibling.id === target.id && siblingPath === targetPath) targetCount += 1; + } + if (targetCount !== 1) { + throw new Error("Saved sibling catalog does not contain its target"); + } + assertAgentSessionNameAvailable( - siblings.map((info) => { - const summary = summaryForInactiveSession(info); - return { - id: summary.sessionId, - ...(summary.sessionName ? { name: summary.sessionName } : {}), - depth: setDepth, - status: classifySessionRosterStatus(summary), - ...(summary.parentSessionPath - ? { parentSessionPath: canonicalSessionPath(summary.parentSessionPath) } - : {}), - }; - }), + [ + { + id: parentId, + depth: setDepth - 1, + status: "inactive" as const, + sessionPath: parentSessionPath, + }, + ...siblings.map((info) => { + const summary = summaryForInactiveSession(info); + return { + id: summary.sessionId, + ...(summary.sessionName ? { name: summary.sessionName } : {}), + depth: setDepth, + status: classifySessionRosterStatus(summary), + parentSessionPath, + }; + }), + ], { name, depth: setDepth, - parentSessionPath: target.parentSessionPath ? canonicalSessionPath(target.parentSessionPath) : undefined, + parentSessionPath, ignoreSessionId: target.id, }, ); diff --git a/packages/coding-agent/test/agent-messages.test.ts b/packages/coding-agent/test/agent-messages.test.ts new file mode 100644 index 0000000000..46d8c08deb --- /dev/null +++ b/packages/coding-agent/test/agent-messages.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, it } from "vitest"; +import { + AGENT_FAMILY_REACH_ERROR, + assertAgentFamilyReach, + buildAgentFamilyRoster, +} from "../src/core/agent-messages.js"; + +describe("agent message structural family validation", () => { + it("excludes malformed family edges while retaining catalog-resolved depth-two siblings", () => { + const root = { id: "root", depth: 0, status: "running" as const, sessionPath: "/root" }; + const otherRoot = { id: "other-root", depth: 0, status: "running" as const, sessionPath: "/other" }; + const child = { + id: "child", + depth: 1, + status: "idle" as const, + parentSessionPath: "/root", + sessionPath: "/child", + }; + const malformedRoot = { + id: "malformed-root", + depth: 0, + status: "idle" as const, + parentSessionId: "root", + parentSessionPath: "/root", + }; + const contradictoryChild = { + id: "contradictory-child", + depth: 1, + status: "idle" as const, + parentSessionId: "root", + parentSessionPath: "/other", + }; + const depthSkippingDescendant = { + id: "depth-skipping-descendant", + depth: 2, + status: "idle" as const, + parentSessionId: "root", + parentSessionPath: "/root", + }; + const malformedDeepSiblingA = { + id: "malformed-deep-sibling-a", + depth: 2, + status: "idle" as const, + parentSessionId: "root", + parentSessionPath: "/root", + }; + const malformedDeepSiblingB = { + id: "malformed-deep-sibling-b", + depth: 2, + status: "idle" as const, + parentSessionId: "root", + parentSessionPath: "/root", + }; + const catalog = [ + root, + child, + malformedRoot, + contradictoryChild, + depthSkippingDescendant, + malformedDeepSiblingA, + malformedDeepSiblingB, + ]; + + // A root carrying a parent claim, contradictory dual claims, and a skipped + // depth must not become a direct family edge. + for (const malformed of [malformedRoot, contradictoryChild, depthSkippingDescendant]) { + expect(() => assertAgentFamilyReach(root, malformed, catalog)).toThrow(AGENT_FAMILY_REACH_ERROR); + expect(() => assertAgentFamilyReach(malformed, root, catalog)).toThrow(AGENT_FAMILY_REACH_ERROR); + } + expect(() => assertAgentFamilyReach(otherRoot, contradictoryChild, catalog)).toThrow(AGENT_FAMILY_REACH_ERROR); + + // Two malformed depth-two rows that claim the root are not pseudo-siblings, + // and neither leaks into a roster. + expect(() => assertAgentFamilyReach(malformedDeepSiblingA, malformedDeepSiblingB, catalog)).toThrow( + AGENT_FAMILY_REACH_ERROR, + ); + expect(buildAgentFamilyRoster(malformedDeepSiblingA, catalog).entries).toEqual([]); + expect(buildAgentFamilyRoster(root, catalog).entries.map((entry) => entry.id)).toEqual(["child"]); + + // A real depth-one parent in the supplied catalog restores legitimate + // depth-two siblings without weakening the malformed-edge exclusions above. + const deepParent = { id: "deep-parent", depth: 1, status: "running" as const, sessionPath: "/deep-parent" }; + const deepSiblingA = { + id: "deep-sibling-a", + depth: 2, + status: "idle" as const, + parentSessionId: "deep-parent", + parentSessionPath: "/deep-parent", + }; + const deepSiblingB = { + id: "deep-sibling-b", + depth: 2, + status: "idle" as const, + parentSessionPath: "/deep-parent", + }; + const deepCatalog = [deepParent, deepSiblingA, deepSiblingB]; + expect(() => assertAgentFamilyReach(deepSiblingA, deepSiblingB)).toThrow(AGENT_FAMILY_REACH_ERROR); + expect(assertAgentFamilyReach(deepSiblingA, deepSiblingB, deepCatalog)).toBe("sibling"); + expect(assertAgentFamilyReach(deepSiblingB, deepSiblingA, deepCatalog)).toBe("sibling"); + }); +}); diff --git a/packages/coding-agent/test/daemon-supervisor-lazy-subagents.test.ts b/packages/coding-agent/test/daemon-supervisor-lazy-subagents.test.ts index a7b96197a4..7c90d2fd41 100644 --- a/packages/coding-agent/test/daemon-supervisor-lazy-subagents.test.ts +++ b/packages/coding-agent/test/daemon-supervisor-lazy-subagents.test.ts @@ -314,6 +314,180 @@ describe("daemon supervisor passive subagent topology", () => { ); }); + it("fails closed when a saved sibling catalog has conflicting identity or topology", () => { + const directory = mkdtempSync(join(tmpdir(), "prime-supervisor-saved-sibling-conflict-")); + tempDirs.push(directory); + const parentSessionPath = join(directory, "parent.jsonl"); + const base = { + cwd: directory, + created: new Date(0), + modified: new Date(0), + messageCount: 0, + firstMessage: "", + allMessagesText: "", + parentSessionPath, + rlmDepth: 1, + }; + const target = { ...base, id: "target", path: join(directory, "target.jsonl") }; + const sibling = { ...base, id: "sibling", path: join(directory, "sibling.jsonl"), name: "taken" }; + const supervisor = new DaemonSupervisor(join(directory, "daemon.sock"), { + defaultSessionConfig: { agentDir: directory, cwd: directory }, + descriptorDir: join(directory, "workers"), + }) as unknown as SupervisorInternals; + + for (const conflictingSiblings of [ + [target, { ...sibling, id: target.id }], + [target, { ...sibling, path: target.path }], + [target, { ...sibling, parentSessionPath: join(directory, "other-parent.jsonl") }], + [target, { ...sibling, rlmDepth: 2 }], + ]) { + expect(() => supervisor.assertSavedSiblingNameAvailable(conflictingSiblings, target, "taken")).toThrow( + "Saved sibling catalog is structurally ambiguous", + ); + } + expect(() => supervisor.assertSavedSiblingNameAvailable([sibling], target, "taken")).toThrow( + "Saved sibling catalog does not contain its target", + ); + }); + + it("anchors saved sibling relative parents at their own session files", () => { + const directory = mkdtempSync(join(tmpdir(), "prime-supervisor-relative-saved-siblings-")); + tempDirs.push(directory); + const base = { + cwd: directory, + created: new Date(0), + modified: new Date(0), + messageCount: 0, + firstMessage: "", + allMessagesText: "", + rlmDepth: 1, + }; + const target = { + ...base, + id: "target", + path: join(directory, "children", "target.jsonl"), + parentSessionPath: "../parent.jsonl", + }; + const sibling = { + ...base, + id: "sibling", + path: join(directory, "children", "nested", "sibling.jsonl"), + parentSessionPath: "../../parent.jsonl", + name: "taken", + }; + const supervisor = new DaemonSupervisor(join(directory, "daemon.sock"), { + defaultSessionConfig: { agentDir: directory, cwd: directory }, + descriptorDir: join(directory, "workers"), + }) as unknown as SupervisorInternals; + + expect(() => supervisor.assertSavedSiblingNameAvailable([target, sibling], target, "available")).not.toThrow(); + expect(() => + supervisor.assertSavedSiblingNameAvailable( + [target, { ...sibling, parentSessionPath: "../other-parent.jsonl" }], + target, + "available", + ), + ).toThrow("Saved sibling catalog is structurally ambiguous"); + }); + + it("uses one canonical saved-sibling reservation for equivalent relative parents during create", async () => { + const directory = mkdtempSync(join(tmpdir(), "prime-supervisor-relative-saved-create-reservation-")); + tempDirs.push(directory); + const base = { + cwd: directory, + created: new Date(0), + modified: new Date(0), + messageCount: 0, + firstMessage: "", + allMessagesText: "", + rlmDepth: 1, + }; + const firstPath = join(directory, "children", "first.jsonl"); + const secondPath = join(directory, "children", "nested", "second.jsonl"); + const siblings = [ + { ...base, id: "first", path: firstPath, parentSessionPath: "../parent.jsonl" }, + { ...base, id: "second", path: secondPath, parentSessionPath: "../../parent.jsonl" }, + ]; + let releaseLaunch!: () => void; + const launchGate = new Promise((resolve) => { + releaseLaunch = resolve; + }); + const supervisor = new DaemonSupervisor(join(directory, "daemon.sock"), { + defaultSessionConfig: { agentDir: directory, cwd: directory }, + descriptorDir: join(directory, "workers"), + }) as unknown as SupervisorInternals; + const resident = worker("opened"); + const launchWorker = vi.fn(async () => { + await launchGate; + return resident; + }); + Object.assign(supervisor, { + catalog: { siblings: vi.fn(async () => siblings) }, + launchWorker, + }); + + const first = supervisor.createOrReuseWorker("client", { + type: "create", + name: "shared", + sessionPath: firstPath, + }); + await vi.waitFor(() => expect(launchWorker).toHaveBeenCalledOnce()); + await expect( + supervisor.createOrReuseWorker("client", { type: "create", name: "shared", sessionPath: secondPath }), + ).rejects.toThrow("an agent of that name already exists at depth 1 under this parent"); + releaseLaunch(); + await expect(first).resolves.toBe(resident); + }); + + it("uses one canonical saved-sibling reservation for equivalent relative parents during rename", async () => { + const directory = mkdtempSync(join(tmpdir(), "prime-supervisor-relative-saved-rename-reservation-")); + tempDirs.push(directory); + const base = { + cwd: directory, + created: new Date(0), + modified: new Date(0), + messageCount: 0, + firstMessage: "", + allMessagesText: "", + rlmDepth: 1, + }; + const firstPath = join(directory, "children", "first.jsonl"); + const secondPath = join(directory, "children", "nested", "second.jsonl"); + const siblings = [ + { ...base, id: "first", path: firstPath, parentSessionPath: "../parent.jsonl" }, + { ...base, id: "second", path: secondPath, parentSessionPath: "../../parent.jsonl" }, + ]; + let releaseRename!: () => void; + const renameGate = new Promise((resolve) => { + releaseRename = resolve; + }); + const rename = vi.fn(async () => { + await renameGate; + }); + const supervisor = new DaemonSupervisor(join(directory, "daemon.sock"), { + defaultSessionConfig: { agentDir: directory, cwd: directory }, + descriptorDir: join(directory, "workers"), + }) as unknown as SupervisorInternals; + Object.assign(supervisor, { catalog: { siblings: vi.fn(async () => siblings), rename } }); + const client = { id: "client", attachedActiveSessionIds: new Set() }; + + const first = supervisor.handleCommand(client, { + type: "rename_saved_session", + sessionPath: firstPath, + name: "shared", + }); + await vi.waitFor(() => expect(rename).toHaveBeenCalledOnce()); + await expect( + supervisor.handleCommand(client, { + type: "rename_saved_session", + sessionPath: secondPath, + name: "shared", + }), + ).rejects.toThrow("an agent of that name already exists at depth 1 under this parent"); + releaseRename(); + await expect(first).resolves.toMatchObject({ success: true }); + }); + it("publishes an opening reservation before named create validation awaits", async () => { const directory = mkdtempSync(join(tmpdir(), "prime-supervisor-named-create-race-")); tempDirs.push(directory);