fix: Publish images from one job in sequence #70

Merged
ahmad merged 4 commits from feature/66-serialise-pushes into main 2026-09-09 08:13:45 +00:00
Owner

Issue

Closes #66. The consumer propagation is filed separately as #69.

Problem

ahmad/imamah run 2564: publish is a matrix over api and web, both pushing to the same package, and the two images share base layers. The api push died with failed to push …: unknown. On the server, the same second:

GetOrInsertBlob() [E] duplicate key value violates unique constraint "UQE_package_blob_blake2b"
router: failed PUT /v2/ahmad/imamah/blobs/uploads/… panic … nil pointer dereference

Two identical blobs inserted concurrently; one loses the unique-constraint race and the loser's error path dereferences nil. Because buildkit can only report unknown, it reads as flaky CI rather than as a defect.

Solution

max-parallel: 1 on the publish matrix. One push at a time, so a release cannot race itself.

Review notes

Why serialised and not retried, since the issue leaves the choice open. It says either route satisfies the criteria, and prefers a retry if it can be made to distinguish a transient push failure from a real one. It cannot. failed to push …: unknown is buildkit saying it could not classify the server's fault — there is no status, no code, nothing but the word. A retry keyed on it would equally re-run a bad credential or a full disk two or three times before failing, which turns a clear failure into a slow one. The issue calls serialising the honest fallback for exactly this case.

So criterion 3 (a bounded retry) is deliberately not implemented, and I would rather say that here than have the merge imply the box is ticked.

What this does not fix. It removes the race between one release's own pushes, which is the observed failure. Two different repositories releasing in the same second can still collide, and nothing in this file prevents that — that needs the server-side nil dereference, tracked in amtronics/infrastructure-vps and explicitly out of scope here.

The cost. Wall-clock on multi-image releases: two images that built in parallel now build in sequence. For ahmad/namey, which has three, it is three times the push phase rather than two. That is the cheaper thing to spend against an intermittent red release that reads as flakiness and costs a re-run plus the time to decide it was flakiness.

The comment carries the reasoning, not just the line. This is a template people copy; max-parallel: 1 with no explanation is the kind of line someone deletes while tidying, and the failure it prevents is intermittent enough that nothing would connect the two.

Verified

check-actions, its own test suite, the behaviour suite and the README lint all exit 0. The parsed strategy reads {'max-parallel': 1, 'matrix': {'service': ['api', 'web']}} — the matrix entries are unchanged, so every image is still built and still tagged twice.

Risks and trade-offs

A future single-image consumer copying this template inherits max-parallel: 1 on a one-entry matrix, where it does nothing. Harmless, and the neighbouring comment already tells them to delete the matrix entirely.

If the wall-clock cost turns out to matter more than it looks — namey's three images are the case to watch — the alternative is not a retry but a shared push lock, which is a bigger change and wants a measurement first.

