fix(repo-config): size-gate the fetcher, harden pr-check, trim workflow-types

Three review findings on the repo-config surface.

`fetcher.ts` had no size gate before the base64 decode, unlike its sibling
`pr-check.ts`. Adds the gate, sharing `MAX_CONFIG_BYTES` from `schema.ts` so
the two read paths cannot drift. The empty-`content` case is folded in on
purpose: over 1 MB the Contents API returns `content: ""` with
`encoding: "none"`, which decoded to "", parsed to null, and surfaced as a
root-level schema error blaming the owner's document for a size limit.

`touchesConfigFile` was the one GitHub call in `pr-check.ts` that could
reject, so `runPrConfigCheck` was not the total function its siblings are. A
secondary rate limit or a revoked `pull_requests: read` now no-ops the check
instead of throwing into the caller.

`workflow-types.ts` carried workflow-runner scaffolding with no consumer on
this branch, including a second `HandlerResultSchema` whose shape already
disagreed with the registry's. Trimmed 20 export statements to the 9 that
have callers; the rest land with the isolated runner.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KUPpJPtxAaHWrBsjytRGyM

Author: chrisleekr

src/repo-config/fetcher.ts

@@ -24,7 +24,7 @@ import { parse as parseYaml } from "yaml";
 import type { z } from "zod";
 
 import { redactSecrets } from "../utils/sanitize";
-import { type GithubAppConfig, githubAppConfigSchema } from "./schema";
+import { type GithubAppConfig, githubAppConfigSchema, MAX_CONFIG_BYTES } from "./schema";
 
 /**
  * Outcome of reading one repo's config.
@@ -172,6 +172,25 @@ export async function fetchRepoConfig(input: FetchRepoConfigInput): Promise<Repo
     return { kind: "invalid", message: `${path} is not a file` };
   }
 
+  // Size gate BEFORE the base64 decode, mirroring `pr-check.ts`, so an
+  // oversize blob is never materialised. This path runs per job rather than
+  // once per scheduler tick, and every cache miss re-pays it.
+  //
+  // Empty `content` is folded in here on purpose: over 1 MB the Contents API
+  // returns `content: ""` with `encoding: "none"`, which would otherwise
+  // decode to "", parse to null, and be reported as a root-level schema error
+  // blaming the owner's document for what is really a size limit.
+  if (data.size > MAX_CONFIG_BYTES || data.content.length === 0) {
+    log.warn(
+      { event: "repo_config.invalid", owner, repo, kind: "too-large", size: data.size },
+      "repo-config: config file is too large to validate",
+    );
+    return {
+      kind: "invalid",
+      message: `${path} is ${String(data.size)} bytes, over the ${String(MAX_CONFIG_BYTES)} byte limit`,
+    };
+  }
+
   const raw = Buffer.from(data.content, "base64").toString("utf-8");
   const value = parseAndValidate(raw, data.sha, { owner, repo, log });
   cacheResult(cacheKey, res.headers.etag, value);

src/repo-config/pr-check.ts

@@ -33,10 +33,7 @@ import type { z } from "zod";
 import { config } from "../config";
 import { redactSecrets, sanitizeContent } from "../utils/sanitize";
 import { buildScopedMarker, upsertMarkerComment } from "../workflows/ship/scoped/marker-comment";
-import { githubAppConfigSchema } from "./schema";
-
-/** GitHub's own editor refuses far larger files; 64 KB is well past any real config. */
-const MAX_CONFIG_BYTES = 64 * 1024;
+import { githubAppConfigSchema, MAX_CONFIG_BYTES } from "./schema";
 
 /** Rendered issue cap. Beyond this the comment stops being readable. */
 const MAX_RENDERED_ISSUES = 10;
@@ -201,12 +198,32 @@ function toIssues(issues: readonly z.core.$ZodIssue[]): ConfigCheckIssue[] {
  * missed edit costs the author a comment, never a wrong verdict.
  */
 async function touchesConfigFile(input: RunPrConfigCheckInput, path: string): Promise<boolean> {
-  const files = (await input.octokit.paginate(input.octokit.rest.pulls.listFiles, {
-    owner: input.owner,
-    repo: input.repo,
-    pull_number: input.prNumber,
-    per_page: 100,
-  })) as { filename: string }[];
+  let files: { filename: string }[];
+  try {
+    files = (await input.octokit.paginate(input.octokit.rest.pulls.listFiles, {
+      owner: input.owner,
+      repo: input.repo,
+      pull_number: input.prNumber,
+      per_page: 100,
+    })) as { filename: string }[];
+  } catch (err) {
+    // Degrade like every other GitHub call on this path: a secondary rate
+    // limit, a 5xx, or a revoked `pull_requests: read` must no-op the check,
+    // not reject out of `runPrConfigCheck` into the caller. Same trade the
+    // 3000-file cap already accepts: a missed edit costs the author a
+    // comment, never a wrong verdict.
+    input.log.info(
+      {
+        event: "repo_config.pr_check.list_files_failed",
+        err,
+        owner: input.owner,
+        repo: input.repo,
+        prNumber: input.prNumber,
+      },
+      "repo-config: could not list pull request files, skipping config check",
+    );
+    return false;
+  }
   return files.some((file) => file.filename === path);
 }
 

src/repo-config/schema.ts

@@ -319,6 +319,13 @@ export const DEFAULT_REPO_DEFAULTS: RepoDefaults = { extra_allowed_tools: [] };
  * written against an earlier revision of this schema still parses. Bump
  * `version` only for a breaking rename or semantic change.
  */
+/**
+ * Byte cap on the config blob, shared by `fetcher.ts` and `pr-check.ts` so the
+ * two read paths cannot drift. GitHub's own editor refuses far larger files;
+ * 64 KB is well past any real config.
+ */
+export const MAX_CONFIG_BYTES = 64 * 1024;
+
 export const githubAppConfigSchema = z
   .strictObject({
     version: z.literal(1),

src/shared/workflow-types.ts

@@ -30,13 +30,6 @@ export const RepoMemoryCategorySchema = z.enum([
   "env",
   "gotchas",
 ]);
-export const RepoMemoryEntrySchema = z.object({
-  id: z.uuid(),
-  category: RepoMemoryCategorySchema,
-  content: z.string().min(1).max(1000),
-  pinned: z.boolean(),
-});
-export type RepoMemoryEntry = z.infer<typeof RepoMemoryEntrySchema>;
 
 const reviewLearningActionSaveSchema = z.object({
   directive: z.string().min(1).max(2000),
@@ -63,73 +56,6 @@ export const DaemonActionsSchema = z.object({
 });
 export type DaemonActions = z.infer<typeof DaemonActionsSchema>;
 
-const appliedReviewLearningIdsField = z.array(z.string().max(64)).max(50).optional();
-const daemonActionsField = DaemonActionsSchema.optional();
-export const WORKFLOW_RUNNER_HUMAN_MESSAGE_MAX_CHARS = 50_000;
-const boundedHumanMessage = z
-  .string()
-  .min(1)
-  .max(WORKFLOW_RUNNER_HUMAN_MESSAGE_MAX_CHARS)
-  .optional();
-const boundedFailureReason = z.string().min(1).max(WORKFLOW_RUNNER_HUMAN_MESSAGE_MAX_CHARS);
-
-/** Result returned by one workflow handler before controller-side settlement. */
-export const HandlerResultSchema = z.discriminatedUnion("status", [
-  z.object({
-    status: z.literal("succeeded"),
-    state: z.unknown(),
-    humanMessage: boundedHumanMessage,
-    appliedReviewLearningIds: appliedReviewLearningIdsField,
-    daemonActions: daemonActionsField,
-  }),
-  z.object({
-    status: z.literal("failed"),
-    reason: boundedFailureReason,
-    state: z.unknown().optional(),
-    humanMessage: boundedHumanMessage,
-    daemonActions: daemonActionsField,
-  }),
-  z.object({
-    status: z.literal("incomplete"),
-    reason: boundedFailureReason,
-    state: z.unknown().optional(),
-    humanMessage: boundedHumanMessage,
-    appliedReviewLearningIds: appliedReviewLearningIdsField,
-    daemonActions: daemonActionsField,
-  }),
-  z.object({
-    status: z.literal("handed-off"),
-    state: z.unknown().optional(),
-    humanMessage: boundedHumanMessage,
-    childRunId: z.string().min(1),
-    daemonActions: z.never().optional(),
-  }),
-]);
-export type HandlerResult = z.infer<typeof HandlerResultSchema>;
-
-export const PriorPlanStateSchema = z.object({
-  plan: z.string().min(1).max(100_000),
-});
-export type PriorPlanState = z.infer<typeof PriorPlanStateSchema>;
-
-const WorkflowRunSnapshotStateSchema = z.object({
-  recommendedNext: z.enum(["plan", "stop"]).optional(),
-  pr_number: z.number().int().positive().optional(),
-});
-
-/** Bounded workflow history projected into a single-attempt runner payload. */
-export const WorkflowRunSnapshotSchema = z.object({
-  id: z.uuid(),
-  status: z.enum(["queued", "running", "succeeded", "failed", "incomplete"]),
-  state: WorkflowRunSnapshotStateSchema,
-  createdAt: z.iso.datetime(),
-});
-export type WorkflowRunSnapshot = z.infer<typeof WorkflowRunSnapshotSchema>;
-
-export function workflowRunnerId(attemptId: string): string {
-  return `workflow-runner:${attemptId}`;
-}
-
 export type {
   Registry,
   RegistryEntry,

test/repo-config/fetcher.test.ts

@@ -25,7 +25,13 @@ function b64(text: string): string {
 
 function fileResponse(yaml: string, etag?: string): unknown {
   return {
-    data: { type: "file", content: b64(yaml), sha: "abc123" },
+    data: {
+      type: "file",
+      content: b64(yaml),
+      sha: "abc123",
+      // Real responses always carry `size`; the size gate reads it.
+      size: Buffer.byteLength(yaml, "utf-8"),
+    },
     headers: etag !== undefined ? { etag } : {},
   };
 }
@@ -67,6 +73,41 @@ describe("fetchRepoConfig", () => {
     expect(getContent.mock.calls[0]?.[0]).not.toHaveProperty("ref");
   });
 
+  it("rejects an oversize blob before decoding it", async () => {
+    // Mirrors the pr-check gate: the decode must never be paid for a file
+    // that is going to be rejected anyway. This path runs per job, not once
+    // per scheduler tick.
+    const getContent = mock(() =>
+      Promise.resolve({
+        data: { type: "file", content: b64(VALID_YAML), sha: "abc123", size: 64 * 1024 + 1 },
+        headers: {},
+      }),
+    );
+    const result = await fetchFrom(getContent);
+
+    expect(result.kind).toBe("invalid");
+    expect(result.kind === "invalid" ? result.message : "").toContain("over the");
+  });
+
+  it("reports a >1MB blob as a size problem, not a schema problem", async () => {
+    // Over 1 MB the Contents API returns `content: ""` with `encoding: "none"`.
+    // Decoding that yields "", which parses to null and would otherwise be
+    // reported as `(root): expected object, received null`, blaming the
+    // owner's document for what is really a size limit.
+    const getContent = mock(() =>
+      Promise.resolve({
+        data: { type: "file", content: "", encoding: "none", sha: "abc123", size: 2_000_000 },
+        headers: {},
+      }),
+    );
+    const result = await fetchFrom(getContent);
+
+    expect(result.kind).toBe("invalid");
+    const message = result.kind === "invalid" ? result.message : "";
+    expect(message).toContain("over the");
+    expect(message).not.toContain("expected object");
+  });
+
   it("returns absent on 404 and negative-caches it", async () => {
     const getContent = mock(() => Promise.reject(httpError(404)));
     expect((await fetchFrom(getContent)).kind).toBe("absent");