This is an automated email from the ASF dual-hosted git repository.
terrymanu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere.git
The following commit(s) were added to refs/heads/master by this push:
new 5b3ac0c3f37 Enforce scoped verification and local review workflow
(#39704)
5b3ac0c3f37 is described below
commit 5b3ac0c3f37bf8cb337e6062b2e5ea33b837b0c1
Author: Liang Zhang <[email protected]>
AuthorDate: Thu Sep 3 12:33:36 2026 +0800
Enforce scoped verification and local review workflow (#39704)
---
.codex/context/change-completion.md | 16 ++--
.codex/harness/agents/cases.toml | 94 ++++++++++++++++++++++
.codex/harness/agents/policy-sources.toml | 4 +-
.../code-implementation/references/verification.md | 9 ++-
.codex/skills/review-pr/SKILL.md | 20 ++---
.../review-pr/scripts/build_review_inventory.py | 10 ++-
.codex/skills/review-pr/scripts/review_common.py | 26 +++++-
.codex/skills/review-pr/scripts/review_ledger.py | 10 ++-
.../review-pr/tests/test_build_review_inventory.py | 36 ++++++++-
.../skills/review-pr/tests/test_review_ledger.py | 16 ++++
AGENTS.md | 6 +-
11 files changed, 213 insertions(+), 34 deletions(-)
diff --git a/.codex/context/change-completion.md
b/.codex/context/change-completion.md
index 33d7941cacb..cb68187af39 100644
--- a/.codex/context/change-completion.md
+++ b/.codex/context/change-completion.md
@@ -22,14 +22,14 @@ For an authorized change, build, implement, or fix request,
excluding a standalo
1. Perform the post-write task-delta audit required by the Strict Scope and
Task-Delta Gate in `AGENTS.md`, using the read-only `git diff`, surrounding
context, and acceptance checklist as evidence.
2. Confirm every task-introduced file and hunk is necessary, expected changed
files contain only their frozen change intent, prohibited paths have zero task
delta, direct reuse was considered, architecture changes were authorized, and
every test protects owned behavior. Inspect Java changes for newly added
local-variable `final`, scan changed code and Skills for local absolute paths,
and remove only current-task violations.
3. Treat scripts, searches, formatting, compilation, passing tests, and
benchmarks as evidence, not proof of semantic compliance. Judge the final
behavior, contracts, architecture, user request, and affected performance paths
directly; fix every safe in-scope violation instead of reporting it as an
accepted risk.
-4. Apply the `$code-simplification` trigger and limits defined in
`.codex/context/cross-cutting-skills.md`.
-5. Review the effective local candidate against the same code-correctness
gates used by `$review-pr`: root cause and fix mapping, affected behavior, side
effects and regressions, contracts and architecture, test validity, and
adversarial cases. Apply `$code-review-and-quality` when available, or its
equivalent review, for the general quality axes; it does not replace these
shared correctness gates.
-6. When the implementation targets an existing PR, apply the `$review-pr`
pre-handoff workflow defined in `AGENTS.md`. PR-specific public-head and
remote-evidence checks remain part of formal PR review.
-7. Verify the functional and performance non-regression gate in
`.codex/skills/code-implementation/references/rules/non-regression.md` after
the last relevant write. Treat any invalidated or inconclusive required
evidence as incomplete work, not residual risk.
-8. Fix every safe in-scope required finding, rerun invalidated checks, and
repeat the applicable reviews. If a finding requires scope expansion, an
unresolved architecture choice, or a high-risk action, stop at its existing
authorization gate instead of fixing it automatically.
-9. Hand off and propose a commit message only after one complete applicable
review pass finds zero new required issues. Before that review passes, do not
hand off or propose a commit message. Do not defer a locally discoverable
required finding to a later formal PR review.
-10. Stop when no required in-scope finding remains. Do not iterate for
optional polish, broad cleanup, or risky refactoring.
+4. Apply the `$code-simplification` trigger and limits defined in
`.codex/context/cross-cutting-skills.md`. Apply `$code-review-and-quality` when
available, or its equivalent review, for the general quality axes; it does not
replace the required `$review-pr` Formal Review.
+5. Run the applicable verification in
`.codex/skills/code-implementation/references/verification.md`, including the
final file-filtered Spotless and Checkstyle commands, then verify the
functional and performance non-regression gate in
`.codex/skills/code-implementation/references/rules/non-regression.md`. Treat
any invalidated, unmatched, or inconclusive required evidence as incomplete
work, not residual risk.
+6. Invoke `$review-pr` in Formal Review Mode on the effective local candidate
for every task covered by this loop. For a standalone local task, review the
original task baseline plus only the task-owned delta; for a task targeting an
existing PR, also apply the PR-specific public-head and remote-evidence checks.
+7. If Formal Review returns `Not Mergeable` with safe in-scope required
findings, fix them, return to the task-delta audit, rerun every invalidated
check, and invoke `$review-pr` again. Do not defer a locally discoverable
required finding to another review.
+8. If Formal Review returns `Review Incomplete`, `Not Mergeable` with
`Feedback Mode: Needs Discussion`, or a finding requires scope expansion, an
unresolved architecture choice, or a high-risk action, stop at the existing
evidence, decision, or authorization gate and keep the task incomplete.
+9. Hand off only after the latest complete Formal Review returns `Review
Result: Mergeable`, and include that final formal report as its own fenced
`markdown` block. Do not present an earlier or non-passing review as the final
report.
+10. After the fenced `markdown` formal report, provide the proposed commit
message in a fenced `text` block and the Git commands in a fenced shell block
labeled `bash` or `sh`, both outside the review report. Generate `git commit
--dry-run --only -m <message> -- <exact-task-paths>` and `git commit --only -m
<message> -- <exact-task-paths>` for only task-owned files that contain no
unrelated hunks; list any task-created untracked file in an exact `git add --
<new-task-paths>` command firs [...]
If a report or verdict is disproved, fix the highest-leverage rule, schema,
validator, prompt, or regression case before correcting the artifact, unless
the user explicitly requests a one-off correction.
-The final response must lead with the outcome and include changed files and
rationale, commands with exit codes, verification status, remaining risks, and
the next action only when one is still required. When Git commit authorization
is absent, provide a proposed commit message without staging or committing.
+The final response must lead with the outcome and include changed files and
rationale, commands with exit codes, verification status, the final Formal
Review report, remaining risks, and the next action only when one is still
required. When Git commit authorization is absent, provide the separate
proposed commit message and exact task-file commands without staging or
committing.
diff --git a/.codex/harness/agents/cases.toml b/.codex/harness/agents/cases.toml
index 57ec3de48be..6de22581160 100644
--- a/.codex/harness/agents/cases.toml
+++ b/.codex/harness/agents/cases.toml
@@ -1663,3 +1663,97 @@ required_reasons = ["code_policy_required",
"output_capture_required"]
policy_binding_profile = "test-write"
policy_assertions = [{source = "code-implementation", statements = ["For every
code-affecting write, read
`.codex/skills/code-implementation/references/rules/implementation.md`,
`.codex/skills/code-implementation/references/rules/artifact-removal-and-contract-impact.md`,
and `.codex/skills/code-implementation/references/rules/non-regression.md`.",
"Apply each contract, impact, or removal rule only when its own trigger
matches."]}, {source = "contracts-and-removal", statements = ["Regener [...]
critical = true
+
+[[cases]]
+id = "final_style_checks_target_task_files"
+group = "coding / regular"
+description = "Runs final Spotless and Checkstyle verification only for the
task-changed files."
+phase = "verification and ordinary completion loop"
+ordinary_loop = true
+profile = "code-write"
+prompt = "The final code write changed only
`module-a/src/main/java/example/Foo.java` and
`module-a/src/test/java/example/FooTest.java`; unrelated working-tree changes
exist elsewhere. Run the mandatory final Spotless and Checkstyle verification
before Formal Review, using the smallest safe scope, and do not edit, review,
hand off, or change Git."
+decision = "proceed"
+required_actions = ["run_local_checks", "wrap_high_output"]
+allowed_actions = ["inspect_local", "run_local_checks", "wrap_high_output"]
+forbidden_actions = ["edit_code", "edit_non_code", "run_pre_handoff_review",
"propose_commit_message", "mutate_git", "mutate_remote"]
+required_reasons = ["output_capture_required"]
+required_summary_terms = ["Spotless", "Checkstyle", "only"]
+policy_binding_profile = "code-write"
+policy_assertions = [{source = "code-verification", statements = ["After the
last code-affecting action and before the pre-handoff Formal Review, run both
Spotless and Checkstyle for every applicable task-changed code file; run
Spotless for a documentation-only change when the configured formatter governs
that file.", "Treat a filtered command as passing only after its output and the
post-command task-delta inspection prove that every applicable task file was
selected, no non-allowlisted [...]
+critical = true
+
+[[cases]]
+id = "standalone_local_completion_invokes_review_pr"
+group = "coding / regular"
+description = "Requires an actual review-pr Formal Review for a standalone
local task before handoff."
+phase = "ordinary completion loop"
+ordinary_loop = true
+profile = "code-write"
+prompt = "A standalone authorized local implementation has no public PR. Its
task delta, final file-scoped checks, and non-regression evidence are complete,
but `$review-pr` has not run. Complete the task now with commit artifacts."
+decision = "proceed"
+required_actions = ["run_pre_handoff_review"]
+allowed_actions = ["inspect_local", "run_pre_handoff_review"]
+forbidden_actions = ["edit_code", "edit_non_code", "propose_commit_message",
"mutate_git", "mutate_remote"]
+required_reasons = ["pre_handoff_review_required"]
+required_summary_terms = ["Formal Review", "Mergeable"]
+policy_binding_profile = "code-write"
+policy_assertions = [{source = "agents", statements = ["Every authorized
change, build, implement, or fix request requires a pre-handoff `$review-pr`
Formal Review of the effective local candidate, except a governed standalone
restoration or rollback.", "Hand off or propose commit artifacts only after the
latest complete Formal Review returns `Review Result: Mergeable`."]}, {source =
"change-completion", statements = ["Invoke `$review-pr` in Formal Review Mode
on the effective local cand [...]
+critical = true
+
+[[cases]]
+id = "not_mergeable_reenters_completion_loop"
+group = "coding / regular"
+description = "Fixes safe review-pr findings and repeats invalidated checks
and Formal Review until Mergeable."
+phase = "ordinary completion loop"
+ordinary_loop = true
+profile = "code-write"
+prompt = "`$review-pr` Formal Review returned `Not Mergeable` with one
confirmed safe in-scope Change Request. Fix it and complete the task; the fix
invalidates the final Spotless, Checkstyle, and review evidence, and those
Maven checks may output more than 100 lines. Do not hand off or propose commit
artifacts before a new passing review."
+decision = "proceed"
+required_actions = ["edit_code", "fix_review_findings", "run_local_checks",
"wrap_high_output", "run_pre_handoff_review"]
+allowed_actions = ["inspect_local", "edit_code", "fix_review_findings",
"run_local_checks", "wrap_high_output", "run_pre_handoff_review"]
+forbidden_actions = ["edit_non_code", "propose_commit_message", "mutate_git",
"mutate_remote"]
+required_reasons = ["safe_in_scope_review_finding",
"pre_handoff_review_required", "output_capture_required"]
+required_summary_terms = ["Spotless", "Checkstyle"]
+required_summary_term_groups = [["Formal Review", "review"], ["Mergeable",
"passing"]]
+policy_binding_profile = "code-write"
+policy_assertions = [{source = "change-completion", statements = ["If Formal
Review returns `Not Mergeable` with safe in-scope required findings, fix them,
return to the task-delta audit, rerun every invalidated check, and invoke
`$review-pr` again.", "Hand off only after the latest complete Formal Review
returns `Review Result: Mergeable`, and include that final formal report as its
own fenced `markdown` block."]}, {source = "code-verification", statements =
["Capture high-volume output [...]
+critical = true
+
+[[cases]]
+id = "final_review_and_git_artifacts_are_separate"
+group = "operations"
+description = "Outputs task-file commit artifacts after the passing Formal
Review was returned separately."
+phase = "completion output gate"
+ordinary_loop = true
+profile = "code-write"
+prompt = "The latest `$review-pr` Formal Review of a standalone local task
returned `Review Result: Mergeable`, and its complete formal report was already
returned as a separate artifact. The only task-owned tracked files are
`module-a/src/main/java/example/Foo.java` and
`module-a/src/test/java/example/FooTest.java`, neither contains unrelated
hunks, and no Git write is authorized. Return only the proposed commit message
and exact manual commit commands now."
+decision = "proceed"
+required_actions = ["propose_commit_message"]
+allowed_actions = ["inspect_local", "propose_commit_message"]
+forbidden_actions = ["edit_code", "edit_non_code", "mutate_git",
"mutate_remote"]
+required_reasons = ["review_passed", "git_read_only"]
+required_summary_terms = ["git commit --dry-run --only", "git commit --only",
"Foo.java", "FooTest.java"]
+forbidden_summary_terms = ["### Result", "### Evidence", "### Coverage"]
+policy_binding_profile = "code-write"
+policy_assertions = [{source = "change-completion", statements = ["After the
fenced `markdown` formal report, provide the proposed commit message in a
fenced `text` block and the Git commands in a fenced shell block labeled `bash`
or `sh`, both outside the review report.", "Generate `git commit --dry-run
--only -m <message> -- <exact-task-paths>` and `git commit --only -m <message>
-- <exact-task-paths>` for only task-owned files that contain no unrelated
hunks; list any task-created unt [...]
+critical = true
+
+[[cases]]
+id = "standalone_local_review_pr_artifact"
+group = "operations"
+description = "Reviews a standalone local candidate without requiring a public
PR and keeps Git artifacts outside the review."
+phase = "review scope and output gate"
+ordinary_loop = true
+profile = "review-pr"
+prompt = "The repository completion loop invoked Formal Review for a
standalone local candidate with an established task baseline and attributed
local delta but no public PR. The complete Code Correctness Review found no
blocker. Return the review artifact now; the outer workflow, not this reviewer,
will provide any commit message and Git commands."
+decision = "proceed"
+required_actions = []
+allowed_actions = []
+forbidden_actions = ["edit_code", "edit_non_code", "propose_commit_message",
"mutate_git", "mutate_remote"]
+required_reasons = []
+required_summary_terms = ["### Result", "Review Result: Mergeable", "###
Evidence", "### Coverage"]
+required_summary_term_groups = [["standalone", "no public PR"], ["baseline",
"established task-local delta", "attributed local candidate"]]
+forbidden_summary_terms = ["git commit"]
+policy_binding_profile = "review-pr"
+policy_assertions = [{source = "agents", statements = ["Complete reviews or
recommendations use Formal Review with `### Result`, one `Review Result`, `###
Evidence`, and `### Coverage`; Coverage identifies local-only changes."]},
{source = "review-pr", statements = ["For a standalone local candidate, use the
active task's original working-tree baseline plus only its attributed local
commits, index changes, and working-tree changes as the effective candidate.",
"Use the frozen task bounda [...]
+critical = true
diff --git a/.codex/harness/agents/policy-sources.toml
b/.codex/harness/agents/policy-sources.toml
index cb4364bf25d..624ff268e1b 100644
--- a/.codex/harness/agents/policy-sources.toml
+++ b/.codex/harness/agents/policy-sources.toml
@@ -70,7 +70,7 @@ id = "change-completion"
path = ".codex/context/change-completion.md"
kind = "context"
semantic = true
-references = ["AGENTS.md", ".codex/context/cross-cutting-skills.md",
".codex/skills/code-implementation/references/rules/non-regression.md",
".codex/skills/review-pr/SKILL.md"]
+references = ["AGENTS.md", ".codex/context/cross-cutting-skills.md",
".codex/skills/code-implementation/references/rules/non-regression.md",
".codex/skills/code-implementation/references/verification.md",
".codex/skills/review-pr/SKILL.md"]
[[sources]]
id = "policy-maintenance"
@@ -196,7 +196,7 @@ id = "review-pr"
path = ".codex/skills/review-pr/SKILL.md"
kind = "skill"
semantic = true
-references = ["AGENTS.md", "CODE_OF_CONDUCT.md",
".codex/skills/review-pr/references/evidence-access.md",
".codex/skills/review-pr/references/high-risk-review.md",
".codex/skills/review-pr/references/sql-parser-review.md",
".codex/skills/review-pr/references/review-corrections.md",
".codex/skills/code-implementation/references/rules/implementation.md",
".codex/skills/code-implementation/references/rules/testing.md",
".codex/skills/code-implementation/references/rules/artifact-removal-and [...]
+references = ["AGENTS.md", "CODE_OF_CONDUCT.md",
".codex/context/change-completion.md",
".codex/skills/review-pr/references/evidence-access.md",
".codex/skills/review-pr/references/high-risk-review.md",
".codex/skills/review-pr/references/sql-parser-review.md",
".codex/skills/review-pr/references/review-corrections.md",
".codex/skills/code-implementation/references/rules/implementation.md",
".codex/skills/code-implementation/references/rules/testing.md",
".codex/skills/code-implementatio [...]
[[sources]]
id = "review-pr-evidence"
diff --git a/.codex/skills/code-implementation/references/verification.md
b/.codex/skills/code-implementation/references/verification.md
index 4d51e4d3dd8..a8574d15971 100644
--- a/.codex/skills/code-implementation/references/verification.md
+++ b/.codex/skills/code-implementation/references/verification.md
@@ -31,10 +31,11 @@ Keep background unit tests under 60 seconds. Capture
high-volume output accordin
For every user-forbidden tool, API, assertion, or pattern, run a scoped final
search and report the command and result. Do not rely only on plan compliance.
-After the last file-changing action:
+After the last code-affecting action and before the pre-handoff Formal Review,
run both Spotless and Checkstyle for every applicable task-changed code file;
run Spotless for a documentation-only change when the configured formatter
governs that file.
-1. Run `./mvnw spotless:apply -Pcheck -T1C` for code or documentation changes.
-2. Run `./mvnw checkstyle:check -Pcheck -T1C` when production, test, or
project-rule files changed.
-3. Do not manually reformat afterward. Any later edit invalidates formatting
and requires the applicable checks again.
+1. Resolve the exact task-changed files governed by Spotless and their owning
Maven projects, then run `./mvnw -pl <explicit-owner>
-DspotlessFiles='<comma-separated exact absolute-path regular expressions>'
spotless:apply -Pcheck -T1C` for each smallest owner set. Construct each
regular expression from the current workspace and repository-relative task path
without hard-coding a local workspace path, and ensure it can match only an
allowlisted task file before running this file-modifying goal.
+2. For production or test Java changes, run `./mvnw -pl <explicit-owner>
-Dcheckstyle.includes='<comma-separated source-root-relative task files>'
-Dcheckstyle.includeResources=false -Dcheckstyle.includeTestResources=false
checkstyle:check -Pcheck -T1C` once per owning Maven project. For a changed
resource or project-rule file governed by Checkstyle, use its exact supported
resource filter; if the configured plugin cannot isolate that file, stop and
report the minimum unavoidable scope i [...]
+3. Treat a filtered command as passing only after its output and the
post-command task-delta inspection prove that every applicable task file was
selected, no non-allowlisted file was written, and the command exited
successfully. A successful command that matched no intended file is not
evidence.
+4. Do not manually reformat afterward. Any later edit invalidates the affected
Spotless and Checkstyle evidence and requires both applicable checks to run
again before another Formal Review.
Record every required command, exit code, decisive result, and remaining
unverified path. A required check with missing, incomparable, unstable, or
inconclusive evidence is not a pass.
diff --git a/.codex/skills/review-pr/SKILL.md b/.codex/skills/review-pr/SKILL.md
index a69f47464b0..bc67635fe07 100644
--- a/.codex/skills/review-pr/SKILL.md
+++ b/.codex/skills/review-pr/SKILL.md
@@ -236,6 +236,7 @@ readiness conclusion.
omission-prone review. It mechanically accounts for files, clusters, risk
axes, finding classifications, proof fields, and review passes; it does not
judge semantic correctness.
+- For a standalone local candidate with index or working-tree changes, provide
the same exact task-path file to both scripts with `--candidate-files <path>`.
The scripts resolve those paths from the computed merge-base through the
working tree, include matching untracked files, and fail when a listed path is
unchanged.
- Mutate one ledger sequentially; do not run ledger commands concurrently.
- Keep temporary ledger data private and remove only the exact ledger created
for the current review.
@@ -263,15 +264,12 @@ unclear.
## Local Candidate Scope
-- Use the latest public PR head plus authorized local commits, index changes,
- and working-tree changes as the effective candidate.
-- Verify with read-only Git that local `HEAD` equals or descends from the
public head.
-- If that relationship cannot be established, record the unavailable
effective-candidate lineage as an incomplete gap; if the refs prove divergence,
resolve the correct review basis before selecting a verdict.
-- Review from the public PR merge-base through the working tree. Scope is the
- union of GitHub files and the authorized local delta; exclude unrelated local
- changes.
+- First classify the local candidate as standalone or as targeting an existing
public PR.
+- For a standalone local candidate, use the active task's original
working-tree baseline plus only its attributed local commits, index changes,
and working-tree changes as the effective candidate. Use the frozen task
boundary and task-delta audit as authoritative scope, exclude unrelated local
changes, and do not require a public head, GitHub metadata, or public lineage.
+- For a local candidate targeting an existing PR, use the latest public PR
head plus authorized local commits, index changes, and working-tree changes as
the effective candidate. Verify with read-only Git that local `HEAD` equals or
descends from the public head; if that relationship cannot be established,
record the unavailable lineage as an incomplete gap, and if the refs prove
divergence, resolve the correct review basis before selecting a verdict.
+- Review a PR-backed local candidate from the public PR merge-base through the
working tree, use the union of GitHub files and the authorized local delta, and
exclude unrelated local changes.
- Apply Code Correctness Review through the canonical assessment, including
the same proof and completion gates, triggered high-risk criteria, convergence
loop, and Formal Decision Contract.
-- State in `### Coverage` that the effective candidate includes authorized
local changes, identify their scope separately from the public PR, and never
present them as the public PR state.
+- In `### Coverage`, identify whether the candidate is standalone or PR-backed
and record its baseline and attributed local delta. For a PR-backed candidate,
identify authorized local changes separately from the public PR and never
present them as the public PR state.
- Keep this Skill review-only. The outer active implementation loop fixes safe
in-scope findings and reruns Formal Review; scope expansion, architecture
choices, and high-risk actions return to their existing authorization gates.
This Skill never activates that loop.
## Multi-Round and Challenged Findings
@@ -288,9 +286,7 @@ latest commits, exposed by the previous fix, or missed in
the previous review.
## Output Contract
-Every result returned by this Skill must be exactly one fenced `markdown` block
-with no prose before or after it. The first non-empty line must be
-```` ```markdown ```` and the last non-empty line must be ```` ``` ````.
+Every standalone result returned by this Skill must be exactly one fenced
`markdown` block with no prose before or after it. When the repository
completion loop invokes Formal Review, this fenced block remains the complete
review artifact, but the outer workflow may append only the separately fenced
non-review handoff artifacts required by `.codex/context/change-completion.md`
outside it. The first non-empty line of the review artifact must be ````
```markdown ```` and its last non-empty [...]
Use the user's language for formal results. Draft GitHub-facing discussion
replies in English unless the user requests another language. Use stable
labels,
@@ -322,7 +318,7 @@ For each blocking issue include:
- `Required Change` for Change Request, or `Discussion Needed` for Needs
Discussion.
-Do not add patch-level changes after selecting Needs Discussion. Do not
include placeholder headings. In `### Coverage`, report the candidate type,
reviewed head, authoritative requirements and files accounted for, behavior
clusters, completed discovery lenses, unresolved gaps, and CI scope. For a
local candidate, identify authorized local commits, index changes, or
working-tree changes separately from the public PR state. In Code Correctness
Review, state that the result is code-scope o [...]
+Do not add patch-level changes after selecting Needs Discussion. Do not
include placeholder headings. In `### Coverage`, report the candidate type,
reviewed baseline or head, authoritative requirements and files accounted for,
behavior clusters, completed discovery lenses, unresolved gaps, and CI scope.
For a standalone local candidate, identify the task baseline and attributed
local delta; for a PR-backed candidate, identify authorized local commits,
index changes, or working-tree chang [...]
### PR Discussion Reply
diff --git a/.codex/skills/review-pr/scripts/build_review_inventory.py
b/.codex/skills/review-pr/scripts/build_review_inventory.py
index 63669278bbc..b9b3582ca71 100755
--- a/.codex/skills/review-pr/scripts/build_review_inventory.py
+++ b/.codex/skills/review-pr/scripts/build_review_inventory.py
@@ -26,7 +26,9 @@ import sys
from collections import defaultdict
from typing import Iterable
-from review_common import ChangedFile, categorize, compare_github_files,
final_paths, get_repo_root, parse_name_status, run_git
+from review_common import (
+ ChangedFile, categorize, compare_github_files, final_paths, get_repo_root,
parse_name_status, resolve_candidate_changes, run_git,
+)
MAX_ITEMS = 30
@@ -57,7 +59,9 @@ def build_inventory(args: argparse.Namespace) -> dict[str,
object]:
base_sha = run_git(["rev-parse", args.base_ref], repo_root).strip()
head_sha = run_git(["rev-parse", args.head_ref], repo_root).strip()
merge_base = run_git(["merge-base", args.base_ref, args.head_ref],
repo_root).strip()
- changed_files = parse_name_status(run_git(["diff", "--name-status",
f"{merge_base}..{args.head_ref}"], repo_root))
+ candidate_files = getattr(args, "candidate_files", None)
+ changed_files = resolve_candidate_changes(repo_root, merge_base,
candidate_files) if candidate_files else parse_name_status(
+ run_git(["diff", "--name-status", f"{merge_base}..{args.head_ref}"],
repo_root))
dirty_worktree = run_git(["status", "--short"], repo_root,
allow_empty=True).splitlines()
dirty_sample, dirty_truncated = limited(dirty_worktree)
previous_delta: list[ChangedFile] = []
@@ -73,6 +77,7 @@ def build_inventory(args: argparse.Namespace) -> dict[str,
object]:
"head_sha": head_sha,
"merge_base": merge_base,
"changed_file_count": len(changed_files),
+ "candidate_files": {"provided": bool(candidate_files)},
"github_files": compare_github_files(final_paths(changed_files),
args.github_files, MAX_ITEMS),
},
"dirty_worktree": {
@@ -155,6 +160,7 @@ def parse_args() -> argparse.Namespace:
parser.add_argument("--head-ref", required=True, help="PR head ref or SHA")
parser.add_argument("--previous-head", help="Previous reviewed PR head ref
or SHA")
parser.add_argument("--github-files", help="File containing GitHub changed
paths, one per line")
+ parser.add_argument("--candidate-files", help="Exact repository-relative
local candidate paths, one per line")
parser.add_argument("--format", choices=("markdown", "json"),
default="markdown", help="Output format")
return parser.parse_args()
diff --git a/.codex/skills/review-pr/scripts/review_common.py
b/.codex/skills/review-pr/scripts/review_common.py
index 8159afd1ef5..9a013ff461a 100644
--- a/.codex/skills/review-pr/scripts/review_common.py
+++ b/.codex/skills/review-pr/scripts/review_common.py
@@ -22,7 +22,7 @@ from __future__ import annotations
import subprocess
from dataclasses import dataclass
-from pathlib import Path
+from pathlib import Path, PurePosixPath
from typing import Any, Iterable
@@ -63,6 +63,30 @@ def parse_name_status(output: str) -> list[ChangedFile]:
return result
+def resolve_candidate_changes(repo_root: Path, baseline_ref: str,
candidate_files_path: str) -> list[ChangedFile]:
+ candidate_paths = [line.strip() for line in
Path(candidate_files_path).read_text(encoding="utf-8").splitlines() if
line.strip()]
+ if not candidate_paths:
+ raise RuntimeError("Candidate file list is empty")
+ if len(candidate_paths) != len(set(candidate_paths)):
+ raise RuntimeError("Candidate file list contains duplicate paths")
+ invalid_paths = [each for each in candidate_paths
+ if PurePosixPath(each).is_absolute() or "." == each or
".." in PurePosixPath(each).parts]
+ if invalid_paths:
+ raise RuntimeError(f"Candidate file list contains invalid
repository-relative paths: {', '.join(invalid_paths)}")
+ literal_paths = [f":(literal){each}" for each in candidate_paths]
+ tracked_changes = parse_name_status(run_git(["diff", "--name-status",
baseline_ref, "--", *literal_paths], repo_root))
+ untracked_paths = run_git(["ls-files", "--others", "--exclude-standard",
"--", *literal_paths],
+ repo_root, allow_empty=True).splitlines()
+ changed_files = [*tracked_changes, *(ChangedFile(status="A", path=each)
for each in untracked_paths if each)]
+ changed_by_path = {each.path: each for each in changed_files}
+ if len(changed_by_path) != len(changed_files):
+ raise RuntimeError("Candidate scope resolves the same final path more
than once")
+ unchanged_paths = [each for each in candidate_paths if each not in
changed_by_path]
+ if unchanged_paths:
+ raise RuntimeError(f"Candidate file list contains paths that are not
changed from the baseline: {', '.join(unchanged_paths)}")
+ return [changed_by_path[each] for each in candidate_paths]
+
+
def categorize(path: str) -> str:
if "RELEASE-NOTES.md" == path:
return "release-notes"
diff --git a/.codex/skills/review-pr/scripts/review_ledger.py
b/.codex/skills/review-pr/scripts/review_ledger.py
index 43ad024f691..6361cfa49be 100755
--- a/.codex/skills/review-pr/scripts/review_ledger.py
+++ b/.codex/skills/review-pr/scripts/review_ledger.py
@@ -32,7 +32,9 @@ from collections import Counter
from pathlib import Path
from typing import Any, Iterable
-from review_common import categorize, compare_github_files, final_paths,
get_repo_root, parse_name_status, run_git
+from review_common import (
+ categorize, compare_github_files, final_paths, get_repo_root,
parse_name_status, resolve_candidate_changes, run_git,
+)
LEDGER_KIND = "review-pr-coverage-ledger"
@@ -173,7 +175,9 @@ def cmd_init(args: argparse.Namespace) -> int:
base_sha = run_git(["rev-parse", args.base_ref], repo_root).strip()
head_sha = run_git(["rev-parse", args.head_ref], repo_root).strip()
merge_base = run_git(["merge-base", args.base_ref, args.head_ref],
repo_root).strip()
- changed_files = parse_name_status(run_git(["diff", "--name-status",
f"{merge_base}..{args.head_ref}"], repo_root))
+ candidate_files = getattr(args, "candidate_files", None)
+ changed_files = resolve_candidate_changes(repo_root, merge_base,
candidate_files) if candidate_files else parse_name_status(
+ run_git(["diff", "--name-status", f"{merge_base}..{args.head_ref}"],
repo_root))
ledger_dir = create_ledger_dir(repo_root, args.pr, head_sha)
ledger_file = ledger_dir / LEDGER_FILE_NAME
now = int(time.time())
@@ -192,6 +196,7 @@ def cmd_init(args: argparse.Namespace) -> int:
"head_sha": head_sha,
"merge_base": merge_base,
"changed_file_count": len(changed_files),
+ "candidate_files": {"provided": bool(candidate_files)},
"github_files": compare_github_files(final_paths(changed_files),
args.github_files),
},
"files": [{
@@ -447,6 +452,7 @@ def build_parser() -> argparse.ArgumentParser:
init.add_argument("--base-ref", required=True, help="Base ref used to
compute merge-base")
init.add_argument("--head-ref", required=True, help="PR head ref")
init.add_argument("--github-files", help="File containing GitHub changed
paths")
+ init.add_argument("--candidate-files", help="Exact repository-relative
local candidate paths, one per line")
init.set_defaults(func=cmd_init)
mark_file = subparsers.add_parser("mark-file", help="Set one authoritative
file's coverage state")
mark_file.add_argument("--ledger", required=True, help="Ledger file or
directory")
diff --git a/.codex/skills/review-pr/tests/test_build_review_inventory.py
b/.codex/skills/review-pr/tests/test_build_review_inventory.py
index 347a6ebfa12..7786eb3fc9c 100644
--- a/.codex/skills/review-pr/tests/test_build_review_inventory.py
+++ b/.codex/skills/review-pr/tests/test_build_review_inventory.py
@@ -16,6 +16,7 @@
#
import sys
+import tempfile
import unittest
from argparse import Namespace
from pathlib import Path
@@ -27,6 +28,7 @@ if str(SCRIPTS_DIR) not in sys.path:
sys.path.insert(0, str(SCRIPTS_DIR))
import build_review_inventory as inventory
+import review_common as common
class BuildReviewInventoryTest(unittest.TestCase):
@@ -41,7 +43,7 @@ class BuildReviewInventoryTest(unittest.TestCase):
"M\tsrc/main/java/Foo.java\nA\tdocs/guide.md\n",
" M unrelated.java\n",
]
- args = Namespace(base_ref="base", head_ref="head", previous_head=None,
github_files=None)
+ args = Namespace(base_ref="base", head_ref="head", previous_head=None,
github_files=None, candidate_files=None)
actual = inventory.build_inventory(args)
self.assertEqual("merge-base", actual["scope"]["merge_base"])
self.assertEqual(2, actual["scope"]["changed_file_count"])
@@ -49,6 +51,38 @@ class BuildReviewInventoryTest(unittest.TestCase):
self.assertEqual("src/main/java/Foo.java",
actual["changed_files_by_category"]["production-java"][0]["path"])
run_git.assert_any_call(["diff", "--name-status", "merge-base..head"],
Path("/repo"))
+ @patch.object(inventory, "resolve_candidate_changes")
+ @patch.object(inventory, "run_git")
+ @patch.object(inventory, "get_repo_root", return_value=Path("/repo"))
+ def test_build_inventory_uses_exact_candidate_files(self, _, run_git,
resolve_candidate_changes):
+ run_git.side_effect = ["base-sha\n", "head-sha\n", "merge-base\n", " M
unrelated.java\n"]
+ resolve_candidate_changes.return_value =
[common.ChangedFile(status="M", path="AGENTS.md")]
+ args = Namespace(base_ref="base", head_ref="head", previous_head=None,
github_files=None, candidate_files="candidate.txt")
+ actual = inventory.build_inventory(args)
+ self.assertEqual(1, actual["scope"]["changed_file_count"])
+ self.assertEqual({"provided": True},
actual["scope"]["candidate_files"])
+ self.assertEqual("AGENTS.md",
actual["changed_files_by_category"]["docs"][0]["path"])
+ resolve_candidate_changes.assert_called_once_with(Path("/repo"),
"merge-base", "candidate.txt")
+
+ def test_resolve_candidate_changes_includes_untracked_files(self):
+ with tempfile.TemporaryDirectory() as temp_dir:
+ candidate_file = Path(temp_dir) / "candidate.txt"
+ candidate_file.write_text("AGENTS.md\n.codex/new.toml\n",
encoding="utf-8")
+ with patch.object(common, "run_git",
side_effect=["M\tAGENTS.md\n", ".codex/new.toml\n"]) as run_git:
+ actual = common.resolve_candidate_changes(Path("/repo"),
"baseline", str(candidate_file))
+ self.assertEqual([common.ChangedFile(status="M", path="AGENTS.md"),
+ common.ChangedFile(status="A",
path=".codex/new.toml")], actual)
+ literal_paths = [":(literal)AGENTS.md", ":(literal).codex/new.toml"]
+ run_git.assert_any_call(["diff", "--name-status", "baseline", "--",
*literal_paths], Path("/repo"))
+ run_git.assert_any_call(["ls-files", "--others", "--exclude-standard",
"--", *literal_paths], Path("/repo"), allow_empty=True)
+
+ def test_resolve_candidate_changes_rejects_unchanged_paths(self):
+ with tempfile.TemporaryDirectory() as temp_dir:
+ candidate_file = Path(temp_dir) / "candidate.txt"
+ candidate_file.write_text("AGENTS.md\n", encoding="utf-8")
+ with patch.object(common, "run_git", side_effect=["", ""]),
self.assertRaisesRegex(RuntimeError, "not changed"):
+ common.resolve_candidate_changes(Path("/repo"), "baseline",
str(candidate_file))
+
if __name__ == "__main__":
unittest.main()
diff --git a/.codex/skills/review-pr/tests/test_review_ledger.py
b/.codex/skills/review-pr/tests/test_review_ledger.py
index 210089862c9..8e22d1b7d96 100644
--- a/.codex/skills/review-pr/tests/test_review_ledger.py
+++ b/.codex/skills/review-pr/tests/test_review_ledger.py
@@ -29,6 +29,7 @@ SCRIPTS_DIR = Path(__file__).resolve().parents[1] / "scripts"
if str(SCRIPTS_DIR) not in sys.path:
sys.path.insert(0, str(SCRIPTS_DIR))
+import review_common as common
import review_ledger as ledger
@@ -89,6 +90,21 @@ class ReviewLedgerTest(unittest.TestCase):
errors = ledger.validate_ledger(actual_ledger)
self.assertIn("GitHub file list does not match local triple-dot
scope", errors)
+ def test_init_uses_exact_candidate_files(self):
+ changed_files = [common.ChangedFile(status="M", path="AGENTS.md")]
+ args = Namespace(repo_root=".", pr="local-task", base_ref="base",
head_ref="head", github_files=None,
+ candidate_files="candidate.txt")
+ with patch.object(ledger, "get_repo_root",
return_value=Path("/repo")), \
+ patch.object(ledger, "run_git", side_effect=["base-sha\n",
"head-sha\n", "merge-base\n"]), \
+ patch.object(ledger, "resolve_candidate_changes",
return_value=changed_files) as resolve_candidate_changes, \
+ patch.object(ledger, "create_ledger_dir",
return_value=Path("ledger")), \
+ patch.object(ledger, "write_ledger") as write_ledger,
redirect_stdout(StringIO()):
+ self.assertEqual(0, ledger.cmd_init(args))
+ actual = write_ledger.call_args.args[1]
+ self.assertEqual({"provided": True},
actual["scope"]["candidate_files"])
+ self.assertEqual("AGENTS.md", actual["files"][0]["path"])
+ resolve_candidate_changes.assert_called_once_with(Path("/repo"),
"merge-base", "candidate.txt")
+
def test_confirmed_findings_must_have_distinct_fix_boundaries(self):
actual_ledger = self.create_valid_ledger()
actual_ledger["findings"] = [self.create_confirmed_finding("F1"),
self.create_confirmed_finding("F2")]
diff --git a/AGENTS.md b/AGENTS.md
index 0487613cae2..461c5335962 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -172,9 +172,11 @@ Optional cross-cutting Skills remain governed by
`.codex/context/cross-cutting-s
## Change Completion Gate
-A pre-handoff review is required for every authorized change, build,
implement, or fix request, excluding a standalone restoration or rollback
governed by its existing gates. After the last write, audit the task delta,
complete the applicable verification, and review the effective local candidate
against the same code-correctness gates used by `$review-pr`: root-cause and
fix mapping, affected behavior, side effects and regressions, contracts and
architecture, test validity, and adversar [...]
+Every authorized change, build, implement, or fix request requires a
pre-handoff `$review-pr` Formal Review of the effective local candidate, except
a governed standalone restoration or rollback. After the last write, audit the
task delta and complete verification. Review root-cause and fix mapping,
affected behavior, side effects and regressions, contracts and architecture,
test validity, and adversarial cases.
-Fix every safe in-scope required finding, rerun every invalidated check, and
repeat the applicable review. If a finding requires scope expansion, an
unresolved architecture choice, or a high-risk action, stop at its existing
authorization gate instead of fixing it automatically. Hand off or propose a
commit message only after one complete applicable review pass finds zero new
required issues.
+Fix every safe in-scope required finding, rerun invalidated checks, and invoke
`$review-pr` again. Stop at the applicable gate for scope expansion, unresolved
architecture, or a high-risk action. Hand off or propose commit artifacts only
after the latest complete Formal Review returns `Review Result: Mergeable`.
+
+After Mergeable, output the formal report, commit message, and exact task-file
`git commit --dry-run --only` and `git commit --only` commands in individual
fenced blocks.
## Functional and Performance Non-Regression Gate
