Uh oh!
There was an error while loading. Please reload this page.
[Bugfix #1142] Expose exitCode to cron conditions; let condition-true failure runs deliver alerts - #1143
Conversation
…ion-true failure runs deliver
- runCommand resolves { output, exitCode } for plain non-zero exits instead
of rejecting; only spawn failures and timeout/kills still reject (reported
as exitCode -1 / 124)
- evaluateCondition takes exitCode as a second scope variable, so
condition: "exitCode != 0" no longer throws ReferenceError
- Delivery gate: with a condition, the condition alone decides delivery
(failure runs can now alert); without one, deliver only on exit 0 as before
- last_result semantics unchanged (success/failure by exit code)
- Regression tests for exitCode conditions, output-only conditions,
no-condition failure runs, and the timeout path
- Document the condition environment in the afx skill and agent-farm.md,
both treeswaleedkadous
commented
Jul 6, 2026
Architect ReviewNice, tight execution — this carries the issue's prescribed design faithfully, and the docs work goes beyond the ask in a good way. Low-risk change (production diff is 37/22 in one file,
One intentional semantic change worth naming for the record (it's documented, and correct): an output-only condition that evaluates true on a failed run now delivers, where before failure suppressed everything. That's the point of the fix — "condition alone decides" — just noting it's a behavior change for pre-existing condition-bearing tasks, not a pure bugfix. CMAP unanimous (3× APPROVE/HIGH), CI 6/6 green. |
Uh oh!
There was an error while loading. Please reload this page.
…writer claim Merge auto-resolved — no textual conflict with #1143's tower-cron rewrite, since my change is in tower-routes/session-submit and theirs is in tower-cron. RE-VERIFIED rather than ported. #1143 rewrote the cron delivery DECISION (exitCode resolves as data; a conditioned task now delivers whenever its condition is truthy, failures included) but NOT the delivery MECHANISM: deliverMessage still calls writeMessageToSession directly (tower-cron.ts:338), taking no lock. So the enumeration stands — cron remains an uncovered writer. What changed is the claim's WEIGHT, and the docstring now says so: delivery used to require a clean exit, so the uncovered-writer risk is now exercised on more occasions than when the list was written. The claim is unchanged; its exposure is not. ALSO FIXED — a real regression the merge surfaced, caught by this spec's own phase-7 parity tests. #1143 documented the new condition/exitCode cron semantics in .claude/skills/afx/SKILL.md but NOT in the .codex twin, in BOTH trees (workspace and skeleton). That is precisely the "updated one, forgot the twin" defect the phase-7 tests were written to catch, and they caught it on the first run after the merge: x keeps the two skill trees byte-identical (spec-1273 phase 7) x self-hosted root: provider skill sets and bytes (#1196 parity) x shipped skeleton: provider skill sets and bytes (#1196 parity) Ported the block verbatim to both .codex copies. Docs-only, no behaviour change. Codex-driven agents would otherwise have had no documentation of `condition` or `exitCode` at all. Suite 4106 passing, 0 failed, build clean.
Records three things worth carrying to the review: 1. My own porch drift — I had wandered into phase-3 work while porch sat on phase_2 waiting for my consults. When several instructions arrive mid-turn, the orchestrator's state is the thread most likely to drop, because nothing prompts for it. 2. The --delay relocation and why the spec criterion was amended rather than silently changed: it was authored against the pre-1280-rewrite world. 3. main was red and it was NOT mine — #1143 updated both .claude copies of the afx skill and neither .codex copy. Verified against origin/main directly rather than assuming my merge caused it, then escalated instead of fixing a file I had been told not to touch. The architect's root cause on that generalises and is recorded: #1143's green CI predated the parity guards the repo has since grown, so it was true when produced and false when used — the same shape as this project's recurring lesson (an artifact asserting something adjacent to the truth), one level up.
Summary
Fixes the two-layer bug in
tower-cron.tswhere (1) conditions referencingexitCodethrewReferenceErroron every run, and (2) failure runs could never deliver alerts — making "alert me when this command fails" inexpressible.Root Cause
evaluateCondition()builtnew Function('output', 'return ' + condition)— onlyoutputwas in scope, socondition: "exitCode != 0"threwReferenceErrorevery run (12k+ WARN lines in a real tower.log). The catch forcedshouldNotify = false.runCommand()rejected on any non-zero exit, so the exit code was never available as data.if (shouldNotify && result === 'success')suppressed delivery on exactly the runs a monitoring task cares about.Fix
runCommand()resolves{ output, exitCode }. A plain non-zero exit (numericerror.code, no kill signal) resolves with that exit code — it's data, not an error. Spawn failures and timeout/kills still reject, reported asexitCode -1/124respectively.evaluateCondition(condition, output, exitCode = 0)exposes both variables to the condition. Output-only conditions are unchanged.conditiondelivers exactly when the condition is truthy (including failed runs). A task without aconditionkeeps today's behavior — deliver only on exit 0 — so existing tasks don't start alert-spamming on flaky commands. Non-delivering failures keep the WARN log.last_resultincron_tasksstill records success/failure by exit code — only delivery logic changed.output: string,exitCode: number) is now documented in the afx skill's## afx cronsection and a new### afx cronsection inagent-farm.md, in BOTH trees (instance + skeleton).Test Plan
tower-cron.test.ts(34 total, all pass):exitCode != 0condition delivers on non-zero exit, no ReferenceErrorexitCode != 0condition does not deliver on clean exitevaluateConditionunit coverage for exitCode scope + defaultpnpm buildpasses.Fixes#1142
🤖 Generated with Claude Code