Remove unused CreateBranchAsync/DeleteBranchAsync from IGitHubClient (#43)

[sterob-2]

Summary

Remove two dead methods from the GitHub client surface: CreateBranchAsync and DeleteBranchAsync. This reduces unused code, simplifies the interface, and does not change runtime behavior because branch operations are performed locally via LibGit2Sharp.

Changes

• Modify src/Orchestrator.App/Core/Interfaces/IGitHubClient.cs
• Modify src/Orchestrator.App/Infrastructure/GitHub/OctokitGitHubClient.cs
• Modify tests/

Testing

• Not run (automated via CI)

Issue

#43

[sterob-2]

Code Review: Issue 43 - Remove unused CreateBranchAsync/DeleteBranchAsync from IGitHubClient

STATUS: CHANGES_REQUESTED
UPDATED: 2026-01-11 19:49:11 UTC

Decision

CHANGES_REQUESTED

Findings

• [Major] BUILD_AND_TEST_VERIFICATION: No build/test results are included. Removing interface members can cause compile-time failures in other projects, tests, mocks, or explicit interface implementations. Run 'dotnet build' for the solution and 'dotnet test' (full test suite) and ensure CI is green before merging.
• [Major] COMPILATION_IMPACT: Two members were removed from IGitHubClient (CreateBranchAsync, DeleteBranchAsync). Perform a repository-wide search for these symbol names to find remaining callers, explicit interface implementations, Moq setups, generated code, or other projects that will fail to compile once merged. (src/Orchestrator.App/Core/Interfaces/IGitHubClient.cs:28)
• [Major] TEST_UPDATES_REQUIRED: Any unit/integration tests, Moq setups, or test helpers that referenced CreateBranchAsync/DeleteBranchAsync must be removed or updated. Moq setups or test interfaces referencing removed members will fail to compile. Search the tests/ tree and update/delete affected tests prior to merge. (tests/)
• [Minor] IMPLEMENTATION_CONFORMANCE: Verify all concrete types that implement IGitHubClient. The OctokitGitHubClient implementations were deleted (see OctokitGitHubClient.cs around the removed region), but other implementations or explicit interface implementations in the repo may still declare these methods and now be inconsistent. (src/Orchestrator.App/Infrastructure/GitHub/OctokitGitHubClient.cs:244)
• [Minor] CLEANUP: The removed methods referenced configuration and exception/API types (e.g., _cfg.Workflow.DefaultBaseBranch, ApiException, NotFoundException). After removal, search for now-unused configuration keys, private fields, or using directives and remove them (or suppress warnings) to avoid compiler warnings and to keep the codebase tidy. (src/Orchestrator.App/Infrastructure/GitHub/OctokitGitHubClient.cs:244)
• [Minor] API_COMPATIBILITY: IGitHubClient appears to be internal, which reduces external breakage risk. However, if the assembly is packaged/published, exposed via InternalsVisibleTo, or consumed by other repos, removing members could still be breaking. Confirm packaging/publishing and any external consumers and document the change in the changelog if necessary. (src/Orchestrator.App/Core/Interfaces/IGitHubClient.cs:1)
• [Minor] SECURITY_CONSIDERATION: Removing branch-create/delete API surface reduces functionality. Verify any authorization, audit, or logging responsibilities previously handled in those methods are preserved elsewhere so no security/audit gaps are introduced (for example, if those methods performed permission checks or logging).
• [Minor] REPO_NOISE: This PR introduces many files under orchestrator/prompts/ and orchestrator/specs/ that look like generated artifacts. Confirm these were intentionally committed; they increase review noise and may contain content not intended for the primary codebase. (orchestrator/prompts/)

Summary

The PR removes two unused branch-related members (CreateBranchAsync, DeleteBranchAsync) from IGitHubClient and deletes their implementations from OctokitGitHubClient. The change is small and focused and aligns with the stated goal of removing dead API surface. Approve subject to verification steps: run a full solution build and the full test suite, perform a repo-wide symbol search for the removed names, and do a small cleanup pass (remove any now-unused usings/fields and update tests/docs/changelog) before merging.

[sterob-2]

Code Review: Issue 43 - Remove unused CreateBranchAsync/DeleteBranchAsync from IGitHubClient

STATUS: CHANGES_REQUESTED
UPDATED: 2026-01-11 19:49:59 UTC

Decision

CHANGES_REQUESTED

Findings

• [Major] BUILD_AND_TEST_VERIFICATION: No build/test results are included with this change. Removing interface members can cause compile-time failures in other projects, explicit interface implementations, generated code, or test mocks. Before merging, run 'dotnet build' for the solution and 'dotnet test' (full test suite) and ensure CI is green.
• [Major] COMPILATION_IMPACT: Two members were removed from IGitHubClient (CreateBranchAsync, DeleteBranchAsync). Perform a repository-wide search for these symbol names to locate remaining callers, explicit interface implementations, Moq setups, or references in other projects that will fail to compile once merged. (src/Orchestrator.App/Core/Interfaces/IGitHubClient.cs:28)
• [Major] TEST_UPDATES_REQUIRED: Any unit/integration tests, Moq setups, or test helpers that referenced CreateBranchAsync/DeleteBranchAsync must be removed or updated. Moq setups or test interfaces referencing removed members will cause compile failures. Search the tests/ tree and update/delete affected tests prior to merging. (tests/)
• [Minor] IMPLEMENTATION_CONFORMANCE: Verify all concrete types that implement IGitHubClient. The OctokitGitHubClient implementations were deleted (see OctokitGitHubClient.cs around the removed region), but other implementations or explicit interface implementations in the repo may still declare these methods and now be inconsistent. (src/Orchestrator.App/Infrastructure/GitHub/OctokitGitHubClient.cs:244)
• [Minor] CLEANUP: The removed methods referenced configuration and exception/API types (e.g., _cfg.Workflow.DefaultBaseBranch, ApiException). After removal, search for now-unused configuration keys, private fields, or using directives and remove them (or suppress warnings) to avoid compiler warnings and keep the codebase tidy. (src/Orchestrator.App/Infrastructure/GitHub/OctokitGitHubClient.cs:244)
• [Minor] API_COMPATIBILITY: IGitHubClient is internal which reduces the risk of breaking external consumers. However, if the assembly is packaged/published or used by other repos via InternalsVisibleTo, removing members could still be breaking. Confirm packaging/publishing and external consumers and document the change (changelog/migration notes) if needed. (src/Orchestrator.App/Core/Interfaces/IGitHubClient.cs:1)
• [Minor] SECURITY_CONSIDERATION: Removing branch-create/delete API surface reduces functionality. Verify that any authorization, audit, or logging responsibilities previously handled in those methods (if any) are preserved elsewhere so no security or audit gaps are introduced.
• [Minor] REPO_NOISE: This PR adds many files under orchestrator/prompts/ and orchestrator/specs/ that look like generated artifacts and increase review noise. Confirm these were intentionally committed and don't contain sensitive or irrelevant content. (orchestrator/prompts/)

Summary

The PR cleanly removes two unused branch-related members (CreateBranchAsync, DeleteBranchAsync) from IGitHubClient and deletes their implementations from OctokitGitHubClient. The change is small and sensible (reduces dead surface). Approve provided the repository is validated: run a full solution build and the full test suite, perform a repo-wide symbol search for the removed names, and clean up any now-unused usings/fields/tests/docs/changelog before merging.

[sterob-2]

Code Review: Issue 43 - Remove unused CreateBranchAsync/DeleteBranchAsync from IGitHubClient

STATUS: CHANGES_REQUESTED
UPDATED: 2026-01-11 19:50:45 UTC

Decision

CHANGES_REQUESTED

Findings

• [Major] BUILD_AND_TEST_VERIFICATION: No build or test results are included. Removing interface members can cause compile-time failures in other projects, explicit interface implementations, generated code, or test mocks. Before merging, run a full solution build and the full test suite (dotnet build; dotnet test) and ensure CI is green.
• [Major] COMPILATION_IMPACT: Two members were removed from IGitHubClient (CreateBranchAsync, DeleteBranchAsync). Perform a repository-wide search for these symbol names to find remaining callers, explicit interface implementations, Moq setups, generated code, or other projects that will fail to compile once merged. (src/Orchestrator.App/Core/Interfaces/IGitHubClient.cs:28)
• [Major] TEST_UPDATES_REQUIRED: Any unit/integration tests, Moq setups, or test helpers that referenced CreateBranchAsync/DeleteBranchAsync must be removed or updated. Moq setups or test interfaces referencing removed members will fail to compile. Search the tests/ tree and update/delete affected tests prior to merge. (tests/)
• [Minor] IMPLEMENTATION_CONFORMANCE: Verify all concrete types that implement IGitHubClient. The OctokitGitHubClient implementations were deleted (around the removed region), but other implementations (or explicit interface implementations) may still declare these methods and become inconsistent with the interface. (src/Orchestrator.App/Infrastructure/GitHub/OctokitGitHubClient.cs:244)
• [Minor] CLEANUP: The removed methods referenced configuration and exception/API types (e.g., _cfg.Workflow.DefaultBaseBranch, ApiException, NotFoundException). After removal, search for now-unused private fields, configuration keys, using directives or exception types and remove them or suppress warnings to avoid compiler warnings and to keep the codebase tidy. (src/Orchestrator.App/Infrastructure/GitHub/OctokitGitHubClient.cs:244)
• [Minor] API_COMPATIBILITY: IGitHubClient is internal (lower risk), but if the assembly is packaged/published or consumed by other repos (or uses InternalsVisibleTo), removing members could still be breaking. Confirm whether this assembly is distributed/published and, if so, document the breaking change or follow deprecation/versioning policy. (src/Orchestrator.App/Core/Interfaces/IGitHubClient.cs:1)
• [Minor] SECURITY_CONSIDERATION: Removing branch-create/delete API surface reduces functionality. Verify any authorization, audit, or logging responsibilities previously handled in those methods are preserved elsewhere so no security/audit gaps are introduced (e.g., if those methods previously performed permission checks or logging).
• [Minor] REPO_NOISE: This PR introduces many files under orchestrator/prompts/ and orchestrator/specs/ that look like generated artifacts and increase review noise. Confirm these were intentionally committed and do not contain sensitive or irrelevant content. (orchestrator/prompts/)

Summary

This PR removes two unused branch-related members (CreateBranchAsync, DeleteBranchAsync) from IGitHubClient and deletes their implementations from OctokitGitHubClient. The change is small, focused, and reasonable provided repository-wide verification is performed (build, tests, and search for remaining references). Approve subject to the verification and cleanup steps below.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud