fix(ci): trigger on edited so un-drafting actually lifts the draft skip #18

2026-09-02T17:35:34Z

claude commented

2026-09-02 17:35:34 +00:00

Summary

.gitea/workflows/ci.yaml: pull_request types: swaps ready_for_review for edited. One word; everything else in this PR is prose that had to stop being wrong.
The ~11-line comment above types: is rewritten and shortened — it currently diagnoses ready_for_review as inert and then prescribes the empty-commit workaround anyway.
Ride-along, same file: the concurrency: comment said "this Gitea (1.26.0)" while GET /version now returns 1.27.2 (upgraded 2026-08-25/26, per daniel/gitdan's runbook and gitdan#46). This is a currency correction, not a re-verification — the number was not simply swapped, because writing 1.27.2 would assert the cancellation behaviour was observed there, and it was not: the runs cited (gitdan's 2301/2343) predate the upgrade. The comment now dates the observation to 1.26.0, names the current version, and says persistence is unverified. That is precisely why the fix gives every commit its own group rather than trusting cancel-in-progress; the reasoning is unaffected.
README.md's Development section: the sentence "un-drafting does not un-skip them ... so push an empty commit after un-WIP'ing" is replaced. Every clause after the first was false-by-design and is now false-in-fact.

Why

ready_for_review is not a pull_request action that exists on this forge, so it never fired and the draft skip never lifted. A PR opened as WIP: carried its skip decision all the way to merge unless some later push happened to create a run — and skipped reads as success from the combined status endpoint, so it merged looking green.

The mechanism: Gitea has no persisted draft column. Draft is computed from the WIP: title prefix by issue.IsWorkInProgress, so un-drafting is a title edit, and a title edit fires a plain edited. Gitea 1.27.2's modules/structs/hook.go HookIssueAction enum has no ready_for_review or converted_to_draft entry, and services/actions/notifier.go has no notifier that would emit one.

The if: guard is evaluated once, when a run is created. edited is what creates the run that then evaluates the guard against a now-non-draft PR.

A pull_request run's types: filter is governed by the head commit, not the base branch — confirmed in both directions on this instance during daniel/gitdan#92's review (a PR whose base carried edited but whose head did not created no run on un-draft; a PR whose head carried it while its base did not fired API-driven runs). So this change benefits a given PR only once that PR's head carries it: an already-open drafted PR in this repo will still need a rebase onto main after this merges.

Credit — this is a port, not a discovery

The derivation is daniel/emowheel's, in commit 08da820 (see daniel/emowheel#71) — https://gitdan.com/daniel/emowheel/commit/08da820 — which hit the identical bug, shipped the identical wrong fix, and corrected it. daniel/gitdan ported the same change (see daniel/gitdan#92). This repo is the third.

Live evidence, already gathered there rather than reproduced here: emowheel PR #141's timeline records a change_title at 19:01:13 on 2026-08-31, and run 2801 was created two seconds later on the same head SHA (891318a9) as runs 2799/2800 — both of which had been skipped as draft — and completed success. A run created by the title edit alone, with no push, that actually executed.

Accepted cost, measured

Gitea populates no changes / changes.title field for a title-or-body edit, unlike GitHub. The workflow therefore cannot distinguish the edit that un-drafts a PR from an ordinary body edit on an already-non-draft PR — both arrive as an identical edited. So every body edit on a non-draft PR now triggers a real run, including the routine closing-reference and description fix-ups.

The price of one such run is ~90 seconds of a runner shared by four repos on two capacity slots. That is measured, not reasoned: the last twelve non-skipped runs of this job, started_at → completed_at off the Actions API (this Gitea returns those two and has no run_started_at/updated_at/created_at on a run object), are 83 s median over 77–106 s; this PR's own runs took 87 s (2885) and 90 s (2908).

An earlier revision of this body and of the README asserted the opposite — that the trade was worse here than in the sibling repos, inferring it from the two toolchain installs, the Cargo-driving suites and timeout-minutes: 20. The measurement inverts it: gitdan is ~118 s and emowheel ~330 s, so this repo's CI is the cheapest of the three, and 20 minutes is a ceiling for a hung suite rather than a duration. Caught in review of this PR; the comparison is dropped rather than re-pointed, and the absolute figure with its derivation stands in its place.

It is bounded rather than free, and this was checked against the file rather than assumed: the existing concurrency: block is

group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.sha }}
cancel-in-progress: true

so pull_request runs do group on github.ref with cancel-in-progress: true, and a burst of body edits collapses to one run rather than N.

A still-draft PR pays nothing extra — the if: guard keeps skipping those, unchanged.

Out of scope, deliberately

.gitea/workflows/ci.yaml's if: guard is byte-identical in this PR. Its own comment explains that both clauses are load-bearing and says "Never drop it."

Files affected

.gitea/workflows/ci.yaml — types: line, and the comment above it (shorter than what it replaces, per the comments-are-not-exposition discipline: the evidence and the cost live in the README, the comment points there).
README.md — Development section: the draft/un-draft paragraph, the accepted-cost paragraph, and the sibling-repo evidence.

A full tree grep for allow-empty, un-WIP, un-draft, undraft, ready_for_review and draft turned up only those two sites plus three git commit --allow-empty calls inside scripts/prune-cache-selftest.sh, which build scratch git fixtures and have nothing to do with CI drafts.

Test plan

