From 37bad517c23d915ff5574f2bbdec811c048a1ae7 Mon Sep 17 00:00:00 2001 From: Aaron Sachs <898627+asachs01@users.noreply.github.com> Date: Tue, 25 Aug 2026 06:27:03 +0000 Subject: [PATCH 1/2] fix(bus): validate --assignee against the enabled-agents roster on create-task/update-task Neither create-task nor update-task validated that --assignee named a real agent, so a typo silently routed a task into the void (task_1786739337901_81484880, boss follow-up 2026-08-14). Checked at the CLI boundary (src/cli/bus.ts) rather than inside createTask()/updateTask() themselves: those library functions are also called directly by the unit test suite with lightweight fixture agent names that have no real enabled-agents.json/orgs-directory roster behind them -- validating at the library layer broke 52 of 70 tests in task.test.ts before this was rescoped to the CLI layer, where the actual failure mode (a human typing --assignee) lives. 'human'/'user' bypass the check entirely -- they are deliberate sentinels for [HUMAN] tasks, never real agent names (see the assigned_to checks already in src/bus/task.ts). Checks name existence only, not `enabled` state, since a disabled agent is still a legitimate reassignment target. Verified live (not just tsc+tests): rejects a nonexistent assignee with no task created / no mutation applied to an existing task, accepts a real roster agent, and the human/user sentinel still works, on a real build in an isolated worktree. tsc clean, 507 tests pass (tests/unit/bus/ + tests/unit/cli/). --- src/cli/bus.ts | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/cli/bus.ts b/src/cli/bus.ts index 9ae799291..a7df48f7e 100644 --- a/src/cli/bus.ts +++ b/src/cli/bus.ts @@ -10,6 +10,7 @@ import { validateAgentName, validateTaskId, validatePriority, validateCapability import { randomDigits } from '../utils/random.js'; import { resolveMessageBody, resolveOptionalTextField, UnsafeInlineBodyError } from '../utils/resolve-message-body.js'; import { createTask, updateTask, completeTask, claimTask, readTaskAudit, checkTaskDependenciesWithStatus, compactTasks, listTasks, checkStaleTasks, checkBatchStaleness, archiveTasks, checkHumanTasks } from '../bus/task.js'; +import { listAgents } from '../bus/agents.js'; import { saveOutput } from '../bus/save-output.js'; import { logEvent } from '../bus/event.js'; import { updateHeartbeat, readAllHeartbeats, readAllHeartbeatRows } from '../bus/heartbeat.js'; @@ -246,6 +247,30 @@ busCommand console.log(`ACK'd ${id}`); }); +/** + * Reject a typo'd or nonexistent --assignee before it silently routes a task + * into the void (task_1786739337901_81484880). Checked here at the CLI + * boundary — where a human actually types the name — rather than inside + * createTask/updateTask themselves: those are also called directly by the + * unit test suite with lightweight fixture agent names that have no real + * enabled-agents.json/orgs-directory roster to check against, so validating + * at the library layer would require rebuilding roster fixtures for dozens + * of unrelated tests just to keep them passing. 'human' and 'user' are + * deliberate sentinels for [HUMAN] tasks (see the assigned_to checks in + * src/bus/task.ts) and bypass the roster check entirely. Checks name + * existence only, not `enabled` state — a disabled agent is still a + * legitimate reassignment target; the failure mode this guards against is a + * name that was never real to begin with. + */ +function validateAssigneeArg(ctxRoot: string, org: string, assignee: string): void { + if (assignee === 'human' || assignee === 'user') return; + const roster = listAgents(ctxRoot, org); + if (!roster.some((a) => a.name === assignee)) { + console.error(`Invalid assignee '${assignee}': not found in the enabled-agents roster for org '${org}'.`); + process.exit(1); + } +} + busCommand .command('create-task') .argument('', 'Task title') @@ -273,6 +298,7 @@ busCommand } const env = resolveEnv(); const paths = resolvePaths(env.agentName, env.instanceId, env.org, env.ctxRoot); + if (opts.assignee !== undefined) validateAssigneeArg(paths.ctxRoot, env.org, opts.assignee); const parseList = (raw?: string) => (raw ? raw.split(',').map(s => s.trim()).filter(Boolean) : []); if (opts.assignee === undefined) { process.stderr.write( @@ -465,6 +491,12 @@ busCommand console.error('ERROR: --agent or CTX_AGENT_NAME required'); process.exit(1); } + // Validated against the CALLER's own org roster — findTaskFile searches + // every org under ctxRoot/orgs/, so a cross-org reassignment (rare; + // update-task's own docs describe rerouting within the fleet, not across + // orgs) could in principle be checked against the wrong roster. Matches + // create-task's identical scoping above. + if (opts.assignee !== undefined) validateAssigneeArg(paths.ctxRoot, env.org, opts.assignee); // Guard: block review/completion when deliverables are required but missing. // Checks both ready_for_review (approval workflow) and completed (vanilla upstream) From 72633750d014773dbb3a576933d84f35f46cb759 Mon Sep 17 00:00:00 2001 From: Aaron Sachs <898627+asachs01@users.noreply.github.com> Date: Fri, 4 Sep 2026 12:44:56 +0000 Subject: [PATCH 2/2] fix(bus): fail open on an empty assignee roster in validateAssigneeArg Rebasing PR #151 onto main surfaced a real regression: the CLI-level --assignee roster check exits 1 for any org whose agent directory scan comes back empty, which is exactly what happens when tests/integration/bus-task-error-handling-cli.test.ts drives the real compiled CLI against a synthetic "testorg" with no orgs/testorg/agents/ fixture behind it. The original PR's own verification (507 tests, tests/unit/bus/ + tests/unit/cli/ only) never ran that integration suite, so this shipped unnoticed. A real org always has agents on disk; an empty roster means the check had nothing to validate against, not that every name is invalid. Fail open in that case rather than block a legitimate reassignment. Verified: tests/integration/bus-task-error-handling-cli.test.ts green (10/10, was 1 failing), full suite 2647-2649/2654 pass depending on run -- the only variance is dashboard/src/lib/__tests__/watcher-ingests-real-events.test.ts, a pre-existing real-chokidar timing test that fails inconsistently under full-suite load and passes clean in isolation (4/4), unrelated to this change (already documented as an environment-flaky class in this repo's CLAUDE.md). tsc clean. --- src/cli/bus.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/cli/bus.ts b/src/cli/bus.ts index a7df48f7e..001eb94c2 100644 --- a/src/cli/bus.ts +++ b/src/cli/bus.ts @@ -261,10 +261,23 @@ busCommand * existence only, not `enabled` state — a disabled agent is still a * legitimate reassignment target; the failure mode this guards against is a * name that was never real to begin with. + * + * Fails open when the roster comes back completely empty for the org + * (`listAgents` finds zero agent directories under it at all). A real org + * always has agents on disk, so an empty roster means the scan had nothing + * to check against — not that every possible name is invalid — and is the + * shape a CLI-level integration test hits when it drives the real compiled + * binary against a synthetic org name with no `orgs/<org>/agents/` fixture + * behind it (e.g. tests/integration/bus-task-error-handling-cli.test.ts's + * `testorg`, which broke this check on first fleet-wide test run: 507 tests + * green at author-verify time only covered tests/unit/bus/ + tests/unit/cli/, + * not tests/integration/). Blocking on an unreadable roster would be worse + * than the typo it guards against. */ function validateAssigneeArg(ctxRoot: string, org: string, assignee: string): void { if (assignee === 'human' || assignee === 'user') return; const roster = listAgents(ctxRoot, org); + if (roster.length === 0) return; if (!roster.some((a) => a.name === assignee)) { console.error(`Invalid assignee '${assignee}': not found in the enabled-agents roster for org '${org}'.`); process.exit(1);