v0.8.5: cookie banner, cli updates, knowledgebase connectors improvements - #6843
Merged
Conversation
#6819) The repo is public and both skills publish permanently. `/ship` already listed what to omit, but the rule only fired inside that skill — a PR opened directly with `gh pr create` skipped it entirely, which is how a customer name, a knowledge base id, and verbatim sheet and column names reached a public PR description. Ship's list now covers every artifact rather than just the title and body, names verbatim customer content as its own category, draws the line on aggregate counts (fine detached from a tenant, not fine attributed to one), and carries a pre-publish grep so the check is mechanical instead of remembered. Babysit had no such guidance at all despite posting replies continuously, and triage is precisely where prod evidence gets pasted in. It now has a short section plus a hard rule, pointing at ship's list rather than restating it.
… name (#6817) * fix(knowledge): parse the stored artifact, not the document's display name A connector document's `filename` is a display name that deliberately disagrees with the bytes on disk: the sync engine records the source file's name (`Report.pdf`) while storing the text the connector already extracted from it under a `.txt` key, with `mimeType: 'text/plain'`. `processDocumentAsync` discards the processing filename the sync engine computes and rebuilds its input from the document row, so the parser was chosen from the display name and re-parsed extracted text as the source binary. In production that failed 1,379 SharePoint PDFs with `Invalid PDF structure.` and silently double-wrapped 364 spreadsheets — those reported `completed`, wrapping a second fake sheet around the connector's own extraction, because SheetJS accepts almost any input. Parser selection now prefers the extension of the object actually fetched, falling back to the filename/MIME path when the URL is not ours or the key carries no extension a parser claims. Both ingestion paths are honest under that rule because `fitStorageKeyName` preserves extensions through truncation: an upload keys on its original name, a connector document keys on what it stored. This layer is what covers the stuck-document retry sweep, which rebuilds its own input from the same display name — the sweep is the path that reprocesses the already-failed documents, so a fix confined to `processDocumentAsync` would have left the remediation itself broken. The defect predates the connectors that expose it: Box fetches Box-side text representations for `pdf`/`docx`/`xlsx` and stores them under the source name too, so it was latent there before SharePoint and OneDrive reached binary formats. `connectorArtifactFileName` now owns the `.txt` suffix that the parser choice depends on, so the invariant is structural instead of a convention repeated at four call sites per function. * fix(knowledge): raise the connector sync ceiling and tie it to the stale lock A 2,600-document library exhausted the 30-minute budget and the run was killed mid-listing, leaving the connector's `syncing` lock set until the scheduler reclaimed it. Raising the ceiling is not a lone constant, because reclaiming a stale lock flips the connector to `error` and frees it for another sync. A TTL at or below the run ceiling would hand the lock to a successor while the first sync is still writing — two syncs racing the same `(connectorId, externalId)` rows. The previous values, a 1800s run against a hard-coded 120-minute TTL declared in a different file, held that invariant only by coincidence. Both now derive from one another, with a test pinning the margin so the next raise cannot silently break it.
…6818) * fix(forks): stop copying connector-managed knowledge base documents A fork copies a KB's documents but never its connectors, so a connector-sourced document arrives with `connector_id` nulled and its `external_id` intact. The sync engine keys every existing/tombstone/ exclusion lookup off `connector_id`, so that copy is invisible to it - never updated, reconciled, or purged - and `doc_connector_external_id_idx` does not constrain it either, since its `connector_id` is NULL. Attaching a connector in the child then re-ingests every page as a NEW row on top of the snapshot. Each fork hop re-copies the previous hop's orphans and adds one more generation, so a prod -> UAT -> staging chain leaves three rows per page and a knowledge search returns the same page three times, one of them serving content frozen at the fork date. Exclude connector-managed documents from all four doors a document can enter a fork through: the whole-KB content copy, the in-transaction placeholder pre-creation, the sync-only copy into an already-mapped KB, and the content fill (guarded for payloads planned by a pre-change worker mid-rollout). The placeholder path matters as much as the copy loop - filtering only the content phase would leave a permanently archived row behind a persisted `knowledge_document` mapping. Skipped on both sides, the reference clears like any other uncopied document's. A document whose connector was deleted already has a null `connector_id` (the FK is ON DELETE SET NULL) and is static in the source too, so it still copies. One count(*) per copied KB logs what was left behind, since a fully connector-synced KB now forks to zero documents. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(forks): keep the skipped-document count from failing a copied KB The connector-managed count feeds a log line, but it sat inside the KB's try block, so a transient failure on a COUNT(*) would roll back a copy that had otherwise succeeded and clear every reference to it. Move it into a helper that swallows its own error. Counting is not copying: only the copy itself may fail a resource. Test proven red by removing the catch - the mutation reports a knowledge-base failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(forks): clean up full-KB placeholders planned before the exclusion The mapped-KB fill guarded a pre-change plan, but the full-KB path did not: a placeholder planned by an old worker for a connector-managed document is simply no longer returned by the page query, so nothing fills it and it stays archived behind a live mapping that a remapped document-selector still resolves to. Report those child ids as failed documents so the shared cleanup clears their references and drops the rows, and delete their persisted identity so a later sync does not resolve to a row cleanup removes. Keyed on the SOURCE being connector-managed, which can never become copyable, so it cannot race a concurrent attempt mid-fill the way a "source is gone" check could. The mapping drop is now one helper shared with the mapped-KB catch. Test proven red by removing the reconciliation block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(forks): make the stale-plan probe best-effort The probe ran inside the KB try, so a transient SELECT would reach the catch, roll back a complete copy, delete the child base, and clear every reference to it. Weighing it as "load-bearing, so fail closed" was wrong: the probe runs on EVERY copied KB that has referenced documents, while the state it repairs exists only inside a rollout window. Failing closed traded a common-path outage against a rare-squared one. It now swallows its own failure with a loud error log, leaving that pre-existing state in place rather than destroying a good copy. Test proven red by removing the catch - the mutation reports the KB failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(emails): add sub-processor change notification template * improvement(emails): link the objection address and preference URL
…6822) * fix(forks): name the workspace a sync overwrites instead of "target" Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(forks): name the target workspace in the blocker resolution line too Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
#6825) * improvement(docs): clear leftovers from the reverted revisions A cleanup pass over the final state. Every finding was residue from an approach this PR tried and abandoned, or a claim that stopped being true when it did. - Delete the copy-button svg sizing rule: a later rule sets `display: none` on that same element ungated, so sizing it was never observable. Superseded by the mask approach. - Drop the paragraph in page.tsx arguing about a custom Shiki factory. The factory was deleted; nothing configures one now. - Correct shiki-curl-json.ts, which still claimed the grammar "reaches the client path too". It does not — that was the justification for choosing a grammar over a transformer, so leaving it stated the opposite of the truth. Now records where it applies, where it does not, and why not to retry. - Correct the global.css section header, which claimed the component owns the shell while the next rule defines it here. - Qualify the `--copy-glyph` declarations with `:has(> svg[class*="lucide"])`, which the group's own comment asserts of every rule in it. - Correct `getCode`'s TSDoc: the gutter is a `::before`, and pseudo-element content never reaches `textContent`, so line numbers were never what the clone guards. It guards transformer-emitted `.nd-copy-ignore` nodes. - Compose `chipGeometryClass` and emcn's `ChipChevronDown` in the API example selector instead of restating their literals. - Merge the duplicated `div[role="region"]` rule. The tablist pair stays split: biome's `noDuplicateProperties` reads a nested `@variant` setting the same property as a duplicate and fails the build — recorded so it is not remerged. - Note that fumadocs ships its own gutter for `lines`-meta fences, which cannot be suppressed from here and would paint a second column. * fix(docs): drop a highlighter registration that can never fire fumadocs-openapi calls `renderCodeBlock` with a hard-coded `"json"` from both of its call sites (`request-tabs.js:76`, `response-tabs.js:48`), so the docs `CodeBlock` it routes through never receives a shell language. The `getHighlighter('js', { langs: [curlJsonBodyGrammar] })` registering the shell-scoped JSON-body injection therefore did nothing but await on every API sample render, and the docblock claiming the grammar covers those samples was wrong. - Delete the call and its imports. - State the grammar's real coverage: prose fences only, via `langs`. Both API reference paths are unreachable — samples are JSON, and the cURL usage tabs highlight client-side off fumadocs' own factory. - Correct `code-block.tsx`'s TSDoc, which still said API samples come from fumadocs' own renderer. They come through this component; `UsageTab` is the renderer that bypasses it. - Re-home a comment orphaned when two CSS rules merged — it had drifted onto the rule below and read as documenting it. - Drop a `.nd-copy-ignore` claim about transformers emitting those nodes; nothing here does, and upstream parity is the reason the clone exists.
…of extracting them (#6821) * feat(connectors): hand source files to the document pipeline instead of extracting them A connector that extracted text itself stranded the document on a second, weaker parser. The shared pipeline routes PDFs to OCR — the only way a scanned page is readable at all — and owns every other format's parser, but its OCR branch is gated on `mimeType === 'application/pdf'` and connector documents were stored as `text/plain`, so a connector PDF could never reach it. The same file dragged into the UI was read by OCR; synced through a connector it got the local parser. `ExternalDocument` can now carry the source file itself, and SharePoint and OneDrive hand over anything the knowledge base can parse rather than extracting it. The sync engine stores those bytes under the file's own name and type, so the pipeline parses them exactly as it would an upload of the same file. Formats that are already text stay on the text path: HTML still reduces to plain text and the rest are UTF-8 decodes, so nothing already indexed changes representation. The MIME type is derived from the extension rather than the source's own declaration, so a provider that omits or mislabels it cannot strand a PDF on the non-OCR path. Re-syncing an existing document now rewrites `mimeType` too, which is what lets one stored as connector-extracted text stop declaring `text/plain`. This removes the duplicate extraction path rather than leaving both in place: `extractConnectorText` is text-only, and the guard against fabricated content moves to the pipeline where parsing now happens. That guard still matters — `DocParser` and `PptxParser` never throw, returning a placeholder sentence or scraped archive bytes on a legacy binary or an image-only deck — so a `degraded` result now fails the document with the same actionable message it produced before, naming the modern container for legacy formats. The in-flight byte budget already accounted for this: `estimateOpSizeBytes` reads the true source size from listing metadata, so batching reserved against the real file all along and merely over-reserved while only text was stored. * fix(knowledge): guard every parser against empty output, not just the file parsers Moving connector parsing into the pipeline exposed a gap on the OCR branch. OCR reads a scanned page with no recoverable text as empty, and the empty-content guard lived inside the file-parser path, so such a document chunked to nothing and reported success — the same silently-complete-but-useless outcome the guard exists to prevent. The check now sits above the parser choice and covers OCR too. Also preserves a source file's extension when its name is too long for a storage key. The extension is what picks the parser; a truncated name would still parse correctly by falling back to the display name, but only by luck. * fix(knowledge): validate the stored artifact against the parser registry Ten of the formats a connector now hands over — docm, dotx, xlsm, xlsb, xltx, pptm, potx, odt, ods and odp — parse fine but are deliberately not offered as upload types. `resolveStoredArtifactExtension` gated on the upload allowlist, so it rejected every one of them and processing failed with `Unsupported file type`. They worked before only because the connector extracted them itself and stored the result as text. The question the gate is asking is whether a parser can read the stored object, which the parser registry answers; the upload allowlist answers a different question about what we accept from a user. Also matches the sibling comment style in the object literal it sits in, and teaches two test mocks the newly imported symbol. * fix(knowledge): carry the MIME type through hydration A listing stub is built before the file is fetched and declares `text/plain` for everything, so a hydrated PDF kept claiming plain text at the top level. Nothing broke today only because storage reads `sourceFile.mimeType` — which is exactly what makes it a trap: anything later reaching for `extDoc.mimeType`, the obvious field, silently loses the OCR routing this change exists to restore. The merge is now `mergeHydratedDocument` rather than an inline spread, so what hydration must carry is a stated contract with a test behind it instead of a literal that is easy to under-specify — which is how the field was missed.
The publish workflow derives the npm version from this field, and on `main` it uses it verbatim: `2.0.0` was already on the registry, so the release step skipped and every change since then stayed unpublished. Staging and dev never showed it because they append `-preview.N` and `-dev.N`, which are always new. So the merged CLI work — the redirect refusal, the default endpoint, folder path encoding, the request bound, the User-Agent, `run --follow`, `runs wait` and `logs follow` — is on `main` but not on npm, and `sim@latest` is still the build whose every write is dropped by the apex redirect. Minor rather than patch: three commands are new and the default endpoint changed.
…6827) The stable channel reads a GitHub release list it shares with web-app releases, SDK tags, and legacy prereleases, but only ever looked at the first 30 entries. Once enough unrelated releases stack on top, the feed 404s and every stable shell silently stops updating. Walk pages (100 per page, up to 5) until one yields a release for the channel, and fail the feed rather than serving an older build when a page cannot be read. Also point the update gate's manual download at a new /api/desktop/update/download redirect, which resolves through the same channel selection. It previously opened GitHub's repository-wide latest release, which can be a tag carrying no desktop artifact at all.
* improvement(logs-block): filter runs by trigger type The Logs block could filter runs by workflow, status, time, cost, and duration, but not by how the run started — even though the underlying tool, the /api/logs contract, and the indexed trigger column all already accepted a comma-separated triggers filter. Adds a basic multi-select and an advanced free-text field behind the canonical `triggers` param, mirroring the block's existing workflow filter. Options come from the same registry the Logs page reads, so both surfaces name a run's origin identically; values sharing a label are merged into one option. Leaving the filter empty omits the param, so existing blocks query exactly as before. * fix(logs-block): declare the triggers input as the string it becomes The generic handler JSON.parses any post-transform input declared 'array' or 'json'. Since `joinIds` has already turned the selection into a comma-separated string by then, the array declaration logged a parse warning on every run, and JSON-looking advanced input would have been turned into an array the tool does not accept. Matches the legacy Logs block, which already declares triggers as a string, and locks the invariant with a test. Also drops `any` from the new test helper. * fix(logs-block): trim each entry when joining filter ids joinIds trimmed only the ends of an advanced-mode string, so a hand-typed 'api, schedule, slack' reached the query as ' schedule' and ' slack'. The filters split on commas without trimming, so those tokens matched no stored trigger and the filter silently returned nothing. Splits and trims every entry instead, which also covers empty tokens from a trailing comma and the multi-value ids behind merged trigger labels. * test(logs-block): lock the shared joinIds output for existing filters joinIds is shared with the workflow and status filters, so the per-entry trimming added for hand-typed triggers must not move their output. Covers every value a stored multi-select or advanced field can hold, plus the block-saved-before-the-filter case where triggers must not reach the query. * test(logs-block): exercise the trigger options against the real registry The fetcher reaches the block and trigger registries through a lazy import to avoid an initialization cycle, so a mocked test cannot show that the import resolves or that the registry is populated when the dropdown asks. Covers the populated list, unique labels, and the merged Sim agent option.
…em layers (#6829) * improvement(ui): align the two full-screen takeovers on one design-system layer Both the session-expired screen and the desktop minimum-version gate are full-screen takeovers, but they disagreed on every piece of chrome: an ad-hoc z-[9999] against a z-50 that sat below --z-dropdown, --bg against --surface-1, a legacy Button against a Chip, and muted grey on their failure copy. The z-index was a real bug, not just an inconsistency. At z-50 the session-expired takeover rendered underneath the desktop browser panel's replacement snapshot, which paints at calc(var(--z-modal) - 1). Adds --z-takeover to the z-scale as the layer above every popper, points both takeovers at it, and aligns their background, primary action, body-copy and error tokens. Also corrects the z-scale in the design-review skill, which listed --z-toast at 500 when it has long been 150. * fix(ui): keep the shell gate above in-app takeovers and stop pre-paint click capture Tying both takeovers to one layer let document order decide which wins, and the desktop update gate mounts earlier than the session-expired screen, so the session overlay silently started covering it — reversing the precedence the old z-[9999] vs z-50 pair had. Names the precedence instead: --z-shell-gate sits above --z-takeover, because an incompatible shell invalidates everything the web app renders inside it. Both takeovers also hid their pre-paint state with opacity, which still hit-tests, so a full-viewport surface could swallow clicks before it painted. Switches them to the visibility toggle ModalContent already uses for the same handshake.
* fix(search): show new chat first for chats * fix(search): prioritize exact blocks on workflow editor * fix(search): keep the command palette inside small viewports The palette dialog was a fixed 500px box centered over the content area (offset right by the sidebar, and the panel on the canvas), so narrow windows pushed it past the right edge — clipping the Ask Sim adornment and the empty state. Its 448px list could also extend below the fold on short windows, where cmdk aligns the selected row against the off-screen bottom edge: the selection parked below the viewport and held arrow keys juddered rows against an edge the user could not see. Clamp the centered left position to a 16px gutter, shrink the width once the viewport is narrower than the dialog plus gutters, and cap the list height so the whole dialog stays on-screen. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(search): stop clipping the first glyph in the command search input Inputs clip glyph ink at their padding box, and the palette input had no left padding, so a leading glyph whose ink reaches its pen origin (the brand font's j) lost its left edge — worst at low browser zoom, where the clip boundary snaps to whole device pixels and eats up to 2 CSS px of the first letter. Give the input 3px of left ink clearance with a compensating negative margin so the text keeps its exact alignment with the result-row titles. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(search): give the first glyph real ink clearance via text-indent Chrome clips input text at the content box, not the padding box, so the previous padding-based clearance was dead space — glyph ink still started exactly at the clip edge, and the first letter kept losing its left edge under low browser zoom. text-indent starts the text 3px inside the clip region, which is clearance the renderer can actually paint into; the compensating negative margin keeps the text aligned with the result-row titles as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(search): stabilize command input glyph clearance --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…releases (#6830) GHSA-5p4m-2wfm-xmqj (quadratic CPU in !!omap resolution) is patched in js-yaml 4.3.1 and 3.15.1. Both direct dependents already pin 4.3.1, but bun.lock still held ten stale nested resolutions — 4.2.0, 4.3.0, and 3.14.2 — under fumadocs, electron-builder/updater, gray-matter, and json-schema-to-typescript. Every one of those ranges (^4.1.0, ^4.1.1, ^3.13.1) already admits the patched release, so this is a lockfile-only dedupe: no manifest change and no overrides block, which would force gray-matter's 3.x range onto js-yaml 4 and break it.
* feat(consent): add a hosted-only cookie consent banner Adds a c15t-backed consent runtime and a Sim-styled banner, mounted from the root layout only when `isHosted` is true. A self-hosted deployment never mounts the runtime, so it makes no request to Sim's consent backend and never sees the banner. The banner is a non-modal card docked bottom-left, opposite the toast stack, built from the same chrome (border, --bg, --shadow-overlay) and from Chip/Switch/Label rather than c15t's own components — the runtime is imported from `@c15t/nextjs/headless`, which ships no UI or stylesheet. "Customize" expands the same card into per-category switches instead of opening a dialog over the app. Visibility and the available actions come from the jurisdiction policy the runtime resolves, and accept and reject are rendered with identical weight. * feat(consent): cookie policy page, CSP allowance, and design-system alignment The consent backend was blocked by our own CSP, so the runtime silently fell back to an offline policy that showed the banner to every visitor worldwide and recorded nothing. The backend origin now lives in lib/consent/constants and the CSP builder allows it from that single source. Banner: mount the runtime beside the app rather than wrapping it, behind a dynamic() boundary, so consent state cannot re-render the page tree and a self-hosted build never fetches the chunk. Align chrome with the toast card (z token, font scale, --text-body/--text-muted pairing) and mirror the light token layer the public shells pin, which a dark-theme visitor on a landing route outside ThemeProvider's forced list would otherwise miss. Read the category list from the store's own getDisplayedConsents() — the shipped defaults mark every category except necessary as display:false, so the hand-rolled filter rendered a one-row list. Docs: add /cookie-policy as a third ProsePage consumer with the cookie inventory in tables (a new table block kind on the shared primitive), cross-reference it from the Privacy Policy, and wire it into the sitemap and llms.txt. The policy promises consent can be changed at any time, so the banner can be reopened from it. * refactor(consent): apply the cleanup pass - Drop the .light DOM probe: it matched the banner's own element, so once set it could never flip back, and it went stale on a theme toggle with no navigation. The card now pins the light layer unconditionally, as every other public surface does. - Hoist the motion/style objects to module scope. - Move a chip's mr-auto into the row layout; chips carry no outer margin. - Use the shadow-overlay utility and --border rather than the legacy alias. - Render <caption> before <colgroup>, which the HTML spec requires. - Make the code formatting of a table column a renderer concern (codeColumns) instead of JSX smuggled into the row content. - Raise the table caption above body weight, and tighten comments. * refactor(consent): apply the simplify pass The consent runtime installs a childList+subtree MutationObserver on document.body for its iframe blocker, for the life of every hosted page — including the workflow canvas — and re-scans each added subtree. Sim gates no iframes by consent, so disableAutomaticBlocking turns it off. Also: collapse the ConsentProvider passthrough into the dynamic() export; move ConsentPreferencesLink under (landing)/cookie-policy so a shell module no longer imports landing chrome; build the three cookie tables from one shape; move the table column widths into the prose chrome layer; only compute the category list when the card is expanded; drop the ConsentCategory cast; express the card width in Tailwind rather than an inline style. Comment corrections: the sibling mount is forced by ssr:false, not by re-render concerns; lib/consent/constants must stay dependency-free because next.config loads it and the browser bundles it; codeColumns exists for biome's useJsxKeyInIterable, not for React; the headless entry omits the components but the provider still injects an inert --c15t-* style block. * fix(consent): address the first review round - Add /cookie-policy to LANDING_ROUTES. It is an app/(landing) route, and every one of those must be exempt from COEP: the header is inherited across soft navigations, so an isolated landing page navigating into /demo leaves the Cal.com booker loading uncredentialed. - Render the withdrawal control as plain text on a self-hosted deployment, where the consent runtime is never mounted and the button had no listener. - Give ConsentPreferencesLink a named props interface.
* feat(account): let users delete their own account Adds a GDPR self-serve account deletion path: a preflight that reports what deletion would remove and every reason it would be refused, and a confirmed delete that erases the account and everything only it can reach. Deletion refuses while the account is still entangled rather than reassigning its content. Most tables reference user.id with ON DELETE CASCADE, and those cascades do not distinguish content in the account's own workspace from content it created inside somebody else's, so each blocker names the existing action that untangles it (leave the workspace, leave the organization, cancel the plan) — all of which already hand work over on their own tested paths. * fix(account): make deletion atomic and re-check privacy at delete time Reorders the teardown so nothing irreversible happens before the deletion is certain: anchors are handed over first (the fallible step, while everything is still recoverable), the workspace and user deletes now share one transaction, and the object-storage purge runs only after that commits. The workspace delete also re-checks inside the transaction that each workspace is still private, so a membership granted between the preview and the delete aborts the whole thing instead of destroying the new member's access. * fix(account): close deletion gaps found in review - Run the whole teardown in one transaction. The billing and ownership handovers now take the caller's transaction, so a refused deletion can no longer leave a workspace reassigned for a deletion that never happened. - Fail closed on a subscription read error. getHighestPriorityPersonalSubscription defaulted to returning null, which read as "no plan" and would have erased an account Stripe was still billing. - Erase the account's profile picture. It is personal data under our own storage prefix; an external provider avatar is left alone. - Enforce the storage purge cap while collecting keys rather than after, so an oversized account cannot exhaust memory before the cap applies.
…orkspace (#6835) * feat(consent): manage cookies from Settings, never from a card in the workspace The banner no longer mounts inside the workspace at all. The gate sits above the dynamic() boundary rather than inside the lazily-loaded module, so the product pays neither the consent chunk nor its init request on the surface with the most hard loads. A signed-in user manages the same choice from Settings -> Privacy, which shares one store with the banner: the options live in ConsentStoreProvider and are not exported, so two call sites cannot drift into two stores. The banner also stops pinning the light token layer and simply inherits. The cause it was working around is that LandingShell pins light on a wrapper inside the page while <html> keeps the visitor's theme, so landing routes missing from ThemeProvider's hand-written list rendered light pages under dark root chrome. LANDING_ROUTES becomes one source of truth in lib/landing/routes, read by both next.config (COEP) and ThemeProvider (forced light) -- the same drift that let /cookie-policy ship without its COEP exemption. Diffed old against new across every real route: 16 landing routes gain the correct theme and nothing regresses. /cli/auth and /credential-groups/complete are added too; both render AuthShell and were never covered. Verified the shared-store assumption directly rather than trusting the docs: getOrCreateConsentRuntime returns the same store and manager for equal options. * fix(consent): keep Privacy on one settings surface Projecting the section into the account plane put it in a catalog that buildPlaneSettingsItems does not gate on requiresHosted, so a self-hosted deployment would list a Privacy entry, and AccountSettingsRenderer's catch-all rendered Mothership for it. The unified settings already gate the section and redirect self-hosted deployments to General, so the section lives there only.
… per secret (#6823) * feat(secrets): record which secrets each run resolves and surface it per secret Redaction stops a value at a boundary but cannot stop code that never emits it — a Function block can print a key one character at a time and nothing ever matches the secret. That is undecidable in general, so this adds the other half of the posture: attribution. Every run now records which configured secrets it actually resolved, under whose identity, through which surface (workflow, Sim agent, MCP). The data already existed in ResolvedSecretTraceRegistry.addActiveEntry and was persisted only for paused runs; this persists it for every terminal path. Execution logs cannot answer this. They store the whole available encrypted environment rather than what a run referenced, they evidence a secret only where value-matching redaction happened to fire, and they expire under logRetentionHours — while "who has touched this key" outlives any single run. - secret_usage: per-UTC-day rollup keyed by workspace, secret, scope, owner, source, workflow, actor. A one-minute schedule touching three secrets would otherwise write thousands of rows a day, which is also why this is not audit_log. workflow_id/actor_user_id use '' sentinels rather than null so the unique key works on Postgres 14 without NULLS NOT DISTINCT, and are not FKs: they are historical facts, and an onDelete would rewrite a key column. - secret_owner_user_id is part of the key. Two people can hold a personal secret under one name and a shared personal secret resolves for a caller who does not own it, so name and scope alone do not identify a secret. It is NOT the actor: a scheduled run resolves the workflow owner's personal slice under the workspace's execution actor. - Direct environment reads are now detected in JS (TypeScript AST), Python (tokenizer-checked) and shell (quote/heredoc-scanned), so a secret read as environmentVariables['K'] or $K enters the run's provenance instead of going unredacted. Each detector prescans for names that are actually configured secrets before paying for a lex or quote-frame pass. - Copilot integration tool calls are covered: resolveCopilotEnvReferences substitutes {{SECRET}} into user-only params, which is a real use. - See usage lives behind a credential-admin gate, using the same predicate that reveals the value; members get a disabled chip explaining why. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(audit): register the secret-usage route in the validation baseline Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(secrets): keep rollup metadata with its run, and stop shadowed bindings faking usage Review round 1. - record.ts: last_execution_id/last_trigger were assigned unconditionally while last_used_at was chosen by greatest(), so two runs completing out of order split one row between them — the newer run's timestamp beside the older run's execution id, making "View log" open a run the row does not describe. Both are now guarded on the timestamp actually advancing, so the row's metadata always belongs to the run that owns its timestamp. - javascript.ts: a local binding named environmentVariables (declaration, parameter, destructured binding, or bare reassignment) made reads off the user's own object look like mounted-secret reads. Any such binding now disables detection for the file; the AST already had parent pointers, so this is a kind check during the existing walk. - python.ts: same class of bug with no parser available, so the rule is an allowlist — every mention of the binding must be a literal subscript or .get(), otherwise detection is off for the file. This also subsumes the cross-line attribute case (other.\n environmentVariables['K']), which the previous space-and-tab look-behind missed. Under-reporting is the safe direction here: a trail that claims a use that never happened is worse than one that misses a use. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(db): format the generated migration snapshot CI runs lint:check across every workspace; the drizzle-kit output in packages/db had never been through biome, so the branch was green locally (where lint had only been run inside apps/sim) and red on CI. Whitespace only — both files are byte-for-byte identical once parsed, and drizzle-kit still reports no pending schema diff against the reformatted snapshot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(secrets): detect every rebinding of the environment identifier, not just declarations Review round 2. A bare `for (environmentVariables of rows)` has no declaration to key off, so the previous check missed it and reads of the loop value were still recorded as secret usage. Rather than extend the hand-rolled node-kind list, this reuses the pair the same file already applies to reject a placeholder in a write position: isDeclarationIdentifier covers declarations, parameters, destructured bindings and imports, and isWriteIdentifier covers every assignment operator, ++/--, destructuring targets, and for-in / for-of initializers. That also closes four forms neither the review nor the original check named: logical (||=) and nullish (??=) assignment, and object and array destructuring assignment. Six of the eight added cases fail against the previous check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(secrets): apply the rebinding rule to shell, and say when a run's log is gone Review round 3, plus the docs that were left claiming the old behavior. - shell.ts: a script that writes a configured name (API_KEY=local, export/local/ readonly, read, for, unset) expands its own value from that point on, not the mounted secret, so recording it claimed a use that never happened. Every mention of the name must now be a `$NAME` / `${NAME}` expansion, matching the allowlist shape the Python detector already uses. Applied per name rather than per file: JavaScript and Python shadow one object holding every secret, whereas rebinding one shell variable says nothing about the rest. - The usage trail deliberately outlives execution logs, so a row routinely names a run whose log has been pruned. The read now left-joins workflow_execution_logs on its unique execution_id and reports availability, and the panel renders the chip disabled with the platform tooltip instead of linking into an empty Logs view. Three states: no run to link, a run whose log is gone, and a live link. - Docs said a direct environmentVariables/$KEY read does not activate masking, which this branch changes. Corrected in credentials.mdx, function.mdx and the logging FAQ, and the recognition limits are now written down: runtime-built names, reassigned bindings, and reads that cannot be told apart from text. Added a "See usage" section covering who can see it and why an empty trail means "nothing recognized" rather than "never used". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(secrets): writing a name is not reading it, and a bare mention is not a rebinding Review round 4. - javascript.ts / python.ts: `environmentVariables.API_KEY = 'x'` and `delete environmentVariables.API_KEY` touch the name without ever reading the mounted value, but the detectors matched the member access and recorded a use that never happened. JavaScript now asks the same isWriteIdentifier the placeholder rewriter uses (its parameter is widened to ts.Node — the body already walked generic nodes, so this is a type change, not a behaviour one) plus a delete check; Python excludes a subscript followed by `=` and a `del` target. - shell.ts: requiring every mention of a name to be an expansion also fired on text that binds nothing — a comment naming the key, or `echo "API_KEY=$API_KEY"` where the literal is an argument rather than an assignment — and dropping those cost masking on a genuine read. It now looks for actual writes: an assignment at command-word position, a binding builtin, `printf -v`, or a `for` target. The two directions are not symmetric, which is why this errs toward detecting the read: missing a write records a use of a secret the script only had in its environment, a misleading audit row and nothing more, since masking still searches for the real value and will not find it. Over-detecting a write suppresses masking on a value that does reach the log. This also makes the code match what the docs already described — skipping after a rebinding, not after any mention. 13 tests added; 11 fail against the previous code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(secrets): an update reads before it stores, and a del target may be parenthesized Review round 5. The first of these is a regression from round 4. - javascript.ts: reusing isWriteIdentifier to answer "is this a read" was wrong. That predicate answers the rewriter's question — is this a target the substitution must refuse — so it treats every assignment operator alike, which is correct there and wrong here: `+=`, `||=`, `??=`, `++` and `--` all load the current value before storing, so they are genuine reads and were silently losing their masking. Only a plain `=` stores without reading. Replaced with a purpose-named predicate, and isWriteIdentifier's parameter is narrowed back to ts.Identifier now that nothing else needs it widened. A test committed last round asserted the wrong behaviour for `+=`; it has been corrected rather than left to pin the bug. - python.ts: `del (environmentVariables['K'])` slipped past a check that looked only at the characters immediately before the match. It now isolates the enclosing logical line and tests whether that is a del statement, which also covers `del((x))`, `del(x)`, `del a, x`, and a del after a semicolon. 12 tests added or corrected; 10 fail against the previous code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(secrets): stop excluding Python writes, which kept leaking in the unsafe direction Review round 6. Greptile found that `del environmentVariables[environmentVariables['K']]` had its inner access — which computes a key, so it is a genuine read — skipped along with the delete, leaving that value unmasked. The narrow fix was another textual rule. Instead this removes the write and delete exclusions from the Python detector entirely, because they were optimizing the wrong direction. `resolvedSecretNames` feeds `outputSecretMatcher`, an exact-value matcher over the output. Naming a secret the code never read costs nothing there: the matcher scans for a value that does not appear. Failing to name one that was read leaves it unmasked. The two error directions are therefore not comparable, and the exclusions bought only audit-trail tidiness while every heuristic they needed has so far leaked into the dangerous side — first a parenthesized target, now a nested read. A `del` or an assignment is reported like any other access. JavaScript keeps its exclusion: a real AST answers the question per node, with no text to misread, and it has produced no such hole. Net 30 lines removed from python.ts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(secrets): report recognized reads instead of proving they are not reads Review round 7. Greptile flagged both directions at once — false usage from reporting a write target, and unmasked secrets from the file-wide shadow flag — so I traced what the signal actually drives before choosing. The chain: the compiler's names feed outputSecretPlaintextsByName and the exact-value matcher, NOT context.resolvedSecretNames, which starts empty. After execution activateOutputSecretProvenance scans the output and adds only names whose plaintext actually appeared; those become __resolvedSecretNames, which tools/index.ts turns into recordResolved calls, which is what the usage trail reads. So a compile-time false positive produces no usage row on the ordinary path — it only hands the matcher a value the code never emits. It does produce one on the !projection.safe fallback, where the system already over-approximates by design. A false negative, by contrast, keeps the value out of the matcher entirely, so a genuinely read secret is never masked on any path. That asymmetry decides it, so every "prove this is not a read" mechanism is gone: - javascript.ts: the file-wide shadow flag. A helper declaring its own environmentVariables discarded genuine reads of the mounted binding everywhere else in the file — Greptile's security finding, and real. - python.ts: the allowlist requiring every mention to be a subscript or .get(). Same hole: passing the dict to a function suppressed unrelated reads. - shell.ts: the rebinding check. It had the same hole in a form nobody flagged — `echo "$API_KEY"; API_KEY=local` dropped the first read, which is of the real secret. What stays is the question of whether the text is code at all — strings, comments, single quotes, quoted heredocs — plus the receiver check that `other.environment Variables['K']` is a different object, and JavaScript's node-precise write/delete exclusion, which cannot suppress a read elsewhere. Net 215 lines removed across the three detectors and their tests. Docs updated: the rule is now stated as reporting rather than proving, and that See usage may occasionally list a secret the code had available but did not read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(secrets): drop the last write-vs-read special case `environmentVariables` is a plain object deserialized from the run payload (route.ts:206), not a handle on the stored secret. Assigning to it changes nothing outside the sandbox and is discarded when the run ends, so separating a write from a read bought almost nothing while leaving JavaScript as the one language still trying to prove a read is not a read. Every language now follows the same rule: report a recognized read of a configured secret name. The only exclusions left are facts rather than inferences — the text is not executable (string, comment, single quote, quoted heredoc), the receiver is a different object, or the name is not statically knowable. Docs note that assigning to the binding does not edit the secret. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(secrets): ship only the fields the trail actually shows Five fields crossed the API and reached no reader: usageDate, firstUsedAt, actorEmail, workflowId and actorUserId. The panel renders the timestamp, the trigger, what used the secret, the actor's name, the run count and the run link; everything else was projected, serialized and discarded. first_used_at is dropped from the table as well. Nothing read it, and inside a per-day bucket "first used that day" says nothing next to "last used that day" — so it was a column written on every run for no question anyone asks. The upsert loses its least() with it. Migration regenerated; the identifier columns behind the joins stay, they simply are not returned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(secrets): report referenced code secrets, not only ones that surface in output The Function route activated a secret's provenance — and therefore its usage row and downstream masking — only when the exact value appeared in the result, stdout, or error. That gate made the trail miss silent use entirely: a key that authenticates an API call and is never echoed reported nothing, and so did the founding scenario of this feature, a key exfiltrated character by character. The innocent run that echoed a key got a row; the run worth catching did not. Activation now follows the referenced set the compiler already computes: resolved {{KEY}} bindings plus recognized direct reads, filtered to configured values — the same set the unsafe-projection fallback already activated. An extra name only hands the output matcher a value that never appears; configured-but-unreferenced values are still never included. The output-scan activation path and its surface helper are deleted rather than kept alongside. One old test pinned the gate ("does not activate a referenced secret that does not cross the Function result"); it now asserts the reverse, with the reasoning attached. Two new tests pin the char-split exfiltration and the silent API-call case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(secrets): shell escaping is backslash parity, not adjacency Review round 8. `\\$API_KEY` is an escaped backslash followed by a LIVE expansion — bash prints `\` plus the value — while `\$API_KEY` is an escaped dollar and stays literal. Checking only the character adjacent to `$` read every even run as escaped, dropping a real read from usage and masking alike; verified against bash before fixing. The scanner now counts the run of backslashes before the `$` and skips only odd runs, the same parity rule logicalLineEndAfterContinuations in this file already applies to line continuations. Six-case parity table added; the three even-run cases fail against the previous check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(secrets): recognize destructured environment reads Review round 9. `const { API_KEY } = environmentVariables` delivers the value by name with no property- or element-access node in the AST, so the member-access walk missed it entirely — and a missed read leaves an emitted value unmasked, the dangerous direction. The AST walk now also recognizes the declaration form (shorthand, renames, defaults, string-literal keys), the assignment form ({ KEY } = env), and a ...rest element — which names no key but takes every value, so it reports every configured name; the alternative left `const { ...all } = env; return all` entirely unmasked. A computed key stays unrecognized, the same runtime-name boundary as a computed subscript, and a receiver that is not the bare identifier is not attributed. Nine cases added; the six positive ones fail against the previous walk. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(secrets): one receiver rule for destructured reads, parentheses included Review round 10. Two accurate findings, folded into a generalization instead of two more special cases: - A parameter default (function f({ API_KEY } = environmentVariables)) and a binding-element default are the same by-name delivery as a variable declaration. The detector now keys on the ObjectBindingPattern itself and checks its parent's initializer, so every declaration position follows one rule instead of per-kind arms. - Parentheses group without changing the receiver, so (environmentVariables) is unwrapped before the identifier check — in the destructuring arm AND the member-access arm, which had the same hole unreported. Declined the for-of-over-array-literal finding: the receiver there is a container, not the environment object, and following data flow through containers has no fixed point — the same documented boundary as aliasing and computed keys. A test pins the boundary so it reads as chosen, not missed. Eight cases added; the seven receiver-rule cases fail against the previous code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(secrets): a dot in prose is not a qualifier, and a literal computed key is a subscript Review round 11. Both findings were implementation-narrower-than-rule, fixed by consulting authorities the detectors already had rather than adding new ones: - python.ts: the receiver walk crosses whitespace so a parenthesized `other.` on a previous line is seen — but it landed on a comment's final period (`# Load the value.`) and discarded the genuine read on the next line. The landing position is now checked against the same lexer ranges that filter the candidates, which is also why the receiver check moves after lexing. - javascript.ts: `const { ['API_KEY']: key } = environmentVariables` is the element-access rule in pattern position, so a computed key holding a string literal resolves like a literal subscript; any other computed key keeps the runtime-name boundary a computed subscript already has. Eight cases added; the comment-period case and all three literal-computed-key cases fail against the previous code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ock (#6836) * fix(workflow): attach cmdk-added blocks to the selected block * improvement(workflow): fall back to last-touched block for cmdk adds, park unattached blocks in the rightmost column * fix(workflow): validate positionless-add sources for eligibility before choosing
) * refactor(consent): fold cookie preferences into General > Privacy The consent settings were a top-level tab of their own, which is the wrong weight for something a user opens once. They are now a sub-view of General, reached from the Privacy section that already held the telemetry toggle, and that toggle moves with them so one page owns everything Sim collects. Cookies render only on the hosted service, the only deployment that sets them; telemetry renders everywhere, so the sub-view is useful on a self-hosted deployment too. Each cookie switch commits on change rather than staging behind a Save, matching the telemetry switch directly above it -- one interaction model per page, and no unsaved-consent state. saveConsents('custom') reads selectedConsents from the store at call time and the switch's write is synchronous, so the value a toggle stages is the value it commits. The open sub-view lives in the URL, so it is linkable and Back closes it. * fix(consent): keep the old /settings/privacy link working The section moved into General, so the path no longer resolves. Redirect it to the replacement view through TOP_LEVEL_REDIRECTS, which the route already uses for the integrations and skills moves. * fix(consent): stop two cookie toggles from racing, and revert a failed one Each save sends the whole selectedConsents snapshot, so two quick toggles could finish out of order and land the older choice. The switches now lock while a commit is in flight, exactly as the telemetry switch does on its own mutation, and a failed commit puts the switch back instead of showing a preference that was never recorded. Also stop the General blurb promising cookie controls on a self-hosted deployment, where the sub-view only carries telemetry.
…pace (#6838) * fix(workspaces): explain why org admins can't be removed from a workspace Organization admins hold workspace admin through their org role, not a permissions row, so removal had nothing to revoke. It failed with "User not found in workspace" for someone listed as an Admin on the same screen, and when they also held an explicit row it deleted a grant the derived one immediately replaced — which could drop their org membership and seat, since the seat reconciliation counts rows only. * fix(workspaces): stop offering leave to org admins and surface refusals Sidebar Leave was still offered to non-owner organization admins, whose access is derived and cannot be given up, and the confirm modal swallowed the refusal — so it sat open with no reason shown. The workspaces list now reports whether the viewer's admin access came from their org role, which `permissions: 'admin'` alone could not distinguish from an explicit grant. Also folds a disabled row action's tooltip into its accessible name, since Radix skips disabled items in a menu's roving focus.
Contributor
|
Too many files changed for review (169 files, 100 file limit). Bypass the limit by tagging |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview Desktop update resolution now pages through GitHub releases so unrelated tags cannot hide the newest build; a new Workspace teammates refuse removal of organization admins with explicit UI/API messages; row action menus expose disabled reasons via tooltip and Smaller changes: design-system z-index layers for takeovers/shell gate, ThemeProvider forced light on more public segments, docs code-block CSS alignment, agent skill scrubbing rules for public repos, and connector sync stale-lock constant reuse. Reviewed by Cursor Bugbot for commit d909889. Configure here. |
![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 d909889. Configure here.
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
3 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.