feat(run): kernel-native result_schema — validate + in-loop self-repair #24

Merged
steve merged 2 commits from feat/result-schema-submit into main 2026-07-17 17:32:31 +00:00
6 changed files with 545 additions and 0 deletions
+1
View File
@@ -6,6 +6,7 @@ require (
gitea.stevedudenhoeffer.com/steve/majordomo v0.0.0-20260626223738-1fd7109a42f3
github.com/google/uuid v1.6.0
github.com/robfig/cron/v3 v3.0.1
github.com/santhosh-tekuri/jsonschema/v6 v6.0.2
golang.org/x/crypto v0.53.0
gopkg.in/yaml.v3 v3.0.1
)
+4
View File
@@ -13,6 +13,8 @@ github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDk
github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/dlclark/regexp2 v1.11.0 h1:G/nrcoOa7ZXlpoa/91N3X7mM3r8eIlMBBJZvsz/mxKI=
github.com/dlclark/regexp2 v1.11.0/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8=
github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98=
@@ -52,6 +54,8 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN
github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
github.com/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs=
github.com/robfig/cron/v3 v3.0.1/go.mod h1:eQICP3HwyT7UooqI/z+Ov+PtYAWygg1TEWWzGIFLtro=
github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 h1:KRzFb2m7YtdldCEkzs6KqmJw4nqEVZGK7IN2kJkjTuQ=
github.com/santhosh-tekuri/jsonschema/v6 v6.0.2/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
+22
View File
@@ -492,6 +492,19 @@ func (e *Executor) Run(ctx context.Context, ra RunnableAgent, inv tool.Invocatio
runRes, runErr = runAgent(runCtx, ag, input, inv.Images, agent.WithSteer(steer))
}
// ResultSchema (optional): validate the final answer and let the model
// self-repair on the same conversation before the host sees it. Runs
// BEFORE FinalGuard so the delivery check inspects the answer that will
// actually be returned. Fail-open on compile problems; best-effort on
// persistent violations (the host's fallback validator is authoritative).
if runErr == nil && runRes != nil && len(inv.ResultSchema) > 0 {
Review

🟠 ResultSchema corrective-round loop duplicates FinalGuard agent-creation pattern

error-handling, maintainability, performance · flagged by 4 models

  • run/executor.go:500-565 — Copy-paste duplication with FinalGuard pattern (medium). The ResultSchema corrective-round loop mirrors the FinalGuard nudge-round block (lines 567–631) in structure: both create a new agent.New(model, e.systemPrompt(ra), ...), pass agent.WithHistory(runRes.Messages), use agent.WithSteer(steer), accumulate usage identically, and share the same warn-and-keep-original error-handling shape. If the agent-creation contract, shared options, or error-recovery pat…

🪰 Gadfly · advisory

🟠 **ResultSchema corrective-round loop duplicates FinalGuard agent-creation pattern** _error-handling, maintainability, performance · flagged by 4 models_ - **`run/executor.go:500-565` — Copy-paste duplication with FinalGuard pattern (medium).** The ResultSchema corrective-round loop mirrors the FinalGuard nudge-round block (lines 567–631) in structure: both create a new `agent.New(model, e.systemPrompt(ra), ...)`, pass `agent.WithHistory(runRes.Messages)`, use `agent.WithSteer(steer)`, accumulate usage identically, and share the same warn-and-keep-original error-handling shape. If the agent-creation contract, shared options, or error-recovery pat… <sub>🪰 Gadfly · advisory</sub>
e.applyResultSchema(runCtx, resultSchemaDeps{
inv: inv, ra: ra, runRes: runRes, rec: rec,
model: model, toolbox: toolbox, sharedOpts: sharedOpts,
obs: obs, steer: steer, noteCheckpoint: noteCheckpoint,
})
}
// FinalGuard (optional): the run finished successfully — before the result
// is finalized, let the host inspect it for undelivered work (the
// announce-then-stop failure: an artifact rendered into run storage, a
3
@@ -563,6 +576,15 @@ func (e *Executor) Run(ctx context.Context, ra RunnableAgent, inv tool.Invocatio
// shared step observer (audit/steps/critic) is wired per phase by the phase
// runner; checkpointing is phase-boundary granular (completed phases are
// recorded so a resumed run skips them).
//
// ResultSchema is single-loop-only (FinalGuard's scope) — make the drop
// observable rather than silent.
if len(inv.ResultSchema) > 0 {
slog.Warn("run: result_schema ignored on multi-phase run", "run_id", inv.RunID)
if rec != nil {
rec.LogEvent("result_schema_skipped", map[string]any{"error": "multi-phase runs are not schema-validated"})
}
}
runRes, runErr = e.runPhases(runCtx, ra, phaseDeps{
baseModel: model,
baseToolbox: toolbox,
+295
View File
@@ -0,0 +1,295 @@
package run
import (
"context"
"encoding/json"
"errors"
"fmt"
"io/fs"
"log/slog"
"sort"
"strings"
"github.com/santhosh-tekuri/jsonschema/v6"
"gitea.stevedudenhoeffer.com/steve/majordomo/agent"
llm "gitea.stevedudenhoeffer.com/steve/majordomo/llm"
"gitea.stevedudenhoeffer.com/steve/executus/tool"
)
// result_schema.go — kernel-native structured output for single-loop runs.
//
// When tool.Invocation.ResultSchema is set, the executor validates the loop's
// final answer against the schema and, on violation, sends the loop back for
// bounded corrective rounds ON THE SAME CONVERSATION — the model repairs its
// own answer with full task context, which beats any post-hoc host-side
// reformat (the host can't know what the model meant). Mirrors the FinalGuard
// extra-round mechanics (same model, same observers, small step cap), with one
// deliberate contrast: FinalGuard discards the extra round's text, while a
// schema round's text IS the new candidate answer.
//
// Fail-open posture: a schema that doesn't compile logs a WARN and skips
// validation entirely — the host's pre-dispatch compile is the authoritative
// gate, and the kernel must not brick runs on a host bug. Persistent
// violations keep the last candidate output; hosts layer their own fallback
// validator/envelope on top.
const (
// resultSchemaMaxBytes caps the schema the kernel will compile.
resultSchemaMaxBytes = 64 << 10
// resultSchemaMaxRounds caps corrective rounds per run.
resultSchemaMaxRounds = 2
// resultSchemaRoundMaxSteps caps each corrective round. Producing a
// corrected JSON document needs no tools; 2 steps tolerates one stray
// tool call before the reply.
resultSchemaRoundMaxSteps = 2
// resultSchemaExtractCap bounds the output size scanned for a JSON
// document (the balanced-span scan is quadratic in the worst case).
resultSchemaExtractCap = 64 << 10
)
Review

🟡 denyLoader wraps fs.ErrNotExist, leaking misleading sentinel to callers

security · flagged by 1 model

  • run/result_schema.go:50denyLoader wraps fs.ErrNotExist, leaking a misleading sentinel. The fmt.Errorf("... %w", url, fs.ErrNotExist) makes errors.Is(err, fs.ErrNotExist) return true. A security refusal (external $ref blocked) should not masquerade as a missing-file condition. While no current caller of compileResultSchema checks for fs.ErrNotExist, the sentinel is semantically wrong and could mislead future error-handling paths. Recommendation: Drop the %w and us…

🪰 Gadfly · advisory

🟡 **denyLoader wraps fs.ErrNotExist, leaking misleading sentinel to callers** _security · flagged by 1 model_ - **`run/result_schema.go:50` — `denyLoader` wraps `fs.ErrNotExist`, leaking a misleading sentinel.** The `fmt.Errorf("... %w", url, fs.ErrNotExist)` makes `errors.Is(err, fs.ErrNotExist)` return `true`. A security refusal (external `$ref` blocked) should not masquerade as a missing-file condition. While no current caller of `compileResultSchema` checks for `fs.ErrNotExist`, the sentinel is semantically wrong and could mislead future error-handling paths. **Recommendation:** Drop the `%w` and us… <sub>🪰 Gadfly · advisory</sub>
// denyLoader fails every $ref resolution. jsonschema/v6's default compiler
// resolves file:// refs from local disk (and can be configured for http) —
// a caller-supplied schema must never reach either.
type denyLoader struct{}
func (denyLoader) Load(url string) (any, error) {
return nil, fmt.Errorf("external $ref %q refused: remote/file schema loading is disabled: %w", url, fs.ErrNotExist)
Review

Unnecessary []byte→string copy in compileResultSchema: use bytes.NewReader instead of strings.NewReader(string(raw))

performance · flagged by 1 model

🪰 Gadfly · advisory

⚪ **Unnecessary []byte→string copy in compileResultSchema: use bytes.NewReader instead of strings.NewReader(string(raw))** _performance · flagged by 1 model_ <sub>🪰 Gadfly · advisory</sub>
}
// compileResultSchema compiles a raw schema with external loading disabled.
func compileResultSchema(raw json.RawMessage) (*jsonschema.Schema, error) {
if len(raw) > resultSchemaMaxBytes {
return nil, fmt.Errorf("schema is %d bytes (cap %d)", len(raw), resultSchemaMaxBytes)
}
doc, err := jsonschema.UnmarshalJSON(strings.NewReader(string(raw)))
if err != nil {
return nil, fmt.Errorf("schema is not valid JSON: %w", err)
}
c := jsonschema.NewCompiler()
c.UseLoader(denyLoader{})
if err := c.AddResource("inline://result-schema", doc); err != nil {
return nil, err
}
return c.Compile("inline://result-schema")
Review

🟠 64KiB output cap truncates valid >64KiB JSON answers before validation, misclassifying them as non-conforming

correctness, error-handling, maintainability · flagged by 4 models

  • run/result_schema.go:75 — Output truncation silently breaks validation for valid >64 KiB JSON answers. extractResultJSON (lines 73-77) does strings.TrimSpace(s), then caps input at resultSchemaExtractCap (64 KiB) before the json.Valid whole-string check at line 78. A legitimately conforming JSON document larger than 64 KiB is truncated, fails json.Valid (and the fenced/balanced-span scans, which also operate on the truncated string), and is misclassified as "no JSON document…

🪰 Gadfly · advisory

🟠 **64KiB output cap truncates valid >64KiB JSON answers before validation, misclassifying them as non-conforming** _correctness, error-handling, maintainability · flagged by 4 models_ - **`run/result_schema.go:75` — Output truncation silently breaks validation for valid >64 KiB JSON answers.** `extractResultJSON` (lines 73-77) does `strings.TrimSpace(s)`, then caps input at `resultSchemaExtractCap` (64 KiB) *before* the `json.Valid` whole-string check at line 78. A legitimately conforming JSON document larger than 64 KiB is truncated, fails `json.Valid` (and the fenced/balanced-span scans, which also operate on the truncated string), and is misclassified as "no JSON document… <sub>🪰 Gadfly · advisory</sub>
}
// extractResultJSON finds the answer's JSON document: the whole string, a
// fenced block, or the first parseable balanced {...}/[...] span scanning
// left to right. Input is capped at resultSchemaExtractCap.
func extractResultJSON(s string) (json.RawMessage, bool) {
s = strings.TrimSpace(s)
Review

Fence-extraction '' fallback pass re-matches 'json' blocks, can skip a valid later bare-fenced doc (balanced-span fallback usually recovers)

correctness · flagged by 1 model

  • run/result_schema.go:82 — the "```" fallback pass re-matches "```json" occurrences. The fence loop tries "```json" then "```", and strings.Index(s, "```") also matches the leading backticks of a "```json" fence. So a schema-valid JSON in a later bare- fence can be skipped when an earlier `"json"block is present-but-invalid — the fallback lands onjson\n...from the first fence (which failsjson.Valid) instead of the later block. The balanced-span scan (:94`) us…

🪰 Gadfly · advisory

⚪ **Fence-extraction '```' fallback pass re-matches '```json' blocks, can skip a valid later bare-fenced doc (balanced-span fallback usually recovers)** _correctness · flagged by 1 model_ - **`run/result_schema.go:82` — the `"```"` fallback pass re-matches `"```json"` occurrences.** The fence loop tries `"```json"` then `"```"`, and `strings.Index(s, "```")` also matches the leading backticks of a `"```json"` fence. So a schema-valid JSON in a *later* bare-``` fence can be skipped when an earlier `"```json"` block is present-but-invalid — the fallback lands on `json\n...` from the first fence (which fails `json.Valid`) instead of the later block. The balanced-span scan (`:94`) us… <sub>🪰 Gadfly · advisory</sub>
if len(s) > resultSchemaExtractCap {
s = s[:resultSchemaExtractCap]
}
if json.Valid([]byte(s)) {
return json.RawMessage(s), true
}
// Fenced block: ```json ... ``` or bare ``` ... ```.
for _, fence := range []string{"```json", "```"} {
if i := strings.Index(s, fence); i >= 0 {
rest := s[i+len(fence):]
if j := strings.Index(rest, "```"); j >= 0 {
Review

🟠 O(n²) balanced-span scan: per-bracket balancedSpanEnd(O(n)) called in O(n) outer loop; worst-case ~4B ops at 64 KiB cap

correctness, performance · flagged by 3 models

Finding 1 confirmed (run/result_schema.go:93–103): The outer loop at line 94 iterates over every byte in s; for each {/[, balancedSpanEnd (lines 110-139) scans from that position to end-of-string. A string of n unmatched { characters causes n × n iterations. The file itself acknowledges this at lines 39-40 ("the balanced-span scan is quadratic in the worst case"), and resultSchemaExtractCap = 64 << 10 bounds the absolute worst case. The fast paths at lines 78-91 (bare JSON, f…

🪰 Gadfly · advisory

🟠 **O(n²) balanced-span scan: per-bracket balancedSpanEnd(O(n)) called in O(n) outer loop; worst-case ~4B ops at 64 KiB cap** _correctness, performance · flagged by 3 models_ **Finding 1 confirmed** (`run/result_schema.go:93–103`): The outer loop at line 94 iterates over every byte in `s`; for each `{`/`[`, `balancedSpanEnd` (lines 110-139) scans from that position to end-of-string. A string of n unmatched `{` characters causes n × n iterations. The file itself acknowledges this at lines 39-40 (`"the balanced-span scan is quadratic in the worst case"`), and `resultSchemaExtractCap = 64 << 10` bounds the absolute worst case. The fast paths at lines 78-91 (bare JSON, f… <sub>🪰 Gadfly · advisory</sub>
candidate := strings.TrimSpace(rest[:j])
if json.Valid([]byte(candidate)) && candidate != "" {
return json.RawMessage(candidate), true
}
}
}
}
// First parseable balanced span.
for i := 0; i < len(s); i++ {
if s[i] != '{' && s[i] != '[' {
continue
}
if end, ok := balancedSpanEnd(s, i); ok {
candidate := s[i : end+1]
if json.Valid([]byte(candidate)) {
return json.RawMessage(candidate), true
}
Review

🟠 balancedSpanEnd uses single depth counter for both {} and [] — mismatched bracket types accepted

error-handling, maintainability · flagged by 1 model

  • balancedSpanEnd doesn't match bracket types (run/result_schema.go:110–139) The function uses a single depth counter for both {} and [] pairs. A { can be "closed" by ] and vice versa — e.g., input {"a": 1] would return the span {"a": 1] (depth 1 on {, 0 on ]). The subsequent json.Valid guard in extractResultJSON catches this, so there is no current correctness bug, but the function's contract ("honoring JSON … rules") is misleading. A future caller that trusts `b…

🪰 Gadfly · advisory

🟠 **balancedSpanEnd uses single depth counter for both {} and [] — mismatched bracket types accepted** _error-handling, maintainability · flagged by 1 model_ - **`balancedSpanEnd` doesn't match bracket types** (`run/result_schema.go:110–139`) The function uses a single `depth` counter for both `{}` and `[]` pairs. A `{` can be "closed" by `]` and vice versa — e.g., input `{"a": 1]` would return the span `{"a": 1]` (depth 1 on `{`, 0 on `]`). The subsequent `json.Valid` guard in `extractResultJSON` catches this, so there is no *current* correctness bug, but the function's contract ("honoring JSON … rules") is misleading. A future caller that trusts `b… <sub>🪰 Gadfly · advisory</sub>
}
}
return nil, false
}
// balancedSpanEnd returns the index of the bracket closing the span opened
// at start, honoring JSON string/escape rules.
func balancedSpanEnd(s string, start int) (int, bool) {
depth := 0
inStr := false
esc := false
for i := start; i < len(s); i++ {
c := s[i]
if inStr {
switch {
case esc:
esc = false
Review

🟠 balancedSpanEnd uses single depth counter for both {} and [] — bracket types not matched

correctness · flagged by 1 model

  • run/result_schema.go:127-137balancedSpanEnd doesn't match bracket types ({}, []). The function uses a single depth counter for both brace and bracket pairs. A } can close a [ and vice versa. While json.Valid at line 100 rejects invalid candidates, this can cause the extraction to return a valid-but-wrong JSON span when the output contains multiple JSON values with interleaved bracket types. For example, input [1, 2, 3] {"key": [4, 5} ] — the [ at position 0 o…

🪰 Gadfly · advisory

🟠 **balancedSpanEnd uses single depth counter for both {} and [] — bracket types not matched** _correctness · flagged by 1 model_ - **`run/result_schema.go:127-137` — `balancedSpanEnd` doesn't match bracket types (`{`↔`}`, `[`↔`]`)**. The function uses a single `depth` counter for both brace and bracket pairs. A `}` can close a `[` and vice versa. While `json.Valid` at line 100 rejects invalid candidates, this can cause the extraction to return a *valid-but-wrong* JSON span when the output contains multiple JSON values with interleaved bracket types. For example, input `[1, 2, 3] {"key": [4, 5} ]` — the `[` at position 0 o… <sub>🪰 Gadfly · advisory</sub>
case c == '\\':
esc = true
case c == '"':
inStr = false
}
continue
}
switch c {
case '"':
inStr = true
case '{', '[':
depth++
case '}', ']':
depth--
if depth == 0 {
return i, true
}
Review

Unnecessary []byte→string copy in validateResultJSON: use bytes.NewReader instead of strings.NewReader(string(doc))

performance · flagged by 1 model

🪰 Gadfly · advisory

⚪ **Unnecessary []byte→string copy in validateResultJSON: use bytes.NewReader instead of strings.NewReader(string(doc))** _performance · flagged by 1 model_ <sub>🪰 Gadfly · advisory</sub>
}
}
return 0, false
}
// validateResultJSON returns flattened error paths, nil when the doc conforms.
func validateResultJSON(sch *jsonschema.Schema, doc json.RawMessage) []string {
inst, err := jsonschema.UnmarshalJSON(strings.NewReader(string(doc)))
if err != nil {
return []string{"document is not valid JSON: " + err.Error()}
}
err = sch.Validate(inst)
if err == nil {
return nil
}
var ve *jsonschema.ValidationError
if !errors.As(err, &ve) {
return []string{err.Error()}
}
var out []string
Review

🟠 asValidationError uses direct type assertion instead of errors.As, silently losing structured per-field errors if ValidationError is wrapped

error-handling, maintainability · flagged by 4 models

run/result_schema.go:165

🪰 Gadfly · advisory

🟠 **asValidationError uses direct type assertion instead of errors.As, silently losing structured per-field errors if ValidationError is wrapped** _error-handling, maintainability · flagged by 4 models_ **`run/result_schema.go:165`** <sub>🪰 Gadfly · advisory</sub>
flattenValidationError(ve, &out)
sort.Strings(out)
if len(out) == 0 {
out = []string{ve.Error()}
}
return out
}
// flattenValidationError collects leaf causes as "/path: message" lines.
func flattenValidationError(ve *jsonschema.ValidationError, out *[]string) {
Review

flattenValidationError recurses on Causes with no depth guard — stack overflow on cyclic error graph

error-handling · flagged by 1 model

  • flattenValidationError has no recursion-depth guard (run/result_schema.go:174–185) The function recurses on ve.Causes with no bound. If the jsonschema library ever returns a validation error with a cyclic Causes graph (a library bug, or a future version change), this would stack-overflow. Fix: Add a depth counter (e.g., cap at 100) and truncate with a sentinel "..." entry.

🪰 Gadfly · advisory

⚪ **flattenValidationError recurses on Causes with no depth guard — stack overflow on cyclic error graph** _error-handling · flagged by 1 model_ - **`flattenValidationError` has no recursion-depth guard** (`run/result_schema.go:174–185`) The function recurses on `ve.Causes` with no bound. If the jsonschema library ever returns a validation error with a cyclic `Causes` graph (a library bug, or a future version change), this would stack-overflow. **Fix:** Add a depth counter (e.g., cap at 100) and truncate with a sentinel `"..."` entry. <sub>🪰 Gadfly · advisory</sub>
if len(ve.Causes) == 0 {
loc := "/" + strings.Join(ve.InstanceLocation, "/")
if len(ve.InstanceLocation) == 0 {
loc = "/"
}
*out = append(*out, fmt.Sprintf("%s: %v", loc, ve.ErrorKind))
return
}
for _, c := range ve.Causes {
flattenValidationError(c, out)
}
}
// resultSchemaDeps bundles the per-run state applyResultSchema needs from
// the executor's single-loop path.
type resultSchemaDeps struct {
Outdated
Review

🟠 Schema string literals injected verbatim into LLM correction prompt — prompt injection if ResultSchema originates from untrusted input

security · flagged by 1 model

🪰 Gadfly · advisory

🟠 **Schema string literals injected verbatim into LLM correction prompt — prompt injection if ResultSchema originates from untrusted input** _security · flagged by 1 model_ <sub>🪰 Gadfly · advisory</sub>
inv tool.Invocation
ra RunnableAgent
runRes *agent.Result
rec RunRecorder
model llm.Model
toolbox *llm.Toolbox
sharedOpts []agent.Option
obs func(agent.Step)
steer func() []llm.Message
noteCheckpoint func(llm.Message)
}
// applyResultSchema runs the validate + corrective-round loop, mutating
// d.runRes in place. Panic-isolated like safeFinalNudge: a bug in the
// validator (or a pathological schema) must never convert a successful
// run into a panic error.
func (e *Executor) applyResultSchema(runCtx context.Context, d resultSchemaDeps) {
defer func() {
if r := recover(); r != nil {
slog.Error("run: result_schema validation panicked; keeping run result",
"run_id", d.inv.RunID, "panic", r)
}
}()
sch, cerr := compileResultSchema(d.inv.ResultSchema)
if cerr != nil {
slog.Warn("run: result_schema failed to compile; skipping kernel validation",
"run_id", d.inv.RunID, "error", cerr)
if d.rec != nil {
d.rec.LogEvent("result_schema_skipped", map[string]any{"error": cerr.Error()})
}
return
}
for round := 0; ; round++ {
var errs []string
doc, ok := extractResultJSON(d.runRes.Output)
if ok {
errs = validateResultJSON(sch, doc)
if errs == nil {
// Conforming: the bare validated document IS the answer.
d.runRes.Output = string(doc)
if d.rec != nil {
d.rec.LogEvent("result_schema_valid", map[string]any{"rounds": round})
}
return
}
} else {
errs = []string{"no JSON document found in output"}
}
if round >= resultSchemaMaxRounds {
// Best effort exhausted — keep the last candidate; the host's
// own validator decides what to do with it.
if d.rec != nil {
d.rec.LogEvent("result_schema_unmet", map[string]any{
"rounds": round, "validation_errors": errs,
})
}
return
}
correction := resultSchemaCorrection(d.inv.ResultSchema, errs)
if d.rec != nil {
d.rec.LogEvent("result_schema_round", map[string]any{
"round": round + 1, "validation_errors": errs,
})
}
if d.noteCheckpoint != nil {
d.noteCheckpoint(llm.UserText(correction))
}
roundOpts := append([]agent.Option{
agent.WithToolbox(d.toolbox),
agent.WithMaxSteps(resultSchemaRoundMaxSteps),
agent.WithStepObserver(d.obs),
}, d.sharedOpts...)
roundAg := agent.New(d.model, e.systemPrompt(d.ra), roundOpts...)
roundRes, roundErr := roundAg.Run(runCtx, correction,
agent.WithSteer(d.steer), agent.WithHistory(d.runRes.Messages))
if roundRes != nil {
roundUsage := roundRes.Usage
d.runRes.Usage.Add(roundUsage)
}
if roundErr != nil || roundRes == nil {
slog.Warn("run: result_schema corrective round failed; keeping last output",
"run_id", d.inv.RunID, "error", roundErr)
return
}
// The round's text is the NEW candidate (contrast FinalGuard, whose
// extra-round text is discarded meta-commentary) — but a BLANK round
// reply must not clobber the prior non-empty candidate.
if strings.TrimSpace(roundRes.Output) != "" {
d.runRes.Output = roundRes.Output
}
d.runRes.Messages = roundRes.Messages
}
}
// resultSchemaCorrection builds the corrective user turn for one round.
func resultSchemaCorrection(raw json.RawMessage, errs []string) string {
return fmt.Sprintf(`Your final answer must be a SINGLE JSON document conforming to this JSON Schema (no prose, no code fences):
%s
Your previous answer failed validation:
- %s
Reply now with ONLY the corrected JSON document. Preserve your answer's content; do not invent data; use null or empty values for anything missing.`,
string(raw), strings.Join(errs, "\n- "))
}
+208
View File
@@ -0,0 +1,208 @@
package run
import (
"context"
"encoding/json"
"strings"
"testing"
llm "gitea.stevedudenhoeffer.com/steve/majordomo/llm"
"gitea.stevedudenhoeffer.com/steve/majordomo/provider/fake"
"gitea.stevedudenhoeffer.com/steve/executus/tool"
)
func resultSchemaExecutor(t *testing.T, m llm.Model, rec RunRecorder) *Executor {
t.Helper()
ports := Ports{}
if rec != nil {
ports.Audit = auditFunc{start: func(RunInfo) RunRecorder { return rec }}
}
return New(Config{
Registry: tool.NewRegistry(),
Models: func(ctx context.Context, _ string) (context.Context, llm.Model, error) { return ctx, m, nil },
Ports: ports,
})
}
var personSchema = json.RawMessage(`{"type":"object","required":["name"],"properties":{"name":{"type":"string"}},"additionalProperties":false}`)
// TestResultSchema_ConformingAnswerPassesThrough — a first-try conforming
// answer (fence-wrapped, even) is replaced by the bare JSON with no extra
// model rounds.
func TestResultSchema_ConformingAnswerPassesThrough(t *testing.T) {
fp := fake.New("fake")
fp.Enqueue("m", fake.Reply("Here you go:\n```json\n{\"name\": \"mort\"}\n```"))
m, _ := fp.Model("m")
ex := resultSchemaExecutor(t, m, nil)
res := ex.Run(context.Background(),
RunnableAgent{Name: "g", ModelTier: "m"},
tool.Invocation{RunID: "r-rs1", CallerID: "c", ResultSchema: personSchema}, "who are you")
if res.Err != nil {
t.Fatalf("run error: %v", res.Err)
}
if res.Output != `{"name": "mort"}` {
t.Fatalf("output = %q, want the bare validated JSON", res.Output)
}
if n := fp.CallCount("m"); n != 1 {
t.Fatalf("model calls = %d, want 1 (no corrective round)", n)
}
}
// TestResultSchema_CorrectiveRoundRepairs — a non-conforming first answer
// triggers one corrective round whose text becomes the run output.
func TestResultSchema_CorrectiveRoundRepairs(t *testing.T) {
fp := fake.New("fake")
fp.Enqueue("m",
fake.Reply(`{"name": 42}`), // wrong type
fake.Reply(`{"name": "fixed"}`), // corrective round conforms
)
m, _ := fp.Model("m")
rec := &eventRecorder{}
ex := resultSchemaExecutor(t, m, rec)
res := ex.Run(context.Background(),
RunnableAgent{Name: "g", ModelTier: "m"},
tool.Invocation{RunID: "r-rs2", CallerID: "c", ResultSchema: personSchema}, "go")
if res.Err != nil {
t.Fatalf("run error: %v", res.Err)
}
if res.Output != `{"name": "fixed"}` {
t.Fatalf("output = %q, want the corrected JSON", res.Output)
}
if n := fp.CallCount("m"); n != 2 {
t.Fatalf("model calls = %d, want 2", n)
}
if !recHas(rec, "result_schema_round") || !recHas(rec, "result_schema_valid") {
t.Fatalf("expected round + valid audit events, got %v", rec.events)
}
}
// TestResultSchema_PersistentViolationKeepsLastOutput — rounds exhaust, the
// last candidate stands, and result_schema_unmet is recorded (the host's own
// validator is the fallback authority).
func TestResultSchema_PersistentViolationKeepsLastOutput(t *testing.T) {
fp := fake.New("fake")
fp.Enqueue("m",
fake.Reply("not json at all"),
fake.Reply("still prose"),
fake.Reply("prose forever"),
)
m, _ := fp.Model("m")
rec := &eventRecorder{}
ex := resultSchemaExecutor(t, m, rec)
res := ex.Run(context.Background(),
RunnableAgent{Name: "g", ModelTier: "m"},
tool.Invocation{RunID: "r-rs3", CallerID: "c", ResultSchema: personSchema}, "go")
if res.Err != nil {
t.Fatalf("run error: %v", res.Err)
}
if res.Output != "prose forever" {
t.Fatalf("output = %q, want the last candidate kept", res.Output)
}
if n := fp.CallCount("m"); n != 1+resultSchemaMaxRounds {
t.Fatalf("model calls = %d, want %d", n, 1+resultSchemaMaxRounds)
}
if !recHas(rec, "result_schema_unmet") {
t.Fatalf("expected result_schema_unmet, got %v", rec.events)
}
}
// TestResultSchema_NoSchemaUnchanged — anchor: without a schema the output
// and call count are untouched.
func TestResultSchema_NoSchemaUnchanged(t *testing.T) {
fp := fake.New("fake")
fp.Enqueue("m", fake.Reply("plain prose answer"))
m, _ := fp.Model("m")
ex := resultSchemaExecutor(t, m, nil)
res := ex.Run(context.Background(),
RunnableAgent{Name: "g", ModelTier: "m"},
tool.Invocation{RunID: "r-rs4", CallerID: "c"}, "go")
if res.Err != nil || res.Output != "plain prose answer" {
t.Fatalf("output = %q err = %v", res.Output, res.Err)
}
if n := fp.CallCount("m"); n != 1 {
t.Fatalf("model calls = %d, want 1", n)
}
}
// TestResultSchema_CompileFailureFailsOpen — an uncompilable schema skips
// validation (WARN + audit event), never bricks the run.
func TestResultSchema_CompileFailureFailsOpen(t *testing.T) {
fp := fake.New("fake")
fp.Enqueue("m", fake.Reply("prose"))
m, _ := fp.Model("m")
rec := &eventRecorder{}
ex := resultSchemaExecutor(t, m, rec)
res := ex.Run(context.Background(),
RunnableAgent{Name: "g", ModelTier: "m"},
tool.Invocation{RunID: "r-rs5", CallerID: "c",
ResultSchema: json.RawMessage(`{"type": "not-a-type"}`)}, "go")
if res.Err != nil {
t.Fatalf("run error: %v", res.Err)
}
if res.Output != "prose" {
t.Fatalf("output = %q, want untouched", res.Output)
}
if !recHas(rec, "result_schema_skipped") {
t.Fatalf("expected result_schema_skipped, got %v", rec.events)
}
}
// TestCompileResultSchema_ExternalRefsFailClosed — http(s) AND file $refs
// must not resolve (jsonschema/v6's default loader reads local disk).
func TestCompileResultSchema_ExternalRefsFailClosed(t *testing.T) {
for _, ref := range []string{
Review

http:// $ref not covered by deny-loader test; unconditional denyLoader is correct but test gap creates regression risk

security · flagged by 1 model

run/result_schema_test.go:164-177

🪰 Gadfly · advisory

⚪ **http:// $ref not covered by deny-loader test; unconditional denyLoader is correct but test gap creates regression risk** _security · flagged by 1 model_ **`run/result_schema_test.go:164-177`** <sub>🪰 Gadfly · advisory</sub>
"https://example.com/schema.json",
"file:///etc/passwd",
} {
raw := json.RawMessage(`{"$ref": "` + ref + `"}`)
if _, err := compileResultSchema(raw); err == nil {
t.Errorf("$ref %q compiled — external loading must fail closed", ref)
} else if !strings.Contains(err.Error(), "refused") && !strings.Contains(err.Error(), "not exist") {
// Any error is acceptable as long as it FAILS; the refused
// wording just confirms the deny loader was consulted.
t.Logf("$ref %q failed with: %v", ref, err)
}
}
}
// TestExtractResultJSON_Shapes — extraction tolerance.
func TestExtractResultJSON_Shapes(t *testing.T) {
cases := []struct {
in string
want string
ok bool
}{
{`{"a":1}`, `{"a":1}`, true},
{"prefix text {\"a\": {\"b\": \"}\"}} suffix", `{"a": {"b": "}"}}`, true},
{"```json\n[1,2]\n```", `[1,2]`, true},
{"no json here", "", false},
{"broken { \"a\": ", "", false},
}
for _, c := range cases {
got, ok := extractResultJSON(c.in)
if ok != c.ok || (ok && string(got) != c.want) {
t.Errorf("extract(%q) = (%q, %v), want (%q, %v)", c.in, got, ok, c.want, c.ok)
}
}
}
// recHas reports whether the recorder saw an event type.
func recHas(r *eventRecorder, name string) bool {
Review

*recHas free function should be a method on eventRecorder (same package, non-idiomatic name)

maintainability · flagged by 2 models

  • recHas free function should be a method (run/result_schema_test.go:201–208). recHas(r *eventRecorder, name string) takes the receiver as a plain parameter. eventRecorder lives in final_guard_test.go in the same package run, so func (r *eventRecorder) has(name string) bool is fully valid Go and matches every other type in the codebase that exposes a predicate. The recHas name is also non-idiomatic (Go convention: method has, not package-level recHas). Low-churn fix: rena…

🪰 Gadfly · advisory

⚪ **recHas free function should be a method on *eventRecorder (same package, non-idiomatic name)** _maintainability · flagged by 2 models_ - **`recHas` free function should be a method** (`run/result_schema_test.go:201–208`). `recHas(r *eventRecorder, name string)` takes the receiver as a plain parameter. `eventRecorder` lives in `final_guard_test.go` in the same `package run`, so `func (r *eventRecorder) has(name string) bool` is fully valid Go and matches every other type in the codebase that exposes a predicate. The `recHas` name is also non-idiomatic (Go convention: method `has`, not package-level `recHas`). Low-churn fix: rena… <sub>🪰 Gadfly · advisory</sub>
for _, e := range r.events {
if e == name {
return true
}
}
return false
}
+15
View File
@@ -19,6 +19,7 @@ package tool
import (
"context"
"encoding/json"
"fmt"
"sync"
"time"
@@ -238,6 +239,20 @@ type Invocation struct {
// persisted agent row. Skill runs ignore this field.
SystemPromptPrepend string
// ResultSchema, when non-empty, is a JSON Schema (draft 2020-12) the
// run's final answer must conform to. The executor validates the
// single-loop final output against it and, on violation, runs up to
// two bounded corrective rounds on the same conversation so the
// model self-repairs with full task context; a conforming answer is
// replaced by the bare validated JSON document. The kernel is
// FAIL-OPEN on schema problems (an uncompilable schema logs a WARN
// and skips validation — the host's own pre-dispatch compile is the
// authoritative gate) and best-effort on persistent violations (the
// last output stands; hosts keep their own fallback validator /
// error envelope). Multi-phase runs are not validated, matching
// FinalGuard's single-loop-only scope.
ResultSchema json.RawMessage
// SuppressDelivery, when true, instructs the skill executor to
// SKIP its OutputTarget Delivery (Deliver / DeliverError) entirely.
// The run still produces an output string (returned from Run) and