Skip to content

fix(cli): doctor ignores unrelated app node_modules for dependency checks - #2391

Open
pujitha24 wants to merge 1 commit into
github:devfrom
pujitha24:auto/issue-2367
Open

pujitha24 wants to merge 1 commit into
github:devfrom
pujitha24:auto/issue-2367

Conversation

@pujitha24

Copy link
Copy Markdown
Contributor

What

squad doctor no longer shows the "run npm install or check dependencies" warning for the vscode-jsonrpc and copilot-sdk checks when the current directory only has an unrelated node_modules. That advice now appears only when cwd is a Squad checkout (packages/squad-cli) or has @bradygaster/squad-cli installed locally. In all other cases, the existing info-level "expected for global installs" result is returned.

Why

squad doctor treated any existing <cwd>/node_modules as a Squad development context. In a standalone install, that directory belongs to the host application. The warning told users to run npm install, which cannot repair the runtime Squad actually uses.

Closes #2367

How

The "run npm install" warning is now gated on cwd being a Squad checkout or having @bradygaster/squad-cli installed locally. Real failures (malformed exports, unpatched session.js) are unchanged.

This PR does not resolve and check the invoked installation's own runtime dependencies. That is a larger change and is left out. The only effect here is that the misleading warning is no longer shown for an unrelated node_modules.

Tests: added two tests in test/cli/doctor.test.ts.

  • An unrelated node_modules gives info severity and no npm-install advice. This test fails without the change and passes with it.
  • A local squad-cli install still warns.

Results from npx vitest run test/cli/doctor.test.ts after npm run build: 68 passed, 2 failed. The 2 failures (git sync hooks tests) also fail on the unmodified base and are unrelated.


⚠️ Quick Check

  • If SDK/CLI source files changed: completed the applicable Changeset step below (npx changeset add / .changeset/*.md, direct CHANGELOG.md entry for maintainers, or skip-changelog label for no user-facing changes)

PR Readiness Checklist

The PR readiness bot will validate these automatically after push.
Check each item before requesting review. See CONTRIBUTING.md for full details.

Branch & Commit

  • Branch created from dev (not main)
  • Branch is up to date with dev (git fetch upstream && git rebase upstream/dev)
  • Verified diff contains only intended changes (git diff --cached --stat)
  • PR is not in draft mode (mark ready when checks pass)
  • Commit history is clean (squash fixups before review)

Build & Test

  • npm run build passes
  • npm test passes (all tests green)
  • npm run lint passes (type check clean)
  • npm run lint:eslint passes
  • For migration PRs (>20 files): include test output summary in PR description

Changeset

  • Changeset added via npx changeset add (if packages/squad-sdk/src/ or packages/squad-cli/src/ changed)
  • Or direct CHANGELOG.md entry (maintainers only — write-protected for external contributors)
  • Or skip-changelog label applied (if no user-facing changes)

Docs

N/A. This is a bug fix to warning behavior, with no new feature or module.

  • README section updated (if new feature/module)
  • Docs feature page (if new user-facing capability)

Exports

N/A. No new modules.

  • package.json subpath exports updated (if new module)

Breaking Changes

None

Waivers

Waived: npm test passes (all tests green), reason: the 2 failing git sync hooks tests in test/cli/doctor.test.ts also fail on the unmodified base and are unrelated to this change, approved by: pending (Lead or Quality role owner must approve in PR comments before merge)


AI assistance: this change was drafted with Claude Code.

…ecks

Motivation: `squad doctor` treated any existing `<cwd>/node_modules` as a Squad
development context. In a standalone install, that directory belongs to the
host application, so the vscode-jsonrpc and copilot-sdk checks warned
"run npm install or check dependencies", advice that cannot repair the runtime
Squad actually uses.

Approach: only give the "run npm install" warning when cwd is a Squad checkout
(packages/squad-cli) or has @bradygaster/squad-cli installed locally.
Otherwise the existing info-level "expected for global installs" result is
returned. Real failures (malformed exports, unpatched session.js) are unchanged.
This does not resolve and check the invoked installation's own runtime
dependencies; that is a larger change and is left out. The only effect here is
that the misleading warning is no longer shown for an unrelated node_modules.

Validation: added two tests in test/cli/doctor.test.ts (unrelated node_modules
gives info severity and no npm-install advice; local squad-cli install still
warns). The first fails without the change and passes with it. Ran
`npx vitest run test/cli/doctor.test.ts` after `npm run build`: 68 passed,
2 failed. The 2 failures (git sync hooks tests) also fail on the unmodified
base and are unrelated. `npm run lint` (tsc) passed; `npm run lint:eslint`
reported 0 errors.

Report: https://github.com/bradygaster/squad/issues/2367
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Assisted-by: claude-sonnet-5-5 (via Claude Code)
Copilot AI balanced review requested due to automatic review settings November 5, 2026 00:40
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

🟡 Impact Analysis — PR #2391

Risk tier: 🟡 MEDIUM

📊 Summary

Metric Count
Files changed 3
Files added 1
Files modified 2
Files deleted 0
Modules touched 3

🎯 Risk Factors

  • 3 files changed (≤5 → LOW)
  • 3 modules touched (2-4 → MEDIUM)

📦 Modules Affected

root (1 file)
  • .changeset/doctor-ignores-app-node-modules.md
squad-cli (1 file)
  • packages/squad-cli/src/cli/commands/doctor.ts
tests (1 file)
  • test/cli/doctor.test.ts

This report is generated automatically for every PR. See #733 for details.

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

🛫 PR Readiness Check

ℹ️ This comment updates on each push. Last checked: commit 05ee730

PR Scope: 📦🔧 Mixed (product + infrastructure)

⚠️ 2 item(s) to address before review

Status Check Details
✅ Single commit 1 commit — clean history
✅ Not in draft Ready for review
✅ Branch up to date Up to date with dev
❌ Copilot review No Copilot review yet — it may still be processing
✅ Changeset present Changeset file found
✅ Scope clean No .squad/ or docs/proposals/ files
✅ No merge conflicts No merge conflicts
✅ Copilot threads resolved No Copilot review threads
❌ CI passing 15 check(s) still running

Files Changed (3 files, +43 −4)

File +/−
.changeset/doctor-ignores-app-node-modules.md +5 −0
packages/squad-cli/src/cli/commands/doctor.ts +12 −4
test/cli/doctor.test.ts +26 −0

Total: +43 −4


This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues remain for the change that limits misleading dependency warnings to Squad-related contexts.

No open issues 🎉

What changed in this PR

This PR addresses the misleading dependency advice reported in #2367 by narrowing when squad doctor recommends running npm install. It does not add inspection of the invoked installation’s runtime dependencies.

Changes:

  • Limits missing-dependency warnings to Squad checkouts or directories with a local Squad CLI installation.
  • Adds regression tests for unrelated application node_modules and local Squad installations.
  • Includes a CLI patch changeset.
File Description
test/​cli/​doctor.test.ts Tests informational results for unrelated dependencies and retained warnings for local installs.
packages/​squad-cli/​src/​cli/​commands/​doctor.ts Adds a shared Squad-context check for both dependency diagnostics.
.changeset/​doctor-ignores-app-node-modules.md Records the warning-behavior fix for a CLI patch release.

Review effort: Balanced

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(cli): standalone doctor checks unrelated application node_modules instead of active runtime

2 participants