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
29 changes: 28 additions & 1 deletion packages/gittensory-engine/src/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,13 @@ export const EMPTY_SELF_HOST_AI_MODEL_CONFIG: SelfHostAiModelConfig = {
/** Per-repo before/after screenshot-capture config under `review.visual` (#3609 / #3610). Generic by design —
* every self-hoster wires their OWN repo's preview-deploy setup and route shape with config, not code. */
export type VisualConfig = {
/** `review.visual.production_url`: the repo's "before" production URL — e.g. `https://metagraph.sh` for a
* repo whose live site differs from the operator's own `PUBLIC_SITE_ORIGIN` env var (a single GLOBAL value
* with no per-repo awareness, correct for at most one repo on a multi-repo self-host instance). ALWAYS wins
* over `PUBLIC_SITE_ORIGIN` when set, mirroring `preview.url_template`'s precedence over GitHub-native
* discovery. null (default) ⇒ byte-identical to today (falls back to `PUBLIC_SITE_ORIGIN`). Validated at
* parse time against the same SSRF guard (`isSafeHttpUrl`) the renderer itself unconditionally applies. */
productionUrl: string | null;
preview: VisualPreviewConfig;
routes: VisualRoutesConfig;
themes: VisualTheme[];
Expand Down Expand Up @@ -787,6 +794,7 @@ export type VisualRoutesConfig = {
};

export const EMPTY_VISUAL_CONFIG: VisualConfig = {
productionUrl: null,
preview: { urlTemplate: null },
routes: { paths: [], maxRoutes: null },
themes: [],
Expand Down Expand Up @@ -2301,6 +2309,7 @@ function overlaySelfHostAiModelConfig(base: SelfHostAiModelConfig, override: Sel

function overlayVisualConfig(base: VisualConfig, override: VisualConfig): VisualConfig {
return {
productionUrl: pickOverlayNullable(override.productionUrl, base.productionUrl),
preview: { urlTemplate: pickOverlayNullable(override.preview.urlTemplate, base.preview.urlTemplate) },
routes: {
paths: pickOverlayStringList(override.routes.paths, base.routes.paths),
Expand Down Expand Up @@ -2543,6 +2552,7 @@ function parseSelfHostAiModelConfig(value: JsonValue | undefined, warnings: stri

function visualConfigPresent(config: VisualConfig): boolean {
return (
config.productionUrl !== null ||
config.preview.urlTemplate !== null ||
config.routes.paths.length > 0 ||
config.routes.maxRoutes !== null ||
Expand Down Expand Up @@ -2588,6 +2598,20 @@ const VISUAL_URL_TEMPLATE_DUMMY_VARS: Record<string, string> = {
"{head_sha}": "0000000000000000000000000000000000000000",
};

/** Parse `review.visual.production_url` — validated at CONFIG-READ time against the exact same SSRF guard
* (`isSafeHttpUrl`) the renderer itself unconditionally applies to every URL it navigates to. Unlike
* `preview.url_template`, this is a plain static origin with no `{number}`/`{head_sha}` placeholders to
* substitute — the "before" shot is always the SAME production page, just at a different path per route. */
function parseVisualProductionUrl(value: JsonValue | undefined, warnings: string[]): string | null {
const url = parsePublicSafeText(value, "review.visual.production_url", warnings);
if (url === null) return null;
if (!isSafeHttpUrl(url)) {
warnings.push(`Manifest "review.visual.production_url" must be a valid HTTPS URL targeting a public host; ignoring it.`);
return null;
}
return url;
}

/** Parse `review.visual.preview.url_template` — validated at CONFIG-READ time against the exact same SSRF
* guard (`isSafeHttpUrl`) the renderer itself unconditionally applies to every URL it navigates to,
* regardless of source (`src/review/visual/shot.ts`). This is deliberately redundant with that runtime
Expand Down Expand Up @@ -2617,6 +2641,8 @@ function parseVisualConfig(value: JsonValue | undefined, warnings: string[]): Vi
}
const record = value as Record<string, JsonValue>;

const productionUrl = parseVisualProductionUrl(record.production_url, warnings);

const previewRecord = record.preview !== null && typeof record.preview === "object" && !Array.isArray(record.preview) ? (record.preview as Record<string, JsonValue>) : undefined;
if (record.preview !== undefined && record.preview !== null && previewRecord === undefined) {
warnings.push(`Manifest "review.visual.preview" must be a mapping; ignoring it.`);
Expand All @@ -2636,7 +2662,7 @@ function parseVisualConfig(value: JsonValue | undefined, warnings: string[]): Vi
const themeStorageKey = parsePublicSafeText(record.theme_storage_key, "review.visual.theme_storage_key", warnings);
const actionsFallback = normalizeOptionalBoolean(record.actions_fallback, "review.visual.actions_fallback", warnings) === true;

return { preview: { urlTemplate }, routes: { paths, maxRoutes }, themes, gif, enabled, themeStorageKey, actionsFallback };
return { productionUrl, preview: { urlTemplate }, routes: { paths, maxRoutes }, themes, gif, enabled, themeStorageKey, actionsFallback };
}

function parseAutoReviewTitleKeywords(value: JsonValue | undefined, warnings: string[]): string[] {
Expand Down Expand Up @@ -2943,6 +2969,7 @@ export function reviewConfigToJson(review: FocusManifestReviewConfig): JsonValue
}
if (visualConfigPresent(review.visual)) {
const visual: Record<string, JsonValue> = {};
if (review.visual.productionUrl !== null) visual.production_url = review.visual.productionUrl;
if (review.visual.preview.urlTemplate !== null) visual.preview = { url_template: review.visual.preview.urlTemplate };
if (review.visual.routes.paths.length > 0 || review.visual.routes.maxRoutes !== null) {
const routes: Record<string, JsonValue> = {};
Expand Down
20 changes: 16 additions & 4 deletions src/review/visual/capture.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Realtime visual capture (reviewbot→gittensory convergence — visual port). taopedia-style before/after.
//
// before = production (PUBLIC_SITE_ORIGIN); after = the PR's preview-deploy URL, discovered the
// before = production (review.visual.production_url, falling back to the global PUBLIC_SITE_ORIGIN env var);
// after = the PR's preview-deploy URL, discovered the
// provider-agnostic way (Deployments API → commit checks → cloudflare-bot PR comment). Each page is
// rendered once here (in the queue consumer, which has the time budget), stored as a PNG in R2
// (env.REVIEW_AUDIT), and embedded either as <PUBLIC_API_ORIGIN>/gittensory/shot?key=<r2key> (this
Expand Down Expand Up @@ -30,7 +31,11 @@ import { encodeScrollGif, isScrollGifAvailable } from "./scroll-gif";

const NAMESPACE = "gittensory";
const DEFAULT_ROUTES = ["/"];
const DEFAULT_ROUTE_FILE = /apps\/gittensory-ui\/src\/routes\/(.+?)\.(?:tsx|jsx)$/i;
// The app-folder segment is a wildcard, not hardcoded to gittensory-ui: metagraphed's UI (apps/ui/src/routes/)
// uses the identical TanStack flat-file convention `routeForFile` below implements, just under a different app
// folder name. Only ever matched against the CURRENT repo's own changed-file paths (see mapFilesToRoutes'
// caller), so widening this carries no cross-repo ambiguity risk.
const DEFAULT_ROUTE_FILE = /apps\/[^/]+\/src\/routes\/(.+?)\.(?:tsx|jsx)$/i;
// Each route renders desktop + mobile for before + after (up to 4 PNGs). Cap routes to bound browser-render
// wall-clock — Browser Rendering is the costliest binding.
const MAX_ROUTES = 2;
Expand Down Expand Up @@ -368,6 +373,11 @@ async function captureScrollGif(
* #3612 / #4109). Absent ⇒ byte-identical to today (GitHub-native discovery, automatic route inference,
* single default-theme capture, built-in route cap, no scroll-GIF, no localStorage theme forcing). */
export type VisualCaptureConfig = {
/** `review.visual.production_url` (#3611 follow-up): overrides `env.PUBLIC_SITE_ORIGIN` (a single GLOBAL
* value with no per-repo awareness) as the "before" base for THIS repo. ALWAYS wins when set, mirroring
* `preview.urlTemplate`'s precedence over discovery. null/undefined ⇒ falls back to `env.PUBLIC_SITE_ORIGIN`,
* byte-identical to today. */
productionUrl?: string | null | undefined;
preview?: VisualPreviewInput | null | undefined;
routes?: VisualRoutesInput | null | undefined;
themes?: readonly ShotTheme[] | null | undefined;
Expand All @@ -391,8 +401,10 @@ export type VisualCaptureConfig = {
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 ?? "";
// before = production. review.visual.production_url (#3611 follow-up) ALWAYS wins when set -- PUBLIC_SITE_ORIGIN
// is a single GLOBAL env var (e.g. https://gittensory.aethereal.dev) with no per-repo awareness, correct for at
// most one repo on a multi-repo self-host instance; every other repo needs its own override here.
const prodBase = visualConfig?.productionUrl ? visualConfig.productionUrl : (env.PUBLIC_SITE_ORIGIN ?? "");

// 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
Expand Down
12 changes: 7 additions & 5 deletions src/review/visual/paths.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
// Visual-path classifier (reviewbot→gittensory convergence — visual capture port).
//
// PORTED VERBATIM from reviewbot's src/agents/gittensory/capabilities.ts `isVisualPath` (the three
// VISUAL_PATTERNS). This is the EMPHATIC gate: screenshots fire ONLY for WEB-VISIBLE changes — a
// frontend page (apps/gittensory-ui/**), a public asset (public/**, e.g. an OG image), or a
// front-of-house source extension (.tsx/.jsx/.css/.scss/.sass/.less/.html/.svg/.astro/.vue/.svelte/.mdx).
// A backend change (.ts/.md/.json/.py/...) matches NONE of these, so capture never triggers for it.
// VISUAL_PATTERNS), with the first pattern's app-folder segment widened to a wildcard (#3611 follow-up) so it
// isn't gittensory-ui-only — see capture.ts's DEFAULT_ROUTE_FILE for the same generalization. This is the
// EMPHATIC gate: screenshots fire ONLY for WEB-VISIBLE changes — any frontend app folder (apps/*/**, e.g.
// apps/gittensory-ui/** or apps/ui/**), a public asset (public/**, e.g. an OG image), or a front-of-house
// source extension (.tsx/.jsx/.css/.scss/.sass/.less/.html/.svg/.astro/.vue/.svelte/.mdx). A backend change
// (.ts/.md/.json/.py/...) matches NONE of these, so capture never triggers for it.
//
// PURE — no imports, no I/O. Callers MUST filter changed files through this before any capture.

const VISUAL_PATTERNS: RegExp[] = [
/^apps\/gittensory-ui\//i,
/^apps\/[^/]+\//i,
/(^|\/)public\//i,
/\.(tsx|jsx|css|scss|sass|less|html|svg|astro|vue|svelte|mdx)$/i,
];
Expand Down
70 changes: 66 additions & 4 deletions test/unit/focus-manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3988,6 +3988,7 @@ describe("review.visual (#3609 preview.url_template / #3610 routes)", () => {
},
});
expect(m.review.visual).toEqual({
productionUrl: null,
preview: { urlTemplate: "https://pr-{number}.preview.example.com" },
routes: { paths: ["/pricing", "/docs"], maxRoutes: 3 },
themes: [],
Expand Down Expand Up @@ -4090,7 +4091,68 @@ describe("review.visual (#3609 preview.url_template / #3610 routes)", () => {
it("resolveReviewVisualConfig: null manifest yields empty defaults; a set manifest passes through", () => {
expect(resolveReviewVisualConfig(null)).toEqual({ ...EMPTY_VISUAL_CONFIG });
const manifest = parseFocusManifest({ review: { visual: { routes: { paths: ["/app"] } } } });
expect(resolveReviewVisualConfig(manifest)).toEqual({ preview: { urlTemplate: null }, routes: { paths: ["/app"], maxRoutes: null }, themes: [], gif: false, enabled: null, themeStorageKey: null, actionsFallback: false });
expect(resolveReviewVisualConfig(manifest)).toEqual({ productionUrl: null, preview: { urlTemplate: null }, routes: { paths: ["/app"], maxRoutes: null }, themes: [], gif: false, enabled: null, themeStorageKey: null, actionsFallback: false });
});
});

describe("review.visual.production_url (#3611 follow-up — per-repo override of the global PUBLIC_SITE_ORIGIN env var)", () => {
it("parses a valid production_url, marks present, and round-trips", () => {
const m = parseFocusManifest({ review: { visual: { production_url: "https://metagraph.sh" } } });
expect(m.review.visual.productionUrl).toBe("https://metagraph.sh");
expect(m.review.present).toBe(true);
expect(reviewConfigToJson(m.review)).toEqual({ visual: { production_url: "https://metagraph.sh" } });
});

it("absent production_url stays null and does not mark review present on its own", () => {
expect(parseFocusManifest({}).review.visual.productionUrl).toBeNull();
expect(parseFocusManifest({ review: { visual: {} } }).review.present).toBe(false);
});

it("rejects a non-HTTPS production_url with a warning", () => {
const bad = parseFocusManifest({ review: { visual: { production_url: "http://metagraph.sh" } } });
expect(bad.review.visual.productionUrl).toBeNull();
expect(bad.warnings.some((w) => /review\.visual\.production_url.*valid HTTPS URL/.test(w))).toBe(true);
});

it("rejects a production_url resolving to a private/internal host with a warning", () => {
const bad = parseFocusManifest({ review: { visual: { production_url: "https://prod.internal" } } });
expect(bad.review.visual.productionUrl).toBeNull();
expect(bad.warnings.some((w) => /review\.visual\.production_url.*valid HTTPS URL/.test(w))).toBe(true);
});

it("rejects a malformed production_url with a warning", () => {
const bad = parseFocusManifest({ review: { visual: { production_url: "not-a-url-at-all" } } });
expect(bad.review.visual.productionUrl).toBeNull();
expect(bad.warnings.some((w) => /review\.visual\.production_url.*valid HTTPS URL/.test(w))).toBe(true);
});

it("composes with preview.url_template — both configured independently and both round-trip", () => {
const m = parseFocusManifest({
review: { visual: { production_url: "https://metagraph.sh", preview: { url_template: "https://pr-{number}.example.com" } } },
});
expect(m.review.visual.productionUrl).toBe("https://metagraph.sh");
expect(m.review.visual.preview.urlTemplate).toBe("https://pr-{number}.example.com");
expect(reviewConfigToJson(m.review)).toEqual({
visual: { production_url: "https://metagraph.sh", preview: { url_template: "https://pr-{number}.example.com" } },
});
});

it("resolveReviewVisualConfig passes a configured production_url through", () => {
const manifest = parseFocusManifest({ review: { visual: { production_url: "https://metagraph.sh" } } });
expect(resolveReviewVisualConfig(manifest).productionUrl).toBe("https://metagraph.sh");
});

it("overlay: a per-repo production_url wins over a global-default value", () => {
const globalDefault = parseReviewConfigMapping({ visual: { production_url: "https://gittensory.aethereal.dev" } }, []);
const perRepo = parseReviewConfigMapping({ visual: { production_url: "https://metagraph.sh" } }, []);
expect(overlayReviewConfig(globalDefault, perRepo).visual.productionUrl).toBe("https://metagraph.sh");
});

it("overlay: an unset per-repo production_url falls back to the global-default value", () => {
const globalDefault = parseReviewConfigMapping({ visual: { production_url: "https://gittensory.aethereal.dev" } }, []);
const perRepo = parseReviewConfigMapping({ visual: { routes: { paths: ["/app"] } } }, []);
expect(overlayReviewConfig(globalDefault, perRepo).visual.productionUrl).toBe("https://gittensory.aethereal.dev");
expect(overlayReviewConfig(globalDefault, perRepo).visual.routes.paths).toEqual(["/app"]);
});
});

Expand Down Expand Up @@ -4175,7 +4237,7 @@ describe("review.visual.gif (#3612 scroll-through GIF capture)", () => {

it("composes with themes — both configured independently and both round-trip", () => {
const m = parseFocusManifest({ review: { visual: { gif: true, themes: ["dark"] } } });
expect(m.review.visual).toEqual({ preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: ["dark"], gif: true, enabled: null, themeStorageKey: null, actionsFallback: false });
expect(m.review.visual).toEqual({ productionUrl: null, preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: ["dark"], gif: true, enabled: null, themeStorageKey: null, actionsFallback: false });
expect(reviewConfigToJson(m.review)).toEqual({ visual: { themes: ["dark"], gif: true } });
});

Expand Down Expand Up @@ -4278,7 +4340,7 @@ describe("review.visual.theme_storage_key (#4109 localStorage theme-forcing fall

it("composes with themes — both configured independently and both round-trip", () => {
const m = parseFocusManifest({ review: { visual: { themes: ["dark"], theme_storage_key: "theme" } } });
expect(m.review.visual).toEqual({ preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: ["dark"], gif: false, enabled: null, themeStorageKey: "theme", actionsFallback: false });
expect(m.review.visual).toEqual({ productionUrl: null, preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: ["dark"], gif: false, enabled: null, themeStorageKey: "theme", actionsFallback: false });
expect(reviewConfigToJson(m.review)).toEqual({ visual: { themes: ["dark"], theme_storage_key: "theme" } });
});

Expand Down Expand Up @@ -4333,7 +4395,7 @@ describe("review.visual.actions_fallback (#4112 GitHub-Actions build-and-serve f

it("composes with gif — both configured independently and both round-trip", () => {
const m = parseFocusManifest({ review: { visual: { actions_fallback: true, gif: true } } });
expect(m.review.visual).toEqual({ preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: [], gif: true, enabled: null, themeStorageKey: null, actionsFallback: true });
expect(m.review.visual).toEqual({ productionUrl: null, preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: [], gif: true, enabled: null, themeStorageKey: null, actionsFallback: true });
expect(reviewConfigToJson(m.review)).toEqual({ visual: { gif: true, actions_fallback: true } });
});

Expand Down
Loading