feat(open): attach to agent sessions in ttal open session

[birdmanmandbir]

Summary

• ttal open session <uuid> now falls back to agent tmux sessions (ttal-{team}-{agent}) when no worker session exists and the task has an agent name tag
• Added resolveAgentSession helper in internal/open/session.go with full test coverage
• Updated TUI openSession action to apply the same fallback logic via m.cfg

Test plan

• TestResolveAgentSession covers: no tags, non-agent tags, agent tag match, first match wins, empty team path
• make test passes
• make ci passes

[birdmanmandbir]

PR Review — feat: ttal open session agent session attachment

Important Issues (3)

1. DRY — agent resolution logic duplicated between open/session.go and tui/actions.go
internal/tui/actions.go:59-74

The tag-iteration loop (agentfs.HasAgent → config.AgentSessionName) is re-implemented inline in the TUI instead of calling the resolveAgentSession helper already extracted in open/session.go. Any change to the session-naming convention or agent-matching rules needs to be applied in two places.

Fix: export ResolveAgentSession from internal/open (accept tags []string, teamName, teamPath string) and have the TUI call it. The TUI still owns its tea.ExecProcess attachment; it just delegates the "which session name?" step:

// tui/actions.go
if cfg != nil {
    if agentSession, found := open.ResolveAgentSession(t.Tags, cfg.TeamName(), cfg.TeamPath()); found {
        if tmux.SessionExists(agentSession) {
            c := exec.Command("tmux", "attach-session", "-t", agentSession)
            return tea.ExecProcess(c, func(err error) tea.Msg { ... })
        }
    }
}

If a circular import prevents this, an internal/agentresolver package is the right extraction point.

---

2. Config error swallow is too broad
internal/open/session.go:36-43

The comment correctly documents the intent (degrade gracefully on first-time setup), but swallowing all config.Load() errors means a corrupted config.toml, wrong file permissions, or a TOML parse error will silently cause the agent session lookup to be skipped. The user sees "no worker or agent session" when the real problem is a broken config — hard to diagnose.

Tighten the swallow to os.ErrNotExist only:

cfg, err := config.Load()
if err != nil {
    if !errors.Is(err, os.ErrNotExist) {
        return fmt.Errorf("could not load config for agent session lookup: %w", err)
    }
} else if agentSession, found := resolveAgentSession(...); found {
    ...
}

---

3. execFinishedMsg lacks context in both TUI attach paths
internal/tui/actions.go:53-55 and 67-69

A failed tmux attach-session returns execFinishedMsg{err: err} with no context, producing Error: exit status 1 in the status bar. The user can't tell whether the worker session or agent session attachment failed.

Wrap at the callback, consistent with the existing pattern in advanceTask/openPR:

return tea.ExecProcess(c, func(err error) tea.Msg {
    if err != nil {
        return execFinishedMsg{err: fmt.Errorf("attach agent session %q: %w", agentSession, err)}
    }
    return execFinishedMsg{}
})

---

Suggestions (2)

4. Wrap lookPath error
internal/open/session.go:67
fmt.Errorf("tmux not found in PATH") discards the underlying error. Use %w to preserve the diagnostic.

5. "First match wins" test case doesn't verify ordering
internal/open/session_test.go:51-57
The case uses ["astra", "feature"] with only one valid agent file. It would pass even with reverse iteration. Add a second agent file and a second agent tag to actually pin the ordering guarantee.

---

Strengths

• Clean fallback structure: worker-first, agent-second, graceful error message.
• resolveAgentSession correctly extracted as a pure helper in the CLI path — makes it easily testable.
• Comment at lines 33-35 documenting the config-swallow intent is exactly right.
• syscall.Exec (CLI) vs tea.ExecProcess (TUI) split is architecturally correct.
• Test table covers the key branches of resolveAgentSession cleanly.

---

VERDICT: NEEDS_WORK

Issues 1-3 should be addressed before merge. Issue 1 (DRY) is the most impactful for long-term maintainability; issues 2-3 are correctness/UX gaps.

[birdmanmandbir]

Triage Update

Fixed

• DRY — duplicated agent resolution logic — Exported ResolveAgentSession(tags []string, teamName, teamPath string) from internal/open; TUI now delegates to it instead of re-implementing the tag loop. Removed agentfs import from tui/actions.go. 417e2c4
• Config error swallow too broad — Tightened to os.ErrNotExist only. Corrupted TOML / bad permissions now surfaces as "could not load config for agent session lookup: ..." instead of silently degrading. 417e2c4
• execFinishedMsg lacks context — Both worker and agent attach paths now wrap errors with session name: "attach agent session %q: %w" / "attach worker session %q: %w". 417e2c4
• lookPath error swallowed — Wrapped with %w to preserve diagnostic. 417e2c4
• "First match wins" test doesn't verify ordering — Added inke.md as a second agent file and a reversed-order test case (["inke", "astra"] → ttal-guion-inke) to pin the guarantee. 417e2c4

[birdmanmandbir]

Re-review — 417e2c4

Fixed

• DRY — duplicated agent resolution logic — ResolveAgentSession exported from internal/open; tui/actions.go now delegates to open.ResolveAgentSession and drops the agentfs import. Clean.
• Config error swallow too broad — errors.Is(err, os.ErrNotExist) guard added; corrupted TOML / bad permissions now surfaces as "could not load config for agent session lookup: ...". Comment updated to match. Clean.
• execFinishedMsg lacks context — Both worker and agent paths now wrap errors with "attach worker session %q: %w" / "attach agent session %q: %w". Consistent with the existing pattern in advanceTask.
• lookPath error swallowed — Wrapped with %w. Clean.
• "First match wins" test doesn't verify ordering — Added inke.md + reversed-order case (["inke", "astra"] → ttal-guion-inke). Both directions now pinned.

VERDICT: LGTM

All review issues resolved. Implementation is correct and well-structured.

[birdmanmandbir]

Triage Update

All issues resolved. Merging.