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
|
|
|
"""Synthetic-event-shape contract test for scripts/reprocess.py (refactor §4.7).
|
|
|
|
|
|
|
|
|
|
Pins the exact S3 event shape reprocess emits and asserts the object key is
|
|
|
|
|
the RAW list_objects_v2 key — reprocess applies NO URL-encoding or decoding.
|
|
|
|
|
|
|
|
|
|
Why this matters: a real S3 event notification URL-encodes the object key, and
|
|
|
|
|
the handler is where any decode would live. reprocess builds its synthetic
|
|
|
|
|
event from the RAW list_objects_v2 obj["Key"] (or the raw --key arg), so it
|
|
|
|
|
must emit that key byte-for-byte. Replaying a pre-decoded (or pre-encoded) key
|
|
|
|
|
would not match what s3.get_object(Bucket, Key) expects downstream.
|
|
|
|
|
|
|
|
|
|
Recon note (honored narrowly): neither the PO nor the WO handler imports
|
|
|
|
|
urllib.parse or calls unquote/unquote_plus today — the raw event key goes
|
|
|
|
|
straight into s3.get_object. So the "handler decodes -> replaying a decoded
|
|
|
|
|
key double-decodes" failure mode is NOT reproducible against today's handlers
|
|
|
|
|
(they never decode once). This test therefore guards ONLY reprocess's own
|
|
|
|
|
contract: emit the raw list_objects_v2 key with no transformation. That keeps
|
|
|
|
|
replay single-decode-correct if a future handler ever adds unquote_plus.
|
|
|
|
|
"""
|
|
|
|
|
|
|
|
|
|
import importlib.util
|
|
|
|
|
import os
|
|
|
|
|
import pathlib
|
|
|
|
|
import sys
|
|
|
|
|
|
|
|
|
|
import pytest
|
|
|
|
|
|
|
|
|
|
# reprocess.py builds its boto3 clients inside main()/functions, so importing
|
|
|
|
|
# it is side-effect-free (no AWS calls, no clients at module scope). Set dummy
|
|
|
|
|
# region/creds defensively anyway so import stays offline-safe even if that
|
|
|
|
|
# ever changes. build_s3_event itself makes no AWS calls.
|
|
|
|
|
os.environ.setdefault("AWS_DEFAULT_REGION", "us-east-1")
|
|
|
|
|
os.environ.setdefault("AWS_ACCESS_KEY_ID", "testing")
|
|
|
|
|
os.environ.setdefault("AWS_SECRET_ACCESS_KEY", "testing")
|
|
|
|
|
os.environ.setdefault("AWS_SESSION_TOKEN", "testing")
|
|
|
|
|
|
|
|
|
|
# scripts/ is not a package (no __init__.py) and is not on sys.path, and
|
|
|
|
|
# tests/conftest.py only wires up per-handler dirs — so load reprocess.py by
|
|
|
|
|
# file path with importlib rather than `import scripts.reprocess`.
|
|
|
|
|
_p = pathlib.Path(__file__).resolve().parents[1] / "scripts" / "reprocess.py"
|
|
|
|
|
_spec = importlib.util.spec_from_file_location("reprocess", _p)
|
|
|
|
|
reprocess = importlib.util.module_from_spec(_spec)
|
|
|
|
|
_spec.loader.exec_module(reprocess)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_build_s3_event_shape_and_raw_key():
|
Migrate to seahaven-prod: deploy role, backfill tooling, account-portability fixes (#125)
* feat(migration): prepare stacks and tooling for the seahaven-prod account move
Phase 1 of the mgmt (328440206208) -> seahaven-prod (011934824531)
migration. No behavior change in-account; everything here is additive or
account-portability hygiene:
- infra/deploy-role/: reviewed OIDC deploy-role artifacts for prod
(trust main-only, cdk-hnb659fds-* AssumeRole, smoke-invoke-lambda scoped
to exactly the two email-processor fn ARNs). Codifies the previously
out-of-band smoke-invoke grant.
- Table resource policies: make_slack_bot_read_policy in cdk/common.py,
applied to purchase-orders, verified-sites, WorkOrders,
WorkOrderComments (NOT pending-site-review; no bot consumer). Grants the
mgmt-resident seahaven-slack-bot roles read-only cross-account access
post-move (bot-side identity grants land in the slack-bot repo).
- scripts/migrate_tables.py: dry-run-default backfill tool implementing
the plan's per-table semantics (superset overwrite, ingested_at cutoff
for WorkOrderComments, backup-gated truncate-and-load for the two
site tables) plus a verify subcommand (count parity, spot checks,
sticky-Cancelled drift check).
- tests/test_resource_policy_helper.py: statement-shape unit tests +
static pins that exactly the four bot-read tables carry the policy.
- Account-literal fixes: account-agnostic fixture bucket in
test_reprocess_contract; runbook/README/po-template-parser account
references updated to prod with historical mgmt notes; README gains the
account-prerequisites list (imported-by-name dependencies).
deploy.yaml is deliberately unchanged (push-to-main auto-deploy kept).
Merge is held until migration Phase 0 completes; flipping the
AWS_DEPLOY_ROLE_ARN repo secret and merging this PR IS the first prod
deploy.
* fix(migration): verify backup AVAILABLE pre-truncate; document wildcard risk acceptance (cross-review FIX/NIT)
* refactor(migration): drop cross-account read grants (slack-bot decommissioned); harden backfill + deploy role
seahaven-slack-bot was decommissioned 2026-07-23 (stack DELETE_IN_PROGRESS,
consumer Lambdas gone); its successor sh-mcp is undeployed and uses
same-account DynamoDB access. So no live consumer reads these tables
cross-account. Per Adam's call, drop the cross-account grants entirely and
re-add correctly-scoped ones if/when sh-mcp deploys to a different account.
- Remove the four table resource policies + make_slack_bot_read_policy helper
+ its constants (cdk/common.py, po_stack.py, wo_stack.py) and the helper's
unit test. Both stacks synth with zero table ResourcePolicy.
- scripts/migrate_tables.py hardening (fixes from the sh-security-review
fan-out on the destructive backfill tool):
* validate --cutoff strictly (parse ISO-8601, require aware UTC, re-emit
canonical second-precision form) so a malformed cutoff can't silently
copy dual-window rows or drop history;
* reject `copy --all` up front (must run tables individually, in order,
with the stream-drain wait) instead of writing three tables then erroring;
* truncate backup gate now also checks recency (<1h) and TableId, not just
status+name;
* verify requires --cutoff whenever a cutoff table is in scope (else it
false-flags dual-window rows as MISSING);
* sticky-cancel is now PREVENTED copy-side (a non-Cancelled source item
never overwrites a dest-Cancelled PO), and the verify comment no longer
overstates what its source-side scan covers;
* spot-check all modes (truncate_load keys are verbatim, so key-existence
is sound there too).
- Deploy role: scope cloudformation:DescribeStacks to this repo's stacks +
CDKToolkit (was Resource:*, disclosed all tenant stacks in the shared prod
account); add a drift check warning on unexpected role policies and drop the
dead SMOKE_POLICY_NAME var; document the shared-account bootstrap-role
accepted risk in the deploy-role README.
* docs(deploy-role): fold in cross-review NITs (DescribeStacks maintenance note, warn-only drift rationale)
* ci: update workflow to use new workflow tag (ruff versioning fix)
* fix(migration): address Open SWE review findings on migrate_tables.py
- Validate the truncate backup on dry-run as well as --execute so a
missing/stale/wrong-incarnation --backup-arn surfaces on the rehearsal
run (finding f_24a48b8900).
- Assert configured keys match the live key schema of both tables before
any key projection, turning config/schema drift into a descriptive
abort instead of a mid-backfill KeyError (finding f_cb6b5a6c59).
- Clarify why key-existence spot-checks are sound for WorkOrderComments:
the copy Puts source items verbatim and the sample uses the same
cutoff filter, so per-account comment_id divergence never enters the
check (finding f_390b7d6c3b is a false positive; comment hardened).
2026-07-23 17:08:47 -04:00
|
|
|
# Account-agnostic fixture (the suffix is never parsed); the real bucket
|
|
|
|
|
# name is account-derived at deploy time.
|
|
|
|
|
bucket = "po-ingest-emails-000000000000"
|
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
|
|
|
# Deliberately contains a SPACE, a literal '+', and a literal '%41'.
|
|
|
|
|
# A real S3 event notification would deliver this key encoded as
|
|
|
|
|
# "inbound/2026/AB+12%2B34+%2541.eml"
|
|
|
|
|
# (space->'+', '+'->'%2B', '%'->'%25'); the handler is where any decode
|
|
|
|
|
# would live. reprocess builds from the RAW list_objects_v2 key, so it
|
|
|
|
|
# must emit "inbound/2026/AB 12+34 %41.eml" unchanged.
|
|
|
|
|
raw_key = "inbound/2026/AB 12+34 %41.eml"
|
|
|
|
|
|
|
|
|
|
event = reprocess.build_s3_event(bucket, raw_key)
|
|
|
|
|
|
|
|
|
|
# 1. Exact top-level shape: a single Records entry.
|
|
|
|
|
assert list(event.keys()) == ["Records"]
|
|
|
|
|
assert len(event["Records"]) == 1
|
|
|
|
|
|
|
|
|
|
# 2. Bucket name is nested at Records[0].s3.bucket.name.
|
|
|
|
|
assert event["Records"][0]["s3"]["bucket"]["name"] == bucket
|
|
|
|
|
|
|
|
|
|
# 3. Object key is nested at Records[0].s3.object.key, byte-for-byte.
|
|
|
|
|
assert event["Records"][0]["s3"]["object"]["key"] == raw_key
|
|
|
|
|
|
|
|
|
|
# 4. RAW / no-URL-encoding, no-URL-decoding on the emitted key.
|
|
|
|
|
emitted = event["Records"][0]["s3"]["object"]["key"]
|
|
|
|
|
assert emitted == raw_key # reprocess applies NO transformation
|
|
|
|
|
# space NOT percent-encoded (a real S3 notification would send %20):
|
|
|
|
|
assert " " in emitted and "%20" not in emitted
|
|
|
|
|
# literal '+' preserved, not turned into a space:
|
|
|
|
|
assert "+" in emitted
|
|
|
|
|
# '%41' NOT decoded to 'A':
|
|
|
|
|
assert "%41" in emitted and "A .eml" not in emitted
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
# --- Selector mutual-exclusion: --all is a MODE, not a modifier ---------------
|
|
|
|
|
# reprocess.main() reaches its first AWS call (sts get_caller_identity) only
|
|
|
|
|
# AFTER the argparse mutual-exclusion gate, so an invalid selector combination
|
|
|
|
|
# raises SystemExit(2) with no AWS access needed. This pins that `--all` cannot
|
|
|
|
|
# be silently combined with a narrower --prefix/--since (which the args.all
|
|
|
|
|
# clobber branch would otherwise discard -> unintended whole-corpus sweep).
|
|
|
|
|
def _expect_argparse_reject(monkeypatch, capsys, argv):
|
|
|
|
|
monkeypatch.setattr(sys, "argv", ["reprocess.py", *argv])
|
|
|
|
|
with pytest.raises(SystemExit) as exc:
|
|
|
|
|
reprocess.main()
|
|
|
|
|
assert exc.value.code == 2 # argparse error exit code
|
|
|
|
|
return capsys.readouterr().err
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_all_rejects_prefix(monkeypatch, capsys):
|
|
|
|
|
err = _expect_argparse_reject(
|
|
|
|
|
monkeypatch,
|
|
|
|
|
capsys,
|
|
|
|
|
["--pipeline", "po", "--all", "--prefix", "inbound/2026/07/"],
|
|
|
|
|
)
|
|
|
|
|
assert "--all cannot be combined with --prefix or --since" in err
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_all_rejects_since(monkeypatch, capsys):
|
|
|
|
|
err = _expect_argparse_reject(
|
|
|
|
|
monkeypatch,
|
|
|
|
|
capsys,
|
|
|
|
|
["--pipeline", "po", "--all", "--since", "2026-07-01T00:00:00Z"],
|
|
|
|
|
)
|
|
|
|
|
assert "--all cannot be combined with --prefix or --since" in err
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_all_rejects_prefix_and_since_together(monkeypatch, capsys):
|
|
|
|
|
_expect_argparse_reject(
|
|
|
|
|
monkeypatch,
|
|
|
|
|
capsys,
|
|
|
|
|
[
|
|
|
|
|
"--pipeline",
|
|
|
|
|
"po",
|
|
|
|
|
"--all",
|
|
|
|
|
"--prefix",
|
|
|
|
|
"inbound/2026/",
|
|
|
|
|
"--since",
|
|
|
|
|
"2026-07-01T00:00:00Z",
|
|
|
|
|
],
|
|
|
|
|
)
|