[detail-app]

Detail bug report: View on Detail

Bug

The referenceChecks filters in packages/effect-firebase/src/lib/firestore/schema/reference.ts disagreed on what a path segment is. Filter 1 (isPathValid) elided empty segments via path.split('/').filter(Boolean), so it accepted non-canonical paths like users/doc123/, /users/abc/posts/def, or posts//1. Filter 2 then read the last segment with the un-filtered path.split('/').pop(), which yields '' for a trailing slash.

On the toCodecJson(ReferenceInstance) codec (and the bare Reference class decode) this produced contradictory validation:

• { id:'', path:'users/doc123/' } was admitted (a malformed path accepted only because the empty id happened to match the trailing-slash pop()).
• { id:'doc123', path:'users/doc123/' } was rejected with Id must match the last part of the path, blaming a correct id when path was the malformed field.

This contradicts the library's shared, tested validateDocPath contract (path.ts), which rejects every empty-segment path and is used by the admin/client/mock backends. The divergence was exposed when db21434 applied the pre-existing contradictory filter pair to the codec's decode path.

Fix

Filter 1 now reuses validateDocPath(path) === undefined (imported from ../path.js) instead of the local isPathValid, so empty-segment paths are rejected consistently with the rest of the library. The message string is kept verbatim ("Path must not be empty and must contain an even number of parts"), so existing regex assertions still match.

The local isPathValid is deliberately left unchanged because Reference.makeFromPath uses it as its guard — its leading/trailing/consecutive-slash normalization is tested contract (reference.spec.ts) that must keep working. On every path that passes the tightened filter 1, path.split('/').pop() is the real last segment, so filter 2 needs no change regardless of ordering.

Closes #138

Testing

• Added referenceChecks.regression.spec.ts pinning the previously-untested empty-segment branch of referenceChecks on both decode surfaces: the codec now rejects { id:'', path:'users/doc123/' }, { id:'doc123', path:'users/doc123/' } (with /even number of parts/, not the misleading id error), and { id:'def', path:'/users/abc/posts/def' }; the bare class decode also rejects { id:'', path:'users/doc123/' }.
• Verified the tests guard the bug by reverting filter 1 to the old isPathValid form: the four rejection tests fail exactly as described above (three accepted instead of throwing; the correct-id case threw the misleading Id must match… error), then pass again after restoring the fix.
• Confirmed makeFromPath still normalizes users/abc/, /users/abc/posts/def, and users//abc to canonical form (existing reference.spec.ts normalization tests, unchanged).
• Routine checks all pass: effect-firebase unit tests (370 across 16 spec files), package build and typecheck, ESLint and Prettier on both changed files, whole-workspace build (8 projects), downstream package tests (nx run-many -t test, 121 tests across admin/client/mock/devtools/examples), and the CI matrix nx affected -t lint typecheck test build.
• Manual spot-checks of codec and class decode across canonical, mismatched-id, odd-parity, and empty-segment inputs behaved as expected; the codec synthesizes the parent chain via makeFromPath, while the bare class decode constructs from the struct fields (no implicit parent).

---

Automatic Fixes PRs can be configured here.

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge; no outstanding findings remain.

Summary

The PR makes Reference JSON and class decoding reject non-canonical document paths using the shared path validator, while retaining makeFromPath normalization. The follow-up commit adopts the repository’s test import convention and adds consecutive-slash coverage.

_{Reviews (2) · Last reviewed commit: "test(firestore): use @effect/vitest and ..."}