fix(watch): detect rate-limit errors instead of reporting Board is clear

[diberry]

Summary

When squad watch hits a GitHub API rate limit, runCheck() catches the error and returns an empty board state. The watch loop then prints "Board is clear - Ralph is idling" - which is misleading because the board isn't clear, the API just couldn't be reached.

Changes

packages/squad-cli/src/cli/commands/watch/index.ts

• Added RunCheckStatus discriminated union ('ok' | 'rate-limited' | 'error') and RunCheckResult interface
• runCheck() now returns RunCheckResult with status, using isRateLimitError() to classify failures
• reportBoard() accepts optional scanStatus - shows rate-limit or error warning instead of "Board is clear" on failed scans
• Rate-limit/error warnings bypass notifyLevel suppression (always visible)
• Main loop short-circuits remaining phases when scan fails
• Rate-limited rounds do not count as circuit-breaker successes

test/watch-rate-limit.test.ts (new)

8 tests covering rate-limit warning, error warning, normal behavior, notifyLevel bypass, and busy-board fallthrough.

.changeset/watch-rate-limit-detection.md

Patch changeset for @bradygaster/squad-cli.

Closes #806

[github-actions]

🟡 Impact Analysis — PR #893

Risk tier: 🟡 MEDIUM

📊 Summary

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

🎯 Risk Factors

• 3 files changed (≤5 → LOW)
• 3 modules touched (2-4 → MEDIUM)
• Critical files touched: packages/squad-cli/src/cli/commands/watch/index.ts

📦 Modules Affected

root (1 file)

• .changeset/watch-rate-limit-detection.md

squad-cli (1 file)

• packages/squad-cli/src/cli/commands/watch/index.ts

tests (1 file)

• test/watch-rate-limit.test.ts

⚠️ Critical Files

• packages/squad-cli/src/cli/commands/watch/index.ts

---

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

[Copilot]

Pull request overview

Fixes misleading squad watch output by distinguishing successful board scans from GitHub API rate-limit / scan failures, and reporting a warning instead of “Board is clear”.

Changes:

• Introduces RunCheckResult / RunCheckStatus and classifies runCheck() failures via isRateLimitError()
• Updates reportBoard() and the watch round flow to short-circuit phases on failed scans and print rate-limit/error warnings
• Adds vitest coverage for rate-limit/error messaging behavior and includes a patch changeset

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
packages/squad-cli/src/cli/commands/watch/index.ts	Adds scan status plumbing, warning output, and short-circuit behavior on failed scans
test/watch-rate-limit.test.ts	New tests validating warning output and notifyLevel bypass behavior
.changeset/watch-rate-limit-detection.md	Patch changeset describing the user-facing fix

[diberry]

Is this introducing issues that 916 is meant to fix?

[diberry]

🔍 Squad Review — Kaylee (Engineering)

#	Check	Status	Notes
1	Changelog entry	✅	.changeset/watch-rate-limit-detection.md (CLI patch)
2	Squashed to 1 commit	✅	1 commit
3	CI green	✅	All checks passed
4	Copilot comments resolved	✅	2/2 threads resolved with fixes
5	No .squad/ files	✅	Clean
6	No unrelated files	✅	3 files — watch index, rate-limit tests, changeset
7	Tests for changes	✅	est/watch-rate-limit.test.ts — 8 new tests
8	Not a duplicate/reversal	✅	Unique scope (closes #806)

Verdict: ✅ Ready to merge

---

Review by Squad AI team (Kaylee — Engineering) · requested by Dina Berry

[diberry]

Review findings for PR #893

P1 required: isRateLimitError matches '403' too broadly - a 403 from insufficient token scopes or repo access denial is a permanent auth error, not a transient rate limit. Misclassifying it suppresses the error and tells the user to "retry next interval" instead of "check your token". Consider narrowing to '429' plus the explicit GitHub rate-limit message strings ('secondary rate limit', 'api rate limit exceeded').

P2 suggestion: The core isRateLimitError classifier in gh-cli.ts has no direct unit tests - only the reportBoard display logic is tested. A test asserting that a bare 403 Forbidden is NOT classified as rate-limited would catch this.

P2 suggestion: Circuit-breaker failure counter doesn't increment on generic 'error' status - repeated non-rate-limit scan errors would never trip the breaker.

P2 suggestion: notifyLevel: 'none' bypass not tested for 'error' status.

Nit: Test file in test/ root instead of test/cli/.

[diberry]

✅ CI validation passed on fork: diberry#143 — 7/7 checks green