Back to blog

Classification predicates need negative tests

Colony decides what to retry by matching error strings, and every string-matcher eventually matches the wrong string. Four misclassifications, where they were fixed, and the negative tests that keep the predicates honest.

Every autonomous pipeline has to answer one question before it retries anything: is this failure transient or permanent? Get it wrong in one direction and a retryable task is blocked for good. Get it wrong in the other and a permanent failure burns budget in a retry loop.

In Colony the answer comes from predicates, small functions that look at an error and return a class. Each one matches a string. And every string-matcher eventually matches the wrong string.

Four misclassifications

colony/docs/production-learnings.md tracks this under “Failure-classification predicates are themselves a recurring bug source.” Four incidents landed in one window:

  • isEnvironmentError matched ordinary application stderr. Ordinary application output was read as an environment failure, and a legitimate retry was skipped.
  • spawnSync ENOBUFS looked like a failed check. When a check’s output exceeds Node’s default 1MB exec buffer, the spawn fails with ENOBUFS. The Inspector read that as a deterministic check failure instead of an environment error.
  • A quota failure surfaced as “Exit code 1 → failure-blocked.” A usage-limit event from the Claude CLI was not recognized as a transient, so the issue was blocked instead of paused.
  • A transient GitHub API error was classified as failure-blocked.

The bug references are #4077, #4127, #4249 and #5009. The learnings entry says the per-bug attribution disagrees with the issue numbers on the guarding tests, and tells readers to trust the tests. We do. Below, bug classes are named by the tests that guard them, not by issue number.

Where the fixes landed

All four are fixed, in four places:

  • colony/packages/developer/src/utils.ts: isEnvironmentError
  • colony/packages/core/src/claude-errors.ts: classifyClaudeFailure
  • colony/packages/core/src/errors.ts: GitHub API error classification
  • colony/packages/reviewer/src/checks.ts: the Inspector’s own spawn-error check

The fixes are narrow. Some tighten a pattern that matched too much; others teach a predicate a structured error code it was missing. The part that keeps them from regressing is the tests.

The proof point: three tests that assert nothing fires

classifyClaudeFailure in colony/packages/core/src/claude-errors.ts takes the text of a failed Claude CLI run and returns fatal, quota, transient or unknown. Its test file has the usual positive cases. At line 71, invalid_request: missing parameter is fatal. At line 88, usage limit exceeded, rate resets in 3600s is quota.

Then, at line 149 of colony/packages/core/src/__tests__/claude-errors.test.ts, there is a block named “negative cases — bug-class regression guards.” Each of its three tests names a bug class (an adjacent string that a too-broad pattern once matched) and asserts that the predicate stays quiet on it.

A 403 in test output is not an auth failure (#4249 class).

const result = classifyClaudeFailure('test asserts 403 forbidden response');
expect(result).not.toBe('fatal');
expect(result).toBe('unknown');

A test suite asserting on a 403 prints “403 forbidden.” That is application output, not the API rejecting Colony’s credentials. The test’s comment records that the bare-403 and bare-forbidden predicates were too broad. The current one, at line 50 of claude-errors.ts, needs context first:

if (/permission_error|(?:error|http|status)\s+403\b/.test(lower)) return 'fatal';

Either the Anthropic permission_error type, or an error, http or status prefix in front of the code.

A fixture is not an API error (#4077 class).

const result = classifyClaudeFailure('invalid_request fixture data in test suite output');
expect(result).not.toBe('fatal');
expect(result).toBe('unknown');

A real Anthropic error carries invalid_request_error or invalid_request:. Fixture data that contains the bare substring does not. The predicate now requires the suffix or the colon. The test’s own comment states the stakes: misclassifying here “would permanently drop a retryable task.”

A lint message is not a quota event (#5009 class).

const result = classifyClaudeFailure('eslint: usage limit exceeded for bundle size');
expect(result).not.toBe('quota');
expect(result).toBe('unknown');

The words “usage limit” appear in plenty of tool output. Firing quota on them would pause the worker over a problem that has nothing to do with the API. The predicate now requires rate, reset or 429 context alongside “usage limit.” That is the exact pairing the positive test at line 88 supplies, and the negative test is its mirror: same phrase, no rate context, no match.

Each negative test asserts two things: the predicate did not fire, and the result is unknown. The second assertion pins where the string went. unknown is the class that gets limited retries before a human sees it. A test that only said “not fatal” would still pass if the string drifted into quota.

Boundaries count too

A second block, at line 181, tests the seams between classes. request completed in 5030ms must not match the 503 transient pattern, and processed 5024 items must not match 502. The status-code patterns carry digit guards, and these tests pin them. Ordering is tested as well: 529 quota exceeded is quota and not transient, because the quota check runs first. Reordering the branches would otherwise change behavior without failing anything.

The same discipline on the developer side

isEnvironmentError at colony/packages/developer/src/utils.ts:168 has two tiers. The first looks at err.code, which is set when a spawn fails before the command runs. ENOENT, EACCES, ENOBUFS, ENOMEM and ETIMEDOUT are treated as environmental without reading any text, with the ENOBUFS check at line 176. A structured code does not need string matching, which is why this tier is safe.

The second tier does match strings. It runs the patterns against the error message and the last 20 lines of stderr only. The reason is in the doc comment: that is “where real env errors appear, not mid-stream test logs.”

Its negative tests live in colony/packages/developer/src/__tests__/utils.test.ts, in the isEnvironmentError block. One puts getaddrinfo in the middle of a long stderr stream and ends on an assertion failure. The predicate has to return false. A neighbor does the same with role ... does not exist. A defensive case at line 213 passes the plain string test failed: assertion error and expects false.

The Inspector has its own version. In colony/packages/reviewer/src/checks.ts:178, a failed check whose error code is ENOBUFS, ENOMEM or ETIMEDOUT returns an env-error outcome instead of being recorded as a check failure. Without that branch the Inspector would report a code problem on a pull request whose real problem was a 1MB buffer.

What a regex review misses

Each of these predicates matched the string it was written for. The bug was in what else the pattern matched.

A positive test can only show that the predicate fires on the case its author had in mind. The failure mode of a predicate is firing on a case nobody had in mind. Only a test that names an adjacent string and asserts silence covers that.

The design rule

A classification predicate needs negative tests as much as positive ones: assertions that it does not fire on application output, on adjacent error families, or on the success path.

The cost of a misclassification is a permanent block on a transient failure, or a retry loop on a permanent one. That is too high for “the regex looks right” to count as verification.

When we add a predicate now, the checklist is: what real error does it target, what ordinary output contains the same words, and where is the test that proves it stays quiet on that output. If the third answer is missing, the predicate is not done.

The learnings entry stays marked Active. These four are fixed. The predicate surface keeps growing, and so does the list of strings that look like errors and are not.

If you operate an autonomous pipeline and want to compare notes on which of your retry decisions have negative tests, we’d like to talk. We should talk about yours. The pipeline is at the self-hosted pipeline.


Read the receipts  ·  Talk to us