Skip to content

fix(hooks): key the push warning to the commit the review read - #251

Merged
gurinderu merged 3 commits into
mainfrom
fix/push-hook-reviewed-head
Oct 22, 2026
Merged

gurinderu merged 3 commits into
mainfrom
fix/push-hook-reviewed-head

Conversation

@gurinderu

Copy link
Copy Markdown
Owner

The push hook's "no complete craft verdict" warning now keys on the commit the review read, not on HEAD and git status when the run record is written. A record carries reviewedHead/reviewedDirty only when the engine's first checkpoint (stamped by code) names the same commit as the head the engine saw before any lens; dirtiness counts tracked files only. A fix committed while a background review runs no longer counts as reviewed, and an untracked file no longer makes every push warn. craft:adversarial-review now writes a start checkpoint and files with finalize, so its verdict counts too; the line names craft:review for Rust/Nix diffs and craft:adversarial-review for any other. Answers the Mediums of the 0.33.0 release review.

Dropped Low findings:

  • Records filed before this change carry no reviewed head, so the first push after upgrading warns once.
  • A docs-only Approve (nothing to review) files no checkpoint and never counts as a verdict.
  • Dirtiness is taken at the first checkpoint, not at the engine's first step; a tree cleaned in between counts as clean.
  • An adversarial-review run stopped by the usage limit leaves its .partial run directory behind after recover.

The push hook read a record's write-time `head` and `dirty`: a background
review that filed after the session committed a fix marked that unreviewed
commit as reviewed, and any untracked file made every record dirty.

craft-log-run now stamps tracked-only dirtiness on each checkpoint, and
finalize files `reviewedHead`/`reviewedDirty` from the run's earliest
checkpoint (its own directory only). Every other write path files none, and a
payload's copy is dropped. The index carries both columns; hasVerdict matches
only `reviewedHead` with `reviewedDirty === false`. `head` and `dirty` keep
their meaning for their other readers.
The review engine claims the head its detect step saw (startHead) on every
checkpoint; the logger files reviewedHead only when the first checkpoint's
observed head names the same commit, so a fix committed while scout and gate
ran is no longer taken as reviewed. craft:adversarial-review files no reviewed
head, so its verdict no longer silences the push warning.
…d from

craft:review has no language profile for a JS/TS/Python diff, so in such a repo
craft:adversarial-review is the review, and dropping it from the push hook warned
on every push. The adversarial engine now has its scout capture the head first,
writes one start checkpoint after the scout and before the memory recall and any
lens, and files its record with finalize: reviewedTree keys the verdict to the
checkpoint's code-observed head only when it names the commit the scout saw,
the same rule craft:review goes through. hasVerdict counts adversarial-review
again, and the push line names the review that fits the diff.
@gurinderu
gurinderu merged commit e90dc5e into main Oct 22, 2026
3 checks passed
@gurinderu
gurinderu deleted the fix/push-hook-reviewed-head branch October 22, 2026 12:28
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