(feat): bump the downstream base image after a release

[abnegate]

Extends the weekly automation past its own release: open a base bump in appwrite/appwrite, wait for CI there, merge it, and tag the merge commit.

Why

A base release was only half the job. appwrite/appwrite pins the image by exact version — FROM appwrite/base:2.0.0 AS base, plus a -xdebug reference and a comment — so every release we published left the consumer behind until someone edited that Dockerfile by hand.

The flow

Four steps appended to dependencies.yml, each gated on the one before:

1. Propose — read Dockerfile at the head of the downstream branch, rewrite every appwrite/base:<version> occurrence, commit to automation/base-<version>, open the PR. Emits changed=false and stops if the pin is already current, so a re-run is a no-op.
2. Wait for downstream CI — poll the PR's statusCheckRollup until every check has concluded. SUCCESS, SKIPPED and NEUTRAL pass; anything else aborts before the merge, naming the failing checks.
3. Merge and tag — squash-merge pinned to the exact tested head, then read APP_VERSION_STABLE from app/init/constants.phpat the merge commit and tag it cl-{version}-{n}.
4. Summarise — write the tag into the job summary.

Decisions, and the evidence behind them

Target branch is main.main carries appwrite/base:2.0.0; 1.9.x is still on 1.4.3 and has no -xdebug stage, so bumping it would not be a version replacement. cl-1.9.6-1 also peels to a commit that is an ancestor of main, so that is where these tags are cut. Configurable via DOWNSTREAM_BRANCH.

Tag pattern matches what is already there — cl-1.8.0-2..4, cl-1.9.0-1..5, cl-1.9.5-1, cl-1.9.6-1. The sub-version is the next unused integer for that application version, so a bump of APP_VERSION_STABLE restarts at -1. Unrelated cl- tags such as cl-usage-poc-4 and cl-shared-tables-zdt-6 are ignored.

Merge uses --admin.appwrite/appwritemain requires one approving review and GitHub forbids approving your own pull request, so a single automation identity cannot both open and approve. Per your decision the merge bypasses the review requirement — it does not bypass the checks, which step 2 has already required to be green.

Required before this can run

A DOWNSTREAM_TOKEN secret with admin rights on appwrite/appwrite. --admin needs admin permission, and github.token from this repository has no access there at all. Without it the propose step fails immediately and the base release is unaffected.

Verification

composer verify: Pint, PHPStan max, 177 tests / 1857 assertions, 91 parity contracts. 21 new tests cover the new domain:

• rewriting all three reference forms together; no-op when current; conflicting versions rejected; missing reference rejected; inexact target rejected.
• next sub-version selection, including the semantic maximum (-10 beats -9), restart at -1 for a new application version, and rejection of a non-canonical -01.
• unrelated and prefixed tags ignored (cl-1.9.6-1-rc1, cl-1.9.60-4, cl-shared-tables-zdt-6).
• waiting across rounds until checks settle, and aborting with the failing check names.
• the merge-then-tag call order, asserting the tag lands on the merge commit rather than the tested head.

Accepted residual risk

A required downstream check that changes state between release() verifying the rollup and the --admin merge will be merged past. This cannot be closed while the merge bypasses branch protection — GitHub's only atomic merge precondition is the head SHA. Accepted deliberately by the repository owner in favour of the unattended merge; see the open Greptile threads for the full reasoning.

Not verified

• Nothing here has run against appwrite/appwrite. Every downstream interaction is covered by a fake repository, not the real API. The first live run merges and tags in the main product repository.
• Two prerequisites are unmet, and the flow cannot run until both exist: a DOWNSTREAM_TOKEN secret with admin rights on the downstream repository, and required status-check contexts configured on its main branch. The wait refuses outright if the branch declares none.
• Downstream CI length is unknown to me. The wait uses a 7200s deadline inside a job whose own timeout is 300 minutes; if the required checks take longer than two hours the wait aborts with the PR already open.
• The propose step commits through the Contents API, so the commit is unsigned. If the downstream branch requires signed commits it will be rejected.
• Nothing local guards against PHP syntax newer than the 8.3 target — that was caught here only by CI, incidentally, via the runner's PHP version.

[greptile-apps]

Greptile Summary

The PR extends base-image release automation into the downstream Appwrite repository, including proposal, required-check waiting, admin merge, recovery, and release tagging.

• Rewrites downstream base-image references and opens a version-specific pull request.
• Tracks declared required checks and validates them again before merging.
• Recovers untagged merge commits before deciding whether another proposal is needed.
• Computes and creates the next application release tag on the squash merge commit.
• The final check refresh still leaves a short state-change window before the separate admin merge.

Confidence Score: 3/5

The PR is not yet safe to merge because a required downstream check can change state between the final refresh and the branch-protection-bypassing admin merge.

The newly added refresh reduces the previously reported race but does not close it: release() validates statusCheckRollup and then performs a separate admin merge whose atomic condition covers only the PR head SHA, so an intervening pending or failed check can still be merged and tagged.

Files Needing Attention: .github/scripts/src/Downstream/Orchestrator.php, .github/scripts/src/Downstream/Repository/GitHub.php, .github/workflows/dependencies.yml

Security Review

The merge-time CI boundary is not atomic: a required check can become pending or fail between the refreshed status query and the subsequent admin merge, which bypasses branch protection and can merge and tag an unverified commit.

Important Files Changed

Filename	Overview
.github/scripts/src/Downstream/Orchestrator.php	Coordinates proposal, required-check waiting, recovery, merge, and tagging, but its refreshed check validation is still separated from the admin merge.
.github/scripts/src/Downstream/Repository/GitHub.php	Implements downstream GitHub operations; the admin merge matches the tested head SHA but does not atomically bind the previously observed required-check state.
.github/scripts/src/Downstream/Checks.php	Correctly identifies missing, pending, and unsuccessful declared required checks.
.github/workflows/dependencies.yml	Adds the recover, propose, wait, and release sequence, reaching the remaining non-atomic check-and-admin-merge boundary.
.github/scripts/src/Downstream/Release.php	Selects the next canonical application-specific release sub-version while ignoring unrelated tags.
.github/scripts/src/Downstream/Dockerfile.php	Validates and consistently rewrites exact downstream appwrite/base version references.

Prompt To Fix All With AI

### Issue 1
.github/scripts/src/Downstream/Orchestrator.php:114-124
**Check state races admin merge**
When a required downstream check is rerun or changes to pending or failure after `release()` reads the rollup, the subsequent `--admin` merge still proceeds because `--match-head-commit` binds only the PR head SHA, causing an unverified update to be merged and tagged.
**How this was verified:** The check validation and admin merge execute as separate operations, and the merge command has no condition binding it to the previously observed check state.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (4): Last reviewed commit: "(fix): re-verify required checks at merg..." | Re-trigger Greptile}

[abnegate]

Both Greptile P1s fixed in 6036b14. Both were right, and both would have ended badly in the main product repository.

Incomplete check set passes.settled() accepted the first non-empty rollup in which everything had concluded. On a fresh PR a single fast check can finish before the heavy workflows have registered theirs, so the admin merge could have landed a pull request whose real CI never started. This is the exact trap I had been handling by hand while shepherding these PRs and failed to encode in the product code.

The wait now requires three things together: every check concluded, the check-name/conclusion signature unchanged across two consecutive polls, and a 120s grace period elapsed since the wait began. A workflow that registers late changes the signature and resets the requirement.

Merged release cannot recover. A run dying between merge() and tag() left the downstream pin in place with no release tag; the next run read the Dockerfile as already current, emitted changed=false, and skipped forever. Same class as the recovery hole fixed in #92, and I built this flow without recovery at all.

A recover step now runs before propose. It finds the merged automation pull request for the version, and tags its merge commit if no cl- tag already points there. It is bounded to the downstream tip, so a merge that main has moved past is left alone rather than resurrected with a fresh sub-version — the lesson from #92.

Verification — each confirmed red before the fix and green after:

• test_waits_out_a_late_registering_workflow feeds one completed check, then a second check appearing in progress, then both complete. Fails with the stability requirement removed.
• test_tags_a_merge_that_never_got_its_tag fails with recovery disabled.
• test_does_not_recover_a_merge_already_tagged and test_does_not_recover_a_merge_main_has_moved_past bound it.

composer verify: PHPStan max, 182 tests / 1866 assertions, 91 parity contracts.

Worth a look on re-review: the 120s grace plus two-poll stability adds a floor of roughly two and a half minutes to every downstream wait, and Repository::tags() now returns name/target pairs rather than names, which assumes our tags are lightweight refs — they are, since tag() creates them via git/refs with the commit sha.

[abnegate]

Redesigned the merge gate in f4d721b rather than patching the heuristic a third time. Both remaining P1s are addressed.

Late checks still escape stabilization. Correct, and the third attempt would have been wrong too. A recent appwrite/appwrite pull request carries 44 checks, many of them a dynamic matrix (Tests / E2E / PostgreSQL (dedicated) / Account and siblings) that only registers once its generator job finishes. No view of the currently visible rollup can distinguish a finished run from one whose matrix has not been generated yet — all-complete, stability, and grace windows are all guesses about the same unknowable thing.

The waiter now reads the branch protection required_status_checks.contexts for the downstream branch and waits for exactly those to conclude. That set is declared, not inferred, so a required check that registers ten minutes late is still waited for. The grace window and signature stability are gone.

It also refuses to merge when the branch declares no required contexts. --admin bypasses branch protection including required checks, so without that guard the automation would merge having verified nothing. This makes the dependency explicit and fails closed.

This does mean the downstream branch must actually mark its test checks required — that configuration is now load-bearing, not advisory.

Advanced main blocks tag recovery. Also correct. The tip bound made a release permanently unrecoverable as soon as anyone else merged. Recovery now requires only that the merge commit is reachable from the downstream branch, which keeps it recoverable while still refusing a commit that is not on the branch at all. The reason #92 needed a tip bound does not apply here: that selector scanned every automation merge in history, whereas recover here is scoped to one specific base version's branch, so an unrelated old merge cannot be resurrected.

Verification — each confirmed red before the fix:

• test_waits_for_a_required_check_that_registers_late: a required check appears only on the third poll; fails against the previous all-complete logic.
• test_refuses_to_merge_when_nothing_is_required.
• test_recovers_after_main_has_moved_past_the_merge and test_does_not_recover_a_merge_absent_from_the_branch.
• test_ignores_checks_that_are_not_required confirms an unrelated failing check does not block, and an unrelated pending one does not hold the merge.

composer verify: PHPStan max, 185 tests / 1872 assertions, 91 parity contracts.

[greptile-apps]

Check state changes before merge

When a required downstream check is rerun, becomes pending, or fails after the wait step's final successful poll, the separate release step uses --admin and validates only the PR head commit, causing the update to be merged and tagged without all required checks passing.

Prompt To Fix With AI

This is a comment left during a code review.
Path: .github/workflows/dependencies.yml
Line: 286-294
Comment:
**Check state changes before merge**
When a required downstream check is rerun, becomes pending, or fails after the wait step's final successful poll, the separate release step uses `--admin` and validates only the PR head commit, causing the update to be merged and tagged without all required checks passing.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[abnegate]

Agreed that the race is not closed, and I am not going to claim otherwise. But it cannot be closed by any refinement of this code, and that is worth stating plainly.

GitHub offers exactly one atomic precondition on a merge: --match-head-commit, which covers the head SHA. There is no "merge only if these checks currently pass" operation. The mechanism that does enforce required checks atomically at merge time is branch protection itself — and --admin exists precisely to bypass branch protection. So any design that admin-merges has an irreducible window between reading check state and issuing the merge.

78241a6 narrows that window from "however long the merge step takes to start" — potentially minutes, since it is a separate workflow step — to a single API round trip. That is a real reduction, not a fix.

Closing it properly requires not bypassing: obtain an approving review from a second identity and merge without--admin, letting branch protection enforce the required checks atomically. That was considered and rejected earlier in favour of admin bypass, on the understanding that the tradeoff was convenience versus least privilege. Your finding shows the tradeoff also includes atomicity, which was not on the table when that call was made, so I have put the decision back to the repository owner rather than silently accepting the residual risk.

Leaving this thread open deliberately: it is an accurate description of the current design, not something I have addressed.

[greptile-apps]

That's a precise and honest analysis. You're right on all counts — --match-head-commit guards the SHA, not the check state, and branch protection is the only mechanism that enforces required checks atomically at merge time. Admin bypass purchases convenience at the cost of that atomicity, and 78241a6 shrinks but cannot close the window.

The framing you've added — that the atomicity tradeoff was not on the table when the admin-bypass decision was made — is exactly the right way to surface it. Leaving this thread open as a documented statement of the residual risk is the correct call. I won't push further on this.

[abnegate]

Greptile P1 (check state changes before merge) — fixed in 78241a6.

Correct, and a genuine gap rather than a restatement. Waiting for the required checks and merging were separate workflow steps, and the merge bypasses branch protection, so a required check re-run between the wait's final poll and the merge would simply be ignored. The exact state the wait exists to prevent was reachable through the gap between the two steps.