### Issue Closes #66. The consumer propagation is filed separately as **#69**. ### Problem `ahmad/imamah` run 2564: `publish` is a matrix over `api` and `web`, both pushing to the same package, and the two images share base layers. The api push died with `failed to push …: unknown`. On the server, the same second: ``` GetOrInsertBlob() [E] duplicate key value violates unique constraint "UQE_package_blob_blake2b" router: failed PUT /v2/ahmad/imamah/blobs/uploads/… panic … nil pointer dereference ``` Two identical blobs inserted concurrently; one loses the unique-constraint race and the loser's error path dereferences nil. Because buildkit can only report `unknown`, it reads as flaky CI rather than as a defect. ### Solution `max-parallel: 1` on the publish matrix. One push at a time, so a release cannot race itself. ### Review notes **Why serialised and not retried, since the issue leaves the choice open.** It says either route satisfies the criteria, and prefers a retry *if it can be made to distinguish a transient push failure from a real one*. It cannot. `failed to push …: unknown` is buildkit saying it could not classify the server's fault — there is no status, no code, nothing but the word. A retry keyed on it would equally re-run a bad credential or a full disk two or three times before failing, which turns a clear failure into a slow one. The issue calls serialising the honest fallback for exactly this case. So **criterion 3 (a bounded retry) is deliberately not implemented**, and I would rather say that here than have the merge imply the box is ticked. **What this does not fix.** It removes the race between one release's own pushes, which is the observed failure. Two *different repositories* releasing in the same second can still collide, and nothing in this file prevents that — that needs the server-side nil dereference, tracked in `amtronics/infrastructure-vps` and explicitly out of scope here. **The cost.** Wall-clock on multi-image releases: two images that built in parallel now build in sequence. For `ahmad/namey`, which has three, it is three times the push phase rather than two. That is the cheaper thing to spend against an intermittent red release that reads as flakiness and costs a re-run plus the time to decide it was flakiness. **The comment carries the reasoning, not just the line.** This is a template people copy; `max-parallel: 1` with no explanation is the kind of line someone deletes while tidying, and the failure it prevents is intermittent enough that nothing would connect the two. ### Verified `check-actions`, its own test suite, the behaviour suite and the README lint all exit 0. The parsed strategy reads `{'max-parallel': 1, 'matrix': {'service': ['api', 'web']}}` — the matrix entries are unchanged, so every image is still built and still tagged twice. ### Risks and trade-offs A future single-image consumer copying this template inherits `max-parallel: 1` on a one-entry matrix, where it does nothing. Harmless, and the neighbouring comment already tells them to delete the matrix entirely. If the wall-clock cost turns out to matter more than it looks — `namey`'s three images are the case to watch — the alternative is not a retry but a shared push lock, which is a bigger change and wants a measurement first.
fix: Push one image at a time from a release
All checks were successful
PR / validate (pull_request) Successful in 19s
9d03c463a5
Two images from one release share base layers, and the registry inserts
a blob by content hash - pushing the same layer concurrently makes one
insert lose a unique-constraint race whose error path dereferences nil.
Buildkit reports it as 'failed to push: unknown', so it reads as flaky
CI.

Serialised rather than retried: 'unknown' is all a retry could match on,
so it would re-run a bad credential or a full disk before failing.

Closes #66
ahmad changed title from WIP: fix: Push one image at a time from a release to fix: Push one image at a time from a release 2026-09-09 06:18:12 +00:00
Author
Owner

Verification on 9d03c46. PR / validate green — check-actions, its own test suite, the behaviour suite and the README lint.

Criterion 1 — no two pushes upload the same blob concurrently. max-parallel: 1 removes it by construction rather than by timing, and the parsed strategy reads {'max-parallel': 1, 'matrix': {'service': ['api', 'web']}}.

Criterion 2 — every image still published under both tags. Unchanged: the tags, the build args and the cache configuration are untouched, and the matrix still has the same entries. Serialising changes when a job runs, not what it does.

Criterion 3 — a bounded retry — is deliberately not implemented, and I want to be explicit rather than let a merge imply otherwise. The issue offers the two routes as alternatives ("either satisfies the criteria") and prefers a retry only if it can distinguish a transient push failure from a real one. It cannot: buildkit reports this as failed to push …: unknown, which is it saying it could not classify the server's fault. unknown is all a retry could match on, so it would equally re-run a bad credential or a full disk before failing. Serialising is the route the issue calls the honest fallback, and this is the case it described.

Criterion 4 — consumers listed, each with a follow-up. Surveyed from the API rather than from local checkouts, after that exact mistake on #65: six repositories publish more than one image from a parallel matrix — namey (three images), imamah, portfolio, trip, kidpoints, bayan-web. Three more build two images from sequential steps rather than a matrix and are already serial; three publish one image. All of it is on the follow-up, including which repositories need nothing, so the next pass does not re-derive it.

One thing worth knowing that the issue does not say. Bayan/bayan-web pushes into two different packages and is still affected: the constraint is UQE_package_blob_blake2b, on the blob rather than the package, so two images sharing a base layer collide whatever they are called. "Different packages, so no shared blob" is the natural assumption and it is wrong.

What this does not fix, stated because the issue's framing invites the wrong reading. It removes the race between one release's own pushes — which is the observed failure, imamah run 2564, publish racing itself over api and web. Two different repositories releasing in the same second can still collide, and no change to this file prevents that; that needs the server-side fix tracked in amtronics/infrastructure-vps.

Verification on `9d03c46`. `PR / validate` green — `check-actions`, its own test suite, the behaviour suite and the README lint. **Criterion 1 — no two pushes upload the same blob concurrently.** `max-parallel: 1` removes it by construction rather than by timing, and the parsed strategy reads `{'max-parallel': 1, 'matrix': {'service': ['api', 'web']}}`. **Criterion 2 — every image still published under both tags.** Unchanged: the tags, the build args and the cache configuration are untouched, and the matrix still has the same entries. Serialising changes when a job runs, not what it does. **Criterion 3 — a bounded retry — is deliberately not implemented, and I want to be explicit rather than let a merge imply otherwise.** The issue offers the two routes as alternatives ("either satisfies the criteria") and prefers a retry *only* if it can distinguish a transient push failure from a real one. It cannot: buildkit reports this as `failed to push …: unknown`, which is it saying it could not classify the server's fault. `unknown` is all a retry could match on, so it would equally re-run a bad credential or a full disk before failing. Serialising is the route the issue calls the honest fallback, and this is the case it described. **Criterion 4 — consumers listed, each with a follow-up.** Surveyed from the API rather than from local checkouts, after that exact mistake on `#65`: **six** repositories publish more than one image from a parallel matrix — `namey` (three images), `imamah`, `portfolio`, `trip`, `kidpoints`, `bayan-web`. Three more build two images from sequential steps rather than a matrix and are already serial; three publish one image. All of it is on the follow-up, including which repositories need nothing, so the next pass does not re-derive it. **One thing worth knowing that the issue does not say.** `Bayan/bayan-web` pushes into two *different packages* and is still affected: the constraint is `UQE_package_blob_blake2b`, on the blob rather than the package, so two images sharing a base layer collide whatever they are called. "Different packages, so no shared blob" is the natural assumption and it is wrong. **What this does not fix, stated because the issue's framing invites the wrong reading.** It removes the race between one release's own pushes — which is the observed failure, `imamah` run 2564, `publish` racing itself over `api` and `web`. Two *different repositories* releasing in the same second can still collide, and no change to this file prevents that; that needs the server-side fix tracked in `amtronics/infrastructure-vps`.
ahmad_bot left a comment

The diagnosis is sound and I confirmed the premise rather than taking it — but the fix rests on one thing neither of us has evidence for, so this is a comment rather than an approval. No defect found.

The premise is real: those two legs genuinely overlap on this fleet. From ahmad/imamah's task history, run 308 (the green one):

publish (api)   20:42:07 → 20:43:24
publish (web)   20:42:26 → 20:42:54

web starts and finishes entirely inside api's window, so the runner does execute matrix legs concurrently and a release really can race itself. That was worth checking: one runner in the fleet could equally have meant capacity 1, in which case the collision would have had to come from two different repositories and this change would not touch it.

One thing for whoever counts occurrences later. Run 307's earlier attempt at 16:13 has both legs failing without overlapping — api 16:13:35→16:13:44, web 16:13:45→16:13:52, and each dead in under ten seconds. That is a different failure wearing the same colour (it looks like the #65 shape), so a tally of "red publish jobs" is not a tally of this bug.

The unverified part, and it is the whole mechanism. I scanned all 21 repositories on the server: nothing uses max-parallel today, so there is no evidence here that Forgejo's scheduler honours it. On GitHub it is enforced server-side, and Forgejo expands a matrix into independent job rows that are dispatched as runners free up — whether max-parallel reaches that scheduling decision is exactly the kind of thing that is true or false and not arguable. If it is ignored, the line is inert, the comment above it is confidently wrong, and the next collision looks identical to this one.

The check is the same one above and costs nothing: after the first multi-image release on the new template, read /repos/{owner}/{repo}/actions/tasks and compare the two publish (...) windows. Non-overlapping means it works. ahmad/imamah or ahmad/namey will produce one soon enough. Worth posting the timings on #69 when it happens, so the propagation goes out on a mechanism that has been observed rather than assumed.

If it turns out inert, I agree the answer still is not a retry, for the reason you give — unknown is unclassifiable and a retry would re-run a bad credential too. It would be needs: between the legs, or one job pushing both in sequence. Both are uglier and both are checkable.

The rest is right. Serialising over retrying is well argued; saying plainly that criterion 3 is deliberately unimplemented is better than a merge that implies otherwise; and putting the reasoning in the template rather than only in this description is correct for a line whose deletion would be silent and whose failure is intermittent.

The diagnosis is sound and I confirmed the premise rather than taking it — but the fix rests on one thing neither of us has evidence for, so this is a comment rather than an approval. No defect found. **The premise is real: those two legs genuinely overlap on this fleet.** From `ahmad/imamah`'s task history, run 308 (the green one): publish (api) 20:42:07 → 20:43:24 publish (web) 20:42:26 → 20:42:54 `web` starts and finishes entirely inside `api`'s window, so the runner does execute matrix legs concurrently and a release really can race itself. That was worth checking: one runner in the fleet could equally have meant capacity 1, in which case the collision would have had to come from two different repositories and this change would not touch it. **One thing for whoever counts occurrences later.** Run 307's earlier attempt at 16:13 has both legs failing *without* overlapping — `api` 16:13:35→16:13:44, `web` 16:13:45→16:13:52, and each dead in under ten seconds. That is a different failure wearing the same colour (it looks like the `#65` shape), so a tally of "red publish jobs" is not a tally of this bug. **The unverified part, and it is the whole mechanism.** I scanned all 21 repositories on the server: **nothing uses `max-parallel` today**, so there is no evidence here that Forgejo's scheduler honours it. On GitHub it is enforced server-side, and Forgejo expands a matrix into independent job rows that are dispatched as runners free up — whether `max-parallel` reaches that scheduling decision is exactly the kind of thing that is true or false and not arguable. If it is ignored, the line is inert, the comment above it is confidently wrong, and the next collision looks identical to this one. **The check is the same one above and costs nothing:** after the first multi-image release on the new template, read `/repos/{owner}/{repo}/actions/tasks` and compare the two `publish (...)` windows. Non-overlapping means it works. `ahmad/imamah` or `ahmad/namey` will produce one soon enough. Worth posting the timings on `#69` when it happens, so the propagation goes out on a mechanism that has been observed rather than assumed. **If it turns out inert**, I agree the answer still is not a retry, for the reason you give — `unknown` is unclassifiable and a retry would re-run a bad credential too. It would be `needs:` between the legs, or one job pushing both in sequence. Both are uglier and both are checkable. **The rest is right.** Serialising over retrying is well argued; saying plainly that criterion 3 is deliberately unimplemented is better than a merge that implies otherwise; and putting the reasoning in the template rather than only in this description is correct for a line whose deletion would be silent and whose failure is intermittent.
@ -95,0 +108,4 @@
# This removes the race between one release's own pushes. Two repositories
# releasing at the same second can still collide; that needs the server
# fix (tracked in `amtronics/infrastructure-vps`), not this file.
max-parallel: 1
Member

This is the one line whose effect nobody here has observed.

I scanned every repository on the server (21): none uses max-parallel, so there is no in-fleet evidence that Forgejo's scheduler implements it. GitHub enforces it in the server's job dispatcher; Forgejo expands a matrix into independent job rows and dispatches them as runners free up, and whether this key reaches that decision is a property of the version we run, not of the YAML.

It matters more than usual because of how it fails: if the key is ignored, the change is inert, the comment above it states a mechanism that does not exist, and the next collision is indistinguishable from the ones before it — the same failed to push …: unknown that already reads as flaky CI.

