P1B: Refactor (packages/opencode/src/acp/tool.ts): Function with many returns

[ralkubai]

1. Issue

Link to the associated GitHub issue: Closes #100

Full path to the refactored file: packages/opencode/src/acp/tool.ts

What do you think this file does?

It converts opencode's internal tool representations into the shapes the Agent Client Protocol expects — mapping tool names to ACP tool kinds, extracting file locations from tool inputs, and building content and raw-output payloads for completed tool calls.

What is the scope of your refactoring within that file?

Only the toToolKind function (line 38) and the new TOOL_KIND_BY_NAME constant introduced alongside it. No other function was touched.

Which Qlty-reported issue did you address?

38 Function with many returns (count = 7): toToolKind

2. Refactoring

How did the specific issue you chose impact the codebase's maintainability?

The mapping was expressed as a switch with seven separate exit points, so the tool-name-to-kind relationship was spread across roughly thirty lines of control flow. Adding a tool meant adding a case and reasoning about which return it fell through to, and there was no single place to read off the full mapping.

What changes did you make to resolve the issue?

I replaced the switch with a module-level Record<string, ToolKind> lookup table and reduced the function body to a single lookup with ?? "other" as the fallback. Every case label became a key with the same value, and the toLocaleLowerCase() normalization is unchanged.

How do your changes improve maintainability? Did you consider alternatives?

The complete mapping is now readable as data in one place, and adding a tool is a one-line change with no new control flow. The function drops from seven returns to one. I considered keeping the switch but grouping returns via a helper, which would have reduced the count without removing the underlying duplication of structure; the lookup table addresses the cause rather than the symptom.

3. Validation

How did you validate that the change is correct?

The existing test packages/opencode/test/acp/tool.test.ts → "maps OpenCode tool ids to ACP tool kinds" asserts every mapping the original switch handled (bash, shell, webfetch, edit, apply_patch, patch, write, grep, glob, context7_resolve_library_id, context7_get_library_docs, read, task) plus an unknown name falling through to other. Because it exercises all seven original branches, any behavioral drift in the new lookup would fail it. I added two cases (BASH, Apply_Patch) covering the case-insensitivity the lookup now relies on, which was previously untested. All 9 tests in the file pass with 42 assertions, and bun lint reports no issues in this file.

Attach a screenshot of the test coverage showing the lines were executed by the tests.

src/acp/tool.ts shows 82.76% function and 69.55% line coverage. The uncovered ranges are elsewhere in the file; toToolKind and the new TOOL_KIND_BY_NAME table sit in the covered portion.

Attach a screenshot showing the tests that cover the change passing during CI

Attach a screenshot of qlty smells --no-snippets <full/path/to/file.ts> showing fewer reported issues after the changes.

Before:

After:

[ralkubai]

Note on diff size: this branch includes earlier commits from Project 1A (devcontainer setup and cleanup of generated coverage files). The refactor itself is the last commit, 749661b, touching only packages/opencode/src/acp/tool.ts and its test file.