fix(pipeline-conductor): give NOPROGRESS the spec row it shipped without - #8808
Conversation
The probe fires NOPROGRESS and the action table it is read against had no row for it, so the conductor received a line with no defined action -- while the document still told it to diff each fired line's i= by hand, the comparison the tag replaced.
GPT 5.6 Review (fork) — ✅ no blocking findingsReviewed Review detailsNo findings. |
Design Review (Fable 5, fork) — ✅ PASSDesign-level review of Design-Verdict: PASS A real tag-keyed lookup gap, closed at the root: the generic gate reads Verified against base: [DESIGN-REVIEWED] d2ba2b1 |
Opus 4.8 Review (fork) — ✅ no blocking findingsReviewed |
First Principles Review (Fable 5, fork) — ✅ PASSPremise-level review of All claims verified against the base tree. Producing the review now. First-Principles-Verdict: PASS A shipped probe tag finally gets the table row the conductor keys on, the replaced manual procedure is deleted, and a gate reads What this change shipsIntent: make the conductor's action table answer the
Every item is the declared fix or its gate; nothing rides along, no new public surface, no config keys, and the row's two-path age claim was checked against the source rather than taken from the description. [FIRST-PRINCIPLES-REVIEWED] d2ba2b1 |
The suppression fallback substitutes NOPROGRESS for an already-dispositioned report with no age test, so the tag is not warm-only and a cold line was being sent past the nudge it does need.
iamwhatever
left a comment
There was a problem hiding this comment.
Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: the pipeline-conductor probe fires a NOPROGRESS tag that SKILL.md never documented, so the conductor had no action row for it — this adds the row (check the EFFECT, never liveness; route on the line's own age) and replaces the by-eye two-cycle index diff the probe now performs itself, with a contract test pinning the row. CodeQL is not applicable on this fork PR (default-setup emits no check-run); SAST coverage is Semgrep only, latest run success with 0 annotations.
Problem / Motivation
Item 7 of #8192.
NOPROGRESSlanded infleet_probe.pyin #8035 and sits in the probe's_FIRINGset, so the probe prints it — but the action table inpipeline-conductor/SKILL.mdhas no row for it. The conductor receives a fired line and looks it up against a table that answers every other tag, which is the issue's own thesis one level up: a lookup that returns nothing is indistinguishable from a lookup that says "do nothing".Worse than a missing row on its own, the document still carried the procedure the tag replaced. The patrol-loop message told the conductor to "diff each fired line's
i=against the recorded one", and thei=paragraph presented that two-cycle diff as the reader's job. The probe now holds both numbers and makes the comparison itself, so the document offered a second, weaker answer to a question already answered — and the weaker one is enforced by nothing, so it may simply never happen.The general rule the issue states: whoever authorizes a new signal owns its spec row, because the contributor who implements the signal may not own the spec file.
What changed
src/kiro_crew/builtin_skills/pipeline-conductor/SKILL.md, three places:NOPROGRESSrow in the probe action table. It states the firing condition (nothing produced since the last mark, and that mark at least oneidle_alert_secsold), routes to an EFFECT check, never liveness — artifact, remote head, new commit — and then routes the absent-effect case on the line's own age, because two paths reach this tag and they do not mean the same thing. Withinidle_alert_secsthe transcript is warm, held alive by inbound traffic the session never answers, so the first move is not a nudge (a nudge is more of the input that produced the reading) and the ladder is entered at its Inspect step. Pastidle_alert_secsthe session is cold as well as unproductive andIDLE's ladder applies from the top: the classifier ranks the tag below the clock, but the suppression fallback substitutes it for an already-dispositioned report with no age test, so a cold line can carry it.NOPROGRESS → check the EFFECT, never livenessin its tag enumeration, in place of the manuali=diff instruction.i=paragraph attributes the comparison to the probe and says never to diff two cycles by eye;i=is described as the corroborating number rather than the test.No probe behaviour changed. This is the spec side only.
Why it matters
A tag with no row is not a cosmetic documentation gap here, because the conductor's dispositions are tag-keyed lookups against exactly this table. The two readings it could fall back to are both wrong in a specific way: treat it as
IDLEand it nudges — which, on the warm path, is the input that produced the state — or treat it as unrecognized and the one signal that survives a self-deadlocked worker is dropped, since a worker that cannot produce a message also cannot produce any other tag.Leaving the replaced manual procedure in place had its own cost. A conductor following it re-derives what the probe already computed, and the derivation lives in prose that nothing verifies, so the check that was moved into code for enforceability was still documented as optional human diligence.
Tests
New in
test/test_pipeline_conductor_skill_contract.py:test_every_firing_tag_has_a_row_in_the_action_table— loads the probe script and reads_FIRINGfrom it rather than restating the tag list, so it fails for any future tag that ships without a row. That is the issue's general rule turned into a gate.test_the_noprogress_row_routes_to_effect_and_not_to_a_nudge— the row must exist, must route to the effect check, and must route the absent-effect case on the printed age rather than describing the tag as warm-only.test_the_probe_makes_the_no_progress_comparison_itself— the manual two-cycle diff must not be re-documented beside the tag.test_the_loop_message_names_noprogress_instead_of_a_manual_diff— the armed patrol message is the list that actually runs; a tag missing there is an instruction never received.Written before the source change and confirmed red for the right reason — the generic one reported
probe tags with no row in the action table: ['NOPROGRESS'], exactly one tag.Mutation-proved: with
SKILL.mdreverted toorigin/mainand the tests kept,4 failed, 82 passed; with it restored,86 passed.The 8 are
OSError: [WinError 1314] A required privilege is not held by the clientfromTestFleetProbe's symlink fixtures — unprivileged Windows cannot create a symlink. They are environmental and pre-existing; nothing in this change touches the probe or its fixtures.Gates, from the worktree root:
isort --check-onlyexit 0,flake8exit 0,mypy src/kiro_crew/→Success: no issues found in 1296 source files,scripts/check_black_formatting.py→black gate passedover 2 changed files in scope.The second commit is a review-driven correction, taken test-first like the first: the GPT lane pointed out that the original row's "only ... held WARM" claim skipped
IDLE's nudge for a cold session. Checked against the source and it is right —_stalled_since_dispositionis age-gated on the classifier path only, and the suppression fallback has no age test — so the row now routes on the printed age, with the new assertion added and confirmed red before the prose changed.CI on d2ba2b1: all 66 checks pass, including Docs Lint, Fork PR Description, Backend Tests on Linux and Windows, E2E and the four fork-lane AI reviews (GPT no findings, Opus no findings, Design PASS, First Principles PASS).
Pattern harvest
Rule candidate: when a change adds a signal an agent branches on — a probe tag, a verdict word, an exit code — the same change adds its row to the table the agent looks it up in, and a test reads the emitting side's own constant so the next one cannot ship without it. The value is in reading the constant rather than restating the list: a hand-maintained list in the test is one more place the new signal has to be registered, and forgetting to register it is the exact failure being guarded. #8035 added the tag, its tests covered the tag's behaviour thoroughly, and every one of them passed while the conductor had nowhere to look the tag up.
Deliberately out of scope: items 1-6 of #8192. Item 1 is #8736. Item 5's separate complaint — that an expiring
IDLE/NOPROGRESSmark re-fires on an unchanged digest — is untouched and stays open, soRefsrather thanCloses.Refs #8192