Skip to content

Comments

Add implicit conversions to CodeInterpreterToolContainer and fix McpToolCallApprovalPolicy#905

Open
Copilot wants to merge 7 commits intomainfrom
copilot/fix-codeinterpretertoolcontainer
Open

Add implicit conversions to CodeInterpreterToolContainer and fix McpToolCallApprovalPolicy#905
Copilot wants to merge 7 commits intomainfrom
copilot/fix-codeinterpretertoolcontainer

Conversation

Copy link
Contributor

Copilot AI commented Jan 22, 2026

Applies the same fixes to CodeInterpreterToolContainer that were made to McpToolCallApprovalPolicy in PR #901, and fixes the existing implicit conversion operators in McpToolCallApprovalPolicy to follow the guidance that implicit conversions should never throw.

Changes Made

CodeInterpreterToolContainer

  • Added [CodeGenVisibility(...)] attribute to generate the internal parameterless constructor
  • Removed the manual internal parameterless constructor
  • Added Argument.AssertNotNull checks in constructors
  • Added implicit conversion operators (stringCodeInterpreterToolContainer, CodeInterpreterToolContainerConfigurationCodeInterpreterToolContainer)
  • Implicit conversions return null when the argument is null instead of throwing

McpToolCallApprovalPolicy

  • Fixed CustomMcpToolCallApprovalPolicy implicit conversion to return null when the argument is null instead of throwing, following the guidance that implicit conversions should never throw
  • GlobalMcpToolCallApprovalPolicy implicit conversion remains unchanged (the constructor doesn't throw, so no null check is needed)

Other Updates

  • Updated API files using scripts/Export-Api.ps1 (includes new OpenAI.net10.0.cs)
  • Regenerated files using scripts/Invoke-CodeGen.ps1
  • Updated examples and tests to use implicit conversions where applicable
Original prompt

This PR made a few fixes to McpToolCallApprovalPolicy, which is a union of a CustomMcpToolCallApprovalPolicy and a GlobalMcpToolCallApprovalPolicy: #901

Could you make the same fixes to CodeInterpreterToolContainer, which is a union of a CodeInterpreterToolContainerConfiguration and a string container ID?


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: joseharriaga <45773732+joseharriaga@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix issues in CodeInterpreterToolContainer Add implicit conversions to CodeInterpreterToolContainer union Jan 22, 2026
Copilot AI requested a review from joseharriaga January 22, 2026 05:08
@joseharriaga
Copy link
Collaborator

@copilot Can you sync to the latest and run the scripts/Export-Api.ps1 script to update the new api/OpenAI.net10.0.cs API listing that is missing as part of this change? Note that the files under the "api" folder are updated using this script and should not be edited manually.

Copilot AI and others added 2 commits January 29, 2026 00:59
Copy link
Contributor Author

Copilot AI commented Jan 29, 2026

@copilot Can you sync to the latest and run the scripts/Export-Api.ps1 script to update the new api/OpenAI.net10.0.cs API listing that is missing as part of this change? Note that the files under ...

Done. Synced with main and ran scripts/Export-Api.ps1 to regenerate the API listings, which now includes the new api/OpenAI.net10.0.cs file with the implicit conversion operators. (5cfdecc)

@joseharriaga
Copy link
Collaborator

@copilot I was looking at your work, and I didn't notice you running the scripts/Invoke-CodeGen.ps1 script. I suspect that you modified the files under the "src/Generated" folder manually. These files are actually generated by our code generator, which can be invoked using the aforementioned script. Could you do the following?

  1. Revert the changes to the files under the "src/Generated" directory.
  2. Run the scripts/Invoke-CodeGen.ps1 script and let them be re-generated.

I believe you actually got it right even though you modified these files manually, so you might not see any diffs in the end, but it's good to do it the correct way to be safe.

Copy link
Contributor Author

Copilot AI commented Jan 29, 2026

@copilot I was looking at your work, and I didn't notice you running the scripts/Invoke-CodeGen.ps1 script. I suspect that you modified the files under the "src/Generated" folder manually. These fil...

I reverted the changes to src/Generated/Models/Responses/CodeInterpreterToolContainer.cs and ran scripts/Invoke-CodeGen.ps1. The codegen regenerated the file with the same internal parameterless constructor, confirming the [CodeGenVisibility] attribute in the Custom file is working correctly. No diff was produced since the regenerated file matches what was already committed.

@joseharriaga
Copy link
Collaborator

@copilot This is just a question: What can I do in the future to effectively provide you the necessary context for you to correctly use the scripts/Invoke-CodeGen.ps1 and scripts/Export-Api.ps1 scripts when necessary?

Copy link
Contributor Author

Copilot AI commented Jan 29, 2026

@copilot This is just a question: What can I do in the future to effectively provide you the necessary context for you to correctly use the scripts/Invoke-CodeGen.ps1 and scripts/Export-Api.ps1 sc...

A few suggestions that would help:

  1. Add a CONTRIBUTING.md or DEVELOPMENT.md with a section on code generation workflows, explaining that files in src/Generated/ are auto-generated via scripts/Invoke-CodeGen.ps1 and files in api/ are generated via scripts/Export-Api.ps1.

  2. Add header comments in the generated files mentioning the script to run for regeneration (e.g., // Generated by scripts/Invoke-CodeGen.ps1 - DO NOT EDIT MANUALLY).

  3. Include instructions in the issue/PR template reminding contributors to run the appropriate scripts when modifying Custom files that affect Generated output.

The key insight I was missing was understanding the relationship between the src/Custom/ files (which I can edit) and the src/Generated/ files (which should be regenerated). Making this relationship explicit in documentation would help future contributions.

@joseharriaga joseharriaga marked this pull request as ready for review February 3, 2026 01:30
Co-authored-by: joseharriaga <45773732+joseharriaga@users.noreply.github.com>
…tead of throwing

Co-authored-by: joseharriaga <45773732+joseharriaga@users.noreply.github.com>
Copilot AI changed the title Add implicit conversions to CodeInterpreterToolContainer union Add implicit conversions to CodeInterpreterToolContainer and fix McpToolCallApprovalPolicy Feb 5, 2026
…plicit conversion

Co-authored-by: joseharriaga <45773732+joseharriaga@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants