mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
81ff24a2fc0f5fadb6b8361cb54cd2a6c260287a
6439
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Vikhyath MondretiandClaude Opus 5
|
81ff24a2fc |
improvement(secrets): gate Copilot code mounting at use level (#7004)
* improvement(secrets): gate Copilot code mounting at use level Mounting a saved secret into Copilot code required credential-admin on that key, while a workflow Function block resolves the same secret for the same person at use level through getPersonalAndWorkspaceEnv. Copilot reaches that path itself — edit_workflow plus run_workflow — so the admin bar contained nothing. It redirected a Credential Member through a detour that mutates a persisted workflow, while the direct path is ephemeral and files a usage row. The inconsistency was also internal to Copilot: the secret names advertised to the model come from getAccessibleEnvCredentials and getPersonalAndWorkspaceEnv, both role-agnostic, so Copilot listed every secret the caller could use and then refused to mount all but the admin ones. Widen the workspace and shared-personal predicates to any active grant, and drop the matching role filter from the query. Workspace write is still required, revoked and pending grants are still refused, and a caller with no grant still gets nothing. The view gate stays where Copilot cannot route around it: values remain masked under Settings, and See usage remains admin-only, so a member's use is recorded for whoever can rotate the key. Model-egress projection is untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(secrets): stop implying Personal secrets are shareable The Copilot code-execution paragraph listed "any secret shared with you as a Credential Member or Credential Admin" among what mounts, which reads as though a Personal secret can be shared. It cannot through any product surface: CredentialMembersSection renders only for workspace secrets and OAuth credentials, and the personal-credential sync only ever grants the owner. Narrow the sentence to Workspace grants. The comparison table's "Only you can use" row for Personal was correct and is left alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Vikhyath MondretiandSim Pi Agent
|
9ff1d77306 |
feat(library): Best No-Code and Low-Code AI Agent Builders in 2026 (#7003)
Co-authored-by: Sim Pi Agent <pi@sim.ai> |
||
|
Theodore Li
|
8104c33bba |
Fix knowledge connector sync follow-up (#6927)
* Fix knowledge connector sync follow-up * Fix connector sync pause race * fix(knowledge): surface connector sync dispatch failures * fix(knowledge): make connector sync recovery durable * fix(knowledge): deduplicate connector sync dispatches * fix(knowledge): preserve pending connector syncs * fix(knowledge): lock connector sync snapshot |
||
|
Vikhyath MondretiandClaude Opus 5
|
763f3aa1b5 |
fix(deployments): stop superseded activations from dead-lettering (#6522)
* fix(deployments): stop superseded activations from dead-lettering 29 workflow.deployment.prepare.v2 events dead-lettered with "Webhook registration operation is stale", every one at attempts = max_attempts. A full retry budget means the failure is deterministic, which rules out the preparation path: an attempt superseded while preparing is marked superseded, so its next attempt short-circuits at the top of the handler and completes. The branch a retry re-enters is the other one. isTerminalNonActiveOperation covers failed and superseded but not active, so an attempt that activated and was then superseded by the next deploy keeps its own active status, re-enters post-activation work on every retry, and re-fails the same generation fence until the event dies. The fence it fails is correct — it takes the same workflow row lock the generation bump takes, and compares generations exactly — so nothing about the detection is racy; only the reaction to it was wrong. Reaching it needs a handler timeout, which parks the row for the 10-minute reaper instead of the 2s/4s/8s backoff, opening a window wide enough for a redeploy to land. Gate the resume branch on the operation still owning the current generation, matching the sibling cleanup that already does this, and complete the event as a no-op when it does not. The newer generation adopts the leftover work anyway: it collects every retired registration below its own fence. Also reverse the post-activation order. The audit entry, analytics event, socket notification, and workspace event describe a cutover that is already durable, and each is separately checkpointed, but they ran behind retiring the previous generation's external subscriptions — one provider call per retired row, and by far the most failure-prone step there. A single flaky provider silently cost the deploy its audit trail and left clients on the old version until something else refreshed them. Both call sites now share one helper so the order cannot drift apart again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(deployments): pin staging's active-resume tests to the current generation Two tests that staging added alongside durable PostHog delivery drive the `operation.status === 'active'` resume branch this PR now gates, and neither overrides `mockIsDeploymentOperationCurrent` — the suite's `beforeEach` defaults it to `false`. Under the new gate they read as superseded, so the handler short-circuits: the delivery test sees a resolve where it asserts a rejection, and the unconsumed workflow row leaks into the next test. Both are still-current resumes by intent, so they say so explicitly. The checkpoints, not the generation gate, remain what keeps analytics from being captured twice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(analytics): stop PostHog delivery from failing a deploy `deliverOutboxServerEvent` awaited `client.flush()` before letting the deployment outbox checkpoint advance, so an unreachable PostHog failed the event, retried it, and eventually dead-lettered it — while holding the socket notification, the workspace event, and retired-subscription cleanup behind a third party. `flush()` also drains the whole shared client queue, so an unrelated event's network error surfaced here as a failed deploy. It bought no durability the process did not already have: the outbox handler runs in the long-lived app container, where the client flushes on its own 10s interval and again from the `SIGTERM`/`SIGINT` hook in `instrumentation-node.ts`. The helper had one caller and arrived inside an unrelated squashed PR (#5273) with no rationale, against 161 fire-and-forget `captureServerEvent` call sites. Deleted it and restored `captureServerEvent`, whose contract is already "never throws". `insertId` still collapses retried captures. That contract was untested, which is why this regressed unnoticed, so `server.test.ts` now pins it — spying on the real client, since the lazy `require` defeats `vi.mock` and a disabled client would pass every assertion vacuously. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(deployments): fence the cleanup, not the notifications The resume guard sat at the top of the `active` branch, so it skipped every post-activation step whenever `isDeploymentOperationCurrent` went false. That predicate goes false as soon as any newer generation row exists — including one still `preparing` or already `failed` — and in that window this activation is still the live cutover. Nothing newer would ever adopt its audit entry, analytics event, socket notification, or workspace event, so the guard permanently dropped them and completed the outbox event as if they were owed to someone else. Only the two cleanups are generation-fenced. `cleanupInactiveDeploymentsForOperation` already gated itself on that exact predicate and returned quietly; the retired webhook cleanup was the one that instead let the store's `assertCurrentOperation` throw. It now carries the same guard, on the same fence the store asserts — `deploymentVersionId` and `statuses: ['active']` included, so passing the gate actually implies passing the assert. The notifications run unconditionally, still idempotent through their checkpoints. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Vikhyath Mondreti
|
d4195ff682 |
fix(media): stop the ffmpeg output format from choosing its own path (#7001)
`convert` and `extract_audio` built their output path as
`path.join(dir, `out.${format}`)`, and `format` arrives from the tool as an
unconstrained `{type: 'string'}` — no pattern, no enum, and the handler passes it
through untouched. A format of `../../../../../../../../tmp/x.mp4` resolves to
`/tmp/x.mp4`, so FFmpeg writes attacker-influenced media wherever the traversal
points. Verified against a real binary: the run produced a 44,078-byte MP4
outside the temp directory, which the `rm -rf` cleanup then never saw, because
the file was never inside the directory being removed.
Output extensions are now letters and digits only. The pattern admits no `.` and
no separator, so `out.${ext}` is always a single path segment and containment
follows from the validation itself rather than from a second check that could
drift away from it.
`trim` and `fade` build their paths from `extFromMime`, which returns
`mime.split('/')[1]` — that can never contain a separator, so those stay inside
the temp dir and keep accepting values like `x-msvideo` that this stricter
pattern would wrongly reject.
|
||
|
Justin Blumencranz
|
bb0792564a |
fix(chat): copy workspace resources as portable links (#6956)
* fix(chat): copy workspace resources as portable links * refactor(chat): simplify portable resource copying |
||
|
Justin Blumencranz
|
61c405afb6 | fix(chat): copy orchestrator responses as markdown (#6903) | ||
|
Vikhyath Mondreti
|
e406b8a277 |
fix(charts): confine untrusted chart options to canvas render paths (#6998)
A `.chart` document is untrusted input — any workspace member authors one, and a public share link renders it to anonymous visitors on the app origin — but `parseChartSpec` forwarded `option` to `setOption` unchanged. ECharts draws through canvas with two exceptions: a `tooltip` left in its default `renderMode: 'html'` assigns its content to `innerHTML`, and a string `formatter` is that content's template verbatim (only substituted values are escaped), while `toolbox` assigns `dataView.lang` to `innerHTML` and fills a `saveAsImage` popup with `document.write`. Force the render mode and drop the toolbox so the document has no DOM sink, rather than filtering the values that flow through one. The walk is deep: `baseOption`, `media[].option` and timeline `options[]` each carry their own tooltip, a tooltip declared only under `media` still instantiates the component in HTML mode, and a `media` entry can override a top-level `renderMode`. Also escape the series name in the pptx chart renderer's bubble tooltip — it comes from the uploaded document and is interpolated into a hand-built `innerHTML` string, where ECharts escapes only the markup it builds itself. |
||
|
Bill LeoutsakosandBill Leoutsakos
|
6f919ef192 |
fix(terminal): keep running-command tooltips concise (#6991)
Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> |
||
|
Vikhyath Mondreti
|
cbf3aad569 |
fix(workflows): scope the canonical sub-block index to the active surface (#6990)
* fix(workflows): scope the canonical sub-block index to the active surface A block that is both an action and a trigger holds one `subBlocks` array — its own fields plus its trigger's, spread in after them. The two sets routinely share a `canonicalParamId` under different ids, so indexing them together collapses a trigger field into an action pair whose `basicId` it can never be. Every group-relative question about that field then answers for the dormant surface. The serializer was never affected: `shouldSerializeSubBlock` drops the inactive surface before the canonical collapse reads it, so it resolves against a value map the dormant surface cannot appear in. Every other caller resolves against the block's full value map, so the scoping has to live in the index. - add `getCanonicalSubBlocksForSurface` / `buildCanonicalIndexForSurface`, and move the three sites that already had the filter inline onto them - `getCardSubBlocks` derives its own index instead of accepting one; it already took `triggerMode`, and accepting an index is what let all three callers pass one built for the other surface - scope the remaining consumers that resolve against a full value map: the canvas card, autolayout, both preview surfaces, the dependsOn gate, the canonical value hook, reactive conditions, and the copilot selector lint - keep a canonical group with no advanced member out of the legacy `advancedMode` path, which deleted its basic member and republished nothing - merge legacy type-scoped tool modes as a baseline under index-scoped ones, so the first re-toggle stops reverting the ids the user has not touched * fix(workspace-forking): scope the canonical gates to the block's active surface `createCanonicalModeGates` indexed a block's whole `subBlocks` array, so on a mixed action/trigger block a trigger field sharing a `canonicalParamId` with an action pair was read as a member of THAT pair. Being neither its `basicId` nor in its `advancedIds`, `isDormantMember` answered true the moment the shared mode resolved to advanced — and a fork acts on that by clearing the value, so a configured trigger field was silently wiped on fork/sync. Reachable without any explicit toggle: a block configured as an action with a manual id and then switched to trigger mode leaves the pair's value heuristic resolving to advanced on its own. - `createCanonicalModeGates` takes the surface and scopes its index - thread `triggerMode` through `RemapForkContext`, `SubBlockTransform`, `clearDependentsOnRemap`, `collectClearedDependents`, the reference scanners, and the promote cleared-ref collectors - nested tool params and the dependent scan are unchanged: a tool is always the action surface, and the dependent scan already narrows its configs * fix(workspace-forking): keep the dormant surface classified as it was before scoping Surface scoping decides canonical membership for the ACTIVE surface. Applying it to every key also re-classified the dormant surface's own values: they stopped being dormant members, which meant the remap no longer cleared them AND started detecting them as references — turning a stale action selector on a trigger-mode block into a mapping requirement that can block promote/sync. The gates now pick the index per key: the scoped one for anything the active surface defines (the fix — a trigger field gets its own group instead of being read as a stranded member of an action pair), the whole array for everything else, which is byte-for-byte the pre-scoping behavior. Also adds `check:canonical-index`, an audit that fails any call building a canonical index off a config's whole `subBlocks`, or calling the fork gates without a surface, unless annotated with why. This defect shipped three times in three subsystems; the 14 sites that legitimately mean one fixed surface now say so at the call. * fix(audits): stop the canonical-index guard flagging its own source The audit holds `buildCanonicalIndex(` and `createCanonicalModeGates(` as string literals to search for, and its own regex matched them — the arg-count rule then fired on the literal. It passed locally only because the file was still untracked when it ran, so `git ls-files` did not list it; committing it made the audit scan itself and fail CI on the first run. Exempts the audit's own source alongside the module that defines the primitives. Verified the guard still fails on both regression shapes after the exemption. |
||
|
Vikhyath MondretiandClaude Opus 5
|
91e927442b |
fix(media): bound the ffmpeg tool's child processes, inputs and scale targets (#6989)
* fix(media): bound the ffmpeg tool's child processes, inputs and scale targets The copilot ffmpeg tool shells out to FFmpeg in the Sim app process — not in a sandbox — and nothing bounded the run. runCommand had no timer, no kill and no signal, so a transcode ran until it finished and survived the request that asked for it: a stopped copilot turn left the encode pinning both cores of a shared instance. Every operation now shares one 10-minute wall-clock deadline, and both the deadline and the caller's cancellation SIGKILL the child. The budget is per-operation rather than per-command because concat runs a full re-encode per input, so a per-command timeout would let N inputs multiply into N timeouts. Cancellation is wired to context.abortSignal, whose every abort reason is an explicit user stop — the copilot lifecycle tracks a passive client disconnect separately and does not abort on it — so an encode dies when the user asks and not before. (userStopSignal, which assertServerToolNotAborted reads, has no producer on this path.) ffprobe had the same shape in miniature: fluent-ffmpeg's static ffprobe hands back no process handle, so a timeout could only race the callback and leave a wedged prober alive, and concat probes once per input. It now runs through execFile, which takes timeout, killSignal and signal natively. Scale targets reached the filter graph unvalidated, where libavfilter sizes its per-frame buffers from them — scale=30000:30000 is ~2.7 GB a frame, allocated in a child that shares the instance's memory. Dimensions are now rejected outside 16-4096 with a message the model can act on, plus an area cap. concat's targets come from the source container rather than a caller assertion, so those are clamped instead. Two further holes: readOut buffered the output with no ceiling, and CRF-18 re-encodes routinely exceed their input, so the input budget did not bound it; and the handler accepted unlimited input files. Both are capped, and the byte check now runs against the recorded size before the download rather than after. FFMPEG_LIMITS is the single source for the four numbers the Go tool catalog mirrors into its schema, pinned by ffmpeg-schema-parity.test.ts so the model is never told a ceiling the executor does not enforce. Companion: simstudioai/copilot#PENDING Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(media): fit concat inside the shared area budget and stop input prep on cancel Two review findings, both real. concat clamped each probed axis independently, so a 4096x4096 source produced a normalization target of exactly that — nearly double the area budget scale_pad enforces through the same scale=/pad= graph. The pair is now scaled down together, so aspect ratio survives and one ceiling governs both entry points. Dimensions come back even too, which yuv420p requires and per-axis rounding did not guarantee. Preparing the inputs is itself the expensive half of a many-file call — up to the whole byte budget in storage reads — and the abort signal was only consulted after the loop, so an explicit stop was observed only once every download had already finished. The loop now checks it per input. Also corrects the rationale comments on the mirrored limits. maxItems/minimum/ maximum do not reach the model: copilot's NormalizeToolParameters allowlists type/properties/items/description/enum/required, so the bounds travel only the contract path, where Sim's router validates against them. That is still worth having — it turns an out-of-range argument into a structured rejection before any storage read or child process — but the model learns the limits from the parameter descriptions, and the comments now say so instead of claiming the schema teaches it. * fix(media): floor the scaled concat axes so the area bound actually holds The scale factor lands both axes on a product of exactly the budget, so an axis allowed to round up can put the pair back over it — and when both round up and both land even, nothing pulls them back. A brute force over every dimension pair in 16..4096 finds 219,280 that violate the bound under round-to-even, worst 2694x3520 -> 2688x3512, 3072 pixels over. Under floor-to-even: none. Flooring keeps each axis at or below its exact target, so the product cannot exceed the budget. Probed dimensions are already integers, so this changes only the scaled path. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Vikhyath Mondreti
|
2c61b2989c |
fix(analytics): stop dropping client events captured before PostHog init (#6988)
* fix(analytics): stop dropping client events captured before PostHog init posthog.capture is a no-op until init() has run — its body sits behind if (this.__loaded) with no buffer and no warning. PostHogProvider reaches init through two dynamic imports while a caller needs only one, so mount-time events and error-boundary crash reports during first paint landed in that dead window and were discarded silently. Also routes the workflow canvas error boundary through captureException so its crashes reach error tracking with a parsed stack. * fix(analytics): drop unactionable browser noise from error tracking ResizeObserver loop notices were 92% of everything client-side exception autocapture reported. They are specified behaviour, not faults — the observer defers the remaining notifications to the next frame — and they carry no stack that points anywhere. Alongside opaque cross-origin "Script error." reports and cancellation signals (fetch AbortError, Monaco's CancellationError), they bury the handful of real crashes. Filters them in before_send so they never leave the browser. Fails open on anything unrecognized: a wrong guess here would delete product analytics, not just over-report. * fix(analytics): match cancellations on error name, not coerced type Two holes in the exception filter, both found reading PostHog's coercers rather than trusting the shape. The DOMException coercer always reports type "DOMException" and folds the name into the value as "AbortError: signal is aborted without reason", so matching AbortError on type alone never fired for a real aborted fetch — the filter was dead code for exactly the events it was written for. Match the error name wherever the coercer put it. The filter also applied to deliberate captureException reports, which our error boundaries make. Only exceptions the browser raised itself (mechanism.handled === false) are now eligible; a report we chose to send is never dropped. * test(analytics): make posthog client tests order-independent Readiness is module-level and settles once, so the capture and exception cases silently depended on an earlier case having settled the gate and failed when run in isolation. Walk the lifecycle in one case instead. Moves the disabled-analytics case to its own file, where vitest's per-file isolation supplies an unsettled gate — dropping the vi.resetModules() and dynamic import the repo's testing conventions rule out. Replaces the hand-rolled setTimeout flush with vi.waitFor. |
||
|
Waleed
|
b654bc11cd |
improvement(navigation): eliminate hidden workspace requests (#6979)
# Conflicts: # apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/deploy.tsx |
||
|
Waleed
|
ab35ff4c8c |
fix(resources): make context menus selection-aware (#6980)
* fix(resources): make context menus selection-aware * fix(resources): correct bulk action scopes * fix(knowledge): avoid overstating select-all scope * fix(knowledge): keep select-all actions consistent |
||
|
Waleed
|
e46fc1d1f5 |
improvement(settings): reduce navigation and section load latency (#6992)
* improvement(settings): reduce navigation and section load latency * improvement(settings): seed a section's first-paint data on the server A settings section paid three serial hops: the route payload, its lazily loaded chunk, and only then its own queries. The first two are addressed; this starts on the third, which is what fills in after the heading paints. Generalises the existing general-settings prefetch into a per-section registry, awaited before dehydration and started as soon as workspace access is established, so the body renders populated the moment its chunk lands instead of blanking again while it fetches. Adds credential-groups as the first section beyond general/billing/admin. It qualifies on both counts the registry requires: `listCredentialGroupSettings` is a `defineAuthorizedWorkspaceUseCase`, so prefetching through it applies the same authorization and entitlement checks a client request would, and it returns exactly what the hook caches under `credentialGroupKeys.list`. The registry is deliberately sparse. An audit of all 29 sections found that most authorize in their route handler rather than their data layer, so prefetching them means first lifting that check out of the route — and two must never be prefetched naively at all, because the route is also what redacts the response: SSO strips the OIDC client secret before it leaves the server, and MCP withholds upstream credential headers from read-only members. Reaching past either would ship secrets into the dehydrated payload. * fix(settings): keep optional prefetch failures nonfatal * fix(settings): surface organization load failures |
||
|
Waleed
|
b1759f3545 |
improvement(menus): band by what the action acts on, not by verb (#6994)
Replaces "at most one separator" with the test that actually earns a rule: does the next group stop acting on the thing the user clicked. Most row menus have only the destructive transition and keep their one rule. Three menus have a second, and now show it — the logs row menu between this log and the page's filters, and the table row and column menus where they stop acting on the clicked cell or column and start creating siblings. Count follows content rather than a fixed number. The workflow panel menu had six items across four scopes and no separator at all, so "Delete workflow" sat flush against "Duplicate workflow"; it now carries the destructive rule every other menu has. Two label fixes: the logs menu wrote 'Retrying...' with ASCII dots two lines above 'Stopping…' with the character, and "Import CSV…" was the only ellipsis anywhere in the workspace menu surface while the same action reads "Import CSV" in the list menu. |
||
|
Waleed
|
37590bdf73 | fix(csv): neutralize formula-leading exports (#6993) | ||
|
|
c26529a82e |
feat(bitbucket): add repository webhook triggers (#6934)
* feat(bitbucket): add repository webhook triggers * fix(bitbucket): harden webhook trigger delivery * fix(bitbucket): address final trigger review * chore(bitbucket): address review conventions * fix(bitbucket): harden triggers and connector sync --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> Co-authored-by: Waleed Latif <walif6@gmail.com> |
||
|
Theodore Li
|
7173a3fb37 |
feat(status): surface major service incidents (#6987)
* feat(status): surface major service incidents * fix(status): isolate status endpoint failures |
||
|
Vikhyath Mondreti
|
7ea8692e7d |
fix(files): read a file nobody vouched for, as an untracked one already is (#6986)
* fix(files): read a file nobody vouched for, as an untracked one already is A workspace file whose secret provenance could not be recorded was refused by every model and runtime boundary at once — attachments, mounts, and the tool routes that parse, transcribe or describe a file. A file that was never tracked at all returned exact-empty and worked fine. Both say exactly as much about their contents: nothing. So the refusal was a permanent penalty for having tried to record provenance and failed, with no way back, since nothing rewrites a file's provenance but another content write. Workspace files now do what the other durable surfaces do — proceed and record an audit entry naming the surface and the count, so the people who own the secrets are told. The state is distinguished from the four that really are unknown (missing row, version mismatch, stale binding, malformed), which stay refused; only a current sidecar that says so reads as unrecorded. Absence survives a merge rather than dropping through to exact, so a derived file cannot launder an unrecorded input into a positive claim. Script migration 0007 clears the files already stranded in the old state, taking the parent lock first in id order to match the live writer, with the unknown status re-checked under it so a concurrent write is never undone. Nothing changes for a legacy file: no sidecar row still means untracked, still reads exact-empty. The enforcement flag stays in place. * fix(files): narrow an initialization to a status the column accepts The union carries three states; the sidecar's CHECK constraint accepts two. Only the replace path discriminated — initialize forwarded the status verbatim, so an 'unrecorded' would have reached the database as a value it rejects, aborting the enclosing transaction rather than writing a bad row. Nothing passes one today, which is why it belongs here and not in a caller. Also names the storage mismatch on the type itself: a stored 'unknown' reads back as 'unrecorded', not as the union's 'unknown'. Same word, two layers, different meanings. * fix(provenance): repair only what the reader calls an absence The repair selected on status alone, which is wider than the branch it was meant to undo. The reader answers 'unrecorded' only for a sidecar that is version 1, bound to the file's current bytes, and holding a well-formed entries array; anything else is a fault it refuses and no policy relaxes. Clearing a fault was not a smaller claim but a larger one. Deleting the sidecar sets secret_provenance_version to NULL, and a NULL version reads as exact-empty — so a file refused because its provenance described bytes it no longer holds would have come back positively vouched for, silently, with no audit entry. Four such rows exist in production today. Both the candidate query and the delete now carry every condition, so a concurrent write cannot lose the distinction between an absence and a fault. Stale, unversioned and malformed sidecars stay refused exactly as the surface refuses them, and recover as they always have, on the next content write. * fix(provenance): store an absence apart from a refusal Relaxing a stored `unknown` assumed it always meant "nobody recorded this". It did not. The sidecar collapsed two opposite claims into one value: bytes nobody recorded, and bytes a writer refused on purpose — a child of an archive whose parent provably held secrets, a generated asset whose safety decision came back false, a transcode whose scanner knew secrets were present and could not locate them. Relaxing the second would have walked known-secret-bearing content through every model boundary at once. The decision type already told them apart: a registry that never ran is `safe: true`, while every refusal is `safe: false`. Only storage lost it. Writers now persist `unrecorded` and `unknown` separately, readers relax the first and refuse the second, and the constraint is widened to accept it. Only workspace files store this distinction, deliberately. Every other durable surface produces a non-exact sidecar from one condition — an incomplete incoming bundle or registry — which is always an absence. Files are the only surface that derives one stored object from another, so they are the only one that can refuse on purpose. The shared policy is unchanged and uniform: read an absence and audit it, refuse a taint. Also aligns the metadata batch classifier, which answered `unknown` where the single-file reader answers `unrecorded`, leaving two classifiers describing one policy differently. * test(provenance): assert the enforcement switch on a row it can act on The test that pins "closing the surface refuses an unrecorded file again" used a stored unknown, which the split had just turned into a taint. A taint is refused whatever the flag says, so the assertion held with enforcement off too and proved nothing about the switch this posture rests on. It now uses a recorded absence, and fails if the flag is ignored. * fix(files): copy a recorded absence as an absence The copy path required an exact source, so a stored unrecorded one fell in with the refusals and the target was written as a taint the source never carried. A workspace fork or a chat file copy therefore turned a readable file into a permanently refused one — the pathology this surface exists to undo, reached by copying — and nothing rewrites a file's provenance but another content write. Safe past the scope checks it now precedes: those exist to stop one workspace's secret entries landing in another's file, and a recorded absence has no entries to carry. The legacy Function export marker keeps writing unknown. Its record had resolved secret names in scope and an unreadable bundle, so whether that is an absence is not something this change can establish, and guessing in the readable direction is what caused this round. |
||
|
Justin Blumencranz
|
fb8f0d66d1 | improvement(files): improve file sharing UI (#6983) | ||
|
Vikhyath Mondreti
|
8c7a2f1df0 |
fix(uploads): require an explicit byte ceiling on workspace-file downloads (#6985)
* fix(uploads): require an explicit byte ceiling on workspace-file downloads Workspace files are admitted at 5 GB because they stream straight to object storage, but a tool that pulls one back to hand it to a third party buffers the whole thing in the shared app process. maxBytes was optional on every download helper, so 51 call sites had silently inherited "unbounded". Make maxBytes required on all five entry points so a new call site cannot inherit it again, and give each existing site a ceiling: the destination's own documented limit where the route already declared one, otherwise the 100 MB this codebase already uses for buffered work. Multi-attachment routes were the worse case — Gmail, Outlook, SendGrid and SMTP downloaded every attachment via Promise.all and only summed the sizes once they were all resident, so their pre-check on declared sizes protected nothing. Add downloadServableFilesWithinBudget, which walks the list against a shrinking budget, and use the same running budget in Slack, Jira, Discord and Quiver. * fix(uploads): bound Sim-page asset inlining before the bytes are resident The ceiling on the rendered page checked the finished document, by which point renderSimPageDocumentWithAssets had already downloaded every referenced image concurrently with no per-download limit and base64-inlined them — so the allocation the check exists to prevent had already happened. Pick the inline set from recorded sizes before fetching anything, against a per-document budget as well as the existing per-image one, and give each download its own ceiling in case a row understates its object. An image that does not fit keeps its URL reference, exactly as an oversized one already did. * improvement(uploads): charge the page-render budget by delivered bytes Planning the inline set from recorded sizes left the aggregate ceiling resting on metadata being accurate, and needed a paragraph explaining why that was safe. Downloading one image at a time and subtracting what each download actually returned needs no such argument: the budget cannot be exceeded whatever a row says, and the peak is one image rather than the sum of them. Also drop two rough edges the first pass left behind — an SFTP total-size check that became unreachable once the download carried the remaining budget, and a Dataverse error helper whose optional size argument existed only to paper over one caller that had not passed it. |
||
|
Vikhyath MondretiandClaude Opus 5
|
28cfa4361a |
fix(deploy): resolve subblock values to their configuration on both sides of change detection (#6984)
* fix(deploy): resolve subblock values to their configuration on both sides of change detection Focusing a deployed trigger block flipped the deploy button from Live to Update for a change the user never made, and redeploying could not clear it. Deploy materializes every declared `defaultValue` into `webhook.providerConfig` (`getConfigValue`). Opening a trigger block's panel reads that derived artifact back into live state through a non-persisting `setValue`, so the block gains keys the DB draft — which deploy snapshots — does not have. The comparison then reported a difference for a field nobody set. Because deploy snapshots the draft, the next deployment lacked the keys too: a fixpoint. Measured on production: 89 of 89 recent `generic_webhook` deployments were missing `acceptOtherMethods`, and 1,046 of 2,139 active webhooks carry at least one `providerConfig` key their block has no entry for. This is the tenth instance of one failure mode — |
||
|
Vikhyath Mondreti
|
b861062370 |
fix(provenance): clear the rows that went unknown after the first repair (#6982)
* fix(provenance): clear the rows that went unknown after the first repair 0005 is finished and will not run again — the runner records a name in script_migrations and never offers it back, which is the contract a run-once repair wants. But it cleared the backlog that existed at the instant it ran, and the writer that produced that backlog kept running until the fix in this branch. Nothing heals such a row in place, so each one goes on reporting on every later read; a few dozen of them account for thousands of log lines a week. A second entry rather than deleting the first's tracking row: the registry is append-only, and a repair that ran twice should say so twice. It shares 0005's walk rather than restating it — the parent-first lock ordering and the status re-check under that lock are subtleties worth having once — and is idempotent, so it costs one empty query if there is nothing left to repair. * chore(provenance): record the deploy ordering the second pass depends on promote-images needs migrate, so a script migration runs while the previous image is still serving. A row an old instance creates between this walk and the end of the rollout sits behind the cursor, and the name is recorded on success, so it is never offered again. Widening the walk would not help — the exposure is the minutes after it returns, not the milliseconds during — so the requirement is to ship it in a release after the writer fix is already promoted, which the file now says. |
||
|
Vikhyath Mondreti
|
a721a76044 |
fix(provenance): stop requiring a projection of roots no model sees (#6981)
* fix(provenance): stop requiring a projection of roots no model sees A secretProvenance selection is the opposite mechanism to a projected model input: the value reaches an internal API unchanged, with its provenance alongside it in the private bundle, precisely so nothing has to be substituted. selectBlockBoundaryPaths marked those roots required to project anyway. That made a projection failure fatal for tools with no way to project. createStructuredModelProjection rescues only a mode: 'project' tool with an applyProjected, so for the twenty-odd secretProvenance-only tools it returns undefined on its first check. table_insert_row declares no modelInput at all and posts row data to the table API; when the Table block's params threw on its projected data the whole run's registry latched, costing provenance for every later boundary — including the table write that prompted it, whose rows were then stored unknown and re-reported on every later read. Track those paths, require none of them. A root is required to project when a model will see it, which is what modelInput declares. Also separate a crossing that carried no provenance from one that was rejected, so a run that failed before producing any stops reporting a by-design state as an originating fault; and carry the first guard's location into the diagnostics a refusal reports, so a downstream reporter names where rather than only what. * fix(provenance): report where the guard tripped on a refusal too The registry retained the first guard's location, but the refusal reporter copies named fields out of the diagnostics and never picked it up — so the line an operator actually reads when a projection is refused still named only the reason. Nested rather than spread flat: the guard's `inputPath` names where it tripped and the refusal's names where the refusal happened, which differ whenever a latch travels, so flattening would overwrite one with the other. |
||
|
Theodore Li
|
6a5e2502c3 | improvement(ship): skip confirmation prompts (#6977) | ||
|
Bill LeoutsakosandBill Leoutsakos
|
be508b0789 |
fix(utils): prevent randomInt power-of-two hangs (#6955)
Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> |
||
|
Bill LeoutsakosandBill Leoutsakos
|
87ceaf2e20 |
fix(auth): recover from SSO provider lookup errors (#6966)
Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> |
||
|
Waleed
|
4b4ab2479f |
improvement(menus): one separator per menu, before the destructive action (#6974)
* improvement(menus): one separator per menu, before the destructive action Menus banded themselves into semantic groups — navigation, status, edit, copy, destructive — behind two to four separators each. No toolbar in the app renders a divider: every header is a flat gap-1 chip row and every bulk action bar a flat gap-[5px] run. The bands therefore taught a taxonomy the user met on no other surface, and because each band is conditional, the same action landed in a different group depending on which siblings happened to be visible. Pin sat alone in one caller of the shared workflow menu and beside Duplicate in another. Every menu now carries at most one rule, immediately before the destructive group. Order is untouched, so the toolbar-mirroring the ordering rule requires is unaffected. Two separator bugs fixed. The logs row menu had two unconditional separators above conditional items, so a log already filtered by its workflow with no active filters ended on a dangling rule. The shared workflow menu guarded its destructive rule on showLeave alone while the Leave item required showLeave && onLeave, so a caller passing showLeave from a permission check with a conditional onLeave would trail a rule under the last item; every term in both guards is now the exact render condition of the item it stands for. Removes groupNonDestructiveActions and separateNavigationAction. Between them they moved one separator for one caller, four of six branches were unreachable, and separateNavigationAction had no observable effect anywhere in the repo. The separator matrix was previously untested, which is how the showLeave asymmetry survived; it now has invariants including a flag sweep. * fix(menus): build every separator guard from its items' exact conditions Review caught two places where the grouping rule and the code disagreed. The tables row menu guarded its rule on `onMove` while the Move submenu needs a non-empty `moveOptions`, so a table whose other actions were all absent and whose move list was empty would draw the rule with nothing above it — the exact looseness the rule warns about. The logs row menu puts its one rule after Retry and Cancel Run rather than before a destructive group, which the rule as written did not cover. Retry is the primary action on a failed run and belongs at the top; the rule now describes the separator as fencing the consequential group at whichever end it sits, and names the logs menu as the one place that group leads. Also aligns three empty-space menu labels with the header chips they mirror: "Add document" and "Create chunk" were the only create actions not matching their toolbar, and the files menu said "Upload file" where its header says "Upload". Run order in the two column run menus now matches the action bar and the row menu — incomplete before all, not all before incomplete. * fix(menus): apply the one-rule grouping to the folder context menu The folder row menu kept a separator at its canEdit permission boundary plus one before Delete — the same shape already corrected in the file row menu, missed because the sweep that found it did not cover this file. Open and Pin above are unconditional, so the surviving rule is always backed on both sides. Also records the standing exception the sweep surfaced: the text editor, terminal, and browser page menus emulate native OS menus, whose banding the user learns outside Sim. That is the ordering rule's own principle — mirror the surface they already read — so those keep their banding while our own resource and row menus, whose toolbars are flat, take one rule. * fix(menus): keep empty-row actions together |
||
|
Waleed
|
d1b01849a8 |
improvement(perf): cut server-only and unused code out of the workspace client bundles (#6975)
* improvement(perf): cut server-only and unused code out of the workspace client bundles Every workspace route shipped JavaScript it never executes. Four independent import edges, each fixed by moving a symbol rather than changing behaviour: - js-tiktoken's BPE rank tables (5.4 MB source / 2.5 MB wire) reached the workflow editor because the tokenization barrel re-exported the exact counters alongside the character heuristics. Split into lib/tokenization/accurate.ts, which the barrel no longer re-exports. - crypto-browserify (~105 KB gzip, all 26 workspace routes) came from the Salesforce and Gong triggers importing webhook provider modules that reach node:crypto through @sim/security. The two symbols they actually needed are now in crypto-free modules. - tables, files and knowledge each imported one dependency-free hook from the sidebar-hooks barrel, whose other exports reach the 5 MB generated tool-metadata artifact. Deep-imported per the code-splitting rule in sim-imports.md. - lib/workflows/subblocks/display.ts imported a string constant from a React module under app/, inverting the app/lib layering. Moved to lib/. Also adds a loading boundary to chat/[chatId]. Without one, a dynamic route is prefetched as nothing, so clicking a chat held the previous chat on screen for the whole server round trip. Measured on a production build, JS downloaded before the load event: /w/[workflowId] 7.38 MB -> 4.80 MB (-35%) /logs 4.57 MB -> 4.44 MB /knowledge 4.35 MB -> 4.22 MB /home 4.57 MB -> 4.44 MB crypto-browserify no longer appears in any shipped chunk. The tool-registry boundary baseline is retightened so the reclaimed graph weight cannot silently regress. * improvement(sidebar): move useContextMenu to shared hooks Review flagged the four workspace routes deep-importing `useContextMenu` from the sidebar's hooks barrel as a barrel-convention violation. Fair — the code-splitting exception in sim-imports.md is written for `lazy()` splits, and these are static imports. The hook was in the wrong place to begin with. It is entirely generic — no sidebar-specific references, just right-click state and positioning — and nine consumers across tables, files, knowledge, home, the terminal and the preview editor already reached across features to get it. Moved to `@/hooks`, the repo's shared-hooks location, and every consumer including the sidebar's own now imports it from there. This satisfies the barrel convention rather than making an exception to it, and keeps the graph win: tables, knowledge and files stay off the sidebar barrel's path to `stores/workflow-diff -> serializer -> tools/metadata`, unchanged at 20.19 / 20.89 / 21.37 MB of reachable source. |
||
|
Waleed
|
3a04426f37 |
improvement(settings): make workspace settings navigation feel instant (#6964)
* improvement(settings): make settings section navigation feel instant
Every settings tab switch ran four sequential round-trips with no visual
feedback: a cold RSC request, then the panel render, then the section's
lazy chunk, then its queries. The heading was pushed up from the section
body, so the most static thing on the page arrived last and visibly
blanked between sections.
- resolve the section heading in the route layout from its navigation
entry, so it paints with the shell instead of after the body's chunk
- add loading.tsx to all four settings planes, so a click commits the
navigation immediately instead of holding the outgoing section
- give every code-split section a shared skeleton fallback, reused by the
route boundary and the in-page Suspense boundary
- prefetch the route payload on sidebar hover/focus; the rows are buttons
for the unsaved-changes guard, so they never got Next's <Link> prefetch
- scope the general-settings server prefetch to the three sections that
read it, instead of blocking all 28 on it
- set staleTimes so returning to a tab reuses the client router cache
rather than re-running the access gate
- warm workspace credentials under the type the secrets panel queries;
the previous warm wrote a different cache entry and never landed
* improvement(settings): drop the skeleton and the app-wide router cache change
Follow-up review of the diff against the rest of the platform.
- The 4-row body skeleton had no precedent at this layer. Every route-level
fallback in the app renders real chrome over empty content instead:
ResourceChromeFallback renders its header and column headers with rows={[]},
and the credit-usage fallback renders its real title and description over
nothing. Skeleton is only ever used for in-component sub-regions. The
loading boundaries now render an empty body, so the heading is what signals
arrival and nothing shifts when the real body lands. This also removes the
shared dynamic() options module, leaving all four section renderers
untouched by this PR.
- Remove the experimental.staleTimes block. static: 180 silently downgraded
Next 16's own default of 300, and dynamic: 30 is an app-wide change to
client router cache reuse that this PR does not need: the segment cache
already floors prefetch entries at 30s, so hover prefetch pays off without
it. It deserves its own PR and its own measurement.
- Restore the six section chunk warms that existed before this PR. Dropping
them alongside the 28-section map was an unintended regression; they were
already in the module graph, so warming them costs nothing.
* improvement(settings): fix header regressions found in review
Six independent review passes over the diff. Three real defects, all
introduced by the header-meta fallback or the loading boundary.
- A denied organization section rendered the section's catalog heading,
description and Docs link above a "you do not have access" body, because
SettingsUnavailable renders its own centred heading and registers nothing.
It now claims an empty header, which is how a body opts out of the meta
fallback. Releasing the header is an explicit null rather than an
EMPTY_CONFIG sentinel, so "no body owns this" is stated instead of implied.
- The account credit-usage route resolves to its parent billing section, so
the shell painted "Billing" in the server frame before hydration swapped in
"Credit usage". The shell now only supplies meta for a section's own route,
not for detail routes beneath it.
- Adding loading.tsx put the page inside a Suspense boundary, where
notFound() and redirect() can no longer set the response status: a legacy
or unknown settings URL loaded directly answered 200 and redirected in a
second round trip instead of 307/404. Segment-level routing moved into the
layout, above the boundary, which is also where it belonged.
Also from review:
- Parallelize two pairs of independent awaits in the access gate. Every await
there sits in front of the section body, so this shortens the exact wait the
PR is about.
- Note in settings-header that the layout effect is load-bearing: a passive
effect would let the previous section's title show for a frame.
- Correct the loading.tsx docs, which claimed the empty body matched every
other route-level fallback. It is the only null one; the accurate statement
is that the shell above it already renders the chrome.
- Tests: cover resolveSettingsSection's alias table (previously untested on
either side of the move) and the general-settings prefetch gate. Strengthen
the wholesale-substitution test, which passed against a field-merge
implementation because SettingsPanel always emits a description key. All
four verified against mutants.
* improvement(settings): scope the change to the workspace settings plane
Two review findings, both from extending the mechanism to the account,
organization and self-host planes without extending the fixes with it.
- The loading boundary softened 404s on those three planes. Segment
validation was moved above the boundary for the workspace plane only, so a
direct load of an unknown or legacy segment on the others answered 200 and
soft-404ed after hydration.
- SettingsUnavailable's opt-out registers in a layout effect, which does not
run during SSR. A direct load of a denied organization section still painted
the denied section's catalog title, description and Docs link until
hydration — the exact caption the opt-out was added to prevent.
Feeding the header server-side on those planes needs per-section access at
layout level, which is a routing change well beyond this PR. So the three
standalone loading boundaries, the standalone shell's meta, the
SettingsUnavailable opt-out and the standalone sidebar's route prefetch are
all reverted: without a loading boundary in the subtree the scheduler skips
the segment request, so that prefetch bought nothing on its own.
What ships is the workspace settings plane, where the same mechanism is
correct end to end: segment validation and the heading both resolve in the
layout, above the boundary, and a denied section redirects rather than
rendering an unavailable body under a header.
* improvement(settings): stop re-resolving credential-group availability
The access gate asked `isCredentialGroupsAvailable` for an answer the host
context had already derived from the same owner billing one await earlier, so
every workspace-section navigation paid a second feature-flag lookup to learn
what was already on `hostContext.features`.
* improvement(settings): resolve only the entitlements the gate can act on
The access gate fanned out four entitlement lookups for every workspace
section and then built the whole navigation list to ask whether one section
was in it.
- `inbox` and `sandboxes` feed only `locked`, which marks a section as needing
an upgrade rather than hiding it. The gate reads membership alone, so those
two billing round-trips could never change the outcome for any section.
Removing them is behaviour-identical, not a narrowing.
- `forks` is read only by the `forks` entry, so every other section was
resolving a lineage check it could not act on.
`permissionConfig` is deliberately left alone: its keys hide sections, so
skipping the lookup for a section that turns out to be config-gated would
reveal it. That fails open, where the other two fail closed.
Opening a section such as secrets or byok now awaits nothing beyond the
already-conditional permission-group read.
Also corrects the chunk-warmer rationale. Measurement showed the cost is the
boundary audit counting `import()` as a graph edge, not parsed JS — each
section is already `dynamic()`-imported by the panel — and that code-splitting
the sidebar moves exactly one module, so it cannot unlock warming the rest.
|
||
|
Waleed
|
8937bb3550 |
fix(kb): let the server say a connector sync is queued (#6968)
* fix(kb): let the server say a connector sync is queued The connector chip inferred "a sync is coming" from `createdAt` inside a 2-minute window, because nothing on the row distinguished a queued sync from an idle connector until a worker took the lock. The guess was wrong under queue backlog and under client clock skew, and it forced a pile of client state to stand in for it. Adds `pending`, written as the sync is handed to the queue and cleared when a worker takes the lock or the hand-off is found to have been lost. It is a phase of the same lock `syncing` holds, so it opens the lease and takes an ownership token the same way — the lease is what the scheduler ages a stranded queue entry against (`updatedAt` cannot serve: a pending connector is still editable, so any unrelated write would renew the recovery it should trigger), and the token is what proves a late release belongs to this dispatch. Deletes the 2-minute window, the in-flight id sets, the 5-minute cooldown timers and the forced re-render they needed. The cooldown lived in a ref inside a modal, so it evaporated whenever the modal closed; the disable now comes from durable server state and is shared across tabs. Also fixes, all found while tracing the lifecycle: - An on-demand sync on a paused or disabled connector silently resumed it for good. Nothing could put the pause back: success writes `active`, a lost queue entry writes `error`, and the due-sweep keeps syncing that. Refused. - A failed hand-off no longer advances the connector's auto-disable breaker. A queue outage would otherwise increment every connector in the fleet until they all disabled themselves for a fault that was never theirs. - Manual sync on an established connector gave no feedback at all: the poll only ran while the predicate matched, which it never did. - Four over-broad invalidations that refetched every cached chunk page and chunk search in a base when one connector document was excluded. - The dead-process reporter re-sent a PATCH per stale document on every poll. * fix(kb): refuse to start a queued run on a paused connector The queue outlives the decision to sync. Pausing a connector after its run was queued cleared the queue entry's token but left the task itself alive, and the lock CAS accepted any row that was not already `syncing` — so the worker took the paused row and wrote its own terminal `active` over the pause. Moves the rule to the two points that can enforce it: an explicit `LOCKABLE_CONNECTOR_STATUSES` allowlist on the lock acquisition, and the same allowlist on `markSyncPending`, which closes the mirror race where a dispatch already in flight rewrites a just-paused row back to `pending`. Queueing and starting now agree on one rule, and a skipped hand-off is reported as its own outcome rather than a concurrency conflict. Also patches the connector detail cache alongside the list on an optimistic status write, so an already-expanded card starts its own sync poll instead of showing stale history behind the list's spinner. * fix(kb): make a queued sync prove it is the run that was queued `markSyncPending` minted an ownership token but only `releaseFailedDispatch` checked it, so the worker could consume a queue entry that was not its own. A task delayed past its lease is reclaimed and replaced; the status check alone let that stale task take the replacement's entry and run superseded options — a plain sync where the user had just asked for a full resync — while the replacement was turned away as `sync_in_progress`. Carries the token in the task payload and matches it at lock acquisition, the same discipline `holdsSyncLockToken` already applies to the `syncing` phase, extended to the phase before it. A superseded run is now reported as such rather than as a concurrency conflict. The payload field is optional for the rollout window only: tasks already in the queue carry no token, and stranding them would be worse than letting them fall back to the status check for one deploy. * fix(kb): report a paused connector as paused, not superseded Pausing a queued connector releases its token, so testing ownership before status reported every pause-while-queued — the common case — as a superseded dispatch. The mismatch is the symptom there; the status is the reason. * fix(kb): stop a status update landing on a run that already started The update's guards ran against a row read moments earlier and the write carried no compare-and-set, so a worker taking the lock in between meant the write landed on a `syncing` row — overwriting the run's status and, because leaving `pending` also clears the lock columns, wiping the token its heartbeat and terminal write match on. That stranded a sync that had already begun. The write is now conditional on the status the request was authorized against, and a lost race is reported as a conflict rather than "not found". Also restores the in-flight guard on the pause control. The optimistic status flip relabels it Pause -> Resume immediately, so a second click could send `active` before the first pause settled and resume a connector the user meant to pause. Read from the mutation's own pending state rather than the local id set this PR removed — React Query already knows which row is in flight. |
||
|
Waleed
|
8689237b4f |
improvement(files): match the preview toolbar's height to the tab strip above it (#6973)
The toolbar had no height of its own — `py-1` around a 30px chip came to 39px against the tab strip's 41px. Two stacked bars two pixels apart read as a mistake rather than as two different bars, and the toolbar's chips still stood taller than the 26px tabs. It now takes the same 40px content box over a 1px border, so the chips centre in the same band the tabs do. Covers all four previews that mount it — pdf, docx, pptx and the zoomable image/svg surface — none of which override its height. |
||
|
Justin Blumencranz
|
b02fee1e3a | fix(tables): omit ignored column diagnostics (#6970) | ||
|
Waleed
|
dce34932b4 |
improvement(menus): tighten dropdown spacing and cap the @ mention list (#6972)
Menu rows composed their own gap (8px) instead of the platform's chipContentGap (6px), so a menu row and the chip it opens over spaced their icon/label pairs differently. Surface padding was 6px against an 8px row radius and a 12px surface radius, so a first or last row's rounding cut across the corner it sat in; 4px makes them concentric. Separators carried a 13px gutter against a 0px gap between rows in the same group, which read as the groups floating apart. DropdownMenuLabel padded to an uncontrolled, font-dependent height and sat two type steps below its rows in a colour heavier than them; it now composes the shared row height at one step down in --text-muted. The @ mention list showed every integration. The per-family preview cap defaulted to uncapped, and integrations are 300+ near-identical rows sorted first, so the unfiltered list was the whole catalogue and no other family was reachable without scrolling past all of it. Capping is now the default, families are labelled, the menu is capped shorter than the action-menu default since it floats over the chat input, and the integrations shown are curated popular ones rather than whatever sorts first alphabetically. Both hand-rolled menus that render plain buttons for Radix focus reasons now compose the exported dropdownMenuRowClass instead of re-deriving row chrome. |
||
|
Waleed
|
4c8fec1a41 |
improvement(tabs): fade the tab strip with a mask, and shrink the tab chip (#6971)
The scroll-edge fade was a gradient div tinted with the surface colour, laid over the tabs. Tabs paint their own fills, so at an edge that washed a pill toward the surface colour instead of dissolving it, and it was only correct while whatever sat behind the strip was exactly that colour. A mask fades pill and label together to real transparency — the way the command palette fades its results, and the way every other horizontal fade in the app is drawn. The ramp goes to 24px, close to the palette's, since a short one reads as a cut. The tab chip drops to 26px, leaving 7px above and below instead of 5px. The collapse toggle and the action buttons stay 30px: a tab paints a fill so its box is visible and wants air, where those are bare glyphs whose box only shows on hover. The close button now centres itself rather than encoding (band - 24) / 2, which would have put it off-centre the moment the band changed. |
||
|
Bill LeoutsakosandBill Leoutsakos
|
8251e82900 |
fix(knowledge): prevent processing status overlap (#6958)
Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> |
||
|
Theodore Li
|
a781a325c5 | fix(invitations): break outbox import cycle (#6969) | ||
|
Waleed
|
8177f9abeb |
fix(tabs): restore the resource header's spacing and clear the scroll fades (#6967)
Porting the tabs onto the shared strip had taken the header from 43px to 34px, which moved the overlaid collapse toggle from 6.5px below the panel's top edge to 2px while its right inset stayed at 16px — the corner read lopsided. The header goes to 40px: still shorter than it was, with the toggle back to 5px. The toggle also gets its 8px radius back, dropped in that port for no reason anyone asked for. Selecting a partly-hidden tab scrolled it flush against the container edge, which is exactly where the fade gradient sits, so it arrived half-faded and still looked cut off. Reveal now insets by the fade width and clamps at the scroll extremes, where no gradient is drawn. Floating tabs cap at 160px so no single tab dominates the row. |
||
|
Theodore Li
|
818f7141e0 | fix(tables): disable dispatcher task retries (#6963) | ||
|
Justin Blumencranz
|
b7073a4dd4 |
feat(tables): return only selected columns from the Table block query (#6954)
* feat(tables): return only selected columns from the Table block query * fix(tables): size row batches by stored bytes and surface stale picks on empty lists * fix(tables): bound projected batches by the widest stored row seen |
||
|
Justin Blumencranz
|
3d40003482 | improvement(tables): backfill default views for existing tables (#6899) | ||
|
|
afccfe98a2 |
fix(selectors): respect trigger credentials in trigger mode (#6952)
* fix(selectors): respect trigger credentials in trigger mode * fix(selectors): isolate trigger selector context * fix(selectors): isolate action credential fallback * fix(selectors): scope fork reconfigs to trigger mode --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai> |
||
|
Waleed
|
c16883e9dc |
improvement(tabs): give the resource tabs a quieter floating look (#6960)
* improvement(tabs): give the resource tabs a quieter floating look Adds a `floating` variant to the shared TabStrip and uses it for the mothership resource tabs. Only the active tab carries a shape; the rest are bare labels divided by a hairline, sized to their content up to a cap. The browser and terminal strips keep the attached look, which stays the default. * improvement(tabs): align the floating tab ramp and icon size with the platform Each surface token now does the job it is named for: a bare tab hovers to --surface-hover instead of the Button variant's --surface-4, and a selected tab takes the rung between hover and --surface-active. Tab icons match the action icons beside them at 16px. |
||
|
Vikhyath MondretiandClaude Opus 5
|
71129cd112 |
feat(custom-blocks): let a publisher decide whether their block's runs reach consumer traces (#6950)
* feat(custom-blocks): let a publisher decide whether their block's runs reach consumer traces Joining a custom block's child run into its caller's trace shipped on by default, gated at read time by whether the person reading could already open the source workspace. That gate is doing the wrong job: a custom block's whole point is that consumers need no access to the source, so the check refuses exactly the readers the feature exists for, and it makes the answer depend on who is looking rather than on what the block's owner agreed to publish. The decision moves to the party whose data it is. `custom_block.trace_child_runs` is set by the publisher in Settings, applies org-wide, and is the entire policy — nothing downstream re-checks a caller. `getCustomBlockAuthority` already resolves per invocation and is the one lookup both the canvas handler and the Agent-tool runner pass through, so one column covers both surfaces and no consumer input can assert it. It defaults to FALSE. With the viewer check gone, an opted-in block publishes the source workflow's block names, inputs, outputs, and prompts to anyone who can read a consuming workflow's log. That is the same boundary curated outputs and redacted errors hold, so it opens by an affirmative act of the publisher or not at all — never as the residue of a column default on rows nobody revisited. Closed means the handle is withheld outright rather than persisted behind a flag: with no `childExecutionId` there is nothing for a reader, a migration, or a later refactor to join. What replaces it is a `_childTraceDisabled` marker, because a boundary span with no children renders exactly like a leaf block and an untraced run would otherwise read as one that did nothing. The consumer-facing failure `ref` is untouched either way — it is the only thing that makes an untraced failure reportable. Custom blocks invoked as Agent tools now join too. The child's handle already reached the agent's persisted `toolCalls[].result` (`postProcessToolOutput` strips only `__`-prefixed keys); nothing lifted it onto the tool span. Both span builders lift and strip it, and `hydrateChildTraces` needs no change — its boundary walk already recurses. The same handle is stripped from the model-facing copy of the tool result in `executeProviderTool`, the single point where the raw and model copies diverge: an opaque execution id in a tool result reads to a model like data the tool returned. The live SSE stream keeps one condition beyond the policy: an identified consumer. Not an authorization check — no workspace query — but chat deployments and the public API leave `liveTraceViewerUserId` unset because their consumer may be anonymous, and opting into org-wide tracing is not consent to stream a publisher's raw agent tokens to the internet. Copilot deliberately cannot set the field; exposing a team's internals org-wide is a human decision, not one an agent makes while publishing on their behalf. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(custom-blocks): read the publisher's trace policy at read time, not from the handle's presence Treating a persisted `childExecutionId` as proof of publisher consent is only true for handles this PR's writer produced. Every handle written before it meant something else — "a child ran; authorize the reader" — and the rows carrying them outlive the migration, so removing the reader check turned them into an open door: a consumer could open an old parent log and receive the source workflow's block names, inputs, outputs, and prompts from a block whose publisher never opted in. `hydrateChildTraces` now resolves the policy live, per boundary, from `custom_block.trace_child_runs`. The child log row's `workflowId` is the key — publish enforces one block per workflow — which also covers an Agent-tool boundary, whose span carries no block type to look up. A workflow with no block row (never published, or since deleted) has no publisher left to consent and stays shut, as does a failed policy read. This is not redundant with the write-time withholding. The handler still emits no handle for a block that was closed when the run executed, so such a run stays closed forever even if the block is opened later; this check decides whether the runs that DO carry a handle may still be shown. Turning the policy off therefore also closes what is already recorded, which is what a governance switch has to do to mean anything. Reported by Greptile on #6950. Also drops `any` from the trace-policy tests: outputs read through `Record<string, unknown>` (the handler's declared return does not name these internal keys) and failures narrow through `ChildWorkflowError.isChildWorkflowError`, which pins the failure type as well as its fields. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(logs): sum the child-trace drop counters from the struct, not a hand-listed set `totalDropped` re-listed four of the five counters, so a read whose only drops were policy refusals computed zero and skipped the log entirely. That is the commonest drop there is now — every handle written before the publisher policy existed refuses at that gate — so the one signal telling an operator the live check is closing joins went silent exactly when it started mattering. Summed from the struct instead. A hand-maintained list beside a struct is stale the moment a field is added, which is precisely how `policyClosed` was left out. Reported by Cursor Bugbot on #6950. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(db): renumber the custom-block trace migration around a 0299 collision Staging landed its own 0299 (`table_run_dispatches.heartbeat_at`) while this branch was open. The two migrations are independent — different tables, no shared statement — so only the number and drizzle's snapshot chain collided. Regenerated rather than hand-merged: a drizzle snapshot is a full-schema dump whose `prevId` links it to its parent, so editing one by hand to sit after a migration it was not generated against is how the chain silently stops matching the database. Staging's 0299 and its snapshot are taken verbatim; this is 0300, generated against them, and its SQL is byte-identical to what it replaced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Vikhyath MondretiandClaude Opus 5
|
fe0b92bb6d |
feat(secrets): show where a secret is referenced, beside its usage log (#6947)
* feat(secrets): show where a secret is referenced, beside its usage log
"See usage" answered who has run something with a key. It could not answer the
question a rotation actually starts from — where is this wired in — because a
secret four blocks depend on but nothing has executed yet has no usage rows at
all, so the panel read "This secret has not been used yet" for a live key.
The usage view now carries two tabs. Logs is the existing trail, unchanged and
still the default, since that is what the header action has always opened.
References is new: the blocks that name the secret as {{KEY}}, grouped under
their workflow, then the custom tools and MCP servers whose own bodies carry it.
Detection is the workspace-fork remapper's. remapSubBlocks already walks nested
tool-input params, resolves canonical basic/advanced pairs, and skips dormant
and condition-hidden members, so calling it per block inherits every rule a
fork already obeys. Only the aggregation is new: scanWorkflowReferences
collapses its output to unique (kind, sourceId) pairs and discards the workflow
— right for building a mapping table, wrong for locating a key. Nothing under
ee/workspace-forking changed.
- Candidates come from strpos(sub_blocks::text, name) > 0, deliberately not
LIKE: `_` is a LIKE single-character wildcard and nearly every env key
contains one, so SB_ACTION_ROUTER_SECRET would match text it does not occur
in. The prefilter can over-match but never under-match; the scanner decides.
The plan is an index scan on workflow by workspace, nested-looped into
workflow_blocks, so cost tracks the workspace rather than the table.
- Scope gates the read but does not narrow it. A {{KEY}} names a key, not a
scope, so the same sites answer for a workspace secret and the personal one it
shadows; narrowing here would report a personal secret as unreferenced the
moment a workspace variable of the same name existed.
- References reports one field per block, not a list. The remapper dedupes a
block's references by (kind, sourceId), so a block naming the secret twice
yields one entry — the type says so and a test pins it, because the row
renders that field as its whole description.
- Reads live state, not deployed: a draft workflow referencing the key must
show. Blocks are capped and the cap is reported as `truncated` rather than
silently trimming the list.
- Authorization is the existing usage gate, renamed requireSecretTrailReadAccess
and shared verbatim, so the two tabs can never disagree about who may look.
UI is existing primitives only — ChipModalTabs for the strip, DetailSection per
workflow over RESOURCE_LIST_STACK rows, IntegrationTile for the block glyph so a
block reads here as it does on an integrations row, SettingsEmptyState for the
gates. No new component, no new class.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): close the reference-scan scope bypass and bound its output
Review round 1.
- use-cases.ts: `scope` was a caller-controlled assertion the reference scan never
narrowed by, so `scope=personal` returned from the shared gate before any check
and handed any workspace member the admin-gated reference map for any workspace
secret. The usage trail can trust that scope because it filters the read by
`secretOwnerUserId`; a name-based workspace-wide scan cannot. References now
authorize on what the NAME resolves to — a workspace secret under that name is
admin-gated outright, and absent one the caller must actually hold a personal
secret of that name, which also stops a member enumerating arbitrary names.
`scope` is dropped from the input, the contract, the hook and the query key
rather than merely ignored: a parameter that does not exist cannot be asserted.
The trail gate keeps its old name and a note saying why only a scope-narrowed
read may reuse it.
- scan.ts: the prefilter matched the bare name, so `API_KEY` also read every block
holding `{{API_KEY_TEST}}` or the words "the API_KEY value" — and those false
positives counted against the row cap, so on a workspace with enough of them
genuine references sorted later were never read at all. It now matches the
reference syntax (`{{name}}`, with the whitespace ENV_REF_PATTERN allows), so a
candidate is a real occurrence and the cap means what it says. A name outside the
env-key charset short-circuits, which is also what makes it safe to inline into
the regex unescaped. Verified against a real workspace: the exact key still
returns its 16 blocks, its prefix now returns 0 where it previously matched all
16, and a metacharacter name touches no query.
- scan.ts: capping tool and server ROWS did not bound the output — one MCP server
emits an entry per matching header plus one for its url, so 200 rows could expand
past the contract's 400-entry bound and make the route reject its own response,
turning a successful scan into a 500 and the tab into "Could not load
references." Emission now stops at the bound and reports `truncated`.
- secret-references-panel.tsx: the empty-state early return preceded the truncation
banner, so a capped scan that filtered everything out claimed the secret was
unreferenced. Both paths now share one note, and silence from a capped scan reads
as absence of evidence rather than evidence of absence.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): cover unicode whitespace, legacy keys, and the shadowed-personal tab
Review round 2 — all three follow from round 1's own fixes.
- scan.ts: the syntax prefilter anchored on `[[:space:]]`, but the two engines
disagree about what whitespace is. `ENV_REF_PATTERN`'s `\s` accepts U+00A0,
U+202F and U+3000; Postgres `[[:space:]]` matches only the ASCII set. So a value
pasted with a non-breaking space inside the braces is a reference the executor
resolves and the prefilter silently dropped — the one failure direction this
feature must never take, since the answer it gives is "unused, safe to delete".
Anchoring on `[^[:alnum:]_]` instead accepts every whitespace encoding while
still rejecting a longer key on either side, and needs no code-point list that
could drift. It can admit a non-reference like `{{-NAME-}}`; that costs one
candidate row, and the scanner re-checks every candidate regardless. Erring loose
here is deliberate. (Greptile's `{{\tAPI_KEY\t}}` example was already handled —
tab is ASCII — but the unicode half of the finding was real.)
- use-cases.ts: the gate read `keyAccess.knownKeys` as "a workspace secret exists
under this name", but that set only covers names with an `env_workspace`
credential row. A legacy value written before the ACL existed has no row and
still wins at run time, so it fell through to the personal branch and handed a
non-admin the reference map for exactly the oldest keys. It now reads the
authoritative `workspace_environment.variables` map through a new
`hasWorkspaceEnvValue`, which is documented against `knownKeys` so the two are
not confused again. `getWorkspaceEnvKeyAdminAccess` keeps its existing contract —
its `knownKeys` still answers the ACL question its other callers ask.
- secret-references-panel.tsx: a personal secret shadowed by a same-named workspace
variable could open the view (its owner may read their own Logs) but References
always hit the workspace refusal and rendered a generic load error — a tab
offered in a state where it cannot succeed. The refusal is correct; the tab now
states the shadowing instead of asking for a map it will be denied, reusing the
wording the detail page already shows. No request is made in that state.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): re-check the reference gate's volatile input after the scan
Review round 3.
The name-resolution gate reads whether a workspace value exists, then scans. A
workspace secret created between the two makes the map now in hand admin-gated,
so a personal owner could receive it without workspace-secret administration.
The window is small and the data is derivable — a workspace member can already
open every workflow and read its `{{KEY}}` references — but the gate's stated
contract is that references follow the same predicate as revealing the value, and
a point-in-time check that can be overtaken does not honour that. An advisory lock
or a snapshot transaction would serialize a read-only view against secret writes
for it, which is the wrong trade.
Instead the one volatile input is re-read after the scan and the request fails
closed if it flipped. `requireSecretReferencesReadAccess` now reports which branch
authorized: an `admin` grant holds however the name resolves and pays nothing,
while a `personal` grant — the only one resting on absence — is re-checked. A
non-admin loses nothing they were entitled to keep; the request is refused the way
it would have been a moment later.
Adds a `listSecretReferencesUseCase` suite covering both denial paths, the legacy
value, the personal owner, the admin short-circuit, and the race itself.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): accept JSON-escaped whitespace in the reference prefilter
Review round 4.
The prefilter reads a `::text` rendering of a JSON column, and `jsonb::text`
renders a real tab inside a string value as the literal pair `\` `t`. `t` is
alphanumeric, so `[^[:alnum:]_]` could not consume it and the row was discarded
before `ENV_REF_PATTERN` ever ran — the References tab omitting a live reference
and reporting `truncated: false` while doing it.
Round 3's fix was verified against a raw text value rather than the JSON
rendering, which is exactly why it looked correct: `E'{{\tAPI_KEY\t}}'` matches,
`jsonb_build_object('v', E'{{\tAPI_KEY\t}}')::text` does not.
The gap between `{{` and the name now accepts three encodings at once — raw
characters (covering every Unicode space, which Postgres `[[:space:]]` misses),
JSON two-character escapes, and `\uXXXX` (how a vertical tab survives the same
rendering). Verified against the real jsonb rendering: tab, newline, carriage
return, vertical tab and form feed all recover, U+00A0 / U+3000 / space / plain
keep matching, and `{{API_KEY_TEST}}`, `{{MY_API_KEY}}` and prose are still
rejected — so the row cap keeps meaning what it says.
Plain-text columns (`custom_tools.code`, `mcp_servers.url`) carry no JSON
escaping, but tool code is JavaScript source and can contain the same escape
sequences literally, so the one predicate is right for every column.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(secrets): land the References link on the block, and name its field
Feedback round.
- Logs leads the tab strip. It was already the default tab; the order now says so.
- The usage view drops its resource heading for a plain "Usage" title. The back
chip already names the secret, so the tile and the subtitle underneath were
saying it a second time. `CredentialDetailLayout` gains an optional `title`
that renders the same element, class and column position the settings shell
gives `SettingsPanel` — which is how the sibling Forks "Activity" view titles
itself. Existing callers pass nothing and are unchanged.
- A block row now lands on the block instead of the workflow's default framing.
The editor had no URL params at all, so `?block=` is its first: read once on
arrival, acted on, and stripped. It is a navigation signal rather than canvas
state — the carve-out in sim-url-state.md is about pan, zoom, selection and
drag, which are socket-synced or high-frequency; this is neither, and it rides
in the link so a middle-click or reload keeps it where an in-memory handoff
could not.
The consuming effect mirrors the note-search reveal in the same file, including
the three details that make that one work: read from `displayNodes` so a target
arriving before its node mounts is retried on the mounting commit, route
selection through `resolveSelectionConflicts`, and latch in a ref. It also
claims `userFocusedWorkflowIdRef` the way a node click does, because `onInit`
re-reads that inside its own rAF and would otherwise `fitView` over the camera —
and that ref is reset by exactly the `workflowIdParam` change a deep link causes.
The panel opens for free: `syncPanelWithSelection` already follows selection.
`useSearchParams` needs a Suspense boundary and the editor's ancestry has none,
so the read lives in a leaf under its own `fallback={null}` rather than wrapping
the editor and adding a `loading.tsx` to its mount path. `next build` passes.
- A tool-input reference showed `tools-tool-0-code`. Those
`{subBlockId}-tool-{index}-{paramId}` keys are documented as an ephemeral,
client-only projection of the canonical `tool-input` value and are not meant to
be persisted, but older rows carry them — so the scanner reported whichever the
record yielded last. They are dropped before scanning, which is right even where
the two disagree: `tool.params` is what executes, so a mirror the canonical no
longer matches describes a reference that no longer runs.
- The row now shows the field's label from the block config rather than its
storage id — "API Key", "Tools", "Code", "Bot Token" — falling back to the id
when the block or field is unregistered.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): make the reference prefilter exactly as tight as the authority
Review round 6.
The gap between `{{` and the name accepted any non-word character, so
`{{-API_KEY-}}` and `{{"API_KEY"}}` matched in SQL while `ENV_REF_PATTERN`
rejects them. The previous commit called that free — "costs a candidate row and
nothing else" — which was wrong: a candidate row is a slot under
BLOCK_SCAN_LIMIT, so enough near-misses sorted earlier exhaust the cap before a
genuine reference is read, and the tab reports a live key as unused. That is the
same failure the tightening in round 1 was meant to remove, reintroduced by the
round 4 loosening that fixed JSON-escaped whitespace.
The gap now enumerates exactly the whitespace `\s` accepts, in each encoding it
can arrive in: `[[:space:]]` for raw ASCII, `\\[tnrf]` and `\\u000[bB]` for the
JSON escapes, and an explicit class for the Unicode spaces Postgres emits
verbatim but `[[:space:]]` does not match.
That class is generated from a code-point table rather than written literally.
Writing it by hand put a run of invisible characters in the source — a reviewer
cannot check them, and a formatter or editor can silently mangle them. The table
is the readable form and `toPgEscape` renders it.
Verified against the real jsonb rendering, 17 cases: raw space, tab, newline,
carriage return, vertical tab, form feed, U+00A0, U+202F, U+3000 and an embedded
reference all match; `{{-API_KEY-}}`, `{{"API_KEY"}}`, `{{API_KEY_TEST}}`,
`{{MY_API_KEY}}`, prose and an across-braces span all do not. Every candidate the
SQL admits is now a real occurrence, so the cap counts references and nothing
else.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): cap the reference scan on results, not candidates
Review round 7.
The prefilter now matches reference syntax exactly, but `remapSubBlocks` filters
further on semantics SQL cannot see: it drops dormant canonical members and
condition-hidden fields. So a block whose only `{{KEY}}` sits in a hidden field is
a genuine candidate that yields nothing, and with the cap counting candidates,
enough of those sorted earlier displaced active references out of the answer.
Unlike the previous two rounds this is not fixable by tightening the prefilter —
no SQL predicate can evaluate canonical modes or field conditions. So the cap
moves to what it should have counted all along: blocks REPORTED. Candidates are
read a page at a time up to a ceiling far above the result limit, so filtered rows
are absorbed as extra reads instead of taking result slots.
Paging rather than one large read because the alternative is holding every
candidate block's `sub_blocks` in memory at once; peak memory is now one page.
`blockId` joins the ordering as a final tiebreak, since OFFSET paging over a
non-unique sort can repeat or skip rows across pages — which here would
double-report a block or silently lose one.
This does not make the scan unconditionally complete, and the ceiling says so:
bounded work and guaranteed completeness cannot both hold, so the only real
choice is where the bound sits and whether it counts something the reader can
see. It now counts results.
Query plan re-checked with the OFFSET in place: still an index scan on workflow by
workspace, nested-looped into workflow_blocks. Test added that pins the fix — 2,500
prose candidates sorted ahead of one real reference, which the previous cap
dropped entirely.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): drop the paging that caused drift, and stop false truncation
Review round 8.
- Paging removed. It bought headroom and paid with drift: `OFFSET` is positional,
so a block renamed, inserted or deleted between page queries shifts the result
set, and the scan skips a live reference or reports one twice. That is a worse
failure than the one paging was added to fix, and it was self-inflicted last
round. Candidates are read in one statement again — one statement is one
snapshot, so neither skew nor duplication is possible — with the ceiling
lowered to 4,000 so a single read stays a sane amount of memory. Result-capping
survives, which was the actual point: filtered rows are still absorbed as extra
reads rather than taking result slots.
- `truncated` no longer fires on an exact landing. The block path now uses the
limit-plus-one read and strict `>` the resource paths already used, so a scan
that ends precisely on a bound reports complete instead of warning about
references that were all returned.
- The deep-link target is released when its block does not exist. It was cleared
only on a match, so a link to a since-deleted block left the id set with the
param already stripped: the effect re-checked on every canvas update forever and
shadowed a later link to the same block. Once any node has mounted the canvas is
populated, so an id still absent is gone and the target is dropped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): gate the deep-link release on the workflow being ready
Review round 9.
Round 8 released a deep-link target once `displayNodes` was non-empty, reading
that as "the canvas is populated, so a missing id is deleted". It is not: arriving
from another workflow the store still holds that graph, so nodes are present while
the linked workflow is still hydrating — and a valid `?block=` target was dropped
before its own blocks ever mounted.
The file already had the right predicate. `isWorkflowReady` pins
`hydration.phase === 'ready'`, `hydration.workflowId === workflowIdParam` and
`activeWorkflowId === workflowIdParam`, which is exactly "the graph now loaded is
this workflow's". Absence is only conclusive under that, and a node count never
was — it says something mounted, not whose.
This is the second fix to this release condition in two rounds, both from guessing
at a readiness signal instead of using the one the component already computes for
the same question.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Waleed
|
b440f4ec11 |
improvement(mship): preview recent runs in @ mention, with the logs icon (#6951)
The Logs family flooded the `@` picker with up to 50 near-identical rows, named after their workflow and drawn with the workflow icon, so they read as workflow snapshots and buried every other family. - Preview the 5 most recent runs while the query is empty; typing still searches the full fetched set. The cap spans the workspace rather than one workflow, so a few background runs cannot evict a run just started - Draw the row with the Logs icon, matching the sidebar, the search palette, and the chip the selection turns into - Trail the row with relative time, and with the dot `Badge` draws at `sm` for a run that did not simply succeed, so runs of one workflow are told apart at a glance - Make `@logs` reach the family, which nothing in a row's text names Mentioning a log also resolved to nothing: a log row is keyed by `id` but its run is addressed by `execution_id`, and the picker sent the former where the server resolves the latter. The run id now rides on the resource and every menu builds that resource through one helper, rather than eight inline literals that each silently dropped it. |
||
|
Waleed
|
f0a0970d2f |
improvement(tabs): port resource tabs onto the shared TabStrip (#6953)
Replace the hand-rolled resource tab bar with the shared TabStrip primitive, and generalize the strip where this caller needed more than it offered. |
||
|
Justin Blumencranz
|
cd935e8ea4 |
feat(workflows): generate short machine-nature names (#6906)
* feat(workflows): generate short machine-nature names * fix(workflows): remove vehicle name terms * feat(workflows): expand generated name vocabulary |
||
|
Justin Blumencranz
|
2252ac0ee6 |
fix(workflows): deduplicate generated workflow names (#6935)
* fix(workflows): deduplicate generated workflow names * fix(workflows): retry deduplicated name races (#6935) - recompute generated names after workflow-name conflicts - preserve exact-name and unrelated constraint behavior - cover the concurrent-create retry path |