Skip to content

Hide HTTP routers from agent tooling - #3960

Merged
vigoo merged 3 commits into
mainfrom
gol-569-http-router-tooling-visibility
Sep 28, 2026
Merged

vigoo merged 3 commits into
mainfrom
gol-569-http-router-tooling-visibility

Conversation

@vigoo

@vigoo vigoo commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • hide HTTP router agent types from public agent catalogs, MCP capabilities, CLI constructor help, and ordinary create/invoke workflows
  • prevent bridge and REPL client generation for routers while clearing stale REPL metadata for router-only projects
  • retain router provisioning, configuration, HTTP deployment, and runtime state, and exclude hidden router methods from MCP native-tool collision checks
  • cover all seven HTTP-handler tooling conformance vectors across CLI, registry, and worker-service tests

Verification

  • bun run --cwd golem-service-base/tests/fixtures/http-handlers test
  • targeted golem-cli, golem-worker-service, and golem-registry-service tests
  • targeted CLI bridge integration test
  • cargo fmt -p golem-cli -p golem-worker-service -p golem-registry-service -- --check
  • CARGO_INCREMENTAL=0 cargo clippy -p golem-cli -p golem-worker-service -p golem-registry-service --all-targets --no-deps -- -D warnings

Resolves GOL-569

@vigoo
vigoo requested a review from a team September 24, 2026 12:28
@netlify

netlify Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for golemcloud canceled.

Name Link
🔨 Latest commit 3100b46
🔍 Latest deploy log https://app.netlify.com/projects/golemcloud/deploys/6aba145ae306190008004afd

.agents
.keys()
.filter_map(|agent_name| self.registered_agent_types.get(agent_name))
.filter(|agent| {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we exclude (or explicitly reject) HttpRouter entries at the start of MCP compilation, rather than only excluding their method names from collision detection? They are still pushed into registered_agent_types, and their security_scheme still participates in unique_scheme_names. A hidden router can therefore make an otherwise valid native-tool deployment fail with McpDeploymentConflictingSecuritySchemes, or make a router-only MCP deployment pass the non-empty check and expose an empty server. Filtering the effective capability set before the empty/security checks would keep deployment behavior aligned with the runtime visibility change.

repl_language: GuestLanguage,
targets: &[BridgeSdkTarget],
) -> BTreeMap<GuestLanguage, ReplMetadata> {
let mut repl_metadata_by_language = BTreeMap::from([(repl_language, ReplMetadata::default())]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This empty metadata entry is written by the new build-plan executor, but gen_bridge_with_manifest_mode_filter_and_additional_collision_targets still returns when plan.targets is empty before calling write_repl_metadata. A router-only plan through the exported gen_bridge / gen_external_bridge path therefore leaves stale REPL metadata behind. Could we either write the metadata before that return or remove those older wrappers if they are no longer supported, and cover stale-file replacement at the entrypoint level?

implemented_by: test_implementer(),
webhook_domain_and_segments: None,
};
let definition = executable_test_tool("foo-bar", "baz");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This regression currently does not exercise the new collision filter: the router capability name is foo-bar_baz, while the native export asserted below is foo_bar_baz, so the test succeeds even if the HttpRouter filter is removed. Could we construct a case that actually fails without the filter, or, if native and agent naming make cross-kind collisions impossible, remove or simplify the dead collision check and test the intended invariant directly?

}
let repl_language = target
.target_language
.expect("REPL bridge target requires a target language");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we avoid the two new expect("REPL bridge target requires a target language") calls? Today every production REPL target comes from new_repl_bridge_sdk_target(language), so the invariant holds, but it is not expressed by CustomBridgeSdkTarget: target_language remains optional because the same type also represents ordinary custom targets, and both planning functions accept that weaker type. A future internal misrouting would therefore turn into a CLI panic. Either a dedicated REPL target type with a required language, or a contextual anyhow error at these boundaries, would preserve the invariant without a panic.

@vigoo
vigoo merged commit 7d1d700 into main Sep 28, 2026
69 checks passed
@vigoo
vigoo deleted the gol-569-http-router-tooling-visibility branch September 28, 2026 09:01
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 28, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants