test: keybind + git utilities — parsing, matching, error handling

[anandgupta42]

What does this PR do?

Adds 27 new tests across two previously untested utility modules that are widely used in critical code paths.

1. Keybind — src/util/keybind.ts (22 new tests)

This module powers keyboard shortcut parsing, matching, and display across 6+ TUI components (dialog-select, textarea-keybindings, dialog-mcp, dialog-skill, permission, keybind context). Zero tests existed. A parsing bug would silently break user keybindings. New coverage includes:

• parse(): simple keys, modifier combos (ctrl+shift+a), all meta aliases (alt/meta/option), super modifier, <leader> prefix, esc→escape normalization, "none" sentinel, comma-separated multi-bindings
• toString(): modifier ordering, delete→del mapping, leader prefix formatting, undefined handling
• match(): identical bindings, undefined first arg, super field normalization (undefined treated as false), non-matching keys
• Roundtrip: parse → toString for ctrl+shift+a, and verification that meta aliases (meta/option) normalize to alt on roundtrip (lossy but correct behavior)

2. git() — src/util/git.ts (5 new tests)

This thin wrapper around Process.run is used in 8+ files (file/index, project, vcs, storage, github, pr, worktree). It shapes Process.run results into GitResult and has a .catch() branch for thrown errors. Zero tests existed. New coverage includes:

• Happy path: rev-parse returns stdout with exitCode 0
• Unknown subcommand returns non-zero exit code
• stderr is populated on checkout of nonexistent branch
• Custom env vars are passed through (verified via GIT_CONFIG_COUNT injection — the config value only exists because of the env var)
• Non-existent cwd returns exitCode != 0 with empty stdout

Type of change

• New feature (non-breaking change which adds functionality)

Issue for this PR

N/A — proactive test coverage via /test-discovery

How did you verify your code works?

bun test test/util/keybind.test.ts   # 22 pass
bun test test/util/git.test.ts       #  5 pass

Checklist

• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

https://claude.ai/code/session_014mq5rKy5AhzTbSNH9rhxFF

Summary by CodeRabbit

• Tests
  • Added comprehensive test coverage for git operations and keybinding functionality to ensure reliability and correct behavior across the codebase.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

_{Tip: disable this comment in your organization's Code Review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Two new test suites added for git and keybind utilities. The git test suite validates command execution, exit codes, stderr handling, environment variable propagation, and error cases. The keybind test suite covers parsing, string formatting, matching logic, and roundtrip transformations.

Changes

Cohort / File(s)	Summary
Test Utilities packages/opencode/test/util/git.test.ts, packages/opencode/test/util/keybind.test.ts	Added comprehensive test suites for git and keybind utilities with coverage for success cases, error handling, environment variables, edge cases, and roundtrip transformations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

contributor

Poem

🐰 Tests now guard the git and key,
Each case checked with certainty,
Parse and bind with confidence true,
The code hops forward, shiny and new!

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change—adding tests for keybind and git utilities with focus on parsing, matching, and error handling.
Description check	✅ Passed	The description covers all required sections: summary of what changed and why, detailed test plan with verification commands, and completed checklist. All critical information is present and well-organized.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 test/hourly-20260326-2115

---

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 and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

packages/opencode/test/util/keybind.test.ts (1)

45-49: Consider adding a test for <leader> without a following key.

The parse tests cover <leader>a but don't test <leader> alone. While toString tests this case (lines 99-104), a corresponding parse test would ensure symmetry.

Optional test for parsing leader-only binding

   test("parses <leader> prefix", () => {
     const [info] = Keybind.parse("<leader>a")
     expect(info.leader).toBe(true)
     expect(info.name).toBe("a")
   })
+
+  test("parses <leader> without following key", () => {
+    const [info] = Keybind.parse("<leader>")
+    expect(info.leader).toBe(true)
+    expect(info.name).toBe("")
+  })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/test/util/keybind.test.ts` around lines 45 - 49, Add a unit
test that calls Keybind.parse("<leader>") and asserts the parsed result marks
leader as true and has an empty name to mirror the existing toString coverage;
locate the test near the other parse cases (the existing "parses <leader>
prefix" test) and assert on Keybind.parse's returned info.leader and info.name
(expect info.leader toBe(true) and info.name toBe("")).

packages/opencode/test/util/git.test.ts (1)

52-56: Hardcoded /tmp/ path may not be portable across platforms.

The path /tmp/nonexistent-dir-... assumes a Unix-like filesystem. On Windows, this path doesn't exist and the test behavior may differ (though it would still fail with a non-zero exit code). Consider using path.join(os.tmpdir(), ...) for better portability, or document that this test is Unix-specific.

Portable alternative using Node's os.tmpdir()

+import * as os from "os"
+import * as path from "path"
+
 // ...
 
   test("returns exitCode 1 and empty stdout when cwd does not exist", async () => {
-    const result = await git(["status"], { cwd: "/tmp/nonexistent-dir-" + Math.random().toString(36) })
+    const nonexistentPath = path.join(os.tmpdir(), "nonexistent-dir-" + Math.random().toString(36))
+    const result = await git(["status"], { cwd: nonexistentPath })
     expect(result.exitCode).not.toBe(0)
     expect(result.stdout.length).toBe(0)
   })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/test/util/git.test.ts` around lines 52 - 56, The test uses
a hardcoded "/tmp/..." path which isn't portable; update the test that calls
git(["status"], { cwd: ... }) to construct the non-existent directory using
Node's os.tmpdir() and path.join (e.g., path.join(os.tmpdir(),
`nonexistent-dir-${Math.random().toString(36)}`)), and add the necessary imports
for os and path at the top of the test file; keep the assertion logic the same
so the test still expects a non-zero exitCode and empty stdout.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/opencode/test/util/git.test.ts`:
- Around line 52-56: The test uses a hardcoded "/tmp/..." path which isn't
portable; update the test that calls git(["status"], { cwd: ... }) to construct
the non-existent directory using Node's os.tmpdir() and path.join (e.g.,
path.join(os.tmpdir(), `nonexistent-dir-${Math.random().toString(36)}`)), and
add the necessary imports for os and path at the top of the test file; keep the
assertion logic the same so the test still expects a non-zero exitCode and empty
stdout.

In `@packages/opencode/test/util/keybind.test.ts`:
- Around line 45-49: Add a unit test that calls Keybind.parse("<leader>") and
asserts the parsed result marks leader as true and has an empty name to mirror
the existing toString coverage; locate the test near the other parse cases (the
existing "parses <leader> prefix" test) and assert on Keybind.parse's returned
info.leader and info.name (expect info.leader toBe(true) and info.name
toBe("")).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 5f8df291-efbd-409a-b4c9-cc86e1fc3a55

📥 Commits

Reviewing files that changed from the base of the PR and between abcaa1d and 8fa12f7.

📒 Files selected for processing (2)

• packages/opencode/test/util/git.test.ts
• packages/opencode/test/util/keybind.test.ts

[anandgupta42]

Consolidated into #498. Tests deduplicated and merged into a single PR.