Files
Maven/docs/reports/2026-09-05-slice3-structural-validation.md
T

189 lines
9.0 KiB
Markdown

# Slice 3: Structural validation between ActionCandidate resolution and execution
**Date:** 2026-09-05
**Base:** 6a402bf (slice 2 committed)
**Status:** complete
## 1. Files changed
| File | Change |
|------|--------|
| `internal/router/actioncandidate.go` | +76 lines: `ActionField`, `ActionValidationIssue`, `ActionValidationResult`, `ValidateActionCandidate` |
| `internal/router/actioncandidate_test.go` | +98 lines: 6 unit tests for `ValidateActionCandidate` |
| `cmd/mavend/actions_act.go` | +11 lines: validation call, invalid-candidate early return |
| `cmd/mavend/actionresolve.go` | +37 lines: `noteActionValidation` tracing function |
| `cmd/mavend/actionresolve_test.go` | +128 lines: 5 integration tests (malformed, unresolved, destructive, irreversible, tracing) |
## 2. Validation contract
```go
type ActionValidationResult struct {
Unresolved bool // Fn empty → proposeGap path
Valid bool // Fn non-empty, structure sound
Issues []ActionValidationIssue // non-empty when Invalid
}
type ActionValidationIssue struct {
Field ActionField // "fn" or "args"
Reason string // machine-readable tag
}
type ActionField string
const (
FieldFn ActionField = "fn"
FieldArgs ActionField = "args"
)
```
Three disjoint outcomes:
- **Unresolved**: Fn is empty. Not a validation error. Routes to proposeGap / clarification.
- **Valid**: Fn non-empty, structurally admissible. Proceeds to risk policy and execution.
- **Invalid**: Fn non-empty but malformed. Refused with `ActFail`.
## 3. Exact validation rules introduced
| Rule | Check | Outcome on fail |
|------|-------|-----------------|
| `unresolved` | `c.Fn == ""` | Unresolved (not invalid) |
| `blank_function_name` | `strings.TrimSpace(c.Fn) == ""` when Fn is non-empty | Invalid |
The model is deliberately small. This is not a general validation framework.
## 4. Where each rule existed previously
| Rule | Previous location | Migration type |
|------|-------------------|----------------|
| Unresolved → proposeGap | `cmd/mavend/actions_act.go:82` (`!dec.Slots.HasFn`) | Preserved: the existing `!dec.Slots.HasFn` check now comes after validation, same observable behavior |
| Blank Fn → refuse | No previous check existed | New: defensive check for a structurally malformed candidate that the route/matcher should never produce |
**Observation:** The current route and matcher never produce a blank (whitespace-only) Fn. The `blank_function_name` rule is a defensive gate against future producer defects, not a migration from existing behavior.
## 5. Before/after action flow
**Before (slice 2):**
```
RouteDecision → ResolveActionCandidate → ActionCandidate
→ actionAct writes Fn/Args back into dec.Slots
→ refusesCommand check
→ task-status intercept
→ Praxis intercept
→ Hexis intercept
→ !HasFn → proposeGap
→ tool.Executor.Exec → error switch
```
**After (slice 3):**
```
RouteDecision → ResolveActionCandidate → ActionCandidate
→ ValidateActionCandidate
├─ unresolved → (continues to existing flow; !HasFn → proposeGap)
├─ invalid → ActFail (early return)
└─ valid → (continues)
→ actionAct writes Fn/Args back into dec.Slots
→ refusesCommand check
→ task-status intercept
→ Praxis intercept
→ Hexis intercept
→ !HasFn → proposeGap
→ tool.Executor.Exec → error switch
```
The validation gate sits between resolution and the bridge write. Unresolved candidates skip the validation gate entirely and flow through the existing path unchanged.
## 6. How unresolved differs from invalid
| | Unresolved | Invalid |
|---|---|---|
| **Fn** | empty (`""`) | non-empty but malformed (e.g. whitespace-only) |
| **Cause** | Matcher miss, non-act intent | Structural defect in route/matcher output |
| **Trace** | `action-validation:declined:unresolved` | `action-validation:declined:invalid:<reason>` |
| **Response** | proposeGap (existing proposal/scaffold path) | `ActFail` ("не получилось выполнить команду.") |
| **Execution** | does not reach tool executor | does not reach tool executor |
The distinction is preserved: unresolved is "no match found" (the normal case for unknown verbs), while invalid is "a match was found but it's structurally broken" (a defect that should never happen in practice).
## 7. Tests added
### Unit tests (internal/router)
| Test | Pins |
|------|------|
| `TestValidateActionCandidate_UnresolvedEmptyFn` | Empty Fn → unresolved, not invalid |
| `TestValidateActionCandidate_UnresolvedMatcherMiss` | Matcher miss candidate → unresolved |
| `TestValidateActionCandidate_ValidRoute` | Route-resolved candidate → valid |
| `TestValidateActionCandidate_ValidMatcher` | Matcher-resolved candidate → valid |
| `TestValidateActionCandidate_ValidNoArgs` | Zero-arg tool → valid |
| `TestValidateActionCandidate_BlankFn` | Whitespace-only Fn → invalid with `blank_function_name` issue |
### Integration tests (cmd/mavend)
| Test | Pins |
|------|------|
| `TestActValidation_MalformedCandidate_BlankFn` | Blank Fn does not execute, produces response |
| `TestActValidation_UnresolvedCandidate_ProposeGap` | Matcher miss still flows to proposeGap |
| `TestActValidation_DestructiveValid_StillConfirms` | Destructive valid act still triggers confirm turn |
| `TestActValidation_IrreversibleValid_NeedsAuthedSurface` | Irreversible valid act still reaches authed-surface refusal |
| `TestActValidation_ValidationTracing` | Validation outcome recorded in decision trace |
### Existing regression tests preserved (all pass)
| Test | What it pins |
|------|-------------|
| `TestActRouteSource_NoMatcherInvoke` | Route-resolved act executes, matcher not invoked |
| `TestActMatcherSource_FallbackMatch` | Matcher-resolved act executes |
| `TestActMatcherMiss_ProposeGap` | Matcher miss → proposeGap |
| `TestActDestructive_ConfirmationUnchanged` | Destructive → confirm turn |
| `TestActTaskStatus_InterceptUnchanged` | task_status intercepted before tool execution |
| `TestActStage0_SameResult` | Stage-0 grammar act executes |
| `TestActLearnedRouter_NoFn_FallbackMatch` | LLM-routed act without Fn → matcher fallback |
| `TestActPathSpeaksTheTiers` | Risk tiers spoken correctly |
| `TestActionActMarkerReferentMovesOnlyTheNamedStoredTask` | Task-status move unchanged |
| `TestActOffAllowlistIsStillRefused` | Off-allowlist act refused |
## 8. Before/after suite results
**Before:** all tests in `internal/router` and `cmd/mavend` pass.
**After:** all tests pass, including 6 new unit tests and 5 new integration tests.
```
internal/router: PASS (0.940s) — 13 actioncandidate tests (7 resolve + 6 validate)
cmd/mavend: PASS (23.5s) — 21 act-path tests (15 existing + 5 new + 1 tracing)
```
## 9. Confirmation that risk/confirmation/execution behavior is unchanged
- **Risk tiers:** `tool.RiskOf` and `tool.PolicyFor` are not touched. Validation happens before risk policy.
- **Confirmation:** `ErrNeedsConfirm` → confirm turn is unchanged. Tested by `TestActDestructive_StillConfirms`.
- **Irreversible:** `ErrNeedsAuthedSurface` → refusal is unchanged. Tested by `TestActIrreversibleValid_NeedsAuthedSurface`.
- **Execution:** `tool.Executor.Exec` is not modified. Validation is a pure pre-screen.
- **Ecosystem intercepts:** Praxis and Hexis intercepts are unchanged. Validation sits before them; unresolved candidates pass through to them as before.
## 10. Remaining downstream consumers of Decision.Slots
After the bridge write (`dec.Slots.Fn = candidate.Fn`, etc.), the following branches read `dec.Slots`:
| Consumer | File | Reads |
|----------|------|-------|
| `resolveTaskStatus` | `cmd/mavend/actions_task.go:109` | `dec.Slots.Fn` (compared to `TaskStatusFn`) |
| `handlePraxisAct` | `cmd/mavend/ecosystem_acts.go:110` | `dec.Slots.HasFn` (guard), `dec.Slots.Fn`, `dec.Slots.Args` |
| `handleHexisAct` | `cmd/mavend/ecosystem_acts.go:660` | via `ActHasEntityTarget(dec)` which reads `dec.Slots.HasFn`, `dec.Slots.Args`, `dec.Slots.Text` |
| `proposeGap` | `cmd/mavend/confirm.go:191` | `dec.Utterance` (not Slots) |
| `tool.Executor.Exec` | `internal/tool/tool.go:156` | called with `dec.Slots.Fn, dec.Slots.Args` directly |
| `actPhrase` | `cmd/mavend/actions_act.go` | `dec.Slots.Fn, dec.Slots.Args` |
| `park` | `cmd/mavend/confirm.go` | `dec.Slots.Fn, dec.Slots.Args` |
## 11. Whether ActionCandidate can become authoritative in a later slice
Yes. The bridge write is the only thing coupling ActionCandidate to Decision.Slots. In a later slice:
1. `resolveTaskStatus` could accept `ActionCandidate` directly instead of reading `dec.Slots.Fn`.
2. `handlePraxisAct` and `handleHexisAct` could accept the candidate instead of `dec.Slots.HasFn`.
3. `tool.Executor.Exec` already takes `name string, args []string` — it could take the candidate's `Fn` and `Args` directly.
4. `ActHasEntityTarget` could accept `ActionCandidate` instead of `Decision`.
The bridge would be removed once all consumers read from the candidate. No semantic changes required — this is a mechanical refactoring of argument passing.
## 12. Commit hash
Pending commit on top of 6a402bf.