From 02eb969347c3b71c2eea72434a54f8abafbc1396 Mon Sep 17 00:00:00 2001 From: Ali Al Dallal Date: Mon, 17 Aug 2026 23:55:18 -0400 Subject: [PATCH 1/2] feat: real-webview bridge smoke -- WKWebView parity check registry (goal 0097, re-scoped) Playwright's webkit project was researched and rejected as the parity mechanism (patched WebKit-main, never attaches to an app's own embedded webview; no macOS WebDriver exists for third-party WKWebView). The honest mechanism is driving the real desktop window over Wails3's own -tags mcp control bridge: internal/webviewbridgesmoke builds and launches the real app in isolated data, refuses to run alongside an already-live task dev or installed Mill.app, and runs a named check registry (window sanity, Atlas board render, note-card flip, and the two burned selection/border-color rendering classes) against it. scripts/webview-bridge-smoke.sh is the entrypoint; ci.yml gets a non-required macos-latest job until it has a CI track record. testing.md records the new layer and the researched-and-rejected webkit-project premise. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01FW5GkkAG8du7tNdYLk2zSd --- .claude/rules/testing.md | 20 ++ .github/workflows/ci.yml | 37 +++ internal/webviewbridgesmoke/checks.go | 284 ++++++++++++++++++++++++ internal/webviewbridgesmoke/guard.go | 46 ++++ internal/webviewbridgesmoke/main.go | 234 +++++++++++++++++++ internal/webviewbridgesmoke/mcp.go | 135 +++++++++++ internal/webviewbridgesmoke/mcp_test.go | 125 +++++++++++ internal/webviewbridgesmoke/procio.go | 40 ++++ scripts/webview-bridge-smoke.sh | 14 ++ 9 files changed, 935 insertions(+) create mode 100644 internal/webviewbridgesmoke/checks.go create mode 100644 internal/webviewbridgesmoke/guard.go create mode 100644 internal/webviewbridgesmoke/main.go create mode 100644 internal/webviewbridgesmoke/mcp.go create mode 100644 internal/webviewbridgesmoke/mcp_test.go create mode 100644 internal/webviewbridgesmoke/procio.go create mode 100755 scripts/webview-bridge-smoke.sh diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index 6f1a8ab7..3b9d3f82 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -126,6 +126,26 @@ layer per capability," never "a seed per thing": express (hover, drag, truncation, pointer-events regressions). - **Smoke/liveness** — app-level boot + advisory external liveness (the seeded integrations' endpoints), non-blocking. +- **Real-webview engine parity** (`scripts/webview-bridge-smoke.sh`, + `internal/webviewbridgesmoke`, goal 0097) — a scripted, named check + registry driven over Wails3's own `-tags mcp` control bridge against + the REAL desktop window, catching engine-behavior divergence + (focus/selection/rendering classes) between macOS's real WKWebView + and every other layer above, which all run Chromium. Exists because + a real WebKit-only defect (a selection ring / focus-halo difference) + shipped invisible to the whole Chromium-based suite. **Researched + and rejected**: a Playwright `webkit`-project was the original + premise, but primary sources showed Playwright's `webkit` build is + patched WebKit-main that never attaches to an app's own embedded + webview, and no macOS WebDriver exists for a third-party WKWebView + at all — the OSS convergence for real parity is driving the actual + embedded webview via an app bridge, which is what this layer does. + Playwright's `webkit` browser stays installed as a local debugging + probe only, never CI-badged as parity. Non-required CI job + (`webview-bridge-smoke` in `ci.yml`) until it has a track record; + local pre-release gate in the meantime. Revisit trigger: grow the + check registry when a WebKit-only bug escapes it, same discipline as + the manual-only registry below. - **Manual-only registry** — OS-bound checks (hotkey delivery, real clipboard, tray) listed explicitly with reasons, never silently absent (see goal 0010's enforcement). Non-seed instance: the diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9e5dd2b6..ad69672d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -374,6 +374,43 @@ jobs: path: frontend/playwright-report retention-days: 7 + # docs/goals/0097 (re-scoped): the real-webview parity smoke + # (scripts/webview-bridge-smoke.sh, internal/webviewbridgesmoke) drives + # a genuine -tags mcp desktop build over Wails3's own MCP bridge -- + # the only mechanism that exercises macOS's real WKWebView engine, + # since Playwright's "webkit" build never attaches to it (researched- + # and-rejected premise, .claude/rules/testing.md). GitHub-hosted + # macos-latest runners DO support a real windowed app with no xvfb- + # style workaround needed (confirmed via a working precedent: Simon + # Willison's Datasette Desktop runs real-window Playwright Electron + # tests on macos-latest with zero special display config -- the + # self-hosted-runner GUI-session caveats found in research are a + # self-hosted concern, GitHub's own hosted image already provides a + # console session). Non-required (`continue-on-error`, absent from + # ci-gate's `needs:` below) until it's proven stable in real CI runs -- + # promote once it has a track record, per the goal's own CI- + # feasibility instruction not to iterate on live CI cycles up front. + webview-bridge-smoke: + needs: [changes, frontend] + if: success() && needs.changes.outputs.code == 'true' + runs-on: macos-latest + timeout-minutes: 15 + continue-on-error: true + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version: '1.25' + cache: true + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: '22' + cache: 'npm' + cache-dependency-path: frontend/package-lock.json + - run: npm ci + working-directory: frontend + - run: ./scripts/webview-bridge-smoke.sh + govulncheck: # macos-latest, not ubuntu-latest: govulncheck-action has no way to pass # -tags, and the default (desktop) build tags only compile cleanly diff --git a/internal/webviewbridgesmoke/checks.go b/internal/webviewbridgesmoke/checks.go new file mode 100644 index 00000000..3b8956e6 --- /dev/null +++ b/internal/webviewbridgesmoke/checks.go @@ -0,0 +1,284 @@ +package main + +import ( + "fmt" + "time" +) + +// check is one named, self-contained verification driven entirely over +// the real MCP bridge against the real desktop window -- never a +// Playwright/Chromium stand-in. Each entry's reason states specifically +// what a Chromium-only suite cannot see that this check can, per the +// manual-only-registry discipline (.claude/rules/testing.md) this +// mirrors: explicit membership with reasons, additions deliberate. +type check struct { + name string + reason string + run func(c *mcpClient) (string, error) +} + +var registry = []check{ + { + name: "isolated-data-badge", + reason: "confirms the real desktop window is serving throwaway MILL_SETTINGS_PATH/MILL_EXECUTION_DB_PATH, never real user data, before any further check touches it (mirrors frontend/e2e/fixtures/server.ts's own per-server guard).", + run: checkIsolatedDataBadge, + }, + { + name: "app-info-window-sane", + reason: "the real Wails process reports exactly one live window on darwin -- proves a genuine desktop process booted, not just that a binary exists.", + run: checkAppInfo, + }, + { + name: "atlas-board-renders", + reason: "the real WKWebView actually mounts the SPA and renders the Atlas board -- a page that never finishes loading in the real engine would pass a Chromium-only suite untouched.", + run: checkAtlasBoardRenders, + }, + { + name: "note-card-flip-interaction", + reason: "a real WKWebView pointer event round-trips into React state (js_eval per the coordinator's own brief for this check) -- the flip is Atlas's most basic interaction, gates everything after it.", + run: checkNoteCardFlip, + }, + { + name: "note-card-selection-ring", + reason: "the burned class of bug this goal exists for: the selected note-card's box-shadow ring (AtlasNoteCardNode.module.css's :global(.react-flow__node.selected) rule) rendered none in real WebKit while Chromium rendered it fine.", + run: checkNoteCardSelectionRing, + }, + { + name: "sticky-border-color-flip", + reason: "the second burned class: a selected sticky note's border-color flip to the accent token (AtlasStickyNode.module.css) -- a real Note, created live via AtlasService.CreateNote so the check exercises the same call_bound_method path an agent driving Mill would use.", + run: checkStickyBorderColorFlip, + }, +} + +func checkIsolatedDataBadge(c *mcpClient) (string, error) { + var found bool + if err := c.callJSON("js_eval", map[string]any{ + "js": `return !!document.querySelector('[data-testid="isolated-data-badge"]');`, + }, &found); err != nil { + return "", err + } + if !found { + return "", fmt.Errorf("isolated-data-badge not present -- refusing to trust this window's data isolation") + } + return "badge visible", nil +} + +func checkAppInfo(c *mcpClient) (string, error) { + var info struct { + OS string `json:"os"` + Windows []struct { + Name string `json:"name"` + } `json:"windows"` + } + if err := c.callJSON("app_info", map[string]any{}, &info); err != nil { + return "", err + } + if info.OS != "darwin" { + return "", fmt.Errorf("expected os darwin, got %q", info.OS) + } + if len(info.Windows) != 1 { + return "", fmt.Errorf("expected exactly 1 window, got %d", len(info.Windows)) + } + return fmt.Sprintf("os=%s window=%q", info.OS, info.Windows[0].Name), nil +} + +// pollJSEval retries a boolean-returning js_eval snippet until it's +// true or the deadline passes -- the real webview's page load and +// React's own render pass are both async relative to the MCP HTTP +// server becoming reachable. +func pollJSEval(c *mcpClient, js string, timeout time.Duration) error { + deadline := time.Now().Add(timeout) + var lastErr error + for time.Now().Before(deadline) { + var ok bool + if err := c.callJSON("js_eval", map[string]any{"js": js}, &ok); err != nil { + lastErr = err + } else if ok { + return nil + } + time.Sleep(300 * time.Millisecond) + } + if lastErr != nil { + return fmt.Errorf("timed out polling: %w", lastErr) + } + return fmt.Errorf("timed out polling (condition never became true)") +} + +func checkAtlasBoardRenders(c *mcpClient) (string, error) { + var nav struct { + Clicked bool `json:"clicked"` + Tag string `json:"tag"` + } + if err := c.callJSON("js_eval", map[string]any{ + "js": `const candidates = [...document.querySelectorAll('a,button,[role="link"],[role="button"]')]; + const link = candidates.find(el => (el.textContent||'').trim() === 'Atlas' || el.getAttribute('aria-label') === 'Atlas'); + if (!link) throw new Error('Atlas nav entry not found among ' + candidates.length + ' candidates'); + link.click(); + return { clicked: true, tag: link.tagName };`, + }, &nav); err != nil { + return "", err + } + if err := pollJSEval(c, `return !!document.querySelector('[data-testid="atlas-board"]');`, 10*time.Second); err != nil { + return "", fmt.Errorf("atlas board never rendered after clicking nav (%s): %w", nav.Tag, err) + } + return "atlas-board rendered after nav click", nil +} + +func checkNoteCardFlip(c *mcpClient) (string, error) { + var result struct { + Before string `json:"before"` + After string `json:"after"` + Label string `json:"label"` + } + if err := c.callJSON("js_eval", map[string]any{ + "js": `const card = document.querySelector('[data-testid="atlas-note-card"]'); + if (!card) throw new Error('no seeded note card found to click'); + const before = card.dataset.flipped; + const label = card.getAttribute('aria-label') || ''; + card.click(); + await new Promise(r => setTimeout(r, 200)); + const after = document.querySelector('[data-testid="atlas-note-card"]').dataset.flipped; + return { before, after, label };`, + }, &result); err != nil { + return "", err + } + if result.Before != "false" || result.After != "true" { + return "", fmt.Errorf("expected flipped false->true, got %s->%s (card: %s)", result.Before, result.After, result.Label) + } + return fmt.Sprintf("flipped false->true (%s)", result.Label), nil +} + +type ringSnapshot struct { + BoxShadow string `json:"boxShadow"` + Selected bool `json:"selected"` +} + +func readRing(c *mcpClient, selector string) (ringSnapshot, error) { + var snap ringSnapshot + err := c.callJSON("js_eval", map[string]any{ + "js": fmt.Sprintf(`const el = document.querySelectorAll(%q)[0]; + if (!el) throw new Error('element not found: %s'); + const wrapper = el.closest('.react-flow__node'); + const style = getComputedStyle(el); + return { boxShadow: style.boxShadow, selected: wrapper ? wrapper.classList.contains('selected') : false };`, selector, selector), + }, &snap) + return snap, err +} + +func checkNoteCardSelectionRing(c *mcpClient) (string, error) { + selector := `[data-testid="atlas-note-card"]` + before, err := readRing(c, selector) + if err != nil { + return "", err + } + if before.Selected { + return "", fmt.Errorf("card already selected before the check ran -- board state isn't clean") + } + // The real mouse_click tool dispatches the full pointer/mouse event + // sequence (per the tool's own description) -- unlike a js_eval + // .click(), this is what React Flow's own selection handling + // (goal 0092's shift-click toggle, tied to real pointerdown) needs. + if _, err := c.call("mouse_click", map[string]any{ + "selector": selector, + "modifiers": []string{"shift"}, + }); err != nil { + return "", err + } + after, err := readRing(c, selector) + if err != nil { + return "", err + } + if !after.Selected { + return "", fmt.Errorf("shift-click did not select the card") + } + if before.BoxShadow != "none" { + return "", fmt.Errorf("expected no ring before selection, got box-shadow %q", before.BoxShadow) + } + if after.BoxShadow == "none" { + return "", fmt.Errorf("selection ring did not render: box-shadow is none after shift-click select") + } + return fmt.Sprintf("box-shadow none -> %s on select", after.BoxShadow), nil +} + +// atlasCard mirrors internal/domain/atlas.Card's JSON shape closely +// enough to read Title/ID/ParentID off AtlasService.Cards() -- the +// note-nesting parent this check needs is whatever card the seeded +// "Getting started" card itself lives under, not a hardcoded ID. +type atlasCard struct { + ID string `json:"ID"` + Title string `json:"Title"` + ParentID string `json:"ParentID"` +} + +func checkStickyBorderColorFlip(c *mcpClient) (string, error) { + var cards []atlasCard + if err := c.callJSON("call_bound_method", map[string]any{ + "name": "github.com/alicoding/mill/internal/services/atlassvc.AtlasService.Cards", + "args": []any{}, + }, &cards); err != nil { + return "", err + } + var parentID string + for _, card := range cards { + if card.Title == "Getting started" { + parentID = card.ParentID + break + } + } + if parentID == "" { + return "", fmt.Errorf("seeded card \"Getting started\" not found -- can't place the check's sticky note at the right board level") + } + + var note struct { + ID string `json:"ID"` + } + if err := c.callJSON("call_bound_method", map[string]any{ + "name": "github.com/alicoding/mill/internal/services/atlassvc.AtlasService.CreateNote", + "args": []any{"webview-bridge-smoke check", map[string]any{"X": 340, "Y": 340}, parentID}, + }, ¬e); err != nil { + return "", err + } + + selector := `[data-testid="atlas-sticky-note"]` + if err := pollJSEval(c, fmt.Sprintf(`return !!document.querySelector('%s');`, selector), 10*time.Second); err != nil { + return "", fmt.Errorf("sticky note never rendered after AtlasService.CreateNote: %w", err) + } + + before, err := readStickyStyle(c, selector) + if err != nil { + return "", err + } + if _, err := c.call("mouse_click", map[string]any{ + "selector": selector, + "modifiers": []string{"shift"}, + }); err != nil { + return "", err + } + after, err := readStickyStyle(c, selector) + if err != nil { + return "", err + } + if before.BorderColor == after.BorderColor { + return "", fmt.Errorf("border-color did not flip on selection: stayed %q", before.BorderColor) + } + if after.BoxShadow == "none" { + return "", fmt.Errorf("sticky selection ring did not render: box-shadow is none after shift-click select") + } + return fmt.Sprintf("border-color %s -> %s, box-shadow none -> %s", before.BorderColor, after.BorderColor, after.BoxShadow), nil +} + +type stickySnapshot struct { + BorderColor string `json:"borderColor"` + BoxShadow string `json:"boxShadow"` +} + +func readStickyStyle(c *mcpClient, selector string) (stickySnapshot, error) { + var snap stickySnapshot + err := c.callJSON("js_eval", map[string]any{ + "js": fmt.Sprintf(`const el = document.querySelector(%q); + if (!el) throw new Error('element not found: %s'); + const style = getComputedStyle(el); + return { borderColor: style.borderColor, boxShadow: style.boxShadow };`, selector, selector), + }, &snap) + return snap, err +} diff --git a/internal/webviewbridgesmoke/guard.go b/internal/webviewbridgesmoke/guard.go new file mode 100644 index 00000000..1782be40 --- /dev/null +++ b/internal/webviewbridgesmoke/guard.go @@ -0,0 +1,46 @@ +package main + +import ( + "fmt" + "os/exec" + "strconv" + "strings" +) + +// guardNoOtherInstance refuses to launch a second real desktop window +// alongside one that's already running -- mirrors internal/devguard's +// own approach (a `ps` scan for this repo's own dev-loop marker) plus +// two checks devguard doesn't need: the INSTALLED app (a real user +// session, not a dev loop) and the MCP port itself, since this script +// launches a genuine second Cocoa window rather than serving over +// server-mode HTTP. Refuses only -- never kills a conflicting process. +func guardNoOtherInstance() error { + psOutput, err := exec.Command("ps", "-axwwo", "pid=,command=").Output() //nolint:gosec,noctx // static args, one-shot CLI tool + if err != nil { + return fmt.Errorf("couldn't list processes to check for a conflicting Mill instance: %w", err) + } + for _, line := range strings.Split(string(psOutput), "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + fields := strings.SplitN(line, " ", 2) + if len(fields) != 2 { + continue + } + if _, err := strconv.Atoi(fields[0]); err != nil { + continue + } + command := fields[1] + if strings.Contains(command, "wails3 dev") && strings.Contains(command, "-config ./build/config.yml") { + return fmt.Errorf("task dev is already running (%s) -- stop it first; a second concurrent desktop window risks the same crash class internal/devguard guards `task dev` against", command) + } + if strings.Contains(command, "Mill.app/Contents/MacOS/mill") { + return fmt.Errorf("the installed Mill.app is already running (%s) -- quit it first before running the bridge smoke, which launches its own separate desktop window", command) + } + } + if portInUse(mcpHost, mcpPort) { + return fmt.Errorf("%s:%d is already bound -- another -tags mcp instance (or something else) is already listening there", mcpHost, mcpPort) + } + return nil +} diff --git a/internal/webviewbridgesmoke/main.go b/internal/webviewbridgesmoke/main.go new file mode 100644 index 00000000..e4978bfb --- /dev/null +++ b/internal/webviewbridgesmoke/main.go @@ -0,0 +1,234 @@ +// Command webviewbridgesmoke drives the real desktop app (a -tags mcp +// build, Wails3's own MCP control bridge -- see +// .claude/skills/run-mill/SKILL.md's spike notes) through a small named +// registry of checks (checks.go), over the real macOS WKWebView engine +// instead of Playwright's Chromium/webkit browsers. Exists because +// docs/goals/0097's DoR research found no macOS WebDriver for a +// third-party app's embedded WKWebView, and Playwright's own "webkit" +// build never attaches to it either (patched WebKit-main, not Safari) -- +// this bridge is the one mechanism that drives the actual shipped +// engine. +// +// Invoke via scripts/webview-bridge-smoke.sh, not directly -- the +// script is the documented entrypoint (local pre-release gate, or a +// non-required CI job, per the goal's CI-feasibility verdict). +package main + +import ( + "context" + "fmt" + "net" + "os" + "os/exec" + "path/filepath" + "strconv" + "time" +) + +const ( + mcpHost = "127.0.0.1" + mcpPort = 9099 + appBootTimeout = 30 * time.Second +) + +func main() { + if err := run(); err != nil { + fmt.Fprintf(os.Stderr, "webview-bridge-smoke: %v\n", err) + os.Exit(1) + } +} + +func run() error { + repoRoot, err := repoRootFromWD() + if err != nil { + return err + } + + if err := guardNoOtherInstance(); err != nil { + return err + } + + tmpDir, err := os.MkdirTemp("", "mill-webview-bridge-smoke-") + if err != nil { + return fmt.Errorf("create temp dir: %w", err) + } + defer func() { _ = os.RemoveAll(tmpDir) }() + + binPath := filepath.Join(tmpDir, "mill-bridge-smoke") + if err := buildApp(repoRoot, binPath); err != nil { + return err + } + + proc, stderrTail, err := launchApp(repoRoot, binPath, tmpDir) + if err != nil { + return err + } + defer stopProcess(proc) + + client := newMCPClient(mcpHost, mcpPort) + if err := waitForBridge(client, proc, appBootTimeout); err != nil { + return fmt.Errorf("%w\napp stderr tail:\n%s", err, stderrTail()) + } + + return runRegistry(client) +} + +func runRegistry(client *mcpClient) error { + var failed int + for _, chk := range registry { + detail, err := chk.run(client) + if err != nil { + failed++ + fmt.Printf("FAIL %-28s %v\n", chk.name, err) + var gap *bridgeGapError + if ok := asBridgeGapError(err, &gap); ok { + return fmt.Errorf("stopping: check %q needs bridge tool %q, which the bridge does not support -- report this, never substitute a different tool: %s", chk.name, gap.tool, gap.message) + } + continue + } + fmt.Printf("PASS %-28s %s\n", chk.name, detail) + } + if failed > 0 { + return fmt.Errorf("%d/%d checks failed", failed, len(registry)) + } + fmt.Printf("all %d checks passed\n", len(registry)) + return nil +} + +func asBridgeGapError(err error, target **bridgeGapError) bool { + if gap, ok := err.(*bridgeGapError); ok { + *target = gap + return true + } + return false +} + +func repoRootFromWD() (string, error) { + wd, err := os.Getwd() + if err != nil { + return "", fmt.Errorf("getwd: %w", err) + } + dir := wd + for { + if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil { + return dir, nil + } + parent := filepath.Dir(dir) + if parent == dir { + return "", fmt.Errorf("no go.mod found above %s -- run from inside the mill repo", wd) + } + dir = parent + } +} + +// buildApp always rebuilds the frontend (production mode, matching what +// ships) before the -tags mcp Go binary -- same stale-build-identity +// reasoning frontend/e2e/global-setup.ts documents for its own rebuild: +// this script exists specifically to gate a release-shaped build, so a +// stale frontend/dist would silently smoke-test the wrong artifact. +func buildApp(repoRoot, outPath string) error { + npmBuild := exec.Command("npm", "run", "build") //nolint:gosec,noctx // static args, one-shot CLI invocation, no cancellation source to plumb through (matches internal/devguard's own precedent) + npmBuild.Dir = filepath.Join(repoRoot, "frontend") + npmBuild.Stdout = os.Stdout + npmBuild.Stderr = os.Stderr + if err := npmBuild.Run(); err != nil { + return fmt.Errorf("npm run build: %w", err) + } + + goBuild := exec.Command("go", "build", "-tags", "mcp", "-o", outPath, ".") //nolint:gosec,noctx // outPath is our own generated temp path, not untrusted input; one-shot CLI tool + goBuild.Dir = repoRoot + goBuild.Stdout = os.Stdout + goBuild.Stderr = os.Stderr + if err := goBuild.Run(); err != nil { + return fmt.Errorf("go build -tags mcp: %w", err) + } + return nil +} + +// launchApp spawns the real desktop binary directly (no shell), isolated +// from any real Mill data the same way frontend/e2e/fixtures/server.ts +// isolates each worker's server -- a throwaway settings/execution-db +// pair under tmpDir, never the user's real files. +func launchApp(repoRoot, binPath, tmpDir string) (*os.Process, func() string, error) { + cmd := exec.Command(binPath) //nolint:gosec,noctx // binPath is our own just-built temp binary, not untrusted input; lifecycle is managed via stopProcess, not a context + cmd.Dir = repoRoot + cmd.Env = append(os.Environ(), + "WAILS_MCP_HOST="+mcpHost, + "WAILS_MCP_PORT="+strconv.Itoa(mcpPort), + "MILL_SETTINGS_PATH="+filepath.Join(tmpDir, "settings.json"), + "MILL_EXECUTION_DB_PATH="+filepath.Join(tmpDir, "execution.db"), + ) + stdoutPipe, err := cmd.StdoutPipe() + if err != nil { + return nil, nil, fmt.Errorf("stdout pipe: %w", err) + } + stderrPipe, err := cmd.StderrPipe() + if err != nil { + return nil, nil, fmt.Errorf("stderr pipe: %w", err) + } + if err := cmd.Start(); err != nil { + return nil, nil, fmt.Errorf("start %s: %w", binPath, err) + } + tail := tailLines(stdoutPipe, stderrPipe, 50) + return cmd.Process, tail, nil +} + +// waitForBridge polls app_info until the MCP HTTP server answers, or the +// process exits early (a crash-on-boot must fail fast, not wait out the +// full timeout). +func waitForBridge(client *mcpClient, proc *os.Process, timeout time.Duration) error { + deadline := time.Now().Add(timeout) + var lastErr error + for time.Now().Before(deadline) { + _, callErr := client.call("app_info", map[string]any{}) + if callErr == nil { + return nil + } + lastErr = callErr + if procExited(proc) { + return fmt.Errorf("app process exited before the MCP bridge became reachable: %w", lastErr) + } + time.Sleep(300 * time.Millisecond) + } + return fmt.Errorf("timed out waiting for MCP bridge at %s:%d: %w", mcpHost, mcpPort, lastErr) +} + +func procExited(proc *os.Process) bool { + // A nil signal probes liveness without affecting the process -- + // documented Unix kill(2) behaviour, exec.Process exposes no + // higher-level equivalent. + return proc.Signal(nil) != nil +} + +// stopProcess quits exactly the PID this script itself launched -- +// never a broader pkill/killall (forbidden: a broad pkill has taken +// down a real production mill-server before). +func stopProcess(proc *os.Process) { + if proc == nil { + return + } + _ = proc.Signal(os.Interrupt) + done := make(chan struct{}) + go func() { + _, _ = proc.Wait() + close(done) + }() + select { + case <-done: + case <-time.After(5 * time.Second): + _ = proc.Kill() + <-done + } +} + +func portInUse(host string, port int) bool { + ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) + defer cancel() + dialer := net.Dialer{} + conn, err := dialer.DialContext(ctx, "tcp", net.JoinHostPort(host, strconv.Itoa(port))) + if err != nil { + return false + } + _ = conn.Close() + return true +} diff --git a/internal/webviewbridgesmoke/mcp.go b/internal/webviewbridgesmoke/mcp.go new file mode 100644 index 00000000..fa45a89a --- /dev/null +++ b/internal/webviewbridgesmoke/mcp.go @@ -0,0 +1,135 @@ +package main + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "time" +) + +// mcpClient is a minimal JSON-RPC 2.0 client for the Wails MCP bridge +// (-tags mcp, WAILS_MCP_HOST/WAILS_MCP_PORT -- see +// .claude/skills/run-mill/SKILL.md's spike notes and +// github.com/wailsapp/wails/v3/pkg/application/mcp_protocol_enabled.go). +// The server is stateless (its own DELETE handler is a no-op), so a +// bare "tools/call" works with no prior "initialize" handshake -- this +// client only ever sends that one method. +type mcpClient struct { + endpoint string + http *http.Client + nextID int +} + +func newMCPClient(host string, port int) *mcpClient { + return &mcpClient{ + endpoint: fmt.Sprintf("http://%s:%d/mcp", host, port), + http: &http.Client{Timeout: 30 * time.Second}, + } +} + +type jsonrpcRequest struct { + JSONRPC string `json:"jsonrpc"` + ID int `json:"id"` + Method string `json:"method"` + Params any `json:"params"` +} + +type jsonrpcError struct { + Code int `json:"code"` + Message string `json:"message"` +} + +type jsonrpcResponse struct { + Result *toolCallResult `json:"result"` + Error *jsonrpcError `json:"error"` +} + +type toolCallResult struct { + Content []struct { + Text string `json:"text"` + } `json:"content"` + IsError bool `json:"isError"` +} + +// bridgeGapError marks a JSON-RPC "unknown tool" response specifically -- +// the caller must treat this as a hard stop (the bridge API itself +// can't do the check), never retry or substitute a different tool. +type bridgeGapError struct { + tool string + message string +} + +func (e *bridgeGapError) Error() string { + return fmt.Sprintf("bridge does not support tool %q: %s", e.tool, e.message) +} + +// call invokes one MCP tool and returns its result content as raw text +// (JSON-encoded for non-string return values, per the server's own +// mcpToolResult -- see the reference above). A tool that ran but +// reported isError:true surfaces as a plain error carrying its +// message; an unknown-tool JSON-RPC error surfaces as *bridgeGapError. +func (c *mcpClient) call(tool string, args map[string]any) (string, error) { + c.nextID++ + req := jsonrpcRequest{ + JSONRPC: "2.0", + ID: c.nextID, + Method: "tools/call", + Params: map[string]any{"name": tool, "arguments": args}, + } + body, err := json.Marshal(req) + if err != nil { + return "", fmt.Errorf("marshal request: %w", err) + } + httpReq, err := http.NewRequestWithContext(context.Background(), http.MethodPost, c.endpoint, bytes.NewReader(body)) + if err != nil { + return "", fmt.Errorf("build request: %w", err) + } + httpReq.Header.Set("Content-Type", "application/json") + resp, err := c.http.Do(httpReq) + if err != nil { + return "", fmt.Errorf("post %s: %w", c.endpoint, err) + } + defer func() { _ = resp.Body.Close() }() + respBody, err := io.ReadAll(resp.Body) + if err != nil { + return "", fmt.Errorf("read response: %w", err) + } + var rpc jsonrpcResponse + if err := json.Unmarshal(respBody, &rpc); err != nil { + return "", fmt.Errorf("decode response %q: %w", string(respBody), err) + } + if rpc.Error != nil { + if rpc.Error.Code == -32602 && len(rpc.Error.Message) >= 12 && rpc.Error.Message[:12] == "unknown tool" { + return "", &bridgeGapError{tool: tool, message: rpc.Error.Message} + } + return "", fmt.Errorf("tool %s: rpc error %d: %s", tool, rpc.Error.Code, rpc.Error.Message) + } + if rpc.Result == nil || len(rpc.Result.Content) == 0 { + return "", fmt.Errorf("tool %s: empty result", tool) + } + text := rpc.Result.Content[0].Text + if rpc.Result.IsError { + return "", fmt.Errorf("tool %s reported an error: %s", tool, text) + } + return text, nil +} + +// callJSON is call() plus unmarshaling the returned text into out -- +// the common case for every check below, which all want a structured +// value back rather than raw text. +func (c *mcpClient) callJSON(tool string, args map[string]any, out any) error { + text, err := c.call(tool, args) + if err != nil { + return err + } + if out == nil { + return nil + } + if err := json.Unmarshal([]byte(text), out); err != nil { + return fmt.Errorf("tool %s: decode result %q: %w", tool, text, err) + } + return nil +} diff --git a/internal/webviewbridgesmoke/mcp_test.go b/internal/webviewbridgesmoke/mcp_test.go new file mode 100644 index 00000000..41dd4b09 --- /dev/null +++ b/internal/webviewbridgesmoke/mcp_test.go @@ -0,0 +1,125 @@ +package main + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" +) + +// fakeMCPServer answers tools/call the same way Wails3's own MCP bridge +// does (mcpToolResult/mcpToolError, mcp_protocol_enabled.go) -- pure +// protocol-shape testing, no real desktop app needed. +func fakeMCPServer(t *testing.T, respond func(name string, args map[string]any) jsonrpcResponse) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var req struct { + Params struct { + Name string `json:"name"` + Arguments map[string]any `json:"arguments"` + } `json:"params"` + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + t.Fatalf("decode request: %v", err) + } + resp := respond(req.Params.Name, req.Params.Arguments) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(resp) + })) +} + +func newTestClient(t *testing.T, srv *httptest.Server) *mcpClient { + t.Helper() + c := newMCPClient("127.0.0.1", 0) + c.endpoint = srv.URL + return c +} + +func TestCall_SuccessResult_ReturnsContentText(t *testing.T) { + srv := fakeMCPServer(t, func(name string, args map[string]any) jsonrpcResponse { + return jsonrpcResponse{Result: &toolCallResult{ + Content: []struct { + Text string `json:"text"` + }{{Text: `{"ok":true}`}}, + }} + }) + defer srv.Close() + + text, err := newTestClient(t, srv).call("app_info", map[string]any{}) + if err != nil { + t.Fatalf("call: %v", err) + } + if text != `{"ok":true}` { + t.Fatalf("got %q", text) + } +} + +func TestCall_IsErrorResult_ReturnsPlainError(t *testing.T) { + srv := fakeMCPServer(t, func(name string, args map[string]any) jsonrpcResponse { + return jsonrpcResponse{Result: &toolCallResult{ + IsError: true, + Content: []struct { + Text string `json:"text"` + }{{Text: "element not found"}}, + }} + }) + defer srv.Close() + + _, err := newTestClient(t, srv).call("js_eval", map[string]any{}) + if err == nil { + t.Fatal("expected an error, got nil") + } +} + +func TestCall_UnknownTool_ReturnsBridgeGapError(t *testing.T) { + srv := fakeMCPServer(t, func(name string, args map[string]any) jsonrpcResponse { + return jsonrpcResponse{Error: &jsonrpcError{Code: -32602, Message: "unknown tool: not_a_real_tool"}} + }) + defer srv.Close() + + _, err := newTestClient(t, srv).call("not_a_real_tool", map[string]any{}) + var gap *bridgeGapError + if !asBridgeGapError(err, &gap) { + t.Fatalf("expected a *bridgeGapError, got %T: %v", err, err) + } + if gap.tool != "not_a_real_tool" { + t.Fatalf("got tool %q", gap.tool) + } +} + +func TestCall_OtherRPCError_ReturnsPlainError(t *testing.T) { + srv := fakeMCPServer(t, func(name string, args map[string]any) jsonrpcResponse { + return jsonrpcResponse{Error: &jsonrpcError{Code: -32603, Message: "internal error"}} + }) + defer srv.Close() + + _, err := newTestClient(t, srv).call("app_info", map[string]any{}) + var gap *bridgeGapError + if asBridgeGapError(err, &gap) { + t.Fatal("a generic internal error must not be misclassified as a bridge gap") + } + if err == nil { + t.Fatal("expected an error") + } +} + +func TestCallJSON_DecodesResultIntoOut(t *testing.T) { + srv := fakeMCPServer(t, func(name string, args map[string]any) jsonrpcResponse { + return jsonrpcResponse{Result: &toolCallResult{ + Content: []struct { + Text string `json:"text"` + }{{Text: `{"os":"darwin"}`}}, + }} + }) + defer srv.Close() + + var out struct { + OS string `json:"os"` + } + if err := newTestClient(t, srv).callJSON("app_info", map[string]any{}, &out); err != nil { + t.Fatalf("callJSON: %v", err) + } + if out.OS != "darwin" { + t.Fatalf("got %q", out.OS) + } +} diff --git a/internal/webviewbridgesmoke/procio.go b/internal/webviewbridgesmoke/procio.go new file mode 100644 index 00000000..3324d816 --- /dev/null +++ b/internal/webviewbridgesmoke/procio.go @@ -0,0 +1,40 @@ +package main + +import ( + "bufio" + "io" + "strings" + "sync" +) + +// tailLines drains stdout+stderr concurrently (an unread pipe fills its +// OS buffer and blocks the child process) and keeps the last max lines +// of each, combined, for a failure report -- same tail-buffer shape +// frontend/e2e/fixtures/server.ts uses for its own spawned server. The +// returned func snapshots the tail under lock, safe to call from a +// different goroutine than the ones draining the pipes. +func tailLines(stdout, stderr io.Reader, max int) func() string { + var mu sync.Mutex + lines := make([]string, 0, max) + add := func(line string) { + mu.Lock() + defer mu.Unlock() + lines = append(lines, line) + if len(lines) > max { + lines = lines[len(lines)-max:] + } + } + drain := func(r io.Reader) { + scanner := bufio.NewScanner(r) + for scanner.Scan() { + add(scanner.Text()) + } + } + go drain(stdout) + go drain(stderr) + return func() string { + mu.Lock() + defer mu.Unlock() + return strings.Join(lines, "\n") + } +} diff --git a/scripts/webview-bridge-smoke.sh b/scripts/webview-bridge-smoke.sh new file mode 100755 index 00000000..77f44d5f --- /dev/null +++ b/scripts/webview-bridge-smoke.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash +# Runs the real-webview parity smoke (docs/goals/0097, re-scoped): builds +# a -tags mcp desktop binary, launches the real app, drives the named +# check registry in internal/webviewbridgesmoke over Wails3's own MCP +# control bridge, quits it cleanly. macOS-only (Wails3's `mcp` build tag +# excludes ios/android; the desktop build itself is Mill's macOS target). +# Local pre-release gate until CI feasibility lands as a wired job -- +# see .claude/rules/testing.md's engine-parity entry for the verdict. +set -euo pipefail + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +cd "$repo_root" + +go run ./internal/webviewbridgesmoke From 5058aa7ca97627f2efb2cbec5d2c3eb9b431df80 Mon Sep 17 00:00:00 2001 From: Ali Al Dallal Date: Tue, 18 Aug 2026 00:52:14 -0400 Subject: [PATCH 2/2] test: coverage floor + honest CI verdict for the webview bridge smoke (goal 0097) Unit-test coverage for internal/webviewbridgesmoke's decision logic: guard.go's conflict detection (findConflictingProcess/parsePSOutput, mirroring internal/devguard's own seam), checks.go's full check registry against a scripted fakeMCPCaller (every assertion branch: success, precondition, gesture-didn't-select, ring-never-rendered, bridge-gap-propagates), and main.go's runRegistry/repoRootFrom/ portInUse extracted for testability with no behavioral change. Package coverage 12.2% -> 55.9%; whole-repo total 70.9% locally (floor stays 69.5%, untouched). testing.md now records the CI job's first live attempt against a real macos-latest runner: it failed launching the app (MCP bridge never became reachable, connection refused, empty captured stderr) after both builds succeeded. Recorded as non-required/informational pending investigation, with the empty-stderr silent-exit as the first investigation lead -- not iterated further tonight. The operative parity gate stays the local pre-release run until this is resolved. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01FW5GkkAG8du7tNdYLk2zSd --- .claude/rules/testing.md | 23 +- internal/webviewbridgesmoke/checks.go | 20 +- internal/webviewbridgesmoke/checks_test.go | 341 +++++++++++++++++++++ internal/webviewbridgesmoke/guard.go | 72 +++-- internal/webviewbridgesmoke/guard_test.go | 86 ++++++ internal/webviewbridgesmoke/main.go | 28 +- internal/webviewbridgesmoke/main_test.go | 137 +++++++++ internal/webviewbridgesmoke/mcp.go | 9 + 8 files changed, 676 insertions(+), 40 deletions(-) create mode 100644 internal/webviewbridgesmoke/checks_test.go create mode 100644 internal/webviewbridgesmoke/guard_test.go create mode 100644 internal/webviewbridgesmoke/main_test.go diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index 3b9d3f82..d8940f9d 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -141,11 +141,28 @@ layer per capability," never "a seed per thing": at all — the OSS convergence for real parity is driving the actual embedded webview via an app bridge, which is what this layer does. Playwright's `webkit` browser stays installed as a local debugging - probe only, never CI-badged as parity. Non-required CI job - (`webview-bridge-smoke` in `ci.yml`) until it has a track record; - local pre-release gate in the meantime. Revisit trigger: grow the + probe only, never CI-badged as parity. Revisit trigger: grow the check registry when a WebKit-only bug escapes it, same discipline as the manual-only registry below. + **CI status: non-required/informational, first live attempt + failed.** The `webview-bridge-smoke` job in `ci.yml` ran on a real + `macos-latest` GitHub-hosted runner and failed launching the app: + `app process exited before the MCP bridge became reachable: dial + tcp 127.0.0.1:9099: connect: connection refused`, with an EMPTY + captured stderr tail — the go/npm builds both succeeded, but the + spawned app process produced no diagnostic output at all before + exiting. First investigation lead: a silent, near-instant exit with + zero stderr is consistent with an early native-windowing failure + (no real console/WindowServer session available the way the + precedent research assumed from Electron's own hosted-runner CI — + Electron's windowing stack may not need the same session a genuine + Wails/Cocoa `NSWindow` does), though the log alone doesn't prove + that specific cause; config-dir/signing weren't ruled in or out + either. Not iterated further pending a dedicated investigation + pass. Until resolved, the OPERATIVE parity gate is the LOCAL + pre-release run (`scripts/webview-bridge-smoke.sh`, run by hand + before a release) — the CI job stays wired and green-or-red + visible, but nothing depends on it passing yet. - **Manual-only registry** — OS-bound checks (hotkey delivery, real clipboard, tray) listed explicitly with reasons, never silently absent (see goal 0010's enforcement). Non-seed instance: the diff --git a/internal/webviewbridgesmoke/checks.go b/internal/webviewbridgesmoke/checks.go index 3b8956e6..bfa98c5e 100644 --- a/internal/webviewbridgesmoke/checks.go +++ b/internal/webviewbridgesmoke/checks.go @@ -14,7 +14,7 @@ import ( type check struct { name string reason string - run func(c *mcpClient) (string, error) + run func(c mcpCaller) (string, error) } var registry = []check{ @@ -50,7 +50,7 @@ var registry = []check{ }, } -func checkIsolatedDataBadge(c *mcpClient) (string, error) { +func checkIsolatedDataBadge(c mcpCaller) (string, error) { var found bool if err := c.callJSON("js_eval", map[string]any{ "js": `return !!document.querySelector('[data-testid="isolated-data-badge"]');`, @@ -63,7 +63,7 @@ func checkIsolatedDataBadge(c *mcpClient) (string, error) { return "badge visible", nil } -func checkAppInfo(c *mcpClient) (string, error) { +func checkAppInfo(c mcpCaller) (string, error) { var info struct { OS string `json:"os"` Windows []struct { @@ -86,7 +86,7 @@ func checkAppInfo(c *mcpClient) (string, error) { // true or the deadline passes -- the real webview's page load and // React's own render pass are both async relative to the MCP HTTP // server becoming reachable. -func pollJSEval(c *mcpClient, js string, timeout time.Duration) error { +func pollJSEval(c mcpCaller, js string, timeout time.Duration) error { deadline := time.Now().Add(timeout) var lastErr error for time.Now().Before(deadline) { @@ -104,7 +104,7 @@ func pollJSEval(c *mcpClient, js string, timeout time.Duration) error { return fmt.Errorf("timed out polling (condition never became true)") } -func checkAtlasBoardRenders(c *mcpClient) (string, error) { +func checkAtlasBoardRenders(c mcpCaller) (string, error) { var nav struct { Clicked bool `json:"clicked"` Tag string `json:"tag"` @@ -124,7 +124,7 @@ func checkAtlasBoardRenders(c *mcpClient) (string, error) { return "atlas-board rendered after nav click", nil } -func checkNoteCardFlip(c *mcpClient) (string, error) { +func checkNoteCardFlip(c mcpCaller) (string, error) { var result struct { Before string `json:"before"` After string `json:"after"` @@ -153,7 +153,7 @@ type ringSnapshot struct { Selected bool `json:"selected"` } -func readRing(c *mcpClient, selector string) (ringSnapshot, error) { +func readRing(c mcpCaller, selector string) (ringSnapshot, error) { var snap ringSnapshot err := c.callJSON("js_eval", map[string]any{ "js": fmt.Sprintf(`const el = document.querySelectorAll(%q)[0]; @@ -165,7 +165,7 @@ func readRing(c *mcpClient, selector string) (ringSnapshot, error) { return snap, err } -func checkNoteCardSelectionRing(c *mcpClient) (string, error) { +func checkNoteCardSelectionRing(c mcpCaller) (string, error) { selector := `[data-testid="atlas-note-card"]` before, err := readRing(c, selector) if err != nil { @@ -210,7 +210,7 @@ type atlasCard struct { ParentID string `json:"ParentID"` } -func checkStickyBorderColorFlip(c *mcpClient) (string, error) { +func checkStickyBorderColorFlip(c mcpCaller) (string, error) { var cards []atlasCard if err := c.callJSON("call_bound_method", map[string]any{ "name": "github.com/alicoding/mill/internal/services/atlassvc.AtlasService.Cards", @@ -272,7 +272,7 @@ type stickySnapshot struct { BoxShadow string `json:"boxShadow"` } -func readStickyStyle(c *mcpClient, selector string) (stickySnapshot, error) { +func readStickyStyle(c mcpCaller, selector string) (stickySnapshot, error) { var snap stickySnapshot err := c.callJSON("js_eval", map[string]any{ "js": fmt.Sprintf(`const el = document.querySelector(%q); diff --git a/internal/webviewbridgesmoke/checks_test.go b/internal/webviewbridgesmoke/checks_test.go new file mode 100644 index 00000000..5e8f9d96 --- /dev/null +++ b/internal/webviewbridgesmoke/checks_test.go @@ -0,0 +1,341 @@ +package main + +import ( + "encoding/json" + "errors" + "strings" + "testing" + "time" +) + +// calledTool records one invocation against fakeCaller, for tests that +// assert on call order/arguments (e.g. that CreateNote's parentID comes +// from the right seeded card). +type calledTool struct { + tool string + args map[string]any +} + +// fakeCaller is a scripted mcpCaller: each tool name has its own FIFO +// queue of responses, popped in call order -- lets a test give the +// same tool (js_eval, most often) a different answer on its 2nd/3rd +// call within one check, matching a real before/after read. +type fakeCaller struct { + queues map[string][]func(args map[string]any) (string, error) + calls []calledTool +} + +func newFakeCaller() *fakeCaller { + return &fakeCaller{queues: map[string][]func(args map[string]any) (string, error){}} +} + +func (f *fakeCaller) on(tool string, fn func(args map[string]any) (string, error)) { + f.queues[tool] = append(f.queues[tool], fn) +} + +func (f *fakeCaller) onJSON(tool string, v any) { + b, err := json.Marshal(v) + if err != nil { + panic(err) + } + f.on(tool, func(map[string]any) (string, error) { return string(b), nil }) +} + +func (f *fakeCaller) onError(tool string, err error) { + f.on(tool, func(map[string]any) (string, error) { return "", err }) +} + +func (f *fakeCaller) call(tool string, args map[string]any) (string, error) { + f.calls = append(f.calls, calledTool{tool, args}) + q := f.queues[tool] + if len(q) == 0 { + return "", &bridgeGapError{tool: tool, message: "unknown tool: " + tool} + } + fn := q[0] + f.queues[tool] = q[1:] + return fn(args) +} + +func (f *fakeCaller) callJSON(tool string, args map[string]any, out any) error { + text, err := f.call(tool, args) + if err != nil { + return err + } + if out == nil { + return nil + } + return json.Unmarshal([]byte(text), out) +} + +func TestRegistry_Shape(t *testing.T) { + seen := map[string]bool{} + for _, chk := range registry { + if chk.name == "" { + t.Errorf("a registry entry has an empty name") + } + if seen[chk.name] { + t.Errorf("duplicate registry entry name %q", chk.name) + } + seen[chk.name] = true + if chk.reason == "" { + t.Errorf("check %q has no reason -- registry discipline requires one", chk.name) + } + if chk.run == nil { + t.Errorf("check %q has a nil run func", chk.name) + } + } + if len(registry) == 0 { + t.Fatal("registry is empty") + } +} + +func TestCheckIsolatedDataBadge(t *testing.T) { + t.Run("badge present", func(t *testing.T) { + f := newFakeCaller() + f.onJSON("js_eval", true) + detail, err := checkIsolatedDataBadge(f) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if detail != "badge visible" { + t.Errorf("got %q", detail) + } + }) + + t.Run("badge missing", func(t *testing.T) { + f := newFakeCaller() + f.onJSON("js_eval", false) + if _, err := checkIsolatedDataBadge(f); err == nil { + t.Fatal("expected an error when the badge is missing") + } + }) +} + +func TestCheckAppInfo(t *testing.T) { + t.Run("darwin, one window", func(t *testing.T) { + f := newFakeCaller() + f.onJSON("app_info", map[string]any{"os": "darwin", "windows": []map[string]any{{"name": "mill"}}}) + detail, err := checkAppInfo(f) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(detail, "darwin") { + t.Errorf("got %q", detail) + } + }) + + t.Run("wrong os", func(t *testing.T) { + f := newFakeCaller() + f.onJSON("app_info", map[string]any{"os": "linux", "windows": []map[string]any{{"name": "mill"}}}) + if _, err := checkAppInfo(f); err == nil { + t.Fatal("expected an error for a non-darwin os") + } + }) + + t.Run("wrong window count", func(t *testing.T) { + f := newFakeCaller() + f.onJSON("app_info", map[string]any{"os": "darwin", "windows": []map[string]any{}}) + if _, err := checkAppInfo(f); err == nil { + t.Fatal("expected an error for zero windows") + } + }) +} + +func TestPollJSEval(t *testing.T) { + t.Run("returns nil as soon as the condition is true", func(t *testing.T) { + f := newFakeCaller() + f.onJSON("js_eval", true) + if err := pollJSEval(f, "return true;", time.Second); err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + + t.Run("retries until true", func(t *testing.T) { + f := newFakeCaller() + f.onJSON("js_eval", false) + f.onJSON("js_eval", false) + f.onJSON("js_eval", true) + start := time.Now() + if err := pollJSEval(f, "...", time.Second); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if time.Since(start) < 500*time.Millisecond { + t.Error("expected pollJSEval to have actually retried (2x300ms sleep) before succeeding") + } + }) + + t.Run("times out when the condition never becomes true", func(t *testing.T) { + f := newFakeCaller() + for range 3 { + f.onJSON("js_eval", false) + } + if err := pollJSEval(f, "...", 50*time.Millisecond); err == nil { + t.Fatal("expected a timeout error") + } + }) +} + +func TestCheckAtlasBoardRenders(t *testing.T) { + t.Run("nav click then board renders", func(t *testing.T) { + f := newFakeCaller() + f.onJSON("js_eval", map[string]any{"clicked": true, "tag": "A"}) + f.onJSON("js_eval", true) + detail, err := checkAtlasBoardRenders(f) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(detail, "atlas-board") { + t.Errorf("got %q", detail) + } + }) + + t.Run("nav entry not found propagates the error", func(t *testing.T) { + f := newFakeCaller() + f.onError("js_eval", errors.New("Atlas nav entry not found among 3 candidates")) + if _, err := checkAtlasBoardRenders(f); err == nil { + t.Fatal("expected an error when the Atlas nav entry can't be found") + } + }) +} + +func TestCheckNoteCardFlip(t *testing.T) { + t.Run("flips false to true", func(t *testing.T) { + f := newFakeCaller() + f.onJSON("js_eval", map[string]any{"before": "false", "after": "true", "label": "Getting started"}) + detail, err := checkNoteCardFlip(f) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(detail, "Getting started") { + t.Errorf("got %q", detail) + } + }) + + t.Run("never flipped is an error", func(t *testing.T) { + f := newFakeCaller() + f.onJSON("js_eval", map[string]any{"before": "false", "after": "false", "label": "x"}) + if _, err := checkNoteCardFlip(f); err == nil { + t.Fatal("expected an error when the card never flipped") + } + }) +} + +func TestCheckNoteCardSelectionRing(t *testing.T) { + t.Run("box-shadow none -> non-none on shift-click select", func(t *testing.T) { + f := newFakeCaller() + f.onJSON("js_eval", ringSnapshot{BoxShadow: "none", Selected: false}) + f.on("mouse_click", func(map[string]any) (string, error) { return "ok", nil }) + f.onJSON("js_eval", ringSnapshot{BoxShadow: "0 0 0 2px accent", Selected: true}) + detail, err := checkNoteCardSelectionRing(f) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(detail, "none ->") { + t.Errorf("got %q", detail) + } + }) + + t.Run("already selected before the check ran is an error", func(t *testing.T) { + f := newFakeCaller() + f.onJSON("js_eval", ringSnapshot{BoxShadow: "none", Selected: true}) + if _, err := checkNoteCardSelectionRing(f); err == nil { + t.Fatal("expected an error for a pre-selected card") + } + }) + + t.Run("shift-click that fails to select is an error", func(t *testing.T) { + f := newFakeCaller() + f.onJSON("js_eval", ringSnapshot{BoxShadow: "none", Selected: false}) + f.on("mouse_click", func(map[string]any) (string, error) { return "ok", nil }) + f.onJSON("js_eval", ringSnapshot{BoxShadow: "none", Selected: false}) + if _, err := checkNoteCardSelectionRing(f); err == nil { + t.Fatal("expected an error when shift-click never selects the card") + } + }) + + t.Run("ring never renders after selection is an error", func(t *testing.T) { + f := newFakeCaller() + f.onJSON("js_eval", ringSnapshot{BoxShadow: "none", Selected: false}) + f.on("mouse_click", func(map[string]any) (string, error) { return "ok", nil }) + f.onJSON("js_eval", ringSnapshot{BoxShadow: "none", Selected: true}) + if _, err := checkNoteCardSelectionRing(f); err == nil { + t.Fatal("expected an error when box-shadow stays none after selection") + } + }) +} + +func TestCheckStickyBorderColorFlip(t *testing.T) { + seedCards := []atlasCard{{ID: "card-1", Title: "Getting started", ParentID: "space-1"}} + + t.Run("border-color and ring both flip on selection", func(t *testing.T) { + f := newFakeCaller() + f.onJSON("call_bound_method", seedCards) + f.onJSON("call_bound_method", map[string]any{"ID": "note-1"}) + f.onJSON("js_eval", true) // pollJSEval: sticky rendered + f.onJSON("js_eval", stickySnapshot{BorderColor: "rgb(1,1,1)", BoxShadow: "none"}) + f.on("mouse_click", func(map[string]any) (string, error) { return "ok", nil }) + f.onJSON("js_eval", stickySnapshot{BorderColor: "rgb(9,9,9)", BoxShadow: "0 0 0 3px accent"}) + + detail, err := checkStickyBorderColorFlip(f) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(detail, "rgb(1,1,1) -> rgb(9,9,9)") { + t.Errorf("got %q", detail) + } + + // The note must nest under "Getting started"'s own ParentID + // (the auto-entered board level), not under Getting started's + // own ID or the meta root -- regression for the level-mismatch + // this check's design had to reason through explicitly. + var createCall *calledTool + for i := range f.calls { + if f.calls[i].tool == "call_bound_method" && strings.Contains(f.calls[i].args["name"].(string), "CreateNote") { + createCall = &f.calls[i] + } + } + if createCall == nil { + t.Fatal("CreateNote was never called") + } + args, _ := createCall.args["args"].([]any) + if len(args) != 3 || args[2] != "space-1" { + t.Errorf("expected CreateNote's parentID to be space-1 (Getting started's own ParentID), got args=%v", args) + } + }) + + t.Run("seeded card not found is an error, no note created", func(t *testing.T) { + f := newFakeCaller() + f.onJSON("call_bound_method", []atlasCard{{ID: "x", Title: "Some other card", ParentID: "y"}}) + if _, err := checkStickyBorderColorFlip(f); err == nil { + t.Fatal("expected an error when \"Getting started\" isn't in the seed") + } + for _, c := range f.calls { + if c.tool == "call_bound_method" && strings.Contains(c.args["name"].(string), "CreateNote") { + t.Fatal("CreateNote must not be called once the parent card lookup fails") + } + } + }) + + t.Run("border-color unchanged after selection is an error", func(t *testing.T) { + f := newFakeCaller() + f.onJSON("call_bound_method", seedCards) + f.onJSON("call_bound_method", map[string]any{"ID": "note-1"}) + f.onJSON("js_eval", true) + f.onJSON("js_eval", stickySnapshot{BorderColor: "rgb(1,1,1)", BoxShadow: "none"}) + f.on("mouse_click", func(map[string]any) (string, error) { return "ok", nil }) + f.onJSON("js_eval", stickySnapshot{BorderColor: "rgb(1,1,1)", BoxShadow: "0 0 0 3px accent"}) + if _, err := checkStickyBorderColorFlip(f); err == nil { + t.Fatal("expected an error when border-color doesn't change") + } + }) + + t.Run("a genuine bridge gap (unknown tool) propagates, never substituted", func(t *testing.T) { + f := newFakeCaller() + f.onError("call_bound_method", &bridgeGapError{tool: "call_bound_method", message: "unknown tool: call_bound_method"}) + _, err := checkStickyBorderColorFlip(f) + var gap *bridgeGapError + if !asBridgeGapError(err, &gap) { + t.Fatalf("expected a *bridgeGapError, got %T: %v", err, err) + } + }) +} diff --git a/internal/webviewbridgesmoke/guard.go b/internal/webviewbridgesmoke/guard.go index 1782be40..a6b6fc17 100644 --- a/internal/webviewbridgesmoke/guard.go +++ b/internal/webviewbridgesmoke/guard.go @@ -7,6 +7,57 @@ import ( "strings" ) +// process is one line of `ps -axwwo pid=,command=` output -- same shape +// as internal/devguard's own, kept local since the two packages can't +// share an unexported type across package main boundaries. +type process struct { + pid int + command string +} + +// parsePSOutput parses `ps -axwwo pid=,command=` output, tolerant of a +// malformed line the same way internal/devguard's parseProcesses is +// (skips it rather than failing the whole scan). +func parsePSOutput(output string) []process { + var procs []process + for _, line := range strings.Split(output, "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + fields := strings.SplitN(line, " ", 2) + pid, err := strconv.Atoi(fields[0]) + if err != nil { + continue + } + command := "" + if len(fields) == 2 { + command = strings.TrimSpace(fields[1]) + } + procs = append(procs, process{pid: pid, command: command}) + } + return procs +} + +// findConflictingProcess scans for either of the two conflicting- +// instance shapes this script refuses to run alongside: this repo's +// own `task dev` (same marker internal/devguard matches on: `wails3 +// dev` plus its `-config ./build/config.yml` invocation path, so an +// unrelated project's own dev loop never false-positives) or the +// installed Mill.app. Returns the matched process and a reason +// sentence naming which one, or found=false when neither is running. +func findConflictingProcess(procs []process) (proc process, reason string, found bool) { + for _, p := range procs { + if strings.Contains(p.command, "wails3 dev") && strings.Contains(p.command, "-config ./build/config.yml") { + return p, fmt.Sprintf("task dev is already running (%s) -- stop it first; a second concurrent desktop window risks the same crash class internal/devguard guards `task dev` against", p.command), true + } + if strings.Contains(p.command, "Mill.app/Contents/MacOS/mill") { + return p, fmt.Sprintf("the installed Mill.app is already running (%s) -- quit it first before running the bridge smoke, which launches its own separate desktop window", p.command), true + } + } + return process{}, "", false +} + // guardNoOtherInstance refuses to launch a second real desktop window // alongside one that's already running -- mirrors internal/devguard's // own approach (a `ps` scan for this repo's own dev-loop marker) plus @@ -19,25 +70,8 @@ func guardNoOtherInstance() error { if err != nil { return fmt.Errorf("couldn't list processes to check for a conflicting Mill instance: %w", err) } - for _, line := range strings.Split(string(psOutput), "\n") { - line = strings.TrimSpace(line) - if line == "" { - continue - } - fields := strings.SplitN(line, " ", 2) - if len(fields) != 2 { - continue - } - if _, err := strconv.Atoi(fields[0]); err != nil { - continue - } - command := fields[1] - if strings.Contains(command, "wails3 dev") && strings.Contains(command, "-config ./build/config.yml") { - return fmt.Errorf("task dev is already running (%s) -- stop it first; a second concurrent desktop window risks the same crash class internal/devguard guards `task dev` against", command) - } - if strings.Contains(command, "Mill.app/Contents/MacOS/mill") { - return fmt.Errorf("the installed Mill.app is already running (%s) -- quit it first before running the bridge smoke, which launches its own separate desktop window", command) - } + if _, reason, found := findConflictingProcess(parsePSOutput(string(psOutput))); found { + return fmt.Errorf("%s", reason) } if portInUse(mcpHost, mcpPort) { return fmt.Errorf("%s:%d is already bound -- another -tags mcp instance (or something else) is already listening there", mcpHost, mcpPort) diff --git a/internal/webviewbridgesmoke/guard_test.go b/internal/webviewbridgesmoke/guard_test.go new file mode 100644 index 00000000..2d94f3ae --- /dev/null +++ b/internal/webviewbridgesmoke/guard_test.go @@ -0,0 +1,86 @@ +package main + +import ( + "strings" + "testing" +) + +func TestParsePSOutput(t *testing.T) { + t.Run("parses well-formed lines", func(t *testing.T) { + output := " 123 wails3 dev -config ./build/config.yml -port 9245\n456 /bin/zsh -c echo hi\n" + procs := parsePSOutput(output) + if len(procs) != 2 { + t.Fatalf("expected 2 processes, got %d", len(procs)) + } + if procs[0].pid != 123 || procs[0].command != "wails3 dev -config ./build/config.yml -port 9245" { + t.Errorf("unexpected first process: %+v", procs[0]) + } + if procs[1].pid != 456 || procs[1].command != "/bin/zsh -c echo hi" { + t.Errorf("unexpected second process: %+v", procs[1]) + } + }) + + t.Run("skips blank lines and a line with no valid integer PID", func(t *testing.T) { + procs := parsePSOutput("\n \nnotapid some command\n789 real process\n") + if len(procs) != 1 || procs[0].pid != 789 { + t.Fatalf("expected exactly the one valid-PID line, got %+v", procs) + } + }) + + t.Run("empty input parses to zero processes", func(t *testing.T) { + if procs := parsePSOutput(""); len(procs) != 0 { + t.Fatalf("expected zero processes, got %+v", procs) + } + }) +} + +func TestFindConflictingProcess(t *testing.T) { + t.Run("matches this repo's own task dev invocation", func(t *testing.T) { + procs := []process{ + {pid: 1, command: "/bin/zsh -c some unrelated thing"}, + {pid: 2, command: "wails3 dev -config ./build/config.yml -port 9245"}, + } + proc, reason, found := findConflictingProcess(procs) + if !found || proc.pid != 2 { + t.Fatalf("expected to find PID 2, got proc=%+v found=%v", proc, found) + } + if !strings.Contains(reason, "task dev is already running") { + t.Errorf("unexpected reason: %s", reason) + } + }) + + t.Run("matches the installed Mill.app", func(t *testing.T) { + procs := []process{ + {pid: 7, command: "/Applications/Mill.app/Contents/MacOS/mill"}, + } + proc, reason, found := findConflictingProcess(procs) + if !found || proc.pid != 7 { + t.Fatalf("expected to find PID 7, got proc=%+v found=%v", proc, found) + } + if !strings.Contains(reason, "installed Mill.app is already running") { + t.Errorf("unexpected reason: %s", reason) + } + }) + + t.Run("an unrelated project's own wails3 dev (different config path) never false-positives", func(t *testing.T) { + procs := []process{ + {pid: 1, command: "wails3 dev -config ./other-project/build/config.yml -port 5173"}, + } + if _, _, found := findConflictingProcess(procs); found { + t.Fatal("expected no match across a different config path") + } + }) + + t.Run("a bare 'wails3 build' never matches", func(t *testing.T) { + procs := []process{{pid: 1, command: "wails3 build DEV=true"}} + if _, _, found := findConflictingProcess(procs); found { + t.Fatal("expected no match") + } + }) + + t.Run("returns found=false when nothing conflicts", func(t *testing.T) { + if _, _, found := findConflictingProcess(nil); found { + t.Fatal("expected found=false for an empty process list") + } + }) +} diff --git a/internal/webviewbridgesmoke/main.go b/internal/webviewbridgesmoke/main.go index e4978bfb..d65c6e69 100644 --- a/internal/webviewbridgesmoke/main.go +++ b/internal/webviewbridgesmoke/main.go @@ -70,13 +70,18 @@ func run() error { return fmt.Errorf("%w\napp stderr tail:\n%s", err, stderrTail()) } - return runRegistry(client) + return runRegistry(client, registry) } -func runRegistry(client *mcpClient) error { +// runRegistry runs each check in order and reports PASS/FAIL, stopping +// immediately (never substituting a different tool) the moment a check +// hits a genuine bridge API gap rather than a mere assertion failure -- +// caller and checks are parameters, not the package globals, so the +// aggregation logic here is exercisable against a scripted fake. +func runRegistry(caller mcpCaller, checks []check) error { var failed int - for _, chk := range registry { - detail, err := chk.run(client) + for _, chk := range checks { + detail, err := chk.run(caller) if err != nil { failed++ fmt.Printf("FAIL %-28s %v\n", chk.name, err) @@ -89,9 +94,9 @@ func runRegistry(client *mcpClient) error { fmt.Printf("PASS %-28s %s\n", chk.name, detail) } if failed > 0 { - return fmt.Errorf("%d/%d checks failed", failed, len(registry)) + return fmt.Errorf("%d/%d checks failed", failed, len(checks)) } - fmt.Printf("all %d checks passed\n", len(registry)) + fmt.Printf("all %d checks passed\n", len(checks)) return nil } @@ -108,14 +113,21 @@ func repoRootFromWD() (string, error) { if err != nil { return "", fmt.Errorf("getwd: %w", err) } - dir := wd + return repoRootFrom(wd) +} + +// repoRootFrom walks up from startDir looking for go.mod -- split out of +// repoRootFromWD so the walk itself is testable against a real temp +// directory tree without needing to chdir the test process. +func repoRootFrom(startDir string) (string, error) { + dir := startDir for { if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil { return dir, nil } parent := filepath.Dir(dir) if parent == dir { - return "", fmt.Errorf("no go.mod found above %s -- run from inside the mill repo", wd) + return "", fmt.Errorf("no go.mod found above %s -- run from inside the mill repo", startDir) } dir = parent } diff --git a/internal/webviewbridgesmoke/main_test.go b/internal/webviewbridgesmoke/main_test.go new file mode 100644 index 00000000..116d1a85 --- /dev/null +++ b/internal/webviewbridgesmoke/main_test.go @@ -0,0 +1,137 @@ +package main + +import ( + "context" + "errors" + "net" + "os" + "path/filepath" + "testing" +) + +func TestRepoRootFrom(t *testing.T) { + t.Run("finds go.mod in an ancestor directory", func(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "go.mod"), []byte("module x\n"), 0o600); err != nil { + t.Fatalf("write go.mod: %v", err) + } + nested := filepath.Join(root, "a", "b", "c") + if err := os.MkdirAll(nested, 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + got, err := repoRootFrom(nested) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Both sides may carry a macOS /private symlink prefix + // inconsistently -- EvalSymlinks normalizes both before compare. + wantResolved, _ := filepath.EvalSymlinks(root) + gotResolved, _ := filepath.EvalSymlinks(got) + if gotResolved != wantResolved { + t.Errorf("got %q, want %q", got, root) + } + }) + + t.Run("errors when no go.mod exists above the start dir", func(t *testing.T) { + root := t.TempDir() + nested := filepath.Join(root, "a", "b") + if err := os.MkdirAll(nested, 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + if _, err := repoRootFrom(nested); err == nil { + t.Fatal("expected an error when no go.mod is found") + } + }) +} + +func TestAsBridgeGapError(t *testing.T) { + t.Run("matches a *bridgeGapError", func(t *testing.T) { + var gap *bridgeGapError + err := error(&bridgeGapError{tool: "x", message: "unknown tool: x"}) + if !asBridgeGapError(err, &gap) || gap.tool != "x" { + t.Fatalf("expected a match, got gap=%+v", gap) + } + }) + + t.Run("does not match a plain error", func(t *testing.T) { + var gap *bridgeGapError + if asBridgeGapError(errors.New("boom"), &gap) { + t.Fatal("a plain error must never be misclassified as a bridge gap") + } + }) +} + +func TestRunRegistry(t *testing.T) { + t.Run("all checks passing returns nil", func(t *testing.T) { + checks := []check{ + {name: "a", reason: "r", run: func(mcpCaller) (string, error) { return "ok", nil }}, + {name: "b", reason: "r", run: func(mcpCaller) (string, error) { return "ok", nil }}, + } + if err := runRegistry(newFakeCaller(), checks); err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + + t.Run("a failing check is reported but later checks still run", func(t *testing.T) { + ran := map[string]bool{} + checks := []check{ + {name: "a", reason: "r", run: func(mcpCaller) (string, error) { ran["a"] = true; return "", errors.New("nope") }}, + {name: "b", reason: "r", run: func(mcpCaller) (string, error) { ran["b"] = true; return "ok", nil }}, + } + err := runRegistry(newFakeCaller(), checks) + if err == nil { + t.Fatal("expected an aggregate error when a check fails") + } + if !ran["a"] || !ran["b"] { + t.Fatalf("expected both checks to run, got %+v", ran) + } + }) + + t.Run("a bridge gap stops the registry immediately, never substituted", func(t *testing.T) { + ran := map[string]bool{} + checks := []check{ + {name: "a", reason: "r", run: func(mcpCaller) (string, error) { + ran["a"] = true + return "", &bridgeGapError{tool: "dom_query", message: "unknown tool: dom_query"} + }}, + {name: "b", reason: "r", run: func(mcpCaller) (string, error) { ran["b"] = true; return "ok", nil }}, + } + err := runRegistry(newFakeCaller(), checks) + if err == nil { + t.Fatal("expected an error") + } + if ran["b"] { + t.Fatal("a bridge gap must stop the registry before later checks run") + } + }) +} + +func TestPortInUse(t *testing.T) { + listen := func(t *testing.T) net.Listener { + t.Helper() + var lc net.ListenConfig + ln, err := lc.Listen(context.Background(), "tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + return ln + } + + t.Run("true for a port something is actually listening on", func(t *testing.T) { + ln := listen(t) + defer func() { _ = ln.Close() }() + port := ln.Addr().(*net.TCPAddr).Port + if !portInUse("127.0.0.1", port) { + t.Error("expected portInUse to report true for a bound port") + } + }) + + t.Run("false for a port nothing is listening on", func(t *testing.T) { + ln := listen(t) + port := ln.Addr().(*net.TCPAddr).Port + _ = ln.Close() + if portInUse("127.0.0.1", port) { + t.Error("expected portInUse to report false for a closed port") + } + }) +} diff --git a/internal/webviewbridgesmoke/mcp.go b/internal/webviewbridgesmoke/mcp.go index fa45a89a..09790795 100644 --- a/internal/webviewbridgesmoke/mcp.go +++ b/internal/webviewbridgesmoke/mcp.go @@ -10,6 +10,15 @@ import ( "time" ) +// mcpCaller is the seam checks.go's registry depends on instead of the +// concrete *mcpClient -- lets checks_test.go exercise every check's +// assertion logic against a scripted fake, with no real HTTP server or +// desktop app involved. +type mcpCaller interface { + call(tool string, args map[string]any) (string, error) + callJSON(tool string, args map[string]any, out any) error +} + // mcpClient is a minimal JSON-RPC 2.0 client for the Wails MCP bridge // (-tags mcp, WAILS_MCP_HOST/WAILS_MCP_PORT -- see // .claude/skills/run-mill/SKILL.md's spike notes and