fix: allow --recursive to delete document paths and their subcollections#8
Merged
hummer98 merged 1 commit intoJul 8, 2026
Merged
Conversation
`firex delete <doc-path> --recursive` was rejected with "--recursive can only be used with collection paths", an artificial CLI-side restriction. The underlying operation (delete a doc plus everything nested under it, mirroring Firestore's Firestore.recursiveDelete() which accepts either a CollectionReference or a DocumentReference) is a legitimate use case that callers previously had to work around by manually enumerating and deleting each subcollection before deleting the document. Add BatchProcessor.deleteDocument(), which deletes a document's subcollections (reusing the existing deleteCollection() recursion) and then the document itself, and route DeleteCommand to it when a document path is passed with --recursive instead of rejecting it. Fixes #5
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.
Root cause
firex delete <doc-path> --recursivewas unconditionally rejected by anartificial CLI-side check in
src/commands/delete.ts:The firebase-admin SDK's
Firestore.recursiveDelete()accepts either aCollectionReferenceor aDocumentReferenceand recursively deleteseverything underneath, subcollections included — there was no underlying
technical reason to disallow document paths. The restriction forced
callers to manually enumerate every subcollection and delete each one
individually before deleting the document itself.
Fix
src/commands/delete.ts.BatchProcessor.deleteDocument()insrc/domain/batch-processor.ts,which deletes a document's subcollections (reusing the existing
deleteCollection()recursion, the same pattern already used for thecollection-path case) and then deletes the document itself.
DeleteCommandnow routes document path +--recursiveto a newdeleteDocumentRecursivehandler that callsBatchProcessor.deleteDocument, the document-path analogue of howcollection path +
--recursivealready routes toBatchProcessor.deleteCollection.prompt.confirmDeleteDocumentRecursive,both locales) so the confirmation message makes clear subcollections are
included.
Checked
src/mcp/tools/delete.ts(the MCP tool wrapper): it does notduplicate this validation — it has no equivalent guard to remove, so no
change was needed there for this issue.
Tests
src/domain/batch-processor.test.ts: newdeleteDocumentdescribeblock covering delete-with-no-subcollections, recursive
subcollection deletion, and confirmation-denied cases.
src/commands/delete.test.ts: new tests confirming a document path isstill classified correctly, that
--recursiveon a document path nowroutes to
BatchProcessor.deleteDocument(the old restriction is gone),and a regression test confirming collection path +
--recursivestillroutes to
BatchProcessor.deleteCollectionas before.npm run typecheck,npm test(1046 tests), andnpm run buildall passlocally.
npm run lintcould not be run — this repository has no ESLintconfig file checked in at any point in its history (confirmed on
master), sonpm run lintfails with "ESLint couldn't find aconfiguration file" independent of this change; this is a pre-existing
issue unrelated to this fix.
Fixes #5