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
7 changes: 7 additions & 0 deletions .gittensory.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,13 @@ review:
# comment | commit. Default: null/comment (byte-identical -- no write access to any PR branch).
# e2e_test_delivery: comment

# Opts THIS repo into the manifest_missing_tests AUTO-TRIGGER (#4196, part of the #4189 epic) -- an
# unprompted generation run whenever a PR looks like it needs tests. Separate from features.e2eTests, which
# only unlocks the maintainer-initiated paths (the `@gittensory generate-tests` command and the PR-panel
# checkbox). Bool | null. Default: null/false -- the auto-trigger never fires, even with e2eTests already
# on; a maintainer who wants unprompted generation opts in here explicitly, per repo.
# e2e_test_auto_trigger: false

# Inline-comment layer toggles (#1956 / #1958). Bool | null. Default: null/false — byte-identical.
# Requires operator flag GITTENSORY_REVIEW_INLINE_COMMENTS + cutover allowlist + review.inline_comments: true.
# inline_comments: false
Expand Down
7 changes: 7 additions & 0 deletions config/examples/gittensory.full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,13 @@ review:
# comment | commit. Default: null/comment (byte-identical -- no write access to any PR branch).
# e2e_test_delivery: comment

# Opts THIS repo into the manifest_missing_tests AUTO-TRIGGER (#4196, part of the #4189 epic) -- an
# unprompted generation run whenever a PR looks like it needs tests. Separate from features.e2eTests, which
# only unlocks the maintainer-initiated paths (the `@gittensory generate-tests` command and the PR-panel
# checkbox). Bool | null. Default: null/false -- the auto-trigger never fires, even with e2eTests already
# on; a maintainer who wants unprompted generation opts in here explicitly, per repo.
# e2e_test_auto_trigger: false

# Inline-comment layer toggles (#1956 / #1958). Bool | null. Default: null/false — byte-identical.
# Requires operator flag GITTENSORY_REVIEW_INLINE_COMMENTS + cutover allowlist + review.inline_comments: true.
# inline_comments: false
Expand Down
21 changes: 18 additions & 3 deletions packages/gittensory-engine/src/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,15 @@ export type FocusManifestReviewConfig = {
* is a confirmed Gittensor miner, to protect the external, upstream-computed score from ever including a
* maintainer-authored line the miner didn't write themselves — see `src/github/e2e-test-commit.ts`. */
e2eTestDelivery: E2eTestDeliveryMode | null;
/** `review.e2e_test_auto_trigger` (#4196, part of the #4189 epic): opts THIS repo into the `manifest_missing_tests`
* auto-trigger, which promotes that advisory finding into an actual unprompted generation run whenever a PR looks
* like it needs tests -- separate from `features.e2eTests`, which only unlocks the maintainer-initiated paths
* (the `@gittensory generate-tests` command and the PR-panel checkbox). Deliberately independent and OFF by
* default: enabling `e2eTests` for on-demand use must never, by itself, start firing generation unprompted on
* every under-tested PR (the exact loophole this field closes) -- a maintainer who *wants* the auto-trigger opts
* in explicitly per repo. null/false (default, absent) ⇒ the auto-trigger never fires, even with e2eTests on;
* true additionally requires e2eTests to already be enabled (this field alone does nothing). */
e2eTestAutoTrigger: boolean | null;
/** `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. Also consumed by
* AI-generated E2E test coverage (`resolveE2eTestGenInstructions` in `ai-e2e-test-gen.ts`, #4200) when
Expand Down Expand Up @@ -1034,7 +1043,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, fixHandoff: null, autoMergeSummary: null, suggestions: null, changedFilesSummary: null, effortScore: null, impactMap: null, cultureProfile: null, selftune: null, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: null, e2eTestDelivery: 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, sharedConfigSource: null },
review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, fixHandoff: null, autoMergeSummary: null, suggestions: null, changedFilesSummary: null, effortScore: null, impactMap: null, cultureProfile: null, selftune: null, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: null, e2eTestDelivery: null, e2eTestAutoTrigger: 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, sharedConfigSource: null },
features: { ...EMPTY_FEATURES_CONFIG },
contentLane: { ...EMPTY_CONTENT_LANE_CONFIG },
repoDocGeneration: { ...EMPTY_REPO_DOC_GENERATION_CONFIG },
Expand Down Expand Up @@ -1065,7 +1074,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, fixHandoff: null, autoMergeSummary: null, suggestions: null, changedFilesSummary: null, effortScore: null, impactMap: null, cultureProfile: null, selftune: null, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: null, e2eTestDelivery: 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, sharedConfigSource: null },
review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, fixHandoff: null, autoMergeSummary: null, suggestions: null, changedFilesSummary: null, effortScore: null, impactMap: null, cultureProfile: null, selftune: null, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: null, e2eTestDelivery: null, e2eTestAutoTrigger: 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, sharedConfigSource: null },
features: { ...EMPTY_FEATURES_CONFIG },
contentLane: { ...EMPTY_CONTENT_LANE_CONFIG },
repoDocGeneration: { ...EMPTY_REPO_DOC_GENERATION_CONFIG },
Expand Down Expand Up @@ -2170,7 +2179,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, fixHandoff: null, autoMergeSummary: null, suggestions: null, changedFilesSummary: null, effortScore: null, impactMap: null, cultureProfile: null, selftune: null, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: null, e2eTestDelivery: 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, sharedConfigSource: null };
const empty: FocusManifestReviewConfig = { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, fixHandoff: null, autoMergeSummary: null, suggestions: null, changedFilesSummary: null, effortScore: null, impactMap: null, cultureProfile: null, selftune: null, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: null, e2eTestDelivery: null, e2eTestAutoTrigger: 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, sharedConfigSource: 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 @@ -2231,6 +2240,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
const maxFindings = parseMaxFindingsConfig(r.max_findings, warnings);
const commentVerbosity = normalizeOptionalEnum(r.comment_verbosity, "review.comment_verbosity", COMMENT_VERBOSITY_LEVELS, warnings);
const e2eTestDelivery = normalizeOptionalEnum(r.e2e_test_delivery, "review.e2e_test_delivery", E2E_TEST_DELIVERY_MODES, warnings);
const e2eTestAutoTrigger = normalizeOptionalBoolean(r.e2e_test_auto_trigger, "review.e2e_test_auto_trigger", warnings);
const pathInstructions = parseReviewPathInstructions(r.path_instructions, warnings);
const instructions = parsePublicSafeText(r.instructions, "review.instructions", warnings);
const excludePaths = parseReviewExcludePaths(r.exclude_paths, warnings);
Expand Down Expand Up @@ -2264,6 +2274,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
maxFindingsPresent(maxFindings) ||
commentVerbosity !== null ||
e2eTestDelivery !== null ||
e2eTestAutoTrigger !== null ||
pathInstructions.length > 0 ||
instructions !== null ||
excludePaths.length > 0 ||
Expand Down Expand Up @@ -2303,6 +2314,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
maxFindings,
commentVerbosity,
e2eTestDelivery,
e2eTestAutoTrigger,
pathInstructions,
instructions,
excludePaths,
Expand Down Expand Up @@ -2402,6 +2414,7 @@ function computeReviewConfigPresent(review: Omit<FocusManifestReviewConfig, "pre
maxFindingsPresent(review.maxFindings) ||
review.commentVerbosity !== null ||
review.e2eTestDelivery !== null ||
review.e2eTestAutoTrigger !== null ||
review.pathInstructions.length > 0 ||
review.instructions !== null ||
review.excludePaths.length > 0 ||
Expand Down Expand Up @@ -2447,6 +2460,7 @@ export function overlayReviewConfig(
maxFindings: overlayMaxFindingsConfig(base.maxFindings, override.maxFindings),
commentVerbosity: pickOverlayNullable(override.commentVerbosity, base.commentVerbosity),
e2eTestDelivery: pickOverlayNullable(override.e2eTestDelivery, base.e2eTestDelivery),
e2eTestAutoTrigger: pickOverlayNullable(override.e2eTestAutoTrigger, base.e2eTestAutoTrigger),
pathInstructions: override.pathInstructions.length > 0 ? [...override.pathInstructions] : [...base.pathInstructions],
instructions: pickOverlayNullable(override.instructions, base.instructions),
excludePaths: pickOverlayStringList(override.excludePaths, base.excludePaths),
Expand Down Expand Up @@ -2972,6 +2986,7 @@ export function reviewConfigToJson(review: FocusManifestReviewConfig): JsonValue
}
if (review.commentVerbosity !== null) out.comment_verbosity = review.commentVerbosity;
if (review.e2eTestDelivery !== null) out.e2e_test_delivery = review.e2eTestDelivery;
if (review.e2eTestAutoTrigger !== null) out.e2e_test_auto_trigger = review.e2eTestAutoTrigger;
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 }));
if (review.excludePaths.length > 0) out.exclude_paths = [...review.excludePaths];
Expand Down
38 changes: 19 additions & 19 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,7 @@ import {
resolveReviewPreMergeChecks,
resolveReviewPromptOverrides,
resolveReviewMemoryManifestToggle,
resolveE2eTestAutoTriggerManifestToggle,
resolveReviewVisualConfig,
type AiReviewCadence,
type FocusManifestFinding,
Expand Down Expand Up @@ -624,15 +625,6 @@ const PR_PANEL_RETRIGGER_COMMAND_AUTHORIZATION: RepositoryCommandAuthorizationPo
default: ["maintainer", "collaborator"],
commands: { "review-now": ["maintainer", "collaborator"] },
};
// #4589: the generate-tests checkbox is hardcoded to maintainer-only regardless of what a repo's own
// .gittensory.yml commandAuthorization might configure for the text-command version of generate-tests (which
// CAN be widened to collaborator/confirmed_miner) -- a one-click checkbox is meaningfully lower-friction than
// typing a command, so it gets a hard floor that can't be misconfigured away. Mirrors
// PR_PANEL_RETRIGGER_COMMAND_AUTHORIZATION's exact same override pattern, one tier narrower (no collaborator).
const PR_PANEL_GENERATE_TESTS_COMMAND_AUTHORIZATION: RepositoryCommandAuthorizationPolicy = {
default: ["maintainer"],
commands: { "generate-tests": ["maintainer"] },
};
const PR_PUBLIC_SURFACE_ACTIONS = new Set([
"opened",
"reopened",
Expand Down Expand Up @@ -9285,7 +9277,12 @@ async function maybeApplyManifestPolicyGate(
// tests" from scratch, per the issue's own requirement -- this is why the auto-trigger lives inside this
// exact manifestPolicyGateMode-gated block instead of a parallel code path: that is the only place this
// finding is computed at all today.
if (args.pr.headSha && policyFindings.some((finding) => finding.code === "manifest_missing_tests") && e2eTestGenAvailable) {
// Deliberately gated by its OWN separate manifest toggle (review.e2e_test_auto_trigger) on top of
// e2eTestGenAvailable -- enabling features.e2eTests only unlocks the maintainer-initiated command/checkbox
// paths below; it must never, by itself, start firing generation unprompted on every under-tested PR. A
// repo opts into the auto-trigger explicitly, in addition to the base feature.
const e2eAutoTriggerOptedIn = resolveE2eTestAutoTriggerManifestToggle(manifest);
if (args.pr.headSha && policyFindings.some((finding) => finding.code === "manifest_missing_tests") && e2eTestGenAvailable && e2eAutoTriggerOptedIn) {
const e2eTargetKey = `${args.repoFullName}#${args.pr.number}`;
// Double-generation guard: an unchanged head SHA re-entering this pass (a re-review/sweep tick, not a
// new push) must never re-spend an LLM call or repost a duplicate suggestion. A genuinely NEW push
Expand Down Expand Up @@ -11874,7 +11871,7 @@ async function maybePublishPrPublicSurface(
// #4589: only rendered when there's an actual gap AND the checkbox would work for this repo -- same
// condition testCoverageBody gates its own (informational) collapsible on, so the two always agree.
...(missingTestsFinding && e2eTestGenAvailable
? { generateTestsLabel: `${PR_PANEL_GENERATE_TESTS_MARKER} Generate an AI Playwright test for this PR` }
? { generateTestsLabel: `${PR_PANEL_GENERATE_TESTS_MARKER} **[BETA]** Generate an AI Playwright test for this PR` }
: {}),
...(beforeAfter.length > 0 ? { beforeAfter } : {}),
...(changedFilesSummaryEnabledForReview
Expand Down Expand Up @@ -13337,12 +13334,15 @@ async function maybeProcessPrPanelRetrigger(
* `runE2eTestGenerationAndDeliver` core `@gittensory generate-tests` (#4195) and the `manifest_missing_tests`
* auto-trigger (#4196) already use, rather than a full panel re-render.
*
* Hardcoded to `commandAuthorization: PR_PANEL_GENERATE_TESTS_COMMAND_AUTHORIZATION` (maintainer-only, one
* tier narrower than the retrigger's own maintainer+collaborator floor) regardless of what a repo's own
* `.gittensory.yml` might configure for the text-command version of `generate-tests` — a one-click checkbox
* must never be wider than the deliberately narrow default the text command itself already has. An
* unauthorized click is a SILENT no-op (no comment fetch, no patch, no revert, no explanation) — audit-logged
* only, exactly mirroring `maybeProcessPrPanelRetrigger`'s own denial behavior above.
* Authorization uses the repo's OWN `settings.commandAuthorization` — same as the text-command version of
* `generate-tests` above, and configurable like every other command (#4589 follow-up: this used to hardcode a
* maintainer-only override here, overriding whatever `.gittensory.yml` configured; a self-hoster who wants
* contributors/confirmed miners to trigger test generation can now widen it there instead). Out of the box —
* no override configured — this still resolves to maintainer-only, since `DEFAULT_COMMAND_AUTHORIZATION_POLICY`
* already restricts `generate-tests` to `["maintainer"]` and `normalizeCommandRoleList` clamps any configured
* widening to `maintainer`/`collaborator`/`confirmed_miner` (the spoofable raw `pr_author` role is always
* dropped for this command). An unauthorized click is a SILENT no-op (no comment fetch, no patch, no revert, no
* explanation) — audit-logged only, exactly mirroring `maybeProcessPrPanelRetrigger`'s own denial behavior above.
*/
async function maybeProcessPrPanelGenerateTests(
env: Env,
Expand Down Expand Up @@ -13389,7 +13389,7 @@ async function maybeProcessPrPanelGenerateTests(
issue,
actor,
commandName: "generate-tests" as GittensoryMentionCommandName,
settings: { ...settings, commandAuthorization: PR_PANEL_GENERATE_TESTS_COMMAND_AUTHORIZATION },
settings,
pr,
needsMinerDetection: false,
});
Expand All @@ -13404,7 +13404,7 @@ async function maybeProcessPrPanelGenerateTests(
deliveryId,
repoFullName,
commentId: comment.id,
allowedRoles: commandAuthorizationAllowedRoles(PR_PANEL_GENERATE_TESTS_COMMAND_AUTHORIZATION, "generate-tests"),
allowedRoles: commandAuthorizationAllowedRoles(settings.commandAuthorization, "generate-tests"),
},
});
await recordGithubProductUsage(env, "e2e_tests_generation_denied", {
Expand Down
10 changes: 10 additions & 0 deletions src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,16 @@ export function resolveReviewMemoryManifestToggle(manifest: FocusManifest | null
return manifest?.review.reviewMemory === true;
}

/** Resolve `review.e2e_test_auto_trigger` (#4196, part of the #4189 epic) from a possibly-null manifest (null =
* load failure ⇒ reads as unset/false). Mirrors `resolveReviewMemoryManifestToggle` exactly — true ONLY when the
* manifest explicitly set `review.e2e_test_auto_trigger: true`; null/false/absent ⇒ false. The caller ADDITIONALLY
* requires `features.e2eTests` to already be enabled for this repo (via `resolveConvergedFeature`) — this toggle
* alone never activates generation, it only decides whether an already-enabled repo also gets the unprompted
* `manifest_missing_tests` auto-trigger on top of the maintainer-initiated command/checkbox paths. */
export function resolveE2eTestAutoTriggerManifestToggle(manifest: FocusManifest | null): boolean {
return manifest?.review.e2eTestAutoTrigger === true;
}

/** Resolve `review.pre_merge_checks` from a possibly-null manifest (null = load failure ⇒ no checks). Centralized
* so the gate caller resolves them in one place with the null-manifest branch covered here (unit-tested) rather
* than inline in the processor. (#review-pre-merge-checks) */
Expand Down
Loading