8 commits

Author	SHA1	Message	Date
Adam Moussa	004474fc1d	chore: tidy .claude/workflows — drop completed refactor phase scripts, keep remaining phase 6 (#120) ... * chore: cleanup completed refactor workflow files * chore: add remaining refactor phase 6 workflow	2026-07-20 17:02:06 -04:00
Adam Moussa	ff368beaa4	test: consolidate test roots — one loader, shared support, enforced CI floor (phase 8) (#118) ... * test: consolidate test roots — one repo-root loader, shared support package, missing-scenario suites, enforced ruff/coverage floor (refactor phase 8) tests/conftest.py only loads for the tests/ root, not a standalone `pytest lambdas/po/email_processor/tests` run, so it could never carry session invariants like the dummy AWS env or the moto stubber registration. Add a single repo-root conftest.py (pytest.ini pins rootdir there, so it loads for every invocation) that sets the dummy AWS credentials/region, imports moto BEFORE any handler module so boto3 sessions pick up its stubber hook (carrying the explanatory comment verbatim from the old _po_parser_support.py), and exposes one load_lambda_module(pipeline, name) — the sys.modules save/restore dance stays, since template_parser is still a duplicated bare name across pipelines needing per-exec sibling binding. Add tests/support/ as the shared package both pipelines' local _*_parser_support.py modules delegate to: a superset FakeTable (PO's update_item recording + WO's put_item and keyed single-row store), FakeDynamoResource, load_email, and load_golden with parse_float=Decimal kept (load-bearing for exact money comparison at PO magnitudes — WO's prior load_golden had no parse_float and must not regress PO by losing it). Rewrite _wo_parser_support.py off the bare `import handler` / `from handler import parse_raw_email` strategy that was the source of the bare-name sys.modules collision the other two loaders defend against. Move test_po_merge.py and test_pad_zip.py into lambdas/po/email_processor/tests/ (PO-specific, belongs beside the code) via git mv so history follows; test_parse_raw_email.py and test_ses_auth.py stay at the repo root since they're genuinely cross-pipeline, parameterized over both handlers. Delete tests/test_local.py: it globs a nonexistent samples/ dir, is WO-only, and imports a handler at collection time, bypassing the loader gate entirely — the golden suites already cover its role. Its pytest.ini exclusion comment goes with it. New scenario coverage, all built on the single loader + support package: - PO+WO Bedrock transport errors (ThrottlingException, missing 'content' key, empty content list, non-JSON model text), asserting PO's pre-call ai_fallback metric survives with no partial write and the exception propagates; WO's no-datapoint-on-throttle behavior is pinned with a documenting test rather than "fixed" by reordering. - Handler-level SES-auth reject seam per pipeline: no auth monkeypatch + empty ALLOWED_DKIM_DOMAINS asserts zero Bedrock calls, zero writes, no raise — closing the hole where deleting the gate line today still passes every test. - web_ui coverage for both PO and WO (0% before this): fail-closed on unset ARN and on a Secrets Manager exception, TTL cache refresh, Bearer/X-Auth-Token/header-case-insensitivity, wrong-token 401 with no table scan, non-ASCII token, and a hostile-field-escaping regression lock. PO web_ui has no __init__.py, so these go through the loader rather than package imports. - A moto-backed mirror of test_po_merge for WO merge semantics (table 'WorkOrders'): null-status never clobbers wo_status, created_at immutable via if_not_exists, status->wo_status mapping, None fields absent from SET, record_type only-when-present. - Small pins: the PO-DC-02 64-char EMF clamp regression and per-pipeline multi-record failure-isolation (all-or-retry contract). The reprocess.py synthetic-event-shape contract test already landed in Phase 7, so it isn't duplicated here. Two WO product-code fixes ride along, since this is the phase that exercises them: (a) the invalid_status reason-code fix in template_parser.py's status check, which previously returned malformed_site_code for the same failure validate_ai_fallback already labels invalid_status, making one failure surface two codes depending on path (grepped the dashboards/metric filters for malformed_site_code first — no external references found, safe to diverge the two codes); (b) wrapping the WO Bedrock call in handler.py so a transport failure emits ai_fallback/bedrock_error in an except-and-reraise. This is deliberately not a naive reorder: the emit sits in the except block, not pre-call, so a gate-rejected email still emits only ai_fallback_rejected and wo_stack's "a rejected email emits nothing else" alarm contract doesn't double-count. A test computes the emitted series by hand to pin the no-double-count behavior. Neither change touches the handler event/return contract. _validate_new_po_values in the PO template_parser.py is split into per-rule helpers, and the V4 anchor-frame dataclass now carries summary_matches/price so V13 can consume them; extract_new_po (C901=35) is included in the split. Add ruff.toml enabling C901/PLR so the mccabe/complexity suppressions scattered through the tree stop being decorative; derived_fields.py is under the shadow-bake freeze so its violations are silenced via a per-file ignore with a justification comment instead of an in-file edit, and the handful of other pre-existing violations surfaced by turning the config on get the same per-file-ignore treatment with a reason, or a fix where the file isn't frozen. scripts/ is added to the CI lint scope. CI gains an explicit --cov module list (lambdas/po and wo email_processor + web_ui, po/site_extractor, lambdas/shared) plus --cov-fail-under=80, since web_ui and site_extractor lack __init__.py markers and a bare --cov=lambdas silently skips them for the missing package marker; .coveragerc omits the test dirs themselves from the count. The Phase 0 AST bundle-consistency test stays in the standard pytest run. .gitignore picks up the resulting .coverage data file. docs/po-template-parser.md gets a small correction: the EXTRACTION_PROMPT declares quantity/price as "number or null", not JSON strings, so parse_float=Decimal already handles a conforming Bedrock response — the doc previously implied the coercion path was the primary mechanism rather than a defensive net for non-conforming responses. * test: lock attribute-context quote escaping in web_ui hostile-field test The escaping regression lock asserted only the element-context vector (raw <script> absent, &lt;script&gt; present) while its docstring claimed quotes were covered -- the payload's " and ' were never asserted on, so a quote-escaping regression on the onclick row-link sink (attribute breakout -> event-handler injection) would have passed green. /sh-security-review finding WC-01 (confirmed medium, test-integrity). Add assertions that the onclick sink's JSON string renders its opening quote as &quot; (raw " after window.location= fails), that the payload's quote characters appear only entity-escaped, and that the raw payload never appears anywhere in the body. Mutation-verified: the test now fails when the sink's quote-escaping is dropped. * test: address Open SWE review — xfail the web_ui non-ASCII auth pin, document subset coverage-floor override - tests/test_web_ui_auth.py: replace the TypeError characterization pin with an xfail(strict, raises=TypeError) asserting the DESIRED fail-closed (False) behavior. Documents the intended fix and auto-fails (xpass) once web_ui_auth is corrected, instead of requiring a passing test to be knowingly deleted. The module stays frozen this phase; the underlying hmac.compare_digest ASCII-only defect is tracked as a follow-up. - pytest.ini: document that the aggregate 80% floor (enforced in CI via the reusable workflow's bare pytest) red-exits local subset runs by design, with the --cov-fail-under=0 override for iteration. Floor stays in addopts because the centralized ci-python-sam workflow exposes no per-run test command.	2026-07-20 16:19:15 -04:00
Adam Moussa	ca4f43a2cc	feat: decompose email-processor handlers into flat siblings + lazy boto3 clients (refactor phase 5) (#113) ... Both email-processor God-handlers split along the seams that already work in the flat-sibling pattern established by lambdas/shared/, so bare-name imports keep working under the existing bundling glob. PO (5-way split): handler.py keeps only the event loop, fail-closed auth, and email_type routing. extraction.py holds extract_with_claude and _EMAIL_TAG_RE, importing EXTRACTION_PROMPT from prompts.py and parse_raw_email from shared/email_parsing.py rather than recreating a PO-local copy. enrichment.py is a pure code move of enrich_parsed and pad_zip (PO-only; WO has no enrichment stage) with zero behavior change. telemetry.py holds the EMF ParseMethod emit wrappers. persistence.py holds _write_fields/_merge_update/save_*, collapsing the byte-identical save_new_po/save_revision bodies into one _save_merge helper that both now call through, preserving the sticky Cancelled ConditionExpression guard for both callers; save_cancellation stays separate. WO (5 concerns, no enrichment stage): the handler loop keeps validate_ai_fallback and the re.fullmatch(r"[0-9]+", work_order_id) key guard ahead of both save_work_order and save_event, since the guard protects the DynamoDB partition key and the '#'-delimited comment_id range-key segment. _header_date_iso and comment_id determinism stay colocated with persistence.py's save_event for the retry-idempotent event_id key. EXTRACTION_PROMPT (PO) moves to prompts.py with cross-reference headers to derived_fields.py's authoritative trade/site/fiscal rule tables; handler.py re-exports it (from prompts import EXTRACTION_PROMPT) since four tests dereference handler.EXTRACTION_ PROMPT directly. WO's prompt moves the same way. I/O modules (extraction.py's bedrock client, persistence.py's dynamodb resource, handler.py's s3 client) get lazy cached boto3 accessors; pure modules (enrichment.py, prompts.py, telemetry.py) import no boto3. Test monkeypatch surfaces move to the module that now owns the client (e.g. persistence.dynamodb) everywhere tests patch it, and the moto-before-handler-import ordering in _po_parser_support.py is preserved so the moto-backed suites don't hit real AWS. Behavior-preservation pins, verified with tests: PO still emits ParseMethod=ai_fallback before the Bedrock call, with ai_fallback_rejected as the additive second datapoint on rejection. WO still emits after its gate with mutually-exclusive ai_fallback / ai_fallback_rejected. Shadow DerivedFieldAgreement telemetry stays ai_fallback-only. derived_fields.py is untouched (diff against feature/phase-3-shared-extraction is empty). handler(event, context) signatures and the save_* public contract are unchanged on both pipelines; goldens unchanged. PO_EXPECTED_TOP_LEVEL_MODULES and its WO equivalent in tests/test_bundle_consistency.py are updated for the new sibling modules so the AST bundle-consistency test still fails on an unshipped or uncommented-out sibling.	2026-07-20 15:34:53 -04:00
Adam Moussa	f8eb18f02b	feat: collapse duplicated CDK into cdk/common.py plain helpers (refactor phase 4) (#112) ... The ~379 lines po_stack.py and wo_stack.py defined identically (DynamoDB alarms, the sender-auth-rejected metric filter + alarm, the standard per-Lambda alarm set, the Bedrock InvokeModel grant, the raw-email bucket, the async DLQ, the template-fallback-rate math alarm) move into cdk/common.py. Every helper is a PLAIN function taking (scope, id, ...), called with each stack's own Stack as scope and the exact literal construct ids used inline before, so every synthesized logical ID is byte-stable. A Construct subclass would reparent the tree and make CloudFormation attempt to replace the RETAIN-protected purchase-orders/WorkOrders tables and named buckets -- data loss -- so it is forbidden. Per-function alarm variance (PO p99 vs WO p95 duration, po-web-ui throttles+duration only, site-extractor no DLQ alarm, workorder-web-ui zero alarms) is preserved through call-site arguments, not baked into the helpers. make_bedrock_invoke_statement derives the inference-profile and us-east-1 foundation-model ARNs from Stack.of(scope).account/.region instead of the hardcoded 328440206208/us-east-1 literals. The environment stays account-agnostic (region-only), so the account resolves to the AWS::AccountId pseudo-parameter: the derived ARN resolves at deploy to the same ARN the literal named in-account (a benign in-place IAM policy update, never a replacement) and is account-portable rather than pinned to the frozen management account. The account= pin evaluated for cdk.Environment was deliberately NOT added: resolving every account-derived value (bucket names, Lambda::Permission source account, SNS action ARN) to literals makes CloudFormation flag the RETAIN email buckets as requiring replacement against the deployed account-agnostic templates -- a data-loss risk that outranks the pin, which buys nothing (the resolved values are unchanged). Also: net-new CfnOutputs for the five Lambda function ARNs and the owned/consumed table names, exact-pin constructs==10.6.0, and fix the stale aws-cdk-lib 2.259.0 -> 2.261.0 version comment. The common.py extraction is zero-cdk-diff on both stacks (byte-stable logical IDs, no asset/property change); the only deltas versus deployed are the intended benign Bedrock IAM in-place update and the additive CfnOutputs. Mandatory GPT-4.1 cross-family review ran on the Bedrock IAM move; its BLOCK was a verified false positive (it read AWS::AccountId as a wildcard -- it is a deploy-time-resolved concrete value naming one account and one inference-profile, region is pinned us-east-1, and the grant is strictly more least-privilege-correct than the hardcoded literal).	2026-07-20 18:14:57 +00:00
Adam Moussa	30112cc680	feat: extract lambdas/shared/ — single-source ses_auth, web_ui auth, email parsing, EMF emitter (refactor phase 3) (#111) ... Four modules move into the handbook-mandated lambdas/shared/ location, collapsing duplicated logic that had to be kept in sync by hand across the PO and WO pipelines: - ses_auth.py: the PO and WO copies were verified sha256-identical against the feature/phase-7-ops-recovery baseline before the move (no drift since the last audit). shared/ses_auth.py is the exact bytes of that one copy; both originals are git rm'd (the PO copy via rename, the WO copy as a straight delete). Bundling lands the module flat in /asset-output for both email processors, so the handlers keep `from ses_auth import authenticate_inbound_email` unchanged — zero handler diff for this move, which is what keeps fail-closed auth byte-identical through the change. - web_ui_auth.py: extracts the byte-identical _get_auth_token / _header / is_authenticated block plus the four token-cache globals out of both web_ui handlers. The per-stack INFRA-74 comments stay in each handler as-is (deliberately drifted wording, stack-specific) rather than being unified into the shared module. Fail-closed semantics (unset ARN or Secrets Manager exception -> deny) are unchanged. - email_parsing.py: parse_raw_email ships as the superset version that returns cc unconditionally. WO's output is bit-identical to before; PO simply ignores the cc field rather than being "cleaned up" to consume it. No second variant is kept. - emf.py: a generic emitter parameterized by namespace, dimension sets, and properties. Every call site's emitted EMF envelope is unchanged, including the load-bearing [["ParseMethod"],["ParseMethod","TemplateId"]] dimension-set shape the alarms and metric filters depend on. Emission ordering is untouched: PO still emits ai_fallback before the Bedrock call, WO still emits its mutually-exclusive ai_fallback/ai_fallback_rejected after its gate. The deliberate-double-count comments survive. _emit_derived_agreement_metric was found living inside derived_fields.py, so per the DERIVED-FIELDS exception it is left as a third, unconverted copy (derived_fields.py and the shadow DerivedFieldAgreement telemetry stay untouchable while that bake runs) — a comment there points at shared/emf.py for the eventual follow-up. Bundling: both email-processor cdk bundling commands gain a trailing `cp shared/*.py /asset-output/` (they were already cp-only post-Phase 7, so no pip step or manylinux pin is reintroduced). Both web_ui functions gain the same widened-root staging so web_ui_auth.py ships beside their handler; site_extractor's from_asset is untouched. Tests: PO_EXPECTED_TOP_LEVEL_MODULES gains the shared modules that now ship, the AST sibling-import check resolves imports whose source now lives under shared/, and the new shared cp line has its own revert/mutation detection. _SIBLING_MODULES resolution and _po_parser_support.py now load ses_auth/email_parsing/emf from shared/; the two-copy ses_auth byte-identity fixture-hygiene test is retired as obsolete now that there is one copy, and the ses_auth fixture parameterization over two identical copies is dropped. The sys.modules save/restore dance for template_parser (still duplicated per-pipeline) is left in place.	2026-07-20 13:38:23 -04:00
Adam Moussa	75fe91c198	Ops/recovery tooling + dependency hygiene (refactor phase 7) (#110) ... * feat: ops/recovery tooling + dependency hygiene (refactor phase 7) Generalize scripts/reprocess.py from a PO-only full-sweep script into a pipeline-general recovery tool. Targeted replay (--key/--prefix/--since) is now the default, and the full inbound/ sweep is demoted behind an explicit --all that documents its five hazards (async concurrency does not serialize, use RequestResponse if order matters, metric double-count, Bedrock re-bill, out-of-order field regression). --pipeline po|wo resolves the correct function + bucket; dry-run-by-default / --execute is preserved. A new tests/test_reprocess_contract.py pins the synthetic S3 event shape and asserts the raw list_objects_v2 key is emitted untransformed (the handler is the single decode point; a pre-decoded key would corrupt keys containing spaces or '+'). Add docs/runbook-dlq-recovery.md: the async on-failure DLQ has no console redrive-to-source, so it documents the receive -> extract key -> targeted reprocess --key -> verify -> purge procedure, the real recovery windows (14-day DLQ breadcrumb, 90-day raw-email S3 that overrides the table RETAIN policy and is the true replay floor), and that sender-auth and ai_fallback_rejected drops are fail-closed skips that never reach the DLQ. Linked from the README alarms and scripts sections. Drop the vendored boto3 floor pin from both email-processor requirements (the Lambda runtime provides boto3; lambda-template.md empty-with-comment form). With nothing left to install, the email-processor bundling becomes cp-only -- the whole pip step is removed, which is the only acceptable way the manylinux2014_aarch64 pin disappears (removing the pin while keeping a pip install caused the PR #34 x86-wheel outage). Exact-pin moto==5.2.2 and add pinned po/web_ui + po/site_extractor manifests (excluded from their bundles, so hash-neutral) so their new Dependabot entries have something to act on; add Dependabot entries for /tests, /lambdas/po/web_ui, and /lambdas/po/site_extractor. cdk diff is confined to exactly the two email processors' asset hashes on both stacks. The wo/web_ui dead-manifest reduction was deliberately left out: that manifest already ships inside the plain (non-bundled) WebUI asset on main, so reducing or excluding it would redeploy workorder-web-ui for no functional change -- deferred to keep the blast radius to the two intended targets. The untracked 44 MB lambdas/po/email_processor/package/ dir was removed from the filesystem (asset-hash-neutral given Phase 2's package/ exclude); it is untracked, so there is nothing to commit for it. * Reject --all combined with --prefix/--since in reprocess.py --all is a distinct mode (the demoted full-prefix sweep), but the args.all branch unconditionally set prefix=inbound/ and since=None, so passing it alongside a narrower selector silently discarded that selector. `--all --since 2026-07-01` swept the entire corpus instead of the bounded window, triggering every documented --all hazard (Bedrock re-bill, metric double- count, merged-field regression) on objects the operator never targeted -- contradicting the tool's safety goal. Add the missing mutual-exclusion guard alongside the existing --key one, and pin --all+--prefix, --all+--since, and all three together as argparse rejections.	2026-07-20 12:53:34 -04:00
Adam Moussa	df03f3497f	feat: widen email-processor asset roots to lambdas/ with scoped globs + excludes (refactor phase 2) (#109) ... Both email-processor Code.from_asset calls now bundle from lambdas/ instead of their per-function subdirectory, so Phase 3's shared/ module is reachable from the asset root once it lands. The bundling commands were rewritten for the new cwd (pip install -r <po|wo>/ email_processor/requirements.txt -t /asset-output && cp <po|wo>/ email_processor/*.py /asset-output/), preserving the ARM64 --platform manylinux2014_aarch64 --only-binary=:all: pin exactly — its removal shipped x86 wheels into the ARM64 function and caused a 100% outage (PR #34). All five from_asset calls (both email processors, po web_ui, po site_extractor, wo web_ui) now exclude **/__pycache__/**; the two widened ones also exclude **/tests/** and **/package/**. Without the package/ exclude, the stale untracked 44 MB lambdas/po/email_processor/package/ dir (local-only, never present in CI) would diverge local vs CI asset hashes and force spurious redeploys — from_asset doesn't honor .gitignore. That dir is left in place; deleting it is Adam's call. WO's prod zip shrinks as deliberate cleanup, not a byte-identical match to PO: the old `cp -r .` shipped tests/ (real scrubbed .eml fixtures), __pycache__/, and requirements.txt into production. The acceptance bar for WO is runtime-imported module set unchanged + smoke, not a byte-identical zip; PO keeps the byte-identical first-party file set guarantee. tests/test_bundle_consistency.py is updated in the same change to recognize the scoped `cp po/email_processor/*.py` (resp. wo) glob as the new unconditionally-safe shape, without loosening the allowlist-revert detection, the detection-logic mutation test, or the PO_EXPECTED_TOP_LEVEL_MODULES exact-set pin. No code moved under lambdas/ in this change (git diff main...HEAD -- lambdas/ is empty); only CDK asset wiring and its tests changed.	2026-07-17 15:47:01 -04:00
Adam Moussa	cb5539bd68	feat: deploy-pipeline guards — healthcheck, smoke gate, bundle glob + AST test (refactor phase 0) (#107) ... * feat: deploy-pipeline guards — healthcheck, smoke gate, bundle glob + AST test (refactor phase 0) Deploys of po-email-processor and workorder-email-processor had no verification step, so an init-time ImportError in the bundled zip could ship silently and only surface on the next real S3 event. This adds a synchronous post-deploy smoke gate wired into the deploy workflow: both Lambdas are invoked with {"healthcheck": true} and the FunctionError field is checked, since an Unhandled init error still returns HTTP 200 on RequestResponse invokes and would false-pass a plain exit-code check. The healthcheck branch is the first statement in each handler, before any boto3/S3 use or ses_auth, and only fires on a top-level direct invoke ("healthcheck" is not a key AWS ever sets on a real S3 ObjectCreated event, so mail content can't reach this path). It emits no EMF metrics and no log text that could match the sender-auth-rejected metric filter, so two deploys in one window won't trip the alarm. Separately, the PO stack's asset bundling copied a hand-maintained four-file allowlist into the zip, so every new sibling module handler.py imports had to be added by hand or the deploy shipped a Lambda that ImportErrors at cold start (bit us for template_parser in PR #105 and nearly for derived_fields in PR #2). Replaced it with a non-recursive ./*.py glob so top-level source files ship automatically while tests/ and the stale package/ dir still cannot, and added an AST-based bundle-consistency test that parses each handler's first-party imports and fails CI if the bundling command would omit any of them (a revert to an incomplete allowlist, or code moved into a subdirectory the glob doesn't cover). Includes the refactor-evaluation report that scoped this phase. * fix: review nits — unambiguous bundling-command extraction, smoke payload-parse message, dead asserts - tests/test_bundle_consistency.py: _extract_bundling_command now collects all command=[...] matches and demands exactly one per stack file, instead of silently returning whichever ast.walk visits first if a second bundled function is ever added. - scripts/post-deploy-smoke.sh: distinguish an unparseable response payload from a payload mismatch so the failure message says what actually happened (the previous "could not parse" branch was unreachable — the inline python always exited 0). - test_po_healthcheck.py: drop the substring assertions on stdout that were dead behind the stricter `captured.out == ""` assertion; keep the stderr filter-pattern check. Review follow-up on PR #107; no behavior change to any shipped code path.	2026-07-17 13:18:45 -04:00