6128 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
Vikhyath Mondreti	f4ee41bf24	improvement(blog): simplify provenance post structure (#6631)	2026-08-12 11:27:53 -07:00
Siddharth Ganesan	3ae83b17f7	fix(desktop): fix desktop prod ci #6628	2026-08-12 11:12:30 -07:00
Waleed	81108d5a4e	fix(v2): serve HEAD, advertise PATCH, and document the reachable 403 (#6623) ... * fix(v2): serve HEAD, advertise PATCH, and document the reachable 403 Three HTTP-semantics defects on the v2 surface, all found by probing the published contract rather than the happy path. **HEAD answered 500 on every v2 endpoint.** Next implements a missing `HEAD` export by aliasing it onto `GET` and dropping the body when it sends, so a route's `GET` legitimately runs with `request.method === 'HEAD'`. The builders' method guard compared that against the contract's declared method and threw, so `HEAD /api/v2/workflows` and every sibling replied 500 — which is what health checkers, uptime monitors, link checkers, and some CDNs send, all of them reading the API as hard-down. RFC 9110 §9.3.2 makes HEAD identical to GET but for the body, which is exactly what running the GET path produces. Fixed once in `methodMatchesContract`, shared by all five route builders; every other mismatch stays a hard error so a handler exported under the wrong verb still fails loudly. **CORS advertised `GET,POST,OPTIONS,PUT,DELETE`** while the v2 spec has 17 `PATCH` operations, so a browser preflight for any of them was rejected. It also advertised `PUT`, which two operations use — the shape of a hand-maintained list outgrown by its surface. The list stays hand-written because middleware cannot import the contract tree without pulling Zod into the edge bundle, but it is now pinned by a test that sweeps the real contracts and fails on any method it omits. **Six operations omitted a 403 their siblings documented** — three knowledge reads and three file-upload operations. Traced from the code rather than the spec: `requirePermission` throws `NoWorkspaceAccessError` for no access at all (concealed as 404) but `InsufficientWorkspacePermissionsError` for access below `minimumRole` (a real 403), and `PersonalApiKeysDisabledError` reaches every operation a personal API key can call. So 403 was reachable on all six and the omission was an accident of hand-assembled error lists, not a policy. They now use the shared `RESOURCE_ERRORS` / `RESOURCE_CONFLICT_ERRORS` sets, and two operations spelling those same sets by hand were normalized onto them. All 128 documented operations now declare 403. The rules for HEAD, for the 403/404 split, and for using the shared error sets are recorded in `.agents/skills/v2-api-conventions/SKILL.md`. * test(proxy): update the CORS policy assertion to the served method list `proxy.test.ts` pinned the previous hand-written method string, so widening `resolveApiCorsPolicy` to advertise PATCH and HEAD left it asserting a list the middleware no longer returns. The literal is kept rather than imported from `proxy.ts` so the test still pins the exact wire value independently of the implementation. * refactor(v2): retire the error sets that could omit Forbidden The three knowledge reads and three upload operations lost their `403` by assembling `[...VALIDATED_ERRORS, ...]` by hand, and `VALIDATED_ERRORS` / `STANDARD_ERRORS` were the only exported sets that omit `Forbidden`. Migrating the last consumers to the shared `RESOURCE_*` sets left both unreferenced, so deleting them turns the fix from a one-time cleanup into an invariant: there is no longer a building block from which a workspace-scoped operation can assemble an error list without `Forbidden`. Regenerating the specs produces no diff, so the migration is output-neutral. Also folds `method-match.test.ts` into `definition.test.ts` to match the repo's `feature.ts` -> `feature.test.ts` convention, types `contractMethod` as `HttpMethod` so a contract declaring `HEAD` is unrepresentable, and drops the duplicated Next-aliasing rationale so `methodMatchesContract`'s TSDoc is its single home. * fix(cors): expose the API response headers a browser client needs Without `Access-Control-Expose-Headers` a browser can read only the six CORS-safelisted response headers, so the rate-limit budget, the `Retry-After` a 429 or 503 asks the caller to observe, and the request/run correlation ids were all on the wire but invisible to `fetch()`. Server-to-server callers were unaffected, which is why it went unnoticed. Exposed on the default `/api` policy only. The per-route `CORS_RULES` entries are wildcard-origin public endpoints and opt in individually if they ever need it, so this does not widen what an anonymous cross-origin caller can read from them.	2026-08-12 11:09:33 -07:00
Waleed	6541a22aa6	fix(v2): tell a caller when to come back on every failure meant to be retried (#6625) ... * fix(v2): tell a caller when to come back on every failure meant to be retried Three related gaps in retry signalling, found auditing the v2 surface against RFC 9110/6585 and against how Stripe, GitHub and Google's AIPs handle the same problems. **No 503 carried `Retry-After`.** Every one of them — the three route builders' `unhandledErrorResponse`, the execute and resume routes, and `serviceFailureResponse` — funnels through `v2Error`, so the default lands there, keyed on the response *status*: `Retry-After` is defined against the status, and the status is the only half of the code/status pair a client sees. A caller that supplies its own value still wins. RFC 9110 §15.6.4 makes this a `MAY` rather than a `SHOULD`, so it is a deliberate improvement, not a conformance fix: without it a client's only defensible policy on a 503 is an immediate retry, and Sim raises 503 exactly when a dependency is too degraded to absorb one. **A 429 that already knew its wait threw it away.** The admission descriptors declare `retryAfterSeconds` per denial, but mapping a descriptor onto a preprocess error copied only `statusCode`, `code` and `retryable`. A concurrency denial therefore reached the client as a bare 429 with no `Retry-After` despite the policy layer having named the wait five seconds earlier. The value now travels `descriptor.retryAfterSeconds` → `PreprocessExecutionError.retryAfterMs` → `ExecuteWorkflowServiceFailure.retryAfterMs` → `serviceFailureResponse`, so the transport reads a number the policy owns instead of re-guessing one. The 503 default is now only the floor for paths with no policy signal. **One failure must not advise a retry at all.** `ASYNC_ENQUEUE_AMBIGUOUS` is a 503 whose enqueue may have succeeded — it deliberately retains its execution-ID claim because a job may already exist. Telling that caller to come back in five seconds invites a client with no `X-Run-Id` to start, and bill, a second run of the same workflow. It opts out via `omitRetryAfter` and returns the run id so the caller reconciles instead. `ADMISSION_RETRY_AFTER_SECONDS` is reused rather than restated, so the execute route's capacity 429 and every other surface's 503 cannot drift apart. Also records the audit in `.agents/skills/v2-api-conventions/SKILL.md`: the retry rule, the cursor-tampering invariants, and reasoned rejections of RFC 9457 problem+json, the `RateLimit-*` draft fields, renaming `X-RateLimit-*` under RFC 6648, 422-for-semantic-validation, `Location` on 201, ETag/`If-Match`, and `merge-patch+json` — each with the spec text and the industry evidence, so they are not re-litigated. `Deprecation`/`Sunset` on v1 is left open pending a retirement date, which is a product decision. * docs(v2): name the one 503 that omits Retry-After in the shared contract The shared ServiceUnavailable description claimed every 503 carries the header, which the ASYNC_ENQUEUE_AMBIGUOUS response deliberately does not. It now says the header is normally present and names that exception, so the published contract matches the runtime behaviour for all 128 operations.	2026-08-12 11:09:23 -07:00
Waleed	9dfd9db3b0	feat(xai): wire reasoning effort through the Grok adapter (#6627) ... * feat(xai): wire reasoning effort through the Grok adapter The catalog never declared reasoningEffort for xAI and the adapter never sent reasoning_effort, so the flag was dead for every Grok model. Values are per-model and verified against the live API rather than the docs, which are wrong in three places: grok-4.5 does accept xhigh, grok-4.3 supports the parameter at all (undocumented) including none, and grok-4.20-0309-reasoning rejects it outright despite being a reasoning model. Also corrects grok-4.5's missing cachedInput and drops an inline comment the new provider TSDoc now covers. * test(xai): type the provider test helper instead of casting to any * fix(agent): correct reasoning-effort copy that still claimed GPT-5 only	2026-08-12 11:08:27 -07:00
Theodore Li	afa02939bb	fix(docs): include API key header in generated code samples (#6630)	2026-08-12 13:58:39 -04:00
Waleed	74212ef333	feat(models): add grok-4.6 and make it the xAI flagship (#6624) ... Verified every field against xAI's live API with the staging hosted key: context_length 500000, $2.00/$0.50/$6.00 per 1M, temperature capped at 2, tool calling and max_completion_tokens both accepted. Also feature the latest blog post.	2026-08-12 09:34:51 -07:00
Waleed	878856b6a9	chore(ci): declare least-privilege permissions on the desktop e2e workflow (#6622)	2026-08-12 09:33:00 -07:00
Waleed	5cf60f6a41	fix(v2): give every collection one pagination contract and close four envelope holes (#6620) ... * fix(v2): give every collection one pagination contract and close four envelope holes A fractional `limit` reached Postgres as `LIMIT 2.5` and answered 500 on both `GET /workflows` and `GET /audit-logs`: each list re-declared the param inline, and these two copies lost their `.int()`. The same divergence left `limit` validated five different ways and five collections emitting `nextCursor` while accepting no `limit` at all, or accepting one and silently discarding it. Adds `v2PaginationFields()` in `contracts/v2/shared.ts` — a bounded integer `limit` and an opaque `cursor` — and adopts it across all 17 paged lists, so the family cannot drift again. `/files`, `/logs` and `/tables` keep the truncate-and- clamp leniency they published, now as an explicit named mode rather than three hand-rolled copies. Gives `/skills`, `/custom-tools`, `/secrets`, `/credentials` and `/knowledge` real pagination using the existing cursor codecs: a keyset for the four whose page comes from one ordered SQL read, and the offset cursor for `/skills`, whose merge of the static builtin registry into DB rows cannot be expressed as a SQL keyset. Each keyset sort now ends in a unique `id`; knowledge tie-broke on `createdAt`, which cannot separate rows sharing a millisecond. Two correctness fixes pagination forced: the secrets visibility filter moved from a post-query JS pass into SQL, because trimming rows after the page is cut returns fewer than `limit` while `nextCursor` claims more; and the skills list stopped selecting the 50k-char `content` column only to discard it. Also restores the canonical error envelope where it had holes: a malformed JSON body returned a bare `{"error":string}` because the envelope was a per-route opt-in only 8 of 77 routes remembered, and an unknown `/api/v2` path returned an HTML 404. Both are now defaults — `V2_PARSE_DEFAULTS` on the builders and the two raw routes, and a catch-all whose body is byte-identical to the rollout gate's so an unknown path stays indistinguishable from an ungated one. Consolidates the keyset paging block (`resumeKeyset`/`keysetPage` in `list-query.ts`) that had been open-coded in six modules, and folds the bespoke `InvalidWorkflowListCursorError` into the `OrchestrationError` every other list already used. Prevention: the contract sweep in `list-pagination.test.ts` now also asserts that every paged list rejects a fractional `limit`, that every list query is `.strict()`, and that the three clamping lists still truncate. The fractional- limit assertion is what caught `/audit-logs`. Documented in `.agents/skills/v2-api-conventions/SKILL.md`. 405 responses still carry no `Allow` header — Next.js generates those before any handler runs. Recorded as a known gap. * fix(v2): bind the offset cursor to the query state it counts positions in An offset names a position in one exact sequence. `GET /skills` accepted a bare `{offset}` cursor and applied it to whatever sequence the next request asked for, so following `nextCursor` with a different `search`, `sortBy` or `sortOrder` silently skipped rows, repeated them, or landed past the end and returned an empty page while the cursor implied more. Fixed in the codec rather than the route so the sibling could not keep the gap: `decodeOffsetCursor` now takes a scope stamp and rejects a cursor minted under a different one, which is what `decodeSortedCursor` has always done for keysets. `offsetCursorScope()` builds the stamp from every param that filters or orders the sequence; `limit` is excluded because it selects how much of the sequence to return, not what the sequence is, so paging with a different page size still works. `GET /knowledge/{id}/documents` had the identical latent gap and gets the same treatment — the compiler surfaced it as soon as the signature changed.	2026-08-12 09:08:00 -07:00
Waleed	0c4fb1374a	fix(copilot): keep a file-preview failure from stripping a tool call's arguments (#6621) ... The workspace file preview adapter runs while the tool-call frame is still on the wire, so the execution context it gets is turn-scoped and carries no toolCallId — the file delegation requires one, so resolving a path target threw on every call. The SSE handler swallows that throw and abandons the rest of the event, so the frame never registered its arguments and the call was later dispatched with an empty payload, failing schema validation. - bind the frame's own tool call id before entering the file use cases - resolve the preview target best effort, matching the preview base load - stop a preview failure from dropping the tool-call frame in the stream loop - drop the synthetic toolCallId the stream fixtures put on a turn context	2026-08-12 09:01:42 -07:00
Waleed	7b3e6a63b0	fix(executor): carry child provenance across the workflow agent tool result (#6619) ... A workflow invoked as an agent tool resolves `{{VAR}}` to the real decrypted value in the child run, but the tool result handed back to the model vendor was projected through a registry that had dropped those entries — so the plaintext crossed to the vendor verbatim. Mechanism. Model-facing projection runs `registry.forkForPropagatedEntries()`, which keeps only entries a result explicitly carried. `EnvResolver` records a resolution without `propagated`, unlike every other boundary that hands a value onward. The child shares the caller's registry object, so `blockLogs`, trace spans and error diagnostics still redact (they read the unforked registry) — only the model fork loses them. The exposure is wider than the child's final output: `mapChildOutputToParent` puts the full `childTraceSpans` — every child block's inputs and outputs — into the returned object, and `postProcessToolOutput` strips only `__`-prefixed keys. The previous implementation ran the child over HTTP: the execute route emitted `__resolvedSecretTraceProvenance`, the tool imported it as `propagated: true`, and `transformResponse` curated the body so `childTraceSpans` never crossed. The in-process branch returns before any of that. Fix. `runWorkflowTool` exports committed provenance for the value it returns and imports it back with `{ trusted: true }`, which marks those entries propagated — the same crossing the custom-block branch already performs in `workflow-handler`. Output-projection only: the returned result is unchanged, and the child executes exactly as before. Redacted values render as `{{NAME}}`, matching the literal the model saw before this regression. Values shorter than `MIN_SUBSTITUTABLE_LITERAL_LENGTH` are still not redacted anywhere — that floor governs detection as well as substitution, and is a documented accepted cost. A test pins the behavior rather than leaving it silent.	2026-08-12 02:03:16 -07:00
Waleed	082deb2349	fix(executor): copy the env map at the workflow-tool boundary (#6618) ... Follow-up hardening to #6611, which began forwarding the invoking run's environment variables into a workflow run as an agent tool. A runtime audit of that change confirmed nothing today writes through `ctx.environmentVariables`, so this is not a live defect. But `tools/index.ts` was the only consumer handing the map across an execution boundary by reference, and it hands it to the longest-lived consumer there is: the child holds it for its entire run. `agent-handler`, `function-handler`, `condition-handler` and `providers/utils` all copy via `normalizeStringRecord` before handing the map anywhere. A future write through the child's reference would corrupt the parent's env and every later sibling tool call in the same agent turn — a cross-run bug with no local symptom. A shallow spread is exact here: the value is typed `Record<string, string>`, and the sub-Executor already re-copies it through `normalizeStringRecord` (`executor.ts:73`), so the child receives a byte-identical map either way. The spread also subsumes the previous `?? {}`, since spreading `undefined` yields `{}`. The test mutates the forwarded map and asserts the parent context is unchanged; it fails without the spread.	2026-08-12 02:00:57 -07:00
Waleed	2054947503	test(export): pin the table and tool-param loss the sanitized export accepts (#6613) ... * test(export): pin the table and tool-param loss the sanitized export accepts #6591 enabled `redactOpaqueCredentialInputs` on the workflow export path, closing a real leak. It also made export lossy for tables and unauthoritative tool params, and nothing pinned that trade in either direction. Adds round-trip fixtures (an api block with two table sub-blocks, an agent block with a custom tool) plus assertions for the current loss, records the security/usability trade on the flag that governs it, and deletes a duplicate `sanitizeForExport` in credential-extractor that omitted the redaction flag and had zero production importers. No behavior change. * test(export): make the env-ref leak sweep load-bearing and drop test any The sweep asserted against a token the fixture no longer contained, so it passed vacuously. Both the fixture and the assertion now read one symbol, which is the only form that cannot drift. Also types the re-imported block lookup instead of casting through any.	2026-08-12 01:57:02 -07:00
Waleed	366829b6b0	fix(v2): derive the log and run status enums from the persisted status list (#6612) ... * fix(v2): derive the log and run status enums from the persisted status list `GET /api/v2/logs` and `GET /api/v2/logs/{runId}` parse the raw `workflow_execution_logs.status` column against a six-value enum that omits `paused`, so a run holding that value returns 500. The list response is validated whole-page, so one such row 500s every page it lands on, and the row is durable until the run is resumed, cancelled, or failed. `paused` is not written by an ordinary human-in-the-loop pause — that path persists `pending` (logging-session.ts:1180). It is written by `PauseResumeManager.markResumeAttemptFailed`, which fires on any `ResumeAdmissionError`: a workspace over its usage limit, an archived or undeployed workflow, or a concurrent resume losing the claim race. That is a routine business path. The enum was supposed to be protected by an `AssertNever` exhaustiveness gate, but the gate was vacuous: it compared against `PersistedWorkflowExecutionStatus`, a hand-written union that was itself missing `paused`, because the write goes through a raw `sql` CASE fragment Drizzle cannot type-check. Adding `paused` to both lists would leave the same vacuous gate in place for the next status. Instead, `PERSISTED_WORKFLOW_EXECUTION_STATUSES` becomes the single runtime source of truth, `PersistedWorkflowExecutionStatus` is derived from it, and both v2 contracts derive their enums from the const rather than re-declaring them. Both surfaces pass the column through verbatim, so their reported set is the persisted set by definition — there is no editorial choice for a gate to force, only the question of whether a newly persisted status should be public, which the option-list tests now pin. The `[...V2_PERSISTED_RUN_STATUSES, 'paused']` append on the runs contract is deleted rather than adjusted; it would otherwise be a duplicate. Alternatives rejected: - A `.catch()` or `safeParse` in the presenters is dead code: `v2-json-route.ts:271` re-parses the whole body with the same schema. - Normalizing `markResumeAttemptFailed` to write `pending` would remove the distinction the resume claim query at human-in-the-loop-manager.ts:973 relies on, and leaves the contract wrong for any other future status. - Typing the Drizzle column does not help: the offending write is a raw `sql` fragment, and `packages/db` cannot import the app's status list. The v2 workflows spec changes are reordering and description only — the value set there already contained `paused`. The v2 logs spec gains `paused`, which is additive and safe while the whole `/api/v2` surface is behind the off-by-default `v2-api` flag; it must land before v2 GA, after which it would be breaking. * fix(v2): document both provenances of a reported paused run status * fix(v2): stop promising a paused discriminator the response cannot always provide * fix(v2): describe the paused discriminator as the code actually records it	2026-08-12 01:51:55 -07:00
Waleed	47f143016e	fix(docs): restore api-reference URL continuity and fix translated SDK bodies (#6617) ... Two docs-only defects introduced by #5273 (`263e3ca67e`), which re-founded the public API reference on the v2 surface. 1. Ten translated SDK snippets produce a deterministic 400. The streaming example in the five translated `api-reference/typescript.mdx` and `python.mdx` pages was repointed from `/api/workflows/{id}/execute` to `/api/v2/workflows/{id}/execute` and nothing else was changed — fr/ja/zh typescript.mdx are literally one-line diffs. `message` stayed at the body root. That was correct against v1, whose route treats the whole non-control body as workflow input, but `v2ExecuteWorkflowBodySchema` ends in `.strict()` and the route parses before executing, so every copied snippet returns `400 Unrecognized key: "message"`. The same commit fixed the English bodies to `input: { … }`, so this is an oversight, not a decision. The ten fences now match `en/api-reference/typescript.mdx:959` and `python.mdx:681`. Not relaxing `.strict()`: it is deliberate house style across the v2 contract and is what makes a typo'd option fail loudly instead of silently. 2. Thirty-two published operation pages 404 with no redirect. Replacing the single v1 `openapi.json` with seven v2-only specs changes page identity, because fumadocs derives every generated page as `slugify(tag)/operationId` from the specs at build time. Re-deriving both sets gives 52 old slugs and 128 new ones: 32 disappear and 20 keep their URL while silently retargeting v1 -> v2 (`knowledge-bases/updateKnowledgeBase` also flips PUT -> PATCH). All 52 are in the live sitemap — parsing `<loc>` from docs.sim.ai/sitemap.xml gives 458 URLs of which 56 are `/api-reference/`: the four static pages plus all 52 generated ones by name, including every one of the 32 that die. They are 200 today under an allow-all robots.txt. The spec swap itself is deliberate and CI-enforced (`check-openapi-specs.ts` requires every published operation under `/api/v2/`), so restoring the v1 operations is not an option. The missing piece is the redirect map, in a file that already carried 56 such rules from earlier doc moves. `permanent: true` (308) is used only for a true 1:1 successor — same operation, renamed. A 308 is cached indefinitely and effectively unrecallable, so anything that collapses two pages onto one, changes the identifier model, or lands on a merely adjacent operation is `permanent: false` (307). That splits 21/11. Four destinations differ from the mapping proposed in review, each on evidence from the specs rather than from the operation names: - `workflows/getJobStatus` is not destination-less. The v2 queued-execution receipt (`QueuedWorkflowRun`) returns `statusUrl` `/api/v2/workflows/{id}/runs/{runId}`, so `workflow-runs/getWorkflowRunV2` is the successor poll target — far better than a generic landing page. - The three HITL read operations go to `getWorkflowRunV2`, not to the resume page: `WorkflowRunStatus` carries a `paused` object with `contextId`, `pausedAt`, and `pauseKind`. Pointing a GET doc at a POST doc would be wrong. - `human-in-the-loop/listPausedExecutions` goes to `listWorkflowRunsV2`, whose `status` filter includes `paused`. - `tables/batchUpdateRows` is 307, not 308. v2 `updateTableRows` is "Update Rows by Filter" — the successor of v1 `updateRows` (PUT, predicate-based), which keeps its 308. v2 has no by-id batch update at all, so batchUpdateRows lands on a genuinely different operation. 3. A guard, so the map cannot rot silently. `scripts/openapi/docs-redirects.test.ts` recomputes the generated slug set the way fumadocs does and asserts no `/api-reference/` source shadows a live page and every destination resolves. Nothing else in the repo reads docs URLs, so a future spec regeneration would otherwise break the map with no signal. It needs no wiring: `check-openapi.ts` already runs this vitest config. The redirect array moves to `apps/docs/lib/redirects.ts` because the guard cannot import `next.config.ts` — `createMDX()` runs the fumadocs-mdx generator at import time, which made vitest emit an unhandled build error and warn about false positives. The 56 pre-existing rules are byte-identical to before, verified programmatically; `next.config.ts` keeps the same public shape and Next's own `checkCustomRoutes` accepts all 88 rules. Open question for the owner, larger than the redirects: all 82 `/api/v1` route files survive on staging, so a live public API now ships with zero reference docs, while the documented `/api/v2` surface returns 404 for any caller outside the off-by-default `v2-api` flag cohort. Is that the intended end state or transitional?	2026-08-12 01:49:59 -07:00
Waleed	34d65df7d6	fix(sdk): make the 0.2.0 SDK release safe to publish (#6616) ... The v2 SDK migration (#5273, #6564) shipped five breaking changes in both SDKs but got the release mechanics wrong in three separate ways, and left one of the two rewrites unable to complete a single successful call. Versions. packages/ts-sdk/package.json read 0.1.3 -- a patch digit added inside an unrelated compatibility commit, never deliberated. npm expands ^0.1.2 to >=0.1.2 <0.2.0, so every existing consumer would have picked the break up on a lockfile refresh: AsyncExecutionResult.jobId renamed to runId, executionId dropped from that interface, a failed sync run now throwing instead of resolving {success:false}, the request body reshaped, and the endpoint moved to /api/v2 with no fallback. 0.2.0 excludes every existing range, so the upgrade becomes opt-in. packages/python-sdk carries the identical break and was never bumped at all, so its publish job would have skipped green at the "version already exists" gate and left the repo and PyPI silently divergent; it moves 0.1.2 -> 0.2.0 in lockstep, along with the __version__ string in simstudio/__init__.py, which tracks pyproject and would otherwise have started lying. setup.py is left at 0.1.1: it is unchanged from main and demonstrably unread (0.1.2 published from pyproject while setup.py already said 0.1.1). It wants deleting, in its own commit. A 404 fallback was considered and rejected. The legacy 202 body's statusUrl points at /api/jobs/{jobId}, so mapping jobId onto runId would hand the caller an id that getWorkflowRun cannot resolve against that same old server -- a successful execute followed by an inexplicable failure on the next call is a worse contract than a clean 404. Both READMEs instead state the minimum server version and name the endpoint to check for. Cancelled runs. packages/python-sdk computed success as status != 'failed', so a run cancelled out of band reported success=True. The TypeScript SDK uses a closed whitelist and reports False, and before the migration both SDKs read the server's own value, which was False -- so this was a Python regression, not merely an inconsistency. Fixed by mirroring the whitelist. The v2 contract enumerates exactly completed|failed|paused|cancelled, so narrowing the blacklist to a whitelist cannot drop a live value, and a status added later now defaults to "not successful" rather than silently reporting True. WorkflowExecutionResult gains a status field because Python, unlike TypeScript, does not throw on 'failed' -- so success=False alone is ambiguous there in a way it is not in the TypeScript SDK, which is why status is not added to both. Rate-limit header. Found while auditing the two SDKs for further divergence, and the reason the Python bump could not have shipped as it stood: every authenticated v2 response now carries X-RateLimit-Reset as an ISO 8601 timestamp (recorded by v2RateLimits.publicApi, stamped by withRouteHandler). The Python SDK parsed it with int(), raising a bare ValueError that no handler in execute_workflow catches -- so every successful v2 execution raised instead of returning. None of the legacy endpoints the SDK previously called record a rate-limit snapshot, which is why the latent int() survived until the v2 move. The TypeScript SDK already branches on the format; _parse_reset_header mirrors it, including degrading an unrecognised value to 0, because a quota hint must not take down the call it rode in on. Timing metadata. The v2 rewrite stopped forwarding startedAt/endedAt, which main passed through and the TypeScript SDK still reports; restored under the same startTime/endTime keys the TypeScript SDK uses. Tests: cancelled/failed/paused status coverage, the ISO reset header, and the restored metadata keys, each verified red against the unfixed line first. The TypeScript suite gains matching cancelled/paused and ISO-reset pins -- they pass against today's source by design, and were confirmed to fail against a deliberately degraded copy so they are not toothless. Deliberately not included: a CI guard failing a PR that changes SDK source without a version bump. It would have caught this twice over, but it is a new script and workflow rather than a fix to the defect at hand. Review revision. bun.lock recorded packages/ts-sdk at 0.1.3 and was left stale by the first pass, so the repo asserted two versions for the same workspace package -- in a change whose whole thesis is that the version strings had diverged. It does not break CI (bun 1.3.14 accepts the mismatch under --frozen-lockfile, confirmed here), but 092311ea68 bumped the lock in lockstep with package.json, and the next unfrozen install would otherwise drop the line into an unrelated PR. _parse_reset_header gated the numeric branch on str.isdigit(), which accepts characters int() rejects ('²'.isdigit() is True, int('²') raises) -- and that int() sits outside the try, so the one function added to stop a quota hint raising could still raise, contradicting its own docstring. str.isdecimal() is exactly the set int() accepts. The tolerates-unparseable test is parametrized over both forms and was confirmed red on '²' against isdigit. Docs and docstrings: apps/docs api-reference/python.mdx mirrors the README's dataclass block and was the only copy left without the new status field. RateLimitInfo now names its units, because reset is epoch seconds for the legacy integer and milliseconds for the ISO form that v2 sends. execute_workflow's Args entry still described the pre-v2 body shape ("spread at root level"); every input is nested under input now, and this is the commit that ships that help() text to PyPI. The "declared last so positional construction keeps working" sentence was a maintainer's note that belongs in this message, not in every user's help(WorkflowExecutionResult).	2026-08-12 01:48:37 -07:00
Waleed	5a63eb8567	fix(copilot): clamp the legacy int4 size when materializing a chat upload (#6615) ... `materialize_file(operation: 'save')` wrote the HEADed object size straight into `workspace_files.size`, which is still `integer NOT NULL`. Since the `size_bytes` widening (0289), a mothership chat attachment may be up to MAX_WORKSPACE_FILE_SIZE (5 GiB): `upload-session/service.ts` gives `mothership_attachment` that ceiling, and `finalizers.ts` already dual-writes the row as `size = 2147483647, size_bytes = <exact>`. Saving such an upload then re-read the true size from `headObject` and issued `SET size = 3221225472` against int4. Postgres raises 22003; the retry filter matches only 23505, so it rethrows, the transaction rolls back and the tool returns `success: false` with no way for the user to complete the save. No corruption — int4 overflow errors, it never truncates — but the file can never be saved. Every other `workspace_files` size writer already pairs `toLegacyWorkspaceFileSize(bytes)` with `sizeBytes: bytes` (metadata.ts x4, workspace-file-manager.ts:243/1706, finalizers.ts:367). This call site was simply missed when the widening landed; the fix converges it with the other six rather than inventing a third shape. Storage accounting keeps using the exact `verifiedSize`, so quota and usage are unaffected. The size source itself also had to widen. `head?.size ?? row.size` fell back to the clamped int4 column, and since this change now writes `sizeBytes` too, that fallback would overwrite an exact `size_bytes` with the clamp — the object is gone, so nothing could recover it, and the row would look internally consistent afterwards. The fallback is live whenever `hasCloudStorage()` is false, since the early return at the HEAD miss is cloud-only. Reading `row.sizeBytes ?? row.size` is the same coalescing shape the readers already use (workspace-file-manager.ts:227, finalizers.ts:399, metadata.ts:46), and the row comes from a full `select()` so the column is present. The clamp is derived once next to `verifiedSize` rather than inline in the update because the value is loop-invariant. Two sibling writers were examined and deliberately left alone. `copy-files.ts` reads `task.size` out of the int4 column itself, so it is arithmetically incapable of overflow, and its missing `sizeBytes` is unreachable behind the 100 MB fork download cap. `workspace-file-manager.ts:963` takes a caller-supplied size, but its insert branch needs an orphaned storage object with no `workspace_files` row, and converting loose external input from a DB error into a JS throw deserves its own review rather than a release patch; it is the next instance of this bug and should be filed as a follow-up. Both new tests were proven red against the unfixed code: the clamp test fails with "expected 3221225472 to be 2147483647", the fallback test with "expected undefined to be 3221225472".	2026-08-12 01:43:52 -07:00
Waleed	b33f8e82fe	fix(executor): give the workflow agent tool the caller's env and PII policy (#6611) ... A workflow attached as an Agent (or Pi) tool ran its entire child execution with an empty environment-variable map and no block-output redaction policy. Mechanism. `tools/index.ts` short-circuits `workflow_executor` into `runWorkflowTool`, which builds its synthetic parent `ExecutionContext` with `buildCustomBlockExecutionContext`. That builder was written for the custom-block (deploy-as-block) path and hardcoded `environmentVariables: {}` — safe there only because `WorkflowBlockHandler.executeCore` re-derives the publisher's env inside `if (isCustomBlock)`. The workflow-tool path's synthetic block carries `metadata.id: 'workflow_input'`, so `isCustomBlock` is false, the re-derivation is skipped, and `{}` flows through `childEnvVarValues` into the sub-Executor. `DAGExecutor` has no fallback and `EnvResolver` returns the raw reference on a miss, so a child block field of `Bearer {{MY_API_KEY}}` was transmitted to the third party verbatim and 401'd — silently, with the variable name disclosed. The same builder never set `piiBlockOutputRedaction`, so `block-executor`'s in-flight masking was disabled for every child block of orgs that had explicitly enabled that stage. Both landed as unnoticed side effects of #5273, whose stated goals were admission slots, log rows, cost roll-up and structured errors; #6539 later patched a third dropped field on the same context without noticing these two. Fix. Thread both values through the runner `options` bag — never `params._context`, which spreads model-reachable `contextParams._context` first and would let a model inject its own env map or disable redaction. `executeTool` reads them off the trusted `executionContext`, which also covers the Pi block, whose tool loop calls `executeTool` with `executionContext: ctx` on the identical path. `environmentVariables` is required rather than optional-with-a-default. Silent omission is precisely the failure mode here and in #6539; making it required turns the next caller's omission into a compile error. `runCustomBlockTool` now passes `{}` explicitly, so that path is unchanged at runtime. `piiBlockOutputRedaction` stays optional deliberately: `undefined` is its correct value for the many tenants with no policy, whereas `{}` for env is a wrong identity rather than a default. The builder's TSDoc states both halves of that asymmetry. Identity semantics — this restores function but does not restore main's identity. On main this tool was an HTTP hop into execution-core, which derived the env from the CHILD workflow's owner, so the child got the child owner's personal env plus the child workspace's env. Forwarding the caller's map gives the child the PARENT CALLER's personal env: a different identity, not a subset. That is the deliberate choice, because it is byte-identical to the long-standing canvas workflow block, it is bounded to one workspace by `assertChildWorkflowInWorkspace` on this branch, and it is the only variant consistent with the parent `resolvedSecretTraceRegistry` this path already forwards. The narrow case that worked on main and still will not: a same-workspace child owned by another member that relied on THAT member's personal environment variable. The `deployed_block_executor` call site deliberately gets neither value: custom blocks skip the same-workspace assert and run cross-workspace under the publisher's identity, so the consumer's env and redaction rules are the wrong tenant's. A test pins that so a later refactor cannot unify the branches silently. Tests. Three suites pin the fix itself (runner, builder, `executeTool` dispatch) and go red without it. A fourth case in `workflow-handler.test.ts` pins the last hop — `ctx.environmentVariables` -> `childEnvVarValues` -> the sub-Executor's `envVarValues`, plus `piiBlockOutputRedaction` — on the NON-custom branch. That hop is untouched staging code, so that case passes either way by construction; it exists so a future change to the branch that distinguishes the two paths cannot silently undo this fix downstream of the builder. Out of scope, deliberately: `enforceCredentialAccess` is dropped by the same synthetic context, but on main this path ran under an internal JWT with `useAuthenticatedUserAsActor === false`, so forwarding the parent's value would TIGHTEN behavior versus main and could break currently-working child runs mid-release. It needs its own deliberate change — and it now compounds with this one, since the child runs with the parent's decrypted env while credential-access enforcement stays off. The `input` redaction stage (masking the LLM-authored inputMapping) is also not restored — `ExecutionContext` has no field for it and the canvas workflow block never had it either. Re-enabling masking inside child runs is a live behavior change for affected tenants: `redactObjectStrings` runs with `onFailure: 'throw'`, so a child agent tool call that currently succeeds unmasked can now fail closed, which is main's semantic restored. This belongs in the release note.	2026-08-12 01:43:22 -07:00
Waleed	8f85ded35f	fix(workflows): pin a stored block retry policy when loading it (#6614) ... `workflow_blocks.retry` is a jsonb column written verbatim. Three of its writers never validate what they store: the realtime batch-add and replace-state ops take untyped block records, and the admin/superuser import routes persist externally-authored workflow JSON. `load.ts` then asserted the blob was already a `BlockRetryConfig` and handed it straight to the HTTP boundary, where `workflowBlockStateSchema` bounds `maxTries` to 2..5 and `waitBetweenTriesMs` to 0..5000. That schema is shared between the PUT `/state` body, where the bound is right, and the GET `/api/workflows/[id]` and `/state` responses, where it is fatal. The response `.parse` in the shared route builder throws a ZodError, which is not an `OrchestrationError`, so the error policy declines it and it falls through to a 500. One out-of-range or partial stored value therefore made a workflow permanently unopenable, with no in-product repair — every UI write path reads the workflow first. The feature already declares clamp-on-read as its contract: the commit that added it says bounds are clamped on read rather than rejected, the TSDoc on `resolveBlockRetryConfig` says the same, and `block-retry.test.ts` asserts it. Execution has always honoured that. Only the read boundary disagreed, so that is what this fixes: the loader now constructs a real `BlockRetryConfig` from the blob through `normalizeBlockRetryTries` / `normalizeBlockRetryWaitMs`, filling defaults for missing fields and carrying `enabled` across unchanged. `loadWorkflowFromNormalizedTablesRaw` is the single read choke point for both apps — `@sim/workflow-persistence` for the Next app and the realtime server's full-state emit — so one edit repairs every reader, including rows that are already out of range, and the row self-heals on the next save. It matches `clampParallelBatchSize` a few lines below, which already pins a stored subflow value on the same path. Alternatives rejected: - Validating on write. It leaves every existing bad row fatal forever, and it would have to be repeated across three realtime ops plus roughly a dozen `saveWorkflowToNormalizedTables` callers, none of which share a validation seam. - Bounding `BlockRetrySchema` in `@sim/realtime-protocol`. Its own TSDoc is correct that batch-add and replace-state bypass it, so this closes one writer and leaves the 500. - Relaxing the response contract. It stops the 500 but leaves the editor rendering a number execution will never run. A test now pins the write bound so that shortcut fails loudly. - `resolveBlockRetryConfig`. It returns null for a disabled policy, which would erase the numbers a builder configured every time state is read. Tests: six cases in `packages/workflow-persistence/src/load.test.ts` (four red before this change) covering out-of-range enabled, out-of-range disabled with `enabled` preserved, missing fields, a non-boolean flag, an untouched in-range policy, and NULL meaning "runs once"; plus a contract test that the write bound still rejects out-of-range input.	2026-08-12 01:42:11 -07:00
Waleed	d60b6c6273	fix(mcp): stop refusing tool results authored by another workspace member (#6610) ... `projectWorkflowToolOutput` required the resolved-secret provenance scope to carry the ACTING caller's userId, but the executor stamps that scope with the workflow AUTHOR: on the MCP bridge `execute-service.ts` sets `isClientSession: false` and `workflowUserId: workflow.userId`, so `execution-core.ts` resolves `personalEnvUserId` to the author, while the route builds its scope from `actorUserId`. Author and actor differ in the ordinary team configuration -- both attach and serve authorize on workspace membership only, and the workflow row the route selects does not even include `userId`. The refusal fired only after `executeWorkflowService` had returned, so every affected `tools/call` ran the workflow, wrote its log row, consumed an admission slot and resolved billing attribution, and then answered HTTP 500 / JSON-RPC -32603 'Tool execution failed'. MCP clients retry 500s, re-charging each time. On a public server the actor is pinned to `server.createdBy`, so a server whose creator is not the workflow author was permanently broken for every caller -- including anonymous ones and the creator -- with no recoverable setting. Provenance always carries a scope even for a secret-free workflow, so the failure did not depend on the workflow using secrets at all. The check was collateral of the #5273 rewrite that moved this bridge in-process: main's `projectWorkflowMcpModelContent` had no scope precondition, the sibling Copilot bridge added by the same commit has none, and the registry this route calls documents cross-scope provenance as an ANONYMIZATION signal, never a refusal. Comparing the tenant only is the minimal correct fix. Two alternatives were rejected. Forcing `anonymous: true` on the import is unnecessary and harmful: the registry's own `scopesMatch` already compares both fields, so a user-only difference anonymizes every entry and yields the opaque placeholder, whereas forcing it unconditionally would also strip named redaction from the author's own calls. Comparing against the workflow author instead would mean re-deriving the executor's `isClientSession ? sessionUserId : workflowUserId` rule inside a route -- the same duplication that produced this bug. Follow-up, not fixed here: this bridge imports the whole provenance bundle where main used the value-filtered `importCrossingProvenance`, so a very large env bundle can still hit `MAX_MATCHER_NODES` and produce the same billed 500. Tests pin cross-author success on both the public and the private API-key branches, that a cross-author secret redacts to `[REDACTED_SECRET]` while the author's own call keeps `{{OWNER_TOKEN}}`, and that a workspace mismatch still refuses. Each was verified to fail against the unfixed route or against the rejected alternatives.	2026-08-12 01:36:23 -07:00
Waleed	d96f3f95c8	perf(db): drop 0287's two zero-row scans from the ACCESS EXCLUSIVE hold (#6609) ... drizzle-orm 0.45.2 runs every pending migration inside ONE transaction (node_modules/drizzle-orm/pg-core/dialect.js:60-71), and migrate.ts:212 sets `statement_timeout = 0`. So the ACCESS EXCLUSIVE that 0287:2's ADD COLUMN takes on `workflow_blocks` is held, unbounded, through 0288 and 0289 to COMMIT — `lock_timeout` at migrate.ts:213 bounds acquisition only, exactly as that file's own TSDoc at :76-78 says. Every editor load, workflow save, executor block read, and realtime canvas op queues behind it platform-wide, and migrations run before image promotion (ci.yml:113-133), so the stall lands on 100% old-version traffic. Two of the three statements inside that hold did nothing. `data.errorEnabled` never existed in a released version — `git log origin/main -S errorEnabled` returns zero commits across main's entire history — so both statements filtered on it match zero rows, and the file's own comment said as much. There is no index on the `data` expression, so each was a full sequential scan of the whole table. Measured on PostgreSQL 17.9 against a 328 MB / 200k-row fixture built to the same bytes-per-row shape as the reported production table: 0287:2 ADD COLUMN 0.5 ms metadata-only, takes AccessExclusiveLock 0287:11 edge backfill 21 ms Nested Loop -> Index Scan on the PK 0287:20 (deleted) 47 ms Seq Scan, 200,000 rows removed, 0 matched 0287:22 (deleted) 47 ms Seq Scan, 200,000 rows removed, 0 matched A concurrent primary-key SELECT started 50 ms into the transaction was blocked 53-219 ms before and 22-25 ms after — the latter indistinguishable from the 21-33 ms control with no migration running at all. The two deleted statements accounted for 80,520 buffer accesses (~629 MB of in-lock I/O on that fixture) and 81% of the transaction's work. Editing an already-merged migration in place is safe here specifically because drizzle writes `hash` but never reads it back: the skip test at dialect.js:56-63 compares `created_at` against `folderMillis` only. The edit is therefore a no-op on every database that already applied 0287 (staging, dev, branch DBs) and takes effect only where it has not run. `meta/_journal.json` and the snapshot prevId chain are untouched. The only casualty is a stale `data.errorEnabled` key on branch databases a developer created it on. Nothing reads it: save.ts:50 and load.ts:89 both read the `error_enabled` COLUMN, and load.ts passes `data` through untouched. Alternatives rejected, each checked against the code rather than assumed: - An embedded `COMMIT;` to end the transaction early. 0289's four CREATE TYPE, its CREATE TABLE, and its three CREATE INDEX all lack IF NOT EXISTS, so a mid-batch failure in autocommit leaves them applied-but-unjournaled and the replay dies on 42710 — which migrate.ts:219 only retries for 55P03. That turns a transient stall into a wedged deploy. - Moving the statements to a new file after 0289. All pending files share one transaction, so it buys exactly zero lock reduction. - Rewriting the surviving backfill as `WHERE id IN (SELECT source_block_id FROM workflow_edges WHERE ...)`. Measured both: planner-equivalent. `source_handle` is unindexed, so both forms seq-scan `workflow_edges` (7.5 ms, identical) and then index-scan `workflow_blocks` on the primary key — it never scans that table. The IN form adds a HashAggregate and 1,170 more buffers, so it is marginally worse. Left as shipped. - Promoting the `data-backfill` lint from warn to annotate. `readAnnotation` only requires a non-empty reason and 0287 already supplies one per statement, so the rule would fire zero findings. 0288's nullable `retry` column is correct as-is and unchanged. Both delete-and-reinsert save paths on the deployed version (save.ts:30-63 and the realtime REPLACE_STATE handler) reset `error_enabled` to false and `retry` to NULL for a workflow saved by an old replica during the rollout; the ordinary realtime block upsert does not, because its `set` clause omits both columns. That residue is tolerable: everything the surviving backfill writes is re-derived from the edge set at workflow-block.tsx:754 and lib/workflows/persistence/utils.ts:196-201, and `retry` ships in this same release so it has no installed base. `bun run check:migrations origin/main` drops from three data-backfill warnings to one. The real migrator applied all 289 journal entries to a fresh PostgreSQL 17.9 database with the edited file, producing `error_enabled boolean not null default false` and `retry jsonb`.	2026-08-12 01:32:00 -07:00
Waleed	d1bc99a9c4	improvement(integrations): add Managed Agents templates and guard docs links (#6608) ... * improvement(integrations): add Managed Agents templates and guard docs links The Claude Managed Agents block shipped without templates or suggested skills, so its integration detail page had nothing to offer and the "Add to Sim" chat handoff was the only affordance. Add nine templates and seven skills, each grounded in an operation the block actually exposes. Its docsLink also pointed at integrations/managed-agent while the page is managed_agent, so the link 404'd. Five more blocks had the same class of bug via a stale tools/ prefix. Nothing validated these, because the catalog check only compares deployment fields. Add that validation, and collapse the three copies of the docs-URL contract onto one exported helper so the checker and the generator cannot drift apart. * fix(integrations): resolve docsLink once when checking the vendor allowlist The stale-allowlist predicate read block.docsLink directly while the main loop read the resolved link, so an allowlisted block that dropped its explicit docsLink produced undefined from the optional chain, negated to true, and was treated as still vendor-linked — the stale entry went undetected. Record vendor-linked types during the single pass that already resolves each link, so both checks agree by construction. * fix(managed-agents): name every integration a template's alsoIntegrations claims alsoIntegrations is documented as the blocks a template's prompt references, and it drives which catalog pages the template cross-lists on plus the icon cluster on the detail page. Three prompts named a service only implicitly, or not at all: the runaway stopper claimed Slack without mentioning it, the PR reviewer said "pull request" rather than GitHub, and the weekly report said "emails" rather than Gmail. Name the service in each prompt so the field is accurate and the templates surface on the right pages. * fix(integrations): point every visible integration at Sim's own docs Six blocks had a vendor documentation URL in docsLink — cursor, enrich, enrow, google_groups, qdrant, and similarweb — which was accidental rather than deliberate. Each already has a generated Sim page, and each already carries the vendor's homepage on BlockMeta.url, so the vendor link in docsLink only sent readers away from our own documentation. Point all six at their Sim page and drop the allowlist that had been tolerating them. Every visible integration gets a generated page, so a docsLink outside docs.sim.ai is now always an error.	2026-08-12 01:31:47 -07:00
Waleed	892401a8d0	fix(uploads): sign Azure upload URLs create-only (#6607) ... getBlobPresignedUploadUrl signed its SAS with BlobSASPermissions.parse('w'). Per Azure's service-SAS reference, `w` is "create or write content" and permits overwriting an existing blob; `c` is "write a new blob" and does not. main signed `c` before this signer moved out of core/storage-service.ts, so Azure deployments lost create-only enforcement in the move. The `If-None-Match: '*'` the signer returns cannot carry the guarantee on its own: an Azure service-SAS string-to-sign covers the resource, times, permissions and the five rsc* response-header overrides, never request headers, so a client is free to drop it. The header is signed on the other two providers -- inside the PutObjectCommand on S3, and as x-goog-if-generation-match in signed extensionHeaders on GCS -- which is why only Azure regressed. Without this, a signed upload URL stayed a plain overwrite grant on the final key for its full hour. A caller could replace the object after complete had already verified size and content type, written the workspace file row and metered storage from that verified HEAD, leaving durable metadata and billing describing content that no longer exists. The multipart block-staging signer keeps `w`: block staging is overwrite-shaped and matches main. The existing test asserted parse('w'), so it locked the defect in; it now asserts create-only, and its name states the guarantee so a future flip reads as deleting a security property rather than adjusting a value.	2026-08-11 23:10:42 -07:00
Waleed	b5d9e93797	fix(mcp): audit an upsert that rewrites or revives a server (#6602) ... * fix(mcp): audit an upsert that rewrites or revives a server Registering a URL that already exists takes the upsert branch and rewrites the live row — name, transport, headers, timeout, enabled, auth type, the connection reset, and the URL's query string, since the server id hashes only origin and pathname. That branch recorded no audit row at all: the ADDED audit was gated on `!result.updated`. main recorded ADDED for these (wrong action, but a row existed), so this restores coverage and fixes the action. Reachable from the settings POST /api/mcp/servers and from Copilot's manage_mcp_tool `add`, neither of which passes existingServerBehavior. The v2 POST passes 'reject' so it only reaches the upsert on a revival. A rewrite is now MCP_SERVER_UPDATED carrying updatedFields; a revival of a soft-deleted row stays MCP_SERVER_ADDED. updateValues is typed Partial<$inferInsert> so Object.keys is column-safe. Analytics gating is unchanged: mcp_server_connected still fires only for a genuine insert. * fix(mcp): redact audit URLs and drop unwritten columns from updatedFields Two review findings on the new upsert audit. The upsert assigns every column unconditionally, so `description` is present on updateValues but undefined when the registration omits it. Drizzle skips undefined in .set(), so deriving keys without checking values made the audit claim a column the write never touched. Filter by value; null stays, since clearing a value is a write. MCP URLs carry tokens in their query string — that is why a silent rewrite of one matters — and audit rows are readable by org admins who need no workspace MCP access. Newly auditing rewrites would persist those tokens verbatim, so every MCP audit row now records the URL through sanitizeUrlForLog, which strips query and fragment. Applied to the add, update and delete rows alike: redacting only the new path would leave the same credential in the row a first registration already writes. A null url stays null rather than becoming an empty string.	2026-08-11 22:13:46 -07:00
Waleed	068422b3ff	refactor(audit): derive updatedFields through one shared helper (#6604) ... * refactor(audit): derive updatedFields through one shared helper Six copies of Object.keys(updateData).filter(k => k !== 'updatedAt') across four files decided, independently, which columns an audit row reports. The exclusion set is an audit convention, not a local detail, so it moves to @sim/audit as auditUpdatedFields and the exclusion becomes a single edit. The admin organizations route evaluated the expression twice in one handler and filed it under the metadata key `fields` while every other site uses `updatedFields`, so any consumer filtering on updatedFields silently missed org updates. It now computes once and uses the shared key; nothing reads metadata.fields. auditMock carries the real implementation rather than a stub, since callers under test derive their audit metadata through it. The two suites that hand-roll an @sim/audit factory source it from there. * test(audit): pin the testing mock's copy of auditUpdatedFields @sim/audit devDepends on @sim/testing, so the mock cannot import the real helper without closing a package cycle. Assert parity from the audit side instead, where the dependency already runs the safe direction, so a change to the exclusion convention cannot leave mocked callers validating behavior the deployed helper no longer has.	2026-08-11 22:11:57 -07:00
Waleed	a40e379784	chore: remove the stray .agiloft-spec working notes (#6605) ... Transcribed Agiloft REST docs committed by accident in #6562 alongside the alrest repointing. Nothing imports or links it, it sits in a dot-directory at the repo root rather than anywhere docs live, and the integration it informed has shipped. Contents are the vendor's own public examples — placeholder credentials and localhost hosts only, so nothing sensitive was exposed while it was public.	2026-08-11 22:11:47 -07:00
Waleed	e65345b63d	test(table): pin the executor auth pairing on the table read route (#6603) ... * test(table): pin the executor auth pairing on the table read route fetchTableSchema reaches GET /api/table/[tableId] with a legacy type:'internal' token, which only works while that route authenticates through checkSessionOrInternalAuth. Its sibling table routes already moved to the delegation policy, which rejects that token outright, so migrating this one without moving the caller in the same change would break every table tool on an Agent block. Assert the route still authenticates through the legacy path so that migration fails here first, and record on the caller why it is deliberately not on buildExecutorDelegationHeaders yet. * test(table): assert the Bearer header reaches the legacy verifier Address review: the pairing test sent no Authorization header and its name claimed to verify token acceptance, which is pinned separately in lib/auth/internal.test.ts. Send a representative header, assert it reaches checkSessionOrInternalAuth unmodified, and scope the name and docs to what this guard actually covers — the route's choice of verifier.	2026-08-11 22:09:15 -07:00
Waleed	0d640aabbc	fix(uploads): make execution attachment completion replay-safe (#6601) ... finalizeExecutionAttachment reported a completedFileId. That marker is what routes a replayed completion into loadCompletedUploadPurpose, which handled only workspace_file and threw a bare Error otherwise -- unclassified, so the route rendered a generic 500. Its structurally identical twin, finalizeMothershipAttachment, correctly reports nothing. Both are metadata-backed and idempotent by storage key, exactly as the finalizeUploadPurpose TSDoc already states, so neither needs the marker: their replays are correct through the finalizer itself. Drop it from the execution finalizer so the two twins agree. loadCompletedUploadPurpose becomes an exhaustive switch, matching the sibling finalizeUploadPurpose switch, so adding a purpose is a compile error until its replay behavior is decided rather than a runtime 500. The residual arm throws a classified UploadSessionError('internal') instead of a bare Error. markUploadSessionCompleted no longer clears a marker it was not given. A finalizer that records one inside its own registration transaction -- markUploadSessionFileRegistered does this for workspace_file -- would otherwise have it overwritten with null, and both the abort guard and the expiry sweep key on it: cleanupExpiredUploadSessions only treats a finalizing session as disposable when completedFileId is null. This is a no-op for every current path, since markUploadSessionCompleted moves the session to completed, which is neither abortable nor a cleanup candidate. Latent only. No shipped client replays a completion: the sole producer, uploadWorkflowAttachments, mints a fresh session per file and never retries, requestJson does not retry, and a concurrent double-submit is already a clean 409 from claimSession. Tests pin the invariant rather than the symptom: a Record over the purpose union is a compile-time gate on which route each purpose replays through, and the cases assert that idempotent purposes report no marker and reject cleanly if they ever reach the loader.	2026-08-11 22:01:19 -07:00
Vikhyath Mondreti	a6ebfec7d2	fix(files): restore CSV preview cancellation (#6596) ... * fix(files): restore CSV preview cancellation * fix	2026-08-11 21:52:46 -07:00
Theodore Li	0877ecb6bd	fix(tables): tolerate row deletion during run cancellation (#6600) ... * fix(tables): tolerate row deletion during run cancellation * fix(tables): unwrap row deletion errors	2026-08-12 00:51:28 -04:00
Waleed	22b3569627	fix(files): order file folders in SQL like every other folder list (#6599) ... The list use case defaulted to a name sort whose defaults were written for the new v2 contract, so the internal route — whose contract exposes no sort params — could no longer reach the repository's sortOrder ASC, createdAt ASC ordering. Surfaces that render the payload positionally (the @-mention Folders group, the add-resource search results, Copilot's list_file_folders) silently flipped from newest-first to alphabetical, and the Files browser's SSR prefetch hydrated a different order than its own refetch. Push the sort down to the query like the workflow, knowledge, and table folder lists already do, reusing FOLDER_SORTS. Omitting sortBy keeps the position ordering; v2 always sends one from its contract defaults. A name sort now also uses the database collation and the shared createdAt tiebreak instead of a JS comparator over UTF-16 code units.	2026-08-11 21:48:01 -07:00
Waleed	933eea50df	fix(mcp): audit the columns an MCP server update wrote, not the params it got (#6598) ... Every PATCH /api/mcp/servers/[id] audit row listed oauthClientId, oauthClientIdProvided and oauthClientSecretProvided — on edits that never touched credentials — while omitting the connectionStatus/lastConnected/ lastError resets the write actually performed. The route always sends `oauthClientId: body.oauthClientId || null` and `*Provided: ... !== undefined`, and null and false both survive a `value !== undefined` filter. The two *Provided flags are control params, not columns at all. Only the writer knows which columns a write touched, so updateMcpServer now returns updatedFields from its updateData and both the internal audit wrapper and the v2 use case record it. This matches workflow-mcp-lifecycle and credentials/orchestration, which already report written columns this way.	2026-08-11 21:43:02 -07:00
Theodore Li	326cb94c27	fix(knowledge): return pending status for new documents (#6597)	2026-08-12 00:41:37 -04:00
Theodore Li	50e5dff53e	fix(workflows): redact run and export secrets (#6591) ... * fix(workflows): redact run and export secrets * fix(workflows): preserve redacted run outputs * fix(workflows): retain safe trace output fallback * fix(workflows): stop deriving outputs from traces	2026-08-12 00:32:44 -04:00
Theodore Li	9923fafe5e	fix(tables): prevent legacy group auto-run dispatch (#6595)	2026-08-12 00:32:16 -04:00
Vikhyath Mondreti	eb26b42680	fix(logs, workflows): snapshot fetch optionality, restore consistent draft read snapshots (#6594) ... * fix(logs): snapshot fetch optionality * fix(workflows): restore consistent draft read snapshots	2026-08-11 21:27:05 -07:00
Waleed	a61cbf7287	fix(tools): bind schema-enrichment reads to executor delegations (#6593) ... * fix(tools): bind schema-enrichment reads to executor delegations Two schema-enrichment callers still sent the deprecated legacy internal JWT to routes that moved onto delegation-only auth, so both 401'd and swallowed the failure: - tools/params.ts fetched a child workflow's input fields with an unsubjected buildAuthHeaders(), leaving the Agent block's workflow_executor inputMapping untyped so the model guessed the child's field names. - tools/schema-enrichers.ts fetched KB tag definitions the same way, which dropped the tags/tagFilters parameter from the knowledge tools entirely. Extract the executionId binding rule into executionScopeForTarget so the three enrichment call sites share one definition. * improvement(tools): surface tag-definition read failures at error level Enrichment degrades silently by design, so the log line is the only signal a credential break leaves. Drain the body on the failure path too.	2026-08-11 21:22:16 -07:00
Waleed	a1a05c6ece	fix(api): widen the cancel-execution reason contract to what the route emits (#6592) ... The internal cancel route mints four outcomes the cancellation service never produces — queue_cancelled, already_cancelled, active_resume_signal_failed and cancellation_not_finalized — but the contract enumerated only the five service reasons. requestJson validates every 2xx against that contract, so a stop that genuinely applied threw on the client. Keep the service enum narrow for the public v2 contract, which delegates wholly to the service and cannot emit the other four, and validate the internal route against a superset. Route the seven success bodies through one contract-typed constructor so drift is a compile error, and delete the dead duplicate cancel contract left behind in contracts/logs.ts.	2026-08-11 21:22:05 -07:00
Theodore Li	91f31a4983	fix(api): wait for knowledge dispatch and encode filenames (#6583) ... * fix(api): wait for knowledge dispatch and encode filenames * fix(knowledge): make document uploads durable * fix(knowledge): serialize document processing attempts * test(knowledge): grant processing attempt claim * fix(knowledge): restore chunk retry metadata * fix(knowledge): reclaim stale processing attempts * fix(knowledge): preserve processing claim ownership * fix(knowledge): restore processing takeover semantics	2026-08-12 00:18:02 -04:00
Siddharth Ganesan	380aca2b61	fix(desktop): fix background tab spawning (#6590) ... * Update tab spawning * update import v0.7.69-staging.26236.1	2026-08-11 20:32:14 -07:00
Theodore Li	23318a13a0	fix(security): redact workflow snapshot secrets (#6581) ... * fix(security): redact opaque workflow snapshot inputs * fix(security): document fail-closed tool redaction * fix(security): redact malformed tool params * fix(security): redact nested credential references * fix(security): isolate opaque tool schemas	2026-08-11 23:14:15 -04:00
Theodore Li	f306b517c1	fix(files): preserve slashes in folder paths (#6589) ... * fix(files): preserve slashes in folder paths * fix(files): resolve escaped folder lookups	2026-08-11 23:13:18 -04:00
Theodore Li	766526b22e	fix(logs): restore narrow v1 detail query (#6588)	2026-08-11 22:55:42 -04:00
Waleed	083319be8b	fix(api): conceal cross-tenant resource denials on internal routes (#6586) ... The v2 routes rewrite DelegatedWorkspaceAuthorizationError, NoWorkspaceAccessError, and WorkspaceApiKeyScopeAuthorizationError to a 404 so a caller with no reach into a workspace cannot confirm a resource exists. The internal routes reach the same application use cases and still answered 403, so the same probe worked from the other surface. Same-workspace role denials stay 403 on both.	2026-08-11 19:50:04 -07:00
Theodore Li	5e1862ed20	fix(tables): preserve group auto-run semantics (#6579)	2026-08-11 22:46:31 -04:00
Justin Blumencranz	a72457b9f5	fix(docs): preserve items response fields (#6587)	2026-08-11 19:43:58 -07:00
Waleed	7c05e36049	fix(api): close five defects found auditing the v2 migration against main (#6575) ... * fix(tables): stop a column retype from nulling empty-string cells A type conversion rewrote every cell holding '' to null. Main only nulled a blank the target type could not read; '' is a real stored value that both string and json columns accept, so string->json and json->string silently destroyed those cells. Worse on a required target: countEmptyCells matches only a missing key, SQL NULL, or '[]', so '' passes the required guard and the rewrite then wrote null behind a constraint that had just succeeded. The per-cell decision is now the pure retypeCellRewrite, restoring main's rule: null a blank only when the target cannot read it, otherwise coerce. * fix(execution): release the concurrency slot when a group cancel is refused The stop-the-work effects (durable Redis abort record, queue-job cancel, in-process abort) all fire before the workflow-group sidecar is consulted, and none can be undone. When the sidecar refuses the claim we throw a conflict, which skipped releaseExecutionSlot and stranded the plan concurrency reservation until it expired. Every conflict return is a terminal-or-absent state - a missing log row, a log already completed or errored, or a terminal cell - so a refusal never means the run is still executing. The slot is released before the throw, keeping the exact success && !isPausedCancellationPath predicate rather than a blanket finally that would free reservations for live runs. * fix(uploads): recover an ambiguous PUT instead of discarding the object Main recovered an upload whose bytes committed but whose response was lost, via a verify endpoint. The session client retries the PUT instead, but every provider now signs a create-only precondition, so the retry returns 409/412, is classified non-retryable, and the session aborts - deleting the object that had already landed. A transient blip on the final ack cost the whole upload. A conflict on a retry attempt is now treated as our own earlier PUT having committed, and completion proceeds. That is safe because completeUploadSession independently verifies the object through assertObjectIdentity, which rejects on uploadId mismatch before anything durable is registered. A first-attempt conflict still fails loudly. * fix(folders): enforce the workspace folder ceiling on the create path Readers bound the active path index at MAX_FOLDERS_PER_WORKSPACE and throw once a workspace exceeds it, but POST /api/folders reached createFolder, which has no maxFolderRows field and never counts. A workspace could therefore be driven past the ceiling, after which the 27 capped read sites failed on a state the product had allowed. createFolder now asserts room inside its transaction, right after the mutation lock, so the count cannot be raced. The refusal is a typed conflict rendering 409 with an actionable message rather than a 500. The check counts rows directly instead of loading the path index, so an already-over-cap workspace gets a clean refusal rather than a read error, and no reader gained a cap. folderMutationStatus also gained the payload_too_large mapping it was missing, which had been rendering a delete-cascade cap breach as an unexplained 500. * fix(skills): route internal skill writes through the shared use cases The internal route made the workspace authorization decision itself, never consulting the skills operation policy, never loading canonical workspace context, and recording an audit entry with no operation id or actor projection. v2 and Copilot already went through the use cases; only this surface did not. GET/POST/DELETE now authenticate, parse, call the shared use case, and present. Request and response shapes are unchanged. Two behavior changes fall out: a write against a deleted workspace is now refused with 404 rather than accepted, and permission-denial text matches the rest of the platform. Legacy internal-JWT auth is dropped because no principal kind expresses that caller and nothing calls it: the whole repo references /api/skills only in two comments, no tool declares an internalRoute to it, and the executor reads skills through a direct listSkills call rather than over HTTP. * fix(folders): enforce the workspace ceiling on the remaining create paths Folder duplication, admin workspace import, and workspace forking all inserted folders without consulting the ceiling that 27 read sites enforce, so any of them could leave a workspace whose reads then fail. Each now asserts room for the rows it is about to add rather than one at a time: duplication measures the whole subtree up front, forking counts its bulk insert, and import counts per segment because that is genuinely one row. assertFolderCollectionHasRoom gained an additionalRows notion for the bulk case, and short-circuits when nothing is being added so an over-cap workspace still reads and still syncs. Duplication deliberately does not take the folder mutation lock. Holding it across the copy would block folder creation workspace-wide for an unbounded time - there is no cap on workflows per subtree and duplicateWorkflow runs sequentially - and narrowing it is impossible because an advisory transaction lock cannot be released early; splitting the transaction would leave a half-copied tree on failure. A rare few-row overshoot near the ceiling is the better trade, and it matches what forking already does. A test asserts the lock is absent so re-adding it is a visible decision. Admin import gained the transaction and lock it never had. Its folder-full refusal escapes the per-workflow result list, because a full tree is a property of the workspace and would otherwise be buried as N failures behind a 200. The fork and promote routes had no catch at all, and withRouteHandler only classifies HttpError, so a refusal rendered as an opaque 500 - twice over, since drizzle wraps the throw. Both now project a classified conflict as 409 and rethrow anything unclassified. * fix(uploads): bound the signed PUT lifetime and advertise its real expiry A single-PUT transfer was signed for the whole 24h upload-session TTL, because expiresAt was reused as both the session lifetime and the signing lifetime. Multipart part URLs in the same file kept 1h, and the pre-migration presigned route signed every PUT for 1h, so the widening was unintended rather than a policy change. No provider clamps below 24h. The PUT presign is now clamped at the provider boundary by a shared UPLOAD_URL_TTL_MS, which the part-URL path also uses so the two cannot drift. An expired PUT URL is deliberately not recoverable: unlike multipart, which re-signs per part call because its progress is durable, a PUT is not resumable, so an expired URL and an interrupted PUT have identical recovery. Nothing leaks, since every provider signs a create-only precondition. Clamping alone would have made the contract lie: the URL would die an hour before the session's advertised expiresAt, with nothing telling an integrator why the 403 happened. The PUT transfer now carries its own expiresAt, mirroring the multipart part-URL field. It is provider-dependent on purpose - cloud transfers report the clamped signature expiry, while the local data plane has no signature and admits against the session, so reporting an hour there would have been a new inaccuracy in the other direction. * chore: delete the dead presigned-upload and skill-adapter paths The presigned upload routes and the internal skills adapters were both replaced during the v2 migration, leaving their implementations behind with no callers. Removed generatePresignedUploadUrl and verifyPresignedUploadReceipt with their three provider helpers, QUOTA_EXEMPT_STORAGE_CONTEXTS and the types it orphaned, and the performCreateSkill/performUpdateSkill/performDeleteSkill adapters with recordSkillEvent and statusForSkillOrchestrationError. Each was verified unreachable across apps, packages, scripts and ee, including barrel re-exports and string access, not just direct imports. recordSkillEvent needed the closest look, since deleting an audit writer can silently drop coverage. The use cases declare the same action, resource, and description, and the framework adds the operation and actor the old helper lacked; recordAudit back-fills actorName and actorEmail from the user table when both are omitted, so the one field the helper passed is not lost. The self-hosting architecture doc described a directUploadSupported flag on an endpoint that no longer exists, and now describes the upload-session flow that replaced it. * refactor(folders): keep the cheap resource facts out of the schema graph Reading a folder resource type's label or its lock support meant importing folderResourceConfig, which imports the db schema for every table it serves and from there reaches lib/table/service, the executor, and the tool registry. That mattered as soon as lib/folders/queries needed a label: queries is reached from workspace-file-manager, which is reached from the files and chat pages, so one import edge put roughly 4,700 modules into those page graphs and broke the tool-registry boundary audit. Labels and lock support now live in a leaf module that imports only a type, and config composes them so there is still one source of truth. The three folder routes that pulled the whole config in for a single boolean read the leaf instead. * fix(skills): apply an upsert batch in one transaction The internal skills route looped the batch, calling an independently committing use case per item. A rejection on a later item left the earlier ones written and audited while the request reported failure - the compound-mutation rule in CLAUDE.md exists for exactly this. No new transaction plumbing was needed: upsertSkills already wraps its whole item loop in one db.transaction, so the partial commit came from calling it N times rather than once. upsertSkillBatch now validates and per-skill authorizes every item before issuing a single write, and createSkill and updateSkill became thin wrappers over it so v2 and Copilot keep one authority for the rules. The compound operation declares the read floor that skills.update already used, and the use case additionally authorizes skills.create when any item lacks an id, still ahead of every write. A read-only member who is a skill editor keeps their edit, and creates are not authorized more loosely than before. Audit projects one entry per committed skill, and analytics moved after the commit so nothing is reported for a rolled-back item. Note metadata.operation for these writes is now skills.upsert rather than skills.create/update; the action field still carries the distinction. * fix(security): close two disclosure gaps and finish the slot-leak fix The payer-pool gate only covered the workspace branch. A personal API key that omits workspaceId takes the account branch, where getHighestPrioritySubscription resolves an organization subscription from any member row regardless of role - so a plain member read the organization-wide credit and storage pool by dropping one query parameter. The account branch is now gated by the same authority, and the storage pool is not queried when it may not be disclosed. Forcing that branch self-scoped instead would have downgraded plan, period, and status, which is what a member needs to see whether the org is blocked. GET /api/v1/logs/executions/[executionId] emitted the workflow snapshot raw, carrying password sub-block values and oauth-input credential ids. It now shares the sanitizer the v2 read already used, extracted so there is one implementation rather than two. Env-var references are still preserved. cancelWorkflowGroupExecution itself was unguarded, so an unexpected throw from its transaction escaped ahead of every release site - the same reservation leak this branch set out to close, still open on the adjacent path. It now releases through the shared predicate and rethrows, because a failed transition means the cell state is unknown and a success-shaped answer would be a lie. The comment claiming the abort record cannot be taken back was false and now states the real reason: a refusal is always a terminal-or-absent state. * fix(v2-api): cover every persisted run status and every capped body The workflow-runs endpoints carried the same omission the logs contract had: the execution logger persists redacting, the run schemas did not list it, and because validation is whole-response one such row returned 500 for an entire page. Both schemas now derive from PersistedWorkflowExecutionStatus behind the same AssertNever gate, so a future status is a type error rather than a production 500. The single-run read keeps its extra queued value, which only it can observe. That schema was also serving as the run-list status filter. Widening it would have accepted a filter value the application input cannot express, so the reported set and the accepted filter are now separate schemas. Routes declaring maxBodyBytes without payloadTooLargeResponse fell back to a bare string with no error code and no private cache header. Rather than patch the four, the default moved into the builders - all three had the hole - which covers 58 body-bearing handlers, and a route override still wins. The five per-route overrides that merely restated the default are gone. Also documents the 413 on the one knowledge route that has a real body cap, adds the rollout gate's 404 to the last v2 operation missing it, and rewords the nextCursor description, which read as though every list were a full-set list. * fix(api): make the shared traits and lifetimes single-sourced The folder resource-traits leaf composed labels into config but restated lock support independently, so the routes reading the trait and the orchestration reading the config could disagree about which resources lock. Config now composes both, and supportsLocking is required rather than optional so a new resource type cannot silently omit it. The upload commit claimed the PUT clamp and the multipart part URLs shared a constant and could not drift. That was true only of the advertised expiry - the three provider signers each hardcoded an hour, so changing the constant would have moved what we advertise while leaving what we sign, recreating exactly the mismatch the clamp removed. Each provider now receives the lifetime in its own unit from the one constant. Table restore hand-rolled its status map and returned the driver message verbatim at 500, leaking the failed statement and its bound parameters - the same defect this branch closed at nine other sites. It and import-csv now use the shared projection; import-csv's result type also had to carry the lock the classifier already set, so a 423 can name it. Deletes v2RowWriteError, which had no callers and would have rendered a locked table as 400 by discarding the 423 it was handed.	2026-08-11 19:37:40 -07:00
Vikhyath Mondreti	261435cabe	fix(authz): enforce credential and workspace boundaries (#6585) ... * fix(authz): enforce credential and workspace boundaries * fix(api): preserve fork policy in workflow detail * fix(knowledge): preserve credential access guidance	2026-08-11 19:35:58 -07:00
Waleed	1874ceccda	fix(api): collapse the internal error envelope and restore requestId (#6584) ... * fix(api): collapse the internal error envelope and restore requestId The builders shipped two internal error envelopes: internalOrchestrationErrorPolicy emitted { success: false, error } while internalPlainOrchestrationErrorPolicy emitted { error }. That split approximated pre-builder behavior, where the shape depended on which branch failed - guard clauses returned { error } and a route's terminal try/catch returned { success: false, error }. A per-route policy cannot express a per-branch rule, so the two disagreed on the same status across families. Collapse to the bare { error } shape. It is what messageFromErrorBody reads on the client and what most migrated routes already emitted. requestJson throws ApiClientError for any non-2xx, so no typed client ever observes the discriminator. success: true on success bodies is a separate contract and is untouched. Also restore requestId to internal error bodies. withRouteHandler stamps it on the bodies it generates, but the builder overrides dropped it, leaving it only on the x-request-id header - invisible when a user pastes an error out of the UI. It is now applied at the createJsonErrorResponse chokepoint and in both wrapper overrides, and is omitted when there is no active request scope. * fix(api): stamp requestId on internal auth and parse failures	2026-08-11 19:32:59 -07:00
Theodore Li	9f3c20290b	fix(tables): cap row pages and use native cursors (#6582)	2026-08-11 22:24:50 -04:00