fix: smoke test reads container output; run smoke on PRs - #9
Merged
Conversation
The image smoke test read /work/.refine/log.json from the host after the
container run, but the container writes it as root, so a non-root CI runner
(and real Linux users) hit EACCES. Assert on the container's own stdout
markers ('preparing shadow worktree' + 'done: stop_reason=') instead, which
still proves the shadow/git path is sound without reading root-owned files.
Also run the smoke test in the ci.yml 'image build (smoke)' job so it is
validated on every PR (including release PRs) — a smoke failure can no
longer slip through and leave a phantom release tag.
fs.rmSync failed with EACCES trying to unlink the container's root-owned .refine/log.json from the non-root host. Wrap cleanup in try/catch; the ephemeral CI runner reclaims the temp dir regardless.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #7. The v0.1.4 release run's smoke job failed: test 1 (model-ready) passed, but test 2 read
/work/.refine/log.jsonfrom the host after the container run and hitEACCES— the container writes output as root, which a non-root CI runner can't read (Docker Desktop masks this on Mac; real Linux users hit it too).Fix
preparing shadow worktree+done: stop_reason=) plus exit ∈ {0,1}, instead of reading the root-owned bind-mount file. Still proves the shadow/git path is sound (a dubious-ownership abort exits with another code and never printsdone:).image build (smoke)ci.yml job, so it's validated on every PR (including release PRs). A smoke failure can no longer slip through post-merge and leave a phantom release tag.Locally validated: container exits 1 (max_iterations) and prints both markers.
Follow-up (noted, not in scope here)
On Linux,
docker runwrites refined output as root into the user's dir. Worth a--user/chown pass in the CLI later; tracked separately.After merge, release-please cuts 0.1.5, which finally publishes everything (models + pi 0.80.8 + shadow fix) to npm + GHCR. (v0.1.3/v0.1.4 were tagged by earlier failed runs but published nothing; I'll drop those GitHub Releases.)