Skip to content

fix(cli): add npm version check to squad upgrade command#96

Open
diberry wants to merge 1 commit intodevfrom
squad/46-upgrade-cli-check
Open

fix(cli): add npm version check to squad upgrade command#96
diberry wants to merge 1 commit intodevfrom
squad/46-upgrade-cli-check

Conversation

@diberry
Copy link
Copy Markdown
Owner

@diberry diberry commented Mar 28, 2026

Closes #46

Adds npm version check to the squad upgrade command to detect when an upgrade is available.

Fork PR for Copilot review gate.

@diberry
Copy link
Copy Markdown
Owner Author

diberry commented Mar 29, 2026

🐕 FIDO Quality Review — PR #96 (Upgrade CLI Check)

Verdict: 🔴 Reject — scope creep with dangerous deletions

✅ The core feature is solid

  • checkForNewerCLI() is well-designed: returns structured CLIUpdateCheck data, separating concern from the banner display in notifyIfUpdateAvailable().
  • bypassCache option for the upgrade path is correct — you want a fresh check after upgrading.
  • Cache TTL (24h), fetch timeout (3s), SQUAD_NO_UPDATE_CHECK opt-out all good.
  • Tests cover: network failure → null, opt-out, cache hit, prerelease handling, malformed version, bypassCache.

🔴 BLOCKING: Massive unrelated file deletions

This PR deletes 8 files / 1,240+ lines that have nothing to do with the npm version check feature:

Deleted File Impact
.github/workflows/squad-ci.yml (partial) Removes changelog-gate, exports-map-check, and samples-build CI jobs — 265 lines of CI protection gone
.github/PR_REQUIREMENTS.md PR review guidance deleted
.github/PULL_REQUEST_TEMPLATE.md PR template deleted
scripts/check-exports-map.mjs Exports validation script deleted
test/check-exports-map.test.ts Tests for above deleted
test/guard-removal.test.ts Guard removal tests deleted (211 lines)
.copilot/skills/bleed-check/SKILL.md Skill definition deleted
.squad/skills/fork-first-pipeline/SKILL.md Skill definition deleted

These deletions are not mentioned in the PR description, not justified, and remove safety infrastructure. The CI jobs in particular (changelog-gate, exports-map-check) were protecting against quality regressions. They need their own PR with rationale.

⚠️ Additional notes (would be non-blocking if scope were clean)

  1. Malformed version handling is implicit: When npm returns 'not-a-version', compareVersions() calls parseVersion() which throws, caught by the outer try/catch → returns null. Works, but if someone changes parseVersion to not throw (returning defaults instead), this silently becomes a bug. An explicit format check before comparison would be safer.

  2. fetchLatestVersion doesn't validate the returned string — just returns whatever dist-tags.latest contains from npm. Add a semver format check.

  3. Decision files added are goodbooster-ci-deletion-guard.md and retro-copilot-git-safety.md document important decisions.

Action required

Split out the file deletions into a separate PR with justification. The core checkForNewerCLI feature can ship independently.

diberry pushed a commit that referenced this pull request Mar 29, 2026
Restores 8 deleted files (skills, CI template, PR template, tests, docs,
scripts) and reverts unrelated modifications to squad-ci.yml and decisions
files. Removes 2 unrelated inbox decision files added out of scope.

The PR should only contain the npm version check feature changes.

Closes #96

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@diberry diberry added squad:pr-needs-review PR lifecycle: awaiting team review and removed squad:pr-needs-review PR lifecycle: awaiting team review labels Mar 31, 2026
@diberry diberry marked this pull request as ready for review April 1, 2026 03:38
@diberry diberry added the squad:pr-needs-preparation PR lifecycle: needs rebase and squash label Apr 1, 2026
@diberry diberry force-pushed the squad/46-upgrade-cli-check branch 2 times, most recently from 8028b26 to 1b16f0d Compare April 1, 2026 04:13
@diberry diberry added squad:pr-needs-review PR lifecycle: awaiting team review and removed squad:pr-needs-preparation PR lifecycle: needs rebase and squash labels Apr 1, 2026
@diberry
Copy link
Copy Markdown
Owner Author

diberry commented Apr 1, 2026

🔧 Preparation Complete

Advancing to squad:pr-needs-review.

@diberry diberry force-pushed the squad/46-upgrade-cli-check branch from 1b16f0d to 766769a Compare April 1, 2026 04:32
When squad upgrade reports project files are up to date, also check
npm for a newer CLI version and display a nudge if available.

- Extract checkForNewerCLI() from self-update.ts for reuse
- Wire npm check into upgrade command path (cli-entry.ts)
- Change 'Already up to date' -> 'Project files up to date' for clarity
- Refactor notifyIfUpdateAvailable() to use shared check function
- Add ./self-update export to package.json (required for test imports)
- Add CHANGELOG entry under [Unreleased] (single entry, removed stale duplicate section)

Closes #46

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@diberry diberry force-pushed the squad/46-upgrade-cli-check branch from 766769a to 36f03d9 Compare April 1, 2026 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

squad:pr-needs-review PR lifecycle: awaiting team review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant