diff --git a/src/queue-intelligence.ts b/src/queue-intelligence.ts index 0f3195b55a..67838da668 100644 --- a/src/queue-intelligence.ts +++ b/src/queue-intelligence.ts @@ -171,11 +171,39 @@ export async function analyzePRQueue( * loopover itself), which the MCP read/actuation allowlists' analogous risk doesn't share. * LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS is the env var; config-as-code, never hardcoded per #config-as-code. */ export function isPublicScoreTermSafeForRepo(env: { LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS?: string }, repoFullName: string): boolean { - const entries = (env.LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS ?? "") + return parsePublicScoreTermAllowedRepos(env.LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS).includes(repoFullName.toLowerCase()); +} + +/** The parsed, lowercased allowlist entries. Takes the RAW env string (not an env object) so both callers can + * share it regardless of how their own `env` parameter is typed — `exactOptionalPropertyTypes` forbids + * re-wrapping a `string | undefined` back into an optional property. */ +function parsePublicScoreTermAllowedRepos(raw: string | undefined): string[] { + return (raw ?? "") .split(/[\s,]+/) .map((entry) => entry.trim().toLowerCase()) .filter(Boolean); - return entries.includes(repoFullName.toLowerCase()); +} + +/** + * True when the bare-score exemption allowlist is EMPTY — i.e. the over-broad `\bscore\w*\b` check is enforced + * for every repo, so any AI review narrative using the ordinary word "score"/"scoring" has its WHOLE assessment + * discarded in favour of the generic "did not include a separate narrative summary" placeholder. + * + * This exists because the #public-score-terms-scoping fix is CONFIG-DEPENDENT: the code path shipped, but it is + * inert until an operator populates LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS. Unset (the shipped default) the + * deployment silently behaves exactly like the pre-fix build that metagraphed#8038 was filed against, with no + * signal anywhere -- which is how that bug stayed live and "recurring" long after it was considered fixed. A + * unit test cannot catch this: it verifies the mechanism works WHEN enabled, never that a production env var + * was actually set. Warn-only, and deliberately not a hard boot failure: an empty allowlist is the correct, + * safe configuration for a deployment whose repos really do carry private trust/reward data -- the operator + * just needs to make that an informed choice rather than an unnoticed default. Pure predicate; the caller + * (server.ts) owns the actual shout, mirroring shouldWarnRagEmbedUnavailable. + */ +export function shouldWarnPublicScoreTermsAllowlistUnset(env: Record): boolean { + // Same `Record` shape as shouldWarnRagEmbedUnavailable so this accepts + // `process.env` directly at the boot-time call site (an optional-only object type does not structurally + // match Node's ProcessEnv). + return parsePublicScoreTermAllowedRepos(env.LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS).length === 0; } /** `allowBareScoreTerm` (#public-score-terms-scoping, default false = today's unchanged behavior): the bare diff --git a/src/server.ts b/src/server.ts index 462b17691a..65681fc415 100644 --- a/src/server.ts +++ b/src/server.ts @@ -31,6 +31,7 @@ import { subscriptionCliEnv, withAiGenerationCapture, } from "./selfhost/ai"; +import { shouldWarnPublicScoreTermsAllowlistUnset } from "./queue-intelligence"; import { cookieValue, credentialsToEnv, @@ -612,6 +613,22 @@ async function main(): Promise { }), ); } + // #public-score-terms-scoping: the bare-`score` exemption is inert until an operator populates the repo + // allowlist, and unset (the shipped default) every AI review narrative that merely uses the word + // "score"/"scoring" is silently replaced by the generic "did not include a separate narrative summary" + // placeholder -- the exact metagraphed#8038 behaviour the exemption was written to fix. Shout once at boot so + // a config-dependent fix can't sit inert and unnoticed the way that one did. Warn-only: an empty allowlist is + // the correct setting for a deployment whose repos really do carry private trust/reward data. + if (shouldWarnPublicScoreTermsAllowlistUnset(process.env)) { + console.error( + JSON.stringify({ + level: "error", + event: "selfhost_public_score_terms_allowlist_unset", + message: + "LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS is unset, so the bare-\"score\" public-comment check is enforced for every repo. Any AI review whose narrative uses the ordinary word \"score\"/\"scoring\" will have its WHOLE summary dropped and replaced by the generic no-narrative placeholder. Set it to the repos whose own public schema legitimately uses score vocabulary (e.g. JSONbored/metagraphed). Explicit phrases (\"trust score\", \"reward\", wallet/hotkey/seed phrase, ...) stay blocked everywhere regardless.", + }), + ); + } // Dedicated RAG embed provider (keeps the review chain frontier-only): when AI_EMBED_BASE_URL is set, embeddings // route to a SEPARATE openai-compatible endpoint (e.g. ollama at http://ollama:11434/v1, model bge-m3) instead of // the review chain — so a Claude/Codex outage never falls reviews back to a weak local model. Unset ⇒ absent ⇒ diff --git a/test/unit/queue-intelligence.test.ts b/test/unit/queue-intelligence.test.ts index f58f4656d3..29c5a4c63f 100644 --- a/test/unit/queue-intelligence.test.ts +++ b/test/unit/queue-intelligence.test.ts @@ -3,6 +3,7 @@ import { analyzePRQueue, generatePublicComment, isPublicScoreTermSafeForRepo, + shouldWarnPublicScoreTermsAllowlistUnset, sanitizePublicComment, FORBIDDEN_PUBLIC_COMMENT_WORDS, } from "../../src/queue-intelligence"; @@ -422,3 +423,36 @@ describe("isPublicScoreTermSafeForRepo (#public-score-terms-scoping)", () => { expect(isPublicScoreTermSafeForRepo(env, "JSONbored/loopover")).toBe(false); }); }); + +// The #public-score-terms-scoping fix is CONFIG-DEPENDENT: the code path shipped, but stays inert until an +// operator sets the env var, and unset the deployment behaves exactly like the pre-fix build metagraphed#8038 +// was filed against — every AI review narrative using the ordinary word "score"/"scoring" silently loses its +// WHOLE summary, with no signal anywhere. No unit test can assert a production env var was set, so the boot +// path shouts instead; these pin the predicate that drives that shout. +describe("shouldWarnPublicScoreTermsAllowlistUnset", () => { + it("warns when the allowlist is unset, empty, or only separators (the inert-fix states)", () => { + expect(shouldWarnPublicScoreTermsAllowlistUnset({})).toBe(true); + expect(shouldWarnPublicScoreTermsAllowlistUnset({ LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS: "" })).toBe(true); + expect(shouldWarnPublicScoreTermsAllowlistUnset({ LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS: " " })).toBe(true); + expect(shouldWarnPublicScoreTermsAllowlistUnset({ LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS: " , , " })).toBe(true); + }); + + it("stays quiet once at least one repo is configured", () => { + expect(shouldWarnPublicScoreTermsAllowlistUnset({ LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS: "JSONbored/metagraphed" })).toBe(false); + expect( + shouldWarnPublicScoreTermsAllowlistUnset({ LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS: "JSONbored/loopover, JSONbored/metagraphed" }), + ).toBe(false); + }); + + // The warning and the exemption must agree on what "configured" means, or the shout could fire for a + // deployment that IS configured (noise) or stay silent for one that is not (the failure this exists to catch). + it("agrees with isPublicScoreTermSafeForRepo about what counts as configured", () => { + const env = { LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS: "JSONbored/metagraphed" }; + expect(shouldWarnPublicScoreTermsAllowlistUnset(env)).toBe(false); + expect(isPublicScoreTermSafeForRepo(env, "JSONbored/metagraphed")).toBe(true); + // "*" parses to a non-empty entry, so the warning is silent — but it grants NO repo the exemption. + const wildcard = { LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS: "*" }; + expect(shouldWarnPublicScoreTermsAllowlistUnset(wildcard)).toBe(false); + expect(isPublicScoreTermSafeForRepo(wildcard, "JSONbored/loopover")).toBe(false); + }); +});