Ship chore/shipyard-0.22.3-installer-wrapper-hooks-schema - #617
Conversation
(A) install-shipyard.sh — rewrite as a thin wrapper around Shipyard's
official installer (v0.22.1+ `SHIPYARD_VERSION` + `SHIPYARD_INSTALL_DIR`
env-var support). Binary now lands at `~/.local/bin/shipyard`, the
canonical location that upstream owns.
The old script installed to `~/.pulp/shipyard/<v>/shipyard` with a
symlink at `~/.pulp/bin/shipyard`. When that pulp-private bin dir came
first on PATH, the stale symlink shadowed the canonical
`~/.local/bin/shipyard` silently — users would run an old binary
without realizing. Cleanup step in the new script removes
`~/.pulp/bin/shipyard` + `~/.pulp/shipyard/` on install so the
upstream PATH entry takes over cleanly.
Pin bumped `tools/shipyard.toml` → v0.22.3:
- v0.22.1: install.sh env-var support (the reason this wrapper is
possible at all).
- v0.22.2: completes the daemon-spawn fix that v0.22.1's commit
message advertised but didn't actually include.
- v0.22.3: removes "Automated by Shipyard." branding and "Ship"
internal-term leakage from auto-opened PR bodies so Pulp PRs
read as first-party.
Verified end-to-end locally:
./tools/install-shipyard.sh --status # reports pinned v0.22.3
./tools/install-shipyard.sh # delegates, lands 0.22.3
shipyard --version # → "shipyard, version 0.22.3"
(B) hooks/hooks.json — wrap the top-level event map under a `hooks`
key. Claude Code's plugin loader requires the nested form:
{ "hooks": { "PostToolUse": [...] } }
not the flat form Pulp was using:
{ "PostToolUse": [...] }
Every known-working plugin (ralph-loop, prompt-repeater) uses the
nested form. Pulp's PostToolUse hooks — `docs-reminder.sh` and
`cli-plugin-sync.sh` — were silently no-ops on session-load. No
logic change, just the schema wrapper.
Version-Bump: sdk=patch reason="installer wrapper + hooks schema fix"
Skill-Update: skip skill=ci reason="no skill content change"
![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: 0eaf4129d5
ℹ️ 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".
| echo "→ Installing Shipyard $VERSION via upstream install.sh" | ||
| echo " source: $UPSTREAM_INSTALLER" | ||
|
|
||
| # Re-repair the queue file on fresh-install path too (covers --force and | ||
| # first-time install). The pre-short-circuit call above handles the | ||
| # "already installed but queue truncated" case. | ||
| repair_truncated_queue_file | ||
| SHIPYARD_VERSION="$VERSION" bash <(curl -fsSL "$UPSTREAM_INSTALLER") |
There was a problem hiding this comment.
This wrapper no longer repairs a zero-byte queue.json before returning, so the documented recovery path (./tools/install-shipyard.sh) no longer fixes the JSONDecodeError failure mode from #528. The existing regression test tools/scripts/test_install_shipyard_queue_repair.py now fails in the “already installed” scenario because the queue file stays empty, which means a truncated queue can still block all subsequent Shipyard invocations.
Useful? React with 👍 / 👎.
| SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" | ||
| PULP_ROOT="$(dirname "$SCRIPT_DIR")" | ||
| PIN_FILE="$SCRIPT_DIR/shipyard.toml" | ||
| UPSTREAM_INSTALLER="https://raw.githubusercontent.com/danielraffel/Shipyard/main/install.sh" |
There was a problem hiding this comment.
Downloading and executing install.sh from the main branch introduces non-deterministic and unaudited behavior: the same Pulp commit can run different installer code over time. This weakens the repository’s Shipyard pinning model and reintroduces supply-chain risk that the previous release-asset + checksum flow avoided.
Useful? React with 👍 / 👎.
| if [ -L "$HOME/.pulp/bin/shipyard" ]; then | ||
| echo "→ Removing legacy symlink $HOME/.pulp/bin/shipyard" | ||
| rm -f "$HOME/.pulp/bin/shipyard" |
There was a problem hiding this comment.
The script removes the legacy ~/.pulp/bin/shipyard symlink before attempting the network install. If curl or the upstream installer fails, users who still resolve shipyard through that symlink lose their only working binary in that session. Cleanup should happen only after a successful replacement (or preserve a rollback path).
Useful? React with 👍 / 👎.
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. |
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"
1 participant
No description provided.