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..6c8af686422e --- /dev/null +++ b/scripts/ui-lab/UILab.swift @@ -0,0 +1,104 @@ +import AppKit +import SwiftUI + +/// Rendering support for ui-lab harnesses (see scripts/ui-lab/ui-lab.py). +/// 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 + 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-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(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") + } + } + } + + @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) + guard let data else { + fail("could not encode \(url.path) as PNG") + } + do { + try data.write(to: url) + print(url.path) + } catch { + 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 + 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..e50a90b500c8 --- /dev/null +++ b/scripts/ui-lab/harnesses/gpu-spinner.swift @@ -0,0 +1,35 @@ +// 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, 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 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 + } +} 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..b9a6a4e83f8d --- /dev/null +++ b/scripts/ui-lab/ui-lab.py @@ -0,0 +1,189 @@ +#!/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/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 +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/gpu-spinner.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*$") +# `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")) + + +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 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(Path(__file__).read_bytes()) # flags and source rewriting + for path in files: + digest.update(str(path).encode()) + digest.update(path.read_bytes()) + 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: + compiled = [] + for index, path in enumerate(files): + text = path.read_text() + if path != harness: + # One module: package imports resolve to shims or nothing. + 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(partial), *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") + 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: + 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..ea966499c44f 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"]), @@ -53,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 new file mode 100755 index 000000000000..493ab32e9e15 --- /dev/null +++ b/scripts/wire-app-sources.py @@ -0,0 +1,321 @@ +#!/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) and +normalizes the project. + + 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. + +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 + ) + 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 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]: + 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 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 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) + 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( + ( + 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 + # 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: + 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() + 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) + 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 rel in parse(text).wired_paths: + print(f"already wired: {rel}", flush=True) + continue + text = wire(text, rel) + changed = True + print(f"wired: {rel}", flush=True) + if changed: + # The pre-commit hook and check-pbxproj.sh require normalized output. + # 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 + + +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..6f6d836b9599 --- /dev/null +++ b/skills/cmux-testing/references/ui-lab.md @@ -0,0 +1,61 @@ +# 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/GPUSpinnerStyle.swift +// ui-lab: source Sources/Sidebar/GPUSpinnerNSView.swift +// ui-lab: shim SidebarAppearanceColorResolver + +import AppKit + +UILab.main { + 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 + 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, 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..3b7c60265789 --- /dev/null +++ b/tests/test_ui_lab.py @@ -0,0 +1,64 @@ +#!/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))) + + +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 new file mode 100644 index 000000000000..7cd1080a47af --- /dev/null +++ b/tests/test_wire_app_sources.py @@ -0,0 +1,162 @@ +#!/usr/bin/env python3 +"""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 + +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 */ + 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 */ + 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 */ + G0000000000000000000000C /* Cloud */ = { + isa = PBXGroup; + children = ( + F0000000000000000000000A /* AAA.swift */, + ); + path = Cloud; + sourceTree = ""; + }; + G0000000000000000000000S /* Sources */ = { + isa = PBXGroup; + children = ( + G0000000000000000000000C /* Cloud */, + F0000000000000000000000B /* Wired.swift */, + F0000000000000000000000C /* Rooted.swift */, + ); + path = Sources; + sourceTree = ""; + }; +/* End PBXGroup section */ + +/* Begin PBXNativeTarget section */ + N0000000000000000000000A /* cmux */ = { + isa = PBXNativeTarget; + buildPhases = ( + S0000000000000000000000A /* Sources */, + ); + name = cmux; + }; + N0000000000000000000000T /* cmuxTests */ = { + isa = PBXNativeTarget; + buildPhases = ( + S0000000000000000000000T /* Sources */, + ); + name = cmuxTests; + }; +/* End PBXNativeTarget section */ + +/* Begin PBXSourcesBuildPhase section */ + S0000000000000000000000A /* Sources */ = { + isa = PBXSourcesBuildPhase; + files = ( + B0000000000000000000000A /* AAA.swift in Sources */, + B0000000000000000000000B /* Wired.swift in Sources */, + B0000000000000000000000C /* Rooted.swift in Sources */, + ); + }; + S0000000000000000000000T /* Sources */ = { + isa = PBXSourcesBuildPhase; + files = ( + B0000000000000000000000T /* Wired.swift in Sources */, + ); + }; +/* End PBXSourcesBuildPhase section */ + }; +} +""" + + +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_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) + 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_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) + self.assertEqual(wire_app_sources.wire(PROJECT, "Sources/Top.swift"), once) + + +if __name__ == "__main__": + unittest.main()