tool: separate a tool that is off from a backend that is down

An enabled MCP or smarthome row with no backend returned ErrNotEnabled,
and actionAct reads ErrNotEnabled as "this is unknown, draft a
proposal". So a tool Kami had already approved, whose server happened to
be restarting, produced a second proposal row and an answer saying the
tool needs approval. The right answer is that the server is down.
ErrNotConnected carries that, and the act path maps it, ErrNoServer and
ErrToolGone to replies that say which of the three happened.

Found in review of #71.
This commit is contained in:
kami
2026-08-01 14:11:57 +04:00
parent 5e0417306b
commit 52f56947bb
3 changed files with 27 additions and 6 deletions
+7
View File
@@ -53,6 +53,13 @@ func (h *reactiveHandler) actionAct(ctx context.Context, dec router.Decision) st
return "выполнить «" + phrase + "»? скажи «да» или «нет»."
case errors.Is(err, tool.ErrNotEnabled):
return h.proposeGap(ctx, dec)
case errors.Is(err, tool.ErrNotConnected), errors.Is(err, mcp.ErrNotConnected), errors.Is(err, mcp.ErrNoServer):
// The row is enabled and the backend is gone. Drafting a proposal
// for it (the ErrNotEnabled path) would be answering the wrong
// question.
return "этот инструмент включён, но сервер, который его выполняет, сейчас не подключён."
case errors.Is(err, mcp.ErrToolGone):
return "сервер больше не предлагает этот инструмент — я сняла его с разрешённых, посмотри на /tools."
case errors.Is(err, mcp.ErrNeedsArgs):
// An MCP tool that wants named arguments a spoken verb cannot
// supply. Guessing them would be a wrong act, so she says so
+9 -2
View File
@@ -60,6 +60,13 @@ var (
ErrNotEnabled = errors.New("tool not on the enabled allowlist")
// ErrNeedsConfirm — the fn is enabled but destructive; needs a confirm turn.
ErrNeedsConfirm = errors.New("destructive tool needs confirmation")
// ErrNotConnected — the row is enabled and well formed, but the thing it
// dispatches to is not wired: the mcp block was dropped from the config
// while enabled MCP rows remained, or the same for the house. Held apart
// from ErrNotEnabled because the act path turns that one into a fresh
// proposal, and drafting a new proposal for a tool that already exists and
// is enabled is a lie about what is wrong.
ErrNotConnected = errors.New("tool is enabled but its backend is not connected")
)
// MCPCaller is the seam for an act that is an MCP tool call rather than a
@@ -134,7 +141,7 @@ func (e *Executor) Exec(ctx context.Context, name string, args []string, confirm
// confirmed. Only the dispatch differs.
if server, remote, ok := mcp.ParseCmd(t.Cmd); ok {
if e.mcp == nil {
return "", ErrNotEnabled
return "", fmt.Errorf("%w: %s is an MCP tool and no mcp block is configured", ErrNotConnected, name)
}
ctx, cancel := context.WithTimeout(ctx, e.timeout)
defer cancel()
@@ -148,7 +155,7 @@ func (e *Executor) Exec(ctx context.Context, name string, args []string, confirm
// row but can never compose a target of its own.
if entityID, service, ok := smarthome.ParseCmd(t.Cmd); ok {
if e.home == nil {
return "", ErrNotEnabled
return "", fmt.Errorf("%w: %s is a house tool and no smarthome block is configured", ErrNotConnected, name)
}
ctx, cancel := context.WithTimeout(ctx, e.timeout)
defer cancel()
+11 -4
View File
@@ -178,8 +178,15 @@ func TestExecMCPRowWithoutCallerRefuses(t *testing.T) {
ran := false
e := NewExecutor(api, time.Second)
e.run = func(context.Context, []string) (string, error) { ran = true; return "", nil }
if _, err := e.Exec(context.Background(), "vikunja_list_tasks", nil, false); !errors.Is(err, ErrNotEnabled) {
t.Fatalf("err = %v, want ErrNotEnabled", err)
err := func() error { _, e2 := e.Exec(context.Background(), "vikunja_list_tasks", nil, false); return e2 }()
// ErrNotConnected, NOT ErrNotEnabled: the act path turns ErrNotEnabled into
// a fresh proposal, and drafting a proposal for a row that already exists
// and is enabled answers the wrong question.
if !errors.Is(err, ErrNotConnected) {
t.Fatalf("err = %v, want ErrNotConnected", err)
}
if errors.Is(err, ErrNotEnabled) {
t.Fatal("an enabled row with a missing backend must not read as not-enabled")
}
if ran {
t.Fatal(`"mcp" must never be run as a binary`)
@@ -222,8 +229,8 @@ func TestExecSmartHomeRow(t *testing.T) {
}
// No house configured ⇒ the row refuses rather than being exec'd.
if _, err := newExec(nil).Exec(context.Background(), "home_light_x_off", nil, true); !errors.Is(err, ErrNotEnabled) {
t.Fatalf("unconfigured house: err = %v, want ErrNotEnabled", err)
if _, err := newExec(nil).Exec(context.Background(), "home_light_x_off", nil, true); !errors.Is(err, ErrNotConnected) {
t.Fatalf("unconfigured house: err = %v, want ErrNotConnected", err)
}
if ran {
t.Fatal(`"smarthome" was run as a binary`)