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
22 changes: 22 additions & 0 deletions .gittensory.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,28 @@ settings:
# claude_effort: null # Overrides CLAUDE_AI_EFFORT for this repo. String or null. Default (env unset): medium.
# codex_model: null # Overrides CODEX_AI_MODEL for this repo. String or null.
# codex_effort: null # Overrides CODEX_AI_EFFORT for this repo. String or null. Default (env unset): medium.
# # Per-repo before/after screenshot-capture config (#3609 preview / #3610 routes). Only takes effect when
# # the operator has ALSO enabled GITTENSORY_REVIEW_SCREENSHOTS + this repo's cutover allowlist -- this
# # config narrows/redirects that feature, it never turns it on by itself. All-null/empty/default ⇒
# # byte-identical to today (GitHub-native preview discovery, automatic file-to-route inference).
# visual:
# preview:
# # The repo's "after" preview URL, with {number}/{head_sha}/{head_sha_short} placeholders substituted
# # at capture time. ALWAYS wins over GitHub-native preview discovery (Deployments API / commit checks /
# # cloudflare-bot PR comment) when set -- the only option for a provider (e.g. Cloudflare Workers
# # Builds' non-production branch builds) that never surfaces a GitHub-visible deployment at all. Must
# # resolve to a valid HTTPS URL targeting a public host. String or null. Default: null (discovery unchanged).
# url_template: "https://pr-{number}.myapp.workers.dev"
# routes:
# # An explicit, always-screenshotted route list. When non-empty, REPLACES automatic file-to-route
# # inference entirely -- for a repo whose routing convention isn't gittensory-ui's TanStack file-based
# # one. Empty/default ⇒ automatic inference (falling back to "/" when nothing matches).
# paths:
# - "/pricing"
# - "/docs"
# # Overrides the built-in cap (2) on how many routes get screenshotted per PR, whether they come from
# # `paths` above or automatic inference. Positive integer or null. Default: null (built-in default).
# max_routes: 3
# # Maintainer overrides for the public review-panel CONTENT (not what gittensory measures). The
# # Gittensor attribution + register link is always appended to the footer regardless; maintainer text
# # failing the public-safe filter is dropped, never published.
Expand Down
16 changes: 16 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,11 +366,13 @@ import {
resolveReviewPathInstructions,
resolveReviewPreMergeChecks,
resolveReviewPromptOverrides,
resolveReviewVisualConfig,
type FocusManifestFinding,
type FocusManifest,
type ReviewPathInstruction,
type ReviewProfile,
type SelfHostAiModelConfig,
type VisualConfig,
} from "../signals/focus-manifest";
import { decideReviewEligibility } from "../review/review-eligibility";
import {
Expand Down Expand Up @@ -6367,6 +6369,15 @@ export async function resolveReviewManifestForAiReview(
return cachedManifest ?? (await loadRepoFocusManifest(env, repoFullName).catch(() => null));
}

/** Resolve `review.visual` (#3609 preview.url_template / #3610 routes) for the before/after capture pipeline —
* a deterministic, non-AI feature, so it's resolved independently rather than reusing the AI-review manifest
* cache above. Fail-safe: a manifest-load error yields the empty defaults (byte-identical to no config
* configured), matching every other `resolveReview*` accessor's null-manifest behavior. */
export async function resolveVisualCaptureConfig(env: Env, repoFullName: string): Promise<VisualConfig> {
const manifest = await loadRepoFocusManifest(env, repoFullName).catch(() => null);
return resolveReviewVisualConfig(manifest);
}

async function resolveReviewEnrichmentGithubToken(
env: Env,
repoFullName: string,
Expand Down Expand Up @@ -9225,6 +9236,10 @@ async function maybePublishPrPublicSurface(
if (screenshotsAllowed(env, repoFullName) && visualFiles.length > 0) {
try {
const token = await createInstallationToken(env, installationId);
// review.visual (#3609 / #3610): an explicit per-repo preview-URL template / route list. Absent config
// (the default for every repo today) ⇒ EMPTY_VISUAL_CONFIG ⇒ buildCapture's discovery/inference
// behavior is byte-identical to pre-#3609.
const reviewVisualConfig = await resolveVisualCaptureConfig(env, repoFullName);
const capture = await buildCapture(
env,
token,
Expand All @@ -9237,6 +9252,7 @@ async function maybePublishPrPublicSurface(
},
visualFiles,
githubRateLimitAdmissionKeyForInstallation(installationId),
reviewVisualConfig,
);
beforeAfter = capture.routes;
// Visual self-poll: the FIRST capture returns a "loading" placeholder for the AFTER shot when the
Expand Down
99 changes: 74 additions & 25 deletions src/review/visual/capture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,26 @@ function joinUrl(base: string, path: string): string {
return `${base.replace(/\/+$/, "")}${path.startsWith("/") ? path : `/${path}`}`;
}

/** Per-repo `review.visual.preview` config, as resolved by the caller from the manifest (#3609). */
export type VisualPreviewInput = { urlTemplate?: string | null | undefined };

/**
* Substitute `{number}`/`{head_sha}`/`{head_sha_short}` in a `review.visual.preview.url_template` (#3609).
* Pure string substitution — `number` and `headSha` are GitHub-controlled facts about the PR, never
* attacker-supplied free text, so this carries no injection risk regardless of the template's own content
* (which is maintainer-authored and already validated at parse time — see parseVisualUrlTemplate). A missing
* headSha leaves the sha placeholders empty rather than throwing; the resolved URL still goes through the
* SAME isSafeHttpUrl check every other capture URL does (in captureShot), so an unresolved/malformed result
* degrades to a null render, never a crash.
*/
export function resolvePreviewUrlTemplate(template: string, vars: { number: number; headSha?: string | undefined }): string {
const headSha = vars.headSha ?? "";
return template
.split("{number}").join(String(vars.number))
.split("{head_sha_short}").join(headSha.slice(0, 7))
.split("{head_sha}").join(headSha);
}

/**
* Map changed UI files to navigable routes, honoring TanStack Router's file conventions (flat routing uses
* `.` as the path separator; folders use `/`):
Expand All @@ -69,14 +89,30 @@ function joinUrl(base: string, path: string): string {
* posts.$id.tsx -> "/" (dynamic param has no concrete value to render)
* Anything we can't resolve to a concrete path falls back to "/" so we never screenshot a 404.
*/
export function mapFilesToRoutes(files: string[], pattern: RegExp = DEFAULT_ROUTE_FILE): string[] {
export function mapFilesToRoutes(files: string[], pattern: RegExp = DEFAULT_ROUTE_FILE, maxRoutes: number = MAX_ROUTES): string[] {
const routes = new Set<string>();
for (const file of files) {
const match = file.match(pattern);
if (match) routes.add(routeForFile(match[1] as string));
}
if (routes.size === 0) for (const route of DEFAULT_ROUTES) routes.add(route);
return [...routes].slice(0, MAX_ROUTES);
return [...routes].slice(0, maxRoutes);
}

/** Per-repo `review.visual.routes` config, as resolved by the caller from the manifest (#3610). */
export type VisualRoutesInput = { paths?: readonly string[] | null | undefined; maxRoutes?: number | null | undefined };

/**
* Resolve which routes to screenshot for this PR: an explicit, always-screenshotted `paths` list from
* `review.visual.routes` REPLACES automatic file-to-route inference entirely when non-empty (simpler and
* more robust for a repo whose routing convention isn't gittensory-ui's TanStack file-based one); absent/
* empty config falls through to `mapFilesToRoutes` unchanged, so this is byte-identical to today by default.
* `maxRoutes` applies to either path — an explicit list is capped too, not just inferred routes.
*/
export function resolveVisualRoutes(files: string[], config?: VisualRoutesInput | null): string[] {
const maxRoutes = config?.maxRoutes && config.maxRoutes > 0 ? config.maxRoutes : MAX_ROUTES;
if (config?.paths && config.paths.length > 0) return [...config.paths].slice(0, maxRoutes);
return mapFilesToRoutes(files, DEFAULT_ROUTE_FILE, maxRoutes);
}

/** Resolve one TanStack route-file name (extension already stripped) to a navigable path. */
Expand Down Expand Up @@ -133,42 +169,55 @@ async function capturePage(
return { url: onDemand };
}

/** Per-repo `review.visual` config, as resolved by the caller from the manifest (#3609 / #3610). Absent ⇒
* byte-identical to today (GitHub-native discovery, automatic route inference, built-in route cap). */
export type VisualCaptureConfig = { preview?: VisualPreviewInput | null | undefined; routes?: VisualRoutesInput | null | undefined };

/**
* Build the before/after capture for a PR: resolve the preview URL, derive routes from the changed UI files,
* render desktop + mobile before/after for each route, and return the route URL set (for the visual-preview
* collapsible). Fully fail-safe — a missing preview / failed render degrades to placeholders or dashes; this
* NEVER throws (the caller also wraps it in try/catch so a capture failure can't sink a review).
*/
export async function buildCapture(env: Env, token: string, target: CaptureTarget, visualFiles: string[], rateLimitAdmissionKey?: GitHubRateLimitAdmissionKey | undefined): Promise<CaptureResult> {
export async function buildCapture(env: Env, token: string, target: CaptureTarget, visualFiles: string[], rateLimitAdmissionKey?: GitHubRateLimitAdmissionKey | undefined, visualConfig?: VisualCaptureConfig | null | undefined): Promise<CaptureResult> {
const repo = parseRepo(target.repoFullName);
const apiVersion = "2022-11-28";
// before = production (PUBLIC_SITE_ORIGIN, e.g. https://gittensory.aethereal.dev).
const prodBase = env.PUBLIC_SITE_ORIGIN ?? "";

// after = the PR's preview deploy. Prefer the URL carried on the target (a deployment_status webhook set
// it — no extra API call); otherwise look it up from Deployments, then commit checks, then the
// cloudflare-bot PR comment. The lookups also tell us when the latest deploy FAILED (vs is still building)
// so we can show a terminal "deploy failed" card instead of a spinner.
let previewBase = typeof target.previewUrl === "string" ? target.previewUrl : "";
// after = the PR's preview deploy. An explicit review.visual.preview.url_template (#3609) ALWAYS wins —
// a maintainer-configured template is a stronger signal than inference, and is the only option for a
// provider (e.g. Cloudflare Workers Builds' non-production branch builds) that never surfaces a
// GitHub-visible deployment at all. Otherwise, prefer the URL carried on the target (a deployment_status
// webhook set it — no extra API call); otherwise look it up from Deployments, then commit checks, then
// the cloudflare-bot PR comment. The lookups also tell us when the latest deploy FAILED (vs is still
// building) so we can show a terminal "deploy failed" card instead of a spinner.
let previewBase = "";
let previewFailed = target.previewFailed === true;
let previewPending = false;
if (!previewBase && !previewFailed) {
try {
const status = await getLatestDeploymentStatus({ token, repo, sha: target.headSha, ref: target.headRef, apiVersion, rateLimitAdmissionKey });
previewBase = status.url ?? "";
previewFailed = status.failed;
} catch {
previewBase = "";
}
if (!previewBase && !previewFailed && target.previewFromChecks && target.headSha) {
previewBase = (await findPreviewUrlFromChecks({ token, repo, sha: target.headSha, apiVersion, rateLimitAdmissionKey })) ?? "";
if (!previewBase && target.prNumber) {
previewBase = (await findPreviewUrlFromPrComments({ token, repo, prNumber: target.prNumber, apiVersion, rateLimitAdmissionKey })) ?? "";
const urlTemplate = visualConfig?.preview?.urlTemplate;
if (urlTemplate) {
previewBase = resolvePreviewUrlTemplate(urlTemplate, { number: target.prNumber, headSha: target.headSha });
} else {
previewBase = typeof target.previewUrl === "string" ? target.previewUrl : "";
if (!previewBase && !previewFailed) {
try {
const status = await getLatestDeploymentStatus({ token, repo, sha: target.headSha, ref: target.headRef, apiVersion, rateLimitAdmissionKey });
previewBase = status.url ?? "";
previewFailed = status.failed;
} catch {
previewBase = "";
}
if (!previewBase && target.headSha) {
const buildState = await getPreviewBuildState({ token, repo, sha: target.headSha, apiVersion, rateLimitAdmissionKey });
if (buildState === "failed") previewFailed = true;
else if (buildState === "building" || buildState === "succeeded") previewPending = true;
if (!previewBase && !previewFailed && target.previewFromChecks && target.headSha) {
previewBase = (await findPreviewUrlFromChecks({ token, repo, sha: target.headSha, apiVersion, rateLimitAdmissionKey })) ?? "";
if (!previewBase && target.prNumber) {
previewBase = (await findPreviewUrlFromPrComments({ token, repo, prNumber: target.prNumber, apiVersion, rateLimitAdmissionKey })) ?? "";
}
if (!previewBase && target.headSha) {
const buildState = await getPreviewBuildState({ token, repo, sha: target.headSha, apiVersion, rateLimitAdmissionKey });
if (buildState === "failed") previewFailed = true;
else if (buildState === "building" || buildState === "succeeded") previewPending = true;
}
}
}
}
Expand All @@ -180,7 +229,7 @@ export async function buildCapture(env: Env, token: string, target: CaptureTarge
const failedPlaceholder = shotBase ? `${shotBase}/${NAMESPACE}/shot?placeholder=failed` : undefined;
const afterPlaceholder = previewFailed ? failedPlaceholder : loadingPlaceholder;

const routes = mapFilesToRoutes(visualFiles);
const routes = resolveVisualRoutes(visualFiles, visualConfig?.routes);
const captureRoutes: CaptureRoute[] = [];
for (const path of routes) {
const beforePage = prodBase ? joinUrl(prodBase, path) : "";
Expand Down
Loading
Loading