From 7d2000ec31a063b277223b9107877f52305f549e Mon Sep 17 00:00:00 2001 From: Nora Date: Wed, 24 Jun 2026 15:25:20 -0600 Subject: [PATCH] =?UTF-8?q?docs(spec):=20SP25=20SFTP=20Polling=20Enablemen?= =?UTF-8?q?t=20=E2=80=94=20initial=20design?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...-cyclone-sftp-polling-enablement-design.md | 181 ++++++++++++++++++ 1 file changed, 181 insertions(+) create mode 100644 docs/superpowers/specs/2026-06-24-cyclone-sftp-polling-enablement-design.md diff --git a/docs/superpowers/specs/2026-06-24-cyclone-sftp-polling-enablement-design.md b/docs/superpowers/specs/2026-06-24-cyclone-sftp-polling-enablement-design.md new file mode 100644 index 0000000..0bcad84 --- /dev/null +++ b/docs/superpowers/specs/2026-06-24-cyclone-sftp-polling-enablement-design.md @@ -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 `_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 `` 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 `_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. +- **`_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.