From b6b900615c5fc83ddcb954fb15ec7bc3072b4002 Mon Sep 17 00:00:00 2001 From: Ulas Can Zorer Date: Mon, 18 May 2026 22:52:43 +0200 Subject: [PATCH 1/2] fix: align context bundle traversal with effective-dep-graph semantics [MYMR-207] --- lib/context/_core/agent.ts | 27 +++++-- lib/context/_core/planning.ts | 32 ++++++-- lib/context/_core/review.ts | 32 ++++++-- lib/graph/effective-deps.ts | 144 ++++++++++++++++++++++++++++----- tests/context/agent.test.ts | 35 ++++++++ tests/context/planning.test.ts | 34 ++++++++ tests/context/review.test.ts | 37 +++++++++ 7 files changed, 304 insertions(+), 37 deletions(-) diff --git a/lib/context/_core/agent.ts b/lib/context/_core/agent.ts index cf03100..f9dce56 100644 --- a/lib/context/_core/agent.ts +++ b/lib/context/_core/agent.ts @@ -1,6 +1,9 @@ import "server-only"; -import { getDependencyChain, getDownstreamTx } from "@/lib/data/traversal"; +import { + buildDepAdjacency, + walkEffectiveDepsBounded, +} from "@/lib/graph/effective-deps"; import { fetchDependencyTasks, fetchEdgeNotesBySource, @@ -39,13 +42,27 @@ export async function buildAgentContext( const assignees = task.assignees; const links = task.links; - const downstream = await getDownstreamTx(tx, taskId, 2); - - const [deps, upstreamEdgeNotes] = await Promise.all([ - getDependencyChain(taskId, task.projectId, 2, tx), + const [{ adj, taskStatus }, upstreamEdgeNotes] = await Promise.all([ + buildDepAdjacency(task.projectId, tx), fetchEdgeNotesBySource(task.projectId, taskId, tx), ]); + const reverseAdj = new Map(); + for (const [src, targets] of adj) { + for (const t of targets) { + const list = reverseAdj.get(t) ?? []; + list.push(src); + reverseAdj.set(t, list); + } + } + + const deps = [ + ...walkEffectiveDepsBounded(taskId, adj, taskStatus, 2).keys(), + ].map((id) => ({ id })); + const downstream = [ + ...walkEffectiveDepsBounded(taskId, reverseAdj, taskStatus, 2).keys(), + ].map((id) => ({ id })); + const prLink = links.find((l) => l.kind === "pull_request"); const headerLines: string[] = [ diff --git a/lib/context/_core/planning.ts b/lib/context/_core/planning.ts index 1c3fe56..2db0261 100644 --- a/lib/context/_core/planning.ts +++ b/lib/context/_core/planning.ts @@ -1,6 +1,9 @@ import "server-only"; -import { getDependencyChain, getDownstreamTx } from "@/lib/data/traversal"; +import { + buildDepAdjacency, + walkEffectiveDepsBounded, +} from "@/lib/graph/effective-deps"; import { fetchDependencyTasks, fetchEdgeNotesBySource, @@ -31,7 +34,23 @@ export async function buildPlanningContext( ): Promise { return withUserContext(ctx.userId, async (tx) => { const task = await getTaskFullTx(tx, taskId); - const downstream = await getDownstreamTx(tx, taskId, 2); + + const { adj, taskStatus } = await buildDepAdjacency(task.projectId, tx); + const reverseAdj = new Map(); + for (const [src, targets] of adj) { + for (const t of targets) { + const list = reverseAdj.get(t) ?? []; + list.push(src); + reverseAdj.set(t, list); + } + } + const deps = [ + ...walkEffectiveDepsBounded(taskId, adj, taskStatus, 2).keys(), + ].map((id) => ({ id })); + const downstream = [ + ...walkEffectiveDepsBounded(taskId, reverseAdj, taskStatus, 2).keys(), + ].map((id) => ({ id })); + const project = await getProjectHeader(task.projectId, tx); if (!project) { console.error("Task has no joinable project", { @@ -78,10 +97,11 @@ export async function buildPlanningContext( ); } - const [deps, upstreamEdgeNotes] = await Promise.all([ - getDependencyChain(taskId, task.projectId, 2, tx), - fetchEdgeNotesBySource(task.projectId, taskId, tx), - ]); + const upstreamEdgeNotes = await fetchEdgeNotesBySource( + task.projectId, + taskId, + tx, + ); if (deps.length > 0) { const prereqLines: string[] = []; diff --git a/lib/context/_core/review.ts b/lib/context/_core/review.ts index 0e0ffe0..278d070 100644 --- a/lib/context/_core/review.ts +++ b/lib/context/_core/review.ts @@ -1,6 +1,9 @@ import "server-only"; -import { getDependencyChain, getDownstreamTx } from "@/lib/data/traversal"; +import { + buildDepAdjacency, + walkEffectiveDepsBounded, +} from "@/lib/graph/effective-deps"; import { fetchDependencyTasks, fetchEdgeNotesBySource, @@ -60,7 +63,23 @@ export async function buildReviewContext( ): Promise { return withUserContext(ctx.userId, async (tx) => { const task = await getTaskFullTx(tx, taskId); - const downstream = await getDownstreamTx(tx, taskId, 2); + + const { adj, taskStatus } = await buildDepAdjacency(task.projectId, tx); + const reverseAdj = new Map(); + for (const [src, targets] of adj) { + for (const t of targets) { + const list = reverseAdj.get(t) ?? []; + list.push(src); + reverseAdj.set(t, list); + } + } + const deps = [ + ...walkEffectiveDepsBounded(taskId, adj, taskStatus, 2).keys(), + ].map((id) => ({ id })); + const downstream = [ + ...walkEffectiveDepsBounded(taskId, reverseAdj, taskStatus, 2).keys(), + ].map((id) => ({ id })); + const project = await getProjectHeader(task.projectId, tx); if (!project) { console.error("Task has no joinable project", { @@ -76,10 +95,11 @@ export async function buildReviewContext( const taskRef = task.taskRef; const links = task.links; - const [deps, upstreamEdgeNotes] = await Promise.all([ - getDependencyChain(taskId, task.projectId, 2, tx), - fetchEdgeNotesBySource(task.projectId, taskId, tx), - ]); + const upstreamEdgeNotes = await fetchEdgeNotesBySource( + task.projectId, + taskId, + tx, + ); const prLink = links.find((l) => l.kind === "pull_request"); diff --git a/lib/graph/effective-deps.ts b/lib/graph/effective-deps.ts index b4b66c5..090e2d9 100644 --- a/lib/graph/effective-deps.ts +++ b/lib/graph/effective-deps.ts @@ -33,26 +33,31 @@ export type EffectiveDepGraph = { }; /** - * Build the effective dependency graph for a project. + * Load the raw dependency-traversal substrate for a project: the + * `depends_on` adjacency map, the full task-id → status map (all tasks, + * cancelled included so the walks can pass through them), and the + * active-only task info map (cancelled excluded). * - * Treats cancelled tasks as transparent: walks through them to find the - * nearest active prerequisite, but excludes them from the result graph. - * Used by getBlockedTasks, getCriticalPath, and deriveTaskStatesSlim (which - * in turn backs getReadyTasks and getPlannableTasks) so they all share - * consistent transitive-aware semantics. + * This is the exact substrate `buildEffectiveDepGraph` needs and the + * depth-bounded bundle walks (`walkEffectiveDepsBounded`) also need, kept + * as one source of truth so the analyze tools and the context bundles + * derive their dependency graphs from identical data. * * @param projectId - UUID of the project. - * @param conn - Drizzle client or transaction handle. Callers running under a - * `withUserContext` transaction must pass the active `tx` so the underlying - * reads participate in the same RLS-scoped frame; standalone callers pass - * the bare `db` pool client (data-layer scope only — boundary enforced by - * the lint rule on this directory). - * @returns The effective dependency graph (active-only nodes, transitive edges). + * @param conn - Drizzle client or transaction handle. Callers running under + * a `withUserContext` transaction must pass the active `tx` so the reads + * participate in the same RLS-scoped frame. + * @returns The adjacency map, the all-tasks status map, and the active-task + * info map. */ -export async function buildEffectiveDepGraph( +export async function buildDepAdjacency( projectId: string, conn: Conn, -): Promise { +): Promise<{ + adj: Map; + taskStatus: Map; + activeTasks: Map; +}> { const allTasks = await listTasksForGraph(projectId, conn); const activeTasks = new Map(); @@ -70,24 +75,57 @@ export async function buildEffectiveDepGraph( } } + const adj = new Map(); if (allTasks.length === 0) { - return { - activeTasks, - effectiveDeps: new Map(), - effectiveDependents: new Map(), - }; + return { adj, taskStatus, activeTasks }; } const taskIds = allTasks.map((t) => t.id); const dependsOnEdges = await listDependsOnEdges(taskIds, conn); - const adj = new Map(); for (const e of dependsOnEdges) { const list = adj.get(e.sourceTaskId) ?? []; list.push(e.targetTaskId); adj.set(e.sourceTaskId, list); } + return { adj, taskStatus, activeTasks }; +} + +/** + * Build the effective dependency graph for a project. + * + * Treats cancelled tasks as transparent: walks through them to find the + * nearest active prerequisite, but excludes them from the result graph. + * Used by getBlockedTasks, getCriticalPath, and deriveTaskStatesSlim (which + * in turn backs getReadyTasks and getPlannableTasks) so they all share + * consistent transitive-aware semantics. + * + * @param projectId - UUID of the project. + * @param conn - Drizzle client or transaction handle. Callers running under a + * `withUserContext` transaction must pass the active `tx` so the underlying + * reads participate in the same RLS-scoped frame; standalone callers pass + * the bare `db` pool client (data-layer scope only — boundary enforced by + * the lint rule on this directory). + * @returns The effective dependency graph (active-only nodes, transitive edges). + */ +export async function buildEffectiveDepGraph( + projectId: string, + conn: Conn, +): Promise { + const { adj, taskStatus, activeTasks } = await buildDepAdjacency( + projectId, + conn, + ); + + if (taskStatus.size === 0) { + return { + activeTasks, + effectiveDeps: new Map(), + effectiveDependents: new Map(), + }; + } + const effectiveDeps = new Map>(); for (const activeId of activeTasks.keys()) { effectiveDeps.set(activeId, walkEffectiveDeps(activeId, adj, taskStatus)); @@ -145,3 +183,69 @@ function walkEffectiveDeps( return result; } + +/** + * Walk forward from an active source over the effective dependency graph, + * stopping at maxDepth active "walls". Cancelled tasks are transparent and + * depth-free: the walk passes through them without consuming a depth slot, + * so A -> B(cancelled) -> C(active) yields C at effective depth 1. + * + * BFS by effective depth: the first time an active task is seen is at its + * minimum effective depth, matching the shallowest-depth behaviour of the + * old recursive CTE. The cancelled-frontier is drained at the current + * active depth before BFS advances, so a chain of any number of cancelled + * middles stays depth-free. + * + * @param source - Starting active task id (not included in the result). + * @param adj - source -> targets adjacency map for depends_on edges. + * @param taskStatus - task id -> status map for ALL project tasks. + * @param maxDepth - Maximum number of active hops to include (e.g. 2). + * @returns Map of active-task-id -> effective depth (1..maxDepth). + */ +export function walkEffectiveDepsBounded( + source: string, + adj: Map, + taskStatus: Map, + maxDepth: number, +): Map { + const result = new Map(); + // Seed `visited` with `source` so a cycle back to it never records the + // source itself (the result is the set of deps, source-exclusive). + const visited = new Set([source]); + const visitedCancelled = new Set(); + let queue: { id: string; depth: number }[] = [{ id: source, depth: 0 }]; + + while (queue.length > 0) { + const next: { id: string; depth: number }[] = []; + + for (const node of queue) { + // Drain the cancelled frontier at this active depth before advancing, + // so passing through cancelled middles never consumes a depth slot. + const cancelledFrontier: string[] = [node.id]; + while (cancelledFrontier.length > 0) { + const cur = cancelledFrontier.pop()!; + for (const target of adj.get(cur) ?? []) { + const status = taskStatus.get(target); + if (status === undefined) continue; + if (status === "cancelled") { + if (visitedCancelled.has(target)) continue; + visitedCancelled.add(target); + cancelledFrontier.push(target); + continue; + } + // Active wall at active depth node.depth + 1. + const wallDepth = node.depth + 1; + if (wallDepth > maxDepth) continue; + if (visited.has(target)) continue; + visited.add(target); + result.set(target, wallDepth); + next.push({ id: target, depth: wallDepth }); + } + } + } + + queue = next; + } + + return result; +} diff --git a/tests/context/agent.test.ts b/tests/context/agent.test.ts index e3535e3..edcea55 100644 --- a/tests/context/agent.test.ts +++ b/tests/context/agent.test.ts @@ -39,6 +39,41 @@ describe("buildAgentContext under app_user", () => { expect(result).toContain("Parent task"); }); + test("cancelled middle is transparent: C surfaces, B does not", async () => { + // A depends_on B(cancelled) depends_on C(active). The bundle for A must + // show C as a prerequisite (reached through the cancelled middle at + // effective depth 1) and must never list B. + const fx = await seedUserOrgProject("agent-ctx-cancel"); + const sr = serviceRoleConnect(); + let aTaskId: string; + try { + const [a] = await sr<{ id: string }[]>` + INSERT INTO tasks (project_id, title, sequence_number, description) + VALUES (${fx.projectId}, 'Source task A', 1, 'root') + RETURNING id`; + const [b] = await sr<{ id: string }[]>` + INSERT INTO tasks (project_id, title, sequence_number, description, status) + VALUES (${fx.projectId}, 'Cancelled middle B', 2, 'skipped', 'cancelled') + RETURNING id`; + const [c] = await sr<{ id: string }[]>` + INSERT INTO tasks (project_id, title, sequence_number, description) + VALUES (${fx.projectId}, 'Active wall C', 3, 'the real blocker') + RETURNING id`; + await sr`INSERT INTO task_edges (source_task_id, target_task_id, edge_type) + VALUES (${a.id}, ${b.id}, 'depends_on')`; + await sr`INSERT INTO task_edges (source_task_id, target_task_id, edge_type) + VALUES (${b.id}, ${c.id}, 'depends_on')`; + aTaskId = a.id; + } finally { + await sr.end({ timeout: 5 }); + } + + const ctx = makeAuthContext(fx.userId); + const result = await buildAgentContext(ctx, aTaskId); + expect(result).toContain("Active wall C"); + expect(result).not.toContain("Cancelled middle B"); + }); + test("rejects cross-team callers (RLS isolation)", async () => { const fxA = await seedUserOrgProject("agent-ctx-a"); const fxB = await seedUserOrgProject("agent-ctx-b"); diff --git a/tests/context/planning.test.ts b/tests/context/planning.test.ts index bc73e7e..55b6b1c 100644 --- a/tests/context/planning.test.ts +++ b/tests/context/planning.test.ts @@ -36,6 +36,40 @@ describe("buildPlanningContext under app_user", () => { expect(result).toContain("Parent task"); }); + test("cancelled middle is transparent: C surfaces, B does not", async () => { + // A depends_on B(cancelled) depends_on C(active). The planning bundle + // for A must show C as a prerequisite and never list B. + const fx = await seedUserOrgProject("planning-ctx-cancel"); + const sr = serviceRoleConnect(); + let aTaskId: string; + try { + const [a] = await sr<{ id: string }[]>` + INSERT INTO tasks (project_id, title, sequence_number, description) + VALUES (${fx.projectId}, 'Source task A', 1, 'root') + RETURNING id`; + const [b] = await sr<{ id: string }[]>` + INSERT INTO tasks (project_id, title, sequence_number, description, status) + VALUES (${fx.projectId}, 'Cancelled middle B', 2, 'skipped', 'cancelled') + RETURNING id`; + const [c] = await sr<{ id: string }[]>` + INSERT INTO tasks (project_id, title, sequence_number, description) + VALUES (${fx.projectId}, 'Active wall C', 3, 'the real blocker') + RETURNING id`; + await sr`INSERT INTO task_edges (source_task_id, target_task_id, edge_type) + VALUES (${a.id}, ${b.id}, 'depends_on')`; + await sr`INSERT INTO task_edges (source_task_id, target_task_id, edge_type) + VALUES (${b.id}, ${c.id}, 'depends_on')`; + aTaskId = a.id; + } finally { + await sr.end({ timeout: 5 }); + } + + const ctx = makeAuthContext(fx.userId); + const result = await buildPlanningContext(ctx, aTaskId); + expect(result).toContain("Active wall C"); + expect(result).not.toContain("Cancelled middle B"); + }); + test("rejects cross-team callers (RLS isolation under app_user)", async () => { const fxA = await seedUserOrgProject("planning-ctx-a"); const fxB = await seedUserOrgProject("planning-ctx-b"); diff --git a/tests/context/review.test.ts b/tests/context/review.test.ts index 3eec754..5f79640 100644 --- a/tests/context/review.test.ts +++ b/tests/context/review.test.ts @@ -35,6 +35,43 @@ describe("buildReviewContext under app_user", () => { expect(result).toContain("execution body"); }); + test("cancelled middle is transparent: C surfaces, B does not", async () => { + // A depends_on B(cancelled) depends_on C(active). A is in_review with a + // plan/execution record (mirrors the populated-context test setup); the + // review bundle must show C as a prerequisite and never list B. + const fx = await seedUserOrgProject("review-ctx-cancel"); + const sr = serviceRoleConnect(); + let aTaskId: string; + try { + const [a] = await sr<{ id: string }[]>` + INSERT INTO tasks (project_id, title, sequence_number, status, + implementation_plan, execution_record) + VALUES (${fx.projectId}, 'Source task A', 1, 'in_review', + 'plan body', 'execution body') + RETURNING id`; + const [b] = await sr<{ id: string }[]>` + INSERT INTO tasks (project_id, title, sequence_number, status) + VALUES (${fx.projectId}, 'Cancelled middle B', 2, 'cancelled') + RETURNING id`; + const [c] = await sr<{ id: string }[]>` + INSERT INTO tasks (project_id, title, sequence_number) + VALUES (${fx.projectId}, 'Active wall C', 3) + RETURNING id`; + await sr`INSERT INTO task_edges (source_task_id, target_task_id, edge_type) + VALUES (${a.id}, ${b.id}, 'depends_on')`; + await sr`INSERT INTO task_edges (source_task_id, target_task_id, edge_type) + VALUES (${b.id}, ${c.id}, 'depends_on')`; + aTaskId = a.id; + } finally { + await sr.end({ timeout: 5 }); + } + + const ctx = makeAuthContext(fx.userId); + const result = await buildReviewContext(ctx, aTaskId); + expect(result).toContain("Active wall C"); + expect(result).not.toContain("Cancelled middle B"); + }); + test("rejects cross-team callers (RLS isolation under app_user)", async () => { const fxA = await seedUserOrgProject("review-ctx-a"); const fxB = await seedUserOrgProject("review-ctx-b"); From 5121eecefc15e12e86b175a2961659fe24867bf6 Mon Sep 17 00:00:00 2001 From: Ulas Can Zorer Date: Mon, 18 May 2026 23:06:18 +0200 Subject: [PATCH 2/2] refactor: drop orphaned getDependencyChain wrapper [MYMR-207] --- lib/data/traversal.ts | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/lib/data/traversal.ts b/lib/data/traversal.ts index 32b6188..a2772c6 100644 --- a/lib/data/traversal.ts +++ b/lib/data/traversal.ts @@ -4,7 +4,6 @@ import { and, eq, sql } from "drizzle-orm"; import { type Conn } from "@/lib/db/raw"; import { withUserContext, type Tx } from "@/lib/db/rls"; import { tasks, projects, taskEdges } from "@/lib/db/schema"; -import { fetchDependencyChain } from "@/lib/db/raw/fetch-dependency-chain"; import { fetchDownstream } from "@/lib/db/raw/fetch-downstream"; import { asIdentifier, composeTaskRef } from "@/lib/graph/identifier"; import { buildEffectiveDepGraph } from "@/lib/graph/effective-deps"; @@ -47,39 +46,6 @@ export async function getAncestors( return [{ id: project.id, type: "project", title: project.title }]; } -// --------------------------------------------------------------------------- -// Dependency chain — internal helper (recursive CTE) -// --------------------------------------------------------------------------- - -/** A task in a dependency chain with depth. */ -type DependencyNode = { - id: string; - depth: number; -}; - -/** - * Follow `depends_on` edges recursively up to maxDepth. Internal — - * caller asserted task access first. - * - * Defense-in-depth: every step joins `tasks` and filters on `projectId` so - * a stale or hand-crafted cross-project edge cannot pull tasks from another - * project (and therefore another team) into the result. - * - * @param taskId - UUID of the starting task. - * @param projectId - UUID of the project the starting task belongs to. - * @param maxDepth - Maximum traversal depth (default 10). - * @param conn - RLS-scoped {@link Conn} from an active `withUserContext` frame. - * @returns Array of dependency tasks with depth. - */ -export async function getDependencyChain( - taskId: string, - projectId: string, - maxDepth = 10, - conn: Conn, -): Promise { - return fetchDependencyChain(conn, taskId, projectId, maxDepth); -} - // --------------------------------------------------------------------------- // Connected tasks — internal helper (1-hop neighbors) // ---------------------------------------------------------------------------