Skip to content

ci(release): promote v0 only after PyPI publish, verify the published action - #81

Open
uipreliga wants to merge 4 commits into
mainfrom
fix/verify-published-action
Open

ci(release): promote v0 only after PyPI publish, verify the published action#81
uipreliga wants to merge 4 commits into
mainfrom
fix/verify-published-action

Conversation

@uipreliga

@uipreliga uipreliga commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Rewritten after a second review round (40345b6). The previous description
documented behavior that review changed — most importantly, preflight referenced a
step output that does not exist, which would have made it red on 100% of runs. This
body describes the branch as it now stands; the per-finding walk-through is in
the review-response comment.

Problem

Consumers pin uses: UiPath/coder_eval@v0. The composite action installs
coder-eval==<action.yml's version: default>, a pin the release commit bumps. In the OLD
release.yml the release job pushed main + the version tag, moved v0, then built the
wheel; PyPI publishing happens in a SEPARATE publish-pypi job (needs: release, behind
a pypi deployment environment for OIDC Trusted Publishing). So v0 moved to an
action.yml pinning version X before X existed on PyPI.

Two reachable paths:

  1. publish-pypi fails, or waits on the environment gate.
  2. The tag move sat before uv build, so a build failure stranded the pin with PyPI
    never involved.

Either way every @v0 consumer's uv tool install coder-eval==X 404s, and nothing
detected it. release.yml's own comment named this seam; the continue-on-error on the
Release step plus its "Flag missing GitHub Release" annotation were a workaround for it.

Separately, nothing verified the published composite: action-dogfood in
pr-checks.yml runs uses: ./ with version: local, which proves a PR's code works but
never touches v0 or PyPI.

Part 1 — prevention (release.yml)

The v0 tag move and the GitHub Release creation moved out of release into a new
promote job gated on needs: [release, publish-pypi]. Nothing a consumer can resolve
happens until the wheel is published. Marketplace listings are cut from a published
Release, so creating one also announces a version — hence both moved, not just the tag.

Supporting changes that make the recovery story actually hold:

  • Monotonic, not merely idempotent. promote refuses to promote anything but the
    newest vX.Y.Z tag. Force-push is only self-idempotent and says nothing about
    ordering: GitHub keeps "Re-run failed jobs" live for 30 days, so replaying an older
    release's promote would walk v0 backwards and silently downgrade every consumer.
  • skip-existing: true on publish-pypi. Without it, an upload that succeeds but
    whose step then fails (lost response, job timeout) gets 400 File already exists
    forever — so promote could never run for a version that is published, which is the
    stranded state from the other direction.
  • …and an artifact-identity assertion to pay for it. skip-existing makes twine treat
    that 400 as success without comparing content, so on its own a green publish stops
    proving the wheel this run built is the one PyPI serves — and nothing downstream
    re-established it (promote moves v0 on job success alone; preflight checks
    reachability, not identity). A new step compares urls[].digests.sha256 from PyPI's
    version JSON against sha256sum dist/*. Mismatch is fatal — it must stop promote.
    An unreadable index is a warning, because the JSON API can lag an upload by seconds
    and a transient must not redden a publish that succeeded.
  • publish-pypi carries no if:. It used to gate on
    if: needs.release.outputs.version != '', which was both dead (the release job already
    exit 1s on an empty version) and dangerous: on a partial "Re-run failed jobs" attempt a
    lost output resolved the job to SKIPPED-green, which — since promote needs it —
    also skipped the promotion, for a fully green run that published no wheel and never
    moved v0. The implicit success() on needs: release is the real gate; emptiness is
    asserted in-job, so a lost output is red.
  • Prereleases discriminated on github.ref, the same signal "Determine release mode"
    already uses, rather than on a needs output — same hazard as above. Emptiness is
    enforced inside promote, so a lost output is red.
  • Release creation normalizes an existing draft/prerelease
    (gh release edit --draft=false --prerelease=false --latest) instead of treating mere
    existence as done, which could report success while announcing nothing.
  • Least privilege. Both create-github-app-token mints declare
    permission-contents: write (omitting permission-* yields a token holding every
    permission of the installation — and this is the app with the main-branch ruleset
    bypass); promote declares permissions: contents: read, dropping the workflow-level
    packages: write that only the GHCR steps need.
  • The old continue-on-error + "Flag missing GitHub Release" scaffolding is removed.
    It existed only because a failure there would have skipped publish-pypi and stranded
    the tag; promote is strictly downstream, so a failure can no longer skip anything
    upstream, and the job is re-runnable — it can fail loudly instead.

Residuals, accepted and documented in-file:

  • vX.Y.Z and main are still pushed by the release job, so if publish-pypi fails
    they briefly reference an unpublished version. Narrower than the @v0 window by design
    (@v0 is the documented pin; @vX.Y.Z/@main are opt-in) and cleared by re-running
    publish-pypi. Closing it entirely means publishing to PyPI before pushing any git ref,
    which requires carrying the bumped commit between jobs as an artifact — not worth the
    new failure modes.
  • The GHCR agent image is deliberately not covered by the promote ordering: it is
    still pushed (and :latest still moved) inside the release job, best-effort. It is an
    internal convenience rather than a ref a stranger's pipeline resolves, and it must be
    built in the job holding the bumped pyproject. Stated in the promote header rather
    than left as an inconsistency.

Part 2 — detection (verify-published-action.yml)

For drift a release cannot cause: a PyPI yank, the pinned setup-uv SHA, runner-image
changes, the @anthropic-ai/claude-code npm package, model deprecation, or the listing
being renamed/delisted.

Tier 1 preflight — free, deterministic, gates tier 2. The hard gate is the
consumer contract: the version action.yml at the v0 tag pins must be installable
from PyPI, and that same pin is what the install/smoke step installs. Also asserts the pin
anchor is readable, that the major is still v0 (the uses: below can't be an
expression, so a 1.0.0 bump must fail loudly — and the error enumerates the doc surfaces
that hardcode the major, since CE026 checks the slug, not the major), and that the
Marketplace listing resolves.

Tag lag is classified, not failed:

v0 state Verdict
At newest tag, pin matches pass
At newest tag, pin ≠ that release hard fail — release.yml's pin bump didn't take
Lags, newest version is on PyPI hard failpromote didn't run; re-run it
Lags, newest version absent from PyPI warning — release incomplete, @v0 consumers healthy
Lags, PyPI gave no definitive answer warning — inconclusive

Every HTTP probe in this tier — both PyPI ones and the Marketplace one — uses the same
transient split: only a definitive 4xx is a verdict; 000 / 403 / 429 / 5xx mean we
learned nothing. The hard gate stays red on an unproven pin, but it no longer says
"stranded pin, re-run publish-pypi" for a version PyPI already has.

Tier 2 e2e — cents. Consumes the action as a stranger would: uses: UiPath/coder_eval@v0, default version:, no repo checkout, task YAML written inline.
Doubles as a live proof that the documented Node + @anthropic-ai/claude-code
prerequisite steps still work. Skipped for branch-dispatched prereleases, which cannot
change the published artifact.

The gate is artifacts at the literal paths the workflow passes in with:, not the
step's exit code and not steps.run.outputs.* (the action step is continue-on-error,
and composite-output propagation through a failed step is undocumented). It requires
run.json, a JUnit report with at least as many <testcase> elements as task_results
rows (parse-alone let an empty report through), and non-zero tokens — plus assertions that
distinguish our breakage from the model's:

  • Zero tokens branches on the error_category every run.json row already carries:
    agent_api_error / agent_rate_limit / agent_timeout / agent_crash warn as
    inconclusive; anything else (auth, billing, config, sandbox, or no category) is the hard
    "wiring is broken" error. A daily cron will eventually meet a transient, and sending the
    operator to audit credentials for an Anthropic outage is how a check earns being ignored.
  • ERROR / BUILD_FAILED with a non-upstream category hard-fail rather than being
    tolerated as a model flake — those are exactly the harness failures this gate exists for.
  • Output wiring must be exact when the step went green; when it went red, a missing
    output is ambiguous (runner behavior) and only warns, but a present-but-wrong one is
    not ambiguous and warns explicitly rather than falling through.
  • The step must be green when every task reported SUCCESS, catching a regression in
    the action's own exit logic while still tolerating a model flake.
  • The run dir uploads on always(), so the routinely-tolerated red step keeps the
    evidence that explains it.

Triggers on Release completion regardless of conclusion — a failed publish-pypi
makes the run's conclusion failure, so gating on success would skip the check exactly
when it matters. Plus a daily cron and workflow_dispatch.

Part 3 — guardrails, so this class cannot ship again

The first round of this PR shipped a reference to a step output that does not exist. It
was invisible to ruff, pyright, pytest and the CE runner, and actionlint models
steps.*.outputs as an open string map, so it passed clean there too. Two additions close
that:

  • CE035 — workflow output-key parity (tests/lint/workflow_outputs.py, wired as
    tests/test_custom_lint.py::TestCE035WorkflowOutputParity). Resolves every
    ${{ steps.<id>.outputs.<key> }} and ${{ needs.<job>.outputs.<key> }} in
    .github/workflows/** + action.yml to a writer that actually produces it. Sound
    boundaries: third-party uses: are skipped (their metadata is not on disk), and a body
    whose writers are not statically readable is skipped rather than guessed at. Its negative
    test is the exact shape of the shipped bug.
  • tests/test_verify_published_workflow.py (8 tests) binds the four couplings nothing
    asserted: the workflow_run: ["Release"] display-name link to release.yml's name:;
    Marketplace slug parity between the workflow's shell pipeline and the tested
    marketplace_slug() over a punctuation/whitespace table; all three
    # <-- kept in sync pin-anchor readers (both seds executed against the real
    action.yml); and the inline consumer task YAML loading through the real load_task.

The slug case was live, not hypothetical: the shipped tr ' ' '-' pipeline turned
Coder Eval (CI gate) into coder-eval-(ci-gate), which 404s, while marketplace_slug()
(and therefore the doc links CE026 pins) produced coder-eval-ci-gate. They agreed only
because action.yml's name: is the one input where both are the identity function.

Docs

CONTRIBUTING.md gains a § Releasing runbook — the three-job table, which jobs are
re-runnable, four named recovery flows, and a table of every annotation the nightly emits
with what it means and what to do. CLAUDE.md's tree line now names promote and the
verification workflow instead of describing the old single-job shape.

Verification

Every guard was exercised by reproducing the failure, not by reading:

  • 4 parity breakages in a throwaway clone: stale major tag, stranded pin (amend step
    failed), detached # <-- kept in sync anchor, v1 bump that would rot the hardcoded
    @v0.
  • Both lag-classification branches, incl. that the publish-pypi-failure case warns with
    the right diagnosis instead of hard-failing with the wrong one.
  • 10 e2e-gate fixture paths against the extracted Python: happy path, upstream
    rate-limit tolerated, zero-tokens-no-category fatal, auth error fatal, harness error
    fatal, model flake tolerated, agent crash tolerated, all-SUCCESS-with-red-step fatal,
    empty JUnit fatal, no rows fatal.
  • 5 digest-check fixture paths: digests match, foreign wheel pre-uploaded (fatal),
    filename absent (fatal), index unreachable (warning, publish stands), empty dist (fatal).
  • The monotonicity refusal, both directions.
  • CE035's negative test reproduces the shipped reference verbatim (a step echoing
    pin/newest read as outputs.version) and asserts the rule flags it, alongside the
    missing-step-id and undeclared-needs-output cases and the two skip boundaries.
  • 000000 from || echo 000 confirmed empirically; gh release edit flags confirmed present.
  • actionlint clean on both files; bash -n and ast.parse clean over every run: body
    and embedded Python block.
  • make check clean, custom lint 175 passed, full suite 3894 passed.

make verify fails at pyright on 3 unresolved imports in codex_agent.py for the
optional [codex] extra, which isn't installed locally. Pre-existing and unrelated —
this PR contains no src/ changes.

Cannot be verified pre-merge

workflow_run and schedule only activate once the file is on main, and there is
deliberately no pull_request trigger — so this PR's own checks do not exercise the new
workflow at all
. After merge, workflow_dispatch proves tier 1 immediately (free, no API
spend); CE035 means the failure that made that first dispatch mandatory can no longer be
the one that greets it.

Runner behaviors that remain assumptions, all now failing safe: whether needs.*.outputs
survive a partial re-run (both publish-pypi and promote fail loudly either way), and
whether composite outputs propagate through a failed step (the gate no longer depends on
it). One deliberate publish-pypi failure + re-run after merge would settle both.

Behavior changes worth a second opinion:

  • If the pypi environment has required reviewers, promote now waits behind that
    approval before v0 moves. Correct, but the tag previously moved before the gate.
  • permission-contents: write on the app-token mints first executes on a real release. A
    wrong scope key would 422 at mint time; contents is the correct GitHub App permission
    key, but it is worth watching on the next release.

Deliberately deferred

Booked in .claude/harness-candidates.md rather than silently dropped: extracting the two
oversized inline blocks into .github/scripts/ with fixture tests (CE040 — agreed in
principle, but it is a refactor of a workflow that cannot be exercised pre-merge, and
CE035 + the new tests close the specific classes); CE036 (ban the skipped-green job
gate) and CE037 (if: failure() in a job containing a continue-on-error step), both
shapes now hand-fixed twice; exercising the action's score gate in the failing direction
(needs a second paid agent run — belongs in action-dogfood, which already pays); and
extending CE026's REQUIRED_PREREQ_TOKENS anchor to the e2e job, now a third copy of the
prerequisite steps.

🤖 Generated with Claude Code

… action

Consumers pin `UiPath/coder_eval@v0`, and the composite action installs
`coder-eval==<action.yml's version: default>`. The release job moved `v0` and cut
the GitHub Release *before* the wheel was on PyPI, so a failure after the tag
move stranded `@v0` on a pin that cannot resolve — `uv tool install` 404s and
every consumer's pipeline breaks. It was reachable two ways: publish-pypi is a
separate `needs: release` job that can fail or wait on the `pypi` environment
gate, and the tag move sat before "Build wheel + sdist", so a build failure
stranded the pin with PyPI never involved.

Prevention, not detection:

- Move the `v0` promotion and the GitHub Release into a new `promote` job gated
  on `needs: [release, publish-pypi]`. Nothing consumer-visible happens until
  the wheel is published.
- `promote` is idempotent (force-push tag move, existence-guarded release
  create), so a failure is recovered by re-running the failed jobs — unlike the
  `release` job, which would bump a second version. That is what lets these
  steps fail loudly and removes the `continue-on-error` + annotation dance that
  existed only because a failure would have skipped publish-pypi.

Detection, for what ordering cannot cover (a yank, a rename, a delisting):

- New `verify-published-action.yml`. Tier 1 is free and deterministic: assert
  the major tag points at the newest release, that action.yml *at that tag*
  pins that version, that the version is on PyPI (retried for index
  propagation), that the Marketplace listing resolves, and that the wheel
  installs. Tier 2 consumes the action as a stranger would — `@v0`, default
  `version:`, no repo checkout, task YAML written inline.
- Triggered on Release completion regardless of conclusion: a failed
  publish-pypi makes the run conclusion `failure`, so gating on success would
  skip the check exactly when it matters. Plus a daily cron and dispatch.
- The e2e gate is ARTIFACTS, not the step's exit code. action.yml exits with
  coder-eval's own code, and coder-eval exits 1 on any failed task, so
  `minimum-task-score: 0.0` does not stop a model flake from reddening the
  build. It asserts run.json, a parseable JUnit, wired outputs, and non-zero
  tokens — "does the published action work", not "is the model still good".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Claude finished @uipreliga's task in 1m 40s —— View job


Code Review in Progress

Review Checklist:

  • Read .github/code_review.md for review guidelines
  • Read CLAUDE.md for project conventions
  • Get full diff with git diff origin/main...HEAD
  • Read .github/workflows/release.yml (full file)
  • Read .github/workflows/verify-published-action.yml (full file)
  • Analyze release workflow changes for correctness
  • Review verification workflow design
  • Check cross-file consistency
  • Perform "what's missing" analysis
  • Provide comprehensive review feedback

Starting code review now...

uipreliga and others added 2 commits August 4, 2026 14:59
Four findings from a multi-model review (gemini-3.1-pro, gpt-5.6-sol), all
verified by reproducing the failure before fixing:

- `verify-published-action.yml`: `NEWEST=$(git tag -l … | grep … | head -1)` under
  `set -euo pipefail` aborts the step when grep matches nothing (exit 1) or head
  closes the pipe early (141), so the `if [ -z "$NEWEST" ]` diagnostic below it was
  dead code — a repo with no release tags got a bare exit 1 with no message.
  Reproduced both ways; `|| true` lets the emptiness check own every failure mode.

- `verify-published-action.yml`: the Marketplace probe treated `403` and `000` as
  proof of delisting. GitHub commonly serves 403 to unauthenticated page fetches
  from CI runners, and `000` is curl failing outright (DNS/network/TLS) — both are
  "we learned nothing", not "it's gone". They now warn alongside 429/5xx; only 4xx
  proper still hard-fails. This was the exact cry-wolf failure the step's own
  comment set out to avoid.

- `verify-published-action.yml`: the e2e gate ignored the action step's exit code
  entirely, which also hid regressions in the action's OWN exit logic (e.g. a
  broken score gate reddening a run whose every task succeeded) — a genuine
  "published action is broken" signal. Now conditional: tolerate a red step when
  any task under-performed (model flake), require green when all reported SUCCESS.
  Verified it fires on the regression case and stays quiet on the flake case.

  Note the reviewer's proposed patch keyed on `final_status`, which does not exist
  in run.json — `eval_result_to_task_dict` writes `status`. Implemented against the
  real key and confirmed the suggested form would have been dead on arrival. The
  same typo was live in this workflow's own diagnostic line (printing
  `status=None` every run); fixed.

- `release.yml`: `gh release view` also matches a DRAFT or prerelease, so promote
  could skip creation and report success while announcing nothing to the
  Marketplace. Now normalizes with `gh release edit --draft=false
  --prerelease=false --latest`, making the job's idempotency claim true in fact.

Also records two deferred harness candidates: CE034 for the dead-guard shell
pattern (confirmed NOT caught by actionlint+shellcheck, so the existing actionlint
candidate does not subsume it), and runtime-key parity for the `run.json` keys that
shell consumers depend on but no test binds.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lthy lag

Second review pass (Opus) on top of the gemini/gpt-5 findings. Six issues, each
reproduced before fixing:

1. `v0` force-move was idempotent but NOT monotonic. GitHub keeps "Re-run failed
   jobs" live for 30 days, so replaying an OLD release's promote (0.9.5 fails at
   publish-pypi, operator ships 0.9.6, later cleans up the red 0.9.5 run) walked
   `v0` BACKWARDS and silently downgraded every consumer. The removed comment
   claimed re-running "is always safe" — force-push is only self-idempotent and
   says nothing about ordering. Now refuses to promote anything but the newest
   release tag, with a message naming the version to promote instead.

2. preflight treated the holding state THIS PR introduces as a defect. Because
   promote now moves `v0` only after publish-pypi, `v0` legitimately lags for the
   whole interval — including publish-pypi failing (where @v0 consumers are
   perfectly HEALTHY on the previous release) and the `pypi` environment approval
   window. Old code hard-failed at "consumers are not getting the newest release"
   and never reached the accurate stranded-pin diagnostic; every nightly during an
   approval window would have gone red on a working artifact. The two halves of
   this PR contradicted each other. The hard gate is now the consumer contract —
   the version @v0's action.yml PINS must be installable — and lag is classified:
   newest on PyPI => promote didn't run (hard fail, actionable); newest absent =>
   release merely incomplete (warning, consumers unaffected).

3. `publish-pypi` was not re-runnable, which the whole recovery story assumes. An
   upload that succeeds but whose step then fails (lost response, timeout) gets
   400 "File already exists" forever, so promote could never run for a version
   that IS published. Added `skip-existing: true`.

4. `|| echo 000` double-appended: curl's own `-w '%{http_code}'` already prints
   000 on transport failure, so CODE became the literal "000000" and matched
   neither the transient allowlist nor 5xx. A DNS/TLS blip was reported as
   "renamed or delisted" / a stranded pin. Verified `000000` empirically; the
   previous commit's attempt to allowlist "000" was therefore ineffective. Removed
   the append in both probes and split "unreachable" from "absent" in the messages.

5. e2e gate was load-bearing on composite `outputs:` surviving a
   continue-on-error failure — undocumented behavior, and if it does not hold
   every model flake reddens the workflow with "did not set the junit-path
   output", defeating the artifact-gate design. File checks now use the literal
   paths the workflow itself passes in `with:`; output wiring is asserted
   separately, hard only when the step went green (where propagation is
   guaranteed) and as a warning otherwise.

6. promote's `if:` failed in the SKIP direction. Gated on
   `needs.release.outputs.released_version != ''`, a lost output on a partial
   re-run resolves to skipped-green: green re-run, tag never moved, no Release.
   Now discriminates prereleases on `github.ref` (the same signal
   "Determine release mode" uses, and one that cannot evaporate), with emptiness
   enforced inside the job so a lost output is RED, not silent.

Also fixed two Low findings while here: removed dead `git config user.email/name`
(a lightweight `git tag -f` needs no committer identity), and scoped the paid e2e
tier off branch-dispatched prereleases, which cannot change the published artifact.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@uipreliga uipreliga left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: coder_eval — pr:81 (2 files) axis:1,2,3,4,5,6,7,8 — reviewed at local branch HEAD e88efe0, which is 2 commits AHEAD of the pushed PR head 0cad104

Scope: pr:81 (2 files) axis:1,2,3,4,5,6,7,8 — reviewed at local branch HEAD e88efe0, which is 2 commits AHEAD of the pushed PR head 0cad104 · branch fix/verify-published-action · e88efe0 · 2026-08-04T22:18Z · workflow variant

Change class: complex — restructures release promotion ordering across three interdependent jobs and adds a new verification workflow with nontrivial gating, trigger, and failure-mode control flow; correctness requires reasoning about partial-failure and re-runnability, not just reading the diff

The Python core remains excellent — type safety, API surface, and architecture all near-perfect (10/10, 10/10, 9.9/10) and the release chain's design rationale is unusually well documented — but every real risk sits in this change's untested workflow shell: a reference to a nonexistent steps.parity.outputs.version makes preflight red on 100% of runs (so the paid e2e tier this PR exists to add can never execute), a dead if: on publish-pypi can turn a re-run into a silent green that publishes nothing yet skips the v0 promotion, and several gates confidently misattribute upstream PyPI/model outages to broken wiring; bottom line: the design is sound and the fixes are small and localized, but the workflow glue needs one focused correctness pass — plus a way to test it — before this can be trusted as a release gate.

Summary

Axis Score 🔴 🟠 🟡 🔵 Top Issue
1. Code Quality & Style 6 / 10 1 0 2 0 Preflight references a nonexistent steps.parity.outputs.version (verify-published-action.yml:202, 247), so git show v:action.yml exits 128, preflight is red on every trigger, and the paid e2e tier never runs
2. Type Safety 10 / 10 0 0 0 0
3. Test Health 9.5 / 10 0 0 1 0 Release/action metadata re-derived inline in workflow shell instead of reusing the existing unit-tested helpers (Marketplace slug vs marketplace_slug(), newest-release-tag pipeline vs release.yml's promote guard, hardcoded UiPath/coder_eval@v0 owner+major)
4. Security 9.3 / 10 0 0 1 2 skip-existing: true (release.yml:349) removes the loud duplicate-upload failure that incidentally proved PyPI serves this run's artifact; nothing in release → promote → verify re-asserts artifact identity before v0 moves
5. Architecture & Design 9.9 / 10 0 0 0 1 workflow_run couples to release.yml by display name with no guard, so renaming the workflow silently disables release-time verification
6. Error Handling & Resilience 8.3 / 10 0 1 1 2 promote was hardened against the skipped-green hazard, but publish-pypi's dead if: needs.release.outputs.version != '' (release.yml:321) can still silently skip it — which also skips promote, making its in-job enforcement unreachable
7. API Surface & Maintainability 10 / 10 0 0 0 0
8. Evaluation Harness Quality 9.5 / 10 0 0 1 0 verify-published-action.yml's zero-token gate hard-fails with a fixed "wiring is broken" diagnosis, misattributing a transient model/API outage that run.json's own status/error_category fields already distinguish

Overall Score: 9.1 / 10 · Weakest Axis: Code Quality & Style at 6 / 10
Totals: 🔴 1 · 🟠 1 · 🟡 6 · 🔵 5 across 8 axes.

Blockers

  1. [Axis 1] Preflight references a nonexistent steps.parity.outputs.version (verify-published-action.yml:202, 247), so git show v:action.yml exits 128, preflight is red on every trigger, and the paid e2e tier never runs (.github/workflows/verify-published-action.yml:202) — The parity step (id: parity) emits exactly three outputs — lines 124-128 are echo "pin=$PIN" / echo "newest=$VERSION" / echo "lagging=$LAGGING". There is no version output (the shell variable is VERSION, the output key is newest), and grep -n 'id:' confirms parity is the only step with that id. Two later steps reference the non-existent key:
  • line 202: TAG_REF: v${{ steps.parity.outputs.version }} → expands to the literal v, so line 205 runs git show "v:action.yml"fatal: invalid object name 'v'. Under set -euo pipefail (line 204) the pipeline's non-zero status kills the step, so "Verify Marketplace listing resolves" fails on 100% of runs with a confusing git error.
  • line 247: VERSION: ${{ steps.parity.outputs.version }} → line 250 becomes uv tool install "coder-eval==".

Because preflight always goes red and e2e declares needs: preflight (line 262), the paid tier this PR exists to add can never execute. Fix: either add echo "version=$VERSION" to the output block at line 124-128, or — better — reuse the values already emitted. For line 202, the pin was read via git show "$MAJOR:action.yml" at line 92, so the Marketplace step should key off the same major tag. For line 247 use ${{ steps.parity.outputs.pin }}, not the newest version: the pin is the consumer contract this tier verifies (line 137's step already asserts the pin is on PyPI), and under the legitimate lagging=true state (lines 119-122) installing the newest version would fail while @v0 consumers are perfectly healthy.

Also fix the comment at lines 200-201 — "The version tag, which the parity step above proved is the same commit the major tag points at" is false in the lagging=true branch, where lines 109-121 explicitly establish that $MAJOR_SHA != $NEWEST_SHA.
2. [Axis 6] promote was hardened against the skipped-green hazard, but publish-pypi's dead if: needs.release.outputs.version != '' (release.yml:321) can still silently skip it — which also skips promote, making its in-job enforcement unreachable (.github/workflows/release.yml:321) — Line 321: if: needs.release.outputs.version != ''. The promote job's header comment (lines 386-391) states the exact hazard this shape carries: "were this gated on needs.release.outputs.released_version != '' and that output failed to carry over into a partial 'Re-run failed jobs' attempt, the job would resolve to SKIPPED-GREEN -- the operator sees a green re-run while the major tag never moves and no Release is cut." The author applied the mitigation to promote (gate on github.ref, enforce emptiness in-job via "Validate version shape", lines 413-424) but left publish-pypi on the old shape.

Under the author's own stated premise, the recovery flow this PR is built around — publish-pypi fails on the pypi environment gate, operator clicks "Re-run failed jobs" — resolves as: needs.release.outputs.version empty → publish-pypi SKIPPED → promote (needs: [release, publish-pypi], line 383) skipped because a skipped need is not success() → whole run GREEN, wheel never published, v0 never moved. That is a strictly worse outcome than the red job the promote redesign guarantees, and the new verify workflow only downgrades it to a ::warning (verify-published-action.yml lines 183-191), not a red.

The guard is also unreachable by design: the Resolve published version step already hard-fails on an empty version (line 188: if [ -z "$V" ]; then echo "no version resolved" >&2; exit 1; fi), so needs.release.outputs.version is never empty on a successful release job. Fix: drop the if: from publish-pypi entirely (the implicit success() on needs: release is the real gate), or mirror promote — gate on nothing and add an in-job [ -n "$VERSION" ] || exit 1 assertion so a lost output is red, not silently skipped.

Non-blocking, but please consider before merge

  1. [Axis 1] Rationale comments contradict or have drifted from the code they explain (released_version gating note vs promote's own rationale, 1 -> 1b -> 3 numbering, docker-publish.yml cross-file job layout, concurrency cancellation claim, stale test_release_notes.py rationale) (.github/workflows/release.yml:62) — Lines 61-64 add released_version: ${{ steps.release.outputs.version }} with the comment "The promote job gates on this, so a prerelease never moves the major tag or cuts a GitHub Release." That is contradicted 330 lines later by the code it describes: promote's gate is if: github.ref == 'refs/heads/main' (line 392), and lines 384-391 spell out that it is "discriminated on the DISPATCHED REF rather than on a needs output" and that "the job's if: deliberately no longer gates on" the version. A reader who trusts line 62 will conclude a prerelease is blocked by an empty-output check that does not exist.

The output itself is also a second source of truth for state already derivable. promote only runs on main (line 392); on main steps.mode never sets a version, so "Resolve published version" computes V="${REL:-$PRE}" = $REL (line 187), making outputs.version and outputs.released_version identical on every path where promote executes. Either drop released_version and have promote consume the existing needs.release.outputs.version, or keep it and rewrite the comment to say what actually enforces emptiness — the "Validate version shape" step at lines 413-424.
2. [Axis 1] Workflow decision logic and inline consumer task YAML live in oversized inline run: blocks with zero pre-merge test/lint coverage, despite the .github/scripts extract-and-unit-test precedent (.github/workflows/verify-published-action.yml:349) — The new workflow's logic lives in oversized inline scripts: "Check tag / pin parity" spans lines 59-129 (~70 lines, 7 decision points: empty-$NEWEST, git rev-parse verify, $MAJOR != v0, empty-$PIN, MAJOR_SHA = NEWEST_SHA, nested PIN != VERSION) and "Verify action mechanics" spans lines 349-415 (~66 lines that switch languages mid-step — bash checks at 356-372, then a python3 <<'PY' heredoc at 378-415 carrying four more branches). By the repo's own calibration (radon average B/5.33; the F/E outliers are the acknowledged debt) these are 10-20-branch units, i.e. the 🟡 band — and unlike Python they are invisible to make check, make lint, pyright, and shellcheck; actionlint parses the YAML but does not analyse embedded script semantics (which is why the broken steps.parity.outputs.version reference in finding #1 passed it clean).

Mitigation, again following .github/scripts/release_notes.py + tests/test_release_notes.py: move the parity resolution and the run.json assertions into .github/scripts/ modules with unit tests over fixture inputs (a tag list, a run.json blob), leaving each run: block as a few lines of invocation. The run.json assertions especially deserve it — they encode the cross-repo consumer contract (task_results / status / total_tokens / weighted_score), which is exactly the shape a fixture test protects against drift.
3. [Axis 3] Release/action metadata re-derived inline in workflow shell instead of reusing the existing unit-tested helpers (Marketplace slug vs marketplace_slug(), newest-release-tag pipeline vs release.yml's promote guard, hardcoded UiPath/coder_eval@v0 owner+major) (.github/workflows/verify-published-action.yml:209) — FAILURE: change action.yml's name: to a punctuated title such as Coder Eval (CI gate). CE026 updates the docs links to the correct slug coder-eval-ci-gate via marketplace_slug and make verify stays green, while the workflow's tr pipeline yields coder-eval-(ci-gate), the URL 404s, and line 236 fires ::error title=Marketplace listing missing - reddening preflight and skipping the paid e2e tier for a listing that is perfectly healthy.

EVIDENCE: line 209 is a second, weaker slugger:
SLUG=$(echo "$NAME" | tr '[:upper:]' '[:lower:]' | tr ' ' '-')
The tested one, tests/lint/action_docs.py:165-168::marketplace_slug, does more: re.sub(r"\s+", "-", listing_name.strip().lower()) then re.sub(r"[^a-z0-9._-]", "", slug) - it collapses whitespace runs, strips leading/trailing whitespace, and drops punctuation. The tr version does none of those. Nothing binds them: CE026 drives off tests/lint/action_docs.py:83-87::default_doc_paths, which returns README.md plus docs/**/*.md only, so the workflow is outside its reach. They agree today solely because action.yml:6 is name: coder_eval - the one input for which both are identity.

FIX: add a test asserting the shell pipeline's output equals marketplace_slug(action_listing_name(ACTION_YML)) over a table of names including one with punctuation and one with a double space, mirroring how CE026 already pins doc links to the same name:. That keeps the parity assertion at make-verify time instead of release time.
4. [Axis 4] skip-existing: true (release.yml:349) removes the loud duplicate-upload failure that incidentally proved PyPI serves this run's artifact; nothing in release → promote → verify re-asserts artifact identity before v0 moves (.github/workflows/release.yml:349) — release.yml:349 newly adds skip-existing: true to pypa/gh-action-pypi-publish. That flag makes twine treat PyPI's 400 "File already exists" as success WITHOUT comparing content, so after this diff a green publish-pypi no longer proves the wheel/sdist built in this run are the ones PyPI serves — it only proves a file of that name is present. Nothing downstream re-establishes the link: promote (line 383, needs: [release, publish-pypi]) moves the consumer-pinned major tag purely on that job succeeding (line 462-463, git tag -f "$MAJOR" "v${VERSION}" / git push -f origin "$MAJOR"), and the new preflight gate only asserts reachability, not identity — verify-published-action.yml:142-150 does URL="https://pypi.org/pypi/coder-eval/${PIN}/json" and passes on [ "$CODE" = "200" ]. Net effect: a wheel pre-uploaded under the release's exact version (compromised maintainer account or leaked legacy API token — hence PR:H/AC:H) is silently accepted, v0 is promoted to an action.yml pinning it, and every uses: UiPath/coder_eval@v0 consumer installs it on a fully green release. Before this diff the duplicate upload failed the job loudly. Fix is nearly free because the JSON the preflight already fetches carries the digests: after the publish step, compare digests.sha256 from https://pypi.org/pypi/coder-eval/<version>/json against sha256sum dist/* and fail on mismatch (keep skip-existing for the re-run story it was added for). Keep the finding even if you prefer a different fix — the gap is that no step in the release→promote→verify chain asserts artifact identity. CVSS:3.1/AV:N/AC:H/PR:H/UI:N/S:C/C:N/I:H/A:N
5. [Axis 6] PyPI probes classify HTTP codes without the transient (403/429/5xx) split the sibling Marketplace probe performs, misdiagnosing in both directions (false 'Stranded action.yml pin' hard error; false benign 'Release incomplete' warning that hides a stranded promote) (.github/workflows/verify-published-action.yml:164) — The retry loop (lines 144-156) distinguishes only two outcomes after 6 attempts: CODE = 000 → "PyPI unreachable … inconclusive" (lines 160-163), and everything else → line 164: echo "::error title=Stranded action.yml pin::coder-eval==${PIN} is NOT on PyPI (HTTP $CODE), but @v0 points at an action.yml that installs it. Every 'uses: UiPath/coder_eval@v0' consumer fails at install. Re-run the Release workflow's publish-pypi job, then its promote job." A sustained PyPI/Fastly 429, 503 or 403 therefore produces a confidently-wrong actionable instruction: it tells the operator to re-run publish-pypi + promote for a version that is already published and healthy.

The same job already gets this right 70 lines later for the Marketplace probe, line 233: elif [ "$CODE" = "000" ] || [ "$CODE" = "403" ] || [ "$CODE" = "429" ] || [ "$CODE" -ge 500 ] 2>/dev/null; then::warning title=Marketplace check inconclusive. Fix: apply the same 4xx-vs-transient split to the PyPI step — treat only 404 (and other definitive 4xx that are not 403/429) as a stranded pin; treat 000/403/429/5xx as inconclusive with a ::warning. Note this branch is also the input to the "Classify major-tag lag" step's contrast at lines 177-191, which already models 5xx-ish codes as non-proof, so the two steps currently disagree about the same HTTP code.
6. [Axis 8] verify-published-action.yml's zero-token gate hard-fails with a fixed "wiring is broken" diagnosis, misattributing a transient model/API outage that run.json's own status/error_category fields already distinguish (.github/workflows/verify-published-action.yml:395) — Lines 395-398 are the only signal that survives continue-on-error, and they hard-fail with a fixed diagnosis:

          if tokens <= 0:
              print("::error::no tokens consumed across any task -- the agent never reached the model "
                    "(credential passthrough, agent runtime, or backend wiring is broken)")

total_tokens is None (→ 0 via the or 0 at line 390) whenever EvaluationResult.total_token_usage is unset, and Orchestrator._aggregate_token_usage (src/coder_eval/orchestrator.py lines 967-977) only sets it if self.result.iterations: and if usages: — so a run whose turns all die before any token_usage is recorded reports zero tokens. That covers a transient Anthropic 429/529 across retries and a sandbox-setup failure, not just credential/runtime breakage. On a daily cron (line 28) this will eventually fire and send the operator to check credential passthrough for an upstream outage — the same conflation of "is the published action working" with "is the model available" that lines 310-317 argue the artifact gate exists to prevent.

The discriminator is already in the artifact being read: eval_result_to_task_dict writes both "status": result.final_status and "error_category": (result.error_details or {}).get("error_category") into every run.json row (src/coder_eval/reports_experiment.py lines 136, 189). Recommendation: when tokens <= 0, branch on the rows' status/error_category — emit ::warning (inconclusive, upstream) for an agent/API error category and keep ::error for the genuine wiring failure, mirroring the 000-vs-4xx split the preflight steps already use at lines 158-165 and 226-238.

