diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f1993e4..37115147 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,8 +25,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Crush has no flag for. It goes away with the same uninstall, and nothing outside lich's markers is touched. -### Added - - **An agent can close a session, and decide what happens to its worktree.** It could open one and hand it work, but never tidy up: every checkout an agent made stayed until somebody removed it by hand. `lich close` — and the diff --git a/frontend/src/lib/session/delegate-prompt.test.ts b/frontend/src/lib/session/delegate-prompt.test.ts index 6b235c65..dc5ba81a 100644 --- a/frontend/src/lib/session/delegate-prompt.test.ts +++ b/frontend/src/lib/session/delegate-prompt.test.ts @@ -7,7 +7,7 @@ describe("delegatePrompt", () => { expect(delegatePrompt("codex", "docs")).toBe('Ask the "docs" session to ') }) - it("spells the command where the sender has none", () => { + it("spells the command for every other sender", () => { for (const kind of ["opencode", "crush", "omp", "shell"]) { expect(delegatePrompt(kind, "docs")).toBe('lich send "docs" "') } diff --git a/frontend/src/lib/session/delegate-prompt.ts b/frontend/src/lib/session/delegate-prompt.ts index 01b64dab..e4a85c6c 100644 --- a/frontend/src/lib/session/delegate-prompt.ts +++ b/frontend/src/lib/session/delegate-prompt.ts @@ -4,9 +4,16 @@ import type { SessionKind } from "./sessions" -// The providers lich registers its MCP server with at spawn. Mirrors -// providers.AcceptsMCPServer on the Go side — the two have to agree, because an -// agent offered a tool it does not have would answer the user with an error. +// The providers whose sessions are handed lich's tools on their own command +// line at spawn (providers.AcceptsMCPServer). The kind alone answers it, and +// that answer cannot go stale. +// +// opencode and Crush sessions can have the same tools from the companion plugin +// (agentplugin.HasTools), and are still given the command here on purpose. What +// the plugin reports is a fact about what is installed now, not about a session +// started before the install — it only reaches one on its next restart — and an +// agent offered a tool it does not have answers the user with an error. The +// command works in all four and costs nothing. // // The session's declared kind, never the live agent readout: a shell card // running `claude` by hand was spawned without the registration, so it has the diff --git a/internal/agentplugin/agentplugin.go b/internal/agentplugin/agentplugin.go index a3a68743..a531f3f5 100644 --- a/internal/agentplugin/agentplugin.go +++ b/internal/agentplugin/agentplugin.go @@ -159,8 +159,10 @@ func (s *Service) HasTools(provider string) bool { // reading a session's silence as an answer: a provider that never reports is // silent whatever happens in it. // -// It shells out to the provider's CLI, so it is not for a hot path. The relay -// calls it once per errand. +// It reads the provider's own plugin state — a file for Claude Code and +// opencode, a CLI call for Codex and Crush — so it is not for a hot path. The +// relay asks it once per errand, and HasTools once more before that; neither +// question is cached, so each one pays for itself. func (s *Service) Installed(provider string) bool { if !slices.Contains(supported, provider) || !s.available(provider) { return false diff --git a/internal/cli/mcp.go b/internal/cli/mcp.go index 43fd1db6..a1c5ab1e 100644 --- a/internal/cli/mcp.go +++ b/internal/cli/mcp.go @@ -35,11 +35,6 @@ import ( // only lose a working session over a date. const mcpProtocolVersion = "2025-06-18" -// mcpServerName is the name the tools are namespaced under in a client's tool -// list (`mcp__lich__send_to_session` in Claude Code). It is also the key lich -// registers itself under at spawn. -const mcpServerName = "lich" - // mcpMaxWait caps how long any tool here blocks, whatever the caller asks for. // // Claude Code moves an MCP call that runs past 120 seconds into the background @@ -153,7 +148,7 @@ func mcpHandshake(params json.RawMessage) map[string]any { return map[string]any{ "protocolVersion": version, "capabilities": map[string]any{"tools": map[string]any{}}, - "serverInfo": map[string]any{"name": mcpServerName, "version": "1"}, + "serverInfo": map[string]any{"name": relay.MCPServerName, "version": "1"}, } } diff --git a/internal/spawn/close.go b/internal/spawn/close.go index 449600f8..502f13cf 100644 --- a/internal/spawn/close.go +++ b/internal/spawn/close.go @@ -114,6 +114,10 @@ func (s *Service) Close(fromID, target, projectName, worktree string, force bool // the checkout does — one left behind would offer a resume into a directory that // no longer exists. func (s *Service) removeCheckout(found located, active string, force bool) error { + // A check that cannot answer costs the message, never the work: `git worktree + // remove` without --force refuses a dirty checkout on its own + // (project.RemoveWorktree), so an unreadable status ends in git's own refusal + // instead of the sentence written for this case. dirty, err := s.worktrees.WorktreeDirty(found.session.Path) if err == nil && dirty && !force { return fmt.Errorf( @@ -159,7 +163,10 @@ type located struct { // findSession resolves a session by the label on its card or the name it answers // to in the peer roster, exactly as the relay does — an agent holding either -// should not have to know which one it has. +// should not have to know which one it has. The label is tried first and wins a +// tie against another session's roster name, because that is how `lich send` +// reads the same name (relay.resolve); one product cannot disagree with itself +// about which session a name addresses. // // Unlike the relay it looks past the live ones: a card whose terminal was never // opened is still a session, and closing it is the one thing you can do with it. @@ -169,7 +176,7 @@ func findSession(projects []store.Project, fromID, target, projectName string) ( return located{}, fmt.Errorf("no session given to close") } - var matches []located + var byLabel, byName []located for _, p := range projects { if projectName != "" && !strings.EqualFold(p.Name, projectName) { continue @@ -179,12 +186,18 @@ func findSession(projects []store.Project, fromID, target, projectName string) ( if cwd == "" { cwd = p.Path } - if strings.EqualFold(sess.Label, target) || - strings.EqualFold(relay.RosterName(cwd, sess.ID), target) { - matches = append(matches, located{project: p, session: sess}) + switch { + case strings.EqualFold(sess.Label, target): + byLabel = append(byLabel, located{project: p, session: sess}) + case strings.EqualFold(relay.RosterName(cwd, sess.ID), target): + byName = append(byName, located{project: p, session: sess}) } } } + matches := byLabel + if len(matches) == 0 { + matches = byName + } switch len(matches) { case 1: diff --git a/internal/spawn/close_test.go b/internal/spawn/close_test.go index dab9a131..3ea2099a 100644 --- a/internal/spawn/close_test.go +++ b/internal/spawn/close_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/omartelo/lich/internal/project" + "github.com/omartelo/lich/internal/relay" "github.com/omartelo/lich/internal/store" ) @@ -172,6 +173,35 @@ func TestCloseRefusesTheCallersOwnSession(t *testing.T) { } } +// One string can be a card's label and another session's roster name at the +// same time, and the two surfaces that resolve it — `lich send` and `lich +// close` — have to land on the same session. relay.resolve gives the label the +// tie; so does this. +func TestCloseResolvesALabelBeforeAnotherSessionsRosterName(t *testing.T) { + // The roster name s5 answers to: its project's directory, then four + // characters of its id. + const name = "lich-s5" + if got := relay.RosterName("/src/lich", "s5"); got != name { + t.Fatalf("roster name = %q, want %q — the collision this test needs is gone", got, name) + } + sessions := &fakeSessions{projects: []store.Project{{ + ID: "p1", Name: "lich", Path: "/src/lich", + Sessions: []store.Session{ + {ID: "s5", Label: "Session 5", Kind: "claude"}, + {ID: "s6", Label: name, Kind: "claude"}, + }, + }}} + svc := New(sessions, &fakeWorktrees{}, &fakeTerminal{}, &fakeEvents{}) + + closed, err := svc.Close("s1", name, "", "", false) + if err != nil { + t.Fatalf("Close: %v", err) + } + if closed.ID != "s6" { + t.Errorf("closed %q, want s6 — the session %q is the label of", closed.ID, name) + } +} + func TestCloseRefusesAnUnknownSession(t *testing.T) { svc, _, _, _, _ := closer(t)