diff --git a/cmd/mavend/actions_act.go b/cmd/mavend/actions_act.go index ea024a8..024f1b6 100644 --- a/cmd/mavend/actions_act.go +++ b/cmd/mavend/actions_act.go @@ -60,6 +60,17 @@ 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). + word := "" + var unknown *tool.UnknownTargetError + if errors.As(err, &unknown) { + word = unknown.Target + } + return phraser.A(phraser.ActUnknownTarget, map[string]string{"name": word}) 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 +104,4 @@ func (h *reactiveHandler) actionAct(ctx context.Context, dec router.Decision) st } return phraser.A(phraser.ActDone, nil) } + 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..cb7bf9e 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,8 +74,27 @@ 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") ) +// UnknownTargetError carries the word the executor could not place, because the +// reply names it: "«роутер» — не знаю такой цели" is actionable and "не +// получилось" sends him to the log. errors.Is(err, ErrUnknownTarget) holds. +type UnknownTargetError struct{ Target string } + +func (e *UnknownTargetError) Error() string { + return fmt.Sprintf("%s: %q", ErrUnknownTarget, e.Target) +} + +func (e *UnknownTargetError) Unwrap() error { return ErrUnknownTarget } + // MCPCaller is the seam for an act that is an MCP tool call rather than a // process (Vikunja #251). internal/mcp.Manager satisfies it via CallPositional. // nil ⇒ MCP is not configured, and an MCP row refuses to run rather than @@ -144,6 +164,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 "", &UnknownTargetError{Target: 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 +290,41 @@ 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. +// +// Every process row this box enables takes a system identifier (systemctl, +// docker, journalctl, df). A process row that legitimately wanted Russian text +// would want a different dispatch, not a hole in this check. +// +// 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) + } +}