fix(review): append the contributor skill-file link without losing the specific rejection reason

apps/gittensory-ui/public/openapi.json

@@ -9498,6 +9498,9 @@
                 "items": {
                   "type": "string"
                 }
+              },
+              "skillFileUrl": {
+                "type": "string"
               }
             },
             "required": [

migrations/0133_screenshot_table_gate_skill_link.sql

@@ -0,0 +1,8 @@
+-- Contributor skill-file link for the screenshot-table gate (#4540 follow-up). Appended to the
+-- AUTO-GENERATED rejection message (both matrix and presence mode) so a closed contributor always gets
+-- pointed at the exact evidence contract, not just told evidence is missing. Nullable, same "no override
+-- configured" shape as screenshot_table_gate_message (migration 0117) -- deliberately a SEPARATE field
+-- from that one: message is a full replacement a maintainer uses for total control over the wording,
+-- while skill_file_url only ever appends to whichever message is already being shown (auto-generated,
+-- specific-missing-pairs text included), so setting one doesn't cost the other.
+ALTER TABLE repository_settings ADD COLUMN screenshot_table_gate_skill_file_url TEXT;

packages/gittensory-engine/src/focus-manifest.ts

@@ -1883,6 +1883,7 @@ function parseSettingsOverride(value: JsonValue | undefined, warnings: string[],
     if (typeof rawGate.message === "string" && rawGate.message.trim().length > 0) sparseGate.message = validated.message;
     if (Array.isArray(rawGate.requireViewports)) sparseGate.requireViewports = validated.requireViewports;
     if (Array.isArray(rawGate.requireThemes)) sparseGate.requireThemes = validated.requireThemes;
+    if (typeof rawGate.skillFileUrl === "string" && rawGate.skillFileUrl.trim().length > 0) sparseGate.skillFileUrl = validated.skillFileUrl;
     out.screenshotTableGate = sparseGate;
   } else if (r.screenshotTableGate !== undefined) {
     warnings.push(`Manifest "settings.screenshotTableGate" must be an object; ignoring it and keeping any existing policy.`);

packages/gittensory-engine/src/review/screenshot-table-gate.ts

@@ -16,6 +16,7 @@ const MAX_LABEL_CHARS = 100;
 const MAX_PATH_CHARS = 300;
 const MAX_MATRIX_DIMENSION = 12;
 const MAX_MATRIX_TOKEN_CHARS = 40;
+const MAX_SKILL_FILE_URL_CHARS = 300;
 
 // Extensions treated as "an image file" for the committed-image-file check below. Deliberately excludes SVG:
 // an SVG can embed script/foreign-object content, so it is never accepted as review evidence anywhere in this
@@ -83,6 +84,7 @@ export function normalizeScreenshotTableGateConfig(input: unknown, warnings: str
   if (record.message !== undefined && message === undefined) {
     warnings.push("settings.requireScreenshotTable.message must be a non-empty string; using the default message.");
   }
+  const skillFileUrl = normalizeSkillFileUrl(record.skillFileUrl, warnings);
   return {
     enabled,
     whenLabels: normalizeStringList(record.whenLabels, "whenLabels", MAX_LABELS, MAX_LABEL_CHARS, warnings),
@@ -91,9 +93,23 @@ export function normalizeScreenshotTableGateConfig(input: unknown, warnings: str
     requireViewports: normalizeStringList(record.requireViewports, "requireViewports", MAX_MATRIX_DIMENSION, MAX_MATRIX_TOKEN_CHARS, warnings),
     requireThemes: normalizeStringList(record.requireThemes, "requireThemes", MAX_MATRIX_DIMENSION, MAX_MATRIX_TOKEN_CHARS, warnings),
     ...(message !== undefined ? { message } : {}),
+    ...(skillFileUrl !== undefined ? { skillFileUrl } : {}),
   };
 }
 
+/** Validate a `skillFileUrl` override: same trust/validation level as `message` above (a trusted
+ *  maintainer-authored config value, never fetched server-side -- it is only ever embedded as TEXT in a
+ *  GitHub comment/close reason, so there is no SSRF surface here to guard against, unlike a URL the
+ *  server would dereference). Malformed values are dropped with a warning, never silently coerced. */
+function normalizeSkillFileUrl(value: unknown, warnings: string[]): string | undefined {
+  if (value === undefined) return undefined;
+  if (typeof value !== "string" || value.trim().length === 0 || value.trim().length > MAX_SKILL_FILE_URL_CHARS) {
+    warnings.push(`settings.requireScreenshotTable.skillFileUrl must be a non-empty string no longer than ${MAX_SKILL_FILE_URL_CHARS} characters; ignoring it.`);
+    return undefined;
+  }
+  return value.trim();
+}
+
 /** Linear-time markdown table separator check. The previous single-regex form nested unbounded `\\s*` inside a
  *  repeated group and could catastrophically backtrack on attacker-controlled PR bodies; this splits on `|` and
  *  validates each cell independently instead. */
@@ -255,6 +271,13 @@ export function buildScreenshotMatrixMessage(missing: ScreenshotMatrixPair[]): s
   );
 }
 
+/** Append a contributor skill-file link to an auto-generated rejection message (#4540 follow-up). A no-op
+ *  when `skillFileUrl` is unset -- callers only reach this on the AUTO-GENERATED path (a `message`
+ *  override already owns its entire text and is never passed through here). */
+function appendSkillLink(text: string, skillFileUrl: string | undefined): string {
+  return skillFileUrl ? `${text}\n\nSee ${skillFileUrl} for the exact format and examples.` : text;
+}
+
 /** True when the PR is IN SCOPE for the gate: it carries one of `config.whenLabels` OR touches a path matching
  *  one of `config.whenPaths`. Both empty ⇒ every PR is in scope (an operator who enables the gate with no
  *  scoping at all wants it enforced everywhere). Only one non-empty list configured ⇒ that list alone decides
@@ -313,12 +336,12 @@ export function evaluateScreenshotTableGate(input: {
   if (matrixPairs.length > 0) {
     const missing = missingScreenshotMatrixPairs(input.prBody, matrixPairs);
     if (missing.length === 0) return NO_VIOLATION;
-    return { violated: true, reason: config.message ?? buildScreenshotMatrixMessage(missing) };
+    return { violated: true, reason: config.message ?? appendSkillLink(buildScreenshotMatrixMessage(missing), config.skillFileUrl) };
   }
 
   const hasTable = hasImageBearingMarkdownTable(input.prBody);
   const outsideTable = hasImageOutsideTable(input.prBody);
   const committedImage = hasCommittedImageFile(input.changedFiles, config.whenPaths);
   if (hasTable && !outsideTable && !committedImage) return NO_VIOLATION;
-  return { violated: true, reason: config.message ?? DEFAULT_SCREENSHOT_CONTRACT_MESSAGE };
+  return { violated: true, reason: config.message ?? appendSkillLink(DEFAULT_SCREENSHOT_CONTRACT_MESSAGE, config.skillFileUrl) };
 }

packages/gittensory-engine/src/types/manifest-deps-types.ts

@@ -27,11 +27,16 @@ export type ScreenshotTableGateConfig = {
   whenLabels: string[];
   whenPaths: string[];
   action: ScreenshotTableGateAction;
+  // Full replacement for the rejection reason -- see src/types.ts's mirror of this type for the full
+  // rationale (unset ⇒ auto-generated message + skillFileUrl; set ⇒ used verbatim, skillFileUrl ignored).
   message?: string | undefined;
   // Viewport x theme completeness matrix (#4535) -- see src/types.ts's mirror of this type for the full
   // rationale.
   requireViewports: string[];
   requireThemes: string[];
+  // Contributor skill-file link appended to the auto-generated message (#4540 follow-up) -- see
+  // src/types.ts's mirror of this type for the full rationale.
+  skillFileUrl?: string | undefined;
 };
 
 export type CommandAuthorizationRole = "maintainer" | "collaborator" | "pr_author" | "confirmed_miner";

src/db/repositories.ts

@@ -860,6 +860,7 @@ export async function upsertRepositorySettings(env: Env, settings: Partial<Repos
       screenshotTableGateRequireViewportsJson: jsonString(resolved.screenshotTableGate.requireViewports),
       screenshotTableGateRequireThemesJson: jsonString(resolved.screenshotTableGate.requireThemes),
       screenshotTableGateMessage: resolved.screenshotTableGate.message ?? null,
+      screenshotTableGateSkillFileUrl: resolved.screenshotTableGate.skillFileUrl ?? null,
       updatedAt: nowIso(),
     })
     .onConflictDoUpdate({
@@ -948,6 +949,7 @@ export async function upsertRepositorySettings(env: Env, settings: Partial<Repos
         screenshotTableGateRequireViewportsJson: jsonString(resolved.screenshotTableGate.requireViewports),
         screenshotTableGateRequireThemesJson: jsonString(resolved.screenshotTableGate.requireThemes),
         screenshotTableGateMessage: resolved.screenshotTableGate.message ?? null,
+        screenshotTableGateSkillFileUrl: resolved.screenshotTableGate.skillFileUrl ?? null,
         updatedAt: nowIso(),
       },
     });
@@ -7337,6 +7339,7 @@ function parseScreenshotTableGateRow(row: typeof repositorySettings.$inferSelect
     requireViewports: parseJsonStringArray(row.screenshotTableGateRequireViewportsJson),
     requireThemes: parseJsonStringArray(row.screenshotTableGateRequireThemesJson),
     ...(row.screenshotTableGateMessage ? { message: row.screenshotTableGateMessage } : {}),
+    ...(row.screenshotTableGateSkillFileUrl ? { skillFileUrl: row.screenshotTableGateSkillFileUrl } : {}),
   };
 }
 

src/db/schema.ts

@@ -189,6 +189,9 @@ export const repositorySettings = sqliteTable("repository_settings", {
   // labeled before/after row per configured viewport (x theme, when requireThemes is also set).
   screenshotTableGateRequireViewportsJson: text("screenshot_table_gate_require_viewports_json").notNull().default("[]"),
   screenshotTableGateRequireThemesJson: text("screenshot_table_gate_require_themes_json").notNull().default("[]"),
+  // Contributor skill-file link appended to the auto-generated matrix/presence rejection message (#4540
+  // follow-up). Nullable, same "no override configured" shape as screenshotTableGateMessage above.
+  screenshotTableGateSkillFileUrl: text("screenshot_table_gate_skill_file_url"),
   createdAt: text("created_at").notNull().$defaultFn(() => nowIso()),
   updatedAt: text("updated_at").notNull().$defaultFn(() => nowIso()),
 });

src/openapi/schemas.ts

@@ -792,6 +792,7 @@ export const RepositorySettingsSchema = z
         requireViewports: z.array(z.string()),
         requireThemes: z.array(z.string()),
         message: z.string().optional(),
+        skillFileUrl: z.string().optional(),
       })
       .optional(),
     createdAt: z.string().nullable().optional(),

src/review/screenshot-table-gate.ts

@@ -16,6 +16,7 @@ const MAX_LABEL_CHARS = 100;
 const MAX_PATH_CHARS = 300;
 const MAX_MATRIX_DIMENSION = 12;
 const MAX_MATRIX_TOKEN_CHARS = 40;
+const MAX_SKILL_FILE_URL_CHARS = 300;
 
 // Extensions treated as "an image file" for the committed-image-file check below. Deliberately excludes SVG:
 // an SVG can embed script/foreign-object content, so it is never accepted as review evidence anywhere in this
@@ -83,6 +84,7 @@ export function normalizeScreenshotTableGateConfig(input: unknown, warnings: str
   if (record.message !== undefined && message === undefined) {
     warnings.push("settings.requireScreenshotTable.message must be a non-empty string; using the default message.");
   }
+  const skillFileUrl = normalizeSkillFileUrl(record.skillFileUrl, warnings);
   return {
     enabled,
     whenLabels: normalizeStringList(record.whenLabels, "whenLabels", MAX_LABELS, MAX_LABEL_CHARS, warnings),
@@ -91,9 +93,23 @@ export function normalizeScreenshotTableGateConfig(input: unknown, warnings: str
     requireViewports: normalizeStringList(record.requireViewports, "requireViewports", MAX_MATRIX_DIMENSION, MAX_MATRIX_TOKEN_CHARS, warnings),
     requireThemes: normalizeStringList(record.requireThemes, "requireThemes", MAX_MATRIX_DIMENSION, MAX_MATRIX_TOKEN_CHARS, warnings),
     ...(message !== undefined ? { message } : {}),
+    ...(skillFileUrl !== undefined ? { skillFileUrl } : {}),
   };
 }
 
+/** Validate a `skillFileUrl` override: same trust/validation level as `message` above (a trusted
+ *  maintainer-authored config value, never fetched server-side -- it is only ever embedded as TEXT in a
+ *  GitHub comment/close reason, so there is no SSRF surface here to guard against, unlike a URL the
+ *  server would dereference). Malformed values are dropped with a warning, never silently coerced. */
+function normalizeSkillFileUrl(value: unknown, warnings: string[]): string | undefined {
+  if (value === undefined) return undefined;
+  if (typeof value !== "string" || value.trim().length === 0 || value.trim().length > MAX_SKILL_FILE_URL_CHARS) {
+    warnings.push(`settings.requireScreenshotTable.skillFileUrl must be a non-empty string no longer than ${MAX_SKILL_FILE_URL_CHARS} characters; ignoring it.`);
+    return undefined;
+  }
+  return value.trim();
+}
+
 /** Linear-time markdown table separator check. The previous single-regex form nested unbounded `\\s*` inside a
  *  repeated group and could catastrophically backtrack on attacker-controlled PR bodies; this splits on `|` and
  *  validates each cell independently instead. */
@@ -255,6 +271,13 @@ export function buildScreenshotMatrixMessage(missing: ScreenshotMatrixPair[]): s
   );
 }
 
+/** Append a contributor skill-file link to an auto-generated rejection message (#4540 follow-up). A no-op
+ *  when `skillFileUrl` is unset -- callers only reach this on the AUTO-GENERATED path (a `message`
+ *  override already owns its entire text and is never passed through here). */
+function appendSkillLink(text: string, skillFileUrl: string | undefined): string {
+  return skillFileUrl ? `${text}\n\nSee ${skillFileUrl} for the exact format and examples.` : text;
+}
+
 /** True when the PR is IN SCOPE for the gate: it carries one of `config.whenLabels` OR touches a path matching
  *  one of `config.whenPaths`. Both empty ⇒ every PR is in scope (an operator who enables the gate with no
  *  scoping at all wants it enforced everywhere). Only one non-empty list configured ⇒ that list alone decides
@@ -313,12 +336,12 @@ export function evaluateScreenshotTableGate(input: {
   if (matrixPairs.length > 0) {
     const missing = missingScreenshotMatrixPairs(input.prBody, matrixPairs);
     if (missing.length === 0) return NO_VIOLATION;
-    return { violated: true, reason: config.message ?? buildScreenshotMatrixMessage(missing) };
+    return { violated: true, reason: config.message ?? appendSkillLink(buildScreenshotMatrixMessage(missing), config.skillFileUrl) };
   }
 
   const hasTable = hasImageBearingMarkdownTable(input.prBody);
   const outsideTable = hasImageOutsideTable(input.prBody);
   const committedImage = hasCommittedImageFile(input.changedFiles, config.whenPaths);
   if (hasTable && !outsideTable && !committedImage) return NO_VIOLATION;
-  return { violated: true, reason: config.message ?? DEFAULT_SCREENSHOT_CONTRACT_MESSAGE };
+  return { violated: true, reason: config.message ?? appendSkillLink(DEFAULT_SCREENSHOT_CONTRACT_MESSAGE, config.skillFileUrl) };
 }

src/signals/focus-manifest.ts

@@ -569,6 +569,7 @@ export function resolveEffectiveSettings(
       requireViewports: screenshotTableGateOverride.requireViewports ?? base.requireViewports,
       requireThemes: screenshotTableGateOverride.requireThemes ?? base.requireThemes,
       message: screenshotTableGateOverride.message ?? base.message,
+      skillFileUrl: screenshotTableGateOverride.skillFileUrl ?? base.skillFileUrl,
     };
   }
   if (advisoryAiRoutingOverride !== undefined) {

src/types.ts

@@ -1117,6 +1117,11 @@ export type ScreenshotTableGateConfig = {
   whenLabels: string[];
   whenPaths: string[];
   action: ScreenshotTableGateAction;
+  /** Full replacement for the rejection reason -- when set, this is used verbatim and NEITHER the
+   *  auto-generated matrix "still missing: ..." list NOR `skillFileUrl` appear (a maintainer who sets
+   *  this owns the entire message). Leave unset to get the auto-generated, always-accurate message
+   *  (naming the exact missing pairs in matrix mode) with `skillFileUrl` appended when configured --
+   *  that combination is usually what you want; only set `message` for total control over the wording. */
   message?: string | undefined;
   /** Viewport x theme completeness matrix (#4535). Both empty (the default) ⇒ byte-identical to the original
    *  presence-only check (some image-bearing table, anywhere). A non-empty `requireViewports` switches the
@@ -1126,6 +1131,11 @@ export type ScreenshotTableGateConfig = {
    *  empty) has no effect -- the viewport dimension is what turns matrix mode on. */
   requireViewports: string[];
   requireThemes: string[];
+  /** A link to this repo's contributor skill file, appended to the AUTO-GENERATED rejection message
+   *  (#4540 follow-up) so a closed contributor always gets pointed at the exact format/contract instead
+   *  of just being told evidence is missing. Ignored when `message` is set (a full override already
+   *  owns the entire text -- append the link into that string yourself if you want it there too). */
+  skillFileUrl?: string | undefined;
 };
 
 export type CommandAuthorizationRole = "maintainer" | "collaborator" | "pr_author" | "confirmed_miner";

test/unit/focus-manifest.test.ts

@@ -2948,6 +2948,29 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () =
     expect(eff.screenshotTableGate).toEqual({ enabled: true, whenLabels: [], whenPaths: [], action: "close", requireViewports: ["Desktop"], requireThemes: ["Light"] });
   });
 
+  it("wires settings.screenshotTableGate.skillFileUrl into the manifest parser as a sparse override (#4540 follow-up)", () => {
+    const url = "https://github.com/JSONbored/metagraphed/blob/main/.claude/skills/metagraphed/SKILL.md";
+    const parsed = parseFocusManifest({ settings: { screenshotTableGate: { skillFileUrl: url } } });
+    expect(parsed.settings.screenshotTableGate).toEqual({ skillFileUrl: url });
+  });
+
+  it("omits skillFileUrl from the sparse override when the raw manifest doesn't name it (#4540 follow-up)", () => {
+    const parsed = parseFocusManifest({ settings: { screenshotTableGate: { enabled: true } } });
+    expect(parsed.settings.screenshotTableGate).not.toHaveProperty("skillFileUrl");
+  });
+
+  it("resolveEffectiveSettings merges skillFileUrl without clearing the DB layer's other fields (#4540 follow-up)", () => {
+    const db = { screenshotTableGate: { enabled: true, whenLabels: [], whenPaths: [], action: "close", requireViewports: [], requireThemes: [] } } as unknown as RepositorySettings;
+    const eff = resolveEffectiveSettings(db, parseFocusManifest({ settings: { screenshotTableGate: { skillFileUrl: "https://github.com/acme/widget/blob/main/SKILL.md" } } }));
+    expect(eff.screenshotTableGate).toEqual({ enabled: true, whenLabels: [], whenPaths: [], action: "close", requireViewports: [], requireThemes: [], skillFileUrl: "https://github.com/acme/widget/blob/main/SKILL.md" });
+  });
+
+  it("resolveEffectiveSettings keeps the DB layer's skillFileUrl when the manifest override omits it (#4540 follow-up)", () => {
+    const db = { screenshotTableGate: { enabled: true, whenLabels: [], whenPaths: [], action: "close", requireViewports: [], requireThemes: [], skillFileUrl: "https://github.com/acme/widget/blob/main/SKILL.md" } } as unknown as RepositorySettings;
+    const eff = resolveEffectiveSettings(db, parseFocusManifest({ settings: { screenshotTableGate: { enabled: true } } }));
+    expect(eff.screenshotTableGate).toEqual({ enabled: true, whenLabels: [], whenPaths: [], action: "close", requireViewports: [], requireThemes: [], skillFileUrl: "https://github.com/acme/widget/blob/main/SKILL.md" });
+  });
+
   it("wires settings.advisoryAiRouting into the manifest parser as a sparse override (#4364)", () => {
     const parsed = parseFocusManifest({ settings: { advisoryAiRouting: { slop: true, summaries: true } } });
     expect(parsed.settings.advisoryAiRouting).toEqual({ slop: true, summaries: true });