Skip to content

[SYMPP-MCP] Keep tool catalogs stable across assignment claims #622

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Pimpmuckl wants to merge 2 commits into from
Closed

[SYMPP-MCP] Keep tool catalogs stable across assignment claims #622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b9faeba
fix(mcp): keep role catalogs stable across claims
Pimpmuckl Aug 17, 2026
c6e9948
fix(mcp): preserve compact catalog compatibility
Pimpmuckl Aug 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions elixir/lib/symphony_elixir/symphony_plus_plus/mcp/health.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
70 changes: 31 additions & 39 deletions elixir/lib/symphony_elixir/symphony_plus_plus/mcp/server.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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()

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}
Expand All @@ -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}
Expand All @@ -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)
Expand Down Expand Up @@ -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} ->
Expand All @@ -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)

{
Expand All @@ -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)

{
Expand Down Expand Up @@ -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

Expand Down
196 changes: 2 additions & 194 deletions elixir/lib/symphony_elixir/symphony_plus_plus/mcp/surface.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion elixir/lib/symphony_elixir/symphony_plus_plus/mcp/tool_catalog/input_schemas.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading