# v0.3.2 Emergency Hardening (Round 2) — Design

**Date:** 2026-04-09
**Status:** APPROVED — proceed to implementation plan
**Author:** Boris Djordjevic + Claude Opus
**Trigger:** GPT Pro full hardening review returned with 3 outage-class items v0.3.1 didn't address

---

## Goal

Ship a second fast patch (`v0.3.2`) that closes the three remaining outage-class items GPT Pro identified on top of v0.3.1, **plus** documents one doctrinal limitation in the v0.3.0 complaint/bounce guards. Same trunk-based, no-feature-branch, ~1-day pattern as v0.3.1.

## Why now (the GPT Pro findings)

GPT Pro's report validated all 4 v0.3.1 fixes (lock CAS, schema check, subprocess timeout, agent-info sync) and found 3 more critical items that v0.3.1 didn't address. All three are independent of v0.3.1 and small enough to ship as a second emergency patch:

| # | Finding | Severity | Effort |
|---|---|---|---|
| F2.1 | ESP-acceptance-before-local-commit window — crash between `email-cli batch send` Ok and the local recipient UPDATE causes resume to resend the chunk | Critical | m |
| F13.1 | Unsubscribe secret has hardcoded dev fallback `"mlc-unsubscribe-dev"` in production code (`pipeline.rs:190-191, 383-384`) — anyone running without `MLC_UNSUBSCRIBE_SECRET` set is shipping forgeable unsubscribe tokens | Critical | xs |
| F9.1 (revised) | `email_cli.profile` is dead for normal operations — only `profile_test` actually uses it | Critical | xs |

Plus a **doctrinal note** documenting that the v0.3.0 30-day complaint/bounce rate guards in `preflight_checks` are built on a lossy data source (per GPT Pro's F3.2). The user's call: leave the guards firing (they're still useful), document the limitation. No code change to the guards.

## Critical recon: F9.1 fix shape changed from GPT Pro's recommendation

GPT Pro's literal recommendation for F9.1 was: "Thread profile selection through every subprocess call. If `email-cli` expects `--profile <name>`, add it in one centralized `run_json` helper."

**Verified locally against email-cli 0.6.3:** there is NO `--profile <name>` flag at any level. `email-cli --help` shows only `--db` and `--json` as global options. `email-cli send --help` accepts `--account` (sender email address) but no profile. `email-cli profile` only has `add`, `list`, `test`, `help` subcommands — no `profile use <name>` to switch the active profile, no `--profile` flag on other subcommands. Profile selection is implicit: email-cli uses whatever profile is in its database.

GPT Pro's fallback recommendation was: "If it expects environment or config, remove the dead field from `mailing-list-cli` config until you can honor it." This is the correct path, with a softer version for backward compat:

- **Don't remove the field from config** (would break existing user configs)
- **Add a clear doc note** to agent-info + config.rs comment explaining the field is informational only and is not threaded through other commands
- **Add a multi-profile health check**: if `email-cli profile list` returns > 1 profile, warn loudly that there's ambiguity (we can't tell which one is used)
- **File an upstream issue** with email-cli requesting a `--profile <name>` global flag (notes in session handoff, no code change)

Effort drops from `s` (9 call sites) to `xs` (1 health check + doc).

---

## Fix 1 — broadcast_send_attempt write-ahead table (F2.1)

### Problem statement

`src/broadcast/pipeline.rs:286-318` is the duplicate-send window. The pipeline calls `cli.batch_send(&batch_path, &recipients_in_order)` first. ONLY after success does it open a transaction and `UPDATE broadcast_recipient SET status='sent', resend_email_id=?, sent_at=? WHERE broadcast_id=? AND contact_id=?`. If the process dies between the email-cli call returning and the local commit, the recipients are at Resend (delivered or queued) but the local DB has no record. On resume, `broadcast_recipient_already_sent_ids` returns the chunk-1 IDs but NOT chunk-N's, so chunk-N gets re-rendered and re-sent. Recipients receive duplicates.

The v0.3.1 lock fix prevents two PROCESSES from racing. It does not prevent ONE process from losing its mind after a crash.

### Architecture

**Migration 0005** adds a write-ahead attempt log table:

```sql
CREATE TABLE broadcast_send_attempt (
    id INTEGER PRIMARY KEY AUTOINCREMENT,
    broadcast_id INTEGER NOT NULL REFERENCES broadcast(id) ON DELETE CASCADE,
    chunk_index INTEGER NOT NULL,
    request_sha256 TEXT NOT NULL,
    batch_file_path TEXT NOT NULL,
    state TEXT NOT NULL CHECK(state IN ('prepared', 'esp_acked', 'applied', 'failed')),
    esp_response_json TEXT,
    created_at TEXT NOT NULL,
    updated_at TEXT NOT NULL,
    UNIQUE (broadcast_id, chunk_index, request_sha256)
);
CREATE INDEX idx_send_attempt_broadcast ON broadcast_send_attempt(broadcast_id);
CREATE INDEX idx_send_attempt_state ON broadcast_send_attempt(broadcast_id, state);
```

Columns:
- `request_sha256` — SHA-256 of the batch JSON file contents (gives us replay detection if the same chunk is retried)
- `batch_file_path` — for forensic reconstruction
- `state` — `prepared` → `esp_acked` → `applied` (happy path) or `failed` (terminal)
- `esp_response_json` — the raw email-cli batch send response, stored as TEXT, parsed on reconcile

### State transitions

```
       cli.batch_send Ok       local UPDATE tx commit
prepared ────────────────► esp_acked ─────────────────► applied
   │                          (response stored)
   │ cli.batch_send Err
   └─────────────────────► failed
```

### New flow in `send_broadcast`

**At the start of the chunk loop, BEFORE the new chunk processing:**

1. Query for `state='esp_acked'` rows for this broadcast where the recipients haven't been marked sent locally yet
2. For each unreconciled `esp_acked` attempt: parse `esp_response_json`, run the local recipient UPDATE in the same transaction that flips the attempt to `applied`
3. Log a stderr warning per reconciled attempt: `"reconciling chunk N from previous run (esp_acked → applied)"`

**For each chunk:**

1. Build the batch JSON (existing code)
2. Compute SHA-256 of the batch file contents
3. Insert attempt row with `state='prepared'`, capture the row id
4. Call `cli.batch_send(...)`:
   - **On Ok**: in a single transaction, update the attempt row to `state='esp_acked'` AND store `esp_response_json`. Then in another transaction, perform the recipient UPDATE and flip the attempt to `state='applied'`. (Splitting into two transactions is intentional: the esp_acked transaction is the durable record that ESP accepted; the applied transaction is the local correlation. If we crash between them, resume can reconcile from esp_acked.)
   - **On Err**: update attempt to `state='failed'`, propagate the error.

**Indeterminate state handling:**

If a `prepared` attempt row exists for this broadcast on resume (state never advanced to `esp_acked` or `failed`), it means a previous process crashed BETWEEN inserting the row and getting the email-cli response. We can't know if email-cli sent the chunk or not. The safest behavior:

- Log to stderr: `"WARNING: chunk N has indeterminate state from a previous run — email-cli call may or may not have completed at Resend. Retry will potentially duplicate; skip will potentially miss."`
- Return `AppError::Transient { code: 'broadcast_attempt_indeterminate', ... }` with a suggestion that includes `--force-unlock` (since the lock will need to be re-acquired) AND a manual instruction: "Inspect `broadcast_send_attempt` for chunk N, decide whether to retry or mark applied based on Resend's email list, then re-run."

This is intentionally not auto-resolving. **A duplicate send and a missed send are both bad outcomes; the operator must choose which is worse for their use case.** Default is fail-safe: refuse to proceed.

### Honest framing (preserved from GPT Pro)

This fix does NOT make sends exactly-once. It makes them **recoverable without blind duplicate resend after a local crash**. The remaining indeterminate window is microseconds (between `prepared` row insert and the email-cli return), as opposed to today's window which is **the entire duration of email-cli's network call to Resend** (potentially seconds).

### Tests

| Test | What it asserts |
|---|---|
| `attempt_table_created_by_migration_0005` | `broadcast_send_attempt` table exists with all columns |
| `attempt_prepared_then_esp_acked_then_applied` | happy-path transitions for one chunk |
| `attempt_failed_terminal` | failure path leaves row in `failed` state |
| `reconcile_esp_acked_marks_recipients_sent` | injecting an `esp_acked` row + crashed local state, then reconcile applies it |
| `indeterminate_prepared_blocks_send` | injecting a `prepared` row + new send returns `broadcast_attempt_indeterminate` error |
| `request_sha256_uniqueness` | INSERT OR IGNORE on duplicate `(broadcast_id, chunk_index, request_sha256)` |

---

## Fix 2 — Unsubscribe secret hard-fail (F13.1)

### Problem statement

`src/broadcast/pipeline.rs:190-191` and `src/broadcast/pipeline.rs:383-384` both contain:

```rust
let unsubscribe_secret = std::env::var(&config.unsubscribe.secret_env)
    .unwrap_or_else(|_| "mlc-unsubscribe-dev".to_string());
```

This is a developer convenience left in the live send path. The `mlc-unsubscribe-dev` fallback means:

- Anyone running `mailing-list-cli broadcast send` without setting `MLC_UNSUBSCRIBE_SECRET` ships unsubscribe links signed with a public, predictable HMAC key
- The token verifier accepts any token signed with that key
- An attacker who knows the fallback (it's in the source code) can forge unsubscribe tokens for any contact
- This isn't a hypothetical: a v0.3.1 user who forgot to export the env var is silently using the dev key

### Architecture

Replace the `unwrap_or_else` with `?` returning `AppError::Config` (exit 2). The change is 4 lines per call site, 2 call sites total.

```rust
let unsubscribe_secret = std::env::var(&config.unsubscribe.secret_env)
    .map_err(|_| AppError::Config {
        code: "missing_unsubscribe_secret".into(),
        message: format!(
            "environment variable `{}` is not set; cannot sign unsubscribe links",
            config.unsubscribe.secret_env
        ),
        suggestion: format!(
            "Export {} with at least 16 bytes of random data (e.g., `export {}=$(openssl rand -hex 32)`) before running broadcast send",
            config.unsubscribe.secret_env, config.unsubscribe.secret_env
        ),
    })?;
```

### Impact on existing tests

The existing tests/cli.rs already sets `MLC_UNSUBSCRIBE_SECRET` for any test that exercises send_broadcast. The smoke test sets it. Real-Resend usage requires it. The only thing that breaks is "running send without the env var set" — which is exactly the case we want to break.

### Tests

| Test | What it asserts |
|---|---|
| `send_fails_fast_without_unsubscribe_secret` | calling `send_broadcast` with `MLC_UNSUBSCRIBE_SECRET` unset returns `AppError::Config { code: 'missing_unsubscribe_secret' }` |

---

## Fix 3 — email_cli.profile dead-field doc + multi-profile health check (F9.1, revised)

### Problem statement

GPT Pro F9.1: "The configured `email_cli.profile` is effectively ignored for real operations." Verified: only `profile_test` (used by health check) actually passes the profile name to email-cli. Every other call site (`agent_info`, `segment_create`, `contact_create`, `domain_list`, `batch_send`, `send`, `email_list`) ignores `self.profile`.

**However**: GPT Pro's literal fix ("thread profile through every call") is **impossible** because email-cli 0.6.3 has no `--profile` flag at any level. There is no way to tell email-cli "use profile X" per-command. Profile selection is database-implicit.

### Architecture (revised from GPT Pro)

Three changes, all xs effort:

**A. Update `EmailCli::profile` field documentation** to clarify the limitation:

```rust
pub struct EmailCli {
    pub path: String,
    /// The configured email-cli profile name. Currently used ONLY by
    /// `profile_test()` for the health check; NOT threaded through other
    /// commands because email-cli 0.6.3 has no global `--profile <name>`
    /// flag. Profile selection is implicit (whatever profile is in
    /// email-cli's database). For multi-profile operators, use
    /// `health` to detect ambiguity.
    pub profile: String,
    last_call: Mutex<Instant>,
}
```

**B. Add `email_cli_single_profile` health check** in `src/commands/health.rs`:

The check calls `email-cli profile list`, parses the response, and:
- If 0 profiles: FAIL — operator hasn't set up email-cli at all
- If 1 profile: OK
- If > 1 profile: WARN — there's ambiguity in which profile is used; configure-time `[email_cli].profile` is informational only

The warn case is a NEW health state. Currently health checks are `ok | fail`. Adding `warn` requires extending the enum. Keep it simple: WARN reduces to `ok` for the overall status (no exit-2) but the message field surfaces the ambiguity.

Actually, simpler: emit as `ok` with an explanatory message. The operator reads the message. No new state needed.

**C. Update agent-info `env_vars` and the `health` command description** to mention this:

```rust
"health": "Run a system health check (config, email-cli on PATH, sender_domain_verified, db reachable, schema_version current, email-cli profile uniqueness)",
```

Plus add a top-level note in agent-info under a new key:

```rust
"known_limitations": [
    "email-cli profile selection is database-implicit; the `[email_cli].profile` config field is used only for the health-check `profile test` call. If multiple email-cli profiles exist, behavior is ambiguous — use `mailing-list-cli health` to detect this.",
    "30-day complaint/bounce rate guards in `broadcast send` preflight are computed from a lossy local event mirror (paginated by email ID, not by event). Treat the rates as approximate. See docs/email-cli-gap-analysis.md."
]
```

**D. (Documentation only, no code)** — file an upstream issue with `paperfoot/email-cli` requesting:
- Add a global `--profile <name>` flag that all commands honor
- OR add an `email-cli profile use <name>` to set the active profile for the database
- OR add `EMAIL_CLI_PROFILE` env var support

Notes in the session handoff. No commit.

### Tests

| Test | What it asserts |
|---|---|
| `health_passes_with_single_profile` | health emits ok when stub returns 1 profile |
| `health_warns_on_multiple_profiles` | health emits ok with a warning message when stub returns 2 profiles |

The stub email-cli script needs an `email-cli profile list` mode. Easy to add.

---

## Fix 4 — Doctrinal note: complaint/bounce guards are approximate (per user pick)

### Problem statement (GPT Pro F3.2)

The `event` table is populated by `webhook poll` which paginates `email-cli email list` by email ID and reads `last_event` per row. This means:
1. Later state changes on already-seen emails are invisible
2. Even visible state is lossy (only the most recent event)
3. `historical_send_rates` (the function backing the v0.3.0 30-day complaint/bounce guards) reads from this table

**User's decision (from AskUserQuestion 2026-04-09):** Leave the guards firing — they're still useful safety nets even on imperfect data — but document the limitation so operators don't over-trust them.

### Architecture

No code change to `historical_send_rates` or `preflight_checks`. Add documentation in three places:

1. **agent-info `known_limitations` array** (already added in F9.1 fix, see above)
2. **AGENTS.md "Production hardening" section** — add a one-sentence asterisk
3. **Docstring on `historical_send_rates`** in `src/db/mod.rs` — explain the limitation in comments so future contributors understand

### Tests

None. Documentation only.

---

## Migration impact

| Migration | Status |
|---|---|
| 0001_initial | shipped pre-v0.2 |
| 0002_event_idempotency_and_kv | shipped v0.2 |
| 0003_template_html_source | shipped v0.2 (no-op) |
| 0004_broadcast_locks | shipped v0.3.1 |
| **0005_broadcast_send_attempt** | **shipped v0.3.2 (this spec)** |
| 0006_content_snapshots_and_revenue | **planned for v0.4** — was 0004 in session 5, then 0005 after v0.3.1; **must be renumbered to 0006 for v0.4** |

**ACTION REQUIRED for the v0.4 plan author** (still pending): the content-snapshot + revenue migration MUST be `0006_content_snapshots_and_revenue`. Updated in session-7 handoff at the end of v0.3.2.

## Doctrine alignment

- **Zero new runtime crates.** 14-crate budget intact. ✓
- **Single-tenant, local-first.** No network surface added. ✓
- **No interactive prompts.** F2.1 indeterminate state returns an error with operator instructions, not a prompt. ✓
- **Semantic exit codes.** Indeterminate attempt = exit 1 (Transient), missing secret = exit 2 (Config), multi-profile warning = exit 0 (ok with message). ✓
- **Self-describing via agent-info.** New `known_limitations` block. ✓
- **TDD.** Every fix has tests written first (where applicable). ✓

## Out of scope (deferred)

- **Audit log table (F4.1).** v0.5+.
- **db/mod.rs split (F7.1).** v0.5+.
- **Structured logging / tracing (F3.1).** v0.5+ — separate proposal.
- **Schema migration `db migrate ls/dry-run/check` family (F8.1).** v0.5+.
- **Streaming pipeline at 100k+ (F10.1).** v0.7+.
- **Webhook event source rebuild (F3.2).** v0.5+ — needs upstream coordination with email-cli.
- **Read-modify-write race fixes outside the send loop (F1.2).** v0.5+ — touches many commands, not outage-class on its own.
- **Partial send terminal state `partial` (F2.2).** v0.5+ — would conflict with v0.4 monetization scope.
- **Contract violations in health/report engagement/broadcast send (F5.1).** v0.5+.
- **Error class collapse fix (F6.1).** v0.5+.
- **Error code catalog (F6.2).** v0.5+.
- **Strict response parsing (F9.3).** v0.5+.
- **Chaos / fault-injection tests (F11.1).** v0.5+.
- **Footgun cleanup (F12.1, F12.2, F12.3).** v0.5+ — except F13.1 above.
- **File mode hardening (F13.2).** v0.6+.
- **Doctrine tests (F14.1).** v0.5+.
- **`invocation_id` propagation (F15.1).** v0.6+.

This is intentional: v0.3.2 ONLY closes the 3 outage-class items. Everything else goes into v0.5 / v0.6 / v0.7 per GPT Pro's roadmap (which we'll fold into the next plan write).

## Effort + tests

| # | Fix | Files touched | Tests added |
|---|---|---|---|
| 1 | F2.1 broadcast_send_attempt (migration 0005, DB methods, pipeline reconcile + write-ahead) | `src/db/mod.rs`, `src/db/migrations.rs`, `src/broadcast/pipeline.rs` | 6 |
| 2 | F13.1 unsubscribe secret hard-fail | `src/broadcast/pipeline.rs` | 1 |
| 3 | F9.1 dead-field doc + multi-profile health check | `src/email_cli.rs`, `src/commands/health.rs`, `src/commands/agent_info.rs`, `tests/support/stub_email_cli.sh` | 2 |
| 4 | F3.2 doctrinal note (no code) | `src/commands/agent_info.rs`, `src/db/mod.rs` (docstring), `AGENTS.md` | 0 |
| Release | bump Cargo.toml → 0.3.2, README badge, agent-info status, smoke, tag, push, crates, homebrew, handoff | `Cargo.toml`, `README.md`, `src/commands/agent_info.rs`, `Formula/mailing-list-cli.rb` | 0 (smoke manual) |
| **Total** | | **5 source files modified** | **+9 tests (189 → 198)** |

Effort: ~1 day inline TDD execution.

## Release task

Same as v0.3.1: 13-step v0.3.0 regression flow + 2 new v0.3.1 steps (lock + schema) + new v0.3.2 steps:

| Step | What |
|---|---|
| 1-15 | Existing v0.3.1 smoke flow |
| **16 NEW** | F13.1: unset MLC_UNSUBSCRIBE_SECRET, attempt `broadcast send`, expect exit 2 + `missing_unsubscribe_secret` |
| **17 NEW** | F2.1: simulate `esp_acked` attempt by manually inserting a row with state='esp_acked' and a fake response, run `broadcast resume`, verify the attempt is reconciled (state → applied) and recipients marked sent |
| **18 NEW** | F2.1 indeterminate: insert `prepared` attempt row, run `broadcast send`, expect exit 1 + `broadcast_attempt_indeterminate` |
| **19 NEW** | F9.1: simulate `email-cli profile list` returning 2 profiles, run `health`, verify the profile_count message |

---

## Approval status

- [x] User explicitly approved Option A (v0.3.2 emergency patch with the 3 outage items) via AskUserQuestion 2026-04-09
- [x] User explicitly approved leaving v0.3.0 complaint/bounce guards firing with documented limitation
- [x] Spec self-review:
  - **Placeholders:** none
  - **Type consistency:** `broadcast_send_attempt` table fields match across migration, DB methods, and tests
  - **Migration accounting:** 0005 = broadcast_send_attempt, v0.4 = 0006
  - **Honest framing:** F2.1 explicitly does NOT promise exactly-once delivery
  - **Indeterminate state:** explicit fail-safe behavior, operator manual decision required

**Next step:** Invoke `superpowers:writing-plans` to produce the TDD implementation plan, then execute inline.
