fix(git): commit only requested paths, handle deletions, label renames (#942)

[frankbria]

Closes #942.

Five defects, each of which put wrong data in the user's repository or their patch file.

1. create_commit committed the entire index

It staged the requested files, then called repo.index.commit() — which writes the whole index. Anything already staged (an agent run's leftovers, say) was swept into the user's commit, while files_changed reported only the count they asked for.

Now commits via git commit -- <paths>, so unrelated staged work stays staged and out of the commit. A test asserts both halves: the unrelated file is not in the commit, and is still staged afterwards — excluding it must not silently discard the user's other work.

2. Deletions were silently dropped

Any path that did not exist was skipped. So a deletion get_status offered as committable vanished from a 201 response, and a deletions-only request failed with "None of the specified files exist".

Deletions of tracked paths are now staged with index.remove. Genuinely unknown paths come back in a new CommitResult.skipped rather than disappearing from the response entirely.

3. Every rename was labelled "modified"

get_diff_stats reported numstat's raw old => new string as the path — a value matching no file — and the rename marker in the diff header was never found, so the type was always modified.

It now parses the new path (including the dir/{a => b}/f form git uses for directory moves) and labels renamed. The per-file re.search over the whole diff is replaced by a single _index_diff_sections() pass; the old shape was O(files × diff size), worst on exactly the large diffs where it matters.

4. staged_only=True exported unstaged work

On an empty index it fell back to plain git diff — writing unstaged changes to a file whose name and PatchInfo both claim "staged".

Now raises "No staged changes to export". Exporting the wrong changes is worse than exporting none.

5. .codeframe/ was never ignored

The sandbox code assumed it, but nothing wrote the rule — so cf commit --all (or any git add -A) could stage state.db, WAL files and worktree gitlinks into the user's history.

create_or_load_workspace now writes .codeframe/.gitignore containing *. Self-ignoring on purpose: the repo's root .gitignore belongs to the user, may be committed, and is not ours to edit. A self-ignoring directory needs no cooperation from it, works in a repo with no .gitignore at all, and disappears with the directory. An existing marker is never overwritten.

Acceptance criteria

• create_commit commits only the requested paths; a test with an unrelated pre-staged file asserts it is excluded
• Deleted paths are staged and committed; a deletions-only request succeeds and skipped paths are returned
• get_diff_stats reports renamed with the new path and indexes the diff body once
• export_patch(staged_only=True) reports "no staged changes" instead of exporting unstaged work
• .codeframe/ is ignored; a test asserts git add -A stages nothing under it

Tests

15, against a real git repo rather than mocks — the pre-staged-file and deletions-only cases are only meaningful against real index behaviour, and a mocked index would have happily confirmed the broken version.

Regression sweep: 707 passed across git/commit/patch/workspace/artifact selections. ruff and mypy codeframe/ clean.

Known limitations

• git commit -- <paths> bypasses repo.index.commit(), so GitPython's in-memory index object is stale afterwards. Nothing in this codebase reuses it across a commit, but a future caller that does would need to repo.index.reset().
• The .codeframe/.gitignore guard only applies at workspace creation. Existing workspaces get it on their next create_or_load_workspace only if the directory was removed; a migration for already-initialised repos is not included.

[github-actions]

Claude finished @frankbria's task in 9m 54s —— View job

---

GLM precision review — 3 defects found

Scope: concrete defects only (logic errors, error paths, security, data loss, races, contract breaks, resource leaks). Style/architecture/naming/coverage left to the general review. Findings posted as inline comments:

severity	file:line	finding
major	codeframe/core/git.py:282	_is_tracked checks the index (ls-files) but the docstring says HEAD — an already-staged deletion (git rm, or git add of a worktree-deleted file) reads as untracked, so POST /git/commit for it raises None of the specified files exist (400) instead of committing the deletion. Contradicts AC #2.
major	codeframe/core/git.py:59	CommitResult.skipped is computed but never surfaced — CommitResultResponse (ui/routers/git_v2.py:65) omits it, so the HTTP 201 still silently drops unknown paths. The PR's "a 201 cannot silently drop a path" guarantee does not hold over the API.
minor	codeframe/core/git.py:407	Rename change_type detection uses unanchored substring match ("rename from"/"rename to" in file_section); a modified file whose hunk contains that text is mislabeled renamed. The section indexer already anchors the marker — this should too.

Most severe first. No critical/data-loss or security issues found. The five targeted defects are fixed correctly for their primary cases; the findings above are gaps in two of them (the deletion case misses already-staged removals; the skipped-path visibility doesn't reach the API) plus one false-positive in the rename labeling.

• Gather PR diff and context
• Check existing PR comments to avoid duplicates
• Read changed files + callers/callees
• Confirm each finding against real code
• Post inline comments for confirmed defects
• Post summary table

[github-actions]

GLM precision review (re-run on synchronize)

Re-verifying the current commit. Three findings were already posted on the prior run (git.py:282, :59, :407); I will not repeat those and will only surface new defects against the current diff.

• Gather current diff + confirm commit state vs. prior review
• Read changed files + callers/callees
• Hunt for new defects (logic, error paths, security, data loss, races, contracts)
• Confirm each finding against real code with a concrete failure scenario
• Post inline comments for new confirmed findings
• Post summary table

View job run

[github-actions]

Claude finished @frankbria's task in 6m 16s —— View job

---

GLM precision review (re-run on synchronize)

Re-verifying the current commit. Three findings were already posted on the prior run (git.py:282, :59, :407); I will not repeat those and will only surface new defects against the current diff.

• Gather current diff + confirm commit state vs. prior review
• Read changed files + callers/callees
• Hunt for new defects (logic, error paths, security, data loss, races, contracts)
• Confirm each finding against real code with a concrete failure scenario
• Post inline comments for new confirmed findings
• Post summary table

Result

✅ GLM review: no new defects found against the current commit (1725218).

The three prior findings are all correctly resolved, and the fixes introduce no regressions I can substantiate with a concrete failure scenario:

prior finding	fix in 1725218	verified
_is_tracked checked the index, not HEAD (git.py:282)	_is_tracked now checks index or HEAD via ls-tree HEAD	an already-staged deletion (git rm, absent from the index) is found in HEAD → to_remove; covered by test_an_already_staged_deletion_is_committable
second-order: index.remove on the now-absent index entry would fail	guarded by still_indexed = [p for p in to_remove if _in_index(repo, p)]	removal is staged only for paths still indexed; already-staged removals skip index.remove and are still committed via git commit -- <path> (--only mode records the working-tree absence)
skipped computed but dropped by the API (git.py:59)	added to CommitResultResponse and wired at git_v2.py:210	test_skipped_reaches_the_api_response asserts the field on the response model
rename check unanchored (git.py:407)	`re.search(r"^rename (from	to) ", file_section, re.MULTILINE)`

Spot-checks I confirmed against the code (no defect in any):

• git commit -m msg -- *committed runs in git's default --only mode: it commits the working-tree state of the listed pathspecs and leaves unrelated staged paths staged — which is exactly defect Add Claude Code GitHub Workflow #1's contract. Paths are passed as separate argv elements with a -- separator (GitPython → subprocess list, no shell), so neither the message nor the pathspecs are injectable.
• Deletion paths reach to_remove only when _is_tracked is true (index or HEAD); committed = set(to_add) | set(to_remove), so files_changed and skipped partition the request exactly with no double-count and no silent drop.
• _numstat_new_path handles both old => new and the dir/{old => new}/file brace form; the result key matches _index_diff_sections's new_path key (from the diff --git a/… b/<new> header), so the rename section is found and the anchored marker fires. Rename-to target is also indexed, but get_diff_stats always looks up the new path, so there's no stale-section hazard.
• export_patch now raises "No staged changes to export" before the unstaged fallback when staged_only=True; the fell_back_to_plain_unstaged/actual_staged_used flags keep the stats command consistent with whichever diff was actually written in the non-staged path.
• .codeframe/.gitignore (*) self-ignores the state dir; the rule applies even though the marker itself is ignored, an existing marker is never overwritten, and a read-only checkout degrades gracefully (logged warning, not a crash).

Edge cases considered and judged not reportable defects (no concrete failure scenario / pre-existing, not introduced here): paths containing spaces (git quotes them in both the diff --git header and numstat, so rename detection fails — but the old code had the identical a//b/ limitation, so this is not a regression); a git commit -- <paths> request where the named paths have no actual changes now surfaces a 500 instead of silently committing unrelated staged work (more correct, not less).

No inline comments posted — nothing new cleared the concrete-failure bar.