Nits

  1. [Axis 4] New promote job mints an unscoped GitHub App token and inherits packages: write it never uses (.github/workflows/release.yml:401) — Two least-privilege regressions in the job this diff adds. (1) release.yml:399-404 mints a second app token with uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 and only app-id: / private-key: — no permission-* inputs. Per that action's contract, omitting them yields a token holding EVERY permission of the installation, and this is the app the workflow header (line 41-44) describes as the one with the main-branch ruleset bypass ("only the release app has a ruleset bypass"). The job needs exactly contents: write (re-point the tag at line 463, gh release create at line 519), so add permission-contents: write to both mint sites. (2) promote: (line 381) declares no job-level permissions: block, so it inherits the workflow-level permissions: at lines 45-47 — including packages: write # push the versioned agent image to ghcr.io on release, which is only needed by the GHCR steps in the release job. Add permissions: {contents: read} to promote (its writes go through the app token, not GITHUB_TOKEN) and, while there, permissions: {} to verify-published-action.yml's e2e job, which has no checkout and needs none of the contents: read granted at its line 31-32. CVSS:3.1/AV:N/AC:H/PR:H/UI:N/S:U/C:L/I:L/A:N
  2. [Axis 4] Unpinned global npm install of the agent runtime in an unattended nightly job that forwards ANTHROPIC_API_KEY (.github/workflows/verify-published-action.yml:283) — verify-published-action.yml:283 runs run: npm install -g @anthropic-ai/claude-code with no version constraint and no integrity pin, and the same job then forwards a repository secret into the run at line 331 ( ANTHROPIC_API_KEY=${{ secrets.ANTHROPIC_API_KEY }}) on a GitHub-hosted runner with no container isolation. Any upstream/registry compromise of that package executes with the key in its environment. This matches five pre-existing instances in pr-checks.yml (lines 288, 381, 545, 784, 852), so it is the repo's convention rather than a new pattern — but the marginal exposure is genuinely wider here, because this is the first such job on an unattended schedule: (line 27-28, cron: "17 6 * * *") rather than a human-reviewed PR run, so a poisoned publish is pulled and executed nightly with no one watching. Note the tension with intent: the header comment at line 25 lists "the @anthropic-ai/claude-code npm package" as drift this nightly is meant to catch, so a hard pin defeats a stated purpose. Recommended resolution is one of: pin a major/minor (@anthropic-ai/claude-code@^2) so drift is still detected within a reviewed range; or record it as an explicit accepted risk in this file's header, the way action.yml:21-22 and release.yml:480-487 already document their accepted risks. CVSS:3.1/AV:N/AC:H/PR:H/UI:N/S:U/C:L/I:L/A:N
  3. [Axis 5] workflow_run couples to release.yml by display name with no guard, so renaming the workflow silently disables release-time verification (.github/workflows/verify-published-action.yml:18) — Line 18 workflows: ["Release"] matches release.yml:1 name: Release by string. GitHub does not error on an unmatched name — the trigger simply never fires — so renaming release.yml's name: degrades this gate to schedule-only (cron at line 28) with no signal, which is exactly the silently-inert-check failure the file's own comments argue against (lines 11-14, 105-108). This is cheap to guard following the existing precedent in tests/test_pr_review_workflow.py (which already parses a workflow YAML by path): assert yaml.safe_load('.github/workflows/verify-published-action.yml')['on']['workflow_run']['workflows'] == [yaml.safe_load('.github/workflows/release.yml')['name']].
  4. [Axis 6] Upload run on failure never fires in the exact case the e2e design deliberately tolerates, discarding the run dir that would explain a model flake (.github/workflows/verify-published-action.yml:418) — Line 418: if: failure(). The "Run the published action" step is continue-on-error: true (line 320), so when it goes red but the artifact gate passes, the job outcome is success and this step is skipped — runs/verify-published/ (run.json, task.json, task.log) is thrown away precisely in the scenario the design says will happen routinely ("a model flake failing file_exists would redden this step", lines 310-317). Fix: if: always(), or if: failure() || steps.run.outcome != 'success', so the diagnostic artifacts survive a tolerated red action step.
  5. [Axis 6] e2e gate assertions can pass vacuously: JUnit check only parses the XML (zero testcases passes) and the output-wiring check silently skips when the action step is red but outputs are non-empty and wrong (.github/workflows/verify-published-action.yml:370) — Lines 366-372 assert OUT_JUNIT/OUT_RUNDIR only when STEP_OUTCOME = success, then line 370: elif [ -z "$OUT_JUNIT" ] || [ -z "$OUT_RUNDIR" ]; then warns only about empty outputs. A red step that propagates non-empty but incorrect outputs (e.g. a future action.yml regression writing the wrong path to $GITHUB_OUTPUT) falls through both branches with no assertion and no message. Fix: in the non-success branch, compare the outputs when they are non-empty and emit a ::warning on mismatch (still not a hard fail, per the step's stated rationale) so the wiring contract is never silently unchecked.

What's Missing

Parallel paths:

  • 🟠 🟠 The diff's own thesis — nothing consumer-visible is promoted before the wheel is on PyPI — was applied to the v0 tag and the GitHub Release but NOT to the GHCR agent image: release.yml's "Compute image tags"/"Build and push versioned agent image" (lines ~262-300) still push coder-eval-agent:<version> and move :latest inside the release job, before publish-pypi runs, and under continue-on-error: true. So a release whose PyPI publish fails still advertises :latest for a version that does not exist on PyPI (docs/DOCKER_ISOLATION.md tells users to FROM coder-eval-agent:<version>), and a silently-failed push leaves a fully green release with no image at all. Either move the image push behind publish-pypi (as the tag move now is) or state in the promote header why the image is deliberately exempt. (trigger: .github/workflows/release.yml)
  • 🟡 🟡 The new verification tier covers PyPI + Marketplace + the v0 pin, but not the one release artifact published best-effort (continue-on-error: true on all three GHCR steps in release.yml): ghcr.io/uipath/coder-eval-agent:<version>/:latest. A trivial docker manifest inspect (or a skopeo/curl HEAD against the GHCR API) in preflight would close the highest-probability silent gap, since a missing image is the only published artifact whose absence currently produces a green release. (trigger: .github/workflows/verify-published-action.yml)
  • 🟡 🟡 The parity step hard-fails on a major bump with "Bump the uses: in this workflow" (verify-published-action.yml:84-87) — naming only itself, while uses: UiPath/coder_eval@v0 is hardcoded in four doc snippets (README.md:111,148, docs/CI_GATE.md:31,77, docs/tutorials/02-ci-pipeline.md:174) and CE026 checks the Marketplace slug but never the major. On 1.0.0 the nightly reddens and the docs keep telling consumers to pin a stale major; either extend the error message to enumerate the doc surfaces or add a CE026 clause asserting every coder_eval@v<major> matches pyproject.toml's major. (trigger: .github/workflows/verify-published-action.yml)
  • 🟡 🟡 CE026's REQUIRED_PREREQ_TOKENS stays pinned to a single executable reference — action-dogfood in pr-checks.yml (tests/lint/action_docs.py:DOGFOOD_JOB, tests/test_custom_lint.py::test_required_prereqs_match_the_dogfood_job) — but the new e2e job (verify-published-action.yml:278-283) is now a third copy of the same two prereq steps and the truer consumer proof (no checkout, published action, default pin). The lint's PR_CHECKS path was not extended, so the two jobs can drift and the docs will follow only one. (trigger: .github/workflows/verify-published-action.yml)
  • 🟡 🟡 promote was redesigned around the skipped-green hazard (ref gate + in-job "Validate version shape") but the sibling publish-pypi kept the exact shape the new comment condemns (if: needs.release.outputs.version != '', release.yml:321) — and since promote now declares needs: [release, publish-pypi], a skipped publish-pypi skips the promotion too, making the new in-job enforcement unreachable in the one scenario it was written for. The hardening was applied to one of the two jobs that share the pattern. (trigger: .github/workflows/release.yml) (restates: Axis 6: publish-pypi's dead if-gate can still silently skip it)

Tests:

  • 🟠 🟠 Nothing statically binds a steps.<id>.outputs.<key> expression to the keys that step actually writes to $GITHUB_OUTPUT, which is exactly why the steps.parity.outputs.version bug ships red on 100% of runs. The repo already parses workflow YAML in tests (tests/test_pr_review_workflow.py) — a ~20-line test (or a new CEnnn) that collects echo "k=..." >> $GITHUB_OUTPUT keys per step id and asserts every steps.X.outputs.Y reference resolves would have caught it at make verify, before merge. (trigger: .github/workflows/verify-published-action.yml) (restates: Axis 1: Preflight references a nonexistent steps.parity.outputs.version)
  • 🟠 🟠 The inline consumer task YAML (verify-published-action.yml:289-307) is a whole TaskDefinition document (task_id + initial_prompt + success_criteria) that no test validates, while CE029 (tests/lint/doc_examples.py) already validates precisely that shape wherever it appears in Markdown. A rename of setting_sources/permission_mode/allowed_tools, or any extra="forbid" violation, now surfaces only in the paid nightly as an opaque action failure. Extend CE029's extractor to heredoc YAML under .github/workflows/, or add a test that loads the heredoc and constructs TaskDefinition. (trigger: .github/workflows/verify-published-action.yml)
  • 🟡 🟡 The action's score-gate failure direction is still never exercised: both consumer-simulating jobs pass minimum-task-score: "0.0" (verify-published-action.yml:329 and action-dogfood:868), so the gate is only ever proven to pass. The new exit-contract check (lines 400-411) catches a gate that wrongly fails, but nothing catches a gate that wrongly passes — the direction that silently disables every consumer's quality gate. A cheap addition: one more step invoking the action with a score floor the smoke task cannot meet and asserting steps.<id>.outcome == 'failure'. (trigger: .github/workflows/verify-published-action.yml)
  • 🔵 🔵 step-summary (action.yml's only other behavioral input, default true, appending run.md to $GITHUB_STEP_SUMMARY at action.yml:160) is asserted by neither consumer job, despite the new step being titled "Verify action mechanics" — one grep -q against $GITHUB_STEP_SUMMARY would cover it while the run dir is still on disk. (trigger: .github/workflows/verify-published-action.yml)

Daily/nightly:

  • 🟠 🟠 This adds the repo's first token-spending unattended cron (17 6 * * *; the only other cron is codeql's weekly scan) and the blast radius is never stated: recurring Haiku spend on secrets.ANTHROPIC_API_KEY shared with the ADO nightly, who owns a nightly red, and the fact that no workflow in this repo has any failure notification path (no Slack step, no issue-on-failure) — so the artifact-verification signal this PR exists to create lands only in the Actions tab. Combined with the critical undefined-output bug it will be red from day one, which is the documented way a check earns being ignored. (trigger: .github/workflows/verify-published-action.yml)
  • 🟡 🟡 Neither trigger this gate depends on can fire from a branch — schedule: and workflow_run: only run from the default branch — so all 423 lines are unexercisable before merge (which is how the undefined-output bug reaches main), and GitHub silently disables schedule: after 60 days of repo inactivity. The PR states neither. A workflow_dispatch-able dry-run mode (skip the paid tier, or a --check-only input) plus the workflow-name parity test would make the gate provable pre-merge instead of provable-in-production. (trigger: .github/workflows/verify-published-action.yml) (restates: Axis 5: workflow_run couples to release.yml by display name with no guard)
  • 🔵 🔵 Unstated false-red window: the 06:17 cron shares no concurrency group with the Release workflow, so a nightly that lands after publish-pypi succeeds but before promote finishes (e.g. while the pypi environment approval is being granted) sees PyPI 200 plus a lagging major tag and hard-fails with "promote did not run" on a release that is mid-promotion. Worth either a short pre-check re-poll of the major tag or a sentence in the header acknowledging the race. (trigger: .github/workflows/verify-published-action.yml)

Downstream consumers:

  • 🟡 🟡 The # <-- kept in sync pin anchor now has THREE readers with three different tolerances: release.yml's sed demands exactly three spaces before #, tests/test_action_version_pin.py::_PIN_PATTERN accepts [ \t]+, and the new workflow (verify-published-action.yml:92-97) accepts [[:space:]]+. Nothing asserts they agree, so a future reformat can leave the new step reporting "parity OK" on a pin the release-time sed silently refused to bump. Extending test_action_version_pin.py (whose docstring already claims to guard this anchor) with the workflow's regex is a two-line fix. (trigger: .github/workflows/verify-published-action.yml) (restates: Axis 3: Release/action metadata re-derived inline in workflow shell instead of reusing the existing unit-tested helpers)
  • 🟡 🟡 The new gate hardcodes the run.json consumer contract (task_results, status, weighted_score, total_tokens) as shell/Python string keys against reports_experiment.eval_result_to_task_dict, joining action.yml's score gate in the same unbound coupling. This diff records the gap as a deferred candidate in .claude/harness-candidates.md ("Runtime-key parity for run.json consumers outside src/") but ships nothing that closes it, so a key rename in src/ still turns two external gates into no-ops silently. (trigger: .github/workflows/verify-published-action.yml) (restates: Axis 1: Workflow decision logic and inline consumer task YAML live in oversized inline run: blocks with zero pre-merge test/lint coverage)
  • 🟡 🟡 The release procedure gained a third job, a new recovery flow ("re-run publish-pypi, then promote"), and a new failure taxonomy the nightly emits verbatim to operators ("Stranded action.yml pin", "promote did not run", "Release incomplete") — none of which exists outside workflow comments. CONTRIBUTING.md has no release section, and the only prose about release.yml is one line in CLAUDE.md's tree listing that still describes it as maintaining the pin plus the moving tag, naming neither promote nor the new verification workflow. Whoever pages on the first nightly red has to read 700 lines of YAML to find the runbook. (trigger: .github/workflows/release.yml)

Display & mapping dicts:

  • 🟡 🟡 The exit-contract assertion re-encodes a FinalStatus value as the bare string "SUCCESS" (verify-published-action.yml:407) and treats every other status as "model under-performed, tolerate the red step" — collapsing ERROR, BUILD_FAILED, TIMEOUT, and TOKEN_BUDGET_EXCEEDED into the model-flake bucket, even though those are precisely the harness/wiring failures this gate exists to catch. models/enums.py deliberately routes this through _STATUS_CATEGORIES with a no-catch-all assert so a new status cannot silently collapse; the workflow bypasses that SSOT. Assert on the category (succeeded vs error) rather than one literal, so an action-side ERROR with non-zero tokens cannot pass green. (trigger: .github/workflows/verify-published-action.yml)

Harness & Lint Improvements

Static checks (lint / type):

  • [ce-lint] CE035 — workflow output-key parity. New whole-tree lint test class (wire as a @pytest.mark.lint class in tests/test_custom_lint.py alongside CE026–CE031, with the resolver in a new tests/lint/workflow_outputs.py; it reasons over YAML + run: text, so it is not a BaseRule): for every ${{ steps.<id>.outputs.<key> }} and ${{ needs.<job>.outputs.<key> }} expression in .github/workflows/** and action.yml, resolve the writer and fail when the key is never produced. Writers are mechanically enumerable: echo "<key>=…" >> "$GITHUB_OUTPUT" (and printf/heredoc forms) inside the step whose id: matches; the outputs: block of a local composite (uses: ./action.yml:79-86); the outputs: map of the referenced job; a pinned third-party action's declared outputs (allowlisted by SHA, no network). Scale is tractable today: 48 steps.*.outputs.* + 8 needs.*.outputs.* references against 11 $GITHUB_OUTPUT write sites. Claim the next free id at implementation time (CE032–CE034 are reserved by .claude/harness-candidates.md; tests/lint/runner.py's id-uniqueness assert is the SSOT). Prevents: The critical finding reported by 7 of 8 axes: .github/workflows/verify-published-action.yml:202 and :247 read steps.parity.outputs.version, but the parity step writes only pin/newest/lagging (124-128). TAG_REF becomes the bare string v, git show "v:action.yml" exits 128 under set -euo pipefail, preflight is red on 100% of triggers, and e2e (needs: preflight) never runs; line 247 would also emit uv tool install "coder-eval==". An actionlint-only adoption does not close this — it validates the step id but models steps.*.outputs as an open string map, so an unwritten shell key is untyped and unflagged.
  • [ce-lint] CE036 — ban the skipped-green job gate. Fail any job-level if: in .github/workflows/** whose only discriminator is an emptiness/equality test on needs.<job>.outputs.<key> (!= '', == '', …). The repo already documents this exact shape as an anti-pattern in prose (release.yml:386-391: a lost output on a partial "Re-run failed jobs" resolves the job to SKIPPED-GREEN), and the promote redesign in this diff was the manual application of that lesson — the rule turns the prose into enforcement. Escape hatch: inline # noqa: CE036 — <reason> for genuinely value-driven gates that cannot strand a release. Prevents: The high finding at release.yml:321 (if: needs.release.outputs.version != '' on publish-pypi). Since promote now declares needs: [release, publish-pypi] (line 383), a silently-skipped publish-pypi also skips the v0 move and the GitHub Release — a fully green run with nothing published — and skips promote's own "Validate version shape" (413-424), the step written to enforce exactly that scenario. The guard is also provably dead (line 188 already exit 1s on an empty version), which a reviewer had to notice by hand.
  • [ce-lint] CE037 — if: failure() is wrong in a job containing a continue-on-error step. In any job with a continue-on-error: true step, fail on a diagnostic/upload step guarded by if: failure() (or cancelled()) that does not also reference the tolerated step's steps.<id>.outcome; require always() or failure() || steps.<id>.outcome != 'success'. Pure YAML shape check, ~30 lines, same test-class slot as CE035. Prevents: The finding at verify-published-action.yml:418: "Run the published action" is continue-on-error: true (line 320), so in the routine tolerated-red case the design itself predicts (310-317, "a model flake failing file_exists would redden this step") the job outcome is success, if: failure() is skipped, and runs/verify-published/ (run.json, task.json, task.log) — the only evidence explaining the flake — is discarded precisely when needed.
  • [ce-lint] CE038 — workflow_run.workflows entries must name a real workflow. Assert every string under on.workflow_run.workflows in .github/workflows/** equals the name: of some workflow file in that directory. Six-line YAML check; tests/test_pr_review_workflow.py already establishes the precedent of parsing a workflow YAML by path. Prevents: The finding at verify-published-action.yml:18 (workflows: ["Release"] coupled to release.yml:1 by display string). GitHub does not error on an unmatched name — the trigger simply never fires — so renaming release.yml's name: silently degrades release-time verification to schedule-only, the "silently inert check" failure mode the file's own comments (11-14, 105-108) argue against.
  • [ce-lint] CE039 — runtime-key parity for run.json consumers outside src/ (promotes and widens the candidate already booked in .claude/harness-candidates.md). AST-extract the key set written by eval_result_to_task_dict (src/coder_eval/reports_experiment.py, dict literal ~136-189) and assert every string key a non-Python consumer reads — .github/workflows/** and action.yml: task_results, task_id, status, weighted_score, total_tokens, plus error_category once the zero-token branch consumes it — exists in that set. Mirrors how CE030 pins doc/schema parity; grep consumers for r.get("…") / data.get("…") / jq-style key paths. Prevents: The run.json contract half of the maintainability finding at verify-published-action.yml:349 and the vacuous/misdiagnosing gates at :370 and :395. Concretely: the comment at 385-388 exists only because a reviewer proposed final_status, a key absent from run.json that would have made the exit-contract assertion dead on arrival — a prose warning where a mechanical gate belongs. It also makes the recommended fix for the zero-token misdiagnosis (branch on status/error_category) safe to write.
  • [ce-lint] CE040 — cap inline run: bodies; oversized decision logic must live in .github/scripts/. Fail a run: body in .github/workflows/** exceeding ~35 lines, or one that switches interpreter mid-step (bash assertions then a python3 <<'PY' heredoc), unless it is a thin invocation of a .github/scripts/ module. Directly analogous to the existing CE022 statement cap on the simulation dialog loop, and composes with the already-booked CE032 (run the AST rules over embedded Python) / CE033 (quoted heredoc delimiters). Measured bodies in scope today are 70, 66, 37, 27, 24, 19, 6 (verify-published-action.yml) and 31, 22, 19, … (release.yml), so the rule flags exactly the oversized blocks and nothing else. Prevents: The maintainability finding at verify-published-action.yml:349 — and, more importantly, it is the structural reason the critical steps.parity.outputs.version bug survived: it lives inside the 70-line "Check tag / pin parity" block (59-129) with 6 decision points and zero coverage from make check, make lint, pyright, or any test. Extraction is what makes the parity logic, the HTTP classification, and the run.json gate unit-testable at all (precedent: .github/scripts/release_notes.py + tests/test_release_notes.py).
  • [ce-lint] CE041 — one Marketplace slugger (extend CE026 to .github/workflows/**). CE026 already owns Action-listing parity but drives off tests/lint/action_docs.py:83-87::default_doc_paths, which returns only README.md + docs/**/*.md. Extend it to (a) include workflow files when checking github.com/marketplace/actions/<slug> links and (b) forbid any second slug derivation in the repo — grep for a tr '[:upper:]' '[:lower:]' | tr ' ' '-' pipeline (or any hand-rolled lowercase/space substitution) applied to a value read from action.yml's name:. The one tested derivation is tests/lint/action_docs.py:165-168::marketplace_slug. Prevents: The DRY/test-health finding at verify-published-action.yml:209. The shell slugger drops none of the punctuation and collapses none of the whitespace marketplace_slug does: Coder Eval (CI gate)coder-eval-(ci-gate) vs. coder-eval-ci-gate. They agree today only because action.yml:6 is name: coder_eval, the single input where both are identity — so a rename passes make verify green while the workflow 404s and fires ::error title=Marketplace listing missing, reddening preflight for a healthy listing.
  • [ce-lint] CE042 — skip-existing: true requires an in-job artifact-identity assertion. Fail a pypa/gh-action-pypi-publish step that sets skip-existing: true in a consumer-facing publish job unless the same job also contains a digest comparison (grep for digests / sha256sum / an attestation verification step) or carries an inline # noqa: CE042 — <accepted risk>. Shape-only: the rule enforces that some identity gate exists, not how it is written. Prevents: The medium security finding at release.yml:349. skip-existing makes twine treat PyPI's 400 "File already exists" as success without comparing content, so a green publish-pypi no longer proves the wheel built in this run is what PyPI serves. Nothing downstream re-establishes it: promote moves v0 on job success alone (462-463) and preflight asserts only reachability ([ "$CODE" = "200" ], verify-published-action.yml:142-150). A repo-wide grep confirms no sha256/attestation assertion anywhere in the release→promote→verify chain.
  • [bandit-codeql] Adopt the workflow static-analysis tier: actionlint + zizmor (+ shellcheck via actionlint) over .github/workflows/** and action.yml, as a make lint-workflows target folded into make verify plus a pr-checks.yml job (start non-blocking, promote to required). This is the workflow-shaped analogue of the bandit/pip-audit tier and is genuinely absent: a repo-wide grep across Makefile, .pre-commit-config.yaml, and every workflow finds no actionlint, zizmor, or shellcheck invocation (sole hit: a # shellcheck disable=SC2206 comment in action.yml — someone assumed a checker that never runs). It subsumes the booked CE026 floating-uses-ref candidate, and zizmor's excessive-permissions covers the permissions findings without a bespoke rule. State the boundary explicitly: this tier does not replace CE035 (open-map output typing) or CE034 (verified: actionlint+shellcheck do not flag the VAR=$(… | grep …)-under-set -e dead diagnostic). Prevents: The two low security findings — release.yml:399-404 mints a create-github-app-token with no permission-* inputs (full installation scope, and this is the app holding the main-branch ruleset bypass) while promote declares no job-level permissions: and inherits packages: write it never uses (zizmor excessive-permissions); and verify-published-action.yml:283's unpinned npm install -g @anthropic-ai/claude-code in an unattended nightly that forwards ANTHROPIC_API_KEY at line 331 (floating-dependency + credential-exposure rules, making the pin-or-accept decision a recorded one). Also the general net under the shell-quality half of the :349 finding.

Harness improvements (not statically reachable):

  • Extract the three decision units into .github/scripts/ and unit-test them against fixtures — (1) tag/pin/lag parity resolution (verify-published-action.yml:59-129), (2) a single classify_http(code) shared by all three PyPI/Marketplace probes, (3) the run.json e2e gate (:377-411). Add tests/test_verify_published_scripts.py mirroring tests/test_release_notes.py (which loads .github/scripts/release_notes.py by path via importlib), plus a pr-checks.yml job on paths: ['.github/**']. Fixture tables: tag lists (lagging / in-sync / missing-pin), HTTP codes (200, 000, 403, 429, 503, 404), and run.json blobs (zero rows, total_tokens: null, status: ERROR + error_category: agent_rate_limit, all-SUCCESS-with-red-step). Why not static: CE040 can force the extraction and CE039 can pin the key names, but neither can check the logic is right: that a sustained 429 classifies as inconclusive rather than "Stranded action.yml pin", that lagging=true selects the pin rather than the newest version, or that a zero-token run with error_category=agent_rate_limit warns instead of asserting broken credential passthrough. Those are input→verdict behaviors and need executed code over fixtures. Prevents: The HTTP-classification finding at verify-published-action.yml:164 (403/429/5xx misreported as a stranded pin, with an instruction — re-run publish-pypi — PyPI will reject as a duplicate) and its harmless twin at :191; the zero-token misdiagnosis at :395; the :349 no-coverage finding; and it would have caught the critical steps.parity.outputs.version bug on first execution.
  • Add an if: always() guard job to both release.yml and verify-published-action.yml that reads every expected job's result from needs and fails when any is skipped/cancelled on a path where it was supposed to run (on main: release, publish-pypi, promote must all be success). One job, ~10 lines, no new tooling. Why not static: The hazard is a runtime job result, not a YAML shape: a job may be legitimately skipped (prerelease dispatch, non-default branch) or hazardously skipped (lost needs output on a partial re-run). Only the running workflow knows which case it is in, so the assertion must execute inside the run. CE036 removes the known trigger; this catches the class. Prevents: The green-but-nothing-ran outcomes in both the release.yml:321 finding (publish-pypi skipped → promote skipped → GREEN run, wheel unpublished, v0 unmoved) and the critical :202 finding (preflight red → the paid e2e tier this change exists to add never runs, and nothing announces its absence).
  • Make the e2e assertions non-vacuous and keep the evidence. In the gate step: assert the JUnit report contains ≥1 <testcase> and that the count matches the number of task_results rows (today it only calls ET.parse, so a zero-testcase report passes); in the non-success branch, compare non-empty composite outputs against the expected paths and ::warning on mismatch instead of falling through silently; and switch the diagnostic upload to if: always(). Why not static: These are assertions about a produced artifact (testcase count; output values as the runner materializes them for a red composite step), so they exist only at run time. A lint rule can require an always()-guarded upload (CE037) but cannot know whether the XML the action wrote contains any tests. Prevents: The two weak-assertion findings at verify-published-action.yml:370 (a red step propagating non-empty-but-wrong outputs falls through both branches with no message) and :418, plus the JUnit parse-only half of :360.
  • Assert published-artifact identity at release time. After publish-pypi, fetch https://pypi.org/pypi/coder-eval/<version>/json, match each urls[] entry by filename, and compare digests.sha256 against sha256sum dist/*, failing on mismatch (keep skip-existing for the re-run story it was added for); optionally also require a PEP 740 attestation from this workflow. Then have preflight's PyPI probe reuse that digest instead of settling for HTTP 200. Why not static: It needs the live PyPI response and the locally built artifacts; CE042 can only enforce that such a step exists, never that the digests match. Note the pre-diff protection was incidental (a duplicate upload failed loudly) — nothing declared the contract. Prevents: The release.yml:349 security finding: with no identity assertion anywhere in release→promote→verify, a wheel pre-uploaded under the release's exact version is silently accepted, v0 is promoted to an action.yml pinning it, and every uses: UiPath/coder_eval@v0 consumer installs it on a fully green release. The lag classifier compounds it by telling the operator to "re-run promote", i.e. to promote onto the unverified artifact.
  • Treat the new unattended workflow as needing a first-run proof and a watcher. Before relying on it, workflow_dispatch it once and require a green run (which would have surfaced the exit-128 preflight immediately). Ongoing: route a failed scheduled run to a durable signal — open/update a tracking issue or post to the release channel — rather than relying on someone opening the Actions tab; same for a scheduled run where e2e resolved to skipped. Why not static: Nothing about the YAML is wrong; the gap is that a nightly, non-required workflow with no notification channel can stay red for weeks with no observer — exactly how a 100%-failing preflight would have persisted. Detecting it needs run history, not file shape. Prevents: The critical :202 finding's blast radius (silently red preflight ⇒ permanently skipped paid tier) and the :18 finding's silent degradation-to-schedule-only mode.
  • Process note, not automatable: rationale comments explaining a mechanism 300+ lines away are drift-prone; keep rationale with the code it governs (which the .github/scripts/ extraction enables — a docstring next to a tested function). For the specific instance, rewrite release.yml:61-63 to say what actually enforces emptiness (promote is ref-gated at line 392; emptiness is validated in-job by "Validate version shape", 413-424) rather than claiming promote gates on released_version — and do not collapse released_version into version: the verify pass showed they are semantically different off main (a branch dispatch sets version to the rc while released_version stays empty). Why not static: Deciding whether a sentence about promote contradicts an if: expression 330 lines below is semantic judgment; no grep or AST shape separates an accurate rationale from a stale one. Recorded explicitly so the reach of static analysis on this finding class is a deliberate decision, not an omission. Prevents: The comment/rationale-drift finding at release.yml:62 and its grouped instances (verify-published-action.yml:34, :131, :374; release.yml:244, :488), including the false claim at verify-published-action.yml:200-201 that the version and major tags are the same commit — which lines 119-122 explicitly contradict in the lagging=true branch.

Top 5 Priority Actions

  1. Fix the undefined step output that reddens preflight on every single run regardless of the artifact's actual health: /Users/religa/src/coder_eval/.github/workflows/verify-published-action.yml:202 expands TAG_REF to the bare string v (so git show "v:action.yml" exits 128 under set -euo pipefail, killing the job before any Marketplace diagnostic and skipping e2e via needs: preflight) and :247 builds uv tool install "coder-eval==" — key both off the values the parity step actually emits (pin at :125, and the major tag read at :92), and correct the now-false comment at :200-201 that claims the version tag and major tag are the same commit even in the lagging=true branch.
  2. Delete the dead gate at /Users/religa/src/coder_eval/.github/workflows/release.yml:321 (if: needs.release.outputs.version != '') — it can never be false on a successful release job (:188 already hard-fails on an empty version), but under the author's own stated partial-re-run hazard it resolves publish-pypi to SKIPPED-GREEN, which also skips promote (needs: [release, publish-pypi], :383) and therefore makes promote's entire "Validate version shape" hardening (:413-424) unreachable in exactly the recovery flow it was written for, yielding a green run with no wheel and an unmoved v0.
  3. Stop the daily e2e gate from blaming credential/runtime wiring for an upstream outage: at /Users/religa/src/coder_eval/.github/workflows/verify-published-action.yml:395 the tokens <= 0 branch hard-fails with a fixed "the agent never reached the model … wiring is broken" message even though total_tokens is null for a transient 429/529 or sandbox-setup failure, so branch on the status/error_category already written into every run.json row (src/coder_eval/reports_experiment.py:138 and :189) and warn for agent/API categories; while there change :418 from if: failure() to if: always() so the run dir survives the tolerated-red case the design says is routine.
  4. Apply the transient-vs-definitive HTTP split the same job already gets right for the Marketplace probe (:233) to the two PyPI probes at /Users/religa/src/coder_eval/.github/workflows/verify-published-action.yml:164 and :191 — today a sustained 403/429/5xx yields a confidently wrong ::error title=Stranded action.yml pin telling the operator to re-publish an already-published, healthy version (an upload PyPI will reject as a duplicate); keep the branch red as the e2e gate demands, but reserve the "stranded pin" title for a definitive 404.
  5. Close the supply-chain and least-privilege gaps opened by the new promote job: /Users/religa/src/coder_eval/.github/workflows/release.yml:349's skip-existing: true removes the only step that incidentally proved PyPI serves this run's artifact, and nothing before the v0 tag move (:462-463) re-asserts identity — compare urls[].digests.sha256 from the version JSON the preflight already fetches against sha256sum dist/*, add permission-contents: write to the unscoped app-token mints (:80, :401), and drop the inherited packages: write from promote (:381); this is also the moment to extract the two untested 70-line inline blocks (verify-published-action.yml:59-129 and :349-415) into .github/scripts/ with fixture tests following the release_notes.py precedent, including slug parity against tests/lint/action_docs.py:165::marketplace_slug — no static analysis runs over workflow YAML today, which is why finding #1 shipped.

Stats: 1 🔴 · 1 🟠 · 6 🟡 · 5 🔵 across 8 axes reviewed.

@akshaylive

Copy link
Copy Markdown
Collaborator

Nice work here — the release/promote redesign and the new verification tier are well thought through, and the review history shows a lot of care already went into hardening the failure modes. I double-checked the two blockers from the posted review directly against the checked-out branch, and both are real and still present:

1. steps.parity.outputs.version doesn't exist — preflight fails on every run

The parity step only emits pin, newest, and lagging (verify-published-action.yml:124-128):

{
  echo "pin=$PIN"
  echo "newest=$VERSION"
  echo "lagging=$LAGGING"
} >> "$GITHUB_OUTPUT"

But two later steps reference steps.parity.outputs.version, which resolves to empty:

  • Line 202: TAG_REF: v${{ steps.parity.outputs.version }}git show "v:action.yml" fails.
  • Line 247: VERSION: ${{ steps.parity.outputs.version }}uv tool install "coder-eval==" fails.

Since e2e depends on needs: preflight, this means preflight is red on every trigger and the paid e2e tier — the whole point of this PR — never runs. Easiest fix is probably swapping in steps.parity.outputs.pin at both sites, since the pin is the actual consumer contract this tier is meant to verify (and using newest instead would break under the legitimate lagging=true state).

2. publish-pypi's if: still has the skipped-green hazard that promote was just fixed to avoid

release.yml:321 still gates publish-pypi on:

if: needs.release.outputs.version != ''

promote's own comment right above it explains why this shape is risky — a lost output on a partial "Re-run failed jobs" resolves to skipped-green rather than failing loudly — and promote was switched to gate on github.ref instead. But publish-pypi is still on the old pattern, and since promote requires needs: [release, publish-pypi], a skipped publish-pypi silently skips promote too. That's the exact recovery scenario this whole PR is built around, so it'd be good to bring publish-pypi in line with promote's approach (or just drop the guard, since release's "Resolve published version" step at line 188 already hard-fails on an empty version, making this condition dead weight either way).

Everything else in the earlier review (rationale-comment drift, the PyPI probe's missing transient-code handling, GHCR image push not being gated behind publish-pypi, etc.) reads as solid non-blocking feedback for a follow-up pass. Happy to help with either fix if useful!

…, CE035

Both blockers from the multi-model review, plus every non-blocking finding that
held up on inspection.

Blockers:

* `steps.parity.outputs.version` does not exist (the step writes pin/newest/
  lagging), so `TAG_REF` expanded to the bare `v`, `git show "v:action.yml"`
  exited 128 under `set -euo pipefail`, and preflight was red on 100% of runs —
  taking the paid e2e tier (`needs: preflight`) with it. Keyed each reader off a
  value that exists: a new `major` output for the Marketplace step, and `pin`
  for the install/smoke step (installing `newest` fails during a legitimate
  lagging state while @v0 consumers are healthy).
* Removed publish-pypi's `if: needs.release.outputs.version != ''`. Dead ("Resolve
  published version" already exits 1 on empty) and dangerous: on a partial re-run
  it resolved to SKIPPED-green, which — since promote needs [release,
  publish-pypi] — also skipped the promotion, for a green run that published no
  wheel and never moved v0. Emptiness is now asserted in-job, as promote does.

Resilience and diagnosis:

* PyPI probes gain the 403/429/5xx-vs-404 split the Marketplace probe already
  performs, so a throttle no longer reports "Stranded action.yml pin" and sends
  the operator to re-publish a healthy version.
* The zero-token gate branches on run.json's error_category: upstream categories
  warn (inconclusive), everything else stays a hard wiring error.
* ERROR/BUILD_FAILED with a non-upstream category now fail rather than being
  tolerated as model flakes.
* JUnit assertion is no longer vacuous (testcase count >= task_results rows), the
  output-wiring check warns on a present-but-wrong value, and the run-dir upload
  is `always()` so the tolerated-red case keeps its evidence.

Supply chain and least privilege:

* skip-existing made a green publish stop proving PyPI serves this run's wheel;
  a new step compares urls[].digests.sha256 against sha256sum dist/*. Mismatch is
  fatal, an unreadable index is a warning (propagation lag must not redden a
  successful publish).
* permission-contents: write on both app-token mints; promote drops the inherited
  packages: write.

Guardrails, so this class cannot ship again:

* CE035 (tests/lint/workflow_outputs.py) resolves every steps./needs. outputs
  reference to a real writer; its negative test is the exact shape of the bug
  above. actionlint models steps.*.outputs as an open string map and does not
  catch it.
* tests/test_verify_published_workflow.py binds the four couplings nothing
  asserted: the workflow_run display-name link to release.yml, Marketplace slug
  parity with the tested marketplace_slug() over a punctuation table, all three
  `# <-- kept in sync` anchor readers, and the inline consumer task YAML loading
  through the real load_task.

Docs: CONTRIBUTING gains a release runbook (job table, recovery flows, the
nightly's annotation taxonomy); the GHCR image's exemption from the promote
ordering and the unpinned agent-runtime install are recorded as accepted risks.
Deferred items (script extraction, score-gate failure direction, CE036/CE037/
CE040) are booked in .claude/harness-candidates.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@uipreliga

Copy link
Copy Markdown
Collaborator Author

Review addressed — 40345b6

Both blockers, every non-blocking finding, all five nits, and the parts of What's Missing that were fixable without a refactor. make check / make lint (175) / full suite (3894 passed) / actionlint clean on both workflows. The PR description has been rewritten to match the branch.

Blockers

1. steps.parity.outputs.version does not exist. Confirmed: the step writes pin / newest / lagging, so TAG_REF expanded to the bare v, git show "v:action.yml" exited 128 under set -euo pipefail, and preflight was red on 100% of runs — taking the paid e2e tier (needs: preflight) with it.

Fixed by keying each reader off a value that exists, rather than adding the missing output:

  • Marketplace step → a new major output. Your reasoning was right, and there is a second reason to prefer the major tag: in the lagging=true state no Release was cut for the newest tag, so the live listing still reflects the major tag's commit anyway.
  • Install/smoke steppin, as you recommended. Installing newest would fail during a legitimate lagging state while @v0 consumers are perfectly healthy.
  • The false "same commit" comment at :200-201 is gone.

2. publish-pypi's dead if:. Removed entirely. Dead as you showed (Resolve published version already exit 1s on empty) and dangerous in the way the promote header condemns. Emptiness is now asserted in-job, mirroring promote's "Validate version shape", so a lost output is red rather than a silent skip that also skips the promotion.

Non-blocking

# Finding Resolution
1 released_version comment contradicts promote's actual gate Rewritten to name what enforces emptiness. Output kept — you were right that it is semantically distinct off main.
2 Oversized inline run: blocks, no coverage Deferred — see below.
3 Second, weaker Marketplace slugger Shell pipeline now matches marketplace_slug() semantics, plus a test binding them over a table. Coder Eval (CI gate) went from coder-eval-(ci-gate) (404s) to coder-eval-ci-gate.
4 skip-existing removed the only proof PyPI serves this run's artifact New step compares urls[].digests.sha256 against sha256sum dist/*. Mismatch is fatal; an unreadable index is a warning, so JSON-API lag cannot redden a publish that succeeded (which would also block the re-run story skip-existing was added for).
5 PyPI probes lack the transient split Applied the same 000/403/429/5xx split the Marketplace probe uses, to both PyPI probes. The hard gate stays red — only the diagnosis changed, so it no longer tells the operator to re-publish a version PyPI already has.
6 Zero-token gate misattributes upstream outages Branches on error_category: agent_api_error / agent_rate_limit / agent_timeout / agent_crash warn as inconclusive; everything else (auth, billing, config, sandbox, or no category) stays a hard error.

Nits

  1. Least privilegepermission-contents: write on both app-token mints; promote declares permissions: contents: read, dropping the inherited packages: write. I did not add permissions: {} to e2e: the workflow level is already contents: read, so the gain is marginal and it is one more thing that cannot be verified before merge.
  2. Unpinned npm install -g @anthropic-ai/claude-code — recorded as an accepted risk in the file header, per your own note that pinning defeats a stated purpose of the nightly. Pinning here while five copies stay unpinned in pr-checks.yml would buy nothing.
  3. workflow_run display-name coupling — bound by a test.
  4. if: failure() on the uploadalways().
  5. Vacuous e2e assertions — JUnit now asserts <testcase> count ≥ task_results rows; the non-success branch warns on a present-but-wrong output instead of falling through silently.

Also fixed from What's Missing

  • GHCR image exempt from the PR's own thesis — documented in the promote header with the reasoning (internal convenience, must be built in the job holding the bumped pyproject).
  • Major-bump error message now enumerates the doc surfaces that hardcode @v0README.md, docs/CI_GATE.md, docs/tutorials/02-ci-pipeline.md (verified: 5 occurrences).
  • FinalStatus collapseERROR / BUILD_FAILED with a non-upstream category now hard-fail instead of being tolerated as model flakes.
  • Pin-anchor three readers — a test executes release.yml's bumping sed and the workflow's extracting sed against the real action.yml and cross-checks both against test_action_version_pin.py's regex.
  • Inline task YAML unvalidated — a test loads the heredoc through the real load_task.
  • No runbookCONTRIBUTING.md § Releasing: job table, four recovery flows, and every annotation the nightly emits with what it means. CLAUDE.md's tree line now names promote and the verification workflow.

New guardrails

  • CE035 — workflow output-key parity (tests/lint/workflow_outputs.py + TestCE035WorkflowOutputParity). Resolves every steps.<id>.outputs.<key> / needs.<job>.outputs.<key> to a real writer; its negative test is the exact shape of blocker chore: Bump astral-sh/setup-uv from 4.2.0 to 8.3.2 #1. Sound boundaries: third-party uses: are skipped (metadata is not on disk) and a body whose writers are not statically readable is skipped rather than guessed at.
  • tests/test_verify_published_workflow.py (8 tests) — the four couplings above.

Both new Python blocks were also exercised over fixture tables before commit: 10 e2e-gate paths (upstream-tolerated, wiring-fatal, harness-error, vacuous JUnit, exit-contract) and 5 digest-check paths (match, foreign wheel, absent filename, unreachable index, empty dist). All behaved as designed.

Deferred, with reasons

Booked in .claude/harness-candidates.md rather than silently dropped:

  • CE040 / script extraction. I agree with the diagnosis — the 70-line block with zero coverage is structurally why the bug survived — but the fix is a refactor of all 423 lines of a workflow that cannot be exercised pre-merge, and CE035 plus the new tests close the specific classes. Better as its own PR.
  • CE036 (ban the skipped-green job gate) and CE037 (if: failure() in a job with a continue-on-error step) — both shapes have now been hand-fixed twice, which is the argument for the rules.
  • Score-gate failure direction. Needs a second paid agent run per nightly; belongs in action-dogfood, which already pays, not in the cron.
  • CE026 REQUIRED_PREREQ_TOKENS extension to the e2e job, now a third copy of the prereq steps.

Still not verifiable pre-merge

Unchanged from the original description: workflow_run and schedule only activate on main, so this PR's checks still do not exercise the new workflow. workflow_dispatch proves tier 1 immediately after merge, free — and CE035 means the failure that made that first dispatch necessary can no longer be the one that greets it.

One new item in the same category: permission-contents: write on create-github-app-token first executes on a real release. contents is the correct GitHub App permission key, but a wrong scope key would 422 at mint time, so it is worth watching on the next release.

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