Skip to content

Commit df55703

Browse files
authored
Merge pull request #451 from hookdeck/docs/acceptance-concurrency-cancels
docs: acceptance runs can be silently cancelled; release notes state facts
2 parents 069f85a + 3be7bb1 commit df55703

4 files changed

Lines changed: 87 additions & 20 deletions

File tree

‎.agents/skills/hookdeck-cli-release/SKILL.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ Follow **in order**. Treat items with **gate** as blocking unless the maintainer
3030
- [ ] **Release notes:** Draft complete (see **Drafting release notes** and [references/release-notes-template.md](references/release-notes-template.md)); includes **Full Changelog** compare link; **contributor shout-outs only when warranted** (see that section).
3131
- [ ] **CI gate:** Latest commit on the **target branch** has **green** GitHub checks (mandatory for GA on `main`; required for betas on the branch being tagged).
3232
- [ ] **Approval:** Maintainer signed off on tag name, notes, and branch — no unilateral surprise tags.
33+
- [ ] **Acceptance group is idle (gate):** no acceptance run in progress or queued — `gh run list --workflow=test-acceptance.yml --status in_progress` and `--status queued` both empty. The release gate shares the `acceptance-suite` concurrency group, which keeps only one *pending* run: if the release's acceptance run is left pending, a pull request pushed behind it cancels it, and the builds and npm publish that depend on it never run. Don't open or push PRs while a release is building.
3334
- [ ] **Publish:** Write notes to a **temporary file**, run **`gh release create`** (see **Publish with GitHub CLI (`gh`)**), then **`rm`** the temp file. Use `--prerelease` for betas. (Humans may still use the GitHub UI per README.)
3435
- [ ] **Post-publish (optional):** Confirm the **`release`** workflow in Actions completed successfully for the new tag.
3536

‎.agents/skills/hookdeck-cli-release/references/release-notes-voice.md‎

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,31 +52,47 @@ Always end with the compare link:
5252

5353
`**Full Changelog**: https://github.com/hookdeck/hookdeck-cli/compare/<prev>...<new>`
5454

55-
## Summary
55+
## Factual, not editorial
56+
57+
A release note says what was wrong, what the CLI does now, and what the reader has to do, if
58+
anything. It does not interpret, characterise or narrate.
59+
60+
| Editorial — cut it | Factual — write this |
61+
|---|---|
62+
| "The one to act on:" | "If you have used `--hookdeck-config` with v3.0.0 or v3.0.1, run `chmod 600` on that file." |
63+
| "The other three share a shape — an argument you supplied did not survive the round trip." | *(nothing — the bullets already say what each fix is)* |
64+
| "so a script filtering by a list of ids was told they did not exist" | "returned no results for `--id a,b`" |
65+
| "an internal name, so an agent looking for the key it requested did not find it" | "returned `webhook_id` when `connection_id` was requested" |
5666

57-
Two to four short paragraphs. Lead with the capability or the theme, then the shape of the rest.
58-
**Name the shared shape when fixes rhyme** — often one defect wearing different clothes, and saying
59-
so is worth more than the list:
67+
The test for each clause: is it a fact about the software, or an instruction? If it is a reading of
68+
the facts — why it mattered, what it felt like, what it has in common with something else — cut it.
69+
The issue link carries the reasoning for anyone who wants it.
70+
71+
## Summary
6072

61-
> Alongside that is a long list of fixes sharing one shape — the CLI reported success while doing
62-
> something other than what you asked.
73+
One or two sentences: what the release contains, and any action a reader must take. A GA release
74+
names its headline capability; a patch names what it fixes. No theme, no framing.
6375

6476
## Entries
6577

6678
**Second person.** "your project", "your scripts" — never "the user".
6779

68-
**Lead with the fix, bolded, in the reader's terms.** Then the consequence. Then the link.
80+
**Lead with the fix, bolded, in the reader's terms.** Then what was wrong, stated as behaviour.
81+
Then the link.
6982

