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: 4 additions & 3 deletions .gittensory.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -822,13 +822,14 @@ settings:
# minConfidence: 0.85 # Number 0-1. Minimum AI-verifier confidence to treat a candidate as a real match.

# Before/after screenshot-table gate (#2006): deterministic check that a visual/frontend PR's body has a
# before/after image table. Scoped to whenLabels OR whenPaths (either matches); both empty = every PR. Off
# by default.
# before/after image table -- OR (#4110) that the bot's own visual-capture pipeline (review.visual.enabled)
# already produced a real before/after render for this PR's head, which satisfies the gate on its own.
# Scoped to whenLabels OR whenPaths (either matches); both empty = every PR. Off by default.
# screenshotTableGate:
# enabled: false # Default: false.
# whenLabels: [frontend, visual] # Default: [] (no label scoping).
# whenPaths: ["apps/ui/**", "src/**/*.tsx"] # Default: [] (no path scoping).
# action: close # close | request_changes | comment. Default: close.
# action: close # close is the only supported value. Default: close.
# message: "Custom close reason..." # Default: null (built-in message).

# Maintainer AI review tuning (`.gittensory.yml` top-level `review:` block). These knobs shape the advisory AI
Expand Down
4 changes: 1 addition & 3 deletions apps/gittensory-ui/public/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -9265,9 +9265,7 @@
"action": {
"type": "string",
"enum": [
"close",
"request_changes",
"comment"
"close"
]
},
"message": {
Expand Down
7 changes: 4 additions & 3 deletions config/examples/gittensory.full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -835,13 +835,14 @@ settings:
# minConfidence: 0.85 # Number 0-1. Minimum AI-verifier confidence to treat a candidate as a real match.

# Before/after screenshot-table gate (#2006): deterministic check that a visual/frontend PR's body has a
# before/after image table. Scoped to whenLabels OR whenPaths (either matches); both empty = every PR. Off
# by default.
# before/after image table -- OR (#4110) that the bot's own visual-capture pipeline (review.visual.enabled)
# already produced a real before/after render for this PR's head, which satisfies the gate on its own.
# Scoped to whenLabels OR whenPaths (either matches); both empty = every PR. Off by default.
# screenshotTableGate:
# enabled: false # Default: false.
# whenLabels: [frontend, visual] # Default: [] (no label scoping).
# whenPaths: ["apps/ui/**", "src/**/*.tsx"] # Default: [] (no path scoping).
# action: close # close | request_changes | comment. Default: close.
# action: close # close is the only supported value. Default: close.
# message: "Custom close reason..." # Default: null (built-in message).

# Maintainer AI review tuning (`.gittensory.yml` top-level `review:` block). These knobs shape the advisory AI
Expand Down
13 changes: 13 additions & 0 deletions migrations/0125_pull_request_visual_capture_satisfied_sha.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
-- Visual-capture gate satisfaction (#4110, visual-capture convergence epic #3607). The bot's before/after
-- capture pipeline (review.visual.enabled, #4093) can now satisfy the deterministic screenshotTableGate
-- (#2006) exactly like a hand-authored before/after table -- but the capture is computed and persisted by the
-- public-surface publish pass (maybePublishPrPublicSurface), which runs BEFORE the maintenance/gate pass
-- (maybeRunAgentMaintenance) re-reads this same PR row. Persisting the marker lets the maintenance pass see
-- "did the bot already prove this PR visually?" without re-running the capture or threading a new return value
-- through every caller of either function.
--
-- visual_capture_satisfied_sha is the head SHA at which the capture pipeline last produced a REAL before+after
-- render pair (not a placeholder/failed/pending shot) -- scoped to head SHA (mirrors approved_head_sha, 0053 /
-- last_published_surface_sha, 0080: a new commit re-arms the requirement until capture succeeds again for the
-- new head).
ALTER TABLE pull_requests ADD COLUMN visual_capture_satisfied_sha TEXT;
15 changes: 12 additions & 3 deletions packages/gittensory-engine/src/review/screenshot-table-gate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const DEFAULT_SCREENSHOT_TABLE_GATE: ScreenshotTableGateConfig = {
action: "close",
};

const VALID_ACTIONS: readonly ScreenshotTableGateAction[] = ["close", "request_changes", "comment"];
const VALID_ACTIONS: readonly ScreenshotTableGateAction[] = ["close"];

export function isScreenshotTableGateAction(value: unknown): value is ScreenshotTableGateAction {
return typeof value === "string" && (VALID_ACTIONS as readonly string[]).includes(value);
Expand Down Expand Up @@ -72,7 +72,7 @@ export function normalizeScreenshotTableGateConfig(input: unknown, warnings: str
const action = isScreenshotTableGateAction(record.action)
? record.action
: (() => {
if (record.action !== undefined) warnings.push(`settings.requireScreenshotTable.action must be one of close, request_changes, comment; using the default "close".`);
if (record.action !== undefined) warnings.push(`settings.requireScreenshotTable.action must be "close" (the only supported value; #4110 removed request_changes/comment as dead config surface); using the default "close".`);
return DEFAULT_SCREENSHOT_TABLE_GATE.action;
})();
const message = typeof record.message === "string" && record.message.trim().length > 0 ? record.message.trim() : undefined;
Expand Down Expand Up @@ -184,16 +184,25 @@ const NO_VIOLATION: ScreenshotTableGateResult = { violated: false, reason: null

/** PURE evaluator. Off (`enabled: false`) or out-of-scope (no configured label/path match) ⇒ no violation. In
* scope AND (no image-bearing table in the body OR an image pasted outside a table OR a committed image file
* under a scoped path) ⇒ violated, with the configured (or default) templated message as the reason. */
* under a scoped path), UNLESS `botCaptureSatisfied` ⇒ violated, with the configured (or default) templated
* message as the reason. */
export function evaluateScreenshotTableGate(input: {
config: ScreenshotTableGateConfig;
prBody: string | null | undefined;
prLabels: string[];
changedFiles: string[];
/** #4110: true when the bot's own before/after capture pipeline (review.visual.enabled) already produced a
* REAL before+after render pair for this PR's current head — evidence equivalent to a hand-authored table.
* A successful automated capture satisfies the gate on its own, ahead of (and regardless of) the body-table
* anti-gaming checks below — those exist to stop a contributor from FAKING compliance without the bot's
* help, which doesn't apply once the bot has already proven the change visually. Absent/false ⇒
* byte-identical to pre-#4110 behavior (body-table evidence only). */
botCaptureSatisfied?: boolean | undefined;
}): ScreenshotTableGateResult {
const { config } = input;
if (!config.enabled) return NO_VIOLATION;
if (!isScreenshotTableGateInScope(config, input.prLabels, input.changedFiles)) return NO_VIOLATION;
if (input.botCaptureSatisfied === true) return NO_VIOLATION;
const hasTable = hasImageBearingMarkdownTable(input.prBody);
const outsideTable = hasImageOutsideTable(input.prBody);
const committedImage = hasCommittedImageFile(input.changedFiles, config.whenPaths);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ export type CombineStrategy = "single" | "consensus" | "synthesis";

export type OnMerge = "either" | "both";

export type ScreenshotTableGateAction = "close" | "request_changes" | "comment";
// #4110: `request_changes`/`comment` were REMOVED (see src/types.ts's mirror of this type for why) -- `"close"`
// is the only value this gate has ever enforced.
export type ScreenshotTableGateAction = "close";

export type ScreenshotTableGateConfig = {
enabled: boolean;
Expand Down
14 changes: 14 additions & 0 deletions src/db/repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3657,6 +3657,19 @@ export async function markPullRequestSurfacePublished(env: Env, fullName: string
.where(and(eq(pullRequests.repoFullName, fullName), eq(pullRequests.number, number), eq(pullRequests.headSha, headSha)));
}

/** Visual-capture gate satisfaction (#4110): record the head SHA at which the bot's before/after capture
* pipeline just produced a REAL before+after render pair for this PR (see `hasSuccessfulBotCapture`,
* `review/visual/capture.ts`). The screenshotTableGate evaluator treats `visualCaptureSatisfiedSha ===
* headSha` as evidence equivalent to a hand-authored table. Scoped to headSha (mirrors markPullRequestApproved)
* so a later commit re-arms the requirement until capture succeeds again for the new head. */
export async function markPullRequestVisualCaptureSatisfied(env: Env, fullName: string, number: number, headSha: string): Promise<void> {
const db = getDb(env.DB);
await db
.update(pullRequests)
.set({ visualCaptureSatisfiedSha: headSha, updatedAt: nowIso() })
.where(and(eq(pullRequests.repoFullName, fullName), eq(pullRequests.number, number), eq(pullRequests.headSha, headSha)));
}

/** Sweep convergence: stamp the timestamp the scheduled re-gate sweep just recomputed this PR. A plain D1 UPDATE
* — NOT routed through the agent-action-executor chokepoint (#1258) — so it advances even when GitHub writes are
* suppressed (dry-run / paused). selectRegateCandidates orders the sweep by last_regated_at, so a just-regated PR
Expand Down Expand Up @@ -5799,6 +5812,7 @@ function toPullRequestRecordFromRow(row: typeof pullRequests.$inferSelect): Pull
lastPublishedSurfaceSha: row.lastPublishedSurfaceSha,
linkedIssueHardRuleViolatedAt: row.linkedIssueHardRuleViolatedAt,
linkedIssueHardRuleViolationReason: row.linkedIssueHardRuleViolationReason,
visualCaptureSatisfiedSha: row.visualCaptureSatisfiedSha,
};
}

Expand Down
8 changes: 8 additions & 0 deletions src/db/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,14 @@ export const pullRequests = sqliteTable(
// pairing with merge_blocked_sha) -- so a later close can still cite the concrete rule even if the live
// re-parse can no longer reproduce it (the issue was unlinked or its state changed).
linkedIssueHardRuleViolationReason: text("linked_issue_hard_rule_violation_reason"),
// Visual-capture gate satisfaction (#4110): the head SHA at which the bot's before/after capture pipeline
// (review.visual.enabled) last produced a REAL before+after render pair (not a placeholder/failed/pending
// shot) for this PR. Lets the deterministic screenshotTableGate treat a successful automated capture as
// equivalent evidence to a hand-authored before/after table. Keyed to head SHA (mirrors approved_head_sha /
// last_published_surface_sha) -- a new commit re-arms the requirement until capture succeeds again for the
// new head. gittensory-computed (publish-written), omitted from the GitHub-sync SET clause so a later sync
// cannot clobber it.
visualCaptureSatisfiedSha: text("visual_capture_satisfied_sha"),
createdAt: text("created_at").notNull().$defaultFn(() => nowIso()),
updatedAt: text("updated_at").notNull().$defaultFn(() => nowIso()),
},
Expand Down
2 changes: 1 addition & 1 deletion src/openapi/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,7 @@ export const RepositorySettingsSchema = z
enabled: z.boolean(),
whenLabels: z.array(z.string()),
whenPaths: z.array(z.string()),
action: z.enum(["close", "request_changes", "comment"]),
action: z.enum(["close"]),
message: z.string().optional(),
})
.optional(),
Expand Down
34 changes: 29 additions & 5 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import {
markPullRequestsRegated,
markPullRequestReviewsInvalidated,
markPullRequestSurfacePublished,
markPullRequestVisualCaptureSatisfied,
getLatestRegatedAt,
claimRegateFanoutSlot,
recordAgentCommandFeedback,
Expand Down Expand Up @@ -358,7 +359,7 @@ import { randomUUID } from "node:crypto";
import { isRetryableJobError, RetryableJobError } from "./retryable";
import { screenshotsAllowed } from "../review/visual-wire";
import { isVisualPath } from "../review/visual/paths";
import { buildCapture, type CaptureRoute } from "../review/visual/capture";
import { buildCapture, hasSuccessfulBotCapture, type CaptureRoute } from "../review/visual/capture";
import { incr } from "../selfhost/metrics";
import {
renderReviewingPlaceholder,
Expand Down Expand Up @@ -2778,17 +2779,22 @@ async function runAgentMaintenancePlanAndExecute(
);

// Screenshot-table gate (#2006): a DETERMINISTIC check (no AI) that an in-scope (label/path-matched)
// contributor visual/frontend PR's body contains a before/after screenshot table. Off by default
// (settings.screenshotTableGate.enabled === false), so the pure evaluator below is effectively free for the
// common case. Only "close" is wired as an enforcement action here (the other configured actions stay
// advisory, matching the issue's phased rollout) -- the ternary below is the ONLY place that reads `.action`.
// contributor visual/frontend PR's body contains a before/after screenshot table -- OR (#4110) that the
// bot's own visual-capture pipeline already produced a real before/after render for this exact head
// (markPullRequestVisualCaptureSatisfied, written earlier in this same webhook by maybePublishPrPublicSurface
// -- see that function's beforeAfter block -- and re-read here on `pr`, which this caller already re-fetched
// fresh from the DB). Off by default (settings.screenshotTableGate.enabled === false), so the pure evaluator
// below is effectively free for the common case. "close" is the only enforcement action this gate has (#4110
// removed the dead request_changes/comment surface) -- the check below is the ONLY place that reads `.action`.
/* v8 ignore next -- defensive: resolveRepositorySettings always populates screenshotTableGate (getRepositorySettings's DB defaults), so this fallback is unreachable in practice. */
const screenshotTableGateConfig = settings.screenshotTableGate ?? DEFAULT_SCREENSHOT_TABLE_GATE;
const botCaptureSatisfied = Boolean(pr.headSha) && pr.visualCaptureSatisfiedSha === pr.headSha;
const screenshotTableGateResult = evaluateScreenshotTableGate({
config: screenshotTableGateConfig,
prBody: pr.body,
prLabels: pr.labels,
changedFiles: changedPaths,
botCaptureSatisfied,
});
const screenshotTableMatch =
screenshotTableGateResult.violated && screenshotTableGateConfig.action === "close"
Expand Down Expand Up @@ -10260,6 +10266,24 @@ async function maybePublishPrPublicSurface(
? { routes: [], previewPending: false }
: await buildCapture(env, token, captureTarget, visualFiles, githubRateLimitAdmissionKeyForInstallation(installationId), reviewVisualConfig);
beforeAfter = capture.routes;
// Screenshot-table gate satisfaction (#4110): a successful capture (a real before+after render pair
// on at least one route) is evidence equivalent to a hand-authored before/after table -- persist the
// head SHA it was proven at so the LATER maintenance pass (runAgentMaintenancePlanAndExecute, which
// re-reads this PR row fresh) can see it without re-running the capture or threading a new return
// value through every caller of this function. Best-effort: a write failure here just means the gate
// falls back to requiring a body table, never blocks the rest of the review.
if (pr.headSha && hasSuccessfulBotCapture(beforeAfter)) {
await markPullRequestVisualCaptureSatisfied(env, repoFullName, pr.number, pr.headSha).catch((error) => {
console.log(
JSON.stringify({
event: "visual_capture_satisfied_mark_failed",
repoFullName,
pull: pr.number,
message: errorMessage(error).slice(0, 200),
}),
);
});
}
// Visual self-poll: the FIRST capture returns a "loading" placeholder for the AFTER shot when the
// preview deploy isn't live yet (capture.previewPending). Schedule a delayed re-review to re-capture
// the now-ready shot — bounded by `attempt` so a never-resolving preview can't loop (the deployment_status
Expand Down
15 changes: 12 additions & 3 deletions src/review/screenshot-table-gate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const DEFAULT_SCREENSHOT_TABLE_GATE: ScreenshotTableGateConfig = {
action: "close",
};

const VALID_ACTIONS: readonly ScreenshotTableGateAction[] = ["close", "request_changes", "comment"];
const VALID_ACTIONS: readonly ScreenshotTableGateAction[] = ["close"];

export function isScreenshotTableGateAction(value: unknown): value is ScreenshotTableGateAction {
return typeof value === "string" && (VALID_ACTIONS as readonly string[]).includes(value);
Expand Down Expand Up @@ -72,7 +72,7 @@ export function normalizeScreenshotTableGateConfig(input: unknown, warnings: str
const action = isScreenshotTableGateAction(record.action)
? record.action
: (() => {
if (record.action !== undefined) warnings.push(`settings.requireScreenshotTable.action must be one of close, request_changes, comment; using the default "close".`);
if (record.action !== undefined) warnings.push(`settings.requireScreenshotTable.action must be "close" (the only supported value; #4110 removed request_changes/comment as dead config surface); using the default "close".`);
return DEFAULT_SCREENSHOT_TABLE_GATE.action;
})();
const message = typeof record.message === "string" && record.message.trim().length > 0 ? record.message.trim() : undefined;
Expand Down Expand Up @@ -184,16 +184,25 @@ const NO_VIOLATION: ScreenshotTableGateResult = { violated: false, reason: null

/** PURE evaluator. Off (`enabled: false`) or out-of-scope (no configured label/path match) ⇒ no violation. In
* scope AND (no image-bearing table in the body OR an image pasted outside a table OR a committed image file
* under a scoped path) ⇒ violated, with the configured (or default) templated message as the reason. */
* under a scoped path), UNLESS `botCaptureSatisfied` ⇒ violated, with the configured (or default) templated
* message as the reason. */
export function evaluateScreenshotTableGate(input: {
config: ScreenshotTableGateConfig;
prBody: string | null | undefined;
prLabels: string[];
changedFiles: string[];
/** #4110: true when the bot's own before/after capture pipeline (review.visual.enabled) already produced a
* REAL before+after render pair for this PR's current head — evidence equivalent to a hand-authored table.
* A successful automated capture satisfies the gate on its own, ahead of (and regardless of) the body-table
* anti-gaming checks below — those exist to stop a contributor from FAKING compliance without the bot's
* help, which doesn't apply once the bot has already proven the change visually. Absent/false ⇒
* byte-identical to pre-#4110 behavior (body-table evidence only). */
botCaptureSatisfied?: boolean | undefined;
}): ScreenshotTableGateResult {
const { config } = input;
if (!config.enabled) return NO_VIOLATION;
if (!isScreenshotTableGateInScope(config, input.prLabels, input.changedFiles)) return NO_VIOLATION;
if (input.botCaptureSatisfied === true) return NO_VIOLATION;
const hasTable = hasImageBearingMarkdownTable(input.prBody);
const outsideTable = hasImageOutsideTable(input.prBody);
const committedImage = hasCommittedImageFile(input.changedFiles, config.whenPaths);
Expand Down
Loading
Loading