mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 01:50:13 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents split work into tasks. A task can have a parent task. > - Today agents can nest tasks to any depth: parent, child, grandchild, and deeper. > - Deep agent-made trees are hard to follow, and an agent that works on a child task makes grandchildren by default. > - This pull request limits agent-created trees to two levels: a top-level task and its children. > - A request for a grandchild does not fail. The server puts the new task beside the requester and tells the agent. > - Board users keep full control. They can still nest to any depth. Existing deep trees stay valid. > - The pull request also records which task created each new task, so the audit trail stays complete when the parent changes. ## Linked Issues or Issue Description No public issue exists. This is the feature description. Related: #16853 (budgets that include descendant work) also reasons about task trees; this change does not alter its traversal. **Subsystem affected** Issues (tasks): create, child create and move in the issue service and routes. Agent tools, chat suggestions and creation provenance use these paths. **Problem or motivation** Agents create subtasks as deep as they want. An agent that runs on a child task gets that child as the default parent, so its follow-ups become grandchildren. The trees become deep and hard to read. The parent link is also the only record of which task created a follow-up, so that record is lost when the parent changes. **Proposed solution** Limit agent-created trees to a parent and its children. When an agent asks for a grandchild, put the new task under the requester's parent and return `parentAdjusted`. Do not reject the request. Keep board users unrestricted. Record the originating task in a new server-set `origin_issue_id` column. Keep `origin_run_id` as the originating run. **Alternatives considered** Reject grandchild requests with an error. We did not do this, because agents then fail mid-task. Add a database constraint. We did not do this, because existing deep trees must stay valid and nesting can return later. ## What Changed - Add `MAX_AGENT_TASK_HIERARCHY_DEPTH` (2) and the `IssueParentAdjustment` type in `packages/shared`. - Add `resolveNewTaskParent` and `planIssueParentChange` in `server/src/services/issues.ts`. `create`, `createChild` and `update` use the agent limit by default (`parentDepthPolicy: "flatten"`). - The issue create, `/children` and PATCH routes pass `allow_nested` for board users. For agents, the routes check authorization against the parent that the server actually writes. - The requester keeps its behavior when the server moves a task beside it: `blockParentUntilDone` blocks the requester, and the new task inherits the requester's workspace and privacy. - The server never moves a task above a low-trust boundary root that contains the requester. - These subsystems keep any depth, because they find their work by parent: watchdog reviews, monitor recovery, email-channel placement, and routines that a person configured. - An agent move of a task that has children skips only the parent change and returns `reason: "has_children"`. - Add `issues.origin_issue_id` (nullable, `on delete set null`, indexed) in migration `0434`. Only the server sets it. The `createdFrom` detail field, the `createdFromIssueId` filter and task-creation provenance read it first. - Chat prompts now let a chat agent create one task, a task with children, or separate tasks. The conversation is never a parent. - The runner `create_task` tool accepts an optional `parentTaskId`. A chat agent can create a top-level task and then put subtasks under it. The run must be able to read and change the parent. A conversation is never a parent. The depth limit still applies. Project tools map the field to `parentId` through the issue route. The protocol manifest, semantic contracts and eval fixture are regenerated. - Watchdog runs that have an active watchdog keep their requested parent on all create paths, because their access is limited to the watched subtree. - Fix a bug that the test drive found: the chat (MCP) `create_task` ignored the requested `status`, so a `backlog` task became `todo` and started to run. It now uses the same rule as the native tool. - Update `skills/paperclip`, `doc/SPEC-implementation.md`, and add the plan `doc/plans/2026-11-05-two-level-task-hierarchy.md`. ## Verification - `cd server && npx vitest run src/__tests__/agent-task-hierarchy-depth.test.ts src/__tests__/agent-task-hierarchy-depth-routes.test.ts`: 19 new tests for service and route behavior, all pass. - Related suites (64 files: issues service, thread interactions, privacy, watchdogs, provenance, runner tool authority, decisions, routines, project tools): 1208 tests pass. Two tests changed because of the intended behavior and pass now. Two `email-channels.integration` tests fail locally because the test cannot find a bundled skill file from this worktree path. This change does not touch that code. - `packages/db`: migration safety, snapshot drift and journal tests pass. - Typecheck passes for `server`, `packages/shared`, `packages/db`, `cli` and `ui` (local `ui` errors come only from generated Storybook catalog files that are absent). - Local test drive on an isolated instance with real Claude agents: 29 of 29 use cases pass, including an agent on a child task (follow-ups placed beside it, with the notice) and a chat that creates a top-level task with three subtasks. - I did not run the full `pnpm test:run` or `pnpm build` locally. CI runs them. ## Risks - Behavior change for agents: some follow-ups that were grandchildren are now siblings. The parent of the requester is woken when all its children finish, which now includes these tasks. - The runner `create_task` input contract adds one optional field, so the protocol manifest and the pack contract change. The tool output does not change: it shows the actual `parentId`, and the activity log records `parentAdjusted`. - Migration `0434` adds a nullable column, a foreign key and an index. It does not backfill rows. - `IssueCreationSource.run` is now nullable. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edits, local test runs). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
217 lines
9.4 KiB
TypeScript
217 lines
9.4 KiB
TypeScript
import { and, eq, sql } from "drizzle-orm";
|
|
import { issues, type Db } from "@paperclipai/db";
|
|
import { MAX_AGENT_TASK_HIERARCHY_DEPTH, type IssueParentAdjustment } from "@paperclipai/shared";
|
|
|
|
/**
|
|
* `flatten` (the default) applies the agent hierarchy limit: a new parent edge
|
|
* may not make the task deeper than MAX_AGENT_TASK_HIERARCHY_DEPTH. Board users
|
|
* pass `allow_nested` and may build trees of any depth.
|
|
*/
|
|
export type IssueParentDepthPolicy = "flatten" | "allow_nested";
|
|
|
|
/** One issue on the path from a requested parent up to its tree root. */
|
|
export type ParentChainRow = {
|
|
id: string;
|
|
identifier: string | null;
|
|
parentId: string | null;
|
|
conversationAgentId: string | null;
|
|
executionPolicy: unknown;
|
|
};
|
|
|
|
export type ParentPlacement = { effectiveParentId: string | null; adjustment: IssueParentAdjustment | null };
|
|
|
|
// Ancestor walks are not depth-capped: placement needs the real root even for
|
|
// legacy trees deeper than any fixed bound. Both loaders stop on a cycle.
|
|
|
|
/** Root of a low-trust authorization boundary recorded on an issue's execution policy. */
|
|
function lowTrustBoundaryRootIssueId(executionPolicy: unknown): string | null {
|
|
const policy = executionPolicy as { authorizationPolicy?: { trustBoundary?: { rootIssueId?: unknown } | null } | null } | null;
|
|
const root = policy?.authorizationPolicy?.trustBoundary?.rootIssueId;
|
|
return typeof root === "string" && root.length > 0 ? root : null;
|
|
}
|
|
|
|
/**
|
|
* Places a new parent edge given the requested parent's ancestor chain
|
|
* (`chain[0]` is the requested parent, the last entry its root). Under
|
|
* `flatten`, a parent already at the maximum depth is replaced by its ancestor
|
|
* at the allowed depth, which makes the task a sibling of the requester. Never
|
|
* throws for depth.
|
|
*/
|
|
export function placeTaskParent(chain: ParentChainRow[], policy: IssueParentDepthPolicy): ParentPlacement {
|
|
const requested = chain[0];
|
|
if (!requested || policy === "allow_nested") {
|
|
return { effectiveParentId: requested?.id ?? null, adjustment: null };
|
|
}
|
|
// A parent at depth d (root = 1) gives the new task depth d + 1.
|
|
const maxParentDepth = MAX_AGENT_TASK_HIERARCHY_DEPTH - 1;
|
|
if (chain.length <= maxParentDepth) return { effectiveParentId: requested.id, adjustment: null };
|
|
const targetIndex = chain.length - maxParentDepth;
|
|
// Low-trust boundaries are subtrees. Never move a task above the root of a
|
|
// boundary that contains the requester; keep the requested parent instead.
|
|
const boundaryRoots = new Set(chain.flatMap((row) => lowTrustBoundaryRootIssueId(row.executionPolicy) ?? []));
|
|
if (chain.slice(0, Math.max(targetIndex, 0)).some((row) => boundaryRoots.has(row.id))) {
|
|
return { effectiveParentId: requested.id, adjustment: null };
|
|
}
|
|
const target = maxParentDepth > 0 ? chain[targetIndex] : null;
|
|
// Conversations never take new children; legacy trees under one fall back to top level.
|
|
const effective = target && !target.conversationAgentId ? target : null;
|
|
return {
|
|
effectiveParentId: effective?.id ?? null,
|
|
adjustment: {
|
|
reason: "max_agent_depth",
|
|
applied: true,
|
|
requestedParent: { id: requested.id, identifier: requested.identifier },
|
|
effectiveParent: effective ? { id: effective.id, identifier: effective.identifier } : null,
|
|
},
|
|
};
|
|
}
|
|
|
|
/**
|
|
* Callers authorize the parent they request. When depth placement moves a task
|
|
* above that parent, the ancestor is written only if the caller also authorized
|
|
* it and says so with `expectedParentId` (the routes and the runner do). Other
|
|
* callers get standalone work instead: no structural parent, never an edge
|
|
* under an ancestor nobody checked.
|
|
*/
|
|
export function confirmPlacement(placement: ParentPlacement, expectedParentId: string | null | undefined): ParentPlacement {
|
|
if (expectedParentId !== undefined || !placement.adjustment?.applied || placement.effectiveParentId === null) {
|
|
return placement;
|
|
}
|
|
return { effectiveParentId: null, adjustment: { ...placement.adjustment, effectiveParent: null } };
|
|
}
|
|
|
|
/** Loads a requested parent's ancestor chain in one recursive query. Empty when it does not exist. */
|
|
export async function loadParentChain(
|
|
db: Pick<Db, "execute">,
|
|
companyId: string,
|
|
parentId: string,
|
|
): Promise<ParentChainRow[]> {
|
|
const result = await db.execute(sql`
|
|
WITH RECURSIVE chain AS (
|
|
SELECT ${issues.id} AS id, ${issues.identifier} AS identifier, ${issues.parentId} AS parent_id,
|
|
${issues.conversationAgentId} AS conversation_agent_id, ${issues.executionPolicy} AS execution_policy,
|
|
1 AS depth, ARRAY[${issues.id}] AS path
|
|
FROM ${issues}
|
|
WHERE ${issues.companyId} = ${companyId} AND ${issues.id} = ${parentId}
|
|
UNION ALL
|
|
SELECT parent.id, parent.identifier, parent.parent_id, parent.conversation_agent_id, parent.execution_policy,
|
|
chain.depth + 1, chain.path || parent.id
|
|
FROM ${issues} AS parent
|
|
JOIN chain ON parent.id = chain.parent_id
|
|
WHERE parent.company_id = ${companyId}
|
|
AND NOT parent.id = ANY(chain.path)
|
|
)
|
|
SELECT id, identifier, parent_id, conversation_agent_id, execution_policy FROM chain ORDER BY depth
|
|
`);
|
|
const rows = (Array.isArray(result) ? result : (result as { rows?: unknown[] }).rows ?? []) as Array<{
|
|
id: string; identifier: string | null; parent_id: string | null; conversation_agent_id: string | null; execution_policy: unknown;
|
|
}>;
|
|
return rows.map((row) => ({
|
|
id: row.id,
|
|
identifier: row.identifier,
|
|
parentId: row.parent_id,
|
|
conversationAgentId: row.conversation_agent_id,
|
|
executionPolicy: row.execution_policy,
|
|
}));
|
|
}
|
|
|
|
/** Loads an ancestor chain one issue at a time through a caller-supplied, company-scoped reader. */
|
|
export async function loadParentChainWith(
|
|
load: (id: string) => Promise<ParentChainRow | null>,
|
|
parentId: string,
|
|
): Promise<ParentChainRow[]> {
|
|
const chain: ParentChainRow[] = [];
|
|
const visited = new Set<string>();
|
|
let nextId: string | null = parentId;
|
|
while (nextId && !visited.has(nextId)) {
|
|
visited.add(nextId);
|
|
const row = await load(nextId);
|
|
if (!row) break;
|
|
chain.push(row);
|
|
nextId = row.parentId;
|
|
}
|
|
return chain;
|
|
}
|
|
|
|
/**
|
|
* Resolves the parent a new edge should use. Unknown parents pass through
|
|
* unchanged so the caller's own existence and authorization checks still apply.
|
|
*/
|
|
export async function resolveNewTaskParent(
|
|
db: Pick<Db, "execute">,
|
|
companyId: string,
|
|
requestedParentId: string | null | undefined,
|
|
policy: IssueParentDepthPolicy = "flatten",
|
|
): Promise<ParentPlacement> {
|
|
if (!requestedParentId || policy === "allow_nested") {
|
|
return { effectiveParentId: requestedParentId ?? null, adjustment: null };
|
|
}
|
|
const chain = await loadParentChain(db, companyId, requestedParentId);
|
|
if (chain.length === 0) return { effectiveParentId: requestedParentId, adjustment: null };
|
|
return placeTaskParent(chain, policy);
|
|
}
|
|
|
|
/**
|
|
* Plans a parent change for an existing issue from the requested parent's
|
|
* chain. Under `flatten`, a move into a child is placed beside it, and a move
|
|
* of an issue that has its own children is skipped (`parentId: undefined`),
|
|
* since either would create grandchildren. Never throws for depth.
|
|
*/
|
|
export function planParentChange(
|
|
existing: { parentId: string | null },
|
|
chain: ParentChainRow[],
|
|
requestedParentId: string,
|
|
hasChildren: boolean,
|
|
currentParent: { id: string; identifier: string | null } | null,
|
|
policy: IssueParentDepthPolicy,
|
|
): { parentId: string | null | undefined; adjustment: IssueParentAdjustment | null } {
|
|
const requested = chain[0];
|
|
if (policy === "allow_nested" || requestedParentId === existing.parentId || !requested) {
|
|
return { parentId: requestedParentId, adjustment: null };
|
|
}
|
|
if (hasChildren) {
|
|
return {
|
|
parentId: undefined,
|
|
adjustment: {
|
|
reason: "has_children",
|
|
applied: false,
|
|
requestedParent: { id: requested.id, identifier: requested.identifier },
|
|
effectiveParent: currentParent,
|
|
},
|
|
};
|
|
}
|
|
const placement = placeTaskParent(chain, "flatten");
|
|
return { parentId: placement.effectiveParentId, adjustment: placement.adjustment };
|
|
}
|
|
|
|
/** Database-backed {@link planParentChange} for the issue service. */
|
|
export async function planIssueParentChange(
|
|
db: Pick<Db, "execute" | "select">,
|
|
existing: { id: string; companyId: string; parentId: string | null },
|
|
requestedParentId: string,
|
|
policy: IssueParentDepthPolicy = "flatten",
|
|
) {
|
|
if (policy === "allow_nested" || requestedParentId === existing.parentId) {
|
|
return { parentId: requestedParentId as string | null | undefined, adjustment: null as IssueParentAdjustment | null };
|
|
}
|
|
const chain = await loadParentChain(db, existing.companyId, requestedParentId);
|
|
const [child] = await db
|
|
.select({ id: issues.id })
|
|
.from(issues)
|
|
.where(and(eq(issues.companyId, existing.companyId), eq(issues.parentId, existing.id)))
|
|
.limit(1);
|
|
const [currentParent] = existing.parentId && child ? await db
|
|
.select({ id: issues.id, identifier: issues.identifier })
|
|
.from(issues)
|
|
.where(and(eq(issues.companyId, existing.companyId), eq(issues.id, existing.parentId))) : [];
|
|
return planParentChange(existing, chain, requestedParentId, Boolean(child), currentParent ?? null, policy);
|
|
}
|
|
|
|
/** Adds `parentAdjusted` to a create/update result only when the parent was adjusted. */
|
|
export function withParentAdjustment<T extends object>(
|
|
issue: T,
|
|
adjustment: IssueParentAdjustment | null,
|
|
): T & { parentAdjusted?: IssueParentAdjustment } {
|
|
return adjustment ? { ...issue, parentAdjusted: adjustment } : issue;
|
|
}
|