feat(run): kernel-native result_schema — validate + in-loop self-repair #24
@@ -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
|
||||
)
|
||||
|
||||
@@ -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=
|
||||
|
||||
@@ -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 {
|
||||
|
|
||||
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
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
)
|
||||
|
gitea-actions
commented
🟡 denyLoader wraps fs.ErrNotExist, leaking misleading sentinel to callers security · flagged by 1 model
🪰 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)
|
||||
|
gitea-actions
commented
⚪ 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")
|
||||
|
gitea-actions
commented
🟠 64KiB output cap truncates valid >64KiB JSON answers before validation, misclassifying them as non-conforming correctness, error-handling, maintainability · flagged by 4 models
🪰 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)
|
||||
|
gitea-actions
commented
⚪ Fence-extraction ' correctness · flagged by 1 model
🪰 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 {
|
||||
|
gitea-actions
commented
🟠 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 ( 🪰 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
|
||||
}
|
||||
|
gitea-actions
commented
🟠 balancedSpanEnd uses single depth counter for both {} and [] — mismatched bracket types accepted error-handling, maintainability · flagged by 1 model
🪰 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
|
||||
|
gitea-actions
commented
🟠 balancedSpanEnd uses single depth counter for both {} and [] — bracket types not matched correctness · flagged by 1 model
🪰 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
|
||||
}
|
||||
|
gitea-actions
commented
⚪ 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
|
||||
|
gitea-actions
commented
🟠 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
🪰 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) {
|
||||
|
gitea-actions
commented
⚪ flattenValidationError recurses on Causes with no depth guard — stack overflow on cyclic error graph error-handling · flagged by 1 model
🪰 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 {
|
||||
|
gitea-actions
commented
🟠 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- "))
|
||||
}
|
||||
@@ -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{
|
||||
|
gitea-actions
commented
⚪ http:// $ref not covered by deny-loader test; unconditional denyLoader is correct but test gap creates regression risk security · flagged by 1 model
🪰 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 {
|
||||
|
gitea-actions
commented
⚪ *recHas free function should be a method on eventRecorder (same package, non-idiomatic name) maintainability · flagged by 2 models
🪰 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
|
||||
}
|
||||
@@ -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
|
||||
|
||||
🟠 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 newagent.New(model, e.systemPrompt(ra), ...), passagent.WithHistory(runRes.Messages), useagent.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