Confirming it is one API call after the next multi-image release. The concurrency is visible in the task history, which is how I verified the premise for this PR:

ahmad/imamah run 308
  publish (api)   20:42:07 → 20:43:24
  publish (web)   20:42:26 → 20:42:54     ← inside api's window

After the template lands, the same two rows should no longer overlap. GET /repos/{owner}/{repo}/actions/tasks carries run_started_at and updated_at per job, and needs no log access.

Nothing to change in this diff — I would merge it and check, rather than hold it. But #69 should not propagate to eleven repositories until one release has been observed serialising, because propagating an inert line is how a fix becomes folklore.

**This is the one line whose effect nobody here has observed.** I scanned every repository on the server (21): none uses `max-parallel`, so there is no in-fleet evidence that Forgejo's scheduler implements it. GitHub enforces it in the server's job dispatcher; Forgejo expands a matrix into independent job rows and dispatches them as runners free up, and whether this key reaches that decision is a property of the version we run, not of the YAML. It matters more than usual because of how it fails: if the key is ignored, the change is inert, the comment above it states a mechanism that does not exist, and the next collision is indistinguishable from the ones before it — the same `failed to push …: unknown` that already reads as flaky CI. **Confirming it is one API call after the next multi-image release.** The concurrency is visible in the task history, which is how I verified the premise for this PR: ahmad/imamah run 308 publish (api) 20:42:07 → 20:43:24 publish (web) 20:42:26 → 20:42:54 ← inside api's window After the template lands, the same two rows should no longer overlap. `GET /repos/{owner}/{repo}/actions/tasks` carries `run_started_at` and `updated_at` per job, and needs no log access. Nothing to change in this diff — I would merge it and check, rather than hold it. But `#69` should not propagate to eleven repositories until one release has been observed serialising, because propagating an inert line is how a fix becomes folklore.
test: Probe whether the runner honours max-parallel
All checks were successful
PR / validate (pull_request) Successful in 20s
PR / maxparallel-probe (one) (pull_request) Successful in 27s
PR / maxparallel-probe (three) (pull_request) Successful in 27s
PR / maxparallel-probe (two) (pull_request) Successful in 26s
256f3a102a
Temporary. Removed once the timings are read.
test: Probe job-level concurrency instead
All checks were successful
PR / validate (pull_request) Successful in 17s
PR / maxparallel-probe (one) (pull_request) Successful in 27s
PR / maxparallel-probe (three) (pull_request) Successful in 27s
PR / maxparallel-probe (two) (pull_request) Successful in 27s
19b63eb40d
fix: Publish images from one job in sequence
All checks were successful
PR / validate (pull_request) Successful in 17s
db8e1f5abe
max-parallel does not serialise a matrix on this runner. Measured: three
legs that should have run one at a time overlapped in the task history,
and job-level concurrency behaved the same way. Steps within one job are
the only ordering this scheduler honours.

The publish job now builds and pushes each image as its own step, which
is the shape larda, expiro and suhba-web already use and the reason
their releases never hit this. Adding an image means adding a step.

Closes #66
ahmad changed title from fix: Push one image at a time from a release to WIP: fix: Publish images from one job in sequence 2026-09-09 07:19:47 +00:00
Author
Owner

You were right to withhold, and the thing you could not verify turned out to be false. max-parallel is not honoured on this runner. The fix as written was inert.

I did not wait for the next multi-image release — the same question is answerable in ninety seconds on this PR. A throwaway job on this branch, three legs each sleeping 25s, max-parallel: 1:

maxparallel-probe (one)    07:14:00 → 07:14:27
maxparallel-probe (three)  07:14:20 → 07:14:47   ← started while (one) was still running
maxparallel-probe (two)    07:14:28 → 07:14:55

Serialised, that is 81 seconds; it took 55. The legs overlap, so the key reaches nothing. Forgejo is 16.0.3+gitea-1.22.0.

Then I tried the alternative before proposing it, since I had just been caught assuming once. Job-level concurrency with cancel-in-progress: false:

maxparallel-probe (one)    07:16:14 → 07:16:41
maxparallel-probe (three)  07:16:32 → 07:16:59   ← same overlap
maxparallel-probe (two)    07:16:42 → 07:17:09

Also ignored. Both probes are removed from the branch; the diff is templates/release.yml alone.

What actually serialises is what three repositories on this fleet already do. ahmad/larda, ahmad/expiro and suhba/suhba-web build two images from two sequential steps in one job, which is why the survey for #69 found them already safe. I had recorded that as a fact about them without noticing it was the answer. Steps within a job run in order — that is not a scheduler feature that might be unimplemented, it is what a job is.

So publish is now one job with one docker/build-push-action step per image, no matrix. Adding an image means adding a step. The comment on the job records both measurements, so nobody re-tries max-parallel on the strength of it working elsewhere.

On your other two points. The overlap you found in imamah run 308 is the premise this rests on and I am glad you measured it rather than trusting mine. And your warning about run 307 — two legs failing without overlapping, in under ten seconds each, which is the #65 shape — is worth keeping on #69: a tally of red publish jobs is not a tally of this bug, and now that #65 is merged that particular confusion should stop generating new cases.

#69 needs rewriting and I will do that next: it currently says "add max-parallel: 1 to six repositories", which would have propagated an inert line to all of them — exactly what you said not to do. It becomes "restructure the matrix into sequential steps", with the three repositories that already have that shape as the worked example rather than as an exclusion.

You were right to withhold, and the thing you could not verify turned out to be false. **`max-parallel` is not honoured on this runner.** The fix as written was inert. I did not wait for the next multi-image release — the same question is answerable in ninety seconds on this PR. A throwaway job on this branch, three legs each sleeping 25s, `max-parallel: 1`: ``` maxparallel-probe (one) 07:14:00 → 07:14:27 maxparallel-probe (three) 07:14:20 → 07:14:47 ← started while (one) was still running maxparallel-probe (two) 07:14:28 → 07:14:55 ``` Serialised, that is 81 seconds; it took 55. The legs overlap, so the key reaches nothing. Forgejo is `16.0.3+gitea-1.22.0`. **Then I tried the alternative before proposing it**, since I had just been caught assuming once. Job-level `concurrency` with `cancel-in-progress: false`: ``` maxparallel-probe (one) 07:16:14 → 07:16:41 maxparallel-probe (three) 07:16:32 → 07:16:59 ← same overlap maxparallel-probe (two) 07:16:42 → 07:17:09 ``` Also ignored. Both probes are removed from the branch; the diff is `templates/release.yml` alone. **What actually serialises is what three repositories on this fleet already do.** `ahmad/larda`, `ahmad/expiro` and `suhba/suhba-web` build two images from **two sequential steps in one job**, which is why the survey for `#69` found them already safe. I had recorded that as a fact about them without noticing it was the answer. Steps within a job run in order — that is not a scheduler feature that might be unimplemented, it is what a job *is*. So `publish` is now one job with one `docker/build-push-action` step per image, no matrix. Adding an image means adding a step. The comment on the job records both measurements, so nobody re-tries `max-parallel` on the strength of it working elsewhere. **On your other two points.** The overlap you found in `imamah` run 308 is the premise this rests on and I am glad you measured it rather than trusting mine. And your warning about run 307 — two legs failing without overlapping, in under ten seconds each, which is the `#65` shape — is worth keeping on `#69`: a tally of red publish jobs is not a tally of this bug, and now that `#65` is merged that particular confusion should stop generating new cases. **`#69` needs rewriting** and I will do that next: it currently says "add `max-parallel: 1` to six repositories", which would have propagated an inert line to all of them — exactly what you said not to do. It becomes "restructure the matrix into sequential steps", with the three repositories that already have that shape as the worked example rather than as an exclusion.
ahmad changed title from WIP: fix: Publish images from one job in sequence to fix: Publish images from one job in sequence 2026-09-09 07:19:48 +00:00
Author
Owner

Verification on db8e1f5. PR / validate green.

Criterion 1 — no two pushes upload the same blob concurrently. Now true by construction rather than by a scheduler key: publish has no strategy, and its two build-and-push steps are steps in one job. Steps run in order because that is what a job is — there is nothing here that a scheduler could decline to implement, which is precisely what went wrong with the first attempt. The parsed job confirms strategy is absent and the steps are … Build and push the api image, Build and push the web image.

Criterion 2 — every image still published under both tags. The tags, cache refs and build args are the same strings with ${{ matrix.service }} resolved to the literal, so :api-v1.2.3, :api-<sha>, :web-v1.2.3, :web-<sha> and both buildcache-* refs are unchanged.

Criterion 3 — a bounded retry — remains deliberately unimplemented, unchanged in reasoning: buildkit reports unknown, so a retry cannot tell this from a bad credential.

Criterion 4 — consumers listed. #69 is rewritten: it now says restructure the matrix, not add a key, and names the three repositories that already have the shape as the worked example rather than as an exclusion.

What changed since the last verification, and why it matters more than the diff. I claimed max-parallel: 1 removed the race "by construction". It removed nothing — the key is ignored on this runner, measured twice. The lesson I am taking is not about YAML: I wrote by construction about a mechanism I had not observed, and the phrase is exactly the one that should have made me check.

check-actions, its own test suite, the behaviour suite and the README lint all exit 0.

Verification on `db8e1f5`. `PR / validate` green. **Criterion 1 — no two pushes upload the same blob concurrently.** Now true by construction rather than by a scheduler key: `publish` has no `strategy`, and its two build-and-push steps are steps in one job. Steps run in order because that is what a job is — there is nothing here that a scheduler could decline to implement, which is precisely what went wrong with the first attempt. The parsed job confirms `strategy` is absent and the steps are `… Build and push the api image`, `Build and push the web image`. **Criterion 2 — every image still published under both tags.** The tags, cache refs and build args are the same strings with `${{ matrix.service }}` resolved to the literal, so `:api-v1.2.3`, `:api-<sha>`, `:web-v1.2.3`, `:web-<sha>` and both `buildcache-*` refs are unchanged. **Criterion 3 — a bounded retry — remains deliberately unimplemented**, unchanged in reasoning: buildkit reports `unknown`, so a retry cannot tell this from a bad credential. **Criterion 4 — consumers listed.** `#69` is rewritten: it now says restructure the matrix, not add a key, and names the three repositories that already have the shape as the worked example rather than as an exclusion. **What changed since the last verification, and why it matters more than the diff.** I claimed `max-parallel: 1` removed the race "by construction". It removed nothing — the key is ignored on this runner, measured twice. The lesson I am taking is not about YAML: I wrote *by construction* about a mechanism I had not observed, and the phrase is exactly the one that should have made me check. `check-actions`, its own test suite, the behaviour suite and the README lint all exit 0.
ahmad_bot approved these changes 2026-09-09 07:45:21 +00:00
ahmad_bot left a comment

Approved. You went and measured the thing neither of us could assert, the answer was no, and the change is now built on what the scheduler actually does. That is the right response to "this rests on something unproven" and a better outcome than the version I commented on.

Both probes confirm it, from the task history rather than the job logs:

run 63, `max-parallel: 1`, three legs sleeping 25s
  leg one    07:14:00 → 07:14:27
  leg three  07:14:20 → 07:14:47   ← started 7s before `one` finished
  leg two    07:14:28 → 07:14:55

run 64, job-level `concurrency` with a shared group
  leg one    07:16:14 → 07:16:41
  leg three  07:16:32 → 07:16:59   ← same overlap
  leg two    07:16:42 → 07:17:09

Neither key serialises anything. Had max-parallel: 1 been honoured, three could not have started before 07:14:27.

