fix: fix /children directive on root fragment (take 2) (#19)

Co-authored-by: Codex <noreply@openai.com>

Author: hi-ogawa

src/aria/folk/isomorphic/ariaSnapshot.ts

@@ -584,6 +584,12 @@ export class KeyParser {
     this._skipWhitespace()
 
     const role = this._readIdentifier('role') as AriaTemplateRoleNode['role']
+    // DIVERGENCE(playwright):
+    // we reject explicit `fragment` since that complicates matching algorithm invariant.
+    // this is only meant for internal role generated to model root templates.
+    if (role === 'fragment') {
+      this._throwError('Invalid role "fragment"')
+    }
     this._skipWhitespace()
     // DIVERGENCE(playwright): upstream uses `|| ''`, which makes
     // `- heading [level=1]` produce name="" instead of name=undefined.

src/aria/match.ts

@@ -36,19 +36,16 @@ import { formatTextValue, formatNameValue } from './template'
 //   captureAriaTree always returns a fragment root; parseAriaTemplate may
 //   or may not (it unwraps single-child fragments, following Playwright).
 //
-//   We normalize by flattening: fragment = its children. This happens
-//   inside mergeChildLists so every recursion level handles it uniformly.
+//   We normalize these synthetic root wrappers in matchAriaTree before
+//   entering the recursive merge. User-authored `fragment` template roles are
+//   rejected by the parser, so mergeChildLists never needs general fragment
+//   semantics.
 //
 //   Playwright takes a different approach: instead of flattening, it treats
 //   fragment as a wildcard role (matches any node) and relies on the entry
-//   point to walk root.children directly. Both are equally sound because
-//   fragment nodes never carry meaningful attributes or containerMode in
-//   practice — the parser only sets containerMode on top-level fragments,
-//   then unwraps single-child ones, so surviving fragments always have
-//   multiple children whose list semantics our flatten preserves correctly.
-//   The tradeoff is decomposition: Playwright keeps all match semantics in
-//   matchesNode; we split fragment handling into mergeChildLists, which is
-//   natural since we need that function anyway for three-way merge.
+//   point to walk root.children directly. Our root normalization follows the
+//   same boundary idea while keeping three-way merge decisions in
+//   mergeChildLists.
 //   See: vendor/playwright/packages/injected/src/ariaSnapshot.ts
 //   (matchesNode, matchesNodeDeep)
 //
@@ -115,19 +112,38 @@ export function matchAriaTree(
   root: AriaNode,
   template: AriaTemplateNode
 ): MatchAriaResult {
-  // Enter through mergeChildLists — this is intentional, not just for
-  // fragment normalization. mergeChildLists is the only function that
-  // decides pass/fail (via matchesNode); mergeNode below it is purely
-  // rendering. Wrapping in single-element lists ensures that contract
-  // holds from the top level down.
-  const result = mergeChildLists([root], [template], '')
+  // Normalize synthetic root fragments before entering the recursive merge.
+  // The template root fragment may own /children metadata, so lift its
+  // containerMode before unwrapping it into template roots.
+  const normalizedRoot = root.role === 'fragment' ? root.children : [root]
+  const normalizedTemplate = isAriaTemplateFragement(template)
+    ? (template.children ?? [])
+    : [template]
+  const containerMode = isAriaTemplateFragement(template)
+    ? template.containerMode
+    : undefined
+  const result = mergeChildLists(
+    normalizedRoot,
+    normalizedTemplate,
+    '',
+    containerMode
+  )
+  if (result.pass && containerMode && containerMode !== 'contain') {
+    result.resolved.unshift(`- /children: ${containerMode}`)
+  }
 
   return {
     pass: result.pass,
     resolved: result.resolved.join('\n'),
   }
 }
 
+function isAriaTemplateFragement(
+  template: AriaTemplateNode
+): template is AriaTemplateRoleNode {
+  return template.kind === 'role' && template.role === 'fragment'
+}
+
 // ---------------------------------------------------------------------------
 // Merge internals
 // ---------------------------------------------------------------------------
@@ -244,13 +260,14 @@ function mergeChildLists(
   indent: string,
   containerMode?: ContainerMode
 ): MergeResult {
-  // fragment = its children (a fragment has no semantics of its own)
-  children = children.flatMap((c) =>
-    typeof c !== 'string' && c.role === 'fragment' ? c.children : [c]
-  )
-  templates = templates.flatMap((t) =>
-    t.kind === 'role' && t.role === 'fragment' ? t.children || [] : [t]
-  )
+  if (
+    children.some((c) => typeof c !== 'string' && c.role === 'fragment') ||
+    templates.some((t) => isAriaTemplateFragement(t))
+  ) {
+    throw new Error(
+      'Internal error: fragment wrappers must be unwrapped at matchAriaTree root'
+    )
+  }
 
   if (containerMode === 'equal' || containerMode === 'deep-equal') {
     return mergeChildListsEqual(

src/aria/template.ts

@@ -30,6 +30,9 @@ export function renderAriaTemplate(template: AriaTemplateNode): string {
   if (template.kind === 'text') {
     lines.push(`- text: ${formatTextValue(template.text)}`)
   } else if (template.role === 'fragment') {
+    if (template.containerMode && template.containerMode !== 'contain') {
+      lines.push(`- /children: ${template.containerMode}`)
+    }
     for (const child of template.children || [])
       renderTemplateLines(child, '', lines)
   } else {

test/aria.test.ts

@@ -2537,6 +2537,40 @@ describe('parseAriaTemplate', () => {
     )
   })
 
+  test('unknown alphabetic role is accepted', () => {
+    expect(parseAriaTemplate('- madeuprole')).toMatchInlineSnapshot(`
+      {
+        "kind": "role",
+        "name": undefined,
+        "role": "madeuprole",
+      }
+    `)
+  })
+
+  test('explicit fragment role is rejected', () => {
+    expect(() => parseAriaTemplate('- fragment'))
+      .toThrowErrorMatchingInlineSnapshot(`
+        [Error: Invalid role "fragment":
+
+        fragment
+                ^
+        ]
+      `)
+    expect(() =>
+      parseAriaTemplate(`
+      - main:
+        - fragment:
+          - button
+    `)
+    ).toThrowErrorMatchingInlineSnapshot(`
+      [Error: Invalid role "fragment":
+
+      fragment
+              ^
+      ]
+    `)
+  })
+
   // Block scalars (|) are not supported by the minimal YAML parser.
   test('YAML block scalar (| multiline) is not supported', () => {
     expect(() =>
@@ -4444,6 +4478,137 @@ describe('/children directive', () => {
     `)
   })
 
+  test('/children: equal at root', () => {
+    const html = `
+      <ul>
+        <li>a</li>
+        <li>b</li>
+      </ul>
+    `
+    const template = `
+      - /children: equal
+      - list:
+        - listitem: a
+    `
+    expect(match(html, template)).toMatchInlineSnapshot(`
+      {
+        "actual": "
+      - list:
+        - listitem: a
+        - listitem: b
+      ",
+        "actualResolved": "
+      - /children: equal
+      - list:
+        - listitem: a
+      ",
+        "expected": "
+      - /children: equal
+      - list:
+        - listitem: a
+      ",
+        "pass": true,
+      }
+    `)
+  })
+
+  test('/children: deep-equal at root - fail', () => {
+    const html = `
+      <ul>
+        <li>A</li>
+        <li>B</li>
+      </ul>
+    `
+    const template = `
+      - /children: deep-equal
+      - list:
+        - listitem: a
+    `
+    expect(match(html, template)).toMatchInlineSnapshot(`
+      {
+        "actual": "
+      - list:
+        - listitem: A
+        - listitem: B
+      ",
+        "actualResolved": "
+      - list:
+        - listitem: A
+        - listitem: B
+      ",
+        "expected": "
+      - /children: deep-equal
+      - list:
+        - listitem: a
+      ",
+        "pass": false,
+      }
+    `)
+  })
+
+  test('/children: deep-equal at root - pass', () => {
+    const html = `
+      <div></div>
+    `
+    const template = `
+      - /children: deep-equal
+    `
+    expect(match(html, template)).toMatchInlineSnapshot(`
+      {
+        "actual": "
+
+      ",
+        "actualResolved": "
+      - /children: deep-equal
+      ",
+        "expected": "
+      - /children: deep-equal
+      ",
+        "pass": true,
+      }
+    `)
+  })
+
+  test('/children: equal at non root', () => {
+    const html = `
+      <main>
+        <ul>
+          <li>a</li>
+          <li>b</li>
+        </ul>
+      </main>
+    `
+    const template = `
+      - main:
+        - /children: equal
+        - list:
+          - listitem: a
+    `
+    expect(match(html, template)).toMatchInlineSnapshot(`
+      {
+        "actual": "
+      - main:
+        - list:
+          - listitem: a
+          - listitem: b
+      ",
+        "actualResolved": "
+      - main:
+        - /children: equal
+        - list:
+          - listitem: a
+      ",
+        "expected": "
+      - main:
+        - /children: equal
+        - list:
+          - listitem: a
+      ",
+        "pass": true,
+      }
+    `)
+  })
+
   test('/children: equal — resolved preserves directive on matched branch, purges on failed', () => {
     // Two sibling lists both with /children: equal.
     // First list matches exactly → directive preserved in resolved.
@@ -4546,12 +4711,15 @@ describe('/children directive', () => {
 
   test('renderAriaTemplate preserves /children directive', () => {
     const t = parseAriaTemplate(`
+      - /children: equal
       - list:
         - /children: equal
         - listitem: A
     `)
-    expect(renderAriaTemplate(t)).toMatchInlineSnapshot(`
-      "- list:
+    expect('\n' + renderAriaTemplate(t)).toMatchInlineSnapshot(`
+      "
+      - /children: equal
+      - list:
         - /children: equal
         - listitem: A"
     `)