learnings: audit fixes — preamble in TOC, server-side line-boundary truncation, empty-repo intro (#743)

* learnings: surface preamble in TOC, mirror line-boundary truncation server-side, fix empty-repo intro copy

three audit fixes on top of the recent learnings overhaul (#717):

- `parseLearningsHeadings` now prepends a synthetic `(preamble)` entry
  when a body has non-whitespace content before the first heading. the
  prompt instructs the agent NOT to slurp the whole file when a TOC is
  present, so without this any preamble lines were silently invisible
  (realistic transitional case: an agent partially restructures a
  legacy free-text body and leaves bullets above the first `## `).

- server-side PATCH route now applies the same line-boundary-aware
  truncation as the action (defense in depth via a shared
  `truncateAtLineBoundary` + `MAX_LEARNINGS_LENGTH` exported from
  `action/internal`). the raw `.slice` it used before could leave a
  mid-heading tail on any caller that bypassed the client-side
  truncate, breaking the next-seed TOC parse. removes the duplicated
  cap constant.

- `buildLearningsSection` intro no longer asserts "accumulated by
  previous agent runs" — false for fresh repos with zero history. new
  copy is tense-neutral and works for empty + populated bodies. also
  nudges the agent to re-read after mid-run edits (the inlined TOC
  ranges are a run-start snapshot).

Co-authored-by: Cursor <cursoragent@cursor.com>

* learnings prompt: tighten to single evergreen test, allow tool-quirk bullets when they prevent repeat waste

The blanket "no pullfrog tool quirks" ban was wrong — if the agent burned
calls discovering a quirk this run, recording the workaround prevents the
next run from repeating the waste. Reframe around one litmus ("would a
future run do its work better because this bullet exists?") and trust it
to subsume the scattered don'ts. Drop the 3+ months timeframe (arbitrary)
and the four-example pullfrog/PR/date/play-by-play list (the rule
underneath is "don't anchor facts to repo state that will move"). Cuts
~10 lines from a prompt the model was already mostly ignoring; the
remaining anchor list is narrower and more enforceable.

* audit-learnings-r2: align wiki + tighten re-read nudge

- wiki/prompt.md described the post-run reflection prompt as "bans pullfrog-tool quirks (those belong in tool descriptions, not per-repo learnings), bans PR/review/commit/date references" — that's stale after the prompt rewrite. update to: single-litmus framing, expanded anchor list (now includes version pins + line numbers), and explicit allowance for tool-quirk workarounds when discovery burned calls.
- buildLearningsSection re-read nudge said "re-read after editing" which can be read as "re-read the section you edited". in fact any edit shifts the line numbers of every later section in the TOC, not just the edited one. tighten to make that explicit. mirror the new wording in the wiki example block. update the test substring assertion accordingly.

* postRun: refresh JSDoc to match the reflection prompt rewrite

`buildLearningsReflectionPrompt`'s JSDoc still listed "PR-/review-/commit-/date-anchored facts" and "rediscovery of pullfrog-tool quirks" as failure modes the prompt pushes back on. after b586b4f8 the prompt no longer bans tool-quirk bullets (it explicitly allows them when the agent burned calls discovering the quirk), and the anchor list expanded to cover branch refs, version pins, and line numbers too. update the JSDoc so it describes the prompt that actually exists, and call out the cross-repo drift tradeoff that comes with allowing tool-quirk bullets.

* fix(mcp/issueEvents): narrow event.event before Set.has lookup

octokit's listEventsForTimeline union includes timeline-event members where `event` is `event?: string`. `("event" in event)` does not narrow that property to non-undefined, so `relevantEventTypes.has(event.event)` was passing `string | undefined` to a `Set<string>.has`. typescript only flagged this once `cf-worker-indexing` started seeing the file via the type graph that now reaches mcp through the new `truncateAtLineBoundary` re-export in `action/internal/index.ts`. fix the latent bug at the source: require `typeof event.event === "string"` before the Set lookup.

* learnings: split truncation helpers into MCP-free module

re-exporting `truncateAtLineBoundary` + `MAX_LEARNINGS_LENGTH` from `action/utils/learnings.ts` through `action/internal/index.ts` accidentally pulled the entire MCP type graph into the SDK barrel: `learnings.ts` imports `ToolContext` from `mcp/server.ts`, which transitively wires every tool module under `action/mcp/` into anything that imports from `pullfrog/internal`. for `cf-worker-indexing/tsconfig.json` (`customConditions: ["@pullfrog/source"]`) and the root `tsc` (which compiles the proprietary app routes that import from `pullfrog/internal`), this expanded the type-checked surface and surfaced two latent issues in unrelated files (`mcp/issueEvents.ts`, `utils/subprocess.ts`). a 6-line pure string helper has no business dragging mcp/server.ts into anyone else's type graph.

move both symbols to `action/utils/learningsTruncate.ts`. `learnings.ts` re-exports them so existing callers keep working; `internal/index.ts` re-exports from the truncate-only module so the SDK barrel stays MCP-free.

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Colin McDonnell <colinmcd94@gmail.com>

agents/postRun.ts

@@ -279,11 +279,18 @@ export function shouldRunReflection(mode: string | undefined): boolean {
  * agent has been writing (issue #619 in pullfrog/app). recurring failure
  * modes the framing pushes back on:
  *  - massive multi-paragraph "bullets" that are really mini-articles
- *  - PR-/review-/commit-/date-anchored facts that decay within weeks
- *  - rediscovery of pullfrog-tool quirks that belong in tool descriptions,
- *    not per-repo learnings
+ *  - facts anchored to moving repo state (PR / review / commit / branch
+ *    refs, dates, version pins, line numbers) that decay within weeks
  *  - sections growing into giant flat lists with no internal structure,
  *    forcing future runs to read kilobytes to find one fact
+ *
+ * single litmus delivered in the prompt: "would a future run on this repo
+ * do its work better because this bullet exists?". tool-quirk workarounds
+ * are explicitly allowed when the agent burned calls discovering the
+ * quirk this run — recording the workaround prevents next run from
+ * repeating the waste. tradeoff: the same quirk gets duplicated across
+ * repos, so when a quirk is fixed upstream in tool descriptions the
+ * per-repo bullets go stale and we have no batch-invalidation path.
  */
 export function buildLearningsReflectionPrompt(filePath: string): string {
   return [
@@ -296,18 +303,16 @@ export function buildLearningsReflectionPrompt(filePath: string): string {
     `- **no section over ~300 lines.** when a section is approaching that, split it: introduce \`### \` subsections grouping related bullets, or hoist a coherent group into a new top-level \`## \` section. granular sections mean future runs read targeted line ranges instead of slurping the whole file. this is the most important hygiene rule on long-lived repos.`,
     `- if you find a flat unstructured list (legacy content from before this format), restructure it: read it, group related bullets, rewrite the file with \`## \` / \`### \` headings around them. don't preserve bad structure — fix it.`,
     "",
-    `bullet hygiene:`,
-    `- one fact per line starting with \`- \`. each bullet is ONE specific durable fact, not a paragraph or essay.`,
-    `- aim for ≤ 240 chars per bullet. longer bullets are almost always mixing multiple facts that should be split, or burying the durable claim under PR-specific context that should be cut.`,
-    `- only add bullets when the finding is high-confidence AND broadly useful AND will still be true in 3+ months. skip speculative, one-off, or "maybe" findings.`,
-    `- prune bullets that are clearly wrong, no longer relevant, or low-signal. a focused, accurate file beats a long stale one. compressing two overlapping bullets into one tighter bullet counts as progress.`,
-    `- deduplicate against existing entries (in any section) — if a bullet covers the same fact, update it in place instead of adding a duplicate.`,
+    `the only test: would a future run on this repo do its work better because this bullet exists? useful for future runs in this repo — prevent wasted tool calls, rabbit holes, and mistakes.`,
     "",
-    `do NOT add bullets for:`,
-    `- pullfrog tool quirks (e.g. "\`shell\` timeout is in milliseconds", "\`git\` args must be a JSON array", "\`create_pull_request_review\` drops out-of-hunk comments", "\`push_branch\` may report timeout when push succeeded"). these are universal across repos and belong in the tool descriptions — flag the gap rather than hoarding the workaround per-repo.`,
-    `- references to specific PR numbers, review IDs, commit SHAs, branch names, or person handles ("PR #595 introduced X", "flagged in review 12345", "as of commit abc123"). repo state changes; these decay into noise within weeks.`,
-    `- dated assertions ("as of May 2026", "currently...", "for now..."). if a fact needs a date to be true, it isn't durable enough to belong here.`,
-    `- play-by-play of what THIS run did. learnings are for the NEXT run, not a retrospective.`,
+    `bullet hygiene:`,
+    `- one fact per line starting with \`- \`, ≤ 240 chars.`,
+    `- only add when high-confidence, broadly useful, evergreen.`,
+    `- prune wrong or low-signal bullets; merge overlaps; dedupe across sections.`,
+    "",
+    `don't anchor facts to repo state that will move: PR / review / commit / branch refs, dates, version pins, line numbers. state the rule directly. if it needs the anchor to be load-bearing, it isn't evergreen.`,
+    "",
+    `tool-quirk bullets are fine when you burned calls discovering the quirk and a future run would repeat them. write the workaround, not the war story.`,
     "",
     `if you have nothing substantively new to add AND the existing entries still look healthy and well-structured, leave the file alone — just reply "done" and stop. silence is a valid outcome.`,
   ].join("\n");

internal/index.ts

@@ -45,6 +45,7 @@ export {
   isLeapingIntoActionCommentBody,
   LEAPING_INTO_ACTION_PREFIX,
 } from "../utils/leapingComment.ts";
+export { MAX_LEARNINGS_LENGTH, truncateAtLineBoundary } from "../utils/learningsTruncate.ts";
 export type {
   CreateProgressCommentTarget,
   ProgressComment,

mcp/issueEvents.ts

@@ -27,10 +27,11 @@ export function GetIssueEventsTool(ctx: ToolContext) {
       const relevantEventTypes = new Set(["cross_referenced", "referenced"]);
 
       const parsedEvents = events.flatMap((event) => {
-        // Filter to only events with an 'event' property and relevant types
-        if (!("event" in event) || !relevantEventTypes.has(event.event)) {
-          return [];
-        }
+        // octokit's timeline-event union includes members with `event?:
+        // string`, so `"event" in event` does not narrow it to defined.
+        // require a string before the Set.has() check.
+        if (!("event" in event) || typeof event.event !== "string") return [];
+        if (!relevantEventTypes.has(event.event)) return [];
 
         const baseEvent: Record<string, any> = {
           event: event.event,

utils/instructions.ts

@@ -397,11 +397,13 @@ export function buildLearningsSection(ctx: {
   headings: LearningsHeading[];
 }): string {
   if (!ctx.filePath) return "";
-  const intro = `Repo-level learnings accumulated by previous agent runs live at \`${ctx.filePath}\`. Use this file as durable context (test commands, conventions, gotchas, architecture notes).`;
+  // intro is neutral about whether content exists so an empty fresh-repo
+  // file doesn't open with "accumulated by previous agent runs" (false).
+  const intro = `The repo-level learnings file at \`${ctx.filePath}\` holds durable context (test commands, conventions, gotchas, architecture notes) maintained across runs.`;
   const tocBody =
     ctx.headings.length === 0
-      ? "(no headings yet — file is empty or a flat list. read the whole file. during the post-run reflection turn, structure it with `## ` / `### ` headings so future runs can read targeted ranges.)"
-      : `Read targeted line ranges via your native file tool — do NOT slurp the whole file. Each range starts at the section heading line, so reading the range gives you heading + body together.\n\n${renderLearningsToc(ctx.headings)}`;
+      ? "(no headings yet — the file is empty or contains a flat list. read the whole file if it has content. during the post-run reflection turn, structure it with `## ` / `### ` headings so future runs can read targeted ranges.)"
+      : `Read targeted line ranges via your native file tool — do NOT slurp the whole file. Each range starts at the section heading line, so reading the range gives you heading + body together. The ranges below are a run-start snapshot: any edit shifts the line numbers of every later section, so re-read the TOC range you need before relying on it.\n\n${renderLearningsToc(ctx.headings)}`;
   return `************* LEARNINGS *************\n\n${intro}\n\n${tocBody}`;
 }
 

utils/learnings.ts

@@ -3,6 +3,9 @@ import { dirname, join } from "node:path";
 import type { ToolContext } from "../mcp/server.ts";
 import { apiFetch } from "./apiFetch.ts";
 import { log } from "./cli.ts";
+import { MAX_LEARNINGS_LENGTH, truncateAtLineBoundary } from "./learningsTruncate.ts";
+
+export { MAX_LEARNINGS_LENGTH, truncateAtLineBoundary };
 
 /**
  * Repo-level learnings — operational facts about a repo (setup steps, test
@@ -34,15 +37,6 @@ import { log } from "./cli.ts";
 
 export const LEARNINGS_FILE_NAME = "pullfrog-learnings.md";
 
-/** server-side cap mirrors `MAX_LEARNINGS_LENGTH` in
- * `app/api/repo/[owner]/[repo]/learnings/route.ts`. truncating client-side
- * keeps the PATCH from being rejected with a 400. raised from 10k → 100k
- * once the TOC affordance landed: with line-range reads via the
- * server-parsed TOC the agent doesn't ingest the whole file, so the cap
- * can grow to whatever curation discipline allows. 100k holds ~400-500
- * short bullets. */
-const MAX_LEARNINGS_LENGTH = 100_000;
-
 export function learningsFilePath(tmpdir: string): string {
   return join(tmpdir, LEARNINGS_FILE_NAME);
 }
@@ -62,23 +56,6 @@ export async function seedLearningsFile(params: {
   return path;
 }
 
-/** truncate at the last newline boundary before `cap` so we don't leave
- * a partial line at the tail (a half-truncated `## Headi` confuses the
- * server's next-seed TOC parse and shrinks visible structure). falls
- * back to a hard `slice` when the line boundary would discard a large
- * run of content — i.e. when the tail of `head` is one giant line (rare:
- * minified pastes, fenced log dumps). losing a partial last line is
- * preferable to losing kilobytes of body. */
-const TRUNCATION_LINE_BOUNDARY_TOLERANCE = 4096;
-function truncateAtLineBoundary(body: string, cap: number): string {
-  if (body.length <= cap) return body;
-  const head = body.slice(0, cap);
-  const lastNewline = head.lastIndexOf("\n");
-  if (lastNewline <= 0) return head;
-  if (cap - lastNewline > TRUNCATION_LINE_BOUNDARY_TOLERANCE) return head;
-  return head.slice(0, lastNewline);
-}
-
 /** read the agent-edited learnings file. returns null when the file is
  * missing or unreadable (treated as "no change"). caps content at the
  * server's max length to avoid a 400 round-trip. */

utils/learningsTocRender.test.ts

@@ -82,6 +82,17 @@ describe("buildLearningsSection", () => {
     expect(out).not.toMatch(/\(L\d+-L\d+\)/);
   });
 
+  it("intro phrasing does not assert prior runs — works for fresh empty repos too", () => {
+    const out = buildLearningsSection({
+      filePath: "/tmp/run-1/pullfrog-learnings.md",
+      headings: [],
+    });
+    // load-bearing: fresh repos have zero previous runs. the prior copy
+    // ("accumulated by previous agent runs") was a lie in that case.
+    expect(out).not.toContain("accumulated by previous agent runs");
+    expect(out).toContain("maintained across runs");
+  });
+
   it("renders the TOC inline with the file path and heading guidance", () => {
     const out = buildLearningsSection({
       filePath: "/tmp/run-1/pullfrog-learnings.md",
@@ -92,6 +103,12 @@ describe("buildLearningsSection", () => {
     expect(out).toContain("- Build & test (L1-L18)");
     expect(out).toContain("- Architecture (L19-L60)");
     expect(out).toContain("Each range starts at the section heading line");
+    // re-read affordance: ranges reflect the run-start snapshot, so the
+    // agent needs an explicit nudge to re-read after any mid-run edits.
+    // mid-run edits shift the line numbers of every later section, not
+    // just the edited one — wording is explicit about that.
+    expect(out).toContain("run-start snapshot");
+    expect(out).toContain("any edit shifts the line numbers of every later section");
     // explicit "no hashes, no backticks" in the rendered list
     expect(out).not.toContain("- `## Build");
     expect(out).not.toContain("`## Build");

utils/learningsTruncate.ts

@@ -0,0 +1,42 @@
+/**
+ * pure string helpers for capping and line-boundary-truncating the
+ * `Repo.learnings` body. lives in its own module (vs alongside
+ * `learnings.ts`) so the proprietary root app can re-export it through
+ * `action/internal/index.ts` without dragging the entire MCP type graph
+ * along — `learnings.ts` imports `ToolContext` for its runtime helpers,
+ * and pulling that into the SDK-facing `internal` barrel expands the
+ * type graph reachable from root `tsc` and `cf-worker-indexing` to every
+ * tool module under `action/mcp/`. keeping these helpers MCP-free is the
+ * cheap structural fix.
+ *
+ * see `action/utils/learnings.ts` for the full learnings-file lifecycle.
+ */
+
+/** maximum size of `Repo.learnings` body in chars. action truncates the
+ * read-back BEFORE the PATCH to avoid sending an oversized payload; the
+ * server applies the same truncation as a defense-in-depth backstop (any
+ * caller that misses the client-side step would otherwise persist a
+ * mid-line tail, breaking the next-run TOC parse).
+ *
+ * raised from 10k → 100k once the TOC affordance landed: with line-range
+ * reads via the server-parsed TOC the agent doesn't ingest the whole
+ * file, so the cap is governed by curation discipline rather than a
+ * tight byte ceiling. 100k holds ~400-500 short bullets. */
+export const MAX_LEARNINGS_LENGTH = 100_000;
+
+/** truncate at the last newline boundary before `cap` so we don't leave
+ * a partial line at the tail (a half-truncated `## Headi` confuses the
+ * server's next-seed TOC parse and shrinks visible structure). falls
+ * back to a hard `slice` when the line boundary would discard a large
+ * run of content — i.e. when the tail of `head` is one giant line (rare:
+ * minified pastes, fenced log dumps). losing a partial last line is
+ * preferable to losing kilobytes of body. */
+const TRUNCATION_LINE_BOUNDARY_TOLERANCE = 4096;
+export function truncateAtLineBoundary(body: string, cap: number): string {
+  if (body.length <= cap) return body;
+  const head = body.slice(0, cap);
+  const lastNewline = head.lastIndexOf("\n");
+  if (lastNewline <= 0) return head;
+  if (cap - lastNewline > TRUNCATION_LINE_BOUNDARY_TOLERANCE) return head;
+  return head.slice(0, lastNewline);
+}