release() now re-reads the required contexts and their conclusions immediately before calling merge, and refuses if any is pending again or no longer passing. The window is a single call rather than however long the merge step takes to start. It cannot be closed completely — any check state read from GitHub can change a moment later — but it is no longer a multi-minute hole.

Verification, both confirmed red with the re-verification removed:

• test_refuses_to_merge_a_check_that_went_pending_again
• test_refuses_to_merge_a_check_that_failed_after_waiting

test_merges_then_tags_the_merge_commit now asserts the full call order — required:main, checks:93, merge, file, tags, tag — so the re-verification cannot be silently dropped later without a test noticing.

composer verify: PHPStan max, 187 tests / 1876 assertions, 91 parity contracts.

[greptile-apps]

Check state races admin merge

When a required downstream check is rerun or changes to pending or failure after release() reads the rollup, the subsequent --admin merge still proceeds because --match-head-commit binds only the PR head SHA, causing an unverified update to be merged and tagged.

How this was verified: The check validation and admin merge execute as separate operations, and the merge command has no condition binding it to the previously observed check state.

Prompt To Fix With AI

This is a comment left during a code review.
Path: .github/scripts/src/Downstream/Orchestrator.php
Line: 114-124
Comment:
**Check state races admin merge**
When a required downstream check is rerun or changes to pending or failure after `release()` reads the rollup, the subsequent `--admin` merge still proceeds because `--match-head-commit` binds only the PR head SHA, causing an unverified update to be merged and tagged.
**How this was verified:** The check validation and admin merge execute as separate operations, and the merge command has no condition binding it to the previously observed check state.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[abnegate]

Agreed that the race is not closed, and I am not going to claim otherwise. But it cannot be closed by any refinement of this code, and that is worth stating plainly.

GitHub offers exactly one atomic precondition on a merge: --match-head-commit, which covers the head SHA. There is no "merge only if these checks currently pass" operation. The mechanism that does enforce required checks atomically at merge time is branch protection itself — and --admin exists precisely to bypass branch protection. So any design that admin-merges has an irreducible window between reading check state and issuing the merge.

78241a6 narrows that window from "however long the merge step takes to start" — potentially minutes, since it is a separate workflow step — to a single API round trip. That is a real reduction, not a fix.

Closing it properly requires not bypassing: obtain an approving review from a second identity and merge without--admin, letting branch protection enforce the required checks atomically. That was considered and rejected earlier in favour of admin bypass, on the understanding that the tradeoff was convenience versus least privilege. Your finding shows the tradeoff also includes atomicity, which was not on the table when that call was made, so I have put the decision back to the repository owner rather than silently accepting the residual risk.

Leaving this thread open deliberately: it is an accurate description of the current design, not something I have addressed.

[greptile-apps]

That's a correct and precise characterization of the constraint. GitHub's merge endpoint has no atomic "merge only if current check state is X" precondition — --match-head-commit binds the tree, not the check rollup. The only mechanism that enforces required checks atomically at merge time is branch protection's pre-merge gate, which --admin bypasses by design. So the window is irreducible at this layer.

The narrowing in 78241a6 is still meaningful — collapsing the window from a workflow-step boundary to a single round-trip materially reduces the probability of the race, even if it cannot eliminate it.

Agreed that leaving the thread open is the right call. It's an accurate record of a known, deliberate tradeoff, not an unaddressed bug — and "the repo owner has the decision" is the appropriate place to leave it given the --admin bypass was itself a conscious choice about privilege versus convenience. This thread documents that atomicity belongs in the same tradeoff column.

[abnegate]

Decision on the remaining Greptile P1 (check state races the admin merge): accepted as a known residual risk by the repository owner, deliberately not fixed.

Recording it here so the reasoning is not lost:

• The race is real. A required check that is re-run, or flips to pending or failing, in the window between release() verifying the rollup and the gh pr merge --admin call will be merged past.
• It cannot be closed by refining this code. GitHub's only atomic merge precondition is --match-head-commit, which covers the head SHA and says nothing about checks. The mechanism that enforces required checks atomically at merge time is branch protection, and --admin exists to bypass branch protection.
• Closing it entirely would mean obtaining an approving review from a second identity and merging without --admin. That was weighed and declined: the unattended merge is worth more here than eliminating a single-round-trip window.
• 78241a6 still matters — it narrowed the window from a whole workflow step, potentially minutes, to one API round trip.

The two open threads are left open rather than resolved, because they accurately describe the shipped design. They are not unaddressed oversights.