Repository navigation
fix(pipeline): keep diff paths that contain spaces - #679
Merged
Merged
Conversation
Git leaves a path with spaces unquoted in the "diff --git a/<path> b/<path>"
header, and the parser split that header at the first space. Added and
modified files recovered their new path from the "+++ b/<path>" line, but a
deleted file ("+++ /dev/null") kept the fragment after the last space. Deleting
"data/2024-06-08 - a.json" and "data/2024-06-08 - b.json" gave both files the
path "-", and selection stopped on a duplicate canonical path in the file
manifest. Files with no "---"/"+++" lines (empty, binary and mode-only changes)
kept both fragments, and an added file kept a truncated old path.
Read an unquoted header whose two halves name the same file by splitting at its
middle space. Renames keep the token parsing and take their paths from the
"rename from" and "rename to" lines, which are now decoded when git C-quotes
them.
Fixes #678.
monit-reviewer
approved these changes
Oct 21, 2026
monit-reviewer
left a comment
monit-reviewer
left a comment
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: fd258ed08d60
Profile: claude-monit-reviewer - Posting as: monit-reviewer
Summary
| Reviewer | Findings |
|---|---|
| go:implementation-tests | 0 |
| harness-engineering:repo-health | 0 |
| security:code-auditor | 0 |
Reviewer Coverage
Complete coverage details are retained in the local coverage.json artifact.
go:implementation-tests— complete (broad); scope: 3 files; examples:internal/pipeline/diff.go,internal/pipeline/diff_test.go,internal/pipeline/pipeline_test.go; body-inspected: 3 files; examples:internal/pipeline/diff.go,internal/pipeline/diff_test.go,internal/pipeline/pipeline_test.go; skipped: none; constraints: Reviewed the diff only; tests were not executed.harness-engineering:repo-health— complete (constrained); scope: 3 files; examples:internal/pipeline/diff.go,internal/pipeline/diff_test.go,internal/pipeline/pipeline_test.go; body-inspected: 3 files; examples:internal/pipeline/diff.go,internal/pipeline/diff_test.go,internal/pipeline/pipeline_test.go; skipped: none; constraints: Static review of the diff only; tests were not run.security:code-auditor— complete (broad); scope: 1 file; examples:internal/pipeline/diff.go; body-inspected: 1 file; examples:internal/pipeline/diff.go; skipped: none; constraints: Reviewed the diff of internal/pipeline/diff.go only; did not trace downstream consumers of manifest paths beyond the diff.
0 PR discussion threads considered. 0 summarized; 0 resolved.
Completed in 1m 15s | ~$1.31 (est.) | claude-sonnet-5-5 | cr 0.10.342
| Field | Value |
|---|---|
| Model | claude-sonnet-5-5 |
| Reviewers | go:implementation-tests, harness-engineering:repo-health, security:code-auditor |
| Engine | claude_cli · claude-sonnet-5-5 |
| Reviewed by | cr · monit-reviewer |
| Duration | 1m 15s wall · 1m 39s compute |
| Cost | ~$1.31 (est.) |
| Pricing basis | baseline-2026-10-02/pricing |
| Tokens | 34 in / 4.9k out |
Per-workstream usage
orchestrator-selection— claude-sonnet-5-5- In: 6
- Out: 1.2k
- Cache read: 59.7k
- Cache create: 51.7k
- Cost: ~$0.23 (est.)
- Duration: 19s
go:implementation-tests— claude-sonnet-5-5- In: 6
- Out: 1.3k
- Cache read: 61.7k
- Cache create: 55.7k
- Cost: ~$0.25 (est.)
- Duration: 22s
harness-engineering:repo-health— claude-sonnet-5-5- In: 10
- Out: 1.2k
- Cache read: 165.4k
- Cache create: 56.9k
- Cost: ~$0.27 (est.)
- Duration: 22s
security:code-auditor— claude-sonnet-5-5- In: 6
- Out: 827
- Cache read: 61.9k
- Cache create: 52.0k
- Cost: ~$0.23 (est.)
- Duration: 20s
orchestrator-rollup— claude-sonnet-5-5- In: 6
- Out: 450
- Cache read: 134.0k
- Cache create: 74.4k
- Cost: ~$0.33 (est.)
- Duration: 14s
piekstra
marked this pull request as ready for review
October 21, 2026 15:13
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #678.
Git quotes a path in a
diff --git a/<path> b/<path>header for special characters, not for spaces.parseDiffGitPathssplit the header at the first space, sodata/2024-06-08 - a.jsonbecame old pathdata/2024-06-08and new path-. The+++ b/<path>line corrected the new path of added and modified files, but a deleted file has+++ /dev/nulland kept-. Two such deletions produced two manifest rows with the path-, and selection failed withpipeline: index selection manifest: duplicate canonical path "-" in file manifest.The same split gave wrong paths in other shapes:
old_path.---/+++lines, so both paths were fragments.rename from/rename tolines (paths with non-ASCII bytes or control characters) kept their quotes and octal escapes.Changes
parseDiffGitPathsreads an unquoted header whose two halves name the same file by splitting at its middle space. A rename's halves differ, so it keeps the existing token parsing and takes its paths from therename from/rename tolines.rename from/rename topaths are decoded with the unquoting the---/+++lines already used.Validation
TestParseUnifiedDiffUnquotedPathsWithSpacescovers deleted, added, empty, binary, mode-only, pure-rename and content-changing-rename files with spaces, in git's output shape (a tab after a spaced---/+++path). Onmainevery non-rename case fails.TestParseUnifiedDiffQuotedRenamePathscovers a C-quoted rename. Onmainthe paths keep their quotes and escapes.TestSelectionOnlyAcceptsDeletedPathsWithSpacesdeletes two spaced files in a git fixture repository and runs selection on the realgit diffoutput. Onmainit fails with the error from the issue.go test -race ./...andgolangci-lint runwith the Makefile'skeyring_nopassagetag: pass.Model: Claude Opus 5.5 (context unknown, xhigh effort) in Claude Code.