fix: preserve cloud-only tools and improve import error reporting by alerizzo · Pull Request #9 · codacy/codacy-cloud-cli

alerizzo · 2026-05-27T10:47:27Z

Summary

Cloud-only tool preservation: tools import no longer disables tools that only exist on Codacy Cloud (not supported by the local CLI). This prevents accidentally turning off tools like Codacy Security, Trivy, etc. when importing a local config.
Config-file mode fix: When a tool has useLocalConfigurationFile: true, the import now skips the pattern-reset step and just enables the tool with the config file flag — previously it would needlessly wipe patterns first.
Structured error reporting: Import failures now surface the HTTP status code and API response body details instead of generic error messages, making it much easier to diagnose what went wrong.
Ship-it command: Adds a .claude/commands/ship-it.md for streamlined changeset → branch → commit → push → PR workflow.

Test plan

npm test — all existing and new tests pass
npx ts-node src/index.ts tools import --provider gh --organization <org> --repository <repo> .codacy.yaml --dry-run — verify cloud-only tools show as "unchanged" rather than marked for disable
npx ts-node src/index.ts tools import with a tool that has useLocalConfigurationFile: true — verify it enables without resetting patterns
Trigger an import that hits an API error — verify status code and details are printed

🤖 Generated with Claude Code

- Don't disable tools the local CLI doesn't support (cloud-only tools stay enabled when not present in the config file) - Skip pattern reset when useLocalConfigurationFile is set — just enable the tool with the config file flag - Surface structured API error details (status code, response body) on import failures instead of generic messages - Add ship-it command for streamlined PR workflow Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

codacy-production · 2026-05-27T10:48:34Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 8 duplication

Metric Results

Duplication 8

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

_{TIP This summary will be updated as you push new changes.}

gemini-code-assist

Code Review

This pull request updates the tools import functionality to preserve cloud-only tools (only disabling tools supported by the local CLI), skips pattern resets when useLocalConfigurationFile is enabled, and extracts and displays structured API error details on import failures. It also introduces a new ship-it command script and updates tests. The reviewer feedback recommends adding defensive filtering when parsing local CLI tools to prevent potential runtime errors, and suggests formatting improvements to printImportErrors to avoid duplicate output and improve visual indentation.

gemini-code-assist · 2026-05-27T10:48:39Z

+    if (!info.tools || !Array.isArray(info.tools)) return null;
+    return info.tools
+      .filter((t: any) => t.supported)
+      .map((t: any) => t.id as string);


To prevent potential runtime errors, we should defensively filter the tools returned by the local CLI. If a tool object is null/undefined or is missing a valid string id, calling resolveToolId on it later will throw a TypeError when trying to call toLowerCase() on an undefined value.

Suggested change

if (!info.tools || !Array.isArray(info.tools)) return null;

return info.tools

.filter((t: any) => t.supported)

.map((t: any) => t.id as string);

if (!info.tools || !Array.isArray(info.tools)) return null;

return info.tools

.filter((t: any) => t && t.supported && typeof t.id === "string")

.map((t: any) => t.id);

Good catch — added the typeof t.id === "string" guard to prevent a TypeError if the CLI returns a malformed tool entry.

_{🤖 Generated by /pr-fixup command}

gemini-code-assist · 2026-05-27T10:48:39Z

+function printImportErrors(failures: ImportFailure[]): void {
+  for (const f of failures) {
+    const status = f.status ? ` (${f.status})` : "";
+    console.log(ansis.red(`✗ ${f.tool}: ${f.error}${status}`));
+
+    if (f.details.length === 0) continue;
+
+    const shown = f.details.slice(0, MAX_ERROR_DETAILS);
+    for (const detail of shown) {
+      console.log(ansis.dim(`  ${detail}`));
+    }
+    const remaining = f.details.length - shown.length;
+    if (remaining > 0) {
+      console.log(ansis.dim(`  ... and ${remaining} more`));
+    }
+  }
+  console.log();
+}


To make the error details visually nested under the summary header and avoid duplicating the error list, let's add indentation to the output of printImportErrors.

Suggested change

function printImportErrors(failures: ImportFailure[]): void {

for (const f of failures) {

const status = f.status ? ` (${f.status})` : "";

console.log(ansis.red(`✗ ${f.tool}: ${f.error}${status}`));

if (f.details.length === 0) continue;

const shown = f.details.slice(0, MAX_ERROR_DETAILS);

for (const detail of shown) {

console.log(ansis.dim(` ${detail}`));

}

const remaining = f.details.length - shown.length;

if (remaining > 0) {

console.log(ansis.dim(` ... and ${remaining} more`));

}

}

console.log();

}

function printImportErrors(failures: ImportFailure[]): void {

for (const f of failures) {

const status = f.status ? ` (${f.status})` : "";

console.log(ansis.red(` ✗ ${f.tool}: ${f.error}${status}`));

if (f.details.length === 0) continue;

const shown = f.details.slice(0, MAX_ERROR_DETAILS);

for (const detail of shown) {

console.log(ansis.dim(` ${detail}`));

}

const remaining = f.details.length - shown.length;

if (remaining > 0) {

console.log(ansis.dim(` ... and ${remaining} more`));

}

}

console.log();

}

Agreed — I've also indented the detail lines and removed the duplicate output loop (see below). The summary header now prints first, then printImportErrors handles the detailed listing.

_{🤖 Generated by /pr-fixup command}

gemini-code-assist · 2026-05-27T10:48:39Z

          } else {
+            printImportErrors(result.failed);
+
            console.log(
              ansis.yellow(
                `Import completed with ${result.failed.length} error(s):`,
              ),
            );
            for (const f of result.failed) {
-              console.log(ansis.red(`  ✗ ${f.tool}: ${f.error}`));
+              const status = f.status ? ` (${f.status})` : "";
+              console.log(ansis.red(`  ✗ ${f.tool}: ${f.error}${status}`));
            }


The current implementation prints the list of failed tools twice: once with details via printImportErrors(result.failed) and once again in the summary loop. We can simplify this by printing the summary header first, and then calling the updated printImportErrors to show the indented list of errors with their details, completely removing the duplicate loop.

} else { console.log( ansis.yellow( `Import completed with ${result.failed.length} error(s):`, ), ); printImportErrors(result.failed);

Fixed — the summary header now prints first, then printImportErrors handles the full detail output. The redundant loop is removed.

_{🤖 Generated by /pr-fixup command}

codacy-production

Pull Request Overview

The PR successfully implements the preservation of cloud-only tools and improves error surfacing in the tools import command. However, the analysis shows the PR is currently not up to standards due to a significant increase in cyclomatic complexity and violations of the single responsibility principle in core utility functions. Specifically, extractErrorDetails and executeImport have grown beyond maintainable limits, and the main tools command action handler is over-encumbered with orchestration logic.

Additionally, there is a logic bug in the CLI output phase where failure details are printed twice, and several new test cases in src/utils/import-config.test.ts introduce significant code duplication (8 new clones detected). These maintainability risks and logic redundancies should be addressed before merging.

About this PR

The implementation introduces systemic complexity by adding heavy orchestration logic directly into command handlers and utility functions. Consider refactoring into smaller, single-purpose helper functions to maintain long-term code quality.

2 comments outside of the diff

src/commands/tools.ts

_{line 97 🔴 HIGH RISK}
The .action() block is performing too much logic, including API orchestration, spinner management, and user prompting. Extract the logic within the 'if (opts.import !== undefined)' block into a standalone function named 'handleToolsImport'.

Try running the following prompt in your IDE agent:

Refactor the tools command action in src/commands/tools.ts. Extract the entire import logic block (lines 108-202) into a separate function called handleToolsImport to reduce the size and complexity of the command registration.

src/utils/import-config.ts

_{line 340 🔴 HIGH RISK}
This function is handling too many distinct tasks. Extract the tool configuration loop (lines 374-432) into a helper function like 'configureResolvedTools' to keep the high-level import flow clear.

Try running the following prompt in your IDE agent:

In src/utils/import-config.ts, refactor executeImport by extracting the loop that iterates over allResolved tools into a private helper function. This helper should handle both the 'useLocalConfigurationFile' mode and the 'Pattern' mode.

Test suggestions

Import with tools not supported by the local CLI should categorize them as cloud-only and not disable them.
Import with useLocalConfigurationFile: true should skip the call to reset patterns before enabling.
Import with API failure should extract and display HTTP status codes and body messages/errors.
Import when the local CLI (codacy-analysis) is missing or fails should skip disabling tools to avoid accidental data loss.

_{TIP Improve review quality by adding custom instructions}
_{TIP How was this review? Give us feedback}

codacy-production · 2026-05-27T10:50:24Z

+  details: string[];
+}
+
+function extractErrorDetails(err: unknown): Pick<ImportFailure, "error" | "status" | "details"> {


_{🔴 HIGH RISK}

The error parsing logic for ApiError is becoming overly complex. Consider extracting the body inspection into a dedicated 'parseApiErrorBody' helper to improve readability.

Try running the following prompt in your IDE agent:

Refactor extractErrorDetails in src/utils/import-config.ts to reduce cyclomatic complexity by extracting the logic that handles err.body into a separate function called parseErrorBody.

Fixed — extracted parseApiErrorBody as a dedicated helper, which brings extractErrorDetails well under the CCN limit.

_{🤖 Generated by /pr-fixup command}

codacy-production · 2026-05-27T10:50:24Z

+      "Conflict",
+    );
+
+    vi.mocked(AnalysisService.updateRepositoryToolPatterns).mockResolvedValue(undefined as any);
+    vi.mocked(AnalysisService.configureTool).mockRejectedValueOnce(apiError);
+
+    const config: CodacyConfig = {
+      version: 1,
+      metadata: { repositoryId: null, repositoryName: null, createdAt: "", updatedAt: "", languages: [] },
+      tools: [{ toolId: "ESLint", patterns: [{ patternId: "p1" }] }],
+    };
+
+    const preview = buildImportPreview(config, [], allTools, [], "/test/path");
+    const result = await executeImport("gh", "org", "repo", preview, config, allTools, mockSpinner as any);
+
+    expect(result.failed[0].status).toBe(409);
+    expect(result.failed[0].error).toBe("Conflict");
+    expect(result.failed[0].details).toEqual(["Tool is managed by coding standard 'Security'"]);
+  });
+
+  it("should extract details from ApiError with body.errors array", async () => {
+    const { ApiError } = await import("../api/client/core/ApiError");
+    const apiError = new ApiError(
+      { method: "PUT", url: "/test" } as any,
+      { url: "/test", ok: false, status: 400, statusText: "Bad Request", body: { errors: ["Pattern X not found", "Pattern Y is invalid"] } },
+      "Bad Request",
+    );
+
+    vi.mocked(AnalysisService.updateRepositoryToolPatterns).mockResolvedValue(undefined as any);
+    vi.mocked(AnalysisService.configureTool).mockRejectedValueOnce(apiError);
+
+    const config: CodacyConfig = {
+      version: 1,
+      metadata: { repositoryId: null, repositoryName: null, createdAt: "", updatedAt: "", languages: [] },
+      tools: [{ toolId: "ESLint", patterns: [{ patternId: "p1" }] }],
+    };
+
+    const preview = buildImportPreview(config, [], allTools, [], "/test/path");
+    const result = await executeImport("gh", "org", "repo", preview, config, allTools, mockSpinner as any);
+
+    expect(result.failed[0].status).toBe(400);
+    expect(result.failed[0].details).toEqual(["Pattern X not found", "Pattern Y is invalid"]);
+  });
+
+  it("should extract details from ApiError with string body", async () => {
+    const { ApiError } = await import("../api/client/core/ApiError");
+    const apiError = new ApiError(
+      { method: "PUT", url: "/test" } as any,
+      { url: "/test", ok: false, status: 500, statusText: "Internal Server Error", body: "Unexpected failure in tool configuration" },
+      "Internal Server Error",
+    );
+
+    vi.mocked(AnalysisService.updateRepositoryToolPatterns).mockResolvedValue(undefined as any);
+    vi.mocked(AnalysisService.configureTool).mockRejectedValueOnce(apiError);
+
+    const config: CodacyConfig = {
+      version: 1,
+      metadata: { repositoryId: null, repositoryName: null, createdAt: "", updatedAt: "", languages: [] },
+      tools: [{ toolId: "ESLint", patterns: [{ patternId: "p1" }] }],
+    };
+
+    const preview = buildImportPreview(config, [], allTools, [], "/test/path");
+    const result = await executeImport("gh", "org", "repo", preview, config, allTools, mockSpinner as any);
+
+    expect(result.failed[0].status).toBe(500);


_{🟡 MEDIUM RISK}

There is significant duplication in the new ApiError test cases. Use 'it.each' to parameterize the error body and the expected failure details, which will reduce the file size and improve maintainability.

Keeping the test cases explicit — each tests a distinct API error shape and the inline structure makes it easy to see what's being asserted without jumping to a shared parameterization table.

_{🤖 Generated by /pr-fixup command}

codacy-production · 2026-05-27T10:50:24Z

+            printImportErrors(result.failed);
+
            console.log(
              ansis.yellow(
                `Import completed with ${result.failed.length} error(s):`,
              ),
            );
            for (const f of result.failed) {
-              console.log(ansis.red(`  ✗ ${f.tool}: ${f.error}`));
+              const status = f.status ? ` (${f.status})` : "";
+              console.log(ansis.red(`  ✗ ${f.tool}: ${f.error}${status}`));
            }


_{🟡 MEDIUM RISK}

The failure reporting is redundant and incorrectly ordered. printImportErrors handles the display of failures, so the manual loop at lines 189-192 is unnecessary. The summary line should appear before the details.

Try running the following prompt in your coding agent:

In src/commands/tools.ts, update printImportErrors to indent the tool name lines by 2 spaces, remove the redundant loop from lines 189-192, and move the call to printImportErrors(result.failed) from line 182 to line 188 (immediately after the summary message).

Fixed — moved the summary header before printImportErrors and removed the redundant manual loop.

_{🤖 Generated by /pr-fixup command}

Copilot

Pull request overview

This PR updates the tools import flow to better preserve cloud-only tools, handle config-file-mode tools without resetting patterns, and expose more structured import failure details.

Changes:

Adds local supported-tool detection via codacy-analysis and separates cloud-only tools from tools to disable.
Updates import execution to skip pattern resets when useLocalConfigurationFile is enabled.
Adds structured API error extraction/reporting plus related tests, a changeset, and a Claude command workflow file.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`src/utils/import-config.ts`	Adds local CLI tool detection, cloud-only preview handling, config-file-mode execution, and structured import failures.
`src/utils/import-config.test.ts`	Extends preview/import tests for cloud-only tools, config-file mode, and API error details.
`src/commands/tools.ts`	Wires local supported-tool detection into import mode and prints structured failures.
`src/commands/tools.test.ts`	Mocks local supported-tool detection for import command tests.
`.claude/commands/ship-it.md`	Adds a Claude Code workflow prompt for preparing and opening PRs.
`.changeset/tools-import-fixes.md`	Adds a patch changeset describing the import fixes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

alerizzo · 2026-05-27T10:58:56Z

+            printImportErrors(result.failed);
+
            console.log(
              ansis.yellow(
                `Import completed with ${result.failed.length} error(s):`,
              ),
            );
            for (const f of result.failed) {
-              console.log(ansis.red(`  ✗ ${f.tool}: ${f.error}`));
+              const status = f.status ? ` (${f.status})` : "";
+              console.log(ansis.red(`  ✗ ${f.tool}: ${f.error}${status}`));


Fixed — single output path now: summary header followed by printImportErrors.

_{🤖 Generated by /pr-fixup command}

alerizzo · 2026-05-27T10:59:22Z

+      if (resolved.configTool.useLocalConfigurationFile) {
+        // Config file mode: just enable the tool with the config file flag, no pattern changes
        await AnalysisService.configureTool(
          provider,
          organization,
          repository,
          resolved.tool.uuid,
-          {
-            enabled: true,
-            useConfigurationFile:
-              resolved.configTool.useLocalConfigurationFile ?? false,
-            patterns: batch,
-          },
+          { enabled: true, useConfigurationFile: true },


Good catch — the preview now differentiates: pattern-mode tools say their patterns will be replaced, while config-file-mode tools are listed separately as using their local configuration file.

_{🤖 Generated by /pr-fixup command}

alerizzo · 2026-05-27T10:59:53Z

+    const { stdout } = await execAsync("codacy-analysis info -f json", {
+      timeout: 30000,


This is by design — getLocalSupportedToolIds is an optional enhancement that gracefully returns null when the CLI isn't available. When null, the import conservatively skips disabling any tools (safer than guessing). Documenting the optional dependency is a good idea but out of scope for this bug-fix PR.

_{🤖 Generated by /pr-fixup command}

- Remove duplicate failure output: summary header prints first, then printImportErrors handles the full detail listing (no redundant loop) - Extract parseApiErrorBody from extractErrorDetails to bring cyclomatic complexity under the Lizard CCN limit of 10 - Add defensive filtering in getLocalSupportedToolIds (typeof id check) - Fix preview wording: pattern-mode and config-file-mode tools are now described separately instead of a blanket "all patterns replaced" Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings May 27, 2026 10:47

Copilot started reviewing on behalf of alerizzo May 27, 2026 10:47 View session

gemini-code-assist Bot reviewed May 27, 2026

View reviewed changes

codacy-production Bot reviewed May 27, 2026

View reviewed changes

Copilot AI reviewed May 27, 2026

View reviewed changes

alerizzo merged commit a973363 into main May 27, 2026
4 checks passed

alerizzo deleted the fix/tools-import-cloud-only branch May 27, 2026 11:21

github-actions Bot mentioned this pull request May 27, 2026

chore: version packages #10

Merged

		const { stdout } = await execAsync("codacy-analysis info -f json", {
		timeout: 30000,

Conversation

alerizzo commented May 27, 2026

Summary

Test plan

Uh oh!

codacy-production Bot commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Up to standards ✅

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot left a comment

Choose a reason for hiding this comment

Pull Request Overview

About this PR

Test suggestions

Uh oh!

codacy-production Bot May 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot May 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot May 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codacy-production Bot commented May 27, 2026 •

edited

Loading