Add Flows as first-class Component-owned entities (Slice 1)#41
Merged
Conversation
- Flow and FlowSpec models with OpenAPI parser (Slice 1) - Per-capability addressability and soft-delete granularity - Component detail panel with spec paste and flows palette - ADR-0011 documents ownership, de-dupe pattern, and spec workflow - Includes service layer, tests, and idx_flow_dedup partial index
The React Flow store is seeded once from getCanvas (ADR-0004), so invalidating the query cache after attachFlowSpec doesn't reach the RF store — the pill stayed stale until the next mount. Pass a commitFlowCount callback from the canvas down through the detail panel that updates both the RF store and the cache mirror in lockstep, the same shape commitRename already uses. Verified in-browser: 4-op OpenAPI paste now lights up the pill on the same frame. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAdds Flow and FlowSpec as component-owned domain rows with DB enums/indexes, a bounded OpenAPI FlowSpec parser, server services (attach/add/update/delete/get), TRPC endpoints, canvas UI (detail panel + palette + flow count), and delete/restore cascade support with tests. ChangesFlow Implementation Slice 1
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
src/server/architecture/node.service.ts (1)
619-634: 💤 Low valueFlowSpec conflicts use
conflictingFlowIdsfield.The ConflictError for FlowSpec collisions populates
conflictingFlowIdswith FlowSpec IDs (Line 631). This is semantically inconsistent—clients expecting Flow IDs will receive FlowSpec IDs instead. Consider adding aconflictingFlowSpecIdsfield toConflictErrorDetailsfor clarity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/architecture/node.service.ts` around lines 619 - 634, The ConflictError is currently populated with FlowSpec IDs under the conflictingFlowIds key which is misleading; update the error payload to include a new conflictingFlowSpecIds field (and optionally retain conflictingFlowIds if backward compatibility is required) when throwing the ConflictError in the undo-delete logic that uses stampedFlowSpecs and db.flowSpec.findMany; change the throw in node.service.ts (the ConflictError instantiation) to set conflictingFlowSpecIds: conflicts.map(s => s.id) and update the ConflictErrorDetails/type definition to include conflictingFlowSpecIds so clients receive the correctly named field.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CONTEXT.md`:
- Around line 255-259: Update the "Deletion id" glossary entry to match the
deleteNode/restoreNode cascade: state that a single Deletion id covers the Node
and its subtree, every incident/interior Edge, and every owned Flow and owned
FlowSpec; note these are owner-only write operations (never slug-granted) and
that restoreNode undoes exactly that set (reference deleteNode, restoreNode,
Flow, FlowSpec, Edge, Node, ADR-0002/ADR-0008/ADR-0011 as needed).
In `@docs/plans/flow-routed-connections.md`:
- Around line 291-308: The plan currently reads as pre-implementation even
though "Slice 1 — Flows on Components" has landed; update the document framing
to present Slice 1 as implemented by changing the opening paragraphs to
past-tense/status-complete, remove or relocate the "Open questions (must be
answered before Slice 1)" section (either mark them as resolved or move them to
a new "Open questions for follow-up slices" section), adjust any tense/phrasing
around MCP tools to indicate they are a follow-up (referencing the existing "MCP
tools split off to a follow-up issue" note), and ensure headers like "Slice 1 —
Flows on Components" and "Open questions (must be answered before Slice 1)"
reflect the new status so downstream slices/readers aren’t confused.
In `@prisma/schema.prisma`:
- Around line 236-293: FlowSpec and Flow must use composite relations to enforce
that ownerNodeId and sourceSpecId belong to the same projectId: add a composite
unique on the parent models (Node and FlowSpec) for (projectId, id) and then
change the relations to reference those composite keys — in FlowSpec replace the
ownerNode relation fields from [ownerNodeId] -> [projectId, ownerNodeId] and
references from [id] -> [projectId, id] (relation "FlowSpecOnNode"), and in Flow
replace the sourceSpec relation fields from [sourceSpecId] -> [projectId,
sourceSpecId] and references from [id] -> [projectId, id] (relation
"FlowDerivedFromSpec"); ensure corresponding @@unique([projectId, id]) entries
are added to Node and FlowSpec and update any indexes/migration FKs to use the
new composite foreign keys.
In `@src/app/p/`[slug]/_canvas/canvas.tsx:
- Around line 793-805: The ComponentDetailPanel is rendered whenever canEdit &&
selectedNodeId !== null, but selectedNodeId can point to a deleted node; modify
the logic to first verify the node still exists (e.g., lookup selectedNodeId in
whatever node list/state is used by this canvas) and only render
ComponentDetailPanel when that lookup returns a node, or alternatively clear
selectedNodeId when a node is deleted (ensure the delete handler that removes
nodes also calls closeDetailPanel or sets selectedNodeId = null). Update the
render guard around ComponentDetailPanel (and any callbacks like
commitFlowCount) to reference the node-existence check so stale UI/actions do
not run against a deleted ownerNodeId.
In `@src/app/p/`[slug]/_canvas/component-detail-panel.tsx:
- Around line 45-49: The Escape key handler is only on the panel root and only
fires when focus is inside; add a window-level keydown listener in the component
(useEffect) that calls the existing onClose when e.key === "Escape" and ensure
you remove the listener on cleanup, or alternatively ensure the panel root (the
div with the className and current onKeyDown) gets focused on mount by adding a
ref, tabIndex={-1}, and calling ref.current.focus() in useEffect; reference the
component's onClose prop and the root div element to locate where to apply the
change and keep the existing onKeyDown behavior to preserve intra-panel
handling.
In `@src/server/architecture/flow-parser.ts`:
- Around line 29-30: Update the MAX_DEPTH constant in flow-parser.ts from 32 to
256 to restore the documented 256-level nesting cap (adjust the const MAX_DEPTH
= 32; to MAX_DEPTH = 256), and then update any ADRs, unit/integration tests, and
test assertions that reference or assert the old 32-level limit so they reflect
the new 256 limit; also search for any other code paths or exported symbols that
rely on MAX_DEPTH (or the MAX_OPERATIONS pairing) and ensure their expectations
and docstrings are aligned with the new cap.
- Around line 59-61: The length checks use JS string length (UTF-16 code units)
instead of UTF-8 byte count; update the check in parseFlowSpec (where it
currently does source.length > MAX_FLOW_SPEC_SOURCE_BYTES) to compute UTF-8 byte
length (e.g., Buffer.byteLength(source, 'utf8') or TextEncoder) and compare that
to MAX_FLOW_SPEC_SOURCE_BYTES, and update the attachFlowSpecInput Zod rule
(replace .max(MAX_FLOW_SPEC_SOURCE_BYTES)) with a .refine(...) that validates
UTF-8 byte length against MAX_FLOW_SPEC_SOURCE_BYTES so both parseFlowSpec and
attachFlowSpecInput use the same byte-based limit.
In `@src/server/architecture/flow.service.ts`:
- Around line 184-188: The returned flowCount currently uses parsedFlows.length
which only counts spec-derived Flows and can undercount when hand-authored Flows
were preserved; after reconciliation completes, query the reconciled store for
the actual number of active Flows and return that value instead of
parsedFlows.length (e.g., compute reconciledFlowsCount by calling the
repository/service that lists or counts Flows for this component and use
reconciledFlowsCount in the returned object alongside flowSpec and parseError).
In `@src/server/architecture/node.service.ts`:
- Around line 671-674: The flowSpec.updateMany call lacks the defensive
race-condition handling used for flow.updateMany/Edge updates; wrap the await
db.flowSpec.updateMany({ where: { deletionId }, data: { deletedAt: null,
deletionId: null } }) in a try/catch that catches Prisma
unique-constraint/concurrency errors (PrismaClientKnownRequestError with code
'P2002' or the same error check used by the Edge/Flow blocks) and swallow/ignore
that specific error, rethrowing any other errors so the service does not crash
on a concurrent attach.
---
Nitpick comments:
In `@src/server/architecture/node.service.ts`:
- Around line 619-634: The ConflictError is currently populated with FlowSpec
IDs under the conflictingFlowIds key which is misleading; update the error
payload to include a new conflictingFlowSpecIds field (and optionally retain
conflictingFlowIds if backward compatibility is required) when throwing the
ConflictError in the undo-delete logic that uses stampedFlowSpecs and
db.flowSpec.findMany; change the throw in node.service.ts (the ConflictError
instantiation) to set conflictingFlowSpecIds: conflicts.map(s => s.id) and
update the ConflictErrorDetails/type definition to include
conflictingFlowSpecIds so clients receive the correctly named field.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: affe0aa9-096c-424d-8fcb-1a05045463e8
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
CONTEXT.mddocs/adr/0011-flows-as-first-class-component-owned.mddocs/plans/flow-routed-connections.mdpackage.jsonprisma/migrations/20260529190627_add_flow_models/migration.sqlprisma/schema.prismasrc/app/p/[slug]/_canvas/canvas.tsxsrc/app/p/[slug]/_canvas/component-detail-panel.tsxsrc/app/p/[slug]/_canvas/component-node.tsxsrc/lib/schemas.tssrc/server/api/routers/architecture.tssrc/server/architecture/__tests__/flow-parser.test.tssrc/server/architecture/__tests__/flow.service.test.tssrc/server/architecture/__tests__/helpers/test-db.tssrc/server/architecture/__tests__/node.service.test.tssrc/server/architecture/errors.tssrc/server/architecture/flow-parser.tssrc/server/architecture/flow.service.tssrc/server/architecture/node.service.tssrc/server/architecture/prisma-errors.ts
Comment on lines
+236
to
+293
| model FlowSpec { | ||
| id String @id @default(cuid()) | ||
| projectId String | ||
| project Project @relation(fields: [projectId], references: [id], onDelete: Cascade) | ||
| ownerNodeId String @unique | ||
| ownerNode Node @relation("FlowSpecOnNode", fields: [ownerNodeId], references: [id], onDelete: Cascade) | ||
| kind FlowSpecKind | ||
| source String | ||
| parsedAt DateTime? | ||
| parseError String? | ||
| createdAt DateTime @default(now()) | ||
| updatedAt DateTime @updatedAt | ||
| deletedAt DateTime? | ||
| deletionId String? | ||
| flows Flow[] @relation("FlowDerivedFromSpec") | ||
|
|
||
| @@index([projectId]) | ||
| @@index([deletionId]) | ||
| } | ||
|
|
||
| // A named capability a Component exposes (CONTEXT.md "Flow"; ADR-0011) — an | ||
| // OpenAPI operation, a WS channel, an SSE stream, a function call, an event. | ||
| // Owned by a Component (`ownerNodeId`) and exists on the owner whether or not | ||
| // anything calls it. May be derived from a FlowSpec (`sourceSpecId != null`) | ||
| // or user-authored (`sourceSpecId = null`). `title` is UNTRUSTED display | ||
| // content; `signature` is UNTRUSTED structured content (prompt-injection | ||
| // standing note applies). The de-dupe rule `(ownerNodeId, key)` among active | ||
| // rows follows the ADR-0010 named pattern: service-primary `findFirst` fast | ||
| // path with a partial unique index `idx_flow_dedup` (in prisma/migrations) as | ||
| // a TOCTOU backstop — both translate to the same `ConflictError` shape with | ||
| // `details.conflictingFlowIds`. The partial index expresses | ||
| // `WHERE "deletedAt" IS NULL`, which Prisma schema cannot today; the raw SQL | ||
| // lives in the migration. `NULLS NOT DISTINCT` is intentionally omitted — | ||
| // both columns are NOT NULL, unlike `idx_edge_dedup` where `canvasNodeId` is | ||
| // nullable. | ||
| model Flow { | ||
| id String @id @default(cuid()) | ||
| projectId String | ||
| project Project @relation(fields: [projectId], references: [id], onDelete: Cascade) | ||
| ownerNodeId String | ||
| ownerNode Node @relation("FlowOnNode", fields: [ownerNodeId], references: [id], onDelete: Cascade) | ||
| sourceSpecId String? | ||
| sourceSpec FlowSpec? @relation("FlowDerivedFromSpec", fields: [sourceSpecId], references: [id], onDelete: SetNull) | ||
| kind FlowKind @default(GENERIC) | ||
| key String | ||
| title String | ||
| polarity FlowPolarity | ||
| signature Json? | ||
| createdAt DateTime @default(now()) | ||
| updatedAt DateTime @updatedAt | ||
| deletedAt DateTime? | ||
| deletionId String? | ||
|
|
||
| @@index([projectId, ownerNodeId]) | ||
| @@index([ownerNodeId]) | ||
| @@index([sourceSpecId]) | ||
| @@index([deletionId]) | ||
| } |
There was a problem hiding this comment.
Enforce project consistency on the new Flow foreign keys.
FlowSpec and Flow both store projectId, but the relations only validate each foreign key independently. That means the database will accept rows whose ownerNodeId or sourceSpecId points at an entity from a different project than projectId, which breaks the ownership invariant these models rely on for scoped reads and cascades. Please switch these to composite relations so (projectId, ownerNodeId) and (projectId, sourceSpecId) are validated together, with matching composite uniques on the parent models and migration FKs updated accordingly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@prisma/schema.prisma` around lines 236 - 293, FlowSpec and Flow must use
composite relations to enforce that ownerNodeId and sourceSpecId belong to the
same projectId: add a composite unique on the parent models (Node and FlowSpec)
for (projectId, id) and then change the relations to reference those composite
keys — in FlowSpec replace the ownerNode relation fields from [ownerNodeId] ->
[projectId, ownerNodeId] and references from [id] -> [projectId, id] (relation
"FlowSpecOnNode"), and in Flow replace the sourceSpec relation fields from
[sourceSpecId] -> [projectId, sourceSpecId] and references from [id] ->
[projectId, id] (relation "FlowDerivedFromSpec"); ensure corresponding
@@unique([projectId, id]) entries are added to Node and FlowSpec and update any
indexes/migration FKs to use the new composite foreign keys.
Comment on lines
+29
to
+30
| const MAX_DEPTH = 32; | ||
| const MAX_OPERATIONS = 500; |
There was a problem hiding this comment.
Restore the documented 256-level nesting cap.
The slice contract in the PR context says the bounded loader should allow up to 256 levels, but this hardcodes 32. Any spec nested between 33 and 256 levels will now fail even though it is within the advertised envelope. Please align this constant, plus the matching ADR/test expectations, with the intended cap.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server/architecture/flow-parser.ts` around lines 29 - 30, Update the
MAX_DEPTH constant in flow-parser.ts from 32 to 256 to restore the documented
256-level nesting cap (adjust the const MAX_DEPTH = 32; to MAX_DEPTH = 256), and
then update any ADRs, unit/integration tests, and test assertions that reference
or assert the old 32-level limit so they reflect the new 256 limit; also search
for any other code paths or exported symbols that rely on MAX_DEPTH (or the
MAX_OPERATIONS pairing) and ensure their expectations and docstrings are aligned
with the new cap.
Comment on lines
+671
to
+674
| await db.flowSpec.updateMany({ | ||
| where: { deletionId }, | ||
| data: { deletedAt: null, deletionId: null }, | ||
| }); |
There was a problem hiding this comment.
Missing race fallback for flowSpec.updateMany.
Unlike the flow.updateMany above (Lines 658-669), the flowSpec.updateMany lacks a try/catch for concurrent-write races. If a FlowSpec is attached between the pre-check and this updateMany, Prisma will throw an unhandled unique constraint error. The same defensive pattern used for Edge and Flow applies here.
Proposed fix
- await db.flowSpec.updateMany({
- where: { deletionId },
- data: { deletedAt: null, deletionId: null },
- });
+ try {
+ await db.flowSpec.updateMany({
+ where: { deletionId },
+ data: { deletedAt: null, deletionId: null },
+ });
+ } catch (error) {
+ // FlowSpec's ownerNodeId `@unique` is non-partial, so a collision here
+ // means a concurrent attachFlowSpec created a new FlowSpec on the
+ // same node between the pre-check and this update.
+ throw new ConflictError(
+ "Undo blocked by a concurrent write — retry to see what conflicts.",
+ { conflictingFlowIds: [] },
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| await db.flowSpec.updateMany({ | |
| where: { deletionId }, | |
| data: { deletedAt: null, deletionId: null }, | |
| }); | |
| try { | |
| await db.flowSpec.updateMany({ | |
| where: { deletionId }, | |
| data: { deletedAt: null, deletionId: null }, | |
| }); | |
| } catch (error) { | |
| // FlowSpec's ownerNodeId `@unique` is non-partial, so a collision here | |
| // means a concurrent attachFlowSpec created a new FlowSpec on the | |
| // same node between the pre-check and this update. | |
| throw new ConflictError( | |
| "Undo blocked by a concurrent write — retry to see what conflicts.", | |
| { conflictingFlowIds: [] }, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server/architecture/node.service.ts` around lines 671 - 674, The
flowSpec.updateMany call lacks the defensive race-condition handling used for
flow.updateMany/Edge updates; wrap the await db.flowSpec.updateMany({ where: {
deletionId }, data: { deletedAt: null, deletionId: null } }) in a try/catch that
catches Prisma unique-constraint/concurrency errors
(PrismaClientKnownRequestError with code 'P2002' or the same error check used by
the Edge/Flow blocks) and swallow/ignore that specific error, rethrowing any
other errors so the service does not crash on a concurrent attach.
- Flow pill now includes hand-authored flows (not just spec-derived) - Escape key handler moved to global listener for detail panel - Spec source validated by UTF-8 bytes, not UTF-16 code units - ConflictError field renamed: conflictingFlowIds → conflictingFlowSpecIds - Close detail panel when component is deleted - Update docs to mark Slice 1 shipped
14 tasks
This was referenced May 30, 2026
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
1 participant
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.
Summary
FlowandFlowSpecas first-class rows owned by Components, materializing addressable capabilities (API operations, WS channels, events, etc.) from imported contractsFlowSpecis the 1:1 source of truth on a Component; parsing is server-side, write-time (never read-time); re-paste is non-destructive (soft-deletes orphaned Flows with freshdeletionIdper batch)kind(cosmetic; drives palette icons),key(unique within owner peridx_flow_dedup),polarity(INBOUND/OUTBOUND relative to owner), and optionalsignature(parsed contract fragment as JSON)parseErroruntil their parsers land additivelydeleteNodecascade extended: stamps owned Flows and FlowSpec with samedeletionIdas Node/Edge sweep;restoreNoderestores them togetherP2002on the newidx_flow_deduppartial unique index toConflictErrorwithdetails.conflictingFlowIds(ADR-0010 named pattern, second adopter)sourcestored verbatim, never interpolated (prompt-injection standing note)attach-flow-spec,add-flow,list-flows) deferred to follow-up issue Authenticated MCP route + read resources + llms.txt #18 so Slice 1 ships schema + service + UI cleanlyTesting
pnpm checkpasses (types, linting, format)attachFlowSpeccalls againstidx_flow_deduppartial unique index; soft-delete-then-recreate flow recovery; non-destructive re-parse withdeletionIdbatch isolation;deleteNodecascade sweeps owned Flows and FlowSpec_count.flows > 0pnpm dev: paste valid OpenAPI YAML, see Flows appear and re-persist on re-paste; paste malformed spec, seeparseErrorstored without throw; delete Component, see Flows soft-deleted with samedeletionIdpnpm db:migratesucceeds on clean DB and on repeat (checked viapnpm db:check)Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests