fix(cli): watch's self-pull no longer abandons stashed local changes on pull failure

[omercangumus]

What

SelfPullCapability.execute() now always attempts git stash pop after fetch/pull, regardless of whether the pull succeeded, and reports failure (not success) when the stash genuinely can't be restored.

Why

Closes #1639

Full repro and root cause are in the issue. Short version: the capability stashed uncommitted local changes before git pull --ff-only, but the pop only ran on the path where fetch+pull both succeeded — the whole method was one try block, so a thrown pull error (diverged history, no tracking branch, network blip) jumped straight to the outer catch, skipping the pop entirely, and that catch reported {success: true, summary: 'git pull skipped ...'}. The user's uncommitted work was left sitting in git stash, working tree looking clean, nothing in the round output saying so. Watch runs unattended, so this can happen more than once with no one watching.

How

• packages/squad-cli/src/cli/commands/watch/capabilities/self-pull.ts: fetch/pull failure is now caught locally into pullError instead of via the outer catch. The stash-pop block runs unconditionally right after, tracked via stashRestored. If the stash isn't restored (pop threw, whether or not pull also failed), that's now {success: false, summary: '...run \git stash pop` in manually'}so the watch loop's existing⚠ warning line (index.ts:490-492) actually surfaces it. If pull failed but the stash *did* pop cleanly, behavior is unchanged from before — {success: true, summary: 'git pull skipped (not on tracking branch or conflicts)'}`, since nothing was actually lost.
• Grepped the whole repo for other 'stash' call sites — self-pull.ts is the only one. No sibling move path to fix.

Testing

Real end-to-end repro (not mocked), against the actual bug scenario — diverged local branch with an uncommitted change on top, pull rejected as non-fast-forward:

CAPABILITY RESULT: {"success":true,"summary":"git pull skipped (not on tracking branch or conflicts)"}
STATUS AFTER:
 M file.txt

WORKING TREE FILE AFTER:
line1
local change
UNCOMMITTED WORK - MUST NOT BE LOST

STASH LIST AFTER: ""

Before the fix, the same setup produced STATUS AFTER: empty and the file content silently missing the last line, with it sitting in git stash list instead.

Added to test/cli/watch-capabilities.test.ts (existing SelfPullCapability describe block, same mock style as the rest of the file):

• Updated the existing "stash pop conflict" test — it asserted success: true before, which was the same silent-success bug in miniature; now asserts success: false and the surfaced summary.
• New: dirty tree + pull failure still calls git stash pop (fails on unfixed code — stashCalls never contains ['stash', 'pop'] there since the pop is unreachable) and comes back success: true once the stash is confirmed restored.
• New: dirty tree + pull failure + pop also failing → success: false, summary names both causes.
• Ran the two new/changed tests against unfixed dev first to confirm they fail (3 failures, exactly the ones exercising the bug), then against the fix (all pass).

npx vitest run test/cli/watch-capabilities.test.ts -t "SelfPullCapability"
 Test Files  1 passed (1)
      Tests  10 passed | 45 skipped (55)

Full test/cli/watch-capabilities.test.ts: 54/55 pass. The 1 failure (ExecuteCapability > buildAgentPrompt > checks .squad/ralph-instructions.md inside teamRoot) is a pre-existing Windows path-separator mismatch unrelated to this change — confirmed present on unmodified dev too (stashed my diff, reran, same failure).

Full-suite (npx vitest run, fresh dist for both packages): 108 failures / 7202 passed. None touch self-pull or watch-capabilities. Spot-checked watch-health.test.ts (11 failures in the full run, all Test timed out in 5000ms) — passes 11/11 clean in isolation, so that's resource contention under the full parallel run, not a real regression; it also has zero code relationship to self-pull.ts (only references the string 'self-pull' in capability-name fixtures).

npm run build -w packages/squad-cli passes (after rebuilding squad-sdk first — its dist was stale on this box, unrelated to this PR, same class of issue as documented on prior PRs). npx tsc --noEmit -p packages/squad-cli/tsconfig.json clean. eslint on both changed files: 0 errors, 8 warnings — all pre-existing n/no-sync/no-console style warnings already present in this file's existing patterns, not new.

Diff hygiene: git diff vs git diff -w differ only in the fetch/pull block, which is exactly the 2-space reindent from wrapping it in the new local try — not stray whitespace. Both files are LF in this repo; verified no CRLF got introduced.

---

⚠️ Quick Check

• Changeset added: .changeset/fix-1639-self-pull-stash-loss.md (patch @bradygaster/squad-cli)

PR Readiness Checklist

Branch & Commit

• Branch created from dev
• Branch is up to date with dev
• Verified diff contains only intended changes (2 source/test files + changeset)
• PR is not in draft mode
• Commit history is clean (single commit)

Build & Test

• npm run build -w packages/squad-cli passes
• npm test — targeted suite 10/10 (SelfPullCapability), full file 54/55 (1 pre-existing unrelated failure), full-suite baseline compared against unmodified dev
• npm run lint passes (tsc clean)
• eslint — 0 errors

Changeset

• Changeset added (cli patch)

Docs

• N/A — internal capability behavior fix, no public API/CLI surface changed

Exports

• N/A

---

Breaking Changes

None. CapabilityResult.success for this capability can now be false in a case it previously always reported true for — that's the fix, not a break: the watch loop already treats success: false as a normal per-round warning (⚠ line), not a fatal error, same as every other capability.

Waivers

None.

[github-actions]

🟡 Impact Analysis — PR #1640

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/fix-1639-self-pull-stash-loss.md

squad-cli (1 file)

• packages/squad-cli/src/cli/commands/watch/capabilities/self-pull.ts

tests (1 file)

• test/cli/watch-capabilities.test.ts

---

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

[github-actions]

🛫 PR Readiness Check

ℹ️ This comment updates on each push. Last checked: commit 8b1d71f

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	8 check(s) still running

Files Changed (3 files, +108 −16)

File	+/−
.changeset/fix-1639-self-pull-stash-loss.md	+5 −0
packages/squad-cli/src/cli/commands/watch/capabilities/self-pull.ts	+36 −13
test/cli/watch-capabilities.test.ts	+67 −3

Total: +108 −16

---

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