af-disaster-assistance-gov-agent — Prop-Build Analysis

Document Type: Critical Review & Analysis (companion to prop-build-template.md) Scope: Per-Repo / Per-Module Subject: af-disaster-assistance-gov-agent (DisasterAssistance.gov AI Automation Agent) Reviewer(s): Claude (automated code review) Date: 2026-04-09 Version: 0.1 Confidence Level: Medium What would raise confidence: Running the container locally end-to-end against mock FEMA; access to CloudWatch metrics and ECS task telemetry; interview with Gordon Zheng on two-agent split rationale; execution of pytest + e2e perf harness.

Inputs Reviewed:

• Prop-build doc: /Users/andres/src/af/af-analysis/data/af-disaster-assistance-gov-agent.yaml
• Companion docs: /Users/andres/src/af/af-analysis/data/af-disaster-assistance-gov-agent/{api-examples,data-flow,runbook,deployment}.md
• Source tree: /Users/andres/src/af/af-disaster-assistance-gov-agent/ (fema_agent/src/web_service/{app.py:1271, state.py:388, agent_runner.py:317})
• Dashboards / metrics: not accessed
• ADRs / design docs: specs/001..020 directories (listing only)
• Interviews: none

A.1 Executive Summary

• Overall health: Functional and cleanly split into a Flask data-plane, FSM, agent runner, and survivor parser; architecture is coherent for a per-survivor ephemeral container, but the HTTP surface is oversized and unauthenticated at the app layer.
• Top risk: All 22 Flask endpoints, including destructive and PII-accepting routes, have auth: "none" and rely entirely on network isolation + CloudFront signed URLs — a single misconfiguration in ALB/SG removes all authz (see A.6.5, A.11).
• Top win / thing worth preserving: Sentinel-file completion pattern (temp/agent*.completed) decouples "agent finished successfully" from "subprocess exited cleanly" — robust against unclean Claude Code exits (fema_agent/src/web_service/agent_runner.py).
• Single recommended next action: Gate the Flask data-plane behind a shared-secret header or mTLS verified by the container, even behind the ALB, so app-layer authz exists as defense-in-depth.
• Blocking unknowns: Actual test coverage %, CI pass rate, production error rates, concurrent-task ceiling, and whether specs/019-stuck-agent-watcher is deployed.

A.2 Health Scorecard

Overall score: 3.1 — architecture and docs are above average; security posture and test signal are the drag.

A.3 What's Working Well

• Strength: Sentinel-file completion signal decoupled from subprocess exit code.

  • Location: fema_agent/src/web_service/agent_runner.py:23-91 and dual-location poll.
  • Why it works: Claude Code CLI subprocesses can exit uncleanly; relying on exit code would produce false negatives. The sentinel reifies "agent wrote its completion marker," a stronger invariant.
  • Propagate to: af-fema-real-ai-agent sister repo; any future agent-runtime containers.

• Strength: Two-agent split around the Login.gov human-in-the-loop boundary.

  • Location: State machine fema_agent/src/web_service/state.py:8-16 plus script generator in fema_agent/src/raw_builder/ + intermediate_builder/.
  • Why it works: Makes the unavoidable human 2FA gate an explicit state (WAITING_FOR_LOGIN) rather than a hidden pause, enabling the frontend to render the VNC iframe at the right moment.
  • Propagate to: Any other gov-portal automation repos encountering Login.gov / ID.me.

• Strength: Clear module decomposition under fema_agent/src/ (web_service, survivor_api, raw_builder, intermediate_builder, enums).

  • Location: fema_agent/src/ package layout.
  • Why it works: Parser, builder, and runtime concerns are separable; supports unit testing of pure-data layers without Flask/subprocess stack.

A.4 What to Improve

A.4.1 P0 — Add app-layer authentication to the Flask data-plane

• Problem: Every documented and undocumented route is auth: "none", relying solely on SG + CloudFront signed URLs.
• Evidence: YAML section_3_api.endpoints[*].auth = "none"; fema_agent/src/web_service/app.py:188–838 shows no @requires_auth decorator on any route.
• Suggested change: Add a shared-secret header (HS256 JWT or per-task bearer token via Secrets Manager) in a Flask before_request hook.
• Estimated effort: M
• Risk if ignored: One SG misconfig → full PII breach + /test/force-status allows arbitrary state transitions.

A.4.2 P1 — Split test/internal routes out of the production Flask app

• Problem: /test/agent-auth, /test/agent-complete, /test/force-status, /test/setup-mock, /test/e2e-mock, /agent1/restart, /agent2/restart, /agent*/status_detailed, and /setup-fema ship in the same binary as prod.
• Evidence: fema_agent/src/web_service/app.py:707–838; YAML notes "~12 additional internal/test-only endpoints."
• Suggested change: Gate behind ENABLE_TEST_ROUTES env var defaulted off, or separate Blueprint loaded only in dev/stg.
• Estimated effort: S
• Risk if ignored: Prod blast radius includes arbitrary state manipulation.

A.4.3 P1 — Capture and publish test coverage

• Problem: YAML lists coverage_pct: null; unknown whether fema_agent/tests/ actually exercises FSM, parser validators, and stale-detection.
• Evidence: section_15_testing.unit.coverage_pct: null.
• Suggested change: Wire pytest --cov=fema_agent --cov-report=xml into CI; fail under threshold.
• Estimated effort: S
• Risk if ignored: FSM regressions invisible until production.

A.4.4 P2 — Replace print(..., file=sys.stderr) with structured logging

• Evidence: fema_agent/src/web_service/app.py:357-360, 472-474.
• Suggested change: Python logging with JSON formatter; include request/survivor correlation id.
• Estimated effort: S

A.5 Things That Don't Make Sense

1. Observation: Two agents spawn claude-full subprocesses, but the state machine defines both production (9) and test-only (6) states in the same enum.

   • Location: fema_agent/src/web_service/state.py:8-49.
   • Question for author: Why not a separate TestState enum so the prod FSM's valid-transition matrix stays minimal?

2. Observation: Sentinel file is polled at two locations (pre-archive and post-archive).

   • Location: fema_agent/src/web_service/agent_runner.py:94+.
   • Question for author: Determined ordering, or genuinely defensive against concurrent archive?

A.6 Anti-Patterns Detected

A.6.1 Code-level

• God object / god function — app.py:164–838 (1271 LOC file, single create_app)
• Shotgun surgery
• Feature envy
• Primitive obsession
• Dead code
• Copy-paste / duplication — /survivor-info, /p1, /p2 near-identical handlers
• Magic numbers — AGENT_INACTIVITY_TIMEOUT_SECONDS=600, NOVNC_HEARTBEAT_SECONDS=30
• Deep nesting
• Long parameter lists
• Boolean-flag parameters

A.6.2 Architectural

• Big ball of mud
• Distributed monolith
• Chatty services
• Leaky abstraction — Flask state dict holds Popen handles alongside survivor JSON
• Golden hammer
• Vendor lock-in — Bedrock + Claude Code CLI + chrome-devtools-mcp, no abstraction layer
• Stovepipe
• Missing seams for testing — hard-coded filesystem paths, direct subprocess.Popen, direct Chrome CDP URLs

A.6.3 Data

N/A — no persistent DB in this repo.

A.6.4 Async / Ops

• Poison messages
• Retry storms
• Missing idempotency keys
• Hidden coupling via shared state — global Flask _state mutated by multiple routes + polling thread
• Work queues without visibility

A.6.5 Security

• Secrets in code / .env committed
• Missing authn/z on internal endpoints — 22 routes, zero auth decorators
• Overbroad IAM roles
• Unvalidated input crossing trust boundary — /test/force-status accepts arbitrary state strings
• PII/PHI in logs or error messages (suspected) — validation errors embed field values into 400 descriptions

A.6.6 Detected Instances

A.7 Open Questions

1. Q: Are /test/* routes compiled out or disabled by env in production deploys?
   • Blocks: A.4.2, A.6 #9
   • Who can answer: Gordon Zheng / platform SRE
2. Q: What is the actual pytest coverage today?
   • Blocks: A.2 row 15, A.4.3
   • Who can answer: CI logs
3. Q: Is specs/019-stuck-agent-watcher merged and running in prod?
   • Blocks: Confidence in stale-agent recovery
   • Who can answer: Repo maintainer

A.8 Difficulties Encountered

• Difficulty: No access to CI output, coverage, or runtime metrics.
  • Impact: Scored row 15 conservatively.
  • Fix: Publish CI summary (coverage %, test count, flake rate) into a badge or ci-summary.json.
• Difficulty: Two sister repos (af-fema-real-ai-agent) with "near-identical" contents but no shared library.
  • Impact: Unable to tell which is canonical; duplication risk unmeasured.
  • Fix: Extract shared Python package or document divergence explicitly.

A.9 Risks & Unknowns

A.9.1 Known risks

A.9.2 Unknown unknowns

• Area not reviewed: CDK stack under aws_deployment/ (IAM policies, SG rules, CloudFront rules).
  • Reason: Out of scope for per-repo Part A.
  • Best guess: M — the entire security model relies on these.
• Area not reviewed: fema_agent/tests/ contents.
  • Reason: Time.
  • Best guess: M.
• Area not reviewed: specs/001–020 (20 spec directories).
  • Reason: Time.
  • Best guess: L — docs only.

A.10 Technical Debt Register

A.11 Security Posture (lightweight STRIDE)

A.12 Operational Readiness

A.13 Test & Quality Signals

• Coverage: N/A — coverage_pct: null
• Untested critical paths (suspected): FSM stale-detection, dual-location sentinel poll, _clear_chrome_session_restore
• Missing test types: [ ] unit [x] integration [ ] e2e [x] contract [x] load [x] security/fuzz

A.14 Performance & Cost Smells

• Hot paths: Agent subprocess poll loop; sentinel file stat.
• Suspected bottlenecks: Chrome rendering under X11; Bedrock latency dominates per-page time.
• Oversized infra: 2 vCPU / 4 GB per survivor — verify with telemetry.

A.15 Bus-Factor & Knowledge Risk

• Only-person: Single author (Gordon Zheng). Script generator + raw/intermediate builder entirely author-originated.
• What breaks: DOM-selector regressions when DisasterAssistance.gov changes; claude-full wrapper tuning.
• Tribal knowledge: Mapping between page numbering and script sections; rationale for 2-location sentinel polling.
• Actions: Architecture walkthrough recording; rationale comments in raw_builder/; pair-review script regeneration with second engineer.

A.16 Compliance Gaps

N/A — YAML does not claim HIPAA/SOC 2/state-insurance compliance. FEMA/PII handling regime implied but not asserted. Formal compliance review belongs in af-infra.

A.17 Recommendations Summary