feat(guardrail): screen with the measured policy, and record the hazard that fired (#36) - #36
Merged
Merged
Conversation
…ent (#8) agentloop had the guardrail *policy* (internal/experiments.Route, strict / permissive) and no transport: NewRunnerWithTypeSafeScreen existed but nothing in the service ever called it, so no run was screened and no run could say so. The transport is added where the policy already lives, as the third thin client next to internal/onegw and internal/leankg: one POST, no retry ladder, no key pool, no backend switch. The backend choice is a URL (AGENTLOOP_SYSTEMONE_URL): onegw's POST /v1/systemone by default — PRD §4.3 unchanged, onegw still owns combo/fallback/usage — or TypeSafe / a Laya sidecar directly when an operator names one. A Jev/Laya generation-provider abstraction is deliberately not built: Jev and Laya are evaluation models, not chat models, and the wire that fits both is the decision wire. - internal/systemone: Evaluate (any battery) + Screen (the guardrail battery, the same five questions typesafe_experiments.sh sends, so Go and shell score the same corpus). Both answer envelopes are read — TypeSafe's `noul` and Laya's probabilities["1"] — with no caller-side branch on which answered. - Every failure is an error, never a zero: a missing hazard or a missing severity Score would route as "pass", which is the false negative a guardrail must not produce. - internal/loop: RunnerConfig.Guardrail + RunnerConfig.Policy, the screen at the step boundary, and the verdict recorded on the step (the hazard that fired, plus the severity when the severity is what escalated it). - New exit guardrail_unavailable: a screen that could not be evaluated fails closed, and "the screen said no" is reported differently from "there was no screen". - cmd/agentloop: AGENTLOOP_SYSTEMONE_URL/_KEY/_MODEL + AGENTLOOP_GUARDRAIL_POLICY. Screening is off until a URL is set; off records no verdicts, so an unscreened deploy is distinguishable from a clean one. Verified: go test ./... and make check (fmt, vet, test, lint, prd) green; a stub /v1/systemone driven through the real binary blocks a run with exit_reason=guardrail_block and the hazard row on the step, and an unreachable URL exits guardrail_unavailable rather than passing.
The only real conflict was the tier work landing where this branch had put its own `/v1/systemone` client: `internal/systemone` and #37's `internal/guardrail` are the same client built twice, and #37's is the one on main — wired into the service, with the goal screen and `screen_errors`. Keeping both would have meant two clients, two batteries and two answer parsers for one wire, so this branch is reduced to what it actually adds. Kept from this branch (the parts #37 does not have): - **The hazard, not the label.** A blocked step's record names the hazard that fired (`jailbreak 0.91`) instead of the constant `noul_battery`, and a severity-driven block records the severity row too — a single hazard row would claim a low probability caused it. `internal/loop/screen.go`. - **`RunnerConfig.Policy`** (strict by default, `AGENTLOOP_GUARDRAIL_POLICY` selects permissive). `experiments.Strict` was hard-coded in the loop and in `screenGoal`, so §17's measured strict/permissive split — a product decision the PRD names — was unreachable without a rebuild. - **The answer-shape fix (#37's client, found by driving it).** #37's `probabilityOf` read `confidence` before the hazard, so TypeSafe's real envelope — `{"type":"noul","noul":0.91,"confidence":0.88}`, which is what the API actually sends — reported jailbreak 0.88 where it is 0.91. It also did not read Laya's `probabilities["1"]`. Both are now read, both pinned by `TestBothBackendEnvelopesAgree`, and `confidence` is out of the key list: it is a calibration claim, not a verdict. - **Why the client is here at all**, in `docs/JEV-INTEGRATION.md` §2.1: policy and the wire belong to agentloop, backend selection belongs to onegw, and the "provider abstraction" that would replace all of it is a fiction — Laya is a non-autoregressive encoder and cannot satisfy a generation- provider interface. Dropped from this branch: `internal/systemone` (superseded by `internal/guardrail`), its `GuardrailClient` seam and `guardrail_unavailable` exit reason (a screen that cannot run is an observation recorded in `screen_errors`, not a new terminal state — that is #37's contract and it is the right one), and this branch's duplicate env names. Verified: `make check` green (gofmt, vet, test, golangci-lint, check-prd); stub-sidecar drive through the real binary at `AGENTLOOP_GUARDRAIL_URL` blocks/holds with the hazard row on the step.
) Second merge of `origin/main` (which now carries #37, "the TypeSafe screen is wired"). The conflicts were all one thing: this branch and #37 built the same client twice — `internal/systemone` here, `internal/guardrail` there — so the resolution keeps #37's tree and this branch's four additions on top of it. Why #37's client wins: it is wired into the service (goal screen, tool-result screen, `screen_errors` on the run, `guardrail_wiring_test.go`), it treats a screen that cannot run as a recorded observation rather than a new terminal state, and it is already the API the tests on main exercise. Two clients for one wire, each with its own battery copy and answer parser, is the duplication `docs/DUPLICATION-AUDIT.md` exists to prevent. What this branch adds, i.e. what the conflict resolution preserves: 1. **The hazard, not the label.** A blocked step's record names the question that fired (`jailbreak 0.91`) instead of the constant `noul_battery`, and a severity-driven block also records the severity row — one hazard row would claim a low probability caused a block the severity decided. (`internal/loop/screen.go`, `internal/loop/screen_test.go`) 2. **`RunnerConfig.Policy`.** `experiments.Strict` was hard-coded in the loop and in `screenGoal`, so §17's measured strict/permissive split — which the PRD names as an operator decision — was unreachable without a rebuild. `AGENTLOOP_GUARDRAIL_POLICY=permissive` now selects it, and anything unrecognised stays strict. 3. **The answer-shape fix in `internal/guardrail`.** `probabilityOf` read `confidence` before the hazard, so TypeSafe's real envelope — `{"type":"noul","noul":0.91,"confidence":0.88}`, which is what the API sends — reported jailbreak 0.88 where it is 0.91; a 0.03 error in the direction that under-blocks. Laya's `probabilities["1"]` was not read either. Both envelopes are now read and pinned by `TestBothBackendEnvelopesAgree`; `confidence` is out of the key list because a calibration claim is not a verdict. 4. **`docs/JEV-INTEGRATION.md` §2.1** — why the client lives here: policy and the wire belong to agentloop, backend selection belongs to onegw, and the generation-provider abstraction that would replace it is a fiction, because Laya is a non-autoregressive encoder. Dropped: `internal/systemone`, its `GuardrailClient` seam, and its `guardrail_unavailable` exit reason. Verified after resolving: `make check` green (gofmt, vet, go test, golangci-lint, `docs/check-prd.py`).
Found by driving the wiring, not by a test: a goal the stub scored
`jailbreak 0.9` was recorded as `{hazard: noul_battery, prob: 0, action: block}`.
`screenGoal` routed the verdict and then threw the verdict away — the caller
rebuilt a `ScreenResult` from the action string alone, so the goal path kept
the constant label the step path had just stopped writing. The block was
correct and unexplainable: the run record could not say what was seen.
`screenGoal` now returns the rows with the verdict (one screen call, not two),
built by the same `loop.ScreenRowsFor` the step boundary uses, exported for
exactly that reason — a second implementation is how one call site drifts back
to the label. `TestScreenGoalRecordsTheVerdict` pins it, and the stub sidecar
drive now records `jailbreak 0.9` on a blocked goal.
linhdmn
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes the first version of this branch.
mainmoved twice under it (#35 tier routing, #37 "wire the TypeSafe screen"), and #37 built the same client this branch had — so the conflict is resolved by keeping main's client and this branch's four additions on top of it.origin/mainis now an ancestor of the branch; nothing left to merge.Why main's client wins
internal/guardrailis wired into the service (goal screen, tool-result screen,screen_errors,guardrail_wiring_test.go) and treats a screen that cannot run as a recorded observation. This branch'sinternal/systemone,GuardrailClientseam andguardrail_unavailableexit reason are dropped: two clients for one wire, each with its own battery copy and answer parser, is the duplicationdocs/DUPLICATION-AUDIT.mdexists to prevent.What this branch actually adds
1. The answer-shape bug in main's client.
probabilityOfreadconfidencebefore the hazard. TypeSafe's real envelope is{"type":"noul","noul":0.91,"confidence":0.88}— so the screen reported jailbreak 0.88 where it is 0.91, an error in the direction that under-blocks. Laya'sprobabilities["1"]was not read either. Both envelopes are now read and pinned byTestBothBackendEnvelopesAgree;confidenceis out of the key list because a calibration claim is not a verdict.2. The hazard, not the label. A blocked step now names the question that fired (
jailbreak 0.91) instead of the constantnoul_battery, and a severity-driven block also records the severity row — one hazard row would claim a low probability caused a block the severity decided.loop.ScreenRowsForis exported because two sites screen (the step boundary and the goal) and a second implementation is how one drifts back to the label.3.
RunnerConfig.Policy.experiments.Strictwas hard-coded in the loop and inscreenGoal, so §17's measured strict/permissive split — which the PRD names as an operator decision — was unreachable without a rebuild.AGENTLOOP_GUARDRAIL_POLICY=permissiveselects it; anything unrecognised stays strict, because a typo must not loosen a screen.4.
docs/JEV-INTEGRATION.md§2.1 — why the client lives in agentloop at all: policy and the wire belong here, backend selection belongs to onegw, and the generation-provider abstraction that would replace it is a fiction, because Laya is a non-autoregressive encoder that cannot satisfy one.Found by driving it, not by a test
The last commit is a bug the suite did not catch: a goal the stub scored
jailbreak 0.9was recorded as{hazard: noul_battery, prob: 0, action: block}.screenGoalrouted the verdict and threw it away, so the goal path kept the label the step path had just stopped writing.screenGoalnow returns its rows (one screen call, not two) built by the sameScreenRowsFor, pinned byTestScreenGoalRecordsTheVerdict.Verified
make checkgreen: gofmt,go vet,go test ./..., golangci-lint,docs/check-prd.py./v1/systemoneon:8091viaAGENTLOOP_GUARDRAIL_URL: a blocked goal records[{'hazard': 'jailbreak', 'prob': 0.9, 'action': 'block'}], and each screened step records{hazard: 'jailbreak', prob: 0.91}(previous run, pre-fix, recordednoul_battery/0).Still open (issue #8)
BudgetGuard.