From 822cd4f3120a54cef930551a5af8bb24cb7f1b91 Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:11:21 -0800 Subject: [PATCH 1/2] fix(edi): stop rejected EDI documents returning 500 in the prod image _record_edi_rejection looked up the rejection log with an absolute "app.edi_ingestion.rejection_log". In the consolidated server the top-level app package is another service, so every EDI rejection raised ModuleNotFoundError: the caller got a 500 instead of the 4xx with a rejection_id, and the audit record was lost. Resolve the module against this package. The lookup stays lazy, as the tests rely on it. Remove both PENDING_ELSEWHERE entries so the absolute-import guard has no exceptions: #3793 landed the webhook_compat fix, and the branch named for this one was closed in #3778. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> --- .../ingestion/app/edi_ingestion/routes.py | 6 ++- .../tests/test_no_absolute_app_imports.py | 8 ---- ...test_prod_import_path_tenant_validation.py | 43 +++++++++++++++++++ 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/services/ingestion/app/edi_ingestion/routes.py b/services/ingestion/app/edi_ingestion/routes.py index 8d36158ff..1a8c62de4 100644 --- a/services/ingestion/app/edi_ingestion/routes.py +++ b/services/ingestion/app/edi_ingestion/routes.py @@ -80,9 +80,11 @@ def _record_edi_rejection(**kwargs): Tests swap ``app.edi_ingestion`` modules in and out of ``sys.modules``. Looking up the recorder lazily keeps the route and the test helpers - pointed at the same in-memory rejection store. + pointed at the same in-memory rejection store. Resolve it against this + package, not ``app``: in the consolidated server the top-level ``app`` is + another service, so an absolute name turns every rejection into a 500. """ - rejection_log = importlib.import_module("app.edi_ingestion.rejection_log") + rejection_log = importlib.import_module(".rejection_log", __package__) return rejection_log.record_edi_rejection(**kwargs) diff --git a/services/ingestion/tests/test_no_absolute_app_imports.py b/services/ingestion/tests/test_no_absolute_app_imports.py index 0ff1f976a..987cfc8a3 100644 --- a/services/ingestion/tests/test_no_absolute_app_imports.py +++ b/services/ingestion/tests/test_no_absolute_app_imports.py @@ -16,13 +16,6 @@ INGESTION_APP = Path(__file__).resolve().parent.parent / "app" -# Absolute imports that open PRs already remove. Delete an entry once its fix -# lands; never add one. -PENDING_ELSEWHERE = { - ("webhook_compat.py", "app.tenant_validation"), # PR #3774 - ("edi_ingestion/routes.py", "app.edi_ingestion.rejection_log"), # fix/csv-edi-ingest-monolith-imports -} - def _absolute_app_imports(path: Path) -> list[tuple[int, str]]: found: list[tuple[int, str]] = [] @@ -50,6 +43,5 @@ def test_ingestion_app_has_no_new_absolute_app_imports() -> None: f"{path.relative_to(INGESTION_APP).as_posix()}:{lineno} imports {module}" for path in sorted(INGESTION_APP.rglob("*.py")) for lineno, module in _absolute_app_imports(path) - if (path.relative_to(INGESTION_APP).as_posix(), module) not in PENDING_ELSEWHERE ] assert offenders == [], "use a relative import (from .x / from ..x):\n" + "\n".join(offenders) diff --git a/services/ingestion/tests/test_prod_import_path_tenant_validation.py b/services/ingestion/tests/test_prod_import_path_tenant_validation.py index 3d66f2105..d61d9013d 100644 --- a/services/ingestion/tests/test_prod_import_path_tenant_validation.py +++ b/services/ingestion/tests/test_prod_import_path_tenant_validation.py @@ -423,3 +423,46 @@ def get(self, key): # exercised under the poisoned top-level ``app`` for CSV and EDI. assert "csv_upload" in seen, (csv_resp.status_code, csv_resp.text[:500], seen) assert "edi_856_inbound" in seen, (edi_resp.status_code, edi_resp.text[:500], seen) + + +def test_edi_rejection_is_recorded_under_prod_path(prod_path_webhook_compat): + """A rejected EDI document returns its 4xx and leaves an audit record. + + ``_record_edi_rejection`` resolves the rejection log lazily. With an + absolute ``app.edi_ingestion`` name that lookup raised under the prod + layout, so each rejection became a 500 and lost its audit record. + """ + from fastapi import FastAPI # noqa: PLC0415 + from fastapi.testclient import TestClient # noqa: PLC0415 + from services.ingestion.app.authz import ( # noqa: PLC0415 + IngestionPrincipal, + get_ingestion_principal, + ) + from services.ingestion.app.edi_ingestion import router as edi_router # noqa: PLC0415 + import services.ingestion.app.edi_ingestion.rejection_log as rejection_log # noqa: PLC0415 + + rejection_log.reset_edi_rejections() + app = FastAPI() + app.include_router(edi_router) + app.dependency_overrides[get_ingestion_principal] = lambda: IngestionPrincipal( + key_id="test-key", scopes=["*"], tenant_id=TENANT, auth_mode="test" + ) + + future_dated = _EDI_FIXTURE.read_text(encoding="utf-8").replace( + "BSN*00*ASN-12345*20260310*1200~", "BSN*00*ASN-12345*20991231*1200~" + ) + assert "20991231" in future_dated + + with TestClient(app, raise_server_exceptions=False) as client: + for route in ("", "/document"): + response = client.post( + f"/api/v1/ingest/edi{route}", + data={"traceability_lot_code": "00012345678901-LOT001", "tenant_id": TENANT}, + files={"file": ("asn_856.edi", future_dated.encode(), "application/edi-x12")}, + ) + assert response.status_code == 400, response.text + assert response.json()["detail"]["error"] == "edi_date_in_future" + + records = rejection_log.list_edi_rejections(TENANT) + assert len(records) == 2 + assert {record["reason"] for record in records} == {"edi_date_in_future"} From 0aebf050703eb498b75b9e8f63e96167e9468012 Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Wed, 4 Nov 2026 22:26:53 -0800 Subject: [PATCH 2/2] test(edi): clear partner settings in the prod-path rejection test With EDI_REQUIRE_PARTNER_ID set in the environment, the request failed on the missing partner header before reaching the date check. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> --- .../tests/test_prod_import_path_tenant_validation.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/ingestion/tests/test_prod_import_path_tenant_validation.py b/services/ingestion/tests/test_prod_import_path_tenant_validation.py index d61d9013d..2af46efc5 100644 --- a/services/ingestion/tests/test_prod_import_path_tenant_validation.py +++ b/services/ingestion/tests/test_prod_import_path_tenant_validation.py @@ -425,7 +425,7 @@ def get(self, key): assert "edi_856_inbound" in seen, (edi_resp.status_code, edi_resp.text[:500], seen) -def test_edi_rejection_is_recorded_under_prod_path(prod_path_webhook_compat): +def test_edi_rejection_is_recorded_under_prod_path(prod_path_webhook_compat, monkeypatch: pytest.MonkeyPatch): """A rejected EDI document returns its 4xx and leaves an audit record. ``_record_edi_rejection`` resolves the rejection log lazily. With an @@ -441,6 +441,8 @@ def test_edi_rejection_is_recorded_under_prod_path(prod_path_webhook_compat): from services.ingestion.app.edi_ingestion import router as edi_router # noqa: PLC0415 import services.ingestion.app.edi_ingestion.rejection_log as rejection_log # noqa: PLC0415 + monkeypatch.delenv("EDI_REQUIRE_PARTNER_ID", raising=False) + monkeypatch.delenv("EDI_PARTNER_ALLOWLIST", raising=False) rejection_log.reset_edi_rejections() app = FastAPI() app.include_router(edi_router)