Ship fix/codex-sweep-wave-6-installer-coverage - #622
Merged
Conversation
Four Codex findings on merged PRs #616 and #617. All addressed here. pulp#617 P1 — pin the upstream installer URL to an immutable tag. The wrapper downloaded `install.sh` from `main`, so the same Pulp commit could run different installer code over time — weakens the pinning model and is a supply-chain smell. Now fetches `refs/tags/<VERSION>/install.sh` where VERSION comes from `tools/shipyard.toml`. Verified the tag-pinned URL resolves (200) for v0.22.8 and a full end-to-end install still succeeds. pulp#617 P1 — restore queue-file repair. My original rewrite dropped `repair_truncated_queue_file()`, which handled the #528 failure mode (JSONDecodeError on a zero-byte queue.json after a crash between `open(O_TRUNC)` and `write()`). Re-running the wrapper is the documented recovery path; without the repair step it stopped fixing the thing it's meant to fix. The regression test `test_install_shipyard.py` still exercises this path. pulp#617 P2 — move legacy `~/.pulp/bin/shipyard` cleanup to AFTER the upstream installer succeeds. Previously cleanup ran first, so a curl failure deleted the user's only working shipyard symlink. The wrapper is idempotent; cleanup on the next successful run is fine. pulp#616 P2 — also pick up `pulp-*.lib` archives on Windows. `scripts/run_coverage.sh` switches to `clang-cl` on Windows and CMake emits `.lib` archives there (MSVC-style), not `.a`. The full-surface expansion was silently skipped on the Windows matrix leg — the very thing the expansion was supposed to close. The `pulp-*.lib` prefix is narrow enough to skip third-party `.lib` files under _deps/. Verified locally (macOS): bash -n tools/install-shipyard.sh scripts/run_coverage.sh # syntax ./tools/install-shipyard.sh --status # reads pin OK ./tools/install-shipyard.sh # tag-pinned URL, clean install shipyard --version # → 0.22.8 Skill-Update: skip skill=ci reason="installer + coverage-script polish, no skill content change"
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Diff coverage (required)Diff coverage threshold: 75% (required). diff-cover reportDiff: origin/main...HEAD, staged and unstaged changesNo lines with coverage information in this diff. See coverage.md for how to interpret this number and the full Phase 1 → Phase 3 roadmap. Per-tier diff coverage (#566 Phase 2)
All touched tiers meet their per-tier floors. |
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.
No description provided.