From 9dfbf3f657660d72b31c158b1a7b1a24446b7cba Mon Sep 17 00:00:00 2001 From: chahinebrini Date: Fri, 24 Jul 2026 02:29:13 +0200 Subject: [PATCH] =?UTF-8?q?fix(work):=20single-claim=20semantics=20?= =?UTF-8?q?=E2=80=94=20max=201=20in=5Fprogress=20claim=20per=20agent,=20ne?= =?UTF-8?q?xt=20auto-claim=20only=20after=20review/done,=20server-side=20d?= =?UTF-8?q?ouble-claim=20guard=20(TSK-0237)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- src/cli/commands/start.ts | 49 +++++++-- src/core/services/taskService.ts | 26 +++++ src/server/board/columns.ts | 115 ++++++++++++++++++--- src/server/board/styles.ts | 22 +++- src/server/taskDetail.ts | 2 +- tests/singleClaim.test.ts | 166 +++++++++++++++++++++++++++++++ 6 files changed, 355 insertions(+), 25 deletions(-) create mode 100644 tests/singleClaim.test.ts diff --git a/src/cli/commands/start.ts b/src/cli/commands/start.ts index 3e547b7..98a2769 100644 --- a/src/cli/commands/start.ts +++ b/src/cli/commands/start.ts @@ -21,6 +21,8 @@ interface Listed { taskId?: string; toAgent?: string; assignedTo?: string; + claimedBy?: string; + createdAt?: string; } /** Announce presence (best-effort) and print the joined line. */ @@ -58,17 +60,33 @@ export async function listReviewTasks(ctx: AgentContext): Promise { : svcListTasks(ctx.projectCwd, { status: 'review' }); } +const PRIORITY_RANK: Record = { critical: 4, high: 3, medium: 2, low: 1 }; + /** - * Find the open task addressed to this agent. "Addressed" = task title starts - * with ":" (delegation convention), OR a handoff for the task has - * toAgent === , OR the task is already assignedTo this agent (a reopened - * task that came back for rework). Returns the task + the handoff list (so the - * caller can print the matching handoff without re-fetching). + * Find the ONE open task this agent should claim next. "Addressed" = task + * title starts with ":" (delegation convention), OR a handoff for the + * task has toAgent === , OR the task is already assignedTo this agent + * (a reopened task that came back for rework). + * + * Single-claim semantics (TSK-0237): while the agent already holds an + * in_progress task, NOTHING is addressed — the next auto-claim happens only + * after that task reaches review/done (or the claim is taken back via + * reopen). Among several addressed candidates exactly one wins: highest + * priority, oldest createdAt breaks ties. The rest stays open/assigned. + * Returns the task + the handoff list (so the caller can print the matching + * handoff without re-fetching). */ export async function findAddressedOpenTask( ctx: AgentContext, ): Promise<{ task: Listed; handoffs: Listed[] } | undefined> { const a = ctx.agent.toLowerCase(); + + const inProgress = ctx.serverUrl + ? await remoteClient.listTasks(ctx.serverUrl, { status: 'in_progress' }) + : svcListTasks(ctx.projectCwd, { status: 'in_progress' }); + const busy = inProgress.some((t) => String(t.claimedBy ?? t.assignedTo ?? '').toLowerCase() === a); + if (busy) return undefined; + const tasks = await listOpenRoleTasks(ctx); const handoffs = ctx.serverUrl ? await remoteClient.listHandoffs(ctx.serverUrl) : svcListHandoffs(ctx.projectCwd); @@ -85,7 +103,26 @@ export async function findAddressedOpenTask( ); if (mine.length === 0) return undefined; - return { task: mine[0], handoffs }; + if (mine.length === 1) return { task: mine[0], handoffs }; + + // Priority is not part of the task index — fetch it per candidate (only the + // few addressed ones), rank critical > high > medium > low, oldest first. + const ranked = await Promise.all( + mine.map(async (t) => { + let rank = PRIORITY_RANK.medium; + try { + // Both the remote client and the local service return { task, body }. + const detail = ctx.serverUrl ? await remoteClient.getTask(ctx.serverUrl, t.id) : svcGetTask(ctx.projectCwd, t.id); + const full = (detail as { task?: { priority?: string } }).task; + rank = PRIORITY_RANK[full?.priority ?? 'medium'] ?? PRIORITY_RANK.medium; + } catch { + /* missing priority ⇒ medium */ + } + return { t, rank }; + }), + ); + ranked.sort((x, y) => y.rank - x.rank || String(x.t.createdAt ?? '').localeCompare(String(y.t.createdAt ?? ''))); + return { task: ranked[0].t, handoffs }; } /** Claim the task and print its body + handoff + the review-gate next step. */ diff --git a/src/core/services/taskService.ts b/src/core/services/taskService.ts index 7920135..7b6405f 100644 --- a/src/core/services/taskService.ts +++ b/src/core/services/taskService.ts @@ -82,6 +82,32 @@ export function claimTask(cwd: string, id: string, agentName: string): Task { `Task ${id} cannot be claimed — status is "${task.status}"${task.claimedBy ? ` (claimed by ${task.claimedBy})` : ''}.`, ); } + // Single-claim guard (TSK-0237): an agent holds at most ONE in_progress task. + // Second line of defense behind the work loop's own selection — a direct + // claim (CLI/MCP/board drag) must not bypass it either. The architect is + // exempt (coordinates several threads); there is no --force for agents. + // An uninitialized project (no config, e.g. bare service-level usage) simply + // has no architect exemption — the guard still applies. + let architect: string | undefined; + try { + architect = loadConfig(cwd).roles.architect?.preferredAgent; + } catch { + architect = undefined; + } + if (agentName !== architect) { + const held = listTasks(cwd).find( + (t) => + t.status === 'in_progress' && + t.id !== id && + (t.claimedBy === agentName || (!t.claimedBy && t.assignedTo === agentName)), + ); + if (held) { + throw new Error( + `${agentName} already holds an in_progress claim on ${held.id} — one task at a time: ` + + `submit ${held.id} for review (or wait for a reopen) before claiming ${id}.`, + ); + } + } return updateTask(cwd, id, { status: 'in_progress', assignedTo: agentName, claimedBy: agentName }); } diff --git a/src/server/board/columns.ts b/src/server/board/columns.ts index 4da2cf7..018e31a 100644 --- a/src/server/board/columns.ts +++ b/src/server/board/columns.ts @@ -227,17 +227,15 @@ export function columnsJs(opts: ColumnsJsOpts): string { var status = byStatus(t.status); var isOpen = openConsoles[t.id] ? true : false; var live = status === 'in_progress'; - // Console lives inside the card while a task is worked (in_progress = live) - // and stays available in review so you can see what the agent did. + // Console toggle lives in the card; the console itself opens as a + // dropdown OVERLAY on (see openConsole) — it spans the full + // column width instead of being squeezed into the card grid cell. var console = (live || status === 'review') ? '
' + '' + - '
' + - '
waiting for output…
' + - '
' + '
' : ''; // Progress ring on in-progress (work time) AND review cards (wait time) — @@ -461,13 +459,74 @@ export function columnsJs(opts: ColumnsJsOpts): string { body.insertAdjacentHTML('beforeend', consoleLineHtml(entry)); body.scrollTop = body.scrollHeight; } + // ── Console dropdown overlays ──────────────────────────────────────────── + // The console is NOT rendered inside the card anymore (the card grid cell + // is far too narrow for log lines, and .card{overflow:hidden} would clip + // any absolute panel). Instead each open console gets ONE overlay panel on + // , position:fixed, anchored under its card and spanning the full + // column width — readable logs regardless of how many cards share a row. + function consoleOverlay(id) { + return document.querySelector('.console-overlay[data-console-for="' + id + '"]'); + } + function ensureConsoleOverlay(id) { + var ov = consoleOverlay(id); + if (ov) return ov; + ov = document.createElement('div'); + ov.className = 'console-overlay'; + ov.setAttribute('data-console-for', id); + ov.innerHTML = + '
' + + '' + esc(id) + ' · console' + + '' + + '
' + + '
waiting for output…
'; + document.body.appendChild(ov); + return ov; + } + function positionConsoleOverlay(id) { + var tog = document.querySelector('[data-console-toggle="' + id + '"]'); + var ov = consoleOverlay(id); + if (!tog || !ov) return; + var card = tog.closest('.card'); + var column = tog.closest('.column'); + if (!card || !column) return; + var inset = 13; // column padding (12) + border (1) + var colRect = column.getBoundingClientRect(); + var cardRect = card.getBoundingClientRect(); + ov.style.left = Math.round(colRect.left + inset) + 'px'; + ov.style.width = Math.round(colRect.width - inset * 2) + 'px'; + ov.style.top = Math.round(cardRect.bottom + 6) + 'px'; + } + function openConsole(id) { + ensureConsoleOverlay(id); + positionConsoleOverlay(id); + loadConsole(id); + } + function closeConsole(id) { + var ov = consoleOverlay(id); + if (ov) ov.remove(); + } + // After every board re-render / scroll / resize: keep the open overlays + // glued to their (possibly moved) cards; close orphans whose card is gone. + function repositionConsoles() { + Object.keys(openConsoles).forEach(function(id) { + if (!openConsoles[id]) return; + if (!document.querySelector('[data-console-toggle="' + id + '"]')) { + openConsoles[id] = false; + closeConsole(id); + return; + } + ensureConsoleOverlay(id); + positionConsoleOverlay(id); + }); + } + // Re-open any consoles that were expanded before a re-render (cards rebuild // their innerHTML, so the panel state must be re-applied + reloaded). function reapplyConsoles() { + repositionConsoles(); Object.keys(openConsoles).forEach(function(id) { - if (!openConsoles[id]) return; - var panel = document.querySelector('.card-console[data-console-for="' + id + '"]'); - if (panel) { panel.hidden = false; loadConsole(id); } + if (openConsoles[id]) loadConsole(id); }); } function setConn(state, label) { @@ -809,9 +868,9 @@ export function columnsJs(opts: ColumnsJsOpts): string { }).then(function(ok) { if (ok) deleteTask(id); }); }); - // Clicks/selection inside the console body must not navigate to the task page. + // Clicks/selection inside the console overlay must not trigger card behavior. document.addEventListener('click', function(e) { - if (e.target.closest && e.target.closest('.card-console')) e.preventDefault(); + if (e.target.closest && e.target.closest('.console-overlay')) e.preventDefault(); }); // Live-console toggle lives inside the card — stop the navigation. document.addEventListener('click', function(e) { @@ -823,10 +882,40 @@ export function columnsJs(opts: ColumnsJsOpts): string { openConsoles[id] = willOpen; tog.setAttribute('aria-expanded', willOpen ? 'true' : 'false'); var chev = tog.querySelector('.cc-chevron'); if (chev) chev.textContent = willOpen ? '\\u25be' : '\\u25b8'; - var panel = document.querySelector('.card-console[data-console-for="' + id + '"]'); - if (panel) panel.hidden = !willOpen; - if (willOpen) loadConsole(id); + if (willOpen) openConsole(id); + else closeConsole(id); }); + // The overlay's own close button. + document.addEventListener('click', function(e) { + var btn = e.target.closest && e.target.closest('[data-console-close]'); + if (!btn) return; + e.preventDefault(); e.stopPropagation(); + var id = btn.getAttribute('data-console-close'); + openConsoles[id] = false; + var tog = document.querySelector('[data-console-toggle="' + id + '"]'); + if (tog) { + tog.setAttribute('aria-expanded', 'false'); + var chev = tog.querySelector('.cc-chevron'); if (chev) chev.textContent = '\\u25b8'; + } + closeConsole(id); + }); + // Dropdown behavior: a pointerdown OUTSIDE overlay + toggle closes it. + document.addEventListener('pointerdown', function(e) { + if (e.target.closest && (e.target.closest('.console-overlay') || e.target.closest('[data-console-toggle]'))) return; + Object.keys(openConsoles).forEach(function(id) { + if (!openConsoles[id]) return; + openConsoles[id] = false; + var tog = document.querySelector('[data-console-toggle="' + id + '"]'); + if (tog) { + tog.setAttribute('aria-expanded', 'false'); + var chev = tog.querySelector('.cc-chevron'); if (chev) chev.textContent = '\\u25b8'; + } + closeConsole(id); + }); + }); + // Keep overlays glued to their cards on scroll (any container) + resize. + document.addEventListener('scroll', repositionConsoles, true); + window.addEventListener('resize', repositionConsoles); // ── Task-detail modal ────────────────────────────────────────────────── (function() { diff --git a/src/server/board/styles.ts b/src/server/board/styles.ts index e216a08..ab224b4 100644 --- a/src/server/board/styles.ts +++ b/src/server/board/styles.ts @@ -215,9 +215,8 @@ export function boardV2Css(): string { .card:hover .card-del, .card:focus-within .card-del { opacity: 1; } .card-del:hover { color: #fca5a5; border-color: rgba(239,68,68,.5); background: rgba(239,68,68,.12); } - /* ── In-card live console (ported) ────────────────────────────────── */ + /* ── Console dropdown overlay (full column width, floats above cards) ── */ .card-console-wrap { margin-top: 9px; } - .card:has(.card-console:not([hidden])) { min-height: 230px; } .card-console-toggle { display: inline-flex; align-items: center; gap: 6px; background: transparent; border: 1px solid var(--b2-border); color: var(--b2-muted); font: 700 10px/1 var(--mono); text-transform: uppercase; letter-spacing: .05em; @@ -228,14 +227,27 @@ export function boardV2Css(): string { box-shadow: 0 0 0 3px rgba(56,189,248,.16); animation: ccPulse 1.6s ease-in-out infinite; } .card-console-toggle .cc-chevron { font-size: 9px; opacity: .75; } @keyframes ccPulse { 0%, 100% { opacity: 1; } 50% { opacity: .4; } } - .card-console { margin-top: 7px; } - .card-console[hidden] { display: none; } - .card-console-body { max-height: 180px; overflow-y: auto; background: rgba(0,0,0,.28); + /* The panel itself lives on (position:fixed, JS-anchored under its + card) so it escapes .card{overflow:hidden} and the narrow grid cell. */ + .console-overlay { position: fixed; z-index: 70; background: var(--b2-raised); + border: 1px solid var(--b2-border); border-radius: 10px; padding: 8px 10px 10px; + box-shadow: 0 18px 44px rgba(2,6,18,.55); animation: consoleDrop 160ms ease-out; } + @keyframes consoleDrop { from { opacity: 0; transform: translateY(-4px); } to { opacity: 1; transform: none; } } + .console-overlay-head { display: flex; align-items: center; justify-content: space-between; gap: 10px; + margin-bottom: 6px; font: 700 10px/1 var(--mono); text-transform: uppercase; letter-spacing: .05em; color: var(--b2-muted); } + .console-overlay-close { border: 1px solid var(--b2-border); background: transparent; color: var(--b2-muted); + width: 18px; height: 18px; border-radius: 50%; font: 12px/1 var(--mono); cursor: pointer; display: inline-grid; place-items: center; } + .console-overlay-close:hover { color: var(--text); border-color: var(--accent); } + .card-console-body { max-height: 260px; overflow-y: auto; background: rgba(0,0,0,.28); border: 1px solid var(--b2-border); border-radius: 8px; padding: 8px 10px; font: 11px/1.55 var(--mono); color: #cbd5e1; } .cc-line { display: flex; gap: 8px; white-space: pre-wrap; overflow-wrap: anywhere; padding: 1px 0; } .cc-line .cc-ts { color: var(--b2-muted); opacity: .8; flex: 0 0 auto; } .cc-line .cc-agent { color: var(--accent); font-weight: 600; flex: 0 0 auto; } + /* flex:1 + min-width:0 — the text must claim the line's remaining width and + wrap inside it. Without this, overflow-wrap:anywhere collapses the flex + item to its min-content width (~1 char) and the log renders vertically. */ + .cc-line .cc-text { flex: 1 1 auto; min-width: 0; } .cc-line.level-error .cc-text { color: #fca5a5; } .cc-line.level-warn .cc-text { color: #fcd34d; } .cc-line.level-bridge .cc-text { color: #c4b5fd; } diff --git a/src/server/taskDetail.ts b/src/server/taskDetail.ts index 7e2ff2a..4eb249d 100644 --- a/src/server/taskDetail.ts +++ b/src/server/taskDetail.ts @@ -190,7 +190,7 @@ export function renderTaskDetailHtml(cwd: string, id: string): string { .log-line { display:flex;gap:8px;align-items:baseline;overflow-wrap:anywhere; } .log-ts { color:var(--muted);white-space:nowrap;flex:0 0 auto; } .log-agent { color:var(--accent);white-space:nowrap;flex:0 0 auto; } - .log-text { color:var(--text);min-width:0; } + .log-text { color:var(--text);flex:1 1 auto;min-width:0; } .log-line[data-level="status"] .log-text { color:var(--status-review); } .log-line[data-level="warn"] .log-text { color:var(--status-review); } .log-line[data-level="error"] .log-text { color:#F85149; } diff --git a/tests/singleClaim.test.ts b/tests/singleClaim.test.ts new file mode 100644 index 0000000..a2210ba --- /dev/null +++ b/tests/singleClaim.test.ts @@ -0,0 +1,166 @@ +/** + * Regression tests for TSK-0237: single-claim semantics in the work loop. + * + * Bug (24.07. ~01:54): after the TSK-0230 auto-wake, an agent's waiting work + * loop claimed EVERY task assigned to it in parallel (codex held TSK-0222 + + * TSK-0235 at once) — the board lied about who works on what. + * + * Soll: + * (a) several assigned tasks ⇒ the loop claims exactly ONE (highest + * priority, oldest createdAt breaks ties); the rest stays open. + * (b) the next auto-claim happens only after the active task reaches + * review/done (or is reopened back). + * (c) server-side guard: a claim while holding an in_progress task → error + * (architect exempt, no --force for agents). + * (d) the TSK-0230 auto-wake suite keeps passing. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, rmSync } from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import { init } from '../src/cli/commands/init.js'; +import { workAgent } from '../src/cli/commands/work.js'; +import { startServer } from '../src/server/index.js'; +import { createTask, claimTask, getTask } from '../src/core/services/taskService.js'; +import type { Task } from '../src/core/schema.js'; + +const AGENT = 'kimi'; + +function sleep(ms: number): Promise { + return new Promise((r) => setTimeout(r, ms)); +} + +async function createAndAssign(serverUrl: string, title: string, priority: string): Promise { + const res = await fetch(`${serverUrl}/tasks`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ title, role: 'implementer', priority }), + }); + const task = (await res.json()) as Task; + await fetch(`${serverUrl}/tasks/${task.id}`, { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ assignedTo: AGENT }), + }); + return task.id; +} + +async function taskViaApi(serverUrl: string, id: string): Promise { + const res = (await fetch(`${serverUrl}/tasks/${id}`).then((r) => r.json())) as { task: Task }; + return res.task; +} + +async function claimViaApi(serverUrl: string, id: string, agent: string): Promise { + return fetch(`${serverUrl}/tasks/${id}`, { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ status: 'in_progress', assignedTo: agent }), + }); +} + +describe('single-claim semantics (TSK-0237)', () => { + let cwd: string; + let server: Awaited>; + + beforeEach(async () => { + cwd = mkdtempSync(join(tmpdir(), 'ah-single-claim-')); + init(cwd, { projectName: 'single-claim-test', yes: true }); + server = await startServer(cwd, { host: '127.0.0.1', port: 0 }); + }); + + afterEach(async () => { + await server.app.close().catch(() => undefined); + rmSync(cwd, { recursive: true, force: true }); + }); + + it('(a) claims exactly ONE of several assigned tasks — highest priority first', async () => { + const low = await createAndAssign(server.url, 'kimi: low prio', 'low'); + await sleep(10); + const high = await createAndAssign(server.url, 'kimi: high prio', 'high'); + + await workAgent({ serverUrl: server.url, projectCwd: cwd, agent: AGENT, role: 'implementer', timeoutSec: 2 }); + + const highTask = await taskViaApi(server.url, high); + expect(highTask.status).toBe('in_progress'); + expect(highTask.assignedTo).toBe(AGENT); + + // The other task must stay open — no parallel claim. + const lowTask = await taskViaApi(server.url, low); + expect(lowTask.status).toBe('open'); + expect(lowTask.assignedTo).toBe(AGENT); + + // A second work run while busy claims nothing (single-claim), it just waits. + await workAgent({ serverUrl: server.url, projectCwd: cwd, agent: AGENT, role: 'implementer', timeoutSec: 1 }); + expect((await taskViaApi(server.url, low)).status).toBe('open'); + }, 10_000); + + it('(a2) breaks priority ties by oldest createdAt', async () => { + const older = await createAndAssign(server.url, 'kimi: older medium', 'medium'); + await sleep(10); + const newer = await createAndAssign(server.url, 'kimi: newer medium', 'medium'); + + await workAgent({ serverUrl: server.url, projectCwd: cwd, agent: AGENT, role: 'implementer', timeoutSec: 2 }); + + expect((await taskViaApi(server.url, older)).status).toBe('in_progress'); + expect((await taskViaApi(server.url, newer)).status).toBe('open'); + }, 10_000); + + it('(b) auto-claims the next task only after the active one reaches review', async () => { + const first = await createAndAssign(server.url, 'kimi: first', 'high'); + await sleep(10); + const second = await createAndAssign(server.url, 'kimi: second', 'medium'); + + await workAgent({ serverUrl: server.url, projectCwd: cwd, agent: AGENT, role: 'implementer', timeoutSec: 2 }); + expect((await taskViaApi(server.url, first)).status).toBe('in_progress'); + expect((await taskViaApi(server.url, second)).status).toBe('open'); + + // Submit the active task for review → the agent is free for the next one. + await fetch(`${server.url}/tasks/${first}`, { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ status: 'review' }), + }); + + await workAgent({ serverUrl: server.url, projectCwd: cwd, agent: AGENT, role: 'implementer', timeoutSec: 2 }); + expect((await taskViaApi(server.url, second)).status).toBe('in_progress'); + }, 12_000); + + it('(c) server guard: claim while holding an in_progress task → 400, architect exempt', async () => { + const t1 = await createAndAssign(server.url, 'kimi: held', 'medium'); + const t2 = await createAndAssign(server.url, 'kimi: blocked', 'medium'); + + const ok = await claimViaApi(server.url, t1, AGENT); + expect(ok.status).toBe(200); + + const blocked = await claimViaApi(server.url, t2, AGENT); + expect(blocked.status).toBe(400); + const body = (await blocked.json()) as { error?: string; message?: string }; + expect(JSON.stringify(body)).toContain(t1); + // The blocked task stays open. + expect((await taskViaApi(server.url, t2)).status).toBe('open'); + + // Architect (default config: claude) may hold several threads. + const t3 = await createAndAssign(server.url, 'claude: thread one', 'medium'); + const t4 = await createAndAssign(server.url, 'claude: thread two', 'medium'); + expect((await claimViaApi(server.url, t3, 'claude')).status).toBe(200); + expect((await claimViaApi(server.url, t4, 'claude')).status).toBe(200); + }, 10_000); + + it('(c2) claimTask unit: guard throws, same-task re-claim stays idempotent', () => { + const a = createTask(cwd, { title: 'kimi: a', role: 'implementer' }); + const b = createTask(cwd, { title: 'kimi: b', role: 'implementer' }); + + claimTask(cwd, a.id, AGENT); + expect(() => claimTask(cwd, b.id, AGENT)).toThrowError(new RegExp(a.id)); + // Idempotent re-claim of the SAME task is still a no-op. + expect(() => claimTask(cwd, a.id, AGENT)).not.toThrow(); + // Architect exempt. + const c = createTask(cwd, { title: 'claude: x', role: 'architect' }); + const d = createTask(cwd, { title: 'claude: y', role: 'architect' }); + claimTask(cwd, c.id, 'claude'); + expect(() => claimTask(cwd, d.id, 'claude')).not.toThrow(); + + const { task } = getTask(cwd, b.id); + expect(task.status).toBe('open'); + }); +});