Skip to content

Offer a review's fix only where it can run, and log a refusal once - #187

Closed
madskronborg wants to merge 1 commit into
masterfrom
madskronborg/review-offers-fix-only-where-it-can-run
Closed

madskronborg wants to merge 1 commit into
masterfrom
madskronborg/review-offers-fix-only-where-it-can-run

Conversation

@madskronborg

Copy link
Copy Markdown
Collaborator

A review now checks whether a fix can run before its first question, and offers only the fix options that can. Before this, review offered "Fix findings" and "Fix findings in a full implement run" for every target. For a target with no branch to fix, for example HEAD, or a merge request reviewed from a checkout with no remote, the fix was refused only after it was chosen. The menu then came back with the same options, and each refusal was logged again on every replay.

  • The new host call host.reviewedBranch(runId) gives the reviewed branch, or the reason there is none, in the same words the checkout uses. It cuts and records nothing.
  • Where there is no branch, the question says why. "Fix findings" stays only where a parent Run might have a live implementer to hand the findings to.
  • A fix refused when it starts is not offered again in that Run, and its refusal is logged once.
  • ADR-0054 carries the amendment "a fix that cannot run is not offered", and docs/workflows.md and docs/sdk.md describe the new behaviour.

Closes #185.

Verification: on the branch as it stands, regression, regression-both-ids and typecheck passed (by agent). Collie's check failed, and review-scenarios was unstable.

Verifications this run is held to:

  • check: /home/mk/.local/share/mise/shims/bun run check (in /home/mk/.herdr/worktrees/.collie/madskronborg/review-offers-fix-only-where-it-can-run)

The three gate-fix rounds after the last review were checked by their own tests and dispositions, not by a reviewer. They changed no code:

  • Gate fix 1 re-recorded regression on the final tree. It also corrected an earlier claim: the "release on github … fix round" failure was not proved to exist on the base, because the "base" run had actually run in this branch's checkout.
  • Gate fix 2 re-recorded regression with the same command as the failing reproduction.
  • All three rounds disputed the failed check.

Left open as non-blocking:

  • [minor] The "release on github: a check failing on the head a round pushed goes to a fix round, whose head is proved again" scenario failed twice when run alone on this branch. It was not proved pre-existing. The release workflow does not call the new reviewedBranch, and the scenario passed in both full gate runs.

Not settled by the run

  • assumed in build: The ADR-0054 amendment was not on this branch. The planner pointed me to its uncommitted copy in /home/mk/.herdr/worktrees/.collie/roaming, and I applied that section verbatim and committed it with this change.
  • assumed in build: The Host call is named reviewedBranch and returns the new ReviewedBranch type ({ branch } | { refused }). It is added to the SDK's Host and to the engine's author-facing declaration, and it shares one row/launch/target reader (reviewing) with checkoutReviewed.
  • assumed in build: The pre-check's refusal is logged as 'no fix can run here: '. The fix-time refusal keeps its existing 'fix: nothing fixed, ' line, so the log-once assertion counts only the fix-time line.
  • assumed in build: The question reads 'Only a live implementer can take a fix: .' when Fix findings is kept for a lineage hand-off, and 'No fix is offered: .' otherwise. It sits after 'The rally stopped: …' when both apply.
  • assumed in build: The rewritten no-branch scenario was renamed to 'a review with no branch to fix offers no fix, says why, and starts no fixer', because it no longer asks again.
  • 01-review-offers-fix-only-where-it-can-run.md: after merge: Issue A review offers "Fix findings" from a checkout that refuses every fix, and asks again #185 closes.
  • check failed: gate fix 3 disputed it and changed nothing: The tree is unchanged (b6dd8f0), and the gate has not run again since 21:50, so this still reads that run.
  • It failed 3 host process-kill timing tests in test/check-processes.test.ts and test/host.test.ts, code this diff does not reach. The diff changes only the review workflow, the engine's reviewed-branch lookup, the SDK type, test/support/baseline.ts and docs. 4055 tests passed.
  • The gate's earlier run failed a different set of 10 check-process kill tests.
  • vm-mk's load average is about 95 on 16 cores, with another branch's full suite running.
  • I cannot run the base in this Run's checkout: collie verify runs only there, and putting the base's files into the checkout was refused.
  • No change on this branch would make a 30s kill-timing test pass under that load. A gate re-run on a quiet VM settles it. Otherwise it is the human's call.

A review offered Fix findings for a target with no branch to fix, refused
every answer the same way, and replays wrote the refusal again each time.
The review now asks the host before its first menu whether the reviewed
branch can be established, by the checkout's own resolution and cutting
nothing, and withholds what cannot run, naming why. A fix refused when it
starts stands for the rest of the Run, and each refusal is recorded inside
the Activity that met it.

Closes #185.
@madskronborg madskronborg self-assigned this Oct 22, 2026
@madskronborg

Copy link
Copy Markdown
Collaborator Author

Superseded by #197 (merged, released in 0.93.0), which carries the same host.reviewedBranch pre-check, ADR-0054 amendment and docs. Closing as a duplicate.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

A review offers "Fix findings" from a checkout that refuses every fix, and asks again

1 participant