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
23 changes: 21 additions & 2 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ import { runGittensoryAiSlopAdvisory } from "../services/ai-slop";
import { decidePublicSurface } from "../signals/settings-preview";
import {
buildFocusManifestGuidance,
composeRepoReviewContext,
excludeReviewPaths,
resolveReviewPathInstructions,
resolveReviewPreMergeChecks,
Expand All @@ -287,7 +288,10 @@ import {
type ReviewPathInstruction,
type ReviewProfile,
} from "../signals/focus-manifest";
import { loadRepoFocusManifest } from "../signals/focus-manifest-loader";
import {
loadRepoFocusManifest,
loadRepoReviewContext,
} from "../signals/focus-manifest-loader";
import { resolveRepositorySettings } from "../settings/repository-settings";
import type { LocalBranchAnalysisInput } from "../signals/local-branch";
import {
Expand Down Expand Up @@ -4471,7 +4475,7 @@ async function maybePublishPrPublicSurface(
profile: reviewProfile,
inlineComments: reviewInlineComments,
pathInstructions: reviewPathInstructions,
instructions: reviewInstructions,
instructions: manifestReviewInstructions,
excludePaths: reviewExcludePaths,
} = resolveReviewPromptOverrides(
await loadRepoFocusManifest(env, repoFullName).catch(() => null),
Expand All @@ -4481,6 +4485,21 @@ async function maybePublishPrPublicSurface(
repoFullName,
reviewInlineComments,
);
// Per-repo review CONTEXT (#review-skills): fold the container-private review/CLAUDE.md guide + the matching
// review/skills/*.md modules into the SAME review-instructions slot, so reviews follow each repo's conventions.
// Glob-gated for cost (only skills matching the changed files are injected); absent config dir ⇒ empty ⇒
// byte-identical prompt. getReviewFiles() is memoized, so the second call reuses the loaded diff.
const reviewInstructions =
[
manifestReviewInstructions,
composeRepoReviewContext(
await loadRepoReviewContext(repoFullName),
(await getReviewFiles()).map((file) => file.path),
),
]
.map((part) => part?.trim())
.filter(Boolean)
.join("\n\n") || null;
aiReview = await runAiReviewForAdvisory(env, {
settings,
advisory,
Expand Down
69 changes: 67 additions & 2 deletions src/selfhost/private-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,16 @@
// `.yaml` / `.json` are accepted everywhere `.yml` is. The first existing candidate wins outright (a present
// per-repo file fully REPLACES the global fallback — "fallback" means "used only when no per-repo file exists",
// not a deep merge). The slug is lowercased (GitHub repo full-names are case-insensitive; #1390 already lowercased).
import { readFile } from "node:fs/promises";
import { readFile, readdir } from "node:fs/promises";
import { join, resolve } from "node:path";
import type { RepoFocusManifestFetcher } from "../signals/focus-manifest-loader";
import type {
RepoReviewContext,
RepoReviewSkill,
} from "../signals/focus-manifest";
import type {
RepoFocusManifestFetcher,
RepoReviewContextReader,
} from "../signals/focus-manifest-loader";

/** The bare config filenames tried inside a per-repo folder and at the dir root (global fallback), in priority order. */
const CONFIG_BASENAMES = [".gittensory.yml", ".gittensory.yaml", ".gittensory.json"] as const;
Expand Down Expand Up @@ -76,3 +83,61 @@ export function makeLocalManifestReader(dir: string | undefined): RepoFocusManif
return null;
};
}

/** Per-repo review-context candidate FOLDERS (relative to GITTENSORY_REPO_CONFIG_DIR): `{owner}__{repo}/review` then
* `{repo}/review`. Same owner/repo validation as localConfigCandidates; an invalid full name yields none. (#review-skills) */
function reviewContextFolders(repoFullName: string): string[] {
const slash = repoFullName.indexOf("/");
if (slash <= 0 || slash === repoFullName.length - 1 || slash !== repoFullName.lastIndexOf("/")) return [];
const owner = repoFullName.slice(0, slash).toLowerCase();
const repo = repoFullName.slice(slash + 1).toLowerCase();
if (!GITHUB_OWNER_SEGMENT.test(owner) || !isSafeRepoSegment(repo)) return [];
return [join(`${owner}__${repo}`, "review"), join(repo, "review")];
}

/** Parse a skill markdown file into {name, when, body}. YAML frontmatter (`---\nname:\nwhen:\n---`) is optional; name
* defaults to the filename and `when` to "always". */
export function parseReviewSkill(filename: string, text: string): RepoReviewSkill {
const fm = /^---\s*\n([\s\S]*?)\n---\s*\n?([\s\S]*)$/.exec(text);
const head = fm?.[1] ?? "";
const body = (fm?.[2] ?? text).trim();
const name = /(?:^|\n)name:\s*(.+)/.exec(head)?.[1]?.trim() || filename.replace(/\.md$/i, "");
const whenRaw = /(?:^|\n)when:\s*(.+)/.exec(head)?.[1]?.trim();
const when = (whenRaw ?? "always").replace(/^["']|["']$/g, "") || "always";
return { name, when, body };
}

/** Build the container-local review-context reader over GITTENSORY_REPO_CONFIG_DIR, or null when the dir is unset. Per
* repo (first existing folder wins) reads `review/CLAUDE.md` (the guide) + every `review/skills/*.md` (rubric modules,
* sorted). Missing files/dir degrade to nulls/empty; a per-file read error skips that file. (#review-skills) */
export function makeLocalReviewContextReader(dir: string | undefined): RepoReviewContextReader | null {
const trimmed = (dir ?? "").trim();
if (!trimmed) return null;
const base = resolve(trimmed);
return async (repoFullName: string): Promise<RepoReviewContext> => {
for (const folder of reviewContextFolders(repoFullName)) {
const abs = resolve(base, folder);
let guide: string | null = null;
try {
guide = await readFile(resolve(abs, "CLAUDE.md"), "utf8");
} catch {
// no per-repo review guide
}
const skills: RepoReviewSkill[] = [];
try {
const entries = (await readdir(resolve(abs, "skills"))).filter((f) => f.toLowerCase().endsWith(".md")).sort();
for (const f of entries) {
try {
skills.push(parseReviewSkill(f, await readFile(resolve(abs, "skills", f), "utf8")));
} catch {
// unreadable skill file → skip it
}
}
} catch {
// no skills/ dir
}
if (guide !== null || skills.length > 0) return { guide, skills };
}
return { guide: null, skills: [] };
};
}
15 changes: 13 additions & 2 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,15 @@ import { createPgVectorize, initPgVectorize } from "./selfhost/pg-vectorize";
import { createSqliteQueue } from "./selfhost/sqlite-queue";
import { createSqliteVectorize } from "./selfhost/vectorize";
import { createFsBlobStore } from "./selfhost/blob-store";
import { makeLocalManifestReader } from "./selfhost/private-config";
import {
makeLocalManifestReader,
makeLocalReviewContextReader,
} from "./selfhost/private-config";
import { captureError, flushSentry, initSentry } from "./selfhost/sentry";
import { setLocalManifestReader } from "./signals/focus-manifest-loader";
import {
setLocalManifestReader,
setLocalReviewContextReader,
} from "./signals/focus-manifest-loader";
import type { JobMessage } from "./types";

/** Resolve `<NAME>_FILE` env vars (Docker secrets / multi-line keys) into `<NAME>` at startup. */
Expand Down Expand Up @@ -225,6 +231,11 @@ async function main(): Promise<void> {
setLocalManifestReader(
makeLocalManifestReader(process.env.GITTENSORY_REPO_CONFIG_DIR),
);
// Per-repo review CONTEXT (#review-skills): the same config dir also holds `<repo>/review/CLAUDE.md` + skills/*.md,
// injected into the reviewer prompt so reviews follow each repo's conventions. Unset dir ⇒ null reader ⇒ no change.
setLocalReviewContextReader(
makeLocalReviewContextReader(process.env.GITTENSORY_REPO_CONFIG_DIR),
);
// Error tracking (#1468): opt-in via SENTRY_DSN — a complete no-op when unset. When on, capture uncaught crashes
// + unhandled rejections (flush before exit for the fatal case); per-subsystem captures (queue dead-letter,
// review failures) are wired at their sites.
Expand Down
31 changes: 30 additions & 1 deletion src/signals/focus-manifest-loader.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { listSignalSnapshots, persistSignalSnapshot } from "../db/repositories";
import type { JsonValue } from "../types";
import { nowIso } from "../utils/json";
import { featuresConfigToJson, gateConfigToJson, MAX_FOCUS_MANIFEST_BYTES, parseFocusManifest, parseFocusManifestContent, reviewConfigToJson, settingsOverrideToJson, type FocusManifest, type FocusManifestSource } from "./focus-manifest";
import { featuresConfigToJson, gateConfigToJson, MAX_FOCUS_MANIFEST_BYTES, parseFocusManifest, parseFocusManifestContent, reviewConfigToJson, settingsOverrideToJson, type FocusManifest, type FocusManifestSource, type RepoReviewContext } from "./focus-manifest";
import { GITTENSORY_REPO_FOCUS_MANIFEST_YAML, resolveGittensorySelfRepoFullName } from "../config/gittensory-repo-focus-manifest";

export const REPO_FOCUS_MANIFEST_SIGNAL = "repo-focus-manifest";
Expand Down Expand Up @@ -33,6 +33,35 @@ export function setLocalManifestReader(reader: RepoFocusManifestFetcher | null):
localManifestReader = reader;
}

/**
* Async source for a repo's review CONTEXT (#review-skills): the `review/CLAUDE.md` guide + `review/skills/*.md` rubric
* modules from the container-private config dir. Registered once at boot by the Node entry (server.ts); the filesystem
* access lives inside that injected closure, keeping THIS module Workers-safe. Unset (cloud, or a self-host without the
* dir) ⇒ the loader returns an empty context and the reviewer prompt is byte-identical.
*/
export type RepoReviewContextReader = (
repoFullName: string,
) => Promise<RepoReviewContext>;
let localReviewContextReader: RepoReviewContextReader | null = null;
export function setLocalReviewContextReader(
reader: RepoReviewContextReader | null,
): void {
localReviewContextReader = reader;
}

/** Load the per-repo review context via the registered reader. Local file reads are cheap, so this is NOT cached.
* Unset reader ⇒ empty context; a read error degrades to empty (the reviewer prompt stays byte-identical). */
export async function loadRepoReviewContext(
repoFullName: string,
): Promise<RepoReviewContext> {
if (!localReviewContextReader) return { guide: null, skills: [] };
try {
return await localReviewContextReader(repoFullName);
} catch {
return { guide: null, skills: [] };
}
}

/**
* Fetch a maintainer-owned manifest file from the public GitHub raw endpoint. Network or HTTP
* failures resolve to null so the loader falls back to deterministic signals.
Expand Down
48 changes: 48 additions & 0 deletions src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,54 @@ export function resolveReviewPreMergeChecks(manifest: FocusManifest | null): Pre
return manifest?.review.preMergeChecks ?? [];
}

/** One per-repo review SKILL (#review-skills): a maintainer-maintained rubric module loaded from the container-private
* config dir (`<repo>/review/skills/*.md`). `when` is "always" (repo-wide) or a path glob / brace-list that gates it to
* matching changed files (cost: only relevant skills are injected). */
export type RepoReviewSkill = { name: string; when: string; body: string };
/** The per-repo review CONTEXT (#review-skills): an always-on `review/CLAUDE.md` guide + the skill rubric modules. */
export type RepoReviewContext = { guide: string | null; skills: RepoReviewSkill[] };

/** Hard cap on the injected per-repo review context — a cost guard so a runaway guide/skills set can't bloat every
* prompt. The maintained files are concise by design; this only bites pathological inputs. */
const MAX_REVIEW_CONTEXT_CHARS = 16_000;

/** True when a skill's `when` applies to this PR: "always"/empty ⇒ yes; otherwise the (possibly brace-listed) glob must
* match at least one changed path. Reuses the manifest path matcher so it behaves exactly like path_instructions. */
function reviewSkillApplies(when: string, changedPaths: string[]): boolean {
const w = when.trim();
if (!w || w.toLowerCase() === "always") return true;
const patterns = w
.replace(/^\{|\}$/g, "")
.split(",")
.map((p) => p.trim())
.filter(Boolean);
return patterns.some((pat) =>
changedPaths.some((path) => matchesManifestPath(path, pat)),
);
}

/** Compose the per-repo review context into a prompt section (#review-skills): the always-on guide + every skill whose
* `when` applies to this PR's changed files. Bounded for cost. Null/empty ⇒ "" (byte-identical reviewer prompt). The
* caller folds the result into the `review.instructions` slot, so it inherits the same prompt wrapper + public-safe
* handling. */
export function composeRepoReviewContext(
context: RepoReviewContext | null,
changedPaths: string[],
): string {
if (!context) return "";
const parts: string[] = [];
if (context.guide?.trim()) parts.push(context.guide.trim());
for (const skill of context.skills) {
if (reviewSkillApplies(skill.when, changedPaths) && skill.body.trim())
parts.push(`## skill: ${skill.name}\n${skill.body.trim()}`);
}
if (parts.length === 0) return "";
const joined = parts.join("\n\n");
return joined.length > MAX_REVIEW_CONTEXT_CHARS
? joined.slice(0, MAX_REVIEW_CONTEXT_CHARS)
: joined;
}

/** Filter a PR's changed files down to the set the AI review should see — dropping any whose path matches a
* `review.exclude_paths` glob (generated/vendored/lockfiles). Empty `excludePaths` ⇒ the same array (byte-identical
* review). Pure; the gate/slop/secret-scan operate on the unfiltered files. (#review-exclude-paths) */
Expand Down
43 changes: 43 additions & 0 deletions test/unit/focus-manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
excludeReviewPaths,
resolveReviewPathInstructions,
resolveReviewPreMergeChecks,
composeRepoReviewContext,
resolveReviewPromptOverrides,
reviewConfigToJson,
settingsOverrideToJson,
Expand Down Expand Up @@ -1365,3 +1366,45 @@ describe("review.pre_merge_checks (#review-pre-merge-checks)", () => {
expect(resolveReviewPreMergeChecks(null)).toEqual([]);
});
});

describe("composeRepoReviewContext (#review-skills)", () => {
it("returns '' for null/empty/whitespace-only context", () => {
expect(composeRepoReviewContext(null, ["a.ts"])).toBe("");
expect(composeRepoReviewContext({ guide: null, skills: [] }, ["a.ts"])).toBe("");
expect(composeRepoReviewContext({ guide: " ", skills: [] }, ["a.ts"])).toBe("");
});

it("includes the guide + always/blank-when/glob-matched skills, excluding non-matching ones", () => {
const ctx = {
guide: "Review THIS repo carefully.",
skills: [
{ name: "voice", when: "always", body: "Be decisive." },
{ name: "blank", when: "", body: "Blank-when is always-on." },
{ name: "sql", when: "**/*.sql", body: "Check the index usage." },
{ name: "schema", when: "{**/db/schema.ts,**/*.sql}", body: "Migration parity." },
{ name: "ui", when: "app/**", body: "Should not appear." },
],
};
const out = composeRepoReviewContext(ctx, ["migrations/0079_x.sql"]);
expect(out).toContain("Review THIS repo carefully.");
expect(out).toContain("## skill: voice");
expect(out).toContain("## skill: blank");
expect(out).toContain("## skill: sql"); // **/*.sql matched the .sql file
expect(out).toContain("## skill: schema"); // brace-list matched the .sql file
expect(out).not.toContain("## skill: ui"); // app/** did not match
expect(out).not.toContain("Should not appear.");
});

it("drops empty-body and non-matching skills (⇒ '' when nothing applies)", () => {
const out = composeRepoReviewContext(
{ guide: null, skills: [{ name: "empty", when: "always", body: " " }, { name: "x", when: "src/**", body: "nope" }] },
["README.md"],
);
expect(out).toBe("");
});

it("bounds the injected context to the cost cap", () => {
const out = composeRepoReviewContext({ guide: "x".repeat(20_000), skills: [] }, []);
expect(out.length).toBeLessThanOrEqual(16_000);
});
});
Loading
Loading