fix: list --format json outputs [] for empty collections#9
Merged
Merged
Conversation
list --format json previously fell through to a human-readable "No matching documents found" branch when a query returned zero documents, printing that text (or nothing, with --quiet) instead of valid JSON. This broke the JSON output contract that non-empty results already honor (OutputFormatter.formatDocuments returns '[]' for an empty array). Make the empty-result branch in ListCommand#queryCollection check format === 'json' first and print the literal "[]", matching the existing convention, regardless of --quiet. Non-JSON formats keep their prior text-message behavior unchanged. Fixes #6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
firex list <path> --format jsondid not produce valid JSON for an empty collection: it printed the human-readable textNo matching documents found(or nothing at all with--quiet) instead of[].OutputFormatter.formatDocumentsalready returns'[]'for an empty array (seesrc/presentation/output-formatter.ts), so the non-empty JSON path was already correct — the bug was thatListCommand#queryCollectionshort-circuited on an empty result before reaching that formatter, printing text unconditionally instead.Root cause
In
src/commands/list.ts,queryCollectionhad:This branch ran regardless of
--format, so--format jsonon an empty collection either printed nothing (--quiet) or the text message — neither of which is parseable JSON.Fix
Check
format === 'json'first in the empty-result branch and print the literal[](matching the existing convention already used byOutputFormatter.formatDocumentsfor non-empty JSON output), regardless of--quiet. Non-JSON formats (yaml/table/toon) keep their existing "No matching documents found" text behavior unchanged — only the JSON contract is fixed.Tests
Added to
src/commands/list.test.ts:--format json+ empty collection,quiet=false→ stdout is exactly[]--format json+ empty collection,quiet=true→ stdout is exactly[]--format json+ non-empty collection → unchanged (regression)quiet=false→ still printsNo matching documents found(unchanged)quiet=true→ no output (unchanged)src/mcp/tools/list.ts(MCP tool) was checked and does not share this bug — it always wraps output in{ count, executionTimeMs, documents: [...] }, which is valid JSON even whendocumentsis empty.Local verification
npm run typecheck— passnpm test— 1044/1044 tests pass (59 files)npm run build— passnpm run lint— could not verify: this repo has no ESLint config file anywhere in git history (confirmed viagit log --all --diff-filter=A -- '*eslintrc*' 'eslint.config*'), sonpm run lintfails with "ESLint couldn't find a configuration file" on a clean checkout ofmasteritself, unrelated to this change.Note on CI status (pre-existing, unrelated issue)
CI on
masterhas been failing for several pushes now (including the 0.9.0 and 0.9.1 release commits) at theInstall dependenciesstep (npm ci) with:This is a pre-existing
package-lock.json/peer-dependency issue orthogonal to this fix (reproduced on a pristinemastercheckout withoutlegacy-peer-deps). This PR's CI run is expected to hit the same failure. Flagging it here rather than bundling an unrelated lockfile fix into this PR.Fixes #6