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
20 changes: 15 additions & 5 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5124,6 +5124,10 @@ export async function runAiReviewForAdvisory(
// manifest. Threaded in (not loaded here) so the AI review path makes no extra manifest fetch — absent ⇒
// null ⇒ balanced ⇒ the reviewer prompt is byte-identical.
reviewProfile?: ReviewProfile | null | undefined;
// `.gittensory.yml` review.security_focus (#review-security-focus), resolved by the caller from the
// (already-cached) manifest. Orthogonal to reviewProfile — composes with it rather than replacing it.
// Absent/false ⇒ the reviewer prompt is byte-identical.
reviewSecurityFocus?: boolean | undefined;
// `.gittensory.yml` review.path_instructions (#review-path-instructions), resolved by the caller from the
// 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.
Expand Down Expand Up @@ -5359,6 +5363,7 @@ export async function runAiReviewForAdvisory(
observability: { rag: ragTelemetry },
enrichment,
profile: args.reviewProfile ?? null,
securityFocus: args.reviewSecurityFocus === true,
// Inline comments (#inline-comments): ask the model for line-anchored findings only when the operator flag,
// the cutover allowlist, AND the per-repo manifest toggle all pass. Otherwise the prompt is byte-identical.
inlineFindings: shouldRequestInlineFindings(
Expand Down Expand Up @@ -6478,13 +6483,16 @@ async function maybePublishPrPublicSurface(
},
async () => {
const reviewManifest = await loadRepoFocusManifest(env, repoFullName).catch(() => null);
// `.gittensory.yml` review.profile + review.path_instructions + review.exclude_paths (#review-profile /
// #review-path-instructions / #review-exclude-paths): resolve from the manifest (cached from settings
// resolution, so a cheap cache hit — no extra fetch) and thread them into the AI review. Profile shapes
// nitpickiness; path-instructions add per-path guidance; exclude-paths drop files from review. Absent ⇒
// byte-identical prompt. Fail-safe to defaults on any read error (resolveReviewPromptOverrides).
// `.gittensory.yml` review.profile + review.security_focus + review.path_instructions +
// review.exclude_paths (#review-profile / #review-security-focus / #review-path-instructions /
// #review-exclude-paths): resolve from the manifest (cached from settings resolution, so a cheap cache
// hit — no extra fetch) and thread them into the AI review. Profile shapes nitpickiness; security-focus
// adds elevated scrutiny for a security-defect category (orthogonal to profile); path-instructions add
// per-path guidance; exclude-paths drop files from review. Absent ⇒ byte-identical prompt. Fail-safe to
// defaults on any read error (resolveReviewPromptOverrides).
const {
profile: reviewProfile,
securityFocus: reviewSecurityFocus,
inlineComments: reviewInlineComments,
pathInstructions: reviewPathInstructions,
instructions: manifestReviewInstructions,
Expand Down Expand Up @@ -6565,6 +6573,7 @@ async function maybePublishPrPublicSurface(
}
: null,
profile: reviewProfile,
securityFocus: reviewSecurityFocus,
inlineComments: inlineCommentsEnabledForReview,
pathInstructions: reviewPathInstructions,
pathGuidance: resolveReviewPathInstructions(
Expand Down Expand Up @@ -6615,6 +6624,7 @@ async function maybePublishPrPublicSurface(
confirmedContributor,
files: reviewFilesForAi,
reviewProfile,
reviewSecurityFocus,
reviewPathInstructions,
reviewInstructions,
reviewExcludePaths,
Expand Down
2 changes: 2 additions & 0 deletions src/review/ai-review-cache-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export type AiReviewCacheInput = {
| null
| undefined;
profile: ReviewProfile | null | undefined;
securityFocus: boolean;
inlineComments: boolean;
pathInstructions: readonly ReviewPathInstruction[];
pathGuidance: string;
Expand Down Expand Up @@ -122,6 +123,7 @@ export async function aiReviewCacheInputFingerprint(input: AiReviewCacheInput):
}
: null,
profile: input.profile ?? null,
securityFocus: input.securityFocus,
inlineComments: input.inlineComments,
pathInstructions: input.pathInstructions.map((instruction) => ({
path: instruction.path,
Expand Down
23 changes: 20 additions & 3 deletions src/services/ai-review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,14 @@ export type GittensoryAiReviewInput = {
* consensus-defect pass still runs the same), just how much advisory detail the prose carries.
*/
profile?: ReviewProfile | null | undefined;
/**
* `.gittensory.yml` `review.security_focus` (#review-security-focus): when true, instructs the reviewer to
* prioritize a security-defect category — injection, authn/authz bypass, secret handling, unsafe
* deserialization, SSRF, and path traversal — with elevated scrutiny. ORTHOGONAL to `profile`: it composes
* with (never replaces) the chill/balanced/assertive volume tuning above — a "what to prioritize" axis, not a
* fourth profile level. Absent/false (the default) ⇒ the reviewer prompt is byte-identical to today.
*/
securityFocus?: boolean | undefined;
/**
* `.gittensory.yml` `review.path_instructions` (#review-path-instructions), pre-resolved by the caller to the
* entries whose glob matched THIS PR's changed files (via `resolveReviewPathInstructions`) — a ready-to-append
Expand Down Expand Up @@ -533,15 +541,23 @@ const REVIEW_PROFILE_SUFFIX: Record<"chill" | "assertive", string> = {
"\n\nReview profile: ASSERTIVE. Beyond blocking defects, also surface minor improvements, style/consistency suggestions, and nitpicks — be thorough and exacting, clearly marking each non-blocking item as a nit.",
};

// `.gittensory.yml` review.security_focus → an appended security-prioritization instruction (#review-security-focus).
// ORTHOGONAL to REVIEW_PROFILE_SUFFIX above — it composes with (never replaces) the chill/balanced/assertive volume
// tuning: profile controls HOW MANY findings surface, this controls WHAT KIND the reviewer hunts for with elevated
// scrutiny. False/absent (default) appends nothing (byte-identical).
const SECURITY_FOCUS_SUFFIX =
"\n\nSECURITY FOCUS: Beyond the usual review, prioritize hunting for security defects with elevated scrutiny — injection (SQL/command/template/log), authentication/authorization bypass, unsafe secret handling (hardcoded credentials, logged/leaked tokens), unsafe deserialization, server-side request forgery (SSRF), and path traversal. Treat a credible finding in any of these categories as a blocker even if it would otherwise read as a nit.";

// `.gittensory.yml` review.inline_comments → an appended instruction to ALSO emit line-anchored findings for
// quiet inline PR comments (#inline-comments). Absent/off appends nothing (byte-identical). The model keeps the
// existing 4-field shape and simply ADDS an `inlineFindings` array.
const INLINE_FINDINGS_SUFFIX =
'\n\nINLINE FINDINGS: ALSO include an additional top-level field "inlineFindings" in the SAME JSON object — an array (possibly empty) of your most important findings, each anchored to a specific changed line, for inline PR comments. Each item: {"path": the changed file path EXACTLY as shown in the diff, "line": the 1-based line number in the NEW file (count forward from the "+" start in the nearest "@@ -old +new @@" hunk header) of an ADDED ("+") line you are commenting on, "severity": "blocker" or "nit", "body": the one-sentence finding, "suggestion": optional replacement text for that line}. Include ONLY findings you can place on a specific added line; OMIT any you cannot anchor precisely (a wrong line is worse than none). If a suggestion is blank or you are not confident in an exact replacement, omit the suggestion field and keep the finding. At most ~10 items.';

/** The effective reviewer SYSTEM prompt. Appends the grounding-discipline suffix when the caller supplied one
* (flag GITTENSORY_REVIEW_GROUNDING on), the `review.profile` tone suffix when set, then the inline-findings
* instruction when the caller asked for them; all absent (default) → the base prompt, byte-identical to today. */
* (flag GITTENSORY_REVIEW_GROUNDING on), the `review.profile` tone suffix when set, the `review.security_focus`
* prioritization suffix when on, then the inline-findings instruction when the caller asked for them; all absent
* (default) → the base prompt, byte-identical to today. */
function buildSystemPrompt(input: GittensoryAiReviewInput): string {
const groundingSuffix = input.grounding?.systemSuffix ?? "";
// Review-enrichment brief (#1472): the REES supplies a one-line discipline suffix ("treat a listed CVE/secret as
Expand All @@ -551,6 +567,7 @@ function buildSystemPrompt(input: GittensoryAiReviewInput): string {
input.profile === "chill" || input.profile === "assertive"
? REVIEW_PROFILE_SUFFIX[input.profile]
: "";
const securityFocusSuffix = input.securityFocus === true ? SECURITY_FOCUS_SUFFIX : "";
// `.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 : "";
Expand All @@ -560,7 +577,7 @@ function buildSystemPrompt(input: GittensoryAiReviewInput): string {
? ` 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}${repoInstructionsSuffix}${inlineSuffix}`;
return `${REVIEW_SYSTEM_PROMPT}${groundingSuffix}${enrichmentSuffix}${profileSuffix}${securityFocusSuffix}${pathSuffix}${repoInstructionsSuffix}${inlineSuffix}`;
}

/** One Workers-AI opinion with a per-slot reliable fallback and a 3× retry on the primary. */
Expand Down
29 changes: 20 additions & 9 deletions src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,11 @@ export type FocusManifestReviewConfig = {
fields: Partial<Record<ReviewFieldKey, boolean>>;
/** `review.profile`: chill / balanced / assertive. null (absent) = balanced = byte-identical reviewer prompt. */
profile: ReviewProfile | null;
/** `review.security_focus`: when true, the AI reviewer is told to prioritize a security-defect category
* (injection, authn/authz bypass, secret handling, unsafe deserialization, SSRF, path traversal) with
* elevated scrutiny, ON TOP OF whatever `profile` volume is set — an orthogonal "what to prioritize" axis,
* not a fourth profile level. null/false (default, absent) = byte-identical reviewer prompt. (#review-security-focus) */
securityFocus: boolean | null;
/** `review.inline_comments`: when true, the AI reviewer ALSO leaves quiet, non-blocking inline PR comments on
* specific changed lines (in addition to the decision summary). null/false (default, absent) = no inline
* comments = byte-identical behavior. Operator-gated too (GITTENSORY_REVIEW_INLINE_COMMENTS + allowlist).
Expand Down Expand Up @@ -350,7 +355,7 @@ const EMPTY_MANIFEST: FocusManifest = {
publicNotes: [],
gate: { ...EMPTY_GATE_CONFIG },
settings: {},
review: { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] },
review: { present: false, footerText: null, note: null, fields: {}, profile: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] },
features: { ...EMPTY_FEATURES_CONFIG },
contentLane: { ...EMPTY_CONTENT_LANE_CONFIG },
warnings: [],
Expand Down Expand Up @@ -379,7 +384,7 @@ function emptyManifest(source: FocusManifestSource, warnings: string[] = []): Fo
warnings,
gate: { ...EMPTY_GATE_CONFIG },
settings: {},
review: { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] },
review: { present: false, footerText: null, note: null, fields: {}, profile: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] },
features: { ...EMPTY_FEATURES_CONFIG },
contentLane: { ...EMPTY_CONTENT_LANE_CONFIG },
};
Expand Down Expand Up @@ -929,7 +934,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: [], instructions: null, excludePaths: [], preMergeChecks: [] };
const empty: FocusManifestReviewConfig = { present: false, footerText: null, note: null, fields: {}, profile: null, securityFocus: 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.`);
Expand All @@ -950,6 +955,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
const footerText = footerRecord ? parsePublicSafeText(footerRecord.text, "review.footer.text", warnings) : null;
const note = parsePublicSafeText(r.note, "review.note", warnings);
const profile = parseReviewProfile(r.profile, warnings);
const securityFocus = normalizeOptionalBoolean(r.security_focus, "review.security_focus", 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);
Expand All @@ -960,6 +966,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
footerText !== null ||
note !== null ||
profile !== null ||
securityFocus !== null ||
inlineComments !== null ||
pathInstructions.length > 0 ||
instructions !== null ||
Expand All @@ -970,6 +977,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
note,
fields,
profile,
securityFocus,
inlineComments,
pathInstructions,
instructions,
Expand Down Expand Up @@ -1116,6 +1124,7 @@ export function reviewConfigToJson(review: FocusManifestReviewConfig): JsonValue
if (review.footerText !== null) out.footer = { text: review.footerText };
if (review.note !== null) out.note = review.note;
if (review.profile !== null) out.profile = review.profile;
if (review.securityFocus !== null) out.security_focus = review.securityFocus;
if (review.inlineComments !== null) out.inline_comments = review.inlineComments;
if (review.instructions !== null) out.instructions = review.instructions;
if (review.pathInstructions.length > 0) out.path_instructions = review.pathInstructions.map((entry) => ({ path: entry.path, instructions: entry.instructions }));
Expand Down Expand Up @@ -1149,14 +1158,16 @@ export function resolveReviewPathInstructions(pathInstructions: ReviewPathInstru
return `\n\nPath-specific review instructions from the maintainer — apply these to the changed files that match each glob:\n${lines.join("\n")}`;
}

/** Resolve the AI-reviewer overrides (`review.profile` + `review.path_instructions` + `review.exclude_paths`) from
* 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[]; instructions: string | null; excludePaths: string[] } {
/** Resolve the AI-reviewer overrides (`review.profile` + `review.security_focus` + `review.path_instructions` +
* `review.exclude_paths`) from 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-security-focus / #review-path-instructions / #review-exclude-paths) */
export function resolveReviewPromptOverrides(manifest: FocusManifest | null): { profile: ReviewProfile | null; securityFocus: boolean; 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 ?? [], instructions: manifest?.review.instructions ?? null, excludePaths: manifest?.review.excludePaths ?? [] };
// securityFocus resolves the same way — true ONLY when the manifest explicitly set review.security_focus: true.
return { profile: manifest?.review.profile ?? null, securityFocus: manifest?.review.securityFocus === true, 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
Expand Down
14 changes: 14 additions & 0 deletions test/unit/ai-review-cache-input.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const baseInput = (): AiReviewCacheInput => ({
baseSha: null,
reviewFiles: [],
profile: null,
securityFocus: false,
inlineComments: false,
pathInstructions: [],
pathGuidance: "",
Expand Down Expand Up @@ -241,4 +242,17 @@ describe("aiReviewCacheInputFingerprint", () => {
expect(gatePackChanged).not.toBe(original);
expect(repeated).toBe(original);
});

it("changes when securityFocus toggles, independently of profile (#review-security-focus)", async () => {
const original = await aiReviewCacheInputFingerprint(baseInput());
const securityFocusOn = await aiReviewCacheInputFingerprint({ ...baseInput(), securityFocus: true });
const profileAndSecurityFocus = await aiReviewCacheInputFingerprint({ ...baseInput(), profile: "chill", securityFocus: true });
const profileOnly = await aiReviewCacheInputFingerprint({ ...baseInput(), profile: "chill" });
const repeated = await aiReviewCacheInputFingerprint(baseInput());

expect(securityFocusOn).not.toBe(original);
expect(profileAndSecurityFocus).not.toBe(profileOnly);
expect(profileAndSecurityFocus).not.toBe(securityFocusOn);
expect(repeated).toBe(original);
});
});
1 change: 1 addition & 0 deletions test/unit/ai-review-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const baseFingerprintInput = (): AiReviewCacheInput => ({
baseSha: null,
reviewFiles: [],
profile: null,
securityFocus: false,
inlineComments: false,
pathInstructions: [],
pathGuidance: "",
Expand Down
Loading
Loading