diff --git a/cmd/mavend/actionresolve.go b/cmd/mavend/actionresolve.go new file mode 100644 index 0000000..32c4bea --- /dev/null +++ b/cmd/mavend/actionresolve.go @@ -0,0 +1,54 @@ +package main + +import ( + "context" + + "github.com/kami/maven/internal/decision" + "github.com/kami/maven/internal/router" +) + +// resolveAction produces an ActionCandidate from a routing decision. It is the +// single boundary between routing and action execution: everything downstream +// (refusesCommand, task-status, Praxis, Hexis, proposeGap, tool.Executor.Exec) +// consumes the candidate rather than re-resolving the function. +// +// Delegates to router.ResolveActionCandidate for the resolution logic, then +// records the outcome in the decision trace. +func (h *reactiveHandler) resolveAction(ctx context.Context, dec router.Decision) router.ActionCandidate { + candidate := router.ResolveActionCandidate(dec, h.matcher) + + // Record the resolution outcome in the decision trace. + if dec.Intent == router.IntentAct { + if candidate.ActionResolved() { + noteActionResolution(ctx, string(candidate.Source), candidate.Fn, true) + } else { + noteActionResolution(ctx, "matcher", "", false) + } + } + + return candidate +} + +// noteActionResolution records the action resolution outcome in the decision +// trace. A nil recorder is the normal case in tests. +func noteActionResolution(ctx context.Context, source, fn string, resolved bool) { + rec := decision.From(ctx) + if rec == nil { + return + } + outcome := decision.Declined + reason := "no match" + if resolved { + outcome = decision.Won + reason = "resolved via " + source + if fn != "" { + reason += ": " + fn + } + } + rec.Note(decision.Claim{ + Stage: decision.StageAction, + Claimant: "action-resolve", + Outcome: outcome, + Reason: reason, + }) +} diff --git a/cmd/mavend/actions_act.go b/cmd/mavend/actions_act.go index a30902b..f42afc6 100644 --- a/cmd/mavend/actions_act.go +++ b/cmd/mavend/actions_act.go @@ -11,9 +11,14 @@ import ( "github.com/kami/maven/internal/tool" ) -// actionAct handles router.IntentAct: match a verb to an enabled tool, offer -// it to the ecosystems first, and run it behind the confirm gate and the -// allowlist. proposeGap and the confirm gate itself live in confirm.go. +// actionAct handles router.IntentAct: resolve the action, offer it to the +// ecosystems first, and run it behind the confirm gate and the allowlist. +// proposeGap and the confirm gate itself live in confirm.go. +// +// Action resolution happens in resolveAction (actionresolve.go) — a single +// boundary that produces an ActionCandidate before execution. This function +// consumes the candidate; it no longer decides which function/tool the user +// meant. func (h *reactiveHandler) actionAct(ctx context.Context, dec router.Decision) string { // An allowlist or a model route is evidence about WHAT could run, never // authority to run it. Keep the user's negative command at the execution @@ -23,13 +28,19 @@ func (h *reactiveHandler) actionAct(ctx context.Context, dec router.Decision) st return commandProhibitionReply } - // tool executor: run the matched fn against the enabled allowlist. - // HasFn=false ⇒ try the matcher (for LLM-routed acts where the verb - // didn't go through the stage-0 act grammar). - if !dec.Slots.HasFn && dec.Slots.Text != "" && h.matcher != nil { - if fn, args, ok := h.matcher.Match(dec.Slots.Text); ok { - dec.Slots.Fn, dec.Slots.Args, dec.Slots.HasFn = fn, args, true - } + // Resolve the action: produce an ActionCandidate from the routing + // decision. The candidate carries the resolved function, its arguments, + // and where the resolution came from (route or matcher). + candidate := h.resolveAction(ctx, dec) + + // Write the candidate's resolved values back into Slots so the existing + // branches (task-status, Praxis, Hexis, proposeGap, tool.Executor) work + // unchanged. This is the mechanical adjustment that preserves all existing + // behavior without redesigning those branches. + if candidate.ActionResolved() { + dec.Slots.Fn = candidate.Fn + dec.Slots.Args = candidate.Args + dec.Slots.HasFn = true } // The board is Maven's own store, so a spoken status change is answered here diff --git a/docs/reports/2026-09-05-action-resolution-boundary.md b/docs/reports/2026-09-05-action-resolution-boundary.md new file mode 100644 index 0000000..1dd12d2 --- /dev/null +++ b/docs/reports/2026-09-05-action-resolution-boundary.md @@ -0,0 +1,129 @@ +# Action-Resolution Boundary — Slice Report + +## 1. Files changed + +| File | Status | Purpose | +|---|---|---| +| `internal/router/actioncandidate.go` | **new** | `ActionCandidate` type, `ActionSource` enum, `ResolveActionCandidate` | +| `internal/router/actioncandidate_test.go` | **new** | 7 unit tests for the standalone resolver | +| `cmd/mavend/actionresolve.go` | **new** | `resolveAction` daemon wrapper with decision tracing | +| `cmd/mavend/actionresolve_test.go` | **new** | 8 integration tests for the 8 pinned scenarios | +| `cmd/mavend/actions_act.go` | **modified** | Removed matcher call, consumes candidate | +| `internal/router/eval/reach.go` | **modified** | `Reach()` uses `ResolveActionCandidate` | + +## 2. `ActionCandidate` contract + +```go +type ActionCandidate struct { + Fn string // resolved function/tool identity (empty = unresolved) + Args []string // positional arguments (may be nil) + Source ActionSource // "route" or "matcher" + Producer RouteProducer // cascade stage that produced the decision + Confidence float64 // routing confidence +} + +type ActionSource string +const ( + ActionSourceRoute ActionSource = "route" // Fn/Args resolved upstream + ActionSourceMatcher ActionSource = "matcher" // fallback matcher resolved +) +``` + +## 3. Where `resolveAction` lives and why + +- **Standalone:** `router.ResolveActionCandidate(dec, m)` in `internal/router/actioncandidate.go` — reusable by both daemon and eval harness. +- **Daemon wrapper:** `(h *reactiveHandler) resolveAction(ctx, dec)` in `cmd/mavend/actionresolve.go` — delegates to the standalone function, adds decision tracing. + +Location follows existing ownership: `internal/router/` for routing types and pure resolution logic, `cmd/mavend/` for the daemon's action layer. + +## 4. Before/after act flow + +**Before:** +``` +RouteDecision → actionAct + ├─ if !HasFn: h.matcher.Match(text) ← duplicated ownership + ├─ task-status intercept + ├─ Praxis intercept + ├─ Hexis intercept + ├─ proposeGap (if no fn) + └─ tool.Executor.Exec +``` + +**After:** +``` +RouteDecision → actionAct + ├─ resolveAction → ActionCandidate + │ ├─ HasFn? → source=route + │ └─ else? → h.matcher.Match(text) → source=matcher + ├─ write candidate into Slots (mechanical bridge) + ├─ task-status intercept + ├─ Praxis intercept + ├─ Hexis intercept + ├─ proposeGap (if no fn) + └─ tool.Executor.Exec +``` + +## 5. Removal of matching responsibility from `actionAct` + +The 5-line matcher block (`if !dec.Slots.HasFn && dec.Slots.Text != "" && h.matcher != nil { ... }`) was removed from `actionAct`. It now calls `resolveAction` which delegates to `router.ResolveActionCandidate`. The candidate's resolved values are written back into `dec.Slots` so all downstream branches (task-status, Praxis, Hexis, proposeGap, tool.Executor) work unchanged. + +## 6. Tests added + +**Router unit tests** (`internal/router/actioncandidate_test.go`): +1. `TestResolveActionCandidate_RouteSource` — HasFn=true → source=route +2. `TestResolveActionCandidate_MatcherSource` — no Fn → matcher resolves +3. `TestResolveActionCandidate_MatcherMiss` — no Fn → matcher miss → unresolved +4. `TestResolveActionCandidate_NonAct` — non-act → empty candidate +5. `TestResolveActionCandidate_Stage0Match` — stage-0 → route, confidence=1.0 +6. `TestResolveActionCandidate_LearnedRouterNoFn` — LLM no Fn → matcher fallback +7. `TestResolveActionCandidate_AliasMatch` — alias resolves through matcher + +**Daemon integration tests** (`cmd/mavend/actionresolve_test.go`): +1. `TestActRouteSource_NoMatcherInvoke` — act with HasFn=true, route-sourced +2. `TestActMatcherSource_FallbackMatch` — act without Fn, matcher-sourced +3. `TestActMatcherMiss_ProposeGap` — matcher miss → propose-gap +4. `TestActDestructive_ConfirmationUnchanged` — destructive → confirm +5. `TestActTaskStatus_InterceptUnchanged` — task-status intercepted +6. `TestActStage0_SameResult` — stage-0 act executes same tool +7. `TestActLearnedRouter_NoFn_FallbackMatch` — learned-router no Fn → matcher +8. `TestResolveAction_CandidateSource_Verified` — verifies source for all paths + +## 7. Before/after test results + +``` +ok github.com/kami/maven/cmd/mavend 22.149s +ok github.com/kami/maven/internal/router 1.131s +ok github.com/kami/maven/internal/router/eval 0.762s +``` + +All 15 new tests pass. All existing tests pass unchanged. + +## 8. Route-resolved vs matcher-resolved counts + +The test fixtures make this countable. From the 8 new integration tests: +- **Route-resolved:** 3 (TestActRouteSource, TestActStage0, TestResolveAction_CandidateSource route branch) +- **Matcher-resolved:** 3 (TestActMatcherSource, TestActLearnedRouter, TestResolveAction_CandidateSource matcher branch) +- **Matcher miss:** 2 (TestActMatcherMiss, TestResolveAction_CandidateSource miss branch) + +The decision trace records `action-resolve:route:` or `action-resolve:matcher:` for every act turn, making production measurement possible via the existing `/trace` endpoint. + +## 9. Confirmation that execution/risk/confirmation behavior did not change + +- `TestActPathSpeaksTheTiers` (existing) — safe runs, destructive confirms, irreversible refuses: **PASS** +- `TestActDestructive_ConfirmationUnchanged` (new) — destructive → confirm turn: **PASS** +- `TestActTaskStatus_InterceptUnchanged` (new) — task-status intercepted before tool exec: **PASS** +- `TestSystemSafetyScenarios` (existing) — all 4 safety scenarios: **PASS** +- `TestPraxisAttention_*` (existing) — Praxis interception: **PASS** + +## 10. Ambiguity about who should ultimately own action resolution + +No ambiguity uncovered. The boundary is clean: +- **Router** owns intent classification and slot extraction (stages 0-3). +- **`ResolveActionCandidate`** owns the final resolution step (route vs matcher). +- **`actionAct`** owns execution (confirmation, ecosystem interception, tool exec). + +The eval harness `Reach()` now uses the same `ResolveActionCandidate` function, eliminating the previous duplication. + +## 11. Commit hash + +Pending.