Skip to content

Commit f5cfc62

Browse files
NagyViktNagyVikt
andauthored
fix(plan): stabilize claim recovery and compact listing (#560)
Co-authored-by: NagyVikt <nagy.viktordp@gmail.com>
1 parent db63e96 commit f5cfc62

10 files changed

Lines changed: 286 additions & 22 deletions

File tree

apps/mcp-server/src/tools/plan.ts

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -171,18 +171,19 @@ function recoveryDetailsForClaimFailure(
171171
store: MemoryStore,
172172
code:
173173
| 'PLAN_SUBTASK_NOT_FOUND'
174+
| 'PLAN_SUBTASK_STALE'
174175
| 'PLAN_SUBTASK_DEPS_UNMET'
175176
| 'PLAN_SUBTASK_NOT_AVAILABLE'
176177
| 'PLAN_ARCHIVED'
177178
| 'CLAIM_TAKEOVER_RECOMMENDED'
178179
| 'CLAIM_HELD_BY_ACTIVE_OWNER',
179180
args: { plan_slug: string; subtask_index: number; repo_root?: string },
180181
): Record<string, unknown> {
181-
if (code !== 'PLAN_SUBTASK_NOT_AVAILABLE') return {};
182+
if (code !== 'PLAN_SUBTASK_NOT_AVAILABLE' && code !== 'PLAN_SUBTASK_STALE') return {};
182183
try {
183184
const plan = listPlans(store, {
184185
...(args.repo_root !== undefined ? { repo_root: args.repo_root } : {}),
185-
limit: 200,
186+
limit: 2000,
186187
}).find((candidate) => candidate.plan_slug === args.plan_slug);
187188
if (!plan) return {};
188189
const candidates = plan.next_available
@@ -421,18 +422,20 @@ export function register(server: McpServer, ctx: ToolContext): void {
421422
// bad to good without requiring an operator to call colony plan
422423
// close manually.
423424
sweepCompletedPlansForAutoArchive(store, args.repo_root);
425+
const limit = args.limit ?? 10;
424426
const plans = listPlans(store, {
425427
...(args.repo_root !== undefined ? { repo_root: args.repo_root } : {}),
426428
...(args.only_with_available_subtasks !== undefined
427429
? { only_with_available_subtasks: args.only_with_available_subtasks }
428430
: {}),
429431
...(args.capability_match !== undefined ? { capability_match: args.capability_match } : {}),
430-
...(args.limit !== undefined ? { limit: args.limit } : {}),
432+
limit: 2000,
431433
});
432434
const includeUnpublished = args.include_unpublished ?? true;
433435
const merged = includeUnpublished ? mergeUnpublishedDiskPlans(plans, args.repo_root) : plans;
434436
const detail = args.detail ?? 'compact';
435-
const payload = detail === 'full' ? merged : merged.map(toCompactPlan);
437+
const limited = merged.slice(0, limit);
438+
const payload = detail === 'full' ? limited : limited.map(toCompactPlan);
436439
return { content: [{ type: 'text', text: JSON.stringify(payload) }] };
437440
}),
438441
);
@@ -648,6 +651,7 @@ export type ClaimPlanSubtaskArgs = {
648651
subtask_index: number;
649652
session_id: string;
650653
agent: string;
654+
repo_root?: string | undefined;
651655
};
652656

653657
export type ClaimPlanSubtaskResult =
@@ -657,6 +661,7 @@ export type ClaimPlanSubtaskResult =
657661
ok: false;
658662
code:
659663
| 'PLAN_SUBTASK_NOT_FOUND'
664+
| 'PLAN_SUBTASK_STALE'
660665
| 'PLAN_SUBTASK_DEPS_UNMET'
661666
| 'PLAN_SUBTASK_NOT_AVAILABLE'
662667
| 'CLAIM_TAKEOVER_RECOMMENDED'
@@ -686,7 +691,22 @@ export function attemptClaimPlanSubtask(
686691
session_id: args.session_id,
687692
});
688693
if (archived) return { ok: false, ...archived };
689-
return { ok: false, code: 'PLAN_SUBTASK_NOT_FOUND', message: `no sub-task at ${branch}` };
694+
const planExists = listPlans(store, {
695+
...(args.repo_root !== undefined ? { repo_root: args.repo_root } : {}),
696+
limit: 2000,
697+
}).some((candidate) => candidate.plan_slug === args.plan_slug);
698+
if (planExists) {
699+
return {
700+
ok: false,
701+
code: 'PLAN_SUBTASK_STALE',
702+
message: `stale sub-task pointer: no sub-task at ${branch}`,
703+
};
704+
}
705+
return {
706+
ok: false,
707+
code: 'PLAN_SUBTASK_NOT_FOUND',
708+
message: `unknown plan or sub-task: no sub-task at ${branch}`,
709+
};
690710
}
691711

692712
const allTasks = store.storage.listTasks(2000);
@@ -720,7 +740,7 @@ export function attemptClaimPlanSubtask(
720740
const fresh = readSubtaskByBranch(store, branch);
721741
if (!fresh) {
722742
const err: CodedError = new Error(`no sub-task at ${branch}`);
723-
err.__code = 'PLAN_SUBTASK_NOT_AVAILABLE';
743+
err.__code = 'PLAN_SUBTASK_STALE';
724744
throw err;
725745
}
726746
if (fresh.info.status !== 'available') {
@@ -802,6 +822,7 @@ export function attemptClaimPlanSubtask(
802822
const code = (err as CodedError).__code;
803823
if (
804824
code === 'PLAN_SUBTASK_NOT_AVAILABLE' ||
825+
code === 'PLAN_SUBTASK_STALE' ||
805826
code === 'CLAIM_TAKEOVER_RECOMMENDED' ||
806827
code === 'CLAIM_HELD_BY_ACTIVE_OWNER'
807828
) {

apps/mcp-server/src/tools/ready-queue.ts

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,7 @@ async function maybeAutoClaim(
330330
const claimResult = attemptClaimPlanSubtask(store, {
331331
plan_slug: claim_args.plan_slug,
332332
subtask_index: claim_args.subtask_index,
333+
repo_root: claim_args.repo_root,
333334
session_id: caller.session_id,
334335
agent: caller.agent,
335336
});
@@ -424,24 +425,29 @@ export async function buildReadyForAgent(
424425
]),
425426
);
426427
const role = actorRole(store, { agent: args.agent, session_id: args.session_id });
428+
const liveSubtaskBranches = new Set(
429+
allTasks.map((task) => subtaskBranchKey(task.repo_root, task.branch)),
430+
);
427431
const quotaRelays = quotaRelayReadyItems(store, args, plans, allTasks);
428432
const available = filterReadyForExecutor(
429433
plans.flatMap((plan) =>
430-
plan.next_available.map((subtask) =>
431-
rankSubtask(store, {
432-
plan_slug: plan.plan_slug,
433-
repo_root: plan.repo_root,
434-
subtask,
435-
session_id: args.session_id,
436-
agent: args.agent,
437-
profile,
438-
parent_plan_created_by: tasksById.get(plan.spec_task_id)?.created_by ?? null,
439-
created_at: tasksById.get(subtask.task_id)?.created_at ?? plan.created_at,
440-
proposal_status: tasksById.get(subtask.task_id)?.proposal_status ?? null,
441-
reason: 'ready_high_score',
442-
current_claim: false,
443-
}),
444-
),
434+
plan.next_available
435+
.filter((subtask) => hasLiveSubtaskBranch(liveSubtaskBranches, plan, subtask))
436+
.map((subtask) =>
437+
rankSubtask(store, {
438+
plan_slug: plan.plan_slug,
439+
repo_root: plan.repo_root,
440+
subtask,
441+
session_id: args.session_id,
442+
agent: args.agent,
443+
profile,
444+
parent_plan_created_by: tasksById.get(plan.spec_task_id)?.created_by ?? null,
445+
created_at: tasksById.get(subtask.task_id)?.created_at ?? plan.created_at,
446+
proposal_status: tasksById.get(subtask.task_id)?.proposal_status ?? null,
447+
reason: 'ready_high_score',
448+
current_claim: false,
449+
}),
450+
),
445451
),
446452
role,
447453
);
@@ -452,6 +458,7 @@ export async function buildReadyForAgent(
452458
(subtask) =>
453459
subtask.status === 'claimed' && subtask.claimed_by_session_id === args.session_id,
454460
)
461+
.filter((subtask) => hasLiveSubtaskBranch(liveSubtaskBranches, plan, subtask))
455462
.map((subtask) =>
456463
rankSubtask(store, {
457464
plan_slug: plan.plan_slug,
@@ -631,6 +638,20 @@ function specRootSetupIssue(repoRoot: string | undefined): SpecRootSetupIssue |
631638
};
632639
}
633640

641+
function hasLiveSubtaskBranch(
642+
liveSubtaskBranches: Set<string>,
643+
plan: Pick<PlanInfo, 'repo_root' | 'plan_slug'>,
644+
subtask: Pick<SubtaskInfo, 'subtask_index'>,
645+
): boolean {
646+
return liveSubtaskBranches.has(
647+
subtaskBranchKey(plan.repo_root, `spec/${plan.plan_slug}/sub-${subtask.subtask_index}`),
648+
);
649+
}
650+
651+
function subtaskBranchKey(repoRoot: string, branch: string): string {
652+
return `${repoRoot}\0${branch}`;
653+
}
654+
634655
function codexMcpCall(args: TaskPlanClaimArgs): string {
635656
return `mcp__colony__task_plan_claim_subtask({ agent: ${JSON.stringify(args.agent)}, session_id: ${JSON.stringify(args.session_id)}, repo_root: ${JSON.stringify(args.repo_root)}, plan_slug: ${JSON.stringify(args.plan_slug)}, subtask_index: ${args.subtask_index}, file_scope: ${JSON.stringify(args.file_scope)} })`;
636657
}

apps/mcp-server/src/tools/shared.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,7 @@ export function mcpErrorResponse(
284284
| 'PLAN_WAVE_SCOPE_OVERLAP'
285285
| 'PLAN_FINALIZER_NOT_LAST'
286286
| 'PLAN_SUBTASK_NOT_FOUND'
287+
| 'PLAN_SUBTASK_STALE'
287288
| 'PLAN_SUBTASK_DEPS_UNMET'
288289
| 'PLAN_SUBTASK_NOT_AVAILABLE'
289290
| 'PLAN_SUBTASK_NOT_CLAIMED'

apps/mcp-server/test/plan.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -723,6 +723,29 @@ describe('task_plan_list', () => {
723723
expect(uiPlans).toHaveLength(0);
724724
});
725725

726+
it('defaults task_plan_list to 10 plans while honoring explicit limits up to the cap', async () => {
727+
for (let index = 0; index < 12; index += 1) {
728+
await call<PublishResult>(
729+
'task_plan_publish',
730+
basicPublishArgs({
731+
slug: `limit-plan-${index}`,
732+
title: `Limit plan ${index}`,
733+
}),
734+
);
735+
}
736+
737+
const defaultPlans = await call<PlanRollup[]>('task_plan_list', {
738+
repo_root: repoRoot,
739+
});
740+
const expandedPlans = await call<PlanRollup[]>('task_plan_list', {
741+
repo_root: repoRoot,
742+
limit: 12,
743+
});
744+
745+
expect(defaultPlans).toHaveLength(10);
746+
expect(expandedPlans).toHaveLength(12);
747+
});
748+
726749
it('returns a compact rollup by default that omits description and file_scope', async () => {
727750
await call<PublishResult>('task_plan_publish', basicPublishArgs());
728751

@@ -991,6 +1014,56 @@ describe('task_plan_claim_subtask', () => {
9911014
expect(archivedWarnings()).toHaveLength(1);
9921015
});
9931016

1017+
it('returns recovery details for stale sub-task pointers on an existing plan', async () => {
1018+
await call<PublishResult>(
1019+
'task_plan_publish',
1020+
basicPublishArgs({
1021+
slug: 'stale-claim-recovery',
1022+
subtasks: [
1023+
{
1024+
title: 'Build widget API',
1025+
description: 'Add GET /api/widgets.',
1026+
file_scope: ['apps/api/src/widgets.ts'],
1027+
capability_hint: 'api_work',
1028+
},
1029+
{
1030+
title: 'Build widget page',
1031+
description: 'Render the widget list.',
1032+
file_scope: ['apps/frontend/src/pages/widgets.tsx'],
1033+
capability_hint: 'ui_work',
1034+
},
1035+
],
1036+
}),
1037+
);
1038+
1039+
const res = await client.callTool({
1040+
name: 'task_plan_claim_subtask',
1041+
arguments: {
1042+
plan_slug: 'stale-claim-recovery',
1043+
subtask_index: 9,
1044+
session_id: 'B',
1045+
agent: 'codex',
1046+
},
1047+
});
1048+
expect(res.isError).toBe(true);
1049+
const payload = JSON.parse(
1050+
(res.content as Array<{ type: string; text: string }>)[0]?.text ?? '{}',
1051+
) as {
1052+
code: string;
1053+
error: string;
1054+
plan_slug: string;
1055+
next_available_subtask_index: number | null;
1056+
next_available_count: number;
1057+
next_available: Array<{ subtask_index: number; capability_hint: string | null }>;
1058+
};
1059+
expect(payload.code).toBe('PLAN_SUBTASK_STALE');
1060+
expect(payload.error).toContain('spec/stale-claim-recovery/sub-9');
1061+
expect(payload.plan_slug).toBe('stale-claim-recovery');
1062+
expect(payload.next_available_count).toBe(2);
1063+
expect(payload.next_available_subtask_index).toBe(0);
1064+
expect(payload.next_available.map((s) => s.subtask_index)).toEqual([0, 1]);
1065+
});
1066+
9941067
it('reports a claimed sub-task by bound spec row id', async () => {
9951068
await call<PublishResult>(
9961069
'task_plan_publish',

apps/mcp-server/test/ready-queue.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -792,6 +792,50 @@ describe('task_ready_for_agent', () => {
792792
});
793793
});
794794

795+
it('does not return ready entries whose advertised sub-task index has no branch row', async () => {
796+
const parent = TaskThread.open(store, {
797+
repo_root: repoRoot,
798+
branch: 'spec/stale-ready-pointer',
799+
session_id: 'planner',
800+
title: 'Stale ready pointer',
801+
});
802+
const subtask = TaskThread.open(store, {
803+
repo_root: repoRoot,
804+
branch: 'spec/stale-ready-pointer/sub-0',
805+
session_id: 'planner',
806+
title: 'Stale subtask branch',
807+
});
808+
store.addObservation({
809+
session_id: 'planner',
810+
task_id: subtask.task_id,
811+
kind: 'plan-subtask',
812+
content: 'Stale advertised index\n\nThe metadata points at sub-7, but only sub-0 exists.',
813+
metadata: {
814+
parent_plan_slug: 'stale-ready-pointer',
815+
parent_plan_title: 'Stale ready pointer',
816+
parent_spec_task_id: parent.task_id,
817+
subtask_index: 7,
818+
title: 'Stale advertised index',
819+
description: 'The metadata points at sub-7, but only sub-0 exists.',
820+
file_scope: ['apps/api/stale-ready.ts'],
821+
depends_on: [],
822+
spec_row_id: null,
823+
capability_hint: 'api_work',
824+
status: 'available',
825+
},
826+
});
827+
828+
const result = await call<ReadyResult>('task_ready_for_agent', {
829+
session_id: 'agent-session',
830+
agent: 'codex',
831+
repo_root: repoRoot,
832+
auto_claim: false,
833+
});
834+
835+
expect(result.ready).toEqual([]);
836+
expect(result.total_available).toBe(0);
837+
});
838+
795839
it('returns the empty state when all future sub-tasks are blocked', async () => {
796840
await call('task_plan_publish', {
797841
...publishArgs(
@@ -1313,6 +1357,7 @@ describe('task_ready_for_agent', () => {
13131357

13141358
it('keeps quota relay ready payloads compact for many files and long handoffs', async () => {
13151359
const sessionId = 'quota-session';
1360+
store.startSession({ id: sessionId, ide: 'codex', cwd: repoRoot });
13161361
const thread = TaskThread.open(store, {
13171362
repo_root: repoRoot,
13181363
branch: 'agent/codex/large-quota-relay',
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
schema: spec-driven
2+
created: 2026-05-14
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
## Why
2+
3+
- `task_plan_list` defaults to returning up to 50 plans, which makes routine
4+
MCP calls heavier than needed when the plan registry is large.
5+
- Agents should still be able to request a larger page explicitly, but the
6+
default browse surface should stay compact.
7+
8+
## What Changes
9+
10+
- Lower the core `listPlans()` default limit from 50 to 10.
11+
- Preserve the MCP `task_plan_list.limit` validation cap at 50.
12+
- Add regression coverage that verifies the default returns 10 plans and an
13+
explicit `limit` can still return more.
14+
15+
## Impact
16+
17+
- Affected surfaces: Colony core plan listing and the MCP `task_plan_list`
18+
tool.
19+
- Callers that need larger responses must pass `limit` explicitly.
20+
- No schema or migration changes.
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
## ADDED Requirements
2+
3+
### Requirement: Compact task_plan_list default
4+
The system SHALL default `task_plan_list` and core `listPlans()` responses to at most 10 plans when callers omit an explicit limit.
5+
6+
#### Scenario: Default list is compact
7+
- **WHEN** more than 10 published plans exist
8+
- **AND** a caller invokes `task_plan_list` without `limit`
9+
- **THEN** the response contains at most 10 plans.
10+
11+
#### Scenario: Explicit larger limit remains available
12+
- **WHEN** more than 10 published plans exist
13+
- **AND** a caller invokes `task_plan_list` with an explicit limit above 10 and within the existing cap
14+
- **THEN** the response honors the explicit limit.

0 commit comments

Comments
 (0)