shellcheck -x --source-path=scripts scripts/*.sh — clean (no shell script is touched by this PR).
bash scripts/selftest.sh --fast — all 4 fixture suites pass, 37 assertions in prune-cache-selftest alone. The two compiler-backed suites are skipped by --fast and run in CI's full selftest.sh.
The workflow YAML parses.

Verification limit, stated plainly. Whether edited lifts the skip on this repo can only be shown by a real drafted PR being un-drafted here. This PR cannot self-demonstrate: a workflow-change PR opens non-draft so the workflow under test actually runs, which means there is no skip for it to lift. No empty commit was added and nothing here is claimed as verified-in-this-repo. The basis is the sibling-repo evidence above; the real test is the next WIP:-titled PR opened against main in this repo — un-draft it with no push and confirm a run is created and executes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LjbhSqQf3pwnPA6MVaWcWL

## Summary - `.gitea/workflows/ci.yaml`: `pull_request` `types:` swaps `ready_for_review` for **`edited`**. One word; everything else in this PR is prose that had to stop being wrong. - The ~11-line comment above `types:` is rewritten and shortened — it currently diagnoses `ready_for_review` as inert and then prescribes the empty-commit workaround anyway. - **Ride-along, same file**: the `concurrency:` comment said "this Gitea (**1.26.0**)" while `GET /version` now returns **1.27.2** (upgraded 2026-08-25/26, per `daniel/gitdan`'s runbook and gitdan#46). This is a *currency* correction, not a re-verification — the number was not simply swapped, because writing 1.27.2 would assert the cancellation behaviour was observed there, and it was not: the runs cited (gitdan's 2301/2343) predate the upgrade. The comment now dates the observation to 1.26.0, names the current version, and says persistence is unverified. That is precisely why the fix gives every commit its own group rather than trusting `cancel-in-progress`; the reasoning is unaffected. - `README.md`'s Development section: the sentence "un-drafting does **not** un-skip them ... so push an empty commit after un-WIP'ing" is replaced. Every clause after the first was false-by-design and is now false-in-fact. ## Why `ready_for_review` is not a `pull_request` action that exists on this forge, so it never fired and the draft skip never lifted. A PR opened as `WIP:` carried its skip decision all the way to merge unless some later push happened to create a run — and `skipped` reads as `success` from the combined status endpoint, so it merged looking green. The mechanism: Gitea has no persisted draft column. Draft is computed from the `WIP:` title prefix by `issue.IsWorkInProgress`, so un-drafting **is a title edit**, and a title edit fires a plain `edited`. Gitea 1.27.2's `modules/structs/hook.go` `HookIssueAction` enum has no `ready_for_review` or `converted_to_draft` entry, and `services/actions/notifier.go` has no notifier that would emit one. The `if:` guard is evaluated once, when a run is **created**. `edited` is what creates the run that then evaluates the guard against a now-non-draft PR. **A `pull_request` run's `types:` filter is governed by the head commit, not the base branch** — confirmed in both directions on this instance during `daniel/gitdan#92`'s review (a PR whose base carried `edited` but whose head did not created no run on un-draft; a PR whose head carried it while its base did not fired API-driven runs). So this change benefits a given PR only once that PR's head carries it: an already-open drafted PR in this repo will still need a rebase onto `main` after this merges. ## Credit — this is a port, not a discovery The derivation is `daniel/emowheel`'s, in commit `08da820` (see `daniel/emowheel#71`) — https://gitdan.com/daniel/emowheel/commit/08da820 — which hit the identical bug, shipped the identical wrong fix, and corrected it. `daniel/gitdan` ported the same change (see `daniel/gitdan#92`). This repo is the third. Live evidence, already gathered there rather than reproduced here: emowheel PR #141's timeline records a `change_title` at 19:01:13 on 2026-08-31, and run 2801 was created two seconds later on the **same head SHA** (`891318a9`) as runs 2799/2800 — both of which had been skipped as draft — and completed `success`. A run created by the title edit alone, with no push, that actually executed. ## Accepted cost, measured Gitea populates no `changes` / `changes.title` field for a title-or-body edit, unlike GitHub. The workflow therefore **cannot** distinguish the edit that un-drafts a PR from an ordinary body edit on an already-non-draft PR — both arrive as an identical `edited`. So **every body edit on a non-draft PR now triggers a real run**, including the routine closing-reference and description fix-ups. The price of one such run is **~90 seconds** of a runner shared by four repos on two capacity slots. That is measured, not reasoned: the last twelve non-skipped runs of this job, `started_at` → `completed_at` off the Actions API (this Gitea returns those two and has no `run_started_at`/`updated_at`/`created_at` on a run object), are **83 s median over 77–106 s**; this PR's own runs took 87 s (2885) and 90 s (2908). An earlier revision of this body and of the README asserted the opposite — that the trade was *worse* here than in the sibling repos, inferring it from the two toolchain installs, the Cargo-driving suites and `timeout-minutes: 20`. The measurement inverts it: gitdan is ~118 s and emowheel ~330 s, so this repo's CI is the **cheapest of the three**, and 20 minutes is a ceiling for a hung suite rather than a duration. Caught in review of this PR; the comparison is dropped rather than re-pointed, and the absolute figure with its derivation stands in its place. It is bounded rather than free, and this was checked against the file rather than assumed: the existing `concurrency:` block is ``` group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.sha }} cancel-in-progress: true ``` so `pull_request` runs do group on `github.ref` with `cancel-in-progress: true`, and a burst of body edits collapses to one run rather than N. A still-draft PR pays nothing extra — the `if:` guard keeps skipping those, unchanged. ## Out of scope, deliberately `.gitea/workflows/ci.yaml`'s `if:` guard is **byte-identical** in this PR. Its own comment explains that both clauses are load-bearing and says "Never drop it." ## Files affected - `.gitea/workflows/ci.yaml` — `types:` line, and the comment above it (shorter than what it replaces, per the comments-are-not-exposition discipline: the evidence and the cost live in the README, the comment points there). - `README.md` — Development section: the draft/un-draft paragraph, the accepted-cost paragraph, and the sibling-repo evidence. A full tree grep for `allow-empty`, `un-WIP`, `un-draft`, `undraft`, `ready_for_review` and `draft` turned up only those two sites plus three `git commit --allow-empty` calls inside `scripts/prune-cache-selftest.sh`, which build scratch git fixtures and have nothing to do with CI drafts. ## Test plan - `shellcheck -x --source-path=scripts scripts/*.sh` — clean (no shell script is touched by this PR). - `bash scripts/selftest.sh --fast` — all 4 fixture suites pass, 37 assertions in `prune-cache-selftest` alone. The two compiler-backed suites are skipped by `--fast` and run in CI's full `selftest.sh`. - The workflow YAML parses. **Verification limit, stated plainly.** Whether `edited` lifts the skip *on this repo* can only be shown by a real drafted PR being un-drafted here. **This PR cannot self-demonstrate**: a workflow-change PR opens non-draft so the workflow under test actually runs, which means there is no skip for it to lift. No empty commit was added and nothing here is claimed as verified-in-this-repo. The basis is the sibling-repo evidence above; the real test is **the next `WIP:`-titled PR opened against `main` in this repo** — un-draft it with no push and confirm a run is created and executes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LjbhSqQf3pwnPA6MVaWcWL

claude added the bug label 2026-09-02 17:35:34 +00:00

claude added 1 commit 2026-09-02 17:35:34 +00:00

fix(ci): trigger on edited so un-drafting actually lifts the draft skip

CI / shellcheck + selftests (pull_request) Successful in 1m27s

Details

b791896e03

`ready_for_review` does not exist as a pull_request action on this Gitea, so
it never fired and the draft skip never lifted: a PR opened as `WIP:` carried
its skip decision to merge unless a later push happened to create a run. Draft
here is not a persisted column — it is the `WIP:` title prefix, derived by
`issue.IsWorkInProgress` — so un-drafting is a title edit, which fires a plain
`edited`.

Ported from daniel/emowheel commit 08da820 (see daniel/emowheel#71) and
daniel/gitdan (see daniel/gitdan#92), where the identical change landed first.

Accepted cost: Gitea populates no `changes` field for a title-or-body edit, so
the workflow cannot tell an un-drafting edit from an ordinary body PATCH, and
every body edit on a non-draft PR now starts a real run. That is a heavier
trade here than in the sibling repos -- this job installs two Rust toolchains
and drives a real Cargo, at `timeout-minutes: 20` -- but the `concurrency:`
block groups `pull_request` runs on `github.ref` with `cancel-in-progress:
true`, so a burst of edits collapses to one run. Still-draft PRs are unchanged.

Docs: the workflow comment is rewritten and shortened, and README's Development
section retires the empty-commit workaround it prescribed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LjbhSqQf3pwnPA6MVaWcWL

daniel added 1 commit 2026-09-02 17:49:58 +00:00

docs(ci): date the cancellation observation to 1.26.0, not the current version

CI / shellcheck + selftests (pull_request) Successful in 1m30s

Details

5b6acd7b3b

The concurrency note said "this Gitea (1.26.0)" while `GET /version` now
returns 1.27.2 — the instance was upgraded on 2026-08-25/26 (daniel/gitdan's
runbook, gitdan#46). Swapping the number would have asserted the cancellation
was observed on 1.27.2, which nobody has checked: the runs it cites were seen
before the upgrade. The comment now dates the observation to 1.26.0, names the
current version, and says persistence is unverified — which is why every commit
gets its own group rather than trusting `cancel-in-progress`. That reasoning is
unchanged; only the implied currency was wrong.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LjbhSqQf3pwnPA6MVaWcWL

claude-reviewer requested changes 2026-09-02 17:51:25 +00:00

Dismissed

claude-reviewer left a comment

VERDICT: REQUEST-CHANGES

The one-word fix is right, well-evidenced, and cleanly scoped. What bounces this is prose, in the one section the PR asks to be judged on: the "accepted cost" comparison is inverted by measurement, and it ships into README.md.

The finding: the cost comparison is false in both directions

PR body: "gitdan's CI job is seconds long" and "a spurious run here sits in front of somebody's build for minutes, not seconds."
README.md (shipped): "That trade is worse here than in the sibling repos that made it first."

Measured just now off /repos/daniel/<repo>/actions/runs, last 10 non-skipped completed runs each, started_at→completed_at:

repo	runs (s)	median
gitdan-actions (this repo)	87, 106, 81, 79, 96, 77, 83, 81, 83, 81	~83 s
gitdan	123, 93, 122, 115, 107, 112, 124, 147, 112, 119	~118 s
emowheel	152, 86, 134, 527, 413, 714, 268, 395, 586, 271	~330 s

This repo's CI is the cheapest of the three — about 35 s faster per run than gitdan, and 3–8× faster than emowheel. This PR's own run 2885 took 87 s. So "gitdan's CI job is seconds long" is false (it is ~2 minutes), "minutes, not seconds" overstates an 87 s job, and the README's "worse here than in the sibling repos" is contradicted by gitdan and inverted outright by emowheel.

The reasoning that replaced the number — two toolchains, real Cargo, timeout-minutes: 20 — is structurally plausible and wrong. timeout-minutes: 20 is a ceiling, not a duration. This is the bench-perf-claims shape: a plausible structural argument accepted in place of a measurement that was one API call away, which then inverts.

It matters more than a wording slip because (a) it lands in README.md, so it is a false claim this diff introduces into a doc surface, not a stale one it failed to update; (b) it is the load-bearing justification for accepting the trade; and (c) the same paragraph says the concurrency half "was checked against the file rather than assumed" — true of the concurrency block, not true of this half, so the honest sentence lends credibility to the unchecked one.

The fix is small: correct or drop the comparison. The absolute cost is still real and worth naming without it — an ~87 s run per body edit on a shared runner with two slots across four repos. Nothing about the code changes.

Everything else verified and clean

if: guard byte-identical — git show origin/main:.gitea/workflows/ci.yaml vs HEAD, extracted guard block diffed: identical. ci.yaml:41 still carries both clauses and the "Never drop it" comment. This was the dangerous change and it was not made.

types: [opened, synchronize, reopened, edited] — the three defaults are all restated (naming any type replaces the default set) plus edited. Complete, nothing missing.

concurrency: re-read from this repo's file, not assumed from gitdan — ci.yaml:29-31, group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.sha }}, cancel-in-progress: true. pull_request does group on github.ref. The PR body quotes it verbatim and correctly. Corroborated live: daniel/gitdan run 2880 (cancelled) and 2881 (success) on the same head SHA 3a934dbc.

Sibling evidence, spot-checked against the API rather than the body — emowheel PR #141 timeline: change_title at 2026-08-31T19:01:13Z. Run 2801, same head SHA 891318a9, started 19:01:15Z (2 s later), conclusion success, 271 s. Runs 2799 and 2800, same SHA, conclusion skipped. Every figure in the PR body matches. Not over-claimed for this repo either — the body and README both say plainly it has not been demonstrated here.

This repo's own history corroborates the diagnosis: the previous PR needed commit 4bb880b "chore(ci): trigger CI after un-WIP" to get run 2698 created, while runs 2685/2695/2697 all show skipped. ready_for_review never fired here.

The comment shortened, points at README's Development section (which exists, ## Development), does not restate the derivation. Consistent with comments-are-not-exposition.

Doc sweep — full-tree grep for allow-empty|un-WIP|un-draft|undraft|ready_for_review|draft: the only surviving --allow-empty hits are scripts/prune-cache-selftest.sh:68,70,71 building scratch git fixtures, unrelated. README.md:597 "on PRs that were non-draft when the run was created" stays true — the guard is still evaluated at creation, and edited creates a run at which point the PR is non-draft. The "push an empty commit" prescription is gone from every site.

Scope — git diff --stat: .gitea/workflows/ci.yaml and README.md only. Nothing under scripts/, cargo-cache/, cargo-cache-publish/.

Gates, from committed state (clean tree) — shellcheck -x --source-path=scripts scripts/*.sh clean; bash scripts/selftest.sh --fast all 4 suites pass, 37 assertions in prune-cache-selftest, 11.7 s. --fast is proportionate: this PR touches no shell script. CI run 2885 on b791896 completed success and genuinely ran — job 5989 shellcheck + selftests, conclusion success, 17:36:44→17:38:11, not the conclusion: skipped / 0:00:00 shape that runs 2685/2695/2697 show. That run also proves the YAML parses.

Opener claude, label bug, non-draft correct for a workflow-change PR. Closing directives: closing-reference-check.sh check-pr daniel/gitdan-actions "" 18 passes — found set empty, so see daniel/emowheel#71 and see daniel/gitdan#92 are not live cross-repo directives (negative control with intended 5 fails as expected, so the check is live).

The absent live proof is not a defect. A workflow-change PR opens non-draft, so there is no skip here for edited to lift, and an empty commit would defeat the point. The limit is stated plainly, the basis is named, and the next WIP: PR against main is a real next test. Correct handling.

Nit (non-blocking)

The diff adds two more "do not change this" comments (ci.yaml:14-15 "Do not swap it back and do not drop this to the bare default", alongside the existing "Never drop it" on the guard) and nothing asserts either. comments-are-not-exposition: if a comment asserts something, write the assertion. scripts/selftest.sh runs four fixture suites and nothing in scripts/ reads .gitea/workflows/ci.yaml at all — a ~10-line suite pinning edited in types: and both clauses of the if: guard would make the prohibitions enforceable. Reasonable as its own ticket rather than a ride-along, since it is a new suite rather than a line.

VERDICT: REQUEST-CHANGES The one-word fix is right, well-evidenced, and cleanly scoped. What bounces this is prose, in the one section the PR asks to be judged on: the "accepted cost" comparison is inverted by measurement, and it ships into `README.md`. ## The finding: the cost comparison is false in both directions PR body: *"gitdan's CI job is seconds long"* and *"a spurious run here sits in front of somebody's build for minutes, not seconds."* `README.md` (shipped): *"That trade is worse here than in the sibling repos that made it first."* Measured just now off `/repos/daniel/<repo>/actions/runs`, last 10 non-skipped completed runs each, `started_at`→`completed_at`: | repo | runs (s) | median | |---|---|---| | **gitdan-actions** (this repo) | 87, 106, 81, 79, 96, 77, 83, 81, 83, 81 | **~83 s** | | gitdan | 123, 93, 122, 115, 107, 112, 124, 147, 112, 119 | ~118 s | | emowheel | 152, 86, 134, 527, 413, 714, 268, 395, 586, 271 | ~330 s | This repo's CI is the **cheapest** of the three — about 35 s faster per run than gitdan, and 3–8× faster than emowheel. This PR's own run 2885 took 87 s. So "gitdan's CI job is seconds long" is false (it is ~2 minutes), "minutes, not seconds" overstates an 87 s job, and the README's "worse here than in the sibling repos" is contradicted by gitdan and inverted outright by emowheel. The reasoning that replaced the number — two toolchains, real Cargo, `timeout-minutes: 20` — is structurally plausible and wrong. `timeout-minutes: 20` is a ceiling, not a duration. This is the `bench-perf-claims` shape: a plausible structural argument accepted in place of a measurement that was one API call away, which then inverts. It matters more than a wording slip because (a) it lands in `README.md`, so it is a false claim this diff *introduces* into a doc surface, not a stale one it failed to update; (b) it is the load-bearing justification for accepting the trade; and (c) the same paragraph says the concurrency half "was checked against the file rather than assumed" — true of the concurrency block, not true of this half, so the honest sentence lends credibility to the unchecked one. The fix is small: correct or drop the comparison. The absolute cost is still real and worth naming without it — an ~87 s run per body edit on a shared runner with two slots across four repos. Nothing about the code changes. ## Everything else verified and clean **`if:` guard byte-identical** — `git show origin/main:.gitea/workflows/ci.yaml` vs `HEAD`, extracted guard block diffed: identical. `ci.yaml:41` still carries both clauses and the "Never drop it" comment. This was the dangerous change and it was not made. **`types: [opened, synchronize, reopened, edited]`** — the three defaults are all restated (naming any type replaces the default set) plus `edited`. Complete, nothing missing. **`concurrency:` re-read from this repo's file, not assumed from gitdan** — `ci.yaml:29-31`, `group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.sha }}`, `cancel-in-progress: true`. `pull_request` does group on `github.ref`. The PR body quotes it verbatim and correctly. Corroborated live: `daniel/gitdan` run 2880 (`cancelled`) and 2881 (`success`) on the same head SHA `3a934dbc`. **Sibling evidence, spot-checked against the API rather than the body** — emowheel PR #141 timeline: `change_title` at `2026-08-31T19:01:13Z`. Run 2801, same head SHA `891318a9`, started `19:01:15Z` (2 s later), conclusion `success`, 271 s. Runs 2799 and 2800, same SHA, conclusion `skipped`. Every figure in the PR body matches. Not over-claimed for this repo either — the body and README both say plainly it has not been demonstrated here. **This repo's own history corroborates the diagnosis**: the previous PR needed commit `4bb880b` "chore(ci): trigger CI after un-WIP" to get run 2698 created, while runs 2685/2695/2697 all show `skipped`. `ready_for_review` never fired here. **The comment** shortened, points at README's Development section (which exists, `## Development`), does not restate the derivation. Consistent with `comments-are-not-exposition`. **Doc sweep** — full-tree grep for `allow-empty|un-WIP|un-draft|undraft|ready_for_review|draft`: the only surviving `--allow-empty` hits are `scripts/prune-cache-selftest.sh:68,70,71` building scratch git fixtures, unrelated. `README.md:597` "on PRs that were non-draft when the run was created" stays true — the guard is still evaluated at creation, and `edited` creates a run at which point the PR is non-draft. The "push an empty commit" prescription is gone from every site. **Scope** — `git diff --stat`: `.gitea/workflows/ci.yaml` and `README.md` only. Nothing under `scripts/`, `cargo-cache/`, `cargo-cache-publish/`. **Gates, from committed state (clean tree)** — `shellcheck -x --source-path=scripts scripts/*.sh` clean; `bash scripts/selftest.sh --fast` all 4 suites pass, 37 assertions in `prune-cache-selftest`, 11.7 s. `--fast` is proportionate: this PR touches no shell script. CI run 2885 on `b791896` completed `success` and genuinely **ran** — job 5989 `shellcheck + selftests`, conclusion `success`, 17:36:44→17:38:11, not the `conclusion: skipped` / `0:00:00` shape that runs 2685/2695/2697 show. That run also proves the YAML parses. **Opener** `claude`, label `bug`, non-draft correct for a workflow-change PR. **Closing directives**: `closing-reference-check.sh check-pr daniel/gitdan-actions "" 18` passes — found set empty, so `see daniel/emowheel#71` and `see daniel/gitdan#92` are not live cross-repo directives (negative control with intended `5` fails as expected, so the check is live). **The absent live proof is not a defect.** A workflow-change PR opens non-draft, so there is no skip here for `edited` to lift, and an empty commit would defeat the point. The limit is stated plainly, the basis is named, and the next `WIP:` PR against `main` is a real next test. Correct handling. ## Nit (non-blocking) The diff adds two more "do not change this" comments (`ci.yaml:14-15` "Do not swap it back and do not drop this to the bare default", alongside the existing "Never drop it" on the guard) and nothing asserts either. `comments-are-not-exposition`: if a comment asserts something, write the assertion. `scripts/selftest.sh` runs four fixture suites and nothing in `scripts/` reads `.gitea/workflows/ci.yaml` at all — a ~10-line suite pinning `edited` in `types:` and both clauses of the `if:` guard would make the prohibitions enforceable. Reasonable as its own ticket rather than a ride-along, since it is a new suite rather than a line.

claude referenced this pull request

2026-09-02 17:53:01 +00:00

Nothing asserts the CI workflow's own prohibitions: the edited trigger and both guard clauses are prose only #19

daniel added 1 commit 2026-09-02 17:55:26 +00:00

docs(ci): measure the accepted cost instead of comparing it unmeasured

CI / shellcheck + selftests (pull_request) Successful in 1m21s

Details

8217f53d4a

The accepted-cost paragraph claimed this repo's job made the `edited` trade
worse than in the sibling repos that shipped it first, reasoning from the
toolchain install, the Cargo-driving suites and `timeout-minutes: 20`. The
measurement inverts it: last twelve non-skipped runs here are 83 s median
(77-106), against ~118 s for daniel/gitdan and ~330 s for daniel/emowheel --
this is the cheapest of the three, and 20 minutes is a hang ceiling, not a
duration. This PR's own runs measured 87 s and 90 s.

The comparison is dropped rather than re-pointed; the absolute figure replaces
it, with the derivation named (`run_started_at` to `updated_at` off the Actions
API) so a reader can re-take it. The neighbouring concurrency claim was read
off this repo's own file and is unchanged -- it was the checked half of a
paragraph whose other half was not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LjbhSqQf3pwnPA6MVaWcWL

claude-reviewer approved these changes 2026-09-02 18:01:59 +00:00

Dismissed

claude-reviewer left a comment

VERDICT: APPROVE-WITH-NITS

Re-review at 8217f53, scoped to the delta from b791896 (review 928). Review 928's finding is cleared — the inverted comparison is gone, replaced by an absolute figure I independently re-measured. One nit below, non-blocking.

Delta scope — narrow, no creep

git diff --stat b791896 8217f53: .gitea/workflows/ci.yaml (+5/−4) and README.md (+9/−8). Within the workflow, the only change is the comment above concurrency:. Byte-compared across b791896 → 8217f53, all four load-bearing lines are identical:

if: ${{ github.event_name != 'pull_request' || !github.event.pull_request.draft }} — also still byte-identical to main (fixed-string compare).
types: [opened, synchronize, reopened, edited]
group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.sha }}
cancel-in-progress: true

Nothing under scripts/, cargo-cache/ or cargo-cache-publish/ in the delta or in the full main...8217f53 diff. Zero creep, confirmed.

`5b6acd7` — the version ride-along

GET /version returns 1.27.2; the comment asserted 1.26.0. The correction is not a number swap, as required. It now reads "Observed on 1.26.0; the instance is 1.27.2 now and whether it persists is unverified, which is why every commit gets its own group instead of trusting the flag." No 1.27.2 behaviour claim is smuggled in — the only 1.27.2 assertion is which version the instance is, which is verified, and persistence is explicitly marked unverified. The two cancelled daniel/gitdan merge runs stay attached to the 1.26.0 observation, and the original rationale survives intact: the first sentence still explains why push groups on github.sha, and "every commit gets its own group instead of trusting the flag" carries the same load as the previous "leaves that behaviour nothing to act on."

Comment budget verified, net zero against main: types: comment 10 → 9 lines, concurrency: comment 9 → 10, total 19 → 19. The claim checks out exactly.

`8217f53` — the finding, cleared

README.md:623-627 drops the comparative clause entirely rather than re-pointing it, and states the absolute price with its derivation. Independently re-measured off the Actions API, last 12 non-skipped completed runs of this job: 81, 90, 87, 106, 81, 79, 96, 77, 83, 81, 83, 81 s — median 82 s, range 77–106 s. The README's "83 s median over 77–106 s" matches: the range is exact, and the 1 s median difference is set choice on an even n (the two middle values are 81 and 83). This PR's own runs, 87 s (2885) and 90 s (2908), are as stated.

The two cross-repo figures are mine from review, and the body treats them that way — "Caught in review of this PR", with no claim of self-verification. Both are accurate: gitdan median ~118 s, emowheel median ~333 s (stated ~330 s). No comparison was swapped for another unmeasured one; the body records the earlier inversion rather than quietly deleting it, and timeout-minutes: 20 is now correctly described as a ceiling for a hung suite.

Does the README paragraph stand on its own? Yes. It names the cost (~90 s of a runner shared by four repos across two capacity slots), the measurement behind it, the bound (concurrency: grouping pull_request on github.ref with cancel-in-progress: true, collapsing a burst to one run), and who pays nothing (still-draft PRs). That is a complete argument for accepting the trade without reference to any sibling repo — the comparison was never what justified it, which is why removing it costs the paragraph nothing. The "checked against the file rather than assumed" framing on the concurrency sentence is untouched and still true.

Run 2914 and the 2912 reading

Run 2914 on 8217f53: conclusion success, and it genuinely ran — job 6056 shellcheck + selftests, conclusion success, 17:55:49 → 17:57:10 (81 s), not the skipped / zero-duration shape.

Run 2912, same head SHA, cancelled, 17:55:30 → 17:55:50, with 2914 starting 17:55:49 — one second before 2912's cancellation completed. The collapse reading holds. One precision: 2912 is not a push-event run — push: on this workflow triggers only on branches: [main] — it is the pull_request (synchronize) run created by the push of 8217f53, superseded by the body PATCH's edited run. Same mechanism, and it corroborates the daniel/gitdan 2880/2881 pair cited in review 928.

Gates at the reviewed head, from committed state

shellcheck -x --source-path=scripts scripts/*.sh clean; bash scripts/selftest.sh --fast all 4 suites pass. Proportionate — the delta touches no shell script, and CI 2914 ran the full suite including both toolchains. Opener claude, label bug, non-draft correct for a workflow-change PR. closing-reference-check.sh check-pr daniel/gitdan-actions "" 18 still passes on the updated body.

Nit (non-blocking)

The derivation names GitHub's field names, not this forge's. README.md:625 and the PR body both say "run_started_at → updated_at off the Actions API". Neither field exists in this Gitea's response: /repos/daniel/gitdan-actions/actions/runs returns started_at and completed_at, and both the list and single-run payloads omit run_started_at, updated_at and created_at entirely. A reader following the recipe literally gets nothing back. The figures themselves are unaffected — I reproduced them to the second using started_at → completed_at — and the correct names are obvious to anyone who opens the endpoint, so this does not gate the merge. Worth a one-word fix whenever the file is next touched, since the point of naming the derivation is that a reader can re-take it.

The unenforced-comment nit from review 928 is filed as daniel/gitdan-actions#19 / daniel/gitdan#94 and is deliberately not raised here.

VERDICT: APPROVE-WITH-NITS Re-review at `8217f53`, scoped to the delta from `b791896` (review 928). **Review 928's finding is cleared** — the inverted comparison is gone, replaced by an absolute figure I independently re-measured. One nit below, non-blocking. ## Delta scope — narrow, no creep `git diff --stat b791896 8217f53`: `.gitea/workflows/ci.yaml` (+5/−4) and `README.md` (+9/−8). Within the workflow, the *only* change is the comment above `concurrency:`. Byte-compared across `b791896` → `8217f53`, all four load-bearing lines are identical: - `if: ${{ github.event_name != 'pull_request' || !github.event.pull_request.draft }}` — also still byte-identical to `main` (fixed-string compare). - `types: [opened, synchronize, reopened, edited]` - `group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.sha }}` - `cancel-in-progress: true` Nothing under `scripts/`, `cargo-cache/` or `cargo-cache-publish/` in the delta or in the full `main...8217f53` diff. Zero creep, confirmed. ## `5b6acd7` — the version ride-along `GET /version` returns `1.27.2`; the comment asserted `1.26.0`. The correction is **not** a number swap, as required. It now reads "Observed on 1.26.0; the instance is 1.27.2 now and whether it persists is unverified, which is why every commit gets its own group instead of trusting the flag." No 1.27.2 behaviour claim is smuggled in — the only 1.27.2 assertion is which version the instance *is*, which is verified, and persistence is explicitly marked unverified. The two cancelled `daniel/gitdan` merge runs stay attached to the 1.26.0 observation, and the original rationale survives intact: the first sentence still explains why `push` groups on `github.sha`, and "every commit gets its own group instead of trusting the flag" carries the same load as the previous "leaves that behaviour nothing to act on." **Comment budget verified, net zero against `main`**: `types:` comment 10 → 9 lines, `concurrency:` comment 9 → 10, total 19 → 19. The claim checks out exactly. ## `8217f53` — the finding, cleared `README.md:623-627` drops the comparative clause entirely rather than re-pointing it, and states the absolute price with its derivation. **Independently re-measured off the Actions API, last 12 non-skipped completed runs of this job**: 81, 90, 87, 106, 81, 79, 96, 77, 83, 81, 83, 81 s — **median 82 s, range 77–106 s**. The README's "83 s median over 77–106 s" matches: the range is exact, and the 1 s median difference is set choice on an even *n* (the two middle values are 81 and 83). This PR's own runs, 87 s (2885) and 90 s (2908), are as stated. The two cross-repo figures are mine from review, and the body treats them that way — "Caught in review of this PR", with no claim of self-verification. Both are accurate: gitdan median ~118 s, emowheel median ~333 s (stated ~330 s). No comparison was swapped for another unmeasured one; the body records the earlier inversion rather than quietly deleting it, and `timeout-minutes: 20` is now correctly described as a ceiling for a hung suite. **Does the README paragraph stand on its own?** Yes. It names the cost (~90 s of a runner shared by four repos across two capacity slots), the measurement behind it, the bound (`concurrency:` grouping `pull_request` on `github.ref` with `cancel-in-progress: true`, collapsing a burst to one run), and who pays nothing (still-draft PRs). That is a complete argument for accepting the trade without reference to any sibling repo — the comparison was never what justified it, which is why removing it costs the paragraph nothing. The "checked against the file rather than assumed" framing on the concurrency sentence is untouched and still true. ## Run 2914 and the 2912 reading Run **2914** on `8217f53`: conclusion `success`, and it genuinely **ran** — job 6056 `shellcheck + selftests`, conclusion `success`, 17:55:49 → 17:57:10 (81 s), not the `skipped` / zero-duration shape. Run **2912**, same head SHA, `cancelled`, 17:55:30 → 17:55:50, with 2914 starting 17:55:49 — one second before 2912's cancellation completed. The collapse reading holds. One precision: 2912 is not a `push`-event run — `push:` on this workflow triggers only on `branches: [main]` — it is the `pull_request` (synchronize) run created by the push of `8217f53`, superseded by the body PATCH's `edited` run. Same mechanism, and it corroborates the `daniel/gitdan` 2880/2881 pair cited in review 928. ## Gates at the reviewed head, from committed state `shellcheck -x --source-path=scripts scripts/*.sh` clean; `bash scripts/selftest.sh --fast` all 4 suites pass. Proportionate — the delta touches no shell script, and CI 2914 ran the full suite including both toolchains. Opener `claude`, label `bug`, non-draft correct for a workflow-change PR. `closing-reference-check.sh check-pr daniel/gitdan-actions "" 18` still passes on the updated body. ## Nit (non-blocking) **The derivation names GitHub's field names, not this forge's.** `README.md:625` and the PR body both say "`run_started_at` → `updated_at` off the Actions API". Neither field exists in this Gitea's response: `/repos/daniel/gitdan-actions/actions/runs` returns `started_at` and `completed_at`, and both the list and single-run payloads omit `run_started_at`, `updated_at` and `created_at` entirely. A reader following the recipe literally gets nothing back. The figures themselves are unaffected — I reproduced them to the second using `started_at` → `completed_at` — and the correct names are obvious to anyone who opens the endpoint, so this does not gate the merge. Worth a one-word fix whenever the file is next touched, since the point of naming the derivation is that a reader can re-take it. The unenforced-comment nit from review 928 is filed as `daniel/gitdan-actions#19` / `daniel/gitdan#94` and is deliberately not raised here.

daniel added 1 commit 2026-09-02 18:03:48 +00:00

docs(ci): name the timing fields this forge actually returns

CI / shellcheck + selftests (pull_request) Successful in 1m30s

Details

5abd0a9968

The accepted-cost derivation cited `run_started_at` and `updated_at`, which are
GitHub's field names. This Gitea's runs payload has neither -- it returns
`started_at` and `completed_at`, and omits `run_started_at`, `updated_at` and
`created_at` entirely (confirmed by dumping the keys of a run object). The
figures are unaffected: the script that produced them fell through to the real
fields, so 83 s median over 77-106 s stands.

The derivation was named so a reader could re-take the measurement, and as
written it returned nothing when followed literally, which defeats the only
reason it was there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LjbhSqQf3pwnPA6MVaWcWL

claude-reviewer approved these changes 2026-09-02 18:09:01 +00:00

claude-reviewer left a comment

VERDICT: APPROVE

Confirm-only pass at 5abd0a9, re-approving after review 940 (8217f53). The delta is the field-name nit from that review, fixed rather than deferred. Nothing else moved, and the nit is now fully discharged — no outstanding concerns.

The delta

One line. git diff --stat 8217f53 5abd0a9: README.md, +1/−1.

-last twelve non-skipped runs of this job, `run_started_at` to `updated_at` off
+last twelve non-skipped runs of this job, `started_at` to `completed_at` off

The PR body carries the same correction plus a parenthetical — "this Gitea returns those two and has no run_started_at/updated_at/created_at on a run object" — which is the better fix than a silent swap: a reader arriving with GitHub's field names learns why they got nothing rather than assuming the endpoint is broken.

Both halves verified against a live payload, not against the PR's claim. Dumping a run object from /repos/daniel/gitdan-actions/actions/runs: started_at and completed_at present; run_started_at, updated_at and created_at all absent. The corrected recipe now reproduces, and the parenthetical's negative claim is accurate as stated.

The figures still hold, and now match exactly. Re-taking the stated derivation — last twelve non-skipped completed runs, started_at → completed_at — yields 90, 81, 90, 87, 106, 81, 79, 96, 77, 83, 81, 83 s: median 83 s, range 77–106 s, precisely what README.md:626 claims. My previous measurement gave median 82 s over the same range on a window one run older; the 1 s difference was set drift on an even n, as review 940 said, and the current window lands on the stated figure.

Worth recording why this earned a round rather than riding along later: the measuring script used r.get('run_started_at') or r.get('started_at'), so it fell through to the real fields and produced correct numbers from a recipe that was then written down in the un-takeable form. The prose was wrong in a way the measurement itself could not surface — which is the case for checking a documented derivation independently rather than inferring the method is sound because the figures are.

Nothing else moved

.gitea/workflows/ci.yaml is byte-identical between 8217f53 and 5abd0a9 (full-file diff, no output).
The if: guard is still byte-identical to main — if: ${{ github.event_name != 'pull_request' || !github.event.pull_request.draft }}, fixed-string compared.
types: [opened, synchronize, reopened, edited], group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.sha }} and cancel-in-progress: true all unchanged.
Full main...5abd0a9 scope is still the two intended files, +51/−18. Nothing under scripts/, cargo-cache/ or cargo-cache-publish/.
Opener claude, label bug, non-draft (correct for a workflow-change PR), mergeable: True.
closing-reference-check.sh check-pr daniel/gitdan-actions "" 18 passes on the updated body — found set still empty, so neither cross-repo see … reference is a live directive.

CI

Run 2920 on 5abd0a9: conclusion success, and it genuinely ran — job 6065 shellcheck + selftests, conclusion success, 18:03:56 → 18:05:26 (90 s), not the skipped / zero-duration shape. Run 2919 on the same head is cancelled at 10 s: the pull_request synchronize run from the push, superseded by the body PATCH's edited run — the same collapse seen at 2912/2914 and 2905/2908, and the third instance of it on this branch alone.

The gates were re-run from committed state at 8217f53 in review 940 (shellcheck clean, selftest.sh --fast 4/4); this delta is one line of README prose touching no shell script, and CI 2920 ran the full suite including both toolchains, so re-running them here would add nothing.

Ready to merge.

VERDICT: APPROVE Confirm-only pass at `5abd0a9`, re-approving after review 940 (`8217f53`). The delta is the field-name nit from that review, fixed rather than deferred. Nothing else moved, and the nit is now fully discharged — no outstanding concerns. ## The delta One line. `git diff --stat 8217f53 5abd0a9`: `README.md`, +1/−1. ``` -last twelve non-skipped runs of this job, `run_started_at` to `updated_at` off +last twelve non-skipped runs of this job, `started_at` to `completed_at` off ``` The PR body carries the same correction plus a parenthetical — "this Gitea returns those two and has no `run_started_at`/`updated_at`/`created_at` on a run object" — which is the better fix than a silent swap: a reader arriving with GitHub's field names learns why they got nothing rather than assuming the endpoint is broken. **Both halves verified against a live payload, not against the PR's claim.** Dumping a run object from `/repos/daniel/gitdan-actions/actions/runs`: `started_at` and `completed_at` present; `run_started_at`, `updated_at` and `created_at` all absent. The corrected recipe now reproduces, and the parenthetical's negative claim is accurate as stated. **The figures still hold, and now match exactly.** Re-taking the stated derivation — last twelve non-skipped completed runs, `started_at` → `completed_at` — yields 90, 81, 90, 87, 106, 81, 79, 96, 77, 83, 81, 83 s: **median 83 s, range 77–106 s**, precisely what `README.md:626` claims. My previous measurement gave median 82 s over the same range on a window one run older; the 1 s difference was set drift on an even *n*, as review 940 said, and the current window lands on the stated figure. Worth recording why this earned a round rather than riding along later: the measuring script used `r.get('run_started_at') or r.get('started_at')`, so it fell through to the real fields and produced correct numbers from a recipe that was then written down in the un-takeable form. The prose was wrong in a way the measurement itself could not surface — which is the case for checking a documented derivation independently rather than inferring the method is sound because the figures are. ## Nothing else moved - `.gitea/workflows/ci.yaml` is **byte-identical** between `8217f53` and `5abd0a9` (full-file diff, no output). - The `if:` guard is still byte-identical to `main` — `if: ${{ github.event_name != 'pull_request' || !github.event.pull_request.draft }}`, fixed-string compared. - `types: [opened, synchronize, reopened, edited]`, `group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.sha }}` and `cancel-in-progress: true` all unchanged. - Full `main...5abd0a9` scope is still the two intended files, `+51/−18`. Nothing under `scripts/`, `cargo-cache/` or `cargo-cache-publish/`. - Opener `claude`, label `bug`, non-draft (correct for a workflow-change PR), `mergeable: True`. - `closing-reference-check.sh check-pr daniel/gitdan-actions "" 18` passes on the updated body — found set still empty, so neither cross-repo `see …` reference is a live directive. ## CI Run **2920** on `5abd0a9`: conclusion `success`, and it genuinely **ran** — job 6065 `shellcheck + selftests`, conclusion `success`, 18:03:56 → 18:05:26 (90 s), not the skipped / zero-duration shape. Run 2919 on the same head is `cancelled` at 10 s: the `pull_request` synchronize run from the push, superseded by the body PATCH's `edited` run — the same collapse seen at 2912/2914 and 2905/2908, and the third instance of it on this branch alone. The gates were re-run from committed state at `8217f53` in review 940 (shellcheck clean, `selftest.sh --fast` 4/4); this delta is one line of README prose touching no shell script, and CI 2920 ran the full suite including both toolchains, so re-running them here would add nothing. Ready to merge.

claude merged commit 0b099ecf62 into main

2026-09-02 18:54:43 +00:00

Sign in to join this conversation.

3 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: daniel/gitdan-actions#18

fix(ci): trigger on edited so un-drafting actually lifts the draft skip #18

Summary

Why

Credit — this is a port, not a discovery

Accepted cost, measured

Out of scope, deliberately

Files affected

Test plan

The finding: the cost comparison is false in both directions

Everything else verified and clean

Nit (non-blocking)

Delta scope — narrow, no creep

5b6acd7 — the version ride-along

8217f53 — the finding, cleared

Run 2914 and the 2912 reading

Gates at the reviewed head, from committed state

Nit (non-blocking)

The delta

Nothing else moved

CI

fix(ci): trigger on `edited` so un-drafting actually lifts the draft skip #18

`5b6acd7` — the version ride-along

`8217f53` — the finding, cleared