7083
> - **`hookdeck ci --local` and `hookdeck login --local` no longer rewrite your global config.**
71-
> `--local` added a second write rather than redirecting the first, so it silently switched the
72-
> active project for every other `hookdeck` command on the machine — the opposite of what the flag
73-
> is for. ([#332](https://github.com/hookdeck/hookdeck-cli/issues/332))
84+
> `--local` added a second write rather than redirecting the first, so it also switched the
85+
> active project for every other `hookdeck` command on the machine.
86+
> ([#332](https://github.com/hookdeck/hookdeck-cli/issues/332))
7487
75-
That entry is 52 words and explains a subtle bug completely. Match that density.
88+
That entry states the fix, the mechanism and the effect, with nothing else. (The v2.6.0 original
89+
ended "— the opposite of what the flag is for"; that clause is commentary and is cut here.)
7690

77-
**Make the damage concrete, and bold it where it is the point.**
91+
**State the wrong behaviour precisely** — the command, the input, the output. That is concrete
92+
without being editorial:
7893

79-
> returned **unfiltered totals formatted as if filtered**
94+
> `gateway event list`, `gateway request list` and `gateway transformation list` returned no results
95+
> for `--id a,b`.
8096
8197
**Show a command only when it earns the space** — a new flag people will copy, or output that makes
8298
a failure obvious. Not one per entry.
@@ -141,6 +157,7 @@ The reader stops looking for a workaround.
141157
## What this voice avoids
142158

143159
- **Verbosity.** See the table. This is the failure mode.
160+
- **Editorial framing.** Interpretation, characterisation, narrative. See *Factual, not editorial*.
144161
- **Marketing register.** No "we're excited", "powerful", "seamless".
145162
- **Hedging.** "may", "should" — say what it does.
146163
- **Labels that categorise the reader** ("who it reaches", "for advanced users").

‎.agents/skills/hookdeck-cli-review/SKILL.md‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,41 @@ than the colour:
6666
gh run view --job=<id> --log | grep -E -- '--- (SKIP|PASS): <TestName>'
6767
```
6868

69+
### A cancelled acceptance run is not a run either
70+
71+
Every acceptance run shares one concurrency group, `acceptance-suite`, so runs
72+
against the same test projects cannot collide. The group holds **one running
73+
run and one pending run**. When a third joins, the older *pending* one is
74+
cancelled; `cancel-in-progress: false` protects only the one that is running.
75+
76+
So when several pull requests are opened or pushed close together, every one
77+
but the newest can end with all five acceptance jobs `CANCELLED`. That happened
78+
to four v3.0.2 PRs in one afternoon. It is not a failure, and because
79+
acceptance is not a required check, the merge button stays green.
80+
81+
**Read `CANCELLED` as "never ran", and re-run it — one at a time.** Re-running
82+
two together puts both in the single pending slot and cancels one again. Queue
83+
the next only once the previous has started running:
84+
85+
```
86+
gh run rerun <run-id>
87+
gh run view <run-id> --json status -q .status # wait for in_progress, then the next
88+
```
89+
90+
The release gate lives in the same group. A release whose acceptance run is
91+
*pending* is cancelled by a pull request that joins behind it, and the builds
92+
and npm publish that depend on it never run. Before a release, check that no
93+
acceptance run is in progress or queued:
94+
95+
```
96+
gh run list --workflow=test-acceptance.yml --status in_progress
97+
gh run list --workflow=test-acceptance.yml --status queued
98+
```
99+
100+
And if a workflow change is in the diff: opening that pull request triggers
101+
acceptance too (`.github/workflows/**` is in the path allowlist), so it can
102+
cancel someone else's pending run.
103+
69104
### The one case where it genuinely does not run
70105

71106
`pull_request` does not fire when a pull request's head branch is updated by

‎.github/workflows/acceptance.yml‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,28 @@ on:
1616
# The failure is nasty because it does not look like rate limiting. It surfaces
1717
# as ordinary assertion failures in whichever slice lost the race -- a config
1818
# that came back empty, a filter that returned nothing -- sending whoever reads
19-
# it hunting for a regression that does not exist. It has happened twice: once
20-
# as four jobs timing out with zero assertion failures, and once as a single
21-
# named test failing while a release ran concurrently.
19+
# it hunting for a regression that does not exist. The clear case on record is
20+
# four jobs timing out with zero assertion failures.
2221
#
23-
# cancel-in-progress is deliberately false. Cancelling would be faster, but the
24-
# release workflow calls this as its publish gate, and killing that run to make
25-
# room for a pull request is the wrong trade. Queueing costs wall-clock and
26-
# nothing else.
22+
# (A red TestOutpostTenantPortalAndCustomDomain during a concurrent release was
23+
# once blamed on this too. It was not: the custom-domain API can return a 500
24+
# after the domain is already created, and the retry then gets a 409 for the
25+
# test's own hostname. Not every red run during an overlap is the overlap.)
26+
#
27+
# cancel-in-progress is deliberately false, so a RUNNING run is never killed to
28+
# make room for another -- the release gate included.
29+
#
30+
# But this is not a queue. A concurrency group holds at most one running run and
31+
# ONE pending run. When another run joins, the older pending run is CANCELLED.
32+
# So several pull requests opened or pushed in a burst leave all but the newest
33+
# with acceptance "CANCELLED": not a failure, but not coverage either, and since
34+
# acceptance is not a required check nothing blocks the merge. Re-run them one
35+
# at a time -- re-running two at once cancels one of them again.
36+
#
37+
# The same applies to the release gate when it is pending rather than running:
38+
# a pull-request run joining the group cancels it, and the builds and npm
39+
# publish that `need` it never run. Before `gh release create`, make sure no
40+
# acceptance run is in progress or queued.
2741
concurrency:
2842
group: acceptance-suite
2943
cancel-in-progress: false

0 commit comments

Comments
 (0)