fix(review): preflight reviewer binary before creating review runs (#2092) by ApexYash11 · Pull Request #2767 · AgentWrapper/agent-orchestrator

ApexYash11 · 2026-07-17T12:59:17Z

Summary

Fixes #2092 by validating the reviewer binary before creating any ReviewRun records.

Previously, if the configured reviewer binary (for example codex or claude) was not available on PATH, the review trigger would:

Create ReviewRun records in the running state.
Fail later during reviewer launch when resolving the binary.
Mark the runs as failed with zero findings.

This made an infrastructure failure appear similar to a completed review with no findings.

With this change, reviewer availability is verified before any review runs are created. If the binary is unavailable, the trigger fails immediately with a clear error and no database state is written.

Implementation

Added an optional ReviewerPreflighter interface for reviewer adapters.
Added Launcher.Preflight() to perform reviewer availability checks before launching.
Implemented preflight support for the Codex and Claude Code reviewer adapters.
Invoked preflight during Trigger() immediately after reviewer harness resolution and before any ReviewRun creation.
Preserved the existing runtime failure path (failRuns) for failures that occur after a successful preflight (for example process launch failures).

Testing

Added regression tests covering:

reviewer binary missing → trigger fails before creating any ReviewRun
successful preflight → existing review flow is unchanged
launcher preflight delegation
unsupported reviewer adapters
missing reviewer adapter

Verified with:

go test ./internal/review/... ./internal/adapters/reviewer/... -count=1
go build ./...
go vet ./...

…re creating review runs

nikhilachale · 2026-07-17T13:22:00Z

 	"Bash(git commit:*)",
 }

+// Preflight checks that the claude binary is available on PATH before the


we already have a auth check so use that one using GET /api/v1/agents

Response shape:

{
"supported": [
{ "id": "codex", "label": "Codex", "authStatus": "authorized" }
],
"installed": [
{ "id": "codex", "label": "Codex", "authStatus": "authorized" }
],
"authorized": [
{ "id": "codex", "label": "Codex", "authStatus": "authorized" }
]
}

If you want to force fresh probes first:

POST /api/v1/agents/refresh

For one agent:

POST /api/v1/agents/{agent}/probe

authorized is the list of locally installed agents whose latest auth probe returned authorized.

no need to check binary for this agents we already check them initially when daemon starts
use this api to check does the review agents is authorized or not and than add the fallback you are using

…tions; integrate agent checker for preflight validation

ApexYash11 · 2026-07-17T14:43:06Z

@nikhilachale review it now!!

neversettle17-101

Thanks for moving the reviewer failure earlier. I think this still needs a couple of changes before it matches the worker-agent spawn flow.

The preflight currently runs before we know whether a new reviewer launch is needed. In Trigger, an already-approved/current run or an existing same-SHA running run should be a no-op and return the existing review state. With the preflight at this point, that no-op can now fail just because the reviewer binary is currently unavailable. Worker spawn only validates the binary when it is actually about to launch a process, so I think reviewer preflight should move until after the created runs list is known to be non-empty.

Also, the check is not using the same source of truth as worker spawn. Spawn builds the real adapter argv and validates argv[0] immediately before runtime.Create; this PR checks the agent catalog's authorized list instead. The catalog is advisory/stale-prone and includes auth status, so it can reject cases that spawn would not reject, while still not proving the concrete reviewer command's binary is valid. Can we make the reviewer path build/resolve the actual reviewer command and validate the command binary with the same helper/semantics as worker spawn?

One smaller point: Preflight should probably resolve the reviewer adapter too. Right now it can pass based on an agent catalog entry, then still create failed runs later if the reviewer resolver does not have an adapter for that harness.

…reflight checks for reviewer binaries

ApexYash11 · 2026-07-17T15:47:48Z

@neversettle17-101 check once now

neversettle17-101

Two remaining preflight/launch parity concerns from re-review.

neversettle17-101 · 2026-07-17T17:48:23Z

+	// to start the runtime pane. Uses the same adapter + ReviewCommand path as
+	// Spawn. On failure, created runs are marked failed via failRuns so a
+	// future trigger can retry.
+	if err := e.launcher.Preflight(ctx, harness, worker.Metadata.WorkspacePath); err != nil {


This still preflights before we know whether AO needs to launch a new reviewer process. If reviewRow.ReviewerHandleID is alive, the trigger only needs to notify the existing pane, so a missing binary on the current PATH should not block that flow or mark the new run failed. Can we move preflight into the handleID == "" path immediately before Spawn, so it only runs when a fresh reviewer launch is required?

neversettle17-101 · 2026-07-17T17:48:23Z

+	if len(cmd.Argv) == 0 {
+		return fmt.Errorf("reviewer produced empty command")
+	}
+	if _, err := exec.LookPath(cmd.Argv[0]); err != nil {


This checks only cmd.Argv[0], while worker spawn's validation unwraps commands shaped like env KEY=value ... before resolving the real binary. If a reviewer adapter returns env FOO=bar reviewer ..., this would validate env and miss that the reviewer binary is unavailable. Can we share the worker spawn binary extraction/validation semantics here so preflight and spawn stay aligned?

- Move Preflight into the spawn-only path so it does not block Notify when an existing reviewer pane is reusable. - Unwrap leading env KEY=value prefix before exec.LookPath, mirroring the session manager's launchBinary validation. - Add regression guards: preflighted assertions on no-op and notify paths. - Add launcher tests for env-prefixed argv.

somewherelostt · 2026-07-18T08:37:44Z

Thanks for contributing to Agent Orchestrator.

This PR is being picked up by the current external contributor on-call pair:

If someone is already working on this, please continue as usual.
The on-call pair is added for visibility, tracking, and support, not to take over the work.
If you need help with review, direction, reproduction, or next steps, please tag @neversettle17-101 and @somewherelostt here.

For faster context or live questions, you can also join the AO Discord.

Join the session here:
https://discord.gg/H6ZDcUXmq

Come by if you want to see what is being built, ask questions, or just hang around with the community.

ApexYash11 · 2026-07-20T15:47:36Z

@neversettle17-101 can you review this ??

ApexYash11 added 2 commits July 17, 2026 18:26

feat(reviewer): implement preflight checks for reviewer binaries befo…

35eaf07

…re creating review runs

test: update pairingPayload test with consistent password for clarity

0ef75be

nikhilachale self-requested a review July 17, 2026 13:15

nikhilachale reviewed Jul 17, 2026

View reviewed changes

nikhilachale requested a review from neversettle17-101 July 17, 2026 13:27

refactor(reviewer): remove Preflight interface and related implementa…

9732c7d

…tions; integrate agent checker for preflight validation

neversettle17-101 requested changes Jul 17, 2026

View reviewed changes

refactor(reviewer): streamline agent service integration and update p…

764ac46

…reflight checks for reviewer binaries

nikhilachale requested a review from neversettle17-101 July 17, 2026 16:22

neversettle17-101 reviewed Jul 17, 2026

View reviewed changes

ApexYash11 requested a review from neversettle17-101 July 17, 2026 18:07

somewherelostt assigned neversettle17-101 and somewherelostt Jul 18, 2026

somewherelostt assigned illegalcall Jul 20, 2026

fix: resolve merge conflict in daemon.go

86500ee

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(review): preflight reviewer binary before creating review runs (#2092)#2767

fix(review): preflight reviewer binary before creating review runs (#2092)#2767
ApexYash11 wants to merge 6 commits into
AgentWrapper:mainfrom
ApexYash11:fix/review-preflight

ApexYash11 commented Jul 17, 2026

Uh oh!

nikhilachale Jul 17, 2026

Uh oh!

ApexYash11 commented Jul 17, 2026

Uh oh!

neversettle17-101 left a comment

Uh oh!

ApexYash11 commented Jul 17, 2026

Uh oh!

neversettle17-101 left a comment

Uh oh!

neversettle17-101 Jul 17, 2026

Uh oh!

neversettle17-101 Jul 17, 2026

Uh oh!

somewherelostt commented Jul 18, 2026

Uh oh!

ApexYash11 commented Jul 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

ApexYash11 commented Jul 17, 2026

Summary

Implementation

Testing

Uh oh!

nikhilachale Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

ApexYash11 commented Jul 17, 2026

Uh oh!

neversettle17-101 left a comment

Choose a reason for hiding this comment

Uh oh!

ApexYash11 commented Jul 17, 2026

Uh oh!

neversettle17-101 left a comment

Choose a reason for hiding this comment

Uh oh!

neversettle17-101 Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

neversettle17-101 Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

somewherelostt commented Jul 18, 2026

Uh oh!

ApexYash11 commented Jul 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants