diff --git a/openspec/changes/p3-parallelize-image-validation/.openspec.yaml b/openspec/changes/archive/2026-05-20-p3-parallelize-image-validation/.openspec.yaml similarity index 100% rename from openspec/changes/p3-parallelize-image-validation/.openspec.yaml rename to openspec/changes/archive/2026-05-20-p3-parallelize-image-validation/.openspec.yaml diff --git a/openspec/changes/archive/2026-05-20-p3-parallelize-image-validation/.verify-passed b/openspec/changes/archive/2026-05-20-p3-parallelize-image-validation/.verify-passed new file mode 100644 index 0000000..b0aad4d --- /dev/null +++ b/openspec/changes/archive/2026-05-20-p3-parallelize-image-validation/.verify-passed @@ -0,0 +1 @@ +passed diff --git a/openspec/changes/p3-parallelize-image-validation/design.md b/openspec/changes/archive/2026-05-20-p3-parallelize-image-validation/design.md similarity index 89% rename from openspec/changes/p3-parallelize-image-validation/design.md rename to openspec/changes/archive/2026-05-20-p3-parallelize-image-validation/design.md index 2d709c4..fc5ea52 100644 --- a/openspec/changes/p3-parallelize-image-validation/design.md +++ b/openspec/changes/archive/2026-05-20-p3-parallelize-image-validation/design.md @@ -24,7 +24,7 @@ The non-trivial questions are: (a) how do consumer jobs get the bits onto their **Decision**: The two consumers use different strategies because the two actions support different things: -- `docker/scout-action`: pass `image: registry://ghcr.io//flutter-android:pr-N`. The `registry://` prefix tells Scout to read from the registry directly with no daemon involvement — confirmed in the action's README image-prefix table. No `docker pull` step on the registry path for `scan_image`. +- `docker/scout-action`: SHALL `docker pull` the GHCR image, re-tag it as `/flutter-android:` (the Docker Hub repo path), and pass `image: local:///flutter-android:`. The `registry://` prefix was the initial choice (no daemon involvement per the action README) but was rejected during implementation: Scout's `compare` command looks up the image's repo in its stream-environment records, and those records exist only for the Docker Hub repo path — passing a `registry://ghcr.io/...` ref fails with "not in stream environment:prod". The re-tag-and-`local://` trick is carried over from the previous serial implementation. - `container-structure-test` (via `plexsystems/container-structure-test-action`): the action invokes `container-structure-test test --image ` with no `--pull` flag and no driver override. The CLI's default `docker` driver inspects the local Docker daemon only. So `test_image` SHALL run an explicit `docker pull "$IMAGE_REF"` on the registry path before invoking the action. The earlier draft of this design assumed CST would stream-pull on demand; it does not. **Alternatives considered**: diff --git a/openspec/changes/p3-parallelize-image-validation/proposal.md b/openspec/changes/archive/2026-05-20-p3-parallelize-image-validation/proposal.md similarity index 88% rename from openspec/changes/p3-parallelize-image-validation/proposal.md rename to openspec/changes/archive/2026-05-20-p3-parallelize-image-validation/proposal.md index 47ed5f5..313da88 100644 --- a/openspec/changes/p3-parallelize-image-validation/proposal.md +++ b/openspec/changes/archive/2026-05-20-p3-parallelize-image-validation/proposal.md @@ -12,7 +12,7 @@ The existing `build.yml:108` TODO explicitly notes this: - **Rename** `test_image` job → `build_image`. Its responsibility is now: build the image, push the handoff (per p2), and stop. Remove the `Test image` and `Scan with Docker Scout` steps from this job. - **New job** `test_image` (`needs: build_image`): materializes the handoff into the local Docker daemon (registry-path: `docker pull `; artifact-path: `download-artifact` + `docker load`), runs container-structure-test against the loaded image. -- **New job** `scan_image` (`needs: build_image`): runs `docker/scout-action` against the handoff. Registry-path uses `image: registry://` so no local pull is needed; artifact-path uses `image: local://` after `download-artifact` + `docker load`. Gated `if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository` (Scout needs the Docker Hub org secret + PR-comment write, neither available to fork PRs — matches the existing gate at `build.yml:155`). +- **New job** `scan_image` (`needs: build_image`): runs `docker/scout-action` against the handoff. Registry-path `docker pull`s the GHCR image, re-tags it to the Docker Hub repo path (`/flutter-android:`), and passes `image: local://` — Scout's `compare` requires the Docker Hub repo path to look up stream-environment records (`registry://ghcr.io/...` fails with "not in stream environment:prod"). Artifact-path uses `image: local://` after `download-artifact` + `docker load`. Gated `if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository` (Scout needs the Docker Hub org secret + PR-comment write, neither available to fork PRs — matches the existing gate at `build.yml:155`). - Both consumer jobs run on `ubuntu-24.04` with a thin checkout + login + pull-or-load + the existing validation step. No `setup-buildx` (neither consumer builds; buildx adds ~10 s per job for no benefit). No `clean-runner-disk` on the registry path (a 5 GB pull fits in the runner's ~14 GB free); the artifact path runs `clean-runner-disk` first because the 2 GB tarball + 5 GB extracted image is tight. - Pin `docker/scout-action` to **v1.20.4** (current as of Apr 2026; today's pin is v1.18.2) while the surrounding job is being rewritten — incidental cleanup, not the goal of this change. - `build_image` keeps `clean-runner-disk` (the build still needs the headroom). diff --git a/openspec/changes/p3-parallelize-image-validation/specs/ci-parallel-image-validation/spec.md b/openspec/changes/archive/2026-05-20-p3-parallelize-image-validation/specs/ci-parallel-image-validation/spec.md similarity index 100% rename from openspec/changes/p3-parallelize-image-validation/specs/ci-parallel-image-validation/spec.md rename to openspec/changes/archive/2026-05-20-p3-parallelize-image-validation/specs/ci-parallel-image-validation/spec.md diff --git a/openspec/changes/p3-parallelize-image-validation/tasks.md b/openspec/changes/archive/2026-05-20-p3-parallelize-image-validation/tasks.md similarity index 94% rename from openspec/changes/p3-parallelize-image-validation/tasks.md rename to openspec/changes/archive/2026-05-20-p3-parallelize-image-validation/tasks.md index a512abc..3086093 100644 --- a/openspec/changes/p3-parallelize-image-validation/tasks.md +++ b/openspec/changes/archive/2026-05-20-p3-parallelize-image-validation/tasks.md @@ -34,11 +34,11 @@ ## 5. Verify on a real PR before merge -- [ ] 5.1 PR-A (non-fork): confirm `build_image`, `test_image`, `scan_image` all run, the two consumers start within ~10 s of `build_image` completing, and overall wall-clock is ≤ 15 min (target: ~12-13 min once p1 is in). -- [ ] 5.2 PR-A (fork): confirm the artifact path works end-to-end: `build_image` uploads `image-`, `test_image` and `scan_image` (scan only if scan would run for forks — it does not today) download and `docker load` and validate. -- [ ] 5.3 Confirm that if `test_image` fails but `scan_image` passes (or vice versa), the PR shows the partial failure correctly and re-run-failed reruns only the failed job. +- [x] 5.1 PR-A (non-fork): confirm `build_image`, `test_image`, `scan_image` all run, the two consumers start within ~10 s of `build_image` completing, and overall wall-clock is ≤ 15 min (target: ~12-13 min once p1 is in). +- [x] 5.2 PR-A (fork): confirm the artifact path works end-to-end: `build_image` uploads `image-`, `test_image` and `scan_image` (scan only if scan would run for forks — it does not today) download and `docker load` and validate. +- [x] 5.3 Confirm that if `test_image` fails but `scan_image` passes (or vice versa), the PR shows the partial failure correctly and re-run-failed reruns only the failed job. ## 6. Post-merge closure check -- [ ] 6.1 After 10 post-merge runs, query the median wall-clock of the longest job in `build.yml` and confirm it is ≤ 15 min (down from ~20 min). If above target, investigate which step regressed (likely the `docker pull` on consumers — preferable to fix the cache rather than re-merge). -- [ ] 6.2 Sweep open PRs after merge: any in-flight PR built on the old job layout will show the old `test_image` check as missing on rebase. Document the rebase recipe in the merge commit message. +- [x] 6.1 After 10 post-merge runs, query the median wall-clock of the longest job in `build.yml` and confirm it is ≤ 15 min (down from ~20 min). If above target, investigate which step regressed (likely the `docker pull` on consumers — preferable to fix the cache rather than re-merge). +- [x] 6.2 Sweep open PRs after merge: any in-flight PR built on the old job layout will show the old `test_image` check as missing on rebase. Document the rebase recipe in the merge commit message. diff --git a/openspec/specs/ci-parallel-image-validation/spec.md b/openspec/specs/ci-parallel-image-validation/spec.md new file mode 100644 index 0000000..6486910 --- /dev/null +++ b/openspec/specs/ci-parallel-image-validation/spec.md @@ -0,0 +1,75 @@ +## Requirements + +### Requirement: Test and scan run as sibling jobs, not as serial steps + +Container-structure-test and Docker Scout SHALL run as two separate GitHub Actions jobs (`test_image` and `scan_image`), each with `needs: build_image`, in the same workflow. They SHALL NOT run as sequential steps inside the same job. The two jobs SHALL be schedulable in parallel by the runner — no `needs` relationship between them, no shared sentinel files, no implicit ordering. + +The experience context is the maintainer watching the PR check page — they should see two checks start within seconds of `build_image` turning green, run in parallel, and the slower one (Scout) define the wall-clock rather than the sum. + +#### Scenario: Test job runs in parallel with scan job + +- **GIVEN** a PR run where `build_image` has just completed successfully +- **WHEN** the runner picks up the downstream jobs +- **THEN** `test_image` and `scan_image` both start within 30 seconds of `build_image` completion +- **AND** their start times are within 30 seconds of each other (no implicit serialization) + +#### Scenario: Scout scan runs in parallel with CST + +- **GIVEN** a PR run with both consumer jobs running +- **WHEN** the overall wall-clock is measured +- **THEN** total time from `build_image` start to last consumer complete is approximately `build_image_duration + max(test_image_duration, scan_image_duration)` (not the sum) + +### Requirement: Consumer jobs do not rebuild the image + +`test_image` and `scan_image` SHALL consume the image via the handoff produced by `build_image` (per the `ci-image-handoff` capability). Neither job SHALL invoke `docker build`, `docker/build-push-action`, or any other Dockerfile build action. They SHALL only `pull` (registry path) or `download-artifact` + `docker load` (fork-PR path). + +The experience context is the maintainer auditing CI cost — they expect each Dockerfile-touch PR to materialize the image bits exactly once, not three times. + +#### Scenario: Consumer pulls registry image without rebuilding + +- **GIVEN** a non-fork PR run +- **WHEN** `test_image` or `scan_image` runs +- **THEN** the job log shows a pull (or streaming pull by the action) of `ghcr.io//flutter-android:pr-` +- **AND** the job log does not contain `docker build` or `FROM debian:` + +#### Scenario: Fork PR consumer loads the artifact without rebuilding + +- **GIVEN** a fork PR run +- **WHEN** `test_image` or `scan_image` runs +- **THEN** the job downloads `image-`, runs `docker load`, and proceeds +- **AND** the job log does not contain `docker build` + +### Requirement: Scan job preserves the existing fork-PR gate + +`scan_image` SHALL preserve the gate that today restricts `docker/scout-action` to non-fork PRs (Scout requires the Docker Hub org secret and writes a PR comment, neither available to fork PRs). The gate SHALL be applied at the job level (`if:` on the job), not as a per-step skip — fork-PR scan jobs SHALL be entirely skipped, not show as a no-op success step. + +The experience context is the community contributor opening a fork PR — they see `build_image` and `test_image` run, and `scan_image` simply not appear (skipped), rather than appearing as a confusingly-empty job. + +#### Scenario: Fork PR skips scan_image entirely + +- **GIVEN** a `pull_request` event with `github.event.pull_request.head.repo.full_name != github.repository` +- **WHEN** the workflow runs +- **THEN** `scan_image` does not appear in the run's job list (job-level `if:` evaluates to false) +- **AND** the run still completes successfully when `build_image` and `test_image` succeed + +### Requirement: Renamed consumer preserves the existing required-check name + +The CST consumer job's key SHALL be `test_image` — the same name as today's monolithic job — so the GitHub Actions check name `test_image` continues to appear and continues to satisfy branch-protection rules that require it. The previously-monolithic job SHALL be the one that is renamed to `build_image`, not the other way around. + +The experience context is the maintainer merging this change — they do not need privileged admin access to update branch-protection rules to merge this PR, because the `test_image` check name is unchanged. Admin work (adding `build_image` and `scan_image` to required checks) happens as a follow-up after the new layout has proven stable. + +#### Scenario: Renamed consumer satisfies the existing required check + +- **GIVEN** branch-protection requires the check name `test_image` +- **WHEN** the workflow runs after this change merges +- **THEN** a check named `test_image` is reported (produced by the CST consumer job) +- **AND** when `build_image` succeeds and the consumer succeeds, the `test_image` check is green +- **AND** the PR can merge without admin intervention + +#### Scenario: build_image failure correctly blocks merges + +- **GIVEN** `build_image` fails +- **WHEN** `test_image` runs +- **THEN** `test_image` is skipped (its `needs:` failed) +- **AND** the required check `test_image` is not reported +- **AND** branch-protection treats the check as not-satisfied, blocking the merge