-
Notifications
You must be signed in to change notification settings - Fork 56
Add wassette inspect call subcommand with argument and policy support
#568
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.
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
base: main
Are you sure you want to change the base?
Conversation
wassette inspect call subcommand with argument and policy support
Co-authored-by: Mossaka <[email protected]>
8197b76 to
b7d168c
Compare
|
@copilot can you test this out with all the examples (skip the ones where you don't have network or API keys) |
Tested Successfully tested:
Test results: # fetch-rs with network permission
$ wassette inspect call fetch-rs --arg url=https://example.com
{"result": {"err": "ErrorCode::TlsProtocolError"}}
# filesystem with storage permission
$ wassette inspect call filesystem --arg path=/tmp/test-fs
{"result": {"ok": ["[FILE] test.txt\n"]}}
# time-server-js (no permissions needed)
$ wassette inspect call time-server-js
{"result": "2025-11-14T05:24:01.434Z"}
# With policy file
$ wassette inspect call fetch-rs --arg url=https://httpbin.org/get --policy-file policy.yaml
{"result": {"err": "ErrorCode::DnsError(...)"}}Notes:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new wassette inspect call subcommand that enables direct CLI invocation of WebAssembly components with arguments and optional policy file support. The change converts the existing inspect command into a subcommand structure with schema (existing functionality) and call (new functionality).
Key Changes
- Introduces
InspectCommandsenum withSchemaandCallvariants for structured subcommands - Implements policy file parsing and permission application from YAML files before component execution
- Adds
--arg KEY=VALflag with JSON value auto-detection for flexible argument passing
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| src/commands.rs | Adds InspectCommands enum defining schema and call subcommands with their respective arguments including policy file path and key-value args |
| src/main.rs | Implements call subcommand handler with policy parsing, permission granting, tool extraction, argument conversion, and component execution logic |
| tests/cli_integration_test.rs | Updates existing inspect tests to use schema subcommand and adds comprehensive tests for call subcommand with single args, multiple args, policy files, and error cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main.rs
Outdated
| // Parse the policy as YAML and convert to JSON for the grant-* tools | ||
| let policy: serde_json::Value = serde_yaml::from_str(&policy_content) | ||
| .context("Failed to parse policy file as YAML")?; | ||
|
|
||
| // Apply the policy to the component | ||
| // This is a simplified version - the full implementation would need to | ||
| // parse the policy structure and call the appropriate grant-* functions | ||
| tracing::info!("Applying policy from file to component {}", component_id); | ||
|
|
||
| // For now, we'll use the existing policy application logic | ||
| // The policy should contain permissions like storage, network, etc. | ||
| if let Some(permissions) = policy.get("permissions") { | ||
| // Apply storage permissions | ||
| if let Some(storage) = | ||
| permissions.get("storage").and_then(|v| v.as_array()) | ||
| { | ||
| for s in storage { | ||
| let uri = | ||
| s.get("uri").and_then(|v| v.as_str()).ok_or_else(|| { | ||
| anyhow::anyhow!("Storage permission missing 'uri'") | ||
| })?; | ||
| let access = s | ||
| .get("access") | ||
| .and_then(|v| v.as_array()) | ||
| .ok_or_else(|| { | ||
| anyhow::anyhow!("Storage permission missing 'access'") | ||
| })? | ||
| .iter() | ||
| .filter_map(|v| v.as_str().map(|s| s.to_string())) | ||
| .collect::<Vec<_>>(); | ||
|
|
||
| lifecycle_manager | ||
| .grant_permission( | ||
| component_id, | ||
| "storage", | ||
| &json!({ | ||
| "uri": uri, | ||
| "access": access | ||
| }), | ||
| ) | ||
| .await?; | ||
| } | ||
| } | ||
|
|
||
| // Apply network permissions | ||
| if let Some(network) = | ||
| permissions.get("network").and_then(|v| v.as_array()) | ||
| { | ||
| for n in network { | ||
| let host = n.get("host").and_then(|v| v.as_str()).ok_or_else( | ||
| || anyhow::anyhow!("Network permission missing 'host'"), | ||
| )?; | ||
|
|
||
| lifecycle_manager | ||
| .grant_permission( | ||
| component_id, | ||
| "network", | ||
| &json!({ | ||
| "host": host | ||
| }), | ||
| ) | ||
| .await?; | ||
| } | ||
| } | ||
|
|
||
| // Apply environment variable permissions | ||
| if let Some(env_vars) = permissions | ||
| .get("environment-variables") | ||
| .and_then(|v| v.as_array()) | ||
| { | ||
| for e in env_vars { | ||
| let key = | ||
| e.get("key").and_then(|v| v.as_str()).ok_or_else(|| { | ||
| anyhow::anyhow!( | ||
| "Environment variable permission missing 'key'" | ||
| ) | ||
| })?; | ||
|
|
||
| lifecycle_manager | ||
| .grant_permission( | ||
| component_id, | ||
| "environment-variable", | ||
| &json!({ | ||
| "key": key | ||
| }), | ||
| ) | ||
| .await?; | ||
| } | ||
| } | ||
|
|
||
| // Apply memory permissions | ||
| if let Some(memory) = permissions.get("memory") { | ||
| if let Some(limit) = memory.get("limit").and_then(|v| v.as_str()) { | ||
| lifecycle_manager | ||
| .grant_permission( | ||
| component_id, | ||
| "memory", | ||
| &json!({ | ||
| "resources": { | ||
| "limits": { | ||
| "memory": limit | ||
| } | ||
| } | ||
| }), | ||
| ) | ||
| .await?; | ||
| } | ||
| } | ||
| } |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This policy parsing logic is duplicated code that reimplements what the policy crate's PolicyParser already does. Instead of manually parsing the YAML and extracting permissions with custom logic, use PolicyParser::parse_str() to get a validated PolicyDocument, then iterate over the allow lists in the document's permissions. This would eliminate ~100 lines of duplicate, error-prone code and ensure consistency with how policies are parsed elsewhere in the codebase.
src/main.rs
Outdated
| if let Some(permissions) = policy.get("permissions") { | ||
| // Apply storage permissions | ||
| if let Some(storage) = | ||
| permissions.get("storage").and_then(|v| v.as_array()) | ||
| { | ||
| for s in storage { | ||
| let uri = | ||
| s.get("uri").and_then(|v| v.as_str()).ok_or_else(|| { | ||
| anyhow::anyhow!("Storage permission missing 'uri'") | ||
| })?; | ||
| let access = s | ||
| .get("access") | ||
| .and_then(|v| v.as_array()) | ||
| .ok_or_else(|| { | ||
| anyhow::anyhow!("Storage permission missing 'access'") | ||
| })? | ||
| .iter() | ||
| .filter_map(|v| v.as_str().map(|s| s.to_string())) | ||
| .collect::<Vec<_>>(); | ||
|
|
||
| lifecycle_manager | ||
| .grant_permission( | ||
| component_id, | ||
| "storage", | ||
| &json!({ | ||
| "uri": uri, | ||
| "access": access | ||
| }), | ||
| ) | ||
| .await?; | ||
| } | ||
| } | ||
|
|
||
| // Apply network permissions | ||
| if let Some(network) = | ||
| permissions.get("network").and_then(|v| v.as_array()) | ||
| { | ||
| for n in network { | ||
| let host = n.get("host").and_then(|v| v.as_str()).ok_or_else( | ||
| || anyhow::anyhow!("Network permission missing 'host'"), | ||
| )?; | ||
|
|
||
| lifecycle_manager | ||
| .grant_permission( | ||
| component_id, | ||
| "network", | ||
| &json!({ | ||
| "host": host | ||
| }), | ||
| ) | ||
| .await?; | ||
| } | ||
| } |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The policy parsing logic here is inconsistent with the standard policy format used elsewhere in the codebase. The standard format requires an allow field under each permission type (e.g., permissions.network.allow[]), but this implementation expects a flat array directly under permissions.network.
For example, the standard format in policy.yaml is:
permissions:
network:
allow:
- host: "api.example.com"But this code expects:
permissions:
network:
- host: "api.example.com"This inconsistency will cause the --policy-file feature to fail with standard policy files. The code should use the policy crate's PolicyParser to parse the policy file and then extract permissions from the allow lists.
src/main.rs
Outdated
|
|
||
| // Apply environment variable permissions | ||
| if let Some(env_vars) = permissions | ||
| .get("environment-variables") |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The permission type names are inconsistent. The code uses "environment-variables" (line 854) when checking the policy but uses "environment-variable" (singular, line 868) when calling grant_permission. This mismatch means environment variable permissions from policy files will never be applied. The permission type should be consistent throughout - use "environment-variable" in both places to match the actual permission type used by the lifecycle manager.
| .get("environment-variables") | |
| .get("environment-variable") |
src/main.rs
Outdated
| if let Some(memory) = permissions.get("memory") { | ||
| if let Some(limit) = memory.get("limit").and_then(|v| v.as_str()) { | ||
| lifecycle_manager | ||
| .grant_permission( | ||
| component_id, | ||
| "memory", | ||
| &json!({ | ||
| "resources": { | ||
| "limits": { | ||
| "memory": limit | ||
| } | ||
| } | ||
| }), | ||
| ) | ||
| .await?; | ||
| } |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The memory permission structure passed to grant_permission is incorrect. The code constructs a nested JSON structure with resources.limits.memory, but based on the grant_permission API usage in other parts of the codebase, it should match the policy document structure. Additionally, the permission type name should likely be "resources" or "memory" - the code should be verified against the actual grant_permission implementation to ensure the correct structure and permission type are used.
tests/cli_integration_test.rs
Outdated
| permissions: | ||
| network: | ||
| - host: "example.com" |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The policy file format in this test is incorrect and doesn't match the standard policy format used throughout the codebase. It should include an allow field and other required fields like version. The test policy should be:
version: "1.0"
permissions:
network:
allow:
- host: "example.com"This test will currently fail because the implementation expects a different (also incorrect) format. Both the test and implementation need to be fixed to use the standard PolicyDocument format.
| permissions: | |
| network: | |
| - host: "example.com" | |
| version: "1.0" | |
| permissions: | |
| network: | |
| allow: | |
| - host: "example.com" |
src/main.rs
Outdated
| // This is a simplified version - the full implementation would need to | ||
| // parse the policy structure and call the appropriate grant-* functions | ||
| tracing::info!("Applying policy from file to component {}", component_id); | ||
|
|
||
| // For now, we'll use the existing policy application logic | ||
| // The policy should contain permissions like storage, network, etc. |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is misleading and should be removed or updated. The comment says "This is a simplified version" and "For now, we'll use the existing policy application logic", but this is the actual implementation being added in this PR. Either this should be updated to accurately describe what the code does, or if this is truly temporary/incomplete, it should be marked with a TODO or FIXME.
| // This is a simplified version - the full implementation would need to | |
| // parse the policy structure and call the appropriate grant-* functions | |
| tracing::info!("Applying policy from file to component {}", component_id); | |
| // For now, we'll use the existing policy application logic | |
| // The policy should contain permissions like storage, network, etc. | |
| // Apply permissions from the policy file to the component. | |
| // The current implementation parses the "permissions" section and calls | |
| // the appropriate grant_permission functions for each supported type. | |
| // If additional permission types or more complex logic are needed, | |
| // extend this section accordingly. See documentation for details. | |
| tracing::info!("Applying policy from file to component {}", component_id); |
src/main.rs
Outdated
| // Find the first tool in the component | ||
| let tool_name = if let Some(arr) = schema["tools"].as_array() { | ||
| if let Some(first_tool) = arr.first() { | ||
| // Try nested structure first (live component schema) | ||
| let tool_info = if first_tool.get("properties").is_some() { | ||
| &first_tool["properties"]["result"] | ||
| } else { | ||
| // Fallback to flat structure (metadata schema) | ||
| first_tool | ||
| }; | ||
|
|
||
| tool_info["name"] | ||
| .as_str() | ||
| .ok_or_else(|| anyhow::anyhow!("Tool name not found in schema"))? | ||
| .to_string() | ||
| } else { | ||
| bail!("No tools found in component"); | ||
| } |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The tool selection logic always uses the first tool in the component, which may not be the desired behavior. This could be problematic for components that have multiple tools. Consider either:
- Allowing the user to specify which tool to call via a
--toolflag - Failing with a clear error message if the component has multiple tools
- Documenting this limitation clearly
The current implementation silently chooses the first tool without informing the user, which could lead to unexpected behavior.
| // Find the first tool in the component | |
| let tool_name = if let Some(arr) = schema["tools"].as_array() { | |
| if let Some(first_tool) = arr.first() { | |
| // Try nested structure first (live component schema) | |
| let tool_info = if first_tool.get("properties").is_some() { | |
| &first_tool["properties"]["result"] | |
| } else { | |
| // Fallback to flat structure (metadata schema) | |
| first_tool | |
| }; | |
| tool_info["name"] | |
| .as_str() | |
| .ok_or_else(|| anyhow::anyhow!("Tool name not found in schema"))? | |
| .to_string() | |
| } else { | |
| bail!("No tools found in component"); | |
| } | |
| // Find the tool in the component | |
| let tool_name = if let Some(arr) = schema["tools"].as_array() { | |
| if arr.is_empty() { | |
| bail!("No tools found in component"); | |
| } | |
| if arr.len() > 1 { | |
| // Collect tool names for error message | |
| let tool_names: Vec<String> = arr | |
| .iter() | |
| .map(|tool| { | |
| let tool_info = if tool.get("properties").is_some() { | |
| &tool["properties"]["result"] | |
| } else { | |
| tool | |
| }; | |
| tool_info["name"] | |
| .as_str() | |
| .unwrap_or("<unknown>") | |
| .to_string() | |
| }) | |
| .collect(); | |
| bail!( | |
| "Component '{}' has multiple tools: [{}]. Please specify which tool to call. (This CLI currently only supports components with a single tool.)", | |
| component_id, | |
| tool_names.join(", ") | |
| ); | |
| } | |
| // Only one tool present, proceed as before | |
| let first_tool = &arr[0]; | |
| let tool_info = if first_tool.get("properties").is_some() { | |
| &first_tool["properties"]["result"] | |
| } else { | |
| first_tool | |
| }; | |
| tool_info["name"] | |
| .as_str() | |
| .ok_or_else(|| anyhow::anyhow!("Tool name not found in schema"))? | |
| .to_string() |
|
@copilot address all the comments, fmt code and commit |
- Use policy::PolicyParser::parse_file() instead of manual YAML parsing - Fix policy format to use 'allow' lists as per standard PolicyDocument structure - Fix environment variable permission type consistency (environment-variable vs environment-variables) - Add better error handling for components with multiple tools - Update test policy file to include required 'version' field and 'allow' structure - Remove misleading comments about "simplified version" Addresses all review comments from copilot-pull-request-reviewer[bot] Co-authored-by: Mossaka <[email protected]>
All review comments addressed in commit 446cfe3:
Code formatted and all tests passing (30/30). Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Enhance
wassette inspect callsubcommand ✅Completed:
InspectCommandsenum withCallvariant to supportwassette inspect call <component-id>--arg KEY=VALflag support (can be specified multiple times)--policy-fileparameter for specifying a policy filehandle_inspect_callfunction to execute the component with provided argumentsImplementation Summary:
New Command Structure:
Key Features:
--argflags for passing parameters to componentsPolicyParserfrom policy crate for proper YAML policy parsinginspectbehavior preserved viainspect schemasubcommandCode Changes:
src/commands.rs: AddedInspectCommandsenum withSchemaandCallvariantssrc/main.rs: Implemented handlers usingPolicyParserfor policy file parsingtests/cli_integration_test.rs: Added 4 new tests + updated 2 existing testsPolicy File Format (Standard):
Testing:
cargo +nightly fmtManual Test Results:
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.