Skip to content

fix: make lich close read a name the way lich send does - #227

Merged
omartelo merged 3 commits into
mainfrom
chore/tidy-relay-and-close-seams
Aug 11, 2026
Merged

omartelo merged 3 commits into
mainfrom
chore/tidy-relay-and-close-seams

Conversation

@omartelo

Copy link
Copy Markdown
Collaborator

Six of the seven medium findings from the [Unreleased] changelog review. The relay split (internal/relay/relay.go is 992 lines against the project's 800 ceiling) is deliberately left out — it is a large no-logic diff and it collides with work in flight on that file, so it comes as its own PR.

What changes

lich close resolves a name the way lich send does. findSession matched a card's label and a session's roster name in one pass. A workspace where one string is both — a label on one card, another session's roster name — answered "foo" names 2 sessions and closed nothing, while lich send foo resolved it fine: relay.resolve tries the labels first and only falls back to the roster names when none matched, giving the label the tie on purpose. Two surfaces of one product disagreeing about which session a name addresses. Now the same two passes, with a test pinning the collision.

The skipped WorktreeDirty error is documented instead of silent. No data is at stake — git worktree remove without --force refuses a dirty checkout on its own — so a status git cannot read costs lich's own wording and nothing else. The comment says so.

One constant for the MCP server name. internal/cli declared its own "lich" beside relay.MCPServerName, which exists precisely because more than one package needs it.

Three comments that had stopped being true:

  • 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 plus Installed after — two uncached questions per errand. Collapsing them into one would change the relay's Plugins interface to save a call nobody has measured, so the comment is what changes.
  • delegate-prompt's kind list claimed to mirror providers.AcceptsMCPServer. Since feat: give opencode and Crush sessions lich's own operations #222 that is no longer the same question: the plugin carries lich's operations into opencode and Crush too. The menu keeps the fixed list deliberately — what the plugin reports is true of an install, not of a session started before it, and the shell command works in all four — and the comment now says that rather than pointing at a mirror that moved. No behaviour change, so no changelog entry; its test's name carried the same stale claim and was reworded, assertions untouched.
  • CHANGELOG.md's [Unreleased] had two ### Added headings. The release job copies the section verbatim, so the published notes would have shown both. Merged; entries and their order unchanged.

Test plan

  • gofmt -l . clean, go vet ./... clean
  • go test ./... green (new: TestCloseResolvesALabelBeforeAnotherSessionsRosterName)
  • biome check ., vitest run (802 tests), tsc --noEmit, vite build all green

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/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.
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.
@omartelo
omartelo merged commit 7529753 into main Aug 11, 2026
4 checks passed
@omartelo
omartelo deleted the chore/tidy-relay-and-close-seams branch August 11, 2026 22:29
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.

1 participant