fix local remove

[sker65]

Summary by CodeRabbit

• Chores

  • Version bump to 4.3.3.

• Bug Fixes

  • Improved sync behavior to use remote-first matching, accurately detect changed or removed remote items, and reliably delete corresponding local files and now-empty folders.
  • Test updates to cover the improved cleanup logic and path tracking during synchronization.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Version bumped to 4.3.3 and YAML sync logic switched to remote-first: signatures/returns updated (newTestCases → remoteTestCases, readTestCasesFromDir now attaches filePath), orphaned local-file cleanup and removeEmptyDirectoriesRecursively were added; tests updated accordingly.

Changes

Cohort / File(s)	Summary
Version Bumps README.md, package.json	Updated CLI/package version from 4.3.2 to 4.3.3.
Core Sync Logic src/tools/sync/yaml.ts	Refactored sync flow to remote-first: cleanupFilesystem now accepts { remoteTestCases: SyncTestCase[] }; writeYaml calls cleanup with remoteTestCases; readTestCasesFromDir(startDir) now returns Array<SyncTestCase & { filePath?: string }>; added removeEmptyDirectoriesRecursively(dirPath, rootFolderPath); cleanup deletes files when descriptions differ and removes orphaned local files/dirs.
Tests tests/tools/sync/yaml.spec.ts	Tests updated for new readTestCasesFromDir return shape (filePath), cleanupFilesystem parameter rename (remoteTestCases), and exported removeEmptyDirectoriesRecursively.

Sequence Diagram(s)

sequenceDiagram
    participant Remote as Remote Source
    participant Sync as Sync Module
    participant Local as Local FS
    participant Cleanup as Cleanup Logic

    Remote->>Sync: send remoteTestCases (id, description)
    Local->>Sync: readTestCasesFromDir(startDir) -> localTestCases (+ filePath)
    Sync->>Sync: build maps by id (remoteTestCasesById, localTestCasesById)
    Sync->>Cleanup: compare entries
    alt ID matches but description differs
        Cleanup->>Local: delete local file (use filePath)
        Cleanup->>Local: possibly remove parent dir
    end
    alt Local entry not present in remote
        Cleanup->>Local: delete orphaned local file (use filePath)
        Cleanup->>Local: removeEmptyDirectoriesRecursively(fileDir, root)
    end
    Cleanup->>Sync: cleanup complete

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• fix local remove #346 — Makes identical changes to src/tools/sync/yaml.ts (parameter rename to remoteTestCases, readTestCasesFromDir includes filePath, adds removeEmptyDirectoriesRecursively).

Suggested reviewers

• fabianhinz
• Germandrummer92

Poem

🐰 A tiny hop to four-oh-three-three,
Remote-first whispers through the tree.
Files once wandering find their end,
Folders pruned by a careful friend,
I nibble carrots — sync is free. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'fix local remove' is vague and does not clearly convey the specific changes made. It references a feature area but lacks detail about what is being fixed or how the removal functionality has changed.	Consider using a more descriptive title such as 'Fix local test case removal in sync workflow' or 'Implement remote-first synchronization with orphaned file cleanup' to better communicate the main purpose of the changes.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing touches

• 📝 Generate docstrings

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/tools/sync/yaml.ts`:
- Around line 272-280: The ENOENT occurs because files may already be removed by
an earlier recursive fs.rmSync call; in the loop that iterates localTestCases
and deletes by localTestCase.filePath (the block using remoteTestCasesById,
fs.rmSync(localTestCase.filePath) and removeEmptyDirectoriesRecursively), guard
the deletion by checking the file exists first (e.g., with fs.existsSync or
try/catch around fs.rmSync) before calling fs.rmSync and then call
removeEmptyDirectoriesRecursively only when a deletion actually happened; update
the logic around the localTestCases loop to skip removal if
localTestCase.filePath is missing to avoid double-deletes and ENOENT errors.

♻️ Duplicate comments (1)

src/tools/sync/yaml.ts (1)

180-182: Return type has unnecessary optional marker.

The function signature declares filePath?: string (optional), but line 184 shows the internal array type as { filePath: string } (required), and line 192 always provides filePath. The external type should match the actual behavior.

♻️ Proposed fix

 export const readTestCasesFromDir = (
   startDir: string,
-): Array<SyncTestCase & { filePath?: string }> => {
+): Array<SyncTestCase & { filePath: string }> => {