Brings in 104 commits from origin (main merges plus unrelated refactors).
PR #42760's Builder UX refactor has not landed, and only
api/tests/unit_tests/core/workflow/test_node_factory.py overlaps with this
branch's work.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Parity for the two semantic guards, which Task 8 committed to for exactly this
reason: a guard Edit honours and Fix merely implies is worse than no guard,
because the inconsistency is invisible to whoever reads only one of them.
The hole was not the one I reported at the end of round 2. Probed against the
literal S6 batch, `fix.propose_repair` already refuses to hand on a batch
carrying any rejection -- unlike Edit it never had a fallback to the first
attempt, so the condemned batch was never written. What it did have was a BLIND
surface: the engine-grounded reason was discarded and `_no_fix()` returned the
generic "No safe automatic fix found", which the handler then stored on
`fc.risk` and showed to nobody. The human at the gate was told a fix could not
be found and given nothing to act on -- while the guard had just described the
aggregator, the node and the selector that would have made a manual fix obvious.
So: `_no_fix()` takes an optional reason (one dead-end path, not a second), a
`would_run_wrong` verdict fills it in, and `handle_propose` renders it on an
ErrorCard beside the reply line -- the same division of labour Edit's refusal
uses. The verdict is read off the structured `would_run_wrong` list, never off
the rejection prose, and deliberately NOT off `rejections` at large: those
inline whole `intent.args` and belong in a re-prompt, not on a card, and a
structural refusal says nothing about the batch that remains. A purely
structural refusal therefore still ends at the generic sentence, as before.
The `would_run_wrong` check is written as its own line rather than left to the
broader rejection check it currently duplicates. That redundancy is a property
of the line below it, not of the guard: if Fix is ever loosened the way Edit
was, this is what still holds -- and it is what carries the reason either way.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two semantic guards describe defects the ENGINE ACCEPTS: a new branch wired
into a variable-aggregator that was never told about it, an array re-sent with
an element quietly dropped. Both drafts start, both run green, and neither
produces a `DraftWouldNotStartError` for `apply_repair` to veto. So the guards
had one hole left, and it was in the agent's own fallback:
`build_edit_intents` spent its corrective re-prompt on the guard's reason and
then, if the retry yielded nothing usable, returned the FIRST attempt's
applicable intents -- the very batch the guard had condemned. The guard fired
and the silently-wrong draft was written anyway.
`vet_intents` now carries `would_run_wrong` alongside `rejections`: the subset
of refusals the engine does not make. The two kinds fail differently and a
caller has to tell them apart without reading the text -- a structural refusal
DROPPED its intent, so what survives is a batch nobody judged wrong and keeping
it is a deliberate choice; a semantic verdict drops nothing and condemns what
remains. Reading that off the prose would be the same mistake as keying control
flow on the model's prose.
`build_edit_intents` therefore raises `ProposalWouldRunWrongError` instead of
returning a batch still carrying one, whichever attempt it came from. Nothing is
written and nothing is attempted. `handle_plan_approval` catches it into the
same `_change_not_applied` recovery a refused write gets: the plan stays at its
gate, the reason goes on an error card and into `last_edit_rejection` for the
next attempt.
Surfacing is a real exit now and not the dead end it would have been before this
plan: Task 7 gave the gate a revert and a keep-adjusting action, and Task 6
carries the refusal into the next proposal, so the user can change the rules and
the retry is not blind.
`ProposalWouldRunWrongError` is deliberately neither a `DraftWouldNotStartError`
nor a `ValueError`: the draft WOULD start, and a broad `except ValueError`
around a port write must not swallow it as if the write had been attempted.
`_change_not_applied` takes the activity to mark failed, defaulting to the write
where the usual refusal happens. This exit gives up during "prepare", so failing
the write would have shown a LATER step failed while an earlier one was still
running, with the step between never resolving.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four small things the whole-branch review found, none of which changes what a
guard decides.
`_change_not_applied`'s docstring said the gate's other two exits "both clear
this store". Task 7's fix round moved the clear to `handle_impact_analysis` and
made it conditional on the rules actually changing, so neither
continue_adjusting nor revert clears it -- and two tests assert exactly that. A
maintainer trusting the comment would re-add the clear to the revert path,
which is the bug Task 7 removed. Reworded to name the sole, conditional clear
and the two tests that pin it.
`already_present_predicate` keyed a connect on `(from, to)` alone, so a repair
that re-routes an arm of a branch between the same pair of nodes matched the
edge already there and was dropped as a no-op -- in the dry run and at
`apply_repair`, which runs the same predicate. The handle is now part of the
key on both sides, defaulting to "source" the way graphon reads a missing one.
`_build_node`'s defaults merge was shallow, so `model: {provider, name}`
replaced the whole default block and the node was refused for `model.mode` --
spending the one corrective re-prompt on a field the default was carrying. It
now merges one level into a dict-valued default, with one exclusion that the
suite caught immediately: a block whose `type` the caller did not write is left
alone, because `type` reinterprets its siblings and completing a partial
`authorization` from the default produces a node that looks authorized, sends
no credential, and is written in silence.
`_withheld_args` redacted a `value` shaped like a whole node `data`, which is
not what `set_node_config` sends: `path="headers"` with a header line reached
the retry prompt unredacted. The value is now also redacted as what it is --
the value at that path -- by wrapping it back into its own nesting and running
the one redaction profile over it, so `credentials` stays the single definition
of what a credential is. Unreachable with a stored secret today.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Asked to ADD a case to an if-else, the live S6 edit re-sent the whole `cases`
array -- the only way to append one -- and regenerated the case it was not
asked about: the condition's `id` and `varType` were lost and `= 60` became
`≥ 60`. The engine accepts all of it (graphon's `Condition` has no `id` field
at all, and `≥ 60` is a perfectly valid condition), `diff_graphs` reports one
line, and the user's threshold has silently changed meaning.
`vet_intents` now refuses a `set_node_config` that replaces an existing array
with another array from which an element identity (`id`, `case_id`) present
before is absent after, naming the ids that went missing. The comparison walks
the whole subtree, because the live clobber was one level down: both `case_id`s
survived and the surviving case's CONDITIONS were regenerated.
The check is scoped by (node type, path) to the arrays where those keys MEAN
something, each verified against the engine: if-else `cases` and
`cases.*.conditions` (`Case.case_id` is required and is the edge sourceHandle),
question-classifier `classes` (`ClassConfig.id` is required and documented as
the routing handle), and knowledge-retrieval
`metadata_filtering_conditions.conditions` (the canvas mints an id per
condition and a regenerated filter silently changes what is retrieved).
Unscoped, the rule refused an ordinary edit: an llm `prompt_template` message's
`id` is a frontend uuid the canvas REGENERATES when absent and graphon's
`ChatModelMessage` has no field for, so rewriting a prompt -- the commonest
Builder request there is -- looked exactly like the S6 clobber and parked the
user's change. Inside a scoped array the walk stays recursive, which is what
catches S6's nested condition id.
The op schema already tells the model to re-send existing elements
byte-identical; this is the half that does not depend on it obeying. The
neighbouring shapes stay untouched: an append that repeats every existing
element, a change addressed by index (which writes a scalar, never an array),
and an array whose elements never carried an identity -- a variable-aggregator's
`variables`, which the rejoin guard requires be re-sent whole.
A DELETION is not a clobber either. Removing an if-else case is a legitimate
edit and a whole-array write is the only way to express one, so a guard that
refused it would make the feature unusable -- loudly, but unusably. The two
shapes are distinguishable without asking the model what it meant, which is the
only reason the exception exists: a deletion leaves every element it KEEPS
byte-identical to one of the originals, each claimed once, and keeps fewer than
it was given; a regeneration loses an identity in the same write that modified
or added something else. The live S6 batch is the second kind and is still
refused, as is the mixed write that deletes one element while editing another.
The comparison runs against a HEALED copy of `before`, because `after` is the
dry run's already-healed graph: a draft holding `">="` or a JSON `60` -- the
ESQ1-303 shape -- would otherwise make a survivor the model re-sent verbatim
look modified, and a genuine deletion would be refused for a difference the
heal set introduced. Healing can only make two elements more equal, so it can
only reduce false rejections.
`graph_ops.value_at_path` is the read half of `apply_set_node_config`, through
the same `_resolve_path` walk, so the before/after comparison cannot disagree
with the write about which slot a path names.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The plan's last silent failure: a batch wires a new arm into an existing
variable-aggregator and never extends that aggregator's `variables`. The
aggregator stays engine-VALID -- its existing selectors still resolve, so
`preflight_errors` has nothing to say and no single intent owns the defect --
while graphon's `VariableAggregatorNode._run` walks the selectors, finds
nothing for the new arm and returns SUCCEEDED with `outputs={}`. End still
runs, so `run_finished_without_output` is False and the Builder reports
"All checks passed" over a workflow that produces nothing.
`vet_intents` now refuses a batch that adds an edge `N -> A` where `A` is a
variable-aggregator and nothing on the new arm feeds it, naming `A`, `N` and
the selector to append. Keyed on the batch's own effect on the graph and on
node data, never on model prose.
Kept narrow so it cannot false-reject. It fires only on an edge THIS batch
added (an aggregator the user already wired badly keeps its exemption) and only
when the target is an aggregator. "Feeds it" is read generously in one
direction and strictly in the other:
- generously about WHICH node: a selector rooted at any node the new edges
chain back through, not just the edge's immediate source, because a branch of
more than one node is an ordinary shape and all of them run when that arm
runs. The walk follows only the batch's OWN new edges -- that is the
soundness argument, since walking pre-existing ones would cross into the
graph's exclusive branches, where the S6 aggregator's existing selectors sit;
- strictly about WHICH NAME: the selector has to name a variable the node
actually publishes. `["node7", "output"]` on a node that publishes `text` is
exactly as unresolvable as no selector at all and the run skips it in the
same silence. A type whose published set is not knowable -- a tool's outputs
are whatever the tool returns -- accepts anything: this refuses a name it
KNOWS is wrong, never one it merely cannot confirm.
Both selector homes are scanned (`variables` and `advanced_settings.groups`),
since graphon reads one or the other depending on `group_enabled`.
Which variable a node publishes was prose in Edit's rejoin clause; it is now
two tables in `graph_prompt` -- the one name to suggest and the whole set to
accept -- read off graphon's own `NodeRunResult.outputs`, so the sentence the
model reads and the sentence the guard writes cannot name different variables.
A code node is read off its own declared `outputs`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fix's op schema listed `connect: {from_node, to_node}` and its graph view
printed no handles, so any repair that wired something from an if-else /
question-classifier / human-input node fell back to the "source" handle.
`graph_ops.apply_connect` refuses that, the refusal spent the single
corrective re-prompt, and `propose_repair` returned `_no_fix()` -- the
repair could not be proposed at all, however right it was. Edit was given
the handles in 197cd9c5ad; Fix never was.
The rendering both agents need now lives once in `agent/graph_prompt.py`
(`node_line` / `edge_line`, plus the op list, the operator literals, the
redaction sentinel and the config block), so a branch node looks the same
in either prompt. Edit keeps the one clause that is its own: follow a new
branch to where it rejoins the graph. Edit's prompt text and rendering are
unchanged byte for byte.
The shared schema tells the model to re-send an array's existing elements
byte-identical "as GRAPH shows them", which is only honest if what GRAPH
showed is whole. `fix._culprit_config` used to hand the prompt a Python
repr cut at 1500 characters, so a long `cases` array arrived severed
mid-value and would have been completed from imagination, obediently. It
now renders through the same block Edit uses: whole keys are dropped to
fit the budget and then named.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
edit.plan_approval offered exactly one action, approve_plan, while the card
its own refusal emitted said "Adjust it and approve again" -- an affordance
that did not exist. A chat message kept the PC at the gate and never touched
edit_rules, so an approval the engine refused had no exit at all.
The gate now also offers Continue adjusting and Discard this change plan,
admitted end to end: _ACTIONS_FOR renders them, _BACKEND_ACTIONS_FOR
[EDIT_PLAN_APPROVAL] admits re_fix and undo, and handle_plan_approval routes
both past the early return that used to swallow every non-approve kind. The
discard reuses perform_revert against the checkpoint handle_impact_analysis
already mints; Continue adjusting reuses handle_review's re_fix body, now
shared as _resume_adjusting, which returns to edit.impact_analysis with the
edit-rules form repopulated and unfrozen.
last_edit_rejection is cleared where its cause is: in handle_impact_analysis,
and only when the submitted payload actually changes fc.edit_rules. A stored
refusal goes stale precisely when the rules it refused stop being the rules,
and nowhere else -- so the actions that merely route the user to the form do
not clear it, and neither does edit.reverted's Retry. A revert taken at the
gate writes nothing, which makes it the one path reaching edit.reverted with
a refusal still true of the restored draft; clearing there would have made
Retry re-propose the identical plan blind.
Copy corrected in strings.py and handlers_edit.py to name the three buttons
that now exist.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
At edit.plan_approval the gate re-reads an UNCHANGED draft, so edit_rules and
graph are byte-identical on every re-approval, and _change_not_applied stored
nothing about WHY the port refused the write. The live F4 user approved the
same plan three times and got a byte-identical card each time.
Add DifyBuilderContext.last_edit_rejection: the raw text of the
DraftWouldNotStartError/ValueError the port raised, capped at 2000 chars,
stored by _change_not_applied and passed into build_edit_intents, which quotes
it into the user prompt as a "PREVIOUS ATTEMPT REJECTED BY THE ENGINE" block.
It is engine-sourced by construction -- a PreflightError's every message is
built by re-validating a credentials.redact_node_config copy of the node, and
graph_ops' ValueErrors carry only paths, ids and type names -- so no model
prose enters. Nothing branches on it. Cleared on a successful write, on a new
edit goal, and by the recovery reset.
Close the silent false success that left, rather than only recording it.
new_preflight_problems now takes the ids whose NODE DATA the batch wrote and
gives them no exemption at all: a node this batch wrote must be startable, not
merely no worse than it was. Keyed on locations alone, a repair that swaps one
bad value for another at the same field path -- or fixes one of three missing
fields -- produced nothing new, was written, and the card said "Applied the
changes" over a draft that dies at Graph.init. Since a repair's culprit is by
definition already invalid, that was Fix's main line. Every OTHER node keeps
the location-keyed exemption, so a defect the user left on the canvas still
cannot veto an unrelated change.
That set is deliberately narrower than a change set, on both sides (graph_ops
.DryRun gains the field filter_applicable already computed and discarded;
apply_repair keeps a separate written_nodes beside changed_nodes). connect is
excluded, because apply_connect reports both endpoints and an edge cannot
change either one's validate_node_config verdict -- counting them would let
merely wiring an already-broken node veto the batch. The heal is excluded too,
because heal_nodes_for_preflight scans the whole graph and a node it merely
normalized is not one this batch is answerable for.
Also, from the reviews of preflight.py: route an intent's config/value through
redact_node_config before a structural rejection line inlines it; key a
non-ValidationError crash on its truncated message as well as its class, and
keep that message out of the text the model is shown; and correct the
_CRASH_LOCATION comment -- verified against graphon, the rule is a config key
present AND type absent-or-invalid.
Finally, stop the docstrings overclaiming: redaction covers authorization,
headers and params, and a secret typed into a request body value is NOT
covered and reaches the stored text. Widening the profile is the wrong fix --
it shares the sentinel guard that refuses any write carrying REDACTED, so it
would make http-request bodies uneditable. Recorded in credentials.py as a
follow-up needing a separate persistence-only profile.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`filter_applicable` dry-ran STRUCTURE only. In the live F4 Edit four intents
came back "applicable, 0 rejected", the batch then died at `apply_repair`'s
preflight, and the one corrective re-prompt could never fire -- it is keyed on
`rejected` being non-empty. Every re-approval re-sent a byte-identical prompt
and got a byte-identical batch back.
`filter_applicable` now returns its dry-run graph (`DryRun` NamedTuple) after
running it through the shared `heal_nodes_for_preflight` -- the same
normalizers, at the same point in the sequence, the write chokepoint runs. A
dry run that healed less would report a defect the write was going to fix; one
that healed more would pass a batch the write refuses. It also skips the
intents the port drops as already-present, via a predicate both now share, so
a re-sent duplicate `create_node` no longer burns the re-prompt.
`preflight.vet_intents` then puts that graph through `validate_node_config` and
feeds any NEW problem into the existing re-prompt, quoting the engine's own
message. `new_preflight_problems` is the one definition of that rule, shared
with `apply_repair` so the dry run and the write cannot answer differently.
It is keyed on the set of pydantic error LOCATIONS per node, which is the only
key that gets both halves right. Node identity exempts an already-refused node
from every later verdict -- and since a Fix repair targets a node that is by
definition already invalid, that would leave `propose_repair` with no node-data
coverage on its own culprit. Message text is too fine: pydantic quotes a
truncated repr of the input, so filling one of three missing fields rewrites
the other two messages and an exact-text diff refuses the repair that improved
the node.
The refusal text is rebuilt from a credential-redacted copy of the node. The
verdict still comes from the real one; only the words the model is shown are
withheld, because a model-level pydantic error echoes the node's whole `data`
and that is where a live bearer token sits.
The retry budget is unchanged: one corrective re-prompt, then Edit's existing
fallback to the first attempt's applicable intents and Fix's `_no_fix()`.
Also folds in the Task 1 review: the generator's postprocess calls the shared
heal set plus `derive_if_else_var_types` instead of re-spelling three
normalizers inline, and the valid-operator no-op sweep covers the
list-operator container so `FilterOperator` gets its own check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The S6 edit rewrote an if-else's whole `cases` array to add one branch, and
in doing so silently turned `= 60` into `>= 60`, dropped every condition's
`id`/`varType`, and never touched the variable-aggregator the new arm feeds.
Task 3 gave the agent sight of the current config; this tells it what a
correct mutation of that config looks like.
`_OP_SCHEMA` now teaches, verified against `graph_ops._resolve_path` and
graphon rather than assumed:
* an all-digit path segment indexes an array, so ONE element changes by
index (`cases.1.conditions.0.value`) with its siblings untouched;
* an index past the end of an array is rejected -- there is no append -- so
ADDING an element means re-sending the whole array with every existing
element byte-identical and the new one appended;
* the engine's own 21 comparison literals, and that the comparison forms are
the unicode `≥ ≤ ≠`. `>=`/`<=` are healed deterministically at the port, so
there the prompt is merely stricter; the equality forms are ambiguous
between string `is`/`is not` and number `=`/`≠`, are deliberately not
healed, and the refusal that follows costs the whole batch;
* a new branch must be followed to where it rejoins: if it feeds an existing
variable-aggregator, that node's `variables` must gain the new node's
selector -- with the new node's REAL output variable, which differs by type
(`llm` publishes `text`, `template-transform` `output`, a tool
`text`/`files`/`json`), and the aggregator's existing selectors as the
anchor. A hardcoded name would have recreated the very bug this clause
fixes: `VariableAggregatorNode._run` skips a selector it cannot resolve in
silence, which is triage cause (d), the one that reports "All checks
passed" on empty output;
* the redaction sentinel is never handed back.
One consolidated test, not four substring assertions on a prompt constant: a
bare `assert "<phrase>" in _OP_SCHEMA` pins a wording and asserts nothing, so
the clauses with behaviour behind them are checked against that behaviour --
`_resolve_path` for indexed-vs-appended paths, graphon's `Condition` for the
operator literals. The operator guard parses `_COMPARISON_OPERATORS` and
compares the tuple to graphon's `Literal` exactly, in both directions, since
a substring check over 3kB of prose matches `=`/`is`/`in` anywhere and could
only ever catch an addition.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Edit prompt showed each node as title + type + handles and never its
`data`, so an LLM asked to add one if-else branch had nothing to copy from:
it regenerated the whole `cases` array, silently turned `= 60` into `>= 60`
and dropped `id` and `varType`. `diff_graphs` reported the lot as one line,
"cases updated". Triage edit-branch-failure-2026-09-22 cause (c).
`_graph_context` now inlines the JSON `data` of the nodes impact analysis
named, plus their direct neighbours -- and one hop further past a BRANCH
target's successors, because a branch's arms reconverge. In the live
failure the if-else fed two template-transforms that fed a
variable-aggregator, and that aggregator's `variables` is what says whether
the new arm reaches the End node at all; at one hop it rendered as a bare
one-liner the model could not extend, which is cause (d) and ends in "All
checks passed" on empty output. The extra hop is taken only past a branch,
so a long linear chain does not drag the whole graph into the prompt.
Every other node keeps today's one-line form.
Keys still sitting at their type's default are subtracted, with
`node_defaults` as the sole source of what "default" means: without that an
http-request node's two dozen method/auth/timeout/retry keys crowd the
field the edit is actually about out of the prompt. A config over the
1500-character budget sheds whole trailing keys and then names them, rather
than cutting the JSON mid-string where a purpose field that happened to
serialize last would vanish without a trace.
That subtraction is also what would have un-hidden every credential in the
node: a live bearer token never equals the `no-auth` / `""` default, and
Edit renders the target AND its neighbours, so the exposure would be wider
than Fix's one culprit node. `credentials.redact_node_config` therefore
withholds the `authorization` config and every non-empty `headers` /
`params` value behind an exact sentinel, keeping the KEYS visible so the
model can still reason about which header exists. Every non-empty value,
not the suspicious-looking ones: deciding by name means a denylist, and a
denylist misses `X-Company-Token`.
Fix had the same leak and it is closed here too, not scheduled: a failing
http-request node is one of the likeliest culprits there is, and
`fix._culprit_config` inlined its whole `data` raw.
Redaction alone would be worse than the leak -- the model reads the
placeholder, rewrites `headers` around it and destroys a working
credential -- so `validate_intent_args` refuses any `value` or `config`
carrying the sentinel. That is the one chokepoint both the
`filter_applicable` dry run and `apply_repair`'s write loop pass every
intent through, so the write is rejected with a reason the existing
corrective re-prompt can read and the stored secret stands. The refusal is
keyed on the exact generated string, never on model prose. It also covers
`create_node` / `insert_between`, which carry node data by the same route.
A rewrite that merely OMITS the credential line still passes, and the
comment beside the guard records why that is accepted: it fails loudly at
the next verify run, where the leak closed here was silent.
`build_edit_intents` grows two keyword-only, defaulted parameters:
`edit_target_node_ids`, threaded from `fc.edit_target_node_ids` by
`handle_plan_approval` and used here, and `last_edit_rejection`, declared
but not yet read so the task that feeds a refused write back into the
prompt does not have to change this method's arity a second time. Both
default to empty, so an agent that passes neither gets today's behaviour.
`edit_target_node_ids` is typed `Sequence[str]` rather than `list[str]`:
mypy rejects `list[str] = ()` as an incompatible default, and widening the
annotation keeps the `()` default and the keyword-only shape intact.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The question-classifier default shipped two `classes` with blank names. That
turns a loud preflight refusal into a node that is written, reported as a
success, and silently classifies into two meaningless categories -- and the
fabricated ids `["1", "2"]` pin `declared_branch_handles`, so a semantic
`connect` is dropped while the create survives. That is the failure shape the
registry exists to eliminate.
Draw the line explicitly and apply it to every entry: a default may supply a
field whose empty value is semantically NEUTRAL -- a container, a mode, a config
or auth block -- but never one that carries the node's PURPOSE, where an empty
value produces a node that RUNS and yields nothing. Dropped, with the engine
behaviour each empty value would have produced:
end.outputs End runs OK, so run_finished_without_output is
False -> "All checks passed" on an empty result
answer.answer streams nothing
template-transform.template renders ""
variable-aggregator.variables SUCCEEDED, outputs={} -- triage cause (d)
question-classifier.classes two blank categories + fabricated handle ids
question-classifier.query_variable_selector classifies `query or ""`
knowledge-retrieval.dataset_ids retrieves from no dataset, succeeds empty
The whole `if-else` entry goes too, though nothing is gained or lost by it: its
one case has an empty `conditions` list that graphon combines with `all([])`, so
it always matches -- but `iter_cases()` synthesizes that identical case when
`cases` is absent, so dropping the entry does not cure it. It only stops us
writing a fabricated always-true condition that makes the node look configured.
An empty value that fails LOUDLY is not in that category: there is no silence to
protect against, and dropping it would pay the full batch-refusal cost for
nothing. Verified loud, so these stay -- `llm.model` (never launches, "model not
configured"), `llm.prompt_template` (empty messages are filtered out, leaving
zero, so graphon raises `NoPromptFoundError`), `code.code` (the sandbox runner
calls `main(**inputs_obj)` with no `main` defined), `code.outputs` (graphon
raises `CodeNodeError("Not all output parameters are validated.")` for any key
the code returns), and `http-request.url` (`Executor._init_url` raises
`InvalidURLError("url is required")` before any request goes out).
`template-transform.variables` stays too -- it is structural, and it is the
exact field the live F4 failure was missing.
The placeholder agent genuinely means "no dataset, no output", so it now says
so itself instead of borrowing a default that would say it for every caller.
The engine sweep is re-pinned in both directions: the fields the engine reports
missing from a node built from a default ALONE must be exactly the purpose
fields the registry refuses to fabricate -- no more (a structural regression)
and no fewer (a silent-success regression).
Also: name the `filter_applicable` test after what it actually asserts, since
that function does not return its working graph; and reword the comment in
`graph_normalizers` that quoted a bare `type` marker, one reflow away from
being read as a type comment and silently killing the type check again.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A comment line introduced in 342cfc8545 wrapped so that it began with
``# type: number"``, which mypy reads as a PEP 484 type comment. It fails
to parse, mypy reports "Invalid syntax" and then stops — "errors prevented
further checking" — so the whole module and everything downstream of it
went unchecked. No runtime effect; the rewrap moves ``type:`` off the
start of the line.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An Edit LLM writes only the fields it was thinking about. graphon requires a
handful per node type with no default, so `create_node{template-transform,
{title, template}}` produced a node the draft preflight refused -- and the
refusal killed the WHOLE repair batch, parking the session at
`edit.plan_approval` with an identical card on every approval (triage
`edit-branch-failure-2026-09-22` cause (b)).
Extend the node-type default registry to every type the Build/Edit/Fix agents
may create -- template-transform, code, http-request, variable-aggregator,
answer, question-classifier, if-else, tool -- each taken from the matching
`web/app/components/workflow/nodes/<type>/default.ts` and each cited alongside
the graphon field it covers. Add `default_config_or_empty`, a non-raising
lookup: a type the registry does not cover contributes no defaults rather than
aborting the edit that is creating it.
Merge those defaults UNDER the caller's config at `graph_ops._build_node`, the
one construction point every created node passes through. A field the caller
supplied always wins; only a missing one is filled. That covers Build, Edit and
Fix, `create_node` and `insert_between`, and the `filter_applicable` dry run,
so the dry run sees exactly the node the port will write.
Defaults fill MISSING required fields; a field that is present but WRONG stays
`graph_normalizers.heal_nodes_for_preflight`'s job.
Two editor-only keys are deliberately not copied. `_targetBranches` (if-else,
question-classifier) is frontend state the canvas regenerates from
`cases`/`classes`, which is also where `declared_branch_handles` reads handles
from. A caller-supplied `cases`/`classes` wins the merge while a default
`_targetBranches` would not, so shipping it would attach a stale branch list to
every Builder-created branch node.
Move the registry from `services/dify_builder/` to `core/dify_builder/`: it was
imported by `core/dify_builder/placeholder_agent.py`, against the constraint
that `core/` must not import from `services/`. The dependency now points the
legal way, and the module stays pure -- copy, typing and graphon only.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An LLM writes `>=` where graphon's `SupportedComparisonOperator` (and the
list-operator's `FilterOperator`) spell `≥`; pydantic refuses it at
`Graph.init`, so F4's whole Edit batch was rejected and the session parked
at `edit.plan_approval` with nothing written.
`_normalize_condition_list` -- the walker that already visits all five
condition containers and already holds `comparison_operator` in hand --
now canonicalizes the two ordering forms (`>=`→`≥`, `<=`→`≤`) BEFORE the
value rule, which keys on that operator.
Only the ordering forms. Every equality-family ASCII form is ambiguous:
graphon's string equality is `is`/`is not` (`_assert_is` takes `str|bool`)
and its number equality is `=`/`≠` (`_assert_equal` takes only
numbers/bools), so `==`, `!=` and `<>` carry exactly the ambiguity that
keeps `equals`/`gte` rejected. Ordering carries none: graphon has no string
ordering operator at all, so `>=` can only mean `≥`.
`heal_nodes_for_preflight` is the one shared definition of the
deterministic heals a graph gets before it is preflighted, so the write
chokepoint and the dry run that decides applicability cannot drift apart.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`_declares_variable` accepted `output` alone for a container, so a CORRECT
`{{#node4.item#}}` written by a node INSIDE an iteration was read as
unresolved and silently rewritten to `{{#node4.output#}}` -- this item's
value replaced by the container's entire array. It fired on 9 of 9 live
attempts:
Workflow generator: rewrote unresolved reference node4.item
to declared output node4.output
Silently rewriting a reference the engine already resolves is the one thing
the repair path must never do. It is worse than the loud failure it
replaces, and it is the same contract violation this whole plan exists to
enforce: the normalizer may only turn a graph the engine REFUSES into one it
ACCEPTS.
Verified against graphon rather than assumed, and the two container types
turn out NOT to be the same shape:
- `iteration` publishes THREE names. `output` is the node's own result
(`outputs={"output": flattened_outputs}`, nodes/iteration/iteration_node
.py:551/565/631), read by consumers outside. `item` and `index` are added
to a deep COPY of the pool that only the child engine sees
(`_create_graph_engine`, :827-828), so they resolve for a node inside the
sub-pipeline. Scope is not enforced: the walker carries only the TARGET
of a reference, never the node that made it. The alternative to accepting
all three is the silent rewrite above, which is strictly worse than an
out-of-scope `item` failing loudly at run time.
- `loop` never publishes `item` or `index`. It adds each declared
`loop_variables[].label` to the MAIN pool (nodes/loop/loop_node.py
:279-286, visible inside and outside) and reports those labels plus
`loop_round` as its outputs (:489-492), alongside whatever `data.outputs`
already declares. Those names are now accepted. `output` stays accepted
too although the engine does not publish a loop under that name:
dropping it would turn graphs that generate today into failures, which is
a tightening, not this fix.
Both container types leave `_sole_declared_variable` as a consequence, not a
separate decision: a node with three published names has no SOLE output, so
falling back to `output` is exactly the "which value did the workflow mean?"
that helper refuses to answer for every other multi-output type. An invented
name on a container is now reported instead of guessed. Every other
single-output type keeps the rewrite it has always had, which is what the
non-container test guards.
This is a LATENT defect that the gated snippet work made reachable, and it
ships together with that work: the container-output branch predates this
plan, but the plan is what made the generator write iteration bodies whose
inner nodes reference the per-item variable. It is a separate commit from
the JSON-Schema walker fix and reverts on its own.
Evidence: docs/superpowers/triage/live-verify-2026-09-22/FINDINGS.md section
B8, and `e0_required_probe.py`, whose `{#node4.item#}` UNRESOLVED_REFERENCE
disappears with this change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`_collect_refs_in_data` read ANY two-element list of strings as a
`["node_id", "variable"]` value selector. An llm node's structured-output
schema carries `required: ["content", "speaker_notes"]`, so the walker
harvested a reference to a node called `content`, `_collect_unresolved_refs`
found no such node, and the whole generation aborted with
UNKNOWN_NODE_REFERENCE: Reference {#content.speaker_notes#}
points at unknown node 'content'
Arity 2 is the entire trigger -- a `required` of one name or three names
harvests nothing -- and `enum: ["formal", "casual"]` and
`type: ["string", "null"]` are exposed exactly the same way. The defect is
the walker's, not the schema's: the node builder was told to expose those
two names and wrote a perfectly valid JSON Schema.
This is a LATENT defect that the gated snippet work made reachable, and it
ships together with that work. The two-element branch is byte-for-byte
unchanged since before this plan; what changed is that a JSON Schema now
reaches the walker at all. `f466fd64f8` taught the llm snippet to enable
structured output -- before it, an llm node emitted no schema, so the hazard
was unreachable -- and `af6b0db106`'s rule 16 pins that schema to exactly the
names the plan declares, which makes the two-name case modal rather than
rare. Neither commit touched the walker, so neither can be reverted to fix
this; the fix belongs here.
`_NON_SELECTOR_LIST_KEYS` already gates a key's whole subtree out of selector
harvesting and is shared by all three walkers (harvest, rewrite, sys.query
normalize), so both halves of the fix are one line each there:
- `structured_output` excludes an llm node's entire schema wrapper, which
covers every keyword such a schema will ever grow;
- `required` / `enum` / `type` exclude the schema keywords that hold string
arrays wherever they appear, because the walker is generic and the next
schema-bearing node type will not keep its schema under
`structured_output` (a tool's `output_schema`, say).
Placeholder harvesting is untouched -- `allow_selector` gates only the
selector reading -- and so is the 3-segment
`["node2", "structured_output", "field"]` selector, which is a list VALUE
under some other key, not a subtree under the `structured_output` KEY.
The regression guard is the point of the test set: a genuine
`[node_id, variable]` selector sitting beside the schema in the same node's
data is still harvested AND still validated. An exclusion that swallowed real
references would quietly undo the walker's whole job.
Reproduced and fixed deterministically, with no LLM: see
docs/superpowers/triage/live-verify-2026-09-22/FINDINGS.md and its
`e0_required_probe.py`. The test asserting the byte-for-byte production error
string is gone fails before this change and passes after.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two halves of one contract, gated on the cmd+K owner together.
1. Node configs are generated by isolated parallel LLM calls that share the
plan but not the output names each producer chooses, so a consumer
references `node3.deck` against a code node that declared `slides_json`
and the whole generation dies on UNRESOLVED_REFERENCE. The planner now
declares `node_outputs` -- `{"<node id>": ["<name>", …]}` -- for the only
node types whose output names their own config chooses: code (`outputs`
keys), parameter-extractor (`parameters[].name`), human-input
(`inputs[].output_variable_name`) and an llm meant to emit structured
output (its schema fields).
It reaches the builders the way `start_inputs` already does, but through
one type-aware section rather than the shared plan JSON: the producer is
told to expose exactly those names, every other builder is told they are
the only names it may reference on that node, and an llm producer's names
are listed as `structured_output.<field>` -- serialising them raw would
hand every consumer the one reference form the engine cannot resolve.
The declaration is BOUNDED in the prompt -- at most 6 names per node and at
most 10 declared nodes. cmd+K sign-off consideration: this is an unbounded
map added to the stage with a known live truncation failure (ESQ1-300:
planner output-budget exhaustion, masked by `json_repair`), and the
standing finding there is that `_PLANNER_DEFAULT_MAX_TOKENS` must not move
before the stage is instrumented. Bounding the declaration is the
alternative: the fail-open parse below already makes a truncated or absent
value harmless, so the bound costs nothing a plan needs.
Fail-open throughout: the value comes from an LLM, so `_parse_node_outputs`
keeps only lists of referenceable names (the run time's own
`[a-zA-Z_][a-zA-Z0-9_]{0,29}` grammar) keyed by a planned node of a
declaring type -- honouring a declaration on, say, an http-request node
would point consumers at a name the engine never exposes. A plan without
the key, or with a garbage one, produces byte-identical prompts and
behaviour to before.
2. `_declares_variable`'s llm branch was wrong in both directions. graphon's
`LLMNode._build_run_outputs` always publishes `text`, `reasoning_content`,
`usage` and `finish_reason`, adds `files` when the node saved any, and adds
`structured_output` -- one object, addressed as
`{{#<id>.structured_output.<field>#}}` -- only when structured output is
enabled with a schema. The branch accepted `text` plus any flat
structured-output SCHEMA PROPERTY, and rejected every other real output.
So a legitimate `{{#node2.reasoning_content#}}` failed the generation while
`{{#node2.answer#}}` -- which the engine can never resolve -- was waved
through, saved clean by cmd+K and died at run time. Removing that flat
acceptance is deliberate: a silently broken graph becomes a loud generation
failure. `test_runner_missing.py`'s assertion that froze the old behaviour
is rewritten with the reason.
Because a consumer only ever sees its OWN node's schema snippet, the shared
builder head now names an upstream llm's real outputs and both taught
reference forms, and the reference walker harvests the 3-element
`["node", "structured_output", "field"]` selector it teaches -- previously
invisible, so an end node pointing at a drifted llm passed validation
silently while the equivalent placeholder was repaired. Both forms now
travel one path: validated by `_declares_variable`, repaired to the node's
real output when the node turns out to be schema-less. The harvest is
deliberately type-blind -- only an `llm` publishes a `structured_output`
object, so the selector aimed anywhere else cannot resolve and has to be
REPORTED, `["start", "structured_output", "x"]` being exactly the invented
reference this walk exists to catch. The `llm` check sits on the REWRITE
instead, which is where reading a plain 3-string list as a selector did
damage: an end node's `["n2", "structured_output", "x"]` data tag was
collapsed to `["n2", "a"]` against a code node, a silent edit where a
report would have been loud and recoverable.
`_sole_declared_variable` reads `_llm_is_schema_less` (the enable flag AND
the schema) rather than schema presence alone, so the "flag off, leftover
schema" drift still repairs an invented name to `text` instead of failing
the build; a real structured-output node still refuses to guess.
3. One behaviour change beyond those two halves, and it is deliberate: a
DOTTED reference is now validated by its ROOT segment, the way
`VariablePool.get` resolves one. A `file` start variable's attributes
(`{{#node1.doc.url#}}`, the `FileAttribute` set), a `json_object` start
variable's keys and an object output's keys stay valid -- a code node whose
output really is called `structured_output` keeps its 3-element selector.
On the start node, a dotted name whose root addresses something the pool
cannot traverse is REPAIRED to that root -- `{{#start.docs.url#}}` becomes
`{{#start.docs#}}` -- on the same path as every other reference repair, so
it shows up in the change log. That covers `file-list`, whose
`ArrayFileSegment` subclasses `ArraySegment` rather than `FileSegment`, so
`{{#node1.docs.url#}}` renders as its own literal text at run time instead
of resolving, and every scalar input. The dotted form never resolves and
the root form always does, which keeps this inside the normalizer's
contract; reporting it instead would abort the whole generation, and cmd+K
`/create` / `/refine` never retry. A root that names NOTHING is a genuinely
invented reference with no value to fall back to and is still reported. It
is never injected as a start variable either way: the run time's grammar
has no dot inside a segment, so a form input literally named `topic.url`
could never resolve.
That repair reads the start node's declarations, and the same walk writes
them, so the walk is now TWO passes over one deterministically ordered list
(`_ordered_refs`): every missing start input is declared first, and only
then is each dotted root evaluated against the completed set. One pass over
the harvested SET made the outcome depend on which reference it yielded
first -- a graph carrying both `{{#start.docs#}}` and `{{#start.docs.url#}}`
with `docs` undeclared repaired cleanly under `PYTHONHASHSEED` 0-6 and
aborted on UNRESOLVED_REFERENCE under 7-9, i.e. the abort this repair exists
to remove, arriving at random. `_collect_unresolved_refs` walks the same
ordered list so a given graph always yields the same error list.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Parses the JSON example out of every _NODE_SNIPPETS entry, wraps it as a
node config, and asserts core.workflow.node_factory.validate_node_config
accepts it -- the same call services/dify_builder/preflight.py makes.
FINDINGS.md's Cross-cutting section named this the single highest-value
missing test: before Task 4 (4deede53b5), it fails on parameter-extractor,
whose query example was an array of value_selector arrays where the engine
wants one selector; it only passes with Task 4's fix applied (RED/GREEN
verified by temporarily reverting that one snippet and restoring it).
There is no allow-list of entries excused from validating: every
_NODE_SNIPPETS entry validates standalone, so the parametrize covers all of
them and a snippet that stops validating fails loudly. (The iteration/loop
container snippets, which legitimately cannot validate standalone, live in
node_builder_prompts._CONTAINER_CONFIG_SNIPPETS -- a different dict, out of
this file's scope.)
No production change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The llm paragraph claimed text was a schema-less node's only output; the
engine always exposes reasoning_content, usage and finish_reason too, so
the generator was being told to avoid valid references. The list-operator
snippet now carries the comparison-operator rule itself, because each
node build sees only its own snippet.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The retry instruction already named it; the card the user sees did not,
so an error like "Tool acme/render is not installed for this tenant"
left them guessing which node it meant. Both now render through one
formatter.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The schema-properties lookup was written out three times; the new
schema-less gate made it four. It now has one reader. The
structured_output_switch_on branch is dropped: the engine serialises
structured_output_enabled as the wire/JSON key, and switch_on is only
that field's internal attribute name -- it is never a dict key a raw
graph carries, so the read (and the test reinforcing it) was dead
against real data.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Extend the shared _OUTPUT_ALIASES table so a reference to
__structured_output__ / __structured_output / structured_output / output /
result on an llm node with no usable structured output (the enable flag --
structured_output_enabled or its alias structured_output_switch_on -- is
falsy, or the structured_output schema is empty) resolves to the node's
real default output, text, instead of staying UNRESOLVED_REFERENCE. Gated
in _aliased_output so an LLM node that genuinely has structured output
enabled with a schema is left untouched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A generated http-request authorization without "type" made graphon's
HttpRequestNodeAuthorization.check_config read values.data["type"] and raise
KeyError; pydantic does not wrap it and validate_node_config only catches
ValueError, so it escaped preflight_errors -> apply_repair and the live
Build session ended in state=failed.
- preflight_errors catches any exception per node and reports it as
"node '<id>' (<type>): <ExcName>: <message>" (ValueError messages are
unchanged), then keeps checking the remaining nodes.
- The shared http-request normalizer (called by both the generator
postprocess and apply_repair) fills a missing authorization type:
"api-key" when config carries a non-empty api_key, else "no-auth" with
config None, and reports the node as changed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Final-review minors:
- is_user_supplied_secret: a verbatim match needs a remainder of at least
8 characters, so "Bearer key" no longer counts as supplied because the goal
says "key".
- is_credential_key: Ocp-Apim-Subscription-Key ("subscription" key prefix),
and cookie / credential / credentials / bearer as last segments. No
"header" segment -- accept_header stays non-credential.
- A bare Bearer/Basic/Token word is kept as an auth scheme only on an
Authorization / Proxy-Authorization line; under any other credential key
(X-Auth-Token: token, api_key: token) it is replaced whole.
- endpoint_variable_names also collects a start template inside the parsed
host (https://{{#s.host#}}/v1), via the shared core/dify_builder/urls.py;
path/query templates stay mockable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
url_hosts only sees http(s):// URLs, so a goal saying "our renderer is at
api.pptrender.io/v1/render" (or a requirement value typed the same way) did
not count: build grounded the URL anyway and the requirements scrub blanked
the LLM's https://api.pptrender.io/... default. is_user_supplied_host now also
accepts a dotted host that appears verbatim (case-insensitive) in the trusted
text; both is_user_supplied_url and the scrub's URL check use it. A single
label ("localhost", "renderer") never matches a stray word.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
is_user_supplied_url exempted ANY url containing a Dify template, so
https://api.pptrender.io/v1/render?topic={{#node1.topic#}} (a host the user
never typed) was left in place and every test run hit a host that does not
exist. Only a template in the HOST position (the url starts with one, or its
host contains one) exempts a url now; a literal host is checked as usual.
Grounding such a url replaces only scheme://host[:port] with the start
variable (labelled "<title> base URL") and keeps the path, query and the data
templates in them verbatim; a url whose host is already a template is never
re-grounded, so a second pass is a no-op. The host-position rule lives in the
new stdlib-only core/dify_builder/urls.py (urlsplit reads a template's '#' as
a fragment), shared with endpoint_variable_names' side.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The test form and mock data skipped every start variable an http node
read, so a search query or a language header was never mocked. Only a
host-position URL template, credential-keyed header/param lines and an
api-key auth now count, with one shared is_credential_key in core. A
bare "key" param, an empty credential and a bare "Bearer" are grounded
too, and an existing Dify template is never re-grounded.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Keys are credentials by their last segment (api_key, access_token,
Authorization), not by containing "token" or "password", so
password_policy / token_limit defaults survive. URL hosts stop at the
first non-URL character and drop trailing punctuation, so a goal that
names a host at the end of a sentence or next to CJK text is recognised.
Placeholder tokens like <…> only count under a credential key or an
auth scheme.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
S5b (local E2E 2026-09-22, F2): analyze_goal's "a sensible default value per
field" led the LLM to prefill render_api_url=https://api.yourcompany.com/...
and Authorization: Bearer YOUR_API_KEY for a goal that named neither -- caught
only because "yourcompany" happened to match an existing placeholder-URL
regex; a plausible invented host would slip through and recreate ESQ1-302's
launch failure one step upstream, in the requirements form itself.
Adds services/dify_builder/agent/user_supplied.py (pure, no I/O): url_hosts,
is_user_supplied_url, CREDENTIAL_PLACEHOLDER_RE, is_user_supplied_secret,
trusted_text_for -- shared primitives for telling a literal the user actually
wrote from one an LLM invented, used by this task and consumed by later
tasks in the same plan.
build.analyze_goal now tells the model never to invent such a field's
default, and _scrub_invented_defaults deterministically blanks any top-level
field whose value contains a URL host, or a credential-shaped or
credential-keyed literal, not present in the goal text -- decided on hosts
and literal text, never on LLM prose.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- bind_resources fixtures use the production model label, the full
provider/model id (langgenius/tokener/tokener/deepseek-v4-flash), and the
real Code Interpreter id (code/simple_code); the docstring's ESQ1-302 quote
now shows that shape too.
- The run_test prefill uses without_endpoint_values instead of re-evaluating
endpoint_variable_names(graph) for every key.
- _ground_placeholder_endpoints: two placeholder http-request nodes get two
start variables, and a start variable that already exists is not duplicated.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Build, Edit and Fix mock call sites passed generate_mock_inputs through
unfiltered, so an endpoint variable got a mocked URL. The engine accepts any
URL at launch; the http-request node then fails on connection, which
is_input_failure reads as a config failure, so the flow fed the config-repair
loop (the ESQ1-302 thrash). without_endpoint_values drops the endpoint keys at
every mock call site: the missing required key now fails the launch as
"<var> is required in input form", which routes back to the test-data gate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The planner is told to name a tool by its id, but binding matched only the
label and then the "tool" keyword, so two tool steps naming ids both bound
to the first tool step. _covering_step now tries the resource id before the
label. A tool's "(using ...)" suffix also carries its provider/tool id in
ASCII brackets: the shared generator pins a tool only on that identifier
(tool_catalogue._find_explicit_tool_keys), never on the label. The planner
prompt asks for the id as a separate word, since a CJK-glued id never pins.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"tool" matched "toolkit" and "llm" matched "fulfillment", binding a
resource to an unrelated step instead of falling back. Labels and
keywords now match case-insensitively on ASCII-alphanumeric boundaries,
which still matches CJK step text such as "llm节点".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A loop-back feeds the already-bound plan back into bind_resources,
which only appended: a deselected resource kept its "(using …)" and
suffixes piled up in the text handed to the generator. Trailing
"(using …)" groups are stripped before binding, so a re-bind reflects
exactly the current selection.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bind_resources appended every confirmed resource's label to the LAST
plan step: the ESQ1-302 end-node step read "(using deepseek-v4-flash,
Code Interpreter)" while the step that needed a tool said nothing, and
that is the text the generator was handed. Each resource now lands on
the first step naming its label, else the first step naming its kind's
node type (llm / knowledge-retrieval / tool); the last step is only the
fallback.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_PLACEHOLDER_URL_RE matched "localhost" as a substring anywhere in the
URL, so a real tunnel host like abc123.localhost.run was wrongly
flagged; the check now parses the URL and only matches when the host
itself is localhost or ends with .localhost.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A placeholder endpoint is re-pointed to a required start variable, but
the test-data form pre-filled every text field with a mocked value, so
one click ran against another invented URL. Start variables an
http-request node reads its URL from now stay empty, like upload fields.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The ESQ1-302 draft called https://api.example.com/ppt/generate. The
model had no endpoint, so it invented one, and every test run failed
against a host that does not exist. build_nodes now re-points an
http-request whose URL is empty or a placeholder (example.com,
your-api, <your-domain>, {tenant}, localhost) at a new REQUIRED
start-node variable "<node>_url", so the test-data gate asks the user
for the real endpoint -- or, when they have none, says what the step
needs. A Dify template reference is not a placeholder.
Builder-only: the shared http-request snippet's own example is
https://example.com, and cmd+K keeps its placeholders.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
propose_plan_v1 was shown REQUIREMENTS only. It cannot plan a tool step
for a tool it has never seen, so the ESQ1-302 plan said "http-request
节点：调用外部PPT生成API" while the tenant had a ready Markdown -> PPTX
tool -- and every later stage (resource recommendation, the shared
generator, the directive) took the plan's word for it.
ready_tool_catalogue lists the tenant's READY tools (an installed-but-
unauthorized tool cannot cover a step), sorted and capped at 40; the
planner prompt gains an "Installed tools you may name" section and tells
the planner to plan an http-request only for an endpoint the user
actually supplied. A listing failure degrades to the old no-catalogue
prompt, never a crashed advance. The positional signature is unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fe3f8154b5 told the generator to use a tool node "only when the plan
names an installed tool; otherwise use 'http-request'". That contradicts
the shared planner prompt (INSTALLED-TOOL-FIRST), and the Builder's own
planner was never shown the tool catalogue, so it could not name one:
every third-party step fell through to an http-request with an invented
endpoint (ESQ1-302 called https://api.example.com/ppt/generate while
the tenant had a ready Markdown -> PPTX tool).
The rule now says tool-first, forbids invented and placeholder
endpoints outright, and keeps the half that earned its place (ESQ1-286):
an http-request's changing inputs belong on the start node.
Builder-only: nothing under core/workflow/generator/ changes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The inline repair the shared graph_normalizers replaced logged each
re-homed edge at info; the delegated version returns (and logs) only
the edges it could NOT re-home, so a successful re-home -- the planner's
"else" moved onto "false", a default-handle edge moved onto a class --
left no trace in the generator logs.
_repair_branch_edge_handles now snapshots each edge's sourceHandle (by
edge object) before the shared repair and logs every edge whose handle
changed (source -> target, old -> new) at info; the unresolved warnings
are unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two gaps in the fail-closed branch-handle repair:
- The failure-arm aliases lacked fail_branch / failbranch / error_branch
/ on_fail / fallback. A planner's "fail_branch" edge on a fail-branch
http-request stayed unresolved (only the hyphenated spelling matched
the declared handle exactly). They are looked up through _canon_name
like every other name, so any spacing / hyphen / underscore spelling
hits.
- On an if-else / question-classifier / human-input node with
error_strategy fail-branch and that arm unwired, the fan-out rule
eliminated an unrecognized name onto the one free branch arm (ELSE, a
class, an action) -- even when the edge meant the failure arm. Rule 2
no longer runs for such a node while its fail-branch arm is unwired;
the edge is left for a validator instead. Once fail-branch is wired
the rule applies as before.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_normalize_conditions called its value normalizer as (value, operator)
and, on any TypeError, retried it as (value) -- the fallback that let
the one-argument list-operator normalizer through. It also swallowed a
genuine TypeError raised inside a normalizer, replacing it with an
unrelated "missing argument" error, and needed two type: ignores.
normalize_filter_condition_value now takes (value, operator="") and
ignores the operator (a FilterCondition scalar is never list-wrapped),
so both normalizers share Callable[[Any, str], Any]; the walker calls
it once, with no try/except and no type: ignore.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
build_nodes dry-runs the generator's intents through filter_applicable
and recorded the rejections only when NOTHING applied. A partial reject
-- e.g. a connect on a branch handle the node does not declare, one the
generator postprocess could not re-home and apply_connect now refuses --
kept the applicable intents and dropped the rest with no log line and no
diagnostic, leaving that arm unwired on the canvas without a trace.
A partial reject now logs a warning naming each dropped op, its args
(minus a create_node's config body) and the reason, and appends a
build_nodes diagnostic with the same list; the applicable intents are
still returned. The total-reject path is unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The _repair_branch_edge_handles docstring claimed an ambiguous edge is
left "so _validate_structure fails the graph closed" -- true only while
the INVALID_BRANCH_HANDLE check (its own, separately revertable commit)
is in. It now says the edge is left as it was, logged, and returned by
the shared function so a validator can fail closed when that check is
enabled. The comment above the call in _postprocess_graph still
described the old repair (if-else / question-classifier, default
handle only); it now names every branch type the shared repair covers
and the named-but-wrong handles it re-homes, fail-closed.
Comments only; no behaviour change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
graphon runs an if-else whose `cases` is absent or None (the legacy
top-level `conditions` shape) as one case with id "true"
(IfElseNodeData.iter_cases; if_else_node routes "true" when
uses_legacy_shape), and the canvas migrates it to case_id "true".
declared_branch_handles declared only "false" for it, so the engine's
own IF arm looked undeclared: apply_connect refused a connect on "true",
and with only "true" wired the fan-out rule re-homed the IF arm onto
ELSE in the generator postprocess.
The case ids now mirror iter_cases (legacy -> ["true"]; an explicit
`cases: []` stays case-less), and the one-case IF-alias rule counts the
legacy shape as one case.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The planner names a branch's outgoing source_handle values (rule 12) in
one LLM call; the node builder chooses the case / class / action ids in
another, and nothing told it the planner had already chosen. ESQ1-303's
if-else was built with case "true" against planned edges on
"score_equals_60" / "else", so both arms were skipped.
The builder already receives the planned edges with their handles in
"Normalized plan and topology". The if-else, question-classifier and
human-input snippets now state the contract: declared ids MUST be those
planned handles; the ELSE arm is implicit ("false") and is never a case.
human-input also gets a matching clause for its implicit timeout arm
("__timeout"), which must never be declared as a user action. Shared
with cmd+K /create and /refine; the postprocess repair stays as a
fallback.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_validate_structure fails closed on two defects that reached the canvas
silently: an edge leaving a branch node on a handle postprocess could
not re-home unambiguously (INVALID_BRANCH_HANDLE -- the arm would never
run while the run reports "succeeded", ESQ1-303), and a json / raw-text
/ binary http-request body with a number of items other than one
(INVALID_HTTP_BODY -- the executor rejects it at run time, ESQ1-302, and
no automatic collapse of several key/value items is correct).
cmd+K /create and /refine see these as structured errors with the node
id, like every other structural error; the Builder retries once with the
error fed back.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
apply_connect now rejects a connect from a branch node on a handle it
does not declare. Edit's prompt never mentioned source_handle and its
graph view showed no handles, so such connects defaulted to "source"
and a partial reject was dropped without a retry. The op schema and
graph view now carry the handles, and any rejection gets one corrective
re-prompt whose feedback names the declared handles.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
apply_repair's value normalizers can heal a node the repair's intents
never touched; the diff showed it but changed_nodes did not. Both
normalizers' changed ids now join changed_nodes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
apply_repair runs the same graph_normalizers the generator's postprocess
runs, for the intents the generator never saw: a Fix/Edit set_node_config
that writes a condition value as a JSON number (ESQ1-285's flip-flop) or
an http body item without type is healed before the preflight would
reject it.
apply_connect rejects an edge from a branch node on a handle the node
does not declare -- including the default "source" it used to fill in
-- with the same ValueError the handlers already surface as "Couldn't
apply the fix". Such an edge hangs off nothing and its arm never runs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_postprocess_graph now runs core.workflow.graph_normalizers: condition
values written as JSON numbers become strings, if-else varType is
derived from a declared start-variable type, http-request body items
get the type BodyData cannot default, and _repair_branch_edge_handles
delegates to the shared fail-closed repair (so a named-but-wrong handle
such as "else" is re-homed, not only the default "source").
Shared with cmd+K /create and /refine: every change turns a graph the
engine would have refused into one it accepts, and none alters a graph
the engine already accepted. Nothing is rejected here.
Validated on the real ESQ1-303 dev draft: value "60", varType "number",
handles true/false, edge ids derived from the repaired handles.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A missing sourceHandle is "source" to the engine, so a fail-branch node's
unkeyed success edges are valid and must not be re-homed. The error and
timeout arms exist because of the node's configuration, not because the
author wired a branch, so elimination and default assignment never guess
onto them; named edges reach them through explicit aliases instead.
Ambiguous names (duplicate class names, an alias that disagrees with a
stem match) now fail closed, the one-case IF aliases count case ids
rather than handles, and name keys fold _ - and spaces.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The planner names a branch's outgoing source_handle values in one LLM
call; the node builder chooses the case / class / action ids in another.
ESQ1-303's edges left its if-else on "score_equals_60" and "else"
against a case named "true", so the engine skipped both arms and the
run succeeded with nothing.
declared_branch_handles lists what a node routes on; repair_branch_edge_
handles re-homes an edge only when it is forced (exact / alias / name /
stem match, one arm fanning out under a single invented name, default-
source edges onto unused handles in order) and returns everything
ambiguous untouched; undeclared_branch_handles reports what is left so a
validator can reject it. A wrong guess would silently swap IF and ELSE.
Aliases are trusted as names -- the documented limit -- which is why the
node-builder handle contract, not this repair, is the primary fix.
A fail-branch node's success path stays on "source" (what graphon's
NodeRunResult.edge_source_handle actually emits by default), not an
invented "success" handle nothing ever routes on; a fail-branch
branch-type node (if-else / question-classifier / human-input) just
gains the extra "fail-branch" arm. human-input also always declares its
implicit "__timeout" arm, since every human-input node has a timeout
config and the FE always renders that handle; that arm is excluded from
the "unused declared handles" pool the fan-out/default-order repair
rules count against, since it is routinely left unwired by design and
must not block or absorb repair of the arms the author did configure.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
graphon turns a falsy body.data into an empty list before the executor
counts items, so a json / raw-text / binary body whose data is "" fails
at run time. http_request_body_errors counted any string as one item;
only a non-empty string is one now.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
graphon's BodyData.type (Literal["file","text"]) has no default and the
shared builder prompt only ever shows an empty "none" body, so the LLM
writes {key, value} items and the graph dies at Graph.init (ESQ1-302).
normalize_http_request_bodies fills the type (file when the item carries
a file selector, else text) and empties a "none" body.
The executor sends json / raw-text / binary bodies as ONE rendered
template. Two {key, value} items under a json body have no correct
automatic collapse -- quoting breaks a JSON-valued variable (ESQ1-302's
node3.text is JSON), not quoting breaks plain text -- so
http_request_body_errors reports the node instead of guessing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
varType is a frontend-only hint the canvas uses to pick the operator
list; the runtime ignores it. Builder-made if-else nodes never carried it
(ESQ1-303), which steers the panel to string operators on a number and
reproduces ESQ1-285's runtime error. Derive it only when a start
variable's declared type makes it certain; never overwrite, never guess.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
list-operator's filter conditions are FilterCondition (value: str |
Sequence[str] | bool, default ""), and filtering an array of strings or
numbers requires a plain string. The normalizer treated them like
if-else's Condition: it wrapped an in/not-in scalar into a list and left
None alone, both of which graphon rejects. They now get their own rule:
None becomes "", numbers become text, nothing is wrapped. List items use
the same integral-float rule as scalars.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
graphon's Condition.value is str | Sequence[str] | bool | None. The node
builder wrote "value": 60 for the ESQ1-303 draft; Graph.init builds every
node up front, so that one number stopped the whole workflow. pydantic
also coerces int 1 / 0 to True / False, so bool must be checked before
int.
core/workflow/graph_normalizers.py is the one place both the shared
generator's postprocess and the Builder's write chokepoint will call, so
the two paths cannot disagree. This commit adds the module with the
condition-value normalizer only (if-else cases, sub-variable conditions,
loop break conditions, list-operator filters); nothing calls it yet.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Build's and Edit's re_fix branches (handle_review's continue_adjusting,
handle_reverted's retry_after_revert) reset repair_attempts and
last_repair_error but not unknown_outcome_count. After the cap (count=2)
-> keep_draft -> review -> re_fix, the new cycle's FIRST unknown outcome
re-capped with "couldn't be determined twice in a row". Reset it at all
four sites, as Fix's re_fix already does.
Also pins that Edit's unknown-outcome cap drops a stale diagnosis and
staged repair (seeded, so the assertion is not vacuous).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
Build resets the breaker counter when the user re-enters the repair
loop from review or after a revert; Edit's equivalent branches did not,
so a count left over from an earlier cycle could trip the breaker one
round early.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Edit's affected-path test diagnosed and staged a repair for the same
engine failure with no counter at all, so the loop 6877532f8e closed for
Build stayed open for Edit. The breaker (engine-keyed signature,
consecutive-repeat counter, MAX_REPEATED_REPAIRS) moves to handlers_fix
under public names, Build keeps its underscore names as aliases, and
Edit stops at its gate on the third identical failure. A new edit goal
resets the counter.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fix's re_fix action resets the rest of the repair loop's state but kept
unknown_outcome_count, so after the cap tripped the first unknown
outcome of the new repair cycle re-capped immediately instead of being
re-runnable once.
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>
The first cut flagged any succeeded run where a branch node ran and no
End did, which also caught a legitimate side-effect-only arm that ran
to completion, and treated any fail-branch node as a router. The rule
is now the ESQ1-303 mechanism itself: a branch node ran and none of its
edge targets ran (the engine skipped every arm), in a graph that has an
End that was never reached. The card points at that branch node.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ESQ1-303: both edges of a Builder-made if-else used handles the node
does not declare, so the engine skipped both arms, reached no End node,
and reported "succeeded" with outputs={}. Every verify handler took any
"succeeded" as "All checks passed".
A succeeded run that executed a branch node (if-else, question-
classifier, human-input, or a fail-branch node) and no End node is now
"Finished without output": an error-toned test card plus a card pointing
at the branch node, waiting at the gate with no staged repair -- the
engine reported no error, so there is nothing to diagnose. The rule is
deliberately that narrow: requiring an End node for every succeeded run
would false-fail side-effect-only branches, which the structural
validator does not forbid.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
apply_repair streams a canvas marker per applied intent before its
preflight runs, so a rejected build left the client with markers for
mutations that were never written. The plan-approval catch now emits
revert_checkpoint: nothing was written and the draft is still the
checkpoint taken at approval.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
apply_repair now rejects a graph Graph.init would refuse (PreflightError).
Build's and Edit's plan approval called apply_repair with no guard, so
the rejection would have escaped the handler and left the operation
interrupted mid-step. Both now catch it: the step is marked failed, an
error card names the node and the reason, nothing has been written, and
the plan stays at its gate -- re-approving regenerates the graph.
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>
validate_node_config promises to raise exactly when Graph.init would.
That only holds while it and DifyNodeFactory.create_node run the same
validation steps, so both now call one helper instead of two copies.
No behavior or error-text change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A graph-init failure surfaced as "2 validation errors for
HttpRequestNodeData ..." -- pydantic names the model class, never the
node. graphon logs the node id and re-raises the raw error, so the error
frame the Builder received (ESQ1-302) could not say which node was
wrong, and its repair agent had to guess.
DifyNodeFactory.create_node now re-raises every ValueError from the
validation stage (shared-shape validation, class resolution, concrete
NodeData re-validation) prefixed "node '<id>' (<type>): ". This runs for
EVERY workflow start, so the error text of every graph-init failure
changes, not only the Builder's. The wrapped error is a plain ValueError,
so app_generator's handling takes its ValueError branch (log only under
DEBUG) instead of the ValidationError branch; the published error is the
same.
validate_node_config runs the same three validation steps with no
factory instance and no runtime state -- the dry Graph.init a draft can
be put through before it is written or run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
run_draft populates Run.error, and all three consumers threw it away:
Build and Edit assigned run_error = "" and Fix's Run(...) literal had no
error= at all. A launch failure therefore persisted with an empty error
(the long-open "TRUNCATED_STREAM_ERROR not persisted, err_len=0" item),
diagnose saw nothing, and the repair breaker -- which keys a launch
failure on run.error -- could never fire on one.
Fix's handle_verify also gains the try/except Build and Edit already
have, so a run_draft that raises degrades to a failed Run carrying the
launch error instead of killing the advance.
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>
The migration graph had two heads, both descending from d8e4a6b1c902:
- a9cc8d552db4 (add error to dify_builder_runs) on the Builder branch
- e7b2a9c4d601 (add agent sandbox usage) from main, which arrived here
through merge commit d08c189614
With two heads, `flask upgrade-db` fails at startup with "Multiple head
revisions are present", so the dev dify-api pod built from this branch
was crash-looping and the rollout never completed.
Add an empty merge revision (733e4c46f90e) whose down_revision is
("a9cc8d552db4", "e7b2a9c4d601") instead of re-parenting either one.
Both revisions are already pushed and a9cc8d552db4 is already applied on
the dev database. Re-parenting a9cc8d552db4 onto e7b2a9c4d601 would make
that database believe it is at head while the sandbox usage tables from
e7b2a9c4d601 were never created. The merge revision makes alembic apply
e7b2a9c4d601 on databases that stopped at a9cc8d552db4, then stamp the
merge.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Connect both creation inputs to the instruction-improvement endpoint without overwriting newer edits. Guide users through unavailable model setup and refresh both default-model query caches after configuration.
Include generated contracts, locale strings, and creation-flow tests.
Show resource readiness and authorization status, submit the selected resources and prefilled test inputs, and expose collapsible test output.
Cover the requirements-to-plan flow and interrupted plan recovery, with matching locale updates.
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: the plan card's version_tag was renumbered v2 -> v1
in an earlier commit, but handle_resource_recommendation's own reply_text
still read "Plan v2 ready for approval." -- a card that says v1 next to
prose that says v2 is worse than leaving both at v2, since it's exactly
the user-visible inconsistency the renumber exists to eliminate.
Updated the reply_text to "Plan v1 ready for approval." and its matching
entry in the strings.py literal catalog, so test_catalog_covers_handler_
literals keeps passing for the right reason (a registered v1 string, not
a stale v2 one). Grepped the whole of core/dify_builder/ for any other
quoted v2/v2.1 string this renumber might have missed -- this was the
only one.
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>
handle_capability_check now advances a session that already carries a goal
(fc.goal_text set at creation) on ANY turn, not only send_goal -- the
composer already carried the goal in, so requiring a further send_goal
click was a second decision-free gate. A goal-less (create-from-blank)
session still waits for send_goal to supply one; that action id stays in
_ACTIONS_FOR for exactly that case.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
With build.initial_plan's plan card gone (previous commit), the plan card
emitted by handle_resource_recommendation on confirm_resources is now the
FIRST and only plan card a build shows -- so it becomes v1 instead of v2.
The build-complete plan card in handle_plan_approval follows suit,
v2.1 -> v1.1. A build no longer displays "the same plan twice under two
version numbers".
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>
api/openapi/markdown/console-openapi.md still documented the removed
ConflictPolicyOption schema and ResourceSelectCard.conflict_policy_options
field as live API contract after e8b14b50ff. Regenerated via
api/dev/generate_swagger_markdown_docs.py and kept only the two hunks
that remove the conflict-policy schema; discarded unrelated churn the
regeneration also picked up (the already-merged /workflow-generate/improve
endpoint's docs, never regenerated by that earlier commit).
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>
The App Builder composer's "Add missing details" link needs a backend. It
fires in the pre-creation composer, so there is no app, no draft workflow and
no Builder session yet -- it is tenant-scoped only, which puts it beside
/workflow-generate/suggestions rather than under console/dify_builder.
POST /console/api/workflow-generate/improve expands a short build instruction
into one that states what a builder would otherwise have to ask about: the
trigger and inputs, the steps, the rules that decide between outcomes, the
output and its destination, and the failure path.
Soft-fail like suggestions: no default model, an LLM error, an echo of the
input, or a rewrite shorter than the input all return "" from the generator,
and the route answers 200 with the caller's own text and changed=false. The
action can never strand what the user typed, so there is deliberately no
402/429 mapping.
Deliberately not grounded in the tenant's tools or knowledge bases. Naming
tools here would prejudice the workflow generator's own tool router, which
selects from the full installed catalogue against the instruction it is
actually given -- and it keeps the endpoint free of DB and plugin-daemon
calls.
The boundary guard is split out rather than shared: _workflow_instruction_guard
names ideal_output in its wording, which this payload does not carry. The
existing endpoints' error strings stay byte-identical.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The main merge added Azerbaijani (#42291) as a console locale. The naming
fallback keys its "New app" stem by console locale, so a new locale without a
stem silently falls back to English; the catalog-completeness test caught it on
rebase, which is what that test exists for.
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>
Pause draft saves after version conflicts until the latest graph and hash are reloaded. Allow App Builder canvas refreshes to resolve the conflict without adapting or validating node data.
Refs: ESQ1-282
Mount canvas and panel subscriptions before restoring persisted sessions. Release canceled loading state and ignore stale restore responses so graph generation can resume after a page refresh.
Cover delayed conversation history, stream reconnection, and StrictMode restoration. Fixes ESQ1-283.
Remove Fix entries from test run history. Keep the action below the failed status card in the live Test Run Detail panel and match the App Builder design.
Refs ESQ1-262.
Restore the editing draft before starting Fix from run history. Serialize Fix startup and clear the previous session UI when switching sessions.
Refs ESQ1-262.
After a main-merge pulled in d8e4a6b1c902 (expand_normalized_email_providers),
the Alembic graph had two heads: the Builder line (…→a9cc8d552db4) and the
email-providers migration both forked off c3f1a9b2e6d4. `flask db upgrade head`
then fails with "Multiple head revisions are present", crash-looping dify-api
on startup (the only pod that runs migrations).
Re-parent b28a1b2fbf4d (add_workflow_copilot_tables) from c3f1a9b2e6d4 to
d8e4a6b1c902 so the Builder migrations chain after main's, yielding a single
head a9cc8d552db4. Column-add is independent of the email-providers change, so
the reordering is schema-safe.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014cAkpbcvNmWY827x6SJsrM
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>
A verify/affected-path run that fails because the workflow references a model
that isn't configured/available (ModelNotExistError, "Model X does not exist",
provider/credentials errors) was routed into the config-repair path -> diagnose
+ propose_repair kept proposing graph mutations (if-else / check nodes) that
can't fix a missing model, thrashing across rounds.
Add model_config_error_text() (classifier parallel to is_input_failure; pairs an
existence/credential signal with a model/provider/credential token so unrelated
"<node> does not exist" faults aren't swallowed). In both build and edit failure
branches, when it matches, surface a clear "model not configured" diagnosis with
NO staged repair and route to the await-repair gate: the user configures the
model (global update_model) or changes it, then re-runs. Catalog the two new
user-facing strings so the reply-language layer localizes them.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Dify has no native boolean start-variable type, so a bool requirement the
Builder proposes gets emitted as a `select` — and the LLM often leaves its
`options` empty. An options-less select renders as an empty, unfillable dropdown
in the Builder's test-input form, so the user can never submit test data
(affects every workflow whose requirements include a bool field).
Add _normalize_start_select_variables to the generator postprocess (beside
_normalize_start_file_variables): a `select` start variable with no usable
(non-empty string) options is converted to a `text-input` the user can type
into; a select that declares real options is normalized to those strings and
kept. Idempotent.
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>
Builds failed with an empty graph when the generator LLM wired downstream
references to an HTTP-request node's output as `{#node.response#}`. HTTP-request
nodes expose only {body, status_code, headers, files} — never `response` — and
the reference-repair pass only rewrites to a node's SOLE output, so with four
outputs the ref stayed unresolved: structural validation failed
(runner.py:646 UNRESOLVED_REFERENCE), generate_workflow_graph returned no graph,
and build_nodes produced an empty build ("Couldn't build the workflow").
Add a targeted, unambiguous output-name alias (http-request `response` -> `body`,
the response body) applied in _reconcile_variable_references before the
sole-output fallback, guarded so it only rewrites when `body` is actually
declared. Extensible via _OUTPUT_ALIASES for future known-wrong names.
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.
Panel-header kebab that downloads a session's engine/transport trace (inbound
SSE frames + outbound commands) as JSON, for debugging. Non-reactive per-session
ring buffer, capture taps in the session controller, a pure export assembler +
circular-safe serializer, and the exportDebugLog i18n key across all 24 locales.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Task 1-3 implementers ran vp test but not vp check, so the trace
modules/specs shipped with formatter drift and TS2532 (possibly-undefined
index access) errors that vp test does not catch. Apply the formatter and
use optional chaining on array-index access in the specs, matching the
existing spec convention (calls[0]?.[0]).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A new session created via startBuild/startEdit/startFix/startChecklistFix
without an intervening reset() left the previous session's trace frames
in the buffer, so a debug-log export's meta (describing the new session)
could be attached to a trace still containing the prior session's frames.
Clear traceRef in runCommand's startsSession branch, before the outbound
session_start append, so every new session starts with a fresh buffer.
Also adds two one-line clarifying comments: the inbound trace capture
site notes the payload is captured by reference and serialized only at
export time, and the serializeTraceExport replacer notes its single-
WeakSet cycle guard can also flag shared-but-acyclic sibling references
as [Circular] (acceptable for a debug export).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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>
apply_repair's already-present filter (added for interrupted-step Retry
idempotency) compared create_node/connect intents against before_graph
alone, with no awareness of delete_node intents in the same batch. A
from-scratch build sends delete_node(placeholder_start) + create_node(same
id) in one apply_repair call to replace the canvas's default placeholder
start with the generator's own start node -- the filter dropped the
create_node as "already present" while the delete_node still ran, deleting
the start node with no re-create, so the following connect raised
ValueError: node not found. Mirror handlers_build.py's existing M2 guard:
subtract in-batch delete_node targets from the present-node/edge sets
before filtering, so a delete+recreate of the same id in one batch survives.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drops create_node intents whose id already exists and connect intents whose
edge already exists (referenced against the original draft graph), so a
fix.apply Retry re-applying the same intents is a clean no-op instead of a
duplicate/ValueError. graph_ops validation (raise on a genuinely fresh
duplicate id) is preserved.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
progress_bus.subscribe() eagerly enters the pub/sub subscription (opening a
Redis connection + starting a listener thread) BEFORE the SSE generator
exists, so the subscribe completes before the Celery advance can publish.
But the only close() lived in the generator's finally -- which Python does
not run if the generator is never started. A streaming Response dropped
before its first frame (client aborted, or an after_request/teardown error)
therefore leaked the connection + thread until process restart.
stream_advance_frames now returns a small _ClosingFrameStream wrapper whose
close() closes the subscription directly (idempotent); the WSGI server calls
close() on the response iterable even on early disconnect, so cleanup fires
regardless of iteration. The inner generator keeps its own finally for the
iterated/disconnect paths, so this is strictly additive -- no happy-path
change. All seven call sites get it unchanged.
Adds a regression test (never-iterated stream -> close() -> subscription closed).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two paths ended a streaming advance without a terminal `state`/`error`
frame, leaving the SSE client stalled to the deadline with a stale held
version -> its next action 409s:
1. advance_session published the terminal `state` frame outside the
try/finally and unguarded. If the post-advance view projection
(get_session_view -> read_graph) or the publish itself raised after a
SUCCESSFUL advance, the exception escaped and no terminal frame was
sent. Guard it and fall back to a terminal `error` frame.
2. submit_action_stream emitted a terminal `state` frame on settle only
for `message`. `update_model` also settles synchronously while bumping
the version, so it now emits one too -- otherwise the FE keeps the old
version and 409s on its next action.
Adds regression tests for both (projection-failure -> error frame;
update_model -> state frame).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This branch removed the Dify Builder contract generator (contract_gen.py
and the gen-dify-builder-contract npm script), but autofix.yml still
invoked that script. On every api/** change the "Generate frontend
contracts" step failed (command not found under bash -e), turning the
job red and skipping the autofix-ci commit step -- so even the good
gen-api-contract output never landed. Remove the stale invocation and
the now-redundant contract_gen.py path-filter entries (already covered
by api/**).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
WorkflowService.publish_workflow returns a single Workflow (-> Workflow), but
dify_port.publish still unpacked it as (workflow, retirement_candidates) -- a
stale signature. Every other caller was updated when the return type changed
(via the main merge); only the Dify Builder port was missed, so every publish
crashed with "TypeError: cannot unpack non-iterable Workflow object", surfaced
to the user as a generic "step failed" SSE frame.
Assign the single return value (retirement_candidates was unused anyway). The
existing test_dify_port publish test mocked publish_workflow as a 2-tuple, which
masked the bug -- corrected it to the real -> Workflow signature so it now guards
the regression.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A build for a workflow app whose goal reads like a chatbot produced an empty
canvas while the assistant reported "Workflow built on the canvas.": the plan
was chatbot-shaped, build_nodes fed it to the generator (mode="workflow"), the
LLM emitted a chatflow graph (answer node / no end node), the generator
rejected it as MISSING_TERMINAL -- a class it does NOT retry -- and build_nodes
swallowed the error into []. handle_plan_approval then treated [] as a
successful build: it deleted the placeholder start and advanced to
build.execution, leaving nothing on the canvas.
Two parts:
1. build_nodes now prepends an explicit workflow-topology directive (one
start + >=1 end node, no answer nodes) to the generator instruction, and
retries ONCE on an error/empty result with the specific error fed back as
a corrective nudge -- covering the terminal-node failure the shared
generator skips.
2. handle_plan_approval refuses to fake success: when build_nodes yields zero
create_node intents it surfaces an honest error and stays in
build.plan_approval (retryable) without deleting the placeholder, instead
of emptying the canvas and claiming a build. A loop-back re-approve still
returns the fixed create intents (only filtered as already-present), so
zero create intents unambiguously means a genuine generation failure.
Adds regression tests for both. Note: part 1 improves LLM reliability
(verifiable live); part 2 is the deterministic guarantee against the
false-success empty canvas.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
uv run --project selects the venv but doesn't chdir, so invoking
gen-dify-builder-contract from packages/contracts (as autofix.yml does)
raised ModuleNotFoundError: No module named 'core'. Switch to
--directory so uv chdirs into api/ before running the module. Also
update two contract_gen.py docstrings that still pointed at the old
web/app/components/dify-builder/contract/types.ts output path.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Also repoints 4 additional consumers under web/app/components/dify-builder/
(state.ts, types.ts, use-dify-builder-session.ts, and its test) that
imported the contract via a relative path ('./contract/types' /
'../contract/types') rather than the '@/app/...' alias the brief's grep
pattern searched for. Deleting the old generated file without updating
these would have broken the build; a full `pnpm run type-check` confirms
they were the only additional consumers and the fix introduces no new
errors (baseline vs. post-change error sets are identical, both containing
only 27 pre-existing, unrelated .next/ typegen errors).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
It was stubbed to [] in LlmBuilderAgent, canned in PlaceholderAgent, and never
called by any handler — the Build failure path reuses Fix cognition (diagnose +
propose_repair). Removed from the DifyBuilderAgent Protocol, both implementations,
and all test doubles.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Final-review fix wave for the testdata-gate + start-selector slice:
- I1 (Important): is_input_failure's signal list included the bare
"required" (and generic "variable not found"), which also fire on
config/runtime faults unrelated to test inputs -- misrouting those
failures to the *_AWAIT_TESTDATA gate, where the diagnosed auto-repair
is never offered and re-running just loops at the gate. Narrowed
_INPUT_FAILURE_SIGNALS to ("file variable", "not provided", "missing
input"), favoring precision per the spec's ambiguous-failure ->
config-repair safe default. The real file-input E2E error ("File
variable not found for selector: [...]") still matches via "file
variable".
- M2 (Minor, latent): handle_plan_approval's from-scratch delete-then-
create-filter path could drop a generator create_node/connect that
reuses a just-deleted placeholder start's id, since _already_present
compared against the PRE-delete existing_node_ids/existing_edges --
delete with no re-create, silently vanishing the node. Deleted ids are
now excluded from that comparison set; the loop-back path (no deletes)
is unaffected.
- Cleanups: state.py's Build/Edit state-count comments (14->15, 9->10,
verified against the actual enum members), FormCard's docstring now
lists the "testdata" variant, and handle_plan_approval binds
current_graph's nodes to one local instead of calling .get() 3x and
documents the delete-placeholder branch.
Tests: extended test_is_input_failure_matches_input_signals's sibling
with a config "required" case (red before the signal-list fix, green
after), and added a plan_approval test where the generator's create_node
reuses the deleted placeholder start's id (red before the id-exclusion
fix -- a dangling connect ValueError -- green after).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Clear fc.test_input_ref and fc.verify_run_id on the four re-plan/revert
escape branches in Build and Edit (handle_review's continue_adjusting
and handle_reverted's retry_after_revert) so a rebuild+retest
regenerates fresh schema-shaped mock inputs instead of reusing the
first build's stale ones -- the approve_repair retest path is left
untouched since it must fairly retest against the same failing case.
Also corrects three doc/comment mismatches (llm_agent's 12->13 method
count, an await_repair docstring describing a branch that doesn't
exist, and state.py's Build state-count banner), and adds tests for
the run_draft-raises degrade path and Build/Edit pass-path canvas
event symmetry.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Task B2 review found that removing the old canned test_test_affected_paths_
emits_result_and_reaches_review test dropped two pass-path assertions that
none of the 4 replacement tests reproduced: that the success path fires both
the mark_test_success and mark_review_ready canvas events, and that the
emitted SummaryCard's variant is "review". Restore both onto
test_edit_test_pass_goes_to_review_with_real_run so a regression on either
point is caught again.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rewrite handle_test_affected_paths from a canned always-success stub to a
live handler mirroring Build's handle_test_and_repair (A5): derive mock
inputs once via start_schema/generate_mock_inputs and persist a TestInput
(reused on retest), run run_draft (degrading to a synthesized failed Run on
exception), and mint a real immutable Run. Pass -> edit.review with a green
TestResultCard + summary. Fail -> real diagnose/propose_repair, stage
fc.diagnosis/staged_repair/risk, emit a red TestResultCard + ErrorCard (+
ChangeSetCard when intents are proposed) -> edit.await_repair (the B1 gate).
No auto-apply; apply_repair still runs only inside handle_await_repair's
approve_repair branch.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extends the fail-routes-to-await-repair test to assert card content
(error tone, diagnosis body/node_id, change_set presence, mark_test_error
canvas event) and adds a dedicated test for the no-proposed-intents fail
branch, plus a light card-shape check on the pass path. Closes the
test-coverage gaps left by removing the old canned-arc tests in the A5
live-test rewrite.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Upgrade FakeBuildDifyPort.run_draft from canned stub to real fail-then-pass fake. Add mutable verify_pass flag (default True) and run_draft_inputs tracking, mirroring FakeDifyPort behavior. Emits node events and returns Run with per_node (succeeded when verify_pass, else failed node with error). FakeEditDifyPort inherits the live behavior automatically since it only overrides __init__.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Implement start_schema(graph: Graph) -> StartSchema that reads the start
node's declared input variables from graph.nodes[].data.variables into
{"variables": [...]}. Pure dict access — stays in core without importing
services. Returns {"variables": []} when there is no start node or it
declares no variables.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Final consolidated fix wave from whole-branch review of the Real Build +
Edit Cognition slice:
- _ground now overwrites the model dict on ANY create_node whose config
carries a model dict (llm, question-classifier, parameter-extractor),
not just "llm", so a drifted/hallucinated provider/name on those other
node types no longer flows through ungrounded.
- to_intents now carries a generator edge's sourceHandle/targetHandle
into the connect intent's args (source_handle/target_handle) so
branching nodes (if-else, question-classifier) wire onto the correct
port instead of collapsing onto the default handle; edges without
handles still produce bare {from_node, to_node} args.
- Cleared the 17 ruff errors (PT018/E302/E306/E501) across the slice's
test files by splitting composite asserts, adding missing blank
lines, and wrapping long lines -- no assertions weakened, no noqa.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Docstring said "9 now use real LLM cognition" but enumerated 3 Fix +
6 Build + 3 Edit = 12 methods (Task B3 added the 3 Edit methods
without updating the leading count). Fix the count to 12 of 14 to
match the enumerated list.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
analyze_impact/propose_edit_plan/build_edit_intents now call
services.dify_builder.agent.edit.* via _model_or_none() instead of
delegating to the canned PlaceholderAgent. 9 of the Protocol's 14
Build/Edit/Fix methods now use real LLM cognition; only
generate_mock_inputs and propose_build_repair (returns []) remain
deferred pending the live-test step.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
analyze_impact could raise on a non-list target_node_ids from the LLM (no
isinstance guard, unlike fields/values) -- violated the never-crash
contract. Guard it the same way. Also collapse the fake models' invoke_llm
signatures to **_kwargs (underscore-prefixed, no noqa) per the ARG lint
constraint, and pin two more malformed-shape degrade paths
(build_edit_intents non-list intents, propose_edit_plan non-list plan).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
analyze_impact now returns {fields, values, target_node_ids} instead of a
flat edit_rules dict, mirroring Build's goal-shaped form (A2). Rewire
handle_capability_check to read fc.form_fields/edit_rules/edit_target_node_ids
from that shape and render the FormCard via build_form_fields;
handle_impact_analysis merges the submitted payload on the dynamic
fc.form_fields keys instead of a fixed key tuple; handle_review's re_fix
branch re-renders the form the same way. Delete the now-dead
_EDIT_RULE_FIELDS/_EDIT_RULE_KEYS constants. PlaceholderAgent.analyze_impact
returns the new shape (the old fields become canned fields/values), so both
it and the LLM agent (which still delegates to canned) are shape-uniform.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
handle_test_and_repair's AssistantTurnItem.cards was unconditional even
though error/change_set cards are now only appended when
propose_build_repair returns intents -- the manifest claimed cards that
were never attached in the neutral (empty-repair) path. Make cards
conditional to match what's actually appended, and assert it in both
branches.
Remove three noqa comments added in the prior commit that suppressed
ARG001/ARG005 on new test code (the global constraint disallows noqa
for non-enabled rules); switch the genuinely-unused monkeypatch
args/lambda params to underscore-prefixed names instead, and drop a
build_nodes monkeypatch that exercised nothing. Add
test_remaining_build_methods_thread_model_and_tenant_args to cover
propose_plan_v1/discover_resources/bind_resources/build_nodes/
learn_from_build's model/tenant_id/model_config threading, matching
the existing analyze_goal coverage.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LlmBuilderAgent's 6 Build methods (analyze_goal, propose_plan_v1,
discover_resources, bind_resources, build_nodes, learn_from_build) now
delegate to services.dify_builder.agent.build instead of the canned
PlaceholderAgent. propose_build_repair returns [] -- there's no real
failure signal to diagnose until the live test step is de-canned.
handle_test_and_repair now conditions the error card + apply_repair on
a non-empty propose_build_repair result: placeholder mode keeps today's
"found and fixed" arc, while real cognition (which never reports a
failure yet) emits a neutral "no issues found" review summary instead
of a fake error/fix on a hardcoded node id that real build_nodes
wouldn't have generated anyway.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Grounding only overwrites provider+name on an llm node's model dict now,
preserving whatever mode/completion_params the generator produced instead
of synthesizing a temperature when none was present. knowledge-retrieval
grounding now assigns each node its own dataset_ids list copy instead of
aliasing one shared list across every KB node in the graph.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Fix S110: replace bare except-pass with explicit degrade behavior
- Fix E501: wrap long function signatures to <=120 chars
- Fix I001: sort imports in test file (resources to top)
- Fix PT018: split composite assertions into separate statements
- Add _BoomInstance for provider-error testing
- Add 5 degrade tests (analyze_goal, propose_plan_v1 x2, discover_resources, learn_from_build)
- Remove unused _KIND_BY_PREFIX dict
- Add noqa comments for test mock arguments
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Only map the first start-type node to canonical 'start'; additional starts
retain their original ids to prevent collisions and ambiguous intents.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
FINDING 1 (dataset visibility): DatasetService.get_datasets requires user
parameter and silently omits ONLY_ME/PARTIAL_TEAM datasets on RBAC tenants.
Query Dataset model directly by tenant_id for complete grounding inventory.
FINDING 2 (normalization guard): Move fetch+normalize inside _safe to prevent
malformed records from raising after _safe returns. Use lambda builders to wrap
complete resource transformation including record normalization.
FINDING 3 (test coverage): Add test_mixed_failure_partial_success (one source
fails, others succeed) and test_malformed_tool_record_degrades (missing
tool_name key gracefully degrades).
Minor: compute model id once and reuse for both id and label fields; ensure
E302 two blank lines between test functions.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Implement resources.py with list_tenant_resources() to normalize tenant's
configured models, datasets, and tools to ResourceRef(id, label, meta,
readiness). Each source degrades to empty list on failure for graceful
degradation.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
analyze_goal now returns {fields, values} instead of a flat requirements
dict, so the Build capability-check form renders whatever fields the
agent (placeholder or LLM) proposes rather than a hardcoded 6-field
constant. handlers_fix.build_form_fields whitelists/coerces the agent's
raw field specs into FormField (unknown type -> text). handle_goal_analysis
now merges submitted values against the dynamic fc.form_fields keys
instead of the deleted _REQUIREMENT_KEYS constant.
Also updates test_placeholder_agent.py's analyze_goal shape assertion,
which broke as a direct consequence of the new {fields, values} contract.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
filter_applicable's per-intent try/except only caught (ValueError, TypeError),
so a hallucinated insert_between with a non-dict edge arg raised an uncaught
AttributeError and crashed the whole advance instead of being rejected by the
dry-run. Broaden to except Exception (ruff does not flag it, so no noqa is
added). Add a regression test for the string-edge case.
Also update llm_agent.py's stale module docstring (Fix cognition is now real,
not delegated; Protocol has 14 methods, not 13) and fix a stray closing paren
in a test_handlers_fix.py comment.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_touched_node_types looked up connect intents via a nonexistent node_id key,
always resolving to "" and missing the external-side-effect type of either
endpoint. Branch connect on from_node/to_node instead, and add a test proving
the external risk floor fires on a non-structural set_node_config against an
http-request node.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- test_wiring.py: expected dict was missing SessionView's new `model` field
- model_resolver.normalize_completion_params: a string `stop` was being
char-split into a list of letters instead of wrapped in a single-element
list; added a regression test
- controllers/console/dify_builder._ping: log the exception server-side
before returning the ok:false envelope
- ruff-clean the slice's new/extended test files (ARG001/ARG002/ARG005 via
precise noqa, PT018 compound-assert splits, E306, E501) with no change
to test behavior or assertion meaning
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Behavior-preserving code-quality cleanup across the Dify Builder engine:
extract emit_canvas, build_change_set, mint_checkpoint, action_kind, and
merge_known_keys into handlers_fix.py (shared by handlers_build.py and
handlers_edit.py); extract _add in repository.py for the id-preservation
idiom; extract _make_edge (and unify the node/edge id-collision loops
into _next_unique_id) in graph_ops.py; extract _sync_graph_only in
dify_port.py. No behavior, generated ids, or wire output changed — same
387-test suite passes, ruff clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
FINDING 1: Updated module docstring in handlers_build.py to accurately
reflect that build.complete's completion summary is emitted by the
governance-tail handlers (handle_governance_feedback for automatic/disabled
policies, or handle_await_learning for the ask policy).
FINDING 2: Strengthened two per-policy governance tests to include notice
kind assertions:
- test_governance_automatic_learns_and_reaches_complete: add assert "notice" in kinds
- test_await_learning_accept_learns_and_completes: add assert "notice" in kinds
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rework handle_governance_feedback into the 3-policy branch (automatic/
disabled/ask) reading fc.skill_learning_policy, and add handle_await_learning
to resolve the ask-policy accept_learning/skip_learning turn. Both funnel
into a shared _emit_completion helper for the build-complete summary.
Registered BUILD_AWAIT_LEARNING in build_registry().
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The rename camelCased 'copilot' inside kebab-case filenames in import
specifiers (use-difyBuilder-session, ./difyBuilder-session-view) while the
files themselves were kebab-cased on disk — next build module-not-found.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The runner's check_recovery/recovery_continue/recovery_restart short-circuit
committed the transition and returned immediately, without entering the
advance loop. recovery_restart in FIX/FIX_CHECKLIST mode (the forced path for
structural_invalidating drift) lands on the *working* fix.diagnose /
checklist.diagnose entry state, which the runner must drive by running its
handler -- the early return stranded the session there with no UI actions.
Fix: rest only when the landed state is waiting/terminal (covers stop/resume,
check_recovery, recovery_continue, and BUILD/EDIT restarts unchanged);
otherwise consume the action and fall through to the existing advance loop.
Also: parity-test recovery.entry_state_for against the states
service.create_*_session actually assigns; add classify()-through-runner
coverage for the three remaining recovery classes and target_node_ids guard
branches; pin recovery_ref_for's message for config_only/structural_compatible;
and qualify dify_port.py's graph_ops delegations (module import) so
structural_fingerprint/node_ids no longer read as unqualified self-recursion.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds RecoveryRef to the contract, recovery_ref_for() to recovery.py, and
wires it into get_session_view() beside the existing CheckpointRef pattern.
Regenerates contract_schema.json and the FE types.ts from contract.py
(additive only: RecoveryRef + SessionView.recovery). Also updates
test_wiring.py's exact asdict() round-trip assertion to include the new
recovery: None default, since dataclasses.asdict(SessionView) is exhaustive.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every fc.last_snapshot_hash assignment across handlers_fix.py, handlers_build.py,
and handlers_edit.py now sets fc.last_structure_fingerprint alongside it, using
result.structure_fingerprint at apply-repair sites and
env.dify.structural_fingerprint(graph) at read-graph/restore sites.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Populate the frozen SessionView.checkpoint field from the session context's
active checkpoint_id. When set, build a CheckpointRef with the checkpoint_id
and a generic "Restore point" label; otherwise None.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add 4 end-to-end tests for the complete Edit flow: test_full_edit_flow_goal_to_publish
(capability_check → send_edit_goal → submit_edit_rules → approve_repair → run_affected_tests
→ review → publish_workflow → EDIT_PUBLISH terminal), test_full_edit_flow_keep_draft_completes_without_publish
(same but keep_draft without publishing), test_continue_adjusting_then_reapprove_is_idempotent
(loop back via re_fix), and test_revert_then_retry_after_revert_reapprove_is_idempotent
(loop via undo then re_fix). All tests drive the real Runner + edit_registry + PlaceholderAgent
+ FakeEditDifyPort with version-CAS for each advance().
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
continue_adjusting/revert/retry_after_revert all return to
build.initial_plan without resetting the already-built graph. Re-walking
find_resources -> confirm_resources -> approve_plan then re-emits the same
fixed create_node/connect intents, and the second apply_repair raised
ValueError on the colliding node id, dead-ending the session at
build.plan_approval.
Fix: before applying, read the current draft graph and drop any
create_node whose node_id already exists or connect whose (from, to) edge
already exists, so a re-approve after a loop-back is a no-op apply_repair
rather than a crash. built_node_ids is still set from the full 4-node set
regardless of what was actually applied this pass.
Adds an end-to-end loop test via the real Runner covering both the
continue_adjusting and retry_after_revert paths, asserting the session
reaches build.execution with the graph still at 4 nodes / 3 edges.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Completes Slice 1 Task 9's 'widen all DifyPort implementers' invariant — the
whole-branch review caught this third conformance stub that the per-task
reviews (aware of only WorkflowServiceDifyPort + FakeDifyPort) missed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add contract_gen.py: a pure, deterministic generator that discovers every
StrEnum/dataclass/card defined in contract.py (plus EntryMode,
ConversationItem, and SessionView) via introspection and emits the checked-in
contract_schema.json (JSON Schema draft 2020-12) and
web/.../contract/types.ts. The discriminated ConversationItem union is built
from _Card.__subclasses__(), one oneOf/union branch per card kind.
test_contract.py gains a lockstep test that byte-compares a fresh
generate() call against the checked-in files (guards drift/hand-edits) and a
jsonschema.validate test that proves the generated schema is itself valid
and accepts a real SessionView instance.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wire the existing startChecklistFix into the throwaway debug page: a Checklist Errors (JSON) input plus a Start Checklist Fix button, and send {passed:true} for the recheck action so await_recheck advances to the shared decision gate instead of looping.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
runAction sends base_version equal to view.version, but view was only updated from HTTP responses (create/action/refresh), never the SSE stream. While the engine auto-advances (diagnose to await_verify) the held version stayed frozen at create time, so the next action base_version CAS returned 409 conflict. Reconcile view from snapshot/state frames so its version tracks the session.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The add_workflow_copilot_tables migration (b28a1b2fbf4d) kept down_revision=e4708db55c1d after the branch was rebased onto main, forking the Alembic graph into two heads and letting a fresh DB skip the main lineage (e.g. workflows.version_number), which 500s the editor /workflows/draft. Re-point down_revision to the main head (56124e050600) so the chain is linear.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
create_fix_session now treats failed_run_id as the Dify workflow-run id and
records it as an immutable original-failed CopilotRun, pointing
fc.failed_run_id at that row so the async diagnose can resolve it. Closes the
FE->diagnose gap for a live Fix flow.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add SqlCopilotRepository implementing the Repository Protocol's session
lifecycle (create_session/get_session) and compare_and_advance as a real
DB-level version-CAS: a single UPDATE ... WHERE version=base_version, with
rowcount==0 disambiguated into NotFoundError (missing session) vs
ConflictError (stale version). A UNIQUE(session_id, seq) collision on
conversation items during compare_and_advance is caught and mapped to
ConflictError, making the DB the seq authority. Remaining Repository
methods (checkpoints/runs/test-inputs/list_conversation) are stubbed with
NotImplementedError, deferred to Task 5.
Tested against SQLite in-memory: create/get round-trip, CAS version bump,
stale-version conflict leaving the row unchanged, missing-session
NotFoundError, and duplicate-seq ConflictError.
Implements WorkflowServiceDifyPort, the real DifyPort adapter that wires
the pure P1 domain and Task 1-3 helpers to WorkflowService,
AppGenerateService, and the node-execution repository. Each method opens
its own session, resolves account/app via the identity helpers, and
mutates the draft graph client-side (no server-side patch primitive)
before writing it back with graph_only=True/sync_agent_bindings=False.
Pins the publish workflow_id+commit gotcha and the hash-mismatch
re-mapping. Confirmed via mocked-service unit tests and a runtime_checkable
isinstance(..., DifyPort) check.
Port dify-enterprise/server/pkg/enterprise/biz/copilot/{ports,errors}.go to
api/core/workflow_copilot as pure Python.
- errors.py: ConflictError, NotFoundError, BusyError (Exception subclasses,
replacing Go's sentinel error values).
- models.py: add Diagnosis, Risk, MutationIntent, ChangeSet, NodeEvent,
ApplyResult value dataclasses (Go: ports.go); tighten FixContext.diagnosis/
staged_repair/risk/change_set from Any to these real types now that they
live in the same module (avoids a models<->ports import cycle).
- ports.py: CopilotAgent / DifyPort / Repository as @runtime_checkable
Protocols, snake_case methods per the port plan; DifyPort methods take
actor: Actor instead of Go's ForwardAuth.
- test_ports.py: constructs each value type, asserts trivial stubs satisfy
each Protocol via isinstance, and checks the error types.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extend the Workflow Copilot editor panel to fix pre-publish checklist
errors alongside failed runs.
- Detect blocking checklist items client-side via useChecklist(nodes,
edges, {flowType}) and offer a "Fix N checklist issues" entry next to
the failed-run entry, splitting config errors (auto-fixable) from
unconnected/plugin-missing items (surfaced but not auto-fixed in v1).
- Extend use-copilot-session's create path to POST checklist_errors
(new startChecklistFix) alongside the existing failed_run_id path
(startFix unchanged).
- Drive checklist.await_recheck: re-run the checklist against the
current draft and submit the recheck action with
passed = (fixable remaining === 0) and the full remaining list.
- Add copilot.checklist* i18n keys to en-US, matching the existing
copilot.* keys' en-US-only precedent for this feature.
Add a copilot toggle button next to Test Run in the workflow header and
a left-mounted panel that runs the copilot Fix flow (start fix, live
SSE progress log, approve_repair/run_verify/provide_testdata/publish/
undo/re_fix actions, and a message box), auto-resolving the app id
(workflow store `appId`), workspace id (`currentWorkspaceAtom`), and
the most recent failed run (`useWorkflowRunHistory`) instead of
requiring manual entry.
Extracts the session/SSE request logic from the standalone
`/workflow-copilot-debug` page into a shared
`app/components/workflow-copilot` module (`use-copilot-session`) so
both the debug page and the new panel drive the same implementation.
Adds a `showWorkflowCopilotPanel` store flag (defaults false) mirroring
the existing `showEnvPanel` pattern, so the feature is fully additive
and does not affect rag-pipeline/snippet editors that reuse the same
header/panel shell.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@@ -7,12 +7,14 @@ description: Use when writing, changing, or reviewing Cucumber and Playwright te
`e2e/AGENTS.md` owns the suite architecture, lifecycle, commands, tags, generated-client boundaries, fixtures, and cleanup contracts. Read the nearest feature-scoped `AGENTS.md` when one exists. This skill adds no parallel package policy.
Cucumber owns scenario and hook execution and reporting. Playwright provides browser automation, contexts, pages, locators, actions, assertions, request, and tracing APIs; protocol exceptions remain with the package-defined owner. Their timeout domains remain separate, and importing from `@playwright/test` does not make Playwright Test runner configuration, fixtures, or retries applicable to this suite.
## Topic Routing
Read only the bundled reference required by the change:
- Locator, assertion, isolation, or waiting decisions: [`references/playwright-best-practices.md`][playwright]
- Scenario wording, step granularity, expressions, or tag design: [`references/cucumber-best-practices.md`][cucumber]
- Scenario wording, step granularity, expressions, World state, hook visibility, or tag design: [`references/cucumber-best-practices.md`][cucumber]
Check current official Playwright or Cucumber documentation before introducing a framework pattern that local code and references do not already establish.
@@ -23,7 +25,7 @@ Check current official Playwright or Cucumber documentation before introducing a
3. Read the target scenario, matching step definitions, and lifecycle files only when session or shared state matters.
4. Reuse an existing step when wording and behavior match; add one coherent scenario or step when they do not.
5. Keep browser actions and assertions at the public user boundary; keep setup, seed, polling, and cleanup at their package-defined owners.
6.Run the narrowest tagged scenario and package checks documented in `e2e/AGENTS.md`; broaden only for shared hooks, tags, or support changes.
6.For changes, run the narrowest tagged scenario and package checks documented in `e2e/AGENTS.md`; broaden only for shared hooks, tags, or support changes.
For review requests, lead with reproducible correctness failures, flake sources, or demonstrated architecture drift. Report the behavior verified and any external-runtime, browser, or environment gap.
### 1. Treat scenarios as executable specifications
Cucumber scenarios should describe examples of behavior, not test implementation recipes.
Cucumber scenarios should usually describe behavior declaratively, not reproduce an interaction script.
Apply it like this:
@@ -20,11 +23,15 @@ Apply it like this:
- avoid UI-internal wording such as selector details, DOM structure, or component names
- keep language concrete enough that the scenario reads like living documentation
Procedural detail is appropriate when the interaction mechanism is the behavior under test, such as keyboard navigation, focus movement, or a required multi-actor sequence.
### 2. Keep scenarios focused
A scenario should usually prove one workflow or business outcome. If a scenario wanders across several unrelated behaviors, split it.
A scenario should prove one business rule or coherent outcome. Cucumber's three-to-five-step recommendation is a review heuristic, not a hard limit: when a scenario is longer, check for multiple outcomes, incidental setup, or UI procedure that can move behind a domain step.
Keep each scenario centered on one coherent outcome. Avoid hidden dependencies on another scenario's side effects, and keep unavoidable setup outside the behavior narrative unless the precondition matters to the specification.
Use `Given` for initial context, `When` for the event, and `Then` for the expected outcome. Restarting those phases is a prompt to review the narrative, not an automatic failure. Avoid hidden dependencies on another scenario's side effects.
Use `Rule` when several examples illustrate one named business rule. Use a short `Background` only for shared context the reader needs to understand; do not hide fixture preparation, runtime readiness, or long setup there.
### 3. Reuse steps, but only when behavior really matches
@@ -45,6 +52,8 @@ Write a new step when:
Do not optimize for a low step count by making vague steps. Optimize for a small set of truthful, domain-owned steps.
Step definitions share one global matching namespace regardless of their directory. Organize them by domain capability and avoid feature-coupled glue or expressions broad enough to overlap unrelated behavior.
### 4. Prefer Cucumber Expressions
Use Cucumber Expressions for parameters unless regex is clearly necessary.
@@ -62,18 +71,24 @@ Use regex for a bounded natural-language alternative only when it keeps Gherkin
### 5. Keep step definitions thin and meaningful
Step definitions are glue between Gherkin and automation, not a second abstraction language.
Step definitions are glue between Gherkin and automation, not a second abstraction language. A step should express one coherent domain action or outcome; it may call several implementation helpers when that keeps UI mechanics out of the specification.
Keep each step to one user-visible action or assertion. In JavaScript and TypeScript, use `async function` when the step reads Cucumber World state because Cucumber binds `this`; do not leak state across scenarios through module globals.
Cucumber creates a new World for each scenario. Keep scenario state there rather than in module globals. Cucumber.js supports a `world` proxy for arrow functions, but this suite consistently uses typed `async function (this: DifyWorld, ...)` so ownership and TypeScript discovery remain explicit.
Hooks are invisible to feature readers. Keep them for low-level browser lifecycle, cleanup, and diagnostics; express business-relevant context in Gherkin.
### 6. Use tags intentionally
Tags should communicate selection or execution intent, not become ad hoc metadata. A tag does not change runtime behavior unless configuration or hooks implement it.
Tags may communicate stable capability grouping, selection, fixture dependency, or conditional-hook intent. A tag changes runtime behavior only when a runner, seed profile, or hook owns that meaning.
## Review Questions
- Does the scenario read like a real example of product behavior?
- Does it prove one outcome, or combine several independent phases?
- Are the steps behavior-oriented instead of implementation-oriented?
- Is procedural wording necessary to the behavior being tested?
- Is a reused step still truthful in this feature?
- Could a new expression overlap an existing step in the global namespace?
- Would `Rule` clarify the business rule, or would `Background` hide important setup?
- Is a new tag documenting real behavior, or inventing semantics that the suite does not implement?
- Would a new reader understand the outcome without opening the step-definition file?
- keep scenario state in the runner's scenario-owned context rather than module globals
- model special authentication or session setup through explicit per-scenario fixtures rather than shared mutable state
### 2. Prefer user-facing locators
### 2. Select locators by user contract
Playwright recommends built-in locators that reflect what users perceive on the page.
Prefer built-in locators that match how the target is exposed to users:
Preferred order:
- interactive controls: role and accessible name
- form controls: associated label; use placeholder when it is the relevant stable contract, especially when no label exists
- non-interactive content: visible text or the relevant text alternative
- elements without a meaningful user-facing contract: an intentional test id
1.`getByRole`
2.`getByLabel`
3.`getByPlaceholder`
4.`getByText`
5.`getByTestId` when an explicit test contract is the most stable option
This is a semantic choice, not a fixed ranking. Do not add incorrect roles or accessible names solely to satisfy a locator. When the product element should have user-facing semantics, fix that contract instead.
Avoid raw CSS/XPath selectors unless no stable user-facing contract exists and adding one is not practical.
Also remember:
Locators are strict for single-element actions. Scope to a stable region or use `filter({ has, hasText })` to make the intended target unique. Treat `.first()`, `.last()`, or `.nth()` as a review signal: the positional choice should be intentional and stable, not merely silence ambiguity.
- repeated content usually needs scoping to a stable container
- exact text matching is often too brittle when role/name or label already exists
-`getByTestId` is acceptable when semantics are weak but the contract is intentional
- when a real UI region, card, status, or icon lacks an accessible name, prefer adding that semantic contract in product code before falling back to `getByTestId`
### 3. Use web-first assertions
### 3. Use web-first assertions with the right timeout owner
Playwright assertions auto-wait and retry. Prefer them over manual state inspection.
@@ -64,6 +62,8 @@ If a condition genuinely needs custom retry logic, use Playwright's polling/asse
Use `expect.poll` for non-DOM truth such as API state, backend eventual consistency, generated resources, or captured browser events. For DOM state, use locator assertions so Playwright can apply actionability and web-first retry semantics.
Cucumber step and hook timeouts, Playwright locator/action timeouts, and Playwright assertion timeouts are separate budgets. `browserContext.setDefaultTimeout()` does not change the default five-second assertion timeout. Put an explicit longer assertion timeout only on the readiness owner that needs it; do not raise an outer timeout to mask a shorter inner failure.
### 4. Let actions wait for actionability
Locator actions already wait for the element to be actionable. Do not preface every click/fill with extra timing logic unless the action needs a specific visible/ready assertion for clarity.
@@ -77,15 +77,20 @@ Bad pattern:
- stack arbitrary waits before every action
- wait on unstable implementation details instead of the visible state the user cares about
- use `force: true` to bypass a real hit-target, overlay, or disabled-state failure
When synchronizing on a one-shot popup, download, request, or response, create the wait before triggering the action, then await the event. A scenario-owned listener may instead capture events for a later assertion. Do not use `networkidle` as an application-readiness assertion; wait for the user-visible state or owned backend contract.
### 5. Match debugging to the active harness
Playwright supports traces, screenshots, page snapshots, and browser logs. Configure artifact capture at the runner boundary instead of adding parallel diagnostics to individual scenarios.
Playwright supports traces, screenshots, page snapshots, and browser logs. Configure artifact capture in Cucumber hooks instead of adding parallel diagnostics to individual scenarios. If tracing is introduced, use `browserContext.tracing`; Playwright Test options such as `trace: 'on-first-retry'`, projects, workers, fixtures, reporters, and retries do not configure this harness.
## Review Questions
- Would this locator survive DOM refactors that do not change user-visible behavior?
- Is a positional locator expressing product order, or hiding an ambiguous match?
- Is this assertion using Playwright's retrying semantics?
-Is any explicit wait masking a real readiness problem?
-Does an explicit timeout belong to the condition that is actually slow?
- Was an event wait registered before the action that emits it?
- Does this code preserve per-scenario isolation?
- Is a new abstraction really needed, or does it bypass the runner's scenario-owned context and lifecycle?
@@ -5,14 +5,14 @@ description: Use only when the user explicitly requests a review or audit of fro
# Frontend Code Review
Review the requested scope for concrete, reproducible regressions. This skill owns the review phase and routes directly to its bundled rule packs. For a combined review-and-fix request, establish findings before applying implementation or testing guidance.
Review the requested scope for concrete defects and violations of explicit project contracts. This skill owns review decisions; its references route to canonical rules without activating another skill's implementation workflow.
## Evidence First
1. Establish the review scope from the requested files or current diff.
2. Read the changed lines, their behavior owner, and the nearest scoped `AGENTS.md`.
3. Trace public consumers, generated contracts, primitive APIs, or runtime configuration only when they decide correctness.
4. Report only findings tied to an observable failure, violated contract, security boundary, or demonstrated maintenance risk.
4. Report findings tied to an observable failure, violated contract, security boundary, or demonstrated maintenance risk. Explicit team conventions are contracts: establish their scope and exceptions, and do not invent user impact to justify a convention finding.
## Rule Routing
@@ -27,16 +27,16 @@ Read only the packs matched by the diff:
- Stable Dify runtime invariants in the named paths: [`references/dify-invariants.md`][dify-invariants]
- General TypeScript or styling quality not owned above: [`references/code-quality.md`][code-quality]
Read `packages/dify-ui/README.md`, `packages/dify-ui/AGENTS.md`, `web/docs/overlay.md`, or `web/docs/test.md` only when the reviewed code falls under that contract. Check current official documentation when local code and bundled references do not settle a framework, browser, or accessibility behavior.
Read `packages/dify-ui/README.md`, `packages/dify-ui/AGENTS.md`, `packages/dify-ui/docs/overlays.md`, or `web/docs/test.md` only when the reviewed code falls under that contract. Check current official documentation when local code and bundled references do not settle a framework, browser, or accessibility behavior.
## Severity And Output
- **P0**: security or privacy leak, data loss, production crash, or inaccessible critical workflow.
- **P1**: user-visible regression, invalid API or authorization contract, hydration failure, or broken primary interaction.
- **P2**: concrete maintainability, performance, test, or accessibility defect likely to cause incorrect behavior.
- **P2**: concrete maintainability, performance, test, or accessibility defect, or a material violation of an explicit project contract.
- **P3**: minor actionable cleanup; omit unless the user requested a thorough audit.
Lead with findings ordered by severity. Include a tight file and line reference, the failing contract or reproduction path, impact, and a concrete fix direction. If there are no findings, say `No issues found.` and state any material verification gap. Do not add praise sections, speculative risks, or an unsolicited offer to implement fixes.
Lead with findings ordered by severity. Include a tight file and line reference, the observed failure or applicable project rule, and a concrete fix direction. Explain the rule's applicability for convention findings; describe downstream consequences only when supported by evidence. When no findings remain, say so briefly and state any material verification gap. Do not add praise sections, speculative risks, or an unsolicited offer to implement fixes.
Do not treat that document as the complete accessibility rule set. Combine it with:
Use the sources relevant to the changed contract:
-`packages/dify-ui/README.md`, `packages/dify-ui/AGENTS.md`, and the relevant primitive implementation when code uses `@langgenius/dify-ui/*`.
- Base UI docs and local `.d.ts` contracts when primitive semantics, focus target, labels, or popup reachability are unclear.
- MDN or relevant WAI-ARIA/browser standards when behavior, compatibility, or deprecation status matters.
- The current feature's product semantics, because an accessible primitive can still be used in an inaccessible workflow.
Consult current official documentation or standards when these sources leave a behavior unresolved. The [Web Interface Guidelines] are an optional broader UI reference, not a required fetch for each review.
## Semantic HTML
Flag:
- Clickable `div` or `span` used for actions.
- Router navigation implemented with button or `onClick` when a `Link` / `<a>` is the real semantic element.
- Icon-only buttons without `aria-label` or `aria-labelledby`.
- Icon-only controls without an accessible name; follow the naming rules below and the Dify UI `IconButton` contract.
- Decorative icons missing `aria-hidden="true"`.
- Images without `alt`; use `alt=""` only when truly decorative.
- Heading levels that skip hierarchy in page-level content.
Prefer semantic HTML before ARIA.
## Page Landmarks
When reviewing Web page layouts, navigation, or landmark changes, read [Page landmarks].
Inspect the composed page with its parent layouts for missing or duplicate main regions,
inappropriate nesting or roles, and ambiguous or broken landmark names. Check affected
unit and E2E locators when semantic elements change. Web owns these composition rules;
use the existing naming contract for label-source decisions.
## Accessible Names And Descriptions
Read [Accessible names and descriptions] when a change affects labels, ARIA naming, help/error relationships, or hidden text. That document owns the shared implementation and review contract.
Flag violations supported by the final rendered behavior:
- Missing or insufficient names, redundant name overrides, or naming attributes prohibited by the element's role.
- Overrides that omit visible label wording or suppress necessary descendant information.
- Broken or stale label/description references, including relationships lost when responsive content or overlays unmount.
- Descriptions used instead of names, repeated help text, or essential structured content available only as a flattened description.
Inspect the computed name and description in the relevant state. Matching an `aria-label` string to nearby text alone does not prove redundancy.
## Keyboard And Focus
Flag:
@@ -48,7 +67,6 @@ Flag:
- Missing stable `name` on form fields that submit or validate.
- Incorrect input `type`, `inputMode`, `autoComplete`, or `spellCheck` for email, token, URL, number, search, code, or username fields.
- Labels that are not clickable.
- Submit buttons disabled before a request starts, preventing normal submit behavior.
- Non-submit buttons inside forms missing `type="button"`.
- Errors not associated with fields or not reachable by screen readers.
- Error recovery that does not focus or expose the first invalid field.
@@ -56,16 +74,20 @@ Flag:
- Placeholder text used as the only label.
- Password managers accidentally triggered on non-auth fields because autocomplete is missing or wrong.
Prefer visible labels. If visible surrounding text already labels the control, use a visually hidden label or a precise `aria-label`.
Prefer visible labels and associate them through the appropriate field primitive, a native `label`, or `aria-labelledby`. Do not duplicate an existing label with hidden text or `aria-label`; follow [Accessible names and descriptions] when no suitable visible label exists.
## Disabled, Loading, And Async States
Flag:
- Loading state without `aria-busy`, `role="status"`, or another accessible update path when it changes user interaction.
- Loading controls whose accessible name disappears, or whose user-relevant progress has no
feature-owned status path. Follow the Dify UI Button contract for focused loading buttons; do not
add `aria-busy` to a button as a generic pending-state substitute.
- The same pending state passed to both Dify UI Button `loading` and `disabled`, which duplicates
state ownership and obscures whether `disabled` expresses independent unavailability.
- Spinner or decorative loading icon exposed to screen readers.
- Disabled controls that hide the reason users cannot proceed.
-`aria-disabled`used without manually blocking click, Space, and Enter.
- Controls marked`aria-disabled`that still activate through supported pointer or keyboard input. Check the primitive's handling before requesting manual event guards.
- Toasts, inline validation, or async status changes that are not announced when users need the update to continue.
- Icon-only loading/error affordances without text or accessible status where the state matters.
@@ -90,7 +112,7 @@ Use Popover for explanatory content, rich help, and infotips. Use Tooltip only a
Flag:
- Text in flex/grid children without `min-w-0` when it can overflow.
-Names, labels, file names, model names, workspace names, or user content lacking `truncate`, `line-clamp`, or `break-words`.
-Long names, labels, or user content that overflow, obscure adjacent controls, or become unreadable in supported layouts.
- Right-side icons, badges, checks, or actions that shrink before the text area.
- Empty arrays or empty strings rendering broken layout instead of an empty state.
- Button, tab, badge, chip, menu item, or card text that can overlap sibling controls at common viewport widths.
@@ -107,3 +129,7 @@ Flag:
- Images without dimensions.
- Loading copy using `...` instead of `…`.
- Hardcoded dates, times, numbers, or currency formats instead of `Intl.*`.
[Accessible names and descriptions]: ../../../../packages/dify-ui/docs/accessible-names-and-descriptions.md
For Dify UI/Base UI state styling and callback composition, apply the package's [Styling] contract,
including its documented reasons for direct state access.
Flag:
- New CSS modules or ad hoc CSS when Tailwind utilities and Dify tokens cover the need.
- Component-level plain `.css` files or component CSS imported through `globals.css`; use scoped `*.module.css` only when Tailwind and component variants cannot express the style.
- Generic color utilities where Dify semantic tokens exist.
- Hardcoded magic class values for colors, spacing, radius, shadow, z-index, or typography when Dify tokens, component variants, or documented radius mappings exist.
- Hardcoded magic class values for colors, spacing, radius, shadow, z-index, or typography when Dify tokens, component variants, or standard Tailwind radius classes match the required design value. Apply [Styling] when choosing radius classes.
-`!` important modifiers or important CSS overrides without a narrow, documented reason.
- Manual string concatenation, template strings, array `.join(' ')`, or custom ternaries for conditional or multi-line classes.
- JS conditional class branches for primitive visual states already exposed by Dify UI/Base UI `data-*` selectors.
- Manual class-list assembly through string concatenation, template strings, array `.join(' ')`, or a custom conditional combiner instead of `cn(...)`. Conditional values passed to `cn(...)` remain valid.
- Incoming `className` placed before default classes in `cn(...)`, preventing call-site overrides.
- Arbitrary z-index or one-off layering fixes on overlays.
@@ -43,7 +45,6 @@ Use:
-`cn(...)` from the local package or utility already used by the file.
- Dify semantic tokens and Tailwind v4 utilities.
- Existing component variants before one-off class forks.
- Primitive selectors such as `data-disabled:*`, `data-checked:*`, `data-highlighted:*`, `group-data-*`, `peer-data-*`, and `has-[:focus-visible]` before adding React state or boolean props solely for styling.
- Component-level variants, semantic tokens, and normal cascade/order before `!` overrides. Use `!` only for a contained compatibility override that cannot be expressed through the component API or local selector structure.
## Imports
@@ -66,3 +67,5 @@ Flag:
- Error messages that state only the failure and not the next step.
Use feature-local translation keys by default. Alias only when crossing namespaces. `pnpm i18n:check --file <name>` should pass for any touched translation namespace.
Use these rules for React component structure, ownership, state, props, effects, and module organization.
Use the canonical reference for the changed concern. These links share rules; they do not activate the implementation skill or its workflow.
## Ownership
| Concern | Canonical rules |
| --- | --- |
| Vertical modules, public entrypoints, data/handler placement, wrappers, Props, and types | [Ownership] |
| Local/Jotai state, form drafts, route identity, URL state, and persistence | [State] |
| Effects, navigation, memoization, and subscriptions | [Runtime] |
| Hotkeys, focus, and secondary surfaces | [Interactions] |
Flag:
## Apply Rules In Their Actual Scope
- State, query, mutation, or handlers hoisted above the lowest component that actually uses them.
- Parent components owning row/item actions that do not coordinate a workflow.
- Prop drilling through multiple pass-through layers.
- A page/tab-level section component becoming the data owner without needing a shared snapshot or shared loading/error/empty UI.
- Feature code promoted to shared only because it appears once or might be reused later.
Explicit team conventions are reviewable contracts, including module organization and public API boundaries. Check the documented exception before reporting a violation. Do not infer an exception solely because the code appears to work, or invent a user-facing failure for a convention finding.
Accept repeated TanStack Query calls in siblings when each component independently consumes the data. Cache deduplication is not a reason to hoist by itself.
- For owner placement, trace the consumers and required lifetime. Establish whether the parent coordinates a snapshot, submission, navigation, shared UI, or persistence beforeasking to move state or handlers.
- For component boundaries, identify the ownership or encapsulation the proposed extraction would improve; file length alone establishes neither.
- For props and types, check the domain contract and public API. Do not report private props typing style alone; declaration/export syntax matters only for a documented package rule or concrete type, export, or framework defect.
- For state and Effects, trace the source of truth, external synchronization target, and mount/reset boundary. Controlledness alone does not prove that a draft is lifted or persisted; follow the form and overlay contracts linked by [State].
- For navigation, distinguish ordinary links from mutation success, guarded redirects, command flows, and submission side effects.
## Component Boundaries
## Preserve Existing Product Contracts
Flag:
During refactors, trace the interaction being moved through its real consumer. Navigation, sidebar, dropdown, webapp-list, and app-switching changes must preserve expansion controls, hover persistence, pin/delete actions, routing, keyboard/focus handling, and open-state ownership where present.
- React component files over 300 lines when the file mixes multiple responsibilities that can be split into focused colocated components, hooks, or utilities.
- Shallow wrappers that only rename props or hide the real primitive.
- Extra DOM wrappers that do not provide layout, semantics, accessibility, state ownership, or library integration.
- Dialog/dropdown/popover hidden surfaces that obscure the parent flow when they should be extracted into a small local component.
- Business forms, menu bodies, or one-off helpers moved away from their owner without reuse or semantic value.
Check that the changed owner still handles reachable empty, loading, and missing optional-data states, and that primitive wrappers preserve accessible semantics and the public controlled-state contract. Report the actual lost behavior or explicit rule violation; use the package testing policy when assessing regression coverage.
Prefer colocated components split by actual data and state needs.
## Bad Component Design Patterns
Flag:
- Refactors of existing navigation, sidebar, dropdown, webapp list, or app-switching UI that do not preserve behavior-sensitive interactions such as expand/collapse arrows, hover persistence, pin/delete controls, routing, keyboard/focus handling, or open-state ownership.
- Components that mix data fetching, mutation side effects, popup state, form validation, layout, and row rendering without a clear owner.
- Generic components with many boolean props that encode one feature's workflow.
- A shared component that imports feature-specific copy, routes, or API contracts.
- A feature component that accepts pre-rendered fragments only to avoid placing ownership correctly.
- A child component that receives both raw server data and separately derived flags for the same concept.
- A wrapper that changes accessible semantics of the primitive it wraps.
- A component that exposes controlled props but still keeps a competing private state for the same value.
- A component that cannot render empty, loading, or missing optional API fields without caller-side preprocessing.
When existing components already own interaction logic, prefer reusing or extending them. If a refactor is necessary, preserve the old interaction contract and add or update focused tests for changed behavior.
## Props And Types
Flag:
- Declaration or export rewrites made only for stylistic uniformity, without changing an owned behavior or contract.
- Named `Props` types for trivial one-off props where inline typing is clearer.
- Props named by UI implementation instead of domain/API role.
- API data converted too early or under a generic name that breaks traceability.
- Callers duplicating fallback checks that the lowest rendering component already handles.
Do not flag `FC`, `React.FC`, function declarations, arrow functions, named exports, or default exports by syntax alone. Report them only when the chosen form causes a concrete type, lifecycle, export, framework, or enforced package-contract defect.
## Effects
Flag effects that:
- Transform props/state for rendering.
- Copy one state value into another representing the same concept.
- Handle user actions that belong in event handlers.
- Reset state from props when a keyed reset, stable ID, or render-time derivation would work.
- Fetch data that belongs in framework APIs or TanStack Query.
If an effect remains, it must synchronize with a named external system: browser API, subscription, timer, analytics-on-visibility, non-React widget, or imperative DOM integration.
## State Modeling
Flag:
- Storing derived booleans, disabled flags, default tabs, or loading labels that can be calculated from current query/feature state.
- Local state used to fake server data or generated contract fields.
- UI state persisted to localStorage when it is live app state.
- Feature-local mock shells wired to unrelated existing APIs before the real API is confirmed.
Prefer render-time derivation. Keep true local state for user choices, transient input, controlled popups, and feature UI state that has no server source.
## Navigation
Flag:
- Imperative router navigation for ordinary links.
- Button semantics used for navigation.
- Navigation state hidden in component state when URL state is required for shareable filters, tabs, or pagination.
Use `Link` for normal navigation. Use router APIs for mutation success, guarded redirects, command flows, or form submission side effects.
Use these rules for generated contracts, TanStack Query, mutations, auth/SSR boundaries, URL state, and client persistence.
[Data and queries] owns generated-client, Query options, mutation/cache, imperative access, SSR, authentication, and tenant rules. [State ownership] owns URL state and persistence. Read only the reference needed by the diff; reading a shared reference does not activate its implementation skill.
## Generated Contracts
## Generated Contracts And Query Conventions
Flag:
Review explicit team conventions as contracts, including direct generated options, `skipToken` for missing required input, and shared cache policy. A lint rule may enforce part of a convention; neither a green lint result nor a suppression proves the full contract is satisfied.
-New legacy service/helper wrappers around generated `queryOptions()` or `mutationOptions()`.
-Continuing to use deprecated contract operations when a ready generated contract exists.
-Assuming a generated file means an operation is ready without checking deprecated markers, schema shape, and the actual UI consumer.
-Re-declaring API DTOs in components.
- Adding compatibility layers instead of migrating the pointed line and deleting the old layer.
-Establish whether the changed call belongs to a new or migrated surface and whether the generated operation is ready. Check deprecated markers, schema shape, and the real UI consumer before prescribing a migration.
-Distinguish a pass-through wrapper from a feature hook with actual orchestration. Check whether an independent execution condition or Promise composition justifies the documented query/mutation exception.
-Trace generated input and output types through their boundaries. Identify the exact DTO mirror, field widening, placeholder input, or lost intentional empty value when reporting a violation.
-Check whether a local mutation callback owns feature feedback or replaces shared invalidation, retry, or cache defaults. Match optimistic changes to the current list/detail owner.
Backend Pydantic and OpenAPI schemas own API shape. Generated clients and schemas under `packages/contracts/generated/*` are authoritative at frontend boundaries and use the `{ params, query?, body? }` input shape.
## Imperative Access And SSR
## Queries
- Check freshness, projection, retries, and the caller's execution condition separately against [Data and queries]. Trace Promise ownership to distinguish an awaited hard gate from soft prefetching with an explicit failure owner.
- For Server Components, identify who renders the data and who may revalidate it. Check dehydration, the same-key client consumer, error handling, and the intended Suspense/server-rendered-content contract when the diff changes streaming.
- For auth, setup, roles, branding, or availability, trace authoritative data and the loading/fallback path. A static redirect or placeholder value cannot stand in for a request-dependent decision.
Flag:
## Tenant, URL, And Persistence Boundaries
-`enabled` used to hide missing required input instead of `input: skipToken`.
- Fake fallback IDs or placeholder inputs used to force a query to run.
- Query results copied into local state for rendering.
- Shared query behavior such as invalidation, stale defaults, or retry rules reimplemented at call sites.
-`prefetchQuery` treated as a hard gate or as returning data/errors to the caller.
-Trace the current workspace-switch flow and cache lifetime before reporting missing identity in a query key. Verify backend meaning before treating `workspace_id` and `tenant_id` as interchangeable.
- For URL and storage changes, identify whether the value is shareable navigation state, live app state, a one-shot signal, or a low-frequency preference. Apply [State ownership] to that category and verify its write/reset owner.
Use `useQuery(consoleQuery.xxx.queryOptions(...))` or `useQuery(marketplaceQuery.xxx.queryOptions(...))` directly unless a feature hook performs real orchestration.
Report the violated rule and applicable scope or the concrete failing path. Do not invent runtime impact when the finding is a team-convention violation.
## Mutations
Flag:
- Deprecated `useInvalid` or `useReset`.
-`mutateAsync` used without a need for Promise semantics.
- Awaited mutations without `try/catch`.
- Components owning shared cache invalidation that belongs in query defaults.
- Optimistic updates that do not match current list/detail ownership.
Use generated `mutationOptions()` directly when possible. Put shared cache behavior in `createTanstackQueryUtils(...experimental_defaults...)`.
## SSR, Auth, And Route Boundaries
Flag:
- Request-time auth, setup, workspace role, or tenant decisions moved into static `next.config redirects()`.
- Dynamic role gates depending on `workspaces.current` implemented as static path redirects.
- Authorization logic depending on soft `prefetchQuery`.
- Removing a client fallback before server API unavailable behavior is defined.
- Global placeholder query contracts introduced to solve a route-local Suspense issue.
- Branding-sensitive UI reading placeholder defaults without checking pending/placeholder state.
Separate hard gates from soft prefetches. `fetchQuery` can be a server decision boundary; `prefetchQuery` is cache warmup.
## Workspace And Tenant
Flag:
- Treating workspace switch as ordinary CRUD invalidation when the current app flow performs server switch plus full reload.
- Query keys that omit workspace/tenant identity when the query truly varies by workspace and no full reload boundary applies.
- Mixing `workspace_id` and `tenant_id` without tracing the current backend/API contract.
Current Dify workspace switch should be reviewed as a tenant cache boundary first.
## URL State And Local Storage
Flag:
- Shareable filters, tabs, pagination, selected panels, or search state hidden only in component state.
- One-shot navigation signals modeled as subscribed persistent state.
- Live app state stored in localStorage.
- Direct `window.localStorage`, `globalThis.localStorage`, or raw storage calls in app code.
- High-frequency interaction state persisted on every change instead of on commit/settle.
Use URL state for shareable UI state, feature/Jotai/store state for live UI state, and `@/hooks/use-local-storage` only for low-frequency client-only preferences, dismissed notices, and UI defaults.
[Data and queries]: ../../how-to-write-component/references/data.md
| Forms and field semantics | `packages/dify-ui/docs/forms.md` |
| Selection and typed values | `packages/dify-ui/docs/selection.md` |
| Portals, layers, and floating surfaces | `packages/dify-ui/docs/overlays.md` |
| State styling, callbacks, and Tailwind | `packages/dify-ui/docs/styling.md` |
| Package tests and Storybook | `packages/dify-ui/docs/testing.md` |
## Package Boundary
For consumer code under `web/`, also read `web/AGENTS.md` for application-owned reuse policy and
`packages/dify-ui/README.md` for the available public subpaths.
Flag in `packages/dify-ui`:
- Imports from `web/`.
- Dependencies on Next.js, i18n, ky, Jotai, Zustand, TanStack Query, oRPC, or business APIs.
- Business-specific component behavior that belongs in `web/`.
- Multiple unrelated primitives in one component folder.
`packages/dify-ui` is a primitive layer: Base UI headless components + `cva` + `cn` + Dify design tokens.
## Imports And Exports
Flag:
- Consumer imports from `@langgenius/dify-ui` without a subpath.
- Missing `package.json#exports` entry for a new primitive.
- Internal package imports using workspace subpaths instead of relative paths.
- Exported props using internal-only types that consumers cannot import from the component subpath.
- Canonical primitive boundaries or their associated public types using a redundant `Root` suffix when no higher-level convenience component exists in the same subpath.
Consumers use subpath exports such as `@langgenius/dify-ui/button`.
Canonical boundaries use the primitive name and matching public types (`Select` / `SelectProps`). Keep `Root` only to distinguish a low-level anatomy root from a higher-level convenience component (`CheckboxRoot` / `Checkbox`); implementation aliases should still show their Base UI source (`BaseSelect.Root.Props`).
## Props And State
Flag:
- Flattened props where related values need a discriminated union, such as `value` / `defaultValue`, `multiple` / `value`, or `clearable` / `onChange`.
- React state used only to mirror Base UI state for class names.
- JavaScript conditional class logic for visual states that the Dify UI/Base UI primitive already exposes through `data-*` attributes or CSS variables.
- Controlled props added when uncontrolled DOM state or CSS variables would be enough.
- Thin wrappers that rename Base UI parts without adding semantics.
- Generic Base UI selection primitives wrapped without preserving their value generics, such as `Select.Root<Value, Multiple>`, `RadioGroup<Value>`, or `Radio.Root<Value>`.
- Shared select/radio option components that type selected values as `string` while callers pass enums, unions, booleans, numbers, objects, or nullable placeholder values.
Prefer Base UI/Dify UI data attributes and CSS variables for visual state: `data-open`, `data-checked`, `data-disabled`, `data-highlighted`, `data-popup-open`, `group-data-*`, `peer-data-*`, `has-[:focus-visible]`, and primitive CSS variables such as anchor width or transform origin. Use JS conditional classes for product/business state that the primitive does not expose.
For non-string `Select` and `RadioGroup` values, prefer explicit domain generics at the root and at child value carriers. JSX children do not inherit the parent generic, so `RadioGroup<PromptMode>` should compose with `Radio<PromptMode>`, `RadioItem<PromptMode>`, or option values from a typed collection. For `Select`, prefer the Base UI `items` collection pattern for typed value-to-label rendering, and flag string coercion helpers used only to recover display labels.
## Forms
Flag:
- Form-like UI using unrelated `Input` and `Button` pieces without a submit boundary.
- Text-like fields not composed through `Field`, `FieldLabel`, and `FieldControl` when using Dify UI form semantics.
- Select fields using `FieldLabel` instead of `SelectLabel`.
- Slider fields using a generic label instead of `SliderLabel`.
- Checkbox/radio groups missing `Fieldset` and `FieldsetLegend`.
- Field errors or descriptions rendered without `FieldDescription` / `FieldError` relationships.
`Form` is the submit boundary. Dify UI form primitives are not a form state-management framework; business validation and schema-driven behavior belong in `web/`.
## Overlay Contract
Flag:
- Legacy web overlay imports in new or modified code.
- Manual portals around Dify UI overlay primitives.
- Call-site `z-*` overrides on overlays.
- Missing root `isolation: isolate` assumptions when debugging overlay stacking.
- Repeated backdrop, z-index, or portal chrome at call sites.
- Tooltip used for infotips, long text, or interactive content.
All Dify UI body-portalled overlays use `z-50`. Toast uses `z-60`. DOM order handles stacking between overlays.
## Primitive Selection
Flag:
-`Tabs` used for simple mode/filter/view selection where `SegmentedControl` is the semantic primitive.
-`SegmentedControl` used where `tablist` / `tabpanel` semantics are required.
-`Select` used for searchable or free-form input.
-`Combobox` used for unrestricted search text where no selected option is remembered.
-`Autocomplete` used for closed-list selection.
- Tooltip or PreviewCard used for content that must be reachable on touch or by screen readers.
Use:
-`Autocomplete` for free-form text with optional suggestions.
-`Combobox` for searchable selected values from a collection.
-`Select` for closed, scannable option sets.
-`Popover` for infotips, help text, rich content, or interactions.
## Bad Usage Patterns To Flag
Flag:
- Manually recreating UI behavior or chrome already owned by `@langgenius/dify-ui/*` or `web/app/components/base/*`, such as buttons, inputs, toggle groups, popovers, dropdown menus, alert dialogs, switches, avatars, scroll areas, toasts, borders, focus states, disabled states, segmented controls, or existing feature components.
- Styling a raw Base UI primitive directly in `web/` when a Dify UI primitive exists.
- Wrapping a Dify UI primitive in a feature component that hides its label, error, disabled, or focus contract.
- Replacing a semantic primitive with a generic `div` plus classes to match a screenshot.
- Using `Tooltip` because it is visually convenient when the content is actually help text or needs touch access.
- Adding a `z-*` override to make a child popup appear over a parent dialog.
- Adding a new app-level wrapper around Dialog, Drawer, Popover, Select, or Combobox that repeats portal/backdrop/positioner logic.
- Using dify-ui `Input` as a drop-in replacement for legacy inputs that include search, clear, copy, unit, localized placeholder, or number normalization behavior.
- Building a form row from loose text and controls instead of the matching Field/Form primitives.
- Adding component state only to style `data-open`, `data-checked`, `data-disabled`, or highlighted states that Base UI already exposes.
- Passing booleans down only so children can toggle classes already expressible with primitive `data-*` selectors.
## Tokens, Radius, And Styling
Flag:
-`radius-*` class names.
- Custom Tailwind `borderRadius` extension for Figma radius values.
- Generic colors where semantic Dify tokens exist.
- Hardcoded design values where Dify tokens, component variants, or documented Figma radius mappings exist.
-`!` important modifiers used to fight primitive styles instead of fixing the variant, selector, or component composition.
- Manual class strings that duplicate primitive variants.
-`min-w-(--anchor-width)` on picker popups when it defeats viewport clamping.
Use the Figma radius mapping from `packages/dify-ui/README.md`; for example `--radius/sm` maps to `rounded-md`, and `--radius/md` maps to `rounded-lg`.
Use `!` only for a tightly scoped compatibility override after confirming the primitive API, data attributes, and selector structure cannot express the state.
## Focus Details
Flag focus rings attached to the wrong element. For example, Base UI `Slider.Thumb` focuses an internal `input[type=range]`, so the visible thumb wrapper needs `has-[:focus-visible]` rather than direct wrapper `focus-visible`.
## Custom SVG Icons
Flag:
- New generated React icon components or JSON files under `web/app/components/base/icons/src/...` for custom SVG icons.
- Custom SVG icons consumed outside the Tailwind `i-custom-*` icon class pipeline.
- Generated `packages/iconify-collections/custom-*/icons.json` diffs where unrelated existing icons lost or changed intrinsic `width` or `height`.
New custom SVG icons belong in `packages/iconify-collections/assets/...`. Regenerate with `pnpm --filter @dify/iconify-collections generate`, validate with `pnpm --filter @dify/iconify-collections check:dimensions`, and consume the generated icon with Tailwind `i-custom-*` classes.
Treat the implementation, public types, tests, and stories as evidence for the documented
contract. If they disagree, identify the actual owner before reporting a finding. Read current
official Base UI documentation and installed type declarations for upstream-derived behavior.
Report only a reproducible contract violation or observable defect, not a preference inferred from
For workflow React Flow components, keep this Dify-specific rule:
Use [Dify invariants] for React Flow node/edge consumption and provider availability in RAG Pipe template rendering. Callback-only reads or mutations can use `useStoreApi`.
- UI consumption should use React Flow hooks such as `useNodes` / `useEdges`.
- Callback-only reads or mutations can use `useStoreApi`.
- Node components under `web/app/components/workflow/nodes/[nodeName]/node.tsx` must not depend on workflow stores that are absent in RAG Pipe template rendering.
`web/docs/test.md`is the canonical frontend testing policy. Use this file only to translate that policy into review findings.
Use `web/docs/test.md`for tests owned by `web/` and `packages/dify-ui/docs/testing.md` for tests owned by Dify UI. These owners define their test boundaries, environments, and checks; this reference only adds review questions.
## Request Missing Tests When Risk Justifies Them
@@ -9,10 +9,11 @@ Flag missing coverage when a change alters a reachable contract such as:
- User interaction, navigation, form submission, validation, or permissions.
- Query or mutation behavior, URL state, persistence, or one-shot signals.
- Loading, error, empty, and recovery states that users can encounter.
- A hidden surface whose close-and-reopen behavior changes whether in-progress state resets or persists.
- A regression-prone business rule or bug fix that can be reproduced through a public boundary.
Do not request tests for mechanical changes, pass-through wrappers, implementation details, or visual-only styling unless they affect behavior. Low coverage alone is not a finding.
Do not request tests for mechanical changes, pass-through wrappers, implementation details, or incidental visual styling. Low coverage alone is not a finding.
## Flag Low-Value or Fragile Tests
@@ -29,8 +30,8 @@ Flag tests that:
- Prefer semantic queries and accessible names.
- Prefer real feature components when integration semantics matter.
-Allow intentional child or provider mocks when setup would dominate the test and that boundary is covered independently.
-Do not accept semantically inaccurate mocks of Dify UI or legacy base primitives.
-Require a real-browser or visual verification plan when `happy-dom` cannot represent the risk.
-Check mocks against the owning package's policy; allowed mocks must preserve the public contract and leave the behavior under review real.
-Apply the owning package's admission policy before requesting new automation. Real-browser verification can be a one-off review; a simulator limitation alone does not justify a permanent test.
-Check that assertions prove the claimed outcome and add evidence beyond existing tests; a browser runtime does not make weak assertions stronger.
Treat test quality, determinism, and regression value as the review criteria. Do not use test count or coverage percentage as a proxy for quality.
@@ -5,12 +5,9 @@ description: Use when writing or changing Vitest or React Testing Library tests
# Frontend Testing
`web/docs/test.md` is the single policy owner. Read it before changing frontend tests; this skill adds no separate requirements.
Read the testing policy for the package that owns the changed tests:
1. Identify the observable contract and regression risk.
2. Choose the smallest boundary that includes the behavior owner.
3. Establish the failing case first when practical, then implement one coherent scenario.
4. Run the focused spec before the affected suite and relevant static checks.
5. Report the behavior verified and any remaining browser, visual, or end-to-end risk.
-`web/`: `web/docs/test.md` owns Web test boundaries, environments, and commands.
-`packages/dify-ui/`: `packages/dify-ui/docs/testing.md` owns primitive test boundaries, environments, Storybook, and commands.
Recommend deleting low-value tests as readily as adding missing behavior coverage. Use `web/docs/test.md` for policy and Web commands; use the `packages/dify-ui/README.md` Development section for Dify UI commands.
This skill adds no parallel policy or check sequence. Follow the selected owner's requirements, including its validation commands. Recommend deleting low-value tests as readily as adding missing behavior coverage, and report the contract verified and any material verification gap.
- Dify UI wrappers or primitive state styling: read the package's [Public API authoring] and [Styling] contracts.
- Jotai, form drafts, route identity, URL state, or persistence: read [`references/state.md`][state].
- Generated contracts, nullable API data, Query, mutations, SSR, auth, or workspace state: read [`references/data.md`][data].
- Hotkeys, focus, dialogs, menus, popovers, or other secondary surfaces: read [`references/interactions.md`][interactions] and the overlay guide it references when applicable.
- Hotkeys, focus, dialogs, menus, popovers, or other secondary surfaces: read [`references/interactions.md`][interactions] and the overlay guide it references when applicable. Also read [`references/state.md`][state] when the surface owns a draft or other local session state.
- Effects, navigation, memoization, preloading, or render cost: read [`references/runtime.md`][runtime].
## Workflow
## Scope And Verification
1.Identify the behavior owner and the public contract being changed.
2. Read the nearby implementation, tests, and only the routed skill references.
3. Implement one coherent vertical slice. Do not expand into equivalent patterns elsewhere unless the current contract cannot be completed without them.
4. Verify observable behavior at the narrowest sufficient boundary, then run the checks documented by the owning package: `web/docs/test.md` or `web/docs/lint.md` for Web, and the `packages/dify-ui/README.md` Development section for Dify UI.
Identify the behavior owner, state lifetime, and public contract from the nearby implementation and relevant references. Keep the change within that vertical slice unless the contract requires changes elsewhere. Verify the changed behavior and complete the owning package's required checks. For Web, read `web/docs/test.md` for test work and `web/docs/lint.md` for static checks. Dify UI verification is owned by `packages/dify-ui/docs/testing.md`.
- Common forms include `w-105` instead of `w-[420px]`, `px-2.25` instead of `px-[9px]`, `bg-linear-to-b` instead of `bg-gradient-to-b`, `wrap-break-word` instead of `break-words`, and `field-sizing-content` instead of `[field-sizing:content]`.
[Public API authoring]: ../../../packages/dify-ui/docs/authoring.md
@@ -8,9 +8,10 @@ Read this document when a component consumes generated contracts, nullable API v
- Backend Pydantic and OpenAPI schemas own API shape. Follow the generated `{ params, query?, body? }` input shape; when it is wrong, fix the backend schema and regenerate `packages/contracts/generated/*`.
- Do not hand-write DTO mirrors, widen generated fields or enums, edit generated output, or add a parallel frontend status layer unless it models product state absent from the API.
- Check deprecated markers, schema shape, and the actual consumer before assuming that a generated operation is ready to use.
- When a ready generated operation exists for the changed call, migrate deprecated operations and remove the replaced layer instead of adding compatibility wrappers.
- Normalize only at real boundaries such as user input, search, URL params, filenames, DOM IDs, or a required legacy adapter.
- Preserve `null`, `undefined`, and intentional empty strings until the final boundary. Do not use `value || undefined` when an empty string means clearing a field.
- Build required values in the branch that proves them. Avoid `filter(Boolean)`, truthiness filters, non-null assertions after filters, and placeholder values used only to satisfy types.
- Build required values in the branch that proves them. Do not use truthiness filters, non-null assertions, or placeholders to conceal missing required input or discard valid `0`, `false`, or empty-string values.
## Queries
@@ -18,25 +19,40 @@ Read this document when a component consumes generated contracts, nullable API v
- If query input comes from atom state, keep it in `atomWithQuery`; do not unwrap the atom in a component solely to call `useQuery`.
- For missing required input, branch the whole generated input with `skipToken`. Add `enabled` only for an independent execution condition; do not put `skipToken` inside a placeholder payload or coerce IDs to empty strings.
- Return generated `queryOptions()`, `infiniteOptions()`, or `mutationOptions()` directly from TanStack Query atoms. Pass supported options into the generated call instead of spreading into a parallel object.
-Share the exact options between prefetch and render when they represent the same request. Do not extract option helpers merely to reuse input construction.
-For the same logical request, preserve key, input, operation, and result contracts. Reuse exact options only when
transport, context, and cache policy are shared; imperative and observer freshness may differ. Do not extract a helper
merely to reuse input construction.
- Avoid pass-through service hooks that only rename generated options. Keep feature hooks for actual orchestration or shared domain behavior.
## Mutations And Cache
- Use generated `mutationOptions()` directly for owner-local mutations.
-Put shared invalidation, retries, and cache behavior in `createTanstackQueryUtils(...experimental_defaults...)`. Local callbacks may own toast, close, and navigation effects but must not replace shared cache policy.
-Do not introduce deprecated `useInvalid` or `useReset` APIs.
- Put shared invalidation, retries, and cache behavior in `createTanstackQueryUtils(...experimental_defaults...)`. In oRPC v1, caller options override defaults, including callbacks. Local callbacks may own toast, close, and navigation effects but must preserve shared cache behavior.
- Prefer `mutate(...)`. Use `mutateAsync(...)` only when Promise composition is required, and catch awaited failures.
- Preserve intentional empty values and current list/detail ownership when updating data. Do not add optimistic updates without a verified owner contract.
## Prefetch And Hidden Surfaces
- Prefetch expensive secondary content from the trigger or menu-open event when it benefits the visible path. Do not mount hidden subscribers solely to warm the cache.
-`prefetchQuery` is cache warmup, not an authorization or availability gate. Use a hard fetch boundary when the server must decide whether rendering may proceed.
-Use `query` or `infiniteQuery` for imperative access. `staleTime` defines cache acceptance; `select` projects the return
value without replacing cached query-function data. Imperative queries default to no retries when `retry` is not
configured, and observer-only `enabled` does not prevent an imperative call.
- Migrate deprecated `fetchQuery`, `prefetchQuery`, `ensureQueryData`, and their infinite variants when the current `query` or `infiniteQuery` contract applies. A `staleTime` of `0` treats data as stale; a finite value accepts that freshness window; `Infinity` accepts data until invalidation; `'static'` accepts available data even after invalidation.
-`await` blocks, `return` transfers the Promise, and `void` discards its value without handling rejection. Handle
rejection before discarding a potentially rejecting Promise; use `.catch(noop)` only for intentional silence or
feedback owned elsewhere. Hard server gates await the query and preserve rejection.
## SSR, Authentication, And Workspace
- Static configuration owns path-invariant routing. Request-dependent authentication, setup, role, and tenant decisions belong to SSR or runtime decision boundaries.
- Distinguish soft SSR cache warming from authoritative decisions. Prefetched or placeholder data must not grant access or represent successful availability.
- Treat Server Components as query prefetch-and-dehydrate owners by default. Do not render or pass an imperative query
result when a browser observer can revalidate the same data unless ownership and freshness explicitly prevent drift.
- Non-blocking RSC streaming requires pending-query dehydration without redacting Next.js server errors, a
`HydrationBoundary` around the same-key client consumer, and Suspense when that content must be server-rendered.
- Never reuse tenant-scoped state after switching workspaces. Discard it at the switch boundary or isolate it by workspace identity.
- Trace the current switch flow before choosing cache handling: server switch plus full reload is a tenant boundary, not ordinary CRUD invalidation. Include workspace identity in varying query keys when no full-reload boundary applies, and trace the backend contract before interchanging `workspace_id` and `tenant_id`.
- Do not make product or authorization decisions from bootstrap defaults. Wait for authoritative data, or render an explicit loading or error state.
- Preserve an existing client fallback until server API-unavailable behavior has an explicit owner.
- Keep loading and Suspense behavior inside the feature that owns the request. Do not add fake global data merely to bypass that boundary.
Read this document when a change involves application hotkeys, focus, dialogs, menus, popovers, or other secondary surfaces. Overlay primitive selection and layering are owned by the [overlay guide].
Read this document when a change involves application hotkeys, focus, dialogs, menus, popovers, or other secondary surfaces. Overlay primitive selection and layering are owned by the [overlay contract].
## Focus And Semantics
@@ -10,22 +10,18 @@ Read this document when a change involves application hotkeys, focus, dialogs, m
## Keyboard Commands
-Distinguish application commands from widget-local keyboard semantics. Use `@tanstack/react-hotkeys` for application commands; keep menu navigation, dialog Escape handling, editor behavior, and ARIA widget keys in their local primitive or owner.
-Use `useHotkey` or `useHotkeys` for registered commands. When an existing `onKeyDown` intentionally owns the command, use `matchesKeyboardEvent` rather than duplicating modifier parsing or adding another global listener.
- Keep registration and keycap or menu display derived from one canonical command. Distinguish registered commands, held keys, and display-only accelerators.
- Keep a single-owner command beside its component. Create a feature-local hotkey module only when several production files share it; tests alone do not justify extraction.
- Make availability and scope explicit with `enabled`, `ignoreInputs`, and `target`. Put a target ref on the actual behavior owner rather than creating wrapper DOM solely for hotkey scope.
- Preserve existing `preventDefault` and propagation behavior when migrating command APIs.
- Test observable command behavior, disabled and input scope, target scope, and the registration/display contract at the owning feature boundary.
-Keep widget navigation, dialog dismissal, and editor commands in their local primitive or owner.
-For Web application commands, follow [Keyboard commands] for TanStack registration, typed bindings, targets, event consumption, display, and verification. Do not recreate that policy in a wrapper or a second command framework.
## Secondary Surfaces
- Follow `web/docs/overlay.md` for primitive choice. Dify UI primitives are the default, with package-approved Web wrappers such as `Infotip` where the overlay guide allows them.
- Follow the [overlay contract] for primitive choice and shared mechanics. The nearest consumer `AGENTS.md` owns application-specific composite reuse policy.
- Separate behavior ownership from placement ownership: the action may own trigger, open state, and menu content while the caller owns slots, offsets, and alignment.
- Keep menu and dialog surfaces as siblings when a menu command opens a dialog. Mount the dialog outside popup content.
-Mount controlled overlays unconditionally unless unmounting is required for performance or reset semantics. Prefer keyed or owner-local reset over conditional wrappers.
- Put query and mutation work inside dialog or alert-dialog content when it should mount only after opening.
- Prefer uncontrolled roots when the primitive can own open state. Use controlled state only for business coordination, analytics, cleanup, or explicit reset behavior.
- Do not add manual portals or call-site z-index escalation. Fix ownership and stacking structure at the shared boundary.
-Use the [overlay contract] to determine content lifetime and preserve the Root's closing lifecycle. Follow [state ownership] for draft placement and semantic identity; portal placement alone does not locate the state owner.
- Place query subscriptions and mutation observers at the owner whose lifetime matches when they should run. Mounted-session work may belong inside content; work that must start or stop exactly with `open` needs an explicit open-state condition.
- Prefer primitive-owned open state unless another owner must observe or coordinate it. Analytics callbacks and local cleanup alone do not require a controlled root.
@@ -12,28 +12,34 @@ Read this document when adding, moving, splitting, or refactoring React componen
## Component Ownership
- Put state, data access, loading, empty, error, and handlers in the lowest visual owner that uses them.
- Keep coordination in a parent only when it needs one consistent snapshot or coordinates submission, shared selection, batch behavior, navigation, or cross-section loading and errors.
- Repeated TanStack Query calls in siblings are acceptable when each sibling independently consumes the data; the cache already deduplicates requests.
- Put state, data access, loading, empty, error, and handlers in the lowest owner that uses them and whose mounted lifetime matches the required persistence.
- Keep coordination in a parent only when it needs one consistent snapshot, the value must intentionally survive the local owner's unmount, or the parent coordinates submission, shared selection, batch behavior, navigation, or cross-section loading and errors.
- Repeated TanStack Query hooks for the same key and input in Client Component siblings under one QueryClient are
acceptable; shared cache is not a reason to hoist. Separate Server Component QueryClients do not share it, so
request-level deduplication needs an identified request-local cache or verified framework or transport owner.
- Pass stable domain identity across boundaries. Do not pass raw server data together with separately derived flags for the same concept.
-One pass-through prop layer is acceptable. Repeated forwarding means ownership should move closer to the consumer or into feature-scoped shared state.
-Revisit prop forwarding when intermediate components obscure the behavior owner; keep clear data flow rather than introducing shared state merely to avoid passing props.
- Do not replace prop drilling with one large view-model hook. Move each query, derived value, and handler to the concrete owner that consumes it.
- Keep source selection, defaults, validation, dirty checks, and payload shaping beside the workflow that owns submission.
## Boundaries
- Prefer reusing or extending the component that already owns an interaction over rebuilding that behavior in a parallel component.
- State-heavy wizards, drawers, modals, and secondary workflows can form a small vertical surface with an entrypoint, optional feature-local state, and shallow owners matching real visual regions.
- The entrypoint owns route integration, provider wiring, close behavior, and mounting. Composition owners handle workflow branches; the closest visual owner handles section branches.
- The entrypoint owns route integration, provider wiring, placement, and open-state coordination. A content or session owner keeps state scoped to that mounted surface.
- Judge hook lifetime by the component that declares the hook and the primitive's mount contract, not only by where its rendered controls appear in JSX.
- Separate hidden dialogs, dropdowns, and popovers into small local owners when their content obscures the parent flow.
- Keep cohesive forms, menu bodies, and one-off helpers local unless they have their own state, reuse, or semantic boundary.
- Extract components or hooks when they clarify ownership or hide a cohesive implementation; keep logic local when extraction only shortens the file or relocates the same coordination.
- Avoid wrapper components and wrapper DOM that only rename props, pass children through, or hide the real primitive. A wrapper must own behavior, validation, state, accessibility, layout, or library integration.
- Keep feature workflows out of shared components: do not encode one feature through many boolean props or import its copy, routes, and API contracts into a generic component. Do not pass pre-rendered fragments merely to avoid assigning the behavior to its owner.
- Loading states for page sections, cards, lists, tables, forms, and drawers should use skeletons scoped to the loaded content. Reserve spinners for small inline busy indicators.
## Components And Types
- Choose component declaration and export forms from the actual component contract, framework requirements, and enforced package rules. Existing style is context, not authority; do not rewrite unaffected code solely to normalize `FC`, `function`, arrow-function, named-export, or default-export forms.
- Type simple one-off props inline. Name a `Props` type when it is reused, exported, complex, or materially clearer.
- Use API-generated or API-returned types at component boundaries. Keep one-off UI refinements and conversions beside their owner.
- Name props and converted data after their domain/API role, preserving traceability to the original contract.
- Preserve domain value types for selections. Do not widen enums, unions, booleans, numbers, objects, or nullable values to `string` before a real boundary requires it.
- Avoid generic `common.tsx` buckets and aliases that only rename another type. Name files, values, and public types after their domain role.
- Put fallback and invariant checks in the lowest component that already renders that state. Do not extract helpers whose only purpose is hiding missing display data.
@@ -4,9 +4,10 @@ Read this document when a change introduces Effects, navigation side effects, me
## Effects
- Keep render pure: do not read or write `ref.current` during render except for predictable null-guarded lazy initialization. Update interaction-owned refs in event handlers, synchronize external-system refs after commit, and use state or derivation for rendered values.
- Use Effects only to synchronize with a named external system such as a browser API, subscription, timer, analytics integration, non-React widget, or imperative DOM API.
- Do not use Effects to transform render state, handle user actions, copy query data, reset state from props, or fetch data owned by framework APIs or TanStack Query.
- Initialize query-backed forms with keyed remounts or surface-entry hydration instead of copying data through Effects.
- Initialize query-backed form sessions after their defaults are available instead of copying data through Effects. Use a stable semantic identity key when the represented identity changes; use the intended surface lifecycle for per-session reset.
@@ -9,18 +9,21 @@ Read this document when a change involves Jotai, form drafts, route identity, sh
- Keep server and cache state in TanStack Query. Use existing feature stores for complex, high-frequency interaction state such as workflow canvas drag, resize, and runtime panels.
- Use feature-owned storage only for low-frequency client preferences, dismissed notices, and UI defaults. Live application state does not belong in local storage.
## Forms
## Forms And Sessions
-Prefer uncontrolled Dify UI form and field controls when values are only read at submit time. Initialize query-backed defaults with `defaultValue` and keyed remounts.
-Promote form values to atoms only when another owner reacts to in-progress values, the draft must survive scoped unmounting, or several workflow steps edit the same draft.
-Follow the [form contract] for field controlledness and the [overlay contract] for Root/content mounting and state lifetime. These primitive contracts also apply when reviewing a consumer.
-Choose the Web draft owner from the required lifetime. Keep mounted-session drafts in the content owner; start with the lowest shared React owner when another component needs the draft or it must survive unmounting. Use feature-scoped atoms only when their coordination or persistence contract is needed.
- For query-backed defaults, establish the form session after the required defaults are available. `defaultValue` initializes the current mount; a stable semantic identity key may create a fresh snapshot when the represented identity changes. Do not use a generated key as a routine reset command.
- Keep validation, source priority, fallback behavior, dirty checks, and payload assembly in the workflow that owns submission.
- Derive booleans, disabled flags, default tabs, and loading labels from current state. Do not mirror one value into competing prop, default, and local sources; controlled state alone is not a competing-source defect.
- Do not use local state to fake server data or generated contract fields, or connect a feature mock shell to an unrelated API before its actual contract is confirmed.
## Route And URL State
- Treat `useParams`, route arguments, and `nuqs` as the owners of URL identity and updates.
- Hydrate a primitive atom at the route or surface boundary only when query atoms or shared derived atoms require route identity. Keep URL writes in route and query-state APIs.
- Within one route-owned feature, choose one route-identity source. Do not hydrate route identity into atoms while also threading the same ID through multiple component layers.
- Put shareable filters, tabs, pagination, and search state in the URL. Keep one-shot navigation signals and transient UI state out of persistent subscriptions.
- Put shareable filters, tabs, selected panels, pagination, and search state in the URL. Keep one-shot navigation signals and transient UI state out of persistent subscriptions.
## Jotai And Query
@@ -36,3 +39,6 @@ Read this document when a change involves Jotai, form drafts, route identity, sh
- Use feature-owned storage modules built on `createLocalStorageState`; callers should not scatter direct storage access or raw keys.
- Persist high-frequency interaction state only on commit or after updates settle.
- Do not add ad hoc global event listeners for shared state. Centralize subscriptions through the owning atom, store, or subscription hook.
coverage_version=$(python3 -c 'import tomllib; print(next(p["version"] for p in tomllib.load(open("api/uv.lock", "rb"))["package"] if p["name"] == "coverage"))')
description:'Specific files to translate (space-separated, e.g., "app common"). Required for full mode; leave empty in incremental mode to use en-US files changed since HEAD~1.'
description:'Specific files to translate (space-separated, e.g., "app common").
Required for full mode; leave empty in incremental mode to use en-US
files changed since HEAD~1.'
required:false
type:string
languages:
description:'Specific languages to translate (space-separated, e.g., "zh-Hans ja-JP"). Leave empty for all supported target languages except en-US.'
description:'Specific languages to translate (space-separated, e.g., "zh-Hans
ja-JP"). Leave empty for all supported target languages except en-US.'
required:false
type:string
mode:
description: 'Sync mode:incremental (compare with previous en-US revision) or full (sync all keys in scope)'
description: 'Sync mode:incremental (compare with previous en-US revision) or
You are the i18n sync agent for the Dify repository.
Your job is to keep translations synchronized with the English source files under `${{ github.workspace }}/web/i18n/en-US/`.
Your job is to keep translations synchronized with the English source files under `${{ github.workspace }}/web/i18n/locales/en-US/`.
Use absolute paths at all times:
- Repo root: `${{ github.workspace }}`
- Web directory: `${{ github.workspace }}/web`
- Language config: `${{ github.workspace }}/web/i18n-config/languages.ts`
- Language config: `${{ github.workspace }}/web/i18n/languages.ts`
Inputs:
- Files in scope: `${{ steps.context.outputs.CHANGED_FILES }}`
@@ -194,12 +199,12 @@ jobs:
Required execution plan:
1. Resolve target languages.
- Use the provided `Target languages` value as the source of truth.
- If it is unexpectedly empty, read `${{ github.workspace }}/web/i18n-config/languages.ts` and use every language with `supported: true` except `en-US`.
- If it is unexpectedly empty, read `${{ github.workspace }}/web/i18n/languages.ts` and use every language with `supported: true` except `en-US`.
2. Stay strictly in scope.
- Only process the files listed in `Files in scope`.
- Only process the resolved target languages, never `en-US`.
- Do not touch unrelated i18n files.
- Do not modify `${{ github.workspace }}/web/i18n/en-US/`.
- Do not modify `${{ github.workspace }}/web/i18n/locales/en-US/`.
3. Resolve source changes.
- If `Structured change set available` is `true`, read `/tmp/i18n-changes.json` and use it as the source of truth for file-level and key-level changes.
- For each file entry:
@@ -223,7 +228,7 @@ jobs:
- Match the existing terminology and register used by each locale.
- Prefer one Edit per file when stable, but prioritize correctness over batching.
6. Verify only the edited files.
- Run `vp fmt <repo-relative edited paths such as web/i18n/...>`
- Run `vp fmt <repo-relative edited paths such as web/i18n/locales/...>`
- Run `vp run dify-web#i18n:check ${{ steps.context.outputs.FILE_ARGS }} ${{ steps.context.outputs.LANG_ARGS }}`
- If verification fails, fix the remaining problems before continuing.
7. Stop after the scoped locale files are updated and verification passes.
@@ -237,7 +242,7 @@ jobs:
if:steps.context.outputs.CHANGED_FILES != ''
shell:bash
run:|
if [ -z "$(git -C "${{ github.workspace }}" status --porcelain -- web/i18n/)" ]; then
if [ -z "$(git -C "${{ github.workspace }}" status --porcelain -- web/i18n/locales/)" ]; then
Dify is an open-source platform for building LLM applications, agentic workflows, and RAG pipelines. This monorepo contains the backend API (`api/`), frontend application (`web/`), deployment assets (`docker/`), standalone agent backend (`dify-agent/`), CLI (`cli/`), and end-to-end suite (`e2e/`). Follow the nearest scoped `AGENTS.md` for the files being changed.
Dify is an open-source platform for building LLM applications, agentic workflows, and RAG pipelines. This monorepo contains the backend API (`api/`), frontend application (`web/`), deployment assets (`docker/`), standalone agent backend (`dify-agent/`), CLI (`cli/`), and end-to-end suite (`e2e/`). Follow the nearest scoped `AGENTS.md` for the files being changed. Apply its guidance within the user's requested scope; explicit user instructions take precedence over workflow defaults.
| Bugs in core functions (cloud service, cannot login, applications not working, security loopholes) | Critical |
| Bugs in core functions (cloud service, cannot login, applications not working) | Critical |
| Non-critical bugs, performance boosts | Medium Priority |
| Minor fixes (typos, confusing but working UI) | Low Priority |
@@ -71,6 +73,10 @@ How we prioritize:
1. Please link the issue in the PR description, `fixes #<issue_number>`
1. Get merged!
### AI-assisted contributions
AI tools are welcome, but you are responsible for understanding and verifying every change you submit. Describe the problem, your solution, and actual test results in your own words. Unreviewed or repetitive low-quality submissions may be closed.
<a href="./docs/bn-BD/README.md"><img alt="README in বাংলা" src="https://img.shields.io/badge/বাংলা-d9d9d9"></a>
<a href="./docs/hi-IN/README.md"><img alt="README in हिन्दी" src="https://img.shields.io/badge/Hindi-d9d9d9"></a>
<a href="./docs/id-ID/README.md"><img alt="README dalam Bahasa Indonesia" src="https://img.shields.io/badge/Bahasa%20Indonesia-d9d9d9"></a>
</p>
Dify is an open-source LLM app development platform. Its intuitive interface combines AI workflow, RAG pipeline, agent capabilities, model management, observability features (including [Opik](https://www.comet.com/docs/opik/integrations/dify), [Langfuse](https://docs.langfuse.com), and [Arize Phoenix](https://docs.arize.com/phoenix)) and more, letting you quickly go from prototype to production. Here's a list of the core features:
@@ -104,8 +105,7 @@ Intuitive interface for crafting prompts, comparing model performance, and addin
**4. RAG Pipeline**:
Extensive RAG capabilities that cover everything from document ingestion to retrieval, with out-of-box support for text extraction from PDFs, PPTs, and other common document formats.
**5. Agent capabilities**:
You can define agents based on LLM Function Calling or ReAct, and add pre-built or custom tools for the agent. Dify provides 50+ built-in tools for AI agents, such as Google Search, DALL·E, Stable Diffusion and WolframAlpha.
**5. Agent**: Autonomous agents with a sandbox of their own: they run commands, install software, and handle files to get open-ended tasks done. Describe the agent you want and it builds itself. Give it skills, connect tools from [Dify Marketplace](https://marketplace.dify.ai/?category=tool), MCP servers, or your own APIs, and put it to work as a chat app or as a step in any workflow.
**6. LLMOps**:
Monitor and analyze application logs and performance over time. You could continuously improve prompts, datasets, and models based on production data and annotations.
@@ -122,90 +122,42 @@ All of Dify's offerings come with corresponding APIs, so you could effortlessly
Quickly get Dify running in your environment with this [starter guide](#quick-start).
Use our [documentation](https://docs.dify.ai) for further references and more in-depth instructions.
- **Dify for enterprise / organizations<br/>**
We provide additional enterprise-centric features. [Send us an email](mailto:business@dify.ai?subject=%5BGitHub%5DBusiness%20License%20Inquiry) to discuss your enterprise needs. <br/>
For organizations requiring self-hosting, SSO, RBAC Security and Enterprise Support SLAs, please fill out [this form](https://share-na2.hsforms.com/14-09ff5HS92Sh4m3f4yrcw40s9fk) to speak to a solution representative. <br/>
## Staying ahead
Star Dify on GitHub and be instantly notified of new releases.
If you need to customize the configuration, edit `docker/.env`. The essential startup defaults live in [`docker/.env.example`](docker/.env.example), and optional advanced variables are split under `docker/envs/` by theme. After making any changes, re-run `docker compose up -d` from the `docker` directory. You can find the full list of available environment variables [here](https://docs.dify.ai/getting-started/install-self-hosted/environments).
### Metrics Monitoring with Grafana
Import the dashboard to Grafana, using Dify's PostgreSQL database as data source, to monitor metrics in granularity of apps, tenants, messages, and more.
- [Grafana Dashboard by @bowenliang123](https://github.com/bowenliang123/dify-grafana-dashboard)
### Deployment with Kubernetes
If you'd like to configure a highly available setup, there are community-contributed [Helm Charts](https://helm.sh/) and YAML files which allow Dify to be deployed on Kubernetes.
- [Helm Chart by @LeoQuote](https://github.com/douban/charts/tree/master/charts/dify)
- [Helm Chart by @BorisPolonsky](https://github.com/BorisPolonsky/dify-helm)
- [Helm Chart by @magicsong](https://github.com/magicsong/ai-charts)
- [YAML file by @Winson-030](https://github.com/Winson-030/dify-kubernetes)
- [YAML file by @wyy-holding](https://github.com/wyy-holding/dify-k8s)
- [🚀 NEW! YAML files (Supports Dify v1.6.0) by @Zhoneym](https://github.com/Zhoneym/DifyAI-Kubernetes)
#### Using Terraform for Deployment
Deploy Dify to Cloud Platform with a single click using [terraform](https://www.terraform.io/)
##### Azure Global
- [Azure Terraform by @nikawang](https://github.com/nikawang/dify-azure-terraform)
##### Google Cloud
- [Google Cloud Terraform by @sotazum](https://github.com/DeNA/dify-google-cloud-terraform)
#### Using AWS CDK for Deployment
Deploy Dify to AWS with [CDK](https://aws.amazon.com/cdk/)
##### AWS
- [AWS CDK by @KevinZhao (EKS based)](https://github.com/aws-samples/solution-for-deploying-dify-on-aws)
- [AWS CDK by @tmokmss (ECS based)](https://github.com/aws-samples/dify-self-hosted-on-aws)
#### Using Alibaba Cloud Computing Nest
Quickly deploy Dify to Alibaba cloud with [Alibaba Cloud Computing Nest](https://computenest.console.aliyun.com/service/instance/create/default?type=user&ServiceName=Dify%E7%A4%BE%E5%8C%BA%E7%89%88)
#### Using Alibaba Cloud Data Management
One-Click deploy Dify to Alibaba Cloud with [Alibaba Cloud Data Management](https://www.alibabacloud.com/help/en/dms/dify-in-invitational-preview/)
#### Deploy to AKS with Azure Devops Pipeline
One-Click deploy Dify to AKS with [Azure Devops Pipeline Helm Chart by @LeoZhang](https://github.com/Ruiruiz30/Dify-helm-chart-AKS)
For custom configuration, observability, and deployment options, see [Advanced Setup](docs/ADVANCED_SETUP.md).
## Contributing
For those who'd like to contribute code, see our [Contribution Guide](https://github.com/langgenius/dify/blob/main/CONTRIBUTING.md).
At the same time, please consider supporting Dify by sharing it on social media and at events and conferences.
Dify welcomes contributions of all kinds:
> We are looking for contributors to help translate Dify into languages other than Mandarin or English. If you are interested in helping, please see the [i18n README](https://github.com/langgenius/dify/blob/main/web/i18n-config/README.md) for more information, and leave us a comment in the `global-users` channel of our [Discord Community Server](https://discord.gg/8Tpq4AcN9c).
- **Code**: Read the [Contribution Guide](CONTRIBUTING.md), then browse [good first issues](https://github.com/langgenius/dify/issues?q=is%3Aissue%20state%3Aopen%20label%3A%22good%20first%20issue%22).
- **Ideas and feedback**: Start or join a [GitHub Discussion](https://github.com/langgenius/dify/discussions).
- **Translations**: Follow the [internationalization guide](web/i18n/README.md) to add or update a locale.
- **Community**: Share the apps you build, help other users, and spread the word about Dify.
- [GitHub Discussion](https://github.com/langgenius/dify/discussions). Best for: sharing feedback and asking questions.
- [GitHub Issues](https://github.com/langgenius/dify/issues). Best for: bugs you encounter using Dify.AI, and feature proposals. See our [Contribution Guide](https://github.com/langgenius/dify/blob/main/CONTRIBUTING.md).
- [Discord](https://discord.gg/FngNHpbcY7). Best for: sharing your applications and hanging out with the community.
- [X(Twitter)](https://twitter.com/dify_ai). Best for: sharing your applications and hanging out with the community.
# When the frontend and backend run on different subdomains, set COOKIE_DOMAIN to the site’s top-level domain (e.g., `example.com`). Leading dots are optional.
@@ -23,3 +23,8 @@ Run direct Python commands through `uv run --project api`. Docker-backed integra
- Use Pydantic v2 for request and response models. Reuse domain-specific exceptions and translate them at the controller boundary.
- Use existing Celery task and queue owners for asynchronous work; do not route unrelated jobs through workflow-specific services.
- Celery tasks that may be retried or redelivered must keep side effects idempotent and log affected resource identifiers.
## Typing Best Practises
- Treat nullable types (`T | None`) strictly as union types; document the semantic meaning for each branch.
- Treat default parameter values as introducing distinct function overloads; verify that every added default has demonstrable practical utility and avoids masking unsafe runtime states.
@@ -87,16 +87,17 @@ The scripts resolve paths relative to their location, so you can run them from a
uv sync --group dev
```
1. Run the tests locally with mocked system environment variables in `tool.pytest_env` section in `pyproject.toml`, more can check [Claude.md](../CLAUDE.md)
1. Run the tests locally with mocked system environment variables in `tool.pytest_env` section in `pyproject.toml`, more can check [AGENTS.md](../AGENTS.md)
Continue in the `api` directory from the previous step.
```bash
cd api
uv run pytest # Run all tests
uv run pytest tests/unit_tests/ # Unit tests only
uv run pytest tests/integration_tests/ # Integration tests
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
