Skip to content

fix(work): read attempt diff paths literally, not as Git pathspecs - #301

Open
NopeCharon wants to merge 1 commit into
Code-Amadeus:mainfrom
NopeCharon:fix/literal-attempt-pathspecs
Open

NopeCharon wants to merge 1 commit into
Code-Amadeus:mainfrom
NopeCharon:fix/literal-attempt-pathspecs

Conversation

@NopeCharon

Copy link
Copy Markdown
Contributor

What and why

Fixes #300. collect_git_delta builds the committed and working attempt patches with git diff ... -- <changed files>, and Git reads those arguments as pathspecs. Glob characters and pathspec magic in a filename therefore changed which files were diffed:

  • a changed [id].tsx also matched i.tsx and d.tsx. A pre-existing user edit that the baseline filter had correctly listed under excluded_baseline_paths was still written into the attempt patch and shown in the attempt diff canvas as part of the Work;
  • a changed file whose name starts with : was read as pathspec magic, matched nothing, and its changes were missing from the patch.

These paths always come from Git's own status --porcelain=v1 -z and diff --name-only -z output, so they are literal repository paths. run_git already pins core.quotepath=false for machine consumers; it now also passes --literal-pathspecs. No caller passes a pattern: the other callers are rev-parse, branch --show-current, status, and diff with no paths.

Linked Issue for product-semantic or public-contract changes: none (routine fix; tracked in #300)

Change class

  • Routine fix, documentation, test, maintenance, or presentation-only UI
  • Product-semantic or public-contract change discussed in the linked Issue
  • Isolated, default-off experiment

Owning layer: server/local_git.py (run_git, the Host's Git boundary for attempt diffs)

User-visible effect, or none: the attempt diff canvas shows only the attempt's attributed files, with no other files whose names happen to match a glob, and it includes changes to files whose names start with :.

Compatibility or migration impact, or none: none. Attempt deltas recorded before this fix are not rewritten.

Evidence

Commands and manual journeys run:

  • pytest tests/test_work_artifact_registry.py -k literal: the new test_attempt_patch_reads_changed_paths_literally_not_as_globs fails on current main (the patch contains the pre-existing i.tsx hunk) and passes with the fix.

  • pytest tests/test_work_artifact_registry.py tests/test_unified_diff_canvas.py: 47 passed.

  • The 81 test files that drive Git or the Work ledger (1304 tests) were run on Linux both with and without the change: the result is identical. The same 31 tests fail on unmodified main in that environment because AUIP bundle validation needs a Node runtime, which that machine does not have.

  • ruff check . and git diff --check: clean.

  • Manual check with plain Git: git diff --name-only HEAD -- '[id].tsx' lists [id].tsx and i.tsx; with --literal-pathspecs it lists only [id].tsx. A changed :top.txt is listed only with --literal-pathspecs.

  • Relevant Python tests pass

  • CPU/model-less baseline remains supported

  • Electron npm run build passes when Electron code changed

  • Dependency audit passes when dependencies changed

  • Before/after screenshots are attached for visible UI changes

  • Documentation/examples are updated for changed settings or contracts

Final check

  • This PR addresses one coherent problem without unrelated cleanup
  • It does not add a speculative API, fallback, or compatibility path
  • No secrets, local state, model weights, voice material, or restricted assets are included
  • Third-party notices and provenance are preserved

collect_git_delta builds the committed and working attempt patches with
`git diff ... -- <changed files>`. Git interprets those arguments as
pathspecs, so glob characters and pathspec magic in a filename changed
which files were diffed:

- a changed `[id].tsx` also matched `i.tsx` and `d.tsx`, so a
  pre-existing user edit that the baseline filter had correctly listed
  under excluded_baseline_paths was still written into the attempt patch
  and shown in the attempt diff canvas as part of the Work;
- a changed file whose name starts with `:` was read as pathspec magic,
  matched nothing, and its changes were missing from the patch.

The paths always come from Git's own status and name-only output, so
they are literal repository paths. run_git, which already pins
core.quotepath=false for machine consumers, now also passes
--literal-pathspecs. None of its callers passes a pattern.

Adds a regression test in which an attempt edits `[id].tsx` while a
pre-existing dirty `i.tsx` stays untouched; the attempt patch must list
only `[id].tsx`.

Fixes Code-Amadeus#300
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Attempt diff reads changed paths as Git globs and leaks pre-existing edits

1 participant