From c8c1a7052d245444331a79cab482da809efb78dd Mon Sep 17 00:00:00 2001 From: Charles Yao Date: Sat, 22 Aug 2026 10:46:12 -0700 Subject: [PATCH] feat(workflow-copilot): apply_repair dispatches all 5 verbs + real graph diff feeds change_set Co-Authored-By: Claude Opus 4.8 --- api/core/workflow_copilot/handlers_fix.py | 13 ++- api/core/workflow_copilot/models.py | 2 + api/services/workflow_copilot/dify_port.py | 38 +++++-- .../workflow_copilot/test_handlers_fix.py | 26 +++++ .../workflow_copilot/test_dify_port.py | 106 +++++++++++++++++- 5 files changed, 171 insertions(+), 14 deletions(-) diff --git a/api/core/workflow_copilot/handlers_fix.py b/api/core/workflow_copilot/handlers_fix.py index c302ae1aea9..22c6884fb48 100644 --- a/api/core/workflow_copilot/handlers_fix.py +++ b/api/core/workflow_copilot/handlers_fix.py @@ -200,13 +200,18 @@ def handle_apply(env: Env, turn: Turn, s: Session, fc: FixContext) -> StepResult ``checklist.await_recheck``. Port of ``handlers_fix.go:146``.""" result = env.dify.apply_repair(s.app_id, turn.actor, fc.staged_repair) fc.last_snapshot_hash = result.new_hash - fc.change_set = ChangeSet(changed_nodes=result.changed_nodes, diff="config edit") + # Adapters that don't compute a real diff (e.g. FakeDifyPort in tests) + # leave changes/scope empty -- fall back to the old changed_nodes/ + # "configuration" behavior so those callers stay green. + changes = list(result.changes) if result.changes else list(result.changed_nodes) + scope = result.scope or "configuration" + fc.change_set = ChangeSet(changed_nodes=result.changed_nodes, diff="; ".join(changes) or "config edit") items = append_card( fc, ChangeSetCard( - count=len(result.changed_nodes), - changes=list(result.changed_nodes), - scope="configuration", + count=len(changes), + changes=changes, + scope=scope, full_diff_open=False, ), ) diff --git a/api/core/workflow_copilot/models.py b/api/core/workflow_copilot/models.py index ac1730ba2d0..81ada062cb1 100644 --- a/api/core/workflow_copilot/models.py +++ b/api/core/workflow_copilot/models.py @@ -200,6 +200,8 @@ class ApplyResult: changed_nodes: list[str] = field(default_factory=list) new_hash: str = "" + changes: list[str] = field(default_factory=list) + scope: str = "" @dataclass(kw_only=True) diff --git a/api/services/workflow_copilot/dify_port.py b/api/services/workflow_copilot/dify_port.py index 68b9b66a401..c37692d8210 100644 --- a/api/services/workflow_copilot/dify_port.py +++ b/api/services/workflow_copilot/dify_port.py @@ -59,7 +59,15 @@ from repositories.factory import DifyAPIRepositoryFactory from services.app_generate_service import AppGenerateService from services.errors.app import WorkflowHashNotEqualError from services.workflow_copilot.errors import HashMismatchError, WorkflowNotInitializedError -from services.workflow_copilot.graph_ops import apply_set_node_config +from services.workflow_copilot.graph_ops import ( + apply_connect, + apply_create_node, + apply_delete_node, + apply_insert_between, + apply_set_node_config, + diff_graphs, + validate_intent_args, +) from services.workflow_copilot.identity import load_app, resolve_account from services.workflow_copilot.run_mapping import map_run_result, to_node_event from services.workflow_service import WorkflowService @@ -67,6 +75,15 @@ from services.workflow_service import WorkflowService __all__ = ["WorkflowServiceDifyPort"] +_APPLY_FNS: dict[str, Callable[..., tuple[Graph, list[str]]]] = { + "set_node_config": apply_set_node_config, + "create_node": apply_create_node, + "delete_node": apply_delete_node, + "connect": apply_connect, + "insert_between": apply_insert_between, +} + + def _session_factory() -> sessionmaker[Session]: """One session per adapter call -- never shared/held across calls.""" return sessionmaker(bind=db.engine, expire_on_commit=False) @@ -124,19 +141,22 @@ class WorkflowServiceDifyPort: app = load_app(session, app_id, actor) workflow = _load_draft_workflow_or_raise(app, session=session) - graph: Graph = dict(workflow.graph_dict) + before_graph: Graph = dict(workflow.graph_dict) + graph: Graph = before_graph unique_hash = workflow.unique_hash changed_nodes: list[str] = [] for intent in intents: - if intent.op != "set_node_config": + apply_fn = _APPLY_FNS.get(intent.op) + if apply_fn is None: continue - graph, changed = apply_set_node_config( - graph, intent.args["node_id"], intent.args["path"], intent.args["value"] - ) + validate_intent_args(intent) + graph, changed = apply_fn(graph, **intent.args) changed_nodes.extend(changed) if not changed_nodes: - return ApplyResult(changed_nodes=[], new_hash=unique_hash) + return ApplyResult(changed_nodes=[], new_hash=unique_hash, changes=[], scope="") + + changes, scope = diff_graphs(before_graph, graph) try: updated = WorkflowService().sync_draft_workflow( @@ -156,7 +176,9 @@ class WorkflowServiceDifyPort: except WorkflowHashNotEqualError as exc: raise HashMismatchError(f"draft workflow changed since read: {app_id}") from exc - return ApplyResult(changed_nodes=changed_nodes, new_hash=updated.unique_hash) + return ApplyResult( + changed_nodes=changed_nodes, new_hash=updated.unique_hash, changes=changes, scope=scope + ) def run_draft(self, app_id: str, actor: Actor, inputs: Inputs, on_event: Callable[[NodeEvent], None]) -> Run: with _session_factory()() as session: diff --git a/api/tests/unit_tests/core/workflow_copilot/test_handlers_fix.py b/api/tests/unit_tests/core/workflow_copilot/test_handlers_fix.py index d23b04fe28c..e6bed1f5981 100644 --- a/api/tests/unit_tests/core/workflow_copilot/test_handlers_fix.py +++ b/api/tests/unit_tests/core/workflow_copilot/test_handlers_fix.py @@ -160,6 +160,32 @@ def test_apply_writes_graph_locks_during_work_unlocks_at_await_verify(): assert "output" in fc.change_set.changed_nodes +def test_apply_emits_change_set_card_with_fallback_scope_and_changes_when_adapter_omits_them(): + # NOTE: this assertion already holds against the OLD hardcoded handle_apply + # (scope="configuration", changes=changed_nodes) -- Task 8's fallback branch + # is *designed* to reproduce that exact old behavior for any adapter (like + # FakeDifyPort) that doesn't populate changes/scope, so this is a + # same-behavior regression pin, not a red-first TDD test. It stays green + # through Step 2 below; the real red/green for Task 8's behavior change + # lives in the test_dify_port.py additions above (only WorkflowServiceDifyPort + # computes a real diff). + env, repo = _new_env() + s = _seed_diagnose_session(repo) + + runner = Runner(env, _run_fix_registry()) + turn = Turn(action=Action(kind="request_fix", base_version=1), actor=_actor()) + runner.advance(s.id, turn) + + items = repo.list_conversation(s.id) + change_set_item = next(i for i in items if i.kind == "change_set") + # FakeDifyPort's ApplyResult never populates changes/scope (Slice 1's + # real diff only exists in WorkflowServiceDifyPort) -- handle_apply must + # fall back to changed_nodes / "configuration" so Fix stays green. + assert change_set_item.payload["scope"] == "configuration" + assert change_set_item.payload["changes"] == ["output"] + assert change_set_item.payload["count"] == 1 + + # ---- await_approval ------------------------------------------------------ diff --git a/api/tests/unit_tests/services/workflow_copilot/test_dify_port.py b/api/tests/unit_tests/services/workflow_copilot/test_dify_port.py index e8900d791de..b3b9f3f8ad9 100644 --- a/api/tests/unit_tests/services/workflow_copilot/test_dify_port.py +++ b/api/tests/unit_tests/services/workflow_copilot/test_dify_port.py @@ -200,7 +200,7 @@ def test_apply_repair_syncs_mutated_graph_with_graph_only_and_no_agent_binding_s assert original_graph["nodes"][0]["data"]["code"] == "old" -def test_apply_repair_ignores_non_set_node_config_intents(mock_session: MagicMock): +def test_apply_repair_ignores_unknown_op_intents(mock_session: MagicMock): account = SimpleNamespace(id="acc-1") app = SimpleNamespace(id="app-1", tenant_id="tenant-1") _configure_session_get(mock_session, account=account, app=app) @@ -211,7 +211,11 @@ def test_apply_repair_ignores_non_set_node_config_intents(mock_session: MagicMoc features_dict={}, conversation_variables=[], ) - intents = [MutationIntent(op="connect", args={"from": "node-1", "to": "node-2"})] + # Ops the dispatch table doesn't recognize are silently skipped -- this + # is forward-compat for future verbs, distinct from a *recognized* op + # with bad args (which raises, see test_apply_repair_dispatches_connect_ + # and_reports_dangling_ref_as_value_error below). + intents = [MutationIntent(op="some_future_verb", args={"from": "node-1", "to": "node-2"})] with patch("services.workflow_copilot.dify_port.WorkflowService") as mock_ws_cls: mock_ws_cls.return_value.get_draft_workflow.return_value = workflow @@ -247,6 +251,104 @@ def test_apply_repair_maps_hash_mismatch_to_domain_error(mock_session: MagicMock WorkflowServiceDifyPort().apply_repair("app-1", _actor(), intents) +def test_apply_repair_dispatches_create_node(mock_session: MagicMock): + account = SimpleNamespace(id="acc-1") + app = SimpleNamespace(id="app-1", tenant_id="tenant-1") + _configure_session_get(mock_session, account=account, app=app) + + workflow = SimpleNamespace( + graph_dict={"nodes": [], "edges": []}, + unique_hash="hash-1", + features_dict={}, + conversation_variables=[], + ) + updated_workflow = SimpleNamespace(unique_hash="hash-2") + intents = [MutationIntent(op="create_node", args={"node_type": "llm", "config": {}})] + + with patch("services.workflow_copilot.dify_port.WorkflowService") as mock_ws_cls: + mock_ws_cls.return_value.get_draft_workflow.return_value = workflow + mock_ws_cls.return_value.sync_draft_workflow.return_value = updated_workflow + + result = WorkflowServiceDifyPort().apply_repair("app-1", _actor(), intents) + + assert len(result.changed_nodes) == 1 + _, kwargs = mock_ws_cls.return_value.sync_draft_workflow.call_args + assert len(kwargs["graph"]["nodes"]) == 1 + assert kwargs["graph"]["nodes"][0]["data"]["type"] == "llm" + + +def test_apply_repair_dispatches_connect_and_reports_dangling_ref_as_value_error(mock_session: MagicMock): + account = SimpleNamespace(id="acc-1") + app = SimpleNamespace(id="app-1", tenant_id="tenant-1") + _configure_session_get(mock_session, account=account, app=app) + + workflow = SimpleNamespace( + graph_dict={"nodes": [{"id": "a", "data": {}}], "edges": []}, + unique_hash="hash-1", + features_dict={}, + conversation_variables=[], + ) + intents = [MutationIntent(op="connect", args={"from_node": "a", "to_node": "missing"})] + + with patch("services.workflow_copilot.dify_port.WorkflowService") as mock_ws_cls: + mock_ws_cls.return_value.get_draft_workflow.return_value = workflow + + with pytest.raises(ValueError): + WorkflowServiceDifyPort().apply_repair("app-1", _actor(), intents) + + +def test_apply_repair_computes_real_diff_changes_and_scope_for_structural_edit(mock_session: MagicMock): + account = SimpleNamespace(id="acc-1") + app = SimpleNamespace(id="app-1", tenant_id="tenant-1") + _configure_session_get(mock_session, account=account, app=app) + + workflow = SimpleNamespace( + graph_dict={"nodes": [{"id": "a", "data": {}}], "edges": []}, + unique_hash="hash-1", + features_dict={}, + conversation_variables=[], + ) + updated_workflow = SimpleNamespace(unique_hash="hash-2") + intents = [ + MutationIntent(op="create_node", args={"node_type": "llm", "config": {}, "node_id": "llm-1"}), + MutationIntent(op="connect", args={"from_node": "a", "to_node": "llm-1"}), + ] + + with patch("services.workflow_copilot.dify_port.WorkflowService") as mock_ws_cls: + mock_ws_cls.return_value.get_draft_workflow.return_value = workflow + mock_ws_cls.return_value.sync_draft_workflow.return_value = updated_workflow + + result = WorkflowServiceDifyPort().apply_repair("app-1", _actor(), intents) + + assert result.scope == "structure" + assert "added node llm-1" in result.changes + assert "added a → llm-1" in result.changes + + +def test_apply_repair_computes_configuration_scope_for_set_node_config(mock_session: MagicMock): + account = SimpleNamespace(id="acc-1") + app = SimpleNamespace(id="app-1", tenant_id="tenant-1") + _configure_session_get(mock_session, account=account, app=app) + + workflow = SimpleNamespace( + graph_dict={"nodes": [{"id": "node-1", "data": {"code": "old"}}], "edges": []}, + unique_hash="hash-1", + features_dict={}, + conversation_variables=[], + ) + updated_workflow = SimpleNamespace(unique_hash="hash-2") + intents = [MutationIntent(op="set_node_config", args={"node_id": "node-1", "path": "code", "value": "new"})] + + with patch("services.workflow_copilot.dify_port.WorkflowService") as mock_ws_cls: + mock_ws_cls.return_value.get_draft_workflow.return_value = workflow + mock_ws_cls.return_value.sync_draft_workflow.return_value = updated_workflow + + result = WorkflowServiceDifyPort().apply_repair("app-1", _actor(), intents) + + assert result.scope == "configuration" + assert result.changes == ["node-1: code updated"] + + # ---- run_draft --------------------------------------------------------------