chore: remove confirmed dead code (−755 lines)#126
Merged
Conversation
fshehadeh-sfl
requested review from
aberaud,
atraczyk and
cwenger-sfl
as code owners
July 21, 2026 17:28
Contributor
There was a problem hiding this comment.
Pull request overview
Removes verified dead code and unused devDependencies across the orchestrator, review flow, admin UI API client, and plugin/runtime helpers to reduce maintenance surface area and keep runtime paths aligned with the Docker-based review execution model.
Changes:
- Deleted legacy/unused review agent implementation (
CopilotReviewAgent) and related tests/mocks/types. - Removed dead orchestration helpers/branches (e.g.,
handleGerritEventalias and unreachable non-pushDirectpush path) and adjusted tests accordingly. - Dropped unused admin UI API functions and a no-longer-used default skill source module; removed unused devDependencies.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/runtimeBootstrap.test.ts | Removes mocks/exports for deleted CopilotReviewAgent. |
| tests/unit/orchestratorCommitMessage.test.ts | Removes tests for deleted private footer helpers; narrows file to commit message tests. |
| tests/unit/orchestrator.test.ts | Updates call sites from handleGerritEvent to handleReviewEvent. |
| tests/unit/copilotReviewAgent.test.ts | Deletes tests for the removed legacy review agent. |
| src/review/reviewOrchestrator.ts | Removes the legacy ReviewAgent interface formerly used only by CopilotReviewAgent. |
| src/review/copilotReviewAgent.ts | Deletes the legacy host-side Copilot SDK review agent implementation. |
| src/plugins/registry.ts | Makes getProviderTechnicalCapabilities module-private (internal helper). |
| src/orchestrator/orchestrator.ts | Removes dead alias/methods and unreachable push branch; enforces pushDirect usage and removes now-unused footer helpers. |
| src/bootstrap/runtimeBuilder.ts | Makes integration helper functions module-private (no external callers). |
| src/admin/ui/api.ts | Removes unused SSH key API helpers superseded by generateSshKeyPair(). |
| src/admin/defaultSkillSources.ts | Deletes unused preloaded project skill source constants/helpers. |
| package.json | Removes unused devDependencies (@fontsource/material-icons, @material-icons/svg, simple-icons). |
Remove @fontsource/material-icons, @material-icons/svg, and simple-icons — none are imported anywhere in src/, tests/, or CSS. Verified by full-repo grep before removal.
admin - Delete defaultSkillSources.ts (PRELOADED_PROJECT_SKILL_SOURCE / preloadedProjectSkillSources — zero importers) - Drop generateSshKey() and getSshPublicKey() from ui/api.ts (superseded by generateSshKeyPair; no callers) review - Delete CopilotReviewAgent class + its test file; the class was never instantiated in production (all reviews run via Docker REVIEW_MODE=1); docs already called it 'legacy, unused' - Remove the ReviewAgent interface from reviewOrchestrator.ts (only existed to be implemented by the deleted class) - Clean dead CopilotReviewAgent mock from runtimeBootstrap.test.ts orchestrator - Remove handleGerritEvent() — Gerrit-named alias for handleReviewEvent(); update 4 test call sites - Remove unreachable else-branch in pushProjectChanges: all three VCS connectors implement pushDirect, making vcsConnector.push() path dead; replace with an explicit guard throw - Remove appendTicketFooter() and buildTicketFooter() private methods (only consumed by the dead else-branch) - Drop hasTicketFooter from the ticketFooterFormatter import and the now-unused ticketUrl parameter from pushProjectChanges - Remove the two dead test describe blocks from orchestratorCommitMessage.test.ts (same logic covered by ticketFooterFormatter.test.ts) plugins / bootstrap - Drop export from getActiveIntegrationsByType and getPrimaryActiveIntegration in runtimeBuilder.ts (zero external callers; functions remain as module-private helpers) - Drop export from getProviderTechnicalCapabilities in registry.ts (only called by getPluginCapabilities in the same file) All gates pass: npm test (2129), typecheck, lint.
fshehadeh-sfl
force-pushed
the
chore/dead-code-cleanup
branch
from
July 22, 2026 19:49
bd492c2 to
633e3d9
Compare
Address Copilot review comments on PR #126: - Remove copilotReviewAgent.ts / ReviewAgent references from .github/copilot-instructions.md, .github/context/architecture.md, .github/context/testing.md, docs/ARCHITECTURE.md (file deleted in the prior refactor commit). - Regenerate package-lock.json after dropping @fontsource/material-icons, @material-icons/svg, and simple-icons devDependencies so npm ci stays consistent with package.json.
Comment on lines
1533
to
+1538
| const subjectHash = createHash("sha1").update(fallbackCommitMessage.split("\n")[0] ?? "").digest("hex"); | ||
|
|
||
| let pushResult; | ||
| if (vcsConnector.pushDirect) { | ||
| pushResult = await vcsConnector.pushDirect(handle.hostWorkspacePath, ref, topic, volumeOpts); | ||
| } else { | ||
| pushResult = await vcsConnector.push(handle.hostWorkspacePath, ref, commitMsg, undefined, volumeOpts); | ||
| if (!vcsConnector.pushDirect) { | ||
| throw new Error(`VCS connector for ${reviewSystemLabel} does not implement pushDirect`); | ||
| } | ||
| const pushResult = await vcsConnector.pushDirect(handle.hostWorkspacePath, ref, topic, volumeOpts); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Dead code audit pass — removes everything verified to have zero callers/importers anywhere in the repo. All items were confirmed by full-repo grep before removal.
Changes
Unused devDependencies (commit 1)
@fontsource/material-icons@imported in any CSS file@material-icons/svgsimple-iconsDead code (commit 2)
src/admin/defaultSkillSources.tsdeleted —PRELOADED_PROJECT_SKILL_SOURCE/preloadedProjectSkillSources()had zero importersui/api.ts: removedgenerateSshKey()(superseded bygenerateSshKeyPair()) andgetSshPublicKey()(no callers)src/review/copilotReviewAgent.tsdeleted —CopilotReviewAgentwas never instantiated in production; all reviews run via DockerREVIEW_MODE=1; docs already labelled it "legacy, unused"ReviewAgentinterface removed fromreviewOrchestrator.ts(only existed to be implemented by the deleted class)src/orchestrator/orchestrator.tshandleGerritEvent()removed — one-line alias forhandleReviewEvent(); 4 test call sites updatedelse { vcsConnector.push(...) }branch inpushProjectChangesremoved — all three VCS connectors (GerritVcsConnector,GitHubVcsConnector,GitLabVcsConnector) implementpushDirect, so the non-pushDirectpath was dead; replaced with a guard throwappendTicketFooter()andbuildTicketFooter()private methods removed (only consumed by the dead else-branch)hasTicketFooterdropped from import;ticketUrlparameter removed frompushProjectChanges(only fed the deleted methods)src/bootstrap/runtimeBuilder.tsexportdropped fromgetActiveIntegrationsByTypeandgetPrimaryActiveIntegration— zero external callers; functions stay as module-private helperssrc/plugins/registry.tsexportdropped fromgetProviderTechnicalCapabilities— only called bygetPluginCapabilitiesin the same fileTests cleaned up
tests/unit/copilotReviewAgent.test.tsdeleted (tests the deleted class)CopilotReviewAgentmock removed fromruntimeBootstrap.test.tsdescribeblocks removed fromorchestratorCommitMessage.test.ts(tested the deleted private methods; logic is already covered byticketFooterFormatter.test.ts)handleGerritEvent→handleReviewEventcall site updates inorchestrator.test.tsStats
Gates