diff --git a/api/core/dify_builder/handlers_build.py b/api/core/dify_builder/handlers_build.py index 7e0f8c9aa8c..2637b17d2ba 100644 --- a/api/core/dify_builder/handlers_build.py +++ b/api/core/dify_builder/handlers_build.py @@ -473,7 +473,14 @@ def handle_plan_approval(env: Env, turn: Turn, s: Session, fc: DifyBuilderContex # built nodes (not the Builder's session model). resource_ids were already # string-filtered when stored (handle_resource_recommendation). selected_resource_ids = list(fc.resource_selection.get("resource_ids") or []) - build_result = env.agent.build_nodes(list(fc.plan_items), selected_resource_ids) + # The text the user actually typed (goal + the requirements form as they + # submitted it) -- core/dify_builder cannot import services, so this is + # built inline, matching services.dify_builder.agent.user_supplied's + # trusted_text_for byte-for-byte (see test_plan_approval_passes_trusted_ + # text_matching_user_supplied_trusted_text_for). Lets build_nodes tell an + # endpoint the user actually gave from one the model invented (ESQ1-302/S5b). + trusted_text = f"{fc.goal_text}\n{json.dumps(dict(fc.requirements), ensure_ascii=False)}" + build_result = env.agent.build_nodes(list(fc.plan_items), selected_resource_ids, trusted_text=trusted_text) intents = build_result.intents if not any(intent.op == "create_node" for intent in intents): diff --git a/api/core/dify_builder/placeholder_agent.py b/api/core/dify_builder/placeholder_agent.py index 292013e7610..19fc2df6113 100644 --- a/api/core/dify_builder/placeholder_agent.py +++ b/api/core/dify_builder/placeholder_agent.py @@ -139,47 +139,51 @@ class PlaceholderAgent: "Emit the final report", ] - def build_nodes(self, plan_items: list[str], resource_ids: list[str] | None = None) -> BuildNodesResult: + def build_nodes( + self, plan_items: list[str], resource_ids: list[str] | None = None, *, trusted_text: str = "" + ) -> BuildNodesResult: # Start -> Knowledge-Retrieval -> LLM -> End. Creates and connects are # interleaved so each connect's endpoints already exist when # apply_connect validates them. - return BuildNodesResult(intents=[ - MutationIntent( - op="create_node", - args={ - "node_type": BuiltinNodeTypes.START, - "config": node_defaults.default_config(BuiltinNodeTypes.START), - "node_id": BUILD_START_ID, - }, - ), - MutationIntent( - op="create_node", - args={ - "node_type": BuiltinNodeTypes.KNOWLEDGE_RETRIEVAL, - "config": node_defaults.default_config(BuiltinNodeTypes.KNOWLEDGE_RETRIEVAL), - "node_id": BUILD_KNOWLEDGE_ID, - }, - ), - MutationIntent(op="connect", args={"from_node": BUILD_START_ID, "to_node": BUILD_KNOWLEDGE_ID}), - MutationIntent( - op="create_node", - args={ - "node_type": BuiltinNodeTypes.LLM, - "config": node_defaults.default_config(BuiltinNodeTypes.LLM), - "node_id": BUILD_LLM_ID, - }, - ), - MutationIntent(op="connect", args={"from_node": BUILD_KNOWLEDGE_ID, "to_node": BUILD_LLM_ID}), - MutationIntent( - op="create_node", - args={ - "node_type": BuiltinNodeTypes.END, - "config": node_defaults.default_config(BuiltinNodeTypes.END), - "node_id": BUILD_END_ID, - }, - ), - MutationIntent(op="connect", args={"from_node": BUILD_LLM_ID, "to_node": BUILD_END_ID}), - ]) + return BuildNodesResult( + intents=[ + MutationIntent( + op="create_node", + args={ + "node_type": BuiltinNodeTypes.START, + "config": node_defaults.default_config(BuiltinNodeTypes.START), + "node_id": BUILD_START_ID, + }, + ), + MutationIntent( + op="create_node", + args={ + "node_type": BuiltinNodeTypes.KNOWLEDGE_RETRIEVAL, + "config": node_defaults.default_config(BuiltinNodeTypes.KNOWLEDGE_RETRIEVAL), + "node_id": BUILD_KNOWLEDGE_ID, + }, + ), + MutationIntent(op="connect", args={"from_node": BUILD_START_ID, "to_node": BUILD_KNOWLEDGE_ID}), + MutationIntent( + op="create_node", + args={ + "node_type": BuiltinNodeTypes.LLM, + "config": node_defaults.default_config(BuiltinNodeTypes.LLM), + "node_id": BUILD_LLM_ID, + }, + ), + MutationIntent(op="connect", args={"from_node": BUILD_KNOWLEDGE_ID, "to_node": BUILD_LLM_ID}), + MutationIntent( + op="create_node", + args={ + "node_type": BuiltinNodeTypes.END, + "config": node_defaults.default_config(BuiltinNodeTypes.END), + "node_id": BUILD_END_ID, + }, + ), + MutationIntent(op="connect", args={"from_node": BUILD_LLM_ID, "to_node": BUILD_END_ID}), + ] + ) def learn_from_build( self, diff --git a/api/core/dify_builder/ports.py b/api/core/dify_builder/ports.py index 05e79282e2e..d27b9c7c600 100644 --- a/api/core/dify_builder/ports.py +++ b/api/core/dify_builder/ports.py @@ -91,7 +91,9 @@ class DifyBuilderAgent(Protocol): def bind_resources(self, plan_items: list[str], resource_ids: list[str]) -> list[str]: ... - def build_nodes(self, plan_items: list[str], resource_ids: list[str] | None = None) -> BuildNodesResult: ... + def build_nodes( + self, plan_items: list[str], resource_ids: list[str] | None = None, *, trusted_text: str = "" + ) -> BuildNodesResult: ... def learn_from_build( self, diff --git a/api/services/dify_builder/agent/build.py b/api/services/dify_builder/agent/build.py index 01715d39145..1273d3e0a82 100644 --- a/api/services/dify_builder/agent/build.py +++ b/api/services/dify_builder/agent/build.py @@ -618,6 +618,8 @@ def build_nodes( model_config: dict[str, Any], plan_items: list[str], resource_ids: Sequence[str] = (), + *, + trusted_text: str = "", ) -> BuildNodesResult: try: mc = _generator_model_config(tenant_id, model_config) @@ -660,7 +662,7 @@ def build_nodes( # runtime model follows the user's choice. grounding_mc = _selected_workflow_model(tenant_id, resource_ids) or mc _ground(intents, grounding_mc, tenant_id, plan_items) - for node_id in _ground_placeholder_endpoints(intents): + for node_id in _ground_placeholder_endpoints(intents, trusted_text=trusted_text): logger.info("Dify Builder: http-request %s had a placeholder URL; now read from a start variable", node_id) applicable, rejected = graph_ops.filter_applicable({"nodes": [], "edges": []}, intents, _ALLOWED_NODE_TYPES) if not applicable: @@ -759,9 +761,9 @@ def _is_placeholder_endpoint(url: str) -> bool: return _PLACEHOLDER_URL_RE.search(text) is not None or _is_localhost_host(text) -def _ground_placeholder_endpoints(intents: list[MutationIntent]) -> list[str]: - """Replace every http-request placeholder URL with a REQUIRED start-node - variable and a template reference to it. +def _ground_placeholder_endpoints(intents: list[MutationIntent], *, trusted_text: str = "") -> list[str]: + """Replace every http-request URL the user didn't supply with a REQUIRED + start-node variable and a template reference to it. ESQ1-302 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 @@ -769,7 +771,16 @@ def _ground_placeholder_endpoints(intents: list[MutationIntent]) -> list[str]: the user for the real endpoint -- or, when they have none, tells them what the step needs. Builder-only; the shared generator's own http-request example is ``https://example.com``, so cmd+K keeps its placeholders. - Returns the ids of the http-request nodes it re-pointed. + + A URL is grounded when it LOOKS invented (``_is_placeholder_endpoint``) OR, + with ``trusted_text`` non-empty, when its host never actually appears in + what the user typed (``user_supplied.is_user_supplied_url``) -- a + plausible, real-looking host the model invented (S5b: + ``api.pptrender.io``) is just as much a fabrication as + ``api.example.com``, only harder to spot by shape alone. + ``trusted_text == ""`` (no caller-supplied text) keeps exactly today's + placeholder-only behaviour. Returns the ids of the http-request nodes it + re-pointed. """ start = next((i for i in intents if i.op == "create_node" and i.args.get("node_type") == "start"), None) if start is None: @@ -784,7 +795,9 @@ def _ground_placeholder_endpoints(intents: list[MutationIntent]) -> list[str]: if intent.op != "create_node" or intent.args.get("node_type") != "http-request": continue node_config = dict(intent.args.get("config") or {}) - if not _is_placeholder_endpoint(str(node_config.get("url") or "")): + url = str(node_config.get("url") or "") + not_user_supplied = bool(trusted_text) and not user_supplied.is_user_supplied_url(url, trusted_text) + if not (_is_placeholder_endpoint(url) or not_user_supplied): continue node_id = str(intent.args.get("node_id") or "") var_name = f"{node_id}_url" diff --git a/api/services/dify_builder/agent/llm_agent.py b/api/services/dify_builder/agent/llm_agent.py index 21856f99c7c..e0cb5c74c92 100644 --- a/api/services/dify_builder/agent/llm_agent.py +++ b/api/services/dify_builder/agent/llm_agent.py @@ -133,8 +133,10 @@ class LlmBuilderAgent: def bind_resources(self, plan_items, resource_ids): return build.bind_resources(self.model_or_none(), self._tenant_id, plan_items, resource_ids) - def build_nodes(self, plan_items, resource_ids=None): - return build.build_nodes(self._tenant_id, self._model_config, plan_items, resource_ids or ()) + def build_nodes(self, plan_items, resource_ids=None, *, trusted_text=""): + return build.build_nodes( + self._tenant_id, self._model_config, plan_items, resource_ids or (), trusted_text=trusted_text + ) def learn_from_build(self, goal_text, requirements, plan_items, built_node_ids): return build.learn_from_build( diff --git a/api/tests/unit_tests/core/dify_builder/fakes.py b/api/tests/unit_tests/core/dify_builder/fakes.py index 6c55e88ebe8..5a30b1dfbbd 100644 --- a/api/tests/unit_tests/core/dify_builder/fakes.py +++ b/api/tests/unit_tests/core/dify_builder/fakes.py @@ -438,7 +438,7 @@ class StubAgent: def bind_resources(self, _plan_items, _resource_ids): return [] - def build_nodes(self, _plan_items, _resource_ids=None): + def build_nodes(self, _plan_items, _resource_ids=None, *, trusted_text=""): # noqa: ARG002 return BuildNodesResult(intents=[]) def learn_from_build(self, _goal_text, _requirements, _plan_items, _built_node_ids): diff --git a/api/tests/unit_tests/core/dify_builder/test_handlers_build.py b/api/tests/unit_tests/core/dify_builder/test_handlers_build.py index 7991d5fbe42..574f7d47769 100644 --- a/api/tests/unit_tests/core/dify_builder/test_handlers_build.py +++ b/api/tests/unit_tests/core/dify_builder/test_handlers_build.py @@ -365,6 +365,46 @@ def test_plan_approval_approve_builds_graph_and_reveals_nodes(): assert len(assistant.payload["execution"]["activities"]) == 3 +def test_plan_approval_passes_trusted_text_matching_user_supplied_trusted_text_for(): + """handle_plan_approval threads the user's own text (goal + submitted + requirements) through to build_nodes as trusted_text, so + _ground_placeholder_endpoints can tell an endpoint the user actually gave + from one the model invented (ESQ1-302/S5b). core/dify_builder cannot + import services, so the handler builds this string itself -- pinned here + byte-identical to services.dify_builder.agent.user_supplied.trusted_text_for + on the same input.""" + from core.dify_builder.handlers_build import handle_plan_approval + from services.dify_builder.agent.user_supplied import trusted_text_for + from tests.unit_tests.core.dify_builder.fakes import FakeBuildDifyPort + + class _RecordingAgent(PlaceholderAgent): + def __init__(self): + self.build_nodes_calls: list[str] = [] + + def build_nodes(self, plan_items, resource_ids=None, *, trusted_text=""): + self.build_nodes_calls.append(trusted_text) + return super().build_nodes(plan_items, resource_ids) + + agent = _RecordingAgent() + env, repo = _new_env(dify=FakeBuildDifyPort(), agent=agent) + s = _seed_build_session( + repo, + PcState.BUILD_PLAN_APPROVAL, + plan_items=["Retrieve", "Summarize"], + plan_version_tag="v1", + requirements={"endpoint": "", "currency": "USD"}, + ) + session, fc = repo.get_session(s.id) + turn = Turn(action=Action(kind="approve_repair", base_version=1), actor=_actor()) + handle_plan_approval(env, turn, session, fc) + + assert len(agent.build_nodes_calls) == 1 + trusted_text = agent.build_nodes_calls[0] + assert fc.goal_text in trusted_text + assert "USD" in trusted_text + assert trusted_text == trusted_text_for(fc.goal_text, fc.requirements) + + def test_plan_approval_ignores_non_approve_action(): from core.dify_builder.handlers_build import handle_plan_approval from tests.unit_tests.core.dify_builder.fakes import FakeBuildDifyPort @@ -392,7 +432,7 @@ def test_plan_approval_empty_build_surfaces_error_and_keeps_canvas(): } # generation produced nothing, WITH a specific reason (the real generator returns # e.g. "UNRESOLVED_REFERENCE: ..." / a provider error). The error card must carry it. - env.agent.build_nodes = lambda _plan, _rids=None: BuildNodesResult( + env.agent.build_nodes = lambda _plan, _rids=None, *, trusted_text="": BuildNodesResult( # noqa: ARG005 intents=[], error="UNRESOLVED_REFERENCE: Reference {#node4.x#} not declared" ) s = _session(entry_mode=EntryMode.BUILD, current_state=PcState.BUILD_PLAN_APPROVAL) @@ -431,7 +471,7 @@ def test_plan_approval_error_card_carries_diagnostics_into_the_item_payload(): "errors": [{"code": "UNRESOLVED_REFERENCE", "detail": "...", "node_id": "node2"}], } ] - env.agent.build_nodes = lambda _plan, _rids=None: BuildNodesResult( + env.agent.build_nodes = lambda _plan, _rids=None, *, trusted_text="": BuildNodesResult( # noqa: ARG005 intents=[], error="Reference {#node2.response#} not declared on node 'node2'", diagnostics=diag ) s = _session(entry_mode=EntryMode.BUILD, current_state=PcState.BUILD_PLAN_APPROVAL) @@ -459,7 +499,7 @@ def test_plan_approval_deletes_pre_existing_start_on_from_scratch_build(): "edges": [], } # generator returns a graph whose start id is "node1" (a document variable) - env.agent.build_nodes = lambda _plan, _rids=None: BuildNodesResult( + env.agent.build_nodes = lambda _plan, _rids=None, *, trusted_text="": BuildNodesResult( # noqa: ARG005 intents=[ MutationIntent( op="create_node", @@ -503,7 +543,7 @@ def test_plan_approval_survives_generator_reusing_the_deleted_placeholder_start_ "edges": [], } # generator reuses the SAME id ("start") for its own start node - env.agent.build_nodes = lambda _plan, _rids=None: BuildNodesResult( + env.agent.build_nodes = lambda _plan, _rids=None, *, trusted_text="": BuildNodesResult( # noqa: ARG005 intents=[ MutationIntent( op="create_node", @@ -2499,7 +2539,7 @@ def _stub_agent_building(intents): from tests.unit_tests.core.dify_builder.fakes import StubAgent class _Agent(StubAgent): - def build_nodes(self, _plan_items, _resource_ids=None): + def build_nodes(self, _plan_items, _resource_ids=None, *, trusted_text=""): # noqa: ARG002 return BuildNodesResult(intents=list(intents)) return _Agent() diff --git a/api/tests/unit_tests/services/dify_builder/test_build_nodes.py b/api/tests/unit_tests/services/dify_builder/test_build_nodes.py index 9c733df4906..9f97e47620a 100644 --- a/api/tests/unit_tests/services/dify_builder/test_build_nodes.py +++ b/api/tests/unit_tests/services/dify_builder/test_build_nodes.py @@ -551,7 +551,7 @@ def _gen_graph_with_http(url: str) -> dict: } -def _build_with(gen_graph: dict): +def _build_with(gen_graph: dict, *, trusted_text: str = ""): with ( patch.object(build, "_generator_model_config", return_value=_fake_mc("anthropic", "x")), patch( @@ -564,7 +564,7 @@ def _build_with(gen_graph: dict): return_value=resources.TenantResources(models=[], datasets=[], tools=[]), ), ): - return build.build_nodes("t1", {}, ["Call the PPT API"]).intents + return build.build_nodes("t1", {}, ["Call the PPT API"], trusted_text=trusted_text).intents def test_build_nodes_grounds_a_placeholder_endpoint_into_a_required_start_variable(): @@ -644,6 +644,34 @@ def test_build_nodes_leaves_a_real_endpoint_and_a_templated_url_alone(): assert start.args["config"]["variables"] == [] +def test_build_nodes_grounds_an_endpoint_the_user_never_supplied_even_when_not_placeholder_shaped(): + """A plausible, real-looking host the model invented (S5b/ESQ1-302's shape + one step earlier: ``api.pptrender.io``) must still be grounded once + trusted_text is available -- ``_is_placeholder_endpoint`` alone would miss + it, since nothing about the URL LOOKS invented the way ``example.com`` or + ``your-api.com`` do.""" + goal = "Build a workflow that renders a slide deck from bullet points and returns a download link." + intents = _build_with(_gen_graph_with_http("https://api.pptrender.io/v1/render"), trusted_text=goal) + + http = next(i for i in intents if i.args.get("node_type") == "http-request") + start = next(i for i in intents if i.args.get("node_type") == "start") + assert http.args["config"]["url"] == "{{#s.h_url#}}" + var = next(v for v in start.args["config"]["variables"] if v["variable"] == "h_url") + assert var["required"] is True + + +def test_build_nodes_leaves_an_endpoint_alone_when_the_user_actually_supplied_its_host(): + """The mirror case: trusted_text literally names the same host the + generated URL uses, so it is not a fabrication and must be left alone.""" + goal = "Call https://api.pptrender.io to render a slide deck from bullet points." + intents = _build_with(_gen_graph_with_http("https://api.pptrender.io/v1/render"), trusted_text=goal) + + http = next(i for i in intents if i.args.get("node_type") == "http-request") + start = next(i for i in intents if i.args.get("node_type") == "start") + assert http.args["config"]["url"] == "https://api.pptrender.io/v1/render" + assert start.args["config"]["variables"] == [] + + def test_is_placeholder_endpoint_recognises_the_usual_inventions(): assert build._is_placeholder_endpoint("https://api.example.com/ppt/generate") assert build._is_placeholder_endpoint("http://example.org/x") diff --git a/api/tests/unit_tests/services/dify_builder/test_llm_agent.py b/api/tests/unit_tests/services/dify_builder/test_llm_agent.py index 6bea798439e..abd1f9ca807 100644 --- a/api/tests/unit_tests/services/dify_builder/test_llm_agent.py +++ b/api/tests/unit_tests/services/dify_builder/test_llm_agent.py @@ -70,8 +70,8 @@ def test_remaining_build_methods_thread_model_and_tenant_args(monkeypatch): seen["bind_resources"] = (model, tenant_id, plan_items, resource_ids) return [] - def mock_build_nodes(tenant_id, model_config, plan_items, resource_ids=()): - seen["build_nodes"] = (tenant_id, model_config, plan_items, resource_ids) + def mock_build_nodes(tenant_id, model_config, plan_items, resource_ids=(), *, trusted_text=""): + seen["build_nodes"] = (tenant_id, model_config, plan_items, resource_ids, trusted_text) return [] def mock_learn_from_build(model, goal_text, requirements, plan_items, built_node_ids, _reasoning=None): @@ -105,7 +105,13 @@ def test_remaining_build_methods_thread_model_and_tenant_args(monkeypatch): assert seen["bind_resources"] == ("MODEL", "t1", ["step"], ["rid"]) agent.build_nodes(["step"]) - assert seen["build_nodes"] == ("t1", model_config, ["step"], ()) + assert seen["build_nodes"] == ("t1", model_config, ["step"], (), "") + + # trusted_text forwards through the wrapper unchanged (ESQ1-302/S5b: the + # handler threads the user's own goal+requirements text down to + # build.build_nodes so it can tell a supplied endpoint from an invented one). + agent.build_nodes(["step"], trusted_text="goal text and requirements") + assert seen["build_nodes"] == ("t1", model_config, ["step"], (), "goal text and requirements") agent.learn_from_build("goal", {"x": 1}, ["step"], ["n1"]) assert seen["learn_from_build"] == ("MODEL", "goal", {"x": 1}, ["step"], ["n1"])