diff --git a/.claude/skills/afx/SKILL.md b/.claude/skills/afx/SKILL.md index 4e8fdf4ca..d72328eb8 100644 --- a/.claude/skills/afx/SKILL.md +++ b/.claude/skills/afx/SKILL.md @@ -153,7 +153,20 @@ afx cron enable # Enable afx cron disable # Disable ``` -There is NO `afx cron add` — create YAML files in `.af-cron/` directly. +There is NO `afx cron add` — create YAML files in `.af-cron/` directly: + +```yaml +name: Service Health Check # required, unique per workspace +schedule: "*/15 * * * *" # required, cron expression (or @hourly/@daily/@startup) +command: ./health-check.sh # required, run via shell +message: "Health alert: ${output}" # required, ${output} = trimmed command output +condition: "exitCode != 0" # optional JS expression, see below +target: architect # optional, default architect +timeout: 30 # optional, seconds, default 30 +enabled: true # optional, default true +``` + +`condition` is a JavaScript expression with two variables in scope: `output` (string — the command's trimmed output) and `exitCode` (number — 0 on success, the command's exit code on non-zero exit, 124 on timeout, -1 on spawn failure). With a `condition`, the message is delivered exactly when it evaluates truthy — including on failed runs (e.g. `exitCode != 0` alerts when the command fails). Without a `condition`, the message is delivered only when the command exits 0. ## Other commands diff --git a/codev-skeleton/.claude/skills/afx/SKILL.md b/codev-skeleton/.claude/skills/afx/SKILL.md index 2757f38ea..fa5447a0b 100644 --- a/codev-skeleton/.claude/skills/afx/SKILL.md +++ b/codev-skeleton/.claude/skills/afx/SKILL.md @@ -131,7 +131,20 @@ afx cron enable # Enable afx cron disable # Disable ``` -There is NO `afx cron add` — create YAML files in `.af-cron/` directly. +There is NO `afx cron add` — create YAML files in `.af-cron/` directly: + +```yaml +name: Service Health Check # required, unique per workspace +schedule: "*/15 * * * *" # required, cron expression (or @hourly/@daily/@startup) +command: ./health-check.sh # required, run via shell +message: "Health alert: ${output}" # required, ${output} = trimmed command output +condition: "exitCode != 0" # optional JS expression, see below +target: architect # optional, default architect +timeout: 30 # optional, seconds, default 30 +enabled: true # optional, default true +``` + +`condition` is a JavaScript expression with two variables in scope: `output` (string — the command's trimmed output) and `exitCode` (number — 0 on success, the command's exit code on non-zero exit, 124 on timeout, -1 on spawn failure). With a `condition`, the message is delivered exactly when it evaluates truthy — including on failed runs (e.g. `exitCode != 0` alerts when the command fails). Without a `condition`, the message is delivered only when the command exits 0. ## Other commands diff --git a/codev-skeleton/resources/commands/agent-farm.md b/codev-skeleton/resources/commands/agent-farm.md index 307ee0090..4234fe24a 100644 --- a/codev-skeleton/resources/commands/agent-farm.md +++ b/codev-skeleton/resources/commands/agent-farm.md @@ -478,6 +478,42 @@ afx tower status [options] --- +### afx cron + +Manage scheduled tasks defined as YAML files in `.af-cron/` at the workspace root. The Tower scheduler loads these every tick, runs due commands, and delivers messages through the normal send pipeline. + +```bash +afx cron list # List all cron tasks +afx cron status # Check task status +afx cron run # Run immediately +afx cron enable # Enable +afx cron disable # Disable +``` + +There is NO `afx cron add` — create YAML files in `.af-cron/` directly. + +**Task YAML format:** + +```yaml +name: Service Health Check # required, unique per workspace +schedule: "*/15 * * * *" # required, cron expression (or @hourly/@daily/@startup) +command: ./health-check.sh # required, run via shell +message: "Health alert: ${output}" # required, ${output} = trimmed command output +condition: "exitCode != 0" # optional JS expression, see below +target: architect # optional, default architect +timeout: 30 # optional, seconds, default 30 +enabled: true # optional, default true +``` + +**Condition environment:** `condition` is a JavaScript expression evaluated with two variables in scope: + +- `output` (string) — the command's trimmed output +- `exitCode` (number) — `0` on success, the command's exit code on non-zero exit, `124` on timeout, `-1` on spawn failure + +With a `condition`, the message is delivered exactly when the expression evaluates truthy — including on failed runs, so `condition: "exitCode != 0"` alerts when the command fails. Without a `condition`, the message is delivered only when the command exits 0. + +--- + ### afx db Database debugging and maintenance commands. diff --git a/codev/projects/bugfix-1142-tower-cron-conditions-can-t-se/status.yaml b/codev/projects/bugfix-1142-tower-cron-conditions-can-t-se/status.yaml new file mode 100644 index 000000000..e86e5f684 --- /dev/null +++ b/codev/projects/bugfix-1142-tower-cron-conditions-can-t-se/status.yaml @@ -0,0 +1,16 @@ +id: bugfix-1142 +title: tower-cron-conditions-can-t-se +protocol: bugfix +phase: pr +plan_phases: [] +current_plan_phase: null +gates: + pr: + status: pending + requested_at: '2026-07-06T09:56:04.513Z' +iteration: 1 +build_complete: false +history: [] +started_at: '2026-07-06T09:41:52.658Z' +updated_at: '2026-07-06T09:56:04.513Z' +pr_ready_for_human: true diff --git a/codev/resources/commands/agent-farm.md b/codev/resources/commands/agent-farm.md index 95c22f8e2..aa5d00432 100644 --- a/codev/resources/commands/agent-farm.md +++ b/codev/resources/commands/agent-farm.md @@ -700,6 +700,42 @@ Displays local tower status plus cloud registration details: tower name, ID, con --- +### afx cron + +Manage scheduled tasks defined as YAML files in `.af-cron/` at the workspace root. The Tower scheduler loads these every tick, runs due commands, and delivers messages through the normal send pipeline. + +```bash +afx cron list # List all cron tasks +afx cron status # Check task status +afx cron run # Run immediately +afx cron enable # Enable +afx cron disable # Disable +``` + +There is NO `afx cron add` — create YAML files in `.af-cron/` directly. + +**Task YAML format:** + +```yaml +name: Service Health Check # required, unique per workspace +schedule: "*/15 * * * *" # required, cron expression (or @hourly/@daily/@startup) +command: ./health-check.sh # required, run via shell +message: "Health alert: ${output}" # required, ${output} = trimmed command output +condition: "exitCode != 0" # optional JS expression, see below +target: architect # optional, default architect +timeout: 30 # optional, seconds, default 30 +enabled: true # optional, default true +``` + +**Condition environment:** `condition` is a JavaScript expression evaluated with two variables in scope: + +- `output` (string) — the command's trimmed output +- `exitCode` (number) — `0` on success, the command's exit code on non-zero exit, `124` on timeout, `-1` on spawn failure + +With a `condition`, the message is delivered exactly when the expression evaluates truthy — including on failed runs, so `condition: "exitCode != 0"` alerts when the command fails. Without a `condition`, the message is delivered only when the command exits 0. + +--- + ### afx db Database debugging and maintenance commands. diff --git a/codev/state/bugfix-1142_thread.md b/codev/state/bugfix-1142_thread.md new file mode 100644 index 000000000..246581fed --- /dev/null +++ b/codev/state/bugfix-1142_thread.md @@ -0,0 +1,39 @@ +# Builder thread — bugfix-1142 + +Issue #1142: tower-cron conditions can't see `exitCode` (ReferenceError every run) and failure runs can never deliver alerts. + +## Investigate (2026-07-06) + +Reproduced and confirmed root cause, all in `packages/codev/src/agent-farm/servers/tower-cron.ts`: + +1. `evaluateCondition()` builds `new Function('output', 'return ' + condition)` — `exitCode` is not in scope, so `condition: "exitCode != 0"` throws `ReferenceError` every run (12,453 occurrences in my local `~/.agent-farm/tower.log`). The catch at the call site forces `shouldNotify = false`. +2. `runCommand()` rejects on any non-zero exit, so the exit code is never captured as data — `executeTask` classifies it as `result = 'failure'`. +3. Delivery gate is `if (shouldNotify && result === 'success')` — failure runs only log a WARN. "Alert me when this command fails" is inexpressible. + +Doc surfaces for the condition environment (`output: string`, `exitCode: number`): +- `.claude/skills/afx/SKILL.md` + `codev-skeleton/.claude/skills/afx/SKILL.md` (`## afx cron` section, currently CLI-commands-only) +- `codev/resources/commands/agent-farm.md` + `codev-skeleton/resources/commands/agent-farm.md` (no cron section yet — will add one) + +Fix design is prescribed in the issue (carry it as-is): capture `{output, exitCode}` in `runCommand` (resolve on plain non-zero exit, keep rejecting on `error.killed`/`error.signal`/spawn failure), pass `exitCode` as second param to conditions, deliver on condition-true regardless of success/failure split, keep no-condition tasks delivering only on exit 0, keep `last_result` semantics by exit code. + +Scope: ~100 LOC in one source file + regression tests + docs. Well within BUGFIX. → PHASE_COMPLETE + +## Fix (2026-07-06) + +Implemented the prescribed design in `tower-cron.ts` (commit ecf294ae): +- `runCommand` now resolves `{ output, exitCode }`; plain non-zero exits resolve (exit code is data), only spawn failures / timeout-kills reject (exitCode -1 / 124 in the catch). +- `evaluateCondition(condition, output, exitCode = 0)` — both variables in Function scope. +- Delivery: with a condition, the condition alone decides (failure runs can deliver); without one, deliver only on exit 0 (unchanged). `last_result` semantics unchanged. +- 8 new regression tests (34 total in tower-cron.test.ts, all pass). Full suite: 3440 passed / 48 pre-existing skips. +- Docs: condition environment (`output`, `exitCode`) documented in the afx skill `## afx cron` section and a new `### afx cron` section in agent-farm.md — BOTH trees (codev/ + codev-skeleton/, and .claude/skills + skeleton skills). +- Root `pnpm build` passes. + +## PR (2026-07-06) + +- Porch checks (build + tests) passed; advanced to pr phase. +- PR #1143 opened: https://github.com/cluesmith/codev/pull/1143 (Fixes #1142). +- End-to-end semantics verified against built dist + real Node exec: non-zero exit → `code=3, killed=false, signal=null` (resolves as data); timeout → `code=null, killed=true, signal=SIGTERM` (rejects → 124). Matches the implementation's branch conditions exactly. +- CMAP note: `consult --protocol bugfix --type pr` needed explicit `--project-id bugfix-1142` (auto-detect hit "Multiple projects found" in the worktree). +- CMAP verdicts: gemini=APPROVE (HIGH), codex=APPROVE (HIGH), claude=APPROVE (HIGH). No key issues from any reviewer. +- CI on PR #1143: all 6 checks green, mergeable. +- Requested the `pr` gate via porch done; waiting for human approval. diff --git a/packages/codev/src/agent-farm/__tests__/tower-cron.test.ts b/packages/codev/src/agent-farm/__tests__/tower-cron.test.ts index 6c318edea..d8490561b 100644 --- a/packages/codev/src/agent-farm/__tests__/tower-cron.test.ts +++ b/packages/codev/src/agent-farm/__tests__/tower-cron.test.ts @@ -285,6 +285,22 @@ describe('evaluateCondition', () => { it('throws on invalid condition', () => { expect(() => evaluateCondition("this is not valid js }{", 'test')).toThrow(); }); + + // Regression: #1142 — conditions referencing exitCode threw ReferenceError + it('exposes exitCode to conditions', () => { + expect(evaluateCondition('exitCode != 0', '', 1)).toBe(true); + expect(evaluateCondition('exitCode != 0', '', 0)).toBe(false); + expect(evaluateCondition('exitCode === 124', '', 124)).toBe(true); + }); + + it('defaults exitCode to 0 when omitted', () => { + expect(evaluateCondition('exitCode === 0', 'anything')).toBe(true); + }); + + it('output-only conditions work unchanged alongside exitCode', () => { + expect(evaluateCondition("output.includes('FAIL')", 'all FAIL', 0)).toBe(true); + expect(evaluateCondition("output.includes('FAIL')", 'all pass', 1)).toBe(false); + }); }); describe('executeTask', () => { @@ -457,6 +473,194 @@ describe('executeTask', () => { expect(mockFormatBuilderMessage).toHaveBeenCalledWith('af-cron', 'Count is 42 items'); }); + // Regression: #1142 — "alert me when this command fails" was inexpressible: + // exitCode conditions threw ReferenceError and failure runs never delivered. + it('delivers when an exitCode condition is true on non-zero exit', async () => { + const ws = createTestWorkspace(); + const mockSession = { write: vi.fn() }; + const mockDeps = makeMockDeps({ + getKnownWorkspacePaths: () => [ws], + getTerminalManager: () => ({ + getSession: () => mockSession, + }), + }); + initCron(mockDeps); + + mockExec.mockImplementation((_cmd: string, _opts: unknown, cb: Function) => { + const err = new Error('Command failed') as Error & { code: number; killed: boolean }; + err.code = 2; + err.killed = false; + cb(err, 'service down', ''); + }); + + const task: CronTask = { + name: 'Health Check', + schedule: '*/15 * * * *', + enabled: true, + command: './health-check.sh', + condition: 'exitCode != 0', + message: 'Service Health Alert: ${output}', + target: 'architect', + timeout: 30, + workspacePath: ws, + }; + + const { result } = await executeTask(task); + expect(result).toBe('failure'); // last_result still tracks exit code + // No ReferenceError WARN from condition evaluation + expect(mockDeps.log).not.toHaveBeenCalledWith( + 'WARN', + expect.stringContaining('Condition evaluation failed'), + ); + expect(mockSession.write).toHaveBeenCalled(); + expect(mockBroadcastMessage).toHaveBeenCalledWith( + expect.objectContaining({ content: 'Service Health Alert: service down' }), + ); + }); + + it('does not deliver when an exitCode condition is false on clean exit', async () => { + const ws = createTestWorkspace(); + const mockSession = { write: vi.fn() }; + const mockDeps = makeMockDeps({ + getKnownWorkspacePaths: () => [ws], + getTerminalManager: () => ({ + getSession: () => mockSession, + }), + }); + initCron(mockDeps); + + mockExec.mockImplementation((_cmd: string, _opts: unknown, cb: Function) => { + cb(null, 'all healthy', ''); + }); + + const task: CronTask = { + name: 'Health Check', + schedule: '*/15 * * * *', + enabled: true, + command: './health-check.sh', + condition: 'exitCode != 0', + message: 'Service Health Alert: ${output}', + target: 'architect', + timeout: 30, + workspacePath: ws, + }; + + const { result } = await executeTask(task); + expect(result).toBe('success'); + expect(mockSession.write).not.toHaveBeenCalled(); + }); + + it('does not deliver on non-zero exit when no condition is set', async () => { + const ws = createTestWorkspace(); + const mockSession = { write: vi.fn() }; + const mockDeps = makeMockDeps({ + getKnownWorkspacePaths: () => [ws], + getTerminalManager: () => ({ + getSession: () => mockSession, + }), + }); + initCron(mockDeps); + + mockExec.mockImplementation((_cmd: string, _opts: unknown, cb: Function) => { + const err = new Error('Command failed') as Error & { code: number; killed: boolean }; + err.code = 1; + err.killed = false; + cb(err, '', 'flaky failure'); + }); + + const task: CronTask = { + name: 'No Condition', + schedule: '*/15 * * * *', + enabled: true, + command: 'flaky-command', + message: 'Done: ${output}', + target: 'architect', + timeout: 30, + workspacePath: ws, + }; + + const { result, output } = await executeTask(task); + expect(result).toBe('failure'); + expect(output).toBe('flaky failure'); // stderr captured when stdout empty + expect(mockSession.write).not.toHaveBeenCalled(); + }); + + it('reports timeout as exitCode 124 so exitCode conditions still fire', async () => { + const ws = createTestWorkspace(); + const mockSession = { write: vi.fn() }; + const mockDeps = makeMockDeps({ + getKnownWorkspacePaths: () => [ws], + getTerminalManager: () => ({ + getSession: () => mockSession, + }), + }); + initCron(mockDeps); + + mockExec.mockImplementation((_cmd: string, _opts: unknown, cb: Function) => { + const err = new Error('Command timed out') as Error & { killed: boolean; signal: string }; + err.killed = true; + err.signal = 'SIGTERM'; + cb(err, 'partial', ''); + }); + + const task: CronTask = { + name: 'Timeout Task', + schedule: '*/15 * * * *', + enabled: true, + command: 'sleep 999', + condition: 'exitCode != 0', + message: 'Timed out: ${output}', + target: 'architect', + timeout: 1, + workspacePath: ws, + }; + + const { result } = await executeTask(task); + expect(result).toBe('failure'); + expect(mockSession.write).toHaveBeenCalled(); + expect(mockBroadcastMessage).toHaveBeenCalledWith( + expect.objectContaining({ content: 'Timed out: partial' }), + ); + }); + + it('does not deliver a timeout when no condition is set (WARN-only path)', async () => { + const ws = createTestWorkspace(); + const mockSession = { write: vi.fn() }; + const mockDeps = makeMockDeps({ + getKnownWorkspacePaths: () => [ws], + getTerminalManager: () => ({ + getSession: () => mockSession, + }), + }); + initCron(mockDeps); + + mockExec.mockImplementation((_cmd: string, _opts: unknown, cb: Function) => { + const err = new Error('Command timed out') as Error & { killed: boolean; signal: string }; + err.killed = true; + err.signal = 'SIGTERM'; + cb(err, '', ''); + }); + + const task: CronTask = { + name: 'Timeout No Condition', + schedule: '*/15 * * * *', + enabled: true, + command: 'sleep 999', + message: 'Done', + target: 'architect', + timeout: 1, + workspacePath: ws, + }; + + const { result } = await executeTask(task); + expect(result).toBe('failure'); + expect(mockSession.write).not.toHaveBeenCalled(); + expect(mockDeps.log).toHaveBeenCalledWith( + 'WARN', + expect.stringContaining("Cron command failed for 'Timeout No Condition'"), + ); + }); + it('uses custom cwd when specified', async () => { const ws = createTestWorkspace(); const mockDeps = makeMockDeps({ getKnownWorkspacePaths: () => [ws] }); diff --git a/packages/codev/src/agent-farm/servers/tower-cron.ts b/packages/codev/src/agent-farm/servers/tower-cron.ts index 5ec01626e..e8899a7d6 100644 --- a/packages/codev/src/agent-farm/servers/tower-cron.ts +++ b/packages/codev/src/agent-farm/servers/tower-cron.ts @@ -221,17 +221,21 @@ export async function executeTask(task: CronTask): Promise<{ result: string; out deps.log('INFO', `Executing cron task: ${task.name}`); let output: string; + let exitCode: number; let result: string; try { - output = await runCommand(task.command, { + ({ output, exitCode } = await runCommand(task.command, { cwd: task.cwd ?? task.workspacePath, timeout: task.timeout * 1000, - }); - result = 'success'; + })); + result = exitCode === 0 ? 'success' : 'failure'; } catch (err: unknown) { - const execErr = err as { stdout?: string; stderr?: string; message?: string }; - output = execErr.stdout ?? execErr.stderr ?? execErr.message ?? String(err); + // Infrastructure error: spawn failure or timeout/kill. 124 mirrors the + // coreutils `timeout` convention; -1 means the process never ran cleanly. + const execErr = err as { stdout?: string; stderr?: string; message?: string; killed?: boolean }; + output = execErr.stdout || execErr.stderr || execErr.message || String(err); + exitCode = execErr.killed ? 124 : -1; result = 'failure'; } @@ -242,23 +246,25 @@ export async function executeTask(task: CronTask): Promise<{ result: string; out // Update SQLite state updateTaskState(taskId, task.workspacePath, task.name, nowSeconds, result, truncatedOutput); - // Evaluate condition - let shouldNotify = true; + // Decide delivery. With a condition, the condition alone decides (a non-zero + // exit is data the condition can inspect via exitCode, not noise). Without + // one, deliver only on clean exit so flaky commands don't alert-spam. + let shouldNotify: boolean; if (task.condition) { try { - shouldNotify = evaluateCondition(task.condition, output.trim()); + shouldNotify = evaluateCondition(task.condition, output.trim(), exitCode); } catch (err) { deps.log('WARN', `Condition evaluation failed for '${task.name}': ${err}`); shouldNotify = false; } + } else { + shouldNotify = result === 'success'; } - if (shouldNotify && result === 'success') { + if (shouldNotify) { const renderedMessage = task.message.replace(/\$\{output\}/g, output.trim()); deliverMessage(task, renderedMessage); } else if (result === 'failure') { - // Command itself errored (non-zero exit, timeout, etc.) — log but don't alert. - // Command errors are infrastructure noise, not actionable CI failures. deps.log('WARN', `Cron command failed for '${task.name}': ${output.trim().slice(0, 200)}`); } @@ -266,7 +272,10 @@ export async function executeTask(task: CronTask): Promise<{ result: string; out return { result, output: truncatedOutput }; } -function runCommand(command: string, options: { cwd: string; timeout: number }): Promise { +function runCommand( + command: string, + options: { cwd: string; timeout: number }, +): Promise<{ output: string; exitCode: number }> { return new Promise((resolve, reject) => { exec(command, { cwd: options.cwd, @@ -274,14 +283,20 @@ function runCommand(command: string, options: { cwd: string; timeout: number }): maxBuffer: 1024 * 1024, // 1MB env: process.env, }, (error, stdout, stderr) => { - if (error) { - // Attach stdout/stderr for the caller - (error as unknown as Record).stdout = stdout; - (error as unknown as Record).stderr = stderr; - reject(error); - } else { - resolve(stdout); + if (!error) { + resolve({ output: stdout, exitCode: 0 }); + return; + } + // A numeric code with no kill signal means the process ran and exited + // non-zero — that's data for conditions, not an infrastructure error. + if (typeof error.code === 'number' && !error.killed && !error.signal) { + resolve({ output: stdout || stderr, exitCode: error.code }); + return; } + // Spawn failure or timeout/kill — attach stdout/stderr for the caller + (error as unknown as Record).stdout = stdout; + (error as unknown as Record).stderr = stderr; + reject(error); }); }); } @@ -290,10 +305,10 @@ function runCommand(command: string, options: { cwd: string; timeout: number }): // Condition evaluation // ============================================================================ -export function evaluateCondition(condition: string, output: string): boolean { +export function evaluateCondition(condition: string, output: string, exitCode = 0): boolean { // eslint-disable-next-line no-new-func - const fn = new Function('output', 'return ' + condition); - return !!fn(output); + const fn = new Function('output', 'exitCode', 'return ' + condition); + return !!fn(output, exitCode); } // ============================================================================