feat(app): lifo agent popout with noVNC watch surface

[Dexploarer]

Summary

Introduces the Lifo Agent Popout — a dual-surface system for real-time observation and control of autonomous agent sandbox activity.

Core Feature

• Controller Popout — Dedicated browser popup (1400×860) running a full @lifo-sh runtime with interactive terminal + file explorer. Agent commands execute here in real time.
• Watcher Tab — Read-only embedded /lifo tab under Advanced that mirrors controller output via BroadcastChannel, polls sandbox screenshots (1.8s), and renders live noVNC or screenshot fallback.
• Auto-trigger — useLifoAutoPopout hook listens for WS events containing keywords (lifo, browser, playwright, computeruse, novnc, etc.) and auto-opens the popout.
• Electron PIP — lifo:setPip / lifo:getPipState IPC channels for always-on-top picture-in-picture mode with validated level allowlist.

Security Hardening (review-driven)

• Validate noVNC iframe src against http/https protocol allowlist; removed allow-same-origin from sandbox attribute
• IPC lifo:setPip level validated against explicit VALID_PIP_LEVELS set
• Unified did-create-window handler for both lifo and stream popouts (prevents competing capture targets)
• BroadcastChannel uses session-nonce-suffixed name to prevent same-origin message injection
• Proper xterm terminal disposal before DOM teardown
• Popout window reference nulled on beforeunload
• Removed premature client.connectWs() from hooks
• Screenshot polling pauses when tab is hidden (visibilitychange)
• Failed popup attempts no longer tombstone runId (allows retry after unblock)

Also Included

• CI/build fixes: tsdown resolution, electron tsc-out copy, bun.lock, biome formatting
• Security: stdio MCP config terminal token gate, secret env interpolation blocking, custom-action IP pinning
• Test coverage: e2e auth, MCP config, plugins, subscriptions, connectors, twitter-verify, CUA boundary, x402 boundary
• Docker test scripts cleanup (removed legacy shell-based e2e)
• Sandbox manager: browser container CDP/WS/noVNC endpoint support
• Command palette: added nav-lifo entry

Test plan

• All 83 app test files pass (629 tests)
• TypeScript compilation clean (tsc --noEmit)
• Biome lint clean on all modified files
• lifo-auto-popout.test.ts — auto-trigger, keyword matching, popup-blocked retry
• lifo-popout-init.test.ts — popout URL detection and mode parsing
• lifo-ipc-channels.test.ts — IPC channel allowlist verification
• navigation.test.tsx — lifo tab routing
• Manual: open popout from Advanced > Lifo tab, verify BroadcastChannel sync
• Manual: verify PIP toggle in Electron desktop build
• Manual: confirm noVNC iframe loads only for valid http/https endpoints

🤖 Generated with Claude Code

[github-actions]

PR #628 Review: feat(app): lifo agent popout with noVNC watch surface

1. Classification: feature

This is a significant new feature — a dual-surface (controller popout + watcher tab) system for observing and controlling autonomous agent sandbox activity via a Lifo runtime, with noVNC live desktop mirroring and screenshot fallback.

Pre-classification was "bugfix" — this is incorrect. This is clearly a feature PR (+4625/-6525 lines, 2 new npm dependencies, 903-line new component, new IPC channels, new API surface).

2. Scope verdict: needs deep review

This is a substantial feature touching Electron IPC, sandbox APIs, new npm dependencies, BroadcastChannel cross-tab comms, iframe embedding, and auto-triggered popup windows. It sits at the intersection of "new feature" and "new integration" — both of which require deep review.

3. Code quality: issues found

3a. LifoSandboxView.tsx is 903 lines — exceeds the ~500 LOC guideline.
This single component contains 15 state/ref declarations, 3 memos, 8 callbacks, 7 effects, and ~270 lines of JSX. It should be decomposed:

• Extract createLifoRuntime to lifo-runtime.ts
• Extract the monitor/screen-preview panel to a LifoMonitorPanel sub-component
• Extract the BroadcastChannel sync effect to a useLifoSync hook

3b. WebSocket effect re-subscribes on every heartbeat toggle.
In LifoSandboxView.tsx, the WS event listener effect (around line 550 of the component) has controllerOnline in its dependency array, causing teardown/re-subscribe every time the controller goes online/offline. This risks missed events. Read controllerOnline from a ref instead.

3c. useLifoAutoPopout effect churn from unstable onPopupBlocked callback.
The inline arrow function onPopupBlocked: () => { setActionNotice(...) } in App.tsx creates a new reference every render, causing the hook's main effect to teardown and re-subscribe WS listeners on every render. Either useCallback the handler or use a ref pattern inside the hook.

3d. @lifo-sh/ui version is ^0.4.2 (caret range) while @lifo-sh/core is pinned to 0.4.4.
Mixed version pinning strategies. The @lifo-sh/ui caret range means a future 0.5.0 (or 0.4.3 with breaking changes under pre-1.0 semver) could land via bun install. Both should be exact-pinned.

3e. react-test-renderer.d.ts — custom type declaration for a well-known package.
Why not install @types/react-test-renderer? Custom ambient declarations are harder to maintain and may drift from the actual API.

4. Security: concerns

4a. Auto-popout keyword matching is too broad (MEDIUM).
The regex LIFO_EVENT_KEYWORD_RE matches browser, sandbox, and cdp — extremely common terms that appear in agent events unrelated to computer-use. Combined with the null-runId bypass (which skips the once-per-run throttle entirely), an agent event stream containing common words like "browser" without a runId will open popups on every matching event with zero deduplication. This is both a UX annoyance and a potential attack vector for prompt injection that triggers popup storms.

Recommendations:

• Require at least two keywords or add a mandatory anchor keyword (e.g., lifo or computeruse)
• Add a time-based cooldown (e.g., 5s) for null-runId events
• Cap triggeredRunIdsRef size (currently grows unbounded unlike seenEventIdsRef)

4b. buildLifoPopoutUrl does not validate inputs (MEDIUM).
The baseUrl parameter flows into new URL(baseUrl || window.location.href) — if this function is reused with user-controlled input, it could redirect the popup to an arbitrary origin. The file: protocol branch interpolates targetPath and sessionId without encoding. Currently safe in the sole call site, but the function's interface is fragile.

4c. enableNoVnc defaults to true (LOW-MEDIUM).
Any existing deployment with browser.enabled: true that upgrades to this version will automatically start a noVNC server and expose port 6080. The default should be false to avoid surprising operators with a new network-accessible service.

4d. Container port mapping semantic change is potentially breaking (LOW-MEDIUM).
Previously: { host: cdpPort, container: 9222 } (hardcoded well-known port inside container).
Now: { host: cdpPort, container: cdpPort } (dynamic, requires container image to honor MILADY_BROWSER_*_PORT env vars).

If the browser container image doesn't read these env vars, non-default port configurations silently break. This needs to be coordinated with a container image update.

4e. iframe sandbox attributes are well-configured. sandbox="allow-scripts allow-forms allow-pointer-lock" correctly omits allow-same-origin and allow-top-navigation. The isSafeEndpointUrl URL validation properly restricts to http: / https: protocols. The VALID_PIP_LEVELS allowlist in Electron IPC is correctly implemented. These are positives.

4f. New dependencies are clean. @lifo-sh/core@0.4.4 has zero dependencies and no install scripts. @lifo-sh/ui@0.4.2 depends on @xterm/* packages (well-known, clean). No supply chain concerns.

5. Tests: missing critical coverage

The PR includes 11 test files, but several are thin or miss security-sensitive code paths:

Untested (critical gaps):

• isLifoPopoutUrl in setup.ts — The URL parser that determines child window PIP behavior has ZERO test coverage. This is security-sensitive (malformed URLs could bypass PIP configuration).
• VALID_PIP_LEVELS validation — The IPC handler falls back to "floating" for invalid levels. This security boundary has ZERO test coverage.
• isSafeEndpointUrl in LifoSandboxView.tsx — The iframe src protocol validator has ZERO test coverage.
• lifo-popout.ts utility functions — isLifoPopoutValue, isLifoPopoutModeAtLocation, getPopoutValueFromLocation, buildLifoPopoutUrl, getLifoSessionIdFromLocation have NO dedicated unit tests. Only indirectly exercised through integration tests.
• Keyword matching exhaustiveness — LIFO_EVENT_KEYWORD_RE has 13 keywords, but the classifier tests check only 1 positive and 1 negative case. The normalization pipeline (normalizeSearchableText, collectPayloadStrings) is untested.

Thin/borderline filler:

• lifo-ipc-channels.test.ts (14 lines) — Only checks array membership. The actual IPC handler logic (~80 lines in setup.ts) is untested.
• sandbox-api-client.test.ts — Tests HTTP fetch plumbing but not error handling or edge cases.

Missing from backend:

• headless: true path for getBrowserNoVncEndpoint (should return null)
• Default port values when none are configured
• Conditional noVNC port mapping when disabled

6. Decision: REQUEST CHANGES

This is a well-architected feature from a legendary contributor, with solid security hardening (iframe sandbox, protocol allowlists, IPC level validation, nonce-suffixed BroadcastChannel). However, it needs work before merge:

Must fix:

1. LifoSandboxView.tsx exceeds 500 LOC — decompose into sub-components and extracted hooks/utilities
2. Auto-popout keyword matching is too broad — "browser" and "sandbox" will cause false positives in normal agent operation. Require a mandatory anchor keyword or at least two keyword matches
3. Null-runId bypass needs throttling — add a time-based cooldown to prevent popup storms
4. Add unit tests for lifo-popout.ts — all 6 exported utility functions need dedicated tests, especially buildLifoPopoutUrl and isLifoPopoutModeAtLocation
5. Add unit tests for isSafeEndpointUrl and VALID_PIP_LEVELS validation — these are security boundaries
6. Add tests for isLifoPopoutUrl in setup.ts — the URL parsing logic needs coverage for both pathname and hash routing, and for edge cases (no popout param, invalid values)
7. Default enableNoVnc to false — avoid auto-exposing a new network service on upgrade

Should fix:
8. Pin @lifo-sh/ui to exact version (0.4.2 not ^0.4.2)
9. Fix the WS effect dependency on controllerOnline — use a ref to avoid re-subscription on heartbeat toggles
10. Fix onPopupBlocked callback stability — wrap in useCallback or use a ref pattern
11. Verify the container port mapping change is coordinated with a container image update that honors MILADY_BROWSER_*_PORT env vars

[github-actions]

Review posted. Here's the summary:

---

PR #628: feat(app): lifo agent popout with noVNC watch surface

Decision: REQUEST CHANGES

This is a well-architected feature from a legendary (100/100) contributor with solid security fundamentals — properly sandboxed iframes, protocol allowlists, IPC level validation, and nonce-suffixed BroadcastChannels. The @lifo-sh dependencies are clean (no install scripts, no hidden deps). However, 7 must-fix items prevent merge:

1. 903-line component exceeds the ~500 LOC guideline — needs decomposition
2. Auto-popout keyword matching too broad — "browser" and "sandbox" will cause false-positive popups during normal agent operation
3. Null-runId bypass has no throttling, enabling popup storms
4. Critical security boundaries are untested — isSafeEndpointUrl, VALID_PIP_LEVELS, isLifoPopoutUrl, and all lifo-popout.ts utilities lack unit tests
5. enableNoVnc defaults to true — would auto-expose port 6080 on upgrade
6. Mixed dependency pinning — @lifo-sh/ui uses caret range, should be exact-pinned
7. Container port mapping change from hardcoded to dynamic ports is potentially breaking without a coordinated container image update

[github-actions]

PR #628 Review: feat(app): lifo agent popout with noVNC watch surface

1. Classification: feature (not bugfix as pre-classified)

This introduces the Lifo Agent Popout -- a dual-surface system (controller popout + watcher tab) for real-time observation of autonomous agent sandbox activity, with noVNC live mirroring and screenshot fallback.

2. Scope verdict: in scope (needs deep review)

This is a significant new feature adding a real-time agent observation surface. It directly serves the core mission -- watching and controlling autonomous agent behavior. The feature includes security hardening, test coverage, and proper decomposition. New dependencies (@lifo-sh/core, @lifo-sh/ui) are justified by the terminal/VFS runtime the feature requires.

3. Code quality: pass (with notes)

Positives:

• Well-decomposed: LifoSandboxView (587 LOC) was further split into LifoMonitorPanel, useLifoSync, and lifo-runtime module
• No any types found -- type casts are minimal and justified (as Partial<LifoSyncMessage> for BroadcastChannel data, etc.)
• Proper cleanup patterns throughout (cancelled flags, interval clearing, event listener removal, terminal disposal before DOM teardown)
• Dynamic imports for @lifo-sh/core and @lifo-sh/ui enable code splitting
• Auto-popout has sensible guardrails: strong/weak keyword classification, once-per-runId dedup, 256-entry runId cap, 5s null-runId cooldown, disabled in popout mode

Notes:

• LifoSandboxView.tsx at 587 LOC is slightly over the ~500 LOC guideline. Acceptable given the decomposition already done.
• xterm.css (240 LOC) is vendored directly rather than imported from @xterm/xterm/css/xterm.css. This is a common workaround for bundler issues -- not a blocker.
• The port mapping change in sandbox-manager.ts (from hardcoded container: 9222/container: 5900 to dynamic container: cdpPort/container: vncPort) is a behavioral change requiring the browser container image to honor the MILADY_BROWSER_* env vars. Ensure the container image is updated to match.

4. Security: clear

Thorough hardening observed:

• iframe sandbox: allow-scripts allow-forms allow-pointer-lock WITHOUT allow-same-origin -- correct for untrusted noVNC content
• URL validation: isSafeEndpointUrl() validates http/https protocol before rendering iframe src -- blocks javascript:, data:, ftp: and malformed URLs
• IPC PIP levels: Explicit VALID_PIP_LEVELS allowlist with fallback to "floating" for unrecognized values
• BroadcastChannel: Session-nonce-suffixed channel name (milady-lifo-sync-{sessionId}) via crypto.getRandomValues prevents same-origin message injection
• noVNC endpoint: Constructed server-side with view_only=true and localhost binding; guarded by enableNoVnc flag (default: false) and headless check
• Screenshot rendering: data:image/png;base64,... in <img> tag is XSS-safe (img elements can't execute scripts)
• No postinstall scripts in @lifo-sh/core or @lifo-sh/ui; @lifo-sh/core has zero runtime dependencies; @lifo-sh/ui depends only on well-known xterm.js packages
• Dependencies pinned to exact versions (0.4.4 and 0.4.2)

5. Tests: adequate (with caveats)

Coverage provided (8 new + 7 modified test files):

• lifo-popout-utils.test.ts -- Excellent pure-function coverage of all exported utilities
• lifo-safe-endpoint.test.ts -- Complete coverage of isSafeEndpointUrl including injection vectors
• lifo-auto-popout.test.ts -- Tests hook behavior (once-per-run, focus existing, popup-blocked retry) and both classifier functions
• lifo-popout-init.test.ts -- Tests popout startup fast-path (skip onboarding, set lifo tab, connect WS)
• sandbox-api-client.test.ts -- Tests all three new API client methods with mock fetch
• sandbox-manager.test.ts -- Tests getBrowserNoVncEndpoint(), noVNC port wiring, env var pass-through
• e2e tests updated: startup-chat verifies popout shell rendering and auto-popout enabled flag state
• Navigation tests confirm lifo tab integration

Test quality caveats:

1. Duplicated-logic tests: lifo-pip-levels.test.ts and lifo-popout-url.test.ts copy-paste production logic from setup.ts into the test file and validate the copy. If setup.ts diverges, these tests will silently pass. Ideally, extract isLifoPopoutUrl, isLifoPopoutFlag, and VALID_PIP_LEVELS/resolvePipLevel from setup.ts into importable modules so tests can import the real code. This is the biggest test quality concern in the PR.
2. Missing useLifoSync test: No dedicated test file for the sync hook (BroadcastChannel message handling, heartbeat logic, command mirroring).
3. Missing negative paths: No enabled=false test for auto-popout hook; no error-response tests for sandbox API client; no headless: true test for getBrowserNoVncEndpoint().

6. Decision: APPROVE

This is a well-architected feature from a legendary contributor (100/100 trust). The dual-surface design (controller + watcher with BroadcastChannel sync) is sound. Security hardening is comprehensive -- iframe sandboxing, URL validation, IPC allowlists, session nonces, and sensible defaults (noVNC off by default). The test coverage is substantial for a feature of this complexity.

Recommendations for follow-up (non-blocking):

• Extract isLifoPopoutUrl/VALID_PIP_LEVELS logic from setup.ts into importable modules so electron tests validate real code instead of copies
• Add useLifoSync hook tests
• Add headless: true negative test for getBrowserNoVncEndpoint()
• Consider importing xterm.css from the npm package (@xterm/xterm/css/xterm.css) instead of vendoring

The bun.lock has a large delta (-6497/+2310) from plugin workspace cleanup that is orthogonal to this feature. It makes the diff noisy but is not harmful.

[github-actions]

Code Review: PR #628 — feat(app): lifo agent popout with noVNC watch surface

1. Classification: feature (not bugfix as pre-classified)

New dual-surface system for real-time observation of autonomous agent sandbox activity: a popout controller window with terminal/file explorer, and an embedded watcher tab with noVNC/screenshot mirroring.

2. Scope verdict: in scope

This feature directly serves the agent mission — it enables real-time observation of autonomous agent computer-use activity, which is core agent capability infrastructure, not aesthetic. The sandbox-manager/sandbox-routes backend changes expose noVNC endpoints needed for this surface.

3. Code quality: pass (minor notes)

Positives:

• Good decomposition: lifo-popout.ts (utilities), lifo-runtime.ts (runtime wrapper), useLifoAutoPopout (auto-trigger hook), useLifoSync (BroadcastChannel sync), LifoMonitorPanel (extracted component)
• Clean separation of controller (popout) vs watcher (embedded) modes
• Proper cleanup patterns: cancelled flags, effect teardowns, terminal disposal before DOM teardown
• Visibility-change-based polling pause to avoid background work
• Conservative defaults (enableNoVnc: false)

Minor notes:

• LifoSandboxView.tsx at 587 LOC is above the 500 LOC guideline (decomposed from 903; acknowledged)
• xterm.css (240 lines) is vendored from xterm.js — could be imported from @xterm/xterm/css/xterm.css in the future, but acceptable since @lifo-sh/ui may need specific version pinning
• lifo-pip-levels.test.ts and lifo-popout-url.test.ts duplicate source logic since Electron setup.ts functions aren't exported — acknowledged in test comments, acceptable tradeoff

One behavioral change to note: Browser container port mapping changed from fixed container ports ({ host: cdpPort, container: 9222 }) to dynamic ({ host: cdpPort, container: cdpPort }). This is correct for the new env-variable-driven approach (container reads MILADY_BROWSER_CDP_PORT etc.), but means the container image must honor those env vars. Since the default ports are unchanged (9222/5900), standard users are unaffected.

4. Security: clear

Thorough hardening throughout:

• noVNC iframe: src validated against http:/https: protocol allowlist via isSafeEndpointUrl(); allow-same-origin removed from sandbox attribute
• IPC PIP level: validated against explicit VALID_PIP_LEVELS set; unknown levels fall back to "floating"
• BroadcastChannel: session-nonce suffix prevents same-origin message injection across tabs
• URL detection: isLifoPopoutUrl() requires both /lifo path AND valid popout param — prevents URL confusion
• Auto-popout keywords: strong/weak classification prevents false triggers; bounded recursion (depth 3) in payload scanning; 5s null-runId cooldown; 256-entry runId cap

Dependencies verified:

• @lifo-sh/core@0.4.4: 0 runtime deps, no install scripts, 504KB, integrity hash matches npm registry
• @lifo-sh/ui@0.4.2: only @xterm/* deps, no install scripts, 43KB, integrity hash matches npm registry
• Both pinned to exact versions, same maintainer (sanketsahu), signed packages

5. Tests: adequate

Extensive coverage across 13 new/modified test files:

• lifo-popout-utils.test.ts (208 lines): popout value detection, URL building, session IDs, sync channel naming
• lifo-safe-endpoint.test.ts: protocol allowlist validation (http, https, javascript:, data:, ftp:)
• lifo-auto-popout.test.ts (217 lines): hook behavior, keyword classifiers, popup-blocked retry
• lifo-popout-init.test.ts: popout startup fast-path, onboarding bypass
• sandbox-api-client.test.ts: browser/screenshot/windows API methods
• lifo-ipc-channels.test.ts, lifo-pip-levels.test.ts, lifo-popout-url.test.ts: Electron IPC validation
• Updated e2e smoke, startup, and navigation tests
• sandbox-manager.test.ts: noVNC config, port wiring, disabled state

6. Decision: APPROVE

Well-crafted feature from a legendary contributor. The code is clean, security hardening is proactive, test coverage is thorough, and dependencies are verified. The feature aligns with the project mission of enhancing autonomous agent capabilities. Minor notes above are non-blocking.

[github-actions]

Review submitted. Here's a summary:

PR #628: feat(app): lifo agent popout with noVNC watch surface

Decision: APPROVE

Key findings:

• Classification: This is a feature (not bugfix as pre-classified) -- introduces a dual-surface system for real-time agent sandbox observation
• Scope: In scope -- directly serves the core mission of watching autonomous agent behavior
• Code quality: Pass. Well-decomposed (587 LOC main component, extracted into 4 modules), no any types, proper resource cleanup, dynamic imports for code splitting
• Security: Clear. Comprehensive hardening -- iframe sandbox without allow-same-origin, URL protocol allowlist, IPC level validation, session-nonce BroadcastChannel, noVNC off by default, no postinstall scripts in new deps, dependencies pinned to exact versions
• Tests: Adequate with 8 new + 7 modified test files. Main caveat: two electron tests validate copy-pasted logic rather than actual production code from setup.ts

Non-blocking recommendations were noted for follow-up (extracting Electron logic into importable modules, adding useLifoSync tests, filling missing negative-path tests).

[github-actions]

Review posted. Here's the summary:

Decision: APPROVE

This is a well-crafted new feature (not a bugfix as pre-classified) from a legendary contributor (trust: 100/100). The PR adds a dual-surface system for real-time observation of autonomous agent sandbox activity:

• Controller popout — dedicated browser popup with full @lifo-sh terminal + file explorer
• Watcher tab — embedded read-only view under Advanced with noVNC/screenshot mirroring
• Auto-trigger — keyword-based detection of computer-use WS events to auto-open the popout
• Electron PIP — IPC channels for always-on-top picture-in-picture mode

Key review findings:

• Security: Thorough hardening — iframe URL validation, PIP level allowlist, BroadcastChannel session nonce, keyword classifier with strong/weak classification to prevent false triggers
• Dependencies: @lifo-sh/core@0.4.4 and @lifo-sh/ui@0.4.2 verified on npm — zero install scripts, integrity hashes match, pinned to exact versions
• Tests: 13 new/modified test files with extensive coverage of utilities, hooks, IPC, and e2e navigation
• Code quality: Clean decomposition, proper cleanup patterns, conservative defaults

Minor notes flagged (non-blocking): LifoSandboxView at 587 LOC (above 500 guideline), vendored xterm.css, and a behavioral change in browser container port mapping (safe with defaults).

[github-actions]

PR #628 Review: feat(app): lifo agent popout with noVNC watch surface

1. Classification: feature

This is a significant new feature introducing a dual-surface system (controller popout + watcher tab) for real-time observation of autonomous agent sandbox activity, with noVNC integration and screenshot fallback. The PR description says "bugfix" but this is clearly a new feature — the entire Lifo subsystem is new code.

2. Scope verdict: needs deep review

This adds a new plugin/integration (@lifo-sh/core, @lifo-sh/ui), new Electron IPC channels, new frontend pages/components, new API routes, new dependencies, and architectural decisions. Squarely in the "requires deep review" category.

3. Code quality: issues found

Positives:

• Well-decomposed: LifoSandboxView (587 LOC) was decomposed from an earlier 903 LOC version into LifoMonitorPanel, useLifoSync, and lifo-runtime modules
• Good TypeScript discipline — no gratuitous any types
• VALID_PIP_LEVELS allowlist is solid
• Session nonce on BroadcastChannel prevents same-origin injection
• Keyword matching uses a strong/weak classification to reduce false positive popouts
• Proper cleanup patterns (cancelled flags, interval cleanup, terminal disposal before DOM teardown)

Issues:

1. LifoSandboxView.tsx at 587 LOC is borderline. It's acceptable given it's a primary view component, but the prop-drilling to LifoMonitorPanel (15 props!) suggests an intermediate context or reducer would be cleaner.

2. xterm.css (240 LOC) is vendored directly. This CSS comes from @xterm/xterm which is already a transitive dependency via @lifo-sh/ui. Why not import from the package? Vendoring creates a maintenance burden and drift risk.

3. Stale bun.lock changes. The diff includes unrelated lockfile changes: sharp downgrade from 0.34.5 to 0.33.5, addition of onnxruntime-node, removal of @elizaos/core from electron deps, and version bump from alpha.27 to alpha.52. These are either from a bad rebase or bundling unrelated work into this PR. This is concerning — lockfile changes should be minimal and traceable.

4. react-test-renderer.d.ts type declaration file. This adds a full type declaration for react-test-renderer. If the project uses this library, proper @types/react-test-renderer should be used as a devDependency instead of hand-rolled declarations.

5. lifo-pip-levels.test.ts and lifo-popout-url.test.ts duplicate production logic inline since the functions are defined inside ElectronCapacitorApp.init() and can't be imported. These tests validate a hand-copied version of the logic, not the actual code. If the production code drifts, these tests become useless. The validation functions (isLifoPopoutUrl, isLifoPopoutFlag, VALID_PIP_LEVELS) should be extracted to importable modules so they can be properly unit-tested.

6. enableNoVnc defaults to false in sandbox-manager but the bun.lock diff shows enableNoVnc could be undefined. The ?? fallback chain in eliza.ts ((browserSettings.enableNoVnc as boolean) ?? undefined) passes through undefined, which is fine since SandboxManager defaults it, but the type assertion as boolean is misleading when the value could be any type from settings.

4. Security: concerns

Good security measures:

• noVNC iframe src validated against http/https protocol allowlist via isSafeEndpointUrl()
• allow-same-origin removed from iframe sandbox attribute (good — prevents iframe from accessing parent origin)
• IPC lifo:setPip level validated against explicit VALID_PIP_LEVELS set
• BroadcastChannel uses session-nonce-suffixed name
• ipcMain.removeHandler before ipcMain.handle prevents double-registration

Concerns:

1. New external dependencies @lifo-sh/core@0.4.4 and @lifo-sh/ui@0.4.2 are supply chain risk. These packages are:

   • Published by a single maintainer (sanketsahu)
   • Very new (only 11 versions of core, 6 of UI)
   • @lifo-sh/core is 504 KB unpacked with zero dependencies
   • @lifo-sh/ui brings in @xterm/addon-webgl, @xterm/addon-fit, @xterm/xterm

   While there are no postinstall scripts and the packages are MIT licensed, adding a browser-embedded shell runtime from a low-history npm scope is a non-trivial supply chain decision. What is the relationship between this project and the @lifo-sh packages? Is this a known/endorsed integration?

2. createLifoRuntime() in lifo-runtime.ts boots a kernel with persistence (persist: true), sources shell config files, and provides curl and node commands. This is a full shell environment running in the browser. What are the security boundaries? Can the agent execute arbitrary commands through this that could affect the host system, or is this fully sandboxed within the browser's JS runtime?

3. Auto-popout mechanism (useLifoAutoPopout) listens to ALL WebSocket events and opens popup windows based on keyword matching. The keywords include generic terms like "sandbox", "cdp", "browser", "chromium". While the weak keyword threshold (2+) helps, this could cause unexpected popup behavior for users who aren't expecting it. The popup is opened automatically without user consent beyond initially allowing popups.

4. The collectPayloadStrings function recursively traverses event payloads up to depth 3. An adversarial agent event with deeply nested data could cause unexpected keyword matches triggering popouts.

5. Tests: adequate with caveats

Good coverage:

• lifo-auto-popout.test.ts — 217 lines covering auto-trigger, keyword matching, popup-blocked retry
• lifo-popout-init.test.ts — AppContext integration for popout startup
• lifo-popout-utils.test.ts — 208 lines covering all utility functions
• lifo-safe-endpoint.test.ts — URL validation for noVNC endpoints
• lifo-ipc-channels.test.ts — IPC channel registration
• navigation.test.tsx and triggers-navigation.test.ts — routing updates
• sandbox-api-client.test.ts — API client methods
• sandbox-manager.test.ts — noVNC endpoint and container config
• sandbox-routes.test.ts — route handler updates
• E2E tests updated for navigation and startup

Caveats:

• Electron-side PIP level and popout URL tests duplicate production logic rather than testing the actual code (as noted above)
• No integration test for the BroadcastChannel sync between controller and watcher
• No test for the createLifoRuntime() function itself (would require mocking @lifo-sh/core and @lifo-sh/ui)

6. Decision: REQUEST CHANGES

This is well-engineered work from a legendary contributor and the security hardening is thorough. However, there are blockers that need addressing before merge:

Must fix:

1. Extract isLifoPopoutUrl, isLifoPopoutFlag, and VALID_PIP_LEVELS from ElectronCapacitorApp.init() into importable modules so the test files (lifo-pip-levels.test.ts, lifo-popout-url.test.ts) test the actual production code, not hand-copied duplicates.

2. Clean up the bun.lock. The sharp downgrade (0.34.5 → 0.33.5), onnxruntime-node addition, @elizaos/core removal from electron, and version bumps are unrelated to this feature. Either rebase cleanly against develop or explain why these changes are necessary.

3. Justify the @lifo-sh dependency. Provide context on the relationship between this project and the @lifo-sh packages. A new npm scope with a single maintainer and ~2 weeks of history needs justification for inclusion in a security-conscious project.

Should fix:
4. Import xterm CSS from the @xterm/xterm package rather than vendoring it.
5. Use @types/react-test-renderer instead of the hand-rolled .d.ts file.
6. Consider a feature flag or user setting for auto-popout behavior rather than always-on keyword matching.

[github-actions]

Review posted. Here's a summary:

PR #628 Review: REQUEST CHANGES

This is a well-engineered new feature from a legendary contributor (trust 100/100), introducing a Lifo Agent Popout system with noVNC watch surface for real-time agent sandbox observation. The security hardening is thorough (iframe URL validation, IPC level allowlists, BroadcastChannel session nonces, proper cleanup).

Three blockers requiring changes:

1. Test code duplicates production logic — VALID_PIP_LEVELS, isLifoPopoutUrl, and isLifoPopoutFlag are defined inside ElectronCapacitorApp.init() and hand-copied into test files. These must be extracted to importable modules so tests verify actual production code.

2. Unrelated bun.lock changes — sharp downgrade (0.34.5→0.33.5), onnxruntime-node addition, @elizaos/core removal from electron deps, and version bumps from alpha.27 to alpha.52 need to be explained or cleaned up.

3. @lifo-sh dependency justification needed — New npm scope with a single maintainer and ~2 weeks of publication history requires context on the relationship to this project before inclusion.