Skip to content

fix(algorithm): close ISC criteria only on a structured worker verdict - #1645

Open
elhoim wants to merge 1 commit into
danielmiessler:mainfrom
elhoim:fix/isc-pass-detection
Open

fix(algorithm): close ISC criteria only on a structured worker verdict#1645
elhoim wants to merge 1 commit into
danielmiessler:mainfrom
elhoim:fix/isc-pass-detection

Conversation

@elhoim

@elhoim elhoim commented Jul 26, 2026

Copy link
Copy Markdown

The problem

algorithm.ts decided whether a parallel worker passed its criterion with a bare substring test:

if (stdout.includes(`RESULT: ${cId} PASS`) || stdout.includes(`${cId} PASS`)) {

The second clause subsumes the first, so the RESULT: anchor protected nothing. Both of these strings satisfied it while describing a failure:

  • RESULT: ISC-3 FAIL: I could not make ISC-3 PASS despite two attempts
  • Verified whether ISC-3 PASSES: it does not.

The worker's exit code was ignored for pass detection, used only for a display label. The criterion checkbox was then flipped to [x], and once postCriteria.passing >= postCriteria.total the ISA status was set to COMPLETE with no re-verification.

So a failing worker whose prose happened to mention its own criterion id near the word PASS got that criterion checked off, and enough of those drove the artifact to COMPLETE. For a system whose entire premise is that a claim closes only on evidence, that inverts the doctrine: it reports verification it never performed.

The fix

A structured verdict line, not prose matching. Workers now emit a sentinel-prefixed line:

ISC_RESULT v1 id=<criterion-id> verdict=PASS|FAIL reason=<one line, FAIL only>

The contract lives in one place (IscResult.ts) and is rendered into the worker prompt from the same constants the parser uses, so the two ends cannot drift.

Fail closed on anything less than a clean verdict. The judge returns PASS, FAIL, CRASHED, AMBIGUOUS or NO_SIGNAL, and only PASS closes a criterion. Contradictory output — a PASS and a FAIL line for the same id — is AMBIGUOUS, not a pass. A non-zero exit is CRASHED regardless of what stdout contains, so exit code is a necessary condition for passing.

Regression checks parsed with the same rigour, and a regression FAIL from any worker vetoes that criterion for the whole iteration, since one worker's fix can break what another claims to have passed.

Operator-visible distinctions. NO_SIGNAL and AMBIGUOUS are displayed as themselves rather than collapsed into a generic failure, because "the worker said it failed" and "the worker said nothing parseable" call for different responses. The dead fallback detection comment that had no code beneath it is gone.

Tests

test/tools/IscResult.test.ts, 19 cases:

19 pass
0 fail
27 expect() calls

Separately verified with an adversarial probe written to break the parser rather than confirm it. All 19 cases behave correctly:

  • Both original false-positive strings → not a pass
  • Sentinel quoted inside prose, or wrapped in markdown bold → NO_SIGNAL
  • Contradictory PASS + FAIL for one id → AMBIGUOUS
  • Valid PASS line with a non-zero exit → CRASHED
  • Verdict addressed to a different criterion → NO_SIGNAL
  • Id-prefix collision: id=ISC-30 verdict=PASS does not close ISC-3NO_SIGNAL
  • Malformed or missing verdict key → AMBIGUOUS
  • Empty and whitespace-only output → NO_SIGNAL
  • Genuine PASS closes, genuine FAIL reports as FAIL, both alone and embedded in surrounding output
  • Regression FAIL captured from the structured line while prose mentioning another id near "PASS" is ignored

Note for review

This changes the worker contract, so a worker still emitting the old RESULT: <id> PASS shape now yields NO_SIGNAL and its criterion stays open. That is the intended direction — fail closed rather than open — but it is a behaviour change worth being deliberate about, since an in-flight run started before the upgrade will not close criteria until its workers use the new line.

The parallel loop runner decided whether a worker passed its criterion with
a bare substring test on stdout:

    stdout.includes(`RESULT: ${cId} PASS`) || stdout.includes(`${cId} PASS`)

The second clause subsumes the first, so the `RESULT:` anchor protected
nothing. Both of these describe a failure and both satisfied it:

    RESULT: ISC-3 FAIL: I could not make ISC-3 PASS despite two attempts
    Verified whether ISC-3 PASSES: it does not.

The worker's exit code was ignored for pass detection, so a crashed or
killed worker could still have its criterion checked off. Enough falsely
checked criteria drive the ISA to COMPLETE with no re-verification, which
inverts the doctrine that a claim closes only on evidence.

Replace the substring test with a structured, line-anchored contract in a
new IscResult module that both the worker prompt and the parser read from,
so the two ends cannot drift:

    ISC_RESULT v1 id=<id> verdict=PASS
    ISC_RESULT v1 id=<id> verdict=FAIL reason=<one line>
    ISC_REGRESSION v1 id=<id> verdict=PASS|FAIL

A criterion passes only when the worker exited 0, a well-formed PASS line
names it, no FAIL or regression-FAIL line names it, and no sentinel line in
the output failed to parse. Absent, malformed, and contradictory signals all
fail closed. A regression FAIL from any worker vetoes that criterion for the
whole iteration. The prompt states the contract as `verdict=PASS|FAIL`, so
no valid PASS line exists in the prompt for a worker to echo.

Also drop the "fallback detection" comment that had no code beneath it, and
report per-agent verdicts from the parse rather than from the ISA checkbox
state, which could show PASS for a box checked in an earlier iteration.

Tests: bun test test/tools/IscResult.test.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant