maint: integrate reviewed contribution #3791

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 || {}) };

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);

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;