fix(workflows): support editing global workflows#1557
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughServer and web changes add explicit workflow scoping: a ChangesWorkflow source support (project vs global/home)
Sequence Diagram(s)sequenceDiagram
autonumber
participant Browser
participant WebClient
participant Server
participant FS as Filesystem
Browser->>WebClient: Open workflow in builder (name)
WebClient->>Server: GET /api/workflows/:name?cwd=...
alt project file exists
Server->>FS: read <cwd>/.archon/workflows/name.yaml
FS-->>Server: project workflow
Server-->>WebClient: 200 { workflow, source: "project" }
else project missing but home exists
Server->>FS: read $ARCHON_HOME/workflows/name.yaml
FS-->>Server: global workflow
Server-->>WebClient: 200 { workflow, source: "global" }
else bundled exists
Server-->>WebClient: 200 { workflow, source: "bundled" }
end
Browser->>WebClient: Save workflow (edited)
WebClient->>Server: PUT /api/workflows/:name?cwd=...&source=global (if global)
alt source=global
Server->>FS: write $ARCHON_HOME/workflows/name.yaml
FS-->>Server: ok
Server-->>WebClient: 200 { name, source: "global" }
else project
Server->>FS: write <cwd>/.archon/workflows/name.yaml
FS-->>Server: ok
Server-->>WebClient: 200 { name, source: "project" }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/server/src/routes/api.ts`:
- Line 1: The file was accidentally replaced by a single marker and no longer
exports registerApiRoutes; restore the module to export a registerApiRoutes
function that registers the workflow endpoints using
registerOpenApiRoute(createRoute({...}), handler). Specifically, implement and
export registerApiRoutes that calls registerOpenApiRoute with a GET route (e.g.,
WorkflowGetHandler) to fetch workflows, a PUT route (e.g., WorkflowPutHandler)
to create/update workflows, and a DELETE route (e.g., WorkflowDeleteHandler) to
remove workflows, ensuring each route is created via createRoute({...}) and
wired to its handler; follow the existing route shape and OpenAPI metadata
conventions used elsewhere in packages/server/src/routes and re-export the same
named symbols so imports elsewhere resolve.
In `@packages/server/src/routes/api.workflows.test.ts`:
- Line 1: The file currently contains only a placeholder string and must be
replaced with a real test module: restore
packages/server/src/routes/api.workflows.test.ts to export/define Jest tests
that exercise the global workflow load/save endpoints (e.g., tests named "loads
global workflows" and "saves global workflows"); import the app/server instance
and any test helpers (supertest/request, fixtures or mocked DB), set up/tear
down test data, call the relevant routes (GET/POST or PUT for global workflows),
assert responses and persisted state, and re-enable any mocked auth or DB
helpers previously used by other route tests so coverage for global workflow
load/save is restored. Ensure the module uses proper TypeScript imports/exports
and Jest describe/it blocks rather than the marker string.
In `@packages/web/src/components/workflows/WorkflowBuilder.tsx`:
- Line 1: The file currently contains only a string and must be replaced with
the real React component implementation: restore a functional exported component
named WorkflowBuilder (used by WorkflowBuilderPage.tsx) that implements
source-aware save/load for global workflows; specifically, reintroduce the
component (function WorkflowBuilder or const WorkflowBuilder: React.FC) with its
state/hooks to load workflow data based on source metadata, provide UI handlers
for saving which include source attribution and branch/commit info, and export
default WorkflowBuilder so WorkflowBuilderPage.tsx can import it — locate
references to WorkflowBuilder, its props (if any), and any load/save helpers
used elsewhere in the repo and wire them back into this component to match the
previous behavior.
In `@packages/web/src/lib/api.ts`:
- Line 1: The file currently contains an invalid marker and must be replaced
with the full TypeScript API surface required by the web package: restore
exports for functions like listWorkflows and listProviders and constants like
SSE_BASE_URL, and the types DagNode, CommandEntry, CodebaseResponse,
ConversationResponse, MessageResponse (and any other types used across
ChatPage.tsx, SettingsPage.tsx, DashboardPage.tsx, useProviders.ts, useSSE.ts).
Fix by replacing the marker string with the original module implementation or by
re-exporting the real implementations/types from their canonical module (e.g.,
export { listWorkflows, listProviders, SSE_BASE_URL } from '...'; export type {
DagNode, CommandEntry, CodebaseResponse, ConversationResponse, MessageResponse }
from '...'), making sure all named exports referenced across the web package are
present and properly typed so TypeScript type-checking and imports succeed.
In `@packages/web/src/lib/command-categories.ts`:
- Line 1: This module is empty but other modules import categorizeCommands; add
and export a named function categorizeCommands that matches the signature
expected by NodeLibrary.tsx and CommandPicker.tsx (e.g.,
categorizeCommands(commands: Command[]): Record<string, Command[]> or the exact
type those components expect), implement the logic to group commands into
categories (e.g., by command.category or similar property), and export any
related types used by callers; ensure the exported function name is exactly
"categorizeCommands" and the TypeScript types align with the Command type
imported/used by the callers so compilation succeeds.
🪄 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: 5df9e04e-a957-4ea5-b6a3-b4211ac76ba1
📒 Files selected for processing (5)
packages/server/src/routes/api.tspackages/server/src/routes/api.workflows.test.tspackages/web/src/components/workflows/WorkflowBuilder.tsxpackages/web/src/lib/api.tspackages/web/src/lib/command-categories.ts
3fa7435 to
e6da4e6
Compare
|
@guanghuang related to #1556 — global workflow editing support. |
|
There's significant overlap with #1525 (opened 2026-05-01, also still open), which addresses the same underlying gap - the single-workflow REST endpoints don't resolve ~/.archon/workflows/, so home-scoped workflows show up in the list but break on the detail page. Both PRs touch packages/server/src/routes/api.ts and api.workflows.test.ts and add the same project → global → bundled tier in GET /api/workflows/:name. The interesting differences are:
Closing one in favor of the other, or rebasing #1557 on top of #1525's server fix and keeping only the Web UI/builder changes here, would avoid non-trivial merge conflicts in api.ts and the test file. |
Review SummaryVerdict: minor-fixes-needed This PR adds a Blocking issuesNone. Suggested fixes
Minor / nice-to-have
ComplimentsYour error-handling is thorough and consistent — Reviewed via maintainer-review-pr workflow (Pi/Minimax). Aspects run: error-handling, test-coverage, comment-quality, docs-impact. |
Review SummaryVerdict: blocking-issues The PR adds global workflow support via a Blocking issues
Suggested fixes
Minor / nice-to-have
Compliments
Reviewed via maintainer-review-pr workflow (Pi/Minimax). Aspects: code-review, error-handling, test-coverage, comment-quality, docs-impact. |
- Document the `source` query parameter on PUT and DELETE in the API reference, including the 400 error for invalid values. - Document the new `source: "global"` response value for GET, with the three-tier auto-discovery order spelled out. - Add a DELETE test for `?source=global` to confirm the home-scoped file is removed (PUT already had coverage; DELETE was missing). - Add 400 "Invalid workflow source" validation tests for both PUT and DELETE so the enum rejection path is locked down. The blocking issue from the prior review — PUT/DELETE routes using cwdQuerySchema instead of workflowTargetQuerySchema — was already resolved on the branch via the rebase against current dev; both routes now correctly use workflowTargetQuerySchema. The comment-rot reference (`source:global` string in a comment) was dropped as part of the rebase conflict resolution where dev's home-scoped resolver block was kept.
e6da4e6 to
fa0b9ad
Compare
* fix(workflows): support editing global workflows * fix: address review feedback on global workflow editing - Document the `source` query parameter on PUT and DELETE in the API reference, including the 400 error for invalid values. - Document the new `source: "global"` response value for GET, with the three-tier auto-discovery order spelled out. - Add a DELETE test for `?source=global` to confirm the home-scoped file is removed (PUT already had coverage; DELETE was missing). - Add 400 "Invalid workflow source" validation tests for both PUT and DELETE so the enum rejection path is locked down. The blocking issue from the prior review — PUT/DELETE routes using cwdQuerySchema instead of workflowTargetQuerySchema — was already resolved on the branch via the rebase against current dev; both routes now correctly use workflowTargetQuerySchema. The comment-rot reference (`source:global` string in a comment) was dropped as part of the rebase conflict resolution where dev's home-scoped resolver block was kept. --------- Co-authored-by: Rasmus Widing <rasmus.widing@gmail.com>
Summary
~/.archon/workflowsare listed in the Web UI but cannot reliably be opened in the workflow builder because the detail endpoint does not resolve global workflow files.source=global, the builder preserves loaded workflow source on save, and global commands appear in the builder command library.UX Journey
Before
After
Architecture Diagram
Before
After
Connection inventory:
GET /api/workflowsGET /api/workflows/:name~/.archon/workflowsPUT /api/workflows/:namesource=globalwhen editing a global workflow.Label Snapshot
risk: lowsize: Sserver|web|testsworkflows:builderChange Metadata
bugmultiLinked Issue
Validation Evidence (required)
Commands and result summary:
Security Impact (required)
No)No)No)Yes)Yes, describe risk and mitigation: The existing workflow edit API can now explicitly target Archon's home-scoped workflow directory whensource=globalis supplied. The workflow name still passes existing command-name validation, and writes are constrained togetHomeWorkflowsPath()with a fixed.yamlfilename.Compatibility / Migration
Yes)No)No)Human Verification (required)
What was personally validated beyond CI:
origin/dev; only this feature fix is included; route tests cover loading fromARCHON_HOME/workflowsand saving withsource=global.sourcequery values are rejected; project save path remains the default when no global source is supplied.Side Effects / Blast Radius (required)
source=globalcan update a same-named global workflow instead of creating a project workflow.Rollback Plan (required)
Risks and Mitigations
Summary by CodeRabbit
New Features
Tests