Skip to content

Fix the .NET manual tool resume sample build - #2649

Open
rinceyuan wants to merge 2 commits into
github:mainfrom
rinceyuan:fix/dotnet-sample-experimental-diagnostic
Open

rinceyuan wants to merge 2 commits into
github:mainfrom
rinceyuan:fix/dotnet-sample-experimental-diagnostic

Conversation

@rinceyuan

Copy link
Copy Markdown
Contributor

dotnet/samples/ManualToolResume.cs is a file-based app (it opens with #:project ../src/GitHub.Copilot.SDK.csproj), and it does not build.

$ cd dotnet/samples && dotnet build ManualToolResume.cs
ManualToolResume.cs(41,9): error GHCP001: 'GitHub.Copilot.Rpc.PermissionDecisionApproveOnce'
is for evaluation purposes only and is subject to change or removal in future updates.
Suppress this diagnostic to proceed.

The sample calls HandlePendingPermissionRequestAsync with new PermissionDecisionApproveOnce(), and that type is marked experimental in the generated surface:

// dotnet/src/Generated/Rpc.cs
[Experimental(Diagnostics.Experimental)]
public partial class PermissionDecisionApproveOnce : PermissionDecision

ExperimentalAttribute reports as an error unless suppressed, so the sample cannot be built or run as shipped.

The change

Suppress the diagnostic around that one call, matching how the rest of the repository already handles it:

  • dotnet/src/Client.cs wraps experimental usages in #pragma warning disable GHCP001 / restore in several places.
  • docs/features/usage-and-billing.md tells readers to use exactly #pragma warning disable GHCP001 or a project-level <NoWarn>GHCP001</NoWarn>.
  • docs/setup/in-process-runtime.md lists "Allow the GHCP001 experimental API diagnostic" as the .NET requirement.

I used the bare pragma with no explanatory comment, since that is the form used both in Client.cs and in the docs snippets.

Verification

Both samples build after the change:

$ cd dotnet/samples
$ dotnet build ManualToolResume.cs      # exit 0
$ dotnet build Chat.cs                  # exit 0

Chat.cs already built before this change; only ManualToolResume.cs was affected. The build produces no warnings, and the diff is two added lines with no other edits.

Context

Nothing builds the samples in CI, which is why this went unnoticed — the same gap behind #2648 (the Go samples module). I have not added CI coverage here either, since where that belongs seemed like your call.

For completeness, I checked the other two sample sets while I was at it and they are fine: the Node samples type-check against nodejs/src, and every symbol the Python samples import resolves against python/copilot.

PermissionDecisionApproveOnce is marked [Experimental], so GHCP001 is reported as an error and 'dotnet build ManualToolResume.cs' fails. Suppress it around the call the way dotnet/src/Client.cs and the docs already do.
Copilot AI balanced review requested due to automatic review settings September 14, 2026 03:58
@rinceyuan
rinceyuan requested a review from a team as a code owner September 14, 2026 03:58

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

The stable PermissionDecision.ApproveOnce() factory avoids the unnecessary experimental diagnostic suppression.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.

Comment on lines +39 to +43
#pragma warning disable GHCP001
await session2.Rpc.Permissions.HandlePendingPermissionRequestAsync(
permissionEvent.Data.RequestId,
new PermissionDecisionApproveOnce());
#pragma warning restore GHCP001
Comment on lines +39 to +43
#pragma warning disable GHCP001
await session2.Rpc.Permissions.HandlePendingPermissionRequestAsync(
permissionEvent.Data.RequestId,
new PermissionDecisionApproveOnce());
#pragma warning restore GHCP001
Matches PermissionHandlers.cs and the documented discoverability entry point. The suppression stays because the PermissionDecision base class is itself [Experimental].
@rinceyuan

Copy link
Copy Markdown
Contributor Author

Thanks — I took the factory half of this and verified the rest, which did not hold up.

Adopted: the sample now calls PermissionDecision.ApproveOnce() instead of new PermissionDecisionApproveOnce(). That matches dotnet/src/PermissionHandlers.cs and the factory's own doc comment ("Use these for quick discoverability via PermissionDecision.<dot>"), so it is better sample style regardless.

Kept: the suppression, because removing it does not build. The diagnostic simply moves from the derived type to the base type:

$ dotnet build ManualToolResume.cs      # factory, no pragma
ManualToolResume.cs(41,5): error GHCP001: 'GitHub.Copilot.Rpc.PermissionDecision'
is for evaluation purposes only and is subject to change or removal in future updates.

The base class carries the attribute, so every member of the hierarchy is affected:

// dotnet/src/Generated/Rpc.cs
[Experimental(Diagnostics.Experimental)]
[JsonPolymorphic(...)]
public partial class PermissionDecision

PermissionDecision.ApproveOnce() is declared in the hand-written partial in dotnet/src/PermissionDecision.cs and carries no attribute of its own, but its return type is the experimental base, so the call site still needs the suppression.

Current state builds clean:

$ dotnet build ManualToolResume.cs      # factory + pragma
exit 0, no errors or warnings

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.

2 participants