diff --git a/CI_AUTH_SAMPLER_PLAN.md b/CI_AUTH_SAMPLER_PLAN.md new file mode 100644 index 0000000..29e7191 --- /dev/null +++ b/CI_AUTH_SAMPLER_PLAN.md @@ -0,0 +1,890 @@ +# CI Auth + Liquid Sampler — consolidated findings & implementation plan + +Single source of truth, merged from three sessions (CI split-auth handoff `5121bb47`, +the sampler-results session, and the memory `project_ci_auth_final_design`). Supersedes +`HANDOFF_ci_sampler.md`. + +Status as of **2026-07-01**: both designs validated end-to-end in the `be_market` TEST +harness. Remaining work is (a) resolve the open questions, (b) productionise, (c) port the +proven workflows into the shared `bso_github_actions` repo. + +**Status as of 2026-07-22 (see §10 for full detail): production build is live/near-live in +two pilots (`nl_market`, `lu_market`), plus a v2 of the compact-diff format (`silverfin-cli#265`) +being feedback-tested against both before merge. This is now the fastest-moving section — read +§10 first if you're picking this back up.** + +--- + +## 0. TL;DR + +Two CI use cases, **two deliberately different auth solutions** (not one shared mechanism): + +| | Liquid **tests** | Liquid **sampler** | +|---|---|---| +| Problem | write-contention: per-user OAuth refresh token rotates on every refresh; 8 people racing invalidates it | single-version staging: read-only impossible; ~30–60 min runs; one run per partner | +| Solution | **decouple auth**: `*/10` cron is the sole token writer; test jobs run **read-only + parallel** (refreshToken stripped) → physically can't poison | **serialize**: `concurrency: { group, queue: max }`, own `PARTNER_CONFIG_JSON` secret, sole writer does load→run→write-back | +| Queue? | No — read-only, worst case re-run a job | Yes — FIFO, never cancel | +| Status | **M2 fully validated** | **M3 fully validated on staging** | + +--- + +## 1. Liquid tests (M2) — DONE, validated + +**Design.** Firm OAuth refresh tokens rotate on every refresh, so concurrent refreshes +poison each other. Fix = separate the *writer* from the *readers*: + +- **Sole writer:** a `*/10` cron in each market caller refreshes tokens and is the only job + that writes the config secret. (`bso_github_actions/refresh_token.yml` reusable, adopting + PR #21's partner loop.) Cadence `*/10` chosen over `*/5`/`*/15`: TTL = 2h, GitHub scheduler + lag measured up to **118 min**. +- **Readers:** test jobs strip `refreshToken` via `jq` and run read-only + parallel (PR #27). + A 401 sends an empty refresh token → Silverfin rejects → no rotation → no poison. +- **Merge order:** PR #27 (read-only) merges FIRST, then the read-only/cron change rebases on top. + +**Validated** in `be_market` via inlined `refresh_token_TEST.yml` + `run_tests_TEST.yml` +(CONFIG_JSON_TEST refreshed for firm 1355): +- refresh-writes-secret ✅ +- read-only stripped config ✅ +- 5× concurrency, no cancel ✅ +- multi-handle long job ✅ +- **mid-flight refresh survival** ✅ (held access token kept working ~76 s after a rotation) → no-poison confirmed + +--- + +## 2. Liquid sampler (M3) — validated on staging, this is the active work + +### 2.1 What the sampler is +CLI `run-sampler` (silverfin-cli PR #236) posts partner templates to the `liquid_sampler` +backend, which runs them across sample company files and returns a downloadable diff artifact. +It is a **human-review artifact, not a pass/fail gate** (see 2.4e). + +### 2.2 Auth & environment (all confirmed on staging) +- Backend lives on **staging only** — `bso-staging-beta.staging.getsilverfin.com`. + Prod test partner 84 returned **HTTP 503** → sampler backend is NOT on production. +- **Partner-scoped, single auth per run.** `createSamplerRun(partnerId, …)` uses the partner + api_key from stored partner credentials. One `-p` = one partner = one auth. +- **`--firm-ids` is MANDATORY** — empty → `422 {"firm_ids":["can't be blank"]}`. Not stored in + the repo; the workflow must supply them (currently hard-coded `469 106 1592`; source from a + repo variable/config for production). +- **Staging sits behind an HTTP Basic gateway** → needs `SF_BASIC_AUTH` (base64). The CLI + auto-detects staging (`/staging\.getsilverfin/`) and only then adds `Authorization: Basic …`. + Off staging it's a no-op. (This is the `fix-staging-oauth-basic-auth` branch / PR #257.) +- **Host** comes from `firmCredentials.getHost()`, overridable by `SF_HOST` env var. +- Partner api_key is a single rotate-on-401 credential (can't be stripped like firm tokens) → + it lives in its OWN `PARTNER_CONFIG_JSON` secret; the serialized sampler job is sole writer + (load → run → write-back). Firm `CONFIG_JSON` is never touched. + +### 2.3 Concurrency / queue design +- `concurrency: { group: sampler-staging, queue: max }` → FIFO, **nothing cancelled**. +- Aligns with a **server-side** constraint: the backend enforces one run per partner; a 2nd + overlapping run gets `422 {"base":["A sampler run is already in progress for this partner"]}`. +- `queue: max` is a **GitHub beta** sub-key. Only the sampler depends on it and **no token + safety rides on it**. Fallback: `queue: single` (1 running + 1 pending) or plain + `cancel-in-progress: false`. +- **Never cancel an in-flight sampler job.** Cancelling kills the CLI but NOT the backend run — + it keeps running (~30 min) and BLOCKS the partner (422) until it clears. There is no CLI + cancel for a backend run. + +### 2.4 Backend behavior learnings (important for the production job) +- (a) One sampler run per partner at a time (422 as above) — server-side. +- (b) Cancelling CI ≠ cancelling backend; leaves partner blocked. +- (c) A run can leave a stale in-progress state for a while; no CLI cancel exists. +- (d) `run-sampler` **exits 0 even on 422/failed** → the harness must grep output (ours does). +- (e) **Broken export does NOT fail the run.** Break-detection test (2026-07-01): broke `vkt_1` + (removed an include) + `liquidation_reserve` (deleted 144 lines), sampled both → run reported + "completed successfully", job GREEN, despite the breaks. Per-template errors/diffs live INSIDE + the result artifact. **Grepping for "completed successfully" is NOT a quality gate.** + +### 2.5 This session's additions — result accessibility (DONE, validated) +Goal: make the run's results reachable in **one click**, no HTML pasting, no manual command. + +**CLI** — branch `silverfin-cli@agustin-bso-sampler-easy-results` (pushed): +- Always logs `Sampler report: ` on completion (was swallowed — old code only + `openFile()`'d it, so in CI the URL vanished). `lib/liquidSamplerRunner.js`. +- Only download+open locally when **not** in CI; added `--no-open`. Constructor takes + `{ openReport }`, defaulting to `!process.env.CI`. +- **Spinner suppressed in CI** — static poll line instead of the animated spinner, which had + flooded a captured log with ~23 min of frames (thousands of lines). `bin/cli.js` passes + `openReport: options.open && !process.env.CI`. +- Unit test `tests/lib/liquidSamplerRunner.test.js` (5/5 pass). + +**Workflow** — `be_market@test-ci-auth-revalidate:.github/workflows/run_sampler_TEST.yml`: +- Installs the CLI from `#agustin-bso-sampler-easy-results`. +- On completion, extracts the URL and writes a clickable **📊 Open sampler report** link to + `$GITHUB_STEP_SUMMARY`. Extraction: + `grep -oE 'Sampler report: https?://[^[:space:]]+'` (anchors on the line regardless of + consola's `[success]`/`✔` prefix; the presigned URL has no `)` so it's markdown-safe). + +**Verified** run `28514806782` (2026-07-01, GREEN, 32m43s): +- Installed the right branch ✅; staging host + auth ✅; spinner gone ✅; + `[success] Sampler report: https://storage.googleapis.com/…` printed ✅; verdict `completed` ✅. +- **`result_url` is a presigned GCS link to `results.zip`** (`content-type: application/zip`, + `filename="results.zip"`), NOT a rendered HTML page — clicking downloads a zip. Long/far-future + `Expires`, so the summary link stays valid well after the run. + +### 2.6 Multi-partner changesets (design note, from this session) +A changed template can be mapped to **several** partners (its `config.json` `partner_id` map has +multiple keys). One `run-sampler` call handles exactly one partner and **hard-fails +(`process.exit(1)`) on the first template with no entry for that `-p`** — it does not skip. +So for a changeset spanning partners: **group changed templates by partner, run once per partner** +(matrix), each with its own stored partner credentials. Recommended to do the grouping in the +workflow, keeping the CLI single-partner. + +### 2.7 The result artifact + LLM-assisted review WITHOUT context blowup (this session) +**Decision (2026-07-01): the sampler is a human-review artifact, not a pass/fail gate** (consistent +with 2.4e). The reviewer inspects the diff, optionally with LLM help. + +**`results.zip` contents** (inspected a real one): +``` +results/ + liquid_comparison_report.html # the diff SUMMARY (1.9 KB when 0 changed; 1.84 MB at 32 changed) + sample_entry_ids.yml # the sampled entries + output/{account,reconciliation}_entries//{before,after}/{rendered_text.md,*.liquid,*.json,*.html} # 154 MB raw +``` +The HTML report inlines full diff tables **with unchanged context rows + ~5 lines of markup per +line** → at 32 changed templates it is **1.84 MB ≈ 470K tokens**. Pasting it into a chat is what +blew up the earlier session. **Humans open the HTML in a browser; LLMs must get an extracted view.** + +**Measured reduction (on the 32-changed / 230-sample run):** + +| Representation | Size | ~Tokens | Method | +|---|---|---|---| +| Raw report HTML | 1.84 MB | ~470K | 💥 never send to an LLM | +| Compact diff (+/- lines only, tags stripped) | 363 KB | ~93K | drop unchanged rows + HTML markup | +| Normalized + deduped | 180 KB | ~45K | strip per-company ids/values → 32 items → 19 patterns | +| `named_results` only (root-cause data) | 3.6 KB | ~900 | just the data diff (the `"" → null`) | + +Only two files ever change per entry: `named_results` (data) and `rendered_text.md` (rendered +effect). `named_results` is the compact root cause (e.g. `"street_var": "" → null`); +`rendered_text.md` is the large downstream noise (rendered widgets: ``, `