Repository navigation
bug(workflow-automation): chat-driven workflow creation now succeeds but returns three fixed echo steps for every request #13809
Description
Activity
Proof landed (PR #13928) — the routing decision is yours, and it has a safety half
The defect is now machine-checked: two
xfail(strict=True)tests describe the intended behaviour and fail today, so whoever fixes this must remove them. Plus a characterisation test pinning today's fixed skeleton, and a security test pinning that hostile text cannot reach a generated command.I did not wire the real planner. Reading the route first is what stopped me:
auto_start = request.get("auto_start", True) # start immediately require_approval = request.get("require_approval", False) # no approval
Today the endpoint builds three
echosteps and runs them — useless but harmless. Routing it atcreate_workflow_planbehind those defaults turns it into a route that generates shell commands from user text and executes them without approval. That is a change in what the system may do unattended, not a refactor, and shipping it while the defaults stand would deliver the risky half of the fix without the decision that governs it.The two decisions, with recommendations
1. Which planner? — recommend
create_workflow_plan(option 1 in this issue). It converges on the canonical planner rather than adding a third path, consistent with the #13751 supersession record. Option 2 (teachplan_workflow_stepsto derive actions) would build a second real planner beside the existing one, which is the fork pattern.2. Does
auto_start=Truesurvive? — recommend no, and that it flips before the planner is wired, not after. The ordering matters: land the planner first and there is a window where the default path auto-executes generated commands. Flipping first costs a backward-compatibility break on a route that currently produces nothing of value — the cheapest moment this will ever be.If you would rather keep
auto_start=Truefor compatibility, then option 3 (gate the route behind a flag until approval is the default) is the safer sequencing, and I would take that over wiring the planner into the current defaults.What I would do on your word
Either: flip the defaults, then route at
create_workflow_plan, removing the xfail markers as the fix lands. Or: gate the route and leave the planner alone. Both are a single PR; the choice is which risk you would rather hold.Labelling
needs-decisionand moving on.- added a commit that references this issue
on Aug 10, 2026 Proof landed — PR #13928 merged. Verified in base:
autobot-backend/tests/services/test_chat_workflow_is_request_shaped.pyis present, with twoxfail(strict=True)markers that will XPASS (and therefore fail) the moment the plans become request-shaped, plus a security test that must survive whatever planner replaces this one.The two decisions in the comment above are unchanged and still blocking the fix: which planner, and whether
auto_start=Truesurvives. My recommendation remains to flip the default before wiring the planner, since landing it first leaves a window where the default path auto-executes generated commands.Correction: I called this closeable. It is not — the guard pins the defect as live
I triaged this as closeable on the strength of its last comment — "Proof landed — PR #13928 merged … two
xfail(strict=True)markers" — and another session caught the error before anything was closed. Recording it here because this issue has a merged PR and a delivered-looking history that will mislead the next sweep exactly as it misled me.The inversion
A
strict=Truexfail asserts today's broken behaviour. It fails the moment the behaviour is fixed, which forces whoever fixes it to remove the marker. So the marker's presence is the strongest available evidence that the defect is still there — the opposite of what I read it as.The test file states this itself, at
autobot-backend/tests/services/test_chat_workflow_is_request_shaped.py:23-27:The two tests below are marked
xfail(strict=True)deliberately. They describe the behaviour the endpoint is supposed to have, so when #13809 is fixed they XPASS — which fails — and whoever fixes it must remove the markers.Verified on merged
main, not against the test's description of itselfautobot-backend/orchestrator.pystill builds the fixed three-step skeleton the issue describes:task_id="step_1" agent_type="analyzer" action="analyze_request" description="Analyze user request" task_id="step_3" agent_type="synthesizer" action="synthesize_results" description="Synthesize results"Hardcoded actions, not derived from the request. AC1 is unmet. The merge added one new test file and zero lines of
orchestrator.py— the commit title says so outright: "pin that chat plans are fixed echoes and must not be".What would have to be true to close this
The plans become request-shaped in
orchestrator.py, and the twoxfail(strict=True)markers are removed in the same change — the markers are designed to fail at that moment, so a fix that leaves them in place turns the suite red. That coupling is deliberate and it is also the closure signal: this issue is closeable exactly when those markers are gone and the tests pass on their own.The general lesson, since this is the second shape of it today
"A guard landed" and "the fix landed" are different claims, and a guard's presence points in whichever direction the guard asserts. A guard asserting the fixed behaviour passing is evidence the fix is in. A guard asserting the broken behaviour — an xfail-strict, a ratchet pinned at today's count, a baseline entry — is evidence the defect is in. Reading the first pattern onto the second is how a well-built guard becomes a false closure argument.
Earlier today the same family appeared pointed the other way: a search whose absence of callers was by design, recorded only in a comment at the declaration. Both are cases where the artifact's defence looks like the thing it defends against.
No state changed — nothing ticked, closed or relabelled. The correction is mine; the catch was not.
Owner ruling, 2026-10-03 — planner, approval default, and the security test
Two questions were put to the owner in the session implementing this issue. Recorded here so the PR and the criteria can cite them.
What the code showed before asking.
_extract_command_from_stepreturnsstep.inputs["command"]verbatim when present. If a real planner writes that field, user text reaches the generated command by design. So the property pinned bytest_the_user_request_never_reaches_a_commandholds today only because the endpoint is hollow, and it cannot survive a working fix. A second finding:create_workflow_plan's prompt (orchestration/orchestrator_prompts.py) asks for agent tasks{agent, action, inputs: {}}and never for a shell command. Rerouting alone would produce request-shaped actions whose steps still all runecho 'Executing: <action>'.Ruling:
- Planner (AC4):
create_from_chatusesorchestrator.create_workflow_plan, the single canonical planner.plan_workflow_stepsstops serving chat requests. - Command generation (AC1): an opt-in contract on the shared planning prompt asks for
inputs.commandon shell tasks. Only this chat path uses it; every other caller's prompt is unchanged. - Default (AC3): the server default becomes
require_approval=True,auto_start=False. The frontend's only caller (useWorkflowBuilder.ts) already defaults torequireApproval=true, so this removes a server/client divergence rather than tightening a default anyone relies on. - Per-step confirmation: every step whose command the LLM authored carries
requires_confirmation=True. That holds as a property of the step that executes, independent of the route default. - Security test:
test_the_user_request_never_reaches_a_commandis replaced by an assertion of the property that does hold with a real planner: no generated command runs without human approval. It is asserted on behaviour (route defaults and step flags), not on tokens in generated text.
- Planner (AC4):
- added a commit that references this issue
on Oct 10, 2026
Discovered by verifying the effect of #13730 rather than assuming it.
Context
Before #13730,
create_workflow_from_chat_requestreturnedNonefor every request andapi/workflow.py:292turned that into HTTP 500. The path is now live for the first timesince the #5058/#6951 migrations. That is an improvement, but the path it unblocked does not
produce a useful workflow.
Finding
Every chat request produces the same three placeholder steps, whatever was asked:
Two independent reasons the user's request never reaches the command:
orchestrator.plan_workflow_stepsreturns a hardcoded1-step (SIMPLE) or 3-step (COMPLEX) skeleton with actions
analyze_request/execute_plan/synthesize_results. It puts the request ininputs["query"]and neverderives an action from it. Note this is not the LLM planner —
orchestrator.create_workflow_plandoes real planning, but this path does not call it.manager._extract_command_from_stepfirst looks forstep.inputs["command"], which this plan never sets (it setsquery), then falls back tokeyword-matching the action — but the actions are the three fixed strings above, which
match none of its
update/install/searchbranches. So every step lands on the finalecho 'Executing: {action}'branch.Combined with #13807 (classification silently disabled → every request is
COMPLEX), theendpoint returns an identical 3-echo workflow for every input.
Security note: no injection here
Worth stating explicitly since this generates shell commands: the user's text does not flow
into the command.
_extract_command_from_steponly ever emits a literalechoof a fixedaction name, or a hardcoded
apt/findstring on keyword match — and those branches areunreachable from this plan. Verified with a deliberately hostile request. This issue is about
the capability being hollow, not about an injection risk.
Why file it rather than leave it
/api/workflow-automation/create_from_chatdefaults toauto_start=Trueandrequire_approval=False, so the default path now creates and starts a workflow that doesnothing meaningful — where it previously failed loudly with a 500. A silent no-op is harder to
notice than an error, so unblocking the path without this follow-up trades a visible failure
for an invisible one.
Options
orchestrator.create_workflow_plan(the real LLM planner) instead ofthe fixed
plan_workflow_stepsskeleton — likely the intent, and it converges the twoplanning paths rather than adding a third.
plan_workflow_stepsderive real actions/commands from therequest.
than leaving a live route that returns placeholder work.
Recommendation: 1, consistent with
create_workflow_planalready being the canonicalplanner (see the supersession record in #13751).
Acceptance criteria
fail such a test.
auto_start=Trueremains the right default for a path that executesgenerated commands, and record the reasoning.
plan_workflow_stepsandcreate_workflow_planserving chat requests.