fix: drop temperature for models that reject it — inbound security screening was fail-open - #748
Merged
Merged
Conversation
The adversarial eval crashed on its first real run with `400 \`temperature\` is deprecated for this model`. That was the loud symptom. The quiet one is worse. `LlmScreener.screen()` sends `temperature: 0` on every inbound turn, and its caller turns any exception into `unscreenable` — an explicit fail-open. The screener is constructed with the agent's own model (`buildOrchestrator.ts:329`), whose default is `DEFAULT_ORCHESTRATOR_MODEL = 'claude-opus-4-8'`. That model rejects `temperature: 0` with a 400. So on the repo's own default configuration, #579's inbound security screening returned `unscreenable` for every payload and reported no error. Five more production call sites are affected the same way — the plan-runner's materializer, replanner (x2), gate and gc all pass `temperature: 0`. Measured against the live API rather than inferred, because the rule is not what it looks like: model omitted 0 0.5 1 claude-opus-4-6 OK OK OK OK claude-opus-4-7 OK 400 400 OK claude-opus-4-8 OK 400 400 OK claude-opus-5 OK 400 400 OK claude-sonnet-4-6 OK OK OK OK claude-sonnet-5 OK 400 400 OK claude-haiku-4-5 OK OK OK OK `opus-4-6` accepts the parameter while `opus-4-7` rejects it, so "newer than X" is a plausible and wrong gate. `temperature: 1` is always accepted because it IS the default — which is why the eval's attacker step (temperature 1) survived and only the juror (temperature 0) raised. Fix in the adapter, the layer that owns the wire contract: `buildParams` omits `temperature` when the model does not honour it. That covers all seven call sites at once, including the legacy v1 wrapper, which delegates through the same neutral provider. `supportsTemperature` is exported so a caller can tell that its determinism request will be dropped. Note this is not fallout from the juror-model knob in #730: the juror ran on claude-opus-4-8 at temperature 0 before that change and would have raised the same 400. The eval simply never ran until ANTHROPIC_API_KEY was set. Verification: 6841 tests, 0 fail (baseline 6839 + the two added here); tsc clean; repo lint clean. Mutation check with a rebuild between runs — forcing the gate to always-true and to always-false each turns exactly one new test red.
This was referenced Aug 19, 2026
Weegy
added a commit
that referenced
this pull request
Aug 20, 2026
* feat(#749): make a failing security screener visible Inbound screening (#579) is fail-open by design: a screener that raises yields `unscreenable` and the turn proceeds. That policy is right — an unavailable judge must not take the product down — but until now the only trace was one `console.warn` per event. At turn volume a screener broken for EVERY request and one that blipped once produce the same shape of output, differing only in a volume nobody watches. That is not hypothetical. Until #748, `LlmScreener` sent a `temperature` the default model rejects, so every screen raised, every turn was `unscreenable`, and screening was a no-op that reported no error. It was found because an unrelated eval crashed on the same root cause. Three parts: 1. Structured failure causes. `unscreenable` carried only a free-text `reason`, so "misconfigured" and "busy" were indistinguishable without string-matching prose. `ScreenFailureCause` now separates `provider-rejected` (repeats forever, a human must fix it) from `provider-unavailable` (capacity, self-heals), plus `proxy-unreachable`, `unparseable-verdict`, `not-configured` and an explicit `unknown` so an unclassified failure stays visibly unclassified. The tag is attached where the knowledge lives: `LlmScreener` asks the provider's own `classifyError` rather than reading the message. 2. In-process counters, deliberately NOT in `@omadia/usage-telemetry`. That package is the obvious host and the wrong one: it buffers into Postgres and by its own contract no-ops when no pool is wired. A security counter that disappears in exactly the deployments nobody is watching rebuilds the original bug one layer up. These are integers in memory that always work. 3. `GET /admin/security/screening`, with a derived `healthy` so an operator or a probe need not know the threshold. It is false only on a RUN of failures, never on a single miss — a transient miss is what fail-open is FOR, and flagging it would teach people to ignore the endpoint. The fail-open behaviour itself is unchanged. This is evidence, not enforcement. `ScreenerFailure.failureCause` is deliberately not named `cause`: `Error.cause` is the standard slot for the underlying exception and still carries it. Verification: 6849 tests, 0 fail (baseline 6830 + the 19 added here); tsc clean; lint clean; decoupling ratchet unchanged at 3295. Mutation check with a rebuild between runs — four load-bearing sites (streak reset, snapshot copying, once-per-episode alerting, provider classification) each kill their test when reverted. * fix(#749): satisfy the test-tree typecheck ratchet `middleware/tsconfig.json` covers only `src`, so `npm run typecheck` never sees the test tree — `scripts/check-test-typecheck.mjs` (#573) is what does. An express `listen(port, host, cb)` overload does not accept a bare Promise `resolve` as the callback; wrap it. Fixed rather than baselined: the ratchet's baseline is for debt that predates it, not for debt added in the same PR that trips it.
This was referenced Aug 20, 2026
Weegy
added a commit
that referenced
this pull request
Aug 20, 2026
…icy gate (#777) * feat(#576): durable per-scope sandbox — P1 interface + Docker backend Introduces @omadia/sandbox (middleware/packages/harness-sandbox): the narrow Sandbox contract from issue #576 (qm competitive analysis) — provision/run/ read/write/list/teardown, optional capabilities (process-sessions/backup/ blob-staging) behind type guards rather than interface fields, and an AgentComputerProfile declaring persistence/egress/process-session posture. v1 backend is plain local Docker (DockerSandboxBackend), built on the same injectable-spawn pattern as src/plugins/builder/buildSandbox.ts's executeBuild seam (see dockerExec.ts's execDocker injection point) so the full backend logic is testable with zero real Docker. Wiring, not just declaration: - profile.egress === false becomes docker run --network none. Proven at two levels: a stub-level argv assertion (always runs) and, behind the opt-in SANDBOX_DOCKER_TEST=1 gate, a real container attempting an outbound wget and observing it fail — not just that the flag was passed. - read/write/list are traversal-hardened against a fixed sandbox root (pathGuard.ts's clampSandboxPathPosix), same discipline as the #772 broker and zipExtractor.ts's zip-slip guard: absolute paths, NUL bytes, and any ../ resolution outside the root are rejected before a single docker exec is issued (asserted directly — traversal tests check the stub recorded zero calls). - Container naming is a deterministic function of the scope key (sha256(scopeKey)[0:24]), so provision() re-attaches to an already-running container across backend-instance restarts without needing a DB-backed registry yet — that scope-durability bookkeeping (last-used timestamps for a reaper, RO-layer content-hash tracking, multi-backend routing) is P3's job, not a blocker for this backend working correctly today. No orchestrator touch (by design — P1 scope per the phase cut). No new runtime dependencies: the backend shells out to the docker CLI via node:child_process, mirroring dev-runner-shim's dockerd.ts and buildSandbox.ts's Node-builtins-only constraint. Tests: middleware/test/sandbox/{pathGuard,agentComputerProfile, dockerSandbox}.test.ts. 30 stub-tier tests (always run, no Docker required) + 2 real-Docker tests gated on SANDBOX_DOCKER_TEST=1 (both verified green locally against an actual daemon, including the egress block). Root npm test / typecheck / build / lint all green with this package included in the workspace chain. Mutation-checked: inverting the egress condition in dockerSandbox.ts and disabling the escape check in pathGuard.ts (with a full package rebuild between runs) both broke the corresponding tests; reverted and confirmed green again before committing. * feat(#576): durable per-scope sandbox — P2 execute tool + command-policy gate Adds the `execute` native tool (packages/harness-orchestrator/src/tools/ executeTool.ts), off by default behind operator config `sandbox_execute_enabled` (honest-inert, same convention as the #575 audience floor and #580 command policy — sharper here because this tool's entire job is running arbitrary commands). ## Security boundary — belt and braces, not a new mechanism orchestrator.ts's dispatchTool already runs every tool call through guardToolCommands (#580) at the ONE existing choke point, keyed on a top-level `command` field — execute's input schema uses exactly that key, so it is automatically gated by the EXISTING seam whenever a deployment installs a commandPolicy provider on the turn context. No deployment does that today (the seam is honest-inert until an operator config UI ships). Relying solely on that opt-in seam would mean execute ships fully open in every deployment that hasn't separately configured a policy — the #748 lesson (fail-open + no evidence = an invisible outage) applied to a much sharper edge than a security screener. So the handler ALSO runs its own command-policy check before ever touching a sandbox: resolveCommandPolicy defaults to defaultCommandPolicy() (the DEFAULT_ORG_FLOOR: recursive rm, force-push, destructive SQL, fork bombs, pipe-to-shell), and a throwing resolver is FAIL-CLOSED — refused, never run. require_approval is surfaced as an explicit refusal naming why, never silently escalated. No parallel policy mechanism: both checks call the same decideCommand/defaultCommandPolicy pure primitives from @omadia/channel-sdk. ## Count, don't just log (#749/#750 pattern) New commandPolicyMetrics.ts (mirrors securityScreenMetrics.ts): in-memory counters for allowed/denied/require_approval/truncated/resolve_failed, plus a per-rule-id tally. Wired into BOTH guardToolCommands (every existing #580 branch now records) and executeTool's own belt-and-braces check, so a broken policy resolver shows up as a resolveFailed streak instead of silence. ## Sandbox wiring execute resolves the calling turn's scope via ScopeId (parseSessionScope/formatSessionScope from @omadia/channel-sdk), falling back to a turn-unique key for an unscoped turn rather than a shared literal (the #445 lesson). Provisions/reuses that scope's sandbox via the injected SandboxBackend (DockerSandboxBackend in plugin.ts's wiring) and runs the command there. Registered in harness-orchestrator's plugin.ts — the SAME composition seam the existing ProcessMemory tools use (OB-76) — not src/index.ts or agentBuilder.ts, neither of which this PR touches. No writeCapabilities annotation: that contract is {dataClass, operation} for structured-write idempotency dedupe; execute's effects are arbitrary and untyped, and re-running the "same" shell command is not safely deduplicable the way replaying a structured write is. Deliberately absent, documented at the call site. ## Tests test/sandbox/executeTool.test.ts (17 tests) — every deny/require_approval/ truncated/resolve_failed path proven to never call SandboxBackend.provision (the security-boundary property), and the permitted path proven to reach provision with the correct scope key + run options. test/ commandPolicyMetrics.test.ts (9 tests) — counter module plus guardToolCommands wiring. All new + existing #580 tests green. ## Mutation-check evidence (dist rebuild between runs, per the phase-4b ## prompt's requirement now that @omadia/orchestrator imports @omadia/sandbox) Short-circuited the deny-decision branch in executeTool.ts (`if (false && decision.decision === 'deny')`), rebuilt @omadia/sandbox and @omadia/orchestrator, reran the suite: both the rm -rf and force-push denial tests failed as expected — the stub sandbox returned exit 0 for a command that should never have reached it. Reverted, rebuilt again, confirmed green. Full workspace build/typecheck/lint/test green after merging latest origin/main (7115 tests, 0 fail, 12 pre-existing skips). * fix(#576): reword dockerExec.ts comment to stop tripping the #470 core-decoupling ratchet The comment referencing 'dev-runner-shim' by name matched the ratchet's 'dev-runner' literal pattern (scripts/check-core-decoupling.mjs), which counts references to the Dev Platform being extracted in epic #470 — an unrelated coincidence (dev-runner-shim is a real, unrelated package; the ratchet's pattern list is deliberately broad-literal). Reworded to drop the package name while keeping the same intent (Node-builtins-only constraint on this spawn seam). Confirmed the ratchet passes locally after this change: 'Dev Platform references held at 3296' (baseline unchanged, not lowered — this was never a real Dev Platform reference, just a name collision). * fix(#576): sync package-lock.json peerDependencies entry for @omadia/orchestrator -> @omadia/sandbox npm install after the P1 merge regenerated this — the peerDependencies entry added to harness-orchestrator/package.json in the original P2 commit had not been reflected into package-lock.json's own copy of that block.
Weegy
added a commit
that referenced
this pull request
Aug 20, 2026
…r content hash + reaper (#779) * feat(#576): durable per-scope sandbox — P1 interface + Docker backend Introduces @omadia/sandbox (middleware/packages/harness-sandbox): the narrow Sandbox contract from issue #576 (qm competitive analysis) — provision/run/ read/write/list/teardown, optional capabilities (process-sessions/backup/ blob-staging) behind type guards rather than interface fields, and an AgentComputerProfile declaring persistence/egress/process-session posture. v1 backend is plain local Docker (DockerSandboxBackend), built on the same injectable-spawn pattern as src/plugins/builder/buildSandbox.ts's executeBuild seam (see dockerExec.ts's execDocker injection point) so the full backend logic is testable with zero real Docker. Wiring, not just declaration: - profile.egress === false becomes docker run --network none. Proven at two levels: a stub-level argv assertion (always runs) and, behind the opt-in SANDBOX_DOCKER_TEST=1 gate, a real container attempting an outbound wget and observing it fail — not just that the flag was passed. - read/write/list are traversal-hardened against a fixed sandbox root (pathGuard.ts's clampSandboxPathPosix), same discipline as the #772 broker and zipExtractor.ts's zip-slip guard: absolute paths, NUL bytes, and any ../ resolution outside the root are rejected before a single docker exec is issued (asserted directly — traversal tests check the stub recorded zero calls). - Container naming is a deterministic function of the scope key (sha256(scopeKey)[0:24]), so provision() re-attaches to an already-running container across backend-instance restarts without needing a DB-backed registry yet — that scope-durability bookkeeping (last-used timestamps for a reaper, RO-layer content-hash tracking, multi-backend routing) is P3's job, not a blocker for this backend working correctly today. No orchestrator touch (by design — P1 scope per the phase cut). No new runtime dependencies: the backend shells out to the docker CLI via node:child_process, mirroring dev-runner-shim's dockerd.ts and buildSandbox.ts's Node-builtins-only constraint. Tests: middleware/test/sandbox/{pathGuard,agentComputerProfile, dockerSandbox}.test.ts. 30 stub-tier tests (always run, no Docker required) + 2 real-Docker tests gated on SANDBOX_DOCKER_TEST=1 (both verified green locally against an actual daemon, including the egress block). Root npm test / typecheck / build / lint all green with this package included in the workspace chain. Mutation-checked: inverting the egress condition in dockerSandbox.ts and disabling the escape check in pathGuard.ts (with a full package rebuild between runs) both broke the corresponding tests; reverted and confirmed green again before committing. * feat(#576): durable per-scope sandbox — P2 execute tool + command-policy gate Adds the `execute` native tool (packages/harness-orchestrator/src/tools/ executeTool.ts), off by default behind operator config `sandbox_execute_enabled` (honest-inert, same convention as the #575 audience floor and #580 command policy — sharper here because this tool's entire job is running arbitrary commands). ## Security boundary — belt and braces, not a new mechanism orchestrator.ts's dispatchTool already runs every tool call through guardToolCommands (#580) at the ONE existing choke point, keyed on a top-level `command` field — execute's input schema uses exactly that key, so it is automatically gated by the EXISTING seam whenever a deployment installs a commandPolicy provider on the turn context. No deployment does that today (the seam is honest-inert until an operator config UI ships). Relying solely on that opt-in seam would mean execute ships fully open in every deployment that hasn't separately configured a policy — the #748 lesson (fail-open + no evidence = an invisible outage) applied to a much sharper edge than a security screener. So the handler ALSO runs its own command-policy check before ever touching a sandbox: resolveCommandPolicy defaults to defaultCommandPolicy() (the DEFAULT_ORG_FLOOR: recursive rm, force-push, destructive SQL, fork bombs, pipe-to-shell), and a throwing resolver is FAIL-CLOSED — refused, never run. require_approval is surfaced as an explicit refusal naming why, never silently escalated. No parallel policy mechanism: both checks call the same decideCommand/defaultCommandPolicy pure primitives from @omadia/channel-sdk. ## Count, don't just log (#749/#750 pattern) New commandPolicyMetrics.ts (mirrors securityScreenMetrics.ts): in-memory counters for allowed/denied/require_approval/truncated/resolve_failed, plus a per-rule-id tally. Wired into BOTH guardToolCommands (every existing #580 branch now records) and executeTool's own belt-and-braces check, so a broken policy resolver shows up as a resolveFailed streak instead of silence. ## Sandbox wiring execute resolves the calling turn's scope via ScopeId (parseSessionScope/formatSessionScope from @omadia/channel-sdk), falling back to a turn-unique key for an unscoped turn rather than a shared literal (the #445 lesson). Provisions/reuses that scope's sandbox via the injected SandboxBackend (DockerSandboxBackend in plugin.ts's wiring) and runs the command there. Registered in harness-orchestrator's plugin.ts — the SAME composition seam the existing ProcessMemory tools use (OB-76) — not src/index.ts or agentBuilder.ts, neither of which this PR touches. No writeCapabilities annotation: that contract is {dataClass, operation} for structured-write idempotency dedupe; execute's effects are arbitrary and untyped, and re-running the "same" shell command is not safely deduplicable the way replaying a structured write is. Deliberately absent, documented at the call site. ## Tests test/sandbox/executeTool.test.ts (17 tests) — every deny/require_approval/ truncated/resolve_failed path proven to never call SandboxBackend.provision (the security-boundary property), and the permitted path proven to reach provision with the correct scope key + run options. test/ commandPolicyMetrics.test.ts (9 tests) — counter module plus guardToolCommands wiring. All new + existing #580 tests green. ## Mutation-check evidence (dist rebuild between runs, per the phase-4b ## prompt's requirement now that @omadia/orchestrator imports @omadia/sandbox) Short-circuited the deny-decision branch in executeTool.ts (`if (false && decision.decision === 'deny')`), rebuilt @omadia/sandbox and @omadia/orchestrator, reran the suite: both the rm -rf and force-push denial tests failed as expected — the stub sandbox returned exit 0 for a command that should never have reached it. Reverted, rebuilt again, confirmed green. Full workspace build/typecheck/lint/test green after merging latest origin/main (7115 tests, 0 fail, 12 pre-existing skips). * feat(#576): durable per-scope sandbox — P3 scope durability + RO-layer content hash + reaper Migration 0044_sandbox_registry.sql (chosen with margin: 0040-0043 were occupied by concurrent credential-work PRs at authoring time, including an observed numbering collision at 0040 across two of them that has since been resolved upstream — noted in the migration's own header for the record, not something this PR touches). ## SandboxRegistry — the durable bookkeeping Docker doesn't give for free New SandboxRegistry interface (get/upsert/touch/delete/listAll) in @omadia/sandbox: InMemorySandboxRegistry (tests, and any deployment that hasn't wired a durable store) and PostgresSandboxRegistry (migration 0044's sandbox_registry table). DockerSandboxBackend gained an OPTIONAL registry constructor option — omitted (the default), the backend behaves byte-identical to what P1/P2 shipped (deterministic container naming, zero registry calls); provided, provision() records (scope, container name, profile, lastUsedAt) and re-attaches via the registry's STORED sandboxRef rather than recomputing the deterministic name, which is the seam a future non-deterministic backend needs. Regression-tested directly (dockerSandboxRegistry.test.ts's first suite asserts the no-registry path is unchanged). ## Reaper — orphaned (idle, non-persistent) sandbox cleanup reapOrphanedSandboxes takes "now" as a REQUIRED external parameter, never derived from the registry entries themselves — the #709/#710 clock-race lesson (an idle check must anchor to a clock independent of the row being checked). profile.persistent === true entries are never reaped regardless of idle time. A teardown failure leaves the registry row intact (for retry) and is reported in failedScopeKeys rather than silently dropped, and does not abort the sweep for the rest. Not wired to a scheduler in this PR — the function is a complete, directly callable, fully tested capability (same shape as DockerSandbox.teardown() itself, which also isn't auto-invoked by anything); hooking it to a cron/ routine is a scheduling-system integration, out of #576's substrate scope. ## RO-layer content-hash materialization computeContentHash (deterministic, order-independent, NUL-separated path/content encoding so shifted-boundary inputs can't collide) + syncReadOnlyLayer, which calls Sandbox.write only when the computed hash differs from the caller-supplied previousHash. #576 is deliberately the mechanism here, not the consumer: what actually constitutes a scope's RO layer (org files, skills) is the issue's OWN framing of what "builds on" this sandbox, not something #576 decides — inventing a content source would be scope creep into a separate concept. The primitive is fully exercised against a stub Sandbox (contentHash.test.ts proves the skip-when-unchanged behavior directly against the write call count). ## Tests - contentHash.test.ts (8), reaper.test.ts (6), dockerSandboxRegistry.test.ts (4) — all stub/pure, no Docker or Postgres needed, always run in npm test. - postgresSandboxRegistry.pg.test.ts (5) — gated on GRAPH_PG_TEST_URL/ MEMORY_PG_TEST_URL/DATABASE_URL, same probePgTest convention as postgresCredentialStore.pg.test.ts. Verified locally against a real postgres:16-alpine container (docker run, migration 0044 applied verbatim via psql, all 5 tests green) — not just trusted to work under CI's pg service. ## Mutation-check evidence (dist rebuild between runs) Removed the persistent-skip guard ("if (entry.profile.persistent) continue;") in reaper.ts, rebuilt @omadia/sandbox: the 'never reaps a persistent sandbox' test failed as expected (a persistent scope was reaped). Reverted, rebuilt again, confirmed green. (P1's egress/traversal mutations and P2's deny-bypass mutation were separately re-verified in their own PRs; this round targets the one new security-relevant branch P3 adds.) Full workspace build/typecheck/lint/test green after merging latest origin/main (7223 tests, 0 fail, 12 pre-existing skips). * fix(#576): reword dockerExec.ts comment to stop tripping the #470 core-decoupling ratchet The comment referencing 'dev-runner-shim' by name matched the ratchet's 'dev-runner' literal pattern (scripts/check-core-decoupling.mjs), which counts references to the Dev Platform being extracted in epic #470 — an unrelated coincidence (dev-runner-shim is a real, unrelated package; the ratchet's pattern list is deliberately broad-literal). Reworded to drop the package name while keeping the same intent (Node-builtins-only constraint on this spawn seam). Confirmed the ratchet passes locally after this change: 'Dev Platform references held at 3296' (baseline unchanged, not lowered — this was never a real Dev Platform reference, just a name collision). * fix(#576): sync package-lock.json peerDependencies entry for @omadia/orchestrator -> @omadia/sandbox npm install after the P1 merge regenerated this — the peerDependencies entry added to harness-orchestrator/package.json in the original P2 commit had not been reflected into package-lock.json's own copy of that block.
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
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.
The loud symptom
The adversarial eval crashed on its first real run (it had never executed before, because
ANTHROPIC_API_KEYwas only just set):The quiet one
LlmScreener.screen()sendstemperature: 0on every inbound turn:Its caller turns any exception into
unscreenable— an explicit, documented fail-open (securityScreener.ts:92: "the screener was unavailable/errored/timed out: fail open"). And the screener is constructed with the agent's own model:whose default is
DEFAULT_ORCHESTRATOR_MODEL = 'claude-opus-4-8'— a model that rejectstemperature: 0.So on the repo's own default configuration, #579's inbound security screening returned
unscreenablefor every payload, and reported no error while doing it. Same failure shape as the permanently-greengolden-evalguard-skip, but on a security control.Five further production call sites pass
temperature: 0through the same adapter: the plan-runner'smaterializer,replanner(×2),gateandgc.Why the gate is a measured table, not a version comparison
Measured against the live API on 2026-08-19:
00.51claude-opus-4-6claude-opus-4-7claude-opus-4-8claude-opus-5claude-sonnet-4-6claude-sonnet-5claude-haiku-4-5Two things this table says that a guess would get wrong:
opus-4-6accepts the parameter andopus-4-7rejects it. "Newer than X" is a plausible and wrong rule; so is "the Claude 5 family". Only an explicit list is defensible, so the table lives in the source next to the list.temperature: 1is always accepted, because it is the default — the API only objects to being asked for a value it no longer honours. That is why the eval's attacker step (temperature 1, onclaude-opus-4-8) survived and only the juror (temperature 0) raised. Without this, the stack trace points at the juror and invites the wrong conclusion.The fix
buildParamsinllm-adapter-anthropicomitstemperaturewhen the model does not honour it. The adapter is the layer that owns the wire contract, so one change covers all seven call sites, includingcreateLlmProviderFromNeutral(the legacy v1 wrapper), which delegates through the same neutral provider rather than building its own params.supportsTemperature(model)is exported so a caller can tell in advance that its determinism request will be dropped. On the affected models there is no way to obtaintemperature: 0at all, so dropping it is the only available behaviour — and strictly better than raising into a fail-open catch.Not fallout from #730
Worth stating plainly, because the stack trace suggests otherwise: the juror ran on
claude-opus-4-8attemperature: 0before #730's juror-model knob and would have raised the identical 400. The knob changed which model name appears in the error, nothing else.Verification
npm test— 6841 tests, 0 fail, 0 cancelled (baseline 6839 + the two added here)tsc --noEmitclean;npm run lintcleandist/between runs because the test resolves the package through it — forcing the gate to always-trueand to always-falseeach turns exactly one of the two new tests red. (The first attempt without a rebuild killed neither mutant and looked green; noted here because it is the standing trap in this repo.)The second test guards the other direction: a gate that dropped
temperatureeverywhere would satisfy the first test while silently removing determinism from the models that still support it.Follow-up worth its own issue
The screener's fail-open is correct as a policy but has no signal attached: a screener that errors on 100% of calls looks identical to one that is merely occasionally unavailable. A counter or a log on repeated
unscreenableverdicts would have surfaced this in a day.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.