From ab9769861d0e651f372d864978032b581f073b54 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Thu, 16 Apr 2026 12:26:20 +1000 Subject: [PATCH 1/5] fix: prevent markdown fence balancer from corrupting sequential code blocks The balanceCodeRegions() function was incorrectly detecting 'true nesting' when a document contained sequential code blocks without intermediate language-tagged openers. For example, a ```cpp block followed by a bare ``` block would be merged into a single ```` block, corrupting the markdown by treating the closing/opening fences as content. Root cause: the isTrueNesting condition fired when closerCount > openerCount + 1, but when openerCount was 0 (no intermediate language-tagged fences), this triggered for any 2+ bare closers. Fix: require openerCount > 0 for nesting detection. Without intermediate language-tagged openers, multiple bare closers are sequential code blocks (greedy matching per CommonMark), not nested content. Also unskips 6 tests that were masked by this bug, adds real-world regression tests including the exact pattern from the production bug, and adds idempotency and invariant tests. Test count: 66 passing + 6 skipped -> 82 passing, 0 skipped. --- .../js/markdown_code_region_balancer.cjs | 5 +- .../js/markdown_code_region_balancer.test.cjs | 404 +++++++++++++++--- 2 files changed, 353 insertions(+), 56 deletions(-) diff --git a/actions/setup/js/markdown_code_region_balancer.cjs b/actions/setup/js/markdown_code_region_balancer.cjs index 11f7a46066e..58802eb3317 100644 --- a/actions/setup/js/markdown_code_region_balancer.cjs +++ b/actions/setup/js/markdown_code_region_balancer.cjs @@ -236,8 +236,11 @@ function balanceCodeRegions(markdown) { // True nesting: more closers than openers (e.g., 1 opener, 3 closers) // Nested blocks: closers = openers + 1 (e.g., 2 openers [including us], 2 closers) + // IMPORTANT: nesting requires at least one intermediate opener with a language tag. + // Without intermediate openers, multiple bare closers are sequential code blocks, + // not nested content (e.g., ```cpp...``` followed by ```...``` are two separate blocks). const closerCount = directClosers.length; - const isTrueNesting = closerCount > openerCount + 1; + const isTrueNesting = openerCount > 0 && closerCount > openerCount + 1; if (isTrueNesting) { // TRUE nesting - use the LAST closer and escape middle ones diff --git a/actions/setup/js/markdown_code_region_balancer.test.cjs b/actions/setup/js/markdown_code_region_balancer.test.cjs index f8538f5a71e..6922766c598 100644 --- a/actions/setup/js/markdown_code_region_balancer.test.cjs +++ b/actions/setup/js/markdown_code_region_balancer.test.cjs @@ -44,7 +44,9 @@ End`; }); describe("nested code regions with same indentation", () => { - it("should escape nested backtick fence inside code block", () => { + it("should use greedy matching for bare fences without intermediate language openers", () => { + // Without intermediate language-tagged openers, bare fences form sequential + // code blocks (greedy matching per CommonMark), not nested content. const input = `\`\`\`javascript function test() { \`\`\` @@ -52,17 +54,14 @@ nested \`\`\` } \`\`\``; - const expected = `\`\`\`\`javascript -function test() { -\`\`\` -nested -\`\`\` -} -\`\`\`\``; - expect(balancer.balanceCodeRegions(input)).toBe(expected); + // Already balanced: two sequential code blocks + expect(balancer.balanceCodeRegions(input)).toBe(input); + expect(balancer.isBalanced(balancer.balanceCodeRegions(input))).toBe(true); }); - it("should escape nested tilde fence inside code block", () => { + it("should use greedy matching for bare tilde fences without intermediate language openers", () => { + // Without intermediate language-tagged openers, bare fences form sequential + // code blocks (greedy matching per CommonMark), not nested content. const input = `~~~markdown Example: ~~~ @@ -70,38 +69,53 @@ nested ~~~ End ~~~`; - const expected = `~~~~markdown -Example: -~~~ -nested -~~~ -End -~~~~`; - expect(balancer.balanceCodeRegions(input)).toBe(expected); + // Already balanced: two sequential code blocks + expect(balancer.balanceCodeRegions(input)).toBe(input); + expect(balancer.isBalanced(balancer.balanceCodeRegions(input))).toBe(true); }); - it("should handle multiple nested fences", () => { + it("should use greedy matching for multiple bare fences without intermediate language openers", () => { + // Multiple bare fences without intermediate language-tagged openers + // are sequential code blocks, not nested content. + // With 5 fences (odd), greedy matching pairs 0-1, 2-3, and fence 4 is unclosed. + // The balancer adds a closing fence for the unclosed block. const input = `\`\`\`javascript function test() { \`\`\` first nested \`\`\` -\`\`\` second nested \`\`\` } \`\`\``; - const expected = `\`\`\`\`javascript + const expected = `\`\`\`javascript function test() { \`\`\` first nested \`\`\` -\`\`\` second nested \`\`\` } -\`\`\`\``; - expect(balancer.balanceCodeRegions(input)).toBe(expected); +\`\`\` +\`\`\``; + // Greedy matching: 2 paired blocks + 1 unclosed (auto-closed) + const result = balancer.balanceCodeRegions(input); + expect(result).toBe(expected); + expect(balancer.isBalanced(result)).toBe(true); + }); + + it("should still detect nesting when intermediate language-tagged openers exist", () => { + // When there IS an intermediate opener with a language tag, nesting + // detection should still work (the outer fence needs to be longer). + const input = `\`\`\`markdown +Here's an example: +\`\`\`python +print("hello") +\`\`\` +End +\`\`\``; + const result = balancer.balanceCodeRegions(input); + expect(balancer.isBalanced(result)).toBe(true); }); }); @@ -147,19 +161,15 @@ content }); describe("fence lengths", () => { - // TODO: Edge case - separate sequential blocks with different fence lengths incorrectly identified as nested - it.skip("should require closing fence to be at least as long as opening", () => { + it("should treat shorter bare fence inside longer fence block as content", () => { + // A 5-backtick opener cannot be closed by a 3-backtick fence (CommonMark rule), + // so the 3-backtick fences are content inside the block, not separate blocks. const input = `\`\`\`\`\` content \`\`\` should be escaped \`\`\`\`\``; - const expected = `\`\`\`\`\`\` -content -\`\`\` -should be escaped -\`\`\`\`\`\``; - expect(balancer.balanceCodeRegions(input)).toBe(expected); + expect(balancer.balanceCodeRegions(input)).toBe(input); }); it("should allow longer closing fence", () => { @@ -171,8 +181,7 @@ end`; expect(balancer.balanceCodeRegions(input)).toBe(input); }); - // TODO: Edge case - multiple separate blocks with vastly different fence lengths incorrectly identified as nested - it.skip("should handle various fence lengths", () => { + it("should handle various fence lengths as separate blocks", () => { const input = `\`\`\` three \`\`\` @@ -187,21 +196,16 @@ seven expect(balancer.balanceCodeRegions(input)).toBe(input); }); - // TODO: Edge case - 6-char opener with 3-char inner fences incorrectly calculated - it.skip("should escape shorter fence inside longer fence block", () => { + it("should treat shorter fences inside longer fence block as content", () => { + // A 6-backtick opener cannot be closed by 3-backtick fences, + // so they're content, not closers. const input = `\`\`\`\`\`\` content \`\`\` nested short fence \`\`\` \`\`\`\`\`\``; - const expected = `\`\`\`\`\`\`\` -content -\`\`\` -nested short fence -\`\`\` -\`\`\`\`\`\`\``; - expect(balancer.balanceCodeRegions(input)).toBe(expected); + expect(balancer.balanceCodeRegions(input)).toBe(input); }); }); @@ -317,10 +321,9 @@ content }); describe("complex real-world scenarios", () => { - // TODO: This test is currently skipped due to a known issue with the algorithm - // The algorithm treats fences inside code blocks as real fences, causing incorrect escaping - // See: https://github.com/github/gh-aw/issues/XXXXX - it.skip("should handle AI-generated code with nested markdown", () => { + it("should handle AI-generated code with nested markdown", () => { + // The markdown and javascript fences are sequential blocks with greedy matching. + // markdown opens, first bare ``` closes it, javascript opens, second bare ``` closes it. const input = `# Example Here's how to use code blocks: @@ -335,7 +338,6 @@ function hello() { \`\`\` Text after`; - // No changes expected - the javascript block is separate from the markdown block expect(balancer.balanceCodeRegions(input)).toBe(input); }); @@ -356,8 +358,7 @@ print("hello") expect(balancer.balanceCodeRegions(input)).toBe(input); }); - // TODO: Edge case - separate blocks being incorrectly treated as nested - it.skip("should handle mixed fence types in document", () => { + it("should handle mixed fence types in document", () => { const input = `\`\`\`javascript const x = 1; \`\`\` @@ -372,10 +373,9 @@ generic code expect(balancer.balanceCodeRegions(input)).toBe(input); }); - // TODO: This test is currently skipped due to a known issue with the algorithm - // The algorithm treats fences inside code blocks as real fences, causing incorrect escaping - // See: https://github.com/github/gh-aw/issues/XXXXX - it.skip("should handle deeply nested example", () => { + it("should handle deeply nested example", () => { + // Greedy matching: markdown opens, first bare ``` closes it, + // javascript opens, second bare ``` closes it. Two sequential blocks. const input = `\`\`\`markdown # Tutorial @@ -385,7 +385,6 @@ code here More text \`\`\``; - // No changes expected - the javascript block is separate from the markdown block expect(balancer.balanceCodeRegions(input)).toBe(input); }); @@ -816,6 +815,75 @@ content expect(typeof result).toBe("string"); }); + it("should not corrupt sequential code blocks with different languages", () => { + // Regression test: two separate code blocks where the first has a language tag + // and the second is bare. The balancer should NOT merge them into one block + // by increasing fence lengths. + const input = `## C++ Source + +\`\`\`cpp +template +const T interpolateN(const T &value, const T(&y)[N]) +{ + return y[0]; +} +\`\`\` + +Used for motor thrust curves. + +## Verification Status + +\`\`\` +LEAN_AVAILABLE=true +LAKE_BUILD=passed +\`\`\` + +All 19 build jobs passed.`; + + const result = balancer.balanceCodeRegions(input); + // The output should be identical to the input - two separate balanced blocks + expect(result).toBe(input); + expect(balancer.isBalanced(result)).toBe(true); + }); + + it("should not corrupt three sequential code blocks", () => { + // Three separate code blocks, none nested + const input = `\`\`\`python +print("hello") +\`\`\` + +Some text. + +\`\`\`javascript +console.log("world") +\`\`\` + +More text. + +\`\`\` +plain code +\`\`\``; + + const result = balancer.balanceCodeRegions(input); + expect(result).toBe(input); + expect(balancer.isBalanced(result)).toBe(true); + }); + + it("should not corrupt two code blocks where first has language and second is bare", () => { + // This is the exact pattern that caused the bug: ```lang ... ``` followed by ``` ... ``` + const input = `\`\`\`cpp +code1 +\`\`\` + +\`\`\` +code2 +\`\`\``; + + const result = balancer.balanceCodeRegions(input); + expect(result).toBe(input); + expect(balancer.isBalanced(result)).toBe(true); + }); + it("should handle random fence variations", () => { // Generate random fence lengths and types const fenceChars = ["`", "~"]; @@ -831,5 +899,231 @@ content expect(balancer.isBalanced(result)).toBe(true); } }); + + describe("real-world regression tests", () => { + it("should not corrupt PR body with cpp code block followed by bare verification block", () => { + // Exact pattern from the bug report: a ```cpp block followed by a ``` block + // The balancer was wrapping both in ````...```` and treating the middle + // closing/opening fences as content. + const input = `## C++ Source + +\`\`\`cpp +// src/lib/mathlib/math/Functions.hpp (~line 180) +template +const T interpolateN(const T &value, const T(&y)[N]) +{ + size_t index = constrain((int)(value * (N - 1)), 0, (int)(N - 2)); + return interpolate(value, + (T)index / (T)(N - 1), + (T)(index + 1) / (T)(N - 1), + y[index], y[index + 1]); +} +\`\`\` + +Used for motor thrust curves (N=5 or N=9), RC stick sensitivity curves, and control surface deflection mappings. + +## Verification Status + +> Proofs verified: lake build passed with Lean 4.29.0. 0 sorry remain. + +\`\`\` +LEAN_AVAILABLE=true +Lean (version 4.29.0, x86_64-unknown-linux-gnu) +LAKE_BUILD=passed +\`\`\` + +All 19 build jobs passed (including all 17 prior Lean files).`; + + const result = balancer.balanceCodeRegions(input); + expect(result).toBe(input); + expect(balancer.isBalanced(result)).toBe(true); + }); + + it("should not corrupt markdown with multiple language-tagged blocks and one bare block", () => { + // Pattern: several language-tagged blocks + one bare block at the end + const input = `## Code + +\`\`\`python +def hello(): + print("Hello") +\`\`\` + +## Tests + +\`\`\`python +def test_hello(): + assert True +\`\`\` + +## Output + +\`\`\` +Hello +\`\`\``; + + const result = balancer.balanceCodeRegions(input); + expect(result).toBe(input); + expect(balancer.isBalanced(result)).toBe(true); + }); + + it("should not corrupt a document with 4+ sequential code blocks", () => { + // Common in documentation: many sequential blocks + const input = `\`\`\`bash +npm install +\`\`\` + +\`\`\`javascript +const x = 1; +\`\`\` + +\`\`\`json +{"key": "value"} +\`\`\` + +\`\`\` +plain text output +\`\`\` + +\`\`\`yaml +key: value +\`\`\``; + + const result = balancer.balanceCodeRegions(input); + expect(result).toBe(input); + expect(balancer.isBalanced(result)).toBe(true); + }); + + it("should handle GitHub-style blockquote containing a code block", () => { + // Code blocks inside blockquotes are common in GitHub comments + const input = `Some text before. + +> Here is a quote with code: +> \`\`\` +> echo "hello" +> \`\`\` + +Text after.`; + + const result = balancer.balanceCodeRegions(input); + // The > prefix means these fences have different indentation patterns + // and won't match top-level fences + expect(result).toBe(input); + }); + + it("should handle PR body with footer containing fenced install command", () => { + // Common pattern in gh-aw: PR bodies end with a fenced install command + const input = `## Summary + +Some PR summary text. + +\`\`\`python +code here +\`\`\` + +> Generated by workflow. +> +> To install, run +> \`\`\` +> gh aw add owner/repo/workflow.md@sha +> \`\`\``; + + const result = balancer.balanceCodeRegions(input); + expect(result).toBe(input); + expect(balancer.isBalanced(result)).toBe(true); + }); + }); + + describe("true nesting with intermediate language openers", () => { + it("should handle markdown block containing a language-tagged inner block", () => { + // This IS true nesting: ```markdown contains ```python inside it + // The balancer should handle this by making the result balanced + const input = `\`\`\`markdown +Here's an example: +\`\`\`python +print("hello") +\`\`\` +End of example +\`\`\``; + const result = balancer.balanceCodeRegions(input); + expect(balancer.isBalanced(result)).toBe(true); + }); + + it("should handle nested documentation example with multiple inner blocks", () => { + // Markdown block containing multiple language-tagged inner blocks + const input = `\`\`\`markdown +## Usage + +\`\`\`javascript +const x = 1; +\`\`\` + +\`\`\`python +y = 2 +\`\`\` + +End +\`\`\``; + const result = balancer.balanceCodeRegions(input); + expect(balancer.isBalanced(result)).toBe(true); + }); + }); + + describe("idempotency", () => { + it("should be idempotent - running twice gives same result", () => { + const inputs = [ + "```javascript\ncode\n```", + "```\nblock1\n```\n\n```\nblock2\n```", + "```cpp\ncode\n```\n\n```\noutput\n```", + "```markdown\n```python\ncode\n```\nend\n```", + "```javascript\nunclosed", + "```\nfirst\n```\nsecond\n```\nthird\n```", + ]; + + for (const input of inputs) { + const first = balancer.balanceCodeRegions(input); + const second = balancer.balanceCodeRegions(first); + expect(second).toBe(first); + } + }); + }); + + describe("output correctness invariants", () => { + it("should always produce balanced output or no worse than input", () => { + const inputs = [ + "```\ncode\n```", + "```js\ncode", + "```\na\n```\nb\n```\nc\n```", + "```\na\n```\n```\nb\n```\n```\nc\n```", + "````\na\n```\nb\n```\nc\n````", + "```markdown\n```js\ncode\n```\n```", + "~~~\na\n~~~\n```\nb\n```", + "```\n```\n```", + ]; + + for (const input of inputs) { + const result = balancer.balanceCodeRegions(input); + const inputCounts = balancer.countCodeRegions(input); + const resultCounts = balancer.countCodeRegions(result); + expect(resultCounts.unbalanced).toBeLessThanOrEqual(inputCounts.unbalanced); + } + }); + + it("should never change already-balanced content (beyond line-ending normalization)", () => { + const balanced = [ + "```js\ncode\n```", + "~~~md\ntext\n~~~", + "```\na\n```\n\n```\nb\n```", + "```cpp\ncode\n```\n\n```\noutput\n```", + "````\ncode with ``` inside\n````", + "```js\na\n```\n~~~py\nb\n~~~\n```\nc\n```", + " ```\n indented\n ```", + "```js {highlight}\ncode\n```", + ]; + + for (const input of balanced) { + expect(balancer.balanceCodeRegions(input)).toBe(input); + } + }); + }); }); }); From 72b6bd7e70058c7326fda946fe1b0fef580eeb75 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Fri, 17 Apr 2026 17:28:06 +1000 Subject: [PATCH 2/5] fix: resolve TS2345 error in copilot_driver.cjs appendFileSync type --- actions/setup/js/copilot_driver.cjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actions/setup/js/copilot_driver.cjs b/actions/setup/js/copilot_driver.cjs index 44c6fff1be8..f12d1c4804f 100644 --- a/actions/setup/js/copilot_driver.cjs +++ b/actions/setup/js/copilot_driver.cjs @@ -137,7 +137,7 @@ function buildInfrastructureIncompletePayload(details) { /** * Append one safe-output entry line. - * @param {(path: string, data: string, encoding: string) => void} appendFileSync + * @param {typeof import("fs").appendFileSync} appendFileSync * @param {string} safeOutputsPath * @param {string} payload */ @@ -151,7 +151,7 @@ function appendSafeOutputLine(appendFileSync, safeOutputsPath, payload) { * @param {string} details * @param {{ * safeOutputsPath?: string, - * appendFileSync?: (path: string, data: string, encoding: string) => void, + * appendFileSync?: typeof import("fs").appendFileSync, * logger?: (message: string) => void * }=} options */ From 136951596fd38e9a6f0c8ae38cf9708a12b52cde Mon Sep 17 00:00:00 2001 From: Don Syme Date: Fri, 17 Apr 2026 17:52:35 +1000 Subject: [PATCH 3/5] refactor: remove dead nesting detection code, simplify to pure greedy matching The isTrueNesting branch was unreachable: directClosers are filtered to have hasOpenerBetween === false, so openerCount over i+1..lastDirectCloser is always 0. Remove the dead code path, fenceLengthAdjustments map, and the fence-length rewriting pass. The balancer now uses straightforward CommonMark greedy matching throughout. -95 lines of dead/unreachable code removed. --- .../js/markdown_code_region_balancer.cjs | 112 +++--------------- 1 file changed, 17 insertions(+), 95 deletions(-) diff --git a/actions/setup/js/markdown_code_region_balancer.cjs b/actions/setup/js/markdown_code_region_balancer.cjs index 58802eb3317..e6f34c86023 100644 --- a/actions/setup/js/markdown_code_region_balancer.cjs +++ b/actions/setup/js/markdown_code_region_balancer.cjs @@ -119,14 +119,13 @@ function balanceCodeRegions(markdown) { } } - // Third pass: Match fences, detecting and fixing nested patterns + // Third pass: Match fences using greedy matching (CommonMark rules) // Strategy: // 1. Process fences in order // 2. For each opener, find potential closers // 3. If first closer has intermediate opener, defer this opener // 4. Otherwise, pair with first direct closer (greedy matching) // 5. Make a second pass for deferred openers - const fenceLengthAdjustments = new Map(); // lineIndex -> new length const processed = new Set(); const deferred = new Set(); // Fences to process in second pass const unclosedFences = []; @@ -217,83 +216,20 @@ function balanceCodeRegions(markdown) { processed.delete(i); // Unmark so it can be processed in second pass i++; } else { - // No opener before the first closer, so it's a direct match - // Check if there are MORE closers without intermediate openers - const directClosers = potentialClosers.filter(c => !c.hasOpenerBetween); - - if (directClosers.length > 1) { - // Multiple bare closers without intermediate openers - // Count openers between our opener and the last direct closer to determine if this is true nesting - const lastDirectCloser = directClosers[directClosers.length - 1]; - let openerCount = 0; - for (let k = i + 1; k < lastDirectCloser.index; k++) { - if (processed.has(k)) continue; - const intermediateFence = fences[k]; - if (intermediateFence.language !== "" && intermediateFence.indent.length === openIndentLength) { - openerCount++; - } - } - - // True nesting: more closers than openers (e.g., 1 opener, 3 closers) - // Nested blocks: closers = openers + 1 (e.g., 2 openers [including us], 2 closers) - // IMPORTANT: nesting requires at least one intermediate opener with a language tag. - // Without intermediate openers, multiple bare closers are sequential code blocks, - // not nested content (e.g., ```cpp...``` followed by ```...``` are two separate blocks). - const closerCount = directClosers.length; - const isTrueNesting = openerCount > 0 && closerCount > openerCount + 1; - - if (isTrueNesting) { - // TRUE nesting - use the LAST closer and escape middle ones - const closerIndex = lastDirectCloser.index; - processed.add(closerIndex); - - pairedBlocks.push({ - start: fences[i].lineIndex, - end: fences[closerIndex].lineIndex, - openIndex: i, - closeIndex: closerIndex, - }); - - // Increase fence length so middle closers can no longer close - const maxLength = Math.max(...directClosers.map(c => c.length), openFence.length); - const newLength = maxLength + 1; - fenceLengthAdjustments.set(fences[i].lineIndex, newLength); - fenceLengthAdjustments.set(fences[closerIndex].lineIndex, newLength); - - // Mark middle closers as processed (they're now treated as content) - for (let k = 0; k < directClosers.length - 1; k++) { - processed.add(directClosers[k].index); - } - - i = closerIndex + 1; - } else { - // Nested blocks - use the FIRST direct closer (greedy matching) - const closerIndex = directClosers[0].index; - processed.add(closerIndex); - - pairedBlocks.push({ - start: fences[i].lineIndex, - end: fences[closerIndex].lineIndex, - openIndex: i, - closeIndex: closerIndex, - }); - - i = closerIndex + 1; - } - } else { - // Only one direct closer, use it (normal case) - const closerIndex = firstCloser.index; - processed.add(closerIndex); - - pairedBlocks.push({ - start: fences[i].lineIndex, - end: fences[closerIndex].lineIndex, - openIndex: i, - closeIndex: closerIndex, - }); - - i = closerIndex + 1; - } + // No opener before the first closer — greedy match with it. + // Per CommonMark, the first bare fence that can close our opener does close it. + // Any subsequent bare fences start new blocks (sequential, not nested). + const closerIndex = firstCloser.index; + processed.add(closerIndex); + + pairedBlocks.push({ + start: fences[i].lineIndex, + end: fences[closerIndex].lineIndex, + openIndex: i, + closeIndex: closerIndex, + }); + + i = closerIndex + 1; } } else { // No closer found - check if this fence is inside a paired block @@ -350,23 +286,9 @@ function balanceCodeRegions(markdown) { } } - // Fifth pass: build result with adjusted fence lengths + // Fifth pass: build result (copy lines, then close any unclosed fences) for (let i = 0; i < lines.length; i++) { - if (fenceLengthAdjustments.has(i)) { - const newLength = fenceLengthAdjustments.get(i); - const fenceMatch = lines[i].match(/^(\s*)(`{3,}|~{3,})([^`~\s]*)?(.*)$/); - if (fenceMatch) { - const indent = fenceMatch[1]; - const char = fenceMatch[2][0]; - const language = fenceMatch[3] || ""; - const trailing = fenceMatch[4] || ""; - result.push(`${indent}${char.repeat(newLength)}${language}${trailing}`); - } else { - result.push(lines[i]); - } - } else { - result.push(lines[i]); - } + result.push(lines[i]); } // Fifth pass: close any unclosed fences From 8e59b44d303dfcc1e605b12b5d4649d5e67d9a00 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Fri, 17 Apr 2026 17:59:34 +1000 Subject: [PATCH 4/5] fix: align isBalanced()/countCodeRegions() closer rule with CommonMark Closing fences must be bare (no info string/language tag) per CommonMark. isBalanced() and countCodeRegions() were not checking this, giving false confidence in test assertions. Update both functions and fix tests for ambiguous nesting cases to assert the correct invariant (no worse than input) instead of asserting balanced output. --- .../js/markdown_code_region_balancer.cjs | 8 +++-- .../js/markdown_code_region_balancer.test.cjs | 34 ++++++++++++------- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/actions/setup/js/markdown_code_region_balancer.cjs b/actions/setup/js/markdown_code_region_balancer.cjs index e6f34c86023..b5e20cc7c4a 100644 --- a/actions/setup/js/markdown_code_region_balancer.cjs +++ b/actions/setup/js/markdown_code_region_balancer.cjs @@ -353,7 +353,9 @@ function isBalanced(markdown) { length: fenceLength, }; } else { - const canClose = openingFence !== null && fenceChar === openingFence.char && fenceLength >= openingFence.length; + // Per CommonMark, a closing fence must be bare (no info string/language) + const language = fenceMatch[3] || ""; + const canClose = openingFence !== null && fenceChar === openingFence.char && fenceLength >= openingFence.length && language === ""; if (canClose) { inCodeBlock = false; @@ -403,7 +405,9 @@ function countCodeRegions(markdown) { length: fenceLength, }; } else { - const canClose = openingFence !== null && fenceChar === openingFence.char && fenceLength >= openingFence.length; + // Per CommonMark, a closing fence must be bare (no info string/language) + const language = fenceMatch[3] || ""; + const canClose = openingFence !== null && fenceChar === openingFence.char && fenceLength >= openingFence.length && language === ""; if (canClose) { inCodeBlock = false; diff --git a/actions/setup/js/markdown_code_region_balancer.test.cjs b/actions/setup/js/markdown_code_region_balancer.test.cjs index 6922766c598..987a5e11487 100644 --- a/actions/setup/js/markdown_code_region_balancer.test.cjs +++ b/actions/setup/js/markdown_code_region_balancer.test.cjs @@ -104,9 +104,10 @@ second nested expect(balancer.isBalanced(result)).toBe(true); }); - it("should still detect nesting when intermediate language-tagged openers exist", () => { - // When there IS an intermediate opener with a language tag, nesting - // detection should still work (the outer fence needs to be longer). + it("should not make things worse when intermediate language-tagged openers exist", () => { + // This pattern is inherently ambiguous under CommonMark greedy matching: + // ```markdown pairs with the first bare ```, leaving the final ``` unclosed. + // The balancer cannot resolve this, but must not make it worse. const input = `\`\`\`markdown Here's an example: \`\`\`python @@ -115,7 +116,9 @@ print("hello") End \`\`\``; const result = balancer.balanceCodeRegions(input); - expect(balancer.isBalanced(result)).toBe(true); + const inputCounts = balancer.countCodeRegions(input); + const resultCounts = balancer.countCodeRegions(result); + expect(resultCounts.unbalanced).toBeLessThanOrEqual(inputCounts.unbalanced); }); }); @@ -1033,10 +1036,11 @@ code here }); }); - describe("true nesting with intermediate language openers", () => { - it("should handle markdown block containing a language-tagged inner block", () => { - // This IS true nesting: ```markdown contains ```python inside it - // The balancer should handle this by making the result balanced + describe("ambiguous nesting with intermediate language openers", () => { + it("should not make things worse for markdown block with language-tagged inner block", () => { + // Inherently ambiguous: greedy matching pairs ```markdown with the first + // bare ```, so ```python and the final ``` become separate (unbalanced) blocks. + // The balancer cannot resolve this but must not degrade the output. const input = `\`\`\`markdown Here's an example: \`\`\`python @@ -1045,11 +1049,15 @@ print("hello") End of example \`\`\``; const result = balancer.balanceCodeRegions(input); - expect(balancer.isBalanced(result)).toBe(true); + const inputCounts = balancer.countCodeRegions(input); + const resultCounts = balancer.countCodeRegions(result); + expect(resultCounts.unbalanced).toBeLessThanOrEqual(inputCounts.unbalanced); }); - it("should handle nested documentation example with multiple inner blocks", () => { - // Markdown block containing multiple language-tagged inner blocks + it("should not make things worse for markdown block with multiple inner blocks", () => { + // Multiple language-tagged inner blocks inside a markdown fence. + // Greedy matching pairs the outer opener with the first bare closer, + // leaving subsequent blocks ambiguous. const input = `\`\`\`markdown ## Usage @@ -1064,7 +1072,9 @@ y = 2 End \`\`\``; const result = balancer.balanceCodeRegions(input); - expect(balancer.isBalanced(result)).toBe(true); + const inputCounts = balancer.countCodeRegions(input); + const resultCounts = balancer.countCodeRegions(result); + expect(resultCounts.unbalanced).toBeLessThanOrEqual(inputCounts.unbalanced); }); }); From 2e82516b272d0a982ea7dec7b06ab826d07e9dc5 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Fri, 17 Apr 2026 20:17:18 +1000 Subject: [PATCH 5/5] test: update WASM golden files for agent folder ordering changes --- .../basic-copilot.golden | 11 ++++++----- .../with-imports.golden | 11 ++++++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden b/pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden index 42c213d1b21..31fabc9bafb 100644 --- a/pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden +++ b/pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden @@ -86,14 +86,14 @@ jobs: actions/setup .claude .codex - .gemini .crush + .gemini sparse-checkout-cone-mode: true fetch-depth: 1 - name: Save agent config folders for base branch restoration env: - GH_AW_AGENT_FOLDERS: ".agents .claude .codex .gemini .github .crush" - GH_AW_AGENT_FILES: "AGENTS.md CLAUDE.md GEMINI.md .crush.json" + GH_AW_AGENT_FOLDERS: ".agents .claude .codex .crush .gemini .github" + GH_AW_AGENT_FILES: ".crush.json AGENTS.md CLAUDE.md GEMINI.md" # poutine:ignore untrusted_checkout_exec run: bash "${RUNNER_TEMP}/gh-aw/actions/save_base_github_folders.sh" - name: Check workflow lock file @@ -385,8 +385,8 @@ jobs: - name: Restore agent config folders from base branch if: steps.checkout-pr.outcome == 'success' env: - GH_AW_AGENT_FOLDERS: ".agents .claude .codex .gemini .github .crush" - GH_AW_AGENT_FILES: "AGENTS.md CLAUDE.md GEMINI.md .crush.json" + GH_AW_AGENT_FOLDERS: ".agents .claude .codex .crush .gemini .github" + GH_AW_AGENT_FILES: ".crush.json AGENTS.md CLAUDE.md GEMINI.md" run: bash "${RUNNER_TEMP}/gh-aw/actions/restore_base_github_folders.sh" - name: Clean git credentials continue-on-error: true @@ -574,3 +574,4 @@ jobs: setupGlobals(core, github, context, exec, io, getOctokit); const { main } = require('${{ runner.temp }}/gh-aw/actions/check_membership.cjs'); await main(); + diff --git a/pkg/workflow/testdata/TestWasmGolden_CompileFixtures/with-imports.golden b/pkg/workflow/testdata/TestWasmGolden_CompileFixtures/with-imports.golden index b1a0745cb87..c7f2fb5d162 100644 --- a/pkg/workflow/testdata/TestWasmGolden_CompileFixtures/with-imports.golden +++ b/pkg/workflow/testdata/TestWasmGolden_CompileFixtures/with-imports.golden @@ -86,14 +86,14 @@ jobs: actions/setup .claude .codex - .gemini .crush + .gemini sparse-checkout-cone-mode: true fetch-depth: 1 - name: Save agent config folders for base branch restoration env: - GH_AW_AGENT_FOLDERS: ".agents .claude .codex .gemini .github .crush" - GH_AW_AGENT_FILES: "AGENTS.md CLAUDE.md GEMINI.md .crush.json" + GH_AW_AGENT_FOLDERS: ".agents .claude .codex .crush .gemini .github" + GH_AW_AGENT_FILES: ".crush.json AGENTS.md CLAUDE.md GEMINI.md" # poutine:ignore untrusted_checkout_exec run: bash "${RUNNER_TEMP}/gh-aw/actions/save_base_github_folders.sh" - name: Check workflow lock file @@ -386,8 +386,8 @@ jobs: - name: Restore agent config folders from base branch if: steps.checkout-pr.outcome == 'success' env: - GH_AW_AGENT_FOLDERS: ".agents .claude .codex .gemini .github .crush" - GH_AW_AGENT_FILES: "AGENTS.md CLAUDE.md GEMINI.md .crush.json" + GH_AW_AGENT_FOLDERS: ".agents .claude .codex .crush .gemini .github" + GH_AW_AGENT_FILES: ".crush.json AGENTS.md CLAUDE.md GEMINI.md" run: bash "${RUNNER_TEMP}/gh-aw/actions/restore_base_github_folders.sh" - name: Clean git credentials continue-on-error: true @@ -575,3 +575,4 @@ jobs: setupGlobals(core, github, context, exec, io, getOctokit); const { main } = require('${{ runner.temp }}/gh-aw/actions/check_membership.cjs'); await main(); +