Skip to content

feat(review): offline --local-range mode for pre-submission review (#357 follow-on)#369

Draft
anagnorisis2peripeteia wants to merge 3 commits into
openclaw:mainfrom
anagnorisis2peripeteia:feat/offline-local-review
Draft

feat(review): offline --local-range mode for pre-submission review (#357 follow-on)#369
anagnorisis2peripeteia wants to merge 3 commits into
openclaw:mainfrom
anagnorisis2peripeteia:feat/offline-local-review

Conversation

@anagnorisis2peripeteia

Copy link
Copy Markdown
Contributor

Summary

#357 added review --local-only for advisory review before submitting a PR — but it
still requires the issue/PR to already be open on GitHub: it fetches the item, diff,
body, and comments via the API (--item-number), and rejects closed items. So you can't
actually review your work before opening the PR, which is the stated goal.

This adds --local-range: review the local git range
(merge-base(--base, HEAD)..HEAD) with no GitHub fetch. The Item + ItemContext
are synthesized from git — the diff from git diff, the body from the commit message
(or --body-file) — so the full review (including the real-behavior-proof and Mantis
assessments) runs offline, before a PR exists, and on fork checkouts that the gh-fetch
gitInfo path rejects.

It also folds in three small, related advisory-review affordances that compose with it:

  • --allow-closed: review closed/merged items (regression fixtures, hypothetical re-review).
  • --body-file <file>: substitute the PR body in the prompt — to test the
    real-behavior-proof / Mantis decision against a hypothetical body, or to supply the body
    when reviewing a local range.
  • --additional-policy <file>: layer a repo-specific policy file onto the review prompt.

Changes

  • --local-range (with --base <ref>, default origin/main): buildLocalRangeReview()
    builds a synthetic Item + ItemContext (pullFiles from git diff, body from the
    commit message) and reviewCommand branches three points — offline gitInfo
    ({ mainSha, latestRelease: null }, no fetch), the synthetic candidate, and the synthetic
    context — and suppresses the review-start comment.
  • --allow-closed: relaxes the open-only selection gate in selectCandidates.
  • --body-file: injects an authoritative PR-body section into the review prompt.
  • --additional-policy: reads a policy file and layers it onto the review prompt.

Validation

  • pnpm run check (build, lint, unit + repair tests, changed + full coverage, format):
    passes — fail 0, format clean.

  • Offline review of a real fork checkout (a 3-file, +113/-3 change) produced a valid
    Decision with no GitHub access — the diff was read from git diff, not the API:

    decision: keep_open · confidence: high
    realBehaviorProof.status:    missing       (the commit body carries no after-fix proof)
    telegramVisibleProof.status: not_needed
    mantisRecommendation.status: not_recommended
    

Scope / notes

  • This is advisory/offline only: --local-range implies no GitHub writes (no start
    comment, no posted review).
  • Engine selection is intentionally out of scope for this PR.

Authored with Claude Code.

Add --allow-closed (review closed/merged items: fixtures, hypothetical re-review),
--body-file (substitute a hypothetical PR body to exercise the proof/mantis decision,
and to feed the body to engines that cannot fetch it live), and --additional-policy
(layer a repo-specific policy file). All route through additionalPrompt + the one
selection gate, so they apply to every engine uniformly.
Synthesize the Item + ItemContext from the local git range
(merge-base(--base, HEAD)..HEAD) so the full review — real-behavior proof and
mantis decision — runs BEFORE a PR exists and WITHOUT a GitHub fetch. The diff
comes from `git diff`, the body from the commit message (or --body-file), so it
works offline and on fork checkouts that the gh-fetch path rejects.
Add buildLocalRangeReviewForTest + a temp-git-repo test asserting the synthetic
PR item, the commit-message body, and the git-diff pullFiles are built offline,
and that an empty range throws.
@anagnorisis2peripeteia

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper

clawsweeper Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 24, 2026, 10:55 PM ET / 02:55 UTC.

Summary
The PR adds --local-range, --allow-closed, --body-file, and --additional-policy to synthesize PR review context from a local git range and run advisory review before a GitHub PR exists.

Reproducibility: yes. for the review findings: source inspection shows the new localRange branch enters ordinary reviewCommand, and a scratch git rename check shows the name-status parser can produce an empty patch. I did not run the PR's review command because this review is read-only.

Review metrics: 2 noteworthy metrics.

  • Changed Surface: 2 files changed, +280/-9. A small diff still changes review command execution and offline review semantics.
  • New CLI Switches: 4 options added. --local-range, --allow-closed, --body-file, and --additional-policy create user-facing review workflow and support surface.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Post redacted terminal output, copied live output, or linked logs showing final-head --local-range running offline with GitHub access unavailable; redact private data before posting.
  • [P1] Add tests that prove local-range runs scrub GitHub tokens, use an empty GH_CONFIG_DIR, disable Codex web search/network access, and avoid GitHub writes.
  • Cover rename/copy diffs and synthetic author-association/proof-gate behavior in focused tests.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body includes claimed validation summaries, but no inspectable redacted terminal output, logs, or linked artifact proving the new offline path ran after the fix and avoided GitHub access. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] The advertised offline mode currently runs through the issue/PR review path without the existing local-review token scrubbing, empty GH_CONFIG_DIR, disabled web search, clean-checkout enforcement, or no-network prompt.
  • [P1] This adds a second pre-PR branch-review surface beside pnpm local-review, so maintainers need to decide whether the PR-shaped mode should exist or whether the existing command should be extended instead.
  • [P1] Rename/copy rows from git diff --name-status can be converted into an impossible filename and produce empty patches, reducing review coverage for common local branch diffs.
  • [P1] The synthesized OWNER author association can suppress the external-PR proof gate that this feature is supposed to exercise.
  • [P1] The PR body includes claimed validation summaries, but no inspectable redacted terminal output, logs, or linked artifact proving the offline path avoided GitHub access after the fix.

Maintainer options:

  1. Reuse Existing Offline Engine
    Route the committed-range use case through the current Commit Sweeper local-review path so the established offline contract stays the single supported implementation.
  2. Harden The PR-Shaped Path
    If maintainers want --local-range in reviewCommand, add token scrubbing, empty GH_CONFIG_DIR, web-search/network disabling, clean-checkout handling, rename parsing, synthetic proof metadata, and focused tests before merge.
  3. Pause For Product Direction
    Pause or close this PR if the merged local-review command is intended to remain the only supported pre-PR branch-review surface.

Next step before merge

  • [P1] Needs maintainer direction on whether the PR-shaped local-range surface should exist, plus contributor proof; ClawSweeper should not start an automated repair while proof and product direction are unresolved.

Security
Needs attention: The diff introduces an advertised offline review mode without carrying over the existing offline review credential and network isolation safeguards.

Review findings

  • [P1] Reuse the hardened local-review path — src/clawsweeper.ts:16147-16148
  • [P2] Parse rename and copy rows before building patches — src/clawsweeper.ts:16043-16044
  • [P2] Do not mark synthetic local PRs as OWNER — src/clawsweeper.ts:16059
Review details

Best possible solution:

Either fold committed-range pre-PR review into the existing hardened pnpm local-review engine, or keep a PR-shaped mode only after it matches the same offline, credential, clean-checkout, proof-gate, and diff-parsing contracts.

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

Yes for the review findings: source inspection shows the new localRange branch enters ordinary reviewCommand, and a scratch git rename check shows the name-status parser can produce an empty patch. I did not run the PR's review command because this review is read-only.

Is this the best way to solve the issue?

No. The narrower maintainable path is to reuse the existing hardened local-review implementation or intentionally duplicate all of its offline protections, then fix synthetic metadata and diff parsing before merge.

Full review comments:

  • [P1] Reuse the hardened local-review path — src/clawsweeper.ts:16147-16148
    --local-range enters ordinary reviewCommand instead of the current local-review engine, so the advertised offline run does not inherit clean-checkout enforcement, token scrubbing, empty GH_CONFIG_DIR, disabled web search, or the no-network prompt. Route this mode through the hardened local-review path or copy those guardrails and tests before exposing it.
    Confidence: 0.87
  • [P2] Parse rename and copy rows before building patches — src/clawsweeper.ts:16043-16044
    git diff --name-status emits rename/copy rows with both old and new paths, but this parser keeps everything after the first tab as one filename. For a rename row, the follow-up git diff -- <old\tnew> can return an empty patch, so local-range reviews can miss changed files.
    Confidence: 0.92
  • [P2] Do not mark synthetic local PRs as OWNER — src/clawsweeper.ts:16059
    The synthetic item is a pull request with authorAssociation: "OWNER", while ClawSweeper's real-behavior-proof gate is for external PRs and skips maintainer associations. That undermines this PR's stated goal of exercising proof decisions against local ranges, so use non-maintainer synthetic metadata or explicit local synthetic prompt handling.
    Confidence: 0.84

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 323f7bdf7236.

Label changes

Label justifications:

  • P2: This is a bounded review-workflow feature with limited blast radius but meaningful automation and auth-boundary impact.
  • merge-risk: 🚨 automation: The PR changes review command selection, local/offline execution, context synthesis, and start-comment behavior.
  • merge-risk: 🚨 security-boundary: The proposed offline path can run without the current offline review credential isolation, GH_CONFIG_DIR isolation, and web/network guardrails.
  • merge-risk: 🚨 auth-provider: The new path changes how local Codex auth and GitHub proof credentials may be available to the review subprocess.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body includes claimed validation summaries, but no inspectable redacted terminal output, logs, or linked artifact proving the new offline path ran after the fix and avoided GitHub access. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

Security concerns:

  • [medium] Offline mode lacks credential isolation — src/clawsweeper.ts:16147
    The local-range path is routed through ordinary issue/PR review execution, so it does not inherit token scrubbing, empty GH_CONFIG_DIR, disabled web search, or no-network prompt protections before Codex runs.
    Confidence: 0.87

What I checked:

  • Repository policy read: AGENTS.md was read fully; its automation-safety guidance applies because this PR changes ClawSweeper review command behavior and credential boundaries. (AGENTS.md:1, 323f7bdf7236)
  • PR adds a new local-range path inside issue/PR review: PR head synthesizes local-range data and then runs it through reviewCommand, creating a PR-shaped review route separate from the existing Commit Sweeper local-review engine. (src/clawsweeper.ts:16147, 93b521e89df8)
  • Existing local-review guardrails on current main: Current localReviewCommand deletes GitHub token env vars, requires a clean checkout, points GH_CONFIG_DIR at an empty directory, adds an offline/no-network prompt, and disables Codex web search. (src/commit-sweeper.ts:400, 323f7bdf7236)
  • Review subprocess can receive GitHub proof token: Current runCodex launches Codex with codexEnv({ ghToken: process.env.CLAWSWEEPER_PROOF_INSPECTION_TOKEN, preserveCodexAuth }); the PR does not add a local-range exception or empty GH_CONFIG_DIR isolation. (src/clawsweeper.ts:7408, 323f7bdf7236)
  • Existing docs describe offline contract: Current docs say pnpm local-review is offline by contract, with no GitHub access, token withholding, empty GH_CONFIG_DIR, disabled web search, and no GitHub writes. (docs/commit-sweeper.md:257, 323f7bdf7236)
  • Rename parsing misses patches: A scratch rename repo produced R050 old.txt new.txt; applying the PR parser shape yielded parsed filename old.txt\tnew.txt and git diff -- <that> returned 0 patch bytes. (src/clawsweeper.ts:16043, 93b521e89df8)

Likely related people:

  • brokemac79: Current blame and the merged Add local-only issue and PR review mode #357 history tie the local-only issue/PR review path and adjacent review-command plumbing to this contributor. (role: introduced local review plumbing; confidence: high; commits: 79e768844291, bb22ac97a417; files: src/clawsweeper.ts, src/commit-sweeper.ts)
  • Vincent Koc: Recent current-main commits changed exact-review capacity and admission behavior adjacent to the local issue/PR review workflow this PR extends. (role: recent exact-review automation contributor; confidence: medium; commits: 54eedf801125, 59f4d6981e65, bd03fe34c0a4; files: src/clawsweeper.ts, .github/workflows/sweep.yml)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels Jun 25, 2026
@anagnorisis2peripeteia anagnorisis2peripeteia marked this pull request as draft June 25, 2026 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant