From a3a1a5913ad92dea32586f2fc903be4c14fef47e Mon Sep 17 00:00:00 2001 From: mebagwell Date: Mon, 21 Sep 2026 14:21:04 -0500 Subject: [PATCH 1/3] fix: match any heading level in replaceSection replaceSection only recognized top-level (#) headings. A call targeting a ## or ### heading fell through to the append path and silently created a duplicated top-level section (#34). A second consumer reproduced the same corruption independently. - match headings of any level (# through ######) - end a section at the next same-or-higher level heading - throw on ambiguous multi-match instead of editing the first hit - strip a leading content line that duplicates the target heading, since callers routinely re-include the heading line in the replacement content This is the hotfix that has run without incidents in our production deployment since 2026-09-19. Adds regression tests for each failure mode. --- packages/core/src/okf/bundle.ts | 36 +++++++++++++++++++++++++++------ packages/core/test/okf.test.ts | 31 ++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/packages/core/src/okf/bundle.ts b/packages/core/src/okf/bundle.ts index b4f5e14..b41319e 100644 --- a/packages/core/src/okf/bundle.ts +++ b/packages/core/src/okf/bundle.ts @@ -246,21 +246,45 @@ export class Bundle { } } -/** Replace the content under a top-level heading; append the section if absent. */ +/** Replace the content under a heading of any level; append a top-level + * section if absent; throw on ambiguous (multiple) matches. */ export function replaceSection(body: string, heading: string, content: string): string { const normalized = heading.replace(/^#+\s*/, ""); const lines = body.split("\n"); - const isHeading = (line: string) => /^#\s+/.test(line); - const start = lines.findIndex( - (line) => isHeading(line) && line.replace(/^#\s+/, "").trim() === normalized - ); + const isHeading = (line: string) => /^#{1,6}\s+/.test(line); + const level = (line: string) => line.match(/^#+/)![0].length; + const matches: number[] = []; + for (let i = 0; i < lines.length; i++) { + if (isHeading(lines[i]) && lines[i].replace(/^#{1,6}\s+/, "").trim() === normalized) { + matches.push(i); + } + } + if (matches.length > 1) { + throw new Error( + `replace_section: section "${normalized}" found ${matches.length} times — resolve duplicates via replace_body first.` + ); + } + const start = matches.length ? matches[0] : -1; + if (start !== -1) { + // Normalize: strip a leading content line that duplicates the target heading + const cLines = content.split("\n"); + const fi = cLines.findIndex((l) => l.trim().length > 0); + if ( + fi >= 0 && + /^#{1,6}\s+/.test(cLines[fi]) && + cLines[fi].replace(/^#{1,6}\s+/, "").trim() === normalized + ) { + content = cLines.slice(fi + 1).join("\n"); + } + } if (start === -1) { const suffix = body.trim().length > 0 ? "\n\n" : ""; return `${body.trimEnd()}${suffix}# ${normalized}\n\n${content.trim()}\n`; } + const matchedLevel = level(lines[start]); let end = lines.length; for (let i = start + 1; i < lines.length; i++) { - if (isHeading(lines[i])) { + if (isHeading(lines[i]) && level(lines[i]) <= matchedLevel) { end = i; break; } diff --git a/packages/core/test/okf.test.ts b/packages/core/test/okf.test.ts index 670c90d..126a7e4 100644 --- a/packages/core/test/okf.test.ts +++ b/packages/core/test/okf.test.ts @@ -150,6 +150,37 @@ describe("patch", () => { }); }); +describe("replaceSection (#34)", () => { + it("replaces under a ## heading instead of appending a duplicate section", () => { + const body = "# Title\n\nIntro.\n\n## Goals\n\nold\n\n## Notes\n\nn1\n"; + const out = replaceSection(body, "## Goals", "new goals"); + expect(out).toBe("# Title\n\nIntro.\n\n## Goals\n\nnew goals\n\n## Notes\n\nn1\n"); + }); + + it("ends the section at the next same-or-higher level heading, keeping nested headings", () => { + const body = "## Goals\n\na\n\n### Detail\n\nd\n\n## Notes\n\nn\n"; + const out = replaceSection(body, "Goals", "fresh"); + expect(out).toBe("## Goals\n\nfresh\n\n## Notes\n\nn\n"); + }); + + it("strips a duplicated heading line at the top of the replacement content", () => { + const body = "# Title\n\n## Gotchas\n\nold\n"; + const out = replaceSection(body, "## Gotchas", "## Gotchas\n\n- fresh gotcha"); + expect(out).toBe("# Title\n\n## Gotchas\n\n- fresh gotcha\n"); + expect(out.split("\n").filter((l) => l.trim() === "## Gotchas").length).toBe(1); + }); + + it("throws on ambiguous multi-match instead of editing the first hit", () => { + const body = "## A\n\nx\n\n## B\n\nx\n\n## A\n\ny\n"; + expect(() => replaceSection(body, "A", "z")).toThrow(/found 2 times/); + }); + + it("still appends a top-level section when absent", () => { + const out = replaceSection("body only", "Citations", "[1]"); + expect(out).toBe("body only\n\n# Citations\n\n[1]\n"); + }); +}); + describe("search", () => { beforeEach(async () => { await kb.writeConcept( From 92dea07a4e2851c054165ea1986a30a389512a71 Mon Sep 17 00:00:00 2001 From: mebagwell Date: Mon, 21 Sep 2026 14:35:43 -0500 Subject: [PATCH 2/3] fix: strip ATX closing hashes in heading match replaceSection treated a trailing ATX closing sequence as part of the title, so '## Goals ##' (legal CommonMark) did not match 'Goals' and fell into the append path, producing the duplicated top-level section this PR exists to remove. Strip a trailing '#+' sequence only when preceded by whitespace (CommonMark requires the space; 'Goals##' is the literal title 'Goals##') in both the match loop and the duplicate-strip, with regression tests. Also rename a test whose name contradicted its assertion, and refresh the replace_section schema description, which still described top-level-only matching after the previous commit. --- packages/core/src/agent/tools.ts | 2 +- packages/core/src/okf/bundle.ts | 13 +++++++------ packages/core/test/okf.test.ts | 14 +++++++++++++- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/packages/core/src/agent/tools.ts b/packages/core/src/agent/tools.ts index cd02211..038fd20 100644 --- a/packages/core/src/agent/tools.ts +++ b/packages/core/src/agent/tools.ts @@ -115,7 +115,7 @@ export function buildWriteTools(kb: KnowledgeBase, filesChanged: Set, tr heading: z .string() .min(1) - .describe("Top-level heading name, e.g. 'Schema'. Must be non-empty — to replace the whole body use replace_body instead."), + .describe("Heading name (any level, # through ######), e.g. 'Schema'. Must be non-empty — to replace the whole body use replace_body instead."), content: z.string().describe("New content for that section"), }) .optional(), diff --git a/packages/core/src/okf/bundle.ts b/packages/core/src/okf/bundle.ts index b41319e..6e35d7e 100644 --- a/packages/core/src/okf/bundle.ts +++ b/packages/core/src/okf/bundle.ts @@ -253,9 +253,14 @@ export function replaceSection(body: string, heading: string, content: string): const lines = body.split("\n"); const isHeading = (line: string) => /^#{1,6}\s+/.test(line); const level = (line: string) => line.match(/^#+/)![0].length; + // Title without the leading hashes; an optional ATX closing sequence is + // stripped only when preceded by whitespace (CommonMark §4.2 — "Goals##" + // is the literal title "Goals##"). + const headingTitle = (line: string) => + line.replace(/^#{1,6}\s+/, "").trim().replace(/\s+#+$/, ""); const matches: number[] = []; for (let i = 0; i < lines.length; i++) { - if (isHeading(lines[i]) && lines[i].replace(/^#{1,6}\s+/, "").trim() === normalized) { + if (isHeading(lines[i]) && headingTitle(lines[i]) === normalized) { matches.push(i); } } @@ -269,11 +274,7 @@ export function replaceSection(body: string, heading: string, content: string): // Normalize: strip a leading content line that duplicates the target heading const cLines = content.split("\n"); const fi = cLines.findIndex((l) => l.trim().length > 0); - if ( - fi >= 0 && - /^#{1,6}\s+/.test(cLines[fi]) && - cLines[fi].replace(/^#{1,6}\s+/, "").trim() === normalized - ) { + if (fi >= 0 && /^#{1,6}\s+/.test(cLines[fi]) && headingTitle(cLines[fi]) === normalized) { content = cLines.slice(fi + 1).join("\n"); } } diff --git a/packages/core/test/okf.test.ts b/packages/core/test/okf.test.ts index 126a7e4..300fcdd 100644 --- a/packages/core/test/okf.test.ts +++ b/packages/core/test/okf.test.ts @@ -157,12 +157,18 @@ describe("replaceSection (#34)", () => { expect(out).toBe("# Title\n\nIntro.\n\n## Goals\n\nnew goals\n\n## Notes\n\nn1\n"); }); - it("ends the section at the next same-or-higher level heading, keeping nested headings", () => { + it("replaces the full section, including nested subsections, up to the next same-or-higher heading", () => { const body = "## Goals\n\na\n\n### Detail\n\nd\n\n## Notes\n\nn\n"; const out = replaceSection(body, "Goals", "fresh"); expect(out).toBe("## Goals\n\nfresh\n\n## Notes\n\nn\n"); }); + it("matches headings written with an ATX closing hash sequence", () => { + const body = "# Title\n\nintro\n\n## Goals ##\n\nold\n\n## Notes\n\nn\n"; + const out = replaceSection(body, "## Goals", "new goals"); + expect(out).toBe("# Title\n\nintro\n\n## Goals ##\n\nnew goals\n\n## Notes\n\nn\n"); + }); + it("strips a duplicated heading line at the top of the replacement content", () => { const body = "# Title\n\n## Gotchas\n\nold\n"; const out = replaceSection(body, "## Gotchas", "## Gotchas\n\n- fresh gotcha"); @@ -170,6 +176,12 @@ describe("replaceSection (#34)", () => { expect(out.split("\n").filter((l) => l.trim() === "## Gotchas").length).toBe(1); }); + it("strips a duplicated closing-hash heading line at the top of the replacement content", () => { + const body = "# Title\n\n## Gotchas ##\n\nold\n"; + const out = replaceSection(body, "## Gotchas", "## Gotchas ##\n\n- fresh gotcha"); + expect(out).toBe("# Title\n\n## Gotchas ##\n\n- fresh gotcha\n"); + }); + it("throws on ambiguous multi-match instead of editing the first hit", () => { const body = "## A\n\nx\n\n## B\n\nx\n\n## A\n\ny\n"; expect(() => replaceSection(body, "A", "z")).toThrow(/found 2 times/); From e9ac0f0974ef6e270c6dd94ae83f2e7b839543c7 Mon Sep 17 00:00:00 2001 From: mebagwell Date: Mon, 21 Sep 2026 14:40:22 -0500 Subject: [PATCH 3/3] docs: describe replace_section heading levels patch_concept's tool description and the patchConcept doc comment still said replace_section handles one top-level '# Section'. Agent callers read these strings as the contract, so update both to any heading level (H1-H6) to match the schema and the implementation (#34). --- packages/core/src/agent/tools.ts | 2 +- packages/core/src/okf/bundle.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/agent/tools.ts b/packages/core/src/agent/tools.ts index 038fd20..54a62bf 100644 --- a/packages/core/src/agent/tools.ts +++ b/packages/core/src/agent/tools.ts @@ -103,7 +103,7 @@ export function buildWriteTools(kb: KnowledgeBase, filesChanged: Set, tr }), patch_concept: tool({ description: - "Targeted update of an existing concept: merge frontmatter keys (null deletes a key) and/or replace one top-level '# Section' body section. Prefer this over write_concept for small edits.", + "Targeted update of an existing concept: merge frontmatter keys (null deletes a key) and/or replace one heading body section (any level, H1-H6). Prefer this over write_concept for small edits.", inputSchema: z.object({ path: conceptPath, frontmatter: z diff --git a/packages/core/src/okf/bundle.ts b/packages/core/src/okf/bundle.ts index 6e35d7e..e937548 100644 --- a/packages/core/src/okf/bundle.ts +++ b/packages/core/src/okf/bundle.ts @@ -123,7 +123,7 @@ export class Bundle { /** * Targeted update: merge frontmatter keys (null deletes a key) and/or - * replace the content under one top-level "# Section" heading. + * replace the content under one heading (any level) "# Section". */ async patchConcept( bundlePath: string,