apply_repair's preflight raises PreflightError when the graph it is about
to write would fail at Graph.init. Every handler caught it as a plain
ValueError, so the five apply sites could not tell it apart from a
graph_ops rejection:
- plan approval (Build, Edit) called ANY ValueError "The workflow can't
start", including "node not found: x", sending the user hunting for a
broken node that does not exist;
- the repair-apply gates (Build/Edit await_repair, Fix apply) called a
preflight rejection "The proposed fix no longer applies to the current
draft" -- the fix applied fine; it would leave an unstartable draft.
Add core DraftWouldNotStartError(ValueError) and make PreflightError
subclass it (services imports core; core still imports no services).
Each site now catches it before the ValueError branch, with the same
control flow and the true reason: plan approval keeps "The workflow
can't start" for it and says "Couldn't apply the workflow" otherwise;
the repair gates say "The workflow can't start" for it and keep
"Couldn't apply the fix" otherwise. The shared card/turn construction is
factored (_graph_not_applied, _change_not_applied, and
drop_unapplied_repair in handlers_fix) and the new literals are
catalogued in strings.py.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The unknown-outcome branch (a stream that ended with neither a terminal
nor an error frame) returned the user to the re-run state with no
limit in all three flows. With error frames now reported as failures
that branch is rare, but a stream that keeps ending early could still be
re-run forever. DifyBuilderContext.unknown_outcome_count counts
consecutive unknown outcomes (reset by any determinate one); the second
in a row stops at the gate with a card telling the user to look at the
run itself.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ESQ1-302 and ESQ1-303 both died at Graph.init -- before workflow_started,
so the Builder saw an error frame with no run id and no rows, and spent
runs and approvals discovering what a validation pass could have said
up front. apply_repair now runs that pass (preflight_errors: the same
node-data validation create_node performs, with no factory or runtime
state) over the graph it is about to write and raises PreflightError,
naming each node Graph.init would reject.
Only NEW problems count: a pre-existing broken node the repair does not
touch must not veto an unrelated fix, and a repair that heals it passes.
PreflightError is a ValueError so the handlers' existing stale-intent
guards already surface it as a card.
The real ESQ1-303 dev draft is the fixture: preflight names node2 and
its numeric value, and passes once the value is a string.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
run_draft forwarded the pipeline's ``event: error`` frame to the frontend
and then dropped it, so the ESQ1-302 launch failure (Graph.init rejected
an http-request node before workflow_started) fell into the
truncated-stream path and was reported as "running". The handler's
unknown-outcome branch then sent the user back to re-run, forever.
The loop now keeps the first error frame aside and _finish_run applies
one precedence: terminal frame > error frame > nothing. Chatflow emits
workflow_finished(failed) AND an error frame for one failure, so the
terminal frame must win; a stream that ends with neither still reads as
an unknown outcome, never as a fabricated failure.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The streaming pipeline reports a run that threw as one ``event: error``
frame with top-level message/code and no data (ESQ1-302: Graph.init
rejected a node before workflow_started, so there is no run id either).
run_mapping had no representation for it: _TERMINAL_STREAM_EVENTS lists
only workflow_finished/paused, so the frame was neither terminal nor a
node event and the run came back "running" with the truncated-stream
notice.
error_from_stream_chunk reads such a frame; map_error_frame_run turns it
into a FAILED Run carrying the message (whitespace collapsed, so it is a
stable repair-breaker signature) and the code. map_run_result now keeps
the engine's run-level error text for a failed run instead of dropping
it -- a launch failure has no per-node rows, so that text is the only
evidence diagnose and the breaker have.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
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>
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>
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>
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>
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>
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>
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>
submit_requirements now tail-calls a shared _discover_and_offer_resources
helper straight into build.resource_recommendation, instead of stopping at
build.initial_plan to show a plan card the user must click through with no
real decision to make. The resource-discovery body (progress reporting,
discover_resources, ResourceSelectCard, empty-options notice, assistant
turn) is extracted verbatim into that helper so the straight-through path
and the continue_adjusting/retry_after_revert loop-back (which still
re-enters at build.initial_plan and still requires find_resources) share
one implementation instead of drifting apart.
build.initial_plan survives as the loop-back target but now offers no UI
action (_ACTIONS_FOR entry removed); its handler still accepts
find_resources at the backend for that loop-back.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The resource-recommendation step made the user pick a "conflict
policy" that never decided anything. Remove it end to end: the
ConflictPolicyOption card option, ResourceSelectCard's
conflict_policy_options field, the third bind_resources() parameter
across the port/placeholder/LLM-agent implementations, and the dead
conflict_policy field on the (unused) ResourcesPayload contract type.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
discover_resources initialised its selection to the entire tenant inventory and
only replaced it when the model returned a non-empty pick, so three paths -- no
model, a model that picked nothing, a failed call -- recommended everything. On
a workspace with 57 resources that is not merely noisy: the client pre-checks
every recommendation and submits them all by default, and bind_resources then
appends all 57 labels to the last plan item, which is the text fed to the graph
generator.
Recommending nothing is the honest answer when there is no signal. The card is
still emitted so the confirm gate keeps its active interaction and a later pass
cannot inherit a stale one, and a notice says why rather than leaving a blank
card. The conflict-policy control no longer renders over an empty list.
The test that should have caught this used a one-resource catalog, where "all"
and "none" are indistinguishable; it now uses 57.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The testdata gate is the one place a human types for tens of seconds, and it
opens right after Builder has written the canvas -- long enough for the editor's
own autosave to move the app revision underneath them. Both revision gates then
refused the submit, and because a conflict leaves the session with only
check_recovery as an action, active_interaction drops to None and the client
freezes every input: the form stayed on screen but could no longer be typed into.
A testdata submit carries run inputs, not graph edits, so neither gate applies
to it. Exempting only the draft-changed check would not have been enough --
the stale-app-revision check above it fires first, on the base_app_revision the
client has held since the form was issued.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Spec N4 has the build-complete card headline read "<App name> is ready", with
the annotation 名字只在回复句里念一次；记录卡上不出现 -- said once, in that
sentence, and never repeated on the completion receipt. We said it nowhere: the
card was titled "Build plan" and the engine had no app name to say.
DifyBuilderContext now carries app_name, seeded at session creation through a
get_app_name_fn seam alongside the existing revision/authorize ones. Env.rename_app
returns the name the database actually took rather than None, so the refine
records what landed instead of what it proposed -- the service normalizes after
the engine hands off, and the two can differ.
The headline is interpolated, so it gets a strings.py template or it would
survive localization untranslated. That pattern matches almost anything ending
in "is ready", so it sits last and every more specific template gets first
refusal.
The receipt already omitted the name; there is now a test pinning that.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two App Builder spec changes that share the Builder session contract.
Naming (spec N1-N3): an app created from the homepage prompt is no longer named
by the user. core/dify_builder/naming.py cuts the prompt's opening clause at
create time -- instant and infallible, since it runs inside POST /apps -- and
CreateAppPayload.name becomes optional alongside a new prompt field. The Builder
then refines that name once during goal analysis, where a model and the detected
reply language are already in hand, so the judgement calls that are
English-shaped in pure Python (dropping an imperative lead-in, spotting a
contentless prompt) work in any language. Plan approval freezes the name.
Card options: SessionView gains a Decision carrying the gate's choice as options
inside the card plus a fixed confirm/cancel pair, replacing a button row whose
shape changed at every state. Option ids are the old action ids untouched, so
resolve_action_kind and the per-state legality check are unchanged, and
SessionView.actions keeps being populated -- the deployed web client goes on
working until it reads decision instead.
The regenerated console OpenAPI doc also picks up ErrorCard.diagnostics, missed
when cc9ca8b1dd landed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
api pods lose their logs on restart, so a failed build's real cause
("Workflow generator: structural validation failed: ...") could only be read
from a pod log that may already be gone. Session 01a08fd0 hit exactly that.
Carry the generator's own diagnostics back with the result instead:
BuildNodesResult and ErrorCard gain a ``diagnostics`` list, and build_nodes
records one entry per failed generation attempt -- initial AND the corrective
retry, so an error that changes between attempts stays visible -- plus the
all-rejected and exception paths.
Each entry holds a server-side UTC ``at`` (so the surviving pod-log window can
be searched around it), ``source``, ``attempt``, the generator's ``message``
(the same text the server logs), its ``codes``, and the full structured
``errors`` including ``node_id``. That is strictly more than the log line
carries: for an UNRESOLVED_REFERENCE the export now names the offending node.
The list rides in the ErrorCard's item payload, which is what the streamed
conversation item -- and therefore the exported debug log -- preserves. It is
not rendered in the UI. Retries that eventually succeed keep their breadcrumbs.
Deliberately not mirroring the "structural validation failed" prefix: build.py
cannot tell structural failures from the rest without duplicating the
generator's code taxonomy, which would go stale.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
When build-generate-graph failed at plan-approval, the error card always
showed one hardcoded generic message regardless of the actual cause
(UNRESOLVED_REFERENCE, non-object JSON, credit_balance_exhausted, ...).
Root cause: build_nodes() returned only list[MutationIntent]; on failure it
returned [], so the reason was logged internally but never reached the
handler, which had nothing to show but a fallback string.
Thread the reason out: build_nodes now returns BuildNodesResult{intents,
error}. _generation_error_text() extracts the best reason (top-level error
-> joined errors[].detail/message/code -> generic fallback), and every
failure path populates .error (generator error, all-intents-rejected, and
the except path via str(exc) so provider errors surface verbatim).
handle_plan_approval builds the ErrorCard body from build_result.error when
present, keeping the generic sentence only as a genuine fallback.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Build wrote the Builder's session/cognition model (e.g. deepseek) into every
generated node's model block, ignoring the model the user picked at resource
confirmation (e.g. gpt). _ground always used the generation model, and the
selected model resource only reached the plan as label text (bind_resources) —
so a workflow got a model with no runtime credentials -> ModelNotExistError ->
instant test failure.
Thread the selected resource_ids from resource_selection through
agent.build_nodes into build.build_nodes, resolve the selected MODEL resource
(id is `{provider}/{name}`) via _selected_workflow_model, and ground node model
blocks to it — falling back to the session/generator model when the user
selected no model. The generator still runs on the session model (cognition);
only the built workflow's runtime model follows the user's choice.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Harden the Localizer's batch translation against models that format JSON
differently (the Builder model is user-changeable):
- correlate by numeric INDEX instead of echoing source strings as keys, so a
model that normalizes/truncates long keys no longer misses every lookup
- accept the reply as an index-keyed object OR a positional array, optionally
wrapped one level (_translations_by_index)
- cache only real translations; a failed/partial reply is left UNCACHED so the
next turn retries (self-heals) instead of sticking on the English original
TDD: added parser shape-variation tests + a no-cache-failure/self-heal test;
the fake now speaks the index protocol.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Live e2e revealed M2 never translated the static catalog strings: the model
returns the batch-translate mapping wrapped under a "strings" key (echoing the
request envelope) — {"strings": {src: translation}} — so _fill_cache's
top-level table.get(src) missed every source and cached the English original.
Unwrap one level when the sources aren't top-level keys, and tighten the prompt
to ask for a flat object. The unit-test fake now mirrors the real wrapped shape
so this can't regress (a flat fake hid the bug).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up to the reply-language feature. The update_model action commits its
"Model changed to <name>" notice directly via compare_and_advance, bypassing
Runner._commit and thus the Env.localize_items hook, so that one notice stayed
English. Localize it in the service (when the session has a detected
reply_language) with a "Model changed to {name}" catalog template that
translates the frame and re-inserts the model name verbatim.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up to the reply-language feature. `_model_or_none` was private to
LlmBuilderAgent, so the advance-task wiring reached it via a fragile
`getattr(agent, "_model_or_none", lambda: None)`. Make it part of the
DifyBuilderAgent protocol (typed `object | None` to keep the pure port free of
any model-runtime import) and implement it on every conforming agent
(LlmBuilderAgent renames the private method public; PlaceholderAgent and the
test doubles return None). The Localizer wiring now uses `agent.model_or_none`
directly. Also drops the useless-return ruff nit in the wiring test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Final-review fix wave for the "reply in the user's input language" feature:
- serde.py: context_from_dict omitted reply_language from the explicit field
allowlist, so it round-tripped to "" on every read despite context_to_dict
(asdict) serializing it -- the localization feature silently no-op'd after
the first persisted write. Added it to the constructor call; TDD'd with a
failing-then-passing round-trip test in test_serde.py.
- localize.py: Localizer._cache was per-instance, but a fresh Localizer is
built per advance-task invocation, so the static string catalog was
re-translated every turn instead of once per language. Moved the cache to a
module-level _TRANSLATION_CACHE dict (safe to share: keys are
tenant/session-independent catalog-string+language pairs); added an autouse
fixture in test_localize.py to clear it between tests for isolation.
- localize.py: detect_language and _fill_cache called self._model_provider()
outside their try/except, violating the Env callback contract that these
must never raise. Moved both calls inside their try/except so a raising
provider degrades to the existing safe fallback instead of propagating into
advance/_commit.
- strings.py: documented that Template.translate_fields is advisory only --
the Localizer re-inserts all captured groups verbatim and does not consult
it. No behavior change.
uv run --project api pytest tests/unit_tests/core/dify_builder tests/unit_tests/services/dify_builder -q
-> 722 passed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The build/edit test handlers wrapped run_draft in a bare, non-logging
`except Exception` that turned any launch-time failure into a blank failed
run (dify_run_id="", per_node=[], no error). Diagnose then had nothing to
act on and the repair loop ran blind — the prod symptom where a stale test
input (ValueError "<var> is required in input form") produced an
undiagnosable await_repair loop.
Now the launch error is logged, captured, and persisted so diagnose/routing
can act on it:
- Run.error field + dify_builder_runs.error column (migration a9cc8d552db4)
- handlers_build/edit: logger.exception + launch_error_text(exc) -> Run.error
- is_input_failure() also inspects Run.error (+ "in input form" signal), so a
stale-input launch failure routes to the testdata gate, not blind repair
- diagnose/_degraded_diagnosis surface the launch error when no node executed
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Render change sets directly in plan-style cards with affected node names,
IDs, and change details. Capture node metadata for proposed and applied
changes and persist it with conversation history.
Remove the view_changes action and expand/collapse state. Update generated
contracts, translations, and regression coverage.
Validation: 112 frontend tests and 742 backend unit tests passed.
Integrates the FE team's live-progress + conversation-streaming rework
(durable progress events, repository layer, snapshot-consistent reads,
idempotent message retries, EDIT_PUBLISH->EDIT_COMPLETE) and reconciles its
interrupted-step recovery model with the reconciler already on
feat/dify-builder.
Combined recovery model:
- interrupted (crash mid working-step, lock released, nothing durably
recorded) -> Retry (recovery_continue, re-runs the handler) + Start over
- FAILED (handler raised, recorded by runner.fail) -> Restart only
- a Retry that itself raises lands in FAILED -> Restart (no wedge)
Kept from the PR: EDIT_PUBLISH working / EDIT_COMPLETE terminal, snapshot
refresh in apply_recovery_action, paused reset. Kept from the reconciler:
Retry on interrupted, state-aware access tier (EDIT_PUBLISH added to the
publish tier), apply_repair idempotency filter.
680 dify_builder unit tests pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A recovery_continue (Retry) re-runs the interrupted working handler, which
re-invokes the state's privileged op -- publish (RELEASE) at build.publish/
fix.publish, a draft test run (TEST_AND_RUN) at the test/verify states. Before
this, recovery_continue was classified as base EDIT, so a Retry re-ran the
privileged op below its own authorization tier without re-checking. Make
_app_access_for_action state-aware for recovery_continue so it enforces the
same tier the state's normal action requires. recovery_restart only resets +
re-enters the entry state, so it stays EDIT.
Also: an interrupted working step now always surfaces the Retry/Start-over
offer, overriding a stale drift recovery_class a user may have left set at an
earlier waiting gate (drift recovery only applies at a waiting gate).
Addresses the two findings from the final reconciliation review.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When a session crashes mid working-step (e.g. build.publish; the async advance
raised and released the lock), it was a locked dead-end: working states surface
no UI actions and the composer locked with no way forward.
- Surface a RecoveryRef offer (can_continue=Retry, can_restart=Start over,
recovery_class="") on an interrupted working-state view -- built directly so a
real drift class still wins.
- Allow recovery_continue / recovery_restart at an interrupted working state in
_internal_action_allowed. recovery_continue returns the working state
(non-settled) so the runner re-runs the stalled handler; recovery_restart
resets working fields and re-enters the entry state -- both reuse existing
recovery machinery, no new action kinds / no runner change.
Adds unit tests (offer surfaced, allowance) + regression tests (recovery_continue
re-runs handle_publish and advances; recovery_restart resets to entry state;
repeat-failure re-surfaces the offer with no wedge).
This reconciler was reviewed as a standalone plan on the pre-merge base; here it
is re-applied onto feat/dify-builder after PR #41677 merged in the reworked
service.py + _internal_action_allowed gate (which makes the allowance necessary).
The apply_repair idempotency + delete-guard commits (safe fix.apply Retry) land
alongside it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Charles YaoandClaude Opus 5
twwu
Wu Tianwei