Repository navigation
feat(preallow): judge on haiku with an opus second judge, retry failed judge calls, and read the whole conversation - #17
Merged
Conversation
In Vertex sessions 0.35% of judge calls came back as `empty-reply` (0.04% on the subscription), interleaved with replies to prompts of the same size seconds apart, and each sent the tool call to the classifier as if the judge had reviewed it. A model call that returns no text, or an API error Claude Code classes as `rate_limit`, `overloaded`, or `server_error`, is now made once more after two seconds. A call cut short (`aborted`), refused for what it asked (authentication, invalid request, and the other kinds), or refused by the client is not. The 90-second limit now bounds the judge's whole run rather than each model call: every call gets the time left, and a retry or a call after a check that no longer fits is not made, so the retry does not lengthen the longest wait. The delay runs on `$.clock`, since that wait counts against the hook's ten-second budget and tests can drive it. `judge` now takes its model call, fact reader, and clock as one host, and its model call resolves the engine's `ModelCompleteResult`, so an evaluation outside the engine runs the same retries. The `judged` line says in `isAnswered` whether the model answered, after the engine's `isAnswered`, and in `failure` why not, instead of inside `reason`, and lists the retried failures in `retried`, which keeps the provider's failure rate visible after the retry hides it. This opens preallow 0.0.9.
…fier A call the judge left to review and auto mode then denied was judged afresh when the agent made it again, so a second sample of the same judgment could allow what auto mode had just denied, with nothing new from the user: an uninstall was reviewed, denied, and allowed 26 s later. The plugin now remembers the judge's first-round prompt of each denied call and leaves a call whose first-round prompt matches one to the classifier, logged as `repeated`. That prompt holds the tool, its input, the directory, a subagent's brief, and the user's kept words, so "the same call" needs no separate definition, and a call made after the user writes again is judged with the new words. Only denials are remembered: a review the classifier then allowed ran under its approval, and a judge allow of the call contradicts nothing. Each is kept as a SHA-256 digest, since a prompt can run to a few hundred kilobytes. `judgeCase` builds what the judge reads before `judgeCall` runs it, so `tool.check` can compare the prompt before asking the judge.
…ation `select.ts` picked up to 24,000 characters of the user's kept words for each call: limits found by wording first, then messages sharing words with the call. A prohibition phrased without the words it looked for could be left out, which lets through a call the user ruled out, and because the pick differed per call the user-words part of the prompt was never cached. The judge now reads the whole kept conversation, which `addTurn` already bounds at 64,000 characters, for every call, and selection is gone. The prompt is sent as two blocks of the same JSON document: the conversation first, marked for the prompt cache, then the call's fields. Calls between two user messages, and the second round after a check, read the conversation from the cache, and on Bedrock, Vertex, Foundry, or a gateway, where a mark in `system` is found only while the prompt opens the same way, the policy is read back too. Joined, the blocks are the document the judge read before, so text inside a message still cannot pose as another field; a test parses the joined text. This changes what the judge decides on, so it is a commit of its own for the eval to compare.
The judge reviewed a `git branch -D` because the branch's commits were reachable from no other ref, while their patches were already in the default branch: a pull request merged by rebase leaves the work upstream under other commits, which reachability cannot see. For a ref with commits no other ref holds, the ref facts now say, in `ofThoseWithAnEquivalentPatchIn`, how many of them `git cherry` finds with an equivalent patch in each of two kinds of ref chosen by git's config and refs, never by the judge: each remote's default branch (`refs/remotes/<remote>/HEAD`), where a merged pull request lands, and the branch's configured upstream, which holds the pushed copy of a branch rebased locally. Comparison refs that no longer exist, such as an upstream deleted after its merge, and the ref itself are skipped. The unique set is listed instead of counted so only its commits are counted, and nothing is compared when it is empty. `git cherry` runs with the same read-only, offline prefix and time limit as the other queries. It compares one commit at a time, so a squashed series or a conflict-resolved rebase finds no equivalent and merge commits are not compared; the README states these limits. `facts.ts` was at its size limit, so path facts move to `paths.ts` and ref facts to `refs.ts`, with tests split the same way.
Reading `hooks/withhold.ts` was withheld, because the `AGE-SECRET-KEY-1` it lists as a shape matched itself; the PuTTY header shape matched the test that names it, and the generated gitleaks rule for short-lived Amazon Bedrock keys, a bare prefix, matched its own definition. Any text that names one of these formats, such as documentation of a key file, was withheld the same way. Each private key shape now needs the key's body: PEM and OpenPGP armor 40 base64 characters within 200 characters, newlines escaped or not; a PuTTY key file its private lines; an age identity its 58-character Bech32 body. The other shapes already require a value. `scripts/gitleaks.ts` appends a token body to a rule whose regex is a bare literal, the one upstream rule of that kind, so it matches a token rather than the format's name. Every pattern stays linear, with time tests on long runs of armor and private lines. gitleaks' `private-key` rule is left out of the generated rules: it takes any 64 characters between a key file's markers, such as a placeholder sentence, for a key. The PEM shape takes the header characters that rule took and counts the body across lines of any width, with any line ending and blanks ending a line, which RFC 7468 parsers accept, so a real key it found is still found. Armor around a placeholder of letters and digits split over lines is still withheld, and a key after more than about 200 characters of headers, or a narrow body with indented or blank lines, is not; the README lists both with the known gaps. Plugin tests cannot read files, so `bun run test:sources`, part of `bun run check`, runs the scan over every tracked file of the plugin and fails on a finding. A test fixture that read as a Slack token in the source is now built at run time.
CodeQL alert #1 (`js/incomplete-hostname-regexp`) flags `slack-webhook-url`, whose `hooks.slack.com` carries unescaped dots from the pinned gitleaks config, so each dot matches any character; `sidekiq-sensitive-url` has the same defect in `gems.contribsys.com` and `enterprise.contribsys.com`. `scripts/gitleaks.ts` now escapes, in every rule it converts, a plain dot that joins hostname labels ending in a top-level domain from the list the CodeQL query checks (com, org, edu, gov, uk, net, io), instead of patching the two rules. Dots after a group or a quantifier, and names such as `MailchimpSDK.initialize`, stay as they are; regenerating changes only those two rules.
The judge reads the policy as five decisions in order: leaks, whether the call serves the task and keeps to what the user ruled out, the own-repository allowance, the effects and whether each is recoverable, and the authorization the irreversible ones need. It now describes the fields the judge reads, including the kept conversation's markers, and each fact a check returns with what it does not establish, such as patch equivalence for squashed series or file times for who made a file. The text is the candidate the evaluation chose, unchanged.
…o opus The judge of every call now runs on `haiku` at `high` instead of `opus` at `medium`, and a second judge on `opus` at `medium` judges again what it does not allow. Haiku alone sent too many calls it should allow to the classifier. Evaluated on real calls auto mode had sent to the classifier, on the development split and on a held-out one, the pair allowed no call the policy rules out, the same as the previous default; it sent fewer allowed calls to the classifier on the development split and one more on the held-out one, at about a seventh of the cost. A read-only call whose output happens to hold a secret does not show it, so catching that is the output scan's job, and the README now lists it among the known gaps. A call the first stage does not allow, whether it answered `review`, stayed unsure after a check, could not get its facts, or gave no reply, is judged again from the start by the second stage, with the same prompt and checks of its own, and that verdict is final. This is the configuration that was evaluated. A call whose input is too long to ask the first stage about is not passed to the second, which could not ask either. The new `escalation_model` and `escalation_effort` settings choose it; the defaults apply as a pair, so a user who sets `model` without `escalation_model` gets no second judge they did not pick, and `escalation_effort` alone adds none. `judge` stays one stage, so the evaluation runner keeps calling it per stage; `judgeCall` moves to `hooks/stages.ts`, which binds each stage to the session and runs them. Each stage gets its own 90 seconds, since a shared deadline would leave nothing for the second stage to judge a call the first ran out of time on. The denial memo still applies before either stage. A `judged` line keeps the first stage's fields and gives the second's, the deciding verdict included, in `escalation`.
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.
preallow 0.0.9 judges every call on
haikuathighand has a second judge onopusatmediumjudge again, from the start, every call the first did not allow, including one it gave no verdict on; the second judge's verdict is final. The newescalation_modelandescalation_effortsettings choose the second judge, and the defaults apply as a pair: settingmodelwithoutescalation_modelleaves no second judge. Ajudgedlog line keeps the first judge's fields and gives the second's inescalation.The defaults and the new policy text were chosen on real calls auto mode had sent to the classifier, labeled by hand and kept outside this repository, on a development split and a held-out split. On both, neither the previous default (
opusatmediumalone) nor the pair allowed a call the policy rules out, counting a read-only call whose output happened to hold a secret as the output scan's job; the README now lists that among the known gaps. On the held-out split the pair sent one more call it should allow to the classifier than the previous default, a weighted 0.26% against 0.15%, which misses the 0.25% limit set before the evaluation by that one call; on the development split it sent fewer, 0.40% against 0.63%. I accepted that for the cost: $2.10 per 1,000 calls against $15.69, with 8.4% of calls reaching the second judge.The policy is rewritten as decisions taken in order: leaks, whether the call serves the task and keeps to what the user ruled out, the own-repository allowance, whether each effect can be undone, and the authorization the irreversible ones need. It describes each field the judge reads and each fact a check returns, with what the fact does not establish.
A judge model call that comes back without a reply, or with an API error the engine says may clear on its own (
rate_limit,overloaded,server_error), is made once more two seconds later; each judge's 90 seconds now cover all its model calls for one tool call. Thejudgedline says inisAnsweredwhether the judge's model answered, infailurewhy not, and inretriedwhich failures were retried, so the provider's failure rate stays visible after the retry hides it.A call auto mode denied, made again unchanged before the user writes again, goes to the classifier instead of the judge, which could allow it on a second sample with nothing new from the user; it is logged as
repeated. The plugin keeps a SHA-256 digest of the judge's first prompt for each denied call, so "unchanged" covers the input, the directory, a subagent's brief, and the user's words.The judge now reads the user's whole kept conversation, which the store already bounds at 64,000 characters, instead of up to 24,000 characters selected per call by wording, which could leave out a prohibition phrased without the words selection looked for. The conversation opens the prompt as its own block marked for the prompt cache, so calls until the user writes again read it, and on Bedrock, Vertex, Foundry, or a gateway the policy too, from the cache.
For a ref with commits no other ref holds, the ref facts now count how many of them
git cherryfinds with an equivalent patch in the ref's upstream or a remote's default branch, so deleting a branch merged by rebase need not go to the classifier. Path and ref facts move out offacts.tsintopaths.tsandrefs.ts.The withholding scan no longer withholds text that names a private key's format without a key, such as a PEM header, a key file's markers around placeholder text, an age key's prefix, or the fixed start of a short-lived Amazon Bedrock key; reading the plugin's own source was withheld for this, and
bun run test:sources, now part ofbun run check, scans the plugin's tracked files. gitleaks'private-keyrule is left out of the generated rules for the same reason; the PEM shape takes its header characters and counts 40 characters of a body across lines of any width and line ending, so a key wrapped at any width or with any line ending is still withheld. Armor around a placeholder of 40 or more letters and digits split over lines is withheld too, and a key after more than about 200 characters of headers, or a narrow body with indented or blank lines, is not; the README lists both among the known gaps. The generated Slack webhook and Sidekiq rules escape the dots in their hostnames, which fixes CodeQL alert #1 (js/incomplete-hostname-regexp).Each change is its own commit, and each commit passes the plugin tests, lint, formatting, and the core type check on its own. Every new safeguard has a test that fails when it is removed.