Skip to content

feat(browser): add durable native-window capability namespaces - #648

Open
steipete wants to merge 14 commits into
codex/installed-app-parity-after-browser-645from
codex/native-window-binding-after-parity-1-38
Open

feat(browser): add durable native-window capability namespaces#648
steipete wants to merge 14 commits into
codex/installed-app-parity-after-browser-645from
codex/native-window-binding-after-parity-1-38

Conversation

@steipete

Copy link
Copy Markdown
Collaborator

Summary

  • bind caller-scoped opaque Chrome pages to exact native PID-generation/window receipts and revalidate the binding immediately before every mutating provider call
  • add authenticated Bridge 1.38 browser namespaces with durable owner-private CLI receipts, closed high-level actions, per-namespace runtime/provider ownership, bounded lifecycle drain, and retryable idempotent close
  • retain a read-only native CDP control session for exact tab/window correlation without exposing raw provider target, browser, WebSocket, or window identifiers
  • preserve background delivery by default while signing explicitly authorized foreground browser effects and partial/indeterminate results truthfully

Durable CLI namespace calls require the exact issuing --bridge-socket on every invocation. Explicit DevTools endpoints receive only opaque namespace/generation/connection-receipt-digest attribution. Namespace operations disappear from negotiation if runtime preparation fails.

Stack and proof

This PR is stacked directly on #647 at exact base d4d1902823fff0d8edf640f5b74358bf537c22ba. Nine of the twelve replayed native/Bridge commits are exact git range-diff = matches. The three differing commits contain only reviewed additive unions for the inherited changelog, installed-application client capability negotiation, and ScreenCaptureKit ownership preparation plus Bridge lifecycle generation. Follow-up commit 0ec6ae75f0a470df03117a03c25a694036be4d88 closes the P0–P2 findings found by current-stack review.

Source-only proof:

  • all changed Swift files pass parser checks
  • canonical recursive SwiftFormat reports 0/1742 files requiring changes
  • SwiftLint exits successfully with warnings only and 0 serious violations
  • diff/conflict checks and the DevTools contract script syntax pass
  • background guidance passes 11/11 and docs-site structure passes 2/2
  • final current-byte P0–P2 Autoreview is clean
  • Peter author/committer identity and all five unchanged submodule gitlinks are verified

The repository's normal macOS and CodeQL workflows are main-only and are intentionally suppressed while this stacked PR targets codex/installed-app-parity-after-browser-645; they will not run on this branch relationship. No Swift/Xcode build or UI test was run for this source-only replay, as requested. No AppleScript/JXA/OSA or virtualization was used.

@clawsweeper

clawsweeper Bot commented Aug 27, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@cursor

cursor Bot commented Aug 27, 2026

Copy link
Copy Markdown

PR Summary

High Risk
Introduces authenticated cross-invocation browser authority, owner-private receipt files, and Bridge 1.38 host negotiation—security-sensitive browser automation with fail-closed remote routing.

Overview
Adds durable Bridge 1.38 browser capability namespaces to the CLI so automation can survive separate invocations: namespace-create / namespace-close, high-level browser actions through a signed namespace receipt, and bind-window with opaque bp1_ page capabilities plus exact Chrome PID and WindowServer window ID.

Every namespace call must supply --namespace-file (owner-private, mode 0600 receipt storage with strict validation) and the same issuing --bridge-socket. Namespace commands force remote Bridge routing (requiresBrowserCapabilityNamespace), skip legacy browser MCP, and refuse --no-remote / local fallback. BrowserCommand gains string page_id, namespace flags, and a dedicated adapter path; the runtime executor honors RuntimeOptionsConfigurable so these requirements are not rebuilt from argv alone.

On the agent/runtime side, Chrome channel resolution can retain a read-only DevTools control WebSocket for exact tab/window correlation; get_tab_id is internal-only so raw CDP IDs are not exposed in public tool routing. Session pooling tracks ended sessions more safely, and execution metadata can attach native window binding receipts. Agent guidance now documents bind_window for exact native-window browser work.

Reviewed by Cursor Bugbot for commit d0b807e. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 0ec6ae7. Configure here.

return BrowserCLINamespaceCreateResult(
namespaceReceiptData: receiptData,
response: .text("Browser capability namespace created.")
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create throw leaks remote namespace

Medium Severity

createNamespace creates the Bridge namespace first and only then canonicalizes the receipt. If that second step throws, BrowserCLINamespaceLifecycle.create never sees a receipt and does not roll back, so the host namespace stays live until it expires. Callers can retry and create another one, leaving unpersisted namespaces that cannot be closed.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 0ec6ae7. Configure here.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 27, 2026
@clawsweeper

clawsweeper Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed August 27, 2026, 4:57 AM ET / 08:57 UTC.

ClawSweeper review

What this changes

The PR adds Bridge 1.38 browser namespaces that persist authenticated browser authority across CLI invocations and can bind opaque pages to an exact native Chrome window.

Merge readiness

Blocked until real behavior proof from a real setup is added - 6 items remain

Keep this collaborator-authored feature PR open for maintainer product review, repair of one remaining lifecycle bug, and authority-chain runtime proof.

Priority: P2
Reviewed head: d0b807e351c6bf14c7777ceafeda9d0a38a79667
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The implementation has substantial source-level safeguards, but the remaining lifecycle defect and missing real authority proof make it unready to merge.
Proof confidence 🧂 unranked krab (1/6) Needs real behavior proof before merge: Authority-chain proof required: the PR provides source checks and fixture coverage, but no redacted real Bridge/Chrome trace shows an allowed receipt reaching provider dispatch and a copied, closed, or revoked receipt being refused before I/O. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: Authority-chain proof required: the PR provides source checks and fixture coverage, but no redacted real Bridge/Chrome trace shows an allowed receipt reaching provider dispatch and a copied, closed, or revoked receipt being refused before I/O. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 6 items Namespace creation can leak a live allocation: The adapter creates the remote namespace and then performs receipt validation/canonicalization; either throwing operation bypasses the lifecycle rollback, leaving the server allocation live until expiry.
Existing rollback coverage starts too late: The lifecycle test covers persistence and close after an adapter has successfully returned a receipt, but no test injects adapter-side receipt validation/canonicalization failure after remote creation.
Authority boundary has source-level safeguards: Namespace actions require an attested negotiated session and authenticated peer before authority claims or provider execution; response sanitization is covered against raw provider identifiers.
Findings 1 actionable finding [P2] Roll back an allocated namespace when receipt validation fails
Security None None.

Live Verification

Command: pnpm run build:cli

Result: FAIL (failed) — step 1 run Apps/CLI/.build/debug/peekaboo browser bind-window --json: terminal run was blocked by the previous command: terminal command failed with exit status 1: "pnpm run build:cli"

[WARN] Unsupported platform: wanted: {"cpu":["arm64","x64"],"os":["darwin"],"libc":["any"]} (current: {"os":"linux","cpu":"x64","libc":"glibc"})
$ swift build --package-path Apps/CLI
error: the package manifest at '/tmp/clawsweeper-live-proof-648-bjvnv4/target/Commander/Package.swift' cannot be accessed (/tmp/clawsweeper-live-proof-648-bjvnv
4/target/Commander/Package.swift doesn't exist in file system)
[ELIFECYCLE] Command failed with exit code 1.

Assertions:

  • FAIL expect_output: Browser namespace actions require an explicit --namespace-file.

How this fits together

Peekaboo’s CLI and MCP browser tools send Chrome actions through a local authenticated Bridge host. The new namespace layer stores caller-scoped authority between CLI invocations, then revalidates the Chrome/window relationship before a bound mutation reaches the provider.

flowchart LR
  CLI[CLI browser command] --> Receipt[Owner-private receipt]
  Receipt --> Bridge[Authenticated Bridge host]
  Bridge --> Namespace[Caller-scoped namespace]
  Namespace --> Binding[Native window validation]
  Binding --> Provider[Chrome provider action]
  Provider --> Result[Sanitized result]
Loading

Decision needed

Question Recommendation
Should Peekaboo core permanently expose durable, receipt-backed browser namespaces across separate CLI invocations? Sponsor the durable namespace surface: Keep the feature in core after the lifecycle repair and required real authority proof.

Why: This adds a new persistent browser-authority capability rather than repairing an established contract, and source review cannot establish long-term product ownership of that surface.

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: Authority-chain proof required: the PR provides source checks and fixture coverage, but no redacted real Bridge/Chrome trace shows an allowed receipt reaching provider dispatch and a copied, closed, or revoked receipt being refused before I/O. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Roll back an allocated namespace when receipt validation fails (P2) - createBrowserCapabilityNamespace() has already allocated the remote namespace before receipt validation/canonicalization can throw here. That exception bypasses BrowserCLINamespaceLifecycle.create, so the caller has no receipt with which to close the live allocation; retain a cleanup path for this post-allocation failure and cover it with a fault-injection test.
  • Resolve merge risk (P1) - A throw after remote namespace allocation can leave an unpersisted namespace active until expiry and consume bounded host capacity.
  • Resolve merge risk (P1) - The new persistent authority path lacks redacted real-system evidence that an allowed request reaches provider dispatch while copied, closed, or revoked authority is refused before I/O.
  • Complete next step (P2) - A maintainer must decide whether the new durable browser-authority surface belongs in core; contributor-supplied real authority proof is also required before merge.

Findings

  • [P2] Roll back an allocated namespace when receipt validation fails — Apps/CLI/Sources/PeekabooCLI/Commands/MCP/BrowserCommand+Namespace.swift:112-114
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Diff surface 89 files, +13,968 / -227 lines The change spans CLI, Bridge, browser runtime, tests, and docs, so its authority and lifecycle contracts need end-to-end evidence.

Merge-risk options

Maintainer options:

  1. Repair allocation cleanup and prove authority boundaries (recommended)
    Ensure a post-create receipt failure closes the exact server namespace, add a fault-injection regression, and provide a redacted allowed-versus-denied Bridge/Chrome trace before merge.
  2. Pause the new durable authority surface
    Do not merge the namespace feature if maintainers prefer not to own cross-invocation browser authority in core.

Technical review

Best possible solution:

Retain the scoped, fail-closed namespace design after closing the allocation cleanup gap and demonstrating its allowed and denied authority paths against a real Bridge and Chrome session.

Do we have a high-confidence way to reproduce the issue?

No live reproduction was supplied, but source inspection gives a high-confidence fault path: remote creation succeeds before receipt validation/canonicalization can throw, while lifecycle rollback only receives successful adapter results.

Is this the best way to solve the issue?

No; the scoped design is plausible, but it needs post-create cleanup coverage and real final-effect authority proof before it is the safe maintainable solution.

Full review comments:

  • [P2] Roll back an allocated namespace when receipt validation fails — Apps/CLI/Sources/PeekabooCLI/Commands/MCP/BrowserCommand+Namespace.swift:112-114
    createBrowserCapabilityNamespace() has already allocated the remote namespace before receipt validation/canonicalization can throw here. That exception bypasses BrowserCLINamespaceLifecycle.create, so the caller has no receipt with which to close the live allocation; retain a cleanup path for this post-allocation failure and cover it with a fault-injection test.
    Confidence: 0.93

Overall correctness: patch is incorrect
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against e55661a32aea.

Labels

Label changes:

  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.

Label justifications:

  • P2: This is a substantial but not currently user-reported browser-automation feature with a bounded lifecycle defect.
  • merge-risk: 🚨 security-boundary: The PR creates and consumes persistent caller-scoped browser authority across process invocations.
  • merge-risk: 🚨 availability: An adapter-side receipt failure can leave a bounded remote namespace allocation live until expiry.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Authority-chain proof required: the PR provides source checks and fixture coverage, but no redacted real Bridge/Chrome trace shows an allowed receipt reaching provider dispatch and a copied, closed, or revoked receipt being refused before I/O. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

Likely related people:

  • steipete: Current browser, Bridge, and CLI history on the affected paths is predominantly authored by steipete, including this PR head and the preceding browser hardening work. (role: feature author and recent area contributor; confidence: high; commits: d0b807e351c6, cc4c714c1619, c49ce95a6a68; files: Apps/CLI/Sources/PeekabooCLI/Commands/MCP/BrowserCommand+Namespace.swift, Core/PeekabooCore/Sources/PeekabooBridge/PeekabooBridgeServer.swift, Core/PeekabooCore/Sources/PeekabooAgentRuntime/MCP/Tools/BrowserTool.swift)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Repair the post-allocation receipt failure path and add a focused regression test.
  • Provide redacted real Bridge/Chrome output for one allowed request and one copied, closed, or revoked receipt refused before provider I/O.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)
  • reviewed 2026-08-27T07:35:23.075Z sha 0ec6ae7 :: needs real behavior proof before merge. :: [P2] Rollback a namespace when adapter canonicalization fails

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Aug 27, 2026
@steipete

steipete commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

Maintainer decision for the 4.3.0 queue: keep this PR open, but leave the durable namespace/native-window feature out of 4.3.0. Its branch and implementation remain available for follow-up; this is a scope deferral, not a duplicate/already-fixed closure.

The smaller foreground-only DOM-click work in #660 is being handled separately. Before this feature lands later, it still needs reconciliation with current browser ownership/foreground semantics, allocation-failure cleanup, and the corresponding allowed/refused native-window proof. No release publication or successful live qualification is claimed by this update.

steipete added a commit that referenced this pull request Sep 2, 2026
Refresh PR #660 on current browser ownership and foreground policy.
Add the explicit synthetic DOM-click route through existing capability
and receipt validation, correct Agent guidance, and document its limits.
Do not treat HTMLElement.click as trusted pointer input or background-safe.

Preserve current signer/session/target/dispatch invariants and keep the
separate durable namespace proposal in #648 out of this change.
Run the browser capability, remote-session, and guidance regressions in CI.

Scoped syntax, formatting, lint, static guidance and workflow contracts
passed; updated-scope Codex P0 review is clean. Swift execution and real
browser final-effect proof remain pending; the PR stays draft.
steipete added a commit that referenced this pull request Sep 2, 2026
Refresh PR #660 on current browser ownership and foreground policy.
Add the explicit synthetic DOM-click route through existing capability
and receipt validation, correct Agent guidance, and document its limits.
Do not treat HTMLElement.click as trusted pointer input or background-safe.

Preserve current signer/session/target/dispatch invariants and keep the
separate durable namespace proposal in #648 out of this change.
Run the browser capability, remote-session, and guidance regressions in CI.

Scoped syntax, formatting, lint, static guidance and workflow contracts
passed; updated-scope Codex P0 review is clean. Swift execution and real
browser final-effect proof remain pending; the PR stays draft.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant