Skip to content

fix: scope patch audits to the selected project root - #210

Merged
steipete merged 1 commit into
mainfrom
fix/phase-five-project-audit
Sep 12, 2026
Merged

steipete merged 1 commit into
mainfrom
fix/phase-five-project-audit

Conversation

@steipete

Copy link
Copy Markdown
Contributor

What Problem This Solves

clawpatch --root app fix ... could reject a clean monorepo project because Git reports paths relative to the repository, while Clawpatch filtered state and read file contents relative to app. State files and sibling changes looked like source edits, and actual content edits could be fingerprinted at nonexistent app/app/... paths. Rename audit records also omitted the original path.

User Impact

Nested-project repairs ignore their own state and sibling-project edits, fingerprint the correct source files, and record both sides of renames using project-relative paths.

Why This Change Was Made

Reuse the existing dirtyFiles Git boundary, which already canonicalizes project and Git roots, filters outside-project paths, and preserves rename pairs. Remove the separate status parser from change auditing and tighten the snapshot return type to its actual always-present map.

Evidence

  • Three new regression tests fail on the previous code: clean project/state filtering, changed-content fingerprints, and staged rename paths.
  • Before: built CLI with a clean app project, generated state, and a sibling file exits 3: dirty worktree blocks fix.
  • After: the same built-CLI fixture completes fix --provider mock with exit 0 and status applied, preserving the sibling file. No model API calls are involved in this deterministic workflow proof.
  • pnpm typecheck, pnpm lint, focused audit/Git/workflow tests (131 passed), and pnpm build pass.
  • Isolated Codex autoreview: scoped-clean at P0–P2.

@steipete
steipete requested a review from a team as a code owner September 12, 2026 23:24
@clawsweeper

clawsweeper Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

🦞👀
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 P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. 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 Sep 12, 2026
@clawsweeper

clawsweeper Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed September 12, 2026, 7:26 PM ET / 23:26 UTC.

ClawSweeper review

What this changes

The PR makes patch audits use project-relative Git paths, exclude project state and sibling changes, and record both sides of renames, with regression tests and documentation.

Merge readiness

Ready for maintainer review

This remains a useful, focused fix: current main and v0.8.0 retain the faulty audit path. No blocking correctness or security defect was found.

Priority: P2
Reviewed head: 2aa98850e403cf39d1d3abb53d6f226f0abd8f1b

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused reuse-based repair with relevant CLI evidence and regression coverage, with no blocking finding.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (terminal): The supplied built-CLI before/after result exercises fixCommand through the real Git audit in a nested project: the state/sibling-induced rejection becomes an applied repair with the sibling preserved. Model work is mocked, but the changed audit boundary is exercised; fingerprint and rename cases have supplemental regression coverage.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The supplied built-CLI before/after result exercises fixCommand through the real Git audit in a nested project: the state/sibling-induced rejection becomes an applied repair with the sibling preserved. Model work is mocked, but the changed audit boundary is exercised; fingerprint and rename cases have supplemental regression coverage.
Evidence reviewed 6 items Current main still contains the defect: Main parses repository-relative status paths directly, retains only primary rename paths, and then fingerprints them relative to the selected project root.
Existing Git boundary provides the needed behavior: dirtyFiles canonicalizes roots, preserves both rename paths, and excludes paths outside the project. The audit now delegates to it while retaining state filtering and the non-Git snapshot fallback.
Focused regression coverage: Three new tests cover nested state and sibling exclusion, successive source-content fingerprints, and both staged-rename paths. Existing workflow coverage also exercises non-Git fallback and project-relative Git filtering. Tests were inspected, not executed during this read-only review.
Findings None None.
Security None None.

How this fits together

Clawpatch checks workspace changes before running a repair and fingerprints changed files to record the patch. Those records later identify the files used by the explicit PR-creation workflow.

flowchart TD
  A[Selected project root] --> B[Git workspace changes]
  B --> C[Project-relative path filtering]
  C --> D[Exclude project state]
  D --> E[Clean workspace check]
  D --> F[Before and after fingerprints]
  F --> G[Patch file records]
  G --> H[Explicit PR creation]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Implementation and regression coverage Production +14/-17; tests +61/-0; 3 regression cases The repair removes duplicated auditing logic while covering the three reported failure modes.

Technical review

Best possible solution:

Use one project-relative Git boundary for review filtering and patch auditing, preserving state exclusion, rename pairs, and non-Git fallback behavior.

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

Yes: main combines repository-relative Git paths with project-relative state filtering and file reads, establishing the nested-root failure directly. The contributor reports a matching before/after CLI run; this review did not execute it.

Is this the best way to solve the issue?

Yes: reusing the existing dirtyFiles boundary is the narrowest maintainable repair and matches the downstream expectation that patch paths are project-relative.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

  • add P2: This repairs a bounded nested-project workflow defect and inaccurate patch-file auditing.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The supplied built-CLI before/after result exercises fixCommand through the real Git audit in a nested project: the state/sibling-induced rejection becomes an applied repair with the sibling preserved. Model work is mocked, but the changed audit boundary is exercised; fingerprint and rename cases have supplemental regression coverage.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The supplied built-CLI before/after result exercises fixCommand through the real Git audit in a nested project: the state/sibling-induced rejection becomes an applied repair with the sibling preserved. Model work is mocked, but the changed audit boundary is exercised; fingerprint and rename cases have supplemental regression coverage.

Label justifications:

  • P2: This repairs a bounded nested-project workflow defect and inaccurate patch-file auditing.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The supplied built-CLI before/after result exercises fixCommand through the real Git audit in a nested project: the state/sibling-induced rejection becomes an applied repair with the sibling preserved. Model work is mocked, but the changed audit boundary is exercised; fingerprint and rename cases have supplemental regression coverage.
  • proof: sufficient: Contributor real behavior proof is sufficient. The supplied built-CLI before/after result exercises fixCommand through the real Git audit in a nested project: the state/sibling-induced rejection becomes an applied repair with the sibling preserved. Model work is mocked, but the changed audit boundary is exercised; fingerprint and rename cases have supplemental regression coverage.

Evidence

What I checked:

  • Current main still contains the defect: Main parses repository-relative status paths directly, retains only primary rename paths, and then fingerprints them relative to the selected project root. (src/change-audit.ts:35, b979f94701be)
  • Existing Git boundary provides the needed behavior: dirtyFiles canonicalizes roots, preserves both rename paths, and excludes paths outside the project. The audit now delegates to it while retaining state filtering and the non-Git snapshot fallback. (src/git.ts:88, 2aa98850e403)
  • Focused regression coverage: Three new tests cover nested state and sibling exclusion, successive source-content fingerprints, and both staged-rename paths. Existing workflow coverage also exercises non-Git fallback and project-relative Git filtering. Tests were inspected, not executed during this read-only review. (src/change-audit.test.ts:33, 2aa98850e403)
  • Contributor's real CLI evidence: The supplied complete PR body reports a built-CLI nested-project fixture changing from exit 3 with a dirty-worktree error to exit 0 and applied status while preserving the sibling file. The mock provider substitutes model work; the changed CLI audit, filesystem, and Git boundary remain real. The captured context sourceRevision is 460697452add56d2ea2d629f84266e082c0685b7f62ef67a797698f08f9fe736. (2aa98850e403)
  • Release comparison: The latest release is v0.8.0; its audit implementation still directly parses Git status and selects only primaryPath, so the requested repair is not already shipped. (src/change-audit.ts:35, e78e71836812)
  • Feature-history routing: Current-main history identifies prior work by Peter Steinberger on dirty-worktree filtering, status parsing, and machine-path preservation. GitHub commit metadata resolves the filtering commit author to steipete. Some deeper local history reads failed to retrieve missing objects; the GitHub commit patch supplied the relevant historical context. (src/git.ts:92, 580a0b0c10c2)

Likely related people:

  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

@steipete
steipete merged commit ead0ec4 into main Sep 12, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. 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