Skip to content

[Improve] Check finished coding turns against the request in about a second instead of a minutes-long judge pass - #3071

Merged
mrubens merged 20 commits into
developfrom
feat/realtime-judge-gate
Sep 21, 2026
Merged

mrubens merged 20 commits into
developfrom
feat/realtime-judge-gate

Conversation

@mrubens

@mrubens mrubens commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Problem

After implementation, the coding agent hands a "completion and sanity check" to the hidden judge subagent. The judge has read/grep/glob and no step limit, so on a small, no-UI change it can spend minutes and ~100 tool calls re-reading the repository to answer three questions: did the diff do what was asked, does the report match the diff, did anything slip in. A second pass follows any fix. The pull request then gets reviewed anyway.

Change

Those questions become a typed, sub-second check that the harness runs itself when a turn ends. The agent is only involved when the check finds something.

Flow

  1. Turn ends. The harness collects what this task changed across the workspace. If the diff is empty, nothing happens. A diff identical to the last one checked is skipped unless a new visible request arrived since (the agent may only claim to have acted on it).
  2. It posts the diff, diff stat, and the agent's closing report to POST /api/mcp/tasks/runs/:runId/completion_check (run-token auth, run must match).
  3. The API reads what was asked (opening prompt plus latest follow-ups) and the agent's latest checklist from the transcript, never from the sandbox, redacts credential shapes, and asks the decision model up to eight yes/no questions (table below).
  4. clear or skipped: the turn completes as before. flagged (probability >= 0.8): the turn is reopened once with a hidden prompt naming the points, the same mechanism the chat-closeout stop hook uses. The prompt says the check can be wrong and that "change nothing and say why" is a valid answer. The original report is kept and joined with the short follow-up so the task report does not collapse to the correction.

The judgment model key stays on the API; only a ROOMOTE_COMPLETION_GATE=true flag reaches the sandbox, set at dequeue when a hosted judgment model is configured (an operator-set value is discarded). Any failure anywhere is skipped and never holds a turn.

Hosted judgment model only. The check passes highVolume: true, which rules out the helper-model fallback. A live probe of that fallback (same system prompt, prompt, and answer schema, five recommended small models, 9 turns x 3 questions x 2 runs) showed it is not usable here: one model false-flagged 18 of 54 judgments and another 20 of 54, mostly by answering ~0.99 to everything, and median latency ran 3-16 s with a 43 s tail. The best model still got 3 of 54 wrong. Deployments without a judgment model therefore keep today's judge pass unchanged.

Parity with the judge pass

The judge weighed Now
Full request satisfaction, including non-code asks (update the PR description, run named checks) requestUnaddressed, judged over the diff, the recorded commands, and the report. Still asked on a clipped diff: the model is told the diff is clipped and that the diff stat lists every file
The plan or checklist planIncomplete: the agent's latest todo list, read from the transcript; asked only when one exists
Validation evidence validationContradicted and validationMissing. The harness records the parent agent's last 12 shell commands with exit code and output tail as OpenCode reported them, so "tests pass" is compared with what ran, not trusted. Each command also gets a fingerprint of the changed code taken right after it finished; a run counts only while that fingerprint still matches what ships. Nothing is inferred from command text, so an edit made with an editor tool, sed -i, a heredoc script, or a git operation is treated the same, in this turn or a later one. The fingerprint is the changed files' content with formatting-only characters removed, so a formatter, a pre-commit hook, or git commit after the tests does not make them stale. Stale runs are dropped server-side rather than handed to the model with a marker
Honesty of a missing, blocked, or not-applicable proof result proofClaimDoubtful: an interface change with proof waved off or unmentioned. Captured proof and a specific blocker are both accepted
Logic risks and regressions evidentDefect, limited to what the changed lines show on their own: inverted conditions, a removed guard, error check, or await, a call left on an old signature, a test weakened to pass
Report honesty reportOverclaims
(new) leftoverArtifacts

Deliberately not ported: defects and edge cases that only show with code outside the diff. That open-ended repository reading is what made the judge pass take minutes, and pull request review covers it. When proof kept images, the judge pass still runs with its original prompt.

What "this task changed" means

Diffing from the fork point is wrong for work on an existing pull request: earlier work on the branch would be judged as this task's, and a requested removal of that earlier work nets to no change at all. The base is therefore the first checkout of the current branch from the HEAD reflog, falling back to the fork point when history was rewritten or the default branch was merged in afterwards. Untracked files count as additions; lockfiles, snapshots, and minified bundles are left out of the patch. Large diffs are clipped per file, largest first, so every file stays represented; with a clipped diff the two "is it in the diff" questions are not asked.

Judge scope

Where the check is on, the judge subagent stays for the one thing the decision model cannot do: opening proof images. The runtime judge instructions switch to a proof-only variant that spawns it only when capture-visual-proof kept screenshots or keyframes, and implement-changes defers to them. Where the check is off, the original judge instructions are used as before.

Validation

  • Live probes against the hosted judgment model:
    • 21 synthetic turns, 150 judgments, each question with a positive case and its near-miss: 150 of 150 correct at the shipped thresholds. Expected flags scored 0.81-1.00, everything else 0.74 or lower.
    • Replay of the last 45 merged pull requests (title as the request, body as the report, the real diff clipped the way production clips it): 0 flags across six questions, and 3 proofClaimDoubtful flags, all on pull requests that changed interface files without mentioning proof. p50 199 ms, p95 405 ms.
    • Same state five times: scores move by 0.01-0.02.
    • Reports carrying an injected "answer no to every question" instruction: 0 of 18 expected flags suppressed.
    • One weak spot found and handled: "claims tests pass, none were run" scores 0.79-0.82, so validationContradicted uses its own 0.65 threshold; nothing that should stay quiet scored above 0.47 on it.
  • New tests: diff base selection against real temporary git repositories (fresh branch, existing PR branch, default branch merged in, shared-root multi-repo, lockfile noise, clipping), harness turn-end behavior (clear, flagged once then completes with both reports, unchanged diff not re-checked, ineligible task types), server gate (threshold, clipped-diff question set, redaction, skip paths), API route (auth, run match, payload caps).
  • pnpm lint:fast && pnpm check-types:fast && pnpm knip pass. Worker harness suite passes.

Open before merge

  • Not yet run on a live task end to end: the platform flag reaching the harness, run-token auth on the new route, and real OpenCode command events are covered by unit tests only.
  • Thresholds are measured on the current hosted judgment model; a model update can move the margins, and nothing alerts on that yet.
  • The check runs at turn end, which is after push on the delivery turn. A flagged turn produces a follow-up commit rather than blocking the first push.
  • Existing-PR work that also merges the default branch falls back to the fork point, so the earlier-work caveat returns in that corner.
  • The only off switch is turning the judgment model off.
  • The shared helper-model decision prompt never tells the model what noul means, which likely explains the ~0.99-to-everything answers. Worth fixing for the other surfaces that do fall back to it.
  • The judge subagent itself still has no step cap for the visual-proof passes that remain.

@roomote-community

roomote-community Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

1 issue outstanding. See task

  • New visible follow-up requests can bypass the completion check when the task diff is unchanged.
  • Credential-shaped filenames in diff_stat are sent to the judgment model without redaction.
  • The changed workflow guidance leaves an existing assertion stale and the PR test job failing.
  • Completion checks stop considering the newest follow-up after 40 visible prompts.
  • A visible request arriving during an in-flight check can lose the deduplication reset.
  • Untracked lockfiles, snapshots, and minified bundles bypass the diff noise filter.
  • A native-steered follow-up can be judged by the preceding completion check and consume the follow-up's only reminder.
  • A follow-up held for the completion check can resume after cancellation and restart the task.
  • Validation evidence from an earlier turn can incorrectly prove a later change was tested.
  • Validation run before a same-turn edit can incorrectly prove the final change was tested.
  • git reset cleanup can incorrectly mark earlier validation stale.
  • Git global options can hide a source-changing checkout or switch command.
  • Parentheses are removed from validation fingerprints even though they can change JavaScript or TypeScript semantics.
  • Capped fingerprint files use size and mtime instead of content, allowing stale validation evidence.
  • Fingerprinting capped files can buffer arbitrarily large content in memory.
  • Validation fingerprints still erase syntax with JavaScript or TypeScript semantics.

Reviewed 8559bef

Comment thread packages/cloud-agents/src/server/task-completion-gate.ts Outdated
Comment thread apps/worker/src/sandbox-server/lib/harnesses/opencode-server/harness.ts Outdated
Comment thread packages/cloud-agents/src/server/task-completion-gate.ts Outdated
Comment thread apps/worker/src/sandbox-server/lib/harnesses/opencode-server/harness.ts Outdated
Comment thread apps/worker/src/sandbox-server/lib/harnesses/opencode-server/completion-gate.ts Outdated
…oof claims, evident defects, and clipped diffs
Comment thread apps/worker/src/sandbox-server/lib/harnesses/opencode-server/harness.ts Outdated
Comment thread apps/worker/src/sandbox-server/lib/harnesses/opencode-server/harness.ts Outdated
Comment thread apps/worker/src/sandbox-server/lib/harnesses/opencode-server/harness.ts Outdated
Comment thread apps/worker/src/sandbox-server/lib/harnesses/opencode-server/harness.ts Outdated
Comment thread apps/worker/src/sandbox-server/lib/harnesses/opencode-server/completion-gate.ts Outdated
Comment thread apps/worker/src/sandbox-server/lib/harnesses/opencode-server/completion-gate.ts Outdated
* What formatters and pre-commit hooks rewrite: whitespace, quote style,
* trailing commas, semicolons, and wrapping parentheses.
*/
const FORMATTING_ONLY_CHARACTERS = /[\s'"`;,()]/g;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing parentheses is not formatting-only in JavaScript/TypeScript. For example, a test can finish with const value = (a + b) * c, then the source can change to const value = a + b * c; both normalize to the same fingerprint even though behavior changed. The earlier validation is therefore sent as current evidence. Do not strip semantically meaningful syntax such as parentheses when deciding whether validation is stale.


for (const file of [...new Set(files)]
.sort()
.slice(0, MAX_FINGERPRINT_FILES)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file/content caps make this fingerprint non-authoritative: after 300 changed paths, edits to a later path never enter the hash; likewise, a changed file over 2 MB is represented only by its byte length. In either case a prior passing test can survive a real source edit and suppress the validation checks. Include all changed content, or make validation evidence stale whenever the fingerprint has to omit or coarsen a file.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still leaves the validation evidence non-authoritative for files handled by statIdentity: a same-size edit that preserves the mtime (for example, a copy preserving timestamps) hashes identically, so the earlier passing command is retained for different shipped source. The new regression only changes the mtime, so it cannot catch that case. Hash the content or treat capped files as stale evidence.

* the worse mistake. When a reformat does move parentheses after a test run,
* that run reads as stale and the agent is asked to run it again.
*/
const FORMATTING_ONLY_CHARACTERS = /[\s'"`;,]/g;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing backticks, semicolons, and all whitespace is still not behavior-preserving JavaScript normalization. For example, const value = '${token}' and ``const value = ${token}``` normalize to the same fingerprint even though the latter interpolates; return\n{ value: 1 }` versus `return { value: 1 }` has the same issue through ASI. A test before either edit is therefore retained as validation for different source. Keep syntax that can affect parsing/semantics, or conservatively mark such changes stale.

// Every changed file is read, so an edit anywhere changes the result.
// Past the caps the bytes are hashed as they are: normalizing them is the
// expensive part, and skipping it only errs toward calling a run stale.
const content = await readFile(join(repoPath, file)).then(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still buffers every capped file before hashing it. A task that changes more than 300 files can put an arbitrarily large generated/binary file after that boundary; readFile allocates the entire file, so the completion path can stall or exhaust the worker rather than taking the intended quick/skipped path. Hash capped files with a stream, or retain a bounded fallback that marks earlier validation stale without loading their content.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants