Skip to content

S2.6.6 — The seam rung and its CI job - #335

Merged
dev-pmallapp merged 4 commits into
story/293-s2-6-skill-execution-harnessfrom
task/323-seam-rung-and-ci-job
Sep 12, 2026
Merged

dev-pmallapp merged 4 commits into
story/293-s2-6-skill-execution-harnessfrom
task/323-seam-rung-and-ci-job

Conversation

@dev-pmallapp

Copy link
Copy Markdown
Owner

Closes #323. Refs #293.

Build target 7 of docs/design/stories/293.md. Cases 16, 17, 18 — and case 9, which #322 left open as CLAIM-293.6 half (ii).

What ships

docs/audits/293-transcription-audit.md Case 18's model-judged verdict: 16 rows, one per ## Re-entry row, each naming the transcription step, the mutation it gates, and the reasoning
test/transcription-gap.test.ts Cases 16, 17 and 18's tool-checked denominator
packages/harness/src/injection-roster.ts + --inject Case 9 — five scenarios, one per FailureMode
Scenario.injectBeforeRun2 The per-scenario lie hook, armed between the runs
.github/workflows/ci.yml The seam job — not yet required, by decision
test/ci-workflow.test.ts The four hardcoded lists must agree; ci.yml's jobs must partition

The seam rung ships NOT required, deliberately

