🤖 feat: return workflow plan agent results#3627
Merged
Merged
Conversation
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! 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". |
LeonidasZhak
pushed a commit
to LeonidasZhak/mux
that referenced
this pull request
Jun 30, 2026
## 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. Auto-cleanup checkpoint: 620c2a6 This PR currently contains six accumulated cleanups against `main`: 1. `src/node/services/taskService.ts` — dedupe queued-message foreground backgrounding into a private helper. 2. `src/node/services/workflows/workflowScriptResolver.ts` — reuse the `SKILL_SCRIPT_PATH_PREFIX` constant for canonical-path construction. 3. `src/node/services/workflows/WorkflowTaskServiceAdapter.ts` — extract the duplicated agent-task creation arg shape into a shared `WorkflowTaskCreateArgs` type. 4. `src/constants/workspaceTags.ts` (new) — centralize the workspace-turn task tag keys, removing the cross-layer `"mux.taskHandleId"` string duplication. 5. `src/node/services/tools/workflowProgress.ts` — extract the duplicated "latest phase" lookup into a shared `getLatestPhaseEvent` helper. 6. `src/browser/features/RightSidebar/Workflows/WorkflowBadges.tsx` — map workflow run-status glyphs through an exhaustive `Record<WorkflowRunStatus, LucideIcon>` instead of a repeated `if`-chain. ## This pass Rebased the branch onto the latest `origin/main` (clean) and considered the three new commits since the prior checkpoint: - coder#3624 (`feat: add Workflows right-sidebar tab with live run streaming`) - coder#3626 (`tests: add UI primitive stories and fix design-sync previews`) - coder#3627 (`feat: return workflow plan agent results`) coder#3624 added `WorkflowBadges.tsx`, whose `WorkflowStatusIcon` selected a glyph through a four-branch `if`-chain that re-spelled the identical `className="h-3 w-3 shrink-0"` + `style={{ color }}` props in every branch and fell back to a default `<Circle>` for unmatched statuses. Replaced that with a module-level `WORKFLOW_STATUS_ICON: Record<WorkflowRunStatus, LucideIcon>` map (mirroring the existing `WORKFLOW_STATUS_META` / `WORKFLOW_TONE_VAR` records in the same feature) and render the looked-up icon once. Behavior-preserving: the map reproduces the prior mapping exactly (`pending`/`running` → `Circle`, `backgrounded`/`interrupted` → `Pause`, `completed` → `Check`, `failed` → `X`), and using a `Record` makes the mapping exhaustive so a newly added run status is a compile error here rather than a silent fall-through. coder#3626 (Storybook/design-sync, test-only) and coder#3627 (already extracts its own `__muxAgentReturnValue` runtime helper) offered no additional low-risk dedup. <details> <summary>Prior pass — workflowProgress getLatestPhaseEvent helper</summary> coder#3623 added `src/node/services/tools/workflowProgress.ts`, in which both `buildWorkflowProgressSummary` and `formatWorkflowProgressNote` independently re-spelled the same `run.events.findLast((event) => event.type === "phase")` expression to find the latest phase event. Extracted that into a single private `getLatestPhaseEvent(run)` helper and referenced it from both functions, giving the "latest phase" definition one source of truth so the summary builder and the note formatter can't drift. Behavior-preserving: the helper's body and inferred return type are identical to the inline expression, so both call sites compute exactly the same value. </details> <details> <summary>Prior pass — checkpoint-only advance for coder#3621 (Claude Design integration)</summary> coder#3621 is purely additive and lives entirely under `.design-sync/` (plus one `.gitignore` entry); it contains zero production (`src/`) code. `.design-sync/**` is outside both the ESLint scope (`src/**`) and the TypeScript `include`, so `make static-check` cannot validate the behavior-preservation of any refactor there. That pass advanced the checkpoint without a code change. </details> <details> <summary>Prior pass — workspace-turn task tag keys centralization</summary> The workspace-turn task tag key `"mux.taskHandleId"` is **written** by `TaskService.createWorkspaceTurn` (node, coder#3617/coder#3619) and **read** by `findWorkspaceForTaskTarget` in `TaskToolCall.tsx` (browser, coder#3613) — the same magic string duplicated across the node/browser boundary. Extracted the three workspace-turn task tag keys (`handle`, `ownerWorkspaceId`, `turn`) into a new `@/constants/workspaceTags` module and referenced `WORKSPACE_TURN_TASK_TAGS` from both the node write site and the browser read site. Behavior-preserving: the extracted values are byte-identical to the previous literals, so the persisted tag keys and all lookups are unchanged. </details> <details> <summary>Prior pass — WorkflowTaskServiceAdapter WorkflowTaskCreateArgs type</summary> `WorkflowTaskServiceLike` (in `WorkflowTaskServiceAdapter.ts`) declared the agent-task creation argument object **twice** — once inline in `create(args: { ... })` and again inside `createMany?(args: Array<{ ... }>)` — as byte-identical 10-field shapes. Extracted a named `WorkflowTaskCreateArgs` interface and referenced it from both `create` and `createMany`. Behavior-preserving: structurally identical interface, signatures unchanged. </details> <details> <summary>Prior pass — workflowScriptResolver SKILL_SCRIPT_PATH_PREFIX reuse</summary> In `workflowScriptResolver.ts`, the `skill://` prefix is named by the module-level constant `SKILL_SCRIPT_PATH_PREFIX`, but the two canonical-path builders re-spelled the literal `` `skill://${...}` `` instead of reusing the constant. Both builders now interpolate `${SKILL_SCRIPT_PATH_PREFIX}`. Behavior-preserving: `SKILL_SCRIPT_PATH_PREFIX` is exactly `"skill://"`, so the interpolation is byte-identical. </details> <details> <summary>Prior pass — taskService queued-message dedup</summary> Considered coder#3605 (`fix: background foreground waits for prequeued messages`), which open-coded the same queued-message guard at two `TaskService` wait-registration sites. Both sites now delegate to a single private helper `backgroundForegroundWaitIfQueued(shouldBackgroundOnQueuedMessage, requestingWorkspaceId)`. Behavior-preserving: the helper folds in the `requestingWorkspaceId` truthiness guard and the pushed call is unchanged. </details> ## Validation - `make static-check` is green for the touched code (ESLint, `tsconfig` + `tsconfig.main.json`, Prettier). (`fmt-shell-check` is the only non-passing step, solely because the `shfmt` binary is absent in this environment; no shell files are touched.) - Targeted tests pass: `bun test src/browser/features/RightSidebar/Workflows/workflowDisplay.test.ts src/browser/features/RightSidebar/Workflows/projectWorkflowRun.test.ts` (32 tests) cover the same Workflows feature module the change lives in. ## Risks Minimal. All accumulated changes are pure local extractions / constant reuse; no logic, types, or call semantics change. --- _Generated with `mux` • Model: `anthropic:claude-opus-4-8` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-opus-4-8 thinking=xhigh --> --------- Co-authored-by: mux-bot[bot] <264182336+mux-bot[bot]@users.noreply.github.com>
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
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
Make workflow-owned Plan agents return a
{ reportMarkdown, planFilePath }object instead of a raw markdown string, aligning the workflow-visible Plan result with existing durable task-output field names.Background
Workflow Plan agents are new and complete through
propose_plan, which already records both the plan content and canonical plan file path in durable task output metadata. Returning the same field names to workflow authors lets verifier fan-out steps read the plan file without inventing Plan-only aliases or repeating the full plan content in every prompt.Implementation
{ reportMarkdown, planFilePath }for Plan agents.agent(),parallel(), andpipeline()paths.schema/outputSchemaand updated the defensive TaskService rejection message.Validation
bun test src/node/services/workflows/WorkflowRunner.test.tsbun test src/node/services/workflows/WorkflowTaskServiceAdapter.test.tsbun test src/common/orpc/schemas/workflow.test.tsbun test src/node/services/subagentReportArtifacts.test.tsbun test src/node/services/taskService.test.ts --test-name-pattern "workflow-owned plan"bun test src/node/services/agentSkills/builtInWorkflowSkillDescriptions.test.tsmake typecheckmake lintmake static-checkDogfooding: ran a real Plan → parallel verifier workflow through the CLI. The final report included a concrete
Plan path:and verifier summaries withpath=true, plan=truefor Tests, UX, and Risks. UI dogfooding was attempted against the running local Mux server but blocked by the auth screen; blocker screenshot was captured in the workspace conversation.Risks
The workflow-visible return type for
agentId: "plan"changes from string to object. This is intentional while Plan workflow agents are new; no production workflow usage was found during planning. If an isolated runtime cannot readplanFilePath, workflow authors can fall back toreportMarkdownas documented.📋 Implementation Plan
Plan: Make workflow Plan agents return
{ reportMarkdown, planFilePath }Goal
Make workflow-owned Plan agents return an object that uses the existing durable task-output field names:
This still replaces the current workflow-visible string return value for
agentId: "plan", but it avoids inventing Plan-only aliases. The workflow-visible object matches the fields already used by workflow task reports, UI/report plumbing, and durableStructuredTaskOutputrecords. Plan agents are newly added and currently have no production workflow usage in the repo, so this is the narrow window to make this API consistent before adoption.Verified context and constraints
agent(prompt, options)is implemented in the QuickJS workflow runtime template insrc/node/services/workflows/WorkflowRunner.ts.schema/outputSchemapresent ->result.structuredOutputresult.reportMarkdownpropose_plan, notagent_report.WorkflowTaskServiceAdapter.waitForAgentTask()already returns the durable task output shape:TaskService.handleSuccessfulWorkflowProposePlan()already finalizes workflow Plan tasks with:StructuredTaskOutputSchemaalready includesreportMarkdown, optionalplanFilePath, optionalstructuredOutput, and optionaltaskId; no ORPC/schema migration is expected.src/node/builtinSkills/workflow-authoring.mdcurrently documents the old string-return behavior and must be updated.schema/outputSchemafor Plan agents. The returned object is runtime-generated task metadata, not model structured output.Important runtime-path caveat
planFilePathwill initially map to the existing durableplanFilePathcaptured when the Plan child proposes its plan. Treat it as the canonical plan file path in the Plan task/runtime, not as a universally portable filesystem path.That path is expected to be directly readable by later child agents in local/worktree and SSH-style runtimes where workspaces share the same host filesystem. In Docker/container-isolated runtimes, the path may be scoped to the Plan child container and may not be readable by sibling verifier containers. The object still includes
reportMarkdownas an immutable content snapshot, so workflows retain a fallback.This plan does not add a cross-container immutable plan artifact copier in the first pass. If Docker dogfooding shows this is required for the intended verifier fan-out, handle it as a follow-up by copying the finalized plan snapshot into a workflow-owned artifact location that every later child runtime can read, then make
planFilePathpoint at that portable snapshot.Public API decision
Recommended API
Plan workflow agents return this workflow-visible JavaScript object:
Mapping:
This is a minimal public object, not the raw internal task result. Do not expose
taskId,title, orstructuredOutputas part of this Plan-agent workflow API unless a later requirement explicitly needs them.Rationale:
reportMarkdown/planFilePath).result.planFilePathto verifier agents without repeating the full plan in every prompt.result.reportMarkdownavailable as the immutable snapshot and fallback when path reads are not desirable or fail.Final workflow result caveat:
normalizeWorkflowResultForEvent()treats any object withreportMarkdownas the workflow's final report shape.returnsplanResultdirectly will preserve the plan text as finalreportMarkdown, but should not rely onplanFilePathbeing preserved in the final workflow result event.reportMarkdowntext or instructuredOutputexplicitly.Net product-code LoC estimate: +25 to +40 LoC in
WorkflowRunner.tsplus ~2 changed product-code lines for schema-rejection error messages. Tests/docs are excluded from this product-code estimate.Implementation phases
Phase 1 — Add a central Plan-result mapper in the workflow runtime
File:
src/node/services/workflows/WorkflowRunner.tsAdd a small helper inside the compiled runtime template near
__muxAgent/__muxParallel/__muxPipeline:Notes:
undefined.Quality gate after Phase 1:
bun test src/node/services/workflows/WorkflowRunner.test.tsPhase 2 — Update direct, parallel, and pipeline agent return mapping
File:
src/node/services/workflows/WorkflowRunner.tsIn
__muxAgent, compute Plan status after option normalization:Keep the existing schema rejection, but update the message:
Direct agent call path:
Parallel collection path:
hasSchemaandisPlanAgenton the marker object returned from each thunk.__muxAgentReturnValue(taskResult, branchResult.hasSchema, branchResult.isPlanAgent).Pipeline collection path:
hasSchemaandisPlanAgentin the collected agent metadata.Quality gate after Phase 2:
bun test src/node/services/workflows/WorkflowRunner.test.tsPhase 3 — Update Plan schema-rejection handling outside the JS runtime
File:
src/node/services/taskService.tsUpdate the existing workflow Plan completion rejection message in
handleSuccessfulWorkflowProposePlan()from the old text to:Why this exists:
schema/outputSchemabefore spawning Plan agents.TaskServicestill needs the defensive backend check because a workflow-owned Plan task could already be running or be finalized through another path.Quality gate after Phase 3:
Phase 4 — Add/adjust tests
4.1 Workflow runtime tests
File:
src/node/services/workflows/WorkflowRunner.test.tsAdd focused tests for the public workflow JS API:
Direct Plan agent returns object
Workflow source:
Mock
runAgent()returns:Assert final
reportMarkdowncontains both values.Do not test this by returning
planResultdirectly as the workflow result; field-access assertions avoid conflating the Plan-agent API with final workflow result normalization.Parallel Plan agents return objects in input order
parallel([() => agent(...plan-a...), () => agent(...plan-b...)])..reportMarkdownand.planFilePath, and the final report proves ordering.Pipeline Plan agents pass objects to the next stage
pipeline()with a Plan stage followed by a stage that readsresult.reportMarkdownandresult.planFilePath.Existing schema rejection test stays, with updated message
rejects schema on built-in plan agent stepsbut update expected text to{ reportMarkdown, planFilePath }.Non-Plan no-schema agents still return strings
new agent API returns structured output for schema-backed steps and markdown otherwisetest already covers this. Leave it in place and ensure it still passes.Plan agent missing
planFilePathfails fastrunAgent()returnsreportMarkdownbut omitsplanFilePath.4.2 Adapter/schema tests
Files:
src/node/services/workflows/WorkflowTaskServiceAdapter.test.tssrc/common/orpc/schemas/workflow.test.tssrc/node/services/subagentReportArtifacts.test.tsExpected action:
planFilePathmetadata.4.3 TaskService tests
File:
src/node/services/taskService.test.tsUpdate expectations that assert the old schema-rejection message. Existing search evidence points to workflow-owned Plan schema tests around the
workflow-owned planregion.Quality gate after Phase 4:
Phase 5 — Update workflow-authoring docs/skill text
File:
src/node/builtinSkills/workflow-authoring.mdUpdate the
agent(prompt, options)section:agent()returns a markdown string.reportMarkdown: proposed plan markdown snapshot.planFilePath: canonical path to the plan file captured at proposal time in the Plan task/runtime; it may not be readable from every isolated sibling runtime.schema/outputSchemais not allowed for Plan agents.reportMarkdown; includeplanFilePathin finalreportMarkdownorstructuredOutputwhen the final workflow output must show it.planFilePath, pass or fall back toplanResult.reportMarkdown.Quality gate after Phase 5:
bun test src/node/services/agentSkills/builtInWorkflowSkillDescriptions.test.tsPhase 5.5 — Search for stale wording and usage
Run these searches after code/docs updates and fix any stale references:
Expected outcomes:
reportMarkdown/planFilePath.plan/planPathaliases.Phase 6 — Full validation
Run focused checks first, then broader static validation. This repo sources
.mux/tool_envbeforebashtool calls, so userun_and_reportwhen batching validation; if it is unavailable, run the raw commands after each step name.Raw-command equivalents:
If lint is memory-constrained in this workspace, use the known repo fallback:
Before claiming completion, also run either:
or explicitly report why full static-check was not available and provide the completed subset above.
Dogfooding plan
Dogfooding must prove the new API in a real workflow, not only unit tests.
Dogfood setup
./workflows/dogfood-plan-result.jsbecause workflow execution expects explicit trusted workspace paths; verifygit statusis clean of dogfood artifacts afterward. Script contents:workflow_runtool if available in-agent; otherwise the app/CLI workflow runner).Dogfood quality gates
planResult.planFilePathin the default local/worktree runtime.Dogfood evidence
Always capture terminal evidence from the workflow run, including the final report proving
planResult.planFilePathand verifier summaries were produced.If a running UI/dev server is available, also use
agent-browserper the browser automation instructions. Before running browser commands, load current CLI guidance with:When UI dogfooding is available, capture and attach:
Plan path:and verifier summaries.If visual dogfooding is not possible in the implementation environment, explicitly report the blocker and provide the terminal workflow-run evidence instead; do not claim screenshots/video were captured.
Acceptance criteria
agent(prompt, { id, agentId: "plan" })returns an object with:reportMarkdown: stringplanFilePath: stringparallel(), andpipeline()all receive the same Plan object shape.structuredOutput.schema/outputSchemawith updated wording.workflow-authoringdocs show the newreportMarkdown/planFilePathobject API and verifier fan-out pattern.Risks and mitigations
reportMarkdown/planFilePathaligns with existing task-output conventions and avoids Plan-only aliases.planFilePathmissing in a Plan result__muxPlanAgentResult; successful Plan finalization should always include it.planFilePathunreadable in isolated runtimesreportMarkdownsnapshot in the object; document fallback. Consider follow-up portable snapshot artifact if Docker dogfood requires it.agent()Out of scope for this change
returnMode; the user explicitly chose a default Plan-object return while Plan agents are new.schema/outputSchema.StructuredTaskOutput/ ORPC fields or adding Plan-only aliases such asplan/planPath.planFilePathis insufficient.Generated with
mux• Model:openai:gpt-5.5• Thinking:xhigh• Cost:$44.09