docs(changelog): review gate 4.5b2 part B, its door and known issues
This commit is contained in:
parent
ec5b6483ec
commit
03c9f527a3
2 changed files with 212 additions and 2 deletions
24
CHANGELOG.md
24
CHANGELOG.md
|
|
@ -20,6 +20,26 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
|
|||
stale approval before the edit; with it, 0, 0, 0, 0 and 3 of 3. Six eval cases
|
||||
`evals/skill-annotating-*`, written, not run.
|
||||
|
||||
**Review gate, 4.5b2 part B (decisions 93-105, 128)**
|
||||
|
||||
Part B makes a WARN open the merge lock and the release check only when the operator has answered every counting MAJOR, and gives `review.md` the fields for it (`review_version` 1.2). Each behaviour has a test seen failing first at the part's red base `96fba75` (`tests/fix-round-4-5b2-b-red.sh` proves it).
|
||||
|
||||
- **Every counting MAJOR needs the operator's answer** (ANS). `lib/review/review-answers.mjs` reads a review's `## Answers`. The verdict gate exits 2 with a reason that starts `review-unanswered:` while a counting MAJOR has no answer, also under an ALLOW verdict, in a 1.1 review and outside a git repository. An answer counts only as `accepted` with a reason that is not hollow, and only when the latest K2 `review` record for that finding binds it to the review's current JSON block and `scope_sha_end`, so it goes stale with the review it answers. An answer `fixed` keeps exit 2 (`fixed-pending`). A security finding is never accepted: `record` refuses it, and a hand-written row does not count. `record` needs a filled `--trailer` and commits the top-level approval log; a BLOCKER is never answered.
|
||||
- **The release check reads the same answers** (RLS). `scripts/release-check.mjs` reports not ready (exit 1) for a review with an unanswered counting MAJOR and ready once it is answered, from the same top-level approval log. Its door helpers moved to `lib/util/doors.mjs`, which it re-exports.
|
||||
- **A commit that only appends review answers keeps the review current** (RAP). `lib/review/reviewed-commit.mjs` accepts a commit after the reviewed commit whose only change to the top-level approval log appends K2 `review` records, each a valid link of the chain (its hash, sequence and previous hash), to a log that stays a regular file; any other change to the log still makes the review stale. It holds for a project in a subdirectory too.
|
||||
- **`review.md` is at `review_version` 1.2** (V12). The template and the coordinator write 1.2; its new fields and sections (answers, pre-existing marks, rounds, the policy, what ran) are optional, and the validator checks each one present for its shape, with its own code (`REVIEW_ANSWERS_MALFORMED`, `REVIEW_POLICY_MALFORMED`, `REVIEW_ROUND_MALFORMED`, `REVIEW_PRE_EXISTING_MALFORMED`); an answer `fixed` without its `### Step R<n>` fails. A 1.1 review still validates. `MIGRATION.md` gives the N-1 window; fixtures hold both shapes.
|
||||
- **A finding on a line the range did not add or change is pre-existing** (PRX). `lib/review/pre-existing.mjs` reads the added and changed lines from one `git diff -U0` of `scope_sha_start..scope_sha_end`, with flags no repo config changes; `lib/review/review-finalize.mjs` writes the mark, and the verdict gate derives it again and never trusts the written one. Such a MAJOR leaves the answer set. A BLOCKER or a security finding counts in full whatever the mark, and every uncertain case counts as new: a renamed file, line 0, no `scope_sha_start`, a line next to a pure deletion, a path that is not a regular file at `scope_sha_end`, a file with uncommitted changes in finalize.
|
||||
- **Review rounds carry a finding until it is shown resolved** (RND). `lib/review/review-rounds.mjs prepare` gives round 2 and later the diff from the previous round's `scope_sha_end` and archives the previous review as `review-round-<run>-<n>.md`. Finalize's carry keeps every previous finding until the round gives positive evidence: an engine finding resolves when its engine ran and did not raise it again, a reviewer finding when every reviewer that ran before marks it resolved. A repeated finding folds into the old one; a round 2 or later review that finalize did not finalize is not measured (exit 2); `prepare --workflow` refuses round 2 or later.
|
||||
- **One cap of three review rounds** (CAP). `MAX_REVIEW_ROUNDS` in `lib/review/review-rounds.mjs` is 3: `prepare` refuses a fourth round (exit 1), and `/trekexecute` reads the same constant with `--cap`. The `TREKEXECUTE_MAX_REVIEW_ROUNDS` override is gone.
|
||||
- **An accepted answer carries to a later round only through its own link** (CRY). A carried MAJOR is answered by its earlier answer only when its row's `Carried from` link names an archive of the same run and an earlier round, the archive holds the old finding with the same answer, the link follows the finding's own carry chain and the K2 record still binds it; another run's archive, a later round or another occurrence of the same class keeps exit 2.
|
||||
- **The operator answers each MAJOR, and "fixed" runs the fix loop** (FXL). `/trekreview` Phase 8.8 lists the open MAJORs with `review-answers.mjs pending` and asks about each with `AskUserQuestion`, at most four per call and one finding per question: Fix it, or Accept with a reason, never offered for a security finding. `/trekreview --answers` runs only this step and opens no new round. A "fixed" answer gets its `### Step R<n>`; `/trekexecute` enters its fix loop on `review-unanswered: fixed-pending` and closes the merge with the decline reason `review-unanswered` otherwise.
|
||||
- **A `REVIEW.md` policy caps and skips MINOR and SUGGESTION findings only** (POL). `lib/review/review-policy.mjs` reads one `voyage-review-policy` block (`max-minor`, `skip`) from `REVIEW.md` at the repo root, from git at `scope_sha_start`, as data. A capped or skipped finding goes to Declined to Judge (`policy:cap`, `policy:skip`); a BLOCKER, a MAJOR or a security finding is never moved, and the gate exits 2 on a review that moved one. A malformed, symlinked or in-range changed file applies no policy; the review records the file's sha256.
|
||||
- **The standard path puts back engine findings the coordinator dropped** (ENG). `lib/review/engine-compare.mjs`, run by finalize, puts an engine MAJOR or BLOCKER that `review.md` does not hold back at its own severity, as the workflow path does; the verdict is only raised (a BLOCKER gives BLOCK). A finding under a trimmed title is not put back twice, an engine finding in `out_of_scope` stays declined, and its row rule is pinned equal to the workflow script's.
|
||||
- **The code-correctness reviewer checks the repo's tracked instruction files** (CMD). Its eighth dimension checks the diff against the set `lib/review/instruction-scope.mjs --set` gives: the root and on-path `CLAUDE.md` and `AGENTS.md` and `.claude/rules/*.md`, tracked at the reviewed commit. An untracked file, a file outside the repo or a tracked symlink that resolves outside it is left out; a finding that cites a file outside the set goes to Declined to Judge (`instruction-out-of-set`).
|
||||
- **A changed instruction file gets deep review at any depth** (DPR). A changed `CLAUDE.md`, `AGENTS.md` or `.claude/rules/*.md`, `docs/**` included, gets deep review (`instruction-scope.mjs --triage`); the triage table's rows are unchanged.
|
||||
- **Two instruction-file rule keys at MINOR** (CAT). The rule catalogue gains `CLAUDE_MD_DEVIATION` and `CLAUDE_MD_STALE`, both MINOR, category conformance, with no memory route; it holds 17 keys, and the workflow script's tier table holds both.
|
||||
- **The merge question says how the run was tested and how it is undone** (MEV). Before the merge, `/trekexecute` Phase 8 runs `lib/review/merge-evidence.mjs write`, which derives an internal evidence file from the run's artifacts under the git common dir, never tracked. The Bash guard reads it into a "Tested by" and an "Undone by" line of its merge question; its answer stays "ask", and a missing, stale or unreadable file reads "no run evidence" or "not measured". Files older than 30 days are removed.
|
||||
|
||||
### Changed
|
||||
|
||||
- **The "run the thing" step names Claude Code's built-in `run` skill.** The plan's run step (`/trekplan`), the executor's end-to-end part (`/trekexecute`) and the plan template each say in one sentence that the app is seen and driven with `run`, and that the verdict is still the `End-to-end:` command through the criteria runner, because `run` gives no exit code (decisions 54 and 64).
|
||||
|
|
@ -215,6 +235,9 @@ Fix round 4.5a (decisions 57, 67 and 68): the round's 30 findings, where a Voyag
|
|||
- **An error in the shell reading hides the regex reading's reason** (MAJOR `3890ec2b`, `hooks/scripts/pre-bash-executor.mjs`; part A's review, decision 127). The shell reading now runs first, so an error in it blocks with the unquoted, unreadable reason even where the regex reading would have named the early close. The command is still blocked; only the reason is wrong.
|
||||
- **More spellings of the operator for a second shell pass** (MINOR `402a9416` and `0464e12e`, `hooks/scripts/pre-bash-executor.mjs`; part A's review, decision 127). Measured by the review: quoting inside the delimiter word (`VOYAGE"_ARGS…"`, `VOYAGE\_ARGS…`), a `$'…'` word, and an escaped `<` next to a quoted one (`\<'<'`, `'<'\<`) after `eval cat` pass the guard, and `/bin/bash` and zsh expand the body. The check reads one spelling of the operator and a prefix-stripped word, not the word as a second shell reads it; the mechanism goes to the 4.5c structure review with the full reader.
|
||||
- **Smaller findings of part A's review** (MINOR `397b32ae`, `179386c4`, `73f6727b`, `75c439ab`, `f9ee4fd4`; decision 127). A second, weaker word reader next to the shell reading's word loop; the split-opener rule repeated in three places; the suffix branch repeating the regex reading's quoted test and body lines; a per-opener copy of the rest of the command in the re-read check; and this round's simplify commit `cc277c4` again without a scope. The corpus test's case with the guard switched off also passes on a corpus run killed at its timeout.
|
||||
- **A MAJOR at an unchanged caller can be marked pre-existing** (part B, Requirements Check 6). The pre-existing mark reads only the lines the range added or changed, so a MAJOR at an unchanged caller that a changed signature broke is marked pre-existing and needs no answer. Lines on both sides of a pure deletion count as changed. A BLOCKER or a security finding there still counts in full.
|
||||
- **`/security-review` re-raises run-range findings in round 2** (part B). The engines stay on the run range from round 2, because `/security-review` takes no range, so it raises its findings from the whole run again. Carry folds each into the previous finding it matches: it is carried, not counted as new, and it stays open while the engine raises it.
|
||||
- **Review answers cannot be recorded with the installed 6.5.0 plugin** (part B). Its `/trekreview` has no answers step, and its review stays at 1.1. This delivery's own review runs on 6.5.0, so before its release check the operator answers its MAJORs with this repo's `lib/review/review-answers.mjs` (`pending`, then `record`), the path `docs/release-runbook.md` gives.
|
||||
|
||||
### Doors
|
||||
|
||||
|
|
@ -228,6 +251,7 @@ Fix round 4.5a (decisions 57, 67 and 68): the round's 30 findings, where a Voyag
|
|||
- two-way: fix round 4.5b part 3 (decision 89) — a revert restores part 2's closing-quote search (a value whose closing quote follows whitespace splits again) and the guard's regex-only opener check.
|
||||
- two-way: fix round 4.5b part 4 (decision 90) — a revert restores part 3's literal-delimiter gate and its refusal of a heredoc operator inside a string or after a backslash; the catch-branch test and the TOK wording go with it.
|
||||
- two-way: fix round 4.5b2 part A (decision 91, decision 107, decision 123) — a revert restores part 4's whole-command escape limits, the exact-word check for a split delimiter, the string and body skips of a split opener and the unchecked twice-escaped operator; the corpus, the end-to-end script, the keep-assertion test and the corrected test header go with it.
|
||||
- two-way: review gate 4.5b2 part B — `review_version` 1.2 and the answered-MAJOR merge lock (decisions 93-105, 128); a revert restores the 1.1 gate, 1.2 files still read under the old validator (unknown keys tolerated), and K2 `review` records keep the record shape.
|
||||
|
||||
## v6.5.0 — 2026-10-18 — Three skills for brief, criteria and review, English labels, and three trial-5 fixes
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue