Repository navigation
Conversation
sync-workspace.yml, and boxel-catalog's staging and production deploys, each carried their own copy of the shell that finds the revisions a realm may hold, records the GitHub deployment, and builds the `boxel realm push` arguments. The copies had started to drift. realm-deployment/start finds the revisions (with an optional known revision and a require-revisions guard for the production catalog), builds the --changed-since / --no-realm-events arguments from the CLI's --help, and records an in-progress deployment. realm-deployment/finish marks it success or failure. Composite actions have no post step, so the finish is its own action. sync-workspace.yml uses both by name at @main, since a reusable workflow resolves `./` actions against the caller's checkout. start.sh is tested against stubbed gh and boxel and a scratch repo, run by a path-gated self-test workflow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0730d413d3
ℹ️ 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".
| env: | ||
| ENVIRONMENT: ${{ steps.baseline.outputs.environment }} | ||
| WORKSPACE_URL: ${{ steps.baseline.outputs.environment == 'production' && inputs.production-workspace-url || inputs.staging-workspace-url }} | ||
| uses: cardstack/boxel/.github/actions/realm-deployment/start@main |
There was a problem hiding this comment.
When a consumer pins this reusable workflow to a tag or commit SHA, this reference still downloads the action from the mutable main branch; the matching finish step does the same. Future action changes can therefore alter or break supposedly pinned production syncs, and workflows run from another revision do not exercise their corresponding action implementation. GitHub documents the $/path/to/action self-reference as resolving against the repository and commit containing the workflow, so both composite-action references should use that form.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 54d8c29. Both steps now use $/.github/actions/realm-deployment/{start,finish}. That resolves them in cardstack/boxel at the revision of sync-workspace.yml the caller runs, so a caller that pins the workflow to a tag or SHA gets the actions from that same revision.
habdelra
left a comment
habdelra
left a comment
There was a problem hiding this comment.
[Claude Code 🤖] This review checked two things. First, whether start/finish keep sync-workspace.yml's behavior. Second, whether their inputs can carry boxel-catalog's staging sync and production deploy: the from baseline as known-revision, ref/payload, the catalog/ checkout as repository-path, and the full-push refusal as require-revisions. It did not run the actions on a runner. The remote @main resolution is left to the post-merge item already in the test plan.
No blocking issues. The inputs can carry both catalog callers' semantics. One non-blocking ask: start.sh misbehaves when it runs before the CLI install, and both catalog workflows currently run in that order.
- Fail fast when
boxelisn't on PATH instead of treating it as an old CLI. See the comment onpush_helpinstart.sh.
| # A boxel-cli that doesn't list an option rejects it, so each one is passed | ||
| # only when `realm push --help` lists it. | ||
| push_help=$(boxel realm push --help 2>&1 || true) |
There was a problem hiding this comment.
[Claude Code 🤖] When boxel isn't on PATH, this probe treats it as an old CLI. push-args comes out empty and the step still succeeds, so a staging sync sends every file, with a notice that blames the CLI version. Under require-revisions, the refusal reads "No revision to compare against", which isn't the real cause either. I ran start.sh with boxel missing from PATH: it exits 0 with push-args= empty, and with REQUIRE_REVISIONS=true it exits 1 with that error.
The only thing guarding this is the "after … the boxel-cli install" line in start/action.yml. Both catalog workflows, sync-to-workspace.yml and deploy-production.yml, record their deployment before their Install Boxel CLI step. A step-for-step move onto this action lands right here: staging re-sends the whole catalog, and production refuses with a misleading message.
Please fail fast at the top of start.sh, before anything is listed or recorded:
command -v boxel > /dev/null || {
echo "::error title=$title::boxel isn't on PATH. Run realm-deployment/start after installing @cardstack/boxel-cli."
exit 1
}Please also add a start.test.sh case that removes the boxel stub. (Checking the exit status of --help wouldn't work here: it would turn the intended old-CLI fallback into a failure.)
Regression in the new action. Non-blocking: sync-workspace.yml already installs the CLI before start.
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 54d8c29. start.sh now checks command -v boxel before it lists or records anything. A missing CLI fails the step with "boxel isn't on PATH. Run realm-deployment/start after installing @cardstack/boxel-cli." It no longer falls through to the older-CLI path. start.test.sh has a case that runs it on a PATH with no boxel and expects exit 1 with that error.
start.sh reads the push arguments from `boxel realm push --help`, so a missing CLI looked like an old one and the push would send every file. It now fails before listing or recording anything, with a test case. sync-workspace.yml references the actions as `$/.github/actions/...`, which resolves them at the revision of the reusable workflow the caller runs, so a caller pinned to a tag or SHA gets the matching actions rather than main's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
[Claude Code 🤖] Thanks for the review. The one finding, failing fast when |
Important
Merge order: #6915 has merged; this PR targets
main.sync-to-workspace.ymlanddeploy-production.ymlmove onto these actions in CS-14261. That catalog PR merges after this one, because it calls the actions at@main.What this changes
Three workflows sync a git-sourced realm with
boxel realm push --changed-since, and each carried its own copy of the shell around the push. This PR moves that shell into two composite actions under.github/actions/realm-deployment/:startfinds the revisions the realm may hold, builds the push arguments, and records an in-progress GitHub deployment. It fails before doing any of that whenboxelisn't on PATH. Its push arguments come from the CLI's--help, so a missing CLI would otherwise look like an old one.--changed-sinceper revision, plus--no-realm-events, each only whenboxel realm push --helplists it. The caller splices thepush-argsoutput into its push withread -ra.known-revisiontakes the revisioncatalog-deploy-check.tsreports. That revision stands in for a successful deployment.require-revisionsfails before anything is recorded when the push would send every file.ref,payload,descriptionandrepository-pathcover the rest of what the catalog deploys record.finishmarks the deploymentsuccessorfailurefrom the push step's outcome. Composite actions can't run apoststep, so this is its own action. The caller runs it withif: always().sync-workspace.ymluses both actions as$/.github/actions/realm-deployment/{start,finish}. That form resolves them in cardstack/boxel at the revision of the workflow the caller runs. A./path would resolve against the caller's checkout, and@mainwould ignore a caller's pin. Its concurrency group, checkout, CLI install, profile setup and reporting stay the same.The logic lives in
start.sh, so it can be tested on its own.start.test.shruns it against a scratch git repository, withghandboxelstubbed. The cases cover the success walk, duplicate SHAs, no success, a revision outside the history, a failed listing, a known revision, an older CLI, a missing CLI, and bothrequire-revisionsrefusals. The path-gated Realm Deployment Self-Test workflow runs it and shellcheck.Behavior differences from the inline copies
<environment> realm sync.Test plan
bash .github/actions/realm-deployment/start.test.sh: 12 cases pass. Removing the loop'sbreakon success makes one of them fail.$/yet, and nothing in CI runs it.stagingdeployment. Its log shows the revisions and the--changed-sincearguments, and the deployment finishessuccess.🤖 Generated with Claude Code