From d0dafba53450291d6284ac355b592ff7dee76f66 Mon Sep 17 00:00:00 2001 From: teamleaderleo Date: Sun, 27 Sep 2026 14:34:09 -0400 Subject: [PATCH 1/4] tools: ui-lab renders view code to PNGs in seconds; wire-app-sources scripts/ui-lab/ui-lab.py compiles a harness plus the app sources it names with plain swiftc (a few seconds, cached by input hash), runs it and writes light and dark 2x PNGs and a 4x detail crop. Shims stand in for app types the sources use. --watch re-renders on save. Example harness: the sidebar loading spinner. scripts/wire-app-sources.py adds the four project entries for unwired Sources/**/*.swift files next to a wired sibling and normalizes the project; sync-test-wiring only covers cmuxTests, and a merge that takes main's project.pbxproj silently drops a branch's app files. The app-sources lint now points at it. Co-Authored-By: Claude Opus 5.5 --- scripts/lint-pbxproj-test-wiring.sh | 11 +- scripts/ui-lab/UILab.swift | 86 +++++++++ scripts/ui-lab/harnesses/gpu-spinner.swift | 30 +++ .../ui-lab/shims/RenderableSystemSymbol.swift | 31 +++ .../SidebarAppearanceColorResolver.swift | 16 ++ scripts/ui-lab/ui-lab.py | 154 +++++++++++++++ scripts/verify-local.py | 2 + scripts/wire-app-sources.py | 176 ++++++++++++++++++ skills/cmux-testing/SKILL.md | 3 +- skills/cmux-testing/references/ui-lab.md | 53 ++++++ tests/test-execution.toml | 8 + tests/test_ui_lab.py | 46 +++++ tests/test_wire_app_sources.py | 108 +++++++++++ 13 files changed, 720 insertions(+), 4 deletions(-) create mode 100644 scripts/ui-lab/UILab.swift create mode 100644 scripts/ui-lab/harnesses/gpu-spinner.swift create mode 100644 scripts/ui-lab/shims/RenderableSystemSymbol.swift create mode 100644 scripts/ui-lab/shims/SidebarAppearanceColorResolver.swift create mode 100755 scripts/ui-lab/ui-lab.py create mode 100755 scripts/wire-app-sources.py create mode 100644 skills/cmux-testing/references/ui-lab.md create mode 100644 tests/test_ui_lab.py create mode 100644 tests/test_wire_app_sources.py diff --git a/scripts/lint-pbxproj-test-wiring.sh b/scripts/lint-pbxproj-test-wiring.sh index 0083c54d99ab..c30c8e6f8f0b 100755 --- a/scripts/lint-pbxproj-test-wiring.sh +++ b/scripts/lint-pbxproj-test-wiring.sh @@ -278,9 +278,14 @@ if [ "$TARGET_NAME" = "cmuxTests" ] && [ "$TESTS_REL" = "cmuxTests" ]; then echo "Run ./scripts/sync-test-wiring to reconcile direct $TESTS_REL/*.swift files." echo "Use ./scripts/sync-test-wiring --check for a read-only authoring/CI check." else - # sync-test-wiring only reconciles cmuxTests. Other targets are wired by hand. - echo "sync-test-wiring only reconciles cmuxTests; add the four $TARGET_NAME" - echo "entries above by hand (or in Xcode) for $TESTS_REL/*.swift." + # sync-test-wiring only reconciles cmuxTests. + if [ "$TARGET_NAME" = "cmux" ] && [ "$TESTS_REL" = "Sources" ]; then + echo "Run ./scripts/wire-app-sources.py to add the four entries for each" + echo "unwired Sources/**/*.swift file (next to a wired sibling)." + else + echo "sync-test-wiring only reconciles cmuxTests; add the four $TARGET_NAME" + echo "entries above by hand (or in Xcode) for $TESTS_REL/*.swift." + fi fi echo "This lint remains the defensive $TARGET_NAME Sources-phase guard." exit 1 diff --git a/scripts/ui-lab/UILab.swift b/scripts/ui-lab/UILab.swift new file mode 100644 index 000000000000..9741e77d7add --- /dev/null +++ b/scripts/ui-lab/UILab.swift @@ -0,0 +1,86 @@ +import AppKit + +/// Rendering support for ui-lab harnesses (see scripts/ui-lab/ui-lab.py). +/// A harness calls `UILab.main { ... }`, builds an NSView inside it and calls +/// `UILab.render(_:name:)`; the output +/// directory is the process's first argument. +enum UILab { + static let outputDirectory: URL = { + let path = CommandLine.arguments.dropFirst().first ?? FileManager.default.currentDirectoryPath + return URL(fileURLWithPath: path, isDirectory: true) + }() + + /// A harness's entry point: sets up AppKit (an application, so + /// appearances and system colors resolve) and runs `body` on the main actor. + static func main(_ body: @MainActor () -> Void) { + MainActor.assumeIsolated { + _ = NSApplication.shared + NSApp.setActivationPolicy(.prohibited) + body() + } + } + + /// Writes `-light@2x.png` and `-dark@2x.png`, and, when + /// `detail` is set, `-light@4x.png` cropped to that rect (in view + /// points) for a close look at small glyphs. + @MainActor + static func render(_ view: NSView, name: String, detail: NSRect? = nil) { + for (label, appearanceName) in [("light", NSAppearance.Name.aqua), ("dark", NSAppearance.Name.darkAqua)] { + let appearance = NSAppearance(named: appearanceName)! + write(view, appearance: appearance, scale: 2, rect: view.bounds, file: "\(name)-\(label)@2x.png") + if let detail, label == "light" { + write(view, appearance: appearance, scale: 4, rect: detail, file: "\(name)-\(label)-detail@4x.png") + } + } + } + + @MainActor + private static func write(_ view: NSView, appearance: NSAppearance, scale: CGFloat, rect: NSRect, file: String) { + view.appearance = appearance + var data: Data? + appearance.performAsCurrentDrawingAppearance { + layoutAll(view) + guard let rep = NSBitmapImageRep( + bitmapDataPlanes: nil, + pixelsWide: Int(rect.width * scale), + pixelsHigh: Int(rect.height * scale), + bitsPerSample: 8, + samplesPerPixel: 4, + hasAlpha: true, + isPlanar: false, + colorSpaceName: .deviceRGB, + bytesPerRow: 0, + bitsPerPixel: 0 + ) else { return } + rep.size = rect.size + view.cacheDisplay(in: rect, to: rep) + data = rep.representation(using: .png, properties: [:]) + } + let url = outputDirectory.appendingPathComponent(file) + do { + try data?.write(to: url) + print(url.path) + } catch { + FileHandle.standardError.write("ui-lab: could not write \(url.path): \(error)\n".data(using: .utf8)!) + } + } + + @MainActor + private static func layoutAll(_ view: NSView) { + view.needsLayout = true + view.layoutSubtreeIfNeeded() + view.subviews.forEach(layoutAll) + } + + /// A flipped container: frames are laid out top-down like the sidebar cells. + final class Canvas: NSView { + var fill: NSColor? + override var isFlipped: Bool { true } + override func draw(_ dirtyRect: NSRect) { + if let fill { + fill.setFill() + dirtyRect.fill() + } + } + } +} diff --git a/scripts/ui-lab/harnesses/gpu-spinner.swift b/scripts/ui-lab/harnesses/gpu-spinner.swift new file mode 100644 index 000000000000..65d9a584b699 --- /dev/null +++ b/scripts/ui-lab/harnesses/gpu-spinner.swift @@ -0,0 +1,30 @@ +// ui-lab: source Sources/Sidebar/GPUSpinnerStyle.swift +// ui-lab: source Sources/Sidebar/GPUSpinnerNSView.swift +// ui-lab: shim SidebarAppearanceColorResolver +// +// The sidebar loading spinner in each style and color scheme, at the sizes +// rows use (12 pt, 16 pt). A still frame: the rotation is a Core Animation +// loop that ui-lab does not advance. + +import AppKit + +UILab.main { + let styles: [GPUSpinnerStyle] = [.macOSSpokes, .arc] + let sizes: [CGFloat] = [12, 16] + let cell: CGFloat = 28 + let canvas = UILab.Canvas(frame: NSRect(x: 0, y: 0, width: cell * CGFloat(sizes.count) + 16, height: cell * CGFloat(styles.count) + 16)) + canvas.fill = .windowBackgroundColor + for (row, style) in styles.enumerated() { + for (column, size) in sizes.enumerated() { + let spinner = GPUSpinnerNSView(frame: NSRect( + x: 8 + CGFloat(column) * cell + (cell - size) / 2, + y: 8 + CGFloat(row) * cell + (cell - size) / 2, + width: size, + height: size + )) + spinner.style = style + canvas.addSubview(spinner) + } + } + UILab.render(canvas, name: "gpu-spinner", detail: canvas.bounds) +} diff --git a/scripts/ui-lab/shims/RenderableSystemSymbol.swift b/scripts/ui-lab/shims/RenderableSystemSymbol.swift new file mode 100644 index 000000000000..9920ed4262cc --- /dev/null +++ b/scripts/ui-lab/shims/RenderableSystemSymbol.swift @@ -0,0 +1,31 @@ +import AppKit +import SwiftUI + +/// ui-lab stand-in for Sources/RenderableSystemSymbol.swift's AppKit path: +/// the same symbol configuration (point size, weight, monochrome, template), +/// without the app's caches and font-magnification plumbing. +enum RenderableSystemSymbol { + @MainActor + static func configuredAppKitImage(systemName: String, pointSize: CGFloat, weight: Font.Weight? = nil) -> NSImage? { + guard let base = NSImage(systemSymbolName: systemName, accessibilityDescription: nil) else { return nil } + let configuration = NSImage.SymbolConfiguration(pointSize: pointSize, weight: nsWeight(weight)) + .applying(.preferringMonochrome()) + let image = base.withSymbolConfiguration(configuration) ?? base + image.isTemplate = true + return image + } + + private static func nsWeight(_ weight: Font.Weight?) -> NSFont.Weight { + switch weight { + case .ultraLight?: return .ultraLight + case .thin?: return .thin + case .light?: return .light + case .medium?: return .medium + case .semibold?: return .semibold + case .bold?: return .bold + case .heavy?: return .heavy + case .black?: return .black + default: return .regular + } + } +} diff --git a/scripts/ui-lab/shims/SidebarAppearanceColorResolver.swift b/scripts/ui-lab/shims/SidebarAppearanceColorResolver.swift new file mode 100644 index 000000000000..2871c20b1cb5 --- /dev/null +++ b/scripts/ui-lab/shims/SidebarAppearanceColorResolver.swift @@ -0,0 +1,16 @@ +import AppKit +import SwiftUI + +/// ui-lab stand-in for Sources/Sidebar/SidebarAppearanceSupport.swift's +/// resolver: resolves a dynamic color under the given scheme's appearance. +struct SidebarAppearanceColorResolver { + func resolvedColor(_ color: NSColor, for colorScheme: ColorScheme, opacity: CGFloat? = nil) -> NSColor { + let appearance = NSAppearance(named: colorScheme == .dark ? .darkAqua : .aqua)! + var resolved = color + appearance.performAsCurrentDrawingAppearance { + resolved = color.usingColorSpace(.deviceRGB) ?? color + } + guard let opacity else { return resolved } + return resolved.withAlphaComponent(max(0, min(opacity, 1))) + } +} diff --git a/scripts/ui-lab/ui-lab.py b/scripts/ui-lab/ui-lab.py new file mode 100755 index 000000000000..fb1d44bd719f --- /dev/null +++ b/scripts/ui-lab/ui-lab.py @@ -0,0 +1,154 @@ +#!/usr/bin/env python3 +"""Render cmux view code to PNGs in seconds, without building the app. + +A harness is a Swift file whose top-level code builds views and hands them to +`UILab.render`. Its header names the app sources to compile with it: + + // ui-lab: source Sources/Sidebar/SidebarCompactStatusGlyph.swift + // ui-lab: shim RenderableSystemSymbol + +`source` paths are repo-relative; `shim` names a file in scripts/ui-lab/shims/ +standing in for an app type the sources use. Sources are compiled as one +module with plain `swiftc` (their `import Cmux*` lines are dropped), so a +harness can only pull in files without package or app dependencies beyond +its shims. Keep view code that way when you want it here. + + scripts/ui-lab/ui-lab.py scripts/ui-lab/harnesses/sidebar-compact-status.swift + scripts/ui-lab/ui-lab.py --watch # re-render on every save + scripts/ui-lab/ui-lab.py --out DIR + +Each render writes light and dark PNGs at 2x, plus a 4x crop for detail, and +prints their paths. The binary is cached by input hash, so an unchanged +re-run only renders. This is a design loop, not proof: the CI UI tests +(`scripts/ui-test`) still check the real app. +""" + +from __future__ import annotations + +import argparse +import hashlib +import os +import re +import shutil +import subprocess +import sys +import tempfile +import time +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +LAB = Path(__file__).resolve().parent +DIRECTIVE = re.compile(r"^//\s*ui-lab:\s*(source|shim)\s+(\S+)\s*$") +CACHE = Path(os.environ.get("CMUX_UI_LAB_CACHE", Path.home() / "Library/Caches/cmux-ui-lab")) + + +def inputs(harness: Path) -> list[Path]: + """The Swift files one harness compiles: support, shims, sources, harness.""" + files = [LAB / "UILab.swift"] + for line in harness.read_text().splitlines(): + match = DIRECTIVE.match(line.strip()) + if not match: + continue + kind, value = match.groups() + path = LAB / "shims" / f"{value}.swift" if kind == "shim" else ROOT / value + if not path.exists(): + raise SystemExit(f"ui-lab: {kind} {value} not found at {path}") + files.append(path) + return files + [harness] + + +def build(harness: Path) -> Path: + files = inputs(harness) + digest = hashlib.sha256() + for path in files: + digest.update(str(path).encode()) + digest.update(path.read_bytes()) + digest.update(subprocess.run(["swiftc", "--version"], capture_output=True, text=True).stdout.encode()) + binary = CACHE / digest.hexdigest()[:16] / "lab" + if binary.exists(): + return binary + + work = Path(tempfile.mkdtemp(prefix="cmux-ui-lab-")) + try: + compiled = [] + for index, path in enumerate(files): + text = path.read_text() + if path != harness: + # One module: package imports resolve to shims or nothing. + text = re.sub(r"^(?:@_implementationOnly |public |internal )?import Cmux\w*\s*$", "", text, flags=re.M) + name = "main.swift" if path == harness else f"{index:02d}-{path.name}" + (work / name).write_text(text) + compiled.append(str(work / name)) + binary.parent.mkdir(parents=True, exist_ok=True) + started = time.monotonic() + result = subprocess.run( + ["swiftc", "-Onone", "-swift-version", "5", "-o", str(binary), *compiled], + capture_output=True, + text=True, + ) + if result.returncode != 0: + # Point errors at the real files, not the temp copies. + output = result.stderr + for index, path in enumerate(files): + name = "main.swift" if path == harness else f"{index:02d}-{path.name}" + output = output.replace(str(work / name), str(path)) + sys.stderr.write(output) + raise SystemExit("ui-lab: compile failed") + print(f"ui-lab: compiled {len(files)} files in {time.monotonic() - started:.1f}s", file=sys.stderr) + return binary + finally: + shutil.rmtree(work, ignore_errors=True) + + +def render(harness: Path, out: Path) -> None: + binary = build(harness) + out.mkdir(parents=True, exist_ok=True) + result = subprocess.run([str(binary), str(out)], capture_output=True, text=True) + sys.stdout.write(result.stdout) + sys.stderr.write(result.stderr) + if result.returncode != 0: + raise SystemExit(f"ui-lab: harness exited {result.returncode}") + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument("harness", type=Path) + parser.add_argument("--out", type=Path, help="default: $TMPDIR/cmux-ui-lab/") + parser.add_argument("--watch", action="store_true", help="re-render whenever an input changes") + args = parser.parse_args(argv) + + harness = args.harness.resolve() + out = args.out or Path(tempfile.gettempdir()) / "cmux-ui-lab" / harness.stem + try: + render(harness, out) + except SystemExit as error: + if not args.watch: + raise + print(error, file=sys.stderr) + if not args.watch: + return 0 + + def stamp() -> tuple[float, ...]: + try: + return tuple(path.stat().st_mtime for path in inputs(harness)) + except (OSError, SystemExit): + return () + + last = stamp() + print("ui-lab: watching; Ctrl-C to stop", file=sys.stderr) + while True: + time.sleep(0.4) + current = stamp() + if current and current != last: + last = current + try: + render(harness, out) + except SystemExit as error: + print(error, file=sys.stderr) + + +if __name__ == "__main__": + try: + sys.exit(main()) + except KeyboardInterrupt: + sys.exit(130) diff --git a/scripts/verify-local.py b/scripts/verify-local.py index ecde39f2e2dd..2a9237398f12 100644 --- a/scripts/verify-local.py +++ b/scripts/verify-local.py @@ -29,6 +29,8 @@ ("project", "static_analysis", "Xcode project normalization and version", ["bash", "scripts/check-pbxproj.sh"]), ("config-schema", "static_analysis", "Embedded cmux.json schema", ["python3", "scripts/generate-cmux-config-schema.py", "--check"]), ("test-wiring-sync", "tests", "Test-wiring sync tool", ["python3", "tests/test_sync_test_wiring.py"]), + ("wire-app-sources", "tests", "App-source wiring tool", ["python3", "tests/test_wire_app_sources.py"]), + ("ui-lab", "tests", "ui-lab harness directives", ["python3", "tests/test_ui_lab.py"]), ("launch-policy", "static_analysis", "Generated Claude launch policy", ["python3", "scripts/generate-claude-launch-environment-policy.py", "--check"]), ("test-wiring", "static_analysis", "Swift test wiring and regression guard", ["bash", "tests/test_ci_pbxproj_test_wiring.sh"]), ("package-groups", "static_analysis", "Workspace Swift package groups", ["python3", "scripts/check-workspace-package-groups.py", "--check"]), diff --git a/scripts/wire-app-sources.py b/scripts/wire-app-sources.py new file mode 100755 index 000000000000..cd234b75c93f --- /dev/null +++ b/scripts/wire-app-sources.py @@ -0,0 +1,176 @@ +#!/usr/bin/env python3 +"""Wire Sources/**/*.swift files into the cmux app target. + +`scripts/sync-test-wiring` reconciles cmuxTests only; app sources were added +by hand, and a merge that takes main's project.pbxproj silently drops a +branch's new app files. This adds the four entries Xcode needs for each file +(PBXBuildFile, PBXFileReference, group child, app Sources phase), each placed +next to an already-wired file from the same directory, with IDs derived from +the path so reruns and parallel branches agree. + + scripts/wire-app-sources.py # wire every unwired file + scripts/wire-app-sources.py Sources/X.swift # wire these + scripts/wire-app-sources.py --check # exit 1 if any is unwired + +After a merge that took main's project.pbxproj, run it with no arguments. + +Unwired means `lint-pbxproj-test-wiring.sh --target cmux` would flag it: +not a member of the cmux Sources phase and not in +scripts/pbxproj-sources-wiring-allowlist.txt. +""" + +from __future__ import annotations + +import argparse +import hashlib +import re +import subprocess +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +PBXPROJ = Path("cmux.xcodeproj/project.pbxproj") +ALLOWLIST = Path("scripts/pbxproj-sources-wiring-allowlist.txt") + + +def object_id(seed: str) -> str: + return hashlib.sha1(seed.encode()).hexdigest()[:24].upper() + + +def app_sources_phase(text: str) -> tuple[int, int]: + """Span of the cmux app target's PBXSourcesBuildPhase `files = (...)` list.""" + target = re.search( + r"/\* cmux \*/ = \{\s*isa = PBXNativeTarget;.*?buildPhases = \((.*?)\);", + text, + re.S, + ) + if not target: + raise SystemExit("wire-app-sources: cmux PBXNativeTarget not found") + phase_id = re.search(r"([0-9A-Za-z]+) /\* Sources \*/", target.group(1)) + if not phase_id: + raise SystemExit("wire-app-sources: cmux target has no Sources phase") + block = re.search( + re.escape(phase_id.group(1)) + r" /\* Sources \*/ = \{.*?files = \((.*?)\);", + text, + re.S, + ) + if not block: + raise SystemExit("wire-app-sources: cmux Sources phase block not found") + return block.start(1), block.end(1) + + +def wired_names(text: str) -> set[str]: + start, end = app_sources_phase(text) + return set(re.findall(r"/\* (.+?) in Sources \*/", text[start:end])) + + +def unwired_sources(root: Path, text: str) -> list[str]: + allow = set() + if (root / ALLOWLIST).exists(): + allow = { + line.strip() + for line in (root / ALLOWLIST).read_text().splitlines() + if line.strip() and not line.startswith("#") + } + names = wired_names(text) + result = [] + for path in sorted((root / "Sources").rglob("*.swift")): + rel = path.relative_to(root).as_posix() + if rel not in allow and path.name not in names: + result.append(rel) + return result + + +def quoted(value: str) -> str: + """OpenStep plists leave only these characters unquoted.""" + return value if re.fullmatch(r"[A-Za-z0-9_./$-]+", value) else f'"{value}"' + + +def wire(text: str, rel: str) -> str: + """Adds `rel` (Sources/...) next to a wired sibling from its directory.""" + name = Path(rel).name + directory = Path(rel).parent.relative_to("Sources").as_posix() + prefix = "" if directory == "." else directory + "/" + siblings = re.findall( + r"\t\t([0-9A-Za-z]+) /\* [^*]*? \*/ = \{isa = PBXFileReference; lastKnownFileType = sourcecode\.swift; path = " + + re.escape(prefix) + + r"[^/;]+\.swift; sourceTree = \"\"; \};", + text, + ) + start, end = app_sources_phase(text) + phase = text[start:end] + sibling_ref = next( + (ref for ref in siblings if re.search(r"fileRef = " + ref + r" ", text) and ref in text), + None, + ) + sibling_build = None + for ref in siblings: + build = re.search(r"\t\t([0-9A-Za-z]+) /\* [^*]+ in Sources \*/ = \{isa = PBXBuildFile; fileRef = " + ref + " ", text) + if build and build.group(1) in phase: + sibling_ref, sibling_build = ref, build.group(1) + break + if not sibling_build: + raise SystemExit(f"wire-app-sources: no wired sibling in Sources/{prefix} for {rel}") + + ref_id = object_id("fileref:" + rel) + build_id = object_id("buildfile:" + rel) + lines = text.split("\n") + + def insert_after(pattern: str, new_line: str, within: tuple[int, int] | None = None) -> None: + for i, line in enumerate(lines): + if re.search(pattern, line): + if within: + offset = sum(len(l) + 1 for l in lines[:i]) + if not within[0] <= offset <= within[1]: + continue + indent = re.match(r"\s*", line).group(0) + lines.insert(i + 1, indent + new_line) + return + raise SystemExit(f"wire-app-sources: anchor {pattern!r} not found for {rel}") + + insert_after( + r"^\t\t" + sibling_build + r" /\* .* \*/ = \{isa = PBXBuildFile;", + f"{build_id} /* {name} in Sources */ = {{isa = PBXBuildFile; fileRef = {ref_id} /* {name} */; }};", + ) + insert_after( + r"^\t\t" + sibling_ref + r" /\* .* \*/ = \{isa = PBXFileReference;", + f'{ref_id} /* {name} */ = {{isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = {quoted(prefix + name)}; sourceTree = ""; }};', + ) + insert_after(r"^\t+" + sibling_ref + r" /\* .* \*/,$", f"{ref_id} /* {name} */,") + text = "\n".join(lines) + start, end = app_sources_phase(text) + lines = text.split("\n") + insert_after(r"^\t+" + sibling_build + r" /\* .* in Sources \*/,$", f"{build_id} /* {name} in Sources */,", (start, end)) + return "\n".join(lines) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument("paths", nargs="*", help="Sources/... files; default: every unwired one") + parser.add_argument("--check", action="store_true", help="list unwired files and exit 1 if any") + parser.add_argument("--root", type=Path, default=ROOT) + args = parser.parse_args(argv) + + pbxproj = args.root / PBXPROJ + text = pbxproj.read_text() + targets = args.paths or unwired_sources(args.root, text) + if args.check: + for rel in targets: + print(f"unwired: {rel}") + print(f"wire-app-sources: {'ok' if not targets else f'{len(targets)} unwired'}") + return 1 if targets else 0 + for rel in targets: + if Path(rel).name in wired_names(text): + print(f"already wired: {rel}") + continue + text = wire(text, rel) + print(f"wired: {rel}") + pbxproj.write_text(text) + if targets: + # The pre-commit hook and check-pbxproj.sh require normalized output. + subprocess.run([sys.executable, str(args.root / "scripts/normalize-pbxproj.py"), str(pbxproj)], check=True) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/skills/cmux-testing/SKILL.md b/skills/cmux-testing/SKILL.md index 5bea2484a298..10facd847767 100644 --- a/skills/cmux-testing/SKILL.md +++ b/skills/cmux-testing/SKILL.md @@ -17,6 +17,7 @@ even `verify-local.py --help` and `--list` load repository code. | Parse current Swift edits | `python3 scripts/verify-local.py --only swift-syntax --swift-changed` | | Check new Swift test-file wiring | `python3 scripts/verify-local.py --only test-wiring` | | See what a UI test did, one frame per action | `scripts/ui-test ClassName` or `scripts/ui-test ` ([guide](references/ui-test-frames.md)) | +| Render view code to PNGs in seconds, light and dark, without building the app | `scripts/ui-lab/ui-lab.py [--watch]` ([guide](references/ui-lab.md)) | | Dogfood the app from CI: drive it with a JSON tour and get screenshots and accessibility trees | `scripts/run-e2e.sh --scenario dogfood/scenarios/.json --ref --frames` ([guide](references/dogfood-scenarios.md)) | Add a base ref after `--swift-changed` to include committed changes. Use `--list` @@ -50,7 +51,7 @@ membership in `cmux.xcodeproj/project.pbxproj`. Add through Xcode or follow a wi sibling, then run the wiring check above: an unwired file can otherwise produce a misleading zero-test pass. -After creating, renaming, or deleting a direct `cmuxTests/*.swift` file, run `./scripts/sync-test-wiring`. It deterministically reconciles the `PBXFileReference`, `PBXBuildFile`, `cmuxTests` group child, and `cmuxTests` Sources membership; `--check` performs the same validation without writing. Foreign target membership is rejected with an explicit diagnostic. The `workflow-guard-tests` CI job still runs `./scripts/lint-pbxproj-test-wiring.sh` as a defensive Sources-phase guard. +After creating, renaming, or deleting a direct `cmuxTests/*.swift` file, run `./scripts/sync-test-wiring`. It deterministically reconciles the `PBXFileReference`, `PBXBuildFile`, `cmuxTests` group child, and `cmuxTests` Sources membership; `--check` performs the same validation without writing. Foreign target membership is rejected with an explicit diagnostic. New `Sources/**/*.swift` app files are wired with `./scripts/wire-app-sources.py` (`--check` lists unwired ones); run it after any merge that took main's `project.pbxproj`, which drops a branch's app-source entries. The `workflow-guard-tests` CI job still runs `./scripts/lint-pbxproj-test-wiring.sh` as a defensive Sources-phase guard. ## Test quality diff --git a/skills/cmux-testing/references/ui-lab.md b/skills/cmux-testing/references/ui-lab.md new file mode 100644 index 000000000000..336775e230d7 --- /dev/null +++ b/skills/cmux-testing/references/ui-lab.md @@ -0,0 +1,53 @@ +# ui-lab: render view code without building the app + +`scripts/ui-lab/ui-lab.py` compiles a small harness plus the app source files +it names with plain `swiftc`, runs it, and writes PNGs. A compile takes a few +seconds and an unchanged re-run is cached, so a pixel question (spacing, glyph +weight, color in dark mode) gets an answer in seconds instead of an app build +or a CI UI-test run. + +```bash +scripts/ui-lab/ui-lab.py scripts/ui-lab/harnesses/gpu-spinner.swift +scripts/ui-lab/ui-lab.py --watch # re-render on every save +``` + +Each `UILab.render` call writes `-light@2x.png`, `-dark@2x.png` +and, with `detail:`, a 4x crop of that rect. Paths are printed; the default +directory is `$TMPDIR/cmux-ui-lab/`. + +## Writing a harness + +```swift +// ui-lab: source Sources/Sidebar/GPUSpinnerNSView.swift +// ui-lab: shim SidebarAppearanceColorResolver + +import AppKit + +UILab.main { + let canvas = UILab.Canvas(frame: NSRect(x: 0, y: 0, width: 200, height: 60)) + canvas.fill = .windowBackgroundColor + // build views with the app's real types, add them to canvas + UILab.render(canvas, name: "example", detail: canvas.bounds) +} +``` + +- `source` lines are repo-relative app files. They compile as one module with + the harness; `import Cmux*` lines are dropped. +- `shim` lines name files in `scripts/ui-lab/shims/`: small stand-ins for app + types the sources use (for example `RenderableSystemSymbol`). Keep a shim + faithful to the real type's behavior for what it renders. +- `UILab.Canvas` is flipped (top-down), like the sidebar cells. Mirror the + real layout's metrics in the harness and cite where they come from. + +A file that needs many app or package types does not fit. Split its drawing +into a file with only AppKit/SwiftUI dependencies (as the compact status glyph +does), which also keeps it testable. + +## What it is not + +It renders a still frame of your view code in a mock layout, not the running +app: no window chrome, real cell layout, animation or live data. Confirm the +result in the app with a CI UI test (`scripts/ui-test`) or a dogfood tour +before calling a UI change done, and put that screenshot in the PR. + +Compiling a few files is fine on a laptop; this is not an app build. diff --git a/tests/test-execution.toml b/tests/test-execution.toml index 3ddceea39c6b..49e33a9cf144 100644 --- a/tests/test-execution.toml +++ b/tests/test-execution.toml @@ -740,10 +740,18 @@ lane = "linux-guard" path = "tests/test_tui_package_contract.py" lane = "linux-guard" +[[test]] +path = "tests/test_ui_lab.py" +lane = "linux-guard" + [[test]] path = "tests/test_web_complexity_trusted_workflow.py" lane = "linux-guard" +[[test]] +path = "tests/test_wire_app_sources.py" +lane = "linux-guard" + [[test]] path = "tests/test_write_sidebar_extension_point.py" lane = "linux-guard" diff --git a/tests/test_ui_lab.py b/tests/test_ui_lab.py new file mode 100644 index 000000000000..99c568998985 --- /dev/null +++ b/tests/test_ui_lab.py @@ -0,0 +1,46 @@ +#!/usr/bin/env python3 +"""scripts/ui-lab/ui-lab.py reads a harness's source and shim directives.""" + +import importlib.util +import tempfile +import unittest +from pathlib import Path + +SCRIPT = Path(__file__).resolve().parents[1] / "scripts/ui-lab/ui-lab.py" +spec = importlib.util.spec_from_file_location("ui_lab", SCRIPT) +ui_lab = importlib.util.module_from_spec(spec) +spec.loader.exec_module(ui_lab) + + +class UILabTests(unittest.TestCase): + def test_inputs_are_support_then_directives_in_order_then_the_harness(self): + with tempfile.TemporaryDirectory() as directory: + harness = Path(directory) / "h.swift" + harness.write_text( + "// ui-lab: source Sources/Sidebar/GPUSpinnerStyle.swift\n" + "// ui-lab: shim SidebarAppearanceColorResolver\n" + "// ui-lab: source is prose here, not a directive\n" + "UILab.main {}\n" + ) + files = ui_lab.inputs(harness) + self.assertEqual(files[0], ui_lab.LAB / "UILab.swift") + self.assertEqual(files[1], ui_lab.ROOT / "Sources/Sidebar/GPUSpinnerStyle.swift") + self.assertEqual(files[2], ui_lab.LAB / "shims/SidebarAppearanceColorResolver.swift") + self.assertEqual(files[3], harness) + self.assertEqual(len(files), 4) + + def test_a_missing_source_is_reported(self): + with tempfile.TemporaryDirectory() as directory: + harness = Path(directory) / "h.swift" + harness.write_text("// ui-lab: source Sources/Nope.swift\n") + with self.assertRaises(SystemExit): + ui_lab.inputs(harness) + + def test_every_bundled_harness_names_existing_files(self): + for harness in sorted((ui_lab.LAB / "harnesses").glob("*.swift")): + with self.subTest(harness=harness.name): + self.assertTrue(all(path.exists() for path in ui_lab.inputs(harness))) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_wire_app_sources.py b/tests/test_wire_app_sources.py new file mode 100644 index 000000000000..aa543c40ffc4 --- /dev/null +++ b/tests/test_wire_app_sources.py @@ -0,0 +1,108 @@ +#!/usr/bin/env python3 +"""scripts/wire-app-sources.py adds an unwired app source next to its sibling.""" + +import importlib.util +import tempfile +import unittest +from pathlib import Path + +SCRIPT = Path(__file__).resolve().parents[1] / "scripts/wire-app-sources.py" +spec = importlib.util.spec_from_file_location("wire_app_sources", SCRIPT) +wire_app_sources = importlib.util.module_from_spec(spec) +spec.loader.exec_module(wire_app_sources) + +PROJECT = """// !$*UTF8*$! +{ + objects = { + +/* Begin PBXBuildFile section */ + AAAA00000000000000000001 /* Wired.swift in Sources */ = {isa = PBXBuildFile; fileRef = AAAA00000000000000000002 /* Wired.swift */; }; + TTTT00000000000000000001 /* Wired.swift in Sources */ = {isa = PBXBuildFile; fileRef = AAAA00000000000000000002 /* Wired.swift */; }; +/* End PBXBuildFile section */ + +/* Begin PBXFileReference section */ + AAAA00000000000000000002 /* Wired.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/Wired.swift; sourceTree = ""; }; +/* End PBXFileReference section */ + +/* Begin PBXGroup section */ + GGGG00000000000000000001 /* Sources */ = { + isa = PBXGroup; + children = ( + AAAA00000000000000000002 /* Wired.swift */, + ); + path = Sources; + sourceTree = ""; + }; +/* End PBXGroup section */ + +/* Begin PBXNativeTarget section */ + NNNN00000000000000000001 /* cmux */ = { + isa = PBXNativeTarget; + buildPhases = ( + SSSS00000000000000000001 /* Sources */, + ); + name = cmux; + }; + NNNN00000000000000000002 /* cmuxTests */ = { + isa = PBXNativeTarget; + buildPhases = ( + SSSS00000000000000000002 /* Sources */, + ); + name = cmuxTests; + }; +/* End PBXNativeTarget section */ + +/* Begin PBXSourcesBuildPhase section */ + SSSS00000000000000000001 /* Sources */ = { + isa = PBXSourcesBuildPhase; + files = ( + AAAA00000000000000000001 /* Wired.swift in Sources */, + ); + }; + SSSS00000000000000000002 /* Sources */ = { + isa = PBXSourcesBuildPhase; + files = ( + TTTT00000000000000000001 /* Wired.swift in Sources */, + ); + }; +/* End PBXSourcesBuildPhase section */ + }; +} +""" + + +class WireAppSourcesTests(unittest.TestCase): + def test_finds_and_wires_an_unwired_file_into_the_app_target_only(self): + with tempfile.TemporaryDirectory() as root: + root = Path(root) + (root / "Sources/Sidebar").mkdir(parents=True) + (root / "Sources/Sidebar/Wired.swift").write_text("") + (root / "Sources/Sidebar/Glyph+Resolve.swift").write_text("") + self.assertEqual( + wire_app_sources.unwired_sources(root, PROJECT), + ["Sources/Sidebar/Glyph+Resolve.swift"], + ) + + text = wire_app_sources.wire(PROJECT, "Sources/Sidebar/Glyph+Resolve.swift") + + self.assertIn("Glyph+Resolve.swift", wire_app_sources.wired_names(text)) + # `+` needs quoting in an OpenStep plist. + self.assertIn('path = "Sidebar/Glyph+Resolve.swift";', text) + self.assertEqual(text.count("/* Glyph+Resolve.swift in Sources */"), 2) # build file + app phase + self.assertEqual(text.count("/* Glyph+Resolve.swift */"), 3) # file ref, build file's fileRef, group child + tests_phase = text[text.index("SSSS00000000000000000002 /* Sources */ = {"):] + self.assertNotIn("Glyph+Resolve", tests_phase) + self.assertEqual(wire_app_sources.unwired_sources(root, text), []) + + def test_ids_are_stable_per_path(self): + once = wire_app_sources.wire(PROJECT, "Sources/Sidebar/New.swift") + twice = wire_app_sources.wire(PROJECT, "Sources/Sidebar/New.swift") + self.assertEqual(once, twice) + + def test_a_directory_with_no_wired_sibling_is_an_error(self): + with self.assertRaises(SystemExit): + wire_app_sources.wire(PROJECT, "Sources/Elsewhere/New.swift") + + +if __name__ == "__main__": + unittest.main() From 2fb3b6e8cb26c090178feaeb54d06254a19d9643 Mon Sep 17 00:00:00 2001 From: teamleaderleo Date: Sun, 27 Sep 2026 14:44:53 -0400 Subject: [PATCH 2/4] tools: address review of ui-lab and wire-app-sources wire-app-sources resolves file paths through the real group tree from the Sources group (nested groups with their own path, SOURCE_ROOT refs, lines holding two entries), puts a new file in the deepest group owning its directory with a group-relative path, and matches wired files by path, not basename. Before, a top-level file could land in a nested group whose ref sorted first, and nested-group directories could not be wired. ui-lab: render takes a per-scheme builder, since views that color themselves from a colorScheme property ignore the appearance (the spinner's dark render showed light colors). Package imports are blanked instead of deleted, so compiler line numbers match. The cache key covers ui-lab.py, the SDK and DEVELOPER_DIR; binaries are written atomically and pruned after 14 days; a missing swiftc is a clear error. Co-Authored-By: Claude Opus 5.5 --- scripts/ui-lab/UILab.swift | 24 +- scripts/ui-lab/harnesses/gpu-spinner.swift | 37 +-- scripts/ui-lab/ui-lab.py | 49 +++- scripts/wire-app-sources.py | 273 ++++++++++++++------- skills/cmux-testing/references/ui-lab.md | 20 +- tests/test_ui_lab.py | 18 ++ tests/test_wire_app_sources.py | 122 +++++---- 7 files changed, 373 insertions(+), 170 deletions(-) diff --git a/scripts/ui-lab/UILab.swift b/scripts/ui-lab/UILab.swift index 9741e77d7add..65a48431feba 100644 --- a/scripts/ui-lab/UILab.swift +++ b/scripts/ui-lab/UILab.swift @@ -1,9 +1,10 @@ import AppKit +import SwiftUI /// Rendering support for ui-lab harnesses (see scripts/ui-lab/ui-lab.py). -/// A harness calls `UILab.main { ... }`, builds an NSView inside it and calls -/// `UILab.render(_:name:)`; the output -/// directory is the process's first argument. +/// A harness calls `UILab.main { ... }` and, inside it, +/// `UILab.render(name:) { scheme in ... }` to build and render its view. +/// The output directory is the process's first argument. enum UILab { static let outputDirectory: URL = { let path = CommandLine.arguments.dropFirst().first ?? FileManager.default.currentDirectoryPath @@ -21,12 +22,21 @@ enum UILab { } /// Writes `-light@2x.png` and `-dark@2x.png`, and, when - /// `detail` is set, `-light@4x.png` cropped to that rect (in view - /// points) for a close look at small glyphs. + /// `detail` is set, `-light-detail@4x.png` cropped to that rect (in + /// view points) for a close look at small glyphs. `build` runs once per + /// scheme: views that pick colors from their own `colorScheme` property + /// (like `GPUSpinnerNSView`) need it set, since the appearance alone does + /// not reach them. @MainActor - static func render(_ view: NSView, name: String, detail: NSRect? = nil) { - for (label, appearanceName) in [("light", NSAppearance.Name.aqua), ("dark", NSAppearance.Name.darkAqua)] { + static func render(name: String, detail: NSRect? = nil, build: (ColorScheme) -> NSView) { + for (label, scheme, appearanceName) in [ + ("light", ColorScheme.light, NSAppearance.Name.aqua), + ("dark", ColorScheme.dark, NSAppearance.Name.darkAqua), + ] { let appearance = NSAppearance(named: appearanceName)! + var view: NSView? + appearance.performAsCurrentDrawingAppearance { view = build(scheme) } + guard let view else { continue } write(view, appearance: appearance, scale: 2, rect: view.bounds, file: "\(name)-\(label)@2x.png") if let detail, label == "light" { write(view, appearance: appearance, scale: 4, rect: detail, file: "\(name)-\(label)-detail@4x.png") diff --git a/scripts/ui-lab/harnesses/gpu-spinner.swift b/scripts/ui-lab/harnesses/gpu-spinner.swift index 65d9a584b699..e50a90b500c8 100644 --- a/scripts/ui-lab/harnesses/gpu-spinner.swift +++ b/scripts/ui-lab/harnesses/gpu-spinner.swift @@ -2,9 +2,9 @@ // ui-lab: source Sources/Sidebar/GPUSpinnerNSView.swift // ui-lab: shim SidebarAppearanceColorResolver // -// The sidebar loading spinner in each style and color scheme, at the sizes -// rows use (12 pt, 16 pt). A still frame: the rotation is a Core Animation -// loop that ui-lab does not advance. +// The sidebar loading spinner in each style, at the sizes rows use (12 pt, +// 16 pt). A still frame: the rotation is a Core Animation loop that ui-lab +// does not advance. import AppKit @@ -12,19 +12,24 @@ UILab.main { let styles: [GPUSpinnerStyle] = [.macOSSpokes, .arc] let sizes: [CGFloat] = [12, 16] let cell: CGFloat = 28 - let canvas = UILab.Canvas(frame: NSRect(x: 0, y: 0, width: cell * CGFloat(sizes.count) + 16, height: cell * CGFloat(styles.count) + 16)) - canvas.fill = .windowBackgroundColor - for (row, style) in styles.enumerated() { - for (column, size) in sizes.enumerated() { - let spinner = GPUSpinnerNSView(frame: NSRect( - x: 8 + CGFloat(column) * cell + (cell - size) / 2, - y: 8 + CGFloat(row) * cell + (cell - size) / 2, - width: size, - height: size - )) - spinner.style = style - canvas.addSubview(spinner) + let bounds = NSRect(x: 0, y: 0, width: cell * CGFloat(sizes.count) + 16, height: cell * CGFloat(styles.count) + 16) + UILab.render(name: "gpu-spinner", detail: bounds) { scheme in + let canvas = UILab.Canvas(frame: bounds) + canvas.fill = .windowBackgroundColor + for (row, style) in styles.enumerated() { + for (column, size) in sizes.enumerated() { + let spinner = GPUSpinnerNSView(frame: NSRect( + x: 8 + CGFloat(column) * cell + (cell - size) / 2, + y: 8 + CGFloat(row) * cell + (cell - size) / 2, + width: size, + height: size + )) + spinner.style = style + // The spinner colors itself from this, not the appearance. + spinner.colorScheme = scheme + canvas.addSubview(spinner) + } } + return canvas } - UILab.render(canvas, name: "gpu-spinner", detail: canvas.bounds) } diff --git a/scripts/ui-lab/ui-lab.py b/scripts/ui-lab/ui-lab.py index fb1d44bd719f..b9a6a4e83f8d 100755 --- a/scripts/ui-lab/ui-lab.py +++ b/scripts/ui-lab/ui-lab.py @@ -4,8 +4,8 @@ A harness is a Swift file whose top-level code builds views and hands them to `UILab.render`. Its header names the app sources to compile with it: - // ui-lab: source Sources/Sidebar/SidebarCompactStatusGlyph.swift - // ui-lab: shim RenderableSystemSymbol + // ui-lab: source Sources/Sidebar/GPUSpinnerNSView.swift + // ui-lab: shim SidebarAppearanceColorResolver `source` paths are repo-relative; `shim` names a file in scripts/ui-lab/shims/ standing in for an app type the sources use. Sources are compiled as one @@ -13,7 +13,7 @@ harness can only pull in files without package or app dependencies beyond its shims. Keep view code that way when you want it here. - scripts/ui-lab/ui-lab.py scripts/ui-lab/harnesses/sidebar-compact-status.swift + scripts/ui-lab/ui-lab.py scripts/ui-lab/harnesses/gpu-spinner.swift scripts/ui-lab/ui-lab.py --watch # re-render on every save scripts/ui-lab/ui-lab.py --out DIR @@ -39,6 +39,14 @@ ROOT = Path(__file__).resolve().parents[2] LAB = Path(__file__).resolve().parent DIRECTIVE = re.compile(r"^//\s*ui-lab:\s*(source|shim)\s+(\S+)\s*$") +# `import CmuxFoo`, `@testable import CmuxFoo`, `import struct CmuxFoo.Bar`... +# Blanked, not deleted, so compiler line numbers still match the real file. +PACKAGE_IMPORT = re.compile( + r"^[ \t]*(?:@\w+[ \t]+)*(?:(?:public|internal|package|private|fileprivate)[ \t]+)?" + r"import[ \t]+(?:(?:struct|class|enum|protocol|typealias|func|var|let|actor)[ \t]+)?Cmux\w*[^\n]*$", + re.M, +) +CACHE_DAYS = 14 CACHE = Path(os.environ.get("CMUX_UI_LAB_CACHE", Path.home() / "Library/Caches/cmux-ui-lab")) @@ -57,16 +65,38 @@ def inputs(harness: Path) -> list[Path]: return files + [harness] +def toolchain() -> str: + """What else decides the binary: compiler, SDK and developer dir.""" + try: + version = subprocess.run(["swiftc", "--version"], capture_output=True, text=True, check=True).stdout + sdk = subprocess.run(["xcrun", "--show-sdk-path"], capture_output=True, text=True).stdout + except (OSError, subprocess.CalledProcessError) as error: + raise SystemExit(f"ui-lab: swiftc is not available ({error}); install Xcode or the command line tools") + return version + sdk + os.environ.get("DEVELOPER_DIR", "") + + +def prune_cache() -> None: + cutoff = time.time() - CACHE_DAYS * 86400 + for entry in CACHE.glob("*/lab"): + try: + if entry.stat().st_mtime < cutoff: + shutil.rmtree(entry.parent, ignore_errors=True) + except OSError: + pass + + def build(harness: Path) -> Path: files = inputs(harness) - digest = hashlib.sha256() + digest = hashlib.sha256(Path(__file__).read_bytes()) # flags and source rewriting for path in files: digest.update(str(path).encode()) digest.update(path.read_bytes()) - digest.update(subprocess.run(["swiftc", "--version"], capture_output=True, text=True).stdout.encode()) + digest.update(toolchain().encode()) binary = CACHE / digest.hexdigest()[:16] / "lab" if binary.exists(): + os.utime(binary) return binary + prune_cache() work = Path(tempfile.mkdtemp(prefix="cmux-ui-lab-")) try: @@ -75,14 +105,17 @@ def build(harness: Path) -> Path: text = path.read_text() if path != harness: # One module: package imports resolve to shims or nothing. - text = re.sub(r"^(?:@_implementationOnly |public |internal )?import Cmux\w*\s*$", "", text, flags=re.M) + text = PACKAGE_IMPORT.sub("", text) name = "main.swift" if path == harness else f"{index:02d}-{path.name}" (work / name).write_text(text) compiled.append(str(work / name)) binary.parent.mkdir(parents=True, exist_ok=True) started = time.monotonic() + # Build beside the cache entry, then rename: concurrent runs of the + # same harness never see a half-written binary. + partial = work / "lab" result = subprocess.run( - ["swiftc", "-Onone", "-swift-version", "5", "-o", str(binary), *compiled], + ["swiftc", "-Onone", "-swift-version", "5", "-o", str(partial), *compiled], capture_output=True, text=True, ) @@ -94,6 +127,8 @@ def build(harness: Path) -> Path: output = output.replace(str(work / name), str(path)) sys.stderr.write(output) raise SystemExit("ui-lab: compile failed") + shutil.copy2(partial, binary.with_name(f"lab.{os.getpid()}")) + os.replace(binary.with_name(f"lab.{os.getpid()}"), binary) print(f"ui-lab: compiled {len(files)} files in {time.monotonic() - started:.1f}s", file=sys.stderr) return binary finally: diff --git a/scripts/wire-app-sources.py b/scripts/wire-app-sources.py index cd234b75c93f..bf411dd7558a 100755 --- a/scripts/wire-app-sources.py +++ b/scripts/wire-app-sources.py @@ -4,9 +4,8 @@ `scripts/sync-test-wiring` reconciles cmuxTests only; app sources were added by hand, and a merge that takes main's project.pbxproj silently drops a branch's new app files. This adds the four entries Xcode needs for each file -(PBXBuildFile, PBXFileReference, group child, app Sources phase), each placed -next to an already-wired file from the same directory, with IDs derived from -the path so reruns and parallel branches agree. +(PBXBuildFile, PBXFileReference, group child, app Sources phase) and +normalizes the project. scripts/wire-app-sources.py # wire every unwired file scripts/wire-app-sources.py Sources/X.swift # wire these @@ -14,35 +13,133 @@ After a merge that took main's project.pbxproj, run it with no arguments. -Unwired means `lint-pbxproj-test-wiring.sh --target cmux` would flag it: -not a member of the cmux Sources phase and not in -scripts/pbxproj-sources-wiring-allowlist.txt. +Paths are resolved through the real group tree from the `Sources` group, so +a file lands in the deepest group that owns its directory (the Sources group +itself for `Sidebar/X.swift`, the Cloud group for `Cloud/X.swift`), with a +path relative to that group. Wired means a cmux Sources-phase build file +refers to that resolved path; files in scripts/pbxproj-sources-wiring- +allowlist.txt are left alone. IDs are derived from the path, so reruns and +parallel branches agree. """ from __future__ import annotations import argparse import hashlib +import posixpath import re import subprocess import sys +from dataclasses import dataclass, field from pathlib import Path ROOT = Path(__file__).resolve().parents[1] PBXPROJ = Path("cmux.xcodeproj/project.pbxproj") ALLOWLIST = Path("scripts/pbxproj-sources-wiring-allowlist.txt") +FILE_REF = re.compile( + # Not line-anchored: the project has lines holding two entries. + r"(?P[0-9A-Za-z]+) /\* [^*]*? \*/ = \{isa = PBXFileReference;(?P[^}\n]*)\};" +) +BUILD_FILE = re.compile( + r"(?P[0-9A-Za-z]+) /\* [^*]*? \*/ = \{isa = PBXBuildFile; fileRef = (?P[0-9A-Za-z]+) " +) +GROUP = re.compile( + r"^\t+(?P[0-9A-Za-z]+) /\* [^\n]*? \*/ = \{\n\t+isa = PBXGroup;\n\t+children = \(\n(?P.*?)\t+\);\n(?P.*?)\n\t+\};$", + re.M | re.S, +) +CHILD_ID = re.compile(r"^\t+([0-9A-Za-z]+) /\*", re.M) + + +def setting(body: str, key: str) -> str | None: + match = re.search(r"\b" + key + r' = ("(?:[^"\\]|\\.)*"|[^;]+);', body) + if not match: + return None + value = match.group(1) + return value[1:-1] if value.startswith('"') else value + + +def quoted(value: str) -> str: + """OpenStep plists leave only these characters unquoted.""" + return value if re.fullmatch(r"[A-Za-z0-9_./$-]+", value) else f'"{value}"' + def object_id(seed: str) -> str: return hashlib.sha1(seed.encode()).hexdigest()[:24].upper() +@dataclass +class Group: + id: str + directory: str # repo-relative + depth: int + children: list[str] = field(default_factory=list) + children_span: tuple[int, int] = (0, 0) + + +@dataclass +class Project: + text: str + groups: dict[str, Group] + ref_paths: dict[str, str] # file ref id -> repo-relative path, under Sources + app_phase_span: tuple[int, int] + + @property + def wired_paths(self) -> set[str]: + start, end = self.app_phase_span + phase_ids = set(CHILD_ID.findall(self.text[start:end])) + return { + self.ref_paths[match.group("ref")] + for match in BUILD_FILE.finditer(self.text) + if match.group("id") in phase_ids and match.group("ref") in self.ref_paths + } + + +def parse(text: str) -> Project: + refs = {m.group("id"): m.group("body") for m in FILE_REF.finditer(text)} + raw_groups = {} + for match in GROUP.finditer(text): + raw_groups[match.group("id")] = ( + CHILD_ID.findall(match.group("children")), + setting(match.group("rest"), "path"), + setting(match.group("rest"), "sourceTree"), + match.span("children"), + ) + root_id = next( + (gid for gid, (_, path, tree, _) in raw_groups.items() if path == "Sources" and tree == ''), + None, + ) + if root_id is None: + raise SystemExit("wire-app-sources: the Sources group was not found") + + groups: dict[str, Group] = {} + ref_paths: dict[str, str] = {} + + def walk(gid: str, directory: str, depth: int) -> None: + children, _, _, span = raw_groups[gid] + groups[gid] = Group(gid, directory, depth, children, span) + for child in children: + if child in raw_groups: + _, path, tree, _ = raw_groups[child] + if tree == "": + walk(child, posixpath.normpath(posixpath.join(directory, path)) if path else directory, depth + 1) + elif child in refs and setting(refs[child], "sourceTree") == "": + path = setting(refs[child], "path") + if path: + ref_paths[child] = posixpath.normpath(posixpath.join(directory, path)) + + walk(root_id, "Sources", 0) + # Some refs are repo-relative (`sourceTree = SOURCE_ROOT`) wherever they sit. + for ref, body in refs.items(): + if setting(body, "sourceTree") == "SOURCE_ROOT" and setting(body, "path"): + ref_paths[ref] = posixpath.normpath(setting(body, "path")) + return Project(text, groups, ref_paths, app_sources_phase(text)) + + def app_sources_phase(text: str) -> tuple[int, int]: """Span of the cmux app target's PBXSourcesBuildPhase `files = (...)` list.""" target = re.search( - r"/\* cmux \*/ = \{\s*isa = PBXNativeTarget;.*?buildPhases = \((.*?)\);", - text, - re.S, + r"/\* cmux \*/ = \{\s*isa = PBXNativeTarget;.*?buildPhases = \((.*?)\);", text, re.S ) if not target: raise SystemExit("wire-app-sources: cmux PBXNativeTarget not found") @@ -50,98 +147,88 @@ def app_sources_phase(text: str) -> tuple[int, int]: if not phase_id: raise SystemExit("wire-app-sources: cmux target has no Sources phase") block = re.search( - re.escape(phase_id.group(1)) + r" /\* Sources \*/ = \{.*?files = \((.*?)\);", - text, - re.S, + re.escape(phase_id.group(1)) + r" /\* Sources \*/ = \{.*?files = \((.*?)\);", text, re.S ) if not block: raise SystemExit("wire-app-sources: cmux Sources phase block not found") return block.start(1), block.end(1) -def wired_names(text: str) -> set[str]: - start, end = app_sources_phase(text) - return set(re.findall(r"/\* (.+?) in Sources \*/", text[start:end])) +def allowlisted(root: Path) -> set[str]: + path = root / ALLOWLIST + if not path.exists(): + return set() + entries = set() + for line in path.read_text().splitlines(): + entry = line.split("#", 1)[0].strip() # same as the lint: inline comments allowed + if entry: + entries.add(entry) + return entries def unwired_sources(root: Path, text: str) -> list[str]: - allow = set() - if (root / ALLOWLIST).exists(): - allow = { - line.strip() - for line in (root / ALLOWLIST).read_text().splitlines() - if line.strip() and not line.startswith("#") - } - names = wired_names(text) - result = [] - for path in sorted((root / "Sources").rglob("*.swift")): - rel = path.relative_to(root).as_posix() - if rel not in allow and path.name not in names: - result.append(rel) - return result + wired = parse(text).wired_paths + allow = allowlisted(root) + return [ + rel + for rel in (path.relative_to(root).as_posix() for path in sorted((root / "Sources").rglob("*.swift"))) + if rel not in allow and rel not in wired + ] -def quoted(value: str) -> str: - """OpenStep plists leave only these characters unquoted.""" - return value if re.fullmatch(r"[A-Za-z0-9_./$-]+", value) else f'"{value}"' +def owning_group(project: Project, rel: str) -> Group: + """The deepest group whose directory contains `rel`; among groups with + the same directory, one that already holds a file from that directory.""" + directory = posixpath.dirname(rel) + candidates = [ + group + for group in project.groups.values() + if directory == group.directory or directory.startswith(group.directory + "/") + ] + if not candidates: + raise SystemExit(f"wire-app-sources: {rel} is outside the Sources group") + deepest = max(len(group.directory) for group in candidates) + candidates = [group for group in candidates if len(group.directory) == deepest] + def holds_sibling(group: Group) -> bool: + return any(posixpath.dirname(project.ref_paths.get(child, "")) == directory for child in group.children) + + return max(candidates, key=lambda group: (holds_sibling(group), group.depth)) -def wire(text: str, rel: str) -> str: - """Adds `rel` (Sources/...) next to a wired sibling from its directory.""" - name = Path(rel).name - directory = Path(rel).parent.relative_to("Sources").as_posix() - prefix = "" if directory == "." else directory + "/" - siblings = re.findall( - r"\t\t([0-9A-Za-z]+) /\* [^*]*? \*/ = \{isa = PBXFileReference; lastKnownFileType = sourcecode\.swift; path = " - + re.escape(prefix) - + r"[^/;]+\.swift; sourceTree = \"\"; \};", - text, - ) - start, end = app_sources_phase(text) - phase = text[start:end] - sibling_ref = next( - (ref for ref in siblings if re.search(r"fileRef = " + ref + r" ", text) and ref in text), - None, - ) - sibling_build = None - for ref in siblings: - build = re.search(r"\t\t([0-9A-Za-z]+) /\* [^*]+ in Sources \*/ = \{isa = PBXBuildFile; fileRef = " + ref + " ", text) - if build and build.group(1) in phase: - sibling_ref, sibling_build = ref, build.group(1) - break - if not sibling_build: - raise SystemExit(f"wire-app-sources: no wired sibling in Sources/{prefix} for {rel}") +def wire(text: str, rel: str) -> str: + project = parse(text) + if rel in project.wired_paths: + return text + name = posixpath.basename(rel) + group = owning_group(project, rel) + group_path = posixpath.relpath(rel, group.directory) ref_id = object_id("fileref:" + rel) build_id = object_id("buildfile:" + rel) - lines = text.split("\n") - - def insert_after(pattern: str, new_line: str, within: tuple[int, int] | None = None) -> None: - for i, line in enumerate(lines): - if re.search(pattern, line): - if within: - offset = sum(len(l) + 1 for l in lines[:i]) - if not within[0] <= offset <= within[1]: - continue - indent = re.match(r"\s*", line).group(0) - lines.insert(i + 1, indent + new_line) - return - raise SystemExit(f"wire-app-sources: anchor {pattern!r} not found for {rel}") - - insert_after( - r"^\t\t" + sibling_build + r" /\* .* \*/ = \{isa = PBXBuildFile;", - f"{build_id} /* {name} in Sources */ = {{isa = PBXBuildFile; fileRef = {ref_id} /* {name} */; }};", - ) - insert_after( - r"^\t\t" + sibling_ref + r" /\* .* \*/ = \{isa = PBXFileReference;", - f'{ref_id} /* {name} */ = {{isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = {quoted(prefix + name)}; sourceTree = ""; }};', - ) - insert_after(r"^\t+" + sibling_ref + r" /\* .* \*/,$", f"{ref_id} /* {name} */,") - text = "\n".join(lines) - start, end = app_sources_phase(text) - lines = text.split("\n") - insert_after(r"^\t+" + sibling_build + r" /\* .* in Sources \*/,$", f"{build_id} /* {name} in Sources */,", (start, end)) - return "\n".join(lines) + + # Insert from the end of the file backwards so earlier offsets stay valid. + insertions = [ + (project.app_phase_span[1], f"\t\t\t\t{build_id} /* {name} in Sources */,\n", True), + (group.children_span[1], f"\t\t\t\t{ref_id} /* {name} */,\n", True), + ( + text.index("/* Begin PBXFileReference section */\n") + len("/* Begin PBXFileReference section */\n"), + f'\t\t{ref_id} /* {name} */ = {{isa = PBXFileReference; lastKnownFileType = sourcecode.swift; ' + f'path = {quoted(group_path)}; sourceTree = ""; }};\n', + False, + ), + ( + text.index("/* Begin PBXBuildFile section */\n") + len("/* Begin PBXBuildFile section */\n"), + f"\t\t{build_id} /* {name} in Sources */ = {{isa = PBXBuildFile; fileRef = {ref_id} /* {name} */; }};\n", + False, + ), + ] + for offset, line, before_closing in sorted(insertions, key=lambda item: item[0], reverse=True): + if before_closing: + # The span ends right before the closing `\t\t\t);`; keep the + # list's last line terminated. + offset = text.rindex("\n", 0, offset) + 1 + text = text[:offset] + line + text[offset:] + return text def main(argv: list[str] | None = None) -> int: @@ -156,17 +243,19 @@ def main(argv: list[str] | None = None) -> int: targets = args.paths or unwired_sources(args.root, text) if args.check: for rel in targets: - print(f"unwired: {rel}") - print(f"wire-app-sources: {'ok' if not targets else f'{len(targets)} unwired'}") + print(f"unwired: {rel}", flush=True) + print(f"wire-app-sources: {'ok' if not targets else f'{len(targets)} unwired'}", flush=True) return 1 if targets else 0 + changed = False for rel in targets: - if Path(rel).name in wired_names(text): - print(f"already wired: {rel}") + if rel in parse(text).wired_paths: + print(f"already wired: {rel}", flush=True) continue text = wire(text, rel) - print(f"wired: {rel}") - pbxproj.write_text(text) - if targets: + changed = True + print(f"wired: {rel}", flush=True) + if changed: + pbxproj.write_text(text) # The pre-commit hook and check-pbxproj.sh require normalized output. subprocess.run([sys.executable, str(args.root / "scripts/normalize-pbxproj.py"), str(pbxproj)], check=True) return 0 diff --git a/skills/cmux-testing/references/ui-lab.md b/skills/cmux-testing/references/ui-lab.md index 336775e230d7..6f6d836b9599 100644 --- a/skills/cmux-testing/references/ui-lab.md +++ b/skills/cmux-testing/references/ui-lab.md @@ -18,19 +18,28 @@ directory is `$TMPDIR/cmux-ui-lab/`. ## Writing a harness ```swift +// ui-lab: source Sources/Sidebar/GPUSpinnerStyle.swift // ui-lab: source Sources/Sidebar/GPUSpinnerNSView.swift // ui-lab: shim SidebarAppearanceColorResolver import AppKit UILab.main { - let canvas = UILab.Canvas(frame: NSRect(x: 0, y: 0, width: 200, height: 60)) - canvas.fill = .windowBackgroundColor - // build views with the app's real types, add them to canvas - UILab.render(canvas, name: "example", detail: canvas.bounds) + let bounds = NSRect(x: 0, y: 0, width: 200, height: 60) + UILab.render(name: "example", detail: bounds) { scheme in + let canvas = UILab.Canvas(frame: bounds) + canvas.fill = .windowBackgroundColor + // Build views with the app's real types and add them to canvas. + return canvas + } } ``` +`render` calls its closure once per color scheme, under that scheme's +appearance. Views that color themselves from a `colorScheme` property rather +than the appearance (`GPUSpinnerNSView` does) need it set from `scheme`, or +the dark render shows light colors. + - `source` lines are repo-relative app files. They compile as one module with the harness; `import Cmux*` lines are dropped. - `shim` lines name files in `scripts/ui-lab/shims/`: small stand-ins for app @@ -40,8 +49,7 @@ UILab.main { real layout's metrics in the harness and cite where they come from. A file that needs many app or package types does not fit. Split its drawing -into a file with only AppKit/SwiftUI dependencies (as the compact status glyph -does), which also keeps it testable. +into a file with only AppKit/SwiftUI dependencies, which also keeps it testable. ## What it is not diff --git a/tests/test_ui_lab.py b/tests/test_ui_lab.py index 99c568998985..3b7c60265789 100644 --- a/tests/test_ui_lab.py +++ b/tests/test_ui_lab.py @@ -42,5 +42,23 @@ def test_every_bundled_harness_names_existing_files(self): self.assertTrue(all(path.exists() for path in ui_lab.inputs(harness))) +class PackageImportTests(unittest.TestCase): + def test_package_imports_are_blanked_keeping_line_numbers(self): + source = "\n".join([ + "import AppKit", + "import CmuxSidebar", + "@testable import CmuxFoundation", + "@preconcurrency internal import CmuxSettings", + "import struct CmuxCore.Thing", + "", + "let x = 1 // import CmuxNope stays: not at line start", + ]) + stripped = ui_lab.PACKAGE_IMPORT.sub("", source) + self.assertEqual(stripped.count("\n"), source.count("\n")) + self.assertEqual(stripped.split("\n")[0], "import AppKit") + self.assertNotIn("import Cmux", "\n".join(stripped.split("\n")[:6])) + self.assertIn("import CmuxNope", stripped) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_wire_app_sources.py b/tests/test_wire_app_sources.py index aa543c40ffc4..234134620777 100644 --- a/tests/test_wire_app_sources.py +++ b/tests/test_wire_app_sources.py @@ -1,7 +1,8 @@ #!/usr/bin/env python3 -"""scripts/wire-app-sources.py adds an unwired app source next to its sibling.""" +"""scripts/wire-app-sources.py wires app sources through the real group tree.""" import importlib.util +import sys import tempfile import unittest from pathlib import Path @@ -9,26 +10,44 @@ SCRIPT = Path(__file__).resolve().parents[1] / "scripts/wire-app-sources.py" spec = importlib.util.spec_from_file_location("wire_app_sources", SCRIPT) wire_app_sources = importlib.util.module_from_spec(spec) +sys.modules["wire_app_sources"] = wire_app_sources # dataclasses look it up spec.loader.exec_module(wire_app_sources) +# A Sources group holding `Sidebar/Wired.swift` directly and a nested Cloud +# group with its own `path`, whose file (`AAA.swift`) sorts before every +# other ref, plus a repo-relative (SOURCE_ROOT) ref, and a cmuxTests target. PROJECT = """// !$*UTF8*$! { objects = { /* Begin PBXBuildFile section */ - AAAA00000000000000000001 /* Wired.swift in Sources */ = {isa = PBXBuildFile; fileRef = AAAA00000000000000000002 /* Wired.swift */; }; - TTTT00000000000000000001 /* Wired.swift in Sources */ = {isa = PBXBuildFile; fileRef = AAAA00000000000000000002 /* Wired.swift */; }; + B0000000000000000000000A /* AAA.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0000000000000000000000A /* AAA.swift */; }; + B0000000000000000000000B /* Wired.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0000000000000000000000B /* Wired.swift */; }; + B0000000000000000000000C /* Rooted.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0000000000000000000000C /* Rooted.swift */; }; + B0000000000000000000000T /* Wired.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0000000000000000000000B /* Wired.swift */; }; /* End PBXBuildFile section */ /* Begin PBXFileReference section */ - AAAA00000000000000000002 /* Wired.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/Wired.swift; sourceTree = ""; }; + F0000000000000000000000A /* AAA.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AAA.swift; sourceTree = ""; }; + F0000000000000000000000B /* Wired.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/Wired.swift; sourceTree = ""; }; + F0000000000000000000000C /* Rooted.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Sources/Rooted.swift"; sourceTree = SOURCE_ROOT; }; /* End PBXFileReference section */ /* Begin PBXGroup section */ - GGGG00000000000000000001 /* Sources */ = { + G0000000000000000000000C /* Cloud */ = { isa = PBXGroup; children = ( - AAAA00000000000000000002 /* Wired.swift */, + F0000000000000000000000A /* AAA.swift */, + ); + path = Cloud; + sourceTree = ""; + }; + G0000000000000000000000S /* Sources */ = { + isa = PBXGroup; + children = ( + G0000000000000000000000C /* Cloud */, + F0000000000000000000000B /* Wired.swift */, + F0000000000000000000000C /* Rooted.swift */, ); path = Sources; sourceTree = ""; @@ -36,33 +55,35 @@ /* End PBXGroup section */ /* Begin PBXNativeTarget section */ - NNNN00000000000000000001 /* cmux */ = { + N0000000000000000000000A /* cmux */ = { isa = PBXNativeTarget; buildPhases = ( - SSSS00000000000000000001 /* Sources */, + S0000000000000000000000A /* Sources */, ); name = cmux; }; - NNNN00000000000000000002 /* cmuxTests */ = { + N0000000000000000000000T /* cmuxTests */ = { isa = PBXNativeTarget; buildPhases = ( - SSSS00000000000000000002 /* Sources */, + S0000000000000000000000T /* Sources */, ); name = cmuxTests; }; /* End PBXNativeTarget section */ /* Begin PBXSourcesBuildPhase section */ - SSSS00000000000000000001 /* Sources */ = { + S0000000000000000000000A /* Sources */ = { isa = PBXSourcesBuildPhase; files = ( - AAAA00000000000000000001 /* Wired.swift in Sources */, + B0000000000000000000000A /* AAA.swift in Sources */, + B0000000000000000000000B /* Wired.swift in Sources */, + B0000000000000000000000C /* Rooted.swift in Sources */, ); }; - SSSS00000000000000000002 /* Sources */ = { + S0000000000000000000000T /* Sources */ = { isa = PBXSourcesBuildPhase; files = ( - TTTT00000000000000000001 /* Wired.swift in Sources */, + B0000000000000000000000T /* Wired.swift in Sources */, ); }; /* End PBXSourcesBuildPhase section */ @@ -71,37 +92,54 @@ """ +def group_of(text, rel): + project = wire_app_sources.parse(text) + ref = next(ref for ref, path in project.ref_paths.items() if path == rel) + return next(group.directory for group in project.groups.values() if ref in group.children) + + class WireAppSourcesTests(unittest.TestCase): - def test_finds_and_wires_an_unwired_file_into_the_app_target_only(self): + def test_resolves_paths_through_nested_groups_and_source_root(self): + project = wire_app_sources.parse(PROJECT) + self.assertEqual( + project.wired_paths, + {"Sources/Cloud/AAA.swift", "Sources/Sidebar/Wired.swift", "Sources/Rooted.swift"}, + ) + + def test_a_top_level_file_goes_in_the_sources_group_not_a_nested_one(self): + text = wire_app_sources.wire(PROJECT, "Sources/Top.swift") + self.assertEqual(group_of(text, "Sources/Top.swift"), "Sources") + self.assertIn("Sources/Top.swift", wire_app_sources.parse(text).wired_paths) + + def test_a_nested_group_file_gets_a_group_relative_path(self): + text = wire_app_sources.wire(PROJECT, "Sources/Cloud/New+Thing.swift") + self.assertEqual(group_of(text, "Sources/Cloud/New+Thing.swift"), "Sources/Cloud") + # Group-relative, and quoted because of the `+`. + self.assertIn('path = "New+Thing.swift";', text) + + def test_a_subdirectory_without_its_own_group_uses_a_prefixed_path(self): + text = wire_app_sources.wire(PROJECT, "Sources/Sidebar/Glyph.swift") + self.assertEqual(group_of(text, "Sources/Sidebar/Glyph.swift"), "Sources") + self.assertIn("path = Sidebar/Glyph.swift;", text) + + def test_wires_the_app_target_only(self): + text = wire_app_sources.wire(PROJECT, "Sources/Top.swift") + tests_phase = text[text.index("S0000000000000000000000T /* Sources */ = {"):] + self.assertNotIn("Top.swift", tests_phase) + self.assertEqual(text.count("/* Top.swift in Sources */"), 2) # build file + app phase + + def test_matching_is_by_path_not_basename(self): with tempfile.TemporaryDirectory() as root: root = Path(root) - (root / "Sources/Sidebar").mkdir(parents=True) - (root / "Sources/Sidebar/Wired.swift").write_text("") - (root / "Sources/Sidebar/Glyph+Resolve.swift").write_text("") - self.assertEqual( - wire_app_sources.unwired_sources(root, PROJECT), - ["Sources/Sidebar/Glyph+Resolve.swift"], - ) - - text = wire_app_sources.wire(PROJECT, "Sources/Sidebar/Glyph+Resolve.swift") - - self.assertIn("Glyph+Resolve.swift", wire_app_sources.wired_names(text)) - # `+` needs quoting in an OpenStep plist. - self.assertIn('path = "Sidebar/Glyph+Resolve.swift";', text) - self.assertEqual(text.count("/* Glyph+Resolve.swift in Sources */"), 2) # build file + app phase - self.assertEqual(text.count("/* Glyph+Resolve.swift */"), 3) # file ref, build file's fileRef, group child - tests_phase = text[text.index("SSSS00000000000000000002 /* Sources */ = {"):] - self.assertNotIn("Glyph+Resolve", tests_phase) - self.assertEqual(wire_app_sources.unwired_sources(root, text), []) - - def test_ids_are_stable_per_path(self): - once = wire_app_sources.wire(PROJECT, "Sources/Sidebar/New.swift") - twice = wire_app_sources.wire(PROJECT, "Sources/Sidebar/New.swift") - self.assertEqual(once, twice) - - def test_a_directory_with_no_wired_sibling_is_an_error(self): - with self.assertRaises(SystemExit): - wire_app_sources.wire(PROJECT, "Sources/Elsewhere/New.swift") + for rel in ["Sources/Sidebar/Wired.swift", "Sources/Other/Wired.swift", "Sources/Cloud/AAA.swift", "Sources/Rooted.swift"]: + (root / rel).parent.mkdir(parents=True, exist_ok=True) + (root / rel).write_text("") + self.assertEqual(wire_app_sources.unwired_sources(root, PROJECT), ["Sources/Other/Wired.swift"]) + + def test_wiring_is_idempotent_and_ids_are_stable(self): + once = wire_app_sources.wire(PROJECT, "Sources/Top.swift") + self.assertEqual(wire_app_sources.wire(once, "Sources/Top.swift"), once) + self.assertEqual(wire_app_sources.wire(PROJECT, "Sources/Top.swift"), once) if __name__ == "__main__": From 8bd843b0b4a7cf80e79116acfe6461db89faaa6d Mon Sep 17 00:00:00 2001 From: teamleaderleo Date: Sun, 27 Sep 2026 14:49:36 -0400 Subject: [PATCH 3/4] tools: wire-app-sources reuses a surviving reference and build file When only the app-phase line was lost, the file reference (and often its build file) still exist. Adding new ones duplicated the reference and could reuse its path-derived id, so normalizing failed and left a broken project. Now a surviving reference and an orphaned build file are reused, derived ids are re-salted if taken, and the project is normalized as a staged copy that replaces it only on success. Co-Authored-By: Claude Opus 5.5 --- scripts/wire-app-sources.py | 75 ++++++++++++++++++++++++++-------- tests/test_wire_app_sources.py | 16 ++++++++ 2 files changed, 73 insertions(+), 18 deletions(-) diff --git a/scripts/wire-app-sources.py b/scripts/wire-app-sources.py index bf411dd7558a..ba27875f6904 100755 --- a/scripts/wire-app-sources.py +++ b/scripts/wire-app-sources.py @@ -196,32 +196,57 @@ def holds_sibling(group: Group) -> bool: return max(candidates, key=lambda group: (holds_sibling(group), group.depth)) +def fresh_id(text: str, seed: str) -> str: + """A path-derived id, re-salted if the project already uses it.""" + candidate, salt = object_id(seed), 0 + while re.search(r"\b" + candidate + r"\b", text): + salt += 1 + candidate = object_id(f"{seed}#{salt}") + return candidate + + def wire(text: str, rel: str) -> str: + """Adds whatever `rel` is missing: a file reference in its group, a build + file, and membership in the app Sources phase. A surviving reference or + orphaned build file (often only the phase line was lost) is reused.""" project = parse(text) if rel in project.wired_paths: return text name = posixpath.basename(rel) - group = owning_group(project, rel) - group_path = posixpath.relpath(rel, group.directory) - ref_id = object_id("fileref:" + rel) - build_id = object_id("buildfile:" + rel) - - # Insert from the end of the file backwards so earlier offsets stay valid. - insertions = [ - (project.app_phase_span[1], f"\t\t\t\t{build_id} /* {name} in Sources */,\n", True), - (group.children_span[1], f"\t\t\t\t{ref_id} /* {name} */,\n", True), + insertions = [] + + ref_id = next((ref for ref, path in project.ref_paths.items() if path == rel), None) + if ref_id is None: + group = owning_group(project, rel) + ref_id = fresh_id(text, "fileref:" + rel) + insertions += [ + (group.children_span[1], f"\t\t\t\t{ref_id} /* {name} */,\n", True), + ( + text.index("/* Begin PBXFileReference section */\n") + len("/* Begin PBXFileReference section */\n"), + f'\t\t{ref_id} /* {name} */ = {{isa = PBXFileReference; lastKnownFileType = sourcecode.swift; ' + f'path = {quoted(posixpath.relpath(rel, group.directory))}; sourceTree = ""; }};\n', + False, + ), + ] + + in_some_phase = set(re.findall(r"^\t+([0-9A-Za-z]+) /\* [^\n]*? in Sources \*/,$", text, re.M)) + build_id = next( ( - text.index("/* Begin PBXFileReference section */\n") + len("/* Begin PBXFileReference section */\n"), - f'\t\t{ref_id} /* {name} */ = {{isa = PBXFileReference; lastKnownFileType = sourcecode.swift; ' - f'path = {quoted(group_path)}; sourceTree = ""; }};\n', - False, + match.group("id") + for match in BUILD_FILE.finditer(text) + if match.group("ref") == ref_id and match.group("id") not in in_some_phase ), - ( + None, + ) + if build_id is None: + build_id = fresh_id(text, "buildfile:" + rel) + insertions.append(( text.index("/* Begin PBXBuildFile section */\n") + len("/* Begin PBXBuildFile section */\n"), f"\t\t{build_id} /* {name} in Sources */ = {{isa = PBXBuildFile; fileRef = {ref_id} /* {name} */; }};\n", False, - ), - ] + )) + insertions.append((project.app_phase_span[1], f"\t\t\t\t{build_id} /* {name} in Sources */,\n", True)) + for offset, line, before_closing in sorted(insertions, key=lambda item: item[0], reverse=True): if before_closing: # The span ends right before the closing `\t\t\t);`; keep the @@ -255,9 +280,23 @@ def main(argv: list[str] | None = None) -> int: changed = True print(f"wired: {rel}", flush=True) if changed: - pbxproj.write_text(text) # The pre-commit hook and check-pbxproj.sh require normalized output. - subprocess.run([sys.executable, str(args.root / "scripts/normalize-pbxproj.py"), str(pbxproj)], check=True) + # Normalize a copy (it also validates object ids) and only then + # replace the project, so a failure never leaves a broken file. + staged = pbxproj.with_name("project.pbxproj.wiring") + staged.write_text(text) + result = subprocess.run( + [sys.executable, str(args.root / "scripts/normalize-pbxproj.py"), str(staged)], + capture_output=True, + text=True, + ) + if result.returncode != 0: + staged.unlink(missing_ok=True) + sys.stderr.write(result.stdout + result.stderr) + print("wire-app-sources: normalizing failed; project.pbxproj left unchanged", file=sys.stderr) + return 1 + staged.replace(pbxproj) + print(f"normalized: {pbxproj}", flush=True) return 0 diff --git a/tests/test_wire_app_sources.py b/tests/test_wire_app_sources.py index 234134620777..7cd1080a47af 100644 --- a/tests/test_wire_app_sources.py +++ b/tests/test_wire_app_sources.py @@ -136,6 +136,22 @@ def test_matching_is_by_path_not_basename(self): (root / rel).write_text("") self.assertEqual(wire_app_sources.unwired_sources(root, PROJECT), ["Sources/Other/Wired.swift"]) + def test_a_lost_phase_line_reuses_the_surviving_reference_and_build_file(self): + # Only the app-phase line is gone; the ref and the build file remain. + stripped = PROJECT.replace("\t\t\t\tB0000000000000000000000C /* Rooted.swift in Sources */,\n", "") + self.assertNotIn("Sources/Rooted.swift", wire_app_sources.parse(stripped).wired_paths) + text = wire_app_sources.wire(stripped, "Sources/Rooted.swift") + self.assertEqual(text.count("isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = \"Sources/Rooted.swift\""), 1) + self.assertEqual(text.count("= {isa = PBXBuildFile; fileRef = F0000000000000000000000C"), 1) + self.assertIn("\t\t\t\tB0000000000000000000000C /* Rooted.swift in Sources */,\n", text) + self.assertIn("Sources/Rooted.swift", wire_app_sources.parse(text).wired_paths) + + def test_a_derived_id_already_in_use_is_resalted(self): + taken = wire_app_sources.object_id("fileref:Sources/Top.swift") + busy = PROJECT.replace("F0000000000000000000000A", taken) + text = wire_app_sources.wire(busy, "Sources/Top.swift") + self.assertEqual(text.count(taken + " /* "), busy.count(taken + " /* ")) + def test_wiring_is_idempotent_and_ids_are_stable(self): once = wire_app_sources.wire(PROJECT, "Sources/Top.swift") self.assertEqual(wire_app_sources.wire(once, "Sources/Top.swift"), once) From dd3f2566a7cf15606595a61311b1cdd28656a595 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Sun, 27 Sep 2026 11:56:46 -0700 Subject: [PATCH 4/4] fix: harden ui-lab output and source wiring checks --- scripts/ui-lab/UILab.swift | 12 ++++++++++-- scripts/verify-local.py | 2 ++ scripts/wire-app-sources.py | 19 ++++++++++++++++++- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/scripts/ui-lab/UILab.swift b/scripts/ui-lab/UILab.swift index 65a48431feba..6c8af686422e 100644 --- a/scripts/ui-lab/UILab.swift +++ b/scripts/ui-lab/UILab.swift @@ -67,14 +67,22 @@ enum UILab { data = rep.representation(using: .png, properties: [:]) } let url = outputDirectory.appendingPathComponent(file) + guard let data else { + fail("could not encode \(url.path) as PNG") + } do { - try data?.write(to: url) + try data.write(to: url) print(url.path) } catch { - FileHandle.standardError.write("ui-lab: could not write \(url.path): \(error)\n".data(using: .utf8)!) + fail("could not write \(url.path): \(error)") } } + private static func fail(_ message: String) -> Never { + FileHandle.standardError.write("ui-lab: \(message)\n".data(using: .utf8)!) + exit(1) + } + @MainActor private static func layoutAll(_ view: NSView) { view.needsLayout = true diff --git a/scripts/verify-local.py b/scripts/verify-local.py index 2a9237398f12..ea966499c44f 100644 --- a/scripts/verify-local.py +++ b/scripts/verify-local.py @@ -55,6 +55,8 @@ "Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ConfigValidation/CmuxConfigSchema.generated.swift"), "test-wiring-sync": ("scripts/sync-test-wiring", "scripts/sync_test_wiring.py", "scripts/lint-pbxproj-test-wiring.sh", "scripts/normalize-pbxproj.py", "tests/fixtures/pbxproj-test-wiring/*"), + "wire-app-sources": ("scripts/wire-app-sources.py", "cmux.xcodeproj/project.pbxproj", "Sources/**/*.swift"), + "ui-lab": ("scripts/ui-lab/**", "tests/test_ui_lab.py"), "launch-policy": ( "scripts/claude-launch-environment-policy.json", "Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeSessionEnvironmentPolicy+Generated.swift", diff --git a/scripts/wire-app-sources.py b/scripts/wire-app-sources.py index ba27875f6904..493ab32e9e15 100755 --- a/scripts/wire-app-sources.py +++ b/scripts/wire-app-sources.py @@ -265,7 +265,24 @@ def main(argv: list[str] | None = None) -> int: pbxproj = args.root / PBXPROJ text = pbxproj.read_text() - targets = args.paths or unwired_sources(args.root, text) + if args.paths: + targets = [] + sources_root = (args.root / "Sources").resolve() + for raw in args.paths: + path = (args.root / raw).resolve() + try: + rel = path.relative_to(args.root).as_posix() + path.relative_to(sources_root) + except ValueError: + parser.error(f"path must resolve under Sources/: {raw}") + if path.suffix != ".swift" or not path.is_file(): + parser.error(f"path must be an existing .swift file: {raw}") + targets.append(rel) + if args.check: + wired = parse(text).wired_paths + targets = [rel for rel in targets if rel not in wired] + else: + targets = unwired_sources(args.root, text) if args.check: for rel in targets: print(f"unwired: {rel}", flush=True)