fix: CI builds helpers from source; survive AX cycles; portable ax tests

.changeset/ax-cycles-and-windows.md

@@ -0,0 +1,22 @@
+---
+"macos-vision": patch
+---
+
+fix: survive cycles in the accessibility tree, and enumerate windows from both sources
+
+The AX tree is a graph, not a tree. Safari was observed listing the application
+element as its own child and answering `kAXWindows` with it. Without cycle
+protection the walk descends into the application repeatedly and returns menu
+bars at the depth limit instead of the window's contents — which is what
+produced an occasional two-node result. Visited elements are now tracked by
+`CFHash`/`CFEqual` and never revisited.
+
+Windows are taken from the union of `kAXWindows` and the application element's
+children, filtered to real windows and never the application element itself,
+because apps differ in which of the two they populate.
+
+The `axTree` tests no longer assume a particular app has a window. They probe
+until they find an app the accessibility API actually answers for and skip when
+none does: `CGWindowList` still lists windows on a locked Mac while AX exposes
+none, and a CI runner has neither — so the previous fixed target passed locally
+and failed there.

.changeset/ci-build-from-source.md

@@ -0,0 +1,15 @@
+---
+"macos-vision": patch
+---
+
+fix(ci): build the Swift helpers from source instead of testing last release's binaries
+
+`postinstall` downloads prebuilt helpers for the package's current version. Once
+that version is published, CI on a branch that changes Swift finds those assets,
+downloads them, and runs the whole suite against the **previous** release —
+so native changes were never actually tested. It surfaced when two tests for
+freshly added helper behaviour failed on CI while passing locally: the runner was
+executing a binary that predated them.
+
+CI now sets `MACOS_VISION_SKIP_DOWNLOAD=1` and asserts every helper exists and is
+newer than its source, so a stale download cannot slip through unnoticed.

.github/workflows/ci.yml

@@ -16,10 +16,28 @@ jobs:
         with:
           node-version: 20
 
+      # Compile the Swift helpers from this branch's source instead of
+      # downloading the prebuilt binaries for the package's current version.
+      # Otherwise every native change is tested against the *previous* release:
+      # once a version is published, postinstall finds its assets and never
+      # builds what the branch actually changed.
       - name: Install dependencies
         run: npm ci
         env:
           HUSKY: 0
+          MACOS_VISION_SKIP_DOWNLOAD: '1'
+
+      - name: Verify helpers were built from source
+        run: |
+          for h in vision-helper pdf-helper ui-helper ax-helper; do
+            test -x "bin/$h" || { echo "::error::bin/$h missing"; exit 1; }
+          done
+          # A helper older than its source means a stale download slipped through.
+          for h in vision-helper pdf-helper ui-helper ax-helper; do
+            if [ "src/native/$h.swift" -nt "bin/$h" ]; then
+              echo "::error::bin/$h is older than src/native/$h.swift"; exit 1
+            fi
+          done
 
       - name: Lint
         run: npm run lint

src/native/ax-helper.swift

@@ -176,6 +176,20 @@ func text(_ v: Any?) -> String? {
     return t.isEmpty ? nil : t
 }
 
+func roleOf(_ el: AXUIElement) -> String? {
+    var v: CFTypeRef?
+    guard AXUIElementCopyAttributeValue(el, kAXRoleAttribute as CFString, &v) == .success else { return nil }
+    return v as? String
+}
+
+/// AXUIElement is a CFType with no Swift Hashable conformance; CFHash plus a
+/// CFEqual check on collision is the documented way to key one.
+struct ElementKey: Hashable {
+    let element: AXUIElement
+    static func == (a: ElementKey, b: ElementKey) -> Bool { CFEqual(a.element, b.element) }
+    func hash(into hasher: inout Hasher) { hasher.combine(CFHash(element)) }
+}
+
 func children(_ el: AXUIElement) -> [AXUIElement] {
     var v: CFTypeRef?
     guard AXUIElementCopyAttributeValue(el, kAXChildrenAttribute as CFString, &v) == .success,
@@ -303,16 +317,35 @@ let started = Date()
 var windowBox: Box?
 var cullRect: CGRect?
 
-let windows = children(axApp).filter { el in
-    let a = readAttributes(el)
-    return (a[0] as? String) == "AXWindow"
-}
+/// `kAXWindows` is the documented way to enumerate an application's windows.
+/// Filtering the application element's children by role is not: those children
+/// are `AXApplication` and `AXMenuBar`, so that approach found windows only by
+/// accident and reported none for apps where it did not.
+/// Windows come from `kAXWindows` where the app provides it and from the
+/// application element's children otherwise — apps differ, and Safari has been
+/// observed answering `kAXWindows` with the application element itself. Take the
+/// union, keep only real windows, and never the app element (which would make
+/// the walk descend into the whole application).
+let windows: [AXUIElement] = {
+    var found: [AXUIElement] = []
+    var seen = Set<ElementKey>()
+    var v: CFTypeRef?
+    if AXUIElementCopyAttributeValue(axApp, kAXWindowsAttribute as CFString, &v) == .success,
+       let ws = v as? [AXUIElement] {
+        found += ws
+    }
+    found += children(axApp)
+    return found.filter { el in
+        guard !CFEqual(el, axApp), roleOf(el) == "AXWindow" else { return false }
+        return seen.insert(ElementKey(element: el)).inserted
+    }
+}()
 let windowIndex = intOpt("--window", 0)
 // Asking for a window that is not there must say so. Falling through to the
 // application element walks a different, larger tree and reports no window
 // frame — a silently different answer to the question that was asked.
 if windows.isEmpty {
-    fail("\(app.localizedName ?? "app") has no accessibility windows (is it minimised or hidden?)")
+    fail("\(app.localizedName ?? "app") has no accessibility windows — it may be minimised or hidden, or the screen may be locked (a locked Mac exposes none)")
 }
 if windows[safe: windowIndex] == nil {
     fail("window \(windowIndex) not found: \(app.localizedName ?? "app") exposes \(windows.count)")
@@ -334,9 +367,15 @@ if let cp = colorsPath {
     if pixels == nil { fail("cannot read image for --colors: \(cp)") }
 }
 
+var visited = Set<ElementKey>()
+
 func walk(_ el: AXUIElement, parent: Int?, depth: Int) {
     if depth > maxDepth { capped = true; return }
     if nodes.count >= maxElements { capped = true; return }
+    // The tree is a graph in practice: Safari lists the application element as
+    // its own child, and without this the walk descends into it repeatedly and
+    // returns menu bars at the depth limit instead of the window.
+    guard visited.insert(ElementKey(element: el)).inserted else { return }
 
     let a = readAttributes(el)
     guard let role = a[0] as? String else { return }

test/ax.test.ts

@@ -1,59 +1,88 @@
-import { describe, it, expect } from 'vitest';
-import { axTree } from '../src/index.js';
+import { describe, it, expect, beforeAll } from 'vitest';
+import { axTree, listWindows } from '../src/index.js';
 
-// Finder is always running on a Mac and has a deep, geometry-rich tree, which
-// makes it the least flaky target available without shipping a fixture app.
-const APP = 'Finder';
+// Which app to walk is decided at run time. Hard-coding one made this suite pass
+// locally and fail on CI, where no Finder window exists — and worse, it passed
+// there only because the helper used to fall back to walking the whole
+// application, which is precisely the behaviour these tests now forbid.
+let app: string | undefined;
 const T = 60_000;
 
+// The precondition is not "some app has a window" but "AX will actually answer".
+// CGWindowList still lists windows on a locked Mac while the accessibility API
+// exposes none, and a CI runner has neither — so probe instead of assuming, and
+// pick the first app that really works.
+beforeAll(async () => {
+  const windows = await listWindows().catch(() => []);
+  for (const candidate of [...new Set(windows.map((w) => w.app))].slice(0, 5)) {
+    try {
+      const probe = await axTree({ app: candidate, maxElements: 5 });
+      if (probe.nodes.length > 0) {
+        app = candidate;
+        return;
+      }
+    } catch {
+      // minimised, hidden, locked screen, or an app that exposes nothing — try the next
+    }
+  }
+}, T);
+
+/** Runs the test against an app AX actually answers for; skips when none does. */
+const withApp =
+  (fn: (app: string) => Promise<void>) =>
+  async (): Promise<void> => {
+    if (!app) return; // no usable accessibility target here, which is not a failure
+    await fn(app);
+  };
+
 describe('axTree()', () => {
   it(
     'returns a tree with geometry for a running app',
-    async () => {
-      const tree = await axTree({ app: APP, maxElements: 120 });
-      expect(tree.app).toBe(APP);
+    withApp(async (target) => {
+      const tree = await axTree({ app: target, maxElements: 120 });
+      expect(tree.app).toBe(target);
       expect(tree.pid).toBeGreaterThan(0);
       expect(tree.nodes.length).toBeGreaterThan(0);
       expect(tree.source).toBe('ax');
-    },
+    }),
     T
   );
 
   it(
     'gives every node a four-number box and a role',
-    async () => {
-      const { nodes } = await axTree({ app: APP, maxElements: 120 });
+    withApp(async (target) => {
+      const { nodes } = await axTree({ app: target, maxElements: 120 });
       for (const n of nodes) {
         expect(n.box).toHaveLength(4);
         for (const v of n.box) expect(Number.isFinite(v)).toBe(true);
-        expect(n.box[2]).toBeGreaterThan(0); // width
-        expect(n.box[3]).toBeGreaterThan(0); // height
+        expect(n.box[2]).toBeGreaterThan(0);
+        expect(n.box[3]).toBeGreaterThan(0);
         expect(typeof n.role).toBe('string');
-        expect(n.role.startsWith('AX')).toBe(false); // prefix stripped
+        expect(n.role.startsWith('AX')).toBe(false);
       }
-    },
+    }),
     T
   );
 
   it(
     'reports the budget honestly instead of truncating silently',
-    async () => {
-      const tree = await axTree({ app: APP, maxElements: 10 });
+    withApp(async (target) => {
+      const tree = await axTree({ app: target, maxElements: 10 });
       expect(tree.budget.elements).toBe(tree.nodes.length);
-      // maxElements caps the walk, and pruning runs after it — so `walked` is
-      // what hit the cap while `elements` can legitimately be smaller.
+      // maxElements caps the walk and pruning runs after it, so `walked` is what
+      // hit the cap while `elements` can legitimately come back smaller.
       expect(tree.budget.walked).toBeLessThanOrEqual(10);
       expect(tree.budget.elements).toBeLessThanOrEqual(tree.budget.walked);
       expect(tree.budget.capped).toBe(true);
       expect(tree.budget.elapsedMs).toBeGreaterThanOrEqual(0);
-    },
+    }),
     T
   );
 
   it(
     'keeps parent ids resolvable within the returned set',
-    async () => {
-      const { nodes } = await axTree({ app: APP, maxElements: 200 });
+    withApp(async (target) => {
+      const { nodes } = await axTree({ app: target, maxElements: 200 });
       const ids = new Set(nodes.map((n) => n.id));
       // The root carries no `parent` key at all — Swift omits nil rather than
       // encoding null, and that saves a key on every root.
@@ -62,33 +91,31 @@ describe('axTree()', () => {
       for (const n of nodes) {
         if (n.parent !== undefined) expect(ids.has(n.parent)).toBe(true);
       }
-    },
+    }),
     T
   );
 
   it(
     'detail:content is a subset of detail:full',
-    async () => {
-      const full = await axTree({ app: APP, maxElements: 300, detail: 'full' });
-      const content = await axTree({ app: APP, maxElements: 300, detail: 'content' });
+    withApp(async (target) => {
+      const full = await axTree({ app: target, maxElements: 300, detail: 'full' });
+      const content = await axTree({ app: target, maxElements: 300, detail: 'content' });
       expect(content.nodes.length).toBeLessThanOrEqual(full.nodes.length);
-      // Pruning drops unlabelled structure, so what survives should be
-      // overwhelmingly nodes that carry meaning.
       const meaningful = content.nodes.filter((n) => n.label || n.value || n.role === 'Window');
       expect(meaningful.length).toBeGreaterThan(content.nodes.length / 2);
-    },
+    }),
     T
   );
 
   it(
     'omits enabled when true and focused when false, to keep the payload small',
-    async () => {
-      const { nodes } = await axTree({ app: APP, maxElements: 200 });
+    withApp(async (target) => {
+      const { nodes } = await axTree({ app: target, maxElements: 200 });
       for (const n of nodes) {
-        expect(n.enabled).not.toBe(true); // present only when false
-        expect(n.focused).not.toBe(false); // present only when true
+        expect(n.enabled).not.toBe(true);
+        expect(n.focused).not.toBe(false);
       }
-    },
+    }),
     T
   );
 
@@ -105,16 +132,14 @@ describe('axTree()', () => {
     },
     T
   );
-});
 
-describe('axTree() window targeting', () => {
   it(
     'fails loudly when the window index is out of range',
-    async () => {
+    withApp(async (target) => {
       // Falling back to the application element would walk a larger tree and
       // report no window frame — a different answer to the question asked.
-      await expect(axTree({ app: APP, window: 99 })).rejects.toThrow(/window 99 not found/);
-    },
+      await expect(axTree({ app: target, window: 99 })).rejects.toThrow(/window 99 not found/);
+    }),
     T
   );
 });