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
8 changes: 8 additions & 0 deletions .gittensory.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -971,6 +971,14 @@ settings:
# # SELF-HOST ONLY (no effect on the hosted service) and the heaviest capture mode here — up to 6 extra
# # renders per side, ~4s wall-clock per side measured in practice. Bool. Default: false (no scroll capture).
# gif: false
# # Config-as-code enable/disable for this repo, layered ON TOP OF (never a replacement for) the
# # GITTENSORY_REVIEW_SCREENSHOTS + per-repo cutover-allowlist env-var gate above (#4083). Bool or null.
# # Default: null (unset) ⇒ defers entirely to that env-var gate's own decision -- byte-identical to today.
# # Explicit `false` (set once at the global-default `.gittensory.yml`, or overridden per-repo here) forces
# # capture off for this repo even when the env-var gate would otherwise allow it. Explicit `true` opts this
# # repo back in at a layer where a broader default disabled it -- it does NOT bypass the env-var gate
# # itself, so the env vars remain the outer infra-availability switch.
# enabled: true
# # 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
8 changes: 8 additions & 0 deletions config/examples/gittensory.full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -984,6 +984,14 @@ settings:
# # SELF-HOST ONLY (no effect on the hosted service) and the heaviest capture mode here — up to 6 extra
# # renders per side, ~4s wall-clock per side measured in practice. Bool. Default: false (no scroll capture).
# gif: false
# # Config-as-code enable/disable for this repo, layered ON TOP OF (never a replacement for) the
# # GITTENSORY_REVIEW_SCREENSHOTS + per-repo cutover-allowlist env-var gate above (#4083). Bool or null.
# # Default: null (unset) ⇒ defers entirely to that env-var gate's own decision -- byte-identical to today.
# # Explicit `false` (set once at the global-default `.gittensory.yml`, or overridden per-repo here) forces
# # capture off for this repo even when the env-var gate would otherwise allow it. Explicit `true` opts this
# # repo back in at a layer where a broader default disabled it -- it does NOT bypass the env-var gate
# # itself, so the env vars remain the outer infra-availability switch.
# enabled: true
# # 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: 14 additions & 2 deletions packages/gittensory-engine/src/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,14 @@ export type VisualConfig = {
* capture mode this pipeline has (up to 6 extra renders per side) — false (default, every existing
* manifest) ⇒ byte-identical to today, no scroll frames captured at all. */
gif: boolean;
/** `review.visual.enabled` (#4083): a config-as-code override layered ON TOP OF the outer
* `GITTENSORY_REVIEW_SCREENSHOTS` / `GITTENSORY_REVIEW_REPOS` env-var gate, not a replacement for it. null
* (default, unset at every config layer) ⇒ defers entirely to that gate's own decision. `false` (settable at
* the global-default layer, or overridden per-repo) ⇒ forces capture off for this repo even when the env-var
* gate would otherwise allow it. `true` ⇒ no additional restriction — it does NOT bypass the env-var gate,
* it only opts back in at a layer where a global default of `false` disabled this repo. This is what lets an
* operator flip visual review on/off per-repo purely through the VPS config files, without a redeploy. */
enabled: boolean | null;
};

/** A `prefers-color-scheme` value the capture pipeline can emulate before rendering (#3678). */
Expand Down Expand Up @@ -685,6 +693,7 @@ export const EMPTY_VISUAL_CONFIG: VisualConfig = {
routes: { paths: [], maxRoutes: null },
themes: [],
gif: false,
enabled: null,
};