And the same numbers say something else worth keeping. Three 25-second legs took 55 seconds end to end, with validate occupying a slot for the first 20: legs start exactly as earlier ones finish, two at a time throughout. This runner's capacity looks like 2, which is also why ahmad/imamah's two publish legs overlapped in the first place. It bounds what any workflow here can get from a matrix, and it means serialising two image builds costs less wall-clock than it appears to — they were competing for one saturated host either way.

The change itself:

  • No dangling matrix reference anywhere in the template — I grepped it, because an unsupported context expression renders empty rather than failing, and a leftover ${{ matrix.service }} would have pushed :-v1.2.3 quietly. Clean, and the probe workflow is gone from pr.yml too.
  • One job is stricter than the matrix was, in a good way. Sequential steps mean a failed api build stops before web is pushed. The old matrix could publish one image and not the other — ahmad/imamah run 307 did exactly that, web green and api red — which leaves a half-published release for a deploy to trip over.
  • build-args on the web step only is more honest than applying the placeholder to both; an api image takes no VITE_*.
  • The comment carries the measurement, not just the conclusion. "Both were measured on this runner and both ignored" is what stops someone reinstating a matrix with max-parallel in a year, and it is the kind of claim that is worthless without the word measured.

Unchanged from last time and still right: this fixes a release racing itself, not two repositories racing each other, and the latter needs the server-side nil dereference. Worth carrying that sentence into #69's propagation so nobody reads a quiet week as the whole problem being gone.

Approved. You went and measured the thing neither of us could assert, the answer was no, and the change is now built on what the scheduler actually does. That is the right response to "this rests on something unproven" and a better outcome than the version I commented on. **Both probes confirm it, from the task history rather than the job logs:** run 63, `max-parallel: 1`, three legs sleeping 25s leg one 07:14:00 → 07:14:27 leg three 07:14:20 → 07:14:47 ← started 7s before `one` finished leg two 07:14:28 → 07:14:55 run 64, job-level `concurrency` with a shared group leg one 07:16:14 → 07:16:41 leg three 07:16:32 → 07:16:59 ← same overlap leg two 07:16:42 → 07:17:09 Neither key serialises anything. Had `max-parallel: 1` been honoured, `three` could not have started before 07:14:27. **And the same numbers say something else worth keeping.** Three 25-second legs took 55 seconds end to end, with `validate` occupying a slot for the first 20: legs start exactly as earlier ones finish, two at a time throughout. **This runner's capacity looks like 2**, which is also why `ahmad/imamah`'s two `publish` legs overlapped in the first place. It bounds what any workflow here can get from a matrix, and it means serialising two image builds costs less wall-clock than it appears to — they were competing for one saturated host either way. **The change itself:** - **No dangling `matrix` reference anywhere in the template** — I grepped it, because an unsupported context expression renders *empty* rather than failing, and a leftover `${{ matrix.service }}` would have pushed `:-v1.2.3` quietly. Clean, and the probe workflow is gone from `pr.yml` too. - **One job is stricter than the matrix was, in a good way.** Sequential steps mean a failed api build stops before web is pushed. The old matrix could publish one image and not the other — `ahmad/imamah` run 307 did exactly that, web green and api red — which leaves a half-published release for a deploy to trip over. - **`build-args` on the web step only** is more honest than applying the placeholder to both; an api image takes no `VITE_*`. - **The comment carries the measurement, not just the conclusion.** "Both were measured on this runner and both ignored" is what stops someone reinstating a matrix with `max-parallel` in a year, and it is the kind of claim that is worthless without the word *measured*. **Unchanged from last time and still right:** this fixes a release racing itself, not two repositories racing each other, and the latter needs the server-side nil dereference. Worth carrying that sentence into `#69`'s propagation so nobody reads a quiet week as the whole problem being gone.
ahmad merged commit 94b67b741a into main 2026-09-09 08:13:45 +00:00
ahmad deleted branch feature/66-serialise-pushes 2026-09-09 08:13:45 +00:00
Sign in to join this conversation.
No description provided.