Skip to content

ci(repo): mirror slim images and native OCI artifacts to ECR Public - #6661

Merged
avallete merged 2 commits into
developfrom
avallete/slim-artifact-mirrors
Sep 22, 2026
Merged

avallete merged 2 commits into
developfrom
avallete/slim-artifact-mirrors

Conversation

@avallete

@avallete avallete commented Sep 17, 2026

Copy link
Copy Markdown
Member

Summary

  • Extract GHCR → ECR Public slim-image mirroring into bun scripts (validate, digest checks, repo create, copy-image, copy-natives).
  • Copy :version-native-<target> OCI archives best-effort after the digest-preserving image copy; catalog sync still consumes only image service / version / digest.
  • ADR 0026 records the publish path. CLI/stack fail-through is a follow-up PR.

Companion: supabase/slim-services#308

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superseded by a newer AI review

🤖 AI Review

Adjudicated all 20 reported findings into 19 deduplicated entries. Seventeen are confirmed, one is refuted, and one remains uncertain. The main confirmed risks are regressions for custom registry users and hard-coded OCI authentication; the shared inspection-error misclassification was reported by both reviewers.

Findings

Severity Location Category Sources Claim
🟠 MAJOR packages/stack/src/model/SlimArtifactMirrors.ts:122 compatibility claude Applying the existing image-registry override to native artifacts removes the GitHub Releases fallback and requires the custom registry to carry the newly introduced OCI-native artifacts, breaking existing native users whose mirror only contains images.
🟠 MAJOR packages/stack/src/preparation/SlimNativeOci.ts:32 compatibility codex Custom native registries are not resolved through the registry authentication protocol: the client always requests a hard-coded HTTPS /token endpoint before accessing the registry.
🟡 MINOR packages/stack/src/preparation/SlimServicesSource.ts:386 performance claude Every cache miss resolves the upstream checksum twice because ArtifactStore calls checksum before materialize, while materialize ignores that result and fetches candidate metadata again.
🟡 MINOR packages/stack/src/preparation/RuntimeArtifacts.ts:170 error-classification claude+codex Image inspection failures are incorrectly converted into ContainerPullError even though no pull may have occurred.
🟡 MINOR packages/stack/src/preparation/SlimNativeOci.ts:160 integrity claude The OCI archive is not verified against its manifest descriptor digest; archiveDigest is returned but discarded, and verification uses only SHA256SUMS fetched from the same candidate.
🟡 MINOR packages/stack/src/runtime/resolve-container-image.ts:31 error-handling claude When every image pull candidate fails, the resolver discards all failures except the final candidate's error.
🟡 MINOR packages/stack/src/runtime/resolve-container-image.ts:14 test-coverage claude The new container-image resolver has no test covering first-candidate pull failure followed by second-candidate success, or exhaustion of all candidates.
🟡 MINOR packages/stack/src/preparation/SlimServicesSource.ts:216 configuration claude Stack mirror selection bypasses the provided ConfigProvider and project dotenv values by reading the ambient environment directly.
🟡 MINOR packages/stack/src/preparation/SlimNativeOci.ts:6 maintainability claude Four new exports are unused outside SlimNativeOci and are eligible for unused-export failures in the repository's Knip check.
🟡 MINOR apps/cli/src/command-internal/docker-registry.ts:67 behavior-change claude A custom registry override now rewrites slim images to a single custom-registry candidate, changing the previously shipped behavior in which slim references bypassed the override.
🟡 MINOR apps/cli/src/shared/services/slim-images.ts:149 test-coverage claude The newly accepted ECR slim-image namespace is not covered by CLI tests that protect slim-specific runtime specifications.
🟡 MINOR .github/workflows/mirror-slim-image.yml:190 observability claude Best-effort native mirroring relies on a stated daily drift audit, but no such audit is defined in this repository.
🟡 MINOR packages/stack/src/preparation/SlimNativeOci.ts:60 error-handling codex A malformed OCI title annotation can throw a defect and bypass candidate failover.
⚪ NIT packages/stack/src/preparation/SlimServicesSource.ts:243 error-handling claude Native candidate aggregation retains only each failure's message and drops the underlying causes and structured context.
⚪ NIT packages/stack/src/runtime/resolve-container-image.ts:37 dead-code claude The empty-candidate fallback is unreachable and would incorrectly label an unchecked image as cached if reached.
⚪ NIT packages/stack/src/model/SlimArtifactMirrors.ts:26 dead-code claude The detected claude host hint never affects candidate ordering and is behaviorally identical to default.
⚪ NIT apps/cli/src/command-internal/docker-registry.ts:70 style claude Slim registry option construction is duplicated, and overrideValue is evaluated twice in each copy.
⚪ NIT apps/cli/src/commands/db/shared/pgdelta.seam.layer.ts:263 documentation claude The comment saying slim references never pass through a registry mirror is now false.

Findings outside the diff

  • ⚪ NIT apps/cli/src/commands/db/shared/pgdelta.seam.layer.ts:263 — The comment saying slim references never pass through a registry mirror is now false.
Refuted findings (kept for transparency, not posted as review comments)
  • packages/stack/src/preparation/SlimServicesSource.ts:390 (correctness): Reusing compressedPath across candidate attempts can append a later download to a partial earlier archive because no explicit write flag is supplied.
    Refuted: Effect FileSystem.sink defaults to write/truncate semantics. The integration test at slim-services.integration.test.ts:575-635 exercises this exact reuse: OCI writes the archive then fails its stale checksum, a later GitHub attempt writes the same path, and the resulting archive extracts successfully.

Stats

Claude findings: 17 · Codex findings: 3 · Confirmed: 17 · Refuted: 1 · Uncertain: 1


Models: claude-opus-5 + gpt-5.6-sol · Trigger: auto · Workflow run

This review runs once per PR. A maintainer can request another with a /ai-review comment.

Comment thread packages/stack/src/runtime/SlimArtifactMirrors.ts Outdated
Comment thread packages/stack/src/preparation/SlimNativeOci.ts Outdated
Comment thread packages/stack/src/preparation/SlimServicesSource.ts Outdated
Comment thread packages/stack/src/preparation/RuntimeArtifacts.ts Outdated
Comment thread packages/stack/src/preparation/SlimNativeOci.ts Outdated
Comment thread packages/stack/src/preparation/SlimServicesSource.ts Outdated
Comment thread packages/stack/src/runtime/resolve-container-image.ts Outdated
Comment thread packages/stack/src/runtime/SlimArtifactMirrors.ts Outdated
Comment thread apps/cli/src/command-internal/docker-registry.ts Outdated
Comment thread packages/stack/src/preparation/SlimNativeOci.ts Outdated
Comment thread .github/scripts/mirror-slim-image.ts
Comment thread packages/stack/src/preparation/SlimServicesSource.ts Outdated
Comment thread packages/stack/src/preparation/SlimNativeOci.ts Outdated
Publish digest-preserving GHCR copies of slim catalog images and
:version-native-<target> archives so later CLI fail-through has both hosts.
@avallete
avallete force-pushed the avallete/slim-artifact-mirrors branch from 1311cb0 to ae78c71 Compare September 21, 2026 15:06
@avallete avallete changed the title feat(stack): fail through slim image and native artifact mirrors ci(repo): mirror slim images and native OCI artifacts to ECR Public Sep 21, 2026
Knip treats those internals as unused exports and fails Check code quality.
@avallete

Copy link
Copy Markdown
Member Author

/ai-review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 AI Review

Both independent reviews completed. All 10 deduplicated findings were verified against the checked-out code and trusted repository conventions. No critical or major issues were found; the confirmed findings concern native-copy robustness, diagnostics, coverage, and two repository-convention violations.

Findings

Severity Location Category Sources Claim
🟡 MINOR .github/scripts/slim-mirror-payload.ts:100 error-handling claude One unrecognized native tag aborts the entire native-copy step before any otherwise valid native entries are mirrored.
🟡 MINOR .github/scripts/slim-mirror-payload.ts:90 input-validation claude parseNatives neither bounds nor deduplicates the native array, allowing duplicate valid entries to consume the workflow timeout through repeated registry operations.
🟡 MINOR .github/scripts/mirror-slim-image.ts:141 observability claude Subprocess output is buffered rather than streamed, and copyImage discards stdout when reporting a failed copy.
🟡 MINOR .github/scripts/mirror-slim-image.ts:64 test-coverage claude The test suite does not exercise the real GITHUB_OUTPUT file-writing path used to pass validated values to subsequent workflow steps.
🟡 MINOR .github/scripts/mirror-slim-image.ts:201 design claude A manual workflow_dispatch cannot replay or repair native copies because native entries are read only for repository_dispatch events.
🟡 MINOR .github/scripts/mirror-slim-image.ts:279 correctness claude main returns the number of failed native copies directly as the process exit code, so counts divisible by 256 can wrap to a successful shell status.
🟡 MINOR .github/scripts/mirror-slim-image.ts:30 repository-convention codex The new TypeScript runtime uses raw Promise/async orchestration instead of Effect, contrary to the repository's explicit runtime-code requirement.
⚪ NIT .github/scripts/mirror-slim-image.ts:272 input-validation claude The copy-natives command checks only that SERVICE and VERSION are nonempty rather than applying the payload patterns used by validate.
⚪ NIT .github/scripts/mirror-slim-image.ts:82 error-handling claude A successful manifest-head command that emits empty stdout produces a digest-mismatch message with a blank resolved value and no diagnostic detail.
⚪ NIT .github/scripts/mirror-slim-image.test.ts:1 test-organization codex The new test filename does not use one of the repository's required test suffixes.

Stats

Claude findings: 8 · Codex findings: 2 · Confirmed: 10 · Refuted: 0 · Uncertain: 0


Models: claude-opus-5 + gpt-5.6-sol · Trigger: manual · Workflow run

This review runs once per PR. A maintainer can request another with a /ai-review comment.

Comment thread .github/scripts/slim-mirror-payload.ts
Comment thread .github/scripts/slim-mirror-payload.ts
Comment thread .github/scripts/mirror-slim-image.ts
Comment thread .github/scripts/mirror-slim-image.ts
Comment thread .github/scripts/mirror-slim-image.ts
Comment thread .github/scripts/mirror-slim-image.ts
Comment thread .github/scripts/mirror-slim-image.ts
Comment thread .github/scripts/mirror-slim-image.ts
Comment thread .github/scripts/mirror-slim-image.ts
Comment thread .github/scripts/mirror-slim-image.test.ts
@avallete
avallete added this pull request to the merge queue Sep 22, 2026
Merged via the queue into develop with commit 0456aa3 Sep 22, 2026
206 of 216 checks passed
@avallete
avallete deleted the avallete/slim-artifact-mirrors branch September 22, 2026 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants