Skip to content

fix(skills): rebuild stale repo preflight binary#2810

Merged
mergify[bot] merged 2 commits into
mainfrom
fix/issue-2497-skill-preflight-stale-binary
Jun 7, 2026
Merged

fix(skills): rebuild stale repo preflight binary#2810
mergify[bot] merged 2 commits into
mainfrom
fix/issue-2497-skill-preflight-stale-binary

Conversation

@tmchow

@tmchow tmchow commented Jun 7, 2026

Copy link
Copy Markdown
Owner

Summary

Repo-mode /printing-press setup no longer trusts an existing ./cli-printing-press blindly. If the local repo binary is older than the checked-out source version, the preflight rebuilds it before emitting PRINTING_PRESS_BIN; if that rebuild cannot happen, setup stops instead of continuing with a known-stale binary.

Closes #2497

Approach

The setup contract now parses the checked-out source version from internal/version/version.go, compares it with the repo-local binary's version --json, and rebuilds ./cli-printing-press only when the local binary is strictly older. The setup-checks reference documents the new stale/rebuilt markers, and contract tests cover both stale rebuild and fresh no-op paths with an isolated fake repo.

Risk

The changed behavior is limited to repo-mode setup for the main /printing-press skill. Standalone installs and the other setup contracts keep the existing no-rebuild behavior.

Verification

  • go test ./internal/cli -run '^TestPrintingPressSkill'
  • awk '/<!-- PRESS_SETUP_CONTRACT_START -->/{in_contract=1; next} /<!-- PRESS_SETUP_CONTRACT_END -->/{in_contract=0} in_contract && $0 !~ /^```/{print}' skills/printing-press/SKILL.md | bash -n
  • go test ./internal/pipeline -run '^Test(PrintingPressSetupContract|SkillSetupBlocksMatchWorkspaceContract)'
  • go test ./...
  • go fmt ./...
  • go build -o ./cli-printing-press ./cmd/cli-printing-press
  • git diff --check

@mergify

mergify Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟢 require-ready-label-and-ci

Wonderful, this rule succeeded.
  • #changes-requested-reviews-by = 0
  • #review-threads-unresolved = 0
  • check-success = build-and-test
  • check-success = generated-test
  • check-success = go-lint
  • check-success = golden
  • check-success = pr-title
  • check-success = test
  • any of:
    • label = ready-to-merge
    • all of:
      • head = release-please--branches--main
      • title ~= ^chore\(main\): release
  • any of:
    • -files ~= ^(\.github/workflows/|\.github/scripts/|scripts/|\.github/CODEOWNERS$)
    • author = tmchow
    • approved-reviews-by = mvanhorn
    • approved-reviews-by = tmchow
    • author = mvanhorn
  • any of:
    • check-success = Greptile Review
    • label = queued
    • check-neutral = Greptile Review
    • check-skipped = Greptile Review
    • head ~= ^mergify/merge-queue/
    • all of:
      • head = release-please--branches--main
      • title ~= ^chore\(main\): release

@greptile-apps

greptile-apps Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a correctness gap in repo-mode preflight: instead of blindly trusting ./cli-printing-press, the setup contract now parses the checked-out source version from internal/version/version.go, compares it with the local binary's version --json output using the existing _semver_lt helper (relocated earlier in the contract block), and rebuilds before emitting PRINTING_PRESS_BIN when the binary is strictly older.

  • SKILL.md contract_semver_lt is moved from the version-advisory section to before the new _rebuild_local_press_bin_if_stale function; the call site propagates failure correctly via return 1 2>/dev/null || exit 1; _press_repo and _scope_dir are both set before the call.
  • Pipeline contract tests — Two new end-to-end tests drive the full contract script against a fake git repo with a stub go binary; goLogPath is pre-created with empty content so both test cases can os.ReadFile it unconditionally.
  • setup-checks.md — Section 1 documents the new [local-binary-stale]/[local-binary-rebuilt] marker pair so agents know to continue rather than stop when a rebuild succeeds.

Confidence Score: 5/5

Safe to merge; the stale-binary check is limited to repo-mode setup and short-circuits immediately for any non-repo or binary-absent case.

The change is well-scoped: it only runs in repo mode when both _press_repo=true and a local binary exists, correctly inheriting already-set variables. The _semver_lt relocation preserves availability for both the new check and the existing version-advisory section. Contract tests cover the stale-rebuild path end-to-end.

No files require special attention.

Important Files Changed

Filename Overview
skills/printing-press/SKILL.md Adds _semver_lt, _source_press_version, and _rebuild_local_press_bin_if_stale to the setup contract; _semver_lt is relocated earlier to serve both the new stale-binary check and the existing version-advisory section.
internal/pipeline/contracts_test.go Adds two new end-to-end contract tests backed by a runPrintingPressSetupContract helper with a fake git repo and stub go binary; goLogPath is pre-created addressing the prior thread concern.
internal/cli/printing_press_skill_test.go Adds a content-presence test verifying the new shell function names, log markers, and build command appear in SKILL.md and setup-checks.md.
skills/printing-press/references/setup-checks.md Adds documentation for the [local-binary-stale]/[local-binary-rebuilt] marker pair under section 1 so agents know to continue rather than stop when a rebuild succeeds.

Fix All in Codex Fix All in Claude Code Fix All in Cursor Fix All in Conductor

Reviews (2): Last reviewed commit: "test(cli): harden setup contract go log ..." | Re-trigger Greptile

Comment thread internal/pipeline/contracts_test.go
@tmchow tmchow added the ready-to-merge Allow Mergify to queue and merge this PR when protections pass label Jun 7, 2026
mergify Bot added a commit that referenced this pull request Jun 7, 2026
@mergify mergify Bot added the queued PR is in the Mergify merge queue label Jun 7, 2026
mergify Bot added a commit that referenced this pull request Jun 7, 2026
@mergify mergify Bot merged commit cfe9e8f into main Jun 7, 2026
31 checks passed
@mergify mergify Bot deleted the fix/issue-2497-skill-preflight-stale-binary branch June 7, 2026 19:42
@mergify

mergify Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

This pull request spent 32 minutes 12 seconds in the queue, including 31 minutes 55 seconds running CI.

Required conditions to merge

@mergify mergify Bot removed the queued PR is in the Mergify merge queue label Jun 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Allow Mergify to queue and merge this PR when protections pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

skill: reconcile lefthook binary name + preflight staleness check

1 participant