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): the module docstring claimed FiscalResult/TributoLinha
'are NOT re-mirrored here' but the file defines FiscalResultPayload/
TributoLinhaPayload right below it. Corrects the prose to describe reality:
they ARE re-mirrored, deliberately kept separate from the lib's own pydantic
classes (this module is the wire contract the service owns), converted via
emission.service._to_fiscal_result/_to_tributo.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Honors portability safeguard (c) -- the committed docs/openapi-v1.json was
enforced to stay in sync with app.openapi() (test_openapi_committed.py),
but the app declared almost no error responses: POST /v1/emissoes showed
only 200/422 (no 201), and no route declared 401/404/409/413 or the
structured 409 codes the spec names as contract. A Go reimplementation
reading only the committed OpenAPI as source of truth wouldn't learn them.
Adds responses={...} to every v1 router (emissao, certificados, series,
documentos GET/xml) covering the status codes each route actually returns
-- 201 as the default on POST /v1/emissoes (with 200 documented for the
Idempotency-Key replay case), 401 on every authenticated route, 404 where
the anti-oracle boundary applies, 409 naming the structured codes each
route raises, 413 on the certificate upload's size cap. Regenerated
docs/openapi-v1.json from app.openapi() (uv run python -c '...json.dump...'
per test_openapi_committed.py's own docstring) so the committed==generated
assertion stays green with FIX 1-4's new 409 codes included.
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>
The signed-golden payload builder forwarded only ver_proc from the golden,
letting nat_op/tp_emis/ind_* fall to EmissaoRequest defaults. caso_devolucao_
intra is the only case with a non-default nat_op ('Devolucao de venda'), so
the emitted XML carried <natOp>Venda</natOp> and diverged from the (correct)
golden byte-for-byte. The golden was right and the emission handles natOp
correctly; the harness was feeding incomplete input. Byte-for-byte assertion
unchanged, golden unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Plain asyncio.gather on one event loop could let the first upload commit
before the second's pre-check, making the second a legitimate replace (2
successes, 1 live row) -- correct behavior that fails the '1 conflict'
assertion. Same Event-synchronization pattern as the auto's HTTP race tests:
hold the first past its pre-check until the second also pre-checks, so both
read None before either writes and the partial-unique index decides
deterministically. Code under test untouched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Port certificate lifecycle (parse/encrypt/upload/deactivate) and
FiscalSeries CRUD from the auto, adapted to (product_id, tenant_ref,
branch_ref) tenancy. Closes the "guard retroativo" PATCH /v1/series
next_number regression check that Task 3 deferred to this task.
Routes: POST/GET/DELETE /v1/certificados, POST/GET/PATCH /v1/series.
Ports the three tables that change owner per the design spec (decision #2):
FiscalSeries (was tenants.FiscalDocumentSeries), FiscalCertificate,
FiscalDocument (both from fiscal.models) — organization_id/branch_id
replaced by product_id (FK products) + tenant_ref/branch_ref (opaque
strings) per the porte table. sale_id/service_order_id dropped (no Sale
concept here). document_model is now a plain String(2) + Python enum,
never a Postgres enum (Global Constraints: zero enum PG — the auto's own
version of this column was a real PG enum, a documented debt not repeated
here).
Constraints preserved: UNIQUE chave_acesso, partial-unique
cert-vivo-per-(product_id, branch_ref) (product-scoped in addition to the
auto's branch_id, since branch_ref is an opaque string two different
products could coincidentally share), UNIQUE (product_id, tenant_ref,
branch_ref, document_model, serie) on the series.
documents.service.allocate_fiscal_number ported verbatim (mechanism +
contract): SELECT ... FOR UPDATE + populate_existing=True, no-commit
contract (caller commits together with the FiscalDocument insert, Task 5).
The next_number regression guard is deliberately deferred to Task 4's
PATCH /v1/series endpoint (needs FiscalDocument, which now exists).
AST guard (tests/shared/test_for_update_populate_existing.py) ported and
adapted to scan src/fiscal_svc/, plus two new self-tests proving the
detection logic itself in both directions (flags a missing fix, does not
false-positive on a correctly fixed multi-line chain) — the ported guard
alone only proves "currently green", not "actually detects".
33 tests green via `make k8s-test` (real-migration round trips + unique
constraint violations, N=10 concurrency, identity-map staleness repro,
tenancy-scoping not-found across product/tenant_ref/branch_ref).
Product is the only tenant table this service owns (porte table:
organization_id -> product_id, everything below it -- tenant_ref/
branch_ref -- stays an opaque string owned by the consuming product, never
a row here). Bcrypt-hashed API keys (passlib, same CryptContext shape as
the auto's auth.service), generated once by scripts/create_product.py and
never persisted in the clear. require_product (X-Api-Key -> Product, 401
on missing/wrong/soft-deleted) is the porte adaptation of the auto's
require_permission — every /v1/* route will depend on it instead of a JWT
bearer token.
Also ports shared/base_model.py and shared/errors.py verbatim (Global
Constraints: soft delete mixins, structured 409 bodies).
products migration + real "alembic upgrade head" round-trip test
(tests/migrations/, new shared _helpers.py instead of the auto's ad hoc
cross-file _run_psql reuse). 13 tests green via `make k8s-test`.
FastAPI + SQLAlchemy async + Alembic scaffold, src-layout (mirrors the
sowai-fiscal lib's own convention), pyproject wired to sowai-fiscal@v0.1.0
via git+https (uv.lock pins the commit). Makefile mirrors auto/Makefile's
k8s-test workflow: syncs into /app/fiscal-svc in the SAME auto-tests pod,
against a dedicated fiscal_svc_test database on the shared Postgres
sidecar, serialized by the SAME lock file the auto uses on purpose so the
two repos' test runs never race in the pod. Dockerfile installs git (the
git+https dependency needs it at uv sync time) and splits the dependency
layer from the project's own editable install for build caching.
GET /v1/health -> {"status": "ok"}, verified green via `make k8s-test`.
