issue-140: Implement the mongo_find tool with unit tests #157

Merged

david merged 2 commits from feature/issue-140/implement-the-mongo-find-tool-with-unit-tests into main

2026-09-01 00:19:12 +00:00

david commented

2026-09-01 00:19:11 +00:00

Owner

140: Implement the mongo_find tool with unit tests

Implements 140.

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 35s.

Review findings

Now let me verify the EJSON malformed-filter behavior that finding 1 targets, and check how the fake client is used elsewhere:The bun -e failure was due to the bson v8 snapshot shim (only applied via bunfig preload under bun test). Let me verify the malformed-EJSON behavior through a temp test file run under bun test, then remove it:The probe confirms EJSON.deserialize({ _id: { $oid: 123 } }) throws a BSONError. Now let me remove the probe and check the sibling tools for consistency before fixing:Now I'll apply both fixes. First, the malformed-EJSON test in find.test.ts (finding 1), placed after the driver invalid-argument test:Now fix 2 — make the fake's find error surface as an async rejection from toArray(), mirroring the real driver, and note it in the doc comment:Now re-run all detected gates:Extension gates pass (82 tests, +1 new; tsc clean). Now the repo root gate:All gates pass. Let me verify the final state of both files and confirm no temp files remain:Both findings are fixed, all gates are green, and only the two intended files are modified. Here is the annotated findings document.

Review Findings — 140

Summary

Both Low-severity findings were resolved in a single pass. For the coverage gap, I added a tool-level test to find.test.ts that passes a malformed EJSON filter ({ _id: { $oid: 123 } }) to mongoFindTool.execute; EJSON.deserialize throws a BSONError before the driver is reached, and the test asserts an invalid-query-category ToolError plus that calls.find stayed empty (proving the failure came from the tool's own deserialization path, not the driver). For the sync-throw simplification, I changed the fake's find mock in testClient.ts so findError surfaces as an async rejection from the cursor's toArray() — mirroring the real driver, where query/server errors surface when the cursor is awaited — and added a doc comment explaining the design; a future refactor that moved toArray() outside the try would now fail these tests.

Gates re-run after the changes: bun test in extensions/mongodb (82 pass / 0 fail, +1 new test), bun run check (tsc --noEmit, clean), and repo root npm test (58 pass / 0 fail). No lint tooling is configured anywhere in this repo (only node_modules lint files exist), so there is no lint gate to run.

Critical

(none)

High

(none)

Medium

(none)

Low

extensions/mongodb/src/tools/find.test.ts:126 — The invalid-query category is only exercised via the driver's MongoInvalidArgumentError. The other invalid-query path inside the tool — a bad EJSON filter throwing BSONError from EJSON.deserialize at src/tools/find.ts:79 (e.g. { filter: { _id: { $oid: 123 } } }) — is covered only at the errors.ts unit level, not through the tool. Suggested fix: add a test that passes a malformed EJSON filter to tool.execute and asserts an invalid-query-category ToolError.
extensions/mongodb/src/tools/testClient.ts:33 — The fake's find mock throws findError synchronously, so the new server/unexpected tests (and the pre-existing connection/auth tests) exercise a synchronous throw from .find(), whereas real server errors surface as async rejections from await cursor.toArray() (find.ts:89). Behavior under test is identical because find.ts wraps both calls in the same try, but the sync-throw simplification could mask a future refactor that moved toArray() outside the try block. Suggested fix (optional): throw asynchronously (reject the promise returned by toArray) for the server/unexpected overrides, or add a brief comment in testClient.ts noting the deliberate sync-throw simplification.

## 140: Implement the mongo_find tool with unit tests Implements [140](https://git.excelera.net/david/pi-extensions-and-skills/issues/140). > 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 35s. ## Review findings Now let me verify the EJSON malformed-filter behavior that finding 1 targets, and check how the fake client is used elsewhere:The `bun -e` failure was due to the bson v8 snapshot shim (only applied via bunfig preload under `bun test`). Let me verify the malformed-EJSON behavior through a temp test file run under `bun test`, then remove it:The probe confirms `EJSON.deserialize({ _id: { $oid: 123 } })` throws a `BSONError`. Now let me remove the probe and check the sibling tools for consistency before fixing:Now I'll apply both fixes. First, the malformed-EJSON test in `find.test.ts` (finding 1), placed after the driver invalid-argument test:Now fix 2 — make the fake's `find` error surface as an async rejection from `toArray()`, mirroring the real driver, and note it in the doc comment:Now re-run all detected gates:Extension gates pass (82 tests, +1 new; tsc clean). Now the repo root gate:All gates pass. Let me verify the final state of both files and confirm no temp files remain:Both findings are fixed, all gates are green, and only the two intended files are modified. Here is the annotated findings document. # Review Findings — 140 ## Summary Both Low-severity findings were resolved in a single pass. For the coverage gap, I added a tool-level test to `find.test.ts` that passes a malformed EJSON filter (`{ _id: { $oid: 123 } }`) to `mongoFindTool.execute`; `EJSON.deserialize` throws a `BSONError` before the driver is reached, and the test asserts an `invalid-query`-category `ToolError` plus that `calls.find` stayed empty (proving the failure came from the tool's own deserialization path, not the driver). For the sync-throw simplification, I changed the fake's `find` mock in `testClient.ts` so `findError` surfaces as an async rejection from the cursor's `toArray()` — mirroring the real driver, where query/server errors surface when the cursor is awaited — and added a doc comment explaining the design; a future refactor that moved `toArray()` outside the `try` would now fail these tests. Gates re-run after the changes: `bun test` in `extensions/mongodb` (**82 pass / 0 fail**, +1 new test), `bun run check` (`tsc --noEmit`, clean), and repo root `npm test` (**58 pass / 0 fail**). No lint tooling is configured anywhere in this repo (only `node_modules` lint files exist), so there is no lint gate to run. ## Critical - (none) ## High - (none) ## Medium - (none) ## Low - [x] `extensions/mongodb/src/tools/find.test.ts:126` — The `invalid-query` category is only exercised via the driver's `MongoInvalidArgumentError`. The other invalid-query path inside the tool — a bad EJSON filter throwing `BSONError` from `EJSON.deserialize` at `src/tools/find.ts:79` (e.g. `{ filter: { _id: { $oid: 123 } } }`) — is covered only at the `errors.ts` unit level, not through the tool. Suggested fix: add a test that passes a malformed EJSON filter to `tool.execute` and asserts an `invalid-query`-category `ToolError`. - [x] `extensions/mongodb/src/tools/testClient.ts:33` — The fake's `find` mock throws `findError` synchronously, so the new server/unexpected tests (and the pre-existing connection/auth tests) exercise a synchronous throw from `.find()`, whereas real server errors surface as async rejections from `await cursor.toArray()` (`find.ts:89`). Behavior under test is identical because `find.ts` wraps both calls in the same `try`, but the sync-throw simplification could mask a future refactor that moved `toArray()` outside the `try` block. Suggested fix (optional): throw asynchronously (reject the promise returned by `toArray`) for the server/unexpected overrides, or add a brief comment in `testClient.ts` noting the deliberate sync-throw simplification.

david added 2 commits

2026-09-01 00:19:11 +00:00