Skip to content

Sync fork with upstream smtg-ai/claude-squad (1.0.17 → 1.0.20) - #1

Draft
BippleDops wants to merge 12 commits into
mainfrom
cursor/sync-upstream-65a7
Draft

BippleDops wants to merge 12 commits into
mainfrom
cursor/sync-upstream-65a7

Conversation

@BippleDops

Copy link
Copy Markdown
Owner

Summary

Brings this fork's main up to date with smtg-ai/claude-squad main (ce1ffb4, "Bump version to 1.0.20"). The fork was 0 commits ahead / 12 behind; this branch is exactly upstream/main, so a "Rebase and merge" is a pure fast-forward with no merge commit.

Equivalent alternative: use GitHub's Sync fork button on the repo page instead of merging this PR.

Verification

  • go build ./... — pass
  • go vet ./... — pass
  • go test ./... — pass (app, config, session, session/git, session/tmux, ui)

Related

  • # cursor/paperclip-cs-serve-rebased-65a7 re-applies the fork's two owner commits (cs serve HTTP+SSE API, OpenTelemetry tracing) on top of this sync; it is based on this branch so it shows only those two commits.

Before merging

  • Prefer "Rebase and merge" (or Sync fork) to keep main identical to upstream.
Open in Web Open in Cursor 

estubmo and others added 12 commits May 15, 2026 02:24
## Summary
Fixed completion scripts and error messages to use the actual invoked
binary name instead of hardcoding 'claude-squad'.

- Fish, bash, and zsh completion scripts now work with custom binary
names (e.g., `--name cs`)
- Error messages and help text now reference the actual binary name
- Completion generation dynamically extracts the binary name from
`os.Args[0]`

## Problem
When users installed claude-squad with a custom name (e.g., `install.sh
--name cs`), the generated completion scripts still referenced
`claude-squad`, making them non-functional.

## Solution
Modified `main.go` to:
1. Extract the actual binary name from `os.Args[0]` using
`filepath.Base()`
2. Dynamically set `rootCmd.Use` to this binary name before Cobra
generates completions
3. Update error messages and version output to use the actual binary
name

This ensures all three completion shells (fish, bash, zsh) work
correctly regardless of how the binary is named.
## Summary
- Add Shift+J / Shift+K keybindings to move instances up/down in the
list
- New order is persisted to state.json immediately

## Test plan
- [ ] Press `K` (Shift+K) to move selected instance up
- [ ] Press `J` (Shift+J) to move selected instance down
- [ ] Verify boundary cases (top/bottom) are no-ops
- [ ] Restart `cs` and verify reordered list is preserved
…smtg-ai#285)

## Summary
- Move `SetStatus(Paused)` before `combineErrors` check in `Pause()` so
the state transition always happens after worktree removal, even when
non-critical errors occurred
- Skip already-paused instances in `metadataUpdateDoneMsg` handler to
prevent the polling loop from overwriting `Paused` back to
`Running`/`Ready`

Fixes smtg-ai#284

## Test plan
- [ ] Start a session, press `c` to checkout — should pause normally
- [ ] Simulate IsDirty failure (e.g., externally delete worktree dir,
then press `c`) — should still transition to Paused
- [ ] Verify metadata polling doesn't revert Paused status back to
active

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes smtg-ai#280

## Summary
The 500ms metadata tick in `tickUpdateMetadataCmd` calls `ComputeDiff()`
for every active instance and stores each full diff string in
`DiffStats.Content`, but the diff pane only ever renders one instance at
a time. With N sessions this keeps N copies of full git diff output
resident for zero UI benefit.

This PR keeps the full diff only for the currently-selected instance;
every other instance gets a lightweight `git diff --numstat` summary
that populates only `Added`/`Removed`. The list view's `+X/-Y` counters
are unaffected because they only need the totals.

## Changes
- `session/git/diff.go`: add `DiffNumstat()` which runs `git diff
--numstat <base>` and returns a `*DiffStats` with `Content` empty.
Parsing is extracted into `parseNumstat` so it's unit-testable.
- `session/instance.go`: add `ComputeDiffNumstat()` mirroring
`ComputeDiff()`.
- `app/app.go`: `tickUpdateMetadataCmd` now takes the selected instance
and dispatches full-diff vs numstat accordingly. Both call sites pass
`m.list.GetSelectedInstance()`.
- `session/git/diff_test.go`: unit tests for `parseNumstat` covering
empty output, binary files (`-\t-`), multi-file sums, tabs-in-path, and
trailing newlines.

## Trade-offs
Switching the selected instance incurs up to one tick (~500ms) before
the diff pane repopulates with the full content — previously, the full
diff was always resident for every instance so the switch felt instant.
This is an intentional trade for bounded memory.

## Test plan
- [x] `go test ./...` passes locally
- [x] `gofmt -w .` clean
- [x] Manually verify the diff tab renders correctly for the selected
instance
- [x] Manually verify preview / terminal tabs behave unchanged

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…ry (smtg-ai#302)

## Summary

- `github.com/go-git/go-git/v5` was declared as a direct dependency in
`go.mod` but is not imported anywhere in the codebase — all git
operations use `exec.Command` to shell out to the system `git` binary.
- `go-git`'s transitive dependency `github.com/go-git/go-billy/v5` was
pinned at `v5.6.2`, which is affected by a security advisory requiring
`v5.9.0` or later.
- Running `go mod tidy` removes `go-git` and its full transitive closure
(including `go-billy`, `go-winio`, `ProtonMail/go-crypto`,
`cloudflare/circl`, `golang.org/x/crypto`, `golang.org/x/net`, and
others), resolving the advisory by eliminating the dependency entirely.

## Changes

- `go.mod`: removed `go-git/go-git` direct dep and 14 indirect deps that
came with it
- `go.sum`: updated accordingly

## Test plan

- [ ] `go build ./...` passes
- [ ] `go vet ./...` passes
- [ ] `gofmt -l .` reports no files
- [ ] Existing tests pass (`go test ./...`)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
## Problem

Sessions created from a **pre-existing branch** never get a base commit
recorded, which makes the TUI log an error roughly twice a second for
the lifetime of the session:

```
WARNING app.go:253: could not update diff stats: git command failed:
fatal: ambiguous argument '': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
 (exit status 128)
```

`setupNewWorktree` resolves `HEAD` and assigns `g.baseCommitSHA`, but
`setupFromExistingBranch` returns without ever setting it — on both the
local-branch and remote-tracking paths:

```go
// Create a new worktree from the existing local branch
if _, err := g.runGitCommand(g.repoPath, "worktree", "add", g.worktreePath, g.branchName); err != nil {
    return fmt.Errorf("failed to create worktree from branch %s: %w", g.branchName, err)
}

return nil   // baseCommitSHA is still ""
```

`diff.go` then runs `git diff ""`, which exits 128 on every refresh:

```go
content, err := g.runGitCommand(g.worktreePath, "--no-pager", "diff", g.GetBaseCommitSHA())
```

The instance persists to `state.json` with `"base_commit_sha": ""`, so
restarting does not clear it — the session is permanently stuck
producing failed subprocess calls, and its diff tab stays empty.

## Reproduction

1. In a repo with an existing branch, create a session and select that
branch as the source (the `is_existing_branch` path added in smtg-ai#262).
2. `cat ~/.claude-squad/state.json` — the instance shows
`"base_commit_sha": ""`.
3. Watch the log at the path printed by `cs debug`; the warning above
repeats continuously.

## Fix

Both paths in `setupFromExistingBranch` now resolve the worktree `HEAD`
and store it, so "base" means the same thing it already means for new
branches: the commit the session started from.

## Testing

Added `TestSetupFromExistingBranch_RecordsBaseCommit`, which builds a
real repo, runs `Setup()` with `isExistingBranch: true`, and asserts the
recorded SHA matches the branch tip.

Verified it fails without the production change:

```
--- FAIL: TestSetupFromExistingBranch_RecordsBaseCommit
    worktree_ops_test.go:109: GetBaseCommitSHA() = "", want "b93ed4f66fba0eb41fd3ef9475c686e4671635ad"
```

`go test ./...` passes and `gofmt -l .` is clean.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude <noreply@anthropic.com>
…ai#322)

Fixes smtg-ai#216.

## The bug

When the tmux server goes away — reboot, crash, `tmux kill-server` —
every session goes with it, but the instances in `state.json` are still
recorded as `Running`. On the next start `cs` tries to attach to
sessions that no longer exist.

The problem is that `tmux attach-session` against a missing session
**forks successfully** and then exits 1 on its own. So `pty.Start`
returns no error and `TmuxSession.Restore` reported success while
attached to nothing:

```go
func (t *TmuxSession) Restore() error {
	ptmx, err := t.ptyFactory.Start(exec.Command("tmux", "attach-session", "-t", t.sanitizedName))
	if err != nil {   // only fires if the *fork* fails
		return fmt.Errorf("error opening PTY: %w", err)
	}
	...
```

The instance then sits there `Running` with every `capture-pane` against
it failing, spamming the TUI and the log:

```
ERROR tmux.go:238: error capturing pane content in status monitor: error capturing pane content: exit status 1
ERROR app.go:987: error capturing pane content: exit status 1
```

The preview pane stays blank and there is no way to get the session back
— `r` is gated on `Paused`, so the only exits are killing the instance
or `cs reset`. Several reports in smtg-ai#216 match this shape ("happens when I
come back to sessions after a few days"); other reports in that thread
look like a different, new-session failure, which this PR does not
touch.

## The fix

Three changes:

**1. `Restore` checks that the session exists** and returns a new
`ErrSessionNotFound` sentinel when it does not, so callers can tell
"session is gone" apart from "PTY failed".

**2. Restoring an instance whose session is gone parks it as `Paused`**
instead of leaving it `Running`-but-broken. The worktree and branch are
still on disk, so the existing `Resume` path rebuilds the session from
the same `Program`.

Returning an error from `Start` here would be worse than useless:
`Storage.LoadInstances` aborts on the first `FromInstanceData` failure,
so a single dead session would hide *every* other instance.

**3. `Resume` no longer runs `gitWorktree.Setup` when the worktree is
still valid on disk.** `setupFromExistingBranch` does `worktree remove
-f` + `os.RemoveAll` before re-adding, which is correct after a normal
`Pause` (the directory is already gone), but would throw away
uncommitted work in an instance paused by the new recovery path above.
This is the change that makes recovery non-destructive.

## Verification

Reproduced against a real `state.json` with five instances whose tmux
server had died in a reboot, loading them through
`Storage.LoadInstances` before and after:

```
before:  photos tagger  status=Running  previewErr=error capturing pane content: exit status 1
after:   photos tagger  status=Paused   previewErr=<nil>
```

All five then resumed from the TUI with `r` into working sessions,
worktrees and uncommitted contents intact.

Four tests added — `Restore` returning `ErrSessionNotFound` vs.
attaching normally, and `Start(false)` parking a dead-session instance
as `Paused` vs. restoring a live one as `Running`. `gofmt`, `go vet` and
the full suite are clean.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.

9 participants