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
4 changes: 4 additions & 0 deletions .gittensory.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,10 @@ review:
# finding_categories: false
# When true, inline findings are tagged with a category label (requires inline_comments).

# Fix-handoff blocks (#2176). Bool | null. Default: null/false — byte-identical.
# Requires operator flag GITTENSORY_REVIEW_FIX_HANDOFF + cutover allowlist AND this toggle.
# fixHandoff: false

# Boundary-safe test generation (#2189). Bool | null. Default: null/false — byte-identical.
# Requires operator flag GITTENSORY_REVIEW_TEST_GENERATION AND this toggle.
# test_generation: false
Expand Down
4 changes: 4 additions & 0 deletions config/examples/gittensory.full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,10 @@ review:
# finding_categories: false
# When true, inline findings are tagged with a category label (requires inline_comments).

# Fix-handoff blocks (#2176). Bool | null. Default: null/false — byte-identical.
# Requires operator flag GITTENSORY_REVIEW_FIX_HANDOFF + cutover allowlist AND this toggle.
# fixHandoff: false

# Boundary-safe test generation (#2189). Bool | null. Default: null/false — byte-identical.
# Requires operator flag GITTENSORY_REVIEW_TEST_GENERATION AND this toggle.
# test_generation: false
Expand Down
25 changes: 25 additions & 0 deletions src/review/fix-handoff.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Fix-handoff blocks (#2176, config slice for #1962) — copy-paste remediation guidance the reviewer can emit
// ALONGSIDE the decision summary. Default OFF at every layer, mirroring the inline-comments precedent: the operator
// flag GITTENSORY_REVIEW_FIX_HANDOFF, the per-repo convergence cutover allowlist, AND the per-repo `.gittensory.yml`
// review.fixHandoff toggle are ALL ANDed before a fix-handoff block is ever emitted. This is the config/gate slice:
// pure resolvers only — no emission/render here (that is a separate slice), so the gate/verdict is never touched.

import { isConvergenceRepoAllowed } from "./cutover-gate";

/** True when the operator enabled fix-handoff globally. Flag-OFF (default) ⇒ the caller never emits fix-handoff
* blocks. Truthy follows the codebase convention (same regex as isInlineCommentsEnabled). */
export function isFixHandoffEnabled(env: { GITTENSORY_REVIEW_FIX_HANDOFF?: string | undefined }): boolean {
return /^(1|true|yes|on)$/i.test(env.GITTENSORY_REVIEW_FIX_HANDOFF ?? "");
}

/** PURE: should the reviewer emit fix-handoff blocks for this PR? True ONLY when ALL THREE gates pass — the per-repo
* `.gittensory.yml` toggle (`manifestToggle`), the operator flag, AND the convergence cutover allowlist — so the
* feature is off by default at every layer. Mirrors shouldRequestInlineFindings, keeping the three-way gate in one
* unit-testable place. */
export function shouldEmitFixHandoff(
env: { GITTENSORY_REVIEW_FIX_HANDOFF?: string | undefined; GITTENSORY_REVIEW_REPOS?: string | undefined },
repoFullName: string,
manifestToggle: boolean | undefined,
): boolean {
return manifestToggle === true && isFixHandoffEnabled(env) && isConvergenceRepoAllowed(env, repoFullName);
}
14 changes: 11 additions & 3 deletions src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,10 @@ export type FocusManifestReviewConfig = {
* comments = byte-identical behavior. Operator-gated too (GITTENSORY_REVIEW_INLINE_COMMENTS + allowlist).
* (#inline-comments) */
inlineComments: boolean | null;
/** `review.fixHandoff`: when true, the reviewer emits fix-handoff blocks (copy-paste remediation guidance). null/
* false (default, absent) = no fix-handoff blocks = byte-identical. Operator-gated too (GITTENSORY_REVIEW_FIX_HANDOFF
* + the convergence cutover allowlist) — the manifest toggle is only one of the ANDed gates. (#2176, for #1962) */
fixHandoff: boolean | null;
/** `review.suggestions`: when true, an inline finding whose AI-provided fix is precise enough to anchor to a
* single line is ALSO rendered as a GitHub-native ` ```suggestion ` block a contributor can commit in one
* click. Only takes effect when inline comments are already on (a suggestion has nothing to attach to
Expand Down Expand Up @@ -760,7 +764,7 @@ const EMPTY_MANIFEST: FocusManifest = {
publicNotes: [],
gate: { ...EMPTY_GATE_CONFIG },
settings: {},
review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, testGeneration: null, impactMap: null, cultureProfile: null, findingCategories: null, minFindingSeverity: null, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null },
review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, fixHandoff: null, suggestions: null, changedFilesSummary: null, effortScore: null, testGeneration: null, impactMap: null, cultureProfile: null, findingCategories: null, minFindingSeverity: null, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null },
features: { ...EMPTY_FEATURES_CONFIG },
contentLane: { ...EMPTY_CONTENT_LANE_CONFIG },
repoDocGeneration: { ...EMPTY_REPO_DOC_GENERATION_CONFIG },
Expand Down Expand Up @@ -791,7 +795,7 @@ function emptyManifest(source: FocusManifestSource, warnings: string[] = []): Fo
warnings,
gate: { ...EMPTY_GATE_CONFIG },
settings: {},
review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, testGeneration: null, impactMap: null, cultureProfile: null, findingCategories: null, minFindingSeverity: null, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null },
review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, fixHandoff: null, suggestions: null, changedFilesSummary: null, effortScore: null, testGeneration: null, impactMap: null, cultureProfile: null, findingCategories: null, minFindingSeverity: null, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null },
features: { ...EMPTY_FEATURES_CONFIG },
contentLane: { ...EMPTY_CONTENT_LANE_CONFIG },
repoDocGeneration: { ...EMPTY_REPO_DOC_GENERATION_CONFIG },
Expand Down Expand Up @@ -1766,7 +1770,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: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, testGeneration: null, impactMap: null, cultureProfile: null, findingCategories: null, minFindingSeverity: null, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null };
const empty: FocusManifestReviewConfig = { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, fixHandoff: null, suggestions: null, changedFilesSummary: null, effortScore: null, testGeneration: null, impactMap: null, cultureProfile: null, findingCategories: null, minFindingSeverity: null, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null };
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 Down Expand Up @@ -1803,6 +1807,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
const tone = parsePublicSafeText(r.tone, "review.tone", warnings);
const securityFocus = normalizeOptionalBoolean(r.security_focus, "review.security_focus", warnings);
const inlineComments = normalizeOptionalBoolean(r.inline_comments, "review.inline_comments", warnings);
const fixHandoff = normalizeOptionalBoolean(r.fixHandoff, "review.fixHandoff", warnings);
const suggestions = normalizeOptionalBoolean(r.suggestions, "review.suggestions", warnings);
const changedFilesSummary = normalizeOptionalBoolean(r.changed_files_summary, "review.changed_files_summary", warnings);
const effortScore = normalizeOptionalBoolean(r.effort_score, "review.effort_score", warnings);
Expand Down Expand Up @@ -1836,6 +1841,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
tone !== null ||
securityFocus !== null ||
inlineComments !== null ||
fixHandoff !== null ||
suggestions !== null ||
changedFilesSummary !== null ||
effortScore !== null ||
Expand Down Expand Up @@ -1871,6 +1877,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
tone,
securityFocus,
inlineComments,
fixHandoff,
suggestions,
changedFilesSummary,
effortScore,
Expand Down Expand Up @@ -2333,6 +2340,7 @@ export function reviewConfigToJson(review: FocusManifestReviewConfig): JsonValue
if (review.tone !== null) out.tone = review.tone;
if (review.securityFocus !== null) out.security_focus = review.securityFocus;
if (review.inlineComments !== null) out.inline_comments = review.inlineComments;
if (review.fixHandoff !== null) out.fixHandoff = review.fixHandoff;
if (review.suggestions !== null) out.suggestions = review.suggestions;
if (review.changedFilesSummary !== null) out.changed_files_summary = review.changedFilesSummary;
if (review.effortScore !== null) out.effort_score = review.effortScore;
Expand Down
3 changes: 2 additions & 1 deletion test/unit/focus-manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ describe(".gittensory.yml.example field-exhaustiveness (#1670)", () => {
tone: "tone:",
securityFocus: "security_focus:",
inlineComments: "inline_comments:",
fixHandoff: "fixHandoff:",
suggestions: "suggestions:",
changedFilesSummary: "changed_files_summary:",
effortScore: "effort_score:",
Expand Down Expand Up @@ -784,7 +785,7 @@ describe("compileFocusManifestPolicy", () => {
publicNotes: ["Keep PRs focused.", "Maximize your reward payout"],
gate: { present: false, enabled: null, checkMode: null, pack: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null },
settings: {},
review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, testGeneration: null, impactMap: null, cultureProfile: null, findingCategories: null, minFindingSeverity: null, maxFindings: { blockers: null, nits: null }, commentVerbosity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null },
review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, fixHandoff: null, suggestions: null, changedFilesSummary: null, effortScore: null, testGeneration: null, impactMap: null, cultureProfile: null, findingCategories: null, minFindingSeverity: null, maxFindings: { blockers: null, nits: null }, commentVerbosity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null },
features: { present: false, rag: null, reputation: null, unifiedComment: null, safety: null },
contentLane: { present: false, entryFileGlob: null, providerFileGlob: null, artifactGlob: null, collectionField: null, maxAppendedEntries: null, duplicateKeyFields: [], validatorId: null },
repoDocGeneration: { present: false, enabled: false, scope: ["agents"], allowOverwriteExisting: false, refreshIntervalDays: 7 },
Expand Down
51 changes: 51 additions & 0 deletions test/unit/review-fix-handoff.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { describe, expect, it } from "vitest";
import { parseFocusManifest, reviewConfigToJson } from "../../src/signals/focus-manifest";
import { isFixHandoffEnabled, shouldEmitFixHandoff } from "../../src/review/fix-handoff";

const reviewOf = (fixHandoff: unknown) => parseFocusManifest({ review: { fixHandoff } });
const ON = "acme/widgets";
const ALLOW = { GITTENSORY_REVIEW_FIX_HANDOFF: "1", GITTENSORY_REVIEW_REPOS: ON };

describe("review.fixHandoff config toggle (#2176)", () => {
it("absent ⇒ null and OMITTED on serialize (byte-identical)", () => {
const review = parseFocusManifest({ review: { note: "x" } }).review;
expect(review.fixHandoff).toBe(null);
expect("fixHandoff" in (reviewConfigToJson(review) as Record<string, unknown>)).toBe(false);
});

it("true / false parse, mark present, and round-trip", () => {
for (const v of [true, false]) {
const review = reviewOf(v).review;
expect(review.fixHandoff).toBe(v);
expect(review.present).toBe(true);
const json = reviewConfigToJson(review) as Record<string, unknown>;
expect(json.fixHandoff).toBe(v);
expect(parseFocusManifest({ review: json }).review.fixHandoff).toBe(v);
}
});

it("a non-boolean value warns and falls back to null", () => {
const m = reviewOf("maybe");
expect(m.review.fixHandoff).toBe(null);
expect(m.warnings.some((w) => /review\.fixHandoff/.test(w))).toBe(true);
});
});

describe("fix-handoff env kill-switch + resolver (#2176)", () => {
it("isFixHandoffEnabled: only truthy env values enable", () => {
for (const v of ["1", "true", "yes", "on", "TRUE"]) expect(isFixHandoffEnabled({ GITTENSORY_REVIEW_FIX_HANDOFF: v })).toBe(true);
for (const v of ["0", "false", "off", "", undefined]) expect(isFixHandoffEnabled({ GITTENSORY_REVIEW_FIX_HANDOFF: v })).toBe(false);
});

it("shouldEmitFixHandoff: true ONLY when manifest toggle AND env flag AND cutover allowlist all pass", () => {
// all three on
expect(shouldEmitFixHandoff(ALLOW, ON, true)).toBe(true);
// manifest toggle off / undefined
expect(shouldEmitFixHandoff(ALLOW, ON, false)).toBe(false);
expect(shouldEmitFixHandoff(ALLOW, ON, undefined)).toBe(false);
// env flag off
expect(shouldEmitFixHandoff({ GITTENSORY_REVIEW_FIX_HANDOFF: "0", GITTENSORY_REVIEW_REPOS: ON }, ON, true)).toBe(false);
// repo not on the cutover allowlist
expect(shouldEmitFixHandoff({ GITTENSORY_REVIEW_FIX_HANDOFF: "1", GITTENSORY_REVIEW_REPOS: "other/repo" }, ON, true)).toBe(false);
});
});
2 changes: 1 addition & 1 deletion test/unit/signals-coverage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1127,7 +1127,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 }, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, testGeneration: null, impactMap: null, cultureProfile: null, findingCategories: null, minFindingSeverity: null, maxFindings: { blockers: null, nits: null }, commentVerbosity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { skipDrafts: null, ignoreAuthors: [], ignoreTitleKeywords: [], skipLabels: [], skipDocsOnly: null, maxAddedLines: 0, maxFiles: 0, baseBranches: [], autoPauseAfterReviewedCommits: null }, labelingRules: [], aiModel: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, visual: { preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: [], gif: false }, linkedIssueSatisfaction: null },
review: { present: true, footerText: "Reviewed by the Acme maintainer bot.", note: "Run npm test before pushing.", fields: { relatedWork: false }, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, fixHandoff: null, suggestions: null, changedFilesSummary: null, effortScore: null, testGeneration: null, impactMap: null, cultureProfile: null, findingCategories: null, minFindingSeverity: null, maxFindings: { blockers: null, nits: null }, commentVerbosity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { skipDrafts: null, ignoreAuthors: [], ignoreTitleKeywords: [], skipLabels: [], skipDocsOnly: null, maxAddedLines: 0, maxFiles: 0, baseBranches: [], autoPauseAfterReviewedCommits: null }, labelingRules: [], aiModel: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, visual: { preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: [], gif: false }, linkedIssueSatisfaction: null },
aiReview: { notes: "The change is focused.\n\n**Nits (2)**\n- Add a test for the </details> edge case.\n- Keep the validator helper scoped." },
});
expect(customizedComment).toContain("Reviewed by the Acme maintainer bot."); // custom footer lead
Expand Down