feat(server): let users withhold browser access from agents#7083

🟠 High Layers/ProviderService.ts:253

When agent browser access is disabled for a thread, its previously issued MCP credential remains valid and /mcp continues accepting the old token until expiry. This branch skips issueActiveMcpCredential (the normal revocation path) and only clears McpProviderSession; revoke the thread with revokeActiveMcpThread(threadId) before clearing the provider session.

✅ Resolved in b4d14de34078cf5f87d6ff04c34667954e95b4e3

This is a backend behavior change (a session may now start with no MCP credential, and a stale provider session is cleared), but the only new tests cover prompt text in CodexSessionRuntime.test.ts. Consider adding a focused test in ProviderService.test.ts using ServerSettings.ServerSettingsService.layerTest({ enableAgentBrowserAccess: false }) asserting that startSession issues no credential and clears the thread's MCP provider session, plus the enabled counterpart — the existing suite already wires a test settings layer, so it is a small addition.

^{Posted via Macroscope — Effect Service Conventions}

Deny path skips credential revoke

High Severity

When enableAgentBrowserAccess is off, prepareMcpSession only clears the in-memory MCP map and never calls revokeActiveMcpThread. Before this change, every prepare path went through issueActiveMcpCredential, which always revokes the thread first. Session restarts (runtime mode, cwd, model) call startSession without stopSession, so they relied on that revoke. Orphaned bearer tokens stay valid, and touchActiveMcpThread on later turns refreshes them for up to the full liveness window—longer if turns keep coming. If replacing an old process fails, that agent can keep driving /mcp after the user withheld access.

^{Reviewed by Cursor Bugbot for commit 358706969b796ffbc757b5f50d68f61e2f778f24. Configure here.}

🟡 Medium Layers/CodexSessionRuntime.ts:1833

When MCP is configured through launchArgs, the spawned session has the preview_* tools but the generated developer instructions omit them because this check reads only options.appServerArgs. Use the resolved appServerArgs value here.

Restore defaults skips agent browser access

Medium Severity

enableAgentBrowserAccess has a per-row reset on Integrations, but General → Restore defaults never dirty-tracks or resets it. Peer server toggles like enableProviderUpdateChecks and the other Browser defaults are included. After Restore defaults, agent browser access can stay off with no mention in the confirmation list.

^{Triggered by learned rule: Restore defaults on General settings page is intentionally global — not scoped per-page}

^{Reviewed by Cursor Bugbot for commit 093a16d6a426c5340b4016ec16b7c9141efbb7bf. Configure here.}

This row offers a per-setting reset, but enableAgentBrowserAccess isn't part of the page-level Restore defaults flow. getChangedBrowserSettingLabels (SettingsPanels.logic.ts) and the restoreDefaults patch (SettingsPanels.tsx) cover the four browser client defaults in this same section but not this toggle, so "Restore defaults" leaves agent access off and never mentions it in the confirmation dialog — inconsistent with every other reset-able row here.

Smallest fix: add enableAgentBrowserAccess to the changed-label list (e.g. "Agent browser access", plus the memo dependency) and to the updateSettings({ ... }) restore patch. If bulk restore should deliberately not re-grant agent access, a one-line comment stating that would keep the exception intentional.

^{Posted via Macroscope — UI Consistency}

🟡 Medium browser/browserDefaults.ts:66

When PreviewView toggles the device toolbar during startup, useBrowserDefaults returns the schema-default viewport and the toggle commits that built-in viewport instead of the user's persisted browser default; the viewport is not corrected after hydration. Unlike the imperative accessors, this hook is not hydration-gated, so make the toggle wait for client-settings hydration (or expose hydration-aware hook state) before applying the default.

No longer relevant as of 0d31f1163ef219ee4489380e5e63400c77a4ddac

🟡 Medium settings/SettingsPanels.tsx:671

Confirming an unrelated restore silently resets browser viewport, zoom, appearance, or auto-show preferences changed in another tab while the dialog was open, even though those settings were not listed for restoration. updateSettings always writes all four browserDefault* fields after confirmation; re-read the live settings or update only fields included in the confirmed dirty snapshot.

NumberField is fully controlled here (value passed, no onValueChange), so value changes the primitive initiates itself — ArrowUp/ArrowDown stepping, scroll-wheel — are dropped: the parent never echoes them, and with no increment/decrement parts rendered, typing is the only way to change the size. Every other NumberField call site in the app pairs value with onValueChange for this reason.

Suggest keeping a local draft via onValueChange and persisting in onValueCommitted, i.e. the same draft-then-commit shape BrowserDeviceToolbar uses for the identical width/height pair — and mirroring its aria-invalid so a size commitDimension rejects (over PREVIEW_VIEWPORT_MAX_AREA) isn't silently ignored while the input keeps showing it.

^{Posted via Macroscope — UI Consistency}

This row's select is one size step shorter than the rows directly below it. size="sm" is min-h-8 sm:min-h-7, while every other select rendered inside a SettingsRow (including BrowserZoomSetting and BrowserAppearanceSetting a few lines down, and the Appearance/General panels) uses the default min-h-9 sm:min-h-8, so the Browser section ends up with two different control heights stacked on top of each other.

Suggest dropping size="sm" here and from the two NumberFields below it (their sm height is what currently lines up with this trigger), so the whole section shares the standard control height.

^{Posted via Macroscope — UI Consistency}