Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes all remaining Composer v1/provider artifact support, switching the system to generate/serve/deploy only Composer v2 metadata (p2/) and simplifying related DB, build, deploy, and documentation paths.
Changes:
- Remove provider-group–based v1 artifacts (
p/, provider includes/URLs) and stop writing/usingprovider_groupin application logic. - Update repository build output and HTTP serving to
p2/+ updatedpackages.jsonshape (v2-only). - Simplify R2 sync behavior to upload only
p2/files andpackages.json, and update docs/commands accordingly.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| migrations/017_drop_provider_group_index.sql | Drops the packages(provider_group) index (column retained). |
| internal/repository/builder.go | Builds p2/-only artifacts and updates packages.json format; adjusts integrity checks. |
| internal/repository/builder_test.go | Updates build tests for v2-only output and absence of p/. |
| internal/packages/provider_group.go | Removes provider-group computation (deleted). |
| internal/packages/provider_group_test.go | Removes provider-group tests (deleted). |
| internal/packages/package.go | Stops persisting/updating provider_group in package upserts/updates. |
| internal/packages/api_mapper.go | Stops computing ProviderGroup and removes now-unused helper. |
| internal/http/router.go | Stops serving /p/ from local build; keeps /p2/. |
| internal/http/handlers.go | Removes provider_groups from build-row insert for triggered builds. |
| internal/deploy/r2.go | Uploads only p2/ + packages.json; updates R2 cleanup live-release detection logic. |
| internal/deploy/r2_test.go | Updates R2 tests for new upload/cache/cleanup behaviors. |
| internal/deploy/local.go | Removes unused build-walk helper (now unused after deploy changes). |
| internal/deploy/local_test.go | Updates Cache-Control expectations for removed legacy/release paths. |
| docs/r2-deployment.md | Documents new R2 layout and deploy flow (v2-only). |
| docs/operations.md | Updates operational docs for v2-only artifacts and simplified R2 deploy semantics. |
| docs/architecture.md | Updates architecture overview to reflect v2-only repository artifacts. |
| cmd/wpcomposer/cmd/pipeline.go | Removes provider_groups column usage for build row insertion. |
| cmd/wpcomposer/cmd/dev.go | Removes provider_groups column usage when recording dev builds. |
| cmd/wpcomposer/cmd/deploy.go | Refactors R2 sync invocation and previous-build detection for diffing uploads. |
| cmd/wpcomposer/cmd/build.go | Removes incremental build plumbing tied to v1 artifacts; updates build DB writes. |
Comments suppressed due to low confidence (1)
docs/r2-deployment.md:118
- This section says
wpcomposer pipelineautomatically cleans up old R2 releases after each successful deploy, but the pipeline code currently has R2 cleanup explicitly disabled (seecmd/wpcomposer/cmd/pipeline.gocomment about R2 cleanup being disabled). The docs should reflect the actual behavior (manualwpcomposer deploy --cleanup --r2-cleanuponly) or the pipeline should be updated to perform the cleanup again.
### Cleanup stale R2 releases
The `wpcomposer pipeline` command automatically cleans up old R2 releases after each successful deploy, keeping the live release + 5 most recent + releases within a 24-hour grace window. Cleanup is best-effort — failures are logged as warnings but do not fail the pipeline. If cleanup errors persist, run manual cleanup to investigate.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10078c2f7e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Skip unchanged p2/ files | ||
| if previousBuildDir != "" && fileUnchanged(previousBuildDir, buildDir, relPath) { | ||
| skipped.Add(1) |
There was a problem hiding this comment.
Do not skip unchanged p2 files until the bucket is migrated
When previousBuildDir is set but the target bucket does not already have top-level p2/ objects (for example, the first deploy after the old releases/<id>/p2/... layout or after switching to a fresh bucket), this branch skips every unchanged package and then publishes a root packages.json that points at /p2/%package%.json. The skipped packages were never uploaded to that prefix, so Composer requests for unchanged packages start 404ing after the deploy. The old implementation guarded this optimization by checking the live bucket layout first; that migration check is missing here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
p2/ files already exist at the correct top-level keys on R2. The first deploy after migration won't cause any 404s, and the byte-comparison optimization works correctly since the files are already there.
Removes all code and support related to Composer v1 support and the
generated `p/` build files. No uploaded in R2 are deleted; no data in
the database is deleted.
Impact:
1. DB: Dropped index (`idx_packages_provider_group`), not the column. New writes stop populating `provider_group` but the column stays as dead weight.
2. `packages.json` content changes. The root `packages.json` loses:
- `providers-url`
- `provider-includes`
- `build-id`
- `packages` changes from `{}` to `[]`
Composer v2 clients only use `metadata-url` so they're unaffected. But any Composer v1 client would break (which is the intent — Packagist.org already dropped v1 in Sep 2025).
3. New builds on disk no longer contain a `p/` directory — only `p2/`.
4. R2 uploads stop writing to `releases/<build-id>/` prefixes and stop uploading `p/` files. Only `p2/` + `packages.json` go up.
5. Orphaned R2 data — existing `p/` objects, `releases/` prefixes, and content-addressed files remain on R2 but are no longer referenced. These can be deleted later manually.
4107d31 to
0d0f160
Compare
0d0f160 to
a2b69c8
Compare
Removes all code and support related to Composer v1 support and the generated
p/build files. No uploaded in R2 are deleted; no data in the database is deleted.Impact:
idx_packages_provider_group), not the column. New writes stop populatingprovider_groupbut the column stays as dead weight.packages.jsoncontent changes. The rootpackages.jsonloses:providers-urlprovider-includesbuild-idpackageschanges from{}to[]Composer v2 clients only use
metadata-urlso they're unaffected. But any Composer v1 client would break (which is the intent — Packagist.org already dropped v1 in Sep 2025).p/directory — onlyp2/.releases/<build-id>/prefixes and stop uploadingp/files. Onlyp2/+packages.jsongo up.p/objects,releases/prefixes, and content-addressed files remain on R2 but are no longer referenced. These can be deleted later manually.