chore(devcontainer): playwright chromium for the MCP, self-healing ods web, dev-workflow docs (#13654)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

.devcontainer/Dockerfile

@@ -97,8 +97,9 @@ RUN ln -s /usr/local/bin/bun /usr/local/bin/bunx
 # runner (web/tests/e2e) forks worker processes that require Node — running it
 # under bun alone hangs the runner. Daily JS tooling (Claude Code, opencode)
 # still runs under bun; Node is here only as the runtime for the test runner.
-# The chromium browser binary is installed at test time (web global-setup), not
-# baked here, so its version tracks web/package.json's floating @playwright/test.
+# The test runner's chromium binary is installed at test time, not baked here,
+# so its version tracks web/package.json's floating @playwright/test (the
+# chromium baked further down serves only the pinned Playwright MCP server).
 #
 # Copied from the official node image (digest-pinned in the node_source stage at the
 # top, routed through BASE_IMAGE_REGISTRY) instead of fetched from nodejs.org, so a
@@ -115,18 +116,28 @@ RUN ln -s ../lib/node_modules/npm/bin/npm-cli.js /usr/local/bin/npm \
 # the web-search open_url tool. The browser binary itself is downloaded
 # from the integration test conftest's _install_playwright fixture so
 # the version always matches the lockfile's playwright-python.
-#
-# Playwright doesn't ship Chromium builds for Ubuntu 26.04 yet, so spoof
-# the host platform to the binary-compatible Ubuntu 24.04 build. Arch
-# is detected from dpkg so both arm64 CI runners and x64 dev machines
-# work; we don't bake the override into ENV because that would leak
-# into interactive dev shells where it might mask real problems.
 RUN apt-get update \
-  && PW_ARCH=$(case $(dpkg --print-architecture) in amd64) echo x64;; arm64) echo arm64;; esac) \
-  && PLAYWRIGHT_HOST_PLATFORM_OVERRIDE=ubuntu24.04-${PW_ARCH} \
-       uvx --quiet --from playwright playwright install-deps chromium \
+  && uvx --quiet --from playwright playwright install-deps chromium \
   && apt-get clean && rm -rf /var/lib/apt/lists/*
 
+# Give Playwright's browser cache a fixed, user-independent path. The default
+# (~/.cache/ms-playwright) would be shadowed by the devcontainer's per-user
+# cache volume and differ between root and dev, so revisions baked below would
+# be invisible at runtime. Test-time installs (web e2e, the integration
+# conftest) land their own revisions alongside the baked one.
+ENV PLAYWRIGHT_BROWSERS_PATH=/opt/ms-playwright
+
+# Bake the chromium build for the Playwright MCP server. Playwright browser
+# builds are revision-coupled to the playwright version that installs them, so
+# resolve the exact playwright the pinned MCP bundles and install with that —
+# a chromium installed by any other version would not be found. Keep the pin
+# in sync with .mcp.json. World-writable so test-time installs run as the dev
+# user can add their own revisions next to it.
+ARG PLAYWRIGHT_MCP_VERSION=0.0.78
+RUN PW_VERSION=$(npm view "@playwright/mcp@${PLAYWRIGHT_MCP_VERSION}" dependencies.playwright) \
+  && npx --yes "playwright@${PW_VERSION}" install chromium \
+  && chmod -R 777 /opt/ms-playwright
+
 # Install Python 3.13 via uv into a stable, image-level path so venvs
 # created in bind-mounted workspaces survive across `docker run`s.
 ENV UV_PYTHON_INSTALL_DIR=/opt/uv/python

.devcontainer/claude-code/CLAUDE.md

@@ -33,3 +33,8 @@ In dev mode the frontend proxies `/api/*` straight to the backend (the dev-only
 handler at `web/src/app/api/[...path]/route.ts`), so **`localhost:3000` serves both the UI and
 `/api`** — no reverse proxy needed. You can also hit the backend directly at `localhost:8080` (note:
 **no** `/api` prefix there — e.g. `/health`, `/auth/type`).
+
+**Stop the dev servers when you're finished with them:**
+
+- frontend: `pkill -f "next dev"; pkill -f next-server`
+- backend: `pkill -f "uvicorn onyx.main:app"`

.mcp.json

@@ -2,7 +2,13 @@
   "mcpServers": {
     "playwright": {
       "command": "npx",
-      "args": ["@playwright/mcp@0.0.75", "--headless", "--isolated"]
+      "args": [
+        "@playwright/mcp@0.0.78",
+        "--headless",
+        "--isolated",
+        "--browser",
+        "chromium"
+      ]
     },
     "agent-wiki": {
       "type": "http",

tools/ods/cmd/web.go

@@ -1,14 +1,18 @@
 package cmd
 
 import (
+	"crypto/sha256"
+	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io/fs"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"sort"
 	"strings"
+	"time"
 
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
@@ -26,7 +30,7 @@ func NewWebCommand() *cobra.Command {
 		Use:   "web <script> [args...]",
 		Short: "Run web/package.json bun scripts",
 		Long:  webHelpDescription(),
-		Args: cobra.MinimumNArgs(1),
+		Args:  cobra.MinimumNArgs(1),
 		ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 			if len(args) > 0 {
 				return nil, cobra.ShellCompDirectiveNoFileComp
@@ -48,8 +52,7 @@ func runWebScript(args []string) {
 		log.Fatalf("Failed to find web directory: %v", err)
 	}
 
-	nodeModules := filepath.Join(webDir, "node_modules")
-	if needsInstall, reason := nodeModulesNeedsInstall(nodeModules); needsInstall {
+	if needsInstall, reason := nodeModulesNeedsInstall(webDir); needsInstall {
 		log.Infof("%s, running bun install --frozen-lockfile...", reason)
 		installCmd := exec.Command("bun", "install", "--frozen-lockfile")
 		installCmd.Dir = webDir
@@ -59,8 +62,11 @@ func runWebScript(args []string) {
 		if err := installCmd.Run(); err != nil {
 			log.Fatalf("Failed to run bun install: %v", err)
 		}
+		writeLockStamp(webDir)
 	}
 
+	ensureWorkspaceLibsBuilt(webDir)
+
 	scriptName := args[0]
 	scriptArgs := args[1:]
 	if len(scriptArgs) > 0 && scriptArgs[0] == "--" {
@@ -94,10 +100,17 @@ func runWebScript(args []string) {
 	}
 }
 
+// lockStampName is the file inside node_modules recording the sha256 of the
+// bun.lock that produced it. node_modules lives in a persistent volume in the
+// devcontainer, so it routinely outlives lockfile updates in the workspace —
+// the stamp is what lets us notice.
+const lockStampName = ".ods-bun-lock-sha256"
+
 // nodeModulesNeedsInstall reports whether bun install should be run, along with
-// a human-readable reason. Install is needed when node_modules is missing or
-// exists but is empty.
-func nodeModulesNeedsInstall(nodeModules string) (bool, string) {
+// a human-readable reason. Install is needed when node_modules is missing,
+// empty, or was installed from a different bun.lock than the current one.
+func nodeModulesNeedsInstall(webDir string) (bool, string) {
+	nodeModules := filepath.Join(webDir, "node_modules")
 	entries, err := os.ReadDir(nodeModules)
 	if errors.Is(err, os.ErrNotExist) {
 		return true, "node_modules not found"
@@ -110,9 +123,145 @@ func nodeModulesNeedsInstall(nodeModules string) (bool, string) {
 	if len(entries) == 0 {
 		return true, "node_modules is empty"
 	}
+
+	lockHash, err := fileSHA256(filepath.Join(webDir, "bun.lock"))
+	if err != nil {
+		// No lockfile to compare against; nothing more we can check.
+		return false, ""
+	}
+	stamp, err := os.ReadFile(filepath.Join(nodeModules, lockStampName))
+	if err != nil || strings.TrimSpace(string(stamp)) != lockHash {
+		return true, "node_modules is stale (bun.lock changed since last install)"
+	}
 	return false, ""
 }
 
+// writeLockStamp records the current bun.lock hash after a successful install.
+// Best-effort: a failure only means the next run reinstalls, which is safe.
+//
+// The stamp is replaced via temp file + rename rather than written in place:
+// the devcontainer's node_modules volume is shared across sessions that may
+// run as different users (root agent sessions, the dev user), and an in-place
+// write to a stamp owned by the other user fails — which would silently force
+// a reinstall on every run. Rename needs only directory write permission and
+// atomically replaces the previous owner's file.
+func writeLockStamp(webDir string) {
+	lockHash, err := fileSHA256(filepath.Join(webDir, "bun.lock"))
+	if err != nil {
+		return
+	}
+	nodeModules := filepath.Join(webDir, "node_modules")
+	stampPath := filepath.Join(nodeModules, lockStampName)
+	tmp, err := os.CreateTemp(nodeModules, lockStampName+".tmp-*")
+	if err != nil {
+		log.Debugf("Failed to create stamp temp file in %s: %v", nodeModules, err)
+		return
+	}
+	_, writeErr := tmp.WriteString(lockHash + "\n")
+	// World-readable so sessions running as other users can validate it.
+	chmodErr := tmp.Chmod(0o644)
+	closeErr := tmp.Close()
+	if writeErr != nil || chmodErr != nil || closeErr != nil {
+		_ = os.Remove(tmp.Name())
+		log.Debugf("Failed to write stamp temp file %s: %v/%v/%v", tmp.Name(), writeErr, chmodErr, closeErr)
+		return
+	}
+	if err := os.Rename(tmp.Name(), stampPath); err != nil {
+		_ = os.Remove(tmp.Name())
+		log.Debugf("Failed to replace %s: %v", stampPath, err)
+	}
+}
+
+func fileSHA256(path string) (string, error) {
+	data, err := os.ReadFile(path)
+	if err != nil {
+		return "", err
+	}
+	sum := sha256.Sum256(data)
+	return hex.EncodeToString(sum[:]), nil
+}
+
+// webLibPackages are the bun workspace packages whose exports point at their
+// dist/ build output. bun install links them into node_modules but never runs
+// their builds, so a fresh checkout (or an edit to their sources) leaves the
+// dev server failing on unresolvable exports. Order matters: opal's build
+// consumes shared's output.
+var webLibPackages = []string{"lib/shared", "lib/opal"}
+
+// ensureWorkspaceLibsBuilt builds each workspace library whose dist/ is
+// missing or older than its sources.
+func ensureWorkspaceLibsBuilt(webDir string) {
+	for _, rel := range webLibPackages {
+		pkgDir := filepath.Join(webDir, rel)
+		needsBuild, reason := libNeedsBuild(pkgDir)
+		if !needsBuild {
+			continue
+		}
+		log.Infof("web/%s %s, running bun run build...", rel, reason)
+		buildCmd := exec.Command("bun", "run", "build")
+		buildCmd.Dir = pkgDir
+		buildCmd.Stdout = os.Stdout
+		buildCmd.Stderr = os.Stderr
+		if err := buildCmd.Run(); err != nil {
+			log.Fatalf("Failed to build web/%s: %v", rel, err)
+		}
+	}
+}
+
+// libNeedsBuild reports whether a workspace library's dist/ is missing or
+// stale relative to its sources, along with a human-readable reason. The
+// staleness check compares newest mtimes, walking the package excluding its
+// build output, dependencies, and hidden entries — cheap enough (a few
+// thousand stats at most) to run before every script.
+func libNeedsBuild(pkgDir string) (bool, string) {
+	if _, err := os.Stat(pkgDir); err != nil {
+		// Not a checkout that has this package; nothing to do.
+		return false, ""
+	}
+	distNewest, err := newestMtime(filepath.Join(pkgDir, "dist"), nil)
+	if errors.Is(err, os.ErrNotExist) {
+		return true, "has no dist build"
+	}
+	if err != nil {
+		return true, fmt.Sprintf("dist is unreadable (%v)", err)
+	}
+	srcNewest, err := newestMtime(pkgDir, map[string]bool{"dist": true, "node_modules": true})
+	if err != nil {
+		return false, ""
+	}
+	if srcNewest.After(distNewest) {
+		return true, "dist build is older than its sources"
+	}
+	return false, ""
+}
+
+// newestMtime returns the newest modification time under root, skipping
+// directories named in excludeDirs and hidden entries.
+func newestMtime(root string, excludeDirs map[string]bool) (time.Time, error) {
+	var newest time.Time
+	err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		name := d.Name()
+		if path != root && (excludeDirs[name] || strings.HasPrefix(name, ".")) {
+			if d.IsDir() {
+				return filepath.SkipDir
+			}
+			return nil
+		}
+		info, err := d.Info()
+		if err != nil {
+			return err
+		}
+		if info.ModTime().After(newest) {
+			newest = info.ModTime()
+		}
+		return nil
+	})
+	return newest, err
+}
+
 func webScriptNames() []string {
 	scripts, err := loadWebScripts()
 	if err != nil {

tools/ods/cmd/web_test.go

@@ -0,0 +1,187 @@
+package cmd
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+)
+
+func writeFile(t *testing.T, path string, content string) {
+	t.Helper()
+	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(path, []byte(content), 0o644); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func setMtime(t *testing.T, path string, mtime time.Time) {
+	t.Helper()
+	if err := os.Chtimes(path, mtime, mtime); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestNodeModulesNeedsInstall(t *testing.T) {
+	now := time.Now()
+
+	t.Run("missing node_modules", func(t *testing.T) {
+		webDir := t.TempDir()
+		needs, reason := nodeModulesNeedsInstall(webDir)
+		if !needs {
+			t.Fatal("expected install for missing node_modules")
+		}
+		if reason != "node_modules not found" {
+			t.Fatalf("unexpected reason: %q", reason)
+		}
+	})
+
+	t.Run("empty node_modules", func(t *testing.T) {
+		webDir := t.TempDir()
+		if err := os.Mkdir(filepath.Join(webDir, "node_modules"), 0o755); err != nil {
+			t.Fatal(err)
+		}
+		needs, reason := nodeModulesNeedsInstall(webDir)
+		if !needs {
+			t.Fatal("expected install for empty node_modules")
+		}
+		if reason != "node_modules is empty" {
+			t.Fatalf("unexpected reason: %q", reason)
+		}
+	})
+
+	t.Run("populated without stamp is stale", func(t *testing.T) {
+		webDir := t.TempDir()
+		writeFile(t, filepath.Join(webDir, "bun.lock"), "lock-v1")
+		writeFile(t, filepath.Join(webDir, "node_modules", "pkg", "index.js"), "")
+		needs, _ := nodeModulesNeedsInstall(webDir)
+		if !needs {
+			t.Fatal("expected install when no stamp recorded")
+		}
+	})
+
+	t.Run("stamp matches lockfile", func(t *testing.T) {
+		webDir := t.TempDir()
+		writeFile(t, filepath.Join(webDir, "bun.lock"), "lock-v1")
+		writeFile(t, filepath.Join(webDir, "node_modules", "pkg", "index.js"), "")
+		writeLockStamp(webDir)
+		needs, reason := nodeModulesNeedsInstall(webDir)
+		if needs {
+			t.Fatalf("expected no install with matching stamp, got: %q", reason)
+		}
+	})
+
+	t.Run("lockfile changed after stamp", func(t *testing.T) {
+		webDir := t.TempDir()
+		writeFile(t, filepath.Join(webDir, "bun.lock"), "lock-v1")
+		writeFile(t, filepath.Join(webDir, "node_modules", "pkg", "index.js"), "")
+		writeLockStamp(webDir)
+		writeFile(t, filepath.Join(webDir, "bun.lock"), "lock-v2")
+		setMtime(t, filepath.Join(webDir, "bun.lock"), now)
+		needs, _ := nodeModulesNeedsInstall(webDir)
+		if !needs {
+			t.Fatal("expected install after lockfile change")
+		}
+	})
+
+	t.Run("stamp replaces an existing file it cannot write in place", func(t *testing.T) {
+		// Sessions run as different users against the shared node_modules
+		// volume, so the previous stamp may not be writable — only
+		// replaceable via the directory. Simulate with a read-only stamp.
+		webDir := t.TempDir()
+		writeFile(t, filepath.Join(webDir, "bun.lock"), "lock-v2")
+		writeFile(t, filepath.Join(webDir, "node_modules", "pkg", "index.js"), "")
+		stampPath := filepath.Join(webDir, "node_modules", lockStampName)
+		writeFile(t, stampPath, "stale-hash")
+		if err := os.Chmod(stampPath, 0o444); err != nil {
+			t.Fatal(err)
+		}
+		writeLockStamp(webDir)
+		needs, reason := nodeModulesNeedsInstall(webDir)
+		if needs {
+			t.Fatalf("expected stamp to be replaced despite read-only file, got: %q", reason)
+		}
+	})
+
+	t.Run("no lockfile skips staleness check", func(t *testing.T) {
+		webDir := t.TempDir()
+		writeFile(t, filepath.Join(webDir, "node_modules", "pkg", "index.js"), "")
+		needs, _ := nodeModulesNeedsInstall(webDir)
+		if needs {
+			t.Fatal("expected no install without a lockfile to compare")
+		}
+	})
+}
+
+func TestLibNeedsBuild(t *testing.T) {
+	old := time.Now().Add(-time.Hour)
+	older := old.Add(-time.Hour)
+
+	t.Run("missing package is skipped", func(t *testing.T) {
+		needs, _ := libNeedsBuild(filepath.Join(t.TempDir(), "lib", "nope"))
+		if needs {
+			t.Fatal("expected missing package to be skipped")
+		}
+	})
+
+	t.Run("missing dist", func(t *testing.T) {
+		pkgDir := t.TempDir()
+		writeFile(t, filepath.Join(pkgDir, "src", "index.ts"), "")
+		needs, reason := libNeedsBuild(pkgDir)
+		if !needs {
+			t.Fatal("expected build for missing dist")
+		}
+		if reason != "has no dist build" {
+			t.Fatalf("unexpected reason: %q", reason)
+		}
+	})
+
+	t.Run("dist newer than sources", func(t *testing.T) {
+		pkgDir := t.TempDir()
+		src := filepath.Join(pkgDir, "src", "index.ts")
+		dist := filepath.Join(pkgDir, "dist", "index.js")
+		writeFile(t, src, "")
+		writeFile(t, dist, "")
+		setMtime(t, src, older)
+		setMtime(t, filepath.Join(pkgDir, "src"), older)
+		setMtime(t, pkgDir, older)
+		needs, reason := libNeedsBuild(pkgDir)
+		if needs {
+			t.Fatalf("expected no build when dist is fresh, got: %q", reason)
+		}
+	})
+
+	t.Run("source newer than dist", func(t *testing.T) {
+		pkgDir := t.TempDir()
+		src := filepath.Join(pkgDir, "src", "index.ts")
+		dist := filepath.Join(pkgDir, "dist", "index.js")
+		writeFile(t, src, "")
+		writeFile(t, dist, "")
+		setMtime(t, dist, older)
+		setMtime(t, filepath.Join(pkgDir, "dist"), older)
+		setMtime(t, src, old)
+		needs, _ := libNeedsBuild(pkgDir)
+		if !needs {
+			t.Fatal("expected build when a source is newer than dist")
+		}
+	})
+
+	t.Run("changes under excluded dirs are ignored", func(t *testing.T) {
+		pkgDir := t.TempDir()
+		src := filepath.Join(pkgDir, "src", "index.ts")
+		dist := filepath.Join(pkgDir, "dist", "index.js")
+		dep := filepath.Join(pkgDir, "node_modules", "dep", "index.js")
+		writeFile(t, src, "")
+		writeFile(t, dist, "")
+		writeFile(t, dep, "")
+		setMtime(t, src, older)
+		setMtime(t, filepath.Join(pkgDir, "src"), older)
+		setMtime(t, pkgDir, older)
+		needs, reason := libNeedsBuild(pkgDir)
+		if needs {
+			t.Fatalf("expected node_modules churn to be ignored, got: %q", reason)
+		}
+	})
+}