Skip to content

Fix: re-stage files after task when using ${staged:separator}#173

Draft
mvanhilst-msft wants to merge 4 commits intoalirezanet:masterfrom
mvanhilst-msft:user/mavanhi/fix-staged-separator
Draft

Fix: re-stage files after task when using ${staged:separator}#173
mvanhilst-msft wants to merge 4 commits intoalirezanet:masterfrom
mvanhilst-msft:user/mavanhi/fix-staged-separator

Conversation

@mvanhilst-msft
Copy link
Copy Markdown

Description

Fixes #172

This is what Claude Opus 4.6 thought would be a good fix for this issue. I'm putting this here as a draft to save you some time/tokens if it helps. No need to merge this PR.

Problem

When a task uses ${staged:;} (the separator variant), files modified by the task are never re-staged before the commit is finalized. The commit records the original unformatted content, and formatting changes are left as unstaged modifications.

This is because AddStaticMatchedFiles creates a single ArgumentInfo(ArgumentTypes.Static, ...) for the combined string (e.g., --include=file1;file2). The ExecutableTaskFactory only routes to StagedTask — which contains the re-staging logic — when it finds ArgumentTypes.StagedFile entries, so the ${staged:;} path always gets a plain ExecutableTask with no re-staging.

Solution

Track individual staged files as metadata alongside the combined static argument. A new ExcludeFromCommandArgs flag on ArgumentInfo distinguishes these tracking entries from actual command-line arguments, so they inform the StagedTask routing and re-staging logic without being passed to the child process.

Changes

ArgumentInfo.cs — Added ExcludeFromCommandArgs property (defaults to false for backward compatibility). When true, the entry is metadata-only and excluded from the process command line.

FileArgumentInfo.cs — Propagates the new excludeFromCommandArgs parameter to the base constructor.

ArgumentParser.csAddStagedFiles now calls AddMatchedFiles with excludeFromCommandArgs: true after AddStaticMatchedFiles, emitting hidden FileArgumentInfo(StagedFile) tracking entries for each matched file. AddMatchedFiles accepts the new optional parameter and passes it through to FileArgumentInfo.

ExecutableTaskFactory.cs — Filters ExcludeFromCommandArgs entries when building the command-line string[] for TaskInfo.Arguments, and when computing total command length for chunk decisions. Chunk creation separates tracking args from command args, appending tracking entries to each chunk's metadata so StagedTask routing still works.

StagedTask.cs — Added canPartialExecute check: when all staged file entries are tracking-only (from ${staged:;} where files are embedded in a combined argument), partial execution via temp-file swapping is skipped since it requires individual file arguments. In this case, all staged files are re-staged after task execution.

Tests

Five new unit tests in ArgumentParserTests.cs:

Test Verifies
ParseAsync_WithStagedVariable_NoSeparator_ReturnsStagedFileArgumentType ${staged} still produces StagedFile args (regression guard)
ParseAsync_WithStagedSeparator_ProducesBothStaticAndStagedFileTracking ${staged:;} produces the combined Static arg and hidden StagedFile tracking entries
ParseAsync_WithStagedSeparator_StagedEntriesHaveCorrectRelativePaths Tracking entries carry the correct relative file paths for git add
ParseAsync_WithStagedSeparator_ResultContainsStagedFileType_EnablingRestagingRoute Result contains StagedFile type, enabling StagedTask routing in the factory
ParseAsync_WithStagedSeparator_NoMatchingFiles_ReturnsOnlyStaticArgs No staged .cs files → no StagedFile entries, only static args

Manual verification

Tested end-to-end with jb cleanupcode and a .cs file with deliberate formatting issues:

  • Before fix: Commit contains unformatted code; formatted changes left unstaged.
  • After fix: Commit contains formatted code; working tree is clean.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or that my feature works
  • I have made corresponding changes to the documentation
  • I have commented my code, particularly in hard-to-understand areas
  • New and existing unit tests pass locally with my changes
  • I did test corresponding changes on Windows
  • I did test corresponding changes on Linux
  • I did test corresponding changes on Mac

@alirezanet
Copy link
Copy Markdown
Owner

Hi @mvanhilst-msft,
Thanks for the PR, it LGTM but it would be nice if you could also add an integration test to simulate the issue and test in docker. Please let me know if you need help.

mvanhilst-msft and others added 3 commits March 28, 2026 11:20
…r re-staging)

Add 3 integration tests:
- StagedSeparator_FormattedChanges_ShouldBeCommitted: verifies
  re-stages formatted files before commit
- StagedSeparator_MultipleFiles_FormattedChanges_ShouldAllBeCommitted:
  same for multiple files
- StagedWithoutSeparator_FormattedChanges_ShouldBeCommitted: control test
  confirming  (no separator) works correctly

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

${staged:separator} pattern does not re-stage files after task execution

2 participants