Skip to content

[dotnet-port-api] Port shelltool metadata overrides - #997

Merged
Quim Muntal (qmuntal) merged 2 commits into
mainfrom
dotnet-port-api-shelltool-metadata-30980459955-a66e5eb7ab29be28
Sep 7, 2026
Merged

[dotnet-port-api] Port shelltool metadata overrides#997
Quim Muntal (qmuntal) merged 2 commits into
mainfrom
dotnet-port-api-shelltool-metadata-30980459955-a66e5eb7ab29be28

Conversation

@michelle-clayton-work

Copy link
Copy Markdown
Contributor

Summary

Port shell-tool metadata overrides from the upstream .NET harness work by letting shelltool.LocalConfig override the model-facing tool name and description. Go previously hard-coded run_shell and always synthesized the default description; now callers can keep the defaults or set custom metadata explicitly. The shelltool parity row in the feature-comparison doc was updated to reflect the aligned surface.

Ported .NET PRs

Breaking Changes

No. Existing callers keep the same default tool name (run_shell) and generated description unless they opt into the new LocalConfig fields.

Tests and Examples

  • go test ./tool/shelltool
  • Added focused shelltool coverage for custom name/description overrides
  • No example changes were needed for this narrow config-only parity addition

Notes

  • The broader file-access approval options shipped in the same upstream PR were intentionally skipped here because they are a separate public-surface change and would make this nightly port less narrow.
  • Upstream inspection was based on upstream-agent-framework/main at 1da571860a60f0da4f060bd60c8e7ee8d092cd32; the remote refresh step itself was blocked in this environment, so selection used the locally available upstream reference plus GitHub read-only source inspection.

Generated by .NET to Go API Porting Agent · gpt54 · 153.5 AIC · ⌖ 11.3 AIC · ⊞ 24.1K ·

Closes #788

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 4, 2026 22:28
@github-actions github-actions Bot added the area:tool Changes files in the tool area label Sep 4, 2026
@github-actions github-actions Bot added the size:medium At most 100 changed lines across at most 5 files label Sep 4, 2026

Copilot AI left a comment

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.

🟡 Changes recommended

LocalConfig.toolName() checks TrimSpace but returns the untrimmed name, which can silently introduce invalid/undesired tool identifiers with leading/trailing whitespace.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Ports the upstream .NET shell tool “metadata override” capability into the Go shelltool package, allowing callers to override the model-facing tool name and description via shelltool.LocalConfig, and updates docs/tests accordingly.

Changes:

  • Add LocalConfig.Name / LocalConfig.Description and route Local.Name() / Local.Description() through these overrides (with defaults preserved).
  • Add a focused unit test covering custom name/description overrides.
  • Update the .NET vs Go feature-comparison documentation to reflect configurable shelltool metadata.
File summaries
File Description
docs/dotnet-go-sdk-feature-comparison.md Documents shelltool parity including configurable tool name/description.
tool/shelltool/localshell.go Adds config fields and uses them to provide overridable tool Name()/Description().
tool/shelltool/shelltool_test.go Adds coverage ensuring custom name/description overrides are surfaced.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tool/shelltool/localshell.go
@github-actions github-actions Bot added kind:code Changes production behavior or code kind:docs Changes documentation or comments kind:tests Changes tests, fixtures, or test infrastructure pending-auto-risk Automatic risk classification is in progress risk:medium Contained production impact requiring normal review depth and removed pending-auto-risk Automatic risk classification is in progress labels Sep 4, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Scope: public API

Changed Go contract: shelltool.LocalConfig gains new exported fields Name and Description; Local.Name()/Local.Description() now consult these overrides via new unexported helpers toolName()/toolDescription(), falling back to prior defaults (run_shell and the generated description) when unset. Feature-comparison doc updated to reflect this.

Upstream evidence reviewed: microsoft/agent-framework#6880 (HarnessAgentOptions.ShellToolName, ShellToolDescription, ShellToolRequireApproval, forwarded to ShellExecutor.AsAIFunction(name, description, requireApproval)). Go's shelltool.LocalConfig.AcknowledgeUnsafe (existing field, Local.ApprovalRequired() bool { return !t.exec.opts.AcknowledgeUnsafe }) already covers the approval-toggle half of the upstream change, so this PR correctly scopes itself to the name/description half only, per its own PR description noting the file-access approval-opt-out portion of #6880 was intentionally skipped as a separate surface.

Result: aligned. Both new fields are optional, default to empty string, and preserve existing default behavior (run_shell name, generated description) exactly as upstream's null-fallback semantics do for ShellToolName. Naming (Name/Description on an options struct vs. ShellToolName/ShellToolDescription on HarnessAgentOptions) is an idiomatic, non-semantic difference given Go's shell tool is constructed directly via LocalConfig rather than through a harness-agent options object. Test coverage added (TestNewLocal_customNameAndDescription) matches the behavior change. No divergence found.

This PR adds new exported fields (LocalConfig.Name, LocalConfig.Description), so public-api-change is applied. No behavior gaps identified, so parity-approved is also applied.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • api.github.com

[!TIP]
api.github.com is blocked because GitHub API access uses the built-in GitHub tools by default. Instead of adding api.github.com to network.allowed, use tools.github.mode: gh-proxy for direct pre-authenticated GitHub CLI access without requiring network access to api.github.com:

tools:
  github:
    mode: gh-proxy

See GitHub Tools for more information on gh-proxy mode.

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "api.github.com"

See Network Configuration for more information.

Generated by Go API Consistency Review Agent for #997 · copilot · auto · 22.8 AIC · ⌖ 7.59 AIC · ⊞ 9.5K ·

@github-actions github-actions Bot added parity-approved Go API consistency review found no parity issues public-api-change Pull Request changes public APIs pending-auto-risk Automatic risk classification is in progress risk:medium Contained production impact requiring normal review depth and removed risk:medium Contained production impact requiring normal review depth pending-auto-risk Automatic risk classification is in progress labels Sep 5, 2026
@qmuntal
Quim Muntal (qmuntal) added this pull request to the merge queue Sep 7, 2026
Merged via the queue into main with commit 6a3640e Sep 7, 2026
38 checks passed
@qmuntal
Quim Muntal (qmuntal) deleted the dotnet-port-api-shelltool-metadata-30980459955-a66e5eb7ab29be28 branch September 7, 2026 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:tool Changes files in the tool area kind:code Changes production behavior or code kind:docs Changes documentation or comments kind:tests Changes tests, fixtures, or test infrastructure parity-approved Go API consistency review found no parity issues public-api-change Pull Request changes public APIs risk:medium Contained production impact requiring normal review depth size:medium At most 100 changed lines across at most 5 files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[dotnet-port-api] Port shelltool metadata overrides

3 participants