|
| 1 | +// #9433: config-dependent fixes that ship INERT. |
| 2 | +// |
| 3 | +// Several production-behaviour fixes are gated on env vars that were never set, so the shipped code path |
| 4 | +// stayed inert and the deployment silently behaved exactly like the pre-fix build. No unit test can catch this |
| 5 | +// class: tests verify the mechanism works WHEN ENABLED, never that an operator set the variable. The confirmed |
| 6 | +// instance (#9433) — `LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS` unset, so every AI review narrative |
| 7 | +// containing the ordinary word "score" had its whole summary silently replaced by a placeholder — sat live for |
| 8 | +// weeks with a green test suite. |
| 9 | +// |
| 10 | +// WHY THIS IS A REPORT, NOT MORE BOOT WARNINGS. The obvious fix — one `console.error` per var, mirroring |
| 11 | +// `shouldWarnRagEmbedUnavailable` — does not generalize, and following it blindly makes things worse. Verified |
| 12 | +// while writing this: `LOOPOVER_PUBLIC_STATS_REPOS` is unset on the self-host box and that is CORRECT, because |
| 13 | +// the public-stats surface is served by the Cloudflare Worker (where wrangler.jsonc sets it to four repos). |
| 14 | +// A boot warning there would fire on every self-host deployment forever, for a non-problem — the same alert |
| 15 | +// fatigue that let a genuinely broken backup alert be ignored for 8 days. A warning is only justified when |
| 16 | +// "unset" is wrong for EVERY deployment; that is rare, and each such case still earns its own dedicated |
| 17 | +// warning at its own call site (the two that exist today are correct and stay). |
| 18 | +// |
| 19 | +// What is missing is not more noise but ANSWERABILITY: an operator has no way to ask "which config-gated |
| 20 | +// behaviours are currently inert on this box?" without reading boot logs they have long since scrolled past. |
| 21 | +// This module answers exactly that question, on demand, with zero steady-state noise. |
| 22 | +// |
| 23 | +// SCOPE, deliberately narrow. This reports only vars whose unset state changes OUTPUT CORRECTNESS — review |
| 24 | +// content, gate disposition, or a published number. It does NOT report the ~100 `Default OFF` convergence |
| 25 | +// flags in env.d.ts: those are opt-in features whose absence keeps the review path byte-identical, so listing |
| 26 | +// them would bury the few entries that matter under a wall of working-as-intended noise. |
| 27 | + |
| 28 | +/** One config-gated behaviour that is currently inert. */ |
| 29 | +export type InertConfigEntry = { |
| 30 | + /** The env var an operator would set. */ |
| 31 | + key: string; |
| 32 | + /** What stops working while it is unset — phrased as the OBSERVABLE effect, not the mechanism. */ |
| 33 | + impact: string; |
| 34 | + /** |
| 35 | + * Whether an unset value is wrong for every deployment, or legitimately correct for some. |
| 36 | + * |
| 37 | + * `always-wrong` earns a boot warning too (and today's two both have one). `deployment-specific` must NOT |
| 38 | + * be warned about — it is exactly the `LOOPOVER_PUBLIC_STATS_REPOS` case above, where the same unset value |
| 39 | + * is correct on one runtime and a defect on another, and only the operator knows which they are running. |
| 40 | + */ |
| 41 | + severity: "always-wrong" | "deployment-specific"; |
| 42 | +}; |
| 43 | + |
| 44 | +/** Reads only the vars it names, so it is safe to call with `process.env` directly. */ |
| 45 | +export type InertConfigEnv = Record<string, string | undefined>; |
| 46 | + |
| 47 | +function unset(value: string | undefined): boolean { |
| 48 | + return (value ?? "").trim() === ""; |
| 49 | +} |
| 50 | + |
| 51 | +/** |
| 52 | + * Every config-gated behaviour currently inert in `env`, in a stable order. |
| 53 | + * |
| 54 | + * PURE — no IO, no clock, no logging — so the `/metrics` gauge, a `/ready` field, and a test can all read the |
| 55 | + * identical answer rather than three hand-maintained lists drifting apart (which is the same drift class this |
| 56 | + * whole issue is about). |
| 57 | + */ |
| 58 | +export function inertConfigEntries(env: InertConfigEnv): InertConfigEntry[] { |
| 59 | + const entries: InertConfigEntry[] = []; |
| 60 | + |
| 61 | + // The confirmed #9433 instance. Fail-closed by design, and the fail-closed direction is content-destroying: |
| 62 | + // sanitizePublicComment THROWS on a bare "score" match and the caller degrades to a generic placeholder, so |
| 63 | + // an unset allowlist silently strips narrative sentences on every repo. Correct for a deployment whose repos |
| 64 | + // genuinely carry private trust/reward data, hence deployment-specific rather than always-wrong. |
| 65 | + if (unset(env.LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS)) { |
| 66 | + entries.push({ |
| 67 | + key: "LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS", |
| 68 | + impact: |
| 69 | + "AI review narratives lose any sentence using ordinary scoring vocabulary (\"score\", \"ranking\", \"reward\", \"reviewability\"), silently — the published summary looks fine, just shorter.", |
| 70 | + severity: "deployment-specific", |
| 71 | + }); |
| 72 | + } |
| 73 | + |
| 74 | + // Verified unset on the ORB self-host box and CORRECT there: the public-stats surface runs on the Cloudflare |
| 75 | + // Worker, whose wrangler.jsonc sets it. Reported (never warned) precisely so an operator on a runtime that |
| 76 | + // DOES serve /v1/public/stats can see that the own-ledger half is publishing zeros. |
| 77 | + if (unset(env.LOOPOVER_PUBLIC_STATS_REPOS)) { |
| 78 | + entries.push({ |
| 79 | + key: "LOOPOVER_PUBLIC_STATS_REPOS", |
| 80 | + impact: |
| 81 | + "The own-ledger half of /v1/public/stats reports zero (disposition counts, reversal-grounded accuracy, weekly totals). Harmless on a runtime that does not serve public stats; silently wrong on one that does.", |
| 82 | + severity: "deployment-specific", |
| 83 | + }); |
| 84 | + } |
| 85 | + |
| 86 | + // Redaction is flag-gated while DETECTION is not (verified: reviewInputHasPromptInjection and its hold run |
| 87 | + // unconditionally), so an unset flag never lets a manipulated verdict through — it only means the reviewer |
| 88 | + // sees the raw injected text rather than a defanged copy. Reported because the inconclusive-finding copy |
| 89 | + // tells a reader the content "was redacted before review", which is untrue while this is off. |
| 90 | + if (unset(env.LOOPOVER_REVIEW_SAFETY)) { |
| 91 | + entries.push({ |
| 92 | + key: "LOOPOVER_REVIEW_SAFETY", |
| 93 | + impact: |
| 94 | + "Prompt-injection text is still DETECTED and still holds the PR, but is not defanged before the model sees it — and the public finding claims it was redacted.", |
| 95 | + severity: "deployment-specific", |
| 96 | + }); |
| 97 | + } |
| 98 | + |
| 99 | + return entries; |
| 100 | +} |
| 101 | + |
| 102 | +/** Stable, bounded label values for the `/metrics` gauge — the key set is fixed in code, so cardinality is |
| 103 | + * bounded by construction and an operator can alert on a specific key without a cardinality risk. */ |
| 104 | +export function inertConfigGaugeSamples(env: InertConfigEnv): Array<{ labels: Record<string, string>; value: number }> { |
| 105 | + return inertConfigEntries(env).map((entry) => ({ |
| 106 | + labels: { key: entry.key, severity: entry.severity }, |
| 107 | + value: 1, |
| 108 | + })); |
| 109 | +} |
0 commit comments