Skip to content

Support a wildcard segment in the remap command - #980

Merged
alexeyzimarev merged 3 commits into
mainfrom
georgepayne/ai-2878-remap-wildcard
Sep 17, 2026
Merged

alexeyzimarev merged 3 commits into
mainfrom
georgepayne/ai-2878-remap-wildcard

Conversation

@George-Payne

Copy link
Copy Markdown
Member

AI-2878 — no GitHub issue exists for this; it was filed in Linear.

What & why

kcap remap is the only recovery when a transcript's recorded cwd is gone, but a literal from cannot name a family: a prefix rewrite preserves the tail, so ~/dev/repo/worktrees → ~/dev/repo turns .../worktrees/ai-1 into ~/dev/repo/ai-1, which is equally missing. Only an exact cwd == from hit uses to verbatim, making it one entry per dead worktree and a new one every time a tree is cleaned up — one machine needed 24 across two repos. from now takes a single * standing as a whole path segment, matching exactly one segment with the remainder preserved, and the missing-cwd report prints the rule for a family it can see rather than a truncated path list.

Where to look

Rule selection is by how much of the cwd a rule consumed, not by pattern length, with a literal beating a wildcard of equal reach — that is what keeps one path overridable out of a family, and a literal-only config resolves exactly as it did. A malformed pattern is refused when typed and skipped when read back, but --remove deliberately skips the grammar, so an entry hand-written into the config that no import can use stays removable.

Verification

Command Result
dotnet run --project test/Capacitor.Cli.Tests.Unit -- --treenode-filter '/*/*/*Remap*/*' 81 passed, 0 failed
… --treenode-filter '/*/*/ImportMissingCwdsReportTests/*' 22 passed, 0 failed
… --treenode-filter '/*/*/Import*/*' 275 passed, 0 failed
dotnet publish src/Capacitor.Cli/Capacitor.Cli.csproj -c Release no IL3050/IL2026

A prefix rewrite preserves the tail, so it can never collapse a family of
deleted worktrees onto their project root - only an exact cwd match uses `to`
verbatim, which made it one entry per dead tree. Precedence is the length of
the cwd a rule consumed rather than of the pattern, so a literal still wins a
tie and stays usable as a per-path override inside a family.
An unquoted wildcard reaches the CLI already expanded, with no `*` left to
detect it by - so the advice to quote is offered rather than diagnosed.
`--remove` is the dangerous half: the first expanded path can name a real
literal entry, and deleting one silently is worse than storing one.
@George-Payne George-Payne self-assigned this Sep 17, 2026
@linear-code

linear-code Bot commented Sep 17, 2026

Copy link
Copy Markdown

AI-2878

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Support single-segment wildcards in cwd remap rules

✨ Enhancement 🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Allow one whole-segment wildcard to remap deleted worktree families.
• Suggest safe family remaps when imports detect missing sibling worktrees.
• Reject malformed patterns and shell-expanded argument lists before changing configuration.
Diagram

graph TD
  A["Remap CLI"] -->|validate and store| B["CWD Config"] -->|load rules| C["Import Flow"] -->|apply rules| D["CWD Remapper"] -->|resolved path| E["Repo Detection"]
  C -->|missing paths| F["Family Detector"] -->|wildcard hint| G["Missing Report"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Automatically resolve all worktree layouts
  • ➕ Requires no user configuration for recognized directory families.
  • ➕ Could make missing-cwd recovery transparent during import.
  • ➖ Cannot safely infer repositories for externally stored worktrees.
  • ➖ Risks attributing arbitrary similarly shaped paths to the wrong repository.
  • ➖ Would broaden WorktreePathResolver beyond tool-generated dot-directory layouts.
2. Support general glob or regex patterns
  • ➕ Handles variable-depth and more complex directory layouts.
  • ➕ Provides greater flexibility for unusual repository structures.
  • ➖ Makes preserved-tail boundaries and precedence more ambiguous.
  • ➖ Increases validation, escaping, performance, and compatibility complexity.
  • ➖ Adds unnecessary power when observed layouts require exactly one segment.

Recommendation: Keep the constrained single-segment wildcard implemented by this PR. It solves the observed deleted-worktree problem while preserving literal behavior, deterministic precedence, and straightforward validation; automatic attribution remains limited to layouts whose repository ownership can be proven.

Files changed (10) +703 / -62

Enhancement (2) +190 / -45
ImportCommand.csSuggest wildcard remaps for missing worktree families +100/-25

Suggest wildcard remaps for missing worktree families

• Groups multiple missing sibling directories when their grandparent is an existing repository, suppresses redundant path listings, and prints an actionable family remap. Extracts separator-preserving parent trimming for reuse and testability.

src/Capacitor.Cli/Commands/ImportCommand.cs

CwdRemapper.csMatch single-segment wildcard remap patterns +90/-20

Match single-segment wildcard remap patterns

• Parses literal and wildcard source patterns, matches exactly one path segment, and preserves the remaining cwd tail. Rule selection now favors greatest consumed reach, with literal rules winning equal-reach ties and malformed stored rules skipped.

src/Capacitor.Cli/CwdRemapper.cs

Tests (3) +390 / -0
ImportMissingCwdsReportTests.csTest missing worktree family detection and reporting +90/-0

Test missing worktree family detection and reporting

• Covers sibling grouping, repository validation, wildcard suggestions, mixed family and standalone reports, and fallback listing when ownership cannot be established.

test/Capacitor.Cli.Tests.Unit/Commands/ImportMissingCwdsReportTests.cs

RemapCommandTests.csTest remap wildcard validation and CLI safety +109/-0

Test remap wildcard validation and CLI safety

• Verifies exact argument counts, source and destination wildcard validation, verbatim listing, malformed-entry removal, and wildcard normalization behavior.

test/Capacitor.Cli.Tests.Unit/Commands/RemapCommandTests.cs

CwdRemapperTests.csCover wildcard matching, precedence, and portability +191/-0

Cover wildcard matching, precedence, and portability

• Adds broad coverage for exact one-segment matching, tail preservation, interior wildcards, precedence, malformed stored patterns, path boundaries, Windows separators, and filesystem case policies.

test/Capacitor.Cli.Tests.Unit/CwdRemapperTests.cs

Documentation (4) +68 / -10
README.mdDocument wildcard remap syntax and worktree recovery +13/-2

Document wildcard remap syntax and worktree recovery

• Documents single-segment wildcard semantics, precedence, unconditional rewriting, quoting requirements, and family-remap examples. Explains when import can suggest a wildcard rule and why externally stored worktrees require manual mapping.

README.md

CHANGES.mdRecord the wildcard remap design and rationale +33/-0

Record the wildcard remap design and rationale

• Adds a design-history entry covering the deleted-worktree use case, one-segment grammar, consumed-path precedence, malformed-config handling, and missing-family discovery strategy.

docs/CHANGES.md

help-remap.txtExpand remap help with wildcard matching guidance +21/-7

Expand remap help with wildcard matching guidance

• Updates command help to describe wildcard syntax, precedence, unconditional application, quoting, verbatim management, and representative worktree layouts.

src/Capacitor.Cli.Core/Resources/help-remap.txt

help-usage.txtAdvertise wildcard support in command usage +1/-1

Advertise wildcard support in command usage

• Updates the concise remap command description to state that the source path accepts one wildcard segment.

src/Capacitor.Cli.Core/Resources/help-usage.txt

Other (1) +55 / -7
RemapCommand.csValidate wildcard rules and exact argument counts +55/-7

Validate wildcard rules and exact argument counts

• Accepts one whole-segment wildcard only in the source path and rejects malformed patterns or wildcard destinations. Refuses extra add/remove arguments with shell-quoting guidance while retaining the ability to remove unusable hand-edited entries.

src/Capacitor.Cli/Commands/RemapCommand.cs

@qodo-code-review

qodo-code-review Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (2) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Suggested Windows remaps do not work ✓ Resolved 🐞 Bug ≡ Correctness
Description
ReportMissingCwds unconditionally appends /* to family.Parent, so a Windows parent such as
C:\repo\worktrees produces the mixed-separator pattern C:\repo\worktrees/*. Because
FromPattern.Match compares the wildcard head using literal StartsWith semantics, the suggested
command cannot match backslash-form transcript cwds such as C:\repo\worktrees\ai-1, leaving every
affected worktree session unresolved.
Code

src/Capacitor.Cli/Commands/ImportCommand.cs[R2572-2573]

+            display.Line($"{sessions} {word} under {parent}/ ({family.Paths.Count} paths) belong to {project}, which still exists:");
+            display.Line($"  kcap remap '{parent}/*' {project}");
Relevance

●●● Strong

Recent accepted precedent specifically fixes Windows separators in cwd remapping and missing-cwd
reporting.

PR-#111

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The report constructs the suggested wildcard command by interpolating '{parent}/*', hard-coding a
forward slash after the retained raw parent path. Wildcard matching then requires the pattern head
to pass an exact literal StartsWith check; the existing matcher test explicitly confirms that a
slash-form wildcard head does not match a backslash-form cwd, because separator flexibility applies
only while scanning boundaries.

src/Capacitor.Cli/Commands/ImportCommand.cs[2566-2573]
src/Capacitor.Cli/CwdRemapper.cs[86-101]
test/Capacitor.Cli.Tests.Unit/CwdRemapperTests.cs[228-234]
src/Capacitor.Cli/Commands/ImportCommand.cs[2566-2574]
test/Capacitor.Cli.Tests.Unit/CwdRemapperTests.cs[227-234]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ReportMissingCwds` always appends `/*` when constructing wildcard suggestions. On Windows, `family.Parent` normally contains backslashes, producing a mixed-separator rule such as `C:\repo\worktrees/*`; because the wildcard matcher compares the head literally, the emitted rule cannot match normal backslash-form transcript cwds.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/ImportCommand.cs[2566-2574]
- src/Capacitor.Cli/CwdRemapper.cs[86-101]
- test/Capacitor.Cli.Tests.Unit/Commands/ImportMissingCwdsReportTests.cs[201-221]

## Recommended Fix
Build the displayed wildcard pattern using the separator style already present in `family.Parent`, and use the same separator in the descriptive output, rather than hard-coding `/`. Add a Windows-oriented report test using backslash-form paths that takes the emitted suggestion, applies it to a backslash cwd, and verifies that the path remaps successfully to the project.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Nested worktree sessions miss recovery ✓ Resolved 🐞 Bug ≡ Correctness
Description
DetectWorktreeFamilies groups each missing transcript cwd by its immediate parent rather than
finding the deleted worktree root. When sessions ran in .../worktrees/a/src and
.../worktrees/b/src, their parents differ and no family reaches the new suggestion path, even
though one wildcard rule would recover both.
Code

src/Capacitor.Cli/Commands/ImportCommand.cs[R2600-2603]

+        foreach (var path in missingRoots) {
+            var parent = TrimLastSegment(path);
+
+            if (parent is null || TrimLastSegment(parent) is null) continue;
Relevance

●● Moderate

The nested-path recovery concern is plausible, but no closely matching historical acceptance or
rejection precedent appeared.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The report builds its missing set from exact session cwd values and collapse only removes
descendants when an ancestor is already present; it never discovers the first missing ancestor. The
new detector then groups those values solely by immediate parent, while the existing automatic
resolver only handles dot-prefixed managed worktree layouts.

src/Capacitor.Cli/Commands/ImportCommand.cs[2539-2551]
src/Capacitor.Cli/Commands/ImportCommand.cs[2600-2620]
src/Capacitor.Cli/Commands/ImportCommand.cs[2650-2667]
src/Capacitor.Cli/WorktreePathResolver.cs[41-49]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Family detection groups exact transcript working directories by immediate parent, so sessions recorded below different deleted worktree roots are never recognized as siblings.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/ImportCommand.cs[2539-2553]
- src/Capacitor.Cli/Commands/ImportCommand.cs[2592-2623]
- test/Capacitor.Cli.Tests.Unit/Commands/ImportMissingCwdsReportTests.cs[201-241]

## Recommended Fix
Walk each missing cwd toward an existing ancestor and derive the deleted worktree root before grouping paths. Group those derived roots under the family directory, retain descendant session attribution, and add a test using sessions under two different worktree subdirectories such as `ai-1/src` and `ai-2/src`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. One command test bypasses file helpers ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
Remove_drops_an_entry_the_matcher_would_refuse calls Directory.CreateDirectory and
File.WriteAllTextAsync for state rooted at tmp.Path instead of creating the configuration
through TempDir. This leaves fixture-owned state outside the helper API, so its naming and
filesystem safeguards are not consistently applied.
Code

test/Capacitor.Cli.Tests.Unit/Commands/RemapCommandTests.cs[R164-167]

+        Directory.CreateDirectory(tmp.Path);
+        await File.WriteAllTextAsync(
+            AppConfig.GetConfigPath(root),
+            """{"profiles":{},"cwd_remap":[{"from":"/dev/wt-*","to":"/dev/repo"}]}""");
Relevance

●●● Strong

Accepted precedent requires TempDir helper APIs for direct filesystem creation and writes under
temporary roots.

PR-#603
PR-#972

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The rule explicitly forbids direct Directory.CreateDirectory and File.WriteAllText operations
for locations under TempDir when equivalent helper methods exist. The cited added test performs
both operations against its configuration root.

Rule 2767472: Use TempDir helper for all temporary filesystem state in tests
test/Capacitor.Cli.Tests.Unit/Commands/RemapCommandTests.cs[157-167]
test/Capacitor.Tests.Helpers/TempDir.cs[63-73]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new malformed-config test directly creates and writes filesystem state beneath a `TempDir` root rather than using the fixture's file helper.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/Commands/RemapCommandTests.cs[157-167]

## Recommended Fix
Use the injected temp fixture and create `AppConfig.ConfigFileName` with `Tmp.CreateFile(...)`, removing the direct `Directory.CreateDirectory` and `File.WriteAllTextAsync` calls.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Windows path variants split families ✗ Dismissed 🐞 Bug ≡ Correctness
Description
DetectWorktreeFamilies keys parents with StringComparer.Ordinal even though remap paths follow
Windows case-insensitive filesystem semantics. Missing siblings recorded as C:\Repo\worktrees\a
and c:\repo\worktrees\b enter separate one-item groups, so neither group qualifies for a family
suggestion.
Code

src/Capacitor.Cli/Commands/ImportCommand.cs[R2597-2598]

+    internal static List<WorktreeFamily> DetectWorktreeFamilies(IEnumerable<string> missingRoots, Func<string, bool> isRepo) {
+        var byParent = new Dictionary<string, List<string>>(StringComparer.Ordinal);
Relevance

●●● Strong

Recent accepted Windows path-comparison precedent supports aligning family grouping with filesystem
case policy.

PR-#111
PR-#112

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The remapper explicitly selects case-insensitive comparison on Windows, but the new parent
dictionary always uses ordinal comparison. Since family creation requires at least two entries under
the same key, case variants of the same filesystem parent suppress the suggestion.

src/Capacitor.Cli/CwdRemapper.cs[31-37]
src/Capacitor.Cli/Commands/ImportCommand.cs[2597-2618]
src/Capacitor.Cli/Commands/ImportCommand.cs[2639-2642]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Family detection uses ordinal parent keys, causing equivalent Windows paths with different casing to be treated as different families.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/ImportCommand.cs[2597-2620]
- src/Capacitor.Cli/Commands/ImportCommand.cs[2639-2642]
- src/Capacitor.Cli/CwdRemapper.cs[31-37]

## Recommended Fix
Centralize the host filesystem path comparer and use it for family parent grouping, covered-path membership, ordering where appropriate, and descendant attribution. Add Windows-policy tests with case-variant sibling paths to verify they produce one family and the correct session count.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Windows wildcard remaps do not apply ✗ Dismissed 📘 Rule violation ⚙ Maintainability
Description
Apply passes the result of ExpandHome directly to FromPattern.Match, whose literal head
comparison treats / and \ as different characters. On Windows, the newly documented
~/dev/.../* form expands to a mixed-separator head such as C:\Users\alice/dev/.../, so ordinary
backslash-based transcript paths bypass the rule.
Code

src/Capacitor.Cli/CwdRemapper.cs[R50-52]

+            if (!TryParseFrom(ExpandHome(rule.From, home), out var pattern, out _)) continue;

-            var from = ExpandHome(rule.From, home);
+            var matched = pattern.Match(cwd, comparison);
Relevance

●●● Strong

Recent accepted precedent explicitly requires separator-flexible Windows remapping and OS-aware path
comparisons.

PR-#111

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The checklist requires README CLI documentation to match actual behavior. The README adds a
slash-based ~/.../* example, while the changed matching path expands that pattern and performs a
separator-sensitive literal comparison; ExpandHome preserves the slash after a backslash-based
Windows home.

Rule 2270057: Keep CLI documentation in README.md in sync with user-facing CLI changes
README.md[2061-2072]
src/Capacitor.Cli/CwdRemapper.cs[50-52]
src/Capacitor.Cli/CwdRemapper.cs[86-101]
src/Capacitor.Cli/CwdRemapper.cs[138-146]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Slash-based wildcard rules using `~`, including the form documented by this PR, fail to match ordinary backslash-based transcript paths on Windows.

## Fix Focus Areas
- src/Capacitor.Cli/CwdRemapper.cs[50-52]
- src/Capacitor.Cli/CwdRemapper.cs[86-101]
- src/Capacitor.Cli/CwdRemapper.cs[138-148]

## Recommended Fix
Make literal head and tail matching treat `/` and `\` as equivalent path separators while preserving consumed string indexes, and ensure expanded destination paths do not contain an invalid mixed-separator junction. Add a test using a Windows-style home, the documented slash-based `~/.../*` rule, and a backslash-based cwd.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View medium (1)
6. Three report tests bypass path helpers ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The new report tests construct missing paths beneath directories owned by TempDir using
Path.Combine instead of TempDir.PathTo. Those paths remain outside the helper abstraction, so
future platform-specific path handling added to the fixture will not cover these cases.
Code

test/Capacitor.Cli.Tests.Unit/Commands/ImportMissingCwdsReportTests.cs[R207-210]

+        var sessionCwds = new Dictionary<string, string>(StringComparer.Ordinal) {
+            ["s1"] = Path.Combine(repo, "worktrees", "ai-1"),
+            ["s2"] = Path.Combine(repo, "worktrees", "ai-1"),
+            ["s3"] = Path.Combine(repo, "worktrees", "ai-2"),
Relevance

●●● Strong

Recent accepted precedents require TempDir path helpers instead of direct Path.Combine beneath
temporary roots.

PR-#685
PR-#935

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The checklist requires all paths beneath a TempDir root to be constructed through its members and
explicitly identifies direct Path.Combine usage as a violation. Each cited dictionary builds paths
below a directory created by the fixture.

Rule 2767472: Use TempDir helper for all temporary filesystem state in tests
test/Capacitor.Cli.Tests.Unit/Commands/ImportMissingCwdsReportTests.cs[203-210]
test/Capacitor.Cli.Tests.Unit/Commands/ImportMissingCwdsReportTests.cs[225-232]
test/Capacitor.Cli.Tests.Unit/Commands/ImportMissingCwdsReportTests.cs[245-250]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The newly added report tests use `Path.Combine` to build paths under a `TempDir` root instead of using the fixture's path helpers.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/Commands/ImportMissingCwdsReportTests.cs[203-250]

## Recommended Fix
Construct every path below the fixture root with `Tmp.PathTo(...)` or the relevant `TempDirHandle.PathTo(...)` method, including paths intended to remain absent, and reuse those helper-produced values in assertions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

7. Five command tests bypass temp injection 📘 Rule violation ▣ Testability
Description
Five newly added methods in RemapCommandTests each create and dispose their own TempDir rather
than using an injected fixture property. This spreads temporary-directory ownership through
individual test bodies, leaving future fixture setup or cleanup changes with multiple lifecycle
patterns to maintain.
Code

test/Capacitor.Cli.Tests.Unit/Commands/RemapCommandTests.cs[R105-107]

+    [Test, NotInParallel]
+    public async Task Add_refuses_extra_arguments_rather_than_storing_the_first_two() {
+        using var tmp = new TempDir();
Relevance

● Weak

Recent repository precedent rejected the same manual TempDir-to-injected-fixture conversion in newly
added tests.

PR-#972
PR-#976

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The checklist requires test classes to use a single injected TempDir property and explicitly
disallows new TempDir() calls. The added command tests repeatedly construct manual fixtures.

Rule 2808173: Use injected [TempDir] public required property in test classes instead of manual fields
test/Capacitor.Cli.Tests.Unit/Commands/RemapCommandTests.cs[105-107]
test/Capacitor.Cli.Tests.Unit/Commands/RemapCommandTests.cs[123-126]
test/Capacitor.Cli.Tests.Unit/Commands/RemapCommandTests.cs[143-145]
test/Capacitor.Cli.Tests.Unit/Commands/RemapCommandTests.cs[157-162]
test/Capacitor.Cli.Tests.Unit/Commands/RemapCommandTests.cs[175-178]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Five newly added command tests manually construct `TempDir`, although test classes must use a framework-injected public required property.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/Commands/RemapCommandTests.cs[105-178]

## Recommended Fix
Add `[TempDir] public required TempDir Tmp { get; init; }` to `RemapCommandTests`, replace each local `new TempDir()` with `Tmp`, and remove the corresponding manual disposal scopes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


8. Three report tests bypass temp injection 📘 Rule violation ▣ Testability
Description
Report_suggests_a_wildcard_rule_instead_of_listing_the_family,
Report_still_lists_paths_outside_any_family, and
Report_lists_a_family_whose_project_is_not_a_repository each construct and dispose a new TempDir
inside the test method. These additions duplicate fixture lifecycle management across the class, so
later setup and cleanup changes must account for both manual and framework-managed patterns.
Code

test/Capacitor.Cli.Tests.Unit/Commands/ImportMissingCwdsReportTests.cs[R201-203]

+    [Test, NotInParallel]
+    public async Task Report_suggests_a_wildcard_rule_instead_of_listing_the_family() {
+        using var tmp = new TempDir();
Relevance

● Weak

Recent repository precedent rejected replacing manual TempDir construction with injected fixtures in
test classes.

PR-#972
PR-#976

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The rule prohibits new TempDir() calls in test classes and requires one injected public required
property. Each cited newly added test manually creates its own disposable fixture instead.

Rule 2808173: Use injected [TempDir] public required property in test classes instead of manual fields
test/Capacitor.Cli.Tests.Unit/Commands/ImportMissingCwdsReportTests.cs[201-203]
test/Capacitor.Cli.Tests.Unit/Commands/ImportMissingCwdsReportTests.cs[223-226]
test/Capacitor.Cli.Tests.Unit/Commands/ImportMissingCwdsReportTests.cs[243-246]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Three newly added report tests manually construct `TempDir`, although test classes must receive it through a public required property annotated with `[TempDir]`.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/Commands/ImportMissingCwdsReportTests.cs[201-246]

## Recommended Fix
Add a single `[TempDir] public required TempDir Tmp { get; init; }` property to the test class, replace the three local `new TempDir()` instances with `Tmp`, and remove their manual disposal.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 64 rules
✅ Cross-repo context — repo relationships
Review mode: 🧠 Deep: This is a substantial behavioral change spanning remap parsing/matching, precedence, CLI validation, import reporting, cross-platform path handling, and persistence, creating multiple independent opportunities for subtle defects.

Grey Divider

Tip of the day
💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/Capacitor.Cli/CwdRemapper.cs Resolved
A session's cwd is not always the worktree root, so grouping by the immediate
parent left one recorded in <worktree>/src in a group of one and no family
formed. Walking up to the nearest checkout root makes the slug the unit, which
also separates two dead worktrees from two dead directories inside one. And a
suggested rule takes its separator from the path, since a mixed one never matches.
@George-Payne

Copy link
Copy Markdown
Member Author

Two qodo findings are left open deliberately. Both ask the new tests to take a [TempDir]-injected property instead of using var tmp = new TempDir(). Neither carries an inline comment to answer on, hence the note here.

CLAUDE.md states that rule as "when every test in a class needs one, inject it instead of holding a field". Neither class meets that condition — most tests in RemapCommandTests and ImportMissingCwdsReportTests are pure and need no directory at all — and the tests already in both files use the local form. Injecting would build and dispose a directory for every pure test in the class to spare five of them a using.

Everything else qodo raised is addressed in d4f58236: the family grouping now walks up to the nearest checkout root, so a session recorded in <worktree>/src still forms a family, and a suggested rule takes its separator from the recorded path rather than always /.

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.

2 participants