diff --git a/CHANGELOG.md b/CHANGELOG.md index 20f61b7e..4576a888 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,15 @@ ### Fixed +- **`session_start` no longer tells you to install a hook you already have.** + When a call arrived without a per-call identity, its notice only said + "`plumb hooks install claude-code` stamps every call". With the hook + installed, the usual cause is a daemon too old to accept the key Claude + desktop's connector passes through, so the advice sent people round in a + circle. The notice now adds that, if the hook is installed, `plumb hooks` + checks it and the daemon: it reports a missing or stale hook, and a daemon + that cannot take the stamp. + - **The `git` tool no longer opens an editor.** plumb runs git with no terminal, so `rebase --continue`, `rebase -i`, `cherry-pick -e` and `revert --edit` launched `core.editor` and failed with `cannot exec diff --git a/internal/tools/session_start_stamp.go b/internal/tools/session_start_stamp.go index 742eb436..5c9750c4 100644 --- a/internal/tools/session_start_stamp.go +++ b/internal/tools/session_start_stamp.go @@ -66,12 +66,18 @@ func (t *SessionStart) WithStampChannel(fn func(ctx context.Context) StampChanne // plumb_agent, the hook works there too, so it is the remedy most callers can // apply. The transport remedy (one plumb serve per agent) follows, for a client // that cannot stamp at all. +// +// For a hook that is already installed it names bare `plumb hooks`, which +// reports a missing or stale hook and a daemon that cannot take the stamp. It +// says "checks", not "says why": with a current hook and a daemon that accepts +// the stamp it has nothing to report, so it cannot promise a diagnosis. const stampChannelRefusedNotice = "NOTE: state-changing calls from this session are being refused. " + "This connection serves more than one logical agent and this call carried no per-call identity, " + "so plumb cannot tell which agent's workspace a write belongs to and will not guess. Your session_id " + "declaration IS recorded, but it identifies this call only. Stamp every call: on Claude Code, " + - "`plumb hooks install claude-code` (on Claude desktop, restart the app after upgrading plumb); a client " + - "whose transport can set it sends a per-call _meta identity; otherwise run one plumb serve per agent.\n" + "`plumb hooks install claude-code` (on Claude desktop, restart the app after upgrading plumb); if the hook is " + + "installed, `plumb hooks` checks it and the daemon. A client whose transport can set " + + "it sends a per-call _meta identity; otherwise run one plumb serve per agent.\n" // stampChannelDormantNotice is emitted when this call carried no per-call // identity but the connection is still single-agent. Nothing is refused yet, @@ -79,7 +85,7 @@ const stampChannelRefusedNotice = "NOTE: state-changing calls from this session const stampChannelDormantNotice = "NOTE: this call carried no per-call logical-agent identity. Nothing is " + "refused while you are the only agent on this connection, but once a second agent attaches, your " + "unstamped state-changing calls are refused. On Claude Code, `plumb hooks install claude-code` stamps " + - "every call.\n" + "every call; if the hook is installed, `plumb hooks` checks it and the daemon.\n" // stampChannelNote renders the disclosure, or "" when there is nothing to say: // the accessor is unwired, or the channel is live. Rendered alongside diff --git a/internal/tools/session_start_stamp_test.go b/internal/tools/session_start_stamp_test.go index 6de360f9..9d9d9808 100644 --- a/internal/tools/session_start_stamp_test.go +++ b/internal/tools/session_start_stamp_test.go @@ -165,3 +165,27 @@ func TestStampChannelNoteSilentForNonHookClientUntilShared(t *testing.T) { t.Errorf("a refused non-hook client must be told: %q", got) } } + +// TestStampChannelNotices_PointAnInstalledHookAtItsDiagnosis: a caller whose +// hook IS installed but whose call still arrived unstamped (a daemon too old to +// accept the key Claude desktop's connector passes through) must not be sent to +// install it again. Both notices name `plumb hooks`, which checks the hook and +// the daemon it talks to. +// +// They must not promise it says WHY: with a current hook and a daemon that +// accepts its stamp, bare `plumb hooks` has nothing to report (see +// TestIdentityHookSkewNote in internal/cli), and a promised diagnosis that does +// not come sends the reader round in a circle again. +func TestStampChannelNotices_PointAnInstalledHookAtItsDiagnosis(t *testing.T) { + for name, notice := range map[string]string{"refused": stampChannelRefusedNotice, "dormant": stampChannelDormantNotice} { + if !strings.Contains(notice, "plumb hooks install claude-code") { + t.Errorf("%s notice lost the install remedy: %q", name, notice) + } + if !strings.Contains(notice, "if the hook is installed, `plumb hooks` checks it and the daemon.") { + t.Errorf("%s notice does not point an installed hook at `plumb hooks`: %q", name, notice) + } + if strings.Contains(notice, "says why") { + t.Errorf("%s notice promises a diagnosis bare `plumb hooks` does not always give: %q", name, notice) + } + } +}