Required contexts stand at six and both verifiers PASS. Two independently sufficient measured reasons:

  1. A required context that has never reported sits pending forever (S1.6.4 — Build the claim-lint guard #204).
  2. story/47 and its six open task issues carry no seam job on their heads — promoting now would permanently block PR S2.3 — Execution skills #313 and every S2.3 task PR.

The promotion is #334, which carries all six sites as a checklist. It is deliberately not a sub-issue of #293, so this Story's exit does not depend on it.

The audit's finding, and why case 16 passes

The ordering obligation the skill bodies state is per-row — "every mutating step is preceded by the read that decides it" — never table-wide. Three of the four transcriptions execute rows out of table order, each for a reason recorded row by row, and all three are faithful.

That is exactly why case 16 passes when the harness fails to notice a reordered table. A harness that went red on a reorder would assert an ordering the bodies never claimed — and would go red on three of the four skills as shipped. The reason is written into the assertion's own comment. Do not "repair" it.

Case 9 — proof the harness can fail

run 1  gh api --method POST …/milestones -f title=M9 — universal lifecycle …
run 2  gh api --method POST …/milestones -f title=M9 — universal lifecycle …
harness: FAIL run-2-mutated: scenario "goal-create/injected-truncated-list"
harness: verdict: fail (exit 1)

Asserted on the recorded argv, never the exit code. --inject exiting non-zero is the success condition of the mode — the wrapper still assigns exitCode = verdict.exitCode unconditionally, because inverting it would move the branch-on-the-verdict one file over from decideVerdict, which case 24 statically forbids.

Mutation testing — 12 injected, 1 survived, closed and re-killed

M5: the audit parser took "the first contiguous run of | lines" and read the right rows by accident — it would have silently read the wrong table had one been inserted above. It now asserts ## The audit holds exactly one table.

A control no case asked for: four green runs against mutated skill copies prove nothing unless --skills-dir is load-bearing, and a pure reorder is invisible in the artifact. A fifth uncovered skill must turn the harness red; it does. Without it, case 16 was one ignored flag away from vacuous.

Corrections

  • ci.yml's claim-lint comment was false in both halves — it said claim-lint was in neither branch protection nor ruleset 21656025; it is in both. Corrected, and promotion commit 15e5a36 is now cited. Nothing verifies that a :NN citation still says what the citing document claims #304's class, found in the very comment the new one is modelled on.
  • Every subprocess-spawning test now declares a 120 s timeout. They inherited bun's 5 s default and one had already flaked — and test is a required job.

Chain, on 44ee4fd

1095 tests / 1081 pass / 0 fail / 14 skip / 3967 expect() · lint 0 across 123 files · typecheck clean · skill-lint 4 + 12 · claim-lint 121 files, 0 errors, 1 warning · skill-harness exit 0, --inject exit 1 · both verifiers PASS at six.

Evidence: docs/evidence/323-20260912T102956Z.md.

CI will now report EIGHT checks, not seven — seam runs on every PR from this commit forward, though it is not required.

Cases 16, 17 and 18 of docs/test-plans/293-plan.md. Build target 7's CI job
is deliberately NOT here -- it is a job-list edit whose promotion sequencing
is unresolved, and it lands separately.

docs/audits/293-transcription-audit.md is case 18's model-judged verdict:
sixteen rows, one per `## Re-entry` row across the four skills, each naming
the transcription step that implements it, the mutation that step gates, and
the reviewer's reasoning. It is not an evidence artifact because every file
under docs/evidence/ must match `{issue}-{ts}.md` (evidence-upsert.test.ts)
and because an artifact is immutable once pushed, which is the wrong lifetime
for a record that must be amended whenever a table or a transcription changes.

The audit's finding: the ordering obligation the skill bodies state is
per-row -- every mutating step preceded by the read that decides it -- never
table-wide. Three of the four transcriptions therefore execute their rows out
of table order, each for a reason recorded row by row, and all three are
faithful. That is the same fact that makes case 16 pass when the harness fails
to notice a reordered table, and the reason is written into the assertion's
own comment so it is not "repaired".

Two authorised mutations are dead in the roster -- goal-create's milestone
PATCH and story-create's `--milestone` reassign. Both are the fake forge's
unmodelled shapes, not transcription defects. The audit records them; only
the live rung can close them.

test/transcription-gap.test.ts carries the tool-checked halves: the audited
(skill, row) pair set must equal exactly the set reEntryRows() reads from
disk, by cardinality and not by length; CLAIM-41.8's class in
docs/test-plans/41-plan.md must still read `model-judged`, gate ruling G2 made
mechanical; and a row reorder in each of 4 of 4 bodies must leave the harness
green, with the mutation asserting its own pattern matched first.

Seven mutations were injected against these assertions. Six were killed on
first contact. M5 survived: folding the summary table back under `## The
audit` left the parser reading the right rows by accident, because it took
the first contiguous run rather than asserting the structure it depended on.
The parser now asserts `## The audit` holds exactly one table, and M5 is
killed. A control test proves `--skills-dir` is load-bearing, without which
all four of case 16's green runs would have been vacuous.
Case 9 of docs/test-plans/293-plan.md, closing CLAIM-293.6 half (ii), plus
build target 7's job. The job is NOT promoted to a required context here; the
promotion is sequenced separately and its procedure is written into ci.yml.

--inject selects INJECTION_ROSTER, five scenarios, one per FailureMode. The
truncated-list scenario makes goal-create's run 2 issue a duplicate create:
run 1 POSTs the milestone "M9 — universal lifecycle", the lie drops the last
element of the one-element list, run 2 reads absence and POSTs the identical
title again. The fake has no duplicate check on any identity key, so it lands
and the harness reports run-2-mutated. The assertion is on the recorded argv,
never the exit code -- #320's M9/M9b pair proved a double that refuses and a
double that accepts return the same verdict and differ only in state.

The lie is armed BETWEEN the runs, never before run 1. Every one of the five
is a lie about what run 2 sees; arming it for run 1 leaves nothing genuine for
run 2's create to duplicate, and the harness would prove nothing.

`bun run skill-harness --inject` EXITS 1, and that non-zero exit is the
success condition of the mode. The wrapper still assigns exitCode from
verdict.exitCode unconditionally: inverting it under --inject would move the
branch-on-the-verdict one file over from decideVerdict, which case 24
statically forbids, and a branch does not stop being a branch by relocating.

Two findings, reported and not forced. Three of the five modes collapse to one
signature -- the read-lie trio all crash run 2 on JSON.parse of empty stdout,
because no transcription checks a read's exit code first. That is #320's
finding reproduced one layer up, and it belongs to #331. silent-no-op-mutation
is armed but never engaged: all four transcriptions are correctly idempotent
under honest reads, so run 2 attempts no mutation for it to intercept. It is
verified at createFakeForge's own seam instead.

The seam job runs `bun run skill-harness`, carries no `if:`, no matrix and no
paths filter, and is deliberately not yet required. A never-reported context
sits pending forever (#204), and story/47 and its six task issues carry no
seam job on their heads -- promoting now would permanently block PR #313 and
every S2.3 task PR. ci.yml names all four in-tree promotion sites, flagging
the `required = [...]` list inside the PYEOF heredoc as the one that gets
missed, plus the two out-of-tree settings a human must change afterwards.

test/ci-workflow.test.ts makes that gap mechanical: the four hardcoded lists
must agree as ordered arrays, and ci.yml's job set must PARTITION into
required-and-listed union NOT_REQUIRED, each entry carrying its reason. A
carve-out cannot detect an unclassified new member; a partition must. Five
mutations were injected against it and all five were killed.

The claim-lint comment at ci.yml:114-123 was stale in both halves: it said
claim-lint was not in branch protection or ruleset 21656025, and it is in
both. Corrected, and the promotion commit 15e5a36 is now cited so the next
reader can check it rather than trust it. #304's class, found in the very
comment the new one is modelled on.

Every subprocess-spawning test now declares an explicit 120s timeout. They
inherited bun's 5s default and one had already flaked under parallel load,
which breaks the required `test` job.
The promotion procedure in ci.yml listed its six sites but named no owner,
so the next reader had no way to tell whether it was scheduled or forgotten.
#334 carries the checklist and both unblocking conditions, and is explicitly
not a sub-issue of #293 so the Story's exit does not depend on it.
Measured on 44ee4fd, which frontmatter `commit:` pins as the parent -- an
evidence artifact cannot carry the hash of the commit that contains it.

Chain: 1095 tests / 1081 pass / 0 fail / 14 skip / 3967 expect(); lint 0
across 123 files; claim-lint 121 files, 0 errors, 1 warning; skill-harness
exit 0 and --inject exit 1; both verifiers PASS at SIX contexts.

Adding this artifact takes the suite to 3969 expect() -- the +2 evidence-corpus
delta of #299, reproduced exactly.
@dev-pmallapp
dev-pmallapp marked this pull request as ready for review September 12, 2026 10:35
@dev-pmallapp
dev-pmallapp merged commit 49d3ec5 into story/293-s2-6-skill-execution-harness Sep 12, 2026
8 checks passed
@dev-pmallapp
dev-pmallapp deleted the task/323-seam-rung-and-ci-job branch September 12, 2026 10:38
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.

2 participants