Improve test isolation and coverage for CLI and VS Code by Looted · Pull Request #129 · Looted/kibi

Looted · 2026-03-30T20:22:22Z

No description provided.

…ests

…iders

Copilot

Pull request overview

This PR expands automated coverage across the VS Code extension, CLI traceability helpers, and MCP tools, while improving VS Code hover-provider test isolation via an explicit dependency-injection seam.

Changes:

Add extensive Bun-based unit tests for VS Code providers (Tree/CodeLens/CodeAction/Hover) plus a reusable VS Code mock module.
Add broad unit coverage for MCP tools (kb_upsert, entity query, delete) and CLI traceability/query utilities.
Introduce a dependency-injection seam in KibiHoverProvider to prevent cross-test mock leakage; bump package versions and update changelogs accordingly.

Reviewed changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
packages/vscode/tests/treeProvider.test.ts	Adds comprehensive TreeProvider unit coverage using filesystem fixtures and VS Code mocks.
packages/vscode/tests/shared/vscode-mock.ts	Introduces a reusable VS Code module mock with resettable state for test isolation.
packages/vscode/tests/relationshipCache.test.ts	Adds unit tests for relationship cache behavior (get/set/TTL/inflight).
packages/vscode/tests/hoverProvider.test.ts	Adds hover-provider unit tests (caching, cancellation, CLI parsing) using injected deps.
packages/vscode/tests/codeLensProvider.test.ts	Adds extensive CodeLens provider tests (watchers, debounce, caching, resolve).
packages/vscode/tests/codeActionProvider.test.ts	Adds CodeAction provider tests plus browse/open helper behavior verification.
packages/vscode/src/hoverProvider.ts	Adds `deps?: Partial<HoverProviderDeps>` seam and routes CLI/markdown calls through deps.
packages/vscode/package.json	Bumps `kibi-vscode` version to 0.2.3 and reformats arrays.
packages/vscode/CHANGELOG.md	Adds 0.2.3 entry documenting hover-provider DI seam.
packages/opencode/tests/logger.test.ts	Adds Vitest coverage for structured logging behavior and client reset semantics.
packages/opencode/package.json	Bumps `kibi-opencode` version to 0.5.3.
packages/opencode/CHANGELOG.md	Adds 0.5.3 release notes for workspace/logging improvements.
packages/mcp/tests/tools/upsert.test.ts	Adds extensive `kb_upsert` unit coverage including validation and failure formatting paths.
packages/mcp/tests/tools/entity-query.test.ts	Adds unit tests for entity-query helper functions and Prolog binding parsing.
packages/mcp/tests/tools/delete.test.ts	Adds unit tests for delete handler (existence, dependents, escaping, error handling).
packages/mcp/package.json	Bumps `kibi-mcp` version to 0.5.1 and updates `kibi-cli` dependency to ^0.4.2.
packages/mcp/CHANGELOG.md	Adds 0.5.1 entry describing new upsert coverage and dependency update.
packages/cli/tests/traceability/validate.test.ts	Adds direct edge-case coverage for Prolog parsing helpers via `__test__`.
packages/cli/tests/traceability/symbol-extract.test.ts	Adds broad symbol-extraction tests covering caching and multiple fallback strategies.
packages/cli/tests/traceability/markdown-validate.test.ts	Adds markdown staged validation tests for frontmatter handling and parse errors.
packages/cli/tests/traceability/git-staged.test.ts	Adds staged-file parsing tests with both mocked git output and real temp repo cases.
packages/cli/tests/query/service.test.ts	Adds query service tests covering goal building, codec integration, filtering, pagination.
packages/cli/tests/extractors/symbols-coordinator.test.ts	Adds coordinator tests covering ts-morph delegation and regex fallback behavior.
packages/cli/tests/commands/discovery-shared.test.ts	Adds tests for Prolog attachment, JSON module queries, and table rendering paths.
packages/cli/tests/commands/aggregated-checks.test.ts	Adds tests for aggregated checks parsing, filtering, and error cases.
packages/cli/src/traceability/validate.ts	Exports `__test__` helpers for edge-case testing of internal Prolog parsing utilities.
packages/cli/package.json	Bumps `kibi-cli` version to 0.4.2.
packages/cli/CHANGELOG.md	Adds 0.4.2 entry documenting exported `__test__` helpers.
documentation/tests/TEST-mcp-upsert-coverage.md	Adds a documentation test entity describing validation/coverage steps for MCP upsert.
.changeset/opencode-plugin-logging-remediation.md	Removes changeset file (presumably consumed by versioning).
.changeset/honor-sync-path-bootstrap.md	Removes changeset file (presumably consumed by versioning).

Comments suppressed due to low confidence (2)

packages/vscode/src/hoverProvider.ts:168

queryRelationshipsViaCli builds a shell command by interpolating symbolId into a single string passed to execSync. If symbolId contains shell metacharacters this becomes command-injection prone. Prefer execFileSync/spawnSync with an args array (or strictly validate/escape IDs) so the CLI receives symbolId as a literal argument.

  private async queryRelationshipsViaCli(
    symbolId: string,
  ): Promise<Array<{ type: string; from: string; to: string }>> {
    try {
      const output = this.deps.execCli(
        `bun run packages/cli/bin/kibi query --relationships ${symbolId} --format json`,
        {
          cwd: this.workspaceRoot,
          encoding: "utf8",
          timeout: 10000,
          stdio: ["ignore", "pipe", "ignore"],
        },
      );

packages/vscode/src/hoverProvider.ts:264

queryEntityViaCli interpolates entityId directly into a shell command string. Even if IDs are expected to be well-formed, this is a command-injection vector if a malicious ID ever reaches this codepath. Use execFileSync/spawnSync with args (or enforce a strict ^[A-Z]+-[A-Za-z0-9_-]+$-style validation) before executing.

  private async queryEntityViaCli(
    entityId: string,
  ): Promise<EntityDetails | null> {
    try {
      // Extract entity type from ID prefix (e.g., REQ-001 -> req)
      const typeMatch = entityId.match(/^([A-Z]+)-/);
      if (!typeMatch) return null;

      const typePrefix = typeMatch[1];
      const typeMap: Record<string, string> = {
        REQ: "req",
        SCEN: "scenario",
        TEST: "test",
        ADR: "adr",
        FLAG: "flag",
        EVENT: "event",
        SYM: "symbol",
      };

      const entityType = typeMap[typePrefix];
      if (!entityType) return null;

      const output = this.deps.execCli(
        `bun run packages/cli/bin/kibi query ${entityType} --id ${entityId} --format json`,
        {
          cwd: this.workspaceRoot,
          encoding: "utf8",
          timeout: 10000,
          stdio: ["ignore", "pipe", "ignore"],
        },
      );

Copilot · 2026-03-30T20:29:52Z

packages/vscode/tests/hoverProvider.test.ts

+const {
+  buildHoverMarkdown: realBuildHoverMarkdown,
+  categorizeEntities: realCategorizeEntities,
+  formatLensTitle: realFormatLensTitle,
+} = await import("../src/helpers?real");


The destructuring import from ../src/helpers?real includes realCategorizeEntities and realFormatLensTitle, but they are never used. With biome check . enabled, unused imports/locals can fail CI; remove the unused bindings or add assertions that use them.

Suggested change

const {

buildHoverMarkdown: realBuildHoverMarkdown,

categorizeEntities: realCategorizeEntities,

formatLensTitle: realFormatLensTitle,

} = await import("../src/helpers?real");

const { buildHoverMarkdown: realBuildHoverMarkdown } = await import(

"../src/helpers?real"

);

Copilot · 2026-03-30T20:29:53Z

packages/mcp/tests/tools/upsert.test.ts

+const initialKibiMcpDebug = process.env.KIBI_MCP_DEBUG ?? "";
+
+function createMockProlog(
+  handler: (goal: string) => Promise<QueryResult> | QueryResult,
+) {
+  const query = mock(async (goal: string) => {
+    const result = await handler(goal);
+    return { bindings: {}, ...result };
+  });
+  const invalidateCache = mock(() => {});
+
+  return {
+    query,
+    invalidateCache,
+    prolog: {
+      query,
+      invalidateCache,
+    } as unknown as PrologProcess,
+  };
+}
+
+afterEach(() => {
+  mock.restore();
+  process.env.KIBI_MCP_DEBUG = initialKibiMcpDebug;
+});


initialKibiMcpDebug is captured as process.env.KIBI_MCP_DEBUG ?? "" and later restored via assignment. If the env var was originally unset, this leaves it set to an empty string instead of restoring undefined, which can change behavior for later tests. Capture the original value as string | undefined and restore by deleting the key when it was originally undefined.

Copilot · 2026-03-30T20:29:53Z

packages/mcp/tests/tools/upsert.test.ts

+    expect(transactionGoal).toContain("value_int=30");
+    expect(transactionGoal).toContain("closed_world=false");
+    expect(transactionGoal).toContain('tags=["alpha","beta"]');
+    expect(transactionGoal).toContain('owner="undefined"');


This test asserts owner: undefined is encoded as owner="undefined" in the Prolog transaction goal. That locks in a surprising serialization behavior; it’s usually safer to omit undefined/null properties entirely when building the property list. Consider updating the encoder (and the expectation here) to skip undefined values so accidental undefineds don’t persist into the KB.

Suggested change

expect(transactionGoal).toContain('owner="undefined"');

expect(transactionGoal).not.toContain("owner=");

Copilot · 2026-03-30T20:29:53Z

packages/vscode/tests/treeProvider.test.ts

+const { KibiTreeDataProvider } = await import("../src/treeProvider");
+
+type TreeProviderInstance = InstanceType<typeof KibiTreeDataProvider>;
+
+let tmpDir: string;


TreeProviderInstance type alias is declared but never used. If Biome/TS unused checks run on tests, this can fail CI; remove the alias or use it to type provider where appropriate.

Looted · 2026-03-30T20:30:17Z

@copilot apply changes based on the comments in this thread in a separate PR.

Copilot · 2026-03-30T20:30:26Z

@Looted I've opened a new pull request, #130, to work on those changes. Once the pull request is ready, I'll request review from you.

…ps, unused type) Agent-Logs-Url: https://github.com/Looted/kibi/sessions/3d0a4d79-9903-4572-9709-bdd79879f7d8 Co-authored-by: Looted <6255880+Looted@users.noreply.github.com>

fix: address PR review feedback on test isolation and undefined property encoding

…r coverage Add mock.restore() to afterEach hook to clean up mock.module mocks after each test. This prevents the mock from upsert.test.ts from leaking into symbols.test.ts when running with --coverage flag. Fixes: 9 refreshCoordinatesForSymbolId tests failing on CI Root cause: Bun hoists mock.module() calls; snip without cleanup, mocks leak across test files under coverage instrumentation.

Looted added 9 commits March 30, 2026 17:44

fix(vscode): isolate hover provider test child_process mocking

0a4a0dc

fix(cli): export __test__ helpers from traceability/validate.ts

f6929cf

fix(test): remove prolog-cleanup mock leakage from discovery-shared t…

8c640ac

…ests

fix(test): remove init-helpers mock leakage from discovery-shared tests

484379c

test: add test coverage for CLI commands, MCP tools, and VS Code prov…

f7925a6

…iders

chore: add changeset for MCP upsert test coverage

c0d09e0

chore: add changesets for CLI and VSCode packages

7309d18

Release

8a97c68

Merge branch 'develop' of https://github.com/Looted/kibi into develop

225f1a5

Copilot AI review requested due to automatic review settings March 30, 2026 20:22

Copilot started reviewing on behalf of Looted March 30, 2026 20:23 View session

Copilot AI reviewed Mar 30, 2026

View reviewed changes

Initial plan

9340a20

Copilot AI mentioned this pull request Mar 30, 2026

fix: address PR review feedback on test isolation and undefined property encoding #130

Merged

Copilot AI and others added 5 commits March 30, 2026 20:34

fix: apply PR review feedback (unused imports, env var, undefined pro…

d63d655

…ps, unused type) Agent-Logs-Url: https://github.com/Looted/kibi/sessions/3d0a4d79-9903-4572-9709-bdd79879f7d8 Co-authored-by: Looted <6255880+Looted@users.noreply.github.com>

Merge pull request #130 from Looted/copilot/sub-pr-129

de436ff

fix: address PR review feedback on test isolation and undefined property encoding

Symbols

452b8cc

chore(ci): add symbol refresh diagnostics for CI debugging

3388cf3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve test isolation and coverage for CLI and VS Code#129

Improve test isolation and coverage for CLI and VS Code#129
Looted wants to merge 15 commits intomasterfrom
develop

Looted commented Mar 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Looted commented Mar 30, 2026

Uh oh!

Copilot AI commented Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	expect(transactionGoal).toContain('owner="undefined"');
	expect(transactionGoal).not.toContain("owner=");

Conversation

Looted commented Mar 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Looted commented Mar 30, 2026

Uh oh!

Copilot AI commented Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants