The FIX 4 test asserted a full new 201 emission after the only document
under an idempotency key was soft-deleted. That over-reached: the surviving
FiscalIdempotencyKey (never soft-deleted, by design) still collides on the
UNIQUE(product_id, idempotency_key), and the camada-2 winner lookup — now
filtered by deleted_at IS NULL — returns None and re-raises. Re-emission
under a burned key is a future CANCEL feature (no soft-delete path exists in
F2), out of scope for this MINOR. The fix's real contract is narrow: a replay
must not RETURN a soft-deleted document. Test now proves exactly that on the
query (seeded on a separate session so it doesn't pollute the app's shared
db_session — the source of the MissingGreenlet the end-to-end POST hit).
Detection proven: filter removed -> FAILED; restored -> passed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
MINOR (F2 review): _get_document_by_idempotency_key lacked the
deleted_at IS NULL filter that get_fiscal_document already has. No F2
trigger today (nothing soft-deletes a FiscalDocument yet), but a future
cancel-by-soft-delete would replay a dead document as if it were still the
valid idempotent response, instead of treating the key as free for a new
emission.
Test: tests/emission/test_emissao.py::
test_idempotency_key_apontando_para_documento_soft_deletado_e_tratada_como_ausente
-- manually inserts a soft-deleted FiscalDocument + its FiscalIdempotencyKey
row (constructing the pre-cancel scenario directly, since nothing else in
F2 produces one yet), asserts the query-level replay returns None, and that
replaying via POST /v1/emissoes with that key emits a brand-new document
(201, new number allocated) rather than resolving to the dead one.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
IMPORTANT (F2 review): the service promises to validate structural
coherence (sums, required leiaute fields) per design spec decision #3, but
the one sum that can genuinely diverge -- pagamento.valor vs the document
total -- was unchecked. The lib documents SEFAZ rejects this with 'Valor do
Pagamento difere do total' (sowai_fiscal.xml_builder).
Adds PagamentoTotalMismatchError, computed in the completeness block
(before allocate_fiscal_number) by reusing sowai_fiscal.xml_builder.
soma_itens_quantizados -- the SAME function the lib's own _build_total uses
for vNF/vProd, avoiding a second source of truth for the same total.
Decimal-exact comparison (never float), both sides quantized to cents the
same way _build_pag does. Router maps it to 409 pagamento_total_diverge.
Test: tests/emission/test_emissao.py::
test_pagamento_divergente_do_total_dos_itens_e_409_e_nao_queima_numero --
pagamento.valor != Σ itens -> 409 pagamento_total_diverge, no fiscal number
consumed. The existing golden-backed happy-path tests (already balanced)
continue to prove the matching-totals case still succeeds.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
IMPORTANT (F2 review): certificate is per-tenant, matching series (already
tenant-scoped) and the GET/DELETE anti-oracle boundary. Emission's
_get_live_certificate and the upload-replace pre-check (certificates.
service, renamed _get_live_certificate_by_branch ->
_get_live_certificate_by_tenant_branch) both omitted tenant_ref -- two
tenants of one product reusing branch_ref="matriz" collapsed onto the same
slot: B's upload soft-deleted A's still-live certificate, and A's emission
went on to sign with B's certificate.
Migration 8f1a2c9d4b6e replaces the partial-unique index
ix_fiscal_certificates_product_branch_live with
ix_fiscal_certificates_product_tenant_branch_live on
(product_id, tenant_ref, branch_ref) WHERE deleted_at IS NULL, with a
working downgrade. Upload's two-layer defense (pre-check + IntegrityError ->
CertificateUploadConflictError) still holds against the new index.
Tests:
- tests/emission/test_emissao.py::
test_dois_tenants_do_mesmo_produto_reusando_branch_ref_tem_certificados_isolados
-- two tenants upload for the same product/branch_ref, both stay live;
emission for each signs with its OWN certificate (observable via FIX 1's
CNPJ check: without FIX 2, tenant A's emission would 409
emitente_certificate_cnpj_mismatch because the "live" cert would
actually be B's).
- tests/migrations/test_fiscal_documents_schema.py::
test_two_tenants_can_both_hold_a_live_certificate_for_the_same_branch_ref_on_real_migration
-- real alembic upgrade head, raw INSERTs proving both tenants' certs
land live.
- tests/migrations/test_fiscal_documents_schema.py::
test_two_live_certificates_for_same_product_tenant_branch_violate_unique_index_on_real_migration
(renamed from ..._product_branch_...) -- same (product, tenant, branch)
still rejects a second live certificate on the real migration.
- tests/certificates/test_certificates.py::
test_concurrent_uploads_for_same_product_branch_only_one_wins_the_other_gets_409
updated for the renamed/re-scoped precheck function (still same-tenant
race, still 1 winner + 1 CertificateUploadConflictError).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CRITICAL (F2 review, Opus+Fable): the service decoupled emitente
(free-form payload.emitente.cnpj) from the certificate (looked up by
branch_ref) with nothing re-checking the equality the auto had structurally
via Branch.cnpj. A caller passing emitente.cnpj=B with a branch whose
certificate CNPJ=A got a signed, persisted ASSINADO document with
chave/emit=B but signature=A, having burned a nNF.
Adds EmitenteCertificateCnpjMismatchError, checked in the completeness
block (before allocate_fiscal_number, so a mismatch never consumes a
número), normalizing both sides (digits only) before comparing. Router maps
it to 409 emitente_certificate_cnpj_mismatch; message never leaks the
certificate's CNPJ.
Test: tests/emission/test_emissao.py::
test_emitente_cnpj_divergente_do_certificado_e_409_e_nao_queima_numero --
uploads a cert for CNPJ A, POSTs emissão with the same branch_ref but
emitente.cnpj=B, asserts 409 + no fiscal number consumed (a follow-up
emission with the matching CNPJ gets the number that would have been
burned).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>