fix(api): include canonical query parameters in POST idempotency fingerprint

[Ganador1]

Closes #187

Why

As reported in #187, Facility scopes authenticated POST idempotency by principal, method, query-stripped path, key hash, and request body hash.

However, some POST endpoints use query parameters to alter operation semantics (for example, POST /v1/projects/:id/kb/entries?dry=1 performs validation-only without persistence, while POST /v1/projects/:id/kb/entries performs live persistence).

Because requestHash only hashed request.body, a dry-run validation request followed by a live persistent request reusing the same Idempotency-Key was falsely treated as a replay. The API returned a successful 200 response with idempotency-status: replayed, but the intended write never occurred.

What it does

1. Includes canonical query parameters in the request fingerprint: Computes requestHash as sha256(${stableQuery}|${stableBody}), where stableQuery is a deterministic, key-sorted JSON serialization of request.query.
2. Detects behavior-changing query parameter differences: Reusing an Idempotency-Key with different query parameters is detected as a mismatch and rejected with 409 idempotency_key_reused, matching the existing body-conflict behavior.
3. Preserves parameter-order tolerance: Permuted query parameters (e.g. ?a=1&b=2 vs ?b=2&a=1) produce identical canonical representations and replay correctly without false conflicts.
4. Fails closed for unversioned legacy fingerprints: Replays against legacy records (sha256(stableJson(body))) fail closed with 409 idempotency_key_reused because legacy records are inherently ambiguous and cannot prove whether the original request contained query parameters (such as ?dry=1).
5. Updates API documentation: Clarifies in apps/docs/docs/reference/api.md that canonical query parameters participate in idempotency equivalence.
6. Adds comprehensive test coverage:
   • services/api/test/idempotency.unit.test.ts: Unit test suite validating initial claims, permuted query replays, key reuse rejections with different query parameters, and fail-closed rejections on legacy dry-run vs live writes.
   • services/api/test/api.test.ts: Integration tests covering query permutation replays, query difference rejections, and the exact KB dry vs persistent create reproduction from POST idempotency ignores query parameters that change request behavior #187.

Verification

Commands run:

pnpm lint                                                               # Biome clean across 423 files
pnpm typecheck                                                          # 16 packages passed, 0 errors
node guards/run.mjs                                                     # 2 guards ran, 0 failed
pnpm --filter @facility/api exec vitest run test/idempotency.unit.test.ts # 5/5 passed (4ms)
pnpm test:uncached                                                      # 10 test suites passed

[jungle-lethanh]

Thanks for linking #187 and covering the KB dry/live reproduction. I had deliberately held off on implementation while waiting for maintainer confirmation of the intended query-equivalence contract.
One rollout case may be worth checking: existing idempotency records were fingerprinted as sha256(stableJson(body)), while this change fingerprints stableQuery|stableBody. A retry using a record created before deployment could therefore return 409 idempotency_key_reused for up to 24 hours even when the request is otherwise unchanged.
Is that compatibility behavior intentional? If so, it may be worth documenting. Otherwise, a regression test or transitional fingerprint strategy may be needed.
I’m also happy to validate the final behavior against the original runtime reproduction.

[adrian-lorenzo]

Thanks for the contribution!

The legacy fallback still allows the original bug during a rolling deployment. A legacy record stores only the body hash, so it contains no information about the query that created it. existing.requestHash === legacyRequestHash therefore accepts any query with the same body. I reproduced this by retrying a legacy record with the same body plus ?dry=1; it was replayed instead of returning 409.

Please don’t treat a body-only legacy hash as proof that the query matches. Legacy records are inherently ambiguous, so the transition should fail safely or store explicit fingerprint version/query information. Please also add a regression test for a legacy record receiving the same body with a changed query.

With that fixed, we can approve and merge it!

[Ganador1]

Thanks for the thorough review and catch, @adrian-lorenzo!

I've updated the implementation in commit 9e1c536 to fail safely:

1. Strict Queryless Legacy Replays: A body-only legacy hash match is now strictly restricted to incoming requests without query parameters (stableQuery === "{}").
2. Fail-Safe on Changed Queries: If an incoming request introduces or modifies query parameters (such as ?dry=1), it does not match the legacy body-only hash and fails safely with 409 idempotency_key_reused.
3. Regression Test Added: Added a test case in services/api/test/idempotency.unit.test.ts verifying that attempting to retry an existing legacy record with the same body but with ?dry=1 is rejected with 409 idempotency_key_reused.

[adrian-lorenzo]

Thanks for following up on the review!

The legacy ambiguity remains in the opposite direction. A legacy record may have been created by a request containing ?dry=1; retrying that key and body without a query makes stableQuery === "{}", so isLegacyQuerylessMatch accepts the record and replays the dry response instead of performing the live write. That is the original failure mode.

Please fail closed for every unversioned legacy fingerprint or add explicit fingerprint-version/query information. Add a regression that seeds a legacy dry-run response, sends the queryless live request with the same key and body, and verifies that it is rejected rather than replayed.

Once that case is covered, we can approve it.

[Ganador1]

Thanks for clarifying the reverse ambiguity case, @adrian-lorenzo! That makes total sense: because legacy records stored only sha256(body), a record originally created with ?dry=1 would have replayed its dry response for a subsequent live queryless write, masking the write.

I've updated the implementation in commit fdf259b to fail closed for all unversioned legacy fingerprints:

1. Fail-Closed on Unversioned Legacy Fingerprints: Any record that does not match the canonical query+body requestHash is rejected with 409 idempotency_key_reused. We no longer attempt fallback matching against ambiguous body-only legacy hashes.
2. Regression Test for Legacy Dry-Run vs Live Write: Added a test in services/api/test/idempotency.unit.test.ts that seeds a legacy dry-run response ({ ok: true, validationOnly: true } fingerprinted with the body-only hash), sends a live queryless request with the same key and body, and asserts that it is rejected with 409 idempotency_key_reused rather than replayed.
3. Full Test Suite & Linter Passing: All unit and monorepo suites are passing cleanly.