docs(review-single-pr): refine conflict handling workflow

.claude/skills/review-single-pr/SKILL.md

@@ -1,6 +1,6 @@
 ---
 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, run focused validation, leave Chinese inline review comments, approve, request changes, or assign reviewers after review.
+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, validate Starry app-support test placement, repair safe merge conflicts, run focused validation, leave Chinese inline review comments, approve, request changes, or assign reviewers after review.
 ---
 
 # Review Single PR
@@ -106,14 +106,48 @@ Never review multiple StarryOS QEMU cases in the same checkout at the same time.
 
 ## Merge Conflicts
 
-Handle merge conflicts in either of these cases: the user explicitly asks for conflict handling, or the review has no blocking findings and would otherwise be `APPROVE` while `mergeStateStatus=DIRTY` and `maintainerCanModify=true`. Do not submit approval before the conflict repair is pushed and re-validated against the new head.
+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.
 
-- If `mergeStateStatus` is `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` is `DIRTY` and `maintainerCanModify=true`, create a separate conflict worktree, check out `origin/pr/<pr>` detached, merge `origin/<base>`, resolve conflicts according to PR intent and current base behavior, validate, then commit the merge locally.
-- Before pushing a repaired conflict branch, refresh the PR and confirm the local merge commit's first parent equals the current remote `headRefOid`. If the remote head changed, stop and rebase/re-review instead of pushing.
+First refresh and classify the conflict and approval state:
+
+```bash
+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:
+
+  ```bash
+  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:
+
+  ```bash
+  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
 
@@ -128,6 +162,14 @@ Review the PR against its stated intent, the current base branch, existing proje
 
 For bug fixes, require a reproduction test that fails before the fix and passes after it unless the environment makes that impossible. For raw syscall fixes, prefer direct `syscall(SYS_...)` coverage when libc wrappers could mask return values or errno.
 
+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/normal` in the matching existing case group, such as `qemu-smp1/syscall`, `qemu-smp1/bugfix`, `qemu-smp1/c-regression`, networking, DRM, evdev, or another closest semantic group.
+- If the PR adds a syscall or changes syscall semantics for the app, require a minimal normal 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 normal 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/normal`, or that hide syscall/bugfix coverage only inside `apps/starry` demos.
+
 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.
@@ -193,6 +235,12 @@ For StarryOS grouped QEMU cases, verify that new `test_commands` are actually di
 
 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 --test-group normal ...` case when the PR adds a syscall, fixes a kernel/runtime bug, or claims normal test coverage. App validation does not replace normal regression validation.
+- If the app scenario and normal regression cover different risks, mention both results in the review body.
+
 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.
@@ -220,6 +268,8 @@ Treat these as blocking unless clearly non-blocking:
 - 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;
 - bug fixes lack meaningful reproduction coverage;
+- 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/normal` instead of `apps/starry`, or place syscall/bugfix semantic coverage only under `apps/starry` instead of the matching normal 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;
@@ -273,6 +323,7 @@ Review body must explain in Chinese:
 - 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, and the linked tracking issue;
 - 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;
 - unresolved review conversations that were resolved, and conversations intentionally left open and why;

AGENTS.md

@@ -11,7 +11,7 @@
 - `review-open-prs`: project-local skill at `.claude/skills/review-open-prs/SKILL.md`
 - Use `review-open-prs` when the user wants to audit all open GitHub PRs, review non-self PRs, re-review PRs updated after their last review, use subagents/worktrees for PR review, compare changes with POSIX/Linux/RFC/VirtIO semantics, run local validation, and submit approve or request-changes reviews.
 - `review-single-pr`: project-local skill at `.claude/skills/review-single-pr/SKILL.md`
-- Use `review-single-pr` when the user names one PR number or URL and wants a focused review, re-review, duplicate or overlapping open-PR analysis, Linux/POSIX/RFC/VirtIO comparison, local validation, Chinese inline review comments, approval, request-changes submission, or post-review reviewer assignment.
+- Use `review-single-pr` when the user names one PR number or URL and wants a focused review, re-review, duplicate or overlapping open-PR analysis, Starry app-support test placement checks, merge-conflict handling for otherwise approvable PRs, Linux/POSIX/RFC/VirtIO comparison, local validation, Chinese inline review comments, approval, request-changes submission, or post-review reviewer assignment.
 - `reassign-pr-reviewers`: project-local skill at `.claude/skills/reassign-pr-reviewers/SKILL.md`
 - Use `reassign-pr-reviewers` when the user wants to assign or rebalance GitHub PR reviewers for `rcore-os/tgoskits` from a discussion, ownership matrix, open PR scope, or existing review-request state, including preserving bot requests and handling collaborator permission limits.
 - `board-uboot-fsck-repair`: project-local skill at `.claude/skills/board-uboot-fsck-repair/SKILL.md`