From d94ed2e630fa99c02d90ce4170dedb0d5bcbe0f8 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 20:05:34 +0400 Subject: [PATCH] an act with a target the system cannot have does not run (V-634) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit V-633 gave tools spoken aliases, so a Russian act reaches a tool. It resolves the verb only: the rest of the sentence became argv. "перезагрузи роутер" ran as systemctl restart роутер, which is a real tool, a real word and a target that cannot exist on this box. She then reported systemctl's own confusion as if she had tried something sensible, and on a destructive row she spent a confirm turn on it first. The executor now refuses, ahead of the confirm gate, and names the word it could not place. The check is the script and not a word list: a unit, a container, a host and a path are ASCII here, so a Cyrillic argv element means the alias match swallowed the verb and handed on the next word. Process rows only. An MCP argument is not a target — a task title is Russian and always was — and a house row drops the spoken args already. It does not try to guess the right target. Identity is Nexus's, and a target Nexus resolves reaches Hexis through handleHexisAct before this executor is asked. --- cmd/mavend/actions_act.go | 23 ++++++++++++++ internal/phraser/acts.go | 7 ++++- internal/phraser/acts_ru_v1.json | 4 +++ internal/tool/tool.go | 53 ++++++++++++++++++++++++++++++++ internal/tool/tool_test.go | 53 ++++++++++++++++++++++++++++++++ 5 files changed, 139 insertions(+), 1 deletion(-) diff --git a/cmd/mavend/actions_act.go b/cmd/mavend/actions_act.go index ea024a8..64dcf98 100644 --- a/cmd/mavend/actions_act.go +++ b/cmd/mavend/actions_act.go @@ -4,6 +4,7 @@ import ( "context" "errors" "log" + "strings" "github.com/kami/maven/internal/mcp" "github.com/kami/maven/internal/phraser" @@ -60,6 +61,13 @@ func (h *reactiveHandler) actionAct(ctx context.Context, dec router.Decision) st phrase := actPhrase(dec.Slots.Fn, dec.Slots.Args) h.park(dec.Slots.Fn, dec.Slots.Args, phrase) return phraser.A(phraser.ActConfirm, map[string]string{"name": phrase}) + case errors.Is(err, tool.ErrUnknownTarget): + // The verb reached a tool and the tail did not reach a target, so + // nothing ran. Saying which word she could not place is the whole + // answer: he either renames it or gives the row an alias that + // carries the target, and both are one turn away (V-634). + return phraser.A(phraser.ActUnknownTarget, + map[string]string{"name": unknownTargetOf(err, dec.Slots.Args)}) case errors.Is(err, tool.ErrNeedsAuthedSurface): // Irreversible (internal/tool/risk.go). A confirm turn would not // help: everything that proposed this act — the STT, the router, @@ -93,3 +101,18 @@ func (h *reactiveHandler) actionAct(ctx context.Context, dec router.Decision) st } return phraser.A(phraser.ActDone, nil) } + +// unknownTargetOf pulls the word the executor could not place out of its error, +// falling back to the first arg. The word is what makes the reply usable, and a +// reply naming no word would send him to the log. +func unknownTargetOf(err error, args []string) string { + if _, rest, ok := strings.Cut(err.Error(), `"`); ok { + if word, _, ok := strings.Cut(rest, `"`); ok && word != "" { + return word + } + } + if len(args) > 0 { + return args[0] + } + return "" +} diff --git a/internal/phraser/acts.go b/internal/phraser/acts.go index d3b85d7..975cf3a 100644 --- a/internal/phraser/acts.go +++ b/internal/phraser/acts.go @@ -45,6 +45,10 @@ const ( // is the only authority the voice path can offer, and this is the one act // it is not enough for (Vikunja #449, #523). ActNeedsAuthedSurface = "act_needs_authed_surface" + // ActUnknownTarget — the verb reached a tool and the target did not reach + // anything. Named rather than run, because the alias match swallowed the verb + // and handed on the next word of the sentence (V-634). + ActUnknownTarget = "act_unknown_target" EcoDenied = "eco_denied" EcoDown = "eco_down" @@ -76,7 +80,7 @@ const ( var actKeys = []string{ ActDone, ActDoneOut, ActDoneEntity, ActConfirm, ActConfirmEntity, ActWhich, ActFail, ActFailOut, ActFailEntity, ActServerDown, ActWithdrawn, ActNeedsArgs, - ActNeedsAuthedSurface, + ActNeedsAuthedSurface, ActUnknownTarget, EcoDenied, EcoDown, EcoAmbiguous, EcoUnknownEntity, EcoNoNexus, EcoAboutWhat, EcoRecall, AttentionNone, AttentionList, AttentionFail, AttentionNoneEntity, AttentionListEntity, AttentionFailEntity, @@ -102,6 +106,7 @@ var actFloor = map[string]string{ ActServerDown: "инструмент есть, но сервер не подключён.", ActWithdrawn: "сервер больше не отдаёт этот инструмент — сняла его с разрешённых, посмотри /tools.", ActNeedsArgs: "тут нужны аргументы, из голоса не соберу. угадывать не буду.", + ActUnknownTarget: "«{name}» — не знаю такой цели. назови её как в системе.", ActNeedsAuthedSurface: "это из голоса не выполню — после него ничего не вернуть. запусти сам.", EcoDenied: "{name} отклоняет доступ, проверь токен.", diff --git a/internal/phraser/acts_ru_v1.json b/internal/phraser/acts_ru_v1.json index 4d66e9e..9b05fc8 100644 --- a/internal/phraser/acts_ru_v1.json +++ b/internal/phraser/acts_ru_v1.json @@ -60,6 +60,10 @@ "fixed": true, "variants": ["тут нужны аргументы, из голоса не соберу. угадывать не буду."] }, + "act_unknown_target": { + "fixed": true, + "variants": ["«{name}» — не знаю такой цели. назови её как в системе."] + }, "act_needs_authed_surface": { "fixed": true, "variants": ["это из голоса не выполню — после него ничего не вернуть. запусти сам."] diff --git a/internal/tool/tool.go b/internal/tool/tool.go index 60842d2..47e9963 100644 --- a/internal/tool/tool.go +++ b/internal/tool/tool.go @@ -39,6 +39,7 @@ import ( "os/exec" "strings" "time" + "unicode" "github.com/kami/maven/internal/ipc" "github.com/kami/maven/internal/mcp" @@ -73,6 +74,14 @@ var ( // confirm turn that would help: asking again would imply the second answer // changes the outcome. ErrNeedsAuthedSurface = errors.New("tool is irreversible and voice may not authorise it") + // ErrUnknownTarget — the act matched a tool and the target it carries cannot + // be one. A process row's args become argv for a real program, and a unit, + // container or host is named in ASCII on this box, so a Cyrillic tail is a + // word from the sentence rather than a target. Held apart from every failure + // above because the command never ran: forwarding it would spend a confirm + // turn on an act that cannot succeed, and then report the program's own + // confusion as if she had tried something sensible (V-634). + ErrUnknownTarget = errors.New("the act names a target the system cannot have") ) // MCPCaller is the seam for an act that is an MCP tool call rather than a @@ -144,6 +153,16 @@ func (e *Executor) Exec(ctx context.Context, name string, args []string, confirm if t.Status != "enabled" { return "", ErrNotEnabled } + // A process row's args become argv, so the target has to be able to exist. + // Checked before the confirm gate below, because asking "выполнить X?" about + // an act that cannot run spends a turn on nothing (V-634). The other two + // dispatches are exempt: an MCP tool may take Russian text as an argument, + // since a task title is not a target, and a house row drops the spoken args. + if !isMCPRow(t.Cmd) && !isHouseRow(t.Cmd) { + if bad, ok := firstUnknownTarget(args); !ok { + return "", fmt.Errorf("%w: %q", ErrUnknownTarget, bad) + } + } // The tier decides, not the column (Vikunja #449). RiskOf reads the row and // answers the three questions the boolean never did: which acts are // destructive, whether a confirm sticks (it never does), and what an @@ -260,3 +279,37 @@ func (m *Matcher) Allowlist() []string { return m.names() } func (m *Matcher) Match(utterance string) (string, []string, bool) { return router.DefaultActMatcher{Fns: m.names(), Aliases: m.aliases}.Match(utterance) } + +// firstUnknownTarget reports whether every arg could name something on this box, +// and returns the first that could not. +// +// The check is the script, not a word list: this is not a fourth Russian +// mechanism (CLAUDE.md § "Russian patterns"). A systemd unit, a container, a +// host and a path are written in ASCII, so a non-ASCII rune in an argv element +// means the alias match swallowed the verb and handed on the next word of the +// sentence. "перезагрузи роутер" is the case: restart is a real tool and +// "роутер" is a real word, and `systemctl restart роутер` is neither. +// +// It deliberately does not try to guess the right target. Identity is Nexus's +// (CLAUDE.md § "The ecosystem"), and a target Nexus resolves reaches Hexis +// through handleHexisAct before this executor is asked. +func firstUnknownTarget(args []string) (string, bool) { + for _, a := range args { + for _, r := range a { + if r > unicode.MaxASCII { + return a, false + } + } + } + return "", true +} + +func isMCPRow(cmd []string) bool { + _, _, ok := mcp.ParseCmd(cmd) + return ok +} + +func isHouseRow(cmd []string) bool { + _, _, ok := smarthome.ParseCmd(cmd) + return ok +} diff --git a/internal/tool/tool_test.go b/internal/tool/tool_test.go index 4eea1da..ac98d4c 100644 --- a/internal/tool/tool_test.go +++ b/internal/tool/tool_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "reflect" + "strings" "testing" "time" @@ -320,3 +321,55 @@ func TestExecEmptyCmdRefuses(t *testing.T) { t.Fatal("a row with no cmd ran a program named by the utterance") } } + +// V-634. The alias match resolves the verb and hands on the next word of the +// sentence, so "перезагрузи роутер" became `systemctl restart роутер`: a real +// tool, a real word, and a target that cannot exist on this box. +func TestExecRefusesATargetTheSystemCannotHave(t *testing.T) { + api := fakeAPI{tools: map[string]ipc.Tool{ + "restart": {Name: "restart", Cmd: []string{"systemctl", "restart"}, Status: "enabled"}, + "drop": {Name: "drop", Cmd: []string{"dropdb"}, Destructive: true, Status: "enabled"}, + }} + ran := false + e := NewExecutor(api, 0) + e.run = func(context.Context, []string) (string, error) { ran = true; return "ok", nil } + + _, err := e.Exec(context.Background(), "restart", []string{"роутер"}, false) + if !errors.Is(err, ErrUnknownTarget) { + t.Fatalf("err = %v, want ErrUnknownTarget", err) + } + if ran { + t.Fatal("the program was called with a target that cannot exist") + } + // The word is in the error, because a reply naming no word sends him to the log. + if !strings.Contains(err.Error(), "роутер") { + t.Errorf("err %v does not name the word she could not place", err) + } + // Ahead of the confirm gate: asking about an act that cannot run spends a + // turn on nothing. + if _, err := e.Exec(context.Background(), "drop", []string{"база"}, false); !errors.Is(err, ErrUnknownTarget) { + t.Errorf("destructive row: err = %v, want ErrUnknownTarget before ErrNeedsConfirm", err) + } + // An ASCII target still runs, unchanged. + if _, err := e.Exec(context.Background(), "restart", []string{"nginx"}, false); err != nil { + t.Errorf("restart nginx: %v", err) + } +} + +// An MCP argument is not a target. A task title is Russian and always was. +func TestExecMCPRowKeepsRussianArgs(t *testing.T) { + api := fakeAPI{tools: map[string]ipc.Tool{ + "vikunja_create": { + Name: "vikunja_create", Status: "enabled", + Cmd: []string{"mcp", "vikunja", "create_task"}, + }, + }} + m := &fakeMCP{out: "создала"} + e := NewExecutor(api, time.Second).WithMCP(m) + if _, err := e.Exec(context.Background(), "vikunja_create", []string{"купить хлеб"}, false); err != nil { + t.Fatalf("exec: %v", err) + } + if len(m.args) != 1 || m.args[0] != "купить хлеб" { + t.Fatalf("args = %v, want the Russian title forwarded", m.args) + } +}