Preserve handwritten Git API during generation cleanup

[thomhurst]

Summary

• skip legacy service cleanup for the handwritten Git integration
• keep Linux and Windows generation paths consistent
• allow Git's coverage-only generator mode to validate without deleting IGit.cs/Git.cs before the package build

Validation

• workflow diff check clean
• reproduced failure: Git build lost IGit after pre-generation cleanup

Part of #3996

Summary by CodeRabbit

• Bug Fixes

  • Improved CLI option generation cleanup to preserve handwritten service integrations for tools that do not generate command facades.
  • Replaced tool-specific cleanup handling with catalog-driven behavior for more consistent Linux and Windows results.

• Documentation

  • Added command-facade status to the tool catalog, making cleanup and generation behavior clearer.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a6067fa8-5d06-41e2-b782-b99790f5a2ba

📥 Commits

Reviewing files that changed from the base of the PR and between d073878 and db425d4.

📒 Files selected for processing (5)

• .github/workflows/generate-cli-options.yml
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/ToolCatalogTests.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/GitCliScraper.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/ICliScraper.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/ToolCatalog.cs

Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The CLI catalog now records whether each tool generates a command facade. The workflow passes this value to Linux and Windows cleanup steps. Cleanup deletes service files only for tools with command-facade generation enabled. git sets the value to false.

Changes

Command Facade Cleanup

Layer / File(s)	Summary
Command facade catalog contract tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/ICliScraper.cs, tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/ToolCatalog.cs, tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/ToolCatalogTests.cs	The scraper contract and catalog entry include GenerateCommandFacade. Text and JSON catalog output include the value. Tests verify the field and its default serialization.
Git command facade override tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/GitCliScraper.cs, tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/ToolCatalogTests.cs	GitCliScraper sets GenerateCommandFacade to false and passes it to the tool definition. Tests verify that git is disabled while other tools remain enabled.
Conditional service cleanup .github/workflows/generate-cli-options.yml	The matrix exposes generateCommandFacade. Linux and Windows cleanup steps use this value instead of checking the tool name.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to db425

This change preserves the handwritten Git API during generation cleanup and keeps the related generation paths aligned; no actionable merge-blocking risk remains beyond normal checks and review.

Poem

I’m a rabbit with a catalog bright,
Facades now guide cleanup right.
Git keeps its service lair,
Other tools shed files with care.
Flags make the workflow fair!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preserving the handwritten Git API during generation cleanup.
Docstring Coverage	✅ Passed	Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
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.

✨ 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 issue-3996-handwritten-cleanup-20260823

---

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.}

[greptile-apps]

Greptile Summary

The PR carries command-facade ownership from each scraper into the generation matrix so cleanup preserves Git’s handwritten API while retaining existing cleanup for generator-owned facades.

• Adds GenerateCommandFacade to scraper and catalog metadata, with Git opting out.
• Applies the ownership flag consistently to Linux and Windows cleanup.
• Extends catalog tests to verify Git’s exception and the serialized flag.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
.github/workflows/generate-cli-options.yml	Propagates the facade-ownership flag into both matrices and gates Linux and Windows legacy-service cleanup consistently.
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/ICliScraper.cs	Adds a default metadata contract indicating that generators own command facades unless a scraper opts out.
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/GitCliScraper.cs	Marks Git’s facade as handwritten and reuses that value when constructing its tool definition.
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/ToolCatalog.cs	Carries command-facade ownership into JSON and human-readable catalog output consumed by automation.
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/ToolCatalogTests.cs	Verifies Git’s opt-out, other scrapers’ defaults, and JSON serialization of the new field.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[ICliScraper metadata] --> B[Tool catalog JSON]
  B --> C[GitHub Actions matrix]
  C --> D{Generate command facade?}
  D -->|Yes| E[Remove generator-owned facade files]
  D -->|No: Git| F[Preserve handwritten IGit and Git APIs]
  E --> G[Generate and validate package]
  F --> G

Loading

_{Reviews (2): Last reviewed commit: "fix(generator): drive cleanup from catal..." | Re-trigger Greptile}

[thomhurst]

@claude Please re-review exact head db425d4. The cleanup ownership flag is now catalog-driven and tested.

[claude]

Code review

Reviewed the diff (workflow YAML + ToolCatalog/ICliScraper/GitCliScraper changes). No bugs found — this is a clean, well-tested fix.

What I checked:

• Traced matrix.generateCommandFacade end-to-end: create_matrix() in the tool-catalog job now includes generateCommandFacade in the jq projection, and both the Linux (bash [ "..." = "true" ]) and Windows (PowerShell -eq 'true') cleanup steps gate the legacy-file removal on it identically — the two platform paths stay in sync as the PR description promises.
• Confirmed no other hardcoded git-specific branches remain in .github/workflows/generate-cli-options.yml — the old tool-name special case is fully replaced by the catalog-driven flag, not just supplemented by it.
• ICliScraper.GenerateCommandFacade defaults to true via a default interface member, so every scraper except GitCliScraper (which overrides to false) is unaffected — matches the new ToolCatalogTests assertions (git → false, all others → true).
• GitCliScraper.CreateToolDefinition() now reads GenerateCommandFacade = GenerateCommandFacade (the property) instead of a hardcoded literal, so the catalog value and the actual generation behavior can't drift apart — that's the real fix, since previously these two things (whether the generator produces facade files, and whether CI cleans up old ones) were independently controlled.

Design note (not a blocker): this reuses the same GenerateCommandFacade flag to drive two different behaviors — the generator's decision to emit facade files, and CI's decision to delete stale ones before regenerating. That coupling is exactly what makes this fix correct (a tool that doesn't generate a facade should never have its handwritten facade deleted), so I'd keep it rather than introduce a second flag — just worth calling out for future maintainers who might assume it's single-purpose when adding a new tool.

Nice fix — small, targeted, and it removes special-casing rather than adding more of it.