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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions scripts/cmux-next/check-scrollbars.sh
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,21 @@ def scan(rel, path, rules, strip):
return out

# A SwiftUI ScrollView takes the shared thin scroller on its content, and a List on its rows
# (`.thinScrollers()`); a file needs at least one per ScrollView or List.
# (`.thinScrollers()`); a file needs at least one per ScrollView or List. Only a file that imports SwiftUI
# can name SwiftUI's (no CmuxNext module re-exports it), so `List(expectedMachineID:)` in a Foundation-only
# file (AgentCLIIntentLedger's own List request) is not one.
SWIFTUI_SCROLL = re.compile(r"(?<![A-Za-z])(?:ScrollView|List)\s*[({]")
SWIFTUI_IMPORT = re.compile(r"^\s*(?:@\w+(?:\([^)]*\))?\s+)*(?:(?:public|package|internal|fileprivate|private)\s+)?import\s+(?:(?:struct|class|enum|protocol|typealias|func|var|let)\s+)?SwiftUI\b",
re.M)
STRING = re.compile(r'"(?:[^"\\\n]|\\.)*"')

def swiftui_scrolls(rel, path):
if rel in ALLOWED:
return []
lines = [STRING.sub('""', line).split("//", 1)[0] for line in open(path, encoding="utf-8", errors="replace").read().splitlines()]
text = open(path, encoding="utf-8", errors="replace").read()
if not SWIFTUI_IMPORT.search(text):
return []
lines = [STRING.sub('""', line).split("//", 1)[0] for line in text.splitlines()]
scrolls = [number for number, code in enumerate(lines, 1) if SWIFTUI_SCROLL.search(code)]
if len(scrolls) <= sum(code.count(".thinScrollers()") for code in lines):
return []
Expand Down
4 changes: 2 additions & 2 deletions scripts/cmux-next/crash-safety-baseline.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"cmux-remote": 4,
"cmux-terminal-client": 30,
"cmux-tui": 429,
"cmux-tui-core": 1151,
"cmux-tui-core": 1150,
"cmux-tui-image-paste": 22,
"cmux-tui-platform": 17,
"cmux-tui-remote-access": 5,
Expand Down Expand Up @@ -137,7 +137,7 @@
"swift": {
"dynamic_dispatch": {
"CmuxNextAgentPane": 2,
"CmuxNextBrowser": 9,
"CmuxNextBrowser": 11,
"CmuxNextBrowserAutomation": 5,
"CmuxNextDesign": 1,
"CmuxNextPages": 1,
Expand Down
17 changes: 12 additions & 5 deletions scripts/cmux-next/tests/check-scrollbars.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ SWIFT=Packages/macOS/CmuxNext/Sources/Fixture/Fixture.swift
CSS=webviews/src/fixture/styles.css
TSX=webviews/src/fixture/List.tsx
PIERRE=webviews/src/agent-session/acpmux/conversation/EditDiffHunks.tsx
# SwiftUI scroll views are judged only in a file that imports SwiftUI.
UI=$'import SwiftUI\n'
pass() { check "$1" "$3" || { echo "FAIL: must pass: $2" >&2; exit 1; }; }
fail() { if check "$1" "$3"; then echo "FAIL: must fail: $2" >&2; exit 1; fi; }

Expand All @@ -30,12 +32,17 @@ fail "$SWIFT" "always-visible indicators" ' .scrollIndicators(.visible)'
fail "$SWIFT" "an empty scroller track" ' scroll.autohidesScrollers = false'
pass "$SWIFT" "autohiding scrollers" ' scroll.autohidesScrollers = true'
pass "$SWIFT" "following the system" ' SystemScrollers.follow(scroll)'
fail "$SWIFT" "a SwiftUI ScrollView without the thin scroller" ' ScrollView { rows }'
pass "$SWIFT" "a SwiftUI ScrollView with the thin scroller" ' ScrollView { rows.thinScrollers() }'
fail "$SWIFT" "a SwiftUI ScrollView without the thin scroller" "$UI"' ScrollView { rows }'
pass "$SWIFT" "a SwiftUI ScrollView with the thin scroller" "$UI"' ScrollView { rows.thinScrollers() }'
pass "$SWIFT" "an AppKit scroll view" ' let scroll = NSScrollView()'
fail "$SWIFT" "a SwiftUI List without the thin scroller" ' List(selection: $selection) { rows }'
pass "$SWIFT" "a SwiftUI List with the thin scroller" ' List(selection: $selection) { rows.thinScrollers() }'
pass "$SWIFT" "List in a string" ' case .list: "List (dense rows)"'
fail "$SWIFT" "a SwiftUI List without the thin scroller" "$UI"' List(selection: $selection) { rows }'
fail "$SWIFT" "a SwiftUI List, attribute import" $'@preconcurrency import SwiftUI\n List(selection: $selection) { rows }'
fail "$SWIFT" "a SwiftUI ScrollView, access-level import" $'public import SwiftUI\n ScrollView { rows }'
fail "$SWIFT" "a SwiftUI List, @_spi import" $'@_spi(Private) import SwiftUI\n List(selection: $selection) { rows }'
fail "$SWIFT" "a SwiftUI List, scoped import" $'import struct SwiftUI.List\n List(selection: $selection) { rows }'
pass "$SWIFT" "a List( initializer in a file without SwiftUI" $'import Foundation\n try await send(List(expectedMachineID: machine), timeout: .seconds(10))'
pass "$SWIFT" "a SwiftUI List with the thin scroller" "$UI"' List(selection: $selection) { rows.thinScrollers() }'
pass "$SWIFT" "List in a string" "$UI"' case .list: "List (dense rows)"'
pass "$SWIFT" "a comment" ' // never set scrollerStyle = .legacy here'
fail "$CSS" "overflow: scroll" '.list { overflow-y: scroll; }'
fail "$CSS" "a custom WebKit scrollbar" '.list::-webkit-scrollbar { width: 6px; }'
Expand Down
19 changes: 16 additions & 3 deletions tests/test_cmux_next_prepush_gate.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,11 @@ def _load_git_fixture_env():
SOURCE = Path(__file__).resolve().parents[1]
INSTALLER = "scripts/install-git-hooks.sh"
GATE = "scripts/cmux-next/prepush-gate.py"
# The fixture is a developer's own push: GITHUB_ACTIONS (set on CI) would turn the guardrails off in the hook
# (prepush-gate.py runs them locally only), and the tests that expect a guardrails line would fail only on CI.
AGENT_VARS = ("CMUX_AGENT", "CMUX_REQUIRE_FORMATTERS", "CI", "CLAUDECODE", "CODEX_CI", "CODEX_THREAD_ID",
"CODEX_SANDBOX", "CMUX_WEB_LINT_SKIP_REASON", "CMUX_NEXT_CHECKS_SKIP_REASON")
"CODEX_SANDBOX", "CMUX_WEB_LINT_SKIP_REASON", "CMUX_NEXT_CHECKS_SKIP_REASON",
"CMUX_GUARDRAILS_SKIP_REASON", "GITHUB_ACTIONS")
PKG = "Packages/macOS/CmuxNext"
# The pinned tools are downloaded once per machine; the fixture HOME must not hide that cache.
if os.uname().sysname == "Darwin":
Expand Down Expand Up @@ -73,10 +76,20 @@ def _load_git_fixture_env():
init(owner: Owner) { self.owner = owner }
}
"""
# A try! and nothing else: no blocking read for the guardrails (main-blocking) to refuse on their own.
NEW_WITH_TRY_ONLY = """import Foundation
struct Fresh {
func pattern() -> NSRegularExpression {
try! NSRegularExpression(pattern: "a+")
}
}
"""
# Clean for the scanners and the guardrails: the read goes through the checked seam (CmuxNextCompat/Threading)
# off the main thread.
NEW_CLEAN = """import Foundation
struct Fresh {
func load(_ url: URL) async throws -> Data {
try await Task.detached(priority: .utility) { try Data(contentsOf: url) }.value
try await Task.detached(priority: .utility) { try Data(blockingContentsOf: url) }.value
}
}
"""
Expand Down Expand Up @@ -748,7 +761,7 @@ def test_push_from_a_linked_worktree_leaves_the_repository_alone(self):
self.assertEqual(self.git("diff", "--cached", "--name-only").stdout, "")

def test_skip_reason_lets_the_push_through_and_says_so(self):
self.write(f"{PKG}/Sources/CmuxNextKit/Fresh.swift", NEW_WITH_TRY)
self.write(f"{PKG}/Sources/CmuxNextKit/Fresh.swift", NEW_WITH_TRY_ONLY)
self.commit("adds a try!")

result = self.push(CMUX_NEXT_CHECKS_SKIP_REASON="fixture: the try! is in a file this push does not touch")
Expand Down
79 changes: 75 additions & 4 deletions tests/test_cmux_next_route.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,64 @@
]


def call_arguments(text: str, index: int) -> str:
"""The top-level arguments of the call open at INDEX (just inside its parenthesis), up to the parenthesis
that closes it. String literals ("...", \"\"\"...\"\"\", raw #"..."#) are kept whole at the top level;
comments (//, nested /* */) and everything inside a nested call, array or closure become spaces, so
`path:` matches only the target's own argument."""
out: list[str] = []
depth = 0
closers = {"(": ")", "[": "]", "{": "}"}
while index < len(text):
if text.startswith("//", index):
newline = text.find("\n", index)
index = len(text) if newline < 0 else newline
continue
if text.startswith("/*", index):
nesting, index = 1, index + 2
while index < len(text) and nesting:
if text.startswith("/*", index):
nesting, index = nesting + 1, index + 2
elif text.startswith("*/", index):
nesting, index = nesting - 1, index + 2
else:
index += 1
out.append(" ")
continue
literal = re.match(r'(#*)("""|")', text[index:])
if literal:
hashes, quote = literal.groups()
close = quote + hashes
cursor = index + len(literal.group(0))
while cursor < len(text) and not text.startswith(close, cursor):
cursor += 2 if (text[cursor] == "\\" and not hashes) else 1
cursor = min(len(text), cursor + len(close))
out.append(text[index:cursor] if depth == 0 else " ")
index = cursor
continue
char = text[index]
if char in closers:
depth += 1
elif char in ")]}":
if depth == 0:
return "".join(out)
depth -= 1
elif depth == 0:
out.append(char)
index += 1
return "".join(out)


def manifest_target_paths() -> set[str]:
"""The path of each target Package.swift declares: its own `path:` when it sets one
(CmuxNextTestExitTrace lives in Tests/Support), else Tests/<name> for a test target and
Sources/<name> otherwise."""
manifest = (ROOT / PACKAGE / "Package.swift").read_text(encoding="utf-8")
starts = list(re.finditer(r'\.(target|testTarget|executableTarget)\(\s*name:\s*"([^"]+)"', manifest))
paths = set()
for index, start in enumerate(starts):
end = starts[index + 1].start() if index + 1 < len(starts) else len(manifest)
own = re.search(r'\bpath:\s*"([^"]+)"', manifest[start.end():end])
for start in re.finditer(r'\.(target|testTarget|executableTarget)\(\s*name:\s*"([^"]+)"', manifest):
# Only the target's own top-level `path:` argument: the last target in the list must not read one further
# down (the package dependencies' `.package(path:)` follow it since #24978), nor one in a nested call.
own = re.search(r'\bpath:\s*"([^"]+)"', call_arguments(manifest, start.end()))
kind, name = start.groups()
default = f"{'Tests' if kind == 'testTarget' else 'Sources'}/{name}"
paths.add(f"{PACKAGE}/{own.group(1) if own else default}")
Expand Down Expand Up @@ -125,6 +173,29 @@ def test_dependencies_name_known_targets_and_packages(self):
self.assertLessEqual(set(target["packages"]), set(GRAPH["packages"]))


class ManifestReader(unittest.TestCase):
def path_of(self, manifest: str) -> str | None:
start = re.search(r'\.target\(\s*name:\s*"T"', manifest)
own = re.search(r'\bpath:\s*"([^"]+)"', call_arguments(manifest, start.end()))
return own.group(1) if own else None

def test_a_path_after_the_call_is_not_the_target_s(self):
self.assertIsNone(self.path_of('.target(name: "T", dependencies: ["A"])\n] + [x]\n.package(path: "../Other")'))

def test_the_target_s_own_path_is_read(self):
self.assertEqual(self.path_of('.target(name: "T", path: "Tests/Support/T")'), "Tests/Support/T")

def test_a_nested_path_argument_is_not_the_target_s(self):
self.assertEqual(self.path_of('.target(name: "T", resources: [helper(path: "Nested")], path: "Actual")'), "Actual")
self.assertIsNone(self.path_of('.target(name: "T", resources: [.copy(path: "Nested")])'))

def test_parentheses_in_strings_and_comments_do_not_end_the_call(self):
manifest = ('.target(name: "T", /* ) /* nested ) */ ) */ swiftSettings: [.define("A)")],\n'
' // a comment )\n exclude: [#"raw ) \\"#, """\n ) multi\n """], path: "Real")\n'
'.package(path: "../Other")')
self.assertEqual(self.path_of(manifest), "Real")


class GraphGenerator(unittest.TestCase):
def test_a_literal_too_long_for_a_path_is_not_a_read(self):
"""macOS stat fails with ENAMETOOLONG on a base64 certificate in a test (run 37411671734)."""
Expand Down
Loading