feat: workflow persistence closed loop, Dify/n8n import, recursive packages - #12
feat: workflow persistence closed loop, Dify/n8n import, recursive packages#122233admin wants to merge 2 commits into
Conversation
…ckages Workspace -> Project -> WorkflowDraft -> WorkflowVersion persistence with revision-conflict and single-use validation-run guards; Dify/n8n import with a Validation Run publish gate; recursive Package compilation with scoped node ids and a 16-layer nesting limit; WorkspaceSettings model. Compatibility runtime is simulated/fixture-driven validation, not real Dify/n8n worker dispatch.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds workflow authoring storage, services, APIs, validation and publishing flows; supports recursive package compilation with depth limits; extends external workflow imports for Dify and n8n; and adds a workflow lifecycle strip component with regression coverage. ChangesWorkflow authoring lifecycle
Nested package compilation
External runtime import
Workflow lifecycle strip
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant WorkspacesAPI
participant ValidationRunService
participant WorkflowCompiler
participant Database
Client->>WorkspacesAPI: create validation run
WorkspacesAPI->>ValidationRunService: run_validation(draft)
ValidationRunService->>WorkflowCompiler: compile workflow snapshot
ValidationRunService->>Database: store validation result
Client->>WorkspacesAPI: publish draft
WorkspacesAPI->>ValidationRunService: publish_draft(validation_run_id)
ValidationRunService->>Database: insert workflow version
Database-->>Client: published version response
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
⚔️ Resolve merge conflicts
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 |
|
✅ Health: 8.7 📋 At a glance 🚨 Change risk: 9.7/10 (high)
🔎 More signals (1)🔗 Hidden coupling (2 files)
📊 Full report · ⭐ Star Repowise · 📥 Install bot · Last updated 2026-07-16 04:10 UTC |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive workflow authoring closed loop, adding models, schemas, database migrations, and API endpoints for Workspaces, Projects, WorkflowDrafts, WorkflowVersions, and ValidationRuns. It also extends the compiler to support nested package internals up to a maximum depth of 16 and adds support for importing external workflows from Dify and n8n. The review feedback suggests several robustness improvements: adding regex pattern validation to ensure slugs are URL-safe, validating timezones using zoneinfo in the workspace settings schema, and defensively checking for missing projection data during validation runs to prevent potential crashes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| class WorkspaceCreate(BaseModel): | ||
| name: str = Field(..., min_length=1, max_length=255) | ||
| slug: str = Field(..., min_length=1, max_length=255) |
There was a problem hiding this comment.
Add a regex pattern validation to the slug field to ensure it only contains URL-safe characters (alphanumeric, hyphens, and underscores).
| slug: str = Field(..., min_length=1, max_length=255) | |
| slug: str = Field(..., min_length=1, max_length=255, pattern=r"^[a-z0-9-_]+$") |
|
|
||
| class ProjectCreate(BaseModel): | ||
| name: str = Field(..., min_length=1, max_length=255) | ||
| slug: str = Field(..., min_length=1, max_length=255) |
There was a problem hiding this comment.
Add a regex pattern validation to the slug field to ensure it only contains URL-safe characters (alphanumeric, hyphens, and underscores).
| slug: str = Field(..., min_length=1, max_length=255) | |
| slug: str = Field(..., min_length=1, max_length=255, pattern=r"^[a-z0-9-_]+$") |
| class WorkspaceSettingsUpdate(BaseModel): | ||
| timezone: Optional[str] = None | ||
| deterministic_simulation: Optional[bool] = None | ||
| max_items_per_run: Optional[int] = Field(None, gt=0) |
There was a problem hiding this comment.
Validate that the provided timezone is a valid timezone name using zoneinfo from the standard library to prevent runtime errors during scheduling or execution. Note that you will need to import field_validator from pydantic at the top of the file.
| class WorkspaceSettingsUpdate(BaseModel): | |
| timezone: Optional[str] = None | |
| deterministic_simulation: Optional[bool] = None | |
| max_items_per_run: Optional[int] = Field(None, gt=0) | |
| class WorkspaceSettingsUpdate(BaseModel): | |
| timezone: Optional[str] = None | |
| deterministic_simulation: Optional[bool] = None | |
| max_items_per_run: Optional[int] = Field(None, gt=0) | |
| @field_validator("timezone") | |
| @classmethod | |
| def validate_timezone(cls, v: Optional[str]) -> Optional[str]: | |
| if v is not None: | |
| from zoneinfo import ZoneInfo, ZoneInfoNotFoundError | |
| try: | |
| ZoneInfo(v) | |
| except ZoneInfoNotFoundError: | |
| raise ValueError(f"Invalid timezone: {v}") | |
| return v |
| projection = await start_workflow_run( | ||
| WorkflowRunStartRequest(project=project), session=session | ||
| ) | ||
| validation_run.run_id = projection.runId |
There was a problem hiding this comment.
Enforce defensive programming by checking if projection is None or if projection.runId is missing before accessing its attributes. This prevents potential AttributeError crashes if the workflow run fails to start.
projection = await start_workflow_run(
WorkflowRunStartRequest(project=project), session=session
)
if not projection or not projection.runId:
validation_run.status = "failed"
validation_run.failure_reason = "execution_failed"
await session.flush()
await session.refresh(validation_run)
return validation_run
validation_run.run_id = projection.runId![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/api/v1/workspaces.py (1)
1-283: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd
PUT /workspaces/{workspace_id}—WorkspaceUpdateandauthoring_service.update_workspace()exist, but this router never exposes a workspace update endpoint. If workspace edits are supported, wire this route to the service; otherwise remove the unused schema/service to avoid a misleading API contract.🤖 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 `@backend/api/v1/workspaces.py` around lines 1 - 283, Add a PUT /workspaces/{workspace_id} handler alongside create_workspace and get_workspace, accepting schemas.WorkspaceUpdate, loading the workspace via authoring_service.get_workspace, returning 404 when absent, applying authoring_service.update_workspace, committing and refreshing it, and returning WorkspaceRead. If workspace updates are intentionally unsupported instead, remove the unused WorkspaceUpdate schema and authoring_service.update_workspace implementation.
🧹 Nitpick comments (4)
backend/workflow/compiler.py (1)
513-618: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueMove the nesting guard into
_expand_package_internals
_validate_package_internals()enforcesMAX_PACKAGE_NESTING_DEPTHbeforecompile_workflow_project()reaches this helper, so the recursion is safe here. Add the same check locally if this helper may be reused from another entrypoint.🤖 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 `@backend/workflow/compiler.py` around lines 513 - 618, Add a local MAX_PACKAGE_NESTING_DEPTH guard at the start of _expand_package_internals before recursing into nested internals, so the helper remains safe when called independently of _validate_package_internals. Preserve the existing expansion behavior within the allowed depth and stop or reject deeper nesting using the helper’s established contract.backend/schemas/workflow_authoring.py (3)
15-18: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd slug format validation.
WorkspaceCreate.slugandProjectCreate.slugaccept any string up to 255 chars with no format constraint. Since slugs are typically used in URLs/lookups, unconstrained values (spaces, uppercase, special chars) can produce malformed routes or inconsistent lookups.♻️ Suggested constraint
+from pydantic import field_validator +import re + +_SLUG_RE = re.compile(r"^[a-z0-9]+(?:-[a-z0-9]+)*$") + class WorkspaceCreate(BaseModel): name: str = Field(..., min_length=1, max_length=255) - slug: str = Field(..., min_length=1, max_length=255) + slug: str = Field(..., min_length=1, max_length=255, pattern=_SLUG_RE.pattern) description: Optional[str] = NoneApply similarly to
ProjectCreate.slug.Also applies to: 55-57
🤖 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 `@backend/schemas/workflow_authoring.py` around lines 15 - 18, Update the slug fields in WorkspaceCreate and ProjectCreate to enforce a URL-safe slug format, allowing only lowercase letters, numbers, and single hyphen separators while preserving the existing length limits and required validation.
34-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider
ConfigDictinstead of a plain dict formodel_config.Ruff's RUF012 flags these six
model_config = {"from_attributes": True}lines as mutable class defaults. This is a well-known pydantic-v2 pattern (the value is popped by the model metaclass, not a shared instance default), but usingConfigDict(from_attributes=True)is the idiomatic pydantic v2 form and silences the lint warning with typed config.
[dependency_check]
- Flagging: pydantic v2
model_configconvention vs Ruff RUF012.- Action: appending since this concerns library/linter interaction.
Also applies to: 52-52, 68-68, 90-90, 114-114, 133-133
🤖 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 `@backend/schemas/workflow_authoring.py` at line 34, Replace the plain dictionary assigned to each of the six model_config class attributes with the typed pydantic v2 ConfigDict form, preserving from_attributes=True. Update the relevant schema definitions in workflow_authoring.py and add or reuse the appropriate ConfigDict import.Source: Linters/SAST tools
37-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTimezone validation gap and casing inconsistency with
WorkflowSettings.
WorkspaceSettingsUpdate.timezone/WorkspaceSettingsRead.timezoneaccept arbitrary strings with no validation against real IANA timezones (e.g. viazoneinfo.available_timezones()), risking bad data reaching scheduling logic downstream.Separately, these fields use snake_case (
deterministic_simulation,max_items_per_run) while the semantically equivalentWorkflowSettingsembedded inWorkflowProject.settingsuses camelCase (deterministicSimulation,maxItemsPerRun) perbackend/schemas/workflow.py:185-202. SinceWorkflowDraftRead.snapshot(camelCase settings) andWorkspaceSettingsRead(snake_case) can appear side-by-side in the same API surface, this inconsistency is confusing for API consumers.🤖 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 `@backend/schemas/workflow_authoring.py` around lines 37 - 53, Update WorkspaceSettingsUpdate and WorkspaceSettingsRead to validate timezone values against real IANA timezones, reusing the project’s established timezone-validation approach or zoneinfo.available_timezones(). Align deterministic_simulation and max_items_per_run with WorkflowSettings by exposing the camelCase names deterministicSimulation and maxItemsPerRun while preserving their existing optionality, defaults, and positive-value validation.
🤖 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 `@backend/api/v1/workspaces.py`:
- Around line 55-83: Update update_workspace_settings to catch IntegrityError
around db.commit(), roll back the transaction on failure, and raise
HTTPException with status 422 consistent with other create/update paths; keep
the successful refresh and response flow unchanged.
In `@backend/models/validation_run.py`:
- Around line 1-30: Add a TYPE_CHECKING-only import for WorkflowVersion in
validation_run.py so the forward reference on ValidationRun.version resolves for
static analysis without introducing a runtime import cycle.
In `@backend/models/workflow_authoring.py`:
- Around line 95-122: Add a TYPE_CHECKING-guarded import for ValidationRun in
the module containing WorkflowVersion, keeping it excluded at runtime while
satisfying the forward reference used by validation_run:
Mapped["ValidationRun"].
In `@backend/schemas/workflow.py`:
- Line 242: Update the runtime union used by importExternalRuntimeWorkflow in
frontend/lib/workflow/backend-import.ts to include "dify" and "n8n" alongside
the existing "langgraph" and "langchain" values, keeping it synchronized with
ExternalWorkflowRuntime.
In `@backend/services/validation_run_service.py`:
- Around line 80-86: Update the validation-run workflow in the surrounding
service method to catch failures from start_workflow_run and
list_workflow_run_events, persist the ValidationRun as failed with an
appropriate failure_reason, and return the failed result instead of allowing the
exception to become a 500. Preserve the existing successful event-processing
path and ensure the failure update is committed before returning.
- Around line 160-178: Update publish_draft’s version allocation around the
WorkflowVersion query so concurrent publishes for the same project are
serialized before calculating MAX(version_number)+1. Lock the associated project
row, or use an equivalent atomic allocation mechanism, then insert the
WorkflowVersion while preserving the existing unique constraint and return flow.
In `@backend/services/workflow_authoring_service.py`:
- Around line 122-134: The update_draft function currently performs a non-atomic
Python-side revision check, allowing concurrent updates to overwrite each other.
Make the revision guard part of the database update or configure SQLAlchemy
optimistic locking for WorkflowDraft, ensuring stale writes fail with
DraftRevisionConflictError and only the row matching data.expected_revision is
updated.
- Around line 47-75: Update update_workspace and update_workspace_settings to
exclude or reject client-supplied None values before applying fields, while
preserving valid non-null updates. Ensure Workspace.name and non-null
WorkspaceSettings fields cannot be assigned None before session.flush().
In `@backend/workflow/external_importer.py`:
- Around line 234-269: Preserve n8n branch provenance by adding each
connection’s output-slot index to the edge payload produced by
_extract_n8n_connection_edges(), then update import_external_workflow() and the
ui.externalWorkflow conversion to retain and use both sourcePort and the branch
index instead of rebuilding edges from node names alone. Ensure IF/Switch true
and false branches remain distinguishable through import and round-trip export.
---
Outside diff comments:
In `@backend/api/v1/workspaces.py`:
- Around line 1-283: Add a PUT /workspaces/{workspace_id} handler alongside
create_workspace and get_workspace, accepting schemas.WorkspaceUpdate, loading
the workspace via authoring_service.get_workspace, returning 404 when absent,
applying authoring_service.update_workspace, committing and refreshing it, and
returning WorkspaceRead. If workspace updates are intentionally unsupported
instead, remove the unused WorkspaceUpdate schema and
authoring_service.update_workspace implementation.
---
Nitpick comments:
In `@backend/schemas/workflow_authoring.py`:
- Around line 15-18: Update the slug fields in WorkspaceCreate and ProjectCreate
to enforce a URL-safe slug format, allowing only lowercase letters, numbers, and
single hyphen separators while preserving the existing length limits and
required validation.
- Line 34: Replace the plain dictionary assigned to each of the six model_config
class attributes with the typed pydantic v2 ConfigDict form, preserving
from_attributes=True. Update the relevant schema definitions in
workflow_authoring.py and add or reuse the appropriate ConfigDict import.
- Around line 37-53: Update WorkspaceSettingsUpdate and WorkspaceSettingsRead to
validate timezone values against real IANA timezones, reusing the project’s
established timezone-validation approach or zoneinfo.available_timezones().
Align deterministic_simulation and max_items_per_run with WorkflowSettings by
exposing the camelCase names deterministicSimulation and maxItemsPerRun while
preserving their existing optionality, defaults, and positive-value validation.
In `@backend/workflow/compiler.py`:
- Around line 513-618: Add a local MAX_PACKAGE_NESTING_DEPTH guard at the start
of _expand_package_internals before recursing into nested internals, so the
helper remains safe when called independently of _validate_package_internals.
Preserve the existing expansion behavior within the allowed depth and stop or
reject deeper nesting using the helper’s established contract.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6a483e32-ec88-489f-8aa9-f6b737ded2ad
📒 Files selected for processing (16)
backend/api/v1/__init__.pybackend/api/v1/workspaces.pybackend/migrations/versions/r5s6t7u8v9w0_add_workflow_authoring_tables.pybackend/models/__init__.pybackend/models/validation_run.pybackend/models/workflow_authoring.pybackend/schemas/workflow.pybackend/schemas/workflow_authoring.pybackend/services/validation_run_service.pybackend/services/workflow_authoring_service.pybackend/workflow/compiler.pybackend/workflow/conformance/contracts.pybackend/workflow/external_importer.pytests/integration/test_workflow_authoring_api.pytests/integration/test_workflow_compile_api.pytests/integration/test_workflow_external_import_api.py
| @router.get( | ||
| "/workspaces/{workspace_id}/settings", | ||
| response_model=ApiResponse[schemas.WorkspaceSettingsRead], | ||
| ) | ||
| async def get_workspace_settings( | ||
| workspace_id: str, db: AsyncSession = Depends(get_db) | ||
| ) -> ApiResponse[schemas.WorkspaceSettingsRead]: | ||
| settings = await authoring_service.get_workspace_settings(db, workspace_id) | ||
| if settings is None: | ||
| raise HTTPException(status_code=404, detail="Workspace not found") | ||
| return ApiResponse.ok(schemas.WorkspaceSettingsRead.model_validate(settings)) | ||
|
|
||
|
|
||
| @router.put( | ||
| "/workspaces/{workspace_id}/settings", | ||
| response_model=ApiResponse[schemas.WorkspaceSettingsRead], | ||
| ) | ||
| async def update_workspace_settings( | ||
| workspace_id: str, | ||
| body: schemas.WorkspaceSettingsUpdate, | ||
| db: AsyncSession = Depends(get_db), | ||
| ) -> ApiResponse[schemas.WorkspaceSettingsRead]: | ||
| settings = await authoring_service.get_workspace_settings(db, workspace_id) | ||
| if settings is None: | ||
| raise HTTPException(status_code=404, detail="Workspace not found") | ||
| settings = await authoring_service.update_workspace_settings(db, settings, body) | ||
| await db.commit() | ||
| await db.refresh(settings) | ||
| return ApiResponse.ok(schemas.WorkspaceSettingsRead.model_validate(settings)) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
cat backend/migrations/versions/r5s6t7u8v9w0_add_workflow_authoring_tables.py 2>/dev/null | rg -n "workspace_settings|insert" -A3Repository: 2233admin/opencli-admin
Length of output: 880
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== workspaces api ==\n'
sed -n '1,180p' backend/api/v1/workspaces.py
printf '\n== workflow authoring service ==\n'
sed -n '1,260p' backend/services/workflow_authoring_service.py
printf '\n== migrations mentioning workspace_settings or backfill ==\n'
rg -n "workspace_settings|backfill|timezone|create_workspace" backend/migrations backend/services backend/api -SRepository: 2233admin/opencli-admin
Length of output: 27936
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== schemas/workflow_authoring.py ==\n'
sed -n '1,260p' backend/schemas/workflow_authoring.py
printf '\n== models/workflow_authoring.py ==\n'
sed -n '1,260p' backend/models/workflow_authoring.py
printf '\n== initial schema around workspace tables ==\n'
sed -n '1,170p' backend/migrations/versions/5a9a94795d00_initial_schema.py
printf '\n== workflow authoring migration around workspace settings ==\n'
sed -n '1,120p' backend/migrations/versions/r5s6t7u8v9w0_add_workflow_authoring_tables.py
printf '\n== search for global IntegrityError handling ==\n'
rg -n "IntegrityError|HTTPException\\(status_code=422|status_code=409" backend -SRepository: 2233admin/opencli-admin
Length of output: 25616
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== workspaces api around later handlers ==\n'
sed -n '180,280p' backend/api/v1/workspaces.py
printf '\n== search migrations for any earlier workspaces/workspace_settings definitions ==\n'
rg -n "op\.create_table\(\s*['\"]workspaces['\"]|op\.create_table\(\s*['\"]workspace_settings['\"]|workspace_settings" backend/migrations/versions -S
printf '\n== inspect migration order / filenames ==\n'
ls backend/migrations/versions | sortRepository: 2233admin/opencli-admin
Length of output: 6515
Handle IntegrityError in workspace settings updates
WorkspaceSettingsUpdate.timezone allows null, but workspace_settings.timezone is non-nullable, so this endpoint can raise an unhandled 500 on bad input. Wrap the commit/rollback and return 422 like the other create/update paths.
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 60-60: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
[warning] 75-75: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🤖 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 `@backend/api/v1/workspaces.py` around lines 55 - 83, Update
update_workspace_settings to catch IntegrityError around db.commit(), roll back
the transaction on failure, and raise HTTPException with status 422 consistent
with other create/update paths; keep the successful refresh and response flow
unchanged.
| from sqlalchemy import JSON, Boolean, ForeignKey, Integer, String | ||
| from sqlalchemy.orm import Mapped, mapped_column, relationship | ||
|
|
||
| from backend.models.base import TimestampMixin | ||
|
|
||
|
|
||
| class ValidationRun(TimestampMixin): | ||
| """A single compile+conformance validation attempt gating draft publish.""" | ||
|
|
||
| __tablename__ = "validation_runs" | ||
|
|
||
| draft_id: Mapped[str] = mapped_column( | ||
| String(36), ForeignKey("workflow_drafts.id", ondelete="CASCADE"), nullable=False, index=True | ||
| ) | ||
| draft_revision: Mapped[int] = mapped_column(Integer, nullable=False) | ||
| status: Mapped[str] = mapped_column(String(50), nullable=False, default="pending") | ||
| compile_valid: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) | ||
| compile_errors: Mapped[list | None] = mapped_column(JSON, nullable=True) | ||
| conformance_mode: Mapped[str] = mapped_column(String(50), nullable=False, default="passthrough") | ||
| expected_events: Mapped[list | None] = mapped_column(JSON, nullable=True) | ||
| conformance_result: Mapped[dict | None] = mapped_column(JSON, nullable=True) | ||
| runtime_passport: Mapped[dict | None] = mapped_column(JSON, nullable=True) | ||
| run_id: Mapped[str | None] = mapped_column( | ||
| String(36), ForeignKey("workflow_runs.id", ondelete="SET NULL"), nullable=True | ||
| ) | ||
| failure_reason: Mapped[str | None] = mapped_column(String(2000), nullable=True) | ||
|
|
||
| version: Mapped["WorkflowVersion | None"] = relationship( | ||
| "WorkflowVersion", back_populates="validation_run", uselist=False | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add TYPE_CHECKING import for WorkflowVersion forward reference.
Same issue as in workflow_authoring.py: Line 28 references Mapped["WorkflowVersion | None"] without importing WorkflowVersion, triggering Ruff F821.
🔧 Proposed fix
+from typing import TYPE_CHECKING
+
from sqlalchemy import JSON, Boolean, ForeignKey, Integer, String
from sqlalchemy.orm import Mapped, mapped_column, relationship
from backend.models.base import TimestampMixin
+
+if TYPE_CHECKING:
+ from backend.models.workflow_authoring import WorkflowVersion📝 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.
| from sqlalchemy import JSON, Boolean, ForeignKey, Integer, String | |
| from sqlalchemy.orm import Mapped, mapped_column, relationship | |
| from backend.models.base import TimestampMixin | |
| class ValidationRun(TimestampMixin): | |
| """A single compile+conformance validation attempt gating draft publish.""" | |
| __tablename__ = "validation_runs" | |
| draft_id: Mapped[str] = mapped_column( | |
| String(36), ForeignKey("workflow_drafts.id", ondelete="CASCADE"), nullable=False, index=True | |
| ) | |
| draft_revision: Mapped[int] = mapped_column(Integer, nullable=False) | |
| status: Mapped[str] = mapped_column(String(50), nullable=False, default="pending") | |
| compile_valid: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) | |
| compile_errors: Mapped[list | None] = mapped_column(JSON, nullable=True) | |
| conformance_mode: Mapped[str] = mapped_column(String(50), nullable=False, default="passthrough") | |
| expected_events: Mapped[list | None] = mapped_column(JSON, nullable=True) | |
| conformance_result: Mapped[dict | None] = mapped_column(JSON, nullable=True) | |
| runtime_passport: Mapped[dict | None] = mapped_column(JSON, nullable=True) | |
| run_id: Mapped[str | None] = mapped_column( | |
| String(36), ForeignKey("workflow_runs.id", ondelete="SET NULL"), nullable=True | |
| ) | |
| failure_reason: Mapped[str | None] = mapped_column(String(2000), nullable=True) | |
| version: Mapped["WorkflowVersion | None"] = relationship( | |
| "WorkflowVersion", back_populates="validation_run", uselist=False | |
| ) | |
| from typing import TYPE_CHECKING | |
| from sqlalchemy import JSON, Boolean, ForeignKey, Integer, String | |
| from sqlalchemy.orm import Mapped, mapped_column, relationship | |
| from backend.models.base import TimestampMixin | |
| if TYPE_CHECKING: | |
| from backend.models.workflow_authoring import WorkflowVersion | |
| class ValidationRun(TimestampMixin): | |
| """A single compile+conformance validation attempt gating draft publish.""" | |
| __tablename__ = "validation_runs" | |
| draft_id: Mapped[str] = mapped_column( | |
| String(36), ForeignKey("workflow_drafts.id", ondelete="CASCADE"), nullable=False, index=True | |
| ) | |
| draft_revision: Mapped[int] = mapped_column(Integer, nullable=False) | |
| status: Mapped[str] = mapped_column(String(50), nullable=False, default="pending") | |
| compile_valid: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False) | |
| compile_errors: Mapped[list | None] = mapped_column(JSON, nullable=True) | |
| conformance_mode: Mapped[str] = mapped_column(String(50), nullable=False, default="passthrough") | |
| expected_events: Mapped[list | None] = mapped_column(JSON, nullable=True) | |
| conformance_result: Mapped[dict | None] = mapped_column(JSON, nullable=True) | |
| runtime_passport: Mapped[dict | None] = mapped_column(JSON, nullable=True) | |
| run_id: Mapped[str | None] = mapped_column( | |
| String(36), ForeignKey("workflow_runs.id", ondelete="SET NULL"), nullable=True | |
| ) | |
| failure_reason: Mapped[str | None] = mapped_column(String(2000), nullable=True) | |
| version: Mapped["WorkflowVersion | None"] = relationship( | |
| "WorkflowVersion", back_populates="validation_run", uselist=False | |
| ) |
🧰 Tools
🪛 Ruff (0.15.20)
[error] 28-28: Undefined name WorkflowVersion
(F821)
🤖 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 `@backend/models/validation_run.py` around lines 1 - 30, Add a
TYPE_CHECKING-only import for WorkflowVersion in validation_run.py so the
forward reference on ValidationRun.version resolves for static analysis without
introducing a runtime import cycle.
Source: Linters/SAST tools
| class WorkflowVersion(TimestampMixin): | ||
| """An immutable, published WorkflowProject snapshot.""" | ||
|
|
||
| __tablename__ = "workflow_versions" | ||
| __table_args__ = ( | ||
| UniqueConstraint("project_id", "version_number", name="uq_workflow_versions_project_number"), | ||
| ) | ||
|
|
||
| project_id: Mapped[str] = mapped_column( | ||
| String(36), ForeignKey("projects.id", ondelete="CASCADE"), nullable=False, index=True | ||
| ) | ||
| draft_id: Mapped[str | None] = mapped_column( | ||
| String(36), ForeignKey("workflow_drafts.id", ondelete="SET NULL"), nullable=True, index=True | ||
| ) | ||
| version_number: Mapped[int] = mapped_column(Integer, nullable=False) | ||
| source_revision: Mapped[int] = mapped_column(Integer, nullable=False) | ||
| validation_run_id: Mapped[str] = mapped_column( | ||
| String(36), | ||
| ForeignKey("validation_runs.id", ondelete="RESTRICT"), | ||
| nullable=False, | ||
| unique=True, | ||
| index=True, | ||
| ) | ||
| snapshot: Mapped[dict] = mapped_column(JSON, nullable=False, default=dict) | ||
|
|
||
| project: Mapped["Project"] = relationship("Project", back_populates="versions") | ||
| draft: Mapped["WorkflowDraft | None"] = relationship("WorkflowDraft", back_populates="versions") | ||
| validation_run: Mapped["ValidationRun"] = relationship("ValidationRun", back_populates="version") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add TYPE_CHECKING import for cross-module forward reference.
Line 122 uses Mapped["ValidationRun"] but ValidationRun is never imported in this module, which is exactly what Ruff's F821 hint flags. This is a documented SQLAlchemy/Ruff friction point — SQLA suggests: Mapped["Parent"] (assuming Parent model is in another module not yet imported). Ruff conflicts with rules UP037 and F821. The fix confirmed by SQLAlchemy usage guidance is our forward ref to "User" will be fine as long as you've imported User via TYPE_CHECKING.
🔧 Proposed fix
+from typing import TYPE_CHECKING
+
from sqlalchemy import JSON, Boolean, ForeignKey, Integer, String, UniqueConstraint
from sqlalchemy.orm import Mapped, mapped_column, relationship
from backend.models.base import TimestampMixin
+
+if TYPE_CHECKING:
+ from backend.models.validation_run import ValidationRun📝 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.
| class WorkflowVersion(TimestampMixin): | |
| """An immutable, published WorkflowProject snapshot.""" | |
| __tablename__ = "workflow_versions" | |
| __table_args__ = ( | |
| UniqueConstraint("project_id", "version_number", name="uq_workflow_versions_project_number"), | |
| ) | |
| project_id: Mapped[str] = mapped_column( | |
| String(36), ForeignKey("projects.id", ondelete="CASCADE"), nullable=False, index=True | |
| ) | |
| draft_id: Mapped[str | None] = mapped_column( | |
| String(36), ForeignKey("workflow_drafts.id", ondelete="SET NULL"), nullable=True, index=True | |
| ) | |
| version_number: Mapped[int] = mapped_column(Integer, nullable=False) | |
| source_revision: Mapped[int] = mapped_column(Integer, nullable=False) | |
| validation_run_id: Mapped[str] = mapped_column( | |
| String(36), | |
| ForeignKey("validation_runs.id", ondelete="RESTRICT"), | |
| nullable=False, | |
| unique=True, | |
| index=True, | |
| ) | |
| snapshot: Mapped[dict] = mapped_column(JSON, nullable=False, default=dict) | |
| project: Mapped["Project"] = relationship("Project", back_populates="versions") | |
| draft: Mapped["WorkflowDraft | None"] = relationship("WorkflowDraft", back_populates="versions") | |
| validation_run: Mapped["ValidationRun"] = relationship("ValidationRun", back_populates="version") | |
| from typing import TYPE_CHECKING | |
| from sqlalchemy import JSON, Boolean, ForeignKey, Integer, String, UniqueConstraint | |
| from sqlalchemy.orm import Mapped, mapped_column, relationship | |
| from backend.models.base import TimestampMixin | |
| if TYPE_CHECKING: | |
| from backend.models.validation_run import ValidationRun | |
| class WorkflowVersion(TimestampMixin): | |
| """An immutable, published WorkflowProject snapshot.""" | |
| __tablename__ = "workflow_versions" | |
| __table_args__ = ( | |
| UniqueConstraint("project_id", "version_number", name="uq_workflow_versions_project_number"), | |
| ) | |
| project_id: Mapped[str] = mapped_column( | |
| String(36), ForeignKey("projects.id", ondelete="CASCADE"), nullable=False, index=True | |
| ) | |
| draft_id: Mapped[str | None] = mapped_column( | |
| String(36), ForeignKey("workflow_drafts.id", ondelete="SET NULL"), nullable=True, index=True | |
| ) | |
| version_number: Mapped[int] = mapped_column(Integer, nullable=False) | |
| source_revision: Mapped[int] = mapped_column(Integer, nullable=False) | |
| validation_run_id: Mapped[str] = mapped_column( | |
| String(36), | |
| ForeignKey("validation_runs.id", ondelete="RESTRICT"), | |
| nullable=False, | |
| unique=True, | |
| index=True, | |
| ) | |
| snapshot: Mapped[dict] = mapped_column(JSON, nullable=False, default=dict) | |
| project: Mapped["Project"] = relationship("Project", back_populates="versions") | |
| draft: Mapped["WorkflowDraft | None"] = relationship("WorkflowDraft", back_populates="versions") | |
| validation_run: Mapped["ValidationRun"] = relationship("ValidationRun", back_populates="version") |
🧰 Tools
🪛 Ruff (0.15.20)
[error] 122-122: Undefined name ValidationRun
(F821)
🤖 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 `@backend/models/workflow_authoring.py` around lines 95 - 122, Add a
TYPE_CHECKING-guarded import for ValidationRun in the module containing
WorkflowVersion, keeping it excluded at runtime while satisfying the forward
reference used by validation_run: Mapped["ValidationRun"].
Source: Linters/SAST tools
|
|
||
|
|
||
| ExternalWorkflowRuntime = Literal["langgraph", "langchain"] | ||
| ExternalWorkflowRuntime = Literal["langgraph", "langchain", "dify", "n8n"] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files mentioning ExternalWorkflowRuntime ==\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' 'ExternalWorkflowRuntime' .
printf '\n== backend/schemas/workflow.py around the literal ==\n'
sed -n '220,260p' backend/schemas/workflow.py
printf '\n== frontend/lib/workflow/backend-import.ts ==\n'
sed -n '1,220p' frontend/lib/workflow/backend-import.tsRepository: 2233admin/opencli-admin
Length of output: 3767
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== backend/schemas/workflow.py ==\n'
grep -n 'ExternalWorkflowRuntime' -n backend/schemas/workflow.py || true
sed -n '236,248p' backend/schemas/workflow.py
printf '\n== frontend/lib/workflow/backend-import.ts ==\n'
grep -n 'ExternalWorkflowRuntime' -n frontend/lib/workflow/backend-import.ts || true
sed -n '1,80p' frontend/lib/workflow/backend-import.ts
printf '\n== any other frontend declarations ==\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' 'langgraph"\s*\|\s*"langchain"|langgraph"|langchain"|dify"|n8n"' frontend backendRepository: 2233admin/opencli-admin
Length of output: 8692
Expand the frontend runtime union frontend/lib/workflow/backend-import.ts still only allows "langgraph" | "langchain", so importExternalRuntimeWorkflow() can’t accept "dify" or "n8n". Keep it in sync with backend/schemas/workflow.py.
🤖 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 `@backend/schemas/workflow.py` at line 242, Update the runtime union used by
importExternalRuntimeWorkflow in frontend/lib/workflow/backend-import.ts to
include "dify" and "n8n" alongside the existing "langgraph" and "langchain"
values, keeping it synchronized with ExternalWorkflowRuntime.
| projection = await start_workflow_run( | ||
| WorkflowRunStartRequest(project=project), session=session | ||
| ) | ||
| validation_run.run_id = projection.runId | ||
|
|
||
| events = await list_workflow_run_events(projection.runId, session=session) or [] | ||
| actual_events = [event.model_dump(mode="json") for event in events] |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## validation_run_service.py\n'
nl -ba backend/services/validation_run_service.py | sed -n '1,180p'
printf '\n## workspaces.py\n'
nl -ba backend/api/workspaces.py | sed -n '1,240p'
printf '\n## get_db / session helper search\n'
rg -n "def get_db|async def get_db|sessionmaker|yield .*session|rollback|commit|ValidationRunRequestError" backend -SRepository: 2233admin/opencli-admin
Length of output: 231
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
def show(path, start, end):
print(f"\n## {path}")
for i, line in enumerate(Path(path).read_text().splitlines(), 1):
if start <= i <= end:
print(f"{i:4d}: {line}")
show("backend/services/validation_run_service.py", 1, 220)
show("backend/api/workspaces.py", 1, 280)
PY
printf '\n## get_db / session helper search\n'
rg -n "def get_db|async def get_db|sessionmaker|yield .*session|rollback|commit|ValidationRunRequestError" backend -SRepository: 2233admin/opencli-admin
Length of output: 8492
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## create_validation_run search\n'
rg -n "create_validation_run|ValidationRunRequestError|run_validation\(" backend -S
printf '\n## get_db search\n'
rg -n "def get_db|async def get_db|sessionmaker|rollback|commit|yield .*session" backend -S
printf '\n## likely workspace/api files\n'
fd -a -t f 'workspaces.py' backend . || true
fd -a -t f 'workspace*.py' backend . || true
fd -a -t f '*workspace*' backend . || trueRepository: 2233admin/opencli-admin
Length of output: 15981
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
def show(path, start, end):
print(f"\n## {path}")
for i, line in enumerate(Path(path).read_text().splitlines(), 1):
if start <= i <= end:
print(f"{i:4d}: {line}")
show("backend/api/v1/workspaces.py", 170, 220)
show("backend/database.py", 1, 120)
PYRepository: 2233admin/opencli-admin
Length of output: 6234
Handle workflow runtime failures here. get_db() rolls back the flushed row on exceptions, so it won’t stay pending, but start_workflow_run() / list_workflow_run_events() still escape as a 500 and never persist a failed ValidationRun with failure_reason.
🤖 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 `@backend/services/validation_run_service.py` around lines 80 - 86, Update the
validation-run workflow in the surrounding service method to catch failures from
start_workflow_run and list_workflow_run_events, persist the ValidationRun as
failed with an appropriate failure_reason, and return the failed result instead
of allowing the exception to become a 500. Preserve the existing successful
event-processing path and ensure the failure update is committed before
returning.
| next_version_result = await session.execute( | ||
| select(func.max(WorkflowVersion.version_number)).where( | ||
| WorkflowVersion.project_id == draft.project_id | ||
| ) | ||
| ) | ||
| next_version_number = (next_version_result.scalar_one_or_none() or 0) + 1 | ||
|
|
||
| version = WorkflowVersion( | ||
| project_id=draft.project_id, | ||
| draft_id=draft.id, | ||
| version_number=next_version_number, | ||
| source_revision=draft.revision, | ||
| validation_run_id=validation_run.id, | ||
| snapshot=draft.snapshot, | ||
| ) | ||
| session.add(version) | ||
| await session.flush() | ||
| await session.refresh(version) | ||
| return version |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "version_number|UniqueConstraint|unique=True" backend/migrations/versions/r5s6t7u8v9w0_add_workflow_authoring_tables.pyRepository: 2233admin/opencli-admin
Length of output: 578
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow version model / service / handler locations =="
rg -n "class WorkflowVersion|version_number|publish_draft|already published|concurrent publish|IntegrityError" backend -g '*.py'
echo
echo "== relevant model/migration snippets =="
sed -n '1,220p' backend/models/workflow_authoring.py
echo
sed -n '1,220p' backend/services/validation_run_service.py
echo
sed -n '220,290p' backend/workspaces.pyRepository: 2233admin/opencli-admin
Length of output: 14854
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '210,265p' backend/api/v1/workspaces.pyRepository: 2233admin/opencli-admin
Length of output: 2523
Serialize version allocation in publish_draft. uq_workflow_versions_project_number already prevents duplicate rows, but MAX(version_number)+1 still races under concurrent publishes and one request will bounce with a generic 409. Lock the project row or allocate the next version atomically before inserting.
🤖 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 `@backend/services/validation_run_service.py` around lines 160 - 178, Update
publish_draft’s version allocation around the WorkflowVersion query so
concurrent publishes for the same project are serialized before calculating
MAX(version_number)+1. Lock the associated project row, or use an equivalent
atomic allocation mechanism, then insert the WorkflowVersion while preserving
the existing unique constraint and return flow.
| async def update_workspace( | ||
| session: AsyncSession, workspace: Workspace, data: WorkspaceUpdate | ||
| ) -> Workspace: | ||
| updates = data.model_dump(exclude_unset=True) | ||
| for key, value in updates.items(): | ||
| setattr(workspace, key, value) | ||
| await session.flush() | ||
| await session.refresh(workspace) | ||
| return workspace | ||
|
|
||
|
|
||
| async def get_workspace_settings( | ||
| session: AsyncSession, workspace_id: str | ||
| ) -> Optional[WorkspaceSettings]: | ||
| result = await session.execute( | ||
| select(WorkspaceSettings).where(WorkspaceSettings.workspace_id == workspace_id) | ||
| ) | ||
| return result.scalar_one_or_none() | ||
|
|
||
|
|
||
| async def update_workspace_settings( | ||
| session: AsyncSession, settings: WorkspaceSettings, data: WorkspaceSettingsUpdate | ||
| ) -> WorkspaceSettings: | ||
| updates = data.model_dump(exclude_unset=True) | ||
| for key, value in updates.items(): | ||
| setattr(settings, key, value) | ||
| await session.flush() | ||
| await session.refresh(settings) | ||
| return settings |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "class Workspace\b" -A 20 backend/models/workflow_authoring.py
rg -n "class WorkspaceSettings" -A 20 backend/models/workflow_authoring.pyRepository: 2233admin/opencli-admin
Length of output: 1752
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the update schemas and the endpoints that call these service functions.
rg -n "class WorkspaceUpdate|class WorkspaceSettingsUpdate|update_workspace_settings|update_workspace" backend -A 20 -B 10
# Inspect any IntegrityError handling around workspace update routes.
rg -n "IntegrityError|HTTPException|except.*IntegrityError" backend -A 20 -B 10Repository: 2233admin/opencli-admin
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for global handlers or middleware that translate IntegrityError / DB constraint failures.
rg -n "IntegrityError|exception_handler|add_exception_handler|SQLAlchemyError|DBAPIError|OperationalError" backend -A 20 -B 10
# Check the workspace endpoints around the update paths for local handling.
sed -n '1,130p' backend/api/v1/workspaces.pyRepository: 2233admin/opencli-admin
Length of output: 40976
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '40,90p' backend/api/v1/workspaces.py
sed -n '1,120p' backend/schemas/workflow_authoring.pyRepository: 2233admin/opencli-admin
Length of output: 5221
Reject explicit null in workspace updates. exclude_unset=True still forwards client-sent null, so Workspace.name and the non-null WorkspaceSettings fields can be set to None and fail on flush/commit; the update route doesn’t catch IntegrityError, so this becomes a 500. Filter out None values or validate them explicitly before persisting.
🤖 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 `@backend/services/workflow_authoring_service.py` around lines 47 - 75, Update
update_workspace and update_workspace_settings to exclude or reject
client-supplied None values before applying fields, while preserving valid
non-null updates. Ensure Workspace.name and non-null WorkspaceSettings fields
cannot be assigned None before session.flush().
| async def update_draft( | ||
| session: AsyncSession, draft: WorkflowDraft, data: WorkflowDraftUpdate | ||
| ) -> WorkflowDraft: | ||
| if draft.revision != data.expected_revision: | ||
| raise DraftRevisionConflictError( | ||
| f"draft revision {draft.revision} does not match expected " | ||
| f"{data.expected_revision}" | ||
| ) | ||
| draft.snapshot = data.snapshot.model_dump(mode="json") | ||
| draft.revision += 1 | ||
| await session.flush() | ||
| await session.refresh(draft) | ||
| return draft |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "class WorkflowDraft" -A 30 backend/models/workflow_authoring.py
rg -n "version_id_col" backend/models/workflow_authoring.pyRepository: 2233admin/opencli-admin
Length of output: 1466
🏁 Script executed:
#!/bin/bash
set -e
printf '\n## workflow_authoring_service.py outline\n'
ast-grep outline backend/services/workflow_authoring_service.py --view expanded || true
printf '\n## workflow_authoring_service.py relevant lines\n'
cat -n backend/services/workflow_authoring_service.py | sed -n '1,220p'
printf '\n## search for WorkflowDraft loading / locking / revision conflict handling\n'
rg -n "WorkflowDraft|DraftRevisionConflictError|with_for_update|version_id_col|StaleDataError|expected_revision|revision" backend -g'*.py'Repository: 2233admin/opencli-admin
Length of output: 23089
Make the draft revision update atomic backend/services/workflow_authoring_service.py:122-134
WorkflowDraft has no version_id_col, so this Python-side revision check can still lose concurrent updates. Move the revision guard into the UPDATE itself or add SQLAlchemy optimistic locking so stale writes fail instead of overwriting each other.
🤖 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 `@backend/services/workflow_authoring_service.py` around lines 122 - 134, The
update_draft function currently performs a non-atomic Python-side revision
check, allowing concurrent updates to overwrite each other. Make the revision
guard part of the database update or configure SQLAlchemy optimistic locking for
WorkflowDraft, ensuring stale writes fail with DraftRevisionConflictError and
only the row matching data.expected_revision is updated.
| def _extract_n8n_connection_edges(connections: dict[str, Any]) -> list[dict[str, Any]]: | ||
| """Flatten n8n's ``{sourceName: {main: [[{node, type, index}, ...]]}}`` shape. | ||
|
|
||
| n8n's export keys ``connections`` by node *name* (not the node's ``id`` | ||
| field), and each output port fans out to a list of connection chains — | ||
| unlike the list-of-{source,target} shape every other runtime exports. | ||
| """ | ||
|
|
||
| edges: list[dict[str, Any]] = [] | ||
| for source_name, outputs in connections.items(): | ||
| source = _read_string(source_name) | ||
| if not source or not isinstance(outputs, dict): | ||
| continue | ||
| for port_name, chains in outputs.items(): | ||
| if not isinstance(chains, list): | ||
| continue | ||
| for chain in chains: | ||
| if not isinstance(chain, list): | ||
| continue | ||
| for connection in chain: | ||
| if not isinstance(connection, dict): | ||
| continue | ||
| target = _read_string(connection.get("node")) | ||
| if not target: | ||
| continue | ||
| edges.append( | ||
| { | ||
| "id": f"edge-{len(edges) + 1}", | ||
| "source": source, | ||
| "target": target, | ||
| "sourcePort": _read_string(port_name) or "main", | ||
| } | ||
| ) | ||
| return edges | ||
|
|
||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map relevant file structure and locate the importer and its tests
git ls-files backend/workflow | sed -n '1,200p'
echo
rg -n "def _extract_n8n_connection_edges|import_external_workflow|sourcePort|externalWorkflow|sourceBranchIndex|connect_nodes" backend/workflow -nRepository: 2233admin/opencli-admin
Length of output: 4726
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backend/workflow/external_importer.py (relevant sections) =="
sed -n '1,160p' backend/workflow/external_importer.py
echo
sed -n '234,290p' backend/workflow/external_importer.py
echo
echo "== backend/workflow/compiler.py (sourcePort handling) =="
sed -n '300,345p' backend/workflow/compiler.py
echo
sed -n '560,610p' backend/workflow/compiler.py
echo
sed -n '840,875p' backend/workflow/compiler.py
echo
echo "== backend/workflow/runtime_registry.py (externalWorkflow handling) =="
sed -n '420,520p' backend/workflow/runtime_registry.py
echo
echo "== n8n/external importer tests =="
rg -n "n8n|external_import|externalWorkflow|sourcePort|connect_nodes" backend -g '*test*' -g '*spec*'Repository: 2233admin/opencli-admin
Length of output: 17228
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backend/workflow/external_importer.py (edge helpers) =="
sed -n '160,290p' backend/workflow/external_importer.py
echo
echo "== n8n-specific references in repository =="
rg -n "n8n|branch|index|true branch|false branch|Switch|IF" backend -g '!**/__pycache__/**'Repository: 2233admin/opencli-admin
Length of output: 50379
Preserve n8n branch provenance in imported edges. _extract_n8n_connection_edges() drops the output-slot index, and import_external_workflow() then ignores even the remaining sourcePort by rebuilding edges from the node alone. That collapses IF/Switch true/false branches into indistinguishable connections, so n8n conditional control flow can’t round-trip. Thread the branch index through the edge payload and ui.externalWorkflow if n8n imports need to retain semantics.
🤖 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 `@backend/workflow/external_importer.py` around lines 234 - 269, Preserve n8n
branch provenance by adding each connection’s output-slot index to the edge
payload produced by _extract_n8n_connection_edges(), then update
import_external_workflow() and the ui.externalWorkflow conversion to retain and
use both sourcePort and the branch index instead of rebuilding edges from node
names alone. Ensure IF/Switch true and false branches remain distinguishable
through import and round-trip export.
|
工作流持久化、Dify 导入/编译/运行、递归包与生命周期能力已由统一基准 codex/unified-product-3002 的当前实现取代。关闭并清理该旧分支。 |
1 participant
Closes progress on #11.
Summary
a::b::c), 16-layer nesting limit, per-level cycle detection. Depth-1 output unchanged (regression tested).Backend/migration/test only — no frontend changes.
Known limitations (see #11 for full writeup)
node_libraryorigin, notn8n(catalogId precedence) —n8nbranch is unit-tested but not exercised via the importer's current outputs.Test plan
uv run pytest -m "not live" -q— 1636 passed, 5 failed (pre-existing Windows/GBK locale codec issue, unrelated to this diff), 1 skippeduv run pytest tests/integration/test_workflow_authoring_api.py -v --no-cov— 6 passeduv run pytest tests/integration/test_workflow_external_import_api.py -v— 3 passeduv run alembic upgrade headagainst scratch SQLite — applies cleanly on top ofm2n3o4p5q6r7