docs(spec): SP25 SFTP Polling Enablement — initial design

2026-06-24 15:25:20 -06:00
parent 1860782ad6
commit 7d2000ec31
1 changed files with 181 additions and 0 deletions
@@ -0,0 +1,181 @@
 # SP25 — SFTP Polling Enablement: Design Spec
 **Date:** 2026-06-24
 **Status:** Draft, awaiting user sign-off
 **Branch:** `sp25-sftp-polling-enablement`
 **Aesthetic direction:** No new UI. Tiny backend delta on top of the SP16 scheduler, plus an operator runbook. The polling loop, handlers, idempotency table, and admin endpoints already exist and are not touched.
 ## 1. Scope
 SP16 shipped a complete inbound-MFT polling scheduler — handlers for TA1, 999, 835, and 277CA; admin endpoints `start`, `stop`, `tick`, `status`, `processed-files`; idempotency via the `processed_inbound_files` table; opt-in autostart via the `CYCLONE_SCHEDULER_AUTOSTART` env var. SP13 wired real paramiko SFTP behind the same `SftpClient` interface. What is missing is **the last mile that lets an operator point that loop at `mft.gainwelltechnologies.com` and let it run unattended**.
 SP25 closes that gap. Three changes, all small:
 1. **Portable secrets.** Extend `cyclone.secrets.get_secret()` with a plain env-var fallback ahead of the macOS Keychain lookup, so the same code path serves a macOS workstation, a Linux server, and a Docker container without conditional deployment glue.
 2. **Mutable clearhouse block.** Add `PATCH /api/clearhouse` so the operator can flip `sftp_block.stub` from `true` to `false` (and adjust host, port, paths, poll interval) without touching SQLite directly. The patch re-configures the running scheduler so the next tick uses the new block — no process restart required.
 3. **Operator runbook.** A new `docs/RUNBOOK.md` documents the exact env-var values, the order of operations, and the verification commands. The runbook is the contract between this spec and any operator bringing up SFTP polling on a fresh host.
 Out of scope (explicit, each is its own future SP if requested):
 - A frontend operator UI for scheduler start/stop/tick/status. SP16's endpoints exist; the operator uses curl.
 - Multi-block polling (per-payer or per-tenant SFTP blocks). SP9 explicitly deferred this; today the loop watches the single dzinesco clearhouse block.
 - A separate polling daemon process. SP16 runs in the FastAPI process; splitting it into a sidecar is a bigger redesign and not required for the user's "just turn it on" goal.
 - The `<NAME>_FILE` Docker-secret convention. The user explicitly chose plain env vars; the file-based fallback can be a follow-up if a Docker deployment lands without further changes here.
 - Any 277CA-specific changes. TA1, 999, 835 are the user's target; 277CA already works via the SP16 handler and is left alone.
 ## 2. Goals
 1. **A single curl can flip the loop from stub to real.** The operator does not need SQL access, a restart, or a redeploy to start polling real MFT.
 2. **The secrets path is portable.** The same binary runs on macOS (Keychain) and on a Linux server (env var) with zero code changes between the two.
 3. **The next tick after a PATCH uses the new block.** Operator-visible feedback is "I changed a setting, the next scheduler tick honors it" — not "I changed a setting, the change takes effect on restart."
 4. **The runbook is self-contained.** An operator reading `docs/RUNBOOK.md` can bring up polling on a fresh host without reading the spec, the source, or any commit messages.
 ## 3. Locked decisions
 ### 3.1 Secret lookup order — env var first, Keychain second
 `cyclone.secrets.get_secret()` currently calls `keyring.get_password("cyclone", name)`. On any host without `keyring` installed (Linux servers, Docker containers without the `keyring` extra), it returns `None` and `SftpClient._connect()` then refuses to authenticate, raising a `RuntimeError` that the scheduler surfaces as a failed tick.
 The fix is a three-tier lookup, evaluated in this order:
 1. **Plain env var.** If `<NAME>` is present in `os.environ`, return its `.strip()`-ed value. The variable name matches the Keychain account name verbatim — so `CYCLONE_SFTP_PASSWORD` is the env-var form of the Keychain account `sftp.gainwell.password`. The strip handles the common copy-paste-from-`.env` case where the value has a trailing newline.
 2. **macOS Keychain.** Existing behavior, unchanged. Kept as a fallback so a macOS dev box that prefers the Keychain CLI workflow (`security add-generic-password`) still works without env-var exports.
 3. **None.** Return `None` so the caller can decide what to do. `SftpClient._connect` already handles this with a precise error message.
 Rationale for "env var first, not Keychain first": env vars work everywhere; Keychain only works on macOS. Putting env var first means a Linux server needs nothing special, and a macOS dev box that exports the env var intentionally takes precedence over an unrelated Keychain entry from a previous test. The order is the inverse of the SP23 admin-bootstrap convention (which is `_FILE`-then-env-var) because the user explicitly chose plain env vars over Docker-secret files for this increment.
 ### 3.2 `PATCH /api/clearhouse` — typed, gated, hot-reloading
 A new endpoint sits next to the existing `GET /api/clearhouse` (which is already gated by `matrix_gate`). The body is the full `Clearhouse` Pydantic model (single source of truth — no separate request/response DTOs). Behavior:
 - Validates the body via Pydantic. Invalid shape → 422. Unknown fields → 422 (Pydantic default for an extra-strict model).
 - Writes the new row inside one `SessionLocal()` context.
 - After commit, calls `scheduler_mod.configure_scheduler(sftp_block, force=True)` so the module singleton is replaced.
 - If the scheduler was already running before the PATCH (autostart on, or a previous manual start), the helper restarts it against the new block. The in-flight tick, if any, finishes against the old client; the next tick uses the new one. Coalescing is already handled by `Scheduler._tick_in_progress`.
 - If the scheduler was not running, the PATCH just leaves it stopped.
 Auth: same `Depends(matrix_gate)` as every other `/api/admin/*` and `/api/clearhouse*` endpoint. No new permission tier.
 Validation rules to enforce on the body:
 - `sftp_block.stub` must be a real bool, not a stringified "true"/"false".
 - `sftp_block.host`, when `stub` is `false`, must be non-empty.
 - `sftp_block.paths.inbound` must be non-empty.
 - `sftp_block.password_keychain_account`, when `stub` is `false`, must be non-empty (otherwise `SftpClient._connect` raises an unclear error at next tick).
 The endpoint returns the post-update `Clearhouse` (same shape as `GET /api/clearhouse`) so the operator can pipe-verify the change in one round-trip.
 ### 3.3 No scheduler restart, just a reconfigure
 The scheduler is an `asyncio.Task` with a single `_run()` loop. `configure_scheduler(force=True)` replaces the module-level `_scheduler` singleton. The helper also has to handle the "scheduler was running" case: cancel the existing task with a 30-second wait (matching the existing `Scheduler.stop()` timeout), then start the new instance. The implementation lives in `cyclone.scheduler` as a new `reconfigure_scheduler()` function so the API endpoint stays thin.
 This avoids the operator-visible cliff of "I changed a setting, now I have to restart the API." The existing `stop()` semantics — drain the in-flight tick, then exit — carry over to the reconfigure path.
 ### 3.4 Runbook placement — new `docs/RUNBOOK.md`
 A new top-level doc dedicated to operator procedures. Fits the SP23 Docker posture which already implies operator-facing documentation. The runbook contains:
 - **Prerequisites** — Python 3.11+, `keyring` optional, outbound network reachability to `mft.gainwelltechnologies.com:22`.
 - **Env-var reference** — table of every `CYCLONE_*` env var the operator might set for this increment (password, scheduler autostart, scheduler poll interval, log level).
 - **First-time setup** — set `CYCLONE_SFTP_PASSWORD`, PATCH to flip stub false, restart (or rely on autostart on next launch), verify via `GET /api/admin/scheduler/status`.
 - **Verification commands** — exact curl incantations for status, tick-now, processed-files. Including how to filter for `status=error` rows.
 - **Troubleshooting** — what each error class means (`AuthenticationException` from paramiko → wrong password; `IOError` from `listdir_attr` → wrong inbound path; tick records zero files → SFTP server has nothing in the inbound dir yet).
 - **macOS dev box variant** — using `security add-generic-password` instead of the env var.
 The runbook is committed in the same PR as the code. No doc-only follow-up.
 ### 3.5 Idempotency is unchanged
 The `processed_inbound_files` table, the `(sftp_block_name, name)` unique index, and the `STATUS_*` taxonomy are all unchanged. A re-tick after a PATCH still skips files that were already processed against any previous block (keyed by `sftp_block_name` and the inbound filename). The PATCH does not need to clear or backfill any rows.
 ### 3.6 Audit log stays clean
 Inbound file processing is operational metadata, not part of the HIPAA audit chain (per the existing SP16 scheduler docstring and the SP11 audit-log design). SP25 does not change that boundary — PATCH on the clearhouse row is itself not audited; the resulting inbound-file processing continues to surface via `processed_inbound_files` and the existing pubsub events (`ack_received`, `claim.payer_rejected`, `claim_submitted`, `remittance_written`).
 ## 4. Files
 **Modified:**
 - `backend/src/cyclone/secrets.py` — add env-var lookup ahead of Keychain. New `get_secret()` docstring describing the three-tier lookup. No new public symbols beyond a small constant for the env-var-prefix mapping.
 - `backend/src/cyclone/scheduler.py` — add `reconfigure_scheduler(sftp_block)` helper that cancels any running task (with the existing 30-second drain), replaces the singleton, and restarts if the previous one was running.
 - `backend/src/cyclone/api.py` — add `PATCH /api/clearhouse` next to the existing `GET /api/clearhouse` and `POST /api/clearhouse/submit`. Thin handler: validate body → `store.update_clearhouse()` → `_scheduler_mod.reconfigure_scheduler()` → return the new row.
 - `backend/src/cyclone/store.py` — add `update_clearhouse(block: Clearhouse) -> Clearhouse` that replaces the singleton row in a single session. Idempotent in shape but the call site is the PATCH endpoint, not lifespan, so idempotency is a nice-to-have not a hard requirement.
 **New:**
 - `backend/tests/test_secrets_envvar.py` — covers all three tiers plus the `.strip()` and `None` paths.
 - `backend/tests/test_api_clearhouse_patch.py` — covers happy path, 422 on invalid body, 401 on missing auth, scheduler reconfigure call, `matrix_gate` integration.
 - `backend/tests/test_scheduler_reconfigure.py` — covers the cancel-then-restart behavior, the running-vs-stopped cases, and the in-flight tick drain.
 - `docs/RUNBOOK.md` — the operator doc described in §3.4.
 **New migration:** none. The `clearhouse` table is unchanged. The `processed_inbound_files` table is unchanged.
 ## 5. API surface
 | Method | Path | Body | Returns | Auth |
 |---|---|---|---|---|
 | `PATCH` | `/api/clearhouse` | Full `Clearhouse` model | Updated `Clearhouse` | `matrix_gate` (admin) |
 No new endpoints beyond `PATCH`. No new env-var-driven behavior beyond the documented ones. No new pubsub events.
 ## 6. Env vars (operator-facing)
 | Variable | Default | Purpose |
 |---|---|---|
 | `CYCLONE_SFTP_PASSWORD` | unset | Plain-env-var form of the MFT password. Highest-priority lookup in `secrets.get_secret()`. Stripped of leading/trailing whitespace. |
 | `CYCLONE_SCHEDULER_AUTOSTART` | unset (falsy) | When `1`/`true`/`yes`, the scheduler's `start()` is called from the FastAPI lifespan. Existing behavior, unchanged. |
 | `CYCLONE_SCHEDULER_POLL_SECONDS` | `60` | Poll interval. Existing behavior, unchanged. |
 The user explicitly chose plain env var over Docker-secret file convention. If a Docker deployment lands later, a follow-up SP can add a `<NAME>_FILE` tier ahead of the plain env var without changing this spec's contracts.
 ## 7. Validation rules
 No new `R_*` rule IDs in `cyclone.validation.rules`. The Pydantic model for `Clearhouse.sftp_block` already enforces the field shapes; the endpoint just relies on those. Additional inline checks (non-empty `host` when `stub=false`, etc.) are Pydantic `field_validator`s on `SftpBlock`.
 ## 8. Testing plan
 1. **`test_secrets_envvar.py`** — 8 cases:
   - Env var set, Keychain present → env wins (returned).
   - Env var set with trailing `\n` → returns stripped value.
   - Env var set, Keychain absent → env returned.
   - Env var absent, Keychain present → Keychain returned.
   - Both absent → returns `None`.
   - `keyring` library missing (monkeypatch the lazy import) → falls through to `None`.
   - Env var value is the empty string → treated as absent (return `None`, do not propagate).
   - Env var present but file path is unreadable (deferred — not applicable since `_FILE` is out of scope; skip).
 2. **`test_api_clearhouse_patch.py`** — 7 cases:
   - PATCH happy path: stub=true → stub=false, host="mft.gainwelltechnologies.com", GET reflects change.
   - PATCH with `stub="yes"` (string) → 422.
   - PATCH with missing `sftp_block.host` and `stub=false` → 422.
   - PATCH without session cookie → 401 (matrix_gate).
   - PATCH with `password_keychain_account=""` and `stub=false` → 422.
   - PATCH triggers `configure_scheduler(force=True)` — verified by mocking.
   - PATCH returns the post-update row, identical shape to `GET /api/clearhouse`.
 3. **`test_scheduler_reconfigure.py`** — 5 cases:
   - Scheduler not running → PATCH leaves it stopped, singleton replaced.
   - Scheduler running → PATCH cancels old task, starts new one, no overlapping ticks.
   - PATCH during a tick → in-flight tick completes against old client, next tick uses new client.
   - `reconfigure_scheduler` 30-second drain timeout matches the existing `Scheduler.stop()` semantics.
   - PATCH with stub=true and stub=false in sequence → both calls succeed, no leaked tasks.
 **Target backend test count after SP25: current + 8 + 7 + 5 = current + 20 tests.**
 ## 9. Out of scope (future SPs)
 - **Frontend operator UI** for `start`/`stop`/`tick`/`status`/`processed-files`. The curl-based workflow documented in the runbook is sufficient for now.
 - **Multi-block polling** (per-payer or per-tenant SFTP blocks). The dzinesco singleton is sufficient for one billing office.
 - **Separate polling daemon** (`python -m cyclone poll`). The in-process scheduler is fine while the operator is fine with the API being up.
 - **`<NAME>_FILE` Docker-secret convention** for `get_secret()`. Plain env vars are enough today; add the file tier when a Docker deployment arrives.
 - **Per-file-type fan-out** beyond TA1/999/835/277CA. Future X12 types (270/271/276/278/820/834/ENCR per the HCPF doc) can land as their parsers ship.
 ## 10. Open questions resolved this session
 | # | Question | Resolution |
 |---|---|---|
 | 1 | What does "setup SFTP polling" mean given SP16 exists? | "Just turn it on" — small enablement increment, no from-scratch design. |
 | 2 | Real MFT or stub? | Real Gainwell MFT. |
 | 3 | macOS Keychain vs Docker secret file vs plain env var? | Plain env var (`CYCLONE_SFTP_PASSWORD`). User explicitly chose this. |
 | 4 | Runbook placement? | New `docs/RUNBOOK.md`. |
 | 5 | Hot-reload of the scheduler on PATCH, or restart required? | Hot-reload — the next tick uses the new block. |
 ## 11. Open questions still pending
 None. Ready to implement.