diff --git a/apps/mcp-server/src/tools/ready-queue.ts b/apps/mcp-server/src/tools/ready-queue.ts index 623a37c..33efc44 100644 --- a/apps/mcp-server/src/tools/ready-queue.ts +++ b/apps/mcp-server/src/tools/ready-queue.ts @@ -45,6 +45,7 @@ const QUOTA_READY_FILE_PREVIEW_LIMIT = 3; const QUOTA_READY_TEXT_LIMIT = 240; const PLAN_SUBTASK_KIND = 'plan-subtask'; const PLAN_SUBTASK_CLAIM_KIND = 'plan-subtask-claim'; +const READY_CLAIM_REQUIRED_KIND = 'ready-claim-required'; const QUOTA_RELAY_READY_KIND = 'quota_relay_ready'; export const NO_CLAIMABLE_PLAN_SUBTASKS_EMPTY_STATE = 'No claimable plan subtasks. Publish a Queen/task plan for multi-agent work, reinforce a proposal with task_propose/task_reinforce, or use task_list only for browsing.'; @@ -502,9 +503,12 @@ export async function buildReadyForAgent( ), currentClaims, ).map((task) => applyRuntimeRouting(store, task, args.agent, args.repo_root)); + const pendingReadyClaim = pendingReadyClaimObligation(store, args, planRanked); const ranked = rankReadyEntries(quotaRelays, planRanked); - const selected = ranked.slice(0, args.limit ?? DEFAULT_LIMIT); - const claimable = ranked.find(isClaimableEntry) ?? null; + const selected = pendingReadyClaim + ? [pendingReadyClaim] + : ranked.slice(0, args.limit ?? DEFAULT_LIMIT); + const claimable = pendingReadyClaim ?? ranked.find(isClaimableEntry) ?? null; const setupIssue = specRootSetupIssue(args.repo_root); const ready = await Promise.all( selected.map(async (entry, index) => { @@ -539,7 +543,7 @@ export async function buildReadyForAgent( }), ); - return buildReadyResult( + const result = buildReadyResult( { ready, total_available: available.length + quotaRelays.length, @@ -554,6 +558,94 @@ export async function buildReadyForAgent( available.length === 0 && quotaRelays.length === 0 ? staleBlockerRescueCandidate(plans) : null, planClaimHintForEmptyState(plans, liveSubtaskBranches, args), ); + if (pendingReadyClaim) return applyPendingReadyClaimObligation(result, pendingReadyClaim); + recordReadyClaimObligation(store, result); + return result; +} + +function pendingReadyClaimObligation( + store: MemoryStore, + args: { session_id: string; repo_root?: string }, + ranked: RankedSubtask[], +): RankedSubtask | null { + const rows = store.storage.timeline(args.session_id, undefined, 100); + for (const row of rows.slice().reverse()) { + if (row.kind !== READY_CLAIM_REQUIRED_KIND) continue; + const meta = parseMeta(row.metadata); + if (meta.kind !== READY_CLAIM_REQUIRED_KIND) continue; + if (args.repo_root !== undefined && meta.repo_root !== args.repo_root) continue; + const planSlug = readMetaString(meta.plan_slug); + const subtaskIndex = readMetaInteger(meta.subtask_index); + if (!planSlug || subtaskIndex === null) continue; + const match = ranked.find( + (entry) => entry.plan_slug === planSlug && entry.subtask_index === subtaskIndex, + ); + if (match && match.current_claim === false) return match; + return null; + } + return null; +} + +function applyPendingReadyClaimObligation( + result: ReadyForAgentResult, + pending: RankedSubtask, +): ReadyForAgentResult { + const nextAction = + `Previous task_ready_for_agent call still requires task_plan_claim_subtask for ${pending.plan_slug}/sub-${pending.subtask_index}; claim it before reading the ready queue again.`; + return { + ...result, + ready: result.ready.filter( + (entry) => + !isQuotaRelayReady(entry) && + entry.plan_slug === pending.plan_slug && + entry.subtask_index === pending.subtask_index, + ), + next_action: nextAction, + next_action_reason: nextAction, + next_tool: 'task_plan_claim_subtask', + claim_required: true, + plan_slug: pending.plan_slug, + subtask_index: pending.subtask_index, + reason: pending.reason, + assigned_agent: pending.assigned_agent, + routing_reason: pending.routing_reason, + claim_args: pending.claim_args, + codex_mcp_call: codexMcpCall(pending.claim_args), + }; +} + +function recordReadyClaimObligation(store: MemoryStore, result: ReadyForAgentResult): void { + if (!result.claim_required) return; + if (result.next_tool !== 'task_plan_claim_subtask') return; + const claimArgs = result.claim_args; + if (!claimArgs || !('plan_slug' in claimArgs)) return; + const task = store.storage.findTaskByBranch( + claimArgs.repo_root, + `spec/${claimArgs.plan_slug}/sub-${claimArgs.subtask_index}`, + ); + store.addObservation({ + session_id: claimArgs.session_id, + task_id: task?.id ?? null, + kind: READY_CLAIM_REQUIRED_KIND, + content: `ready claim required ${claimArgs.plan_slug}/sub-${claimArgs.subtask_index}`, + metadata: { + kind: READY_CLAIM_REQUIRED_KIND, + repo_root: claimArgs.repo_root, + plan_slug: claimArgs.plan_slug, + subtask_index: claimArgs.subtask_index, + agent: claimArgs.agent, + file_scope: claimArgs.file_scope, + next_tool: 'task_plan_claim_subtask', + }, + }); +} + +function readMetaString(value: unknown): string | null { + return typeof value === 'string' && value.length > 0 ? value : null; +} + +function readMetaInteger(value: unknown): number | null { + return typeof value === 'number' && Number.isInteger(value) ? value : null; } function buildReadyResult( diff --git a/apps/mcp-server/test/ready-queue.test.ts b/apps/mcp-server/test/ready-queue.test.ts index f7d70e2..c10c9ef 100644 --- a/apps/mcp-server/test/ready-queue.test.ts +++ b/apps/mcp-server/test/ready-queue.test.ts @@ -593,6 +593,68 @@ describe('task_ready_for_agent', () => { expect(result.empty_state).toBeUndefined(); }); + it('rejects repeated ready reads until the required sub-task claim happens', async () => { + await call('task_plan_publish', { + ...publishArgs( + [ + { + title: 'Build repeated ready API', + description: 'Expose the repeated ready endpoint.', + file_scope: ['apps/api/repeated-ready.ts'], + capability_hint: 'api_work', + }, + { + title: 'Document repeated ready API', + description: 'Document the repeated ready endpoint.', + file_scope: ['docs/repeated-ready.md'], + depends_on: [0], + capability_hint: 'doc_work', + }, + ], + { slug: 'repeated-ready-plan' }, + ), + }); + + const first = await call('task_ready_for_agent', { + session_id: 'agent-session', + agent: 'codex', + repo_root: repoRoot, + auto_claim: false, + }); + expect(first.claim_required).toBe(true); + + const repeated = await call('task_ready_for_agent', { + session_id: 'agent-session', + agent: 'codex', + repo_root: repoRoot, + auto_claim: false, + }); + expect(repeated.next_action).toContain( + 'Previous task_ready_for_agent call still requires task_plan_claim_subtask', + ); + expect(repeated.claim_required).toBe(true); + expect(repeated.claim_args).toEqual(first.claim_args); + + await call('task_plan_claim_subtask', { + plan_slug: 'repeated-ready-plan', + subtask_index: 0, + repo_root: repoRoot, + session_id: 'agent-session', + agent: 'codex', + }); + const claimed = await call('task_ready_for_agent', { + session_id: 'agent-session', + agent: 'codex', + repo_root: repoRoot, + auto_claim: false, + }); + expect(claimed.ready[0]).toMatchObject({ + plan_slug: 'repeated-ready-plan', + subtask_index: 0, + reason: 'continue_current_task', + }); + }); + it('hides proposed task rows from executors until approval', async () => { await call('task_plan_publish', { ...publishArgs( diff --git a/openspec/changes/agent-codex-reject-task-ready-for-agent-without-foll-2026-05-15-21-03/.openspec.yaml b/openspec/changes/agent-codex-reject-task-ready-for-agent-without-foll-2026-05-15-21-03/.openspec.yaml new file mode 100644 index 0000000..9f70866 --- /dev/null +++ b/openspec/changes/agent-codex-reject-task-ready-for-agent-without-foll-2026-05-15-21-03/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-05-15 diff --git a/openspec/changes/agent-codex-reject-task-ready-for-agent-without-foll-2026-05-15-21-03/proposal.md b/openspec/changes/agent-codex-reject-task-ready-for-agent-without-foll-2026-05-15-21-03/proposal.md new file mode 100644 index 0000000..c023b48 --- /dev/null +++ b/openspec/changes/agent-codex-reject-task-ready-for-agent-without-foll-2026-05-15-21-03/proposal.md @@ -0,0 +1,19 @@ +## Why + +Agents could call `task_ready_for_agent`, receive a claimable sub-task, and then +call the ready queue again instead of claiming. That made the queue look active +while work stayed unclaimed. + +## What Changes + +Record a per-session ready-claim obligation when `task_ready_for_agent` returns +`claim_required=true`. A repeated ready call for the same session and still +available sub-task returns the same claim instruction with an explicit +"claim before reading again" response. SessionStart also surfaces the pending +obligation for that session. + +## Impact + +Affected surfaces are MCP ready queue responses and the SessionStart ready +nudge. Existing auto-claim success paths satisfy the obligation immediately +because the sub-task status changes to claimed for that session. diff --git a/openspec/changes/agent-codex-reject-task-ready-for-agent-without-foll-2026-05-15-21-03/specs/reject-task-ready-for-agent-without-follow-up-task-plan-claim-subtask/spec.md b/openspec/changes/agent-codex-reject-task-ready-for-agent-without-foll-2026-05-15-21-03/specs/reject-task-ready-for-agent-without-follow-up-task-plan-claim-subtask/spec.md new file mode 100644 index 0000000..1e61a1c --- /dev/null +++ b/openspec/changes/agent-codex-reject-task-ready-for-agent-without-foll-2026-05-15-21-03/specs/reject-task-ready-for-agent-without-follow-up-task-plan-claim-subtask/spec.md @@ -0,0 +1,18 @@ +## ADDED Requirements + +### Requirement: reject-task-ready-for-agent-without-follow-up-task-plan-claim-subtask behavior +The ready queue SHALL keep a per-session claim obligation when it returns a +claimable plan sub-task with `claim_required=true`. + +#### Scenario: Session reads ready queue twice without claiming +- **GIVEN** `task_ready_for_agent` returned a claimable sub-task for a session +- **AND** that session has not called `task_plan_claim_subtask` +- **WHEN** the same session calls `task_ready_for_agent` again while the sub-task is still available +- **THEN** the response repeats the same `task_plan_claim_subtask` arguments +- **AND** the response explains that the prior ready result must be claimed before reading again. + +#### Scenario: Session starts with a pending ready claim +- **GIVEN** a session has an unfulfilled ready-claim obligation +- **WHEN** SessionStart renders the ready-claim nudge +- **THEN** the nudge names the pending plan/sub-task +- **AND** instructs the agent to call `task_plan_claim_subtask` before reading the queue again. diff --git a/openspec/changes/agent-codex-reject-task-ready-for-agent-without-foll-2026-05-15-21-03/tasks.md b/openspec/changes/agent-codex-reject-task-ready-for-agent-without-foll-2026-05-15-21-03/tasks.md new file mode 100644 index 0000000..3e57e36 --- /dev/null +++ b/openspec/changes/agent-codex-reject-task-ready-for-agent-without-foll-2026-05-15-21-03/tasks.md @@ -0,0 +1,39 @@ +## Definition of Done + +This change is complete only when **all** of the following are true: + +- Every checkbox below is checked. +- The agent branch reaches `MERGED` state on `origin` and the PR URL + state are recorded in the completion handoff. +- If any step blocks (test failure, conflict, ambiguous result), append a `BLOCKED:` line under section 4 explaining the blocker and **STOP**. Do not tick remaining cleanup boxes; do not silently skip the cleanup pipeline. + +## Handoff + +- Handoff: change=`agent-codex-reject-task-ready-for-agent-without-foll-2026-05-15-21-03`; branch=`agent/codex/reject-task-ready-for-agent-without-foll-2026-05-15-21-03`; scope=`apps/mcp-server/src/tools/ready-queue.ts; packages/hooks/src/handlers/session-start.ts`; action=`finish verification and cleanup`. +- Copy prompt: Continue `agent-codex-reject-task-ready-for-agent-without-foll-2026-05-15-21-03` on branch `agent/codex/reject-task-ready-for-agent-without-foll-2026-05-15-21-03`. Work inside the existing sandbox, review `openspec/changes/agent-codex-reject-task-ready-for-agent-without-foll-2026-05-15-21-03/tasks.md`, continue from the current state instead of creating a new sandbox, and when the work is done run `gx branch finish --branch agent/codex/reject-task-ready-for-agent-without-foll-2026-05-15-21-03 --base main --via-pr --cleanup`. + +## 1. Specification + +- [x] 1.1 Finalize proposal scope and acceptance criteria for `agent-codex-reject-task-ready-for-agent-without-foll-2026-05-15-21-03`. +- [x] 1.2 Define normative requirements in `specs/reject-task-ready-for-agent-without-follow-up-task-plan-claim-subtask/spec.md`. + +## 2. Implementation + +- [x] 2.1 Implement scoped behavior changes. +- [x] 2.2 Add/update focused regression coverage. + +## 3. Verification + +- [x] 3.1 Run targeted project verification commands. + - `pnpm --filter @colony/mcp-server test -- ready-queue.test.ts` + - `pnpm --filter @colony/hooks test -- session-start.test.ts` + - `pnpm --filter @colony/mcp-server typecheck` + - `pnpm --filter @colony/hooks typecheck` + - `git diff --check` +- [x] 3.2 Run `openspec validate agent-codex-reject-task-ready-for-agent-without-foll-2026-05-15-21-03 --type change --strict`. +- [x] 3.3 Run `openspec validate --specs`. + +## 4. Cleanup (mandatory; run before claiming completion) + +- [ ] 4.1 Run the cleanup pipeline: `gx branch finish --branch agent// --base dev --via-pr --wait-for-merge --cleanup`. This handles commit -> push -> PR create -> merge wait -> worktree prune in one invocation. +- [ ] 4.2 Record the PR URL and final merge state (`MERGED`) in the completion handoff. +- [ ] 4.3 Confirm the sandbox worktree is gone (`git worktree list` no longer shows the agent path; `git branch -a` shows no surviving local/remote refs for the branch). diff --git a/packages/hooks/src/handlers/session-start.ts b/packages/hooks/src/handlers/session-start.ts index 8aaed70..9f95518 100644 --- a/packages/hooks/src/handlers/session-start.ts +++ b/packages/hooks/src/handlers/session-start.ts @@ -244,7 +244,7 @@ export function buildForagingPreface(store: MemoryStore, input: Pick, + input: Pick & Partial>, ): string { if (!input.cwd) return ''; const detected = detectRepoBranch(input.cwd); @@ -256,6 +256,14 @@ export function buildReadyClaimNudgePreface( console.error(`[colony] buildReadyClaimNudgePreface: ${(err as Error)?.message ?? err}`); return ''; } + const pendingClaim = pendingReadyClaimNudge(store, input.session_id, detected.repo_root, plans); + if (pendingClaim) { + return [ + '## Ready Queen sub-tasks', + `Previous task_ready_for_agent call still requires task_plan_claim_subtask for ${pendingClaim.plan_slug}/sub-${pendingClaim.subtask_index}.`, + `Call task_plan_claim_subtask with plan_slug="${pendingClaim.plan_slug}" and subtask_index=${pendingClaim.subtask_index} before reading the ready queue again.`, + ].join('\n'); + } let unclaimed = 0; let claimed = 0; for (const plan of plans) { @@ -275,6 +283,52 @@ export function buildReadyClaimNudgePreface( ].join('\n'); } +function pendingReadyClaimNudge( + store: MemoryStore, + sessionId: string | undefined, + repoRoot: string, + plans: ReturnType, +): { plan_slug: string; subtask_index: number } | null { + if (!sessionId) return null; + const rows = store.storage.timeline(sessionId, undefined, 100); + for (const row of rows.slice().reverse()) { + if (row.kind !== 'ready-claim-required') continue; + const meta = parseJsonRecord(row.metadata); + if (!meta || meta.kind !== 'ready-claim-required') continue; + if (meta.repo_root !== repoRoot) continue; + const planSlug = readMetadataString(meta.plan_slug); + const subtaskIndex = readMetadataInteger(meta.subtask_index); + if (!planSlug || subtaskIndex === null) continue; + const plan = plans.find((candidate) => candidate.plan_slug === planSlug); + const subtask = plan?.subtasks.find((candidate) => candidate.subtask_index === subtaskIndex); + if (subtask?.status === 'available') { + return { plan_slug: planSlug, subtask_index: subtaskIndex }; + } + return null; + } + return null; +} + +function parseJsonRecord(value: string | null): Record | null { + if (!value) return null; + try { + const parsed = JSON.parse(value) as unknown; + return parsed && typeof parsed === 'object' && !Array.isArray(parsed) + ? (parsed as Record) + : null; + } catch { + return null; + } +} + +function readMetadataString(value: unknown): string | null { + return typeof value === 'string' && value.length > 0 ? value : null; +} + +function readMetadataInteger(value: unknown): number | null { + return typeof value === 'number' && Number.isInteger(value) ? value : null; +} + export function buildAttentionBudgetSection( store: MemoryStore, input: Pick, diff --git a/packages/hooks/test/session-start.test.ts b/packages/hooks/test/session-start.test.ts index f047396..f65f62d 100644 --- a/packages/hooks/test/session-start.test.ts +++ b/packages/hooks/test/session-start.test.ts @@ -384,7 +384,7 @@ describe('SessionStart predictive suggestion preface', () => { }); describe('SessionStart ready-claim nudge', () => { - function seedAvailableSubtask(slug: string): void { + function seedAvailableSubtask(slug: string): number { fakeGitCheckout(repo, 'main'); store.startSession({ id: 'publisher', ide: 'codex', cwd: repo }); const parent = TaskThread.open(store, { @@ -416,6 +416,7 @@ describe('SessionStart ready-claim nudge', () => { parent_spec_task_id: parent.task_id, }, }); + return sub.task_id; } it('returns empty when no plans exist in the repo', () => { @@ -431,6 +432,32 @@ describe('SessionStart ready-claim nudge', () => { expect(preface).toContain('task_plan_claim_subtask'); }); + it('surfaces a pending ready-claim obligation for the same session', () => { + const taskId = seedAvailableSubtask('pending-ready-claim'); + store.addObservation({ + session_id: 'agent-session', + task_id: taskId, + kind: 'ready-claim-required', + content: 'ready claim required pending-ready-claim/sub-0', + metadata: { + kind: 'ready-claim-required', + repo_root: repo, + plan_slug: 'pending-ready-claim', + subtask_index: 0, + next_tool: 'task_plan_claim_subtask', + }, + }); + + const preface = buildReadyClaimNudgePreface(store, { + cwd: repo, + session_id: 'agent-session', + }); + expect(preface).toContain( + 'Previous task_ready_for_agent call still requires task_plan_claim_subtask', + ); + expect(preface).toContain('pending-ready-claim/sub-0'); + }); + it('does not nudge when no cwd is provided', () => { seedAvailableSubtask('nudge-target'); expect(buildReadyClaimNudgePreface(store, {})).toBe('');