/** One `review.path_instructions[]` entry: a manifest path glob + the public-safe instructions to apply when a
Expand Down Expand Up @@ -2092,6 +2101,7 @@ function overlayVisualConfig(base: VisualConfig, override: VisualConfig): Visual
},
themes: override.themes.length > 0 ? [...override.themes] : [...base.themes],
gif: override.gif ? override.gif : base.gif,
enabled: pickOverlayNullable(override.enabled, base.enabled),
};
}

Expand Down Expand Up @@ -2321,7 +2331,7 @@ function parseSelfHostAiModelConfig(value: JsonValue | undefined, warnings: stri
}

function visualConfigPresent(config: VisualConfig): boolean {
return config.preview.urlTemplate !== null || config.routes.paths.length > 0 || config.routes.maxRoutes !== null || config.themes.length > 0 || config.gif;
return config.preview.urlTemplate !== null || config.routes.paths.length > 0 || config.routes.maxRoutes !== null || config.themes.length > 0 || config.gif || config.enabled !== null;
}

const VISUAL_THEME_VALUES: readonly VisualTheme[] = ["light", "dark"];
Expand Down Expand Up @@ -2402,8 +2412,9 @@ function parseVisualConfig(value: JsonValue | undefined, warnings: string[]): Vi

const themes = parseVisualThemes(record.themes, warnings);
const gif = normalizeOptionalBoolean(record.gif, "review.visual.gif", warnings) === true;
const enabled = normalizeOptionalBoolean(record.enabled, "review.visual.enabled", warnings);

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

function parseAutoReviewTitleKeywords(value: JsonValue | undefined, warnings: string[]): string[] {
Expand Down Expand Up @@ -2718,6 +2729,7 @@ export function reviewConfigToJson(review: FocusManifestReviewConfig): JsonValue
}
if (review.visual.themes.length > 0) visual.themes = [...review.visual.themes];
if (review.visual.gif) visual.gif = true;
if (review.visual.enabled !== null) visual.enabled = review.visual.enabled;
out.visual = visual;
}
if (review.linkedIssueSatisfaction !== null) out.linkedIssueSatisfaction = review.linkedIssueSatisfaction;
Expand Down
30 changes: 16 additions & 14 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10231,20 +10231,22 @@ async function maybePublishPrPublicSurface(
// (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,
{
repoFullName,
prNumber: pr.number,
...(pr.headSha ? { headSha: pr.headSha } : {}),
...(pr.headRef ? { headRef: pr.headRef } : {}),
previewFromChecks: true,
},
visualFiles,
githubRateLimitAdmissionKeyForInstallation(installationId),
reviewVisualConfig,
);
const captureTarget = {
repoFullName,
prNumber: pr.number,
...(pr.headSha ? { headSha: pr.headSha } : {}),
...(pr.headRef ? { headRef: pr.headRef } : {}),
previewFromChecks: true,
};
// review.visual.enabled (#4083): a config-as-code override layered on top of the screenshotsAllowed
// env-var gate above, not a replacement for it. Unset/true ⇒ defer to that gate's decision (buildCapture
// runs exactly as before); explicit `false` (global default or per-repo, VPS-only) ⇒ force capture off
// for this repo -- a no-routes, non-pending sentinel result, so every line below behaves exactly as an
// ordinary "nothing found" capture would, with no separate code path to maintain.
const capture =
reviewVisualConfig.enabled === false
? { routes: [], previewPending: false }
: await buildCapture(env, token, captureTarget, visualFiles, githubRateLimitAdmissionKeyForInstallation(installationId), reviewVisualConfig);
beforeAfter = capture.routes;
// 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
Expand Down
60 changes: 58 additions & 2 deletions test/unit/focus-manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3745,6 +3745,7 @@ describe("review.visual (#3609 preview.url_template / #3610 routes)", () => {
routes: { paths: ["/pricing", "/docs"], maxRoutes: 3 },
themes: [],
gif: false,
enabled: null,
});
expect(m.review.present).toBe(true);
expect(parseFocusManifest({ review: reviewConfigToJson(m.review) }).review.visual).toEqual(m.review.visual);
Expand Down Expand Up @@ -3840,7 +3841,7 @@ 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 });
expect(resolveReviewVisualConfig(manifest)).toEqual({ preview: { urlTemplate: null }, routes: { paths: ["/app"], maxRoutes: null }, themes: [], gif: false, enabled: null });
});
});

Expand Down Expand Up @@ -3925,7 +3926,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 });
expect(m.review.visual).toEqual({ preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: ["dark"], gif: true, enabled: null });
expect(reviewConfigToJson(m.review)).toEqual({ visual: { themes: ["dark"], gif: true } });
});

Expand All @@ -3935,6 +3936,61 @@ describe("review.visual.gif (#3612 scroll-through GIF capture)", () => {
});
});

describe("review.visual.enabled (#4083 config-as-code enable/disable)", () => {
it("parses enabled: true, marks present, and round-trips", () => {
const m = parseFocusManifest({ review: { visual: { enabled: true } } });
expect(m.review.visual.enabled).toBe(true);
expect(m.review.present).toBe(true);
expect(reviewConfigToJson(m.review)).toEqual({ visual: { enabled: true } });
});

it("parses enabled: false, marks present, and round-trips", () => {
const m = parseFocusManifest({ review: { visual: { enabled: false } } });
expect(m.review.visual.enabled).toBe(false);
expect(m.review.present).toBe(true);
expect(reviewConfigToJson(m.review)).toEqual({ visual: { enabled: false } });
});

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

it("null enabled does not serialize into the round-tripped visual block", () => {
const m = parseFocusManifest({ review: { visual: { gif: true } } });
expect(reviewConfigToJson(m.review)).toEqual({ visual: { gif: true } });
});

it("warns and defaults to null when enabled is not a boolean", () => {
const bad = parseFocusManifest({ review: { visual: { enabled: "yes" } } });
expect(bad.review.visual.enabled).toBeNull();
expect(bad.warnings.some((w) => /review\.visual\.enabled.*must be a boolean/.test(w))).toBe(true);
});

it("marks present via enabled alone (preview + routes + themes + gif all empty)", () => {
const m = parseFocusManifest({ review: { visual: { enabled: false } } });
expect(m.review.present).toBe(true);
});

it("resolveReviewVisualConfig passes a configured enabled: false through", () => {
const manifest = parseFocusManifest({ review: { visual: { enabled: false } } });
expect(resolveReviewVisualConfig(manifest).enabled).toBe(false);
});

it("overlay: a per-repo enabled: false wins over a global-default enabled: true", () => {
const globalDefault = parseReviewConfigMapping({ visual: { enabled: true } }, []);
const perRepo = parseReviewConfigMapping({ visual: { enabled: false } }, []);
expect(overlayReviewConfig(globalDefault, perRepo).visual.enabled).toBe(false);
});

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

describe("review.pre_merge_checks (#review-pre-merge-checks)", () => {
it("parses checks (name + assertions + when_paths + enforce), marks present, and round-trips", () => {
const m = parseFocusManifest({
Expand Down
Loading