v0.9.6: search improvements, forking fixes, tables correctness - #8410
Merged
Merged
Conversation
* refactor: remove dead code and enforce code quality gates * fix(ci): bind script coverage to the executed runner
… against the live schema (#8394) * fix(tables): match unique JSON values exactly and validate row writes against the live schema - Unique checks and the upsert conflict probe matched JSON objects and arrays by containment, so `{"a":1}` counted as a duplicate of `{"a":1,"b":2}` and upsert could overwrite a row that only contained its target. They now also require exact jsonb equality, keeping containment as the GIN-indexed leading clause; JSON values take per-value locks like other types - Row writes validated against a schema snapshot read before their transaction, so a concurrent make-unique, make-required, retype, or column delete could commit violating data. Each write now reads the live schema under the table's schema lock (shared) through `user_table_schema_for_write`, in the statement it already runs first, and validates against it - The background update runner derives each batch's patch from the raw payload against the live schema, through the same helper as the inline bulk update, and refuses unique and required-null patches under the lock * fix(tables): install the schema guard for db:push, refit rows from raw input - user_table_schema_for_write moves from Drizzle migration 0391 to script migration 0026, which db:push runs too. A db:push database (local dev, the CI push provision) never applied 0391, so every guarded row write failed there. The journal ends at 0390 again; the function body and its comments are unchanged. - A writer that finds the schema moved rebuilds the row from the caller's raw input instead of the value it coerced against its snapshot, so a "007" sent while a column changed from number to text is stored as "007", not "7". This covers insert, upsert, update, batch update and the import batch. - The refit re-checks the row's size, which a coercion to a wider type can grow past the limit after the pre-lock check passed. * fix(tables): re-check update batches under a moved schema, own-key reads - The background update runner re-reads a batch's rows inside the batch transaction and re-validates them merged with the re-derived patch when the schema moved since the page was checked, as the inline bulk update does; an unchanged schema adds no query. updatePageByIds takes an async per-batch hook with the transaction for this. - batchInsertRowsWithTx returns the definition it validated against, and batchInsertRows dispatches its insert triggers with it. - Row cells and patch keys are read as own properties (Object.hasOwn), so a legacy column keyed by a prototype name such as `constructor` is not seen in rows or patches that do not hold it. - The long schema-wait test asserts the write is waiting on the schema lock before the holder commits. * fix(tables): read the remaining row cells by own key in bulk validation, replace dedupe, and the upsert probe
…#8397) * fix(selectors): rebind Copilot principals for nested domain use cases Selectors backed by another domain (knowledge documents, table columns, workflows, sandboxes, MCP tools) forwarded the Copilot principal admitted under the selector audience into use cases that accept only their own audience. The refusal is a DelegatedWorkspaceAuthorizationError, which the v2 surface conceals as 404, so Chat saw "Workspace not found" on workspaces the user administers. Fork sync previews hit it whenever a saved dependent value (a table conflict column, a knowledge document) needed validation. Selectors now rebind through bindCopilotWorkspaceOperation, which can also project the single resource a nested call reaches, as the executor and Chat MCP paths do when they mint. A grant already narrowed to one resource is never moved to another. Managed MCP connections now reach their credential-group rule and return its 403 instead of the concealed 404. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(selectors): scope table selector reads to the selected table Tables honor resourceScope.tableId, so the nested read names the table it reads, as the MCP selector names its server or connection. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…8400) * fix(ui): clean up stale workflow state and provider policy handling * chore(tests): address cleanup review findings
* feat(forks): opt-in fork sync for new workflows Workspace forks gain a lineage-wide policy for whether a NEWLY created workflow joins fork sync. Today a workflow joins the moment it is deployed, so deploying something experimental in a parent pushes it into every fork on the next sync with no step where anyone chose that. Opt-out remains the default, so no existing or new workspace changes behaviour until someone flips the toggle. The policy lives on workspace.fork_sync_new_workflows_excluded (default false, the historical behaviour); workflow.fork_sync_excluded is untouched including its default. It is uniform across a fork lineage: settable from any member, fanning out to every ancestor and descendant under an advisory lock keyed on the lineage root, which fork creation and unlink also take. It is forward-only - flipping it never rewrites an existing workflow's sync state - and each changed member records its own audit entry naming where the change was issued from. Genuinely new workflows (create, duplicate, admin/superuser import, a fork's starter) take the workspace policy. A copy (fork creation, promote-create) inherits the SOURCE workflow's flag, because it is the same logical workflow in another workspace; without that an opted-in workflow would copy into a fork already excluded and never sync again. The fork modal gains "Copy unsynced workflows" (off by default, shown only when the source has unsynced deployed workflows, disabled when the combined set would exceed the fork ceiling) so forking an opt-in workspace cannot silently produce an empty fork. The settings section becomes "Synced workflows" with the polarity flipped - checked means the workflow syncs - above a "Sync new workflows by default" toggle row that states its lineage-wide reach. The wire field stays forkSyncExcluded, so the tree owns the single inversion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(forks): break the fork lock cycle and derive review-round values once Round 3 review left 7 threads. Five were the same mistake: a guard added at one read site of a value while the other read sites kept the unguarded one. Each fix below is one derived value used everywhere, or a deleted duplicate path, rather than another guard at another call site. Lock-order deadlock (reported independently by both reviewers). `lineage.ts` already declared `fork-lineage` the coarsest fork lock, but ranked only the three advisory locks and said nothing about `lockForkRevision`, which takes FOR UPDATE on `workspace`. `createFork` took it first, so it held that row while waiting for `fork-lineage`, while `unlinkForkEdge` held `fork-lineage` and waited to UPDATE the same row. Hoist the lineage lock above it, and replace the partial contract with a rank table covering every lock in the module. All six fork transactions now acquire in ascending rank. Stale workspace rows in the synced-workflows list. `useWorkflows` and `useFolders` both set `placeholderData: keepPreviousData`, so a workspace switch served the previous workspace's rows with `isLoading: false` and a click posted workspace A's ids against B. Gate on `isPending || isPlaceholderData`, matching `custom-tools.tsx`. Fork modal submitted a value the switch showed as off. `copyUnsyncedWorkflows` had two readings and submit used the raw one. Derive it once from the request plus the limit, and read that at all four sites. Audit entries named workspaces by id. Return the name from the UPDATE and project `resourceName` from it, so a lineage-wide change no longer reads as one named workspace and N opaque identifiers. Also: give `setForkSyncDefault`'s multi-row UPDATE a deterministic row-lock order, delete the non-barrel re-export left behind when the workflow limit moved to `limits.ts`, and correct a TSDoc invariant that claimed the `fork-target` lock for both callers when only one holds it. Tests, per the repo's test-audit gate: add one `*.integration.ts` that races the two real lock sequences against real Postgres and asserts the pre-fix order deadlocks while the shipped order does not, so the check cannot pass vacuously. Drop the re-added contract-schema test (an identical file was pruned as low-signal in #8295), fold the fork-sync inheritance assertion into its sibling, and reduce the synced-workflows test to the pure tree builder whose checkbox stub no longer implements the polarity it asserts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(forks): drive the lock-order suite through the production helpers cubic's round-4 finding on the new integration suite was right: it mirrored the lock SQL as string constants instead of executing production's, so a `createFork` regression to the pre-fix order would have left it green. Closed on both halves. The fixture now calls the real `setForkLockTimeout`, `acquireForkLineageLock` and `lockForkRevision`, against the tables the last of those actually locks, so a change to the advisory-lock key or to the revision lock's coverage is carried into the test rather than silently diverging from a copy. A third check pins why the pair conflicts at all: the revision lock really does hold the `workspace` row, which is the edge of the cycle. The order inside `createFork` is asserted where it lives, in `create-fork.test.ts`, on invocation order through the admission path. Order is the entire contract here, which is the case the retention bar keeps call ordering for. Verified red for the right reason: reordering the two acquisitions in `create-fork.ts` fails the new unit assertion, and the integration suite's negative control still fails in ~1.03s with the server reporting `deadlock detected` rather than a lock timeout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(forks): scope the lock-order barrier to the unlink backend cubic was right that counting any Lock waiter on the database could let an unrelated session release the barrier before the unlink had blocked, leaving the pre-fix negative control to run with its cycle still open. A weak negative control is the failure mode this suite exists to avoid. The unlink transaction now publishes its own backend pid before it can block, and the barrier waits on exactly that pid. If it never blocks the race did not set up, so it throws rather than quietly proceeding and reporting a pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(forks): settle every lock-order barrier on the failure path Both reviewers independently caught the same real defect: a session that died before resolving its barrier left `raceForkAgainstUnlink` awaiting a deferred that would never settle, so a clean database error surfaced as a 30-second vitest timeout. That hides exactly the diagnostics a concurrency test exists to provide. Every barrier is now settled on the failure path as well as the happy one. The fork session releases `forkHoldsFirstLock` in a `finally`, the unlink session reports a `null` backend when it dies before `pg_backend_pid()` returns, and the barrier is skipped outright once a session has already failed, so the session's own error is what the test reports. Releasing the sessions and draining them moved into a `finally` too, along with closing both connections, so a throw inside the barrier cannot strand the fork transaction on `unlinkMayFinish`. Verified by simulating the failure they described - a fork session that throws before taking its first lock. It now fails in 1s with "simulated early connection failure" instead of timing out at 30s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(forks): drop call-count assertions from the sync-default suite CLAUDE.md forbids tests that assert a mock was called. Three assertions in this suite were exactly that shape - `mock.calls).toHaveLength(0)` on the audit and analytics mocks - and they proved only what the mock itself decided, not anything about the use case. The no-op case now asserts the result instead: `projectAudit` maps over `changedWorkspaces` and `afterSuccess` returns early when it is empty, so an empty result IS "no audit, no analytics", pinned to the value the fan-out actually reads. The admission case keeps its rejection assertion, which is the real guarantee: admission runs before the transaction opens. What stays reads the CONTENT of the entries filed - each one carried by its own workspace id and name - which is a fan-out invariant type-check cannot see and the reason this suite exists. Confirmed it still goes red on the pre-fix code: reverting the audit-name guard fails with "expected 'root-ws' to be 'Name of root-ws'". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * revert(forks): drop the "Copy unsynced workflows" fork override Unchecking a workflow meant "keep it out of forking entirely", and the docs said so: never sent, never received, never copied into a new fork. The override made the last of those three conditional on a modal toggle. Sync participation was never at risk - copies inherited the source's flag, so an overridden copy landed unsynced and could not sync back - but "never copied into a new fork" stopped being unconditional, and that is the guarantee someone is relying on when they uncheck a workflow. It also conflated two different states. "Unsynced" covers both "I deliberately excluded this" and "this was never checked because the lineage default is off", and the override copied both. The first case is the one the guarantee exists for. This restores the single predicate: `forkSyncExcluded` workflows are invisible to fork creation, the diff preview, promote in both directions, and the mapping scan, with no caller able to lift it. Removed the toggle and its section, `copyUnsyncedWorkflows` from the request contract, `includeSyncExcluded` from `listDeployedWorkflows` and `loadSourceDeployedStates`, the `unsyncedDeployedWorkflowCount` field, and the split count query, which goes back to one count carrying the exclusion predicate. `lib/limits.ts` goes too. It existed only so a client component could read `MAX_FORK_DEPLOYED_WORKFLOWS` without pulling the database client into the browser bundle, and the override's toggle was the only client reader. The constant returns to `copy/deploy-bridge.ts`, where it is enforced. Unaffected, and still the point of the feature: the lineage-wide new-workflow default, its forward-only write, and the rule that a genuinely new workflow takes the workspace policy while a copy inherits its source. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(forks): stop projecting a column the source query already pins `listDeployedWorkflows` filters on `fork_sync_excluded = false` and then selected the same column, so every row it returned carried `false` by construction - `SELECT x ... WHERE x = false`. The field only ever varied while `includeSyncExcluded` could make that predicate drop out, which the previous commit removed, so it is scaffolding from the reverted override rather than anything load-bearing. Keeping it would not have been defensive either. If someone widens that predicate they have to revisit the projection anyway, and a constant field hides the coupling between the two instead of enforcing it. Dropped from the query and from `DeployedWorkflowSummary`. The two write sites now state the invariant they actually mean: a copy is not a new workflow, so it is written synced and never takes the target workspace's new-workflow default - which in an opt-out lineage would land a deliberately synced workflow unsynced on the other side. That explicit write is kept precisely because it is a semantic claim, not a read of something the query had already decided. Untouched: the target-side read in `promote-plan.ts`, which queries target workflows with no exclusion filter and genuinely varies. That is what keeps a promote from overwriting a target the user unchecked. Dropped the copy test for an unsynced source, an input no caller can now produce, and renamed its sibling to the property that still holds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(knowledge): give the member-tombstone budget test its own timeout `finishes a pass within its page budget` times out against the shared 30s default on a loaded CI runner. It is a load flake, not a regression: the same commit went green on the `push` integration job and timed out on `migrate`, and it runs in ~4s locally. The cost is real work, not a hang. The test seeds `MEMBER_TOMBSTONE_RECONCILE_PAGES_PER_RUN * 500 + 500` documents specifically so one pass cannot finish inside a single run's budget, then observes and re-lists all of it. That volume is the assertion, so trimming it to fit the default would stop proving the multi-run path. Given its own 120s budget instead, matching the per-test timeouts already used elsewhere in this directory, with a comment recording why. Unrelated to this branch's fork-sync work - the file is byte-identical to staging and arrived with the merge - but it was failing this PR's CI, and a flake left alone becomes one everybody learns to ignore. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(forks): render the fork-sync default with ChipSwitch CLAUDE.md makes the chip family the canonical control chrome, and `ChipSwitch` is what the equivalent settings row already uses (`inbox-enable-toggle.tsx`). Two knock-on details, both forced by the control rather than chosen. `ChipSwitch` is a Radix radio group over a string, so the boolean inversion now runs through named values instead of `!`: `exclude` and `sync` map onto the stored `forkSyncNewWorkflowsExcluded`. And it takes no `id`, so the `Label` drops its `htmlFor` and the group carries its own `aria-label` - the same pairing `inbox-enable-toggle.tsx` uses. It also reads better here. This row's "off" means new workflows stop syncing across the whole lineage, which a thumb position leaves the reader to infer from the label; naming both outcomes puts it on screen. No behavior change beyond the control: the same mutation, the same error toast, the same placeholder-data gate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(tables): write the cell state when a resumed run throws runResumeAndCellTerminal only wrote the cell terminal after a resume returned, so a resume that threw left the cell on its last partial running state, where it could not even be cancelled. The resume manager now records, on the error it rethrows, when the attempt kept its pause resumable (admission refused, run buffer unavailable). The resume job mirrors that onto the cell: back to paused when the pause was kept, failed otherwise. A failed cell write is logged without masking the resume error, which is still rethrown. * fix(tables): mirror what a failed resume did instead of inferring it from the error An admission refusal can also mean the execution already finished, so treating every refusal as a kept pause wrote paused over a completed cell. markResumeAttemptFailed and markResumeFailed now return what their transaction actually did (pause still resumable / execution failed), the manager records that outcome on the rethrown error, and the resume job writes paused, error, or nothing. * fix(tables): type the terminal execution log statuses against the persisted vocabulary * fix(tables): report a failed resume's outcome through a hook instead of the error The outcome rode on the thrown error through a module WeakMap, so it was lost when draining queued resumes threw after the settle. startResumeExecution now takes onAttemptFailed, called right after the settle transaction; a hook failure is logged and never replaces the attempt's error.
…failures (#8402) * fix(function): report code-placeholder compiler invariants as server failures Overlapping source edits and exhausted parser-sentinel/Python-marker allocation are broken compiler invariants, not problems in the user's code, but they threw CodePlaceholderCompileError and the Function route answered them with 422 like a user syntax error. They now throw a sibling CodePlaceholderInvariantError, which the route's instanceof check does not match, so it falls through to the existing 500 path. User placeholder and syntax errors keep their 422. * fix(function): keep an exhausted JavaScript sentinel space a user compile error User code can occupy every sentinel a short placeholder could take, so running out of them is the code's fault, not a compiler invariant.
… timing out in CI (#8404)
…nowledge search (#8405) * fix(knowledge): name embedding quota, key, and deadline failures in knowledge search * fix(knowledge): keep keyless Ollama and mixed fallback quota attribution accurate * test(embeddings): prove the quota pause through returned errors, not mock call counts
* fix(logs): keep compacted child span trees shaped as trees Block output compaction spilled oversized child span lists to large-value references, so a span's children could stop being an array and trace span building threw while finalizing the run. That left pauses unpersisted and runs unfinalized. - Compact child span trees structurally, spilling only each span's payload fields - Drop non-list child spans with a warning when building trace spans - Finalize without spans if building them fails, so logs, pauses, and billing settle * fix(logs): keep an output carrying child spans a record when compacting it * fix(logs): split child spans off block output so oversized state still spills * fix(logs): keep a child span tree whole or drop it, bounded as before A structurally compacted tree had no whole-tree bound, so a large one stayed inline in block logs and pause snapshots. A tree still over the threshold after its payloads spill is now dropped (or rejected), as generic compaction bounded it. Block log outputs compact generically again; new logs never carry child spans there. * fix(logs): keep the skeleton of a child span tree too large to keep whole A tree over the threshold as a whole now keeps its shape, names, timing, status, and cost instead of disappearing, using the same content stripping the execution log applies to oversized traces (moved to a shared module). Only a tree whose skeleton is still over the threshold is dropped. * fix(logs): keep span trees well formed and keep nested child workflows in the skeleton The structural walk now keeps only span objects and drops a child list that is not an array, so building a skeleton can never throw on a malformed entry. The skeleton keeps a nested child workflow's output.childTraceSpans. * fix(logs): skip span fields not in their expected shape when summarizing Per-field compaction can spill an oversized modelToolCalls, toolCalls, or providerTiming to a large-value reference; the skeleton now drops such a field instead of reading it.
…neage locking (#8406) * fix(forks): route new workflows through one row builder and harden lineage locking * fix(workflows): read the fork-sync policy inside each create transaction
…unt and cap oversized read limits (#8407) * improvement(search): accept several same-kind native queries per account and cap oversized read limits * fix(search): describe same-kind native queries and test the read cap on the contract * fix(search): test the read cap through tool output and describe kindless native queries * fix(search): state what a kindless GitHub, GitLab, or HubSpot query covers * fix(search): kindless queries fan out across default collections * fix(search): kindless GitHub queries search code only without date bounds or boolean operators * test(search): keep only the refusal the contract still enforces
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 226 files
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
Contributor
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This was referenced Sep 29, 2026
* docs(library): update apache-2-0-vs-fair-code * Pi Babysit: address PR #8412 feedback --------- Co-authored-by: Sim Pi Agent <pi@sim.ai>
* feat(library): Best AI Workflow Builders for Small Teams in 2026 * Pi Babysit: address PR #8413 feedback --------- Co-authored-by: Sim Pi Agent <pi@sim.ai>
…8415) Co-authored-by: Sim Pi Agent <pi@sim.ai>
…gement-in-2026 (#8416) Co-authored-by: Sim Pi Agent <pi@sim.ai>
…nd never fail a completed run (#8417) * fix(tables): settle a resume whose pause cannot be saved as failed, and never fail a completed run A resumed run that paused but whose pause state could not be persisted failed its log yet returned a paused result, so the cell showed paused and the resume entry was marked completed. It now throws after failing the log, so the attempt settles as failed and reports execution_failed. markResumeFailed also rewrote a completed log as failed when a step after a completed run threw. A completed run's outcome now stands, and an already failed log keeps its original end time. * fix(tables): settle the cell as completed when a completed resume's later step throws Leaving a completed log alone reported no outcome, so the cell stayed on its last running state. markResumeFailed now reports what the attempt left the execution as, and a run that completed before a later step threw settles the cell as completed. The pause point is still marked failed as before. * fix(tables): continue the cascade after a completed resume whose later step threw A completed run's cell is completed, so its downstream workflow groups still start; the failure is still rethrown. A pause that cannot be saved now throws a stable message with the underlying error on cause, so API callers never see internal persistence details. * fix(tables): keep server-side logging for a resumed pause that cannot be saved * fix(tables): continue the cascade only once the completed cell is saved * test(tables): assert which downstream groups a resume's cascade started
…_log rows (#8418) * test(forks): assert the sync-default audit fan-out against real audit_log rows * test(forks): match sync-default audit rows by origin and assert the exact member set * test(forks): derive the reverse fan-out expectation independently of its result
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.