Skip to content

[patch] Rename the Sample project to Invoker.Demo - #48

Merged
matt-edmondson merged 2 commits into
mainfrom
claude/rename-sample-to-invoker-demo
Sep 22, 2026
Merged

matt-edmondson merged 2 commits into
mainfrom
claude/rename-sample-to-invoker-demo

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #38

What changed

ktsu.Sdk derives a project's assembly name and package ID from its solution-relative folder path, so the folder Sample built the demo as ktsu.Sample — a name no single repository should own. dotnet build on main confirms it: Sample -> Sample/bin/Debug/net10.0/ktsu.Sample.dll.

  • Sample/ → Invoker.Demo/, Sample.csproj → Invoker.Demo.csproj, and the solution entry now reads Invoker.Demo. The derived identity is ktsu.Invoker.Demo, matching the .Demo convention for non-shipping demonstration projects.
  • Sample.cs → Demo.cs, with namespace ktsu.Invoker.Sample → ktsu.Invoker.Demo and class Sample → Demo, so the source agrees with the folder it now lives in.
  • IsPackable=false is set explicitly rather than leaning on OutputType=Exe being what keeps the demo off nuget.org.
  • CLAUDE.md records the folder-path-is-an-identity-claim rule alongside the renamed project.

Nothing was published under ktsu.Sample, so this corrects a name rather than breaking a package.

New test

Invoker.Test/ProjectNamingTests.cs guards the convention that produced the bug, in three parts:

  • every project folder is Invoker or Invoker.<Something>
  • every .csproj is named for the folder that decides its identity
  • every solution entry names, and points at, a project file that exists

Verification

  • dotnet test Invoker.sln — 19/19 pass (16 pre-existing, 3 new), and the same in -c Release.
  • Reverted the rename in the working tree with the new test in place: EveryProjectFolderCarriesTheFamilyName and EverySolutionEntryMatchesItsProjectFile both fail, naming Sample. Restored, and they pass.
  • dotnet build now reports Invoker.Demo -> Invoker.Demo/bin/Debug/net10.0/ktsu.Invoker.Demo.dll.
  • dotnet pack -c Release produces only ktsu.Invoker.1.2.33.nupkg; the demo is not packed. (That pack run also surfaces pre-existing CP0016 ApiCompat errors on Invoker/Invoker.csproj, a project this PR does not touch.)

Part of the estate-wide naming audit tracked in ktsu-dev/Sdk#36.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HPfHJ1kKCSoR9tRHPxMVfF


Generated by Claude Code

ktsu.Sdk derives a project's assembly name and package ID from its
solution-relative folder path, so the folder `Sample` built the demo as
`ktsu.Sample` - a name no single repository should own. Rename the folder,
the project file, the solution entry, the source file and its namespace so
the identity becomes `ktsu.Invoker.Demo`, matching the `.Demo` convention
for non-shipping demonstration projects.

Set `IsPackable=false` explicitly rather than leaning on `OutputType=Exe`
keeping it off nuget.org.

Add ProjectNamingTests, which fails if a project folder, its `.csproj`
filename or its solution entry stops agreeing with the family name, so the
derived identity cannot drift back unnoticed.

Fixes #38

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HPfHJ1kKCSoR9tRHPxMVfF

matt-edmondson commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor Author

The github-advanced-security check (run 35577984009, "Code scanning AI findings on PR #48") failed on head 6151ba7, and the failure is not this PR's.

The job never analyzed any code. It failed while opening its model session:

Error creating PR review request: SessionModelError: You have exceeded your monthly quota
  ... "statusCode":402,"errorCode":"quota"
##[error]Process completed with exit code 1.

That is an HTTP 402 against the Copilot API for this account's monthly quota, raised before the agent read the diff, so no change to this branch can clear it — there is no fix to port. Clearing it needs a quota or billing change on the account, which is outside what a PR can do. The separate CodeQL Analyze (csharp) and Analyze (actions) checks are unaffected and run normally.

I tried to re-run the failed job to confirm it reproduces rather than being transient, but the API refuses it: 403 This workflow run cannot be retried — it is a GitHub-managed dynamic run (dynamic/agents/github-advanced-security), not a repository workflow, so I have no means to retry it. A 402 quota error is not transient in any case; it will stay red on this PR until the quota resets or is raised. I'd suggest judging this PR on the .NET Workflow matrix and CodeQL, which are the checks that actually exercise the change.


Generated by Claude Code

Comment thread Invoker.Test/ProjectNamingTests.cs Fixed
Comment thread Invoker.Test/ProjectNamingTests.cs Fixed
Comment thread Invoker.Test/ProjectNamingTests.cs Fixed
Path.Combine discards everything before a rooted argument, so a solution
entry holding an absolute path would have dropped the repository root and
tested a file outside it. Path.Join concatenates, which keeps the check
inside the repository and reports such an entry as an offender.

Addresses the CodeQL cs/path-combine findings on this PR.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HPfHJ1kKCSoR9tRHPxMVfF
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Copy link
Copy Markdown
Contributor Author

SonarCloud Code Analysis fails the quality gate on head 75447c4: 0.0% coverage on new code (required ≥ 80%). This one needs a decision from you — it cannot be fixed from inside this PR.

What Sonar actually counted. Querying the analysis for this PR, the new-code coverage debt is one file:

Component New lines to cover Uncovered
Invoker.Demo/Demo.cs 8 8
Invoker.Demo/AssemblyInfo.cs 0 0
Invoker.Test/ProjectNamingTests.cs 0 0

The new test file is correctly not counted — the scanner detects it as test code. The 8 lines are the demo's existing, never-tested body: renaming Sample/Sample.cs to Invoker.Demo/Demo.cs re-attributed them as new code, so a gate that never applied to them before now does. No behaviour changed, and the same gate would have fired on any PR that touched that file under its old name.

Why I did not just write a test to cover it. The obvious move is a smoke test calling the demo's entry point — Invoker.Demo already grants InternalsVisibleTo("ktsu.Invoker.Test"), so it would compile. It is not safe to do:

Task queuedTask = Task.Run(() => contextDependent.Invoker.Invoke(ContextDependent.DoWork));
contextDependent.Invoker.DoInvokes();

queuedTask is never awaited, and whether DoInvokes() drains it is a race. Invoke from a non-owner thread is InvokeAsync(func).Wait() with no timeout, and an owner-thread Invoke returns immediately without draining the queue — so when the race goes the other way, that invocation blocks forever. In the demo executable that is harmless, because the process exits. Inside the test host it parks a thread pool thread for the lifetime of the run. Trading a possible CI hang for a coverage number is a bad deal, so I left it alone.

What I suggest instead. .github/workflows/dotnet.yml in ktsu-dev/.github already provides the mechanism, and its comment says exactly this is what it is for — a file "that cannot be executed rather than one nobody has got round to testing ... is excluded per repository through the SONAR_COVERAGE_EXCLUSIONS_EXTRA variable, so this workflow stays identical everywhere". Setting that repository variable on ktsu-dev/Invoker to:

**/Invoker.Demo/**

excludes the demo from coverage while leaving it under every other Sonar rule. I cannot set a repository variable, so this is yours to make. Worth noting it is not specific to this PR: every rename in the ktsu-dev/Sdk#36 naming audit that moves a demo or sample project will trip the same gate, so a decision here probably wants to be the pattern for the rest.

Everything else on this PR is green: Test on ubuntu-latest, macos-latest and windows-latest, Discover Test Projects, Analyze & Release, CodeQL, Analyze (csharp) ×2 and Analyze (actions) all pass. The only other red is github-advanced-security, which is the Copilot quota failure described above and also not this PR's.


Generated by Claude Code

@matt-edmondson
matt-edmondson merged commit b8d807f into main Sep 22, 2026
10 of 12 checks passed
@matt-edmondson
matt-edmondson deleted the claude/rename-sample-to-invoker-demo branch September 22, 2026 00:38
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.

The Sample project claims the identity ktsu.Sample

2 participants