| name | review-single-pr |
| description | Review one specified GitHub pull request in this tgoskits repository. Use when the user names a PR number or URL and asks to review, re-review, compare with Linux/POSIX/RFC/VirtIO semantics, check duplicate functionality or related open PRs, verify required tests and their placement/discovery/execution, validate Starry or ArceOS app/tool workflows that CI may miss, repair safe merge conflicts, run focused validation, leave Chinese inline review comments, approve, request changes, or select and assign recommended reviewers from .github/MAINTAINERS.md after review. |
Review Single PR
Normative Use
This skill is a normative review specification, not a suggestion list. When it triggers, read the entire SKILL.md before deciding the review outcome, then follow every applicable requirement unless a higher-priority system or developer instruction conflicts.
Before judging code quality, maintainability, or merge readiness, fully read book/guideline/code-quality.md and treat it as the mandatory baseline for author-side implementation and reviewer-side assessment. Read additional domain guidelines only when the changed behavior is in their scope. In particular, fully read book/guideline/starry/syscall.md whenever the PR changes or claims to change user-visible StarryOS syscall/Linux ABI semantics. This is a semantic trigger: it also applies when the diff is in task, VFS, namespace, signal, socket, credential, memory-management, or another helper rather than the syscall directory. When a domain guideline is inapplicable, record the concrete reason in the review checklist. If context is compacted, resumed from a summary, or an applicable guideline cannot be recalled confidently, re-read that complete guideline before continuing; do not rely on memory or a previous partial read.
Do not submit APPROVE, REQUEST_CHANGES, a no-submit summary, or any PR-facing comment from only the frontmatter, title, partial sections, memory, or a previous review. If context or time pressure prevents reading the full skill, state that limitation and do not claim a complete review-single-pr review.
After reading the full skill, book/guideline/code-quality.md, and every applicable domain guideline, create a review todo/checklist before deciding or submitting any outcome. The checklist must cover all applicable merge-readiness requirements from this skill, including PR metadata and intake, review threads and CI, worktree setup, merge-conflict handling when applicable, review focus, baseline code quality, domain-guideline applicability and compliance, required test coverage and test placement/discovery, duplicate and overlap analysis, validation, blocking findings, submission rules, recommended reviewer assignment, and cleanup. Verify each item one by one as satisfied, not applicable with a concrete reason, or blocking with evidence; do not collapse the checklist into a generic "tests passed" statement.
When requirements overlap, apply the stricter rule. If skipping a requirement is necessary because it is inapplicable or impossible, record the concrete reason and evidence in the review body or user summary.
Offline Benchmark Mode
Use this mode only when the skill is invoked with the exact offline-benchmark argument and the
repository contains .agent-review-context/reviewer.md. The context file is the authoritative
environment and output contract for this mode. If either condition is absent, follow the normal
online workflow in the rest of this skill.
In offline benchmark mode, treat the committed change from bench-base to HEAD as the one change
under review. Read the complete skill, AGENTS.md, book/guideline/code-quality.md, every domain
guideline applicable to the changed semantics, the offline contract, and the supplied output schema
before judging the change. Apply the review-focus,
test-quality, blocking-finding, hardware/ABI, security/soundness, maintainability, and documentation
requirements from this skill to the local diff and relevant in-repository context.
The benchmark repository intentionally has no live PR identity. Mark PR metadata and intake, review
threads, remote CI, related-open-PR searches, worktree creation, merge-conflict repair, network
semantic-source research, command execution for validation, GitHub review submission, reviewer
assignment, and remote cleanup as not applicable. Do not infer a PR number, inspect paths outside
the repository, access the network or GitHub, modify files, create commits or branches, or run
builds and tests. Use only read-only repository inspection and the Git history/diff commands allowed
by the harness.
Return only the JSON object required by .agent-review-context/review.schema.json. Report findings
introduced by bench-base..HEAD, anchor each finding to a changed line on the HEAD side, and use
an empty findings array when no actionable issue exists. Do not submit or draft GitHub-facing
review text in this mode.
Goal
Perform a focused review of exactly one PR, using an isolated worktree and local validation before submitting a GitHub review. The review must also decide whether the PR duplicates existing base-branch functionality or overlaps with other open PRs. After the review decision is submitted, assign suitable human reviewers from .github/MAINTAINERS.md when the PR still needs domain follow-up. The normal outcome is either APPROVE when no blocking issue remains, or REQUEST_CHANGES with Chinese inline comments when the PR has correctness, standards, duplication, test, or CI coverage problems.
This skill is the authoritative single-PR workflow used by review-open-prs: do not fully review all open PRs, but always inspect enough related open PR context to classify duplicate, overlapping, superseded, or conflicting work.
System Skill Priority
For GitHub operations, follow the system GitHub plugin skills first:
- Use
github:github as the default source for repository orientation, PR metadata, patch inspection, comments, labels, reactions, and connector-first behavior.
- Use
github:gh-address-comments when unresolved review threads, requested changes, inline review context, line anchors, or thread resolution state matter.
- Use
github:gh-fix-ci when the review depends on failing GitHub Actions checks or logs.
Prefer the GitHub MCP/connector for structured PR data. Use local git for fetch, detached worktrees, local diffs, and validation. Use gh only for connector gaps such as current-branch PR discovery, GraphQL review-thread state, Actions logs, or review submission when the connector cannot preserve the required inline review anchors.
Intake
- Follow
github:github to resolve repository identity, current user, PR number or URL, title, body, author, base/head refs, headRefOid, draft state, merge state, changed files, patch context, commit messages, existing reviews/comments, and available checks.
- If the PR is authored by the current GitHub user, say so and ask before submitting a formal review.
- Include draft PRs unless the user explicitly says to skip drafts.
- Keep connector state and local checkout state aligned before creating the worktree.
Fallback only when the GitHub MCP/connector cannot provide the needed data:
gh auth status
gh repo view --json nameWithOwner,defaultBranchRef,url
gh pr view <pr> --json number,title,body,author,baseRefName,headRefName,headRefOid,headRepositoryOwner,isDraft,mergeStateStatus,maintainerCanModify,reviewDecision,url,commits
gh pr diff <pr> --patch --color=never
gh pr checks <pr> --watch=false
gh api "repos/<owner>/<repo>/pulls/<pr>/reviews?per_page=100"
gh api "repos/<owner>/<repo>/pulls/<pr>/files?per_page=100"
Review Threads And CI
For prior requested changes, unresolved review conversations, inline review locations, or resolution state, follow github:gh-address-comments: use the GitHub app/MCP for PR metadata and patch context, and use its GraphQL-based gh fallback only when thread-level fields such as isResolved, isOutdated, diffSide, or exact line anchors are required. Do not treat flat connector comments as a complete representation of review-thread state.
When using GraphQL directly, request reviewThreads { nodes { id isResolved isOutdated path line diffSide comments(first: 100) { nodes { author { login } body createdAt } } } }. Detached worktrees cannot rely on current-branch PR inference, so pass <owner>, <repo>, and <pr> explicitly to helpers.
Always inspect unresolved review conversations from previous reviews. If the concrete issue is fixed in the current PR head, resolve the conversation before finishing the review. Keep threads open when the fix is partial, the test is not wired into the runner, or the comment is still behaviorally valid. Resolving old threads does not imply approval if new blocking issues remain.
Resolve fixed conversations with the review-thread API, then fetch threads again and confirm every resolved thread reports isResolved=true:
gh api graphql \
-f query='mutation($threadId:ID!){resolveReviewThread(input:{threadId:$threadId}){thread{id isResolved}}}' \
-f threadId=<thread-id>
For failing, cancelled, missing, or suspicious GitHub Actions checks, follow github:gh-fix-ci: use the GitHub app/MCP for PR context and use gh for Actions check/log inspection because the connector does not expose that workflow end to end. Remote CI is evidence, not a substitute for local review and targeted validation.
When a reviewer or page summary claims CI is "all skipped", or when the GitHub UI and API summaries appear inconsistent, verify the current PR head directly before drawing conclusions:
- Resolve the current PR head SHA (
headRefOid / head.sha) and query checks for that exact SHA, not an older reviewed commit or a stale merge ref.
- Prefer GraphQL
statusCheckRollup and check suites/check runs, because they report GitHub Actions check runs and their aggregate state:
query($owner:String!, $repo:String!, $sha:GitObjectID!) {
repository(owner:$owner, name:$repo) {
object(oid:$sha) { ... on Commit {
checkSuites(first:20) { nodes {
status conclusion
workflowRun { databaseId url workflow { name } }
checkRuns(first:100) { nodes { name status conclusion } }
} }
statusCheckRollup { state contexts(first:100) { nodes {
__typename
... on CheckRun { name status conclusion detailsUrl }
... on StatusContext { context state targetUrl }
} } }
} }
}
}
- REST fallback: query Actions check runs and jobs by the same head SHA, then count conclusions instead of relying on a single UI label:
gh api "repos/<owner>/<repo>/commits/<head-sha>/check-runs?per_page=100"
gh api "repos/<owner>/<repo>/actions/runs?head_sha=<head-sha>&per_page=100"
gh api "repos/<owner>/<repo>/actions/runs/<run-id>/jobs?per_page=100"
- Do not use
GET /repos/<owner>/<repo>/commits/<sha>/status by itself to decide Actions CI state. That endpoint reports classic commit statuses only; it may return pending with an empty statuses array even when GitHub Actions check runs and the workflow have already succeeded.
- Distinguish expected skipped matrix jobs from a truly skipped workflow. In this repository, mutually exclusive
run_host / run_container jobs, branch-restricted publish jobs, or path-filtered jobs may be skipped while sibling jobs in the same workflow are success; that is not "all CI skipped". Summarize the evidence with counts such as success=N, skipped=M, failure=0 and name the relevant successful or skipped checks.
- Treat skipped checks as suspicious only when the PR changed the surface that the skipped check is supposed to cover, when path filters skipped required coverage, or when every relevant check for the changed app/test/architecture was skipped. Otherwise, record them as expected CI matrix/path-filter behavior.
Always inspect CI failures before submitting the review:
- Fetch check summaries and enough logs to classify each non-passing required check:
gh pr checks <pr> --repo <owner>/<repo> --watch=false
gh run view <run-id> --repo <owner>/<repo> --log-failed
- Decide whether each CI failure is caused by this PR's changed surface, a likely unrelated pre-existing/infrastructure failure, or unclear.
- Treat CI as PR-related when the failing job exercises files, crates, cases, commands, platforms, or behavior changed by the PR; when the failure reproduces locally on the PR head but not on base; or when the new/modified tests, configs, or workflow steps cause the failure, hang, skip, or timeout.
- Treat CI as unrelated only when there is concrete evidence: the failure is outside the changed surface, known flaky/infrastructure behavior, already fails on base, or is tracked by an existing issue. Do not mark a failure unrelated merely because local focused validation passed.
- For unrelated CI failures, state that in the review body with the failing check name, observed failure, and why it is unrelated to this PR. Search for an existing issue before finishing. Use multiple searches keyed by the workflow/job name, distinctive error text, runner/platform, affected case, or failing command; do not rely on only one broad query:
gh issue list --repo <owner>/<repo> --state open --search '<workflow or job name>'
gh issue list --repo <owner>/<repo> --state open --search '<distinctive error excerpt>'
gh issue list --repo <owner>/<repo> --state open --search '<runner platform, case, or command>'
Inspect plausible matches before deciding whether they are suitable:
gh issue view <issue-number> --repo <owner>/<repo> --comments
A suitable issue tracks the same workflow/job, same distinctive failure mode, or same infrastructure/component breakage. If a suitable open issue exists, update it before finishing: add a neutral comment with the current PR number/URL, head SHA, check/run URL, failing job and step, representative log excerpt, why the failure appears unrelated to this PR, and any reproduction, rerun, or base-branch evidence. If the issue title/body is stale or too vague and repository permissions allow, update the title/body while preserving previous context; otherwise use a new issue comment.
gh issue comment <issue-number> --repo <owner>/<repo> --body-file issue-update.md
gh issue edit <issue-number> --repo <owner>/<repo> --title '<updated neutral title>' --body-file issue.md
If no suitable open issue exists, create one with a neutral title and body describing the CI job, PR where it was observed, representative log excerpt, why it appears unrelated, and any reproduction or rerun evidence:
gh issue create --repo <owner>/<repo> --title '<neutral CI issue title>' --body-file issue.md
Link the existing, updated, or newly-created issue in the review body. Do not create duplicate issues.
- For PR-related CI failures, submit
REQUEST_CHANGES. The review body and any inline comment must explain the failing check, the concrete failure mode, why it belongs to this PR, and the expected fix direction.
- When causality is unclear after reasonable log inspection, do not approve on CI alone. Either request changes with the concrete uncertainty and next debugging direction, or mark the review blocked/no-submit if the user asked for investigation only.
Worktree
Fetch the PR and base, then review in a detached worktree:
repo_root="$(git rev-parse --show-toplevel)"
repo_parent="$(dirname "$repo_root")"
review_wt="$repo_parent/$(basename "$repo_root")-review-pr<pr>"
git fetch origin '+refs/pull/<pr>/head:refs/remotes/origin/pr/<pr>' '+refs/heads/*:refs/remotes/origin/*'
git worktree add --detach "$review_wt" origin/pr/<pr>
If the worktree already exists, reuse it only when clean and at the current PR head:
git -C "$review_wt" status --short
git -C "$review_wt" rev-parse HEAD
git rev-parse refs/remotes/origin/pr/<pr>
If it is stale and clean, update it non-destructively to the fetched PR head. If it has local changes, create a fresh worktree path or ask how to proceed. Do not modify or revert the user's main worktree while reviewing.
Never review multiple StarryOS QEMU cases in the same checkout at the same time. Use separate worktrees for parallel PR review.
Merge Conflicts
Handle merge conflicts in either of these cases: the user explicitly asks for conflict handling, or this review has no blocking findings and would otherwise be APPROVE while the current PR metadata says mergeStateStatus=DIRTY and maintainerCanModify=true. Do not submit or reaffirm approval before the conflict repair is pushed and re-validated against the new head.
First refresh and classify the conflict and approval state:
gh pr view <pr> --json number,baseRefName,headRefName,headRepositoryOwner,headRefOid,mergeStateStatus,maintainerCanModify,reviewDecision,reviews
gh api "repos/<owner>/<repo>/pulls/<pr>/reviews?per_page=100"
-
reviewDecision=APPROVED is the current aggregate approval state. Historical APPROVED review records are useful context, but do not by themselves mean the PR is currently approved; if aggregate approval is empty, CHANGES_REQUESTED, or review threads remain unresolved, treat conflict repair as a no-submit dry run unless the user specifically asked to push a repair.
-
If mergeStateStatus=UNKNOWN, refresh or wait and query again before acting. Do not infer a current conflict from stale search results.
-
If mergeStateStatus=DIRTY and maintainerCanModify=false, do not repair the branch. When conflict handling was explicitly requested, submit REQUEST_CHANGES explaining that the branch conflicts with base and maintainers cannot push a fix; ask the author to merge/rebase latest base and suggest enabling "Allow edits by maintainers". Otherwise, include the conflict limitation in the review body or user summary.
-
If mergeStateStatus=DIRTY and maintainerCanModify=true, create a separate conflict worktree and verify the contributor branch still matches headRefOid before doing pushable work:
conflict_wt="$repo_parent/$(basename "$repo_root")-conflict-pr<pr>"
git fetch origin '+refs/pull/<pr>/head:refs/remotes/origin/pr/<pr>' '+refs/heads/<base>:refs/remotes/origin/<base>'
git ls-remote "https://github.com/<head-owner>/<repo>.git" "refs/heads/<headRefName>"
git worktree add --detach "$conflict_wt" origin/pr/<pr>
git -C "$conflict_wt" merge --no-ff --no-commit "origin/<base>"
git -C "$conflict_wt" diff --name-only --diff-filter=U
-
In this detached conflict worktree, conflict marker HEAD / stage 2 / "ours" is the PR branch, and origin/<base> / stage 3 / "theirs" is current base. Use git show :1:<path>, git show :2:<path>, and git show :3:<path> when the ancestor, PR side, or base side is unclear.
-
Resolve conflicts semantically according to PR intent and current base behavior. Do not merely keep both sides, and do not resurrect APIs or layouts that base already replaced. Port the PR feature onto the new base abstraction, then keep independent additions from both sides when they do not conflict.
-
PR 837 is the reference example for this rule: the PR added /proc/kallsyms, while base had replaced the old SeqFile pattern with SeqObject plus SpecialFsFile::new_regular_with_perm. The correct repair was to keep the kallsyms feature but express it with the current base API, while also keeping independent base/PR additions such as ktracepoint plus ksym and .tracepoint plus .kallsyms.
-
Before committing the repair, run formatting, marker checks, diff hygiene, and focused validation for the changed surface:
cargo fmt
rg -n '<<<<<<<|=======|>>>>>>>' <conflicted-files>
git -C "$conflict_wt" diff --check
<targeted cargo xtask/cargo test/cargo clippy commands>
git -C "$conflict_wt" add <resolved-files>
git -C "$conflict_wt" commit
-
Before pushing a repaired conflict branch, refresh the PR and confirm the local merge commit's first parent equals the current remote headRefOid; also re-check the fork branch with git ls-remote. If the remote head changed, stop and re-review instead of pushing.
-
Push repaired fork branches with a normal non-force push to the PR head owner and branch, for example git push https://github.com/<head-owner>/<repo>.git HEAD:<headRefName>. Never force-push a contributor branch.
-
After pushing conflict repairs, refresh PR status, update the review worktree to the new head, and rerun the targeted validation that supports approval. Submit APPROVE only if the repaired head still has no blocking findings; otherwise submit REQUEST_CHANGES with the remaining conflict or validation problem.
-
BLOCKED or UNSTABLE may remain because of CI or reviews even after conflicts are gone; do not treat that alone as failed conflict repair.
-
If you performed only a conflict dry run or process exercise, do not push or submit a review. Record the PR number, approval-state nuance, conflicted files, semantic resolution, validation commands/results, and that no GitHub branch was changed; then abort/remove the conflict worktree unless the diagnostics must be kept.
Review Focus
Review the PR against its stated intent, the current base branch, existing project patterns, and relevant external semantics. Understand the implementation logic, not just whether tests pass:
- POSIX/Linux behavior for syscalls, process/session/signal semantics, filesystem errors, sockets, IPv4/IPv6, and
/proc.
- RFC or Linux behavior for networking details such as IPv6 NDP, IPv4-mapped IPv6, dual-stack listeners, route/listen conflicts, and errno behavior.
- VirtIO, PCI, DMA, MMIO, IRQ, and ownership rules for driver changes.
- Axvisor config semantics for
entry_point, kernel_load_addr, memory_regions, map_type, and guest image layout.
starry-test-suit rules when StarryOS test cases or qemu-*.toml files change.
cross-kernel-driver architecture rules when portable driver crates or driver glue change.
For any change that affects or claims to affect user-visible StarryOS syscall/Linux ABI semantics, fully read and apply book/guideline/starry/syscall.md before judging correctness. Follow its evidence hierarchy and comparison workflow even when the changed line is outside the syscall directory. The review must trace indirect helper changes back to every affected syscall entry and must record the Linux version or commit used when behavior is version-dependent.
Review Lenses And Finding Discipline
Review is recall-first: prefer finding every real defect in the changed surface over producing a short or polished review. Do not invent issues, but do not dismiss a plausible in-scope defect with "looks fine"; construct the concrete input, interleaving, device state, guest config, or test-run path that would trigger it, or explain why that scenario is impossible.
Apply these TGOSKits review lenses. A lens runs unless the changed paths and semantics provably contain nothing in its remit:
- Maintainability: process and change shape, commit hygiene, focused PR scope, crate/module boundaries, naming, visibility, comments, and whether the next maintainer can understand the code without archaeology.
- Correctness: runtime behavior under normal, error, concurrent, and hot paths; edge cases such as off-by-one errors, reachable
unwrap/expect/panic, integer overflow, wrong predicates, dropped guards, lost wakeups, resource leaks, and tests that prove the behavior.
- Security/Soundness:
unsafe contracts, pointer provenance, aliasing, user-memory access, trust-boundary validation, privilege checks, TOCTOU, use-after-free, and invariants relied on by untouched unsafe blocks or unsafe trait impls.
- Hardware/ABI: assembly, target JSONs, trap/context layout, SMP/boot handoff, MMIO/DMA/IRQ ownership, cache/coherency assumptions, VirtIO/PCI contracts, device-tree/config layout, and architecture-specific calling or alignment rules.
- Documentation/User-Facing Compatibility: user-facing docs, runbooks, app workflows, test-suit guides, compatibility notes, syscall/kernel parameter descriptions, and whether documentation changes stay current with behavior visible to StarryOS, ArceOS, Axvisor, or users.
For each candidate finding, identify the natural owning lens and avoid duplicating the same investigation under every lens. If several symptoms share one root cause or one fix, keep each symptom at its own actionable location but explain the shared fix once and refer the related comments to it.
Every submitted finding, inline comment, or body-only blocker must include:
- grounding: the project rule, external standard, observed failure, or plain-language defect class, such as "Off by one", "Use after free", or "wrong errno";
- severity: whether it is blocking, major but non-blocking, minor, or nit-level;
- problem: the concrete behavior or maintainability failure and the scenario that exposes it;
- fix direction: the expected repair, not just a complaint;
- evidence: changed line, code path, validation output, CI log, external reference, or reasoning proof.
Before submitting, verify the load-bearing premises of every finding. Re-read the cited code and, when the finding depends on outside behavior, check an authoritative source such as Linux/POSIX/RFC/VirtIO specs, Rust reference/docs, hardware manuals, or existing TGOSKits runner semantics. Keep uncertain but plausible findings only when the uncertainty is explicit; retract findings whose premise is confidently false instead of posting stale or speculative feedback.
For bug fixes (修复 bug), require a regression or reproduction test that fails on the unfixed behavior and passes only after the fix unless concrete evidence shows the environment makes such a test impossible. The reviewer must verify this from the test code, author-provided red/green evidence, or local red/green validation when practical. If the PR fixes a bug but lacks a post-fix-only regression test and no concrete impossibility is documented, treat that as blocking. For raw syscall fixes, prefer direct syscall(SYS_...) coverage when libc wrappers could mask return values or errno.
For any PR that adds behavior, changes semantics, fixes a bug, or claims coverage for a newly exposed path, require tests at the correct project layer unless the PR is clearly documentation-only or the review records a concrete reason tests are impossible. Verify that the tests are not merely present in the diff: they must be in the expected suite or wrapper, discovered by the project runner, built or installed into the runtime image when applicable, selected by the documented command, and capable of failing when the behavior regresses. Treat misplaced tests, orphan assets, tests hidden behind opt-in/manual-only paths, or tests that CI/runner silently skips as missing coverage.
For PRs that add StarryOS app support, separate operator-facing app scenarios from CI-oriented semantic coverage:
- App-level smoke, demo, rootfs preparation, board/QEMU run scripts, and long-running or opt-in workflows belong under
apps/starry/<app-or-scenario>/, following apps/starry/README.md.
- Kernel ABI, syscall, filesystem, process, networking, or other bugfix coverage exposed while enabling the app belongs under
test-suit/starryos/<case> or the closest existing grouped wrapper, such as a qemu/system/<subcase> grouped C subcase.
- If the PR adds a syscall or changes syscall semantics for the app, require a minimal test-suit syscall/regression test that exercises the syscall surface directly; an app smoke passing is not enough.
- If the PR fixes a bug found through the app, require a test-suit bugfix/regression test that reproduces the bug without depending on the full app workflow whenever practical; keep the app scenario in
apps/starry as integration evidence.
- Do not approve app-support PRs that put app workflows only into
test-suit/starryos, or that hide syscall/bugfix coverage only inside apps/starry demos.
- If the PR adds or changes an app-oriented Starry QEMU case under either
apps/starry or test-suit/starryos, run the actual documented app command or exact cargo xtask starry test qemu ... -c <case> path in QEMU for at least the changed/claimed architecture. For multi-arch qemu-*.toml additions, run the architecture most likely to fail from CI or PR history; if any newly added required architecture is already failing in CI, reproduce or classify that architecture before approval.
- Do not approve when the app/test cannot be run as described by the PR, when its success depends on an unavailable or unstable external service without a controlled fallback, or when the command only passes on a narrower target than the PR claims. Report the exact command, architecture, guest-visible failure marker, and whether the failure matches remote CI.
Apply the same runtime-validation expectation to ArceOS apps and app-facing tools. A PR that adds or changes an ArceOS app, apps/** demo, QEMU wrapper, rootfs/app preparation tool, symbolizer/log parser, packaging helper, or other tool that claims to make a StarryOS/ArceOS app usable must be reviewed as an executable workflow, not as a syntax-only or docs-only change.
Do not approve changes that are only shaped to satisfy the added tests, such as hard-coded special cases, skipped behavior, fake state updates, no-op compatibility shims, or logic that does not implement the intended subsystem semantics. Treat this as blocking even when local tests and CI pass.
Do not accept "success path" tests that silently skip on unexpected failure, such as returning early when brk, sbrk, I/O, or socket setup returns ENOMEM/EAGAIN, unless the test prints an explicit skip marker and the review explains why the environment legitimately cannot require success. Bugfix reproduction tests should fail loudly when the fixed behavior is absent.
Do not accept changes that simplify, skip, or weaken existing CI/test requirements unless the PR clearly justifies an equivalent or stronger replacement and the replacement is validated. Treat as blocking when a PR removes cases from required test-suit coverage, narrows architectures, loosens success_regex/fail_regex, converts failures into skips/timeouts, changes workflow path filters so relevant tests no longer run, or moves coverage from CI into an opt-in/manual path without preserving regression coverage.
For PRs that add or change Starry QEMU tests, qemu-*.toml, grouped wrappers, generated QEMU runners, or QEMU success_regex/fail_regex, verify failure propagation as a hard gate. A guest test failure must be observable by cargo xtask starry test qemu ... as a failed case, not only as a log line. Treat this as blocking when a QEMU test binary fails but the wrapper still prints the grouped success marker, the command exits zero, fail_regex cannot match the failure marker, $? is overwritten before being captured, or a failing subcase is converted into an unreviewed skip. Board config changes should still be reviewed against the existing board runner semantics, but do not force them into the QEMU grouped/C runner failure-propagation model.
For new or moved Starry grouped/system tests, check the physical layout against the runner and build wrapper, not only the filename. In current qemu/system grouped C cases, each subcase must remain buildable through the system root CMake project: CMakeLists.txt and src/ live directly under system/<subcase>/, subcase-local qemu-*.toml files are not used, and cargo xtask starry test qemu --arch <arch> -c qemu/<subcase> should select that subcase. A system/<subcase>/c/ layout is blocking unless the PR also updates the root CMake, runner discovery, guide, and rule tests to support it and validates the new behavior.
Crates.io Patch Policy
When a PR touches Cargo.toml, Cargo.lock, dependency metadata, duplicate crate versions, third-party dependency APIs, or code that bridges between dependency-owned and workspace-owned types, inspect whether it adds, changes, or relies on a [patch.crates-io] override. Do not approve PRs that introduce or depend on any crates.io patch, regardless of whether the patch target is a local path, fork, git revision, registry replacement, or another override form.
Normal workspace dependency declarations, such as a workspace member using { path = "...", version = "..." }, are not the same as a crates.io patch. The blocking case is overriding crates.io resolution through [patch.crates-io].
The preferred fix is to keep third-party dependencies using their normal crates.io resolution and adapt through explicit local boundaries:
- use the dependency crate's exported public types, traits, error types, or result aliases instead of referencing or replacing that dependency's internal dependency paths;
- add a crate-private adapter near the boundary when local code needs a local type, error, trait object, or ABI representation;
- replace implicit
? conversions that cross dependency-local and workspace-local types with explicit .map_err(...), TryFrom, wrapper newtypes, or a crate-private extension trait;
- keep dependency-facing trait/API code in the dependency's own exported types when it is still implementing or satisfying that dependency's public boundary;
- if the dependency itself is wrong, prefer an upstream fix, a normal dependency upgrade path, or a clearly scoped local adapter, not a workspace-level crates.io patch in the PR.
For error-type mismatches, convert through stable public information exposed by the dependency. For example, when the dependency exports an errno-bearing error, convert the public code into the local errno/error type at the boundary and provide an explicit fallback for unknown values.
Example: for the kbpf-basic/Starry eBPF boundary, do not accept [patch.crates-io] ax-errno = { path = "components/axerrno" }. Keep kbpf-basic on crates.io ax-errno; use kbpf_basic::BpfError and kbpf_basic::BpfResult; add a crate-private eBPF error adapter that converts err.code() into local ax_errno::LinuxError and then ax_errno::AxError; use that adapter in Starry eBPF/perf entry points that return local AxResult.
Duplicate And Overlap Analysis
This analysis is required for every PR, not only bug fixes. Its purpose is to avoid approving duplicate implementations, stale rework, superseded fixes, or PRs that unknowingly conflict with another open PR.
Build an intent fingerprint before searching:
- PR title, body, linked issue numbers, commit subjects, and author-stated validation.
- Changed crates, modules, test cases, configs, CI files, and generated assets.
- Public APIs, syscall names, errno behavior, protocol terms, device types, runner commands, test binary names, and feature flags touched by the patch.
- The semantic claim being made: new feature, bug fix, test coverage, refactor, config update, CI repair, or dependency/metadata change.
Check current base branch first. Search for equivalent behavior, tests, config entries, public APIs, or previous fixes already present on origin/<base>:
git grep -n -E '<relevant symbols|paths|commands>' origin/<base> -- <likely paths>
git log --oneline --decorate -- <likely paths>
If base already has the same behavior or a newer version of it, treat the PR as stale or duplicate unless it clearly adds distinct value. Verify that distinction by reading the relevant base code, not just matching names.
Then check related open PRs. Use the GitHub MCP/connector to search or list candidate PRs before falling back to gh. Search with multiple terms derived from the intent fingerprint; do not rely only on the PR title. Useful terms include crate/module names, changed path fragments, syscall or API names, test case names, issue numbers, errno values, protocol/device names, CI job names, and config names.
gh pr list --state open --limit 200 --search '<symbol OR path OR issue keyword>'
gh pr view <related-pr> --json number,title,body,author,baseRefName,headRefName,isDraft,updatedAt,files,commits
gh pr diff <related-pr> --patch --color=never
git diff --name-only origin/<base>...origin/pr/<related-pr>
Inspect each plausible related PR enough to classify it:
duplicate: solves the same problem or adds the same test/API/config behavior with no meaningful distinction.
partial-overlap: touches the same surface but the changes are complementary, ordered, or separable.
conflict-risk: likely merge or semantic conflict because both PRs modify the same contract, runner behavior, generated asset, or ABI expectation.
superseded: another PR or current base implements the same intent more completely or in a better-aligned way.
unrelated-after-inspection: matched search terms but does not overlap after reading files/diff/intent.
For partial-overlap or conflict-risk, compare the implementation direction with project semantics and note the expected merge order or follow-up needed. If correctness depends on another PR landing first, do not approve until that dependency is explicit in the PR body or review outcome. For duplicate or superseded, submit REQUEST_CHANGES or leave a neutral project-focused comment explaining which base code or open PR should be preferred and why.
Use git diff origin/<base>...origin/pr/<pr> for the PR patch. Use origin/<base>..origin/pr/<pr> only when intentionally checking stale-branch effects.
Treat a PR as not mergeable when it is superseded by a more complete PR or would regress newer base-branch work. Leave a neutral project-focused comment explaining why the newer PR or base implementation should be preferred. If asked to close such a PR, prefer gh pr comment <pr> --body-file comment.md followed by gh pr close <pr>; avoid shell backticks in inline --comment strings.
Validation
Run focused validation matching the changed surface. Prefer project xtask commands:
cargo fmt --check
cargo xtask clippy --package <crate>
cargo clippy --manifest-path <path>/Cargo.toml --all-features -- -D warnings
cargo xtask starry test qemu --arch <arch> -c <case>
cargo xtask axvisor build ... --vmconfigs <config>
If cargo xtask does not cover a special configuration, inspect the relevant xtask help or source before falling back to native Cargo with matched arguments. Record exact commands and failures.
For dependency metadata changes, inspect dependency resolution instead of relying only on the diff. Check for any crates.io patch first, then inspect the affected dependency subtree:
rg -n '\[patch\.crates-io\]' -g 'Cargo.toml' .
cargo metadata --format-version=1 | jq -r '.packages[] | [.name,.version,.source,.manifest_path] | @tsv' | rg '<affected-crate>'
cargo tree -p <affected-package> | rg '<affected-crate>|<boundary-crate>'
For the kbpf-basic/ax-errno example, useful focused checks are:
cargo metadata --format-version=1 | jq -r '.packages[] | select(.name=="ax-errno") | [.version,.source,.manifest_path] | @tsv'
cargo tree -p starry-kernel | sed -n '/kbpf-basic v0.5.7/,+12p'
The expected result for that example is that local workspace crates still use local components/axerrno, while kbpf-basic resolves its own crates.io ax-errno and local Starry eBPF/perf code performs explicit error conversion at the boundary.
For app/tool workflows that CI does not execute exactly, manual runtime validation is a hard gate. This applies when the PR adds or changes:
- StarryOS user-space app support,
apps/starry/** scenarios, Starry rootfs/app preparation, or Starry QEMU run docs/scripts;
- ArceOS apps,
apps/** demos, test-suit/arceos/** app configs, or ArceOS QEMU run docs/scripts;
- tools or wrappers that prepare, launch, inspect, symbolize, package, or otherwise operate on a StarryOS or ArceOS app workflow;
- README/PR-body runbooks that claim the app/tool is usable while the current CI matrix does not run that exact command and success condition.
Do not approve based only on cargo fmt, clippy, shellcheck, --help, script readability, TOML parsing, case listing, build success, or another reviewer saying an older head passed. Those checks are useful supplements, but they do not prove the app or tool works.
Required manual flow:
- Read the PR body, README, scripts, and config files to find the exact documented workflow and expected success marker.
- Do the documented preparation first, such as
cargo xtask starry rootfs --arch <arch>, managed rootfs download/patching, app asset generation, tool build steps, or log/artifact capture.
- Run the current PR head through the actual runtime command, such as
cargo xtask starry app qemu ..., cargo xtask starry test qemu ..., cargo xtask arceos test qemu ..., or the documented wrapper script that reaches QEMU.
- Verify guest-visible behavior and the tool's real output: success markers, app command output, generated logs, symbolized blocks, packaged artifacts, or other documented postconditions. A command that exits 0 but skips the app behavior is not sufficient.
- For multi-architecture support, run the newly added or most failure-prone architecture, prioritizing any architecture skipped or failing in CI. If the PR claims all-arch support and CI covers only part of it, local validation must cover at least one CI-missing or highest-risk architecture.
- If the workflow cannot be run because required documentation, rootfs preparation, assets, or tool outputs are missing or wrong, submit
REQUEST_CHANGES; that is a PR problem, not an environment limitation.
- If the workflow genuinely needs unavailable hardware, credentials, network services, or unsupported host capabilities, record the exact limitation and do not treat syntax/build checks as proof. Require a controlled fallback, a test-suit regression test, or explicit user acceptance before approval.
For StarryOS grouped QEMU cases, verify that new test_commands are actually discovered and installed into the guest overlay. A qemu-*.toml command such as /usr/bin/<test> must correspond to a case/subcase asset path that the runner discovers and builds. For current qemu/system grouped C cases, prefer the smallest current-structure command that covers the change, such as cargo xtask starry test qemu --arch x86_64 -c qemu/<subcase>, or run the aggregate -c qemu/system when wrapper-level behavior is changed. Treat /usr/bin/<test>: not found, status=127, skipped discovery, unbuilt asset directories, wrong grouped/system subcase layout, unreliable success_regex/fail_regex, hidden exit status, or tests that accept both broken and fixed behavior as blocking.
For every newly added or relocated test, identify the exact runner command that should execute it and verify at least one of these evidence sources: local execution on the current head, current-head CI logs that show the specific case/subcase/binary running, or a deterministic build/discovery check that proves the runner reaches the test. Do not treat a broad aggregate CI pass as proof when the new test could have been skipped by path layout, command filtering, missing install rules, subcase selection, or feature gating.
When reviewing Starry grouped/system shell wrappers, inspect failure branches as code, not only by looking at a successful run. The wrapper must capture $? immediately after the test command, print STARRY_GROUPED_TEST_FAILED or the configured failure marker on failure, avoid printing the all-passed marker after any failure, and cause the outer xtask run to fail.
For bugfix tests in grouped cases, inspect the new test's assertions as well as running the case. A grouped case passing is not sufficient when the new test accepts both the fixed behavior and the broken behavior.
For StarryOS app-support PRs, validate both sides when both are present:
- Run the relevant
apps/starry command or an equivalent documented app workflow when the PR adds or changes app support, unless it needs unavailable hardware, credentials, or long-running services; record any limitation.
- Run the corresponding
cargo xtask starry test qemu --arch <arch> -c <case> case when the PR adds a syscall, fixes a kernel/runtime bug, or claims test-suit coverage. For qemu/system subcases, use -c qemu/<subcase> when possible. App validation does not replace test-suit regression validation.
- If the app scenario and test-suit regression cover different risks, mention both results in the review body.
- Do not stop at
--list, TOML parsing, script inspection, or another reviewer saying an older head passed. Those checks prove discovery only, not that the app works. Run the current head in QEMU whenever the changed app/test is intended to run in QEMU.
- If
tmp/axbuild/rootfs is empty, still try the relevant cargo xtask starry rootfs --arch <arch> or cargo xtask starry test qemu ... path before declaring QEMU unavailable; the xtask flow can download managed rootfs images automatically. Record a blocker only after the xtask download/run path itself fails for an environmental reason.
- Do not run multiple Starry QEMU cases concurrently in one worktree. Run one architecture/case to completion, then move to the next architecture if needed.
When the PR does not add or modify a test case, inspect the PR body and commit messages for any claimed non-board validation method, such as QEMU, host unit tests, cargo xtask, cargo test, cargo clippy, shell scripts, emulators, or reproducible manual commands that do not require physical hardware:
- If such validation is claimed, run it or an equivalent local command before approval. Compare the actual command, target, output, and pass/fail condition with the PR's claim.
- If the claimed validation fails, is not reproducible as written, exercises a different target than claimed, silently skips the changed behavior, or cannot be run for an avoidable reason, submit
REQUEST_CHANGES. Explain the mismatch and the expected fix direction: either make the validation true and reproducible, add an appropriate test, or correct the PR description.
- If the claimed validation cannot be run because the environment is genuinely unavailable, record the exact limitation and do not treat the claim as proof. Require another reproducible non-board validation method or a test unless the user explicitly accepts the limitation.
- If the PR has no test changes and neither the PR body nor commit messages describe a reproducible non-board validation method, do not approve. Request changes asking the author to add a test or document and provide a runnable validation command that covers the changed behavior.
- Physical board-only validation may be useful evidence, but it does not satisfy this no-test fallback rule by itself unless the user explicitly scopes the review to board-only behavior.
Use GitHub check status as required evidence, but not as the only review input:
gh pr checks <pr> --watch=false
Do not approve solely because remote CI passes. Conversely, if required checks are failing, cancelled, or missing for a branch that needs CI coverage, inspect logs and classify the failure before deciding. Treat PR-related CI failures as blocking and request changes with the expected fix direction. If a CI failure is unrelated to the PR, it is not by itself a reason to request changes, but the review body must say why it is unrelated and link the existing issue that was updated or the newly-created tracking issue. A branch with no reported checks is not equivalent to passing; require targeted local validation before approving, and request changes when the changed surface is too large or risky to validate locally.
When GitHub log download fails or returns an empty log, do not infer the check passed or was irrelevant. Use gh pr checks <pr> --repo <owner>/<repo> --watch=false and gh run view <run-id> --json headSha,jobs to confirm the current head, failing job names, conclusions, and failing steps. If the failing job matches a newly added or changed app/test architecture, treat it as PR-related unless concrete evidence proves otherwise.
Blocking Findings
Treat these as blocking unless clearly non-blocking:
- behavior differs from POSIX/Linux/RFC/VirtIO semantics;
- targeted tests, formatting, clippy, or PR-related CI fail;
- a newly added or changed Starry app/QEMU case fails when run as described by the PR, including one architecture among newly added multi-arch
qemu-*.toml cases;
- a Starry QEMU test failure is visible in guest logs but does not make
cargo xtask starry test qemu ... fail, including hidden $?, missing failure marker, overly loose success_regex, or missing fail_regex coverage;
- a PR claims app/QEMU support but only discovery, TOML parsing, or an older-head run was validated;
- a PR adds or changes CI-missing StarryOS user-space support, an ArceOS app, or an app-facing tool/wrapper, but the documented preparation plus QEMU/runtime workflow was not run on the current head;
- an app/tool workflow's documentation is incomplete or wrong enough that the reviewer cannot prepare the environment, launch QEMU, or verify the documented postcondition;
- required tests are missing for new behavior, semantic changes, or bug fixes, without a concrete documented impossibility;
- new or relocated tests are misplaced, not discovered by the project test runner, not built/installed into the runtime image, not selected by the documented command, or do not exercise the fixed ABI surface;
- CI passes only because new coverage is skipped by layout, path filters, feature gating, grouped subcase selection, missing install rules, or manual-only placement;
- a PR has no test changes and lacks a reproducible non-board validation method in the PR body or commit messages;
- a claimed non-board validation method is not actually reproducible or does not match the claimed coverage/result;
success_regex or fail_regex cannot reliably classify the intended StarryOS case result;
- a bug-fix PR lacks a regression or reproduction test that fails on the unfixed behavior and passes only after the fix, unless concrete evidence shows such a test is impossible;
- the PR adds, changes, or relies on any
[patch.crates-io] override, instead of using normal dependency resolution, an upstream fix, a dependency upgrade, or an explicit local boundary adapter;
- merge conflicts are unresolved, conflict repair resurrects outdated base APIs instead of adapting PR intent to current base, or the repaired head was not revalidated after push;
- StarryOS app-support PRs place app workflows under
test-suit/starryos instead of apps/starry, or place syscall/bugfix semantic coverage only under apps/starry instead of the matching test-suit case;
- the implementation is a test-only or fake fix that does not implement the intended behavior;
- submitted buffers, DMA memory, queue tokens, or IRQ ownership can leak, be freed too early, or cross the wrong abstraction layer;
- a change silently makes CI hang, time out, or skip the new coverage;
- a change weakens CI or normal-regression coverage by removing cases, narrowing architectures, loosening pass/fail regexes, skipping relevant workflows, or moving required coverage to manual-only paths without an equivalent validated replacement;
- the PR duplicates existing base-branch behavior, weakens an existing implementation, conflicts with a related open PR, or is superseded by a newer base-branch or open-PR fix;
- the review cannot explain how this PR differs from a plausible related open PR after duplicate and overlap analysis.
All GitHub review text, including inline comments, review body, and replies, must be in Chinese, neutral, and project-focused. Each blocking comment should include the grounding, severity, concrete problem, evidence, and suggested fix direction required by the review-lens discipline above.
Prefer changed lines on the PR diff. Before submitting, verify every inline line exists on the current right side of the diff; if GitHub cannot resolve a line, move to the nearest changed line that demonstrates the issue or put the finding in the review body. Context or unchanged lines may be rejected by the review API.
Submit Review
Before submitting, confirm through the GitHub MCP/connector that the PR head SHA has not changed. Fallback only when connector data is unavailable:
gh pr view <pr> --json number,headRefOid,reviewDecision
If the head changed after analysis or validation, fetch the new head, update the worktree, re-check each finding on current changed lines, and rerun the targeted validation that supports the decision.
Submit the final review through the GitHub MCP/connector when it can send the review event and inline comments with preserved anchors together. If the connector cannot submit inline review comments or cannot preserve line anchors, fallback to the GitHub review REST API via gh:
gh api --method POST repos/<owner>/<repo>/pulls/<pr>/reviews --input review.json
Use the current headRefOid as commit_id, side=RIGHT for inline comments, REQUEST_CHANGES for any blocking issue, and APPROVE only when no blocking issue remains:
{
"commit_id": "<headRefOid>",
"event": "REQUEST_CHANGES",
"body": "...",
"comments": [
{"path": "path/to/file.rs", "line": 123, "side": "RIGHT", "body": "..."}
]
}
Do not submit stale findings against an old head.
If a worker returns a finding on a line that is not present on the current PR diff, move the comment to the nearest changed line that demonstrates the problem or put the finding in the review body.
After submission, re-query the PR. If a new commit landed during review submission, refresh the worktree and submit a follow-up review only if the blocking issue still applies to the new head.
Review body must explain in Chinese:
- what the PR changed;
- the implementation logic and why this approach is correct for the project semantics;
- validation commands and results, including exact failure mode for failing tests;
- required test coverage status, including why tests were required or not applicable, where new tests were placed, how the runner discovers/selects them, and whether local or current-head CI evidence shows the specific tests executing;
- for CI-missing app/tool workflows, the documented preparation performed, the exact QEMU/runtime command run, the architecture, and the guest-visible or tool-output postcondition that proved usability;
- when no tests are added, the PR body/commit-message validation claim that was checked, the command actually run, and whether it matched the claim;
- CI status, including any unrelated failing checks, the evidence for unrelatedness, the linked tracking issue, and whether that issue was updated or created during review;
- duplicate and overlap analysis: base-branch evidence checked, related open PRs inspected, and why the PR is distinct, complementary, duplicate, conflicting, or superseded;
- conflict handling status when applicable: conflicted files, resolution logic, validation after repair, and whether a repair commit was pushed or the work was intentionally kept as a dry run;
- for PR-related CI failures, the failing check, failure mode, and expected fix direction;
- reproduction coverage status for bug fixes, including whether the regression test fails on the unfixed behavior and passes only after the fix;
- unresolved review conversations that were resolved, and conversations intentionally left open and why;
- any behavior that remains unimplemented, partial, or should be completed in future work;
- any known environment limitation.
Do not approve when the review cannot explain the implementation logic beyond "tests pass".
Verify final state:
gh pr view <pr> --json number,reviewDecision,latestReviews
Recommended Reviewer Assignment
After review submission, request reviewers only when the PR still needs domain follow-up. Base the choice on the actual changed surface, review findings, validation risk, and remaining follow-up.
Keep this reviewer request step aligned with reassign-pr-reviewers. Read .github/MAINTAINERS.md before choosing reviewers. Treat it as the local reviewer source of truth and the strict automatic human reviewer allowlist: only R: lines are requestable automatic human reviewers. M: lines are ownership metadata and are not reviewer targets unless the same login also appears on R:. Do not request, retain as an ownership target, or infer a new human reviewer outside the R: allowlist.
Match maintainer sections with F: path hints and K: keyword hints from the PR title, body, changed paths, public APIs, tests, validation commands, review findings, crate/config/feature names, and obvious diff-visible identifiers. If multiple sections match, target all matched R: reviewers. Prefer explicit K: evidence for ambiguous PRs, but valid F: path evidence is sufficient when the changed files clearly fall under a section.
If no K: or F: evidence matches a non-draft PR, default the target reviewer to ZR233, after confirming ZR233 appears on an R: line. Report this as a fallback assignment, not as ownership evidence.
Skip reviewer request updates for draft PRs unless the user explicitly asks to include drafts. Reviewing a draft is allowed by this skill, but reviewer reassignment for drafts is not.
Preserve all existing reviewer requests by default. Existing human reviewers may have been assigned manually by an administrator, including reviewers outside .github/MAINTAINERS.md; carry them forward and do not remove them in the default flow. Existing bot reviewer requests must also be preserved unless the user explicitly says to change bot requests. Bot reviewers are not ownership targets and do not need to appear in .github/MAINTAINERS.md.
The default reviewer request update is add-only: compute ownership targets from .github/MAINTAINERS.md, union the existing reviewer requests into the final desired reviewer state, and add only missing target reviewers. reviewers to remove must be empty unless the user explicitly asks to remove or rebalance reviewer requests. If the user does request removals, still preserve bot reviewers unless bot removal was explicitly requested.
Drop the PR author from new reviewer requests because GitHub cannot request review from the author. Also drop the current GitHub user from new reviewer requests because this workflow just submitted the review.
Before writing reviewer requests, check current reviewer state and permissions:
gh api repos/<owner>/<repo>/pulls/<pr>/requested_reviewers
gh api repos/<owner>/<repo>/collaborators/<login>/permission
Before applying reviewer request updates, record a single-PR dry run with current reviewers, target reviewers, preserved existing reviewers, preserved bot reviewers, reviewers to add, reviewers to remove, matched K:/F: evidence or fallback reason, and skipped reason.
Use the REST requested-reviewers API instead of gh pr edit, because gh pr edit can fail in this repository while querying deprecated Projects classic fields. In the default add-only flow, do not call the DELETE endpoint:
printf '%s\n' '{"reviewers":["<login1>","<login2>"]}' |
gh api -X POST repos/<owner>/<repo>/pulls/<pr>/requested_reviewers --input -
If the user explicitly requested removals or a rebalance, apply allowed removals before additions for each PR, while still preserving bot reviewers unless bot removal was explicitly requested:
printf '%s\n' '{"reviewers":["<login>"]}' |
gh api -X DELETE repos/<owner>/<repo>/pulls/<pr>/requested_reviewers --input -
After assigning, re-query requested_reviewers and confirm the intended reviewers are present. If GitHub rejects a reviewer, record the exact login and API or permission error; do not silently substitute someone outside .github/MAINTAINERS.md.
In the final user summary, state:
- which
.github/MAINTAINERS.md entries matched the PR;
- which reviewers were requested, already present, preserved, skipped, or rejected;
- any fallback assignment to
ZR233;
- any permission/API limitation;
- that only GitHub reviewer metadata was changed, when no code files were edited by the assignment step.
Cleanup
After review submission or an explicit no-submit stop, clean temporary resources before ending:
- Remove clean review and conflict worktrees with
git worktree remove <path>, then run git worktree prune from the main repository.
- Delete temporary files created for review payloads, GraphQL queries, comments, logs, or conflict notes unless the user asked to keep them.
- Do not remove a worktree that has uncommitted conflict-repair work, diagnostics needed for a reported failure, or user-created changes; report the path and reason instead.
- Confirm the main worktree status was not changed by the review workflow.