🤖 fix: default slash workflow project path args#3582
Conversation
Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fed6052a61
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed the Codex finding about sub-project workspaces.
|
|
@codex review Please take another look after the sub-project path default fix. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f7283962f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed the follow-up Codex finding about worktree/SSH checkout paths.
|
|
@codex review Please take another look after the checkout-path default fix. |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
## Summary Long-lived auto-cleanup PR maintained by the **Auto-Cleanup Agent**. Each pass lands at most one extremely low-risk, behavior-preserving cleanup drawn from recently merged `main` activity, then advances the checkpoint below. ## This pass Replaced an inline terminal-status check in `WorkflowService.interruptRunOnAbort` (`src/node/services/workflows/WorkflowService.ts`) with the `isTerminalWorkflowRunStatus` helper introduced by coder#3587 ("show workflow-only sidebar activity"). The guard ```ts if ( run == null || run.status === "completed" || run.status === "failed" || run.status === "interrupted" ) { return; } ``` now reads `if (run == null || isTerminalWorkflowRunStatus(run.status))`. - **Behavior-preserving:** `isTerminalWorkflowRunStatus(status)` is defined as `status === "completed" || status === "failed" || status === "interrupted"` — byte-for-byte the same three-status disjunction (same short-circuit order, same boolean result), so the `run == null || ...` early-return is unchanged. - **Why it helps:** `isTerminalWorkflowRunStatus` is already the canonical terminal-status predicate everywhere else (`taskService.ts`, `tools/task_await.ts`); this was the only remaining hand-inlined copy in `WorkflowService`, so the dedup keeps the terminal-status set defined in exactly one place. - **Scoped:** one file, one guard; the helper was already exported from `@/common/types/workflow` and is now added to the existing import in `WorkflowService.ts` (next to the already-imported `isActiveWorkflowRunStatus`). ## Validation - `bun test src/node/services/workflows/WorkflowService.test.ts` — 58 pass / 0 fail. - `make static-check` — eslint, tsc (both configs), Prettier, and the generated workflow-runtime bundle drift check all pass. The only failing step is `fmt-shell-check`, which requires `shfmt` (not installed in this environment); no shell scripts were touched, so CI (which has `shfmt`) is unaffected. <details> <summary>Prior passes</summary> - Deduplicated the identical `workspaceId`-only metadata JSON Schema that three id-targeted host actions repeat in `src/node/services/workflows/workspaceHostActions.ts` into a module-level `WORKSPACE_ID_ONLY_INPUT_SCHEMA` constant (the third copy was introduced by coder#3583 when it added `workspace.unarchive`). Behavior-preserving: nothing mutates the metadata, and the shared constant has identical key order/values so the generated stub bundle is byte-identical (drift check passing). - Extracted a shared `BlockedBadge` component in `src/browser/features/Tools/WorkflowDefinitionToolCall.tsx` to deduplicate the `<WorkflowBadge tone="danger">blocked</WorkflowBadge>` markup that coder#3578 introduced verbatim in three spots. Also resolved a trivial rebase conflict from coder#3582 in `src/node/services/workflows/workflowArgs.ts`. - Deduplicated the repeated `"Workflow metadata.argsSchema.properties must be an object"` error string in `src/node/services/workflows/workflowArgs.ts` (added in coder#3575) into a module-level `ARGS_SCHEMA_PROPERTIES_ERROR_MESSAGE` constant. - Deduplicated the four mocked-module specifier strings in `src/browser/hooks/useModelsFromSettings.test.ts` into shared local constants so `installUseModelsModuleMocks()` / `restoreUseModelsModuleMocks()` can't drift (after coder#3574). - Removed a redundant `toggleWorkflowExpanded` wrapper in `src/browser/features/Tools/WorkflowRunToolCall.tsx`, left behind by coder#3571. - Deduplicated two repeated fallback error strings in `src/browser/components/AutomationModal/AutomationModal.tsx` (after coder#3560) into `SAVE_AUTOMATION_ERROR_MESSAGE` / `REMOVE_AUTOMATION_ERROR_MESSAGE` module-level constants. - Replaced a duplicated `"wfr_"` literal in `src/node/services/workflows/WorkflowRunner.ts` with the canonical `isWorkflowRunTaskId()` predicate from `src/node/services/tools/taskId.ts` (after coder#3565). - Removed a duplicated `"chat.jsonl"` literal in `src/node/services/analytics/etl.ts` by re-exporting `CHAT_FILE_NAME` from the canonical `src/common/constants/paths.ts` (after coder#3541). - Extracted a `harvestRecordTime` helper in `src/node/services/memoryConsolidationService.ts` so `findNewestHarvestRecord` and `pruneHarvestRecords` derive the `completedAt ?? startedAt` ordering key from one place (after coder#3558). </details> Auto-cleanup checkpoint: a7f047b --- _Generated with `mux` • Model: `anthropic:claude-opus-4-8` • Thinking: `xhigh` • Cost: `n/a`_ <!-- mux-attribution: model=anthropic:claude-opus-4-8 thinking=xhigh costs=n/a --> --------- Co-authored-by: mux-bot[bot] <264182336+mux-bot[bot]@users.noreply.github.com>
Summary
Fix slash-command workflow starts so workflows that declare a
projectPathargument receive the active workspace checkout path (including sub-project checkout paths) from the server-side workflow context instead of failing before a run is created.Background
Running a workflow from a slash command could hit
/workflows/startwith only the typed workflow args. Workflows whosemetadata.argsSchemarequiredprojectPaththen failed during argument normalization withWorkflow argument projectPath is required, which surfaced as a 500.Implementation
projectPathdefault, including sub-project workspaces, without persisting hidden fields into command invocation metadata.WorkflowArgsValidationErrorso workflow argument/schema validation failures map to oRPCBAD_REQUESTinstead of generic internal errors.simplifyworkflow and incorporated its small follow-up simplifications and malformed-schema validation coverage.Validation
bun test src/node/services/workflows/WorkflowService.test.ts --test-name-pattern "applies invocation default args|workflow args"bun test src/node/orpc/router.test.ts --test-name-pattern "router workflow routes"bun test src/node/orpc/router.test.ts --test-name-pattern "projectPath defaults|active sub-project|bad request"make static-checkworkflow_run simplify --base origin/mainRisks
Low. The default context only applies to workflow args declared in a workflow schema and only when the caller omitted that field. Explicit user args and schema defaults keep their existing precedence, persisted slash-command args remain unchanged, and sub-project workspaces now receive the same active checkout path that workflow execution should operate on.
Generated with
mux• Model:openai:gpt-5.5• Thinking:xhigh• Cost:$19.51