Improve tmux targeting safety and cap attached agent tabs

[andyrewlee]

Summary

Describe the change and intended behavior.

Quality Checklist

• Ran make devcheck locally.
• Ran make lint-strict-new locally for changed code.
• If UI/rendering changed, ran make harness-presets.
• If tmux/e2e changed, ran go test ./internal/tmux ./internal/e2e.

---

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 8 additional findings.

[andyrewlee]

Devin Review found 2 new potential issues.

View 10 additional findings in Devin Review.

[andyrewlee]

🟡 SessionTagValue returns error instead of ("", nil) for unset options on some tmux versions

The SessionTagValue error handling was changed from treating ALL exit-code-1 results as "option not set" (returning ("", nil)) to only treating specific stderr messages as benign. The new code at internal/tmux/tmux.go:281 checks for "no such session" or "invalid option" in stderr, but tmux 3.2 (the documented minimum version) uses "unknown option" in cmd-show-options.c for user options (@amux_*) that were never set. This means the function now returns an error for a case that previously returned ("", nil).

Root Cause and Impact

The old code was:

if exitErr.ExitCode() == 1 {
    return "", nil
}

The new code is:

if exitErr.ExitCode() == 1 {
    errOut := strings.ToLower(strings.TrimSpace(string(exitErr.Stderr)))
    if strings.Contains(errOut, "no such session") || strings.Contains(errOut, "invalid option") {
        return "", nil
    }
}
return "", err

In tmux versions where the error message for a never-set user option is "unknown option: @amux_foo" rather than "invalid option: @amux_foo", the strings.Contains checks will not match, and the function falls through to return "", err. This also applies if stderr is empty for any reason.

Currently only test code calls SessionTagValue directly, so production impact is limited, but this is a regression in the public API contract and could cause test failures on tmux 3.2.

Suggested change

	if exitErr.ExitCode() == 1 {
	return "", nil
	errOut := strings.ToLower(strings.TrimSpace(string(exitErr.Stderr)))
	// Missing sessions/tags are expected races; treat as empty.
	if strings.Contains(errOut, "no such session") || strings.Contains(errOut, "invalid option") {
	return "", nil
	}
	}
	}
	if exitErr, ok := err.(*exec.ExitError); ok {
	if exitErr.ExitCode() == 1 {
	errOut := strings.ToLower(strings.TrimSpace(string(exitErr.Stderr)))
	// Missing sessions/tags are expected races; treat as empty.
	if strings.Contains(errOut, "no such session") || strings.Contains(errOut, "invalid option") || strings.Contains(errOut, "unknown option") {
	return "", nil
	}
	}

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[andyrewlee]

🟡 Data race on tab.pendingOutput in busyPTYTabCount (read without lock)

busyPTYTabCount reads len(tab.pendingOutput) at internal/ui/center/model_pty_reader.go:92 without holding tab.mu. All other accesses to pendingOutput are protected by tab.mu (e.g., flushTiming at line 32, detachTab at model_tabs_session.go:48). This is a data race detectable by Go's -race detector.

Detailed Explanation

The pendingOutput field is a []byte slice that is read and written from multiple goroutines. The busyPTYTabCount function iterates over ALL tabs across ALL workspaces and reads pendingOutput without acquiring any lock:

busy := atomic.LoadUint32(&tab.readerActiveState) == 1 || len(tab.pendingOutput) > 0

While readerActiveState is correctly accessed via atomic.LoadUint32, the pendingOutput slice is not protected. Concurrent writes to tab.pendingOutput (e.g., appending PTY output under tab.mu in the flush path) racing with this unsynchronized read constitute a data race under the Go memory model.

The practical impact is limited — the consequence is at most an incorrect flush timing multiplier for a single sample interval — but this will trigger failures under go test -race.

Prompt for agents

In internal/ui/center/model_pty_reader.go, the busyPTYTabCount function at line 92 reads len(tab.pendingOutput) without holding tab.mu. To fix the data race, either: (1) acquire tab.mu.Lock() around the pendingOutput read (but be careful about lock ordering and performance since this iterates ALL tabs), or (2) add an atomic uint32 field like hasPendingOutput to the Tab struct that mirrors whether pendingOutput is non-empty, updated atomically alongside pendingOutput writes, similar to how readerActiveState mirrors readerActive.

---

Was this helpful? React with 👍 or 👎 to provide feedback.