Skip to content

docs: make local review scope and outcomes explicit - #1804

Merged
bkudiess merged 2 commits into
mainfrom
bkudiess-w4-upstream-draft
Oct 22, 2026
Merged

bkudiess merged 2 commits into
mainfrom
bkudiess-w4-upstream-draft

Conversation

@bkudiess

@bkudiess bkudiess commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

Supersedes #1800.

What Problem This Solves

Fixes: local shared reviews can be reported as whole-PR or clean P0-P3 when the helper actually reviewed a narrower comparison, used its P0-only default, or filtered findings.

User Impact

User impact: no product change. Contributors and agents get a short Windows recipe for selecting review scope explicitly and a three-point check before calling a review clean. The PowerShell example requires PowerShell 7.3+, Windows/Standard native argument passing, and a native python.exe.

Why This Change Was Made

The shared helper defaults to --max-priority P0, and auto mode on a dirty worktree reviews only work since HEAD. Exit 0 can also mean filtered. The guide states these traps, gives an explicit mode/base table, and lists the fields to check together before reporting clean.

The shared skill stays authoritative. The second commit shortened the guide from 214 to 126 added lines by replacing copied helper internals (engine, image, preflight, and full result details) with links pinned to openclaw/agent-skills@1444c91. The repository skill now says installation needs separate authorization and keeps the narrative fallback distinct from helper status.

Evidence

Checked on head bafe5365f5ce22c0143f19ca597592889e3f87d7:

  • Key claims match the pinned helper source at 1444c91: --max-priority defaults to P0; auto selects local mode when dirty; filtered is a distinct status; incomplete sets exit 2; status records exit_code and report_produced.
  • All seven pinned upstream links resolve, including the #committed-source-context, #copilot-subagent-review, and #preflight-summary anchors.
  • Exact-head CI: change-classification, fast-validation, Proof-pool contracts, and CI Gate passed. Product build and test jobs were skipped by change classification.
  • Merges cleanly with current main.

Gaps: the shared helper and the full PowerShell example were not executed. Earlier Opus/GPT narrative reviews covered the first commit only; the shortening commit received a single-reviewer narrative review with no findings.

Change Type

  • Bug fix
  • Feature
  • Refactor
  • Docs or instructions
  • Tests or validation
  • Security hardening
  • Chore or infrastructure

Scope

  • Tray or WinUI UX
  • Windows node capability
  • Local MCP or winnode
  • Gateway, connection, or pairing
  • Setup or onboarding
  • Permissions, privacy, or security
  • Tests, CI, or docs

Required proof pools

  • none: documentation and agent-skill guidance only; no product, UI, winnode, Gateway, installer, or native-host behavior changed.

Validation

Run locally on bafe5365f5ce22c0143f19ca597592889e3f87d7 with a clean worktree:

Exact command Result
.\scripts\validate-docs.ps1 Passed, exit 0: 63 Markdown files; 7 named proof pools; documentation flow regression passed
.\scripts\validate-agent-skills.ps1 Passed, exit 0: 11 skills and 20 Markdown files
git diff --check 5c66a7d35ef328c2487df6e4d86ed92d4cc5942d HEAD Passed, exit 0
git merge-tree --write-tree origin/main HEAD Exit 0, no conflicts

Build and Shared/Tray tests were not run: prose-only change, per the documentation validation route.

Real Behavior Proof

  • Environment tested: Windows, local clean worktree.
  • PR head or commit tested: bafe5365f5ce22c0143f19ca597592889e3f87d7
  • Exact steps or command run: documentation and skill validators above; source inspection of the pinned upstream helper and references.
  • Evidence after fix: validators passed; upstream defaults, statuses, fields, and link anchors match the documented claims.
  • Observed result: guidance is consistent with the pinned shared helper and links resolve.
  • Screenshot or artifact links verified? (Yes/No/N/A) N/A
  • Not verified or blocked: no helper run, provider/model authentication, or execution of the full PowerShell example. Installed-copy parity with 1444c91 is not claimed.

Security Impact

  • New permissions or capabilities? (Yes/No) No
  • Secrets or tokens handling changed? (Yes/No) No
  • New or changed network calls? (Yes/No) No
  • Command or tool execution surface changed? (Yes/No) No
  • Data access scope changed? (Yes/No) No
  • If any answer is Yes, explain the risk and mitigation: N/A

Compatibility and Migration

  • Backward compatible? (Yes/No) Yes
  • Config or environment changes? (Yes/No) No
  • Migration needed? (Yes/No) No
  • If yes, list the exact upgrade steps: N/A

Review Conversations

  • I replied to or resolved every bot review conversation addressed by this PR.
  • I left unresolved only conversations that still need maintainer judgment.

No review threads are open. The ClawSweeper review covered b8c718a; it has not re-reviewed bafe536.

Document the shared reviewer invocation, result-reading boundaries and honest fallback. Require safe native PowerShell argument handling and state the non-merge commit limit.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f44c82c5-1366-4233-9691-48f66ab81984
@clawsweeper

clawsweeper Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Oct 21, 2026
@clawsweeper

clawsweeper Bot commented Oct 21, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge.

What this changes

This PR documents explicit review scope, Windows invocation prerequisites, final-result interpretation, and fallback limitations.

Example: Review committed changes plus dirty work against merge base 5c66a7d.

  • Before: Default local mode excludes changes already committed on the branch.
  • After: The guide explicitly selects local mode with that merge base and P0-P3 severity, covering committed and dirty changes.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) Useful, source-backed documentation with passing exact-head documentation CI and no actionable findings.
Proof confidence 🌊 off-meta tidepool Not applicable: Documentation-only change: supplied PowerShell argument probes support the example's guards, while the body explicitly disclaims full helper, provider, and product execution.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Docs · Worth it: Yes · Fix scope: Complete
User problem: Contributors can overstate review coverage or cleanliness when they misunderstand the comparison scope or final outputs.
Reason: The collaborator-authored direction addresses a concrete documentation gap while preserving shared ownership and the existing fallback.

Merge readiness

✅ Ready for maintainer review

Keep open: this collaborator-authored draft adds useful guidance absent from current main, and inspection found no actionable introduced defects.

Priority: P3
Reviewed head: b8c718a56a87130b026e9264b6929a8f0cb84d4e

Before merge

None.

Findings

None.

Agent review details

How this fits together

Repository instructions route contributors to a separately installed shared reviewer, which consumes a Git comparison and supplied context and produces reports and status.

flowchart LR
  A[Contributor task] --> B[Repository review entrypoint]
  B --> C[Windows invocation guide]
  C --> D[Shared reviewer]
  D --> E[Report and status]
  E --> F[Scoped assessment]
Loading

Technical review

Best possible solution:

Retain the source-backed Windows recipe while keeping reviewer implementation and result contracts authoritative in the shared skill.

Do we have a high-confidence way to reproduce the issue?

The documentation gap is directly visible in current main; this PR does not claim to repair a runtime failure.

Is this the best way to solve the issue?

The approach fits the existing ownership boundary and explains scope and result limitations without adding a local reviewer implementation.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 489208e06780.

Provenance checked

  • Canonical shared-review ownership keeps the original intent (0c6e586: Remove drifting repository-local implementations and use one shared installation.)
  • Independent Copilot fallback keeps the original intent (d6fe320: Allow independent review through Copilot without requiring a separate Codex CLI, preserving severity and honest limitation reporting.)
  • Workflow and evidence guidance keeps the original intent (3977f82: Route agents to the appropriate owners and evidence without treating incomplete validation as completed proof.)

Security

None.

Evidence

What I checked:

  • Verified introduction: The pinned parent-to-head diff changes only the review entrypoint and workflow guide; comparison of the verified test merge against its main parent confirms the same scope. (docs/AGENT_WORKFLOWS.md:36, b8c718a56a87)
  • Current main and preserved intent: Main retains the shorter entrypoint. The change preserves canonical shared ownership from chore(autoreview): use the shared canonical skill #1599 and the independent Copilot fallback from docs: allow Copilot autoreview when Codex CLI is unavailable #1730. (.agents/skills/autoreview/SKILL.md:8, 489208e06780)
  • Authoritative dependency contract: The added guide explicitly invokes this repository's helper and cites its pinned contract, establishing policy applicability. Its root and autoreview policies, canonical skill, context/image/result references, and relevant helper source support the documented scope, merge-commit rejection, output publication, and status interpretation. (skills/autoreview/scripts/autoreview:7149, 1444c91f6b45)
  • Discussion and supplied evidence: The full body distinguishes source checks and argument probes from helper/provider execution. Current comments are bot status notices; the author's comment on docs: make local review scope and outcomes explicit #1800 confirms this PR replaces that closed publication with the identical commit. (b8c718a56a87)
  • Policy and validation: Read the complete root and docs instructions and applicable documentation/proof guidance. No additional ancestor instructions, maintainer notes, or pr-simplify skill were present. Exact-head fast-validation, proof-pool contracts, and CI Gate passed. No helper, second reviewer, build, or tests were run during this read-only review; the checkout remained clean. (docs/AGENTS.md:1, b8c718a56a87)
  • Feature-history ownership: Scoped history shows repeated autoreview work by Vincent Koc, Windows harness work by Paul Campbell, fallback guidance by Scott Hanselman, and workflow guidance by Barbara Kudiess. Some older blobs were unavailable locally; commit history and current-line attribution remained readable. (.agents/skills/autoreview/SKILL.md:8, 489208e06780)

Likely related people:

  • Barbara Kudiess: Raw commit 3977f82 adds docs/AGENT_WORKFLOWS.md:3 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 3977f8206ed4; files: docs/AGENT_WORKFLOWS.md)
  • Vincent Koc: Raw commit 0c6e586 adds .agents/skills/autoreview/SKILL.md:8 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 0c6e5867e1b8; files: .agents/skills/autoreview/SKILL.md)
  • Scott Hanselman: Raw commit d6fe320 adds .agents/skills/autoreview/SKILL.md:17 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: d6fe320708a0; files: .agents/skills/autoreview/SKILL.md)

Root-cause cluster

Relationship: canonical
Canonical: #1804
Summary: This PR replaces the closed, unmerged publication of the identical source commit.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Labels

Label changes:

  • add P3: Contributor guidance improves review accuracy without changing product runtime behavior.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P3: Contributor guidance improves review accuracy without changing product runtime behavior.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 21, 2026, 12:03 PM ET / 16:03 UTC.

Keep candidate scope and final-result safeguards while linking shared helper details to their canonical references.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 215eb5ec-a42f-4eff-83c9-612c71b59797
@bkudiess
bkudiess marked this pull request as ready for review October 22, 2026 00:11
@bkudiess
bkudiess merged commit 9ba50f3 into main Oct 22, 2026
21 checks passed
@bkudiess
bkudiess deleted the bkudiess-w4-upstream-draft branch October 22, 2026 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant