diff --git a/scripts/lib/control-pane/work-item-mutations.js b/scripts/lib/control-pane/work-item-mutations.js index 00bcb5252..c26395a9a 100644 --- a/scripts/lib/control-pane/work-item-mutations.js +++ b/scripts/lib/control-pane/work-item-mutations.js @@ -8,6 +8,8 @@ */ const DONE_STATUSES = new Set(['done', 'closed', 'resolved', 'merged', 'cancelled']); +// Only explicitly admitted work may be newly claimed. +const CLAIMABLE_STATUSES = new Set(['open', 'ready']); const PRIORITY_RANK = { critical: 0, high: 1, urgent: 1, medium: 2, normal: 2, low: 3 }; // Kanban lanes the board renders, and the canonical status each maps to on a move. @@ -21,11 +23,7 @@ const VALID_LANES = new Set(Object.keys(LANE_TO_STATUS)); const VALID_ASSIGNEE_KINDS = new Set(['agent', 'human']); function isOpenStatus(status) { - return !DONE_STATUSES.has( - String(status || '') - .trim() - .toLowerCase() - ); + return CLAIMABLE_STATUSES.has(String(status || '').trim().toLowerCase()); } function priorityRank(priority) { @@ -44,8 +42,9 @@ function selectClaimTarget(store, { id } = {}) { } return item; } - const { items } = store.listWorkItems({ limit: 100 }); - return items.filter(item => !item.owner && isOpenStatus(item.status)).sort((a, b) => priorityRank(a.priority) - priorityRank(b.priority))[0] || null; + // The native SQL query filters all eligible items before choosing the + // highest-priority claim; a newer 100-item slice can hide older READY work. + return store.getNextClaimableWorkItem(); } /** @@ -65,13 +64,19 @@ function claimWorkItem(store, { id, owner, assigneeKind, sessionId, status } = { if (!target) { return { claimed: false, reason: 'no-unassigned-open-items' }; } - if (!isOpenStatus(target.status)) { - throw new Error(`Work item ${target.id} is already done; cannot claim.`); - } if (target.owner) { if (target.owner !== owner) { throw new Error(`Work item ${target.id} is already owned by ${target.owner}; cannot claim.`); } + // Repeating an active claim by the same owner is idempotent. + if (['running', 'claimed', 'in-progress'].includes(String(target.status).trim().toLowerCase())) { + return { claimed: true, item: target }; + } + } + if (!isOpenStatus(target.status)) { + throw new Error(`Work item ${target.id} is not READY; cannot claim.`); + } + if (target.owner) { return { claimed: true, item: target }; } const metadata = { ...(target.metadata || {}) }; diff --git a/scripts/lib/state-store/queries.js b/scripts/lib/state-store/queries.js index 7074d5a96..d7e8782d8 100644 --- a/scripts/lib/state-store/queries.js +++ b/scripts/lib/state-store/queries.js @@ -482,6 +482,22 @@ function createQueryApi(db, { prepareRead = sql => db.prepare(sql) } = {}) { FROM work_items ORDER BY updated_at DESC, id DESC `); + // Read the entire native READY set in SQL instead of truncating to the + // 100 most recently updated items before checking their eligibility. + const nextClaimableWorkItemStatement = prepareRead(` + SELECT * + FROM work_items + WHERE (owner IS NULL OR owner = '') + AND LOWER(TRIM(COALESCE(status, ''))) IN ('open', 'ready') + ORDER BY CASE LOWER(COALESCE(priority, '')) + WHEN 'critical' THEN 0 + WHEN 'high' THEN 1 + WHEN 'urgent' THEN 1 + WHEN 'low' THEN 3 + ELSE 2 + END ASC, updated_at DESC, id DESC + LIMIT 1 + `); const getWorkItemStatement = prepareRead(` SELECT * FROM work_items @@ -760,6 +776,11 @@ function createQueryApi(db, { prepareRead = sql => db.prepare(sql) } = {}) { }; } + function getNextClaimableWorkItem() { + const row = nextClaimableWorkItemStatement.get(); + return row ? mapWorkItemRow(row) : null; + } + function getStatus(options = {}) { const activeLimit = normalizeLimit(options.activeLimit, 5); const recentSkillRunLimit = normalizeLimit(options.recentSkillRunLimit, 20); @@ -870,6 +891,7 @@ function createQueryApi(db, { prepareRead = sql => db.prepare(sql) } = {}) { }, listRecentSessions, listWorkItems, + getNextClaimableWorkItem, upsertInstallState(installState) { const normalized = normalizeInstallStateInput(installState); assertValidEntity('installState', normalized); diff --git a/tests/scripts/work-items.test.js b/tests/scripts/work-items.test.js index 05a3868ee..769708a70 100644 --- a/tests/scripts/work-items.test.js +++ b/tests/scripts/work-items.test.js @@ -132,6 +132,110 @@ async function run() { assert.ok(/not found/i.test(missing.stderr), 'reports not found'); }); + await test('claim rejects unadmitted needs-review, blocked, hold, and unowned running items', async () => { + const statuses = ['needs-review', 'blocked', 'hold', 'running', 'in-progress']; + const setup = await createStateStore({ dbPath }); + try { + for (const status of statuses) { + setup.upsertWorkItem({ id: `wi-gate-${status}`, source: 'manual', title: `Gate ${status}`, status, owner: null, metadata: {} }); + } + } finally { setup.close(); } + for (const status of statuses) { + const result = runClaim(dbPath, [`wi-gate-${status}`, '--owner', 'gate-worker', '--as', 'agent']); + assert.notStrictEqual(result.status, 0, `${status} must not be claimable`); + assert.match(result.stderr, /not READY/i); + const store = await createStateStore({ dbPath }); + try { assert.strictEqual(store.getWorkItemById(`wi-gate-${status}`).status, status); } finally { store.close(); } + } + }); + + await test('automatic claim skips higher-priority unready work and picks READY', async () => { + const setup = await createStateStore({ dbPath }); + try { + setup.upsertWorkItem({ id: 'wi-unassigned-high', source: 'manual', title: 'Needs review', status: 'needs-review', priority: 'critical', owner: null, metadata: {} }); + setup.upsertWorkItem({ id: 'wi-unassigned-low', source: 'manual', title: 'Blocked', status: 'blocked', priority: 'high', owner: null, metadata: {} }); + setup.upsertWorkItem({ id: 'wi-auto-blocked-high', source: 'manual', title: 'High priority blocked', status: 'blocked', priority: 'critical', owner: null, metadata: {} }); + setup.upsertWorkItem({ id: 'wi-auto-ready-low', source: 'manual', title: 'Low priority READY', status: 'ready', priority: 'low', owner: null, metadata: {} }); + } finally { setup.close(); } + const result = runClaim(dbPath, ['--owner', 'auto-worker', '--as', 'agent']); + assert.strictEqual(result.status, 0, result.stderr); + const payload = JSON.parse(result.stdout); + assert.strictEqual(payload.claimed, true); + assert.strictEqual(payload.item.id, 'wi-auto-ready-low'); + assert.strictEqual(payload.item.status, 'running'); + }); + + await test('automatic claim returns empty when all unowned items are unready', async () => { + const setup = await createStateStore({ dbPath }); + try { + setup.upsertWorkItem({ id: 'wi-unassigned-high', source: 'manual', title: 'Needs review', status: 'needs-review', priority: 'critical', owner: null, metadata: {} }); + setup.upsertWorkItem({ id: 'wi-unassigned-low', source: 'manual', title: 'Hold', status: 'hold', priority: 'high', owner: null, metadata: {} }); + } finally { setup.close(); } + const result = runClaim(dbPath, ['--owner', 'auto-worker', '--as', 'agent']); + assert.strictEqual(result.status, 0, result.stderr); + const payload = JSON.parse(result.stdout); + assert.deepStrictEqual(payload, { claimed: false, reason: 'no-unassigned-open-items' }); + }); + + await test('claim accepts an explicitly ready item', async () => { + const setup = await createStateStore({ dbPath }); + try { setup.upsertWorkItem({ id: 'wi-explicit-ready', source: 'manual', title: 'Approved', status: 'ready', owner: null, metadata: {} }); } + finally { setup.close(); } + const result = runClaim(dbPath, ['wi-explicit-ready', '--owner', 'approved-worker', '--as', 'agent']); + assert.strictEqual(result.status, 0, result.stderr); + const payload = JSON.parse(result.stdout); + assert.strictEqual(payload.claimed, true); + assert.strictEqual(payload.item.status, 'running'); + assert.strictEqual(payload.item.owner, 'approved-worker'); + }); + + await test('repeat claim by the same owner preserves legacy in-progress idempotence', async () => { + const setup = await createStateStore({ dbPath }); + try { + setup.upsertWorkItem({ id: 'wi-legacy-in-progress', source: 'manual', title: 'Legacy active work', status: 'in-progress', owner: 'legacy-worker', metadata: {} }); + } finally { setup.close(); } + const before = await createStateStore({ dbPath }); + let original; + try { original = before.getWorkItemById('wi-legacy-in-progress'); } finally { before.close(); } + const result = runClaim(dbPath, ['wi-legacy-in-progress', '--owner', 'legacy-worker', '--as', 'agent']); + assert.strictEqual(result.status, 0, result.stderr); + assert.deepStrictEqual(JSON.parse(result.stdout).item, original); + }); + + await test('automatic claim finds older READY work beyond 100 newer unready items', async () => { + const store = await createStateStore({ dbPath }); + try { + for (let i = 0; i < 105; i++) { + store.upsertWorkItem({ + id: `wi-new-unready-${i}`, source: 'manual', title: 'Unready', + status: 'needs-review', priority: 'critical', owner: null, + createdAt: '2026-10-01T00:00:00.000Z', updatedAt: '2026-10-01T00:00:00.000Z' + }); + } + store.upsertWorkItem({ + id: 'wi-old-critical-ready', source: 'manual', title: 'Old critical READY', + status: 'ready', priority: 'critical', owner: null, + createdAt: '2025-01-01T00:00:00.000Z', updatedAt: '2025-01-01T00:00:00.000Z' + }); + store.upsertWorkItem({ + id: 'wi-old-low-ready', source: 'manual', title: 'Old low READY', + status: 'ready', priority: 'low', owner: null, + createdAt: '2025-02-01T00:00:00.000Z', updatedAt: '2025-02-01T00:00:00.000Z' + }); + const newest = store.listWorkItems({ limit: 100 }); + assert.ok(newest.totalCount > 100); + assert.ok(!newest.items.some(item => item.id === 'wi-old-critical-ready')); + } finally { store.close(); } + + const first = runClaim(dbPath, ['--owner', 'critical-worker', '--as', 'agent']); + assert.strictEqual(first.status, 0, first.stderr); + assert.strictEqual(JSON.parse(first.stdout).item.id, 'wi-old-critical-ready'); + + const second = runClaim(dbPath, ['--owner', 'low-worker', '--as', 'agent']); + assert.strictEqual(second.status, 0, second.stderr); + assert.strictEqual(JSON.parse(second.stdout).item.id, 'wi-old-low-ready'); + }); + await test('repeat claim by the same owner is idempotent', async () => { const before = await createStateStore({ dbPath }); let original;