Repository navigation
fix(chain-runner): stop a rejected dispatch from poisoning the reliability signal - #1017
Merged
aaronjmars merged 2 commits intoSep 3, 2026
Conversation
…ility signal dev-loop's target guards (and the generalized $chain_target guard any future chain can reuse) correctly reject a dispatch missing target=external:owner/repo rather than silently falling back to the watched-repos sweep. that part was never broken. the bug is downstream: "Update cron state" runs `if: always()` and defaults to STATUS="failed" for any early exit, so a caller-error rejection that never ran a single skill gets written into memory/cron-state.json identically to a genuine mid-run failure — poisoning the chain's lifetime success-rate signal that skill-health reads, and misreporting the loop as unreliable when it correctly refused bad input. found while dogfooding the same pattern on a fork (#54, where 6 manual dispatches missing target= over two days produced 6 false "chain:dev-loop failed" P0 flags with zero actual skill runs behind them); confirmed this exact structure — both pre-flight guards, same if: always() default — is present on main too, not fork-specific. give the rejection a distinct CHAIN_STATUS=invalid-dispatch and skip the cron-state write for it, at both guard sites. the job itself still fails loud (red x, ::error:: unchanged) for whoever ran the malformed dispatch; only the fleet-wide reliability bookkeeping stops being polluted. a real failure or success still writes exactly as before — verified by the added regression test, which extracts the guard from the workflow (same pattern scripts/tests/test_chain_runner.sh already uses for dispatch_skill) and asserts all three paths. Claude-Session: https://claude.ai/code/session_01SbipDt7VmuDvosRPS8AfEx
The test file was committed but never invoked by ci-tests.yml, so the regression guard it provides was dead. Wire it into the test job next to the sibling chain-runner test. Co-Authored-By: Svector-anu <svector.anu@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
what looked broken
after wiring a cross-model dev-loop chain on my fork,
chain:dev-loopgot flagged repeatedly by skill-health — six manual dispatches over two days, all recorded as failures, dragging the chain's lifetime success rate down and firing false P0s.what's actually happening
not a defect in the loop. every one of those failures happened at the exact same point, before any skill ran:
both target guards (the top-level
dev-loopcheck and the generalized$chain_targetcheck any future chain can reuse) are working exactly as designed — rejecting a dispatch with notargetinput rather than silently falling back tofeature's broadwatchedselector.targetcan't be made conditionally-required in the nativeworkflow_dispatchschema, so it's easy to dispatch this chain manually (CLI or the Actions UI) and forget it. that part of the design is correct and untouched by this PR.the actual bug
Update cron staterunsif: always()and defaultsSTATUS="${CHAIN_STATUS:-failed}"for any early exit — so the correctly-rejected malformed dispatch gets written intomemory/cron-state.jsonidentical in shape to a genuine mid-run failure. that's the signal skill-health reads for lifetime reliability, so non-runs were making a working chain look broken.confirmed both
snapshotandfeature dispatch has no correlation IDfailure paths are unaffected and out of scope — they already correctly setCHAIN_FAILED=true; break, funneling through the proper final-status logic. only the two pre-flight guards bypass it entirely.fix
mark the pre-flight rejection distinctly (
CHAIN_STATUS=invalid-dispatch) at both guard sites, and skip the cron-state write for that case only. the job still fails loud — red x,::error::unchanged — for whoever ran the malformed dispatch; only the fleet-wide bookkeeping stops being polluted. a real failure or success still writes exactly as before.tests
scripts/tests/test_chain_runner.sh— unchanged, still passesscripts/tests/test_chain_runner_invalid_dispatch.sh(new) — extracts the guard verbatim from the workflow (same anchored-sed pattern the sibling test already uses fordispatch_skill()) and asserts:invalid-dispatchexits 0 without reaching the state write and logs why; a real failure (CHAIN_STATUSunset) and a real success both still reach the write unchanged