Skip to content

A reviewer turn ends when its verdict exists, and reviewers are told where the worktree is - #10684

Merged
briansrls merged 1 commit into
mainfrom
reviewer-press
Sep 7, 2026
Merged

briansrls merged 1 commit into
mainfrom
reviewer-press

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

What

Two of the three reviewer-harness gaps the first live review run exposed (2026-09-06, attempt eca90231 on shell-typed-invocation: two verdicts of six, four reviewers spent 20 rounds guessing directories or continued after writing).

  • Completion by artifact. HarnessTurnConfig.completion: HarnessTurnCompletion — CompletesOnProviderStop (worker, probe) or CompletesWhenEntryPresent { directory, name } (reviewer, auditor). After each tool round the loop observes presence via a successful directory listing (filesystem_entry_presence), returns TurnCompletedByArtifact (exit 0, label completed-by-artifact, turn.completed event with the artifact path). Indeterminate listing → counted completion.indeterminate event, turn continues. The recursion is factored into harness_turn_continue so the observation has one continuation.
  • Worktree path in reviewer/auditor guidance. ToolRunsInTheCandidateWorktreeAt { path } replaces the unnamed premise in both guidance modules; harness_reviewer_guidance_at / harness_auditor_guidance_at take the worktree.
  • Belt → CLI plumbing. gunbc_harness_review_argv and the audit argv pass verdict_dir + verdict_name; the CLIs derive verdict_path for the guidance and the seat actor, so the completion subject and the prompt share one authority.

Evidence

Fresh build, claim_batch --hermetic: harness_review_guidance_test 2/2 (now asserts the worktree path is in the prose), harness_goal_audit_guidance_test 3/3, new harness_turn_completion_test 2/2 (artifact completion exits 0 while budget exhaustion exits 1; provider-stop completion is never reached). roadmap_belt_tick_cli belt_review_head_cli and harness_cli harness_reviewer_cli typecheck and run through to the wire with the new arguments.

Not in this PR

An isolated verdict slot per reviewer (the how-to-work reviewer read a sibling's verdict from the shared review directory).

🤖 Generated with Claude Code

https://claude.ai/code/session_019iXPtAqzPaNBZbaaM5rxrY

…the worktree is

Two of the three harness gaps the first live review run exposed (2026-09-06, attempt eca90231 on shell-typed-invocation). (1) The reviewer's turn ended only on the provider's stop reason, so a reviewer that wrote its verdict at round 19 kept spending rounds until the budget refused and its unit reported failure over a verdict the belt accepted. `HarnessTurnConfig` now carries `completion: HarnessTurnCompletion` — `CompletesOnProviderStop` for the worker, `CompletesWhenEntryPresent { directory, name }` for the reviewer and auditor — observed after each tool round by a successful listing of the verdict directory (`extdeps.filesystem.filesystem_io` `filesystem_entry_presence`, never a failed read), and `TurnCompletedByArtifact` exits zero with its own label and `turn.completed` event; an indeterminate listing is a counted `completion.indeterminate` event and the turn continues. (2) Reviewer and auditor guidance said the tool runs "in the candidate's worktree" without naming it, and every slow reviewer spent rounds guessing directories; `ToolRunsInTheCandidateWorktreeAt { path }` now carries the path, as the worker's `WorktreeAt` already did. The belt passes `verdict_dir` and `verdict_name` to the reviewer and auditor CLIs, which derive the path, so the completion subject and the guidance share one authority.

Not in this change: an isolated verdict slot per reviewer (the how-to-work reviewer read a sibling's verdict); the shared review directory stays and is a separate change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019iXPtAqzPaNBZbaaM5rxrY
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 6, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-06T19:29:52.006151Z e3bd8ca PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e3bd8cab5c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

match harness_write_events(config: config, events: stepped) {
HarnessEventsUnwritable { detail: d } => harness_event_write_refusal(config: config, detail: d)
HarnessEventsWritten =>
HarnessEventsWritten => match harness_completion_observe(completion: config.completion) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Enforce artifact completion on provider stops

When an artifact-gated reviewer or auditor emits a terminal response without a tool call—either immediately or after a failed verdict-write—the loop takes the list_length(items: uses) == 0 branch before reaching this observation and returns TurnProviderStopped, which harness_turn_exit maps to exit 0. Consequently, CompletesWhenEntryPresent is not actually required: the belt retains a supposedly successful unit with no verdict and refuses to respawn it. The provider-stop branch should observe the configured artifact and fail when it is absent.

Useful? React with 👍 / 👎.

@briansrls
briansrls merged commit 382952c into main Sep 7, 2026
4 checks passed
@briansrls
briansrls deleted the reviewer-press branch September 7, 2026 19:26
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.

1 participant