feat(workflows): adversarial-verification mean+veto rewire (V2, #2487) - #2511
feat(workflows): adversarial-verification mean+veto rewire (V2, #2487)#2511flora131 wants to merge 13 commits into
Conversation
Declare finalDecision, scoreTablePath, and reviewReportPath without placeholder values that GitHub Code Quality flagged as dead stores. Every loop path assigns them before return. Assistant-model: Grok 4.6
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| Type.String(), | ||
| Type.Record(Type.String(), Type.String()), | ||
| ], { default: DEFAULT_CRITERIA, description: "Criteria record of name-to-description entries, or criteria.md markdown." }), | ||
| accept_mean: Type.Number({ default: 14, description: "Mean score required for acceptance on the 1–20 verification scale." }), |
There was a problem hiding this comment.
accept_mean accepts values outside the documented 1–20 verification scale and the runner forwards the value directly to the deterministic gate. A caller can set it to 0 to approve a full-quorum, non-vetoed result regardless of how low its score is, or set it above 20 to make even perfect reports fail. Constrain this input to the supported scale.
Artifacts
trex-artifacts/accept-mean-boundary-repro.test.ts
- Authored focused Vitest reproduction checks the TypeBox boundary values and executes the actual workflow with valid score-1 and score-20 verifier reports, proving the vulnerable threshold behavior.
trex-artifacts/accept-mean-boundary-01-before.log
- Observed pre-fix Vitest execution completed successfully against the unbounded schema while the reproduction assertions verified acceptance at zero and rejection at twenty-one, confirming the bug.
trex-artifacts/accept-mean-boundary-02-after.log
- Observed temporary bounded-schema Vitest execution completed successfully with the test requiring zero and twenty-one to be rejected, demonstrating that schema bounds prevent the issue.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/workflows/builtin/adversarial-verification.ts
Line: 21
Comment:
**Out-of-scale acceptance threshold**
`accept_mean` accepts values outside the documented 1–20 verification scale and the runner forwards the value directly to the deterministic gate. A caller can set it to `0` to approve a full-quorum, non-vetoed result regardless of how low its score is, or set it above `20` to make even perfect reports fail. Constrain this input to the supported scale.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| Type.Record(Type.String(), Type.String()), | ||
| ], { default: DEFAULT_CRITERIA, description: "Criteria record of name-to-description entries, or criteria.md markdown." }), | ||
| accept_mean: Type.Number({ default: 14, description: "Mean score required for acceptance on the 1–20 verification scale." }), | ||
| reask_limit: Type.Integer({ minimum: 0, default: 1, description: "Maximum bounded re-ask waves for invalid criterion reports." }), |
There was a problem hiding this comment.
reask_limit has no upper bound and directly controls repeated parallel verifier waves for invalid reports. A large invocation value causes each invalid cell to be retried repeatedly before the workflow returns an indeterminate result, consuming excessive model calls, runtime, and cost. Apply a conservative maximum in the schema and enforce the same bound in the runner for direct callers.
Artifacts
trex-artifacts/reask-limit-harness.ts — Focused reask-limit runner harness
- Authored Bun TypeScript harness supplies invalid verifier reports, validates the schema, and records retry waves and calls; takeaway: it directly exercises the claimed path.
trex-artifacts/reask-limit-01-before.log — Baseline invalid-verifier run with reask limit one
- Executed `bun trex-artifacts/reask-limit-harness.ts 1` successfully and recorded schema acceptance, four waves, and twenty verifier calls; takeaway: this establishes the bounded baseline.
trex-artifacts/reask-limit-02-after.log — Large reask-limit invalid-verifier run
- Executed `bun trex-artifacts/reask-limit-harness.ts 50` successfully and recorded accepted input, 102 waves, and 510 verifier calls; takeaway: this confirms uncontrolled amplification.
trex-artifacts/reask-limit-focused-test.log — Focused existing indeterminate-round test
- Executed the repository's indeterminate-round unit test successfully; takeaway: it confirms the second indeterminate round that doubles retry waves.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/workflows/builtin/adversarial-verification.ts
Line: 22
Comment:
**Unbounded verifier re-ask waves**
`reask_limit` has no upper bound and directly controls repeated parallel verifier waves for invalid reports. A large invocation value causes each invalid cell to be retried repeatedly before the workflow returns an indeterminate result, consuming excessive model calls, runtime, and cost. Apply a conservative maximum in the schema and enforce the same bound in the runner for direct callers.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| function schemaShape(schema: unknown): Record<string, unknown> { | ||
| return schema as Record<string, unknown>; |
There was a problem hiding this comment.
The new schemaShape helper casts both its input and record values through unknown, contrary to the repository requirement to use specific types and at the cost of losing useful schema diagnostics in these contract tests.
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: test/unit/builtin-workflows-adversarial-generate.test.ts
Line: 49-50
Comment:
**Ambiguous schema helper types**
The new `schemaShape` helper casts both its input and record values through `unknown`, contrary to the repository requirement to use specific types and at the cost of losing useful schema diagnostics in these contract tests.
**Context Used:** AGENTS.md ([source](https://github.com/bastani-inc/atomic/blob/main/AGENTS.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
Slice V2 of the LLM-as-a-Verifier adoption program (
specs/2026-08-17-verification-criteria-module.md§5.2): the adversarial-verification builtin moves from binary unanimity to per-criterion graded scoring with a deterministic mean+veto gate.criteria(record or criteria.md markdown; defaulttask_fit/evidence/completeness),accept_mean(default 14),reask_limit(default 1)criteria.length × verifier_countsingle-criterion stages scoring 1–20 onVERIFICATION_SCALE.schemawithevidence[]and severity-taggedfindings[]INVALID_VERIFIER_REPORTdeleted: an unparseable report persists as{invalid: true}, gets one bounded re-ask, and is never a counted vote (kills the workflows: adversarial-verification unanimity gate degrades as verifier_count grows; aggregate instead of AND #2255 bug class)decide_verification(fromverification-criteria.ts) is the sole accept path — quorum ∧ mean ≥ accept_mean ∧ no veto; the reducer is demoted to findings-consolidator and cannot flip the decision; Indeterminate rounds repeat once then end not-approved with quorum-failure evidenceapproved,mean_score, per-criterion score table path,repairs_completed,candidate_path,review_report_path,remaining_workBase:
verifier/criteria-module(V1, #2504). Reconciles with V3's prompt-layout wiring at V6 as planned.Evidence
Produced by an implement→review→repair goal run (2 turns; completion/evidence/risk reviewers all returned complete on the final checkout;
remaining_work: none):npm run check— green (includes the coding-agent tsgoerasableSyntaxOnlypass)npx vitest --run --project unit -t "adversarial"— green: per-criterion artifacts, invalid-report fixture re-asked once and never counted as a vote, veto finding forces repair regardless of mean, quorum failure ends not-approvedpackages/**only; tests uncapped — 649 total insertions across 9 files)packages/workflows/CHANGELOG.mdunder## [Unreleased]— ### Breaking Changes + ### AddedSpec contract:
specs/2026-08-17-verification-criteria-module.md§5.2 (Q1: deterministic gate owns the accept; Q2: 3 criteria, accept_mean 14).Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Greptile Summary
This change introduces configurable per-criterion adversarial verification with deterministic acceptance and retry handling. Execution confirmed that the documented 1–20 acceptance scale is not enforced, so callers can approve low-scoring non-vetoed results with a zero threshold or reject perfect results with a threshold above the scale. Execution also confirmed that a large retry setting repeatedly schedules invalid verifier work and can create hundreds of model calls from one workflow run. The contract-test helper also needs specific schema types rather than
unknowncasts.Confidence Score: 1/5
Do not merge until the acceptance and retry inputs are bounded and the generated contract test is brought into compliance with the repository typing rule.
The acceptance-threshold and retry-amplification failures were reproduced through focused executions of the real workflow path with uploaded source and output evidence. These are independent externally controlled behaviors with security consequences.
Files Needing Attention: packages/workflows/builtin/adversarial-verification.ts needs bounds for
accept_meanandreask_limit; packages/workflows/builtin/adversarial-verification-runner.ts should defensively cap retries; test/unit/builtin-workflows-adversarial-generate.test.ts needs specific helper types and boundary assertions.Security Review
Two reproduced issues affect the verification control boundary. An invocation can lower the acceptance threshold to bypass score-based rejection for non-vetoed results, and an unbounded retry input can induce disproportionate model usage, runtime, and cost through repeated invalid-report waves.
What T-Rex did
Comments Outside Diff (1)
General comment
accept_meanvalues outside its documented 1–20 verification scale, and the runner forwards that value unchanged into the acceptance policy.packages/workflows/builtin/adversarial-verification.ts:21usesType.Numberwith a default but nominimumormaximum;packages/workflows/builtin/adversarial-verification-runner.ts:224,299passes the supplied value todecide_verification, which compares it directly against the mean.accept_meanwithminimum: 1andmaximum: 20(and useType.Integertoo if fractional thresholds are not intended), then change the contract test to requireValue.Check(..., 0)andValue.Check(..., 21)to be false.Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(workflows): drop unused adversarial ..." | Re-trigger Greptile
Context used: