diff --git a/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/health.ex b/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/health.ex index e3d91b48f2..d9de3a45ba 100644 --- a/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/health.ex +++ b/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/health.ex @@ -101,6 +101,10 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.Health do "protocol_version" => @protocol_version, "resources" => resource_contract_material(), "schema_version" => @mcp_contract_schema_version, + "startup_tool_sets" => + Map.new([:full, :worker, :architect, :coordinator, :solo], fn profile -> + {Atom.to_string(profile), ToolCatalog.startup_tool_specs(profile, config)} + end), "tool_sets" => %{ "architect" => ToolCatalog.architect_session_tool_specs(current_work_request?: false), "architect_current_work_request" => ToolCatalog.architect_session_tool_specs(current_work_request?: true), diff --git a/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/server.ex b/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/server.ex index 6fa67aebcf..714c034112 100644 --- a/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/server.ex +++ b/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/server.ex @@ -249,24 +249,13 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.Server do defp handle_tool_call_request({:ok, %{"name" => name} = params}, id, %__MODULE__{} = server) when name in @session_claim_tools do - case Surface.tool_specs_for_server(server) do - {:ok, specs} -> - if Enum.any?(specs, &(&1["name"] == name)) do - handle_session_claim_tool(name, params, id, server) - else - error = - {:error, -32_601, "Method not found", - %{ - "tool" => name, - "reason" => "tool_not_callable", - "recovery" => %{"next_action" => "list_tools"} - }} - - dispatch_request_state(error, "tools/call", id, server) - end + {:ok, specs} = Surface.tool_specs_for_server(server) - {:error, reason} -> - dispatch_request_state(worker_error(reason, name), "tools/call", id, server) + if Enum.any?(specs, &(&1["name"] == name)) and session_claim_tool_allowed?(name, server) do + handle_session_claim_tool(name, params, id, server) + else + error = {:error, -32_601, "Method not found", %{"tool" => name, "reason" => "tool_not_callable"}} + dispatch_request_state(error, "tools/call", id, server) end end @@ -276,6 +265,23 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.Server do defp handle_tool_call_request(params_result, id, %__MODULE__{} = server), do: dispatch_request_state(params_result, "tools/call", id, server) + defp session_claim_tool_allowed?(_name, %__MODULE__{session: nil, stale_assignment_role: nil}), do: true + + defp session_claim_tool_allowed?(@local_assignment_claim_tool, %__MODULE__{ + session: %Session{assignment: %{grant_role: "worker"}} + }), + do: true + + defp session_claim_tool_allowed?(@local_assignment_claim_tool, %__MODULE__{session: nil, stale_assignment_role: "worker"}), do: true + + defp session_claim_tool_allowed?(@local_architect_assignment_claim_tool, %__MODULE__{ + session: %Session{assignment: %{grant_role: "architect"}} + }), + do: true + + defp session_claim_tool_allowed?(@local_architect_assignment_claim_tool, %__MODULE__{session: nil, stale_assignment_role: "architect"}), do: true + defp session_claim_tool_allowed?(_name, %__MODULE__{}), do: false + defp observe_tool_call(payload, %__MODULE__{} = server, fun) when is_function(fun, 0) do started_at = FailedCall.monotonic_now() @@ -483,10 +489,8 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.Server do end defp dispatch("tools/list", params, %__MODULE__{} = server) when is_map(params) do - case Surface.tool_specs_for_server(server) do - {:ok, tools} -> {:ok, %{"tools" => tools}} - {:error, reason} -> worker_error(reason, "tools/list") - end + {:ok, tools} = Surface.tool_specs_for_server(server) + {:ok, %{"tools" => tools}} end defp dispatch("tools/list", _params, _server) do @@ -518,7 +522,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.Server do case claim_local_assignment(params, server) do {:ok, result, session} -> updated_server = %{server | session: session, session_refresh_required: false, stale_assignment_role: nil} - {:ok, ToolResult.claim_tool_result(put_surface_relist(result, updated_server)), updated_server} + {:ok, ToolResult.claim_tool_result(result), updated_server} {:error, code, message, data} -> {:error, code, message, data} @@ -529,7 +533,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.Server do case claim_local_architect_assignment(params, server) do {:ok, result, session} -> updated_server = %{server | session: session, session_refresh_required: false, stale_assignment_role: nil} - {:ok, ToolResult.claim_tool_result(put_surface_relist(result, updated_server)), updated_server} + {:ok, ToolResult.claim_tool_result(result), updated_server} {:error, code, message, data} -> {:error, code, message, data} @@ -539,7 +543,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.Server do defp dispatch("tools/call", %{"name" => @assignment_release_tool} = params, %__MODULE__{} = server) do with {:ok, arguments} <- prepare_assignment_release_tool_call(server, params), {:ok, result, updated_server} <- release_current_assignment(arguments, server) do - {:ok, ToolResult.release_tool_result(put_surface_relist(result, updated_server)), updated_server} + {:ok, ToolResult.release_tool_result(result), updated_server} else {:error, code, message, data} -> {:error, code, message, data} {:tool_error, reason} -> invalid_params_error(@assignment_release_tool, reason) @@ -742,7 +746,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.Server do {:ok, arguments} -> case release_current_assignment(arguments, server) do {:ok, result, updated_server} -> - tool_result = build_release_tool_result(server, put_surface_relist(result, updated_server)) + tool_result = build_release_tool_result(server, result) {Response.response(id, tool_result), updated_server} {:tool_error, reason} -> @@ -762,7 +766,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.Server do tool_result = build_tool_result(server, fn -> - ToolResult.claim_tool_result(put_surface_relist(result, updated_server)) + ToolResult.claim_tool_result(result) end) { @@ -782,7 +786,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.Server do tool_result = build_tool_result(server, fn -> - ToolResult.claim_tool_result(put_surface_relist(result, updated_server)) + ToolResult.claim_tool_result(result) end) { @@ -2251,18 +2255,6 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.Server do build_tool_result(server, fn -> ToolResult.release_tool_result(result) end) end - defp put_surface_relist(result, %__MODULE__{} = server) do - case Surface.surface_revision(server) do - {:ok, revision} -> - result - |> Map.put("surface_revision", revision) - |> Map.put("relist", %{"next_action" => "list_tools"}) - - {:error, _reason} -> - Map.put(result, "relist", %{"next_action" => "list_tools"}) - end - end - defp response_text_profile(%__MODULE__{config: %Config{mode: :stdio, surface_profile: :full}}), do: :full defp response_text_profile(%__MODULE__{}), do: :canonical diff --git a/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/surface.ex b/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/surface.ex index fa44b46928..932b32bea2 100644 --- a/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/surface.ex +++ b/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/surface.ex @@ -9,47 +9,22 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.Surface do alias SymphonyElixir.SymphonyPlusPlus.MCP.{ Auth, Config, - CurrentWorkRequest, LocalTrustedTools, - Repository, Response, Session, - SessionBindingTools, ToolCatalog } alias SymphonyElixir.SymphonyPlusPlus.Planning.Renderer, as: PlanningRenderer alias SymphonyElixir.SymphonyPlusPlus.Planning.Repository, as: PlanningRepository alias SymphonyElixir.SymphonyPlusPlus.Planning.Service, as: PlanningService - alias SymphonyElixir.SymphonyPlusPlus.WorkPackages.Repository, as: WorkPackageRepository - alias SymphonyElixir.SymphonyPlusPlus.WorkPackages.WorkPackage - @agent_text_mime_type "text/vnd.toon" @assignment_resource "sympp://assignment/current" - @bootstrap_tools ToolCatalog.bootstrap_tools() @version_resource "sympp://health/version" @spec tool_specs_for_server(map()) :: {:ok, [map()]} | {:error, term()} - def tool_specs_for_server(%{session_refresh_required: true, config: %Config{} = config} = server) do - {:ok, effective_tool_specs(claimable_tool_specs(config), server)} - end - - def tool_specs_for_server(%{config: %Config{} = config, session: session} = server) do - surface_session = if(is_nil(session), do: {:ok, nil}, else: SessionBindingTools.tool_surface_session(server)) - - with {:ok, session} <- surface_session, - {:ok, specs} <- tool_specs_for_session(config, session) do - {:ok, effective_tool_specs(specs, server)} - end - end - - @spec surface_revision(map()) :: {:ok, String.t()} | {:error, term()} - def surface_revision(server) do - with {:ok, specs} <- tool_specs_for_server(server) do - revision = specs |> :erlang.term_to_binary() |> then(&:crypto.hash(:sha256, &1)) |> Base.url_encode64(padding: false) - {:ok, "sha256:" <> revision} - end - end + def tool_specs_for_server(%{config: %Config{} = config}), + do: {:ok, ToolCatalog.startup_tool_specs(config.surface_profile, config)} @spec local_trusted_tools_enabled?(map()) :: boolean() def local_trusted_tools_enabled?(server), do: LocalTrustedTools.enabled?(server) @@ -107,160 +82,6 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.Surface do end end - defp tool_specs_for_session(%Config{} = config, nil) do - {:ok, ToolCatalog.callable_unbound_tool_specs_for_config(config)} - end - - defp tool_specs_for_session(%Config{repo: repo} = config, session) do - with :ok <- Repository.ensure_migrated(repo) do - session - |> Auth.require_session(repo) - |> tool_specs_for_session_result(config) - end - end - - defp tool_specs_for_session_result({:ok, %Session{assignment: %{grant_role: "architect"}} = session}, %Config{}) do - {:ok, architect_session_tool_specs(session)} - end - - defp tool_specs_for_session_result({:ok, %Session{assignment: %{grant_role: "worker"}} = session}, %Config{repo: repo}), - do: {:ok, worker_session_tool_specs(repo, session)} - - defp tool_specs_for_session_result({:ok, %Session{}}, %Config{}), do: {:error, {:unauthorized, :unsupported_grant_role}} - - defp tool_specs_for_session_result({:error, {:service_unavailable, _reason} = reason}, %Config{}), do: {:error, reason} - - defp tool_specs_for_session_result({:error, _reason}, %Config{} = config) do - {:ok, claimable_tool_specs(config)} - end - - defp claimable_tool_specs(%Config{} = config), do: ToolCatalog.callable_claim_tool_specs(config) - - defp architect_session_tool_specs(%Session{} = session) do - ToolCatalog.architect_session_tool_specs(current_work_request?: CurrentWorkRequest.single_scope?(session)) - end - - defp worker_session_tool_specs(repo, %Session{} = session) do - ToolCatalog.worker_session_tool_specs() - |> maybe_advertise_compact_attach_branch(repo, session) - end - - defp maybe_advertise_compact_attach_branch(specs, repo, %Session{} = session) do - if compact_attach_branch_available?(repo, session) do - map_tool_spec(specs, "attach_branch", &put_in(&1, ["inputSchema", "required"], ["head_sha"])) - else - specs - end - end - - defp compact_attach_branch_available?(repo, %Session{} = session) do - with {:ok, %WorkPackage{} = work_package} <- WorkPackageRepository.get(repo, Session.work_package_id(session)), - branch when is_binary(branch) <- normalize_optional_value(work_package.branch_pattern) do - not local_branch_template_pattern?(branch) - else - _missing_or_template -> false - end - end - - defp map_tool_spec(specs, name, fun) do - Enum.map(specs, fn - %{"name" => ^name} = spec -> fun.(spec) - spec -> spec - end) - end - - defp effective_tool_specs(specs, server) do - specs - |> role_scoped_claim_tool_specs(server) - |> advertised_tool_specs(server) - |> Kernel.++(local_trusted_tool_specs(server)) - |> Enum.filter(&(&1["name"] in profile_tool_names(server.config.surface_profile))) - |> dedupe_tool_specs() - |> ToolCatalog.lean_tool_specs() - end - - defp advertised_tool_specs(specs, %{session: nil, session_refresh_required: false} = server) do - if local_trusted_tools_enabled?(server), do: specs, else: hide_trusted_local_tool_specs(specs) - end - - defp advertised_tool_specs(specs, _server), do: hide_trusted_local_tool_specs(specs) - - defp hide_trusted_local_tool_specs(specs), do: Enum.reject(specs, &(&1["name"] in @bootstrap_tools)) - - defp dedupe_tool_specs(specs) do - Enum.uniq_by(specs, & &1["name"]) - end - - defp role_scoped_claim_tool_specs(specs, server) do - case assignment_role(server) do - role when role in ["worker", "architect"] -> - claim_tool = - if role == "worker", - do: ToolCatalog.local_assignment_claim_tool(), - else: ToolCatalog.local_architect_assignment_claim_tool() - - claim_spec = - server.config - |> ToolCatalog.callable_claim_tool_specs() - |> Enum.find(&(&1["name"] == claim_tool)) - - specs - |> Enum.reject(&(&1["name"] in ToolCatalog.session_claim_tools())) - |> maybe_prepend_tool_spec(claim_spec) - - _unbound -> - specs - end - end - - defp assignment_role(%{session: %Session{assignment: %{grant_role: role}}}), do: role - defp assignment_role(%{stale_assignment_role: role}), do: role - defp assignment_role(_server), do: nil - - defp maybe_prepend_tool_spec(specs, nil), do: specs - defp maybe_prepend_tool_spec(specs, spec), do: [spec | specs] - - defp local_trusted_tool_specs( - %{ - session: nil, - session_refresh_required: false, - config: %Config{surface_profile: :full} - } = server - ) do - if local_trusted_tools_enabled?(server) do - LocalTrustedTools.tool_specs(server.config) - else - [] - end - end - - defp local_trusted_tool_specs(_server), do: [] - - defp profile_tool_names(:full), do: ToolCatalog.known_tools() - - defp profile_tool_names(:worker) do - [ - ToolCatalog.health_tool(), - ToolCatalog.assignment_release_tool(), - ToolCatalog.local_assignment_claim_tool() - | ToolCatalog.worker_tools() - ] - end - - defp profile_tool_names(:architect) do - [ - ToolCatalog.health_tool(), - ToolCatalog.assignment_release_tool(), - "get_current_assignment", - ToolCatalog.local_architect_assignment_claim_tool() - | ToolCatalog.bootstrap_tools() ++ ToolCatalog.local_operator_tools() ++ ToolCatalog.architect_tools() - ] - end - - defp profile_tool_names(profile) when profile in [:coordinator, :solo] do - [ToolCatalog.health_tool(), ToolCatalog.assignment_release_tool(), "get_current_assignment" | ToolCatalog.solo_tools()] - end - defp valid_resource_path?(resource_path) when is_binary(resource_path) do String.trim(resource_path) != "" and not String.contains?(resource_path, "/") end @@ -391,19 +212,6 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.Surface do defp require_assignment_introspection(%{grant_role: role}) when role in ["worker", "architect"], do: :ok defp require_assignment_introspection(_assignment), do: {:error, :unsupported_grant_role} - defp local_branch_template_pattern?(pattern) when is_binary(pattern) do - Regex.match?(~r/\{\{\s*[a-zA-Z0-9_]+\s*\}\}/, pattern) - end - - defp normalize_optional_value(value) when is_binary(value) do - case String.trim(value) do - "" -> nil - trimmed -> trimmed - end - end - - defp normalize_optional_value(nil), do: nil - defp auth_error(:unauthorized, resource) do {:error, -32_001, "Unauthorized", %{"resource" => resource, "reason" => "missing_session"}} end diff --git a/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/tool_catalog/input_schemas.ex b/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/tool_catalog/input_schemas.ex index c5c2fd9e76..6117429ef1 100644 --- a/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/tool_catalog/input_schemas.ex +++ b/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/tool_catalog/input_schemas.ex @@ -222,7 +222,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ToolCatalog.InputSchemas do do: schema(%{"reason" => markdown_string_schema("Why this work package is being abandoned.")}, ["reason"]) def worker_tool_input_schema("attach_branch") do - schema(metadata_properties(%{"branch" => string_schema(), "head_sha" => string_schema()}), ["branch", "head_sha"]) + schema(metadata_properties(%{"branch" => string_schema(), "head_sha" => string_schema()}), ["head_sha"]) end def worker_tool_input_schema("attach_pr") do diff --git a/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/tool_catalog/surface_specs.ex b/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/tool_catalog/surface_specs.ex index 9b3797477c..35e0eeda21 100644 --- a/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/tool_catalog/surface_specs.ex +++ b/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/tool_catalog/surface_specs.ex @@ -34,8 +34,25 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ToolCatalog.SurfaceSpecs do end @spec startup_tool_specs(:full | :worker | :architect | :coordinator | :solo, Config.t()) :: [ToolCatalog.tool_spec()] - def startup_tool_specs(profile, %Config{} = config), - do: config |> Map.put(:surface_profile, profile) |> callable_unbound_tool_specs_for_config() |> lean_tool_specs() + def startup_tool_specs(profile, %Config{} = config) do + config = Map.put(config, :surface_profile, profile) + + case profile do + :worker -> + worker_session_tool_specs() ++ local_assignment_claim_tool_specs(config) + + :architect -> + architect_session_tool_specs(current_work_request?: true) ++ local_architect_assignment_claim_tool_specs(config) + + profile when profile in [:coordinator, :solo] -> + introspection_tool_specs() ++ Enum.map(ToolCatalog.solo_tools(), &SoloTools.tool_spec/1) + + :full -> + unbound_tool_specs_for_config(config) ++ local_operator_tool_specs() + end + |> Enum.uniq_by(& &1["name"]) + |> lean_tool_specs() + end defp introspection_tool_specs, do: [health_tool_spec(), assignment_release_tool_spec(), worker_tool_spec("get_current_assignment")] diff --git a/elixir/priv/symphony_plus_plus/mcp_contract.json b/elixir/priv/symphony_plus_plus/mcp_contract.json index 16a2c48b9a..7b24fc68b4 100644 --- a/elixir/priv/symphony_plus_plus/mcp_contract.json +++ b/elixir/priv/symphony_plus_plus/mcp_contract.json @@ -1,6 +1,6 @@ { "version": 7, - "mcp_contract_fingerprint": "6300d025dfa54155c02fc4c297d310824b85741b33113a4696f0659c9f15e4f4", + "mcp_contract_fingerprint": "17d168c3dabecfe05077c37fed9ece2f919d35f876c3bcdc0f511c9c4945fea3", "tool_sets": { "unbound_tools": [ "sympp.health", diff --git a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/claim_session_transport_03_test.exs b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/claim_session_transport_03_test.exs index 9b0203036d..9e69190eda 100644 --- a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/claim_session_transport_03_test.exs +++ b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/claim_session_transport_03_test.exs @@ -488,7 +488,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ClaimSessionTransport03Test do ) assert get_in(response, ["error", "data", "reason"]) == "tool_not_callable" - assert get_in(response, ["error", "data", "recovery"]) == %{"next_action" => "list_tools"} + refute Map.has_key?(response["error"]["data"], "recovery") assert server.session.assignment.grant_role == "worker" assert {:ok, %ClaimLease{status: "active"}} = ClaimLeaseService.current_for_work_package(repo, package.id) end diff --git a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/claim_session_transport_04_test.exs b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/claim_session_transport_04_test.exs index b5b3ac66aa..415dead409 100644 --- a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/claim_session_transport_04_test.exs +++ b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/claim_session_transport_04_test.exs @@ -153,7 +153,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ClaimSessionTransport04Test do assert get_in(other_repo_response, ["error", "data", "reason"]) == "server_not_initialized" end - test "explicit state key stale live server advertises recovery-only tools until new session", %{repo: repo} do + test "explicit state key stale live server keeps the configured catalog", %{repo: repo} do package = create_local_claim_package!(repo, "SYMPP-STATE-STALE-LIVE") assert {:ok, _minted} = AccessGrantService.mint_worker_grant(repo, package.id) @@ -233,13 +233,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ClaimSessionTransport04Test do assert get_in(reinit_response, ["result", "serverInfo", "name"]) == "symphony-plus-plus" - assert Map.keys(tools_by_name) |> Enum.sort() == [ - "claim_local_architect_assignment", - "claim_local_assignment", - "get_current_assignment", - "release_current_assignment", - "sympp.health" - ] + assert tools_by_name == fresh_tools_by_name assert get_in(stale_solo_response, ["error", "data", "reason"]) == "claim_required" assert get_in(stale_solo_response, ["error", "data", "action"]) == "claim_local_assignment" @@ -247,7 +241,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ClaimSessionTransport04Test do assert stale_comment_id == comment.id assert get_in(fresh_init_response, ["result", "serverInfo", "name"]) == "symphony-plus-plus" assert Map.has_key?(tools_by_name, "get_current_assignment") - refute Map.has_key?(fresh_tools_by_name, "read_work_request") + assert Map.has_key?(fresh_tools_by_name, "read_work_request") assert Map.has_key?(fresh_tools_by_name, "claim_local_architect_assignment") assert Map.has_key?(fresh_tools_by_name, "solo_attach") assert Map.has_key?(fresh_tools_by_name, "get_current_assignment") @@ -636,7 +630,8 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ClaimSessionTransport04Test do stale_tool_names = stale_tools |> get_in(["result", "tools"]) |> Enum.map(& &1["name"]) assert "get_current_assignment" in stale_tool_names assert "claim_local_assignment" in stale_tool_names - refute "claim_local_architect_assignment" in stale_tool_names + assert "claim_local_architect_assignment" in stale_tool_names + assert "append_progress" in stale_tool_names assert {:ok, []} = PlanningRepository.list_progress_events(repo, package.id) end @@ -736,7 +731,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ClaimSessionTransport04Test do assert failed_server.session_refresh_required == false end - test "tools list reports claim lease storage failure without advertising an unbound surface", %{repo: repo} do + test "tools list stays available when claim lease storage fails", %{repo: repo} do package = create_local_claim_package!(repo, "SYMPP-STATE-LIST-LEDGER-FAIL") assert {:ok, _minted} = AccessGrantService.mint_worker_grant(repo, package.id) @@ -761,8 +756,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ClaimSessionTransport04Test do failed_server ) - assert get_in(response, ["error", "code"]) == -32_000 - assert get_in(response, ["error", "data", "reason"]) == "ledger_unavailable" + assert is_list(get_in(response, ["result", "tools"])) assert updated_server.session == failed_server.session assert updated_server.session_refresh_required == false end @@ -940,7 +934,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ClaimSessionTransport04Test do stale_tool_names = stale_tools_response |> get_in(["result", "tools"]) |> Enum.map(& &1["name"]) assert "get_current_assignment" in stale_tool_names assert "claim_local_assignment" in stale_tool_names - refute "read_context" in stale_tool_names + assert "read_context" in stale_tool_names {blocked_response, blocked_server} = Server.handle_state( diff --git a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/claim_session_transport_07_test.exs b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/claim_session_transport_07_test.exs index 45204f78b4..f39fc5d3bc 100644 --- a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/claim_session_transport_07_test.exs +++ b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/claim_session_transport_07_test.exs @@ -6,7 +6,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ClaimSessionTransport07Test do alias SymphonyElixir.SymphonyPlusPlus.ClaimLeases.ClaimLease alias SymphonyElixir.SymphonyPlusPlus.MCP.{HTTPStateStore, HTTPTransport} - test "HTTP worker client introspects, claims, re-lists, releases, and reclaims only callable tools", %{repo: repo} do + test "HTTP worker catalog stays stable across claim, release, and reclaim", %{repo: repo} do {package, work_request} = create_http_local_claim_package!(repo, "SYMPP-HTTP-RELEASE-THEN-CLAIM") config = %{local_mcp_config(repo) | surface_profile: :worker} client_key = "client-http-release-then-claim-batch" @@ -16,10 +16,18 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ClaimSessionTransport07Test do state_key = init_result.state_key - unbound_tools = http!(config, client_key, state_key, "unbound-tools", "tools/list", %{}) + startup_tools = http!(config, client_key, state_key, "startup-tools", "tools/list", %{}) - assert tool_names(unbound_tools) == - MapSet.new(["sympp.health", "release_current_assignment", "get_current_assignment", "claim_local_assignment"]) + assert tool_names(startup_tools) == + MapSet.new(["sympp.health", "release_current_assignment", "claim_local_assignment" | @worker_tool_names]) + + unclaimed_read = + http!(config, client_key, state_key, "unclaimed-read", "tools/call", %{ + "name" => "read_context", + "arguments" => %{} + }) + + assert get_in(unclaimed_read.response, ["error", "data", "reason"]) == "claim_required" introspection = http!(config, client_key, state_key, "unbound-assignment", "tools/call", %{ @@ -49,11 +57,12 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ClaimSessionTransport07Test do claim_payload = get_in(claim_result.response, ["result", "structuredContent"]) assert get_in(claim_payload, ["assignment", "work_package_id"]) == package.id - assert claim_payload["surface_revision"] =~ ~r/^sha256:[A-Za-z0-9_-]{43}$/ - assert claim_payload["relist"] == %{"next_action" => "list_tools"} + refute Map.has_key?(claim_payload, "surface_revision") + refute Map.has_key?(claim_payload, "relist") bound_tools = http!(config, client_key, state_key, "bound-tools", "tools/list", %{}) - bound_names = tool_names(bound_tools) + assert tool_specs(bound_tools) == tool_specs(startup_tools) + bound_names = tool_names(startup_tools) assert MapSet.member?(bound_names, "read_context") assert MapSet.member?(bound_names, "get_current_assignment") assert MapSet.member?(bound_names, "claim_local_assignment") @@ -101,10 +110,9 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ClaimSessionTransport07Test do release_payload = get_in(release_result.response, ["result", "structuredContent"]) assert release_payload["binding_cleared"] == true - assert release_payload["surface_revision"] =~ ~r/^sha256:[A-Za-z0-9_-]{43}$/ - assert release_payload["surface_revision"] != claim_payload["surface_revision"] - assert release_payload["relist"] == %{"next_action" => "list_tools"} - assert tool_names(http!(config, client_key, state_key, "released-tools", "tools/list", %{})) == tool_names(unbound_tools) + refute Map.has_key?(release_payload, "surface_revision") + refute Map.has_key?(release_payload, "relist") + assert tool_specs(http!(config, client_key, state_key, "released-tools", "tools/list", %{})) == tool_specs(startup_tools) reclaim_result = http!(config, client_key, state_key, "reclaim", "tools/call", %{ @@ -113,7 +121,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ClaimSessionTransport07Test do }) assert get_in(reclaim_result.response, ["result", "structuredContent", "assignment", "work_package_id"]) == package.id - assert MapSet.member?(tool_names(http!(config, client_key, state_key, "reclaimed-tools", "tools/list", %{})), "read_context") + assert tool_specs(http!(config, client_key, state_key, "reclaimed-tools", "tools/list", %{})) == tool_specs(startup_tools) assert {:ok, %ClaimLease{status: "active"}} = ClaimLeaseService.current_for_work_package(repo, package.id) HTTPStateStore.reset!() @@ -129,6 +137,80 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ClaimSessionTransport07Test do assert get_in(assignment_result.response, ["result", "structuredContent", "assignment", "work_package_id"]) == package.id end + test "HTTP architect claims and slices after one startup tools list", %{repo: repo} do + work_request = + create_work_request!(repo, + id: "WR-HTTP-STABLE-ARCHITECT-CATALOG", + status: "ready_for_slicing" + ) + + assert {:ok, _handoff} = + ArchitectHandoff.create_or_replay(repo, work_request.id, + local_operator?: true, + handoff_opts: [ + claimed_by: ArchitectHandoff.claimed_by(), + database: repo.database_path(), + local_architect_claim?: true + ] + ) + + config = %{local_mcp_config(repo) | surface_profile: :architect} + client_key = "client-http-stable-architect-catalog" + + {:ok, init_result} = + HTTPTransport.handle( + config, + %{"jsonrpc" => "2.0", "id" => "init-stable-architect", "method" => "initialize", "params" => initialize_params()}, + client_key: client_key + ) + + state_key = init_result.state_key + startup_tools = http!(config, client_key, state_key, "architect-tools", "tools/list", %{}) + startup_names = tool_names(startup_tools) + + assert MapSet.member?(startup_names, "slice_work_request") + refute MapSet.member?(startup_names, "read_context") + + slice_arguments = %{ + "work_packages" => [ + %{ + "title" => "Stable catalog regression", + "goal" => "Prove claim authorizes the startup catalog.", + "kind" => "mcp", + "allowed_file_globs" => ["elixir/lib/**"], + "acceptance_criteria" => ["Slice succeeds without another tools/list."], + "validation_steps" => ["mix test test/symphony_elixir/symphony_plus_plus/mcp/claim_session_transport_07_test.exs"], + "stop_conditions" => ["Stop after the regression proves the flow."] + } + ] + } + + unclaimed_slice = + http!(config, client_key, state_key, "unclaimed-slice", "tools/call", %{ + "name" => "slice_work_request", + "arguments" => slice_arguments + }) + + assert get_in(unclaimed_slice.response, ["error", "data", "reason"]) == "claim_required" + + claim = + http!(config, client_key, state_key, "architect-claim", "tools/call", %{ + "name" => "claim_local_architect_assignment", + "arguments" => %{"work_request_id" => work_request.id} + }) + + assert get_in(claim.response, ["result", "structuredContent", "assignment", "grant_role"]) == "architect" + + slice = + http!(config, client_key, state_key, "slice-without-relist", "tools/call", %{ + "name" => "slice_work_request", + "arguments" => slice_arguments + }) + + assert get_in(slice.response, ["result", "structuredContent", "status", "work_request_status"]) == "sliced" + assert [_work_package_id] = get_in(slice.response, ["result", "structuredContent", "work_package_ids"]) + end + defp http!(config, client_key, state_key, id, method, params) do assert {:ok, result} = HTTPTransport.handle( @@ -142,12 +224,14 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ClaimSessionTransport07Test do end defp tool_names(result) do - result.response - |> get_in(["result", "tools"]) + result + |> tool_specs() |> Enum.map(& &1["name"]) |> MapSet.new() end + defp tool_specs(result), do: get_in(result.response, ["result", "tools"]) + defp create_http_local_claim_package!(repo, id) do package = create_local_claim_package!(repo, id, base_branch: "main") diff --git a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/comments_guidance_test.exs b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/comments_guidance_test.exs index c9ccc7205c..cb5df44a7e 100644 --- a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/comments_guidance_test.exs +++ b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/comments_guidance_test.exs @@ -615,10 +615,10 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.CommentsGuidanceTest do |> tools_for_server() |> Map.new(&{&1["name"], &1}) - refute Map.has_key?(implicit_state_tools, "add_work_request_comment") - refute Map.has_key?(implicit_state_tools, "record_work_request_operator_decision") - refute Map.has_key?(remote_tools, "add_work_request_comment") - refute Map.has_key?(remote_tools, "record_work_request_operator_decision") + assert Map.has_key?(implicit_state_tools, "add_work_request_comment") + assert Map.has_key?(implicit_state_tools, "record_work_request_operator_decision") + assert Map.has_key?(remote_tools, "add_work_request_comment") + assert Map.has_key?(remote_tools, "record_work_request_operator_decision") remote_response = Server.handle( @@ -647,8 +647,8 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.CommentsGuidanceTest do |> tools_for_server() |> Map.new(&{&1["name"], &1}) - refute Map.has_key?(memory_tools, "add_work_request_comment") - refute Map.has_key?(memory_tools, "record_work_request_operator_decision") + assert Map.has_key?(memory_tools, "add_work_request_comment") + assert Map.has_key?(memory_tools, "record_work_request_operator_decision") memory_response = Server.handle( diff --git a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/connection_bootstrap_02_test.exs b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/connection_bootstrap_02_test.exs index c967506e5b..4fb3e030b5 100644 --- a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/connection_bootstrap_02_test.exs +++ b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/connection_bootstrap_02_test.exs @@ -110,7 +110,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ConnectionBootstrap02Test do assert is_list(get_in(post_init_response, ["result", "tools"])) end - test "tools list exposes only callable schemas before binding while handcrafted writes stay claim-gated", %{repo: repo} do + test "tools list exposes the stable full catalog while unbound writes stay claim-gated", %{repo: repo} do unbound_server = Server.new(Config.default(repo: repo), initialized: true) unbound_response = @@ -146,7 +146,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ConnectionBootstrap02Test do assert Map.has_key?(unbound_tools_by_name, tool) end - refute Map.has_key?(unbound_tools_by_name, "create_work_request") + assert Map.has_key?(unbound_tools_by_name, "create_work_request") trusted_local_response = Server.handle( @@ -161,13 +161,9 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ConnectionBootstrap02Test do assert Map.has_key?(trusted_local_tools_by_name, "create_work_request") - for tool <- ToolCatalog.architect_tools() do - refute Map.has_key?(unbound_tools_by_name, tool) - end + for tool <- ToolCatalog.architect_tools(), do: assert(Map.has_key?(unbound_tools_by_name, tool)) - for tool <- ToolCatalog.worker_tools(), tool != "get_current_assignment" do - refute Map.has_key?(unbound_tools_by_name, tool) - end + for tool <- ToolCatalog.worker_tools(), do: assert(Map.has_key?(unbound_tools_by_name, tool)) assert get_in(unbound_tools_by_name, ["claim_local_assignment", "inputSchema", "required"]) == ["work_package_id"] assert get_in(unbound_tools_by_name, ["claim_local_assignment", "inputSchema", "properties", "work_package_id", "type"]) == "string" @@ -328,23 +324,16 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ConnectionBootstrap02Test do assert get_in(tools_by_name, ["abandon", "inputSchema", "required"]) == ["reason"] assert get_in(tools_by_name, ["mark_ready", "inputSchema", "properties"]) == %{} - assert get_in(tools_by_name, ["add_comment", "inputSchema", "required"]) == ["body"] + assert get_in(tools_by_name, ["add_comment", "inputSchema", "required"]) == ["target_kind", "target_id", "body"] assert get_in(tools_by_name, ["add_comment", "inputSchema", "properties", "target_kind", "enum"]) == ["work_request", "work_package"] assert get_in(tools_by_name, ["add_comment", "inputSchema", "properties", "body", "maxLength"]) == Comment.max_body_length() assert get_in(tools_by_name, ["add_comment", "inputSchema", "properties", "body", "description"]) =~ "Markdown" - assert get_in(tools_by_name, ["list_comments", "inputSchema", "required"]) == [] - - explicit_work_request_target = %{"required" => ["target_kind"], "properties" => %{"target_kind" => %{"enum" => ["work_request"]}}} - - assert get_in(tools_by_name, ["add_comment", "inputSchema", "if"]) == explicit_work_request_target - assert get_in(tools_by_name, ["add_comment", "inputSchema", "then"]) == %{"required" => ["target_id"]} - assert get_in(tools_by_name, ["list_comments", "inputSchema", "if"]) == explicit_work_request_target - assert get_in(tools_by_name, ["list_comments", "inputSchema", "then"]) == %{"required" => ["target_id"]} + assert get_in(tools_by_name, ["list_comments", "inputSchema", "required"]) == ["target_kind", "target_id"] assert get_in(tools_by_name, ["resolve_comment", "inputSchema", "required"]) == ["comment_id"] assert get_in(tools_by_name, ["resolve_comment", "inputSchema", "properties", "resolution_note", "maxLength"]) == Comment.max_resolution_note_length() assert get_in(tools_by_name, ["resolve_comment", "inputSchema", "properties", "resolution_note", "description"]) =~ "Markdown" - assert get_in(tools_by_name, ["attach_branch", "inputSchema", "required"]) == ["branch", "head_sha"] + assert get_in(tools_by_name, ["attach_branch", "inputSchema", "required"]) == ["head_sha"] assert get_in(tools_by_name, ["attach_branch", "inputSchema", "properties", "head_sha", "type"]) == "string" assert get_in(tools_by_name, ["attach_pr", "inputSchema", "properties", "head_sha", "type"]) == "string" @@ -408,8 +397,8 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ConnectionBootstrap02Test do assert get_in(tools_by_name, ["submit_review_package", "inputSchema", "properties", "head_sha", "type"]) == "string" assert get_in(tools_by_name, ["submit_review_package", "inputSchema", "properties", "acceptance_criteria_met", "type"]) == "boolean" - refute Map.has_key?(tools_by_name, "read_child_status") - refute Map.has_key?(tools_by_name, "mint_child_worker_key") + assert Map.has_key?(tools_by_name, "read_child_status") + assert Map.has_key?(tools_by_name, "mint_child_worker_key") refute Map.has_key?(tools_by_name, "claim_work_key") refute Map.has_key?(tools_by_name, "claim_private_handoff") diff --git a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/delivery_reconcile_01_test.exs b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/delivery_reconcile_01_test.exs index ab0a33016b..4d095b2376 100644 --- a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/delivery_reconcile_01_test.exs +++ b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/delivery_reconcile_01_test.exs @@ -37,9 +37,9 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.DeliveryReconcile01Test do local_server = local_mcp_server(local_mcp_config(repo), "local-work-request-read-state") tools_by_name = tools_for_server(local_server) |> Map.new(&{&1["name"], &1}) - refute Map.has_key?(tools_by_name, "list_work_requests") - refute Map.has_key?(tools_by_name, "read_work_request") - refute Map.has_key?(tools_by_name, "read_delivery_board") + assert Map.has_key?(tools_by_name, "list_work_requests") + assert Map.has_key?(tools_by_name, "read_work_request") + assert Map.has_key?(tools_by_name, "read_delivery_board") assert Map.has_key?(tools_by_name, "get_current_assignment") assert Map.has_key?(tools_by_name, "claim_local_architect_assignment") diff --git a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/failed_call_diagnostics_test.exs b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/failed_call_diagnostics_test.exs index 22e4ff6725..1f1a444b28 100644 --- a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/failed_call_diagnostics_test.exs +++ b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/failed_call_diagnostics_test.exs @@ -141,7 +141,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.FailedCallDiagnosticsTest do refute diagnostic_event_line(not_found_log) =~ missing_session_id end - test "HTTP worker diagnostics recognize restricted catalog tools without request details", %{repo: repo} do + test "HTTP worker diagnostics redact denied cross-role tool details", %{repo: repo} do assert {:ok, _settings} = OperatorSettingsRepository.update(repo, %{"capture_failed_mcp_calls" => true}) package = create_local_claim_package!(repo, "SYMPP-DIAGNOSTIC-WORKER") assert {:ok, _grant} = AccessGrantService.mint_worker_grant(repo, package.id) @@ -218,7 +218,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.FailedCallDiagnosticsTest do assert response["error"]["data"]["reason"] == "architect_grant_required" assert log =~ ~s("tool_name":"create_child_work_package") assert log =~ ~s("failure_reason":"architect_grant_required") - assert log =~ ~s("argument_keys":[]) + assert log =~ ~s("argument_keys":["package"]) refute diagnostic_event_line(log) =~ secret end diff --git a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/local_trusted_comment_tools_test.exs b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/local_trusted_comment_tools_test.exs index b92da85ff6..c0548f3f59 100644 --- a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/local_trusted_comment_tools_test.exs +++ b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/local_trusted_comment_tools_test.exs @@ -172,9 +172,9 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.LocalTrustedCommentToolsTest do |> tools_for_server() |> Map.new(&{&1["name"], &1}) - refute Map.has_key?(worker_tools, "add_work_request_comment") - refute Map.has_key?(worker_tools, "record_work_request_operator_decision") - refute Map.has_key?(worker_tools, "create_work_request") + assert Map.has_key?(worker_tools, "add_work_request_comment") + assert Map.has_key?(worker_tools, "record_work_request_operator_decision") + assert Map.has_key?(worker_tools, "create_work_request") assert Map.has_key?(worker_tools, "list_comments") assert {:ok, other_package} = @@ -293,10 +293,10 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.LocalTrustedCommentToolsTest do assert Map.has_key?(refresh_required_tools, "get_current_assignment") assert Map.has_key?(refresh_required_tools, "claim_local_assignment") assert Map.has_key?(refresh_required_tools, "claim_local_architect_assignment") - refute Map.has_key?(refresh_required_tools, "add_work_request_comment") - refute Map.has_key?(refresh_required_tools, "record_work_request_operator_decision") - refute Map.has_key?(refresh_required_tools, "create_work_request") - refute Map.has_key?(refresh_required_tools, "list_comments") + assert Map.has_key?(refresh_required_tools, "add_work_request_comment") + assert Map.has_key?(refresh_required_tools, "record_work_request_operator_decision") + assert Map.has_key?(refresh_required_tools, "create_work_request") + assert Map.has_key?(refresh_required_tools, "list_comments") refresh_required_response = Server.handle( diff --git a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/solo_schema_01_test.exs b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/solo_schema_01_test.exs index 060dc98b6a..4641e34e62 100644 --- a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/solo_schema_01_test.exs +++ b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/solo_schema_01_test.exs @@ -70,7 +70,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.SoloSchema01Test do defp inject_claim_race(_updates), do: :ok end - test "tools list advertises Solo tools for unbound sessions only", %{repo: repo} do + test "configured role catalogs exclude Solo tools", %{repo: repo} do unbound_server = Server.new(Config.default(repo: repo), initialized: true) unbound_response = @@ -119,7 +119,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.SoloSchema01Test do worker_response = Server.handle( %{"jsonrpc" => "2.0", "id" => "solo-worker-tools", "method" => "tools/list", "params" => %{}}, - Server.new(Config.default(repo: repo), initialized: true, session: worker_session) + Server.new(Config.default(repo: repo, surface_profile: :worker), initialized: true, session: worker_session) ) worker_tools_by_name = @@ -138,7 +138,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.SoloSchema01Test do architect_response = Server.handle( %{"jsonrpc" => "2.0", "id" => "solo-architect-tools", "method" => "tools/list", "params" => %{}}, - Server.new(test_mcp_config(repo), initialized: true, session: architect_session) + Server.new(%{test_mcp_config(repo) | surface_profile: :architect}, initialized: true, session: architect_session) ) architect_tools_by_name = @@ -1121,7 +1121,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.SoloSchema01Test do end end - test "tools list exposes callable unbound recovery after architect grant revocation", %{repo: repo} do + test "tools list keeps the architect catalog after grant revocation", %{repo: repo} do assert {:ok, package} = WorkPackageRepository.create(repo, WorkPackageFactory.attrs(id: "SYMPP-ARCHITECT-TOOLS-REVOKED", kind: "mcp")) assert {:ok, architect_work_key} = create_architect_work_key(repo, package.id, ["read:phase"]) @@ -1129,7 +1129,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.SoloSchema01Test do AccessGrantRepository.claim(repo, architect_work_key.secret, %{claimed_by: "architect-1"}, DateTime.utc_now(:microsecond)) session = MCPHarness.session(architect_assignment, proof_hash: WorkKey.secret_hash(architect_work_key.secret)) - server = Server.new(Config.default(repo: repo), initialized: true, session: session) + server = Server.new(Config.default(repo: repo, surface_profile: :architect), initialized: true, session: session) assert {:ok, _revoked} = AccessGrantService.revoke(repo, architect_assignment.grant_id) @@ -1141,17 +1141,16 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.SoloSchema01Test do "get_current_assignment", "release_current_assignment", "claim_local_architect_assignment", - "solo_attach" + "read_work_request" ] do assert Map.has_key?(tools_by_name, tool) end refute Map.has_key?(tools_by_name, "claim_local_assignment") - refute Map.has_key?(tools_by_name, "read_work_request") refute Map.has_key?(tools_by_name, "read_context") end - test "tools list preserves ledger failures while revalidating bound sessions" do + test "tools list does not depend on bound-session ledger revalidation" do session = Session.new(%Assignment{ grant_id: "grant-1", @@ -1166,12 +1165,11 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.SoloSchema01Test do response = MCPHarness.request( %{"jsonrpc" => "2.0", "id" => "tools-list-ledger-failure", "method" => "tools/list", "params" => %{}}, - config: Config.default(repo: FailingAuthRepo), + config: Config.default(repo: FailingAuthRepo, surface_profile: :architect), session: session ) - assert get_in(response, ["error", "code"]) == -32_000 - assert get_in(response, ["error", "data", "reason"]) == "ledger_unavailable" + assert Enum.any?(get_in(response, ["result", "tools"]), &(&1["name"] == "read_work_request")) end test "tools list keeps static architect schemas while calls use live capabilities", %{repo: repo} do diff --git a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/tool_surface_lean_test.exs b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/tool_surface_lean_test.exs index 3baef16b3b..d2e0207d77 100644 --- a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/tool_surface_lean_test.exs +++ b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/tool_surface_lean_test.exs @@ -6,7 +6,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ToolSurfaceLeanTest do @profiles ~w(worker architect coordinator solo)a @removed_tools ~w(request_child_replan split_work_package publish_phase_update) - test "unbound surface profile lists only immediately callable tools" do + test "surface profiles advertise stable role catalogs before claim" do for profile <- @profiles do tools = listed_tools(profile) names = MapSet.new(tools, & &1["name"]) @@ -17,10 +17,11 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ToolSurfaceLeanTest do case profile do :worker -> - assert MapSet.equal?(names, MapSet.new(["sympp.health", "get_current_assignment", "release_current_assignment", "claim_local_assignment"])) + expected = ["sympp.health", "release_current_assignment", "claim_local_assignment" | ToolCatalog.worker_tools()] + assert MapSet.equal?(names, MapSet.new(expected)) :architect -> - expected = ["sympp.health", "get_current_assignment", "release_current_assignment", "claim_local_architect_assignment"] + expected = ["sympp.health", "get_current_assignment", "release_current_assignment", "claim_local_architect_assignment" | ToolCatalog.architect_tools()] assert MapSet.equal?(names, MapSet.new(expected)) profile when profile in [:coordinator, :solo] -> @@ -54,24 +55,28 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ToolSurfaceLeanTest do assert Process.alive?(agent_pid) end - test "role profiles hide bound tools until claim and keep compact claim schemas" do + test "role profiles expose complete role-correct catalogs and compact claim schemas" do worker = tools_by_name(:worker) architect = tools_by_name(:architect) assert Map.has_key?(worker, "claim_local_assignment") assert Map.has_key?(worker, "get_current_assignment") - refute Map.has_key?(worker, "read_context") - refute Map.has_key?(worker, "mark_ready") + assert Map.has_key?(worker, "read_context") + assert Map.has_key?(worker, "mark_ready") + refute Map.has_key?(worker, "read_work_request") + refute Map.has_key?(worker, "slice_work_request") assert worker["claim_local_assignment"]["inputSchema"]["required"] == ["work_package_id"] assert Map.has_key?(architect, "claim_local_architect_assignment") assert Map.has_key?(architect, "get_current_assignment") - refute Map.has_key?(architect, "read_work_request") - refute Map.has_key?(architect, "dispatch_work_package") + assert Map.has_key?(architect, "read_work_request") + assert Map.has_key?(architect, "dispatch_work_package") + refute Map.has_key?(architect, "read_context") + refute Map.has_key?(architect, "mark_ready") refute Enum.any?(@removed_tools, &Map.has_key?(architect, &1)) end - test "full and default expose callable unbound tools without scoped worker or architect calls" do + test "full and default expose the stable composite catalog" do full = listed_tools(:full) requested_full = @@ -82,8 +87,8 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ToolSurfaceLeanTest do assert Enum.any?(requested_full, &(&1["name"] == "claim_local_architect_assignment")) assert Enum.any?(full, &(&1["name"] == "solo_attach")) assert Enum.any?(full, &(&1["name"] == "get_current_assignment")) - refute Enum.any?(full, &(&1["name"] == "read_context")) - refute Enum.any?(full, &(&1["name"] == "read_work_request")) + assert Enum.any?(full, &(&1["name"] == "read_context")) + assert Enum.any?(full, &(&1["name"] == "read_work_request")) assert Enum.all?(full, &(not Map.has_key?(&1, "title"))) refute Enum.any?(full, &(&1["description"] == "Symphony++ worker tool #{&1["name"]}.")) refute Enum.any?(@removed_tools, fn name -> Enum.any?(full, &(&1["name"] == name)) end) diff --git a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/work_request_tools_01_test.exs b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/work_request_tools_01_test.exs index 24eedc7f0b..4b28093817 100644 --- a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/work_request_tools_01_test.exs +++ b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/work_request_tools_01_test.exs @@ -133,15 +133,17 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.WorkRequestTools01Test do assert get_in(old_name_response, ["error", "code"]) == -32_601 assert get_in(old_name_response, ["error", "data", "tool"]) == "read_work_request_product_tree" - assert get_in(claimed_tools_by_name, ["slice_work_request", "inputSchema", "required"]) == ["work_packages"] + assert get_in(claimed_tools_by_name, ["slice_work_request", "inputSchema", "required"]) == ["work_request_id", "work_packages"] assert get_in(claimed_tools_by_name, ["update_work_package", "inputSchema", "required"]) == [ + "work_request_id", "work_package_id", "expected_contract_revision", "patch" ] assert get_in(claimed_tools_by_name, ["skip_work_package", "inputSchema", "required"]) == [ + "work_request_id", "work_package_id", "current_status" ] @@ -279,7 +281,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.WorkRequestTools01Test do assert get_in(stdio_response, ["error", "code"]) == -32_001 assert get_in(stdio_response, ["error", "data", "tool"]) == "create_work_request" assert get_in(stdio_response, ["error", "data", "reason"]) == "local_mcp_required" - refute Enum.any?(tools_for_server(Server.new(Config.default(repo: repo), initialized: true)), &(&1["name"] == "create_work_request")) + assert Enum.any?(tools_for_server(Server.new(Config.default(repo: repo), initialized: true)), &(&1["name"] == "create_work_request")) implicit_state_response = Server.handle( @@ -295,7 +297,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.WorkRequestTools01Test do assert get_in(implicit_state_response, ["error", "code"]) == -32_001 assert get_in(implicit_state_response, ["error", "data", "reason"]) == "local_mcp_session_required" - refute Enum.any?( + assert Enum.any?( tools_for_server(Server.new(local_mcp_config(repo), initialized: true, local_daemon_trusted: true)), &(&1["name"] == "create_work_request") ) @@ -316,7 +318,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.WorkRequestTools01Test do assert get_in(remote_response, ["error", "code"]) == -32_001 assert get_in(remote_response, ["error", "data", "reason"]) == "local_database_required" refute inspect(remote_response) =~ "ghp_createworksecret" - refute Enum.any?(tools_for_server(local_mcp_server(remote_config, "remote-create-work-request-tools-state")), &(&1["name"] == "create_work_request")) + assert Enum.any?(tools_for_server(local_mcp_server(remote_config, "remote-create-work-request-tools-state")), &(&1["name"] == "create_work_request")) memory_response = Server.handle( diff --git a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/worker_tools_01_test.exs b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/worker_tools_01_test.exs index 5446c1e74a..94361fbad3 100644 --- a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/worker_tools_01_test.exs +++ b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/worker_tools_01_test.exs @@ -635,7 +635,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.WorkerTools01Test do assert get_in(tools_by_name, ["attach_branch", "inputSchema", "required"]) == ["head_sha"] for tool <- ["update_task_plan", "append_progress", "attach_branch", "complete_review"] do - refute Map.has_key?(get_in(tools_by_name, [tool, "inputSchema", "properties"]), "work_package_id") + refute "work_package_id" in get_in(tools_by_name, [tool, "inputSchema", "required"]) end compact_branch_response = attach_tool(repo, session, "attach_branch", %{"head_sha" => "compact-head"}) diff --git a/elixir/test/symphony_elixir/symphony_plus_plus/mcp_http_endpoint_test.exs b/elixir/test/symphony_elixir/symphony_plus_plus/mcp_http_endpoint_test.exs index f0777aed97..8d294704cd 100644 --- a/elixir/test/symphony_elixir/symphony_plus_plus/mcp_http_endpoint_test.exs +++ b/elixir/test/symphony_elixir/symphony_plus_plus/mcp_http_endpoint_test.exs @@ -262,7 +262,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCPHTTPEndpointTest do end for tool <- ["read_context", "read_work_request", "create_child_work_package", "list_work_requests"] do - refute tool in names + assert tool in names end end @@ -351,9 +351,9 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCPHTTPEndpointTest do assert "get_current_assignment" in tool_names assert "append_progress" in tool_names assert "claim_local_assignment" in tool_names - refute "claim_local_architect_assignment" in tool_names + assert "claim_local_architect_assignment" in tool_names refute "claim_private_handoff" in tool_names - refute "solo_attach" in tool_names + assert "solo_attach" in tool_names assignment_tool = post_json(tool_call_request("assignment-tool", "get_current_assignment", %{}), [{"mcp-session-id", session_id}]) @@ -410,7 +410,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCPHTTPEndpointTest do assert "get_current_assignment" in tool_names assert "read_work_request" in tool_names assert "dispatch_work_package" in tool_names - refute "solo_attach" in tool_names + assert "solo_attach" in tool_names read = post_json( @@ -772,7 +772,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCPHTTPEndpointTest do tool_names = tool_names(json_response(tools, 200)) assert "claim_local_assignment" in tool_names - refute "claim_local_architect_assignment" in tool_names + assert "claim_local_architect_assignment" in tool_names assert "get_current_assignment" in tool_names assignment_tool = diff --git a/elixir/test/symphony_elixir/symphony_plus_plus/mcp_http_transport_minimal_test.exs b/elixir/test/symphony_elixir/symphony_plus_plus/mcp_http_transport_minimal_test.exs index db11e05a11..60c63de523 100644 --- a/elixir/test/symphony_elixir/symphony_plus_plus/mcp_http_transport_minimal_test.exs +++ b/elixir/test/symphony_elixir/symphony_plus_plus/mcp_http_transport_minimal_test.exs @@ -196,10 +196,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCPHTTPTransportMinimalTest do HTTPTransport.handle(trusted_config, tools_list_request("trusted-worker-tools"), client_key: "trusted-client", state_key: init.state_key) claimed_names = tool_names(claimed_tools.response) - assert "get_current_assignment" in claimed_names - assert "append_progress" in claimed_names - assert "claim_local_assignment" in claimed_names - refute "claim_local_architect_assignment" in claimed_names + assert claimed_names == names refute "claim_private_handoff" in claimed_names assert {:ok, assignment} = @@ -579,7 +576,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCPHTTPTransportMinimalTest do HTTPTransport.handle(local_config, tools_list_request("tools-after-claim"), client_key: "client-a", state_key: init.state_key) assert "claim_local_assignment" in tool_names(tools.response) - refute "claim_local_architect_assignment" in tool_names(tools.response) + assert "claim_local_architect_assignment" in tool_names(tools.response) assert "get_current_assignment" in tool_names(tools.response) assert {:ok, assignment} = diff --git a/plugins/symphony-plus-plus-mcp/skills/symphony-work-package/SKILL.md b/plugins/symphony-plus-plus-mcp/skills/symphony-work-package/SKILL.md index 35288c192d..9d147b3dc0 100644 --- a/plugins/symphony-plus-plus-mcp/skills/symphony-work-package/SKILL.md +++ b/plugins/symphony-plus-plus-mcp/skills/symphony-work-package/SKILL.md @@ -30,8 +30,9 @@ cross-slice target, successor relation, audit closeout, or concurrency guard. `{"work_package_id":""}`. Include `claimed_by` only when the dispatch payload or operator provided a stable worker identity. A successful first claim atomically activates a `ready_for_worker` package. -4. Follow the successful claim's `relist.next_action` and refresh the MCP tool - list before calling worker tools. Release results require the same refresh. +4. Call worker tools from the stable worker catalog that was advertised at + initialization. Claims and releases change authorization and scope, not the + tool catalog. 5. Replay the same local claim after reconnects. The server heartbeats the current lease, reclaims stale leases with audit evidence, and rejects paused leases or another active owner. Reconnect does not rewrite lifecycle state. diff --git a/plugins/symphony-plus-plus-mcp/skills/symphony-work-package/references/mcp_wiring.md b/plugins/symphony-plus-plus-mcp/skills/symphony-work-package/references/mcp_wiring.md index cee06b0eb1..42f8aa04e2 100644 --- a/plugins/symphony-plus-plus-mcp/skills/symphony-work-package/references/mcp_wiring.md +++ b/plugins/symphony-plus-plus-mcp/skills/symphony-work-package/references/mcp_wiring.md @@ -86,8 +86,8 @@ dispatch. Plugin installation is not worker package dispatch. Normal work-package worker dispatch emits a `worker_bootstrap` payload with `type: ledger_claim`, `mode: local_assignment`, and `claim.tool: claim_local_assignment`. The worker claims -with only the WorkPackage id and optional `claimed_by` owner, follows the -claim response's explicit re-list action, then calls `get_current_assignment`. +with only the WorkPackage id and optional `claimed_by` owner, then calls +`get_current_assignment` from the stable worker catalog advertised at startup. ## Local HTTP Server @@ -134,11 +134,11 @@ MCP session that will do the WorkPackage work. Workers start by calling `claim_local_assignment` in a dedicated S++ MCP session connected to the same ledger as dispatch. Pass `work_package_id` and, when provided, `claimed_by`. Before claim, `get_current_assignment()` succeeds -with `assignment: null` and a claim or reclaim action. After claim, follow -`relist.next_action`, refresh the tool list, then call -`get_current_assignment()` and read package context. The first successful claim -atomically activates a `ready_for_worker` package. Release also returns an -explicit re-list action. +with `assignment: null` and a claim or reclaim action. After claim, call +`get_current_assignment()` and read package context without re-listing tools. +The first successful claim atomically activates a `ready_for_worker` package. +Release changes authorization and scope without changing the advertised tool +catalog. Replaying the same claim heartbeats the current claim lease. If the prior lease is stale, the server may reclaim it and records audit evidence without diff --git a/plugins/symphony-plus-plus-mcp/skills/symphony-work-package/references/worker_prompt.md b/plugins/symphony-plus-plus-mcp/skills/symphony-work-package/references/worker_prompt.md index 30a6a97249..52762c7659 100644 --- a/plugins/symphony-plus-plus-mcp/skills/symphony-work-package/references/worker_prompt.md +++ b/plugins/symphony-plus-plus-mcp/skills/symphony-work-package/references/worker_prompt.md @@ -25,9 +25,9 @@ Before coding: 2. Claim the assignment through `claim_local_assignment`. The first successful claim activates the package atomically; reconnecting the same claim does not change lifecycle state. -3. Follow the successful claim's `relist.next_action`, refresh the MCP tool - list, then call `get_current_assignment()` and treat that assignment as the - scope. +3. Call `get_current_assignment()` from the stable worker catalog and treat + that assignment as the scope. Claim and release do not require another + tool-list refresh. 4. If claim fails because the lease is paused, another active owner exists, or the local ledger scope mismatches, stop and ask the architect or operator to repair that state. Do not request raw secrets.