Add review-patch command for interactive patch inspection

[spoorcc]

Introduces dfetch review-patch which stages the clean upstream source in
the git index and applies the selected patches to the working tree, so any
diff-aware editor sees git diff (working tree vs index) showing exactly
what the patches contribute. The command always restores original state on
exit — no permanent changes to working tree or index.

• New command: dfetch/commands/review_patch.py with --count/-n, --interactive/-i
• GitSuperProject.add_path(), restore_staged(), restore_worktree() for index/worktree control
• GitLocalRepo.add_path(), restore_staged(), restore_worktree() with -- pathspec hardening
• SVN superprojects supported (with a warning; no staging step; uses svn diff hint)
• Interactive TUI uses read_key()/Screen for ← → step-through (zero fetches per step)
• Only one fetch per review session (clean upstream); patches applied/reversed directly
• --count validated >= 0; reported count clamped to min(requested, total)
• 9 unit tests, 4 git BDD scenarios
• Threat model updated with review-patch and transient git index mutation note
• Documentation and changelog updated

Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_017zY8BoH65KBX6cz7Pm8aeF

Summary by CodeRabbit

Release Notes

• New Features

  • Added review-patch command to interactively inspect patches without permanently applying them
  • Support for reviewing specific patch counts via --count
  • Support for stepping through patches one-by-one in interactive mode
  • Automatic restoration of working tree and index after review

• Documentation

  • Added patching guide covering the new review feature
  • Added interactive demo showcasing patch review workflow

• Tests

  • Added comprehensive test suite for patch review functionality
  • Added BDD scenarios for Git and SVN workflows

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds the dfetch review-patch command that resets a subproject to clean upstream (patch_count=0), stages it in the Git index, applies configured patches to the working tree for diff-based inspection, and restores original state via try/finally. Includes an interactive TUI for stepping through patches one-by-one. Extracts a shared Command._iter_projects helper, refactors UpdatePatch to use it, adds Git index methods, unit tests, BDD scenarios, documentation, and CI steps.

Changes

review-patch command feature

Layer / File(s)	Summary
Shared Command._iter_projects helper and UpdatePatch refactor dfetch/commands/command.py, dfetch/commands/update_patch.py	Adds Command._iter_projects static method with per-project RuntimeError catching and aggregate re-raise; refactors UpdatePatch.__call__ to delegate project iteration to _iter_projects, removing manual had_errors tracking and in_directory usage.
Git index operations: add_path, restore_staged, restore_worktree dfetch/vcs/git.py, dfetch/project/gitsuperproject.py	Adds three new GitLocalRepo methods (add_path via git add, restore_staged via git restore --staged, restore_worktree via git restore) and forwards them through matching new methods on GitSuperProject.
SubProject.apply_patches public method dfetch/project/subproject.py	Exposes apply_patches(count: int = -1) as a public API on SubProject, delegating to the existing _apply_patches internal helper.
ReviewPatch command: CLI, review flow, and interactive TUI dfetch/commands/review_patch.py	Implements the complete command: argument parsing for projects, --count, --interactive; TTY and count validation; per-project eligibility checks (no patch, never fetched, local changes); update(patch_count=0) baseline setup; add_path staging for Git; interactive _step_tui with _draw_tui_frame / _apply_step / _silent_patch_ng; guaranteed restoration via nested try/finally.
CLI registration dfetch/__main__.py	Imports dfetch.commands.review_patch and calls ReviewPatch.create_menu(subparsers) to register the subcommand.
Unit tests for ReviewPatch tests/test_review_patch.py	Adds mock-based tests covering Git all-patches, Git count=1, SVN non-Git path, three skip scenarios, and three error conditions (no VCS superproject, non-TTY interactive, negative count).
BDD feature scenarios and Behave step helpers features/review-patch-in-git.feature, features/review-patch-in-svn.feature, features/steps/git_steps.py	Adds Gherkin scenarios for Git (apply all, --count 0, no patch, local changes) and SVN (apply all, --count 0) with output/state assertions; adds @then Behave step asserting git status --porcelain reports no changes.
Documentation, changelog, demo, and asciicast doc/howto/patching.rst, CHANGELOG.rst, doc/generate-casts/review-patch-demo.sh, doc/asciicasts/review-patch.cast, doc/generate-casts/generate-casts.sh	Adds "Reviewing a patch" section to the patching guide, changelog entry, Bash demo script, recorded asciicast, and cast generation step.
CI integration and security metadata .github/workflows/run.yml, security/tm_usage.py	Adds dfetch review-patch step to test-cygwin and matrix run CI jobs; extends tm_usage.py description to document transient Git index staging and potential inconsistency on interrupted execution.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

