Skip to content

Five claims in the skill's Core Behavioral Spine that misstate the contract they condense #1208

Description

@ptr727

The ## The Core Behavioral Spine section of .agents/skills/workflow-ci-contract/SKILL.md is the condensed statement of the contract an agent reads before touching a workflow. Five of its claims either cannot be implemented as written or drop the qualifier that carries the rule. Found by a whole-unit canonical pass on PR #1203's fix branch, and deferred out of it, which corrected only the one sentence a reviewer had raised.

The aggregator described here is a dead gate

one required aggregator gates the merge, treating skipped smoke as pass and blocking on failure or cancelled

The behavior cannot be implemented as stated, because if: always() is the mechanism that makes it possible and nothing in this SKILL.md mentions it anywhere. D1.5 requires it explicitly, "run under if: always() so a failed or skipped dependency cannot skip the gate itself".

Without it the aggregator inherits success() and skips whenever smoke skips, so it does not treat skipped smoke as pass, it stops being a gate at all, and GitHub reads a skipped required check as satisfied. That is precisely the failure D1.5's Prevents clause names, a paths-filter error letting a target-changing PR merge unbuilt. A reader following this unit literally builds a gate that looks right and is dead in its main case.

The expect_release_assets rule inverts for NuGet

A repo with no file target passes expect_release_assets: false at the caller.

"File target" is undefined in the unit, and the obvious reading is wrong for NuGet. D4.3 spends a sentence on exactly this case: "A NuGet target is not among them, since its leaf uploads a release-asset-* carrying the package, so a NuGet-only caller keeps the default true." The no-file-target shapes are Docker-only, PyPI-only and source-only.

A reader with only this unit reasons that a NuGet package goes to a registry rather than onto the release, sets false, and thereby relaxes fail_on_unmatched_files and skips the asset download on a repo that does produce an asset. The PyPI and NuGet asymmetry is not derivable from the unit.

A portable guarantee is stated as a literal

main a clean X.Y.Z, anything else a prerelease X.Y.Z-g<sha>

WORKFLOW.md states this portably and flags divergence on it: "Guarantees say 'default branch' portably. It is implemented as the literal main in several places (the validate gate, the prerelease expression, and version.json's publicReleaseRefSpec). These MUST all reference the repo's actual default branch. A divergence is a defect (section 5A)." D3.2 repeats it.

The unit's other uses of main, the bot push and the weekly Docker schedule, track D4.1, which is main-literal by contract. This one clause is the place the contract is deliberately portable, and the unit gives no cue that the literal is repository-specific. registry/repos.json carries no defaultBranch field, so nothing downstream re-derives it either.

The artifact rule drops its only exemption

Artifacts are an intra-run handoff: consume-then-delete at the point of consumption

D5.1 scopes the requirement to a cross-job transfer artifact and adds "An intermediate consumed only within the same run MAY instead rely on the retention-days: 1 backstop." The unit names that same property, "intra-run handoff", as the description of everything the rule covers, so the MAY-case disappears and the rule reads as unconditional.

In the same bullet, "gated to the half of the consumption whose failure would leave the artifact not yet redundant" is true but not actionable from the unit. Nothing in it lets a reader reach D5.2's if: ${{ !cancelled() && steps.<download-step-id>.outcome == 'success' }}, nor the reason it is required, that a delete carrying no status-check function inherits success() and skips on exactly the failed-push path it was written for.

The methodology reference is an index, not the procedure

Auditing, tracing, and probing a repo's workflows is references/test-methodology.md.

That file says otherwise three times. Its 5A section: "read this as an index into them rather than as the sweep itself" and "Apply only the ones the repo's types imply, and read them in WORKFLOW.md 5A rather than from this list". Its 5B section gives only "The load-bearing ones", S1, S5/S6, S7, S8, S9, S10 and S12/S13, and defers the expected end-state table to WORKFLOW.md 5B. Its Verdict section puts the per-type walkthroughs in WORKFLOW.md section 6.

An agent told the audit "is" this file runs a sweep missing the per-type addenda and the unlisted scenarios, and reports a verdict anyway.

Scope note

A sixth finding, that the adjacent sentence called references/d-guarantees.md "The full catalog, each guarantee with the failure mode it prevents", was corrected on the fix branch to "The condensed catalog is in references/d-guarantees.md", because a reviewer raised the stale framing and the file calls itself condensed. The underlying gap that made "full" wrong is unchanged and belongs to #1205: WORKFLOW.md section 4 carries 44 numbered items and that file carries 39, collapsing all five D9 items into one unlabeled line with no addressable ids, so D9.1 and D9.2, the two this skill's own frontmatter singles out as easy to break, cannot be cited from the file the skill routes to.

Changing this file means regenerating .github/skills/ and .claude-plugin/ with scripts/build_dist.py. Related to #1200, #1204, #1205, #1206, #1207.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions