From 53599e045b7c5db5e70661b8157ae591585006f5 Mon Sep 17 00:00:00 2001 From: omartelo Date: Tue, 11 Aug 2026 18:47:45 -0300 Subject: [PATCH 1/3] fix: resolve a name in lich close the way lich send does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit findSession matched a card's label and a session's roster name in the same pass, so a workspace where one string is both — a label here, another session's roster name there — answered "names 2 sessions" and refused to close anything. `lich send` resolves the same collision: it tries the labels first and only falls back to the roster names when none matched, and the comment here already claimed to do that. Two passes, and the two surfaces agree about which session a name addresses. The check the removal path skips is now documented rather than silent: a status git cannot read costs the sentence written for a dirty checkout, never the safety, because `git worktree remove` without --force refuses one anyway. --- internal/spawn/close.go | 23 ++++++++++++++++++----- internal/spawn/close_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 5 deletions(-) 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) From 0d76ad1b3ebfc378fff2feb2d6cfe39c823a79ab Mon Sep 17 00:00:00 2001 From: omartelo Date: Tue, 11 Aug 2026 18:47:57 -0300 Subject: [PATCH 2/3] refactor: name the MCP server from the one place that owns the name internal/cli declared its own "lich" constant beside relay.MCPServerName, which exists precisely because more than one package needs it and the MCP server already depends on that package. One copy, so a rename cannot leave half the product answering to the old name. --- internal/cli/mcp.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) 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"}, } } From 559c85c86c1d4d24b34c592c4a40e11e4fbb8f90 Mon Sep 17 00:00:00 2001 From: omartelo Date: Tue, 11 Aug 2026 18:47:57 -0300 Subject: [PATCH 3/3] docs: say what the code does about tools, cost and a skipped check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three comments had drifted from what they describe, and the release notes carried a heading twice: - agentplugin.Installed claimed to shell out to a CLI and to be asked once per errand. It reads a file for Claude Code and opencode, runs a CLI for Codex and Crush, and the relay asks HasTools before delivering and Installed after — two uncached questions per errand, not one. - delegate-prompt's list claimed to mirror providers.AcceptsMCPServer. Since the plugin started carrying lich's operations into opencode and Crush that is no longer the same question, and the menu keeps the fixed list on purpose: what the plugin reports is true of an install, not of a session started before it, and the command works in all four. The comment says that now. Its test's name said "where the sender has none", which was the same stale claim; the assertions are untouched. - CHANGELOG's [Unreleased] had two "Added" headings, and the release job copies the whole section verbatim — so the published notes would have shown both. Merged, entries and their order unchanged. --- CHANGELOG.md | 2 -- frontend/src/lib/session/delegate-prompt.test.ts | 2 +- frontend/src/lib/session/delegate-prompt.ts | 13 ++++++++++--- internal/agentplugin/agentplugin.go | 6 ++++-- 4 files changed, 15 insertions(+), 8 deletions(-) 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