• Add an editor-friendly Git working tree review for vendored patches #1283: This PR directly implements the dfetch review-patch command proposed in that issue, including clean upstream staging in the Git index, patch application to the working tree, interactive TUI stepping, and guaranteed state restoration.

Possibly related PRs

• dfetch-org/dfetch#944: This PR's refactor of UpdatePatch.__call__ to use Command._iter_projects directly builds on the PR that originally introduced the UpdatePatch command and its per-project iteration logic.

Suggested labels

enhancement

Suggested reviewers

• ben-edna

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 63.46% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'Add review-patch command for interactive patch inspection' accurately and clearly describes the main change—introduction of a new review-patch command with interactive capabilities, which is the central focus of this changeset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/review-patch-command-xyosoo

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@dfetch/commands/review_patch.py`:
- Around line 113-200: The `_review_project` method has too many branches and
conditional paths, exceeding the cyclomatic complexity limit of 8. Extract the
guard validations (checking for patch existence, on_disk_version, and local
changes) into a separate helper method that returns early if validation fails,
move the interactive review logic and non-interactive logic into their own
helper methods, and simplify the main method to orchestrate validation,
application, and restoration in a clearer sequence. This will distribute the
branching logic across focused helper methods while keeping the main method as a
clear orchestrator.
- Around line 70-78: The --count argument lacks validation, allowing negative
integers that produce unexpected Python slice behavior rather than a meaningful
CLI contract. Add a custom type validator to the add_argument call for --count
to ensure only positive integers are accepted. Additionally, update the logic
around line 173 where the raw count is forwarded to patch_count to validate
against negative values and clamp to valid ranges. Finally, modify the reporting
logic around line 181 to track and report the actual number of patches that were
successfully applied, not the requested count, since the effective count may
differ from what was requested.
- Around line 183-187: The logger.print_info_line call in the review_patch
function currently hardcodes the instruction to use git diff, but this is
inconsistent with non-Git superprojects like SVN that should use their own diff
commands. Make the diff command suggestion in the message VCS-aware by checking
the project's VCS type and conditionally including the appropriate diff command
(git diff for Git projects, svn diff for SVN projects, etc.) in the status
message printed to the user.

In `@dfetch/vcs/git.py`:
- Around line 785-793: Add the `--` separator before the path argument in both
the add_path and restore_staged methods to prevent Git from interpreting
option-style paths as flags. In add_path, insert `"--"` between `"add"` and
`path` in the command list passed to run_on_cmdline. In restore_staged, insert
`"--"` between `"--staged"` and `path` in the command list passed to
run_on_cmdline. This ensures Git treats the path as a positional argument rather
than a potential option flag.

In `@tests/test_review_patch.py`:
- Around line 79-97: Add a new test function to validate that negative count
values are rejected by the ReviewPatch command. Create a test that instantiates
ReviewPatch, mocks the required dependencies (create_super_project,
create_sub_project, in_directory, is_tty) similar to
test_review_count_1_uses_patch_count_1, and then calls cmd(_make_args(count=-1))
while asserting that this raises an appropriate validation error or exception.
This ensures the CLI contract maintains count-based validation and prevents
regression to slice-driven behavior for negative values.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1290abec-833f-46ef-9d9d-5ff586b6111b

📥 Commits

Reviewing files that changed from the base of the PR and between 9aea63f and 0ec01e1.

📒 Files selected for processing (11)

• CHANGELOG.rst
• dfetch/__main__.py
• dfetch/commands/command.py
• dfetch/commands/review_patch.py
• dfetch/commands/update_patch.py
• dfetch/project/gitsuperproject.py
• dfetch/vcs/git.py
• doc/howto/patching.rst
• features/review-patch-in-git.feature
• features/review-patch-in-svn.feature
• tests/test_review_patch.py

[coderabbitai]

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@dfetch/commands/review_patch.py`:
- Line 260: The Ctrl-C handler is currently catching KeyboardInterrupt, clearing
the screen, and returning normally, but the UI message advertises "Ctrl-C abort"
which implies the command should abort. Locate the exception handlers that catch
KeyboardInterrupt (near lines 260 and 319-321 in the review_patch.py file) and
modify them to re-raise the KeyboardInterrupt exception after clearing the
screen, rather than returning normally. This will allow the outer finally block
to restore state before the command properly aborts.
- Line 47: The import statement in review_patch.py violates the command-layer
dependency boundary by directly importing from dfetch.terminal, which is not an
allowed dependency. The imports of Screen, is_tty, read_key, BOLD, DIM, and
RESET must be sourced from one of the allowed layers (dfetch.reporting,
dfetch.project, dfetch.manifest, dfetch.vcs, dfetch.util, or dfetch.log). Either
move these terminal primitives to one of the allowed modules or create a
wrapper/facade in an allowed module that exposes these utilities, then update
the import in review_patch.py to import from the allowed layer instead of
directly from dfetch.terminal.
- Around line 137-147: The mutations to the subproject via subproject.update()
and to git_super via git_super.add_path() are occurring before the try/finally
restore guard begins, which means if either call fails, the restore mechanism in
the finally block will not execute and the worktree/index could be left in an
inconsistent state. Move the subproject.update() call (with patch_count=0) and
the conditional git_super.add_path() call to occur after the try block starts,
so they are protected by the restore guard in the finally block. The same issue
also applies to the code in the range around lines 159-174, so ensure all
mutations that need protection occur within the try block.
- Around line 183-202: The ReviewPatch functionality needs to validate patch
files exist and are accessible before treating them as applicable or performing
worktree operations. Add comprehensive patch file validation in the ReviewPatch
method that performs checks similar to those shown in the diff (verifying patch
existence via subproject.patch, confirming the subproject version exists via
on_disk_version(), and checking for local changes via
has_local_changes_in_dir()) to ensure that chosen_count == -1 or any decision to
apply patches is only made when the patch file is actually valid and accessible.
Ensure this validation logic is applied consistently across all locations where
patches are processed (including the locations at lines 217-221, 233-246, and
290-297) before any worktree replacement operations occur, and validate the
actual patch file object when calling Patch.from_file to catch missing or
out-of-root patch files early.

In `@dfetch/project/subproject.py`:
- Around line 90-97: The apply_patches method accepts a count parameter but does
not validate the input, allowing invalid values like -2 to be passed and cause
unexpected behavior in the slicing logic. Add a validation guard at the
beginning of the apply_patches method that raises a ValueError if count is less
than -1, ensuring only -1 (meaning all patches) or positive values are accepted
as documented in the docstring.

In `@features/steps/git_steps.py`:
- Around line 264-266: In the subprocess.check_output call for the git status
command, add the `--` separator argument between the `--porcelain` option and
the path argument to prevent paths starting with a hyphen from being interpreted
as git options. Modify the command list from `["git", "status", "--porcelain",
path]` to `["git", "status", "--porcelain", "--", path]`.
- Line 10: Move the no-name-in-module pylint suppression from the inline comment
on the import statement (from behave import given, then, when) to the
module-level header. Locate the existing pylint disable comment on line 3 in the
module header and add no-name-in-module to that existing disable list, then
remove the inline # pylint: disable=no-name-in-module comment from the import
statement on line 10.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8c8ecd44-472d-43c4-89d7-b69bfa272e16

📥 Commits

Reviewing files that changed from the base of the PR and between cfc9798 and 6312b9a.

⛔ Files ignored due to path filters (1)

• doc/asciicasts/review-patch.gif is excluded by !**/*.gif

📒 Files selected for processing (18)

• .github/workflows/run.yml
• CHANGELOG.rst
• dfetch/__main__.py
• dfetch/commands/command.py
• dfetch/commands/review_patch.py
• dfetch/commands/update_patch.py
• dfetch/project/gitsuperproject.py
• dfetch/project/subproject.py
• dfetch/vcs/git.py
• doc/asciicasts/review-patch.cast
• doc/generate-casts/generate-casts.sh
• doc/generate-casts/review-patch-demo.sh
• doc/howto/patching.rst
• features/review-patch-in-git.feature
• features/review-patch-in-svn.feature
• features/steps/git_steps.py
• security/tm_usage.py
• tests/test_review_patch.py

[coderabbitai]

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Route terminal primitives through an allowed layer.

dfetch.commands.review_patch imports dfetch.terminal directly, which violates the command-layer dependency boundary. Move or expose Screen, is_tty, read_key, and the style constants through an allowed lower layer before merging.

As per coding guidelines, dfetch/commands/*.py commands may only import from dfetch.reporting, dfetch.project, dfetch.manifest, dfetch.vcs, dfetch.util, and dfetch.log.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dfetch/commands/review_patch.py` at line 47, The import statement in
review_patch.py violates the command-layer dependency boundary by directly
importing from dfetch.terminal, which is not an allowed dependency. The imports
of Screen, is_tty, read_key, BOLD, DIM, and RESET must be sourced from one of
the allowed layers (dfetch.reporting, dfetch.project, dfetch.manifest,
dfetch.vcs, dfetch.util, or dfetch.log). Either move these terminal primitives
to one of the allowed modules or create a wrapper/facade in an allowed module
that exposes these utilities, then update the import in review_patch.py to
import from the allowed layer instead of directly from dfetch.terminal.

Source: Coding guidelines

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Start the restore guard before mutating the project.

subproject.update(... patch_count=0) and git_super.add_path(...) both mutate the worktree/index before the try/finally begins. If either raises, the command can leave the project at clean upstream or with a staged index, bypassing the restore path.

🛡️ Proposed restore-scope fix

         saved_metadata = Path(subproject.metadata_path).read_bytes()
         total_patches = len(list(subproject.patch))
-        subproject.update(
-            force=True,
-            ignored_files_callback=_ignored,
-            patch_count=0,
-            eol_preferences_callback=superproject.eol_preferences,
-        )
-
-        if git_super is not None:
-            git_super.add_path(subproject.local_path)
-
         chosen_count = count if count is not None else -1
         effective = (
             total_patches if chosen_count == -1 else min(chosen_count, total_patches)
         )
@@
         )
         worktree_fully_patched = False
         try:
+            subproject.update(
+                force=True,
+                ignored_files_callback=_ignored,
+                patch_count=0,
+                eol_preferences_callback=superproject.eol_preferences,
+            )
+
+            if git_super is not None:
+                git_super.add_path(subproject.local_path)
+
             worktree_fully_patched = _apply_review(
                 subproject, project.name, chosen_count, interactive, info_msg
             )

Also applies to: 159-174

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dfetch/commands/review_patch.py` around lines 137 - 147, The mutations to the
subproject via subproject.update() and to git_super via git_super.add_path() are
occurring before the try/finally restore guard begins, which means if either
call fails, the restore mechanism in the finally block will not execute and the
worktree/index could be left in an inconsistent state. Move the
subproject.update() call (with patch_count=0) and the conditional
git_super.add_path() call to occur after the try block starts, so they are
protected by the restore guard in the finally block. The same issue also applies
to the code in the range around lines 159-174, so ensure all mutations that need
protection occur within the try block.

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Validate patch files before replacing the worktree.

SubProject._apply_patches skips missing or out-of-root patch files, but ReviewPatch still treats chosen_count == -1 as fully restored/applicable and may only unstage Git afterwards. A missing patch can leave the working tree at clean upstream instead of the original patched state; interactive mode also feeds raw manifest paths directly to Patch.from_file.

🛡️ Proposed pre-validation shape

-from collections.abc import Callable, Generator
+from collections.abc import Callable, Generator, Sequence
@@
 def _can_review_project(
@@
     return True
+
+
+def _patch_paths_for_review(
+    patches: Sequence[str],
+    project_name: str,
+) -> list[str] | None:
+    """Return normalized patch paths, or None when review should be skipped."""
+    cwd = Path(".").resolve()
+    normalized: list[str] = []
+    for patch in patches:
+        patch_path = (cwd / patch).resolve()
+        try:
+            relative_patch_path = patch_path.relative_to(cwd)
+        except ValueError:
+            logger.print_warning_line(
+                project_name,
+                f'skipped - patch "{patch}" is outside {cwd}',
+            )
+            return None
+        if not patch_path.exists():
+            logger.print_warning_line(
+                project_name,
+                f'skipped - patch "{patch}" does not exist',
+            )
+            return None
+        normalized.append(relative_patch_path.as_posix())
+    return normalized

         if not _can_review_project(superproject, subproject, project.name):
             return
 
+        patches = _patch_paths_for_review(subproject.patch, project.name)
+        if patches is None:
+            return
+
         saved_metadata = Path(subproject.metadata_path).read_bytes()
-        total_patches = len(list(subproject.patch))
+        total_patches = len(patches)
@@
             worktree_fully_patched = _apply_review(
-                subproject, project.name, chosen_count, interactive, info_msg
+                subproject, project.name, chosen_count, interactive, info_msg, patches
             )

 def _apply_review(
     subproject: SubProject,
     project_name: str,
     chosen_count: int,
     interactive: bool,
     info_msg: str,
+    patches: list[str],
 ) -> bool:
@@
     if interactive:
-        _step_tui(list(subproject.patch), subproject.local_path, project_name)
+        _step_tui(patches, subproject.local_path, project_name)
         return False

Also applies to: 217-221, 233-246, 290-297

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dfetch/commands/review_patch.py` around lines 183 - 202, The ReviewPatch
functionality needs to validate patch files exist and are accessible before
treating them as applicable or performing worktree operations. Add comprehensive
patch file validation in the ReviewPatch method that performs checks similar to
those shown in the diff (verifying patch existence via subproject.patch,
confirming the subproject version exists via on_disk_version(), and checking for
local changes via has_local_changes_in_dir()) to ensure that chosen_count == -1
or any decision to apply patches is only made when the patch file is actually
valid and accessible. Ensure this validation logic is applied consistently
across all locations where patches are processed (including the locations at
lines 217-221, 233-246, and 290-297) before any worktree replacement operations
occur, and validate the actual patch file object when calling Patch.from_file to
catch missing or out-of-root patch files early.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not swallow Ctrl-C when the UI says abort.

The frame advertises Ctrl-C abort, but the handler clears the screen and returns normally. Re-raise after clearing so the command aborts after the outer finally restores state.

🛠️ Proposed signal-handling fix

         try:
             key = read_key()
         except KeyboardInterrupt:
             screen.clear()
-            return
+            raise

Also applies to: 319-321

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dfetch/commands/review_patch.py` at line 260, The Ctrl-C handler is currently
catching KeyboardInterrupt, clearing the screen, and returning normally, but the
UI message advertises "Ctrl-C abort" which implies the command should abort.
Locate the exception handlers that catch KeyboardInterrupt (near lines 260 and
319-321 in the review_patch.py file) and modify them to re-raise the
KeyboardInterrupt exception after clearing the screen, rather than returning
normally. This will allow the outer finally block to restore state before the
command properly aborts.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Validate count in the new public API to reject unsupported negative values.

Line 90 exposes count publicly, but values like -2 flow into slicing logic and apply an unintended subset of patches, which contradicts the documented contract (-1 means all). Add an explicit guard for < -1.

💡 Proposed fix

 def apply_patches(self, count: int = -1) -> None:
     """Apply patches to the already-fetched working tree without fetching from remote.

     Args:
         count: Number of patches to apply (-1 means all).
     """
+    if count < -1:
+        raise RuntimeError("patch count must be >= 0 or -1")
     self._apply_patches(count)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dfetch/project/subproject.py` around lines 90 - 97, The apply_patches method
accepts a count parameter but does not validate the input, allowing invalid
values like -2 to be passed and cause unexpected behavior in the slicing logic.
Add a validation guard at the beginning of the apply_patches method that raises
a ValueError if count is less than -1, ensuring only -1 (meaning all patches) or
positive values are accepted as documented in the docstring.

[coderabbitai]

📐 Maintainability & Code Quality | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify forbidden inline suppressions in this changed module.
rg -n 'pylint:\s*disable|#\s*noqa|#\s*type:\s*ignore|#\s*pyright:\s*ignore' features/steps/git_steps.py

Repository: dfetch-org/dfetch

Length of output: 327

---

🏁 Script executed:

#!/bin/bash
# Check if this is a test file and review the module structure
head -15 features/steps/git_steps.py
echo "---"
# Check for any pylint/pytest configuration
fd -t f '(pytest\.ini|pylintrc|setup.cfg|pyproject\.toml|tox\.ini)' | head -5

Repository: dfetch-org/dfetch

Length of output: 634

---

🏁 Script executed:

#!/bin/bash
# Check pyproject.toml for pylint configuration related to behave
cat pyproject.toml | grep -A 20 "\[tool.pylint"

Repository: dfetch-org/dfetch

Length of output: 733

---

🏁 Script executed:

#!/bin/bash
# Verify: is no-name-in-module a real issue or false positive from behave?
# Check if behave is installed and exports these symbols
python3 -c "from behave import given, then, when; print('behave exports found')" 2>&1
echo "---"
# Check if there are other test files with similar pattern
rg -l 'from behave import' --type py | head -5

Repository: dfetch-org/dfetch

Length of output: 279

---

🏁 Script executed:

#!/bin/bash
# Check if no-name-in-module can be added to the module-level header
# and if there are other behave step files with the same issue
rg 'from behave import' features/steps/ -A 1 -B 1

Repository: dfetch-org/dfetch

Length of output: 1324

---

🏁 Script executed:

#!/bin/bash
# Check the module-level headers in behave step files
for file in features/steps/{git_steps,generic_steps,add_steps}.py; do
  echo "=== $file ==="
  head -5 "$file"
  echo
done

Repository: dfetch-org/dfetch

Length of output: 888

---

Move the no-name-in-module suppression to the module-level header.

The inline # pylint: disable=no-name-in-module on line 10 violates repository policy. Per coding guidelines, inline suppressions are disallowed except in module-level headers (lines 1–5 of test files). Add no-name-in-module to the existing pylint disable list on line 3, then remove the inline comment from line 10.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@features/steps/git_steps.py` at line 10, Move the no-name-in-module pylint
suppression from the inline comment on the import statement (from behave import
given, then, when) to the module-level header. Locate the existing pylint
disable comment on line 3 in the module header and add no-name-in-module to that
existing disable list, then remove the inline # pylint:
disable=no-name-in-module comment from the import statement on line 10.

Source: Coding guidelines

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Harden the git path argument with -- before the pathspec.

Without --, a path beginning with - can be parsed as an option. Use ["git", "status", "--porcelain", "--", path] for unambiguous behavior.

Suggested patch

-        result = subprocess.check_output(
-            ["git", "status", "--porcelain", path], text=True
-        )
+        result = subprocess.check_output(
+            ["git", "status", "--porcelain", "--", path], text=True
+        )

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	result = subprocess.check_output(
	["git", "status", "--porcelain", path], text=True
	)
	result = subprocess.check_output(
	["git", "status", "--porcelain", "--", path], text=True
	)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@features/steps/git_steps.py` around lines 264 - 266, In the
subprocess.check_output call for the git status command, add the `--` separator
argument between the `--porcelain` option and the path argument to prevent paths
starting with a hyphen from being interpreted as git options. Modify the command
list from `["git", "status", "--porcelain", path]` to `["git", "status",
"--porcelain", "--", path]`.