diff --git a/src/queue/processors.ts b/src/queue/processors.ts index fd81efb412..e4653c4ab4 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -3495,6 +3495,10 @@ export async function runAiReviewForAdvisory( // cached manifest. The CONFIG (not a fetch) is threaded in; the per-PR glob match against `files` happens // here (pure), so the AI path makes no extra manifest fetch. Absent/empty ⇒ byte-identical reviewer prompt. reviewPathInstructions?: ReviewPathInstruction[] | undefined; + // `.gittensory.yml` review.instructions (#review-instructions): a repo-level maintainer brief, resolved by the + // caller from the cached manifest, handed to the reviewer on EVERY review (bounded + public-safe at parse time). + // Absent/null ⇒ byte-identical reviewer prompt. + reviewInstructions?: string | null | undefined; // `.gittensory.yml` review.exclude_paths (#review-exclude-paths), resolved by the caller from the cached // manifest. Globs whose files are dropped from the AI review (diff + grounding + RAG) — generated/lockfiles // the maintainer doesn't want reviewed. Empty ⇒ every file is reviewed (byte-identical). The gate is unaffected. @@ -3677,6 +3681,7 @@ export async function runAiReviewForAdvisory( args.reviewPathInstructions ?? [], files.map((file) => file.path), ), + repoInstructions: args.reviewInstructions ?? null, }); if (result.status !== "ok") return undefined; const findings: AdvisoryFinding[] = []; @@ -4466,6 +4471,7 @@ async function maybePublishPrPublicSurface( profile: reviewProfile, inlineComments: reviewInlineComments, pathInstructions: reviewPathInstructions, + instructions: reviewInstructions, excludePaths: reviewExcludePaths, } = resolveReviewPromptOverrides( await loadRepoFocusManifest(env, repoFullName).catch(() => null), @@ -4485,6 +4491,7 @@ async function maybePublishPrPublicSurface( files: await getReviewFiles(), reviewProfile, reviewPathInstructions, + reviewInstructions, reviewExcludePaths, reviewInlineComments, }); diff --git a/src/services/ai-review.ts b/src/services/ai-review.ts index 5a2db059dc..90e78ff644 100644 --- a/src/services/ai-review.ts +++ b/src/services/ai-review.ts @@ -155,6 +155,12 @@ export type GittensoryAiReviewInput = { * instructions passed the manifest's public-safe filter at parse time). */ pathGuidance?: string | null | undefined; + /** + * `.gittensory.yml` `review.instructions` (#review-instructions) — a repo-level maintainer brief appended to EVERY + * review (vs the per-path pathGuidance). Bounded + public-safe at parse time, so it stays cost-cheap. Absent/null ⇒ + * the reviewer prompt is byte-identical. + */ + repoInstructions?: string | null | undefined; /** * `.gittensory.yml` `review.inline_comments` (#inline-comments) — when true (the caller has already ANDed the * operator flag + cutover allowlist + the per-repo manifest toggle), the reviewer is asked to ALSO emit an @@ -455,8 +461,13 @@ function buildSystemPrompt(input: GittensoryAiReviewInput): string { // `.gittensory.yml` review.path_instructions (#review-path-instructions): the caller pre-resolved the entries // matching this PR's files into a prompt section; empty ⇒ nothing appended (byte-identical). const pathSuffix = input.pathGuidance?.trim() ? input.pathGuidance : ""; + // `.gittensory.yml` review.instructions (#review-instructions): a repo-level maintainer brief appended to every + // review; empty ⇒ nothing appended (byte-identical). + const repoInstructionsSuffix = input.repoInstructions?.trim() + ? ` REPOSITORY REVIEW INSTRUCTIONS (maintainer conventions for this repo — honor them unless they conflict with a real defect): ${input.repoInstructions.trim()}` + : ""; const inlineSuffix = input.inlineFindings ? INLINE_FINDINGS_SUFFIX : ""; - return `${REVIEW_SYSTEM_PROMPT}${groundingSuffix}${enrichmentSuffix}${profileSuffix}${pathSuffix}${inlineSuffix}`; + return `${REVIEW_SYSTEM_PROMPT}${groundingSuffix}${enrichmentSuffix}${profileSuffix}${pathSuffix}${repoInstructionsSuffix}${inlineSuffix}`; } /** One Workers-AI opinion with a per-slot reliable fallback and a 3× retry on the primary. */ diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index 53ad21103a..8d06eeee50 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -130,6 +130,11 @@ export type FocusManifestReviewConfig = { /** `review.path_instructions`: per-path natural-language guidance handed to the AI reviewer when the PR's * changed files match the glob. Empty (default) ⇒ byte-identical reviewer prompt. (#review-path-instructions) */ pathInstructions: ReviewPathInstruction[]; + /** `review.instructions`: a repo-level natural-language brief handed to the AI reviewer on EVERY review (vs the + * per-path path_instructions) — the maintainer's conventions/voice for this repo. Bounded + public-safe at parse + * time (so it stays cost-cheap, unlike ingesting a whole CLAUDE.md). null (default, absent) ⇒ byte-identical + * reviewer prompt. (#review-instructions) */ + instructions: string | null; /** `review.exclude_paths`: globs whose matching files are EXCLUDED from the AI review (diff + grounding + RAG) * — generated/vendored/lockfiles the maintainer doesn't want reviewed. Empty (default) ⇒ every file is * reviewed (byte-identical). Gate/slop/secret-scan are UNAFFECTED — this only narrows the AI review. @@ -266,7 +271,7 @@ const EMPTY_MANIFEST: FocusManifest = { publicNotes: [], gate: { ...EMPTY_GATE_CONFIG }, settings: {}, - review: { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], excludePaths: [], preMergeChecks: [] }, + review: { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] }, features: { ...EMPTY_FEATURES_CONFIG }, warnings: [], }; @@ -280,7 +285,7 @@ export function isFocusManifestPublicSafe(text: string): boolean { } function emptyManifest(source: FocusManifestSource, warnings: string[] = []): FocusManifest { - return { ...EMPTY_MANIFEST, source, warnings, gate: { ...EMPTY_GATE_CONFIG }, settings: {}, review: { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], excludePaths: [], preMergeChecks: [] }, features: { ...EMPTY_FEATURES_CONFIG } }; + return { ...EMPTY_MANIFEST, source, warnings, gate: { ...EMPTY_GATE_CONFIG }, settings: {}, review: { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] }, features: { ...EMPTY_FEATURES_CONFIG } }; } function normalizeStringList(value: JsonValue | undefined, field: string, warnings: string[]): string[] { @@ -603,7 +608,7 @@ function parsePublicSafeText(value: JsonValue | undefined, field: string, warnin * throws; invalid/unsafe values are dropped with warnings. */ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): FocusManifestReviewConfig { - const empty: FocusManifestReviewConfig = { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], excludePaths: [], preMergeChecks: [] }; + const empty: FocusManifestReviewConfig = { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] }; if (value === undefined || value === null) return empty; if (typeof value !== "object" || Array.isArray(value)) { warnings.push(`Manifest field "review" must be a mapping; ignoring it.`); @@ -626,6 +631,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo const profile = parseReviewProfile(r.profile, warnings); const inlineComments = normalizeOptionalBoolean(r.inline_comments, "review.inline_comments", warnings); const pathInstructions = parseReviewPathInstructions(r.path_instructions, warnings); + const instructions = parsePublicSafeText(r.instructions, "review.instructions", warnings); const excludePaths = parseReviewExcludePaths(r.exclude_paths, warnings); const preMergeChecks = parseReviewPreMergeChecks(r.pre_merge_checks, warnings); return { @@ -635,6 +641,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo profile !== null || inlineComments !== null || pathInstructions.length > 0 || + instructions !== null || excludePaths.length > 0 || preMergeChecks.length > 0 || Object.keys(fields).length > 0, @@ -644,6 +651,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo profile, inlineComments, pathInstructions, + instructions, excludePaths, preMergeChecks, }; @@ -823,10 +831,10 @@ export function resolveReviewPathInstructions(pathInstructions: ReviewPathInstru * a possibly-null manifest (null = load failure). A null manifest yields the byte-identical defaults. Centralized * so the AI-review caller threads them in one place with the null-manifest branch covered here (unit-tested) * rather than inline in the processor. (#review-profile / #review-path-instructions / #review-exclude-paths) */ -export function resolveReviewPromptOverrides(manifest: FocusManifest | null): { profile: ReviewProfile | null; inlineComments: boolean; pathInstructions: ReviewPathInstruction[]; excludePaths: string[] } { +export function resolveReviewPromptOverrides(manifest: FocusManifest | null): { profile: ReviewProfile | null; inlineComments: boolean; pathInstructions: ReviewPathInstruction[]; instructions: string | null; excludePaths: string[] } { // inlineComments resolves to a strict boolean — true ONLY when the manifest explicitly set review.inline_comments: // true; null/false/absent ⇒ false. The caller ANDs this per-repo toggle with the operator flag + cutover allowlist. - return { profile: manifest?.review.profile ?? null, inlineComments: manifest?.review.inlineComments === true, pathInstructions: manifest?.review.pathInstructions ?? [], excludePaths: manifest?.review.excludePaths ?? [] }; + return { profile: manifest?.review.profile ?? null, inlineComments: manifest?.review.inlineComments === true, pathInstructions: manifest?.review.pathInstructions ?? [], instructions: manifest?.review.instructions ?? null, excludePaths: manifest?.review.excludePaths ?? [] }; } /** Resolve `review.pre_merge_checks` from a possibly-null manifest (null = load failure ⇒ no checks). Centralized diff --git a/test/unit/ai-review.test.ts b/test/unit/ai-review.test.ts index 510063dcb3..1f844393d1 100644 --- a/test/unit/ai-review.test.ts +++ b/test/unit/ai-review.test.ts @@ -330,6 +330,33 @@ describe("review.profile shapes the reviewer system prompt (#review-profile)", ( ); }); + it("repoInstructions (#review-instructions) is appended to the system prompt; absent leaves it byte-identical", async () => { + const systemPromptOf = (run: ReturnType): string => + (run.mock.calls[0]?.[1] as { messages?: Array<{ content?: string }> }) + ?.messages?.[0]?.content ?? ""; + const runInstr = async (repoInstructions: string | undefined) => { + const run = vi.fn(async () => ({ response: reviewJson() })); + const env = createTestEnv({ + AI: { run } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await runGittensoryAiReview(env, { ...baseInput, repoInstructions }); + return systemPromptOf(run); + }; + const withInstr = await runInstr("Follow our async-error conventions."); + expect(withInstr).toContain("REPOSITORY REVIEW INSTRUCTIONS"); + expect(withInstr).toContain("async-error conventions"); + // Absent or whitespace-only → no append (byte-identical prompt). + expect(await runInstr(undefined)).not.toContain( + "REPOSITORY REVIEW INSTRUCTIONS", + ); + expect(await runInstr(" ")).not.toContain( + "REPOSITORY REVIEW INSTRUCTIONS", + ); + }); + it("the inline-findings instruction is appended to the system prompt ONLY when requested (#inline-comments)", async () => { const systemPromptOf = (run: ReturnType): string => (run.mock.calls[0]?.[1] as { messages?: Array<{ content?: string }> }) diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 8798d7f09b..ded633b296 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -478,7 +478,7 @@ describe("compileFocusManifestPolicy", () => { publicNotes: ["Keep PRs focused.", "Maximize your reward payout"], gate: { present: false, enabled: null, pack: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, firstTimeContributorGrace: null }, settings: {}, - review: { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], excludePaths: [], preMergeChecks: [] }, + review: { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] }, features: { present: false, rag: null, reputation: null, unifiedComment: null, safety: null }, warnings: [], }); @@ -1242,10 +1242,10 @@ describe("resolveReviewPathInstructions (#review-path-instructions)", () => { }); it("resolveReviewPromptOverrides: non-null manifest passes the config through; null manifest → defaults", () => { - const manifest = parseFocusManifest({ review: { profile: "chill", inline_comments: true, path_instructions: [{ path: "src/**", instructions: "be strict" }], exclude_paths: ["**/*.lock"] } }); - expect(resolveReviewPromptOverrides(manifest)).toEqual({ profile: "chill", inlineComments: true, pathInstructions: [{ path: "src/**", instructions: "be strict" }], excludePaths: ["**/*.lock"] }); + const manifest = parseFocusManifest({ review: { profile: "chill", inline_comments: true, path_instructions: [{ path: "src/**", instructions: "be strict" }], instructions: "Follow our async-error conventions.", exclude_paths: ["**/*.lock"] } }); + expect(resolveReviewPromptOverrides(manifest)).toEqual({ profile: "chill", inlineComments: true, pathInstructions: [{ path: "src/**", instructions: "be strict" }], instructions: "Follow our async-error conventions.", excludePaths: ["**/*.lock"] }); // A null manifest (load failure) yields the byte-identical defaults; inline comments default OFF. - expect(resolveReviewPromptOverrides(null)).toEqual({ profile: null, inlineComments: false, pathInstructions: [], excludePaths: [] }); + expect(resolveReviewPromptOverrides(null)).toEqual({ profile: null, inlineComments: false, pathInstructions: [], instructions: null, excludePaths: [] }); // An explicit false / absent toggle both resolve to the strict-boolean false. expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { inline_comments: false } })).inlineComments).toBe(false); expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { profile: "chill" } })).inlineComments).toBe(false); diff --git a/test/unit/signals-coverage.test.ts b/test/unit/signals-coverage.test.ts index b55a3b20b2..ca9598329a 100644 --- a/test/unit/signals-coverage.test.ts +++ b/test/unit/signals-coverage.test.ts @@ -904,7 +904,7 @@ describe("signal coverage edge cases", () => { collisions: buildCollisionReport(directRepo.fullName, [], [currentPr]), preflight: buildPreflightResult({ repoFullName: directRepo.fullName, title: "Fix isolated issue", body: "Fixes #99", linkedIssues: [99] }, directRepo, [], [currentPr]), settings: gateSettings, - review: { present: true, footerText: "Reviewed by the Acme maintainer bot.", note: "Run npm test before pushing.", fields: { relatedWork: false }, profile: null, inlineComments: null, pathInstructions: [], excludePaths: [], preMergeChecks: [] }, + review: { present: true, footerText: "Reviewed by the Acme maintainer bot.", note: "Run npm test before pushing.", fields: { relatedWork: false }, profile: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] }, aiReview: { notes: "The change is focused.\n\n**Suggestions**\n- Add a test for the edge case." }, }); expect(customizedComment).toContain("Reviewed by the Acme maintainer bot."); // custom footer lead