issue-142: Implement the mongo_list_collections tool with unit tests #159

Merged

david merged 2 commits from feature/issue-142/implement-the-mongo-list-collections-tool-with-uni into main

2026-09-01 00:29:49 +00:00

david commented

2026-09-01 00:29:48 +00:00

Owner

142: Implement the mongo_list_collections tool with unit tests

Implements 142.

This pull request was generated automatically by pi-loop and opened as a
Draft for human review. Do not merge without reviewing the changes.

Generated in 5m 17s.

Review findings

Now let me check the repo's git state, AGENTS.md conventions, and how the build/test gates are configured.The working tree is clean and the Stage 1 change is commit 7bbeb15. Now I understand the full picture. Let me apply the Medium finding fix to testClient.ts — making the listCollections mock throw driver errors from the cursor's toArray() (async), keeping a synchronous throw only for MongoInvalidArgumentError, mirroring the real driver.Now let me verify the change and discover the project's build/test gates (root package.json, Makefile, etc.).The change looks correct. Now let me run the project gates: the mongodb extension tests, its TypeScript check, and the root test suite (all discovered from package.json scripts).Mongodb tests pass (89/0) and tsc --noEmit is clean. Now the root test suite:All gates pass. Let me confirm the final working-tree diff is exactly the intended change.All gates pass and the working-tree diff contains exactly the intended testClient.ts change. Here is the annotated findings document.

Review Findings — 142

Summary

The branch (feature/issue-142/..., commit 7bbeb15) adds three error-path unit tests to extensions/mongodb/src/tools/listCollections.test.ts (invalid-query / server / unexpected categories), completing the five-category error coverage the extension's DESIGN.md requires for each tool. Note: the mongo_list_collections implementation itself (src/tools/listCollections.ts) is already merged on origin/main (it landed in issue 139's PR), so this PR is test-only. Gates run: bun test (extensions/mongodb) — PASS (89 pass / 0 fail, including all 8 listCollections tests); bun run check (tsc --noEmit) — PASS; root npm test — PASS (58 pass, unrelated local-install tests); lint — N/A (the repo has no lint tooling configured — no eslint/biome/prettier configs or scripts). No critical or high findings; one medium test-fidelity gap and one low convention nit.

Remediation (single pass): the medium test-fidelity gap is fixed — extensions/mongodb/src/tools/testClient.ts now makes the listCollections mock return a cursor whose toArray() throws the configured driver error (async rejection, mirroring the find fake and the real driver), keeping a synchronous throw only for MongoInvalidArgumentError. Gates re-run after the fix: bun test (extensions/mongodb) PASS (89/0), bun run check PASS, root npm test PASS (58/0). The low convention nit remains unresolved — the commit message cannot be amended in this pipeline stage (no git commit/amend allowed; the captured diff cannot carry a commit-message change), and rewriting AGENTS.md's repo-wide commit convention is a repo-owner decision outside this test-only PR.

Critical

(none)

High

(none)

Medium

extensions/mongodb/src/tools/listCollections.test.ts:128 (also :109, :145) — The three new error tests (and the two pre-existing connection/auth tests they sit beside) exercise the shared fake's synchronous throw from db.listCollections() (extensions/mongodb/src/tools/testClient.ts:65-71), but the real driver surfaces server/network/auth errors only as an async rejection from the cursor's toArray() (synchronous throws are reserved for argument-validation errors like MongoInvalidArgumentError). The find fake deliberately throws from toArray() "mirroring the real driver" (see the testClient.ts header comment), and DESIGN.md's Testing Strategy documents the same rationale — so a regression that moves await …toArray() outside the tool's try block would pass these tests even though it would leak a raw MongoServerError in production. Suggested fix: in testClient.ts, make the listCollections mock return a cursor whose toArray() throws the configured error (mirroring the find fake), keeping a synchronous throw only for MongoInvalidArgumentError; the server/auth/connection assertions then validate the real async path.

Low

extensions/mongodb/src/tools/listCollections.test.ts:1 (commit 7bbeb15) — Commit message is 142: Implement the mongo_list_collections tool with unit tests, but AGENTS.md specifies the format issue-<N>: <summary> (e.g. issue-42: add user auth); the issue- prefix is missing. (This matches the actual merged history for issues 139–141, so the doc and the practice are out of sync.) Suggested fix: amend the commit message to issue-142: Implement the mongo_list_collections tool with unit tests, or update AGENTS.md's commit convention to match the repo's established <N>: form.

## 142: Implement the mongo_list_collections tool with unit tests Implements [142](https://git.excelera.net/david/pi-extensions-and-skills/issues/142). > This pull request was generated automatically by pi-loop and opened as a > **Draft** for human review. Do not merge without reviewing the changes. Generated in 5m 17s. ## Review findings Now let me check the repo's git state, AGENTS.md conventions, and how the build/test gates are configured.The working tree is clean and the Stage 1 change is commit `7bbeb15`. Now I understand the full picture. Let me apply the Medium finding fix to `testClient.ts` — making the `listCollections` mock throw driver errors from the cursor's `toArray()` (async), keeping a synchronous throw only for `MongoInvalidArgumentError`, mirroring the real driver.Now let me verify the change and discover the project's build/test gates (root package.json, Makefile, etc.).The change looks correct. Now let me run the project gates: the mongodb extension tests, its TypeScript check, and the root test suite (all discovered from package.json scripts).Mongodb tests pass (89/0) and `tsc --noEmit` is clean. Now the root test suite:All gates pass. Let me confirm the final working-tree diff is exactly the intended change.All gates pass and the working-tree diff contains exactly the intended `testClient.ts` change. Here is the annotated findings document. # Review Findings — 142 ## Summary The branch (`feature/issue-142/...`, commit `7bbeb15`) adds three error-path unit tests to `extensions/mongodb/src/tools/listCollections.test.ts` (invalid-query / server / unexpected categories), completing the five-category error coverage the extension's DESIGN.md requires for each tool. Note: the `mongo_list_collections` implementation itself (`src/tools/listCollections.ts`) is already merged on `origin/main` (it landed in issue 139's PR), so this PR is test-only. Gates run: `bun test` (extensions/mongodb) — **PASS** (89 pass / 0 fail, including all 8 listCollections tests); `bun run check` (`tsc --noEmit`) — **PASS**; root `npm test` — **PASS** (58 pass, unrelated local-install tests); lint — **N/A** (the repo has no lint tooling configured — no eslint/biome/prettier configs or scripts). No critical or high findings; one medium test-fidelity gap and one low convention nit. **Remediation (single pass):** the medium test-fidelity gap is **fixed** — `extensions/mongodb/src/tools/testClient.ts` now makes the `listCollections` mock return a cursor whose `toArray()` throws the configured driver error (async rejection, mirroring the `find` fake and the real driver), keeping a synchronous throw only for `MongoInvalidArgumentError`. Gates re-run after the fix: `bun test` (extensions/mongodb) **PASS** (89/0), `bun run check` **PASS**, root `npm test` **PASS** (58/0). The low convention nit **remains unresolved** — the commit message cannot be amended in this pipeline stage (no git commit/amend allowed; the captured diff cannot carry a commit-message change), and rewriting AGENTS.md's repo-wide commit convention is a repo-owner decision outside this test-only PR. ## Critical - (none) ## High - (none) ## Medium - [x] `extensions/mongodb/src/tools/listCollections.test.ts:128` (also `:109`, `:145`) — The three new error tests (and the two pre-existing connection/auth tests they sit beside) exercise the shared fake's **synchronous throw** from `db.listCollections()` (`extensions/mongodb/src/tools/testClient.ts:65-71`), but the real driver surfaces server/network/auth errors only as an **async rejection from the cursor's `toArray()`** (synchronous throws are reserved for argument-validation errors like `MongoInvalidArgumentError`). The `find` fake deliberately throws from `toArray()` "mirroring the real driver" (see the testClient.ts header comment), and DESIGN.md's Testing Strategy documents the same rationale — so a regression that moves `await …toArray()` outside the tool's `try` block would pass these tests even though it would leak a raw `MongoServerError` in production. Suggested fix: in `testClient.ts`, make the `listCollections` mock return a cursor whose `toArray()` throws the configured error (mirroring the `find` fake), keeping a synchronous throw only for `MongoInvalidArgumentError`; the server/auth/connection assertions then validate the real async path. ## Low - [ ] `extensions/mongodb/src/tools/listCollections.test.ts:1` (commit `7bbeb15`) — Commit message is `142: Implement the mongo_list_collections tool with unit tests`, but AGENTS.md specifies the format `issue-<N>: <summary>` (e.g. `issue-42: add user auth`); the `issue-` prefix is missing. (This matches the actual merged history for issues 139–141, so the doc and the practice are out of sync.) Suggested fix: amend the commit message to `issue-142: Implement the mongo_list_collections tool with unit tests`, or update AGENTS.md's commit convention to match the repo's established `<N>:` form.

david added 2 commits

2026-09-01 00:29:48 +00:00