Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -1036,6 +1036,12 @@ clean-docs:
@rm -rf docs/dist docs/node_modules docs/.astro
@echo "✓ Documentation artifacts cleaned"

.PHONY: test-docs-remark
test-docs-remark:
@echo "Running docs remark plugin unit tests..."
@node docs/src/lib/remark/inlineMarkdownInHtml.test.js
Comment on lines +1040 to +1042
@echo "✓ Docs remark plugin unit tests passed"

# Sync templates from .github to pkg/cli/templates
# Sync action pins from .github/aw to pkg/actionpins/data and pkg/workflow/data
.PHONY: sync-action-pins
Expand Down
3 changes: 2 additions & 1 deletion docs/astro.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { fileURLToPath } from 'node:url';
import { unified } from '@astrojs/markdown-remark';
import remarkStripEmojis from './src/lib/remark/stripEmojis.js';
import remarkTableDataLabels from './src/lib/remark/tableDataLabels.js';
import remarkInlineMarkdownInHtml from './src/lib/remark/inlineMarkdownInHtml.js';
import rehypeTableWrapper from './src/lib/rehype/tableWrapper.js';
import { WORKSHOP_SLUGS } from './src/lib/workshop/config.ts';

Expand Down Expand Up @@ -40,7 +41,7 @@ export default defineConfig({
trailingSlash: 'always',
markdown: {
processor: unified({
remarkPlugins: [remarkStripEmojis, remarkTableDataLabels],
remarkPlugins: [remarkStripEmojis, remarkTableDataLabels, remarkInlineMarkdownInHtml],
rehypePlugins: [rehypeTableWrapper],
}),
},
Expand Down
112 changes: 112 additions & 0 deletions docs/src/lib/remark/inlineMarkdownInHtml.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// @ts-check

import { unified } from 'unified';
import remarkParse from 'remark-parse';
import remarkRehype from 'remark-rehype';
import rehypeStringify from 'rehype-stringify';
Comment on lines +3 to +6

/**
* Remark plugin that applies markdown transformation to the content of
* specific HTML tags that are treated as opaque HTML blocks by remark-parse.
*
* CommonMark block-level HTML elements such as `<details>` and `<summary>`
* are treated as opaque HTML blocks by remark-parse, so any markdown syntax
* they contain is left as raw text rather than being processed. This plugin
* runs after remark-parse to fill that gap: it visits every `html` MDAST node
* and applies a full markdown transformation to the content of `<summary>`
* (and similar inline-content elements), producing proper HTML.
*
* This is the correct, AST-level fix for GFM alerts that contain
* `<details>/<summary>` with markdown links — replacing the previous
* approach of manipulating rendered HTML strings after compilation.
*/

/**
* Reusable processor for converting markdown inline content to HTML.
*
* `allowDangerousHtml` is required to preserve inline HTML (e.g. `<b>`,
* `<code>`) that authors place inside target tags alongside their markdown.
* This is safe here because:
* - the processor runs **at build time**, never in a browser;
* - inputs are documentation source files committed by repository maintainers
* and are not derived from end-user input;
* - no output is injected into an HTTP response or an untrusted context.
Comment on lines +31 to +33
*/
const inlineProcessor = unified()
.use(remarkParse)
.use(remarkRehype, { allowDangerousHtml: true })
.use(rehypeStringify, { allowDangerousHtml: true });

/**
* Apply markdown transformation to text, producing HTML.
* remark-parse wraps inline content in a `<p>` element; that wrapper is
* stripped so the result can be placed back inside the original HTML tag.
*
* @param {string} text
* @returns {string}
*/
function applyMarkdownTransformation(text) {
const result = String(inlineProcessor.processSync(text));
// Strip the outer <p>…</p> wrapper added by remark for a single paragraph.
return result.replace(/^<p>([\s\S]*?)<\/p>\n?$/, '$1');
}

/**
* @returns {(tree: import('unist').Node) => void}
*/
export default function remarkInlineMarkdownInHtml() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnosing-bugs] applyMarkdownTransformation uses a greedy [\s\S]*? regex to strip the outer <p> wrapper, but processSync can emit multiple paragraphs (e.g. if the summary content has a blank line). In that case the result would begin with <p> and end with </p>\n, so the strip regex matches only the first-to-last </p> — potentially stripping intermediate </p><p> boundaries.

💡 Safer approach

Strip only the leading and trailing <p> tags independently:

function applyMarkdownTransformation(text) {
  const result = String(inlineProcessor.processSync(text));
  return result
    .replace(/^<p>/, '')
    .replace(/<\/p>\n?$/, '');
}

Alternatively, limit inputs to single lines so multi-paragraph summaries are not silently mangled.

@copilot please address this.

return function transform(tree) {
visit(tree);
};
}

/**
* @param {any} node
*/
function visit(node) {
if (!node || typeof node !== 'object') return;

if (node.type === 'html' && typeof node.value === 'string') {
node.value = processMarkdownInHtml(node.value);
}

const { children } = node;
if (Array.isArray(children)) {
for (const child of children) visit(child);
}
}

/**
* Tags whose text content should have markdown transformed to HTML.
* These are inline-text contexts that appear as block-level HTML in markdown
* and therefore bypass normal remark inline processing.
*
* @type {string[]}
*/
const INLINE_TEXT_TAGS = ['summary', 'figcaption', 'caption', 'dt', 'dd', 'th', 'td', 'li'];

/**

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnosing-bugs] The closing-tag regex can match across mismatched open/close tags — e.g. <summary>...</td> would be processed if both tags appear in INLINE_TEXT_TAGS.

💡 Suggested fix

Capture the opening tag name and use a back-reference so only a matching close tag is accepted:

const tagRe = new RegExp(
  `(<(${tagPattern})(?:\\s[^>]*)?>)([\\s\\S]*?)(<\\/\\2>)`,
  'gi',
);
// and update the replacer: (_match, openTag, _tagName, content, closeTag)

Without this, content between an <li> and the next </summary> in a complex HTML block could be incorrectly reprocessed.

@copilot please address this.

* Apply markdown transformation inside the content of specific HTML tags.
*
* - Only targets known inline-text tags listed in INLINE_TEXT_TAGS.
* - Text that already contains an `<a` tag is left untouched to avoid
* double-processing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The double-processing guard (/<a[\s>]/i.test(content)) skips the entire tag content if it contains any existing anchor — including cases where a markdown link appears alongside an already-rendered <a>. In practice this is safe for build-time docs, but if a <summary> ever contains both an authored <a> and a markdown link, the markdown link will silently stay unprocessed. Consider adding a check for remaining [...](...) patterns before short-circuiting, or document this trade-off. @copilot please address this.

*
* @param {string} html
* @returns {string}
*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnosing-bugs] The <a skip guard uses /<a[\s>]/i but the regex pattern string uses 'gi' flags, matching case-insensitively — while the guard itself already uses /i. This is fine, but the guard won't catch <A HREF=...> produced by external HTML (capital-A anchor). Both the existing regex and the guard need to be case-insensitive consistently, which they are — just worth a note that /<A[\s>]/i is equivalent and matches both cases.

Actually the real concern: /<a[\s>]/i will also skip content that contains an <abbr> tag (it matches <ab which starts with <a followed by a space... wait, <abbr is <a + b — the pattern <a[\s>] requires <a followed by whitespace or >, so <abbr> won't match). This is actually fine — just confirm the guard is intentional and add a test for <summary> content containing <abbr> to document the behaviour.

@copilot please address this.

function processMarkdownInHtml(html) {
const tagPattern = INLINE_TEXT_TAGS.join('|');
const tagRe = new RegExp(
`(<(?:${tagPattern})(?:\\s[^>]*)?>)([\\s\\S]*?)(<\\/(?:${tagPattern})>)`,
'gi',
);

return html.replace(tagRe, (_match, openTag, content, closeTag) => {
// Skip content that already contains an anchor tag to avoid double-processing.
if (/<a[\s>]/i.test(content)) return _match;

const processed = applyMarkdownTransformation(content);
return openTag + processed + closeTag;
});
Comment on lines +105 to +111
}
162 changes: 162 additions & 0 deletions docs/src/lib/remark/inlineMarkdownInHtml.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
#!/usr/bin/env node
// @ts-check

/**
* Unit tests for remarkInlineMarkdownInHtml.
*
* Run with: node docs/src/lib/remark/inlineMarkdownInHtml.test.js
*/

import remarkInlineMarkdownInHtml from './inlineMarkdownInHtml.js';

let passed = 0;
let failed = 0;

function assertEqual(actual, expected, label) {
if (actual === expected) {
console.log(` ✓ ${label}`);
passed++;
} else {
console.error(` ✗ ${label}`);
console.error(` expected: ${JSON.stringify(expected)}`);
console.error(` actual: ${JSON.stringify(actual)}`);
failed++;
}
}

function makeHtmlNode(value) {
return { type: 'html', value };
}

function makeTree(...htmlNodes) {
return { type: 'root', children: htmlNodes };
}

function runTransform(tree) {
const transform = remarkInlineMarkdownInHtml();
transform(tree);
return tree;
}

// -------------------------------------------------------------------
// Test: markdown link inside <summary> is converted to <a href="...">
// -------------------------------------------------------------------
console.log('\nmarkdown link inside <summary>:');
{
const tree = makeTree(
makeHtmlNode('<summary>Open a Codespace terminal in [Step 6](06-install-gh-aw.md) to install</summary>'),
);
runTransform(tree);
assertEqual(
tree.children[0].value,
'<summary>Open a Codespace terminal in <a href="06-install-gh-aw.md">Step 6</a> to install</summary>',
'single link converted',
);
}

// -------------------------------------------------------------------
// Test: multiple links inside <summary>
// -------------------------------------------------------------------
console.log('\nmultiple links inside <summary>:');
{
const tree = makeTree(
makeHtmlNode('<summary>See [Step 6](step6.md) and [Step 7](step7.md) for details</summary>'),
);
runTransform(tree);
assertEqual(
tree.children[0].value,
'<summary>See <a href="step6.md">Step 6</a> and <a href="step7.md">Step 7</a> for details</summary>',
'both links converted',
);
}

// -------------------------------------------------------------------
// Test: backtick code spans inside <summary> are converted to <code>
// -------------------------------------------------------------------
console.log('\nbacktick code in <summary>:');
{
const tree = makeTree(
makeHtmlNode('<summary>Install `gh-aw` via [Step 6](step6.md)</summary>'),
);
runTransform(tree);
assertEqual(
tree.children[0].value,
'<summary>Install <code>gh-aw</code> via <a href="step6.md">Step 6</a></summary>',
'backtick converted to <code>, link converted',
);
}

// -------------------------------------------------------------------
// Test: link-like syntax inside backtick code span is NOT converted
// -------------------------------------------------------------------
console.log('\nlink syntax inside code span is not converted:');
{
const tree = makeTree(
makeHtmlNode('<summary>Use `[var](param)` to configure</summary>'),
);
runTransform(tree);
assertEqual(
tree.children[0].value,
'<summary>Use <code>[var](param)</code> to configure</summary>',
'code span link syntax not converted, backtick rendered as <code>',
);
}

// -------------------------------------------------------------------
// Test: <summary> that already contains <a> tags is left untouched
// -------------------------------------------------------------------
console.log('\n<summary> with existing <a> tag not double-processed:');
{
const input = '<summary>See <a href="step6.md">Step 6</a> for details</summary>';
const tree = makeTree(makeHtmlNode(input));
runTransform(tree);
assertEqual(tree.children[0].value, input, 'existing anchor tag not rewritten');
}

// -------------------------------------------------------------------
// Test: html nodes without recognised tags are left unchanged
// -------------------------------------------------------------------
console.log('\nhtml nodes without target tags:');
{
const input = '<blockquote>[link text](url) text</blockquote>';
const tree = makeTree(makeHtmlNode(input));
runTransform(tree);
assertEqual(tree.children[0].value, input, 'non-target tag left unchanged');
}

// -------------------------------------------------------------------
// Test: link inside GFM alert + <summary> (the full real-world case)
// -------------------------------------------------------------------
console.log('\nfull GFM alert with <details>/<summary>:');
{
const tree = makeTree(
makeHtmlNode(
'<details>\n<summary>Using the <b>CCA</b>? Open a terminal in [Step 6](06-install-gh-aw.md) to install `gh-aw`.</summary>\n\n**More info:** See [Step 1](01-prerequisites.md) for the checklist.\n\n</details>',
),
);
runTransform(tree);
const expected =
'<details>\n<summary>Using the <b>CCA</b>? Open a terminal in <a href="06-install-gh-aw.md">Step 6</a> to install <code>gh-aw</code>.</summary>\n\n**More info:** See [Step 1](01-prerequisites.md) for the checklist.\n\n</details>';
assertEqual(tree.children[0].value, expected, 'link in <summary> converted, body outside tag unchanged');
}

// -------------------------------------------------------------------
// Test: nested transform (html nodes inside paragraph children)
// -------------------------------------------------------------------
console.log('\nhtml node nested inside paragraph:');
{
const htmlNode = makeHtmlNode('<summary>See [Step 6](step6.md)</summary>');
const tree = { type: 'root', children: [{ type: 'paragraph', children: [htmlNode] }] };
runTransform(tree);
assertEqual(
htmlNode.value,
'<summary>See <a href="step6.md">Step 6</a></summary>',
'nested html node processed',
);
}

// -------------------------------------------------------------------
// Summary
// -------------------------------------------------------------------
console.log(`\n${passed} passed, ${failed} failed`);
if (failed > 0) process.exit(1);
Loading