docs+ci(pipeline): ObservationSnapshot + transitions table as CI-checked spec (INV-75, #236)#260
Open
kane-coding-agent[bot] wants to merge 5 commits into
Open
docs+ci(pipeline): ObservationSnapshot + transitions table as CI-checked spec (INV-75, #236)#260kane-coding-agent[bot] wants to merge 5 commits into
kane-coding-agent[bot] wants to merge 5 commits into
Conversation
13 tasks
![kane-review-agent[bot]](https://avatars.githubusercontent.com/in/3020954?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review reached a blocking FAIL verdict — see the Review findings: comment on issue #236 for the full list of blocking findings and remediation steps. This PR is sent back to development; reviewDecision is set to CHANGES_REQUESTED until the findings are addressed and a new review passes (INV-52).
There was a problem hiding this comment.
Review reached a blocking FAIL verdict — see the Review findings: comment on issue #236 for the full list of blocking findings and remediation steps. This PR is sent back to development; reviewDecision is set to CHANGES_REQUESTED until the findings are addressed and a new review passes (INV-52).
There was a problem hiding this comment.
Review reached a blocking FAIL verdict — see the Review findings: comment on issue #236 for the full list of blocking findings and remediation steps. This PR is sent back to development; reviewDecision is set to CHANGES_REQUESTED until the findings are addressed and a new review passes (INV-52).
…ked spec (INV-75, #236) Encode the issue-label state machine as data and gate it in CI — zero dispatch behavior change (no runtime wrapper/lib touched). - docs/pipeline/transitions.json (schema_version'd): every legal (from, event, guards[], actions[], to) tuple for the dev + review lifecycles, incl. human-edit events (no-auto-close, manual stalled flip) + the invalid-combinations rule. - docs/pipeline/observation-snapshot.md + schema: typed enumeration of every dispatcher-consulted input with file:line citations, incl. the SSM-indeterminate third liveness state (INV-30). - scripts/gen-state-machine.sh: transitions.json -> mermaid in state-machine.md (marker-delimited, idempotent, --check mode). The diagram is now generated. - scripts/check-spec-drift.sh: (A) regenerate+diff; (B) every guard/action maps via spec-guard-map.json to a resolvable function/predicate anchor; (C) every label literal written by the 4 pipeline files is declared in transitions.json. - CI job 'spec-drift' (no creds, bare ubuntu) + new scripts in shellcheck list. - invariants.md: one-line triage tag on every INV (machine-checked / design-rationale / superseded) + new INV-75; Triage tags legend. - tests/unit/test-spec-drift.sh (TC-SPEC-GATE-NNN): schema golden+negatives, generator idempotence, bidirectional drift injection, guard-map removal/stale, label-write completeness, triage coverage. Review findings addressed: C1 (is_declared SIGPIPE-under-pipefail false positive -> here-string), M1 (variable label writes surfaced as a NOTE), M2 (continuation- line writes joined before scan), L1 (reversed markers rejected loud). Closes #236
…H before nested match() (INV-75, #236) scan()'s Form-2 `while (match(rest, --label-regex))` loop called emit_labels() inside the body; emit_labels() runs its OWN match() which clobbers the global RSTART/RLENGTH, so scan()'s `rest = substr(rest, RSTART + RLENGTH)` advanced by the WRONG (clobbered) offsets and never consumed the match — an infinite loop on the first --add-label literal line (autonomous-dev.sh:563), hanging check-spec- drift.sh (and the spec-drift + unit-tests CI jobs) indefinitely. Fix: capture RSTART/RLENGTH into locals (st/ln) immediately after each match(), in BOTH scan() and emit_labels(), and advance with the saved values. emit_labels is a leaf (no nested match) so it's now self-consistent; scan no longer depends on globals a callee mutates. Verified: the scanner finishes instantly and emits the correct finite set of (label, file:line) write sites. Also defines the missing note() helper that warn_variable_writes() relied on (it was calling an undefined command → the M1 audit pointer was silently lost).
…ry; correct stalled-rearm transition (INV-75, #236) Two [P1] review findings on PR #260: 1. check-spec-drift.sh Check C only validated label VOCABULARY (does the label string exist anywhere in transitions.json), so a new write reusing existing labels in an undeclared combination (e.g. label_swap "$n" "approved" "stalled") passed CI — leaving the acceptance criterion "a PR adding a new label write without a transitions.json entry fails CI" unmet. Add sub-check C.2 "movement": each write SITE performs a label MOVEMENT (its sorted removes-set | sorted adds-set); that movement must equal some transition's (remove-label:, add-label:) actions. New collect_movements() awk (positional label_swap operand parse + gh-issue-edit multi-flag form, comment-skip, continuation-join, set-sort) + declared_movements() jq. Regression tests TC-SPEC-GATE-035 (approved->stalled reusing known labels -> red) and TC-SPEC-GATE-036 (full movement coverage on the real repo). Also add comment-line skip to collect_writes for symmetry with C.2. 2. transitions.json mis-modeled stalled-rearm as stalled -> pending_dev. The runtime (lib-dispatch.sh::mark_stalled) only removes pending-dev + adds stalled and never removes autonomous (the sole autonomous-removal site is review-pass-merged). So a maintainer removing only `stalled` leaves the issue autonomous-only -> it re-enters via Step 2 list_new_issues / dispatch-new (autonomous -> in_progress), NOT pending_dev. Correct the transition to stalled -> autonomous, regenerate the mermaid, update the prose table row + note. Docs updated in-PR (Pipeline Documentation Authority): state-machine.md (generated mermaid + table), invariants.md INV-75 (C.1/C.2 + Test line), design doc (A/B/C check naming + movement description), test-case doc (TC-033..036). Zero dispatch behavior change; ShellCheck -S error clean; full TC-SPEC-GATE suite 36/36.
…ant is INV-77 (#236) origin/main merged INV-75 (CLI adapters, #232) and INV-76 (smoke no-response, #257) while this PR was open, both claiming numbers this PR's executable-spec invariant also used. First-merged keeps the number: - Renumber this PR's spec-gate invariant INV-75 → INV-77 (heading + the .github/workflows/ci.yml comment; the spec-gate scripts/docs/tests are INV-agnostic — they key on the `^## INV-NN:` pattern, not a literal number, so no other ref changes). - Add the issue-#236 triage tag to the two newly-merged invariants (INV-75 adapters → test-cli-adapters.sh; INV-76 smoke → test-lib-agent-smoke.sh) so the triage pass stays complete (TC-SPEC-GATE-040 headings==tags). - The conflict resolution kept BOTH sides on the 7 INVs origin annotated with adapter blockquotes (INV-34/38/50/58/59/61/62): origin's `> Implemented in adapters/<cli>.sh` callout AND this PR's `_Triage` tag, with the triage tag moved directly under each heading to satisfy the 2-line adjacency check. No invariant content rewritten; TC-SPEC-GATE suite 36/36; gen --check + shellcheck green.
kane-coding-agent
Bot
force-pushed
the
docs/spec-gate-236
branch
from
a3b6c91 to
a32365d
Compare
There was a problem hiding this comment.
Review reached a blocking FAIL verdict — see the Review findings: comment on issue #236 for the full list of blocking findings and remediation steps. This PR is sent back to development; reviewDecision is set to CHANGES_REQUESTED until the findings are addressed and a new review passes (INV-52).
…he shared-movement deletion gap (INV-77, #236) Third codex [P1] on PR #260: Check C reduced label-write-site coverage to unique (removes|adds) MOVEMENT signatures, so a concrete write site was NOT pinned to a specific transition row. Reproduced by deleting the `dispatch-pending-dev-pr-exists` row: the checker still passed because the same `pending-dev -> pending-review` movement is declared by `dispatch-review-aware-reroute-review`. Movement-SET membership (C.2) is necessary but not sufficient — two transitions sharing one movement make a row's deletion invisible. Add Check C.3 (bidirectional code-site coverage), modeled on Check B's grep-anchor map: - New docs/pipeline/spec-codesite-map.json: every CODE-BEARING transition (actor not in {maintainer, github} — human/GitHub events have no pipeline write site) -> {file, anchor}, where anchor is a grep-stable literal (function name or distinguishing string) at that transition's write site. 31 entries, 1:1 with the 31 code-bearing transitions. - C.3 reverse: every map key must be a live transition id -> a DELETED row leaves an orphaned entry -> CI red naming it (THE repro). - C.3 forward: every code-bearing transition has a map entry whose anchor still greps in its file -> catches a new untracked transition + a renamed/removed write site (code drift). - new --codesite-map flag + file-existence guard; EXIT trap extended to the 2 new temps. Now C.1 (vocabulary) + C.2 (movement) + C.3 (code-site) together make "a PR adding OR removing a label-write site without the matching transitions.json entry fails CI" actually hold. Regression tests TC-SPEC-GATE-037 (delete shared-movement row -> red via orphaned entry), 038 (stale anchor -> red), 039 (full code-site coverage on the real repo). Docs updated in-PR (Pipeline Documentation Authority): invariants.md INV-77, design doc (Check C + artifacts table), test-case doc. Zero dispatch behavior change; ShellCheck -S error clean; gen --check green; full TC-SPEC-GATE suite 39/39.
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
1 participant
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.
Summary
Make the issue-label state machine an executable, CI-checked spec (the CI-checker half of the executable-spec pillar). Zero dispatch behavior change — no runtime dispatcher/wrapper/lib
.shfile is modified; this PR documents and gates the existing machine.docs/pipeline/transitions.json(schema_version'd) encodes every legal(from, event, guards[], actions[], to)tuple for the dev + review lifecycles, including the human-edit events (no-auto-closeadded, manualstalledflip) and the invalid-combinations rule.docs/pipeline/observation-snapshot.md+ schema: the typed enumeration of every dispatcher-consulted input, each citing thefile:linethat reads it today, including the SSM-indeterminate third liveness state (INV-30).scripts/gen-state-machine.shgenerates the mermaid diagram instate-machine.mdfromtransitions.json(marker-delimited, idempotent,--checkmode). The diagram can no longer drift.scripts/check-spec-drift.sh+ the newspec-driftCI job enforce three checks (see below).invariants.mdcarries a one-line triage tag; new INV-77 records this gate.The three drift checks (CI job
spec-drift, no credentials, bare ubuntu)gen-state-machine.sh --checkregenerates the mermaid region and fails on any diff. Editing the table without regenerating (or hand-editing inside the markers) → CI red.guard/actiontoken intransitions.jsonmaps viadocs/pipeline/spec-guard-map.jsonto a named function or greppable predicate that must still resolve inlib-dispatch.sh/ the wrappers. Unmapped token or stale anchor → CI red naming the pair. (Keys on function names + grep-stable literals, never line numbers.)label_swaparg + every--add/--remove-labelliteral in the 4 pipeline files (continuation-line joined, comment-skipped). C.1 vocabulary: every written label literal is declared intransitions.json. C.2 movement: every write site's(removes-set → adds-set)movement (normalized<sorted-removes>|<sorted-adds>) must equal some transition'sactions[]movement — so a new write that reuses existing labels in an undeclared combination (e.g.label_swap "$n" "approved" "stalled") fails CI even though both labels exist. C.3 code-site coverage:docs/pipeline/spec-codesite-map.jsonpins every code-bearing transition (actor ∉ {maintainer, github}) to a grep-stable code anchor, checked both ways — forward (every such transition has an entry whose anchor still greps) and reverse (every map entry's key is a live transition id). C.2 alone is movement-set membership, so two transitions sharing a movement hide a row's deletion; C.3 closes that — deletingdispatch-pending-dev-pr-exists(itspending-dev→pending-reviewmovement is shared) keeps C.2 green but orphans its map entry → CI red. C.1+C.2+C.3 make the AC "a PR adding or removing a label-write site without the matching transitions.json entry fails CI" hold. A new/orphaned/unmapped/stale entry → CI red naming it; variable-valued writes surface as a NOTE. Regression:TC-SPEC-GATE-035(undeclared movement),036(movement coverage),037(deleted shared-movement row → orphaned entry),038(stale anchor),039(code-site coverage).Deliberate-drift simulation (documented per issue E2E requirement)
tests/unit/test-spec-drift.shinjects each drift and asserts CI goes red with an actionable message:gen --checkred.label_swap/--add-labelwriting an undeclared label → checker red naming it (C.1).label_swap "$n" "approved" "stalled"reusing existing labels in an undeclared combination → checker red naming theapproved → stalledmovement (C.2). TC-SPEC-GATE-036: full movement coverage on the real repo.dispatch-pending-dev-pr-existstransition row (its movement is shared withdispatch-review-aware-reroute-review), regenerate, rerun → checker red naming the orphanedspec-codesite-map.jsonentry (C.3). TC-SPEC-GATE-038: stale code-site anchor → red. TC-SPEC-GATE-039: code-site coverage on the real repo.Design / Test Plan
docs/designs/spec-gate.md)docs/test-cases/spec-gate.md,TC-SPEC-GATE-NNN)transitions.json+observation-snapshotschemas validate (golden accept + negatives reject), backend-detect (python jsonschema / jq fallback) — mirrors the docs(pipeline): adapter-spec v1 — normative agent-CLI adapter contract, verdict/error JSON Schemas, fixture manifest format #229/feat(tests): standalone conformance runner — fixture-manifest-driven adapter tests, hermetic (no live CLIs), promote existing fixtures #230 patterncheck-spec-drift.shpasses against the committed repo (all 3 checks)-S error) on the new scripts; added to the lint listReview findings addressed (pr-review agent)
is_declaredusedprintf | grep -Fxqwhich SIGPIPEs the printf underset -o pipefail→ ~6% random false-positive failures. Fixed to a here-string (no pipe).--remove-label "$t"is now visible, not a silent gap).note()helper the M1 fix relied on.Out of scope (per issue)
transitions.jsonunchanged).invariants.mdcontent (annotation only).Closes #236
CI confirmed green on HEAD
5eaf725: Spec Drift (7s, incl. C.3) · Unit Tests (2m30s, 39/39 TC-SPEC-GATE) · ShellCheck · Pipeline Docs — all SUCCESS on a clean GitHub-hosted runner.