diff --git a/docs/architecture/governance.md b/docs/architecture/governance.md index bb8586f6b2..6f26f6fbbc 100644 --- a/docs/architecture/governance.md +++ b/docs/architecture/governance.md @@ -76,7 +76,9 @@ child prompt-injection defense. `Service.admitMode` (`internal/adapter/server/service.go`) refuses a created or switched mode above the ceiling, and `auto` without an enforcing guardrails checker; a server whose default mode is `auto` refuses to start without one. Unchecked autonomy is the -explicit `yolo` choice, where a configured checker is advisory. `admitStoredMode` +explicit `yolo` choice, where a configured checker never blocks: the engine skips it +unless the reviewer opts in (`YoloReviewPolicyProvider`), and then reviews off the +dispatch path, cancelling reviews still in flight when the run ends. `admitStoredMode` re-checks a persisted session's mode at run, approval resume, fork, and clear, so a mode admitted under a looser deployment fails with `FailedPrecondition` until the owner switches it; history is untouched. A session parked awaiting an approval in such a mode diff --git a/engine/CHANGELOG.md b/engine/CHANGELOG.md index 3c04699626..29f09cf9cf 100644 --- a/engine/CHANGELOG.md +++ b/engine/CHANGELOG.md @@ -13,6 +13,18 @@ The covered surface is the eight core packages (`session`, `governance`, `learni ### Added +- **Yolo advisory review opt-in** — adds `agent.YoloReviewPolicyProvider`. A + `ToolReviewer` that implements it and returns true from `GuardrailYoloReviews` + has its applicable action and inbound reviews in yolo-mode sessions run in the + background: the tool call neither waits for the verdict nor fails closed, and + the verdict is emitted as an advisory guardrail `EvHook` after the tool result + and before the run's terminal event. At most four run at once; calls beyond + that are not reviewed. Run end cancels reviews still in flight and drops their + verdicts. Advisory verdicts are not recorded in the review trajectory. Adds + `ReviewEvidencePreparation.Snapshot`, set for these reviews, asking the + preparer to capture workspace evidence at preparation time because the + reviewed call may change it before the checker reads it. Added (minor). + - **Provider stop detail** — adds `port.Chunk.StopDetail`, the provider's own terminal label (for example `max_tokens`, `refusal`, `length`) that an adapter collapsed into `StopError` on `ChunkDone`. A stop-only `StopError` run now names @@ -455,6 +467,15 @@ The covered surface is the eight core packages (`session`, `governance`, `learni ### Changed +- **Yolo-mode sessions skip contextual review by default** — a yolo-mode session + no longer calls the `ToolReviewer` for action or inbound review unless the + reviewer implements `YoloReviewPolicyProvider` and opts in. Previously review + ran inline and the tool waited for an advisory verdict. A terminal review + failure (for example evidence preparation) no longer fails closed in a yolo + session. Narrower delegated children of a yolo parent keep enforcing. + Changed (breaking for embedders that relied on inline yolo review; pre-v1 + minor bump). + - **Agent-bound authority enforcement** — `Session.BindAuthority` requires a ceiling when `AgentDefinitionName` is set and rejects tools outside that ceiling. `GrantToolAuthority` and `CompleteWorkspaceEnrollment` reject tool grants beyond diff --git a/engine/agent/actionreview.go b/engine/agent/actionreview.go index 6c22d99a97..bf7b1c2d87 100644 --- a/engine/agent/actionreview.go +++ b/engine/agent/actionreview.go @@ -334,7 +334,7 @@ func (e *Engine) establishReviewPrincipal(r *Run) { r.reviewRoot.establishInstructions(instructions) } -func (e *Engine) prepareActionReview(ctx context.Context, r *Run, sess *session.Session, env tool.Environment, call session.ToolCall) actionReview { +func (e *Engine) prepareActionReview(ctx context.Context, r *Run, sess *session.Session, env tool.Environment, call session.ToolCall, snapshot bool) actionReview { e.establishReviewPrincipal(r) target, paths := reviewTarget(call) facts, complete, principalRevision := r.reviewRoot.principalSnapshotWithRevision() @@ -382,7 +382,7 @@ func (e *Engine) prepareActionReview(ctx context.Context, r *Run, sess *session. var prepared PreparedReviewEvidence var prepareErr error if ctx.Err() == nil { - prepared, prepareErr = e.prepareReviewEvidence(ctx, r, sess, env, req, nil) + prepared, prepareErr = e.prepareReviewEvidence(ctx, r, sess, env, req, nil, snapshot) } else { prepareErr = reviewFailure(ReviewFailureTimeout) } @@ -428,12 +428,12 @@ func (e *Engine) authorizeReviewEvidenceRead(ctx context.Context, r *Run, sess * return nil } -func (e *Engine) prepareReviewEvidence(ctx context.Context, r *Run, sess *session.Session, env tool.Environment, req ToolReviewRequest, result *session.ToolResult) (PreparedReviewEvidence, error) { +func (e *Engine) prepareReviewEvidence(ctx context.Context, r *Run, sess *session.Session, env tool.Environment, req ToolReviewRequest, result *session.ToolResult, snapshot bool) (PreparedReviewEvidence, error) { if r.reviewRoot.preparer == nil { return PreparedReviewEvidence{}, nil } prepared, err := r.reviewRoot.preparer.PrepareReviewEvidence(ctx, ReviewEvidencePreparation{ - Request: req, Environment: env, Owner: sess.Owner.Clone(), Result: result, + Request: req, Environment: env, Owner: sess.Owner.Clone(), Result: result, Snapshot: snapshot, Authorize: func(authCtx context.Context, call session.ToolCall) error { paths := tool.LocalFileOperands(call.Name, call.Args) if call.Name != "Read" || len(paths) != 1 { @@ -662,9 +662,9 @@ type actionReviewAssessment struct { grantHit bool } -func (e *Engine) prepareActionAssessment(ctx context.Context, r *Run, sess *session.Session, env tool.Environment, call session.ToolCall) actionReviewAssessment { +func (e *Engine) prepareActionAssessment(ctx context.Context, r *Run, sess *session.Session, env tool.Environment, call session.ToolCall, snapshot bool) actionReviewAssessment { reviewCtx, cancel := context.WithTimeoutCause(ctx, reviewAssessmentTimeout, reviewFailure(ReviewFailureTimeout)) - action := e.prepareActionReview(reviewCtx, r, sess, env, call) + action := e.prepareActionReview(reviewCtx, r, sess, env, call, snapshot) assessment := actionReviewAssessment{action: action, ctx: reviewCtx, cancel: cancel} if action.prepareErr == nil && reviewCtx.Err() == nil { if issuer, ok := r.reviewRoot.reviewer.(ReviewGrantStore); ok && action.repeat { @@ -944,9 +944,10 @@ func (e *Engine) reviewActionWithTail(ctx context.Context, r *Run, sess *session e.publishToolResult(r, session.Event{Type: session.EvToolResult, Turn: turnIdx, ToolResult: ptr(res)}) return res, nil, false, false } + e.startBackgroundActionReview(r, sess, env, turnIdx, call) return session.ToolResult{}, nil, false, true } - assessment := e.prepareActionAssessment(ctx, r, sess, env, call) + assessment := e.prepareActionAssessment(ctx, r, sess, env, call, false) assessAction(ctx, r, &assessment) if ctx.Err() != nil { r.recordGuardrailUsageWhileActive(ctx, sess, assessment.usage) @@ -990,7 +991,7 @@ func (e *Engine) armPostActionGrant(ctx context.Context, r *Run, sess *session.S if !repeat || !hasIssuer || park != nil || cancelled || result.IsError { return } - post := e.prepareActionReview(ctx, r, sess, env, call) + post := e.prepareActionReview(ctx, r, sess, env, call, false) if post.close != nil { defer post.close() } diff --git a/engine/agent/backgroundreview.go b/engine/agent/backgroundreview.go new file mode 100644 index 0000000000..1461250a61 --- /dev/null +++ b/engine/agent/backgroundreview.go @@ -0,0 +1,196 @@ +package agent + +import ( + "context" + "sync" + "time" + + "github.com/stacklok/mecatl/engine/governance" + "github.com/stacklok/mecatl/engine/port" + "github.com/stacklok/mecatl/engine/session" + "github.com/stacklok/mecatl/engine/tool" +) + +// backgroundReviewDrainGrace bounds how long run end waits for cancelled yolo +// advisory reviews to unwind. A var only as a TEST seam, like childDrainCap. +var backgroundReviewDrainGrace = 1 * time.Second + +// maxBackgroundReviews caps concurrent advisory checker runs per run, bounding +// provider load, cost, and held evidence on read-heavy runs. +const maxBackgroundReviews = 4 + +// backgroundReviews is a run's set of opted-in yolo advisory reviews. Each +// review snapshots its request and evidence on the dispatching goroutine, then +// runs the checker off the dispatch path on a context detached from the run's +// own cancellation. At most maxBackgroundReviews run at once; a review over the +// cap is skipped. Advisory verdicts never feed the shared trajectory, so +// enforcing reviews of delegated children see the same facts in dispatch order. Run end cancels them; their advisory events go out through the +// child registry's sealed emit, so none follows the run's terminal event. The +// context is created on first use, so a run without background reviews pays +// no allocations for the set. +type backgroundReviews struct { + mu sync.Mutex + wg sync.WaitGroup + ctx context.Context //nolint:containedctx // run-scoped review lifetime, cancelled at run end + cancel context.CancelFunc + closed bool + active int +} + +// reserve claims a review slot and returns the review context, creating it +// from parent on first use. It reports false once run end has begun draining +// or when maxBackgroundReviews are already running; a true return must be +// followed by start, which releases the slot. +func (b *backgroundReviews) reserve(parent context.Context) (context.Context, bool) { + b.mu.Lock() + defer b.mu.Unlock() + if b.closed || b.active >= maxBackgroundReviews { + return nil, false + } + b.active++ + if b.ctx == nil { + if parent == nil { + parent = context.Background() + } + b.ctx, b.cancel = context.WithCancel(context.WithoutCancel(parent)) + } + return b.ctx, true +} + +// start runs a reserved review and releases its slot when it ends. It reports +// false, releasing the slot at once, when run end began after the reservation. +func (b *backgroundReviews) start(review func()) bool { + b.mu.Lock() + defer b.mu.Unlock() + if b.closed { + b.active-- + return false + } + b.wg.Add(1) + go func() { + defer b.wg.Done() + defer b.releaseSlot() + review() + }() + return true +} + +func (b *backgroundReviews) releaseSlot() { + b.mu.Lock() + b.active-- + b.mu.Unlock() +} + +// drain stops new reviews, cancels in-flight ones, and waits up to the grace for +// them to unwind. It is idempotent and reports whether every review unwound. +func (b *backgroundReviews) drain() bool { + b.mu.Lock() + b.closed = true + cancel := b.cancel + b.mu.Unlock() + if cancel == nil { + return true + } + cancel() + done := make(chan struct{}) + go func() { + b.wg.Wait() + close(done) + }() + timer := time.NewTimer(backgroundReviewDrainGrace) + defer timer.Stop() + select { + case <-done: + return true + case <-timer.C: + return false + } +} + +func (r *Run) drainBackgroundReviews(ctx context.Context) { + if !r.backgroundReviews.drain() { + r.diag.Log(ctx, port.LevelWarn, "abandoning yolo advisory guardrail reviews that did not stop within the run-end grace", + "grace", backgroundReviewDrainGrace.String()) + } +} + +func advisoryDisposition(result ToolReviewResult, err error, clean string) string { + if err != nil || result.Assessment != ReviewAcceptable { + return "pass_advisory" + } + return clean +} + +// launchBackgroundReview runs assess off the dispatch path, records its checker +// usage, and publishes the advisory event built by publish unless run end +// cancelled the review first. release always runs once. +func (r *Run) launchBackgroundReview(ctx context.Context, assess func() session.AuxiliaryUsage, publish func() session.Event, release func()) { + if !r.backgroundReviews.start(func() { + defer release() + usage := assess() + r.enqueueAuxiliaryUsageWhileActive(ctx, RemapAuxiliaryUsage(ctx, r.diag, session.UsageKindGuardrail, usage)) + if ctx.Err() == nil { + r.children.safeEmit(publish()) + } + }) { + release() + } +} + +// startBackgroundActionReview reviews an opted-in yolo action without delaying +// it. The call has already been authorized and proceeds regardless of verdict. +func (e *Engine) startBackgroundActionReview(r *Run, sess *session.Session, env tool.Environment, turnIdx int, call session.ToolCall) { + if r.reviewRoot == nil || !yoloBackgroundReview(r.reviewRoot.reviewer, sess.Mode, call.Name, ReviewJobAction) { + return + } + bg, ok := r.backgroundReviews.reserve(r.ctx) + if !ok { + return + } + sessID := sess.ID + a := e.prepareActionAssessment(bg, r, sess, env, call, true) + r.launchBackgroundReview(bg, func() session.AuxiliaryUsage { + assessAction(bg, r, &a) + return a.usage + }, func() session.Event { + result := a.result + if a.err != nil { + result.Assessment = ReviewUnresolved + } + r.publishActionDetail(bg, sessID, a.action.request.ReviewID, a.action.request, result, a.err) + guardrail := reviewMachinePayload(r.reviewRoot.reviewer, a.action.request, result, a.err, advisoryDisposition(result, a.err, "execute")) + return session.Event{Type: session.EvHook, Turn: turnIdx, Hook: &session.HookPayload{Phase: string(governance.PhasePreToolUse), Tool: call.Name, Decision: session.HookInfo, CallID: call.ID, Guardrail: guardrail}} + }, func() { + a.cancel() + if a.action.close != nil { + a.action.close() + } + }) +} + +// startBackgroundInboundReview inspects an opted-in yolo result that has +// already been released to the model; the verdict is advisory only. +func (e *Engine) startBackgroundInboundReview(r *Run, sess *session.Session, env tool.Environment, turnIdx int, call session.ToolCall, result session.ToolResult) { + if r.reviewRoot == nil || !yoloBackgroundReview(r.reviewRoot.reviewer, sess.Mode, call.Name, ReviewJobInbound) { + return + } + bg, ok := r.backgroundReviews.reserve(r.ctx) + if !ok { + return + } + sessID := sess.ID + a := inboundAssessment{applies: true, mode: sess.Mode, snapshot: true} + e.prepareInboundRequest(bg, r, sess, env, call, result, &a) + r.launchBackgroundReview(bg, func() session.AuxiliaryUsage { + assessInbound(bg, r, &a) + return a.usage + }, func() session.Event { + r.publishInboundDetail(bg, sessID, a) + return inboundReviewEvent(r, turnIdx, call, a, advisoryDisposition(a.result, a.err, "release_result")) + }, func() { + a.cancel() + if a.close != nil { + a.close() + } + }) +} diff --git a/engine/agent/dispatch.go b/engine/agent/dispatch.go index 81371f7a1d..43f890ade1 100644 --- a/engine/agent/dispatch.go +++ b/engine/agent/dispatch.go @@ -361,7 +361,7 @@ func (e *Engine) runReadBatch(ctx context.Context, r *Run, sess *session.Session // Human permission and pre-hook waits are complete. Prepare each sibling // serially, launching its private checker before preparing the next. - e.fanOutReadBatchActionAssessments(ctx, r, sess, env, prepared) + e.fanOutReadBatchActionAssessments(ctx, r, sess, env, turnIdx, prepared) if ctx.Err() != nil { r.recordCompletedActionUsage(ctx, sess, prepared) return out, true @@ -422,7 +422,7 @@ func (e *Engine) runReadBatch(ctx context.Context, r *Run, sess *session.Session // fanOutReadBatchActionAssessments starts every eligible private action review, // then joins them before the ordered decision drain begins. -func (e *Engine) fanOutReadBatchActionAssessments(ctx context.Context, r *Run, sess *session.Session, env tool.Environment, prepared []readBatchPending) { +func (e *Engine) fanOutReadBatchActionAssessments(ctx context.Context, r *Run, sess *session.Session, env tool.Environment, turnIdx int, prepared []readBatchPending) { var reviewWG sync.WaitGroup for i := range prepared { if ctx.Err() != nil { @@ -433,13 +433,15 @@ func (e *Engine) fanOutReadBatchActionAssessments(ctx context.Context, r *Run, s continue } if applies, _ := reviewPolicy(r.reviewRoot.reviewer, sess.Mode, p.call.Name, ReviewJobAction, false); applies { - assessment := e.prepareActionAssessment(ctx, r, sess, env, p.call) + assessment := e.prepareActionAssessment(ctx, r, sess, env, p.call, false) p.assessment = &assessment reviewWG.Add(1) go func(a *actionReviewAssessment) { defer reviewWG.Done() assessAction(ctx, r, a) }(p.assessment) + } else { + e.startBackgroundActionReview(r, sess, env, turnIdx, p.call) } } reviewWG.Wait() @@ -1754,6 +1756,9 @@ func (e *Engine) executePrivate(ctx context.Context, r *Run, sess *session.Sessi if r.reviewRoot != nil && r.reviewRoot.reviewer != nil { assessment = e.prepareInboundAssessment(ctx, r, sess, env, c, res) assessInbound(ctx, r, &assessment) + if !assessment.applies { + e.startBackgroundInboundReview(r, sess, env, turnIdx, c, res) + } } return executionRecord{ result: res, queued: queued, duration: dur, assessment: assessment, postEvents: postEvents, diff --git a/engine/agent/inboundreview.go b/engine/agent/inboundreview.go index 365966f221..590e0e5d16 100644 --- a/engine/agent/inboundreview.go +++ b/engine/agent/inboundreview.go @@ -44,14 +44,23 @@ type inboundAssessment struct { enforce bool mode session.PermissionMode principalRevision uint64 + snapshot bool } // reviewPolicy resolves the reviewer's configured applicability and enforcement // for the EXECUTING session's mode. A shared reviewRoot carries the operator's -// undemoted rules, so a yolo session (no enforcing checker; advisory monitoring -// only) demotes enforcement here while a narrower delegated child of that yolo -// parent keeps it. +// undemoted rules, so a yolo session never reviews inline here while a narrower +// delegated child of that yolo parent keeps enforcing. Opted-in yolo review runs +// through yoloBackgroundReview instead. func reviewPolicy(reviewer ToolReviewer, mode session.PermissionMode, toolName string, job ReviewJob, operationalFailure bool) (bool, bool) { + if mode == session.ModeYolo { + return false, false + } + return configuredReviewPolicy(reviewer, toolName, job, operationalFailure) +} + +// configuredReviewPolicy is the operator's policy independent of permission mode. +func configuredReviewPolicy(reviewer ToolReviewer, toolName string, job ReviewJob, operationalFailure bool) (bool, bool) { applies, enforce := job == ReviewJobAction, true if policy, ok := reviewer.(ReviewPolicyProvider); ok { applies, enforce = policy.GuardrailReviewPolicy(toolName, job, operationalFailure) @@ -59,7 +68,20 @@ func reviewPolicy(reviewer ToolReviewer, mode session.PermissionMode, toolName s // ToolReviewer predated inbound escrow. Reviewers that do not explicitly // advertise policy retain action-only behavior rather than unexpectedly // inspecting and holding every tool result. - return applies, enforce && mode != session.ModeYolo + return applies, enforce +} + +// yoloBackgroundReview reports whether a yolo session runs this review as +// advisory background work: only when the reviewer opts in and a rule applies. +func yoloBackgroundReview(reviewer ToolReviewer, mode session.PermissionMode, toolName string, job ReviewJob) bool { + if mode != session.ModeYolo || reviewer == nil { + return false + } + if opt, ok := reviewer.(YoloReviewPolicyProvider); !ok || !opt.GuardrailYoloReviews() { + return false + } + applies, _ := configuredReviewPolicy(reviewer, toolName, job, false) + return applies } func (e *Engine) prepareInboundAssessment(ctx context.Context, r *Run, sess *session.Session, env tool.Environment, call session.ToolCall, result session.ToolResult) inboundAssessment { @@ -71,6 +93,13 @@ func (e *Engine) prepareInboundAssessment(ctx context.Context, r *Run, sess *ses if !applies { return assessment } + e.prepareInboundRequest(ctx, r, sess, env, call, result, &assessment) + return assessment +} + +// prepareInboundRequest snapshots the review request and evidence for an +// applicable inbound assessment. It must run on the dispatching goroutine. +func (e *Engine) prepareInboundRequest(ctx context.Context, r *Run, sess *session.Session, env tool.Environment, call session.ToolCall, result session.ToolResult, assessment *inboundAssessment) { reviewCtx, cancel := context.WithTimeoutCause(ctx, reviewAssessmentTimeout, reviewFailure(ReviewFailureTimeout)) assessment.ctx, assessment.cancel = reviewCtx, cancel e.establishReviewPrincipal(r) @@ -95,7 +124,7 @@ func (e *Engine) prepareInboundAssessment(ctx context.Context, r *Run, sess *ses var prepared PreparedReviewEvidence var prepareErr error if reviewCtx.Err() == nil { - prepared, prepareErr = e.prepareReviewEvidence(reviewCtx, r, sess, env, assessment.request, &result) + prepared, prepareErr = e.prepareReviewEvidence(reviewCtx, r, sess, env, assessment.request, &result, assessment.snapshot) } else { prepareErr = reviewFailure(ReviewFailureTimeout) } @@ -104,7 +133,6 @@ func (e *Engine) prepareInboundAssessment(ctx context.Context, r *Run, sess *ses } assessment.request.Evidence, assessment.request.EvidenceComplete = prepared.Evidence, prepared.Complete assessment.source, assessment.close, assessment.err = prepared.Source, prepared.Close, prepareErr - return assessment } func assessInbound(parent context.Context, r *Run, assessment *inboundAssessment) { @@ -266,8 +294,12 @@ func (e *Engine) emitInboundReview(r *Run, turnIdx int, call session.ToolCall, a if !assessment.applies { return } + e.emit(r, inboundReviewEvent(r, turnIdx, call, assessment, disposition)) +} + +func inboundReviewEvent(r *Run, turnIdx int, call session.ToolCall, assessment inboundAssessment, disposition string) session.Event { machine := reviewMachinePayload(r.reviewRoot.reviewer, assessment.request, assessment.result, assessment.err, disposition) - e.emit(r, session.Event{Type: session.EvHook, Turn: turnIdx, Hook: &session.HookPayload{Phase: string(governance.PhasePostToolUse), Tool: call.Name, Decision: session.HookInfo, CallID: call.ID, Guardrail: machine}}) + return session.Event{Type: session.EvHook, Turn: turnIdx, Hook: &session.HookPayload{Phase: string(governance.PhasePostToolUse), Tool: call.Name, Decision: session.HookInfo, CallID: call.ID, Guardrail: machine}} } // resolveInbound returns whether the effective tool result was released, distinct diff --git a/engine/agent/loop.go b/engine/agent/loop.go index d3f88c2069..a670abfef2 100644 --- a/engine/agent/loop.go +++ b/engine/agent/loop.go @@ -857,6 +857,9 @@ type Run struct { // conversation content crosses that seam. reviewRoot *reviewRoot ownsReviewRoot bool + // backgroundReviews tracks opted-in yolo advisory reviews still running off + // the dispatch path; drainChildren joins them before the terminal event. + backgroundReviews backgroundReviews // currentPrompt is the accepted genuine prompt for this run. Completion locates // it in the final history; if compaction removed it, automatic admission gets an // invalid span and fails closed. @@ -1798,6 +1801,7 @@ func (e *Engine) prepareRun(ctx context.Context, sess *session.Session, req RunR // drain+seal already; this defer covers them idempotently.) defer close(r.events) defer r.children.seal() + defer r.backgroundReviews.drain() defer cancel() defer r.closeAuxiliaryUsageOwnership() body(ctx, r) @@ -3690,6 +3694,7 @@ var childDrainGrace = 1 * time.Second // BEFORE its clean-terminal calls — by the time this hook runs, the children it // would ask about are already cancelled and the registry sealed. func (e *Engine) drainChildren(ctx context.Context, r *Run) { + r.drainBackgroundReviews(ctx) joins := r.children.cancelLiveBackground() if len(joins) > 0 { if pending := joinChildren(joins, childDrainCap); len(pending) > 0 { diff --git a/engine/agent/review.go b/engine/agent/review.go index 5c37057719..7ffe905469 100644 --- a/engine/agent/review.go +++ b/engine/agent/review.go @@ -135,6 +135,12 @@ type ReviewEvidencePreparation struct { // effective catalog, mode, and bound authority. Preparers must call it before // touching backend metadata or content; model/reviewer input cannot replace it. Authorize func(context.Context, session.ToolCall) error + // Snapshot asks the preparer to capture workspace evidence content now, + // within its bounded first read, instead of reading it lazily during the + // review. The Engine sets it for advisory background reviews, which run + // concurrently with the reviewed call and later calls that may change the + // files. Content beyond the captured read is reported incomplete. + Snapshot bool } // PreparedReviewEvidence owns one review-local finite capability set. Close is @@ -158,13 +164,22 @@ type ReviewEvidencePreparer interface { // checker-down posture. The returns report whether review applies and whether a // finding or unresolved assessment is enforced. A ToolReviewer without this // capability retains action-only enforcement. Report the operator's configured -// enforcement independent of permission mode: the engine demotes it to advisory -// for each executing yolo-mode session, including delegated children that share -// the reviewer. +// enforcement independent of permission mode: the engine never enforces review +// in an executing yolo-mode session, including delegated children that share the +// reviewer, and skips it there unless YoloReviewPolicyProvider opts in. type ReviewPolicyProvider interface { GuardrailReviewPolicy(toolName string, job ReviewJob, operationalFailure bool) (applies, enforce bool) } +// YoloReviewPolicyProvider reports whether the operator opted in to advisory +// review of yolo-mode sessions. Without it, or when it returns false, a yolo +// session calls no checker. When it returns true, each applicable review runs +// in the background: the tool call neither waits for the verdict nor fails +// closed, and the verdict is published as an advisory guardrail event. +type YoloReviewPolicyProvider interface { + GuardrailYoloReviews() bool +} + // PermissionReviewPolicyProvider reports whether an exact child Shell call has // enabled, enforcing, matching action-review coverage. Advisory, skipped, and // unmatched rules must return false. The engine never consults it for a diff --git a/engine/agent/review_deadline_test.go b/engine/agent/review_deadline_test.go index fc1fcb01bc..68a1c5ae2c 100644 --- a/engine/agent/review_deadline_test.go +++ b/engine/agent/review_deadline_test.go @@ -124,7 +124,7 @@ func TestActionPreparationFailureRecordedBeforeAssessment(t *testing.T) { r := &Run{reviewRoot: newReviewRoot(reviewer, preparer, nil)} env := memEnv("/ws") sess := session.New("prep-health", session.ModeDefault, env.Ref(), session.Limits{}, time.Now()) - assessment := NewEngine(Deps{}).prepareActionAssessment(ctx, r, sess, env, session.NewToolCall("call", "Act", []byte(`{}`))) + assessment := NewEngine(Deps{}).prepareActionAssessment(ctx, r, sess, env, session.NewToolCall("call", "Act", []byte(`{}`)), false) defer assessment.cancel() wantRecords := 1 if stage == "before_checker" || stage == "parent_cancelled" { @@ -168,7 +168,7 @@ func TestPreparationTerminalContextErrorIsNotDowngraded(t *testing.T) { preparer := &deadlineEvidencePreparer{delay: 91 * time.Second, err: terminalContextEvidenceError{cause}} env := memEnv("/ws") sess := session.New("terminal-context", session.ModeDefault, env.Ref(), session.Limits{}, time.Now()) - _, err := NewEngine(Deps{}).prepareReviewEvidence(ctx, &Run{reviewRoot: newReviewRoot(nil, preparer, nil)}, sess, env, ToolReviewRequest{}, nil) + _, err := NewEngine(Deps{}).prepareReviewEvidence(ctx, &Run{reviewRoot: newReviewRoot(nil, preparer, nil)}, sess, env, ToolReviewRequest{}, nil, false) var terminal GuardrailReviewTerminalFailure if !errors.As(err, &terminal) || !terminal.GuardrailReviewTerminalFailure() { t.Fatalf("terminal error downgraded: %v", err) diff --git a/engine/agent/yolo_review_test.go b/engine/agent/yolo_review_test.go new file mode 100644 index 0000000000..9a12da94ba --- /dev/null +++ b/engine/agent/yolo_review_test.go @@ -0,0 +1,295 @@ +package agent + +import ( + "context" + "errors" + "iter" + "sync" + "testing" + "time" + + "github.com/stacklok/mecatl/engine/adapter/mockllm" + "github.com/stacklok/mecatl/engine/governance" + "github.com/stacklok/mecatl/engine/port" + "github.com/stacklok/mecatl/engine/session" + "github.com/stacklok/mecatl/engine/tool" +) + +// heldFinalTurn delays the model's second call until release closes (or a +// safety timeout), so background reviews can publish before run end cancels them. +type heldFinalTurn struct { + port.LLMProvider + release chan struct{} + calls int +} + +func (h *heldFinalTurn) Stream(ctx context.Context, req port.LLMRequest) (iter.Seq2[port.Chunk, error], error) { + h.calls++ + if h.calls == 2 { + select { + case <-h.release: + case <-time.After(5 * time.Second): + } + } + return h.LLMProvider.Stream(ctx, req) +} + +// yoloReviewer reviews both jobs. optIn controls GuardrailYoloReviews; gate, when +// set, parks every Review until it is closed or the review context ends, and +// parked (when set) receives once per review that reached the gate. +type yoloReviewer struct { + optIn bool + gate chan struct{} + parked chan struct{} + result ToolReviewResult + err error + + mu sync.Mutex + reviews int + cancelled int +} + +func (r *yoloReviewer) Review(ctx context.Context, _ ToolReviewRequest, _ ReviewEvidenceSource) (ToolReviewResult, session.AuxiliaryUsage, error) { + r.mu.Lock() + r.reviews++ + r.mu.Unlock() + if r.gate != nil { + if r.parked != nil { + r.parked <- struct{}{} + } + select { + case <-r.gate: + case <-ctx.Done(): + r.mu.Lock() + r.cancelled++ + r.mu.Unlock() + return ToolReviewResult{Assessment: ReviewUnresolved}, session.AuxiliaryUsage{}, ctx.Err() + } + } + usage := session.Usage{InputTokens: 5, OutputTokens: 1} + return r.result, session.AuxiliaryUsage{Buckets: map[session.UsageKind]session.TokenUsage{ + session.UsageKindGuardrail: {Total: usage, Models: map[string]session.Usage{"checker/model": usage}}, + }}, r.err +} + +func (*yoloReviewer) GuardrailReviewPolicy(string, ReviewJob, bool) (bool, bool) { return true, true } +func (r *yoloReviewer) GuardrailYoloReviews() bool { return r.optIn } + +func (r *yoloReviewer) counts() (int, int) { + r.mu.Lock() + defer r.mu.Unlock() + return r.reviews, r.cancelled +} + +// runYoloReview drives one Act call in a yolo session. When hooks > 0 the final +// model turn waits until that many guardrail events have been delivered. +func runYoloReview(t *testing.T, reviewer *yoloReviewer, preparer ReviewEvidencePreparer, hooks int, onEvent func(session.Event)) ([]session.Event, int, *session.Session) { + t.Helper() + executions := 0 + catalog := tool.NewCatalog() + catalog.MustRegister(revisionActionTool{executions: &executions}) + llm := &heldFinalTurn{LLMProvider: mockllm.New(mockllm.ToolCallTurn(session.NewToolCall("call", "Act", []byte(`{}`))), mockllm.TextTurn("done")), release: make(chan struct{})} + switch { + case reviewer.parked != nil: + // Let the run end only once both reviews are parked in the checker. + go func() { + <-reviewer.parked + <-reviewer.parked + close(llm.release) + }() + case hooks == 0: + close(llm.release) + } + engine := NewEngine(Deps{ + LLM: llm, + Catalog: catalog, + Policy: terminalFailurePolicy{job: ReviewJobAction}, + ToolReviewer: reviewer, + ReviewEvidencePreparer: preparer, + }) + env := memEnv("/ws") + sess := session.New("yolo-review", session.ModeYolo, env.Ref(), session.Limits{}, time.Unix(0, 0)) + var events []session.Event + seen := 0 + for ev := range engine.Run(context.Background(), sess, env, RunRequest{Text: "act"}).Events() { + events = append(events, ev) + if ev.Hook != nil && ev.Hook.Guardrail != nil { + if seen++; seen == hooks { + close(llm.release) + } + } + if onEvent != nil { + onEvent(ev) + } + } + return events, executions, sess +} + +func guardrailHooks(events []session.Event) []session.Event { + var hooks []session.Event + for _, ev := range events { + if ev.Hook != nil && ev.Hook.Guardrail != nil { + hooks = append(hooks, ev) + } + } + return hooks +} + +func TestYoloSkipsGuardrailReviewWithoutOptIn(t *testing.T) { + reviewer := &yoloReviewer{result: ToolReviewResult{Assessment: ReviewProhibited}} + events, executions, _ := runYoloReview(t, reviewer, nil, 0, nil) + if reviews, _ := reviewer.counts(); reviews != 0 { + t.Fatalf("reviews = %d, want 0: yolo without opt-in must not call the checker", reviews) + } + if executions != 1 { + t.Fatalf("executions = %d, want 1", executions) + } + if hooks := guardrailHooks(events); len(hooks) != 0 { + t.Fatalf("guardrail events = %+v, want none", hooks) + } +} + +func TestYoloOptInReviewsRunInBackgroundWithoutDelayingTheTool(t *testing.T) { + gate := make(chan struct{}) + reviewer := &yoloReviewer{optIn: true, gate: gate, result: ToolReviewResult{Assessment: ReviewProhibited}} + var once sync.Once + // Safety release: a blocking review would otherwise hang the run; releasing it + // late turns that regression into the ordering failure asserted below. + safety := time.AfterFunc(5*time.Second, func() { once.Do(func() { close(gate) }) }) + defer safety.Stop() + events, executions, sess := runYoloReview(t, reviewer, nil, 2, func(ev session.Event) { + // The checker is still parked when the canonical result arrives; only then + // is it released, so a blocking review would deadlock this run. + if ev.Type == session.EvToolResult && ev.ToolResult != nil && ev.ToolResult.CallID == "call" { + once.Do(func() { close(gate) }) + } + }) + if executions != 1 { + t.Fatalf("executions = %d, want 1", executions) + } + resultAt, terminalAt := -1, -1 + for i, ev := range events { + if ev.Type == session.EvToolResult && ev.ToolResult != nil && ev.ToolResult.CallID == "call" { + if ev.ToolResult.IsError || ev.ToolResult.Content != "executed" { + t.Fatalf("tool result = %+v, want the released original", ev.ToolResult) + } + resultAt = i + } + if ev.Type == session.EvResult { + terminalAt = i + } + } + if resultAt < 0 || terminalAt < 0 { + t.Fatalf("missing tool result (%d) or terminal event (%d)", resultAt, terminalAt) + } + phases := map[string]string{} + for i, ev := range events { + if ev.Hook == nil || ev.Hook.Guardrail == nil { + continue + } + if i < resultAt || i > terminalAt { + t.Fatalf("guardrail event at %d, want between tool result %d and terminal %d", i, resultAt, terminalAt) + } + phases[ev.Hook.Phase] = ev.Hook.Guardrail.Disposition + } + if phases[string(governance.PhasePreToolUse)] != "pass_advisory" || phases[string(governance.PhasePostToolUse)] != "pass_advisory" { + t.Fatalf("advisory dispositions = %v, want pass_advisory for both phases", phases) + } + if reviews, _ := reviewer.counts(); reviews != 2 { + t.Fatalf("reviews = %d, want action and inbound", reviews) + } + if got := sess.TokenUsageSnapshot()[session.UsageKindGuardrail].Total; got.InputTokens != 10 { + t.Fatalf("guardrail usage = %+v, want both background reviews recorded", got) + } +} + +func TestYoloOptInTerminalEvidenceFailureNeverFailsClosed(t *testing.T) { + reviewer := &yoloReviewer{optIn: true, result: ToolReviewResult{Assessment: ReviewAcceptable}} + events, executions, _ := runYoloReview(t, reviewer, failingEvidencePreparer{err: errors.New("evidence unavailable")}, 2, nil) + if executions != 1 { + t.Fatalf("executions = %d, want 1: yolo must not fail closed", executions) + } + for _, ev := range events { + if ev.Type == session.EvToolResult && ev.ToolResult != nil && ev.ToolResult.CallID == "call" && (ev.ToolResult.IsError || ev.ToolResult.Content != "executed") { + t.Fatalf("tool result = %+v, want the released original", ev.ToolResult) + } + if ev.Type == session.EvPermissionAsk { + t.Fatal("yolo must not surface a guardrail ask") + } + } + hooks := guardrailHooks(events) + if len(hooks) != 2 { + t.Fatalf("guardrail events = %d, want an advisory failure per phase", len(hooks)) + } + for _, ev := range hooks { + if ev.Hook.Guardrail.ReasonCode != string(ReviewFailureEvidenceFailure) || ev.Hook.Guardrail.Disposition != "pass_advisory" { + t.Fatalf("guardrail payload = %+v, want advisory evidence failure", ev.Hook.Guardrail) + } + } +} + +func TestYoloBackgroundReviewsAreCancelledAtRunEnd(t *testing.T) { + reviewer := &yoloReviewer{optIn: true, gate: make(chan struct{}), parked: make(chan struct{}, 2), result: ToolReviewResult{Assessment: ReviewAcceptable}} + start := time.Now() + events, executions, _ := runYoloReview(t, reviewer, nil, 0, nil) + if elapsed := time.Since(start); elapsed > 4*time.Second { + t.Fatalf("run took %s, want run end to cancel parked reviews at once", elapsed) + } + if executions != 1 { + t.Fatalf("executions = %d, want 1", executions) + } + if reviews, cancelled := reviewer.counts(); reviews != 2 || cancelled != 2 { + t.Fatalf("reviews=%d cancelled=%d, want both parked reviews cancelled at run end", reviews, cancelled) + } + if hooks := guardrailHooks(events); len(hooks) != 0 { + t.Fatalf("guardrail events = %+v, want none from cancelled reviews", hooks) + } +} + +func TestBackgroundReviewsCapConcurrency(t *testing.T) { + var b backgroundReviews + release := make(chan struct{}) + for i := range maxBackgroundReviews { + if _, ok := b.reserve(context.Background()); !ok { + t.Fatalf("reserve %d refused below the cap", i) + } + b.start(func() { <-release }) + } + if _, ok := b.reserve(context.Background()); ok { + t.Fatal("reserve succeeded above maxBackgroundReviews") + } + close(release) + b.wg.Wait() + if _, ok := b.reserve(context.Background()); !ok { + t.Fatal("finished reviews did not release their slots") + } + if b.start(func() {}); !b.drain() { + t.Fatal("drain did not join the last review") + } +} + +type snapshotRecordingPreparer struct { + mu sync.Mutex + snapshots []bool +} + +func (p *snapshotRecordingPreparer) PrepareReviewEvidence(_ context.Context, prep ReviewEvidencePreparation) (PreparedReviewEvidence, error) { + p.mu.Lock() + p.snapshots = append(p.snapshots, prep.Snapshot) + p.mu.Unlock() + return PreparedReviewEvidence{Complete: true}, nil +} + +func TestYoloBackgroundReviewsSnapshotEvidence(t *testing.T) { + reviewer := &yoloReviewer{optIn: true, result: ToolReviewResult{Assessment: ReviewProhibited}} + preparer := &snapshotRecordingPreparer{} + events, _, _ := runYoloReview(t, reviewer, preparer, 2, nil) + if len(guardrailHooks(events)) != 2 { + t.Fatalf("guardrail events = %d, want both advisories", len(guardrailHooks(events))) + } + preparer.mu.Lock() + defer preparer.mu.Unlock() + if len(preparer.snapshots) != 2 || !preparer.snapshots[0] || !preparer.snapshots[1] { + t.Fatalf("snapshot flags = %v, want both background preparations to snapshot", preparer.snapshots) + } +} diff --git a/engine/api/agent.txt b/engine/api/agent.txt index 0796c61941..c26db22153 100644 --- a/engine/api/agent.txt +++ b/engine/api/agent.txt @@ -19,6 +19,7 @@ ifacemethod func (TokenCounter).Count(text string) int ifacemethod func (TokenCounter).CountMessages(msgs []session.Message) int ifacemethod func (ToolReviewer).Review(context.Context, ToolReviewRequest, ReviewEvidenceSource) (ToolReviewResult, session.AuxiliaryUsage, error) + ifacemethod func (YoloReviewPolicyProvider).GuardrailYoloReviews() bool method func (*Engine).Capabilities() port.ProviderCapabilities method func (*Engine).CompactSession(ctx context.Context, sess *session.Session) (ManualCompactionResult, error) method func (*Engine).ContextWindow() int @@ -318,7 +319,7 @@ type ReviewDetail struct{RootSessionID session.SessionID; SessionID session.Sess type ReviewDetailSink interface{PublishReviewDetail(context.Context, ReviewDetail)} type ReviewEvidence struct{Handle string; Kind string; Version string; Continuation string; Complete bool; Content string} type ReviewEvidenceMeta struct{Handle string; Kind string; Display string; Version string; Continuation string; Complete bool} -type ReviewEvidencePreparation struct{Request ToolReviewRequest; Environment tool.Environment; Owner *session.Principal; Result *session.ToolResult; Authorize func(context.Context, session.ToolCall) error} +type ReviewEvidencePreparation struct{Request ToolReviewRequest; Environment tool.Environment; Owner *session.Principal; Result *session.ToolResult; Authorize func(context.Context, session.ToolCall) error; Snapshot bool} type ReviewEvidencePreparer interface{PrepareReviewEvidence(context.Context, ReviewEvidencePreparation) (PreparedReviewEvidence, error)} type ReviewEvidenceRequest struct{ReviewID string; Handle string; Version string} type ReviewEvidenceSource interface{ReadReviewEvidence(context.Context, ReviewEvidenceRequest) (ReviewEvidence, error)} @@ -359,6 +360,7 @@ type ToolReviewRequest struct{ReviewID string; Job ReviewJob; Event governance.H type ToolReviewResult struct{Assessment ReviewAssessment; Concerns []ReviewConcern; Evidence []ReviewEvidenceUse; Missing []ReviewMissingEvidence} type ToolReviewer interface{Review(context.Context, ToolReviewRequest, ReviewEvidenceSource) (ToolReviewResult, session.AuxiliaryUsage, error)} type UserModelReviewer struct{} +type YoloReviewPolicyProvider interface{GuardrailYoloReviews() bool} var ErrApprovalGrantIneligible error var ErrApprovalIntentMismatch error var ErrApprovalNotPending error diff --git a/internal/adapter/permconfig/guardrails_test.go b/internal/adapter/permconfig/guardrails_test.go index efc1be17b3..123f7ebfa3 100644 --- a/internal/adapter/permconfig/guardrails_test.go +++ b/internal/adapter/permconfig/guardrails_test.go @@ -156,3 +156,25 @@ guardrails: t.Fatalf("CLI guardrails must out-rank user-global; got %+v", g) } } + +func TestOperatorGuardrailsYoloAdvisoryParsed(t *testing.T) { + for _, tc := range []struct { + name string + yaml string + want bool + }{ + {name: "default_off", yaml: "\nguardrails:\n model: \"gpt-5\"\n", want: false}, + {name: "opt_in", yaml: "\nguardrails:\n model: \"gpt-5\"\n yoloAdvisory: true\n", want: true}, + } { + t.Run(tc.name, func(t *testing.T) { + env := envWithExplicit("/etc/mecatl/guardrails.yaml", tc.yaml) + g := newWithEnv(Options{ExplicitFiles: []string{"/etc/mecatl/guardrails.yaml"}}, env).OperatorGuardrails() + if g == nil { + t.Fatal("operator guardrails not parsed") + } + if g.YoloAdvisory != tc.want { + t.Fatalf("YoloAdvisory = %v, want %v", g.YoloAdvisory, tc.want) + } + }) + } +} diff --git a/internal/adapter/permconfig/schema.go b/internal/adapter/permconfig/schema.go index b468406699..074c8c1ff7 100644 --- a/internal/adapter/permconfig/schema.go +++ b/internal/adapter/permconfig/schema.go @@ -2059,6 +2059,12 @@ type GuardrailsSection struct { // guardrail checker (an unsafe verdict denies; a checker error fails closed // to the write-escape Ask). Default false = the un-routed posture table. Escape bool `yaml:"escape"` + // YoloAdvisory opts yolo-mode sessions in to advisory guardrail review. By + // default a yolo-mode session calls no checker. When true, each matching + // review runs in the background: the tool never waits for the verdict, is + // never blocked or held, and a checker failure never fails closed. Findings + // appear as advisory guardrail events. Other modes are unaffected. + YoloAdvisory bool `yaml:"yoloAdvisory"` // Rules is the guardrail rule list. Rules []GuardrailRuleSpec `yaml:"rules"` } @@ -2108,6 +2114,7 @@ func (g *GuardrailsSection) strictFields() map[string]any { "defaultMode": &g.DefaultMode, "taskWindow": &g.TaskWindow, "escape": &g.Escape, + "yoloAdvisory": &g.YoloAdvisory, "rules": &g.Rules, } } diff --git a/internal/app/build.go b/internal/app/build.go index 39c968fcce..d9884d521a 100644 --- a/internal/app/build.go +++ b/internal/app/build.go @@ -842,6 +842,9 @@ type Config struct { // fails closed to the write-escape Ask). Default false = the un-routed // posture table (auto read-allow / write-ask). GuardrailsEscape bool + // GuardrailsYoloAdvisory opts yolo-mode sessions in to background advisory + // review (guardrails.yoloAdvisory). Default false: yolo calls no checker. + GuardrailsYoloAdvisory bool // ModelAliases maps a short alias (e.g. "sonnet"/"opus"/"haiku"/"fast") to a // concrete provider model id. Resolved only here; the domain/agent always @@ -5476,6 +5479,11 @@ func guardrailsPostureLine(cfg Config, model string, src guardrailSource, specs // reflects the demoted specs for explicit rules; the note states WHY it is advisory. if guardrailsDemoted(cfg) { out += " — YOLO: AI safety checks are advisory only." + if cfg.GuardrailsYoloAdvisory { + out += " They run in the background and never block." + } else { + out += " Yolo-mode sessions skip them; set guardrails.yoloAdvisory to run them in the background." + } } return out } diff --git a/internal/app/contextual_build_evidence_test.go b/internal/app/contextual_build_evidence_test.go index bcd1b323e3..6a02026a8a 100644 --- a/internal/app/contextual_build_evidence_test.go +++ b/internal/app/contextual_build_evidence_test.go @@ -443,10 +443,10 @@ func TestBuildWiresFiniteScriptEvidenceToContextualReviewer(t *testing.T) { t.Fatalf("Build: %v", err) } defer built.Close() - // ModeYolo: Shell is auto-allowed (allow-all rules injected by Policy.Evaluate), - // so the guardrails block rule fires and routes the call to the contextual reviewer. + // ModeAuto: Shell is auto-allowed and the enforcing guardrails block rule routes + // the call to the contextual reviewer inline (yolo would skip the checker). // ModeDefault would floor Shell at Ask, hanging the test without a human approver. - sess, err := built.Service.CreateSession(context.Background(), session.ModeYolo, session.Limits{MaxTurns: 2}) + sess, err := built.Service.CreateSession(context.Background(), session.ModeAuto, session.Limits{MaxTurns: 2}) if err != nil { t.Fatalf("CreateSession: %v", err) } @@ -724,3 +724,52 @@ func TestBuildContextualEvidenceMatchesBuiltinReadPathDecoding(t *testing.T) { }) } } + +func TestSnapshotWorkspaceEvidenceSurvivesLaterChange(t *testing.T) { + ctx := context.Background() + workspace := memfs.NewWorkspace("/review") + if err := workspace.Write(ctx, "review-script.sh", []byte("SCRIPT_EVIDENCE_MARKER")); err != nil { + t.Fatal(err) + } + if err := workspace.Write(ctx, "big.sh", []byte(strings.Repeat("line\n", 8_000))); err != nil { + t.Fatal(err) + } + ref := session.EnvironmentRef{Kind: session.EnvKindMem, ID: "review", Revision: "r1"} + env := tool.MustEnvironment(ref, workspace, reviewerReadLedger{}, nil) + prepare := func(script string) agent.PreparedReviewEvidence { + t.Helper() + req := agent.ToolReviewRequest{ + ReviewID: "review-" + script, Job: agent.ReviewJobAction, + Event: sessionHookEvent("review-" + script), + EffectiveCall: session.NewToolCall("call-1", "Shell", json.RawMessage(`{"command":"./`+script+`"}`)), + Caller: agent.ReviewCaller{Role: "main", Capabilities: []string{"Shell"}}, Environment: ref, + Capacity: agent.ReviewCapacity{MaxEvidenceHandles: maxReviewEvidenceHandles, MaxEvidenceBytes: maxReviewEvidenceBytes}, + } + prepared, err := (&guardrailActionReviewer{providerID: "provider", modelID: "model"}).PrepareReviewEvidence(ctx, agent.ReviewEvidencePreparation{ + Request: req, Environment: env, Snapshot: true, Authorize: func(context.Context, session.ToolCall) error { return nil }, + }) + if err != nil || len(prepared.Evidence) == 0 || prepared.Source == nil { + t.Fatalf("prepare %s = %+v, err=%v", script, prepared, err) + } + t.Cleanup(prepared.Close) + return prepared + } + + small := prepare("review-script.sh") + if !small.Complete { + t.Fatal("a file within the first read must be fully captured") + } + // The reviewed call (or a later one) changes the file before the checker reads it. + if err := workspace.Write(ctx, "review-script.sh", []byte("changed")); err != nil { + t.Fatal(err) + } + meta := small.Evidence[0] + evidence, err := small.Source.ReadReviewEvidence(ctx, agent.ReviewEvidenceRequest{ReviewID: "review-review-script.sh", Handle: meta.Handle, Version: meta.Version}) + if err != nil || evidence.Content != "SCRIPT_EVIDENCE_MARKER" { + t.Fatalf("snapshot evidence=%+v err=%v, want the content captured at preparation", evidence, err) + } + + if big := prepare("big.sh"); big.Complete { + t.Fatal("a file beyond the first read must be reported incomplete under snapshot") + } +} diff --git a/internal/app/contextual_evidence.go b/internal/app/contextual_evidence.go index 21764dbef6..e942e9bee7 100644 --- a/internal/app/contextual_evidence.go +++ b/internal/app/contextual_evidence.go @@ -128,7 +128,15 @@ func (r *guardrailActionReviewer) PrepareReviewEvidence(ctx context.Context, pre complete = false continue } - _ = content // The bounded first page establishes version and total size. + if prep.Snapshot { + // A background review may run after the reviewed call changed the + // file, so serve the bounded first read instead of the live file. + captured := int64(len(content)) >= total + complete = complete && captured + candidates = append(candidates, reviewEvidenceCandidate{Kind: "text_file", Display: path, Version: encoded, Complete: captured, Authorized: true, Binding: binding, Backend: memoryReviewEvidenceBackend{content: string(content)}}) + continue + } + // The bounded first page establishes version and total size. candidates = append(candidates, reviewEvidenceCandidate{Kind: "text_file", Display: path, Version: encoded, Complete: true, Authorized: true, Binding: binding, Backend: workspaceReviewEvidenceBackend{reader: reader, path: path, size: total, version: encoded, authorize: authorize}}) } } diff --git a/internal/app/guardrails.go b/internal/app/guardrails.go index 7b3693357d..871b702d5c 100644 --- a/internal/app/guardrails.go +++ b/internal/app/guardrails.go @@ -64,6 +64,10 @@ func foldOperatorGuardrails(cfg Config) Config { if g.Escape { cfg.GuardrailsEscape = true } + // YoloAdvisory: YAML-only (no flag); OR-folded like Escape. + if g.YoloAdvisory { + cfg.GuardrailsYoloAdvisory = true + } // Rules: YAML is the sole source. Map the on-disk specs to app.GuardrailRule. if len(cfg.GuardrailsRules) == 0 && len(g.Rules) > 0 { rules := make([]GuardrailRule, 0, len(g.Rules)) @@ -199,12 +203,17 @@ type guardrailActionReviewer struct { base agent.ToolReviewer rules []modelhook.CompiledRule failClosed bool + yoloAdvisory bool grants *modelhook.WaiverHolder providerID, modelID string ruleOrigin string health *guardrailRouteHealth } +// GuardrailYoloReviews implements agent.YoloReviewPolicyProvider: yolo-mode +// sessions review in the background only when the operator opted in. +func (r *guardrailActionReviewer) GuardrailYoloReviews() bool { return r.yoloAdvisory } + // GuardrailReviewPolicy implements agent.ReviewPolicyProvider so configured // matching and enforcement remain explicit on the primary reviewer. func (r *guardrailActionReviewer) GuardrailReviewPolicy(toolName string, job agent.ReviewJob, operationalFailure bool) (applies, enforce bool) { @@ -339,7 +348,7 @@ func buildGuardrailsActionReviewer(cfg Config, provReg *providerRegistry, provid if routed, ok := base.(interface{ GuardrailCheckerRoute() (string, string) }); ok { providerID, modelID = routed.GuardrailCheckerRoute() } - return &guardrailActionReviewer{base: base, rules: rules, failClosed: guardrailFailClosed(cfg.GuardrailsOnCheckerDown), grants: grants, providerID: providerID, modelID: modelID, ruleOrigin: origin, health: cfg.guardrailHealth} + return &guardrailActionReviewer{base: base, rules: rules, failClosed: guardrailFailClosed(cfg.GuardrailsOnCheckerDown), yoloAdvisory: cfg.GuardrailsYoloAdvisory, grants: grants, providerID: providerID, modelID: modelID, ruleOrigin: origin, health: cfg.guardrailHealth} } func attachGuardrailReviewer(deps *agent.Deps, reviewer agent.ToolReviewer, details agent.ReviewDetailSink) { diff --git a/internal/app/guardrails_approve_once_e2e_test.go b/internal/app/guardrails_approve_once_e2e_test.go index 3edd01597c..e1ba8c1161 100644 --- a/internal/app/guardrails_approve_once_e2e_test.go +++ b/internal/app/guardrails_approve_once_e2e_test.go @@ -418,7 +418,7 @@ func runDenyingTurn(t *testing.T, built *Built, id session.SessionID) modeTurnOu // TestGuardrailE2EExecutedYoloAutoDefaultTransitions runs ONE session through // three completed turns on a yolo server with a rejecting checker, switching // mode between runs so the engine transitions live: yolo allows the mutation -// with the checker advisory; auto lets the same checker block it while a +// without consulting the checker (guardrails.yoloAdvisory is off); auto lets the same checker block it while a // configured Deny still denies; default asks for it instead of auto-allowing. func TestGuardrailE2EExecutedYoloAutoDefaultTransitions(t *testing.T) { ctx := context.Background() @@ -461,14 +461,14 @@ func TestGuardrailE2EExecutedYoloAutoDefaultTransitions(t *testing.T) { return err == nil } - // yolo: the mutation is auto-allowed and the rejecting checker is advisory. + // yolo: the mutation is auto-allowed and the checker is skipped by default. yolo := runDenyingTurn(t, built, sess.ID) if len(yolo.policyAsks)+len(yolo.guardrailAsks) != 0 || len(yolo.results) != 1 || yolo.results[0].IsError || !exists("yolo_marker") { t.Fatalf("yolo turn: asks %v/%v results %+v marker %t; want the Shell call to run unasked", yolo.policyAsks, yolo.guardrailAsks, yolo.results, exists("yolo_marker")) } - if reviewer.Calls() == 0 { - t.Fatal("yolo turn: the checker was never consulted, so its advisory verdict proves nothing") + if calls := reviewer.Calls(); calls != 0 { + t.Fatalf("yolo turn: the checker was consulted %d times; yolo must skip it without guardrails.yoloAdvisory", calls) } if _, err := built.Service.SetMode(ctx, sess.ID, session.ModeAuto); err != nil { t.Fatalf("SetMode(auto) after a completed yolo run: %v", err) diff --git a/internal/app/guardrails_yolo_test.go b/internal/app/guardrails_yolo_test.go new file mode 100644 index 0000000000..d56ddac7d6 --- /dev/null +++ b/internal/app/guardrails_yolo_test.go @@ -0,0 +1,102 @@ +package app + +import ( + "context" + "iter" + "testing" + "time" + + "github.com/stacklok/mecatl/engine/adapter/mockllm" + "github.com/stacklok/mecatl/engine/adapter/permpolicy" + "github.com/stacklok/mecatl/engine/agent" + "github.com/stacklok/mecatl/engine/governance" + "github.com/stacklok/mecatl/engine/port" + "github.com/stacklok/mecatl/engine/session" + "github.com/stacklok/mecatl/engine/tool" +) + +// heldFinalTurn delays the parent's second model call until release closes (or +// a safety timeout), so background reviews publish before run end cancels them. +type heldFinalTurn struct { + port.LLMProvider + release chan struct{} + calls int +} + +func (h *heldFinalTurn) Stream(ctx context.Context, req port.LLMRequest) (iter.Seq2[port.Chunk, error], error) { + h.calls++ + if h.calls == 2 { + select { + case <-h.release: + case <-time.After(5 * time.Second): + } + } + return h.LLMProvider.Stream(ctx, req) +} + +// runYoloGuardedGrep drives one guarded Grep through the production reviewer in a +// yolo session and returns the checker's call count and the guardrail events. +func runYoloGuardedGrep(t *testing.T, yoloAdvisory bool) (int, []session.Event) { + t.Helper() + verdict := `{"assessment":"prohibited","concerns":[{"ref":"c1","category":"redirection","rationale":"r","source_ref":""}],"evidence":[],"missing_evidence":[]}` + checker := mockllm.New(mockllm.TextTurn(verdict), mockllm.TextTurn(verdict), mockllm.TextTurn(verdict), mockllm.TextTurn(verdict)) + cfg := Config{ + UseMock: true, + GuardrailsModel: "checker-model", + GuardrailsRules: []GuardrailRule{{Match: "Grep", Phases: []string{"pre", "post"}, Mode: "block"}}, + GuardrailsYoloAdvisory: yoloAdvisory, + } + reviewer := buildGuardrailsActionReviewer(cfg, nil, checker, providerMock, nil) + if reviewer == nil { + t.Fatal("configured guardrail factory returned nil reviewer") + } + catalog := tool.NewCatalog() + catalog.MustRegister(stubTool{name: "Grep"}) + parent := &heldFinalTurn{LLMProvider: mockllm.New(mockllm.ToolCallTurn(session.NewToolCall("guarded", "Grep", []byte(`{}`))), mockllm.TextTurn("done")), release: make(chan struct{})} + if !yoloAdvisory { + close(parent.release) + } + deps := agent.Deps{ + LLM: parent, + Catalog: catalog, Model: "parent-model", + Policy: permpolicy.NewPolicy([]governance.Rule{{Effect: governance.Allow}}, nil), + } + attachGuardrailReviewer(&deps, reviewer, nil) + sess := session.New("yolo", session.ModeYolo, + session.EnvironmentRef{Kind: session.EnvKindLocal, ID: "/ws", Revision: "in-tree-v1"}, + session.Limits{}, time.Unix(1, 0)) + var hooks []session.Event + for ev := range agent.NewEngine(deps).Run(context.Background(), sess, memEnvironment("/ws"), agent.RunRequest{Text: "go"}).Events() { + if ev.Hook != nil && ev.Hook.Guardrail != nil { + if hooks = append(hooks, ev); len(hooks) == 2 { + close(parent.release) + } + } + if ev.Type == session.EvToolResult && ev.ToolResult != nil && ev.ToolResult.IsError { + t.Fatalf("yolo tool result was blocked or withheld: %+v", ev.ToolResult) + } + } + return checker.Calls(), hooks +} + +func TestGuardrailsYoloSkipsCheckerByDefault(t *testing.T) { + calls, hooks := runYoloGuardedGrep(t, false) + if calls != 0 || len(hooks) != 0 { + t.Fatalf("checker calls=%d guardrail events=%d, want none without guardrails.yoloAdvisory", calls, len(hooks)) + } +} + +func TestGuardrailsYoloAdvisoryReviewsInBackground(t *testing.T) { + calls, hooks := runYoloGuardedGrep(t, true) + if calls == 0 { + t.Fatal("guardrails.yoloAdvisory did not reach the checker") + } + if len(hooks) != 2 { + t.Fatalf("guardrail events = %d, want action and inbound advisories", len(hooks)) + } + for _, ev := range hooks { + if ev.Hook.Guardrail.Disposition != "pass_advisory" { + t.Fatalf("disposition = %q, want pass_advisory", ev.Hook.Guardrail.Disposition) + } + } +} diff --git a/internal/app/permission_mode.go b/internal/app/permission_mode.go index 0682443f51..156c37b31f 100644 --- a/internal/app/permission_mode.go +++ b/internal/app/permission_mode.go @@ -210,7 +210,7 @@ func checkerRefusal(cfg Config, checkerConfigured bool) error { // yoloCheckerAdvisoryNote states what a configured checker does NOT do at the // gate-free tier, where demoteForPosture makes it advisory. Shared by the // refusal and the startup line so they cannot disagree. -const yoloCheckerAdvisoryNote = "In yolo-mode sessions a configured checker is observability-only: it has no pre-tool veto, no approve-once human ask, and no fail-closed on checker failure, so a checker outage looks the same as a clean result. Sessions switched to another mode, including auto, keep it enforcing" +const yoloCheckerAdvisoryNote = "In yolo-mode sessions a configured checker never blocks: it does not run unless guardrails.yoloAdvisory is set, and then it reviews in the background with no pre-tool veto, no approve-once human ask, and no fail-closed on checker failure, so a checker outage looks the same as a clean result. Sessions switched to another mode, including auto, keep it enforcing" // headlessTrustRefusal is the headless admission gate. // It runs AFTER resolveTrust, so any legitimate trust source satisfies it, and @@ -273,7 +273,10 @@ func checkerState(cfg Config, configured bool) (state, reason string) { case !compiled: return checkerDisabled, noGuardrailRuleCompiled case cfg.MaxAllowedMode == session.ModeYolo: - return checkerAdvisory, "demoted in yolo-mode sessions; other modes keep it enforcing" + if cfg.GuardrailsYoloAdvisory { + return checkerAdvisory, "background-only in yolo-mode sessions; other modes keep it enforcing" + } + return checkerAdvisory, "skipped in yolo-mode sessions (guardrails.yoloAdvisory is off); other modes keep it enforcing" case !enforcing: return checkerAdvisory, "configured advisory" } diff --git a/internal/configgen/settings.skeleton.yaml b/internal/configgen/settings.skeleton.yaml index eba9a20786..6a9a6a70e7 100644 --- a/internal/configgen/settings.skeleton.yaml +++ b/internal/configgen/settings.skeleton.yaml @@ -88,6 +88,12 @@ # # unsafe verdict denies; a checker error fails closed to the write-escape Ask). # # Default false = the un-routed posture table. # escape: false +# # YoloAdvisory opts yolo-mode sessions in to advisory guardrail review. By default a +# # yolo-mode session calls no checker. When true, each matching review runs in the +# # background: the tool never waits for the verdict, is never blocked or held, and a +# # checker failure never fails closed. Findings appear as advisory guardrail events. +# # Other modes are unaffected. +# yoloAdvisory: false # # Rules is the guardrail rule list. # rules: # - match: "" diff --git a/user-docs/features/security-and-execution/permissions-and-posture.md b/user-docs/features/security-and-execution/permissions-and-posture.md index 73573a1392..6e0ed45ffb 100644 --- a/user-docs/features/security-and-execution/permissions-and-posture.md +++ b/user-docs/features/security-and-execution/permissions-and-posture.md @@ -49,7 +49,7 @@ Pass `--permission-mode` when you start `mecated`, `mecatui`, `mecak8s`, or |`default`|Ask|Ask|Ask|Optional| |`accept-edits`|Allowed|Ask|Ask|Optional| |`auto`|Allowed|Allowed|Ask; headless subagent requests go to a reviewer model|Required, enforcing| -|`yolo`|Allowed|Allowed|Allowed|Optional, advisory only| +|`yolo`|Allowed|Allowed|Allowed|Skipped unless opted in, then advisory only| Whether Mecatl loads project instructions, rules, and allow rules is the separate [project trust](#project-trust) decision; a permission mode never @@ -201,11 +201,16 @@ flag by default, so the checker configured in settings applies. Each can also name a checker model or pass the kill-switch. See [Guardrails for mecak8s](/operating/mecak8s.md#choose-a-guardrails-checker) and the [`mecatequi` action inputs](/building/ci/mecatequi.md#headless-posture-and-permission-asks). -In a session running in `yolo` mode, a configured checker is advisory only. It -has no pre-tool veto, no approve-once human ask, and no fail-closed behavior when -the checker fails, so a checker outage looks the same as a clean result. The -startup line reports it as `advisory`. Choose `auto` when you want the checker to -block findings. On a server started with `yolo`, a session you switch to `auto` +In a session running in `yolo` mode, a configured checker does not run, so tool +calls never wait for it. To keep its findings as warnings, opt in with +`guardrails.yoloAdvisory: true` in your user-global `settings.yaml`. The checker +then reviews each matching call in the background: the tool runs and its result +reaches the model without waiting, findings appear as advisory guardrail events, +and a checker failure never blocks. At most four reviews run at once, and calls +beyond that are not reviewed. Reviews still in progress when a run ends are +cancelled, so their findings are not reported. The startup line +reports the checker as `advisory` and states whether `yolo` skips it. Choose +`auto` when you want the checker to block findings. On a server started with `yolo`, a session you switch to `auto` or a lower mode keeps the checker enforcing. ## Headless roots and project trust diff --git a/user-docs/reference/configuration.md b/user-docs/reference/configuration.md index 598e22fbf3..40ceb1f794 100644 --- a/user-docs/reference/configuration.md +++ b/user-docs/reference/configuration.md @@ -67,6 +67,7 @@ OPERATOR-TIER LLM content-checker (issue #27). Parsed strictly. A project-tier g | `guardrails.defaultMode` | `string` | `(empty)` | DefaultMode sets the enforcement mode for the built-in default rules when no explicit rules are configured: "block" (default) or "advisory". An explicit rules list replaces the defaults entirely (this key is ignored). | | `guardrails.taskWindow` | `int` | `1` | TaskWindow selects the last K explicitly authenticated root prompts (default 1, clamped 1..3). | | `guardrails.escape` | `bool` | `false` | Escape is the guardrail escape knob: when true AND a checker model is configured, an out-of-root FS escape at posture auto routes through the guardrail checker (an unsafe verdict denies; a checker error fails closed to the write-escape Ask). Default false = the un-routed posture table. | +| `guardrails.yoloAdvisory` | `bool` | `false` | YoloAdvisory opts yolo-mode sessions in to advisory guardrail review. By default a yolo-mode session calls no checker. When true, each matching review runs in the background: the tool never waits for the verdict, is never blocked or held, and a checker failure never fails closed. Findings appear as advisory guardrail events. Other modes are unaffected. | | `guardrails.rules` | `[]guardrailrulespec` | `(absent)` | Rules is the guardrail rule list. | | `guardrails.rules[].match` | `string` | `(empty)` | Match is the tool-name matcher (exact / "prefix*" / "*"). | | `guardrails.rules[].phases` | `[]string` | `(absent)` | Phases lists "pre"/"post"; empty = both. |