Run workflow streaming inside the Builder worker to avoid queue deadlocks. Forward native events to shared editor callbacks and wait for graph refresh before retesting repairs.
Keep command cleanup stable, preserve batched canvas events, and allow failed or unfinished historical runs to refresh again.
Starting a NEW Build session on an app whose draft already holds a previous
build's graph silently changed nothing and still reported "graph built".
_already_present matches a create_node by node id ALONE, and the generator
always emits node1, node2, ... So a new plan whose node2 is a
parameter-extractor was filtered out against an existing node2 that is an
llm: to_apply became empty, apply_repair([]) no-oped, change_set fell back
to "graph built", and the session advanced to build.execution over an
untouched canvas. Observed live -- the snapshot taken before apply was
byte-identical to the draft after it, and the subsequent test run "passed"
because it ran the OLD graph.
The id filter itself is deliberate and stays: it is what makes a loop-back
re-approve (continue_adjusting / revert) idempotent instead of raising on a
colliding id. The guard therefore keys on built_node_ids, which names what
THIS session built -- empty only on a session's first build, which is
exactly the broken case. A loop-back still no-ops silently and advances.
Does NOT clear the canvas on the user's behalf: that would destroy an
existing workflow. It reports honestly and stays in plan_approval.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_repair_is_repeating could never fire in production. _note_repair_error
counted repeats by exact string equality on diagnosis.root_cause, which is
LLM prose regenerated on every diagnosis call. Observed live across four
turns of one session: same failure, reworded every time, one turn switching
to Chinese. Equality therefore never held, repair_attempts reset forever,
and the guard was unreachable -- that session burned 9 failed verify runs
and 8 repair approvals with the guard in place.
The call site's own comment reads "ESQ1-285 burned nine approvals this way,
ESQ1-290 six". The reproduction burned nine again.
Key on the ENGINE's output instead: _failure_signature(run) returns the
first failed node's id plus its error text (whitespace-collapsed), falling
back to the launch error, and "" when the run identifies nothing -- so two
unknown failures never compare equal.
Why the existing tests passed while production was broken: every one of them
fed _note_repair_error the SAME literal string twice, and StubAgent.diagnose
always returns one fixed root_cause. They encoded the premise that is false
in production. The new handler-level test drives three turns with an agent
that rewords its root_cause each call while the engine fails identically,
and fails against the old implementation with repair_attempts == 0.
last_repair_error now holds that signature rather than prose. It was never
displayed -- its only use was this comparison.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The brief's one-line _FakeGen.generate_workflow_graph signature carried a
single trailing `# noqa: ARG004`. `ruff format` -- which the plan mandates
after `ruff check --fix` -- reflowed the >120-char signature to one argument
per line, leaving the noqa attached only to `current_graph` and orphaning
tenant_id, mode and model_config. ruff scopes noqa per PHYSICAL line, so
`ruff check` then failed with 3 ARG004 errors while pytest stayed green.
Lint-only; no behaviour change. Verified that `ruff format` now leaves the
file unchanged, so the annotation survives the format pass that broke it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Meeting item 9 asked for per-node usage detail in the generator prompts.
Those prompts are shared with cmd+K, so the detail rides the channel the
Builder already owns instead: _WORKFLOW_TOPOLOGY_DIRECTIVE, prepended to
every generator instruction at build.py:377 and invisible to cmd+K. That
mechanism is proven -- it is why workflow mode never gets 'answer' nodes.
The rules added are the ones that broke real builds: tool nodes expose
text/files/json and not 'result' (ESQ1-296, ESQ1-289), document-extractor
needs a file input, container children belong inside the container
(ESQ1-288), if-else operators must match the variable's type
(ESQ1-285/290), and http-request inputs belong on the start node
(ESQ1-286).
Deliberately does NOT restate the node whitelist: the generator's shared
system prompt already carries it, and duplicating it would burn planner
output budget that ESQ1-300 already implicates.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
propose_plan_v1's system prompt was two sentences with no node vocabulary,
so it planned in business language -- "Send a Feishu notification",
"Connect to the database". Those steps became the generator's
"# User instruction" verbatim, asking it to map work onto nodes that do
not exist, and the failure only surfaced at build-generate-graph, minutes
after the user had approved the plan (ESQ1-279).
The generator already knows the node types: its SHARED system prompt
carries the whitelist. The gap was upstream, in the Builder's own planner.
Naming the vocabulary here makes that existing whitelist sufficient.
Builder-only: nothing under core/workflow/generator/ is touched, so the
cmd+K /create and /refine features are unaffected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two defects, both introduced by the streaming switch in 3838db8aa7, which
together livelocked every Builder test run.
1. Self-deadlock. run_draft executes inside the Celery task
dify_builder_advance_task.advance_session. Streaming mode does not run
the workflow in place: it enqueues workflow_based_app_execution_task
via delay() on first subscribe (app_generate_service.py:322-324) and
returns a Redis subscription. The advance task then blocked on events
from a task needing the worker slot the advance task was holding.
Observed live: advance_session "succeeded in 300.59s" (the idle
timeout) four times over, while each workflow_run finished in under 2s.
Deadlocks every time at worker concurrency 1, and as soon as N test
runs are in flight at concurrency N.
2. Lost outcome. Blocking mode returns a Mapping
{"task_id", "workflow_run_id", "data"}, but the rewrite consumed the
response with `for chunk in response`, which walks a dict's KEYS as
strings. Probed against the real service: 3 chunks, 0 run-id matches,
0 terminal matches. So every run was recorded status="running" with an
empty dify_run_id, and the "running" branch bounced the user back to
build.execution -- forever. The pre-3838db8aa7 code read
response["data"] directly; the rewrite dropped that path.
Blocking mode restored, Mapping responses handled, and the shared tail
factored into _finish_run so the two paths cannot disagree about an
outcome. The stream-consumption path is kept and still tested, for a
future consumer that is NOT a worker -- which is what live per-node
streaming to the frontend will require.
Verified end to end on a local stack: one test run, status succeeded with
a real run id, advance_session 0.04-31.8s, and the session reaching
build.review instead of looping.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four findings from the whole-branch review, all in the repair loop:
I1 -- repair_attempts/last_repair_error were added to the context and to
serde but not to recovery._reset_working_fields (which clears eleven sibling
fields) nor to the re_fix/restart re-plan branches, so a fresh build could
inherit repair_attempts >= the ceiling and decline a repair on its FIRST
failure. They are cleared with their siblings now. The guard also ANDed a
monotonic counter with a consecutive-error test, so A -> B -> B tripped it
although B had survived one repair. _note_repair_error now makes the counter
mean what the docstring says: consecutive repeats of ONE error, reset when
the error changes.
I2 -- the repeated-failure return dropped run/run_id_sink, so repo.save_run
never ran while fc.verify_run_id already pointed at that run: a dangling
pointer and the third failing run lost from the debug export.
I3 -- ESQ1-291's empty-staged-repair guard existed in Build only. Edit's
model-config branch GUARANTEES an empty staged repair and routes to a state
whose approve applied it blindly; handlers_fix.handle_apply had the same
shape. Both are guarded now, reusing Build's notice.
I5 -- apply_repair re-validates intents against the draft as it is NOW, so a
dotted path that was applicable at propose time can raise ValueError at
approve time. Uncaught at all three approve sites, it reached the runner and
drove the session to FAILED -- the exact ESQ1-271 outcome this branch set out
to remove. Each site now degrades to a "Couldn't apply the fix" error card
naming the reason and clears the staged repair (engaging the I3 guard).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous ESQ1-288 fix dropped the iteration/loop start marker, stripped
start_node_id from the container config and dropped the generator's container
entry edge. All three are required by the ENGINE, not just the canvas:
IterationNodeData.start_node_id / LoopNodeData.start_node_id have no default,
node data is validated eagerly at graph construction, and the body frame is
rooted at start_node_id. Every container workflow the Builder produced was
therefore non-runnable -- worse than the canvas crash it replaced.
The real root cause was never the marker node. The generator already emits it
with the correct node-level ReactFlow type ("custom-iteration-start" /
"custom-loop-start", runner.py:1350-1362); _build_node hardcoded "type":
"custom" and threw it away, leaving a node the frontend has no component for
(React error #130).
Carry it through instead, the same way the previous commit carried
parentId/position: a defaulted `flow_type` on _build_node / apply_create_node
(absent -> "custom", so every existing caller is byte-identical), set from the
source node's type in to_intents. The marker, start_node_id and the entry edge
are all persisted again.
Tests: the three tests that asserted the ABSENCE of the marker, start_node_id
and the entry edge are replaced by ones asserting their presence, plus the
round-trip test that would have caught this -- to_intents -> filter_applicable
-> APPLY_FNS -> IterationNodeData/LoopNodeData.model_validate on the persisted
container.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
to_intents read only id and data, so every node-level ReactFlow key --
parentId, extent, zIndex, position -- was dropped on each Builder build.
Iteration and loop children were written un-nested, not inside their
container at all, and the generator's computed layout was replaced by
default positions. The data-level markers (isInIteration, iteration_id)
survived via config, which is why the graphs looked almost right.
apply_create_node already took position; parent_id is new and implies
extent="parent" + zIndex=1002, matching what the generator computes.
Unfiled; found while tracing ESQ1-288. Skipping the synthetic start node
stops the canvas crash, but an iteration whose children are not inside it
still does not work.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Builder-generated iterations/loops crashed the canvas permanently with
React error #130 or TypeError. The backend synthesizes container-start
markers correctly typed, but to_intents was recreating them as generic
nodes, and persisting dangling start_node_id pointers.
Fix three related issues:
1. Skip creating synthetic iteration-start/loop-start nodes (frontend
regenerates them at render time from the container).
2. Strip start_node_id from container configs; the frontend's
workflow-init.ts rewrites it to match the marker it creates, and
persisting our pointer causes TypeError on dereference without guard.
3. Skip edges whose source or target is a skipped node; the generator
emits entry edges to the synthetic marker, which no longer exist.
The generator's own output (cmd+K) is untouched — only the Builder
translation layer is fixed. Zero blast radius.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round-1 review defect: the TOOL branch added for ESQ1-296/ESQ1-289 assumed
output_schema was always a dict once truthy. A non-dict output_schema (or a
non-dict `properties` inside it) raised AttributeError instead of failing
open. That crash is new -- the old branch never read output_schema at all --
and it happens outside the _run_stage try/except and outside the
controller's narrow exception allowlist, so it would take down the SSE
generator mid-stream for both cmd+K and the Builder.
Reachable via refine mode: _hydrate_tool_nodes only stamps a schema when it
finds a matching catalogue entry, so a hand-edited or legacy graph can carry
forward an output_schema of any shape.
Fix: shape-check before every .get() -- output_schema must be a dict and its
properties must be a dict, or the branch returns True.
SHARED WITH CMD+K: fails open whenever the schema is absent, empty, or an
unreadable shape, so a tool we cannot describe behaves exactly as before.
Only provably-wrong references, from a schema we can actually read, are
rejected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_declares_variable returned True unconditionally for TOOL nodes, so any
invented downstream reference survived generation. ESQ1-296 and ESQ1-289
are the same three lines: code/simple_code is a TOOL node, so the real
code-node validation directly above never saw it. They differ only in
consumer -- End tolerates the missing variable and yields a silent null,
if-else fails at run time and feeds the repair loop.
Tool outputs are not opaque: tool_node.py fixes text/files/json and the
generator already copies the provider's output_schema onto the node.
Validate against both, and alias the invented `result` to `text` so the
common mistake self-heals.
SHARED WITH CMD+K: fails open when no output_schema is present, so a tool
we cannot describe behaves exactly as before. Only provably-wrong
references are rejected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Critical defect from Task 3 review: context_from_dict rebuilt
DifyBuilderContext field-by-field and never read repair_attempts or
last_repair_error, even though context_to_dict (asdict) wrote them.
BUILD_AWAIT_REPAIR is a waiting state, so every approve_repair is a
fresh turn that reloads the context via context_from_dict on
submit_action -> repair_attempts silently reset to 0 every turn,
incremented to 1, and could never reach _MAX_REPEATED_REPAIRS. The
_repair_is_repeating guard added in Task 3 was therefore structurally
unreachable in production (ESQ1-285/290's dead loop, still alive).
Adds both fields to the context_from_dict constructor call, following
the existing per-field coercion idioms (int() for the counter, matching
checkpoint_seq; bare .get with a "" default for the string, matching
its string-field neighbors).
Also adds:
- A round-trip + defaults-when-absent test pair in test_serde.py,
following the file's existing convention, reproducing the exact
reload-drops-fields regression.
- An integration test in test_handlers_build.py that drives the
"Repeated failure" branch through handle_test_and_repair itself
(StubAgent + FakeBuildDifyPort, not a direct _repair_is_repeating
call), so a future regression that re-disconnects serde from the
handler would be caught even if the pure guard-function unit tests
still pass in isolation.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
handle_await_repair offered "Apply the fix" unconditionally, so an empty
staged_repair produced approve -> apply nothing -> retest -> same failure
-> offer again, with no LLM in the loop. ESQ1-291's trace shows five such
approvals, every change set {count: 0, changes: []}.
Adds repair_attempts / last_repair_error to the context (both defaulted,
so persisted contexts deserialize unchanged) and _repair_is_repeating for
the follow-on gate. An empty staged repair is now declined with a decision
the user can act on instead of a run that cannot help.
_repair_is_repeating is also wired in at the point a repair is OFFERED
(the config-failure path in handle_test_and_repair, right after
diagnose()): a third identical failure clears staged_repair and surfaces
a "Repeated failure" card instead of proposing a fourth repair
(ESQ1-285/290 burned nine and six approvals this way).
Also catalogs the two new user-facing literals in core/dify_builder/
strings.py so test_catalog_covers_handler_literals keeps passing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An LLM-supplied int node_id reached diff_graphs, which drops falsy and
non-string ids from its node maps, so result.changes came back empty and
build_change_set's fallback joined a list containing an int. The
TypeError took the whole session to `failed` (ESQ1-271) after the
mutation had already been committed to the draft.
validate_intent_args now rejects a non-string or empty node id, so
filter_applicable turns it into a rejection reason the agent can act on.
build_change_set str()s defensively -- that join must never be what kills
a session.
Also extends the same guard to set_node_config's `path` arg: a non-string
path used to reach _resolve_path's str.split() and raise AttributeError,
surfacing "'int' object has no attribute 'split'" to the repair agent
instead of an actionable rejection reason. Same boundary, same defect
shape, deliberately included here rather than deferred.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
apply_set_node_config wrote node["data"][path] as a flat key, so every
dotted path the repair agent emits (cases.0.conditions.1.value) created a
junk key the workflow engine ignores. The dry run passed, the change set
printed a plausible line, and the graph never changed -- which is why
ESQ1-285 survived nine approvals and ESQ1-290 six.
Paths now resolve through lists and mappings, and an unwalkable path
raises. Raising is half the fix: filter_applicable dry-runs intents
through the real apply_* and feeds rejection reasons back to the repair
agent, so a bad path now produces corrective feedback instead of a
silent no-op.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mocking the inputs removed the work of inventing test data, but it also
removed the user's sight of what was actually tested. A green check
produced by {"topic": "test"} is weak evidence about a workflow, and an
invisible mock is a quieter version of the same false confidence a
passing-but-unverified test gives.
So keep the gate and pre-fill it: the form arrives with the mocked values
already in it, unfrozen, so the common case is one click rather than N
fields to type, and anyone who cares can edit before running. The cost
item 5 removed was PRODUCING the data, not confirming it.
Upload fields stay empty and still ask, because nothing can mock a file --
and the mock generator falls back to a plain string for types it does not
know, so it would otherwise pre-fill a file field with "test". New
``upload_variable_names`` and ``without_upload_values`` strip those values;
``needs_upload_inputs`` is re-expressed on top of the first rather than
duplicating the type check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The run id already reaches a client over SSE, which is enough to open the
run on the canvas while the page is up. It is not enough afterwards: SSE
frames do not survive a reload, and the persisted card's ``run_ids`` holds
BUILDER run ids -- uuid4s the engine mints, which resolve to nothing
outside it. So on reopening a session the card was the only record of the
result, and the canvas view was unreachable.
Put ``dify_run_id`` on TestResultCard, beside ``run_ids``, wired at all
nine construction sites across the Build, Edit and Fix flows. Now the
canvas is reachable from the durable conversation too, which is what makes
the card's ``output`` genuinely the client's choice to render or skip
rather than the only surviving copy.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Streaming the test run gives the user a live progress bar but no results:
NodeEvent carries node_id/title/status/error and nothing else, so a client
watching the stream learns which node is running, never what it produced.
The results already exist -- the run is a normal InvokeFrom.DEBUGGER draft
run, its node executions are in the database, and the editor's canvas
already knows how to render exactly that view. It was simply never told
which run to open.
So put the Dify run id where a client can reach it:
- NodeEvent gains run_id, filled from the streaming frame. Every non-ping
frame is stamped with workflow_run_id, so this arrives with the FIRST
node event -- while the run is still going, which is the point.
- The node SSE frame publishes it alongside the existing fields.
- mark_test_success / mark_test_error carry dify_run_id in both the Build
and Edit flows, so the terminal state names its run too.
This is what makes per-node results land on the graph, where a node's
inputs and outputs already have a home, instead of only as a JSON blob on
a conversation card.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three minor whole-branch-review fixups:
- ResourceOption.readiness ("ready"/"missing_config") was computed but never
shown to the model: discover_resources's recommender prompt listed ids and
labels only, so it could recommend an unauthorized tool; _GAP_SYSTEM showed
readiness in the listing (assess_capability_gap) but never defined what
missing_config means. Both prompts now show readiness and state that
missing_config is installed-but-not-authorized, i.e. not usable -- pinned
with tests asserting the literal "missing_config" in the constructed
prompts.
- strings.py: dropped three orphaned catalogue entries ("Here's the initial
plan.", "Ask each time", "Prefer audited") -- the conflict-policy concept
they backed was removed end to end; nothing in the handlers emits them.
- core/workflow/generator/tool_catalogue.py: provider.need_credentials was
read OUTSIDE the per-provider try/except that already guards
get_tools()/tool parsing. This module is shared with the cmd+K /create and
/refine flows, so a provider whose property raised would break those too,
not just the Builder. Moved the read inside the try/except so a bad
provider is skipped like any other.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_terminal_output json.dumps'd a node's outputs with no length limit. Node
outputs are only truncated upstream at ~100,000 chars per string, so a
report-generating workflow -- the Builder's showcase case -- could push
hundreds of KB of JSON into a dify_builder_conversation_items row, the SSE
frame, and the localizer walk on every successful build.
Cap the rendered output at 2000 chars with a visible truncation marker
before it reaches TestResultCard.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
run_draft's streaming path introduced Run.status="running" (outcome
genuinely unknown -- the progress stream ended before the run did) as a
third status alongside "succeeded"/"failed", but neither
handle_test_and_repair (Build) nor handle_verify (Fix) knew about it: both
treated anything short of "succeeded" as a failure, so a truncated run got
labeled "Test run -- Failed" and, in Build, had a graph repair diagnosed
and staged against it while the run might still be executing.
Both handlers now branch on status=="running" before the failure path: a
neutral notice explains the outcome couldn't be determined, and the flow
returns to a re-runnable waiting state (build.execution / fix.await_testdata)
without ever calling diagnose or propose_repair.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Add assess_capability_gap: one plain sentence naming a plan step nothing
installed can perform, or "" when the plan is covered. Grounded only in
installed resources -- there is no marketplace lookup, so it names a missing
CAPABILITY, never a product to install. A missing_config tool (installed but
unauthorized, from the prior task's real readiness) doesn't count as
covering a step either, since the model is shown each option's readiness
alongside its label and kind.
Wired through the full agent seam: the DifyBuilderAgent protocol, the real
LlmBuilderAgent (delegates to build.assess_capability_gap), and the
PlaceholderAgent test double (returns ""). handlers_build.py calls it
after discover_resources and, when non-empty, appends a NoticeItem beside
the existing empty-resources notice -- the two are independent and can
both fire.
Like its sibling cognition functions, it degrades to "" (never raises) on
no model, an LLM error, or an unparseable reply: a gap that can't be
assessed isn't a gap worth inventing. The sentence is LLM-generated prose
already in the user's language (json_language_instruction), not a
catalogue entry, so no static string was added.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Builder recommended every installed tool as ready to use without
ever checking whether its provider actually has credentials configured.
A credentialed tool with no tool_builtin_providers row got bound into a
plan and then failed at test time with an authorization error the user
can't act on.
tool_catalogue.py now carries provider_controller.need_credentials onto
each ToolCatalogueEntry as needs_credentials. resources.py resolves
which providers are actually authorized for the tenant (mirroring
ToolTransformService.builtin_provider_to_user_provider's need_credentials
/ db_provider rule exactly, so the Builder and the console's tools page
can never disagree) and sets ResourceRef.readiness to "missing_config"
for a credentialed tool with no authorized provider row, "ready"
otherwise. Datasets and models are unaffected. If the authorization
query itself fails, readiness fails open to "ready" rather than
mass-labeling every credentialed tool unauthorized on a transient error.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Streaming introduced a correctness regression: if the event stream ended
without a terminal frame (idle timeout, dead publishing worker), run_draft
synthesised status="failed" with an empty dify_run_id. A run that had in fact
SUCCEEDED would then be reported as a failed build and sent into the repair
loop. The blocking path would have waited and returned the truth.
The stream is the fast path, not the authority. Every non-ping frame is
stamped with workflow_run_id by convert_stream_full_response, so the run id
is known from the first frame onwards -- long before the terminal frame that
may never arrive. run_draft now captures it from any frame and, when no
terminal frame arrives, reads the run's real status from the workflow-run row
instead of inventing one.
Note that data["id"] is the node-execution id on node frames, not the run id,
so it is only read as a run id on workflow-level frames.
When even the row cannot settle it -- no run id, no row, or a row still
marked running because the worker died mid-run -- the result is reported as
status="running" with an explicit Run.error, which is what Run.status already
documents for "not over / not known". A truncated stream is now either the
truth or a visible unknown, never a fabricated failure.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
run_draft blocked on AppGenerateService.generate(streaming=False) and then
replayed every finished node-execution row through on_event, so the user
watched a spinner for the whole run and then saw every node light up at once.
It now consumes the streaming generator and fires on_event per frame.
Two things the plan assumed that turned out not to hold:
- The stream does NOT yield dicts. convert_stream_full_response yields dicts,
but that runs in the Celery worker; on our side
BaseAppGenerator.convert_to_event_stream (not overridden) re-wraps every
event as "data: {json}\n\n" and every keep-alive as "event: ping\n\n". An
isinstance(chunk, dict) guard would have dropped every frame. New
stream_chunk_as_mapping accepts both shapes and rejects keep-alives.
- The streaming workflow_paused frame carries workflow_run_id where the
blocking response carried id, so map_run_result would KeyError on it.
Adapted at the call site via run_result_data_from_terminal_chunk;
map_run_result's contract is unchanged for the fix/edit flows.
Session lifetime: _dispatch_generate is a plain function, so all of its
Session work is eager, and the generator it returns only reads a Redis
subscription while the run executes in a Celery worker with its own session.
The consumption loop therefore sits outside the `with` block rather than
pinning a DB connection for the length of a workflow run; a test pins this.
The node-execution read stays at the end -- map_run_result still needs those
rows for per_node[].outputs. Only the progress streams.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The task-5 change to handle_execution's run_test branch (gate only on
needs_upload_inputs) had no test driving the file-declaring branch through
the real Runner: the two nearest tests called handle_execution /
handle_await_testdata directly and never connected into one flow, and no
existing full-flow test's generated graph declares a file variable.
Add test_full_build_flow_file_schema_routes_through_testdata_gate_via_fsm:
builds a real session through runner.advance(), mutates the built graph's
Start node to declare a file variable, then asserts run_test lands at
BUILD_AWAIT_TESTDATA with a testdata form card actually emitted, and that
submitting provide_testdata through the Runner (not a direct handler call)
advances it to build.review. Together with the existing (task-5-updated)
fileless full-flow tests, both branches of the conditional are now
exercised through the state machine end to end.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A passing build test reported "All checks passed" and threw away the
run's actual node outputs. TestResultCard gains an `output: str` field
(pretty-printed JSON of the last node that produced anything, "" when
none); handle_test_and_repair's success card now populates it via a new
_terminal_output(per_node) helper.
_terminal_output is deliberately status-agnostic (no node.status filter):
per-node status spellings differ by source ("success" vs "succeeded"), and
a status filter that silently misses would defeat the point of the card.
FakeDifyPort (tests/unit_tests/core/dify_builder/fakes.py) gains a
settable run_outputs dict, seeded onto the success-path NodeOutput's
outputs, so tests can assert on what the card shows without hand-rolling
a Run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
handle_execution's run_test branch used to stop and ask a human for test
data any time none was prepared yet, even when every declared start
variable was something the agent could invent. Add
needs_upload_inputs(schema) -> bool (true only for a declared file /
file-list variable) and gate BUILD_AWAIT_TESTDATA on it: everything else
is now mocked inline via env.agent.generate_mock_inputs and the flow goes
straight to build.test_and_repair. A schema mixing text and file
variables still stops at the gate, since the file can't be invented.
Updates three full-build-flow tests whose generated graph has no file
start variable, so run_test now lands directly at build.review instead of
pausing at the testdata gate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Code review finding: handle_review's re_fix branch and handle_reverted both
emitted PlanCard("Build plan", "v1") before falling through to
build.initial_plan's unconditional resource discovery, which falls through
again to confirm_resources -- which emits PlanCard("Build plan", "v1") a
SECOND time, now with resources bound. Before this task those two cards were
tagged v1 then v2, so they were visibly different plans; now both say v1,
so the loop-back shows the same plan card twice in one pass. Task 2 already
removed this exact duplicate from the straight-through path
(handle_goal_analysis); it should not survive on the loop-back and not on
the straight-through path.
Fix: handle_review's re_fix branch and handle_reverted still draft
fc.plan_items/fc.plan_version_tag (needed by discovery and the eventual
confirm_resources card) but no longer emit a PlanCard or claim cards=["plan"]
on their AssistantTurnItem (now cards=[], matching the existing
handlers_fix.py precedent for a turn with no card). There is now exactly ONE
plan card per pass, tagged v1, on every path: straight-through,
continue_adjusting, and retry_after_revert.
Updated the two direct-handler unit tests that asserted the now-removed
card, and extended test_review_continue_adjusting_then_reapprove_is_
idempotent (an end-to-end Runner-driven test) to assert re_fix adds zero new
plan cards and the subsequent confirm_resources adds exactly one, tagged v1.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Code review finding: handle_goal_analysis built its own ProgressReporter
(build-review-requirements/build-draft-plan) and finish()ed it, then
tail-called _discover_and_offer_resources, which built a SECOND reporter
under the SAME env.operation_id. ProgressReporter's revision counter starts
at 0 per instance, so the second reporter's activate/finish calls restarted
revision at 1 -- two progress events with revision 1 (and 2, and 3) under
one operation_id. contract.py's ProgressEventData docstring states revision
is monotonic within an operation_id; every other handler across the three
handler files uses exactly one reporter per step -- this was the sole
exception, on the most-travelled build path.
Fix: _discover_and_offer_resources now takes an optional `progress`
parameter. handle_goal_analysis passes its live (still-open) reporter
through instead of finishing it early; the helper adds the discovery steps
onto that SAME reporter via add_steps() and is the one that finish()es it.
handle_initial_plan (the continue_adjusting/retry_after_revert loop-back,
which has no reporter of its own -- a separate CAS commit/operation) still
gets progress=None and the helper builds a fresh one, unchanged.
Added test_submitting_requirements_progress_is_one_operation_with_monotonic_
revisions: drives the real Runner with submit_requirements, captures every
emitted ProgressEventData, and asserts a single operation_id with strictly
increasing, non-repeating revisions across all four progress steps.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Code review finding: build.initial_plan was left in _WAITING with zero
_ACTIONS_FOR entries but a handler still gated on find_resources. Since a
waiting state settles the runner's advance loop (is_waiting(next_state) ->
settled), continue_adjusting from build.review landed a session there with
no button the FE could render and no action the handler would accept --
the loop-back could never advance.
Reclassify build.initial_plan as working (state.py _WORKING, not _WAITING):
it is now a genuine pass-through step, matching build.test_and_repair /
build.publish / build.governance_feedback. handle_initial_plan drops its
find_resources gate and runs resource discovery unconditionally on entry;
the runner drives it with the action already consumed and falls straight
through to build.resource_recommendation within the same advance() call.
Removed the now-stale BUILD_INITIAL_PLAN entry from service.py's
_BACKEND_ACTIONS_FOR (no action to gate on any more) and the compensating
exemption it required in test_every_backend_waiting_action_has_a_ui_path
(the invariant now holds with no special case).
Added a state-classification test (test_build_initial_plan_is_working_not_
waiting) and an end-to-end regression driving the Runner from build.review
through continue_adjusting with no find_resources action ever sent
(test_continue_adjusting_reaches_resources_without_find_resources_action),
which is the scenario this fix exists to unwedge. Fixed the two existing
idempotency flow tests that asserted the old (broken) two-step loop-back.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
twwu
非法操作
zyssyz123
Charles YaoandClaude Opus 5
林玮 (Jade Lin)
Sourav Rajvi
盐粒 Yanli
Wu Tianwei