Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions src/queue-intelligence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string | undefined>): boolean {
// Same `Record<string, string | undefined>` 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
Expand Down
17 changes: 17 additions & 0 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
subscriptionCliEnv,
withAiGenerationCapture,
} from "./selfhost/ai";
import { shouldWarnPublicScoreTermsAllowlistUnset } from "./queue-intelligence";
import {
cookieValue,
credentialsToEnv,
Expand Down Expand Up @@ -612,6 +613,22 @@ async function main(): Promise<void> {
}),
);
}
// #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 ⇒
Expand Down
34 changes: 34 additions & 0 deletions test/unit/queue-intelligence.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
analyzePRQueue,
generatePublicComment,
isPublicScoreTermSafeForRepo,
shouldWarnPublicScoreTermsAllowlistUnset,
sanitizePublicComment,
FORBIDDEN_PUBLIC_COMMENT_WORDS,
} from "../../src/queue-intelligence";
Expand Down Expand Up @@ -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);
});
});