Skip to content

fix(v2): hold create to the rules update enforces, and bind the last two cursors - #6684

Merged
waleedlatif1 merged 1 commit into
stagingfrom
fix/v2-group-create-parity
Aug 14, 2026
Merged

fix(v2): hold create to the rules update enforces, and bind the last two cursors#6684
waleedlatif1 merged 1 commit into
stagingfrom
fix/v2-group-create-parity

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Three defects, one shape: a rule applied to one path and not its sibling. Two were found by probing the live surface after #6681 deployed; the third by reading for the pattern. Each was reproduced before any code changed, and each fix is verified red-then-green.

Create validated almost nothing update validates

createTableGroupUseCase — what POST /api/v2/tables/{tableId}/groups calls — never received the registry checks #6681 added to updateTableGroupUseCase. Confirmed live on a self-created table:

Request Create Update
enrichmentId: "company-info", output outputId: "zz5_nosuch" 201, stored verbatim 400 Enrichment "Company Info" has no output "zz5_nosuch"
enrichmentId: "zz5_no_such_enrichment", output with no outputId 201 400
manual group, fabricated blockId/path 201 400 with a valid-options list

Each 201 stores a column no run can ever write — the runner fills a cell from result[out.outputId] and skips an output with none. The caller discovers it only when a later edit returns the 400 create should have given.

Create now runs the same requireEnrichment / requireKnownEnrichmentOutputIds helpers and the same validateRequestedOutputs check as update. No third validator.

The discriminator is workflowId, not type — load-bearing. workflow-sidebar.tsx creates enrichment-template groups labelled type: 'enrichment' while backed by a real workflowId and carrying no enrichmentId. Keying on the label would have sent that straight into requireEnrichment(undefined) and 400'd the first-party create path. Pinned by a regression test.

A group's producer type could be relabelled into a state create forbids

refineGroupSource enforces "enrichmentId required when type is enrichment" on create; the v2 update body has no such refine, declares type as a "replacement producer type", and declares no enrichmentId — so PATCH {groupId, type:"enrichment"} on a manual group reached the writer, which applies it unconditionally. Unrepairable: the strict body has no field that would make it valid.

The harmful direction is the reverse flip. Relabelling an enrichment group as manual keeps enrichmentId while moving it onto the workflow branch with workflowId === '', so every cell run fails. (The forward flip is survivable — workflowId persists and the runner falls through.) An update may now only restate the type the group already has; re-sending the stored value stays a no-op, so a caller echoing the whole group back is unaffected.

Option (b) — accepting enrichmentId on update and validating the merged state — was rejected: it needs the writer to persist enrichmentId and clear workflowId, i.e. a producer-swap feature nobody asked for.

The last two unbound cursors

GET /workflows/{id}/versions and GET /workspaces/{workspaceId}/members were the only paged reads still minting bare tokens — literally {"version":2} and {"email":"..."} base64'd. Confirmed live: a versions cursor from workflow A is accepted by workflow B, and a hand-crafted {"version":99} is accepted verbatim. The keyset still applies within the authorized parent, so it is a silently-wrong start position, not a leak.

Both now wrap their domain token with the same {scope,inner} binding used everywhere else. No new mechanism.

Guardrail closed too: CURSOR_BINDINGS recorded only query/body params, so [] could not distinguish "no filters" from "parent forgotten" — which is exactly what hid these two. CURSOR_BOUND_PATH_PARAMS now declares the parent path param for all six nested paged lists, with tests that fail actionably if an entry is dropped. The cursor tests were also reworked to mint through the route, so a route that stops binding cannot pass.

In-flight tokens for these two lists are invalidated. They are single-walk and unpersisted, and the refusal message is accurate.

Not fixed, flagged for follow-up

  • The same relabelling hole via workflowId: PATCH {workflowId} on a registry-enrichment group makes it workflow-backed while enrichmentId stays set, so the runner still takes the enrichment branch and ignores the workflow. No first-party caller does this.
  • v2 create refuses a shape the first-party surface produces (type:'enrichment' + workflowId + no enrichmentId), so a public caller cannot create a group the UI can.

Verification

  • Full suite: 1,863 files / 24,939 tests passed, 0 failed
  • type-check 23/23, biome, check:audits 26/26, check:api-validation:strict, check:openapi — all clean
  • One-line OpenAPI description diff regenerated for the type field

…two cursors

Three defects, one shape: a rule applied to one path and not its sibling.
Two were found by probing the live surface after the previous fixes
deployed, and the third by reading for the pattern.

Creating a workflow group through the public surface validated almost
nothing the update path validates. An enrichment group could name an
enrichment the registry does not define, or an output the enrichment does
not have, or carry no output id at all — each a 201 storing a column no
run can ever write, discovered only when the caller later tried to edit
the group and got the 400 create should have given. The workflow half was
the same: a fabricated block-and-path coordinate was stored on create and
refused on update. Create now runs the same two registry helpers and the
same workflow-output check the update path uses.

The discriminator there is the backing workflow id, not the declared
type. The workflow sidebar creates enrichment-template groups labelled
`enrichment` while backed by a real workflow and carrying no enrichment
id, so keying on the label would have refused the first-party create
path outright.

A group's producer type could also be relabelled after the fact into a
state creation refuses. Nothing rejected it and nothing could repair it,
since the update body carries no enrichment id to supply. Relabelling an
enrichment group as workflow-backed is the harmful direction: it keeps
the enrichment id while moving the group onto the workflow branch with an
empty workflow id, so every cell run fails. An update may now only
restate the type the group already has.

The workflow-version and workspace-member lists were the last two paged
reads minting cursors with no route identity, so a token from one parent
resumed another at a position that silently skips rows — the defect the
previous change closed everywhere else. Both now wrap their domain token
with the same scope binding, and the pagination guardrail gained a
declaration of every nested list's parent path param, because the old
one recorded only query filters and so could not tell an unfiltered list
from a forgotten parent.
@vercel

vercel Bot commented Aug 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview Aug 14, 2026 12:01am

Request Review

@cursor

cursor Bot commented Aug 13, 2026

Copy link
Copy Markdown

PR Summary

Cursor Bugbot is generating a summary for commit 717b4bf. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR aligns table-group creation validation with update validation, prevents producer-type relabeling, and scopes the remaining nested-list cursors to their parent resources.

  • Validates workflow and registry-enrichment output coordinates before creating table groups.
  • Rejects table-group updates that attempt to change stored producer provenance.
  • Binds workflow-version and workspace-member pagination cursors to their workflow or workspace.
  • Adds route-level regression tests, pagination audit coverage, and updated OpenAPI documentation.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete changed-code-triggered failures identified.

The new validation and cursor-binding paths match existing contracts and sibling implementations, while the type guard preserves same-type updates and downstream missing-group handling.

Important Files Changed

Filename Overview
apps/sim/lib/table/application/groups.ts Adds create-time output validation and prevents update-time producer-type changes without exposing a concrete regression.
apps/sim/app/api/v2/workflows/[id]/versions/route.ts Consistently binds version cursors to the workflow path parameter when reading and minting tokens.
apps/sim/app/api/v2/workspaces/[workspaceId]/members/route.ts Consistently binds member cursors to the workspace path parameter when reading and minting tokens.
apps/sim/lib/api/contracts/tables.ts Updates the producer-type contract description to reflect the newly enforced immutability.
apps/sim/lib/api/contracts/v2/tests/list-pagination.test.ts Adds an audit guard requiring nested paginated routes to declare all parent path bindings.
apps/sim/lib/table/application/groups.test.ts Covers invalid create coordinates, valid workflow-backed templates, forbidden relabeling, and no-op type echoes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Request[Paginated list request] --> Scope[Derive scope from route and parent ID]
  Scope --> Cursor{Cursor supplied?}
  Cursor -->|No| FirstPage[Load first page]
  Cursor -->|Yes| Verify[Read scoped cursor]
  Verify -->|Scope mismatch or malformed| Reject[Return validation error]
  Verify -->|Valid| Decode[Decode and validate inner keyset]
  Decode --> Load[Load page from keyset]
  FirstPage --> Present[Present results]
  Load --> Present
  Present --> More{More results?}
  More -->|Yes| Mint[Mint cursor with same parent scope]
  More -->|No| Null[Return null nextCursor]
Loading

Reviews (1): Last reviewed commit: "fix(v2): hold create to the rules update..." | Re-trigger Greptile

@waleedlatif1
waleedlatif1 merged commit 9fc6586 into staging Aug 14, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/v2-group-create-parity branch August 14, 2026 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant