From b248c60b156a7dd1ea50d4b1950ddc0bd0e2b09f Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Mon, 21 Sep 2026 15:42:17 +1000 Subject: [PATCH 01/16] docs: design worktree verbs, GitHub owner kinds, and device flow Three additions a consuming desktop launcher needs and this library does not have: worktree list/add/remove/prune, repository enumeration that can see an organisation's private repositories, and a way to obtain a GitHub credential rather than only resolve one. All additive, targeting 3.2.0. Also ignores .worktrees/, matching the convention used for isolated feature work. Co-Authored-By: Claude Opus 5 (1M context) --- .gitignore | 3 + ...-09-21-worktrees-and-github-auth-design.md | 418 ++++++++++++++++++ 2 files changed, 421 insertions(+) create mode 100644 docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md diff --git a/.gitignore b/.gitignore index dc0470a..3b62745 100644 --- a/.gitignore +++ b/.gitignore @@ -651,3 +651,6 @@ Temporary Items # ImGui.ini files imgui.ini + +# Git worktrees used for isolated feature work +.worktrees/ diff --git a/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md b/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md new file mode 100644 index 0000000..a8ee9ac --- /dev/null +++ b/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md @@ -0,0 +1,418 @@ +# Worktree verbs, GitHub owner kinds, and device flow + +Status: approved design, not yet implemented. + +## Why this phase exists + +A desktop launcher is being built against this library. It shows a tree of every repository its user +can reach across Azure DevOps and GitHub, and drives the ordinary git operations from that tree. Its +branch model is one worktree per branch rather than one checkout that switches. + +Three things it needs do not exist here: + +1. **Worktrees.** The library has no worktree verb at all. `GitProbes.IsWorkTreeAsync` asks whether a + path is a working tree, and `GitStatusEntry.WorkTreeState` names one half of a status code. Neither + creates, lists, or removes anything. +2. **GitHub repositories it can actually see.** `GitHubProvider.GetRepositoriesAsync` calls + `GET /users/{login}/repos`, which returns public repositories only. The launcher's repositories are + private and live under an organisation behind SAML single sign-on, so today it would enumerate an + empty list and be right to. +3. **A way to sign in.** Credentials resolve from `ktsu.CredentialCache` or from a `CredentialSource` + callback. Both assume a credential already exists. Nothing in the library obtains one, and a + desktop application cannot ask its user to run `gh auth login` first. + +Everything here is additive. No existing member changes shape, and no existing behaviour changes for +a caller that does not opt in. + +## Versioning + +**`[minor]` — 3.2.0.** New types and new members only. The one existing method whose behaviour is +touched, `GitHubProvider.GetRepositoriesAsync`, keeps its current route and its current documented +coverage under the new property's default. + +## Scope + +In scope: + +- `worktree` list, add, remove and prune. +- An owner-kind selector on `GitHubProvider` routing repository enumeration to the endpoint that can + answer for that kind of owner. +- The single sign-on authorisation URL surfaced on the failure that demands it. +- GitHub's OAuth device flow, as a type that obtains a credential rather than one that stores it. + +Out of scope, deliberately: `worktree move`, `worktree lock` and `worktree unlock` (a launcher UI +exercises none of them, and `lock` protects against a failure mode — removable storage going away — +that this caller does not have); device flow for Azure DevOps (it authenticates through Entra ID, +which the consuming application already does for itself, and `CredentialSource` is the seam that was +built for exactly that); and token refresh (a GitHub OAuth App's device-flow token does not expire, +unlike a GitHub App's). + +## Worktree verbs + +### The model + +```csharp +public sealed record GitWorktree +{ + public required AbsoluteDirectoryPath Path { get; init; } + public GitCommitSha? Head { get; init; } + public GitBranchName? Branch { get; init; } + public bool IsMain { get; init; } + public bool IsBare { get; init; } + public bool IsDetached { get; init; } + public bool IsLocked { get; init; } + public string? LockReason { get; init; } + public bool IsPrunable { get; init; } + public string? PrunableReason { get; init; } +} +``` + +Inert data, like `GitBranch` and `GitSubmodule`. It carries no `GitRepository` and no process runner. +A caller that wants to operate inside a worktree passes its `Path` to `IGitClient.OpenAsync`, which is +the same journey a caller makes from any other path. Handing out a live `GitRepository` from a model +would make this the only model in the library that can execute anything, and would force a decision +about which runner it inherited. + +`Head` and `Branch` are nullable because git's own output omits them. A bare entry reports neither. A +detached entry reports `HEAD` and no `branch`. Making either required would mean inventing a value for +a record git declined to describe. + +`LockReason` and `PrunableReason` are separately nullable from their flags, because `locked` and +`prunable` each appear both bare and with a reason, and "locked for a reason nobody recorded" is a +different fact from "not locked". + +`IsMain` is **positional**. Git's porcelain has no attribute for it: the main working tree is simply +the first record, always. The parser sets it on the first record and on no other. This is recorded +here because it is the one field in the record not read from a named attribute, and because a future +`--porcelain` change that reorders records would break it silently. It exists because a caller +managing worktrees needs to refuse to remove the one that owns the repository, and answering that from +positional knowledge inside the parser is better than every caller reimplementing it. + +### Listing + +```csharp +public interface IGitWorktreeListBuilder : IGitCommandBuilder>; +``` + +No options. `git worktree list --porcelain` takes none this caller wants, and a builder with no +configuration still earns its place by matching every other read verb's shape rather than being the +one verb that is a bare method. + +The porcelain format is blank-line-separated records of `attribute [value]` lines: + +``` +worktree /home/u/project +HEAD 7f3c9a1... +branch refs/heads/main + +worktree /home/u/project-feature +HEAD 2b8e4d0... +branch refs/heads/feature +locked contains uncommitted experiment + +worktree /home/u/project-review +HEAD 9c1a7f2... +detached +prunable gitdir file points to non-existent location +``` + +`branch` arrives fully qualified and is stripped to a bare `refs/heads/` name, the same normalisation +`AzureDevOpsProvider.StripRefsHeadsPrefix` already applies on the hosting side. A caller should not have +to know which half of this library produced a branch name to know its shape. + +### Adding + +```csharp +public interface IGitWorktreeAddBuilder : IGitCommandBuilder +{ + IGitWorktreeAddBuilder CheckingOut(GitBranchName branch); + IGitWorktreeAddBuilder CreatingBranch(GitBranchName branch); + IGitWorktreeAddBuilder CreatingOrResettingBranch(GitBranchName branch); + IGitWorktreeAddBuilder Detached(); + IGitWorktreeAddBuilder From(GitRefName commitish); + IGitWorktreeAddBuilder Force(); + IGitWorktreeAddBuilder WithoutCheckout(); +} +``` + +`CheckingOut`, `CreatingBranch`, `CreatingOrResettingBranch` and `Detached` are four settings of one +mode field, and a later call replaces an earlier one. This follows `IGitBranchListBuilder`'s +`LocalOnly` and `RemoteOnly`, which already document themselves as replacing any previous selection. +Throwing on a second call would be defensible in isolation but would make this the only builder in the +library where ordering is an error rather than a resolution. + +Git's syntax is `git worktree add [-f] [--detach] [-b ] []`, so the mode +settings divide across two places. `CreatingBranch` emits `-b`, `CreatingOrResettingBranch` emits `-B`, +and `Detached` emits `--detach`, all options. `CheckingOut` emits nothing and instead writes the +commit-ish operand. + +`From` writes that **same** operand: the start point for the two branch-creating modes, and the commit +to detach at for `Detached`. So `CheckingOut` and `From` are one field under two names, and the later +call wins, by the same rule that governs the mode field. The two are typed differently because that is +the useful distinction — `CheckingOut` takes a `GitBranchName` and reads as the ordinary case, `From` +takes a `GitRefName` and admits a tag or a raw revision. Neither is a separate slot, and a builder that +called both would be saying the same thing twice. + +The path operand is the builder's constructor argument, so it is never absent. Both it and the +commit-ish are passed after `--end-of-options`, as every caller-supplied operand in this library is. + +**`--guess-remote` is deliberately omitted.** The launcher's case is a worktree for a branch that +exists on the remote but not locally, and plain `git worktree add ` already handles it: +git creates a local tracking branch when the name matches exactly one remote. `--guess-remote` extends +that to the case where the caller names no branch at all, which no caller here does. + +### Removing and pruning + +```csharp +public interface IGitWorktreeRemoveBuilder : IGitCommandBuilder +{ + IGitWorktreeRemoveBuilder Force(); +} + +public interface IGitWorktreePruneBuilder : IGitCommandBuilder; +``` + +`git worktree remove` refuses a worktree with modified or untracked files, and `Force()` is how a +caller says it knows. `PruneWorktrees()` clears administrative entries whose directory has gone, +which is the state a launcher reaches whenever a user deletes a worktree folder in a file manager. + +`--dry-run` on prune is omitted: it would change the result type from `GitCompleted` to a list of what +would have been removed, and a caller that wants to know can call `Worktrees()` and read `IsPrunable` +from records it already has. + +### Why no new exception type + +Under one worktree per branch, `git worktree add` failing because the branch is already checked out +somewhere is not an edge case — it is what happens every time a user asks for a branch they already +have. That frequency argues for a typed exception the way `GitPushRejectedException` and +`GitNothingToCommitException` earned theirs. + +It does not get one. Those two exceptions exist because the caller cannot know the outcome in advance: +whether a push will be rejected depends on the remote's state at the moment of the push. Whether a +branch already has a worktree is answerable from `Worktrees()`, which a caller managing worktrees has +necessarily already called. The failure is reachable only by racing yourself, and `TryExecuteAsync` +returns a non-throwing `GitCommandError` for that. Adding the exception would mean matching git's +English prose to tell a caller something it could have read structurally. + +## GitHub owner kinds + +### The property + +```csharp +public enum GitHubOwnerKind +{ + User, + Organization, + AuthenticatedUser, +} +``` + +```csharp +public GitHubOwnerKind OwnerKind { get; init; } = GitHubOwnerKind.User; +``` + +`User` is the default so that a caller who says nothing gets exactly today's route, today's coverage, +and today's documented contract. This phase adds a way to ask for more, and changes nothing for anyone +who does not. + +### Routing + +| `OwnerKind` | Octokit call | Endpoint | Sees private | +|---|---|---|---| +| `User` | `Repository.GetAllForUser(Owner)` | `GET /users/{login}/repos` | no | +| `Organization` | `Repository.GetAllForOrg(Owner)` | `GET /orgs/{org}/repos` | yes, where the token can | +| `AuthenticatedUser` | `Repository.GetAllForCurrent(...)` | `GET /user/repos` | yes, where the token can | + +`AuthenticatedUser` requests `affiliation=owner,organization_member` and then **filters the result to +`Owner`** client-side, comparing each repository's owner login to `Owner` case-insensitively. GitHub +treats logins as case-insensitive, so an ordinal comparison would drop a caller's repositories over a +capital letter the caller did not choose. + +The filter is what keeps `Owner` meaningful. `GET /user/repos` describes the token's own reachable +repositories and takes no owner parameter, so routing to it unfiltered would silently ignore a +configured `Owner` — which is the precise objection recorded in `GetRepositoriesAsync`'s current +remarks against switching to that endpoint wholesale. Filtering answers it: the endpoint widens what +can be seen, and the filter preserves the invariant that this method describes `Owner`'s repositories +and nobody else's. + +Filtering rather than validating the token's login against `Owner` is a deliberate choice between two +ways of honouring it. Validation costs an extra `GET /user` on every enumeration and rejects the +legitimate case of an organisation the token is a member of. The filter costs nothing and handles both. + +### The single sign-on failure + +A token that is valid but not authorised for an organisation's SAML single sign-on receives `403` with +an `X-GitHub-SSO` header whose value carries the URL the user must visit to authorise it. + +`Translate` already routes `403` without rate-limit headers to `GitHostingAuthenticationException`, +which is the correct bucket and does not change. What changes is the message: when that header is +present, its URL is included. + +Without it, the two failures a caller most needs to tell apart — a bad token, and a good token one +click away from working — are the same exception with the same text, and the fix is a URL the user +cannot see. The header is read case-insensitively by scanning, matching how `TryGetRetryAfterSeconds` +already reads `Retry-After`, and for the same reason. + +## Device flow + +### Shape + +```csharp +public sealed record GitHubDeviceCode +{ + public required string UserCode { get; init; } + public required Uri VerificationUri { get; init; } + public required TimeSpan ExpiresIn { get; init; } + public required TimeSpan Interval { get; init; } +} + +public sealed class GitHubDeviceFlow(GitHubOAuthClientId clientId, IReadOnlyList scopes) +{ + public Task RequestDeviceCodeAsync(CancellationToken cancellationToken = default); + public Task WaitForTokenAsync(GitHubDeviceCode code, CancellationToken cancellationToken = default); +} +``` + +### Why two calls rather than one + +Device flow has an inherent pause in the middle: GitHub issues a short user code, the user types it +into a browser, and only then does polling succeed. That pause is minutes long and the code must stay +on screen throughout. + +A single `AuthenticateAsync(onCodeIssued)` would invoke its callback from whatever thread the HTTP +continuation resumed on, leaving every graphical caller to marshal the code back to a UI thread from +inside a callback it does not control. Two calls invert it: the caller awaits the first, renders the +code however it likes, and awaits the second wherever it wants the wait to happen. The seam lands where +the pause already is. + +`Interval` and `ExpiresIn` are surfaced rather than kept private because a caller showing a countdown +or a "still waiting" hint needs both, and neither is derivable. + +### Why it is not on GitHubProvider + +Every other method on `GitHubProvider` is one request and one response. This is a two-stage exchange +that blocks for minutes and is driven by a human in a browser. Putting it there would give the type two +unrelated lifecycles and would mean a provider instance existed before it had anything to authenticate +with. + +Keeping it separate also keeps the dependency direction clean: the flow produces a `HostingCredential`, +and `GitProvider` already consumes one through `CredentialSource` and the credential cache. Nothing new +connects them. + +### Why it does not store anything + +`WaitForTokenAsync` returns a `HostingCredential` and writes nothing. The caller persists it: + +```csharp +HostingCredential credential = await flow.WaitForTokenAsync(code, cancellationToken); +CredentialCache.Instance.AddOrReplace(persona, new CredentialWithToken { Token = credential.Token! }); +``` + +Two lines at the call site, in exchange for a type that can be tested without a keyring and that does +not decide on its caller's behalf where a secret lives. A caller with its own secret storage is not +forced through `ktsu.CredentialCache` to use device flow. + +The credential is built with `HostingCredential.FromToken`, not `FromBearerToken`. A GitHub OAuth token +travels under Octokit's `Token` scheme, which is what `FromToken` documents itself as meaning. +`FromBearerToken` exists for Entra ID access tokens against Azure DevOps. + +### Transport and polling + +Both calls go through Octokit's `OauthClient` — `InitiateDeviceFlow` and +`CreateAccessTokenForDeviceFlow` — for the same reason `GitHubProvider` uses Octokit: one client +library, one set of failure shapes to translate. `CreateAccessTokenForDeviceFlow` handles +`authorization_pending` and `slow_down` polling internally. + +**This rests on Octokit 14.0.0's actual surface, which is to be verified in the first implementation +step rather than assumed.** If either method is absent or does not poll, the fallback is two +`HttpClient` posts to `https://github.com/login/device/code` and +`https://github.com/login/oauth/access_token` with `Accept: application/json`, polling at `Interval` +and widening by five seconds on each `slow_down`. That fallback is a smaller amount of code than the +translation layer it would replace, so the risk here is low either way. + +### Failures + +No new exception family. Everything lands in the hosting hierarchy that already exists: + +| Condition | Exception | +|---|---| +| `access_denied` (user refused) | `GitHostingAuthenticationException` | +| `expired_token` (code timed out) | `GitHostingAuthenticationException` | +| `incorrect_client_credentials`, `unsupported_grant_type` | `GitHostingRequestException` | +| transport failure, unparsable body | `GitHostingRequestException` | + +Denial and expiry share an exception and are distinguished by message. They are the same fact to a +caller — no credential was obtained, offer to start again — and splitting them would add a type nobody +switches on. + +### The client identifier + +`GitHubOAuthClientId`, a new semantic string in `SemanticTypes/GitProviderTypes.cs`, supplied by the +caller. The library embeds no identifier and ships no default. + +An OAuth App's client ID is not a secret: device flow has no client secret precisely because a desktop +binary cannot keep one. So a consuming application can hold it in ordinary configuration. It is a +parameter here rather than a constant because the identifier belongs to whoever registered the app, and +this library is not that party. + +### The external prerequisite + +**Nothing in this section can be exercised against GitHub until an OAuth App exists.** Someone with +organisation ownership must register one with device flow enabled and approve it for SAML single +sign-on. Scopes: `repo` and `read:org`. + +Until then the implementation is written and tested entirely against a fake transport, which is how the +rest of the hosting layer is tested anyway. No implementation step is blocked. Only a live end-to-end +confirmation is, and that is recorded as an unchecked item rather than a passing test. + +## What this does not do for the launcher + +Recorded so the next design does not assume otherwise. + +**GitHub sign-in is optional in the consuming launcher.** A user who never signs in must still get a +working tree of their Azure DevOps repositories. This needs no change here: +`GitProvider.IsAuthenticated` already reports whether a request would carry a credential, and already +distinguishes that from whether a credential store holds an entry. A caller checks it and skips +enumeration rather than calling and handling a `403`. + +**A worktree layout convention is the caller's.** `AddWorktree` takes any absolute path. The launcher +will enforce one worktree per branch at derived, predictable paths; that policy lives there, and this +library neither imposes nor validates it. + +## Testing + +Three existing patterns in `GitIntegration.Test`, no new ones. + +**Parser tests** over fixture strings, as `GitStatusParserTests` does. The cases that matter are the +ones where porcelain omits fields: a bare entry with no `HEAD` and no `branch`; a detached entry with +`HEAD` and no `branch`; `locked` and `prunable` each with and without a reason; a single-record listing +where the only worktree is also the main one; a multi-record listing asserting `IsMain` on the first and +nowhere else; and trailing blank lines, which `--porcelain` emits and a naive record split turns into a +phantom entry. + +**Builder tests** asserting the exact argument vector through a fake `IGitProcessRunner`. One per option, +plus three proving the mode field resolves to the last call rather than accumulating `-b` alongside +`--detach`. + +**Provider tests** injecting a fake `HttpMessageHandler` through the internal `Handler` seam. One per +`GitHubOwnerKind` asserting the route actually requested; one proving `AuthenticatedUser` drops +repositories belonging to another owner; one asserting a `403` carrying `X-GitHub-SSO` produces a +`GitHostingAuthenticationException` whose message contains the header's URL; and one asserting a `403` +without that header still produces the same exception type, so the addition does not become a +requirement. + +**Device flow tests** against a fake transport: a poll that returns `authorization_pending` before +succeeding, a `slow_down` that widens the interval, an expiry, and a denial. + +## Implementation order + +1. Verify Octokit 14.0.0's `OauthClient` device-flow surface. It is the only unknown, and it decides + whether the device flow section is a translation layer or two HTTP posts. +2. Worktree parser and model. Independent of everything else, and the largest body of parsing. +3. Worktree builders and the four `GitRepository` factories. +4. `GitHubOwnerKind` and the enumeration routing. +5. The `X-GitHub-SSO` message. +6. `GitHubOAuthClientId`, `GitHubDeviceCode`, and `GitHubDeviceFlow`. +7. README and CHANGELOG. + +Steps 2 and 3 have no dependency on 4 through 6 and could be built in either order, or concurrently. From 6952633c89935222f7c93ddd389833882706bca8 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Mon, 21 Sep 2026 15:54:17 +1000 Subject: [PATCH 02/16] docs: plan worktree verbs, GitHub owner kinds, and device flow Eight tasks over the 2026-09-21 design, test-first throughout. Resolves the spec's one open question: Octokit 14.0.0 does expose InitiateDeviceFlow and CreateAccessTokenForDeviceFlow, and the latter polls internally, so the device flow is a translation layer rather than two hand-rolled posts. Co-Authored-By: Claude Opus 5 (1M context) --- .../2026-09-21-worktrees-and-github-auth.md | 2434 +++++++++++++++++ 1 file changed, 2434 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-21-worktrees-and-github-auth.md diff --git a/docs/superpowers/plans/2026-09-21-worktrees-and-github-auth.md b/docs/superpowers/plans/2026-09-21-worktrees-and-github-auth.md new file mode 100644 index 0000000..e449724 --- /dev/null +++ b/docs/superpowers/plans/2026-09-21-worktrees-and-github-auth.md @@ -0,0 +1,2434 @@ +# Worktree Verbs, GitHub Owner Kinds, and Device Flow Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add `git worktree` verbs, GitHub repository enumeration that can see an organisation's private repositories, and a GitHub OAuth device flow, so a desktop launcher can build a tree of every repository its user can reach and manage branches as one worktree each. + +**Architecture:** Three independent additions. The worktree verbs follow the library's existing builder-plus-parser pattern exactly: a `GitWorktree` record, a `GitWorktreeParser` over `git worktree list --porcelain`, four builders, and four factory methods on `GitRepository`. The GitHub change is an owner-kind property that routes `GetRepositoriesAsync` to whichever endpoint can answer for that kind of owner, defaulting to today's route. The device flow is a standalone type in `Hosting/` that obtains a `HostingCredential` and stores nothing. + +**Tech Stack:** .NET 10 and .NET 9 multi-target, C#, MSTest via MSTest.Sdk (Microsoft Testing Platform), Octokit 14.0.0, ktsu.Semantics, ktsu.CredentialCache, ktsu.RunCommand. + +**Spec:** `docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md` + +## Global Constraints + +- **Version: `[minor]` — 3.2.0.** Additive only. No existing member changes shape or behaviour. +- **Never run `dotnet test --nologo`.** Under Microsoft Testing Platform it silently runs zero tests and exits 5. Always plain `dotnet test`. +- **Copyright header on every new file:** `// Copyright (c) 2023-2026 ktsu-dev contributors` +- **Namespace:** `ktsu.GitIntegration` for library files, `ktsu.GitIntegration.Test` for test files. File-scoped, `using` directives inside the namespace, matching every existing file. +- **Tabs, not spaces.** The repository indents with tabs. +- **British spelling in prose and documentation comments** (`behaviour`, `organisation`, `normalise`), matching the existing codebase. +- **Every public member needs an XML doc comment.** The build treats missing documentation as an error. +- **`Ensure.NotNull` in the library, `ArgumentNullException.ThrowIfNull` in tests.** The library takes Polyfill with `PrivateAssets="all"`, so `Ensure` is not visible to the test project. +- **Caller-supplied operands go after `--end-of-options`,** via `GitCommandBuilder.AppendOperands`. +- **Target frameworks:** library `net10.0;net9.0`, tests `net10.0` only. Do not use an API unavailable on net9.0 in library code. + +--- + +## File Structure + +**Created:** + +| File | Responsibility | +|---|---| +| `GitIntegration/Models/GitWorktree.cs` | The inert record describing one worktree | +| `GitIntegration/Parsing/GitWorktreeParser.cs` | Reads `worktree list --porcelain` into records | +| `GitIntegration/Builders/GitWorktreeListBuilder.cs` | `IGitWorktreeListBuilder` and its implementation | +| `GitIntegration/Builders/GitWorktreeAddBuilder.cs` | `IGitWorktreeAddBuilder` and its implementation | +| `GitIntegration/Builders/GitWorktreeWriteBuilders.cs` | Remove and prune, two small builders that change together | +| `GitIntegration/Hosting/GitHubDeviceFlow.cs` | `GitHubDeviceCode` and `GitHubDeviceFlow` | +| `GitIntegration.Test/Parsing/GitWorktreeParserTests.cs` | Parser fixtures | +| `GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs` | Argument-vector assertions for all four builders | +| `GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs` | Device flow against a fake transport | +| `GitIntegration.Test/Fixtures/github-org-repositories.json` | Org listing fixture with a private repository | + +**Modified:** + +| File | Change | +|---|---| +| `GitIntegration/GitRepository.cs` | Four verb factory methods | +| `GitIntegration/Parsing/GitParseValues.cs` | `ToAbsoluteDirectoryPath` | +| `GitIntegration/Models/GitEnums.cs` | `GitHubOwnerKind` | +| `GitIntegration/SemanticTypes/GitProviderTypes.cs` | `GitHubOAuthClientId` | +| `GitIntegration/GitHubProvider.cs` | `OwnerKind`, enumeration routing, `X-GitHub-SSO` in the message | +| `GitIntegration.Test/Hosting/GitHubProviderTests.cs` | Owner-kind and single sign-on tests | +| `GitIntegration.Test/GitRepositoryVerbTests.cs` | `Worktrees()` factory guard tests | +| `GitIntegration.Test/GitRepositoryMutatingVerbTests.cs` | Add, remove, prune factory guard tests | +| `README.md`, `CHANGELOG.md` | Documentation | + +--- + +## Task 1: The worktree model and parser + +**Files:** +- Create: `GitIntegration/Models/GitWorktree.cs` +- Create: `GitIntegration/Parsing/GitWorktreeParser.cs` +- Modify: `GitIntegration/Parsing/GitParseValues.cs` +- Test: `GitIntegration.Test/Parsing/GitWorktreeParserTests.cs` + +**Interfaces:** +- Consumes: `GitParseValues.ToSemantic(string value, string description)`, `GitParseException`, `GitCommitSha`, `GitBranchName`, all existing. +- Produces: `GitWorktree` (record with `Path`, `Head`, `Branch`, `IsMain`, `IsBare`, `IsDetached`, `IsLocked`, `LockReason`, `IsPrunable`, `PrunableReason`); `GitWorktreeParser.Parse(string output)` returning `IReadOnlyList`; `GitParseValues.ToAbsoluteDirectoryPath(string value)` returning `AbsoluteDirectoryPath`. + +### Background: the format being parsed + +`git worktree list --porcelain` emits one record per worktree, records separated by a blank line, and a trailing blank line after the last record. Each record is `attribute` or `attribute value` lines: + +``` +worktree /home/u/project +HEAD 7f3c9a1b2d4e6f8a0c2e4a6b8d0f2a4c6e8b0d2f +branch refs/heads/main + +worktree /home/u/project-feature +HEAD 2b8e4d0f6a8c0e2a4c6e8b0d2f4a6c8e0b2d4f6a +branch refs/heads/feature +locked contains uncommitted experiment + +worktree /home/u/project-review +HEAD 9c1a7f2e4b6d8a0c2e4f6a8b0d2c4e6f8a0b2d4c +detached +prunable gitdir file points to non-existent location + +worktree /home/u/project-bare +bare + +``` + +`HEAD` and `branch` are absent for a bare worktree. `branch` is absent and `detached` present for a detached one. `locked` and `prunable` each appear alone or with a reason. + +- [ ] **Step 1: Write the failing parser tests** + +Create `GitIntegration.Test/Parsing/GitWorktreeParserTests.cs`: + +```csharp +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration.Test; + +using System; +using System.Collections.Generic; + +using ktsu.Semantics.Strings; + +[TestClass] +public sealed class GitWorktreeParserTests +{ + private const string MainOnly = + "worktree /home/u/project\n" + + "HEAD 7f3c9a1b2d4e6f8a0c2e4a6b8d0f2a4c6e8b0d2f\n" + + "branch refs/heads/main\n" + + "\n"; + + private const string ThreeWorktrees = + "worktree /home/u/project\n" + + "HEAD 7f3c9a1b2d4e6f8a0c2e4a6b8d0f2a4c6e8b0d2f\n" + + "branch refs/heads/main\n" + + "\n" + + "worktree /home/u/project-feature\n" + + "HEAD 2b8e4d0f6a8c0e2a4c6e8b0d2f4a6c8e0b2d4f6a\n" + + "branch refs/heads/feature\n" + + "locked contains uncommitted experiment\n" + + "\n" + + "worktree /home/u/project-review\n" + + "HEAD 9c1a7f2e4b6d8a0c2e4f6a8b0d2c4e6f8a0b2d4c\n" + + "detached\n" + + "prunable gitdir file points to non-existent location\n" + + "\n"; + + [TestMethod] + public void ReadsTheOnlyWorktreeAsTheMainOne() + { + IReadOnlyList worktrees = GitWorktreeParser.Parse(MainOnly); + + Assert.AreEqual(1, worktrees.Count); + Assert.AreEqual("/home/u/project", worktrees[0].Path.WeakString); + Assert.AreEqual("7f3c9a1b2d4e6f8a0c2e4a6b8d0f2a4c6e8b0d2f".As(), worktrees[0].Head); + Assert.AreEqual("main".As(), worktrees[0].Branch); + Assert.IsTrue(worktrees[0].IsMain); + Assert.IsFalse(worktrees[0].IsBare); + Assert.IsFalse(worktrees[0].IsDetached); + Assert.IsFalse(worktrees[0].IsLocked); + Assert.IsFalse(worktrees[0].IsPrunable); + } + + [TestMethod] + public void StripsTheRefsHeadsPrefixFromTheBranch() + { + // The hosting half of this library already hands back bare branch names. A caller should not + // have to know which half produced a name to know its shape. + IReadOnlyList worktrees = GitWorktreeParser.Parse(MainOnly); + + Assert.AreEqual("main".As(), worktrees[0].Branch); + } + + [TestMethod] + public void MarksOnlyTheFirstRecordAsMain() + { + IReadOnlyList worktrees = GitWorktreeParser.Parse(ThreeWorktrees); + + Assert.AreEqual(3, worktrees.Count); + Assert.IsTrue(worktrees[0].IsMain); + Assert.IsFalse(worktrees[1].IsMain); + Assert.IsFalse(worktrees[2].IsMain); + } + + [TestMethod] + public void ReadsALockReasonWhenGitGivesOne() + { + IReadOnlyList worktrees = GitWorktreeParser.Parse(ThreeWorktrees); + + Assert.IsTrue(worktrees[1].IsLocked); + Assert.AreEqual("contains uncommitted experiment", worktrees[1].LockReason); + } + + [TestMethod] + public void ReportsALockWithNoReasonAsLockedWithoutOne() + { + // "locked" alone and "locked " are different facts, and a caller showing the reason + // must be able to tell "no reason recorded" from "not locked". + string output = + "worktree /home/u/project\n" + + "HEAD 7f3c9a1b2d4e6f8a0c2e4a6b8d0f2a4c6e8b0d2f\n" + + "branch refs/heads/main\n" + + "locked\n" + + "\n"; + + IReadOnlyList worktrees = GitWorktreeParser.Parse(output); + + Assert.IsTrue(worktrees[0].IsLocked); + Assert.IsNull(worktrees[0].LockReason); + } + + [TestMethod] + public void ReadsADetachedWorktreeWithNoBranch() + { + IReadOnlyList worktrees = GitWorktreeParser.Parse(ThreeWorktrees); + + Assert.IsTrue(worktrees[2].IsDetached); + Assert.IsNull(worktrees[2].Branch); + Assert.AreEqual("9c1a7f2e4b6d8a0c2e4f6a8b0d2c4e6f8a0b2d4c".As(), worktrees[2].Head); + } + + [TestMethod] + public void ReadsAPrunableReason() + { + IReadOnlyList worktrees = GitWorktreeParser.Parse(ThreeWorktrees); + + Assert.IsTrue(worktrees[2].IsPrunable); + Assert.AreEqual("gitdir file points to non-existent location", worktrees[2].PrunableReason); + } + + [TestMethod] + public void ReadsABareWorktreeWithNeitherHeadNorBranch() + { + string output = + "worktree /home/u/project-bare\n" + + "bare\n" + + "\n"; + + IReadOnlyList worktrees = GitWorktreeParser.Parse(output); + + Assert.AreEqual(1, worktrees.Count); + Assert.IsTrue(worktrees[0].IsBare); + Assert.IsNull(worktrees[0].Head); + Assert.IsNull(worktrees[0].Branch); + } + + [TestMethod] + public void DoesNotInventARecordFromTheTrailingBlankLine() + { + // --porcelain emits a blank line after the last record as well as between records. A naive + // split on the separator turns that into a phantom entry. + IReadOnlyList worktrees = GitWorktreeParser.Parse(ThreeWorktrees); + + Assert.AreEqual(3, worktrees.Count); + } + + [TestMethod] + public void ReadsRecordsSeparatedByCarriageReturnLineFeeds() + { + IReadOnlyList worktrees = GitWorktreeParser.Parse(MainOnly.Replace("\n", "\r\n", StringComparison.Ordinal)); + + Assert.AreEqual(1, worktrees.Count); + Assert.AreEqual("main".As(), worktrees[0].Branch); + } + + [TestMethod] + public void ReadsNothingFromEmptyOutput() + { + Assert.AreEqual(0, GitWorktreeParser.Parse(string.Empty).Count); + } + + [TestMethod] + public void RejectsARecordWithNoWorktreePath() + { + // Every attribute is optional except the one naming what the record describes. + string output = + "HEAD 7f3c9a1b2d4e6f8a0c2e4a6b8d0f2a4c6e8b0d2f\n" + + "branch refs/heads/main\n" + + "\n"; + + _ = Assert.ThrowsExactly(() => GitWorktreeParser.Parse(output)); + } + + [TestMethod] + public void IgnoresAnAttributeItDoesNotRecognise() + { + // Git may add attributes. An unknown one must not fail a listing that is otherwise readable. + string output = + "worktree /home/u/project\n" + + "HEAD 7f3c9a1b2d4e6f8a0c2e4a6b8d0f2a4c6e8b0d2f\n" + + "branch refs/heads/main\n" + + "somethingnew value\n" + + "\n"; + + IReadOnlyList worktrees = GitWorktreeParser.Parse(output); + + Assert.AreEqual(1, worktrees.Count); + Assert.AreEqual("main".As(), worktrees[0].Branch); + } +} +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `dotnet test --filter "FullyQualifiedName~GitWorktreeParserTests"` +Expected: compile failure, `GitWorktree` and `GitWorktreeParser` do not exist. + +- [ ] **Step 3: Write the model** + +Create `GitIntegration/Models/GitWorktree.cs`: + +```csharp +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration; + +using ktsu.Semantics.Paths; + +/// +/// One working tree of a repository: where it is, and what it has checked out. +/// +/// +/// Inert data, like and . It carries no +/// and no process runner. A caller that wants to run a verb inside a +/// worktree passes to , which is the same +/// journey it would make from any other path. +/// +public sealed record GitWorktree +{ + /// Gets the worktree's own directory. + public required AbsoluteDirectoryPath Path { get; init; } + + /// + /// Gets the commit checked out here, or when git reported none. + /// + /// + /// Absent for a bare worktree, which has no checkout to report. Present for a detached one, + /// where it is the only thing identifying what is checked out. + /// + public GitCommitSha? Head { get; init; } + + /// + /// Gets the branch checked out here, or when there is none. + /// + /// + /// Absent for a bare or detached worktree. Reported as a bare name with git's + /// refs/heads/ prefix stripped, so that a branch name from this half of the library has + /// the same shape as one from the hosting half. + /// + public GitBranchName? Branch { get; init; } + + /// + /// Gets a value indicating whether this is the repository's main working tree. + /// + /// + /// Positional. Git's porcelain has no attribute for this: the main working tree is simply + /// the first record git emits, always. The parser sets this on the first record and on no other. + /// It exists because a caller managing worktrees needs to refuse to remove the one that owns the + /// repository, and answering that once here beats every caller reimplementing it. + /// + public bool IsMain { get; init; } + + /// Gets a value indicating whether this worktree has no working directory. + public bool IsBare { get; init; } + + /// Gets a value indicating whether this worktree has a commit checked out rather than a branch. + public bool IsDetached { get; init; } + + /// Gets a value indicating whether this worktree is locked against pruning. + public bool IsLocked { get; init; } + + /// + /// Gets the reason this worktree is locked, or when git recorded none. + /// + /// + /// Separately nullable from , because git emits locked both alone + /// and with a reason, and "locked for a reason nobody recorded" is a different fact from "not + /// locked". + /// + public string? LockReason { get; init; } + + /// Gets a value indicating whether git considers this worktree's record removable. + public bool IsPrunable { get; init; } + + /// + /// Gets the reason this worktree is prunable, or when git recorded none. + /// + public string? PrunableReason { get; init; } +} +``` + +- [ ] **Step 4: Add the absolute path conversion** + +Append to `GitIntegration/Parsing/GitParseValues.cs`, inside the class, after `ToRelativeDirectoryPath`: + +```csharp + /// + /// Converts a raw path field into an absolute directory path. + /// + /// + /// The absolute counterpart to , for the fields git reports + /// as whole paths rather than as paths within a repository — a worktree's own directory is the + /// first of them. An empty field is a malformed record, and a path this type refuses is one git + /// produced and this library cannot represent, which is a parse failure rather than a value to + /// pass along unchecked. + /// + /// The raw path as git printed it. + /// The converted path. + /// + /// is empty or cannot be represented as an absolute directory path. + /// + internal static AbsoluteDirectoryPath ToAbsoluteDirectoryPath(string value) + { + if (!string.IsNullOrEmpty(value) && + AbsoluteDirectoryPath.TryCreate(value, out AbsoluteDirectoryPath? path) && + path is not null) + { + return path; + } + + throw new GitParseException( + $"git reported a path that cannot be represented as an absolute directory path: '{value}'."); + } +``` + +- [ ] **Step 5: Write the parser** + +Create `GitIntegration/Parsing/GitWorktreeParser.cs`: + +```csharp +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration; + +using System; +using System.Collections.Generic; + +/// +/// Reads git worktree list --porcelain. +/// +/// +/// The porcelain format is one record per worktree, records separated by a blank line and a blank +/// line after the last. Each line is an attribute name, optionally followed by a space and a value. +/// Only worktree is guaranteed present: a bare worktree reports neither HEAD nor +/// branch, and a detached one reports HEAD and detached but no branch. +/// +internal static class GitWorktreeParser +{ + private const string RefsHeadsPrefix = "refs/heads/"; + + /// + /// Parses a porcelain worktree listing. + /// + /// Everything git wrote to standard output. + /// The worktrees, in the order git listed them, the main one first. + /// A record named no worktree, or a field failed validation. + internal static IReadOnlyList Parse(string output) + { + Ensure.NotNull(output); + + List worktrees = []; + List record = []; + + foreach (string line in output.Split('\n')) + { + string entry = line.TrimEnd('\r'); + + if (entry.Length != 0) + { + record.Add(entry); + continue; + } + + // A blank line closes a record. Closing only a non-empty one is what keeps the trailing + // blank line git always emits from producing a phantom final entry. + if (record.Count != 0) + { + worktrees.Add(ParseRecord(record, isMain: worktrees.Count == 0)); + record.Clear(); + } + } + + // Output not ending in a blank line still closes its last record, so a listing stays readable + // if git ever stops emitting the trailing separator. + if (record.Count != 0) + { + worktrees.Add(ParseRecord(record, isMain: worktrees.Count == 0)); + } + + return worktrees; + } + + /// + /// Turns one record's attribute lines into a worktree. + /// + /// The record's lines, in git's order. + /// Whether this is the first record git emitted. + /// The parsed worktree. + /// The record named no worktree, or a field failed validation. + private static GitWorktree ParseRecord(IReadOnlyList record, bool isMain) + { + string? path = null; + GitCommitSha? head = null; + GitBranchName? branch = null; + bool isBare = false; + bool isDetached = false; + bool isLocked = false; + bool isPrunable = false; + string? lockReason = null; + string? prunableReason = null; + + foreach (string line in record) + { + int separator = line.IndexOf(' ', StringComparison.Ordinal); + string attribute = separator < 0 ? line : line[..separator]; + string value = separator < 0 ? string.Empty : line[(separator + 1)..]; + + switch (attribute) + { + case "worktree": + path = value; + break; + case "HEAD": + head = GitParseValues.ToSemantic(value, "commit id"); + break; + case "branch": + branch = GitParseValues.ToSemantic(StripRefsHeadsPrefix(value), "branch name"); + break; + case "bare": + isBare = true; + break; + case "detached": + isDetached = true; + break; + case "locked": + isLocked = true; + lockReason = value.Length == 0 ? null : value; + break; + case "prunable": + isPrunable = true; + prunableReason = value.Length == 0 ? null : value; + break; + default: + // Unknown attributes are skipped rather than rejected. Git may add one, and a + // listing that is otherwise readable should not fail over a field nobody reads. + break; + } + } + + return path is null + ? throw new GitParseException("git reported a worktree record naming no worktree path.") + : new GitWorktree + { + Path = GitParseValues.ToAbsoluteDirectoryPath(path), + Head = head, + Branch = branch, + IsMain = isMain, + IsBare = isBare, + IsDetached = isDetached, + IsLocked = isLocked, + LockReason = lockReason, + IsPrunable = isPrunable, + PrunableReason = prunableReason, + }; + } + + /// + /// Strips a leading refs/heads/, leaving the value untouched when the prefix is absent. + /// + /// The reference as git printed it. + /// The bare branch name. + private static string StripRefsHeadsPrefix(string reference) => + reference.StartsWith(RefsHeadsPrefix, StringComparison.Ordinal) + ? reference[RefsHeadsPrefix.Length..] + : reference; +} +``` + +- [ ] **Step 6: Run the tests to verify they pass** + +Run: `dotnet test --filter "FullyQualifiedName~GitWorktreeParserTests"` +Expected: PASS, 13 tests. + +- [ ] **Step 7: Run the whole suite** + +Run: `dotnet test` +Expected: PASS, no regressions. + +- [ ] **Step 8: Commit** + +```bash +git add GitIntegration/Models/GitWorktree.cs GitIntegration/Parsing/GitWorktreeParser.cs GitIntegration/Parsing/GitParseValues.cs GitIntegration.Test/Parsing/GitWorktreeParserTests.cs +git commit -m "feat: read git worktree list --porcelain into a typed model" +``` + +--- + +## Task 2: The worktree listing verb + +**Files:** +- Create: `GitIntegration/Builders/GitWorktreeListBuilder.cs` +- Modify: `GitIntegration/GitRepository.cs` +- Test: `GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs` +- Test: `GitIntegration.Test/GitRepositoryVerbTests.cs` + +**Interfaces:** +- Consumes: `GitWorktree`, `GitWorktreeParser.Parse` from Task 1. `GitCommandBuilder(IGitProcessRunner runner, AbsoluteDirectoryPath? repositoryPath)` with abstract `AppendVerbArguments(ICollection)` and `ParseResult(GitProcessResult)`. `RecordingGitProcessRunner` and `TestPaths.Root` from the test project. +- Produces: `IGitWorktreeListBuilder : IGitCommandBuilder>`; `GitRepository.Worktrees()` returning it. + +- [ ] **Step 1: Write the failing tests** + +Create `GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs`: + +```csharp +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration.Test; + +using System.Collections.Generic; +using System.Linq; + +[TestClass] +public sealed class GitWorktreeBuilderTests +{ + private static readonly string[] GlobalArguments = + [ + "-C", TestPaths.Root.WeakString, + "--no-pager", + "-c", "core.quotepath=false", + "-c", "color.ui=false", + ]; + + private static string[] Expect(params string[] verbArguments) => + [.. GlobalArguments, .. verbArguments]; + + [TestMethod] + public void BuildsTheWorktreeListVector() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeListBuilder builder = new(runner, TestPaths.Root); + + IReadOnlyList arguments = builder.BuildArguments(); + + CollectionAssert.AreEqual(Expect("worktree", "list", "--porcelain"), arguments.ToArray()); + } +} +``` + +Append to `GitIntegration.Test/GitRepositoryVerbTests.cs`, inside the existing class: + +```csharp + [TestMethod] + public void WorktreesRequiresAProcessRunner() + { + GitRepository repository = new() { LocalPath = TestPaths.Root }; + + _ = Assert.ThrowsExactly(() => repository.Worktrees()); + } + + [TestMethod] + public void WorktreesRequiresALocalPath() + { + GitRepository repository = new() { ProcessRunner = new RecordingGitProcessRunner() }; + + _ = Assert.ThrowsExactly(() => repository.Worktrees()); + } +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `dotnet test --filter "FullyQualifiedName~GitWorktreeBuilderTests|FullyQualifiedName~GitRepositoryVerbTests"` +Expected: compile failure, `GitWorktreeListBuilder` and `Worktrees` do not exist. + +- [ ] **Step 3: Write the builder** + +Create `GitIntegration/Builders/GitWorktreeListBuilder.cs`: + +```csharp +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration; + +using System.Collections.Generic; + +using ktsu.Semantics.Paths; + +/// +/// Lists the repository's working trees. +/// +/// +/// Reports the main working tree first, which is the order git emits and the only thing identifying +/// it — see . +/// +public interface IGitWorktreeListBuilder : IGitCommandBuilder> +{ +} + +/// +/// Builds git worktree list --porcelain. +/// +/// +/// No options. The porcelain listing already reports every attribute this library models, and git's +/// remaining options on this verb either change the format (-v, which is the human-facing +/// form) or add a field this library reads from the porcelain output anyway (--expire, which +/// only affects which entries are annotated prunable). +/// +/// Runs the assembled command. +/// The repository to scope the command to. +internal sealed class GitWorktreeListBuilder(IGitProcessRunner runner, AbsoluteDirectoryPath repositoryPath) + : GitCommandBuilder>(runner, repositoryPath), IGitWorktreeListBuilder +{ + /// + protected override void AppendVerbArguments(ICollection arguments) + { + Ensure.NotNull(arguments); + + arguments.Add("worktree"); + arguments.Add("list"); + arguments.Add("--porcelain"); + } + + /// + protected override IReadOnlyList ParseResult(GitProcessResult result) => + GitWorktreeParser.Parse(Ensure.NotNull(result).StandardOutput); +} +``` + +- [ ] **Step 4: Add the factory** + +In `GitIntegration/GitRepository.cs`, after the `Tags()` factory: + +```csharp + /// Lists the repository's working trees. + /// A fresh builder. + /// This repository has no . + public IGitWorktreeListBuilder Worktrees() => new GitWorktreeListBuilder(RequireRunner(), RequireLocalPath()); +``` + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `dotnet test --filter "FullyQualifiedName~GitWorktreeBuilderTests|FullyQualifiedName~GitRepositoryVerbTests"` +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add GitIntegration/Builders/GitWorktreeListBuilder.cs GitIntegration/GitRepository.cs GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs GitIntegration.Test/GitRepositoryVerbTests.cs +git commit -m "feat: add the worktree listing verb" +``` + +--- + +## Task 3: The worktree add verb + +**Files:** +- Create: `GitIntegration/Builders/GitWorktreeAddBuilder.cs` +- Modify: `GitIntegration/GitRepository.cs` +- Test: `GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs` +- Test: `GitIntegration.Test/GitRepositoryMutatingVerbTests.cs` + +**Interfaces:** +- Consumes: `GitCommandBuilder`, `AppendOperands(ICollection, params string[])`, `GitCompleted`, `GitBranchName`, `GitRefName`, `AbsoluteDirectoryPath`. +- Produces: `IGitWorktreeAddBuilder` with `CheckingOut(GitBranchName)`, `CreatingBranch(GitBranchName)`, `CreatingOrResettingBranch(GitBranchName)`, `Detached()`, `From(GitRefName)`, `Force()`, `WithoutCheckout()`, each returning `IGitWorktreeAddBuilder`; `GitRepository.AddWorktree(AbsoluteDirectoryPath path)` returning it. + +### Background: the syntax being built + +`git worktree add [-f] [--detach] [--no-checkout] [-b | -B ] []` + +The four mode settings divide across two places. `CreatingBranch` emits `-b`, `CreatingOrResettingBranch` emits `-B`, `Detached` emits `--detach`. `CheckingOut` emits no option and instead writes the `` operand. + +`From` writes that **same** operand. So `CheckingOut` and `From` are one field under two names, and the later call wins. They are typed differently because that is the useful distinction: `CheckingOut` takes a `GitBranchName` and reads as the ordinary case, `From` takes a `GitRefName` and admits a tag or a raw revision. + +- [ ] **Step 1: Write the failing tests** + +Append to `GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs`, inside the class: + +```csharp + [TestMethod] + public void BuildsTheMinimalWorktreeAddVector() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + + CollectionAssert.AreEqual( + Expect("worktree", "add", "--end-of-options", TestPaths.Worktree.WeakString), + builder.BuildArguments().ToArray()); + } + + [TestMethod] + public void PutsACheckedOutBranchInTheCommitIshOperand() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + _ = builder.CheckingOut("feature".As()); + + CollectionAssert.AreEqual( + Expect("worktree", "add", "--end-of-options", TestPaths.Worktree.WeakString, "feature"), + builder.BuildArguments().ToArray()); + } + + [TestMethod] + public void EmitsTheCreateBranchOptionBeforeThePath() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + _ = builder.CreatingBranch("feature".As()); + + CollectionAssert.AreEqual( + Expect("worktree", "add", "-b", "feature", "--end-of-options", TestPaths.Worktree.WeakString), + builder.BuildArguments().ToArray()); + } + + [TestMethod] + public void EmitsTheResettingCreateBranchOption() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + _ = builder.CreatingOrResettingBranch("feature".As()); + + CollectionAssert.AreEqual( + Expect("worktree", "add", "-B", "feature", "--end-of-options", TestPaths.Worktree.WeakString), + builder.BuildArguments().ToArray()); + } + + [TestMethod] + public void CombinesBranchCreationWithAStartPoint() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + _ = builder.CreatingBranch("feature".As()).From("origin/main".As()); + + CollectionAssert.AreEqual( + Expect("worktree", "add", "-b", "feature", "--end-of-options", TestPaths.Worktree.WeakString, "origin/main"), + builder.BuildArguments().ToArray()); + } + + [TestMethod] + public void CombinesDetachmentWithACommitIsh() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + _ = builder.Detached().From("v1.2.0".As()); + + CollectionAssert.AreEqual( + Expect("worktree", "add", "--detach", "--end-of-options", TestPaths.Worktree.WeakString, "v1.2.0"), + builder.BuildArguments().ToArray()); + } + + [TestMethod] + public void TheLastModeSelectionWins() + { + // The four modes are one field, resolved rather than accumulated, matching how + // IGitBranchListBuilder's LocalOnly and RemoteOnly already replace each other. A vector + // carrying both -b and --detach is one git rejects. + RecordingGitProcessRunner runner = new(); + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + _ = builder.CreatingBranch("feature".As()).Detached(); + + string[] arguments = [.. builder.BuildArguments()]; + + CollectionAssert.Contains(arguments, "--detach"); + CollectionAssert.DoesNotContain(arguments, "-b"); + CollectionAssert.DoesNotContain(arguments, "feature"); + } + + [TestMethod] + public void TheLastCommitIshSelectionWins() + { + // CheckingOut and From write the same operand slot, so the same resolution applies. + RecordingGitProcessRunner runner = new(); + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + _ = builder.CheckingOut("feature".As()).From("v1.2.0".As()); + + string[] arguments = [.. builder.BuildArguments()]; + + CollectionAssert.Contains(arguments, "v1.2.0"); + CollectionAssert.DoesNotContain(arguments, "feature"); + } + + [TestMethod] + public void EmitsForceAndNoCheckout() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + _ = builder.Force().WithoutCheckout(); + + CollectionAssert.AreEqual( + Expect("worktree", "add", "--force", "--no-checkout", "--end-of-options", TestPaths.Worktree.WeakString), + builder.BuildArguments().ToArray()); + } + + [TestMethod] + public async Task ReportsCompletionOnSuccess() + { + RecordingGitProcessRunner runner = new() { StandardError = "Preparing worktree (new branch 'feature')\n" }; + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + + GitCompleted completed = await builder.ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.IsNotNull(completed); + } + + public TestContext TestContext { get; set; } = null!; +``` + +Add to the file's `using` directives: `using System.Threading.Tasks;` and `using ktsu.Semantics.Strings;`. + +Append to `GitIntegration.Test/GitRepositoryMutatingVerbTests.cs`, inside the existing class: + +```csharp + [TestMethod] + public void AddWorktreeRejectsANullPath() + { + GitRepository repository = new() + { + LocalPath = TestPaths.Root, + ProcessRunner = new RecordingGitProcessRunner(), + }; + + _ = Assert.ThrowsExactly(() => repository.AddWorktree(null!)); + } + + [TestMethod] + public void AddWorktreeRequiresAProcessRunner() + { + GitRepository repository = new() { LocalPath = TestPaths.Root }; + + _ = Assert.ThrowsExactly(() => repository.AddWorktree(TestPaths.Worktree)); + } +``` + +- [ ] **Step 2: Add the shared test path** + +`TestPaths` lives at the bottom of `GitIntegration.Test/GitRepositoryMetadataTests.cs`, outside the test class: + +```csharp +/// Paths that exist on every platform the tests run on. +internal static class TestPaths +{ + public static AbsoluteDirectoryPath Root { get; } = + (OperatingSystem.IsWindows() ? @"C:\" : "/").As(); +} +``` + +Add `Worktree` alongside `Root`, following the same platform switch: + +```csharp + /// An absolute directory distinct from , used as a worktree destination. + public static AbsoluteDirectoryPath Worktree { get; } = + (OperatingSystem.IsWindows() ? @"C:\project-feature" : "/project-feature").As(); +``` + +- [ ] **Step 3: Run the tests to verify they fail** + +Run: `dotnet test --filter "FullyQualifiedName~GitWorktreeBuilderTests"` +Expected: compile failure, `GitWorktreeAddBuilder` does not exist. + +- [ ] **Step 4: Write the builder** + +Create `GitIntegration/Builders/GitWorktreeAddBuilder.cs`: + +```csharp +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration; + +using System.Collections.Generic; + +using ktsu.Semantics.Paths; + +/// +/// Creates an additional working tree. +/// +/// +/// , , +/// and are four settings of one mode, and a later call replaces an earlier +/// one, as IGitBranchListBuilder's LocalOnly and RemoteOnly already do. A vector +/// carrying two of them at once is one git rejects. +/// +public interface IGitWorktreeAddBuilder : IGitCommandBuilder +{ + /// Checks out an existing branch in the new worktree. Replaces any previous mode. + /// The branch to check out. + /// The same builder, to allow chaining. + public IGitWorktreeAddBuilder CheckingOut(GitBranchName branch); + + /// Creates a branch and checks it out in the new worktree. Replaces any previous mode. + /// Fails when the branch already exists; see . + /// The branch to create. + /// The same builder, to allow chaining. + public IGitWorktreeAddBuilder CreatingBranch(GitBranchName branch); + + /// + /// Creates a branch, resetting it when it already exists, and checks it out. Replaces any + /// previous mode. + /// + /// The branch to create or reset. + /// The same builder, to allow chaining. + public IGitWorktreeAddBuilder CreatingOrResettingBranch(GitBranchName branch); + + /// Checks out a commit rather than a branch. Replaces any previous mode. + /// The same builder, to allow chaining. + public IGitWorktreeAddBuilder Detached(); + + /// + /// Sets the commit-ish the new worktree starts from: the start point for the branch-creating + /// modes, and the commit to detach at for . + /// + /// + /// Writes the same operand as , so the later of the two calls wins. + /// Distinct from it only in taking a , which admits a tag or a raw + /// revision rather than a branch alone. + /// + /// The revision to start from. + /// The same builder, to allow chaining. + public IGitWorktreeAddBuilder From(GitRefName commitish); + + /// Creates the worktree even when git would otherwise refuse. + /// + /// Git refuses when the branch is already checked out in another worktree, and when the + /// destination is a missing-but-registered worktree directory. + /// + /// The same builder, to allow chaining. + public IGitWorktreeAddBuilder Force(); + + /// Registers the worktree without populating its working directory. + /// The same builder, to allow chaining. + public IGitWorktreeAddBuilder WithoutCheckout(); +} + +/// +/// Builds git worktree add. +/// +/// +/// --guess-remote is deliberately not exposed. Plain git worktree add <path> +/// <branch> already creates a local tracking branch when the name matches exactly one +/// remote, which covers creating a worktree for a branch that exists only on the remote. +/// --guess-remote extends that to the case where no branch is named at all, which this +/// builder always names. +/// +/// Runs the assembled command. +/// The repository to scope the command to. +/// Where the new worktree goes. +internal sealed class GitWorktreeAddBuilder( + IGitProcessRunner runner, + AbsoluteDirectoryPath repositoryPath, + AbsoluteDirectoryPath path) + : GitCommandBuilder(runner, repositoryPath), IGitWorktreeAddBuilder +{ + private enum AddMode + { + Plain, + CreateBranch, + CreateOrResetBranch, + Detach, + } + + private readonly AbsoluteDirectoryPath _path = Ensure.NotNull(path); + + private AddMode _mode = AddMode.Plain; + private string? _branch; + private string? _commitish; + private bool _force; + private bool _withoutCheckout; + + /// + public IGitWorktreeAddBuilder CheckingOut(GitBranchName branch) + { + Ensure.NotNull(branch); + + _mode = AddMode.Plain; + _branch = null; + _commitish = branch.WeakString; + return this; + } + + /// + public IGitWorktreeAddBuilder CreatingBranch(GitBranchName branch) + { + Ensure.NotNull(branch); + + _mode = AddMode.CreateBranch; + _branch = branch.WeakString; + return this; + } + + /// + public IGitWorktreeAddBuilder CreatingOrResettingBranch(GitBranchName branch) + { + Ensure.NotNull(branch); + + _mode = AddMode.CreateOrResetBranch; + _branch = branch.WeakString; + return this; + } + + /// + public IGitWorktreeAddBuilder Detached() + { + _mode = AddMode.Detach; + _branch = null; + return this; + } + + /// + public IGitWorktreeAddBuilder From(GitRefName commitish) + { + Ensure.NotNull(commitish); + + _commitish = commitish.WeakString; + return this; + } + + /// + public IGitWorktreeAddBuilder Force() + { + _force = true; + return this; + } + + /// + public IGitWorktreeAddBuilder WithoutCheckout() + { + _withoutCheckout = true; + return this; + } + + /// + protected override void AppendVerbArguments(ICollection arguments) + { + Ensure.NotNull(arguments); + + arguments.Add("worktree"); + arguments.Add("add"); + + if (_force) + { + arguments.Add("--force"); + } + + if (_withoutCheckout) + { + arguments.Add("--no-checkout"); + } + + switch (_mode) + { + case AddMode.CreateBranch: + arguments.Add("-b"); + arguments.Add(_branch!); + break; + case AddMode.CreateOrResetBranch: + arguments.Add("-B"); + arguments.Add(_branch!); + break; + case AddMode.Detach: + arguments.Add("--detach"); + break; + case AddMode.Plain: + default: + break; + } + + // The path and the commit-ish are both caller-supplied operands and share one end-of-options + // marker, which git reads as applying to everything after it. + if (_commitish is null) + { + AppendOperands(arguments, _path.WeakString); + } + else + { + AppendOperands(arguments, _path.WeakString, _commitish); + } + } + + /// + protected override GitCompleted ParseResult(GitProcessResult result) => new(); +} +``` + +Note: the `-b ` value is a library-routed caller value that cannot precede `--end-of-options`, because git requires the option and its value together before the operands. `GitBranchName`'s own validation is what stops a dash-leading value reaching it; check whether `GitBranchName` carries `NotAnOptionAttribute` (grep: `grep -n "GitBranchName" GitIntegration/SemanticTypes/GitRefTypes.cs`) and, if it does not, note it in the pull request rather than changing the type in this task. + +- [ ] **Step 5: Add the factory** + +In `GitIntegration/GitRepository.cs`, after `Worktrees()`: + +```csharp + /// Creates an additional working tree. + /// Where the new worktree goes. + /// A fresh builder. + /// is . + /// This repository has no . + public IGitWorktreeAddBuilder AddWorktree(AbsoluteDirectoryPath path) + { + // Argument validation precedes the state check, matching every other verb taking an operand. + Ensure.NotNull(path); + return new GitWorktreeAddBuilder(RequireRunner(), RequireLocalPath(), path); + } +``` + +- [ ] **Step 6: Run the tests to verify they pass** + +Run: `dotnet test --filter "FullyQualifiedName~GitWorktreeBuilderTests|FullyQualifiedName~GitRepositoryMutatingVerbTests"` +Expected: PASS. + +- [ ] **Step 7: Commit** + +```bash +git add GitIntegration/Builders/GitWorktreeAddBuilder.cs GitIntegration/GitRepository.cs GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs GitIntegration.Test/GitRepositoryMutatingVerbTests.cs +git commit -m "feat: add the worktree creation verb" +``` + +--- + +## Task 4: The worktree remove and prune verbs + +**Files:** +- Create: `GitIntegration/Builders/GitWorktreeWriteBuilders.cs` +- Modify: `GitIntegration/GitRepository.cs` +- Test: `GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs` +- Test: `GitIntegration.Test/GitRepositoryMutatingVerbTests.cs` + +**Interfaces:** +- Consumes: everything Task 3 consumes. +- Produces: `IGitWorktreeRemoveBuilder` with `Force()`; `IGitWorktreePruneBuilder` with no members; `GitRepository.RemoveWorktree(AbsoluteDirectoryPath path)` and `GitRepository.PruneWorktrees()`. + +Both live in one file because they are two small builders over the same verb that change together, matching `GitRemoteWriteBuilders`-style grouping already in the test project. + +- [ ] **Step 1: Write the failing tests** + +Append to `GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs`, inside the class: + +```csharp + [TestMethod] + public void BuildsTheWorktreeRemoveVector() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeRemoveBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + + CollectionAssert.AreEqual( + Expect("worktree", "remove", "--end-of-options", TestPaths.Worktree.WeakString), + builder.BuildArguments().ToArray()); + } + + [TestMethod] + public void EmitsForceOnRemoveBeforeThePath() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeRemoveBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + _ = builder.Force(); + + CollectionAssert.AreEqual( + Expect("worktree", "remove", "--force", "--end-of-options", TestPaths.Worktree.WeakString), + builder.BuildArguments().ToArray()); + } + + [TestMethod] + public void BuildsTheWorktreePruneVector() + { + RecordingGitProcessRunner runner = new(); + GitWorktreePruneBuilder builder = new(runner, TestPaths.Root); + + CollectionAssert.AreEqual( + Expect("worktree", "prune"), + builder.BuildArguments().ToArray()); + } +``` + +Append to `GitIntegration.Test/GitRepositoryMutatingVerbTests.cs`, inside the class: + +```csharp + [TestMethod] + public void RemoveWorktreeRejectsANullPath() + { + GitRepository repository = new() + { + LocalPath = TestPaths.Root, + ProcessRunner = new RecordingGitProcessRunner(), + }; + + _ = Assert.ThrowsExactly(() => repository.RemoveWorktree(null!)); + } + + [TestMethod] + public void PruneWorktreesRequiresAProcessRunner() + { + GitRepository repository = new() { LocalPath = TestPaths.Root }; + + _ = Assert.ThrowsExactly(() => repository.PruneWorktrees()); + } +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `dotnet test --filter "FullyQualifiedName~GitWorktreeBuilderTests"` +Expected: compile failure, the two builders do not exist. + +- [ ] **Step 3: Write the builders** + +Create `GitIntegration/Builders/GitWorktreeWriteBuilders.cs`: + +```csharp +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration; + +using System.Collections.Generic; + +using ktsu.Semantics.Paths; + +/// +/// Removes a working tree and the administrative record of it. +/// +public interface IGitWorktreeRemoveBuilder : IGitCommandBuilder +{ + /// Removes the worktree even when its working directory is not clean. + /// + /// Git refuses to remove a worktree holding modified tracked files or untracked files, since + /// doing so discards them. This says the caller knows. + /// + /// The same builder, to allow chaining. + public IGitWorktreeRemoveBuilder Force(); +} + +/// +/// Removes administrative records for working trees whose directories have gone. +/// +/// +/// No options. --dry-run is deliberately not exposed: it would change this verb's result +/// from into a listing, and a caller that wants to know what would be +/// removed reads from a listing it can already obtain. +/// +public interface IGitWorktreePruneBuilder : IGitCommandBuilder +{ +} + +/// +/// Builds git worktree remove. +/// +/// Runs the assembled command. +/// The repository to scope the command to. +/// The worktree to remove. +internal sealed class GitWorktreeRemoveBuilder( + IGitProcessRunner runner, + AbsoluteDirectoryPath repositoryPath, + AbsoluteDirectoryPath path) + : GitCommandBuilder(runner, repositoryPath), IGitWorktreeRemoveBuilder +{ + private readonly AbsoluteDirectoryPath _path = Ensure.NotNull(path); + + private bool _force; + + /// + public IGitWorktreeRemoveBuilder Force() + { + _force = true; + return this; + } + + /// + protected override void AppendVerbArguments(ICollection arguments) + { + Ensure.NotNull(arguments); + + arguments.Add("worktree"); + arguments.Add("remove"); + + if (_force) + { + arguments.Add("--force"); + } + + AppendOperands(arguments, _path.WeakString); + } + + /// + protected override GitCompleted ParseResult(GitProcessResult result) => new(); +} + +/// +/// Builds git worktree prune. +/// +/// Runs the assembled command. +/// The repository to scope the command to. +internal sealed class GitWorktreePruneBuilder(IGitProcessRunner runner, AbsoluteDirectoryPath repositoryPath) + : GitCommandBuilder(runner, repositoryPath), IGitWorktreePruneBuilder +{ + /// + protected override void AppendVerbArguments(ICollection arguments) + { + Ensure.NotNull(arguments); + + arguments.Add("worktree"); + arguments.Add("prune"); + } + + /// + protected override GitCompleted ParseResult(GitProcessResult result) => new(); +} +``` + +- [ ] **Step 4: Add the factories** + +In `GitIntegration/GitRepository.cs`, after `AddWorktree`: + +```csharp + /// Removes a working tree and the administrative record of it. + /// The worktree to remove. + /// A fresh builder. + /// is . + /// This repository has no . + public IGitWorktreeRemoveBuilder RemoveWorktree(AbsoluteDirectoryPath path) + { + Ensure.NotNull(path); + return new GitWorktreeRemoveBuilder(RequireRunner(), RequireLocalPath(), path); + } + + /// Removes administrative records for working trees whose directories have gone. + /// A fresh builder. + /// This repository has no . + public IGitWorktreePruneBuilder PruneWorktrees() => + new GitWorktreePruneBuilder(RequireRunner(), RequireLocalPath()); +``` + +- [ ] **Step 5: Run the full suite** + +Run: `dotnet test` +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add GitIntegration/Builders/GitWorktreeWriteBuilders.cs GitIntegration/GitRepository.cs GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs GitIntegration.Test/GitRepositoryMutatingVerbTests.cs +git commit -m "feat: add the worktree removal and prune verbs" +``` + +--- + +## Task 5: GitHub owner kinds + +**Files:** +- Modify: `GitIntegration/Models/GitEnums.cs` +- Modify: `GitIntegration/GitHubProvider.cs` +- Create: `GitIntegration.Test/Fixtures/github-org-repositories.json` +- Test: `GitIntegration.Test/Hosting/GitHubProviderTests.cs` + +**Interfaces:** +- Consumes: `GitProvider.Owner` (`GitProviderOwner`), `GitHubProvider.CreateClient()` returning `(GitHubClient, IDisposable)`, `GitHubProvider.ToGitRepository(Octokit.Repository)`, `GitHubProvider.Translate(Octokit.ApiException)`, `FakeHttpMessageHandler` with `Respond(HttpStatusCode, string, params (string, string)[])` and `Requests`. +- Produces: `GitHubOwnerKind` enum with `User`, `Organization`, `AuthenticatedUser`; `GitHubProvider.OwnerKind` init property defaulting to `GitHubOwnerKind.User`. + +**Important:** an existing test, `GitHubProviderTests`, asserts `"/users/contoso/repos"` as the route. The default must keep that test passing untouched. If it fails, the default is wrong, not the test. + +- [ ] **Step 1: Create the organisation fixture** + +Copy `GitIntegration.Test/Fixtures/github-repositories.json` to `github-org-repositories.json`, then edit the copy so that it contains exactly two entries: the first with `"name": "org-public-repo"`, `"private": false`, and an owner block whose `"login"` is `"contoso"`; the second with `"name": "org-private-repo"`, `"private": true`, and the same owner login. Update each entry's `full_name`, `html_url`, and `clone_url` to match its name under `contoso`. Leave every other key exactly as captured. + +Confirm the file is copied to the test output: check `GitIntegration.Test/GitIntegration.Test.csproj` for how the existing fixtures are included (grep: `grep -n "Fixtures" GitIntegration.Test/GitIntegration.Test.csproj`). If they are listed individually rather than by wildcard, add the new file. + +- [ ] **Step 2: Write the failing tests** + +Append to `GitIntegration.Test/Hosting/GitHubProviderTests.cs`, inside the class: + +```csharp + [TestMethod] + public async Task EnumeratesAnOrganisationThroughTheOrgsRoute() + { + // The route is a documented part of this provider's contract, not an Octokit detail: + // GET /orgs/{org}/repos is the only one of the three that reports an organisation's private + // repositories to a token that can see them. + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, Fixture("github-org-repositories.json"), ("Content-Type", "application/json")); + GitHubProvider provider = new() + { + Owner = "contoso".As(), + OwnerKind = GitHubOwnerKind.Organization, + Handler = handler, + }; + + IReadOnlyList repositories = + await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual("/orgs/contoso/repos", handler.Requests[0].Uri.AbsolutePath); + Assert.AreEqual(2, repositories.Count); + Assert.AreEqual("org-public-repo".As(), repositories[0].Name); + Assert.AreEqual("org-private-repo".As(), repositories[1].Name); + } + + [TestMethod] + public async Task EnumeratesTheAuthenticatedAccountThroughTheUserRoute() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, Fixture("github-org-repositories.json"), ("Content-Type", "application/json")); + GitHubProvider provider = new() + { + Owner = "contoso".As(), + OwnerKind = GitHubOwnerKind.AuthenticatedUser, + Handler = handler, + }; + + _ = await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual("/user/repos", handler.Requests[0].Uri.AbsolutePath); + StringAssert.Contains(handler.Requests[0].Uri.Query, "affiliation=owner%2Corganization_member"); + } + + [TestMethod] + public async Task DropsRepositoriesBelongingToAnotherOwnerWhenEnumeratingTheAuthenticatedAccount() + { + // GET /user/repos takes no owner parameter, so routing to it unfiltered would silently ignore + // a configured Owner. The filter is what keeps this method's contract "Owner's repositories". + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, Fixture("github-org-repositories.json"), ("Content-Type", "application/json")); + GitHubProvider provider = new() + { + Owner = "someone-else".As(), + OwnerKind = GitHubOwnerKind.AuthenticatedUser, + Handler = handler, + }; + + IReadOnlyList repositories = + await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual(0, repositories.Count); + } + + [TestMethod] + public async Task MatchesTheOwnerWithoutRegardToCase() + { + // GitHub treats logins as case-insensitive. An ordinal comparison would drop a caller's + // repositories over a capital letter the caller did not choose. + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, Fixture("github-org-repositories.json"), ("Content-Type", "application/json")); + GitHubProvider provider = new() + { + Owner = "CONTOSO".As(), + OwnerKind = GitHubOwnerKind.AuthenticatedUser, + Handler = handler, + }; + + IReadOnlyList repositories = + await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual(2, repositories.Count); + } + + [TestMethod] + public async Task DefaultsToTheUserRouteItAlwaysUsed() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, Fixture("github-repositories.json"), ("Content-Type", "application/json")); + GitHubProvider provider = new() { Owner = "contoso".As(), Handler = handler }; + + _ = await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual("/users/contoso/repos", handler.Requests[0].Uri.AbsolutePath); + } +``` + +- [ ] **Step 3: Run the tests to verify they fail** + +Run: `dotnet test --filter "FullyQualifiedName~GitHubProviderTests"` +Expected: compile failure, `GitHubOwnerKind` and `OwnerKind` do not exist. + +- [ ] **Step 4: Add the enum** + +Append to `GitIntegration/Models/GitEnums.cs`: + +```csharp +/// +/// What kind of account a 's owner is, which decides the endpoint its +/// repository enumeration can use. +/// +/// +/// GitHub answers "which repositories does this owner have" at three different routes with three +/// different coverages, and no single one of them serves every caller. Stating the kind is what lets +/// this provider pick correctly without probing, and without a contract that depends on what GitHub +/// answers to a speculative request. +/// +public enum GitHubOwnerKind +{ + /// + /// A user account other than the token's own. Enumerates that user's public repositories + /// only, whatever credential is supplied. + /// + User, + + /// + /// An organisation. Enumerates the organisation's repositories, including private ones the + /// credential can see. + /// + Organization, + + /// + /// The account the credential belongs to. Enumerates every repository that account owns or can + /// reach through an organisation membership, narrowed to + /// . + /// + AuthenticatedUser, +} +``` + +- [ ] **Step 5: Add the property and the routing** + +In `GitIntegration/GitHubProvider.cs`, add the property after `Name`: + +```csharp + /// + /// Gets what kind of account is. + /// + /// + /// by default, which is the route and the coverage this + /// provider has always had. A caller needing an organisation's private repositories sets + /// ; one needing its own sets + /// . + /// + public GitHubOwnerKind OwnerKind { get; init; } = GitHubOwnerKind.User; +``` + +Replace the body of `GetRepositoriesAsync`, keeping its existing signature, and **rewrite its ``** so the documentation no longer claims the method is public-only unconditionally: + +```csharp + /// + /// + /// Adds to the interface's remarks rather than restating them. + /// says only that implementations differ + /// in coverage and points here for this host's specifics, so the two texts have one job each and + /// neither is a copy of the other. An edit that moves this explanation must leave that pointer + /// aimed somewhere real. + /// + /// The route, and therefore the coverage, is decided by . + /// — the default — calls GET /users/{login}/repos, + /// which returns 's public repositories only; supplying a + /// token does not widen it, because that endpoint does not honour authentication to reveal + /// private repositories. calls + /// GET /orgs/{org}/repos, which does report private repositories the credential can see. + /// calls GET /user/repos with + /// affiliation=owner,organization_member. + /// + /// + /// GET /user/repos takes no owner parameter — it always describes the token's own + /// reachable repositories — so that route's results are filtered to + /// here, case-insensitively, since GitHub treats logins that way. + /// Without the filter, selecting that kind would silently ignore a configured owner, which is the + /// objection that kept this method on the user route in the first place. Filtering rather than + /// validating the token's login against costs no extra request + /// and still serves an organisation the token is merely a member of. + /// + /// + public override async Task> GetRepositoriesAsync(CancellationToken cancellationToken = default) + { + cancellationToken.ThrowIfCancellationRequested(); + + (GitHubClient client, IDisposable createdTransport) = CreateClient(); + using IDisposable transport = createdTransport; + + try + { + IReadOnlyList repositories = OwnerKind switch + { + GitHubOwnerKind.Organization => + await client.Repository.GetAllForOrg(Owner.WeakString).ConfigureAwait(false), + GitHubOwnerKind.AuthenticatedUser => + FilterToOwner(await client.Repository.GetAllForCurrent(AuthenticatedUserRequest).ConfigureAwait(false)), + _ => await client.Repository.GetAllForUser(Owner.WeakString).ConfigureAwait(false), + }; + + return [.. repositories.Select(ToGitRepository)]; + } + catch (ApiException exception) + { + throw Translate(exception); + } + } + + /// + /// The request enumerates with. + /// + /// + /// The affiliation is stated explicitly rather than left to GitHub's default, for the reason the + /// pull request listing states its own filter: this provider's coverage is defined by this + /// library, not by restating whichever default a vendor happens to ship today. + /// + private static RepositoryRequest AuthenticatedUserRequest => new() + { + Affiliation = RepositoryAffiliation.OwnerAndOrganizationMember, + }; + + /// + /// Drops repositories belonging to anyone but . + /// + /// + /// Only needs this: the other two routes carry + /// the owner in the request path and cannot answer for anybody else. Compared with + /// because GitHub logins are case-insensitive. + /// + /// Everything the route reported. + /// The subset this provider's owner has. + private IReadOnlyList FilterToOwner(IReadOnlyList repositories) => + [.. repositories.Where(repository => + string.Equals(repository.Owner?.Login, Owner.WeakString, StringComparison.OrdinalIgnoreCase))]; +``` + +Verify `RepositoryAffiliation.OwnerAndOrganizationMember` is the exact Octokit 14.0.0 member name before building: + +```bash +grep -o 'name="F:Octokit.RepositoryAffiliation[^"]*"' ~/.nuget/packages/octokit/14.0.0/lib/netstandard2.0/Octokit.xml +``` + +If the member is named differently, use the name that grep reports and keep the query-string assertion in the test aligned with whatever Octokit then sends. + +- [ ] **Step 6: Run the tests to verify they pass** + +Run: `dotnet test --filter "FullyQualifiedName~GitHubProviderTests"` +Expected: PASS, including every pre-existing test in that class unchanged. + +- [ ] **Step 7: Commit** + +```bash +git add GitIntegration/Models/GitEnums.cs GitIntegration/GitHubProvider.cs GitIntegration.Test/Hosting/GitHubProviderTests.cs GitIntegration.Test/Fixtures/github-org-repositories.json GitIntegration.Test/GitIntegration.Test.csproj +git commit -m "feat: route GitHub repository enumeration by owner kind" +``` + +--- + +## Task 6: The single sign-on authorisation URL + +**Files:** +- Modify: `GitIntegration/GitHubProvider.cs` +- Test: `GitIntegration.Test/Hosting/GitHubProviderTests.cs` + +**Interfaces:** +- Consumes: `GitHubProvider.Translate(Octokit.ApiException)`, `GitHubProvider.TryGetRetryAfterSeconds(Octokit.ApiException)` as the pattern for reading a header, `GitHostingAuthenticationException`. +- Produces: no new public surface. `Translate` returns the same exception types; only an authentication failure's message changes when the header is present. + +### Background + +A token that is valid but not authorised for an organisation's SAML single sign-on gets `403` with an `X-GitHub-SSO` header. Its value looks like `required; url=https://github.com/orgs/contoso/sso?authorization_request=ABC123`. Without surfacing that URL, "bad token" and "good token one click from working" are the same exception with the same text. + +- [ ] **Step 1: Write the failing tests** + +Append to `GitIntegration.Test/Hosting/GitHubProviderTests.cs`, inside the class: + +```csharp + [TestMethod] + public async Task ReportsTheSingleSignOnAuthorisationUrlOnAForbiddenResponse() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond( + HttpStatusCode.Forbidden, + "{\"message\":\"Resource protected by organization SAML enforcement.\"}", + ("Content-Type", "application/json"), + ("X-GitHub-SSO", "required; url=https://github.com/orgs/contoso/sso?authorization_request=ABC123")); + GitHubProvider provider = new() + { + Owner = "contoso".As(), + OwnerKind = GitHubOwnerKind.Organization, + Handler = handler, + }; + + GitHostingAuthenticationException exception = + await Assert.ThrowsExactlyAsync( + async () => await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false)) + .ConfigureAwait(false); + + StringAssert.Contains(exception.Message, "https://github.com/orgs/contoso/sso?authorization_request=ABC123"); + } + + [TestMethod] + public async Task StillReportsAForbiddenResponseCarryingNoSingleSignOnHeader() + { + // The URL is an addition to the message, never a requirement for classifying the failure. + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond( + HttpStatusCode.Forbidden, + "{\"message\":\"Bad credentials\"}", + ("Content-Type", "application/json")); + GitHubProvider provider = new() { Owner = "contoso".As(), Handler = handler }; + + _ = await Assert.ThrowsExactlyAsync( + async () => await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false)) + .ConfigureAwait(false); + } +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `dotnet test --filter "FullyQualifiedName~GitHubProviderTests"` +Expected: the first test FAILS, the message contains no URL. The second passes already. + +- [ ] **Step 3: Add the header reader** + +`Translate` is a switch expression at `GitIntegration/GitHubProvider.cs:424`, and `TryGetRetryAfterSeconds` sits just below it. The header reader follows that method's pattern exactly: headers are scanned case-insensitively rather than looked up by key, because HTTP header names are case-insensitive while Octokit's header dictionary compares ordinally. + +Add to `GitIntegration/GitHubProvider.cs`, next to `TryGetRetryAfterSeconds`: + +```csharp + /// + /// Reads the authorisation URL from a failed response's single sign-on header, if it carries one. + /// + /// + /// Scanned case-insensitively rather than looked up by key, for the reason + /// gives: HTTP header names are case-insensitive and + /// Octokit's header dictionary compares them ordinally, so a keyed lookup would work only by + /// matching whatever casing Octokit happens to canonicalise this header to. + /// + /// The header's value is a parameter list, required; url=<uri>. Only the URL is read, + /// and anything else in the list is left alone: the caller's whole use for this is a link to open. + /// + /// + /// The failed response. + /// The authorisation URL, or when the header is absent or carries none. + private static string? TryGetSingleSignOnUrl(ApiException exception) + { + const string headerName = "X-GitHub-SSO"; + const string urlParameter = "url="; + + if (exception.HttpResponse?.Headers is not IReadOnlyDictionary headers) + { + return null; + } + + foreach (KeyValuePair header in headers.Where( + candidate => candidate.Key.Equals(headerName, StringComparison.OrdinalIgnoreCase))) + { + foreach (string parameter in header.Value.Split(';')) + { + string trimmed = parameter.Trim(); + + if (trimmed.StartsWith(urlParameter, StringComparison.OrdinalIgnoreCase)) + { + string url = trimmed[urlParameter.Length..]; + return url.Length == 0 ? null : url; + } + } + } + + return null; + } +``` + +- [ ] **Step 4: Wire it into Translate** + +`Translate`'s arms each pass `exception.Message`, so the amended message is computed once before the switch and used by the two authentication arms only. Add below the existing `responseBody` line: + +```csharp + // A token that is valid but unauthorised for an organisation's single sign-on arrives as a + // plain 403, indistinguishable in status and body from a bad credential. The header is the + // only thing carrying the URL that resolves it, and that URL is the whole remedy — without + // it, the two failures a caller most needs to tell apart read identically. + string? singleSignOnUrl = TryGetSingleSignOnUrl(exception); + string authenticationMessage = singleSignOnUrl is null + ? exception.Message + : $"{exception.Message} This organisation requires single sign-on authorisation for " + + $"this credential. Authorise it at: {singleSignOnUrl}"; +``` + +Then change only these two arms, leaving every other arm and the arm ordering untouched. The ordering of the rate-limit, authentication and not-found arms is load-bearing and documented as such in the method's remarks: + +```csharp + AuthorizationException => new GitHostingAuthenticationException(authenticationMessage, Name, exception.StatusCode, responseBody, exception), + ForbiddenException => new GitHostingAuthenticationException(authenticationMessage, Name, exception.StatusCode, responseBody, exception), +``` + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `dotnet test --filter "FullyQualifiedName~GitHubProviderTests"` +Expected: PASS. + +- [ ] **Step 6: Run the full suite** + +Run: `dotnet test` +Expected: PASS. + +- [ ] **Step 7: Commit** + +```bash +git add GitIntegration/GitHubProvider.cs GitIntegration.Test/Hosting/GitHubProviderTests.cs +git commit -m "feat: report the single sign-on authorisation url on a forbidden response" +``` + +--- + +## Task 7: The GitHub device flow + +**Files:** +- Modify: `GitIntegration/SemanticTypes/GitProviderTypes.cs` +- Create: `GitIntegration/Hosting/GitHubDeviceFlow.cs` +- Test: `GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs` + +**Interfaces:** +- Consumes: `HostingCredential.FromToken(string)`, `GitHostingAuthenticationException`, `GitHostingRequestException`, `GitProvider.CreateDefaultHandler()` as the pattern for a shared transport, `FakeHttpMessageHandler`. +- Produces: `GitHubOAuthClientId` semantic string; `GitHubDeviceCode` record with `DeviceCode`, `UserCode`, `VerificationUri`, `ExpiresIn`, `Interval`; `GitHubDeviceFlow(GitHubOAuthClientId clientId, IReadOnlyList scopes)` with `RequestDeviceCodeAsync` and `WaitForTokenAsync`. + +### Correction to the spec + +The spec's `GitHubDeviceCode` omitted the **device code** itself. `Octokit.IOauthClient.CreateAccessTokenForDeviceFlow(string clientId, OauthDeviceFlowResponse response, CancellationToken)` needs that value to resume, and it is distinct from the user code: the user code is the short string a human types, the device code is the opaque one the polling request carries. `GitHubDeviceCode` therefore carries both. Correct the spec's record in the same commit. + +### Confirmed Octokit 14.0.0 surface + +- `IOauthClient.InitiateDeviceFlow(OauthDeviceFlowRequest, CancellationToken)` returns `Task`. +- `OauthDeviceFlowRequest(string clientId)` with a `Scopes` collection. +- `OauthDeviceFlowResponse` carries `DeviceCode`, `UserCode`, `VerificationUri`, `ExpiresIn`, `Interval`. +- `IOauthClient.CreateAccessTokenForDeviceFlow(string, OauthDeviceFlowResponse, CancellationToken)` returns `Task`, and its documentation states it "will poll the access token endpoint, until the device and user codes expire or the user has successfully authorized the app". +- `OauthToken` carries `AccessToken`, `Error`, `ErrorDescription`. +- `GitHubClient.Oauth` exposes the client. + +`ExpiresIn` and `Interval` are integer seconds and are converted to `TimeSpan` at this library's boundary. + +- [ ] **Step 1: Write the failing tests** + +Create `GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs`: + +```csharp +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration.Test; + +using System; +using System.Net; +using System.Threading.Tasks; + +using ktsu.Semantics.Strings; + +[TestClass] +public sealed class GitHubDeviceFlowTests +{ + public TestContext TestContext { get; set; } = null!; + + private const string DeviceCodeBody = + "{\"device_code\":\"dev-abc\",\"user_code\":\"WXYZ-1234\"," + + "\"verification_uri\":\"https://github.com/login/device\"," + + "\"expires_in\":900,\"interval\":5}"; + + private static GitHubDeviceFlow CreateFlow(FakeHttpMessageHandler handler) => + new("Iv1.0123456789abcdef".As(), ["repo", "read:org"]) { Handler = handler }; + + [TestMethod] + public async Task RequestsADeviceCodeAndReportsWhatTheUserNeeds() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + + GitHubDeviceCode code = await CreateFlow(handler) + .RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual("WXYZ-1234", code.UserCode); + Assert.AreEqual("dev-abc", code.DeviceCode); + Assert.AreEqual(new Uri("https://github.com/login/device"), code.VerificationUri); + Assert.AreEqual(TimeSpan.FromSeconds(900), code.ExpiresIn); + Assert.AreEqual(TimeSpan.FromSeconds(5), code.Interval); + } + + [TestMethod] + public async Task SendsTheRequestedScopes() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + + _ = await CreateFlow(handler) + .RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + StringAssert.Contains(handler.Requests[0].Body, "repo"); + StringAssert.Contains(handler.Requests[0].Body, "read:org"); + } + + [TestMethod] + public async Task ReturnsAHostNativeTokenOnSuccess() + { + // FromToken, not FromBearerToken: a GitHub OAuth token travels under Octokit's Token scheme. + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + _ = handler.Respond( + HttpStatusCode.OK, + "{\"access_token\":\"gho_realtoken\",\"token_type\":\"bearer\",\"scope\":\"repo,read:org\"}", + ("Content-Type", "application/json")); + + GitHubDeviceFlow flow = CreateFlow(handler); + GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + HostingCredential credential = await flow.WaitForTokenAsync(code, TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual(HostingCredentialKind.Token, credential.Kind); + Assert.AreEqual("gho_realtoken", credential.Token); + } + + [TestMethod] + public async Task ReportsARefusedAuthorisationAsAnAuthenticationFailure() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + _ = handler.Respond( + HttpStatusCode.OK, + "{\"error\":\"access_denied\",\"error_description\":\"The user denied the request.\"}", + ("Content-Type", "application/json")); + + GitHubDeviceFlow flow = CreateFlow(handler); + GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + GitHostingAuthenticationException exception = + await Assert.ThrowsExactlyAsync( + async () => await flow.WaitForTokenAsync(code, TestContext.CancellationTokenSource.Token).ConfigureAwait(false)) + .ConfigureAwait(false); + + StringAssert.Contains(exception.Message, "access_denied"); + } + + [TestMethod] + public async Task ReportsAnExpiredCodeAsAnAuthenticationFailure() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + _ = handler.Respond( + HttpStatusCode.OK, + "{\"error\":\"expired_token\",\"error_description\":\"The device code has expired.\"}", + ("Content-Type", "application/json")); + + GitHubDeviceFlow flow = CreateFlow(handler); + GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + _ = await Assert.ThrowsExactlyAsync( + async () => await flow.WaitForTokenAsync(code, TestContext.CancellationTokenSource.Token).ConfigureAwait(false)) + .ConfigureAwait(false); + } + + [TestMethod] + public async Task ReportsAnUnusableClientIdentifierAsARequestFailure() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond( + HttpStatusCode.NotFound, + "{\"error\":\"Not Found\"}", + ("Content-Type", "application/json")); + + _ = await Assert.ThrowsExactlyAsync( + async () => await CreateFlow(handler) + .RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false)) + .ConfigureAwait(false); + } + + [TestMethod] + public void RejectsANullClientIdentifier() => + _ = Assert.ThrowsExactly(() => new GitHubDeviceFlow(null!, ["repo"])); + + [TestMethod] + public void RejectsNullScopes() => + _ = Assert.ThrowsExactly( + () => new GitHubDeviceFlow("Iv1.0123456789abcdef".As(), null!)); +} +``` + +Check whether `FakeHttpMessageHandler`'s `RecordedRequest` exposes a `Body` property (grep: `grep -n "record RecordedRequest\|public.*Body" GitIntegration.Test/Fakes/FakeHttpMessageHandler.cs`). If it does not, add one capturing the request content as a string, and cover it with a test in `FakeHttpMessageHandlerTests` in this same task. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `dotnet test --filter "FullyQualifiedName~GitHubDeviceFlowTests"` +Expected: compile failure, `GitHubDeviceFlow` does not exist. + +- [ ] **Step 3: Add the semantic type** + +Append to `GitIntegration/SemanticTypes/GitProviderTypes.cs`: + +```csharp +/// +/// The client identifier GitHub issues when an OAuth App is registered. +/// +/// +/// Not a secret. The device flow has no client secret precisely because a desktop binary cannot keep +/// one, so a consuming application may hold this in ordinary configuration. It is a value this +/// library takes rather than one it ships: the identifier belongs to whoever registered the app. +/// +[HasNonWhitespaceContent] +public sealed record GitHubOAuthClientId : SemanticString { } +``` + +- [ ] **Step 4: Write the flow** + +Create `GitIntegration/Hosting/GitHubDeviceFlow.cs`: + +```csharp +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration; + +using System; +using System.Collections.Generic; +using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; + +using Octokit; +using Octokit.Internal; + +/// +/// What GitHub issues when a device flow begins: the code a human types, and the code the polling +/// request carries. +/// +/// +/// and are different values for different audiences. +/// The user code is short and is displayed; the device code is opaque and is what +/// sends. Showing the device code to a user, or +/// sending the user code in its place, both fail in ways that look like a broken sign-in. +/// +public sealed record GitHubDeviceCode +{ + /// Gets the code the user enters at . + public required string UserCode { get; init; } + + /// Gets the code the token request carries. Not shown to the user. + public required string DeviceCode { get; init; } + + /// Gets the page the user opens to enter . + public required Uri VerificationUri { get; init; } + + /// Gets how long this code remains usable. + /// Surfaced because a caller showing a countdown needs it and cannot derive it. + public required TimeSpan ExpiresIn { get; init; } + + /// Gets the minimum wait GitHub requires between token requests. + public required TimeSpan Interval { get; init; } +} + +/// +/// Obtains a GitHub credential through the OAuth device flow. +/// +/// +/// +/// Two calls rather than one. Device flow has an inherent pause in the middle — GitHub issues a +/// short code, the user types it into a browser, and only then does polling succeed — and that pause +/// is minutes long with the code on screen throughout. A single method taking a "here's the code" +/// callback would invoke it from whatever thread an HTTP continuation resumed on, leaving every +/// graphical caller to marshal the code back to a user interface thread from inside a callback it +/// does not control. Splitting the call puts the seam where the pause already is. +/// +/// +/// Separate from , whose every other method is one request and one +/// response. Nothing here resolves or applies a credential; this type produces one and +/// consumes one, through the credential cache or +/// . +/// +/// +/// Stores nothing. returns the credential and the caller decides +/// where it lives: +/// +/// +/// GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(cancellationToken); +/// // show code.UserCode and open code.VerificationUri +/// HostingCredential credential = await flow.WaitForTokenAsync(code, cancellationToken); +/// CredentialCache.Instance.AddOrReplace(persona, new CredentialWithToken { Token = credential.Token! }); +/// +/// +/// The OAuth App's client identifier. +/// The scopes to request, such as repo and read:org. +public sealed class GitHubDeviceFlow(GitHubOAuthClientId clientId, IReadOnlyList scopes) +{ + /// + /// The transport every flow shares when no was injected. + /// + /// + /// Its own instance rather than 's, matching that type's reasoning: + /// one handler for the process rather than one per call, and + /// is what keeps the settings from drifting apart. + /// + private static readonly SocketsHttpHandler SharedHandler = GitProvider.CreateDefaultHandler(); + + private readonly GitHubOAuthClientId _clientId = Ensure.NotNull(clientId); + private readonly IReadOnlyList _scopes = Ensure.NotNull(scopes); + + /// + /// Gets or initializes the transport this flow issues requests through, or + /// to use the shared one. + /// + /// + /// Internal rather than public, so no transport type appears in this library's public API, and + /// the test project injects a fake through InternalsVisibleTo — the same seam + /// provides. + /// + internal HttpMessageHandler? Handler { get; init; } + + /// + /// Asks GitHub to begin a device flow. + /// + /// Cancels the request. + /// The codes and timings the flow's second half needs. + /// GitHub refused or could not be reached. + public async Task RequestDeviceCodeAsync(CancellationToken cancellationToken = default) + { + cancellationToken.ThrowIfCancellationRequested(); + + GitHubClient client = CreateClient(); + + OauthDeviceFlowRequest request = new(_clientId.WeakString); + + foreach (string scope in _scopes) + { + request.Scopes.Add(scope); + } + + try + { + OauthDeviceFlowResponse response = + await client.Oauth.InitiateDeviceFlow(request, cancellationToken).ConfigureAwait(false); + + return new GitHubDeviceCode + { + UserCode = response.UserCode, + DeviceCode = response.DeviceCode, + VerificationUri = new Uri(response.VerificationUri), + // GitHub reports both as integer seconds; the conversion happens once, here. + ExpiresIn = TimeSpan.FromSeconds(response.ExpiresIn), + Interval = TimeSpan.FromSeconds(response.Interval), + }; + } + catch (ApiException exception) + { + throw new GitHostingRequestException( + $"GitHub refused to begin a device flow: {exception.Message}", exception); + } + } + + /// + /// Waits for the user to authorise the flow, then returns the credential GitHub issues. + /// + /// + /// Polls until the user authorises, the code expires, or is + /// cancelled, so this may block for as long as . Octokit + /// handles the authorization_pending and slow_down responses internally, at the + /// interval GitHub asked for. + /// + /// What returned. + /// Abandons the wait. + /// A host-native token credential. + /// is . + /// The user refused, or the code expired. + /// GitHub refused the request or could not be reached. + public async Task WaitForTokenAsync(GitHubDeviceCode code, CancellationToken cancellationToken = default) + { + Ensure.NotNull(code); + cancellationToken.ThrowIfCancellationRequested(); + + GitHubClient client = CreateClient(); + + // Octokit resumes from its own response type rather than from ours, so the fields it needs + // are handed back in the shape it expects. Only DeviceCode and Interval are read on this + // path; the rest are set so the value is not half-populated if Octokit's use ever widens. + OauthDeviceFlowResponse response = new() + { + DeviceCode = code.DeviceCode, + UserCode = code.UserCode, + VerificationUri = code.VerificationUri.ToString(), + ExpiresIn = (int)code.ExpiresIn.TotalSeconds, + Interval = (int)code.Interval.TotalSeconds, + }; + + OauthToken token; + + try + { + token = await client.Oauth + .CreateAccessTokenForDeviceFlow(_clientId.WeakString, response, cancellationToken) + .ConfigureAwait(false); + } + catch (ApiException exception) + { + throw new GitHostingRequestException( + $"GitHub refused the device flow token request: {exception.Message}", exception); + } + + // GitHub reports a refusal and an expiry as a 200 carrying an error field rather than as a + // failure status, so this is checked before the token is read rather than caught above. + if (!string.IsNullOrEmpty(token.Error)) + { + throw new GitHostingAuthenticationException( + $"GitHub did not issue a token: {token.Error}. {token.ErrorDescription}".TrimEnd()); + } + + return string.IsNullOrEmpty(token.AccessToken) + ? throw new GitHostingRequestException("GitHub reported neither a token nor an error.") + // FromToken, not FromBearerToken: a GitHub OAuth token travels under Octokit's Token + // scheme, which is what FromToken means. FromBearerToken is for an Entra ID access token + // against Azure DevOps. + : HostingCredential.FromToken(token.AccessToken); + } + + /// + /// Creates an Octokit client wired to this flow's transport. + /// + /// + /// Unauthenticated by construction: obtaining a credential is what this type is for, so it has + /// none to send. The handler is wrapped so that disposing the client never disposes a transport + /// this type does not own — outlives every call, and an injected one + /// belongs to whoever supplied it. + /// + /// The client. + private GitHubClient CreateClient() => + new(new ProductHeaderValue("ktsu-GitIntegration"), + new HttpClientAdapter(() => new GitHubProvider.NonOwningHandler(Handler ?? SharedHandler))); +} +``` + +Three things to verify while implementing, because they are read from the package's shape rather than from its source: + +1. `GitProvider.CreateDefaultHandler` and `GitHubProvider.NonOwningHandler` are both currently `private` or `private protected`. Widen each to `internal` (or `private protected` to `internal`) so this type can reach them, and note the widening in the commit message. Do not make either public. +2. `OauthDeviceFlowResponse`'s properties may have no public setters. If they do not, construct it through whichever constructor Octokit exposes (`grep -o 'name="M:Octokit.OauthDeviceFlowResponse[^"]*"' ~/.nuget/packages/octokit/14.0.0/lib/netstandard2.0/Octokit.xml`). If none is usable, replace this type's two Octokit calls with direct posts to `https://github.com/login/device/code` and `https://github.com/login/oauth/access_token` with `Accept: application/json`, polling at `Interval` and widening by five seconds on each `slow_down`, and keep every test in Step 1 unchanged. + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `dotnet test --filter "FullyQualifiedName~GitHubDeviceFlowTests"` +Expected: PASS. + +- [ ] **Step 6: Correct the spec's record** + +In `docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md`, add `DeviceCode` to the `GitHubDeviceCode` listing in the Device flow section, with a sentence saying it is the opaque code the polling request carries, distinct from the user code. + +- [ ] **Step 7: Run the full suite** + +Run: `dotnet test` +Expected: PASS. + +- [ ] **Step 8: Commit** + +```bash +git add GitIntegration/Hosting/GitHubDeviceFlow.cs GitIntegration/SemanticTypes/GitProviderTypes.cs GitIntegration/GitProvider.cs GitIntegration/GitHubProvider.cs GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs GitIntegration.Test/Fakes/FakeHttpMessageHandler.cs docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md +git commit -m "feat: obtain a GitHub credential through the OAuth device flow" +``` + +--- + +## Task 8: Documentation + +**Files:** +- Modify: `README.md` +- Modify: `CHANGELOG.md` +- Modify: `VERSION.md` + +**Interfaces:** +- Consumes: every public member added in Tasks 1 through 7. +- Produces: nothing code depends on. + +- [ ] **Step 1: Check how the version and changelog are maintained** + +Run: `head -30 CHANGELOG.md && cat VERSION.md && grep -rn "version" .github/workflows/*.yml 2>/dev/null | head` + +The ktsu.Sdk build generates some metadata automatically. If `CHANGELOG.md` and `VERSION.md` carry a `[bot]` provenance in `git log`, they are generated: leave both alone and say so in the pull request instead of editing them. + +- [ ] **Step 2: Update the features list** + +In `README.md`, extend the existing **Features** bullets: + +- Add `Worktrees()`, `AddWorktree(...)`, `RemoveWorktree(...)` and `PruneWorktrees()` to the **Fluent Verb Builders** bullet's verb lists, read-only and mutating respectively. +- Add `GitWorktree` to the **Strongly-Typed Results** bullet. +- Extend the **Hosting Provider Abstraction** bullet to say that `GitHubProvider` routes enumeration by `OwnerKind`, and that only `Organization` and `AuthenticatedUser` report private repositories. +- Add a new bullet: **Interactive GitHub Sign-In** — `GitHubDeviceFlow` obtains a credential through GitHub's OAuth device flow, in two calls so a caller can display the user code while the wait runs. + +- [ ] **Step 3: Add usage examples** + +In `README.md`, after the existing **Listing Commits and Diffs** example, add: + +````markdown +### One Worktree per Branch + +```csharp +using ktsu.GitIntegration; +using ktsu.Semantics.Paths; +using ktsu.Semantics.Strings; + +IReadOnlyList worktrees = await repository.Worktrees().ExecuteAsync(); + +GitBranchName branch = "feature/search".As(); + +if (!worktrees.Any(worktree => worktree.Branch == branch)) +{ + AbsoluteDirectoryPath destination = "/repos/project-feature-search".As(); + + await repository.AddWorktree(destination) + .CreatingBranch(branch) + .From("origin/main".As()) + .ExecuteAsync(); +} +``` + +The main working tree reports `IsMain`, which is how a caller refuses to remove the one that owns the +repository. It is positional — git emits it first — rather than an attribute git labels. + +### Signing In to GitHub + +```csharp +using ktsu.CredentialCache; +using ktsu.GitIntegration; +using ktsu.Semantics.Strings; + +GitHubDeviceFlow flow = new("Iv1.0123456789abcdef".As(), ["repo", "read:org"]); + +GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(); +Console.WriteLine($"Open {code.VerificationUri} and enter {code.UserCode}"); + +HostingCredential credential = await flow.WaitForTokenAsync(code); +CredentialCache.Instance.AddOrReplace(persona, new CredentialWithToken { Token = credential.Token! }); +``` + +The two calls are split so the user code can stay on screen for the minutes the wait may take. The +flow stores nothing: where the credential lives is the caller's decision. + +### Enumerating an Organisation's Private Repositories + +```csharp +GitHubProvider provider = new() +{ + Owner = "contoso".As(), + OwnerKind = GitHubOwnerKind.Organization, + PersonaGUID = persona, +}; + +IReadOnlyList repositories = await provider.GetRepositoriesAsync(); +``` + +`OwnerKind` defaults to `GitHubOwnerKind.User`, which enumerates public repositories only — the route +this provider has always used. `Organization` and `AuthenticatedUser` report private repositories the +credential can see. A token that is valid but not authorised for an organisation's single sign-on +raises `GitHostingAuthenticationException` carrying the URL to authorise it at. +```` + +- [ ] **Step 4: Verify the documentation builds** + +Run: `dotnet build` +Expected: no warnings about missing or malformed XML documentation. + +- [ ] **Step 5: Run the full suite one last time** + +Run: `dotnet test` +Expected: PASS, every test. + +- [ ] **Step 6: Commit** + +```bash +git add README.md +git commit -m "docs: document worktree verbs, owner kinds, and device flow" +``` + +--- + +## Open item, carried out of this plan + +**The OAuth App does not exist yet.** Every test here runs against a fake transport, so no task is blocked. But nothing has been confirmed against GitHub itself, and it cannot be until someone with organisation ownership registers an OAuth App with device flow enabled and approves it for SAML single sign-on, scopes `repo` and `read:org`. + +Record this in the pull request description as an unchecked item rather than claiming the device flow is verified end to end. From 1fe6ab04f33f8f17851d85da3bb569ee6b8d3396 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Tue, 22 Sep 2026 10:12:11 +1000 Subject: [PATCH 03/16] feat: read git worktree list --porcelain into a typed model Co-Authored-By: Claude Sonnet 5 --- .../Parsing/GitWorktreeParserTests.cs | 205 ++++++++++++++++++ GitIntegration/Models/GitWorktree.cs | 77 +++++++ GitIntegration/Parsing/GitParseValues.cs | 35 +++ GitIntegration/Parsing/GitWorktreeParser.cs | 146 +++++++++++++ 4 files changed, 463 insertions(+) create mode 100644 GitIntegration.Test/Parsing/GitWorktreeParserTests.cs create mode 100644 GitIntegration/Models/GitWorktree.cs create mode 100644 GitIntegration/Parsing/GitWorktreeParser.cs diff --git a/GitIntegration.Test/Parsing/GitWorktreeParserTests.cs b/GitIntegration.Test/Parsing/GitWorktreeParserTests.cs new file mode 100644 index 0000000..45e0f5f --- /dev/null +++ b/GitIntegration.Test/Parsing/GitWorktreeParserTests.cs @@ -0,0 +1,205 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration.Test; + +using System; +using System.Collections.Generic; + +using ktsu.Semantics.Strings; + +[TestClass] +public sealed class GitWorktreeParserTests +{ + private static readonly string MainOnly = + $"worktree {AbsolutePath("project")}\n" + + "HEAD 7f3c9a1b2d4e6f8a0c2e4a6b8d0f2a4c6e8b0d2f\n" + + "branch refs/heads/main\n" + + "\n"; + + private static readonly string ThreeWorktrees = + $"worktree {AbsolutePath("project")}\n" + + "HEAD 7f3c9a1b2d4e6f8a0c2e4a6b8d0f2a4c6e8b0d2f\n" + + "branch refs/heads/main\n" + + "\n" + + $"worktree {AbsolutePath("project-feature")}\n" + + "HEAD 2b8e4d0f6a8c0e2a4c6e8b0d2f4a6c8e0b2d4f6a\n" + + "branch refs/heads/feature\n" + + "locked contains uncommitted experiment\n" + + "\n" + + $"worktree {AbsolutePath("project-review")}\n" + + "HEAD 9c1a7f2e4b6d8a0c2e4f6a8b0d2c4e6f8a0b2d4c\n" + + "detached\n" + + "prunable gitdir file points to non-existent location\n" + + "\n"; + + /// Builds a platform-appropriate absolute directory path from a simple name, for fixtures. + private static string AbsolutePath(string name) => + OperatingSystem.IsWindows() ? $@"C:\{name}" : $"/{name}"; + + [TestMethod] + public void ReadsTheOnlyWorktreeAsTheMainOne() + { + IReadOnlyList worktrees = GitWorktreeParser.Parse(MainOnly); + + Assert.AreEqual(1, worktrees.Count); + Assert.AreEqual(AbsolutePath("project"), worktrees[0].Path.WeakString); + Assert.AreEqual("7f3c9a1b2d4e6f8a0c2e4a6b8d0f2a4c6e8b0d2f".As(), worktrees[0].Head); + Assert.AreEqual("main".As(), worktrees[0].Branch); + Assert.IsTrue(worktrees[0].IsMain); + Assert.IsFalse(worktrees[0].IsBare); + Assert.IsFalse(worktrees[0].IsDetached); + Assert.IsFalse(worktrees[0].IsLocked); + Assert.IsFalse(worktrees[0].IsPrunable); + } + + [TestMethod] + public void StripsTheRefsHeadsPrefixFromTheBranch() + { + // The hosting half of this library already hands back bare branch names. A caller should not + // have to know which half produced a name to know its shape. + IReadOnlyList worktrees = GitWorktreeParser.Parse(MainOnly); + + Assert.AreEqual("main".As(), worktrees[0].Branch); + } + + [TestMethod] + public void MarksOnlyTheFirstRecordAsMain() + { + IReadOnlyList worktrees = GitWorktreeParser.Parse(ThreeWorktrees); + + Assert.AreEqual(3, worktrees.Count); + Assert.IsTrue(worktrees[0].IsMain); + Assert.IsFalse(worktrees[1].IsMain); + Assert.IsFalse(worktrees[2].IsMain); + } + + [TestMethod] + public void ReadsALockReasonWhenGitGivesOne() + { + IReadOnlyList worktrees = GitWorktreeParser.Parse(ThreeWorktrees); + + Assert.IsTrue(worktrees[1].IsLocked); + Assert.AreEqual("contains uncommitted experiment", worktrees[1].LockReason); + } + + [TestMethod] + public void ReportsALockWithNoReasonAsLockedWithoutOne() + { + // "locked" alone and "locked " are different facts, and a caller showing the reason + // must be able to tell "no reason recorded" from "not locked". + string output = + $"worktree {AbsolutePath("project")}\n" + + "HEAD 7f3c9a1b2d4e6f8a0c2e4a6b8d0f2a4c6e8b0d2f\n" + + "branch refs/heads/main\n" + + "locked\n" + + "\n"; + + IReadOnlyList worktrees = GitWorktreeParser.Parse(output); + + Assert.IsTrue(worktrees[0].IsLocked); + Assert.IsNull(worktrees[0].LockReason); + } + + [TestMethod] + public void ReadsADetachedWorktreeWithNoBranch() + { + IReadOnlyList worktrees = GitWorktreeParser.Parse(ThreeWorktrees); + + Assert.IsTrue(worktrees[2].IsDetached); + Assert.IsNull(worktrees[2].Branch); + Assert.AreEqual("9c1a7f2e4b6d8a0c2e4f6a8b0d2c4e6f8a0b2d4c".As(), worktrees[2].Head); + } + + [TestMethod] + public void ReadsAPrunableReason() + { + IReadOnlyList worktrees = GitWorktreeParser.Parse(ThreeWorktrees); + + Assert.IsTrue(worktrees[2].IsPrunable); + Assert.AreEqual("gitdir file points to non-existent location", worktrees[2].PrunableReason); + } + + [TestMethod] + public void ReadsABareWorktreeWithNeitherHeadNorBranch() + { + string output = + $"worktree {AbsolutePath("project-bare")}\n" + + "bare\n" + + "\n"; + + IReadOnlyList worktrees = GitWorktreeParser.Parse(output); + + Assert.AreEqual(1, worktrees.Count); + Assert.IsTrue(worktrees[0].IsBare); + Assert.IsNull(worktrees[0].Head); + Assert.IsNull(worktrees[0].Branch); + } + + [TestMethod] + public void DoesNotInventARecordFromTheTrailingBlankLine() + { + // --porcelain emits a blank line after the last record as well as between records. A naive + // split on the separator turns that into a phantom entry. + IReadOnlyList worktrees = GitWorktreeParser.Parse(ThreeWorktrees); + + Assert.AreEqual(3, worktrees.Count); + } + + [TestMethod] + public void ReadsRecordsSeparatedByCarriageReturnLineFeeds() + { + IReadOnlyList worktrees = GitWorktreeParser.Parse(MainOnly.Replace("\n", "\r\n", StringComparison.Ordinal)); + + Assert.AreEqual(1, worktrees.Count); + Assert.AreEqual("main".As(), worktrees[0].Branch); + } + + [TestMethod] + public void ReadsNothingFromEmptyOutput() + { + Assert.AreEqual(0, GitWorktreeParser.Parse(string.Empty).Count); + } + + [TestMethod] + public void RejectsARecordWithNoWorktreePath() + { + // Every attribute is optional except the one naming what the record describes. + string output = + "HEAD 7f3c9a1b2d4e6f8a0c2e4a6b8d0f2a4c6e8b0d2f\n" + + "branch refs/heads/main\n" + + "\n"; + + _ = Assert.ThrowsExactly(() => GitWorktreeParser.Parse(output)); + } + + [TestMethod] + public void IgnoresAnAttributeItDoesNotRecognise() + { + // Git may add attributes. An unknown one must not fail a listing that is otherwise readable. + string output = + $"worktree {AbsolutePath("project")}\n" + + "HEAD 7f3c9a1b2d4e6f8a0c2e4a6b8d0f2a4c6e8b0d2f\n" + + "branch refs/heads/main\n" + + "somethingnew value\n" + + "\n"; + + IReadOnlyList worktrees = GitWorktreeParser.Parse(output); + + Assert.AreEqual(1, worktrees.Count); + Assert.AreEqual("main".As(), worktrees[0].Branch); + } + + [TestMethod] + public void RejectsAWorktreePathThatIsNotAbsolute() + { + // AbsoluteDirectoryPath is expected to refuse a relative path, but ToAbsoluteDirectoryPath + // must hold that contract itself rather than merely trust the semantic type to enforce it. + string output = + "worktree relative/project\n" + + "HEAD 7f3c9a1b2d4e6f8a0c2e4a6b8d0f2a4c6e8b0d2f\n" + + "branch refs/heads/main\n" + + "\n"; + + _ = Assert.ThrowsExactly(() => GitWorktreeParser.Parse(output)); + } +} diff --git a/GitIntegration/Models/GitWorktree.cs b/GitIntegration/Models/GitWorktree.cs new file mode 100644 index 0000000..124daca --- /dev/null +++ b/GitIntegration/Models/GitWorktree.cs @@ -0,0 +1,77 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration; + +using ktsu.Semantics.Paths; + +/// +/// One working tree of a repository: where it is, and what it has checked out. +/// +/// +/// Inert data, like and . It carries no +/// and no process runner. A caller that wants to run a verb inside a +/// worktree passes to , which is the same +/// journey it would make from any other path. +/// +public sealed record GitWorktree +{ + /// Gets the worktree's own directory. + public required AbsoluteDirectoryPath Path { get; init; } + + /// + /// Gets the commit checked out here, or when git reported none. + /// + /// + /// Absent for a bare worktree, which has no checkout to report. Present for a detached one, + /// where it is the only thing identifying what is checked out. + /// + public GitCommitSha? Head { get; init; } + + /// + /// Gets the branch checked out here, or when there is none. + /// + /// + /// Absent for a bare or detached worktree. Reported as a bare name with git's + /// refs/heads/ prefix stripped, so that a branch name from this half of the library has + /// the same shape as one from the hosting half. + /// + public GitBranchName? Branch { get; init; } + + /// + /// Gets a value indicating whether this is the repository's main working tree. + /// + /// + /// Positional. Git's porcelain has no attribute for this: the main working tree is simply + /// the first record git emits, always. The parser sets this on the first record and on no other. + /// It exists because a caller managing worktrees needs to refuse to remove the one that owns the + /// repository, and answering that once here beats every caller reimplementing it. + /// + public bool IsMain { get; init; } + + /// Gets a value indicating whether this worktree has no working directory. + public bool IsBare { get; init; } + + /// Gets a value indicating whether this worktree has a commit checked out rather than a branch. + public bool IsDetached { get; init; } + + /// Gets a value indicating whether this worktree is locked against pruning. + public bool IsLocked { get; init; } + + /// + /// Gets the reason this worktree is locked, or when git recorded none. + /// + /// + /// Separately nullable from , because git emits locked both alone + /// and with a reason, and "locked for a reason nobody recorded" is a different fact from "not + /// locked". + /// + public string? LockReason { get; init; } + + /// Gets a value indicating whether git considers this worktree's record removable. + public bool IsPrunable { get; init; } + + /// + /// Gets the reason this worktree is prunable, or when git recorded none. + /// + public string? PrunableReason { get; init; } +} diff --git a/GitIntegration/Parsing/GitParseValues.cs b/GitIntegration/Parsing/GitParseValues.cs index 0ad0ee9..9aba8f3 100644 --- a/GitIntegration/Parsing/GitParseValues.cs +++ b/GitIntegration/Parsing/GitParseValues.cs @@ -2,6 +2,8 @@ namespace ktsu.GitIntegration; +using System.IO; + using ktsu.Semantics.Paths; using ktsu.Semantics.Strings; @@ -90,4 +92,37 @@ internal static RelativeDirectoryPath ToRelativeDirectoryPath(string value) throw new GitParseException( $"git reported a path that cannot be represented as a relative directory path: '{value}'."); } + + /// + /// Converts a raw path field into an absolute directory path. + /// + /// + /// The absolute counterpart to , for the fields git reports + /// as whole paths rather than as paths within a repository — a worktree's own directory is the + /// first of them. An empty field is a malformed record, and a path this type refuses is one git + /// produced and this library cannot represent, which is a parse failure rather than a value to + /// pass along unchecked. is checked explicitly rather + /// than left to 's own creation logic alone, so that this + /// method's documented refusal of a relative path holds even if that type's own validation ever + /// loosens. + /// + /// The raw path as git printed it. + /// The converted path. + /// + /// is empty, relative, or cannot be represented as an absolute + /// directory path. + /// + internal static AbsoluteDirectoryPath ToAbsoluteDirectoryPath(string value) + { + if (!string.IsNullOrEmpty(value) && + Path.IsPathRooted(value) && + AbsoluteDirectoryPath.TryCreate(value, out AbsoluteDirectoryPath? path) && + path is not null) + { + return path; + } + + throw new GitParseException( + $"git reported a path that cannot be represented as an absolute directory path: '{value}'."); + } } diff --git a/GitIntegration/Parsing/GitWorktreeParser.cs b/GitIntegration/Parsing/GitWorktreeParser.cs new file mode 100644 index 0000000..396b44c --- /dev/null +++ b/GitIntegration/Parsing/GitWorktreeParser.cs @@ -0,0 +1,146 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration; + +using System; +using System.Collections.Generic; + +/// +/// Reads git worktree list --porcelain. +/// +/// +/// The porcelain format is one record per worktree, records separated by a blank line and a blank +/// line after the last. Each line is an attribute name, optionally followed by a space and a value. +/// Only worktree is guaranteed present: a bare worktree reports neither HEAD nor +/// branch, and a detached one reports HEAD and detached but no branch. +/// +internal static class GitWorktreeParser +{ + private const string RefsHeadsPrefix = "refs/heads/"; + + /// + /// Parses a porcelain worktree listing. + /// + /// Everything git wrote to standard output. + /// The worktrees, in the order git listed them, the main one first. + /// A record named no worktree, or a field failed validation. + internal static IReadOnlyList Parse(string output) + { + Ensure.NotNull(output); + + List worktrees = []; + List record = []; + + foreach (string line in output.Split('\n')) + { + string entry = line.TrimEnd('\r'); + + if (entry.Length != 0) + { + record.Add(entry); + continue; + } + + // A blank line closes a record. Closing only a non-empty one is what keeps the trailing + // blank line git always emits from producing a phantom final entry. + if (record.Count != 0) + { + worktrees.Add(ParseRecord(record, isMain: worktrees.Count == 0)); + record.Clear(); + } + } + + // Output not ending in a blank line still closes its last record, so a listing stays readable + // if git ever stops emitting the trailing separator. + if (record.Count != 0) + { + worktrees.Add(ParseRecord(record, isMain: worktrees.Count == 0)); + } + + return worktrees; + } + + /// + /// Turns one record's attribute lines into a worktree. + /// + /// The record's lines, in git's order. + /// Whether this is the first record git emitted. + /// The parsed worktree. + /// The record named no worktree, or a field failed validation. + private static GitWorktree ParseRecord(IReadOnlyList record, bool isMain) + { + string? path = null; + GitCommitSha? head = null; + GitBranchName? branch = null; + bool isBare = false; + bool isDetached = false; + bool isLocked = false; + bool isPrunable = false; + string? lockReason = null; + string? prunableReason = null; + + foreach (string line in record) + { + int separator = line.IndexOf(' ', StringComparison.Ordinal); + string attribute = separator < 0 ? line : line[..separator]; + string value = separator < 0 ? string.Empty : line[(separator + 1)..]; + + switch (attribute) + { + case "worktree": + path = value; + break; + case "HEAD": + head = GitParseValues.ToSemantic(value, "commit id"); + break; + case "branch": + branch = GitParseValues.ToSemantic(StripRefsHeadsPrefix(value), "branch name"); + break; + case "bare": + isBare = true; + break; + case "detached": + isDetached = true; + break; + case "locked": + isLocked = true; + lockReason = value.Length == 0 ? null : value; + break; + case "prunable": + isPrunable = true; + prunableReason = value.Length == 0 ? null : value; + break; + default: + // Unknown attributes are skipped rather than rejected. Git may add one, and a + // listing that is otherwise readable should not fail over a field nobody reads. + break; + } + } + + return path is null + ? throw new GitParseException("git reported a worktree record naming no worktree path.") + : new GitWorktree + { + Path = GitParseValues.ToAbsoluteDirectoryPath(path), + Head = head, + Branch = branch, + IsMain = isMain, + IsBare = isBare, + IsDetached = isDetached, + IsLocked = isLocked, + LockReason = lockReason, + IsPrunable = isPrunable, + PrunableReason = prunableReason, + }; + } + + /// + /// Strips a leading refs/heads/, leaving the value untouched when the prefix is absent. + /// + /// The reference as git printed it. + /// The bare branch name. + private static string StripRefsHeadsPrefix(string reference) => + reference.StartsWith(RefsHeadsPrefix, StringComparison.Ordinal) + ? reference[RefsHeadsPrefix.Length..] + : reference; +} From c2a32cd8147510ae1bed736cec8fd1a27adec828 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Tue, 22 Sep 2026 10:14:15 +1000 Subject: [PATCH 04/16] chore: stop ignoring .worktrees in a file ktsu.Sdk regenerates ktsu.Sdk's Sdk.targets copies its own packaged gitignore template over SolutionDir/.gitignore on every build, so an entry added here does not survive. The worktree directory is excluded through .git/info/exclude instead, which is local, uncommitted, and not SDK-managed. --- .gitignore | 3 --- 1 file changed, 3 deletions(-) diff --git a/.gitignore b/.gitignore index 3b62745..dc0470a 100644 --- a/.gitignore +++ b/.gitignore @@ -651,6 +651,3 @@ Temporary Items # ImGui.ini files imgui.ini - -# Git worktrees used for isolated feature work -.worktrees/ From a5dd801dfcdf5716ff34522d656a5a820d6d4a93 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Tue, 22 Sep 2026 10:17:53 +1000 Subject: [PATCH 05/16] feat: add the worktree listing verb Co-Authored-By: Claude Haiku 4.5 --- .../Builders/GitWorktreeBuilderTests.cs | 32 +++++++++++++ GitIntegration.Test/GitRepositoryVerbTests.cs | 18 +++++++ .../Builders/GitWorktreeListBuilder.cs | 47 +++++++++++++++++++ GitIntegration/GitRepository.cs | 5 ++ 4 files changed, 102 insertions(+) create mode 100644 GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs create mode 100644 GitIntegration/Builders/GitWorktreeListBuilder.cs diff --git a/GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs b/GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs new file mode 100644 index 0000000..d723beb --- /dev/null +++ b/GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs @@ -0,0 +1,32 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration.Test; + +using System.Collections.Generic; +using System.Linq; + +[TestClass] +public sealed class GitWorktreeBuilderTests +{ + private static readonly string[] GlobalArguments = + [ + "-C", TestPaths.Root.WeakString, + "--no-pager", + "-c", "core.quotepath=false", + "-c", "color.ui=false", + ]; + + private static string[] Expect(params string[] verbArguments) => + [.. GlobalArguments, .. verbArguments]; + + [TestMethod] + public void BuildsTheWorktreeListVector() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeListBuilder builder = new(runner, TestPaths.Root); + + IReadOnlyList arguments = builder.BuildArguments(); + + CollectionAssert.AreEqual(Expect("worktree", "list", "--porcelain"), arguments.ToArray()); + } +} diff --git a/GitIntegration.Test/GitRepositoryVerbTests.cs b/GitIntegration.Test/GitRepositoryVerbTests.cs index 8cc974a..a019395 100644 --- a/GitIntegration.Test/GitRepositoryVerbTests.cs +++ b/GitIntegration.Test/GitRepositoryVerbTests.cs @@ -28,6 +28,7 @@ [.. repository.Branches().BuildArguments()], [.. repository.Remotes().BuildArguments()], [.. repository.RevParse("HEAD".As()).BuildArguments()], [.. repository.Tags().BuildArguments()], + [.. repository.Worktrees().BuildArguments()], [.. repository.Submodules().BuildArguments()], [.. repository.UpdateSubmodules().BuildArguments()], [.. repository.RevList("HEAD".As()).BuildArguments()], @@ -76,6 +77,7 @@ public void VerbsOnAMetadataOnlyRepositoryExplainWhatIsMissing() _ = Assert.ThrowsExactly(() => _ = repository.Branches()); _ = Assert.ThrowsExactly(() => _ = repository.Remotes()); _ = Assert.ThrowsExactly(() => _ = repository.Tags()); + _ = Assert.ThrowsExactly(() => _ = repository.Worktrees()); _ = Assert.ThrowsExactly(() => _ = repository.Submodules()); _ = Assert.ThrowsExactly(() => _ = repository.UpdateSubmodules()); _ = Assert.ThrowsExactly(() => _ = repository.RevList("HEAD".As())); @@ -145,5 +147,21 @@ public async Task IsClonedReportsFalseForAPathThatIsNotAWorkingTreeAsync() Assert.IsFalse(isCloned); } + [TestMethod] + public void WorktreesRequiresAProcessRunner() + { + GitRepository repository = new() { LocalPath = TestPaths.Root }; + + _ = Assert.ThrowsExactly(() => _ = repository.Worktrees()); + } + + [TestMethod] + public void WorktreesRequiresALocalPath() + { + GitRepository repository = new() { ProcessRunner = new RecordingGitProcessRunner() }; + + _ = Assert.ThrowsExactly(() => _ = repository.Worktrees()); + } + public TestContext TestContext { get; set; } = null!; } diff --git a/GitIntegration/Builders/GitWorktreeListBuilder.cs b/GitIntegration/Builders/GitWorktreeListBuilder.cs new file mode 100644 index 0000000..f9bfc2c --- /dev/null +++ b/GitIntegration/Builders/GitWorktreeListBuilder.cs @@ -0,0 +1,47 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration; + +using System.Collections.Generic; + +using ktsu.Semantics.Paths; + +/// +/// Lists the repository's working trees. +/// +/// +/// Reports the main working tree first, which is the order git emits and the only thing identifying +/// it — see . +/// +public interface IGitWorktreeListBuilder : IGitCommandBuilder> +{ +} + +/// +/// Builds git worktree list --porcelain. +/// +/// +/// No options. The porcelain listing already reports every attribute this library models, and git's +/// remaining options on this verb either change the format (-v, which is the human-facing +/// form) or add a field this library reads from the porcelain output anyway (--expire, which +/// only affects which entries are annotated prunable). +/// +/// Runs the assembled command. +/// The repository to scope the command to. +internal sealed class GitWorktreeListBuilder(IGitProcessRunner runner, AbsoluteDirectoryPath repositoryPath) + : GitCommandBuilder>(runner, repositoryPath), IGitWorktreeListBuilder +{ + /// + protected override void AppendVerbArguments(ICollection arguments) + { + Ensure.NotNull(arguments); + + arguments.Add("worktree"); + arguments.Add("list"); + arguments.Add("--porcelain"); + } + + /// + protected override IReadOnlyList ParseResult(GitProcessResult result) => + GitWorktreeParser.Parse(Ensure.NotNull(result).StandardOutput); +} diff --git a/GitIntegration/GitRepository.cs b/GitIntegration/GitRepository.cs index 8b527a1..a0327d2 100644 --- a/GitIntegration/GitRepository.cs +++ b/GitIntegration/GitRepository.cs @@ -199,6 +199,11 @@ public IGitRevListDivergenceBuilder Divergence(GitRefName upstream, GitRefName l /// This repository has no . public IGitTagListBuilder Tags() => new GitTagListBuilder(RequireRunner(), RequireLocalPath()); + /// Lists the repository's working trees. + /// A fresh builder. + /// This repository has no . + public IGitWorktreeListBuilder Worktrees() => new GitWorktreeListBuilder(RequireRunner(), RequireLocalPath()); + /// Stages changes for the next commit. /// A fresh builder. /// This repository has no . From 4ab188d5a6d6622e92fb92952b880759af0e5b63 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Tue, 22 Sep 2026 10:24:35 +1000 Subject: [PATCH 06/16] feat: add the worktree creation verb Co-Authored-By: Claude Sonnet 5 --- .../Builders/GitWorktreeBuilderTests.cs | 132 +++++++++++ .../GitRepositoryMetadataTests.cs | 4 + .../GitRepositoryMutatingVerbTests.cs | 20 ++ .../Builders/GitWorktreeAddBuilder.cs | 217 ++++++++++++++++++ GitIntegration/GitRepository.cs | 12 + 5 files changed, 385 insertions(+) create mode 100644 GitIntegration/Builders/GitWorktreeAddBuilder.cs diff --git a/GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs b/GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs index d723beb..7cbd9aa 100644 --- a/GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs +++ b/GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs @@ -4,6 +4,9 @@ namespace ktsu.GitIntegration.Test; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; + +using ktsu.Semantics.Strings; [TestClass] public sealed class GitWorktreeBuilderTests @@ -29,4 +32,133 @@ public void BuildsTheWorktreeListVector() CollectionAssert.AreEqual(Expect("worktree", "list", "--porcelain"), arguments.ToArray()); } + + [TestMethod] + public void BuildsTheMinimalWorktreeAddVector() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + + CollectionAssert.AreEqual( + Expect("worktree", "add", "--end-of-options", TestPaths.Worktree.WeakString), + builder.BuildArguments().ToArray()); + } + + [TestMethod] + public void PutsACheckedOutBranchInTheCommitIshOperand() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + _ = builder.CheckingOut("feature".As()); + + CollectionAssert.AreEqual( + Expect("worktree", "add", "--end-of-options", TestPaths.Worktree.WeakString, "feature"), + builder.BuildArguments().ToArray()); + } + + [TestMethod] + public void EmitsTheCreateBranchOptionBeforeThePath() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + _ = builder.CreatingBranch("feature".As()); + + CollectionAssert.AreEqual( + Expect("worktree", "add", "-b", "feature", "--end-of-options", TestPaths.Worktree.WeakString), + builder.BuildArguments().ToArray()); + } + + [TestMethod] + public void EmitsTheResettingCreateBranchOption() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + _ = builder.CreatingOrResettingBranch("feature".As()); + + CollectionAssert.AreEqual( + Expect("worktree", "add", "-B", "feature", "--end-of-options", TestPaths.Worktree.WeakString), + builder.BuildArguments().ToArray()); + } + + [TestMethod] + public void CombinesBranchCreationWithAStartPoint() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + _ = builder.CreatingBranch("feature".As()).From("origin/main".As()); + + CollectionAssert.AreEqual( + Expect("worktree", "add", "-b", "feature", "--end-of-options", TestPaths.Worktree.WeakString, "origin/main"), + builder.BuildArguments().ToArray()); + } + + [TestMethod] + public void CombinesDetachmentWithACommitIsh() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + _ = builder.Detached().From("v1.2.0".As()); + + CollectionAssert.AreEqual( + Expect("worktree", "add", "--detach", "--end-of-options", TestPaths.Worktree.WeakString, "v1.2.0"), + builder.BuildArguments().ToArray()); + } + + [TestMethod] + public void TheLastModeSelectionWins() + { + // The four modes are one field, resolved rather than accumulated, matching how + // IGitBranchListBuilder's LocalOnly and RemoteOnly already replace each other. A vector + // carrying both -b and --detach is one git rejects. + RecordingGitProcessRunner runner = new(); + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + _ = builder.CreatingBranch("feature".As()).Detached(); + + string[] arguments = [.. builder.BuildArguments()]; + + CollectionAssert.Contains(arguments, "--detach"); + CollectionAssert.DoesNotContain(arguments, "-b"); + CollectionAssert.DoesNotContain(arguments, "feature"); + } + + [TestMethod] + public void TheLastCommitIshSelectionWins() + { + // CheckingOut and From write the same operand slot, so the same resolution applies. + RecordingGitProcessRunner runner = new(); + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + _ = builder.CheckingOut("feature".As()).From("v1.2.0".As()); + + string[] arguments = [.. builder.BuildArguments()]; + + CollectionAssert.Contains(arguments, "v1.2.0"); + CollectionAssert.DoesNotContain(arguments, "feature"); + } + + [TestMethod] + public void EmitsForceAndNoCheckout() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + _ = builder.Force().WithoutCheckout(); + + CollectionAssert.AreEqual( + Expect("worktree", "add", "--force", "--no-checkout", "--end-of-options", TestPaths.Worktree.WeakString), + builder.BuildArguments().ToArray()); + } + + [TestMethod] + public async Task ExecuteReportsTheArgumentVectorOnSuccessAsync() + { + // git worktree add writes its confirmation to standard error, not a form this library parses, + // so the result carries the vector rather than a parse of that text. + RecordingGitProcessRunner runner = new() { StandardError = "Preparing worktree (new branch 'feature')\n" }; + GitWorktreeAddBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + + GitCompleted completed = await builder.ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + CollectionAssert.AreEqual(builder.BuildArguments().ToArray(), completed.Arguments.ToArray()); + } + + public TestContext TestContext { get; set; } = null!; } diff --git a/GitIntegration.Test/GitRepositoryMetadataTests.cs b/GitIntegration.Test/GitRepositoryMetadataTests.cs index a160d9f..b412268 100644 --- a/GitIntegration.Test/GitRepositoryMetadataTests.cs +++ b/GitIntegration.Test/GitRepositoryMetadataTests.cs @@ -132,4 +132,8 @@ internal static class TestPaths { public static AbsoluteDirectoryPath Root { get; } = (OperatingSystem.IsWindows() ? @"C:\" : "/").As(); + + /// An absolute directory distinct from , used as a worktree destination. + public static AbsoluteDirectoryPath Worktree { get; } = + (OperatingSystem.IsWindows() ? @"C:\project-feature" : "/project-feature").As(); } diff --git a/GitIntegration.Test/GitRepositoryMutatingVerbTests.cs b/GitIntegration.Test/GitRepositoryMutatingVerbTests.cs index 6f8f8a5..5fb99cb 100644 --- a/GitIntegration.Test/GitRepositoryMutatingVerbTests.cs +++ b/GitIntegration.Test/GitRepositoryMutatingVerbTests.cs @@ -100,4 +100,24 @@ public void ANullArgumentIsReportedBeforeAMissingProcessRunner() Assert.ThrowsExactly(() => _ = repository.Checkout(null!)); Assert.ThrowsExactly(() => _ = repository.AddRemote(null!, Url)); } + + [TestMethod] + public void AddWorktreeRejectsANullPath() + { + GitRepository repository = new() + { + LocalPath = TestPaths.Root, + ProcessRunner = new RecordingGitProcessRunner(), + }; + + _ = Assert.ThrowsExactly(() => repository.AddWorktree(null!)); + } + + [TestMethod] + public void AddWorktreeRequiresAProcessRunner() + { + GitRepository repository = new() { LocalPath = TestPaths.Root }; + + _ = Assert.ThrowsExactly(() => repository.AddWorktree(TestPaths.Worktree)); + } } diff --git a/GitIntegration/Builders/GitWorktreeAddBuilder.cs b/GitIntegration/Builders/GitWorktreeAddBuilder.cs new file mode 100644 index 0000000..a765a26 --- /dev/null +++ b/GitIntegration/Builders/GitWorktreeAddBuilder.cs @@ -0,0 +1,217 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration; + +using System.Collections.Generic; + +using ktsu.Semantics.Paths; + +/// +/// Creates an additional working tree. +/// +/// +/// , , +/// and are four settings of one mode, and a later call replaces an earlier +/// one, as IGitBranchListBuilder's LocalOnly and RemoteOnly already do. A vector +/// carrying two of them at once is one git rejects. +/// +public interface IGitWorktreeAddBuilder : IGitCommandBuilder +{ + /// Checks out an existing branch in the new worktree. Replaces any previous mode. + /// The branch to check out. + /// The same builder, to allow chaining. + public IGitWorktreeAddBuilder CheckingOut(GitBranchName branch); + + /// Creates a branch and checks it out in the new worktree. Replaces any previous mode. + /// Fails when the branch already exists; see . + /// The branch to create. + /// The same builder, to allow chaining. + public IGitWorktreeAddBuilder CreatingBranch(GitBranchName branch); + + /// + /// Creates a branch, resetting it when it already exists, and checks it out. Replaces any + /// previous mode. + /// + /// The branch to create or reset. + /// The same builder, to allow chaining. + public IGitWorktreeAddBuilder CreatingOrResettingBranch(GitBranchName branch); + + /// Checks out a commit rather than a branch. Replaces any previous mode. + /// The same builder, to allow chaining. + public IGitWorktreeAddBuilder Detached(); + + /// + /// Sets the commit-ish the new worktree starts from: the start point for the branch-creating + /// modes, and the commit to detach at for . + /// + /// + /// Writes the same operand as , so the later of the two calls wins. + /// Distinct from it only in taking a , which admits a tag or a raw + /// revision rather than a branch alone. + /// + /// The revision to start from. + /// The same builder, to allow chaining. + public IGitWorktreeAddBuilder From(GitRefName commitish); + + /// Creates the worktree even when git would otherwise refuse. + /// + /// Git refuses when the branch is already checked out in another worktree, and when the + /// destination is a missing-but-registered worktree directory. + /// + /// The same builder, to allow chaining. + public IGitWorktreeAddBuilder Force(); + + /// Registers the worktree without populating its working directory. + /// The same builder, to allow chaining. + public IGitWorktreeAddBuilder WithoutCheckout(); +} + +/// +/// Builds git worktree add. +/// +/// +/// --guess-remote is deliberately not exposed. Plain git worktree add <path> +/// <branch> already creates a local tracking branch when the name matches exactly one +/// remote, which covers creating a worktree for a branch that exists only on the remote. +/// --guess-remote extends that to the case where no branch is named at all, which this +/// builder always names. +/// +/// Runs the assembled command. +/// The repository to scope the command to. +/// Where the new worktree goes. +internal sealed class GitWorktreeAddBuilder( + IGitProcessRunner runner, + AbsoluteDirectoryPath repositoryPath, + AbsoluteDirectoryPath path) + : GitCommandBuilder(runner, repositoryPath), IGitWorktreeAddBuilder +{ + private enum AddMode + { + Plain, + CreateBranch, + CreateOrResetBranch, + Detach, + } + + private readonly AbsoluteDirectoryPath _path = Ensure.NotNull(path); + + private AddMode _mode = AddMode.Plain; + private string? _branch; + private string? _commitish; + private bool _force; + private bool _withoutCheckout; + + /// + public IGitWorktreeAddBuilder CheckingOut(GitBranchName branch) + { + Ensure.NotNull(branch); + + _mode = AddMode.Plain; + _branch = null; + _commitish = branch.WeakString; + return this; + } + + /// + public IGitWorktreeAddBuilder CreatingBranch(GitBranchName branch) + { + Ensure.NotNull(branch); + + _mode = AddMode.CreateBranch; + _branch = branch.WeakString; + return this; + } + + /// + public IGitWorktreeAddBuilder CreatingOrResettingBranch(GitBranchName branch) + { + Ensure.NotNull(branch); + + _mode = AddMode.CreateOrResetBranch; + _branch = branch.WeakString; + return this; + } + + /// + public IGitWorktreeAddBuilder Detached() + { + _mode = AddMode.Detach; + _branch = null; + return this; + } + + /// + public IGitWorktreeAddBuilder From(GitRefName commitish) + { + Ensure.NotNull(commitish); + + _commitish = commitish.WeakString; + return this; + } + + /// + public IGitWorktreeAddBuilder Force() + { + _force = true; + return this; + } + + /// + public IGitWorktreeAddBuilder WithoutCheckout() + { + _withoutCheckout = true; + return this; + } + + /// + protected override void AppendVerbArguments(ICollection arguments) + { + Ensure.NotNull(arguments); + + arguments.Add("worktree"); + arguments.Add("add"); + + if (_force) + { + arguments.Add("--force"); + } + + if (_withoutCheckout) + { + arguments.Add("--no-checkout"); + } + + switch (_mode) + { + case AddMode.CreateBranch: + arguments.Add("-b"); + arguments.Add(_branch!); + break; + case AddMode.CreateOrResetBranch: + arguments.Add("-B"); + arguments.Add(_branch!); + break; + case AddMode.Detach: + arguments.Add("--detach"); + break; + case AddMode.Plain: + default: + break; + } + + // The path and the commit-ish are both caller-supplied operands and share one end-of-options + // marker, which git reads as applying to everything after it. + if (_commitish is null) + { + AppendOperands(arguments, _path.WeakString); + } + else + { + AppendOperands(arguments, _path.WeakString, _commitish); + } + } + + /// + protected override GitCompleted ParseResult(GitProcessResult result) => + new() { Arguments = Ensure.NotNull(result).Arguments }; +} diff --git a/GitIntegration/GitRepository.cs b/GitIntegration/GitRepository.cs index a0327d2..bcef7a4 100644 --- a/GitIntegration/GitRepository.cs +++ b/GitIntegration/GitRepository.cs @@ -204,6 +204,18 @@ public IGitRevListDivergenceBuilder Divergence(GitRefName upstream, GitRefName l /// This repository has no . public IGitWorktreeListBuilder Worktrees() => new GitWorktreeListBuilder(RequireRunner(), RequireLocalPath()); + /// Creates an additional working tree. + /// Where the new worktree goes. + /// A fresh builder. + /// is . + /// This repository has no . + public IGitWorktreeAddBuilder AddWorktree(AbsoluteDirectoryPath path) + { + // Argument validation precedes the state check, matching every other verb taking an operand. + Ensure.NotNull(path); + return new GitWorktreeAddBuilder(RequireRunner(), RequireLocalPath(), path); + } + /// Stages changes for the next commit. /// A fresh builder. /// This repository has no . From ce10c69d929b3b721e7ec57cd23c4d3da490e194 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Tue, 22 Sep 2026 10:30:19 +1000 Subject: [PATCH 07/16] feat: add the worktree removal and prune verbs Co-Authored-By: Claude Haiku 4.5 --- .../Builders/GitWorktreeBuilderTests.cs | 34 +++++++ .../GitRepositoryMutatingVerbTests.cs | 20 ++++ .../Builders/GitWorktreeWriteBuilders.cs | 99 +++++++++++++++++++ GitIntegration/GitRepository.cs | 17 ++++ 4 files changed, 170 insertions(+) create mode 100644 GitIntegration/Builders/GitWorktreeWriteBuilders.cs diff --git a/GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs b/GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs index 7cbd9aa..281bd25 100644 --- a/GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs +++ b/GitIntegration.Test/Builders/GitWorktreeBuilderTests.cs @@ -160,5 +160,39 @@ public async Task ExecuteReportsTheArgumentVectorOnSuccessAsync() CollectionAssert.AreEqual(builder.BuildArguments().ToArray(), completed.Arguments.ToArray()); } + [TestMethod] + public void BuildsTheWorktreeRemoveVector() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeRemoveBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + + CollectionAssert.AreEqual( + Expect("worktree", "remove", "--end-of-options", TestPaths.Worktree.WeakString), + builder.BuildArguments().ToArray()); + } + + [TestMethod] + public void EmitsForceOnRemoveBeforeThePath() + { + RecordingGitProcessRunner runner = new(); + GitWorktreeRemoveBuilder builder = new(runner, TestPaths.Root, TestPaths.Worktree); + _ = builder.Force(); + + CollectionAssert.AreEqual( + Expect("worktree", "remove", "--force", "--end-of-options", TestPaths.Worktree.WeakString), + builder.BuildArguments().ToArray()); + } + + [TestMethod] + public void BuildsTheWorktreePruneVector() + { + RecordingGitProcessRunner runner = new(); + GitWorktreePruneBuilder builder = new(runner, TestPaths.Root); + + CollectionAssert.AreEqual( + Expect("worktree", "prune"), + builder.BuildArguments().ToArray()); + } + public TestContext TestContext { get; set; } = null!; } diff --git a/GitIntegration.Test/GitRepositoryMutatingVerbTests.cs b/GitIntegration.Test/GitRepositoryMutatingVerbTests.cs index 5fb99cb..906f8b8 100644 --- a/GitIntegration.Test/GitRepositoryMutatingVerbTests.cs +++ b/GitIntegration.Test/GitRepositoryMutatingVerbTests.cs @@ -120,4 +120,24 @@ public void AddWorktreeRequiresAProcessRunner() _ = Assert.ThrowsExactly(() => repository.AddWorktree(TestPaths.Worktree)); } + + [TestMethod] + public void RemoveWorktreeRejectsANullPath() + { + GitRepository repository = new() + { + LocalPath = TestPaths.Root, + ProcessRunner = new RecordingGitProcessRunner(), + }; + + _ = Assert.ThrowsExactly(() => repository.RemoveWorktree(null!)); + } + + [TestMethod] + public void PruneWorktreesRequiresAProcessRunner() + { + GitRepository repository = new() { LocalPath = TestPaths.Root }; + + _ = Assert.ThrowsExactly(repository.PruneWorktrees); + } } diff --git a/GitIntegration/Builders/GitWorktreeWriteBuilders.cs b/GitIntegration/Builders/GitWorktreeWriteBuilders.cs new file mode 100644 index 0000000..9fed721 --- /dev/null +++ b/GitIntegration/Builders/GitWorktreeWriteBuilders.cs @@ -0,0 +1,99 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration; + +using System.Collections.Generic; + +using ktsu.Semantics.Paths; + +/// +/// Removes a working tree and the administrative record of it. +/// +public interface IGitWorktreeRemoveBuilder : IGitCommandBuilder +{ + /// Removes the worktree even when its working directory is not clean. + /// + /// Git refuses to remove a worktree holding modified tracked files or untracked files, since + /// doing so discards them. This says the caller knows. + /// + /// The same builder, to allow chaining. + public IGitWorktreeRemoveBuilder Force(); +} + +/// +/// Removes administrative records for working trees whose directories have gone. +/// +/// +/// No options. --dry-run is deliberately not exposed: it would change this verb's result +/// from into a listing, and a caller that wants to know what would be +/// removed reads from a listing it can already obtain. +/// +public interface IGitWorktreePruneBuilder : IGitCommandBuilder +{ +} + +/// +/// Builds git worktree remove. +/// +/// Runs the assembled command. +/// The repository to scope the command to. +/// The worktree to remove. +internal sealed class GitWorktreeRemoveBuilder( + IGitProcessRunner runner, + AbsoluteDirectoryPath repositoryPath, + AbsoluteDirectoryPath path) + : GitCommandBuilder(runner, repositoryPath), IGitWorktreeRemoveBuilder +{ + private readonly AbsoluteDirectoryPath _path = Ensure.NotNull(path); + + private bool _force; + + /// + public IGitWorktreeRemoveBuilder Force() + { + _force = true; + return this; + } + + /// + protected override void AppendVerbArguments(ICollection arguments) + { + Ensure.NotNull(arguments); + + arguments.Add("worktree"); + arguments.Add("remove"); + + if (_force) + { + arguments.Add("--force"); + } + + AppendOperands(arguments, _path.WeakString); + } + + /// + protected override GitCompleted ParseResult(GitProcessResult result) => + new() { Arguments = Ensure.NotNull(result).Arguments }; +} + +/// +/// Builds git worktree prune. +/// +/// Runs the assembled command. +/// The repository to scope the command to. +internal sealed class GitWorktreePruneBuilder(IGitProcessRunner runner, AbsoluteDirectoryPath repositoryPath) + : GitCommandBuilder(runner, repositoryPath), IGitWorktreePruneBuilder +{ + /// + protected override void AppendVerbArguments(ICollection arguments) + { + Ensure.NotNull(arguments); + + arguments.Add("worktree"); + arguments.Add("prune"); + } + + /// + protected override GitCompleted ParseResult(GitProcessResult result) => + new() { Arguments = Ensure.NotNull(result).Arguments }; +} diff --git a/GitIntegration/GitRepository.cs b/GitIntegration/GitRepository.cs index bcef7a4..5679f04 100644 --- a/GitIntegration/GitRepository.cs +++ b/GitIntegration/GitRepository.cs @@ -216,6 +216,23 @@ public IGitWorktreeAddBuilder AddWorktree(AbsoluteDirectoryPath path) return new GitWorktreeAddBuilder(RequireRunner(), RequireLocalPath(), path); } + /// Removes a working tree and the administrative record of it. + /// The worktree to remove. + /// A fresh builder. + /// is . + /// This repository has no . + public IGitWorktreeRemoveBuilder RemoveWorktree(AbsoluteDirectoryPath path) + { + Ensure.NotNull(path); + return new GitWorktreeRemoveBuilder(RequireRunner(), RequireLocalPath(), path); + } + + /// Removes administrative records for working trees whose directories have gone. + /// A fresh builder. + /// This repository has no . + public IGitWorktreePruneBuilder PruneWorktrees() => + new GitWorktreePruneBuilder(RequireRunner(), RequireLocalPath()); + /// Stages changes for the next commit. /// A fresh builder. /// This repository has no . From 655a072597a6962bb5743df316d40a608aab35cf Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Tue, 22 Sep 2026 10:37:48 +1000 Subject: [PATCH 08/16] feat: route GitHub repository enumeration by owner kind Co-Authored-By: Claude Sonnet 5 --- .../Fixtures/github-org-repositories.json | 214 ++++++++++++++++++ .../Hosting/GitHubProviderTests.cs | 96 ++++++++ GitIntegration/GitHubProvider.cs | 75 +++++- GitIntegration/Models/GitEnums.cs | 32 +++ 4 files changed, 406 insertions(+), 11 deletions(-) create mode 100644 GitIntegration.Test/Fixtures/github-org-repositories.json diff --git a/GitIntegration.Test/Fixtures/github-org-repositories.json b/GitIntegration.Test/Fixtures/github-org-repositories.json new file mode 100644 index 0000000..2aae3eb --- /dev/null +++ b/GitIntegration.Test/Fixtures/github-org-repositories.json @@ -0,0 +1,214 @@ +[ + { + "id": 90000001, + "node_id": "MDEwOlJlcG9zaXRvcnk5MDAwMDAwMQ==", + "name": "org-public-repo", + "full_name": "contoso/org-public-repo", + "private": false, + "owner": { + "login": "contoso", + "id": 90000001, + "node_id": "MDQ6VXNlcjkwMDAwMDAx", + "avatar_url": "https://avatars.githubusercontent.com/u/90000001?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/example-user", + "html_url": "https://github.com/example-user", + "followers_url": "https://api.github.com/users/example-user/followers", + "following_url": "https://api.github.com/users/example-user/following{/other_user}", + "gists_url": "https://api.github.com/users/example-user/gists{/gist_id}", + "starred_url": "https://api.github.com/users/example-user/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/example-user/subscriptions", + "organizations_url": "https://api.github.com/users/example-user/orgs", + "repos_url": "https://api.github.com/users/example-user/repos", + "events_url": "https://api.github.com/users/example-user/events{/privacy}", + "received_events_url": "https://api.github.com/users/example-user/received_events", + "type": "User", + "user_view_type": "public", + "site_admin": false + }, + "html_url": "https://github.com/contoso/org-public-repo", + "description": "Testing", + "fork": true, + "url": "https://api.github.com/repos/example-user/example-repo-1", + "forks_url": "https://api.github.com/repos/example-user/example-repo-1/forks", + "keys_url": "https://api.github.com/repos/example-user/example-repo-1/keys{/key_id}", + "collaborators_url": "https://api.github.com/repos/example-user/example-repo-1/collaborators{/collaborator}", + "teams_url": "https://api.github.com/repos/example-user/example-repo-1/teams", + "hooks_url": "https://api.github.com/repos/example-user/example-repo-1/hooks", + "issue_events_url": "https://api.github.com/repos/example-user/example-repo-1/issues/events{/number}", + "events_url": "https://api.github.com/repos/example-user/example-repo-1/events", + "assignees_url": "https://api.github.com/repos/example-user/example-repo-1/assignees{/user}", + "branches_url": "https://api.github.com/repos/example-user/example-repo-1/branches{/branch}", + "tags_url": "https://api.github.com/repos/example-user/example-repo-1/tags", + "blobs_url": "https://api.github.com/repos/example-user/example-repo-1/git/blobs{/sha}", + "git_tags_url": "https://api.github.com/repos/example-user/example-repo-1/git/tags{/sha}", + "git_refs_url": "https://api.github.com/repos/example-user/example-repo-1/git/refs{/sha}", + "trees_url": "https://api.github.com/repos/example-user/example-repo-1/git/trees{/sha}", + "statuses_url": "https://api.github.com/repos/example-user/example-repo-1/statuses/{sha}", + "languages_url": "https://api.github.com/repos/example-user/example-repo-1/languages", + "stargazers_url": "https://api.github.com/repos/example-user/example-repo-1/stargazers", + "contributors_url": "https://api.github.com/repos/example-user/example-repo-1/contributors", + "subscribers_url": "https://api.github.com/repos/example-user/example-repo-1/subscribers", + "subscription_url": "https://api.github.com/repos/example-user/example-repo-1/subscription", + "commits_url": "https://api.github.com/repos/example-user/example-repo-1/commits{/sha}", + "git_commits_url": "https://api.github.com/repos/example-user/example-repo-1/git/commits{/sha}", + "comments_url": "https://api.github.com/repos/example-user/example-repo-1/comments{/number}", + "issue_comment_url": "https://api.github.com/repos/example-user/example-repo-1/issues/comments{/number}", + "contents_url": "https://api.github.com/repos/example-user/example-repo-1/contents/{+path}", + "compare_url": "https://api.github.com/repos/example-user/example-repo-1/compare/{base}...{head}", + "merges_url": "https://api.github.com/repos/example-user/example-repo-1/merges", + "archive_url": "https://api.github.com/repos/example-user/example-repo-1/{archive_format}{/ref}", + "downloads_url": "https://api.github.com/repos/example-user/example-repo-1/downloads", + "issues_url": "https://api.github.com/repos/example-user/example-repo-1/issues{/number}", + "pulls_url": "https://api.github.com/repos/example-user/example-repo-1/pulls{/number}", + "milestones_url": "https://api.github.com/repos/example-user/example-repo-1/milestones{/number}", + "notifications_url": "https://api.github.com/repos/example-user/example-repo-1/notifications{?since,all,participating}", + "labels_url": "https://api.github.com/repos/example-user/example-repo-1/labels{/name}", + "releases_url": "https://api.github.com/repos/example-user/example-repo-1/releases{/id}", + "deployments_url": "https://api.github.com/repos/example-user/example-repo-1/deployments", + "created_at": "2018-05-10T17:51:29Z", + "updated_at": "2026-08-19T00:06:33Z", + "pushed_at": "2024-05-26T07:02:05Z", + "git_url": "git://github.com/example-user/example-repo-1.git", + "ssh_url": "git@github.com:example-user/example-repo-1.git", + "clone_url": "https://github.com/contoso/org-public-repo.git", + "svn_url": "https://github.com/example-user/example-repo-1", + "homepage": "", + "size": 4, + "stargazers_count": 473, + "watchers_count": 473, + "language": null, + "has_issues": false, + "has_projects": true, + "has_downloads": false, + "has_wiki": true, + "has_pages": false, + "has_discussions": false, + "forks_count": 27, + "mirror_url": null, + "archived": false, + "disabled": false, + "open_issues_count": 1, + "license": null, + "allow_forking": true, + "is_template": false, + "web_commit_signoff_required": false, + "has_pull_requests": true, + "pull_request_creation_policy": "all", + "topics": [], + "visibility": "public", + "forks": 27, + "open_issues": 1, + "watchers": 473, + "default_branch": "master" + }, + { + "id": 90000002, + "node_id": "MDEwOlJlcG9zaXRvcnk5MDAwMDAwMg==", + "name": "org-private-repo", + "full_name": "contoso/org-private-repo", + "private": true, + "owner": { + "login": "contoso", + "id": 90000001, + "node_id": "MDQ6VXNlcjkwMDAwMDAx", + "avatar_url": "https://avatars.githubusercontent.com/u/90000001?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/example-user", + "html_url": "https://github.com/example-user", + "followers_url": "https://api.github.com/users/example-user/followers", + "following_url": "https://api.github.com/users/example-user/following{/other_user}", + "gists_url": "https://api.github.com/users/example-user/gists{/gist_id}", + "starred_url": "https://api.github.com/users/example-user/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/example-user/subscriptions", + "organizations_url": "https://api.github.com/users/example-user/orgs", + "repos_url": "https://api.github.com/users/example-user/repos", + "events_url": "https://api.github.com/users/example-user/events{/privacy}", + "received_events_url": "https://api.github.com/users/example-user/received_events", + "type": "User", + "user_view_type": "public", + "site_admin": false + }, + "html_url": "https://github.com/contoso/org-private-repo", + "description": "This repo is for demonstration purposes only.", + "fork": false, + "url": "https://api.github.com/repos/example-user/example-repo-2", + "forks_url": "https://api.github.com/repos/example-user/example-repo-2/forks", + "keys_url": "https://api.github.com/repos/example-user/example-repo-2/keys{/key_id}", + "collaborators_url": "https://api.github.com/repos/example-user/example-repo-2/collaborators{/collaborator}", + "teams_url": "https://api.github.com/repos/example-user/example-repo-2/teams", + "hooks_url": "https://api.github.com/repos/example-user/example-repo-2/hooks", + "issue_events_url": "https://api.github.com/repos/example-user/example-repo-2/issues/events{/number}", + "events_url": "https://api.github.com/repos/example-user/example-repo-2/events", + "assignees_url": "https://api.github.com/repos/example-user/example-repo-2/assignees{/user}", + "branches_url": "https://api.github.com/repos/example-user/example-repo-2/branches{/branch}", + "tags_url": "https://api.github.com/repos/example-user/example-repo-2/tags", + "blobs_url": "https://api.github.com/repos/example-user/example-repo-2/git/blobs{/sha}", + "git_tags_url": "https://api.github.com/repos/example-user/example-repo-2/git/tags{/sha}", + "git_refs_url": "https://api.github.com/repos/example-user/example-repo-2/git/refs{/sha}", + "trees_url": "https://api.github.com/repos/example-user/example-repo-2/git/trees{/sha}", + "statuses_url": "https://api.github.com/repos/example-user/example-repo-2/statuses/{sha}", + "languages_url": "https://api.github.com/repos/example-user/example-repo-2/languages", + "stargazers_url": "https://api.github.com/repos/example-user/example-repo-2/stargazers", + "contributors_url": "https://api.github.com/repos/example-user/example-repo-2/contributors", + "subscribers_url": "https://api.github.com/repos/example-user/example-repo-2/subscribers", + "subscription_url": "https://api.github.com/repos/example-user/example-repo-2/subscription", + "commits_url": "https://api.github.com/repos/example-user/example-repo-2/commits{/sha}", + "git_commits_url": "https://api.github.com/repos/example-user/example-repo-2/git/commits{/sha}", + "comments_url": "https://api.github.com/repos/example-user/example-repo-2/comments{/number}", + "issue_comment_url": "https://api.github.com/repos/example-user/example-repo-2/issues/comments{/number}", + "contents_url": "https://api.github.com/repos/example-user/example-repo-2/contents/{+path}", + "compare_url": "https://api.github.com/repos/example-user/example-repo-2/compare/{base}...{head}", + "merges_url": "https://api.github.com/repos/example-user/example-repo-2/merges", + "archive_url": "https://api.github.com/repos/example-user/example-repo-2/{archive_format}{/ref}", + "downloads_url": "https://api.github.com/repos/example-user/example-repo-2/downloads", + "issues_url": "https://api.github.com/repos/example-user/example-repo-2/issues{/number}", + "pulls_url": "https://api.github.com/repos/example-user/example-repo-2/pulls{/number}", + "milestones_url": "https://api.github.com/repos/example-user/example-repo-2/milestones{/number}", + "notifications_url": "https://api.github.com/repos/example-user/example-repo-2/notifications{?since,all,participating}", + "labels_url": "https://api.github.com/repos/example-user/example-repo-2/labels{/name}", + "releases_url": "https://api.github.com/repos/example-user/example-repo-2/releases{/id}", + "deployments_url": "https://api.github.com/repos/example-user/example-repo-2/deployments", + "created_at": "2014-03-28T17:55:38Z", + "updated_at": "2026-08-19T00:06:15Z", + "pushed_at": "2024-07-12T15:04:33Z", + "git_url": "git://github.com/example-user/example-repo-2.git", + "ssh_url": "git@github.com:example-user/example-repo-2.git", + "clone_url": "https://github.com/contoso/org-private-repo.git", + "svn_url": "https://github.com/example-user/example-repo-2", + "homepage": null, + "size": 190, + "stargazers_count": 603, + "watchers_count": 603, + "language": null, + "has_issues": true, + "has_projects": true, + "has_downloads": false, + "has_wiki": true, + "has_pages": false, + "has_discussions": false, + "forks_count": 177, + "mirror_url": null, + "archived": false, + "disabled": false, + "open_issues_count": 48, + "license": { + "key": "mit", + "name": "MIT License", + "spdx_id": "MIT", + "url": "https://api.github.com/licenses/mit", + "node_id": "MDc6TGljZW5zZTEz" + }, + "allow_forking": true, + "is_template": false, + "web_commit_signoff_required": false, + "has_pull_requests": true, + "pull_request_creation_policy": "all", + "topics": [], + "visibility": "public", + "forks": 177, + "open_issues": 48, + "watchers": 603, + "default_branch": "master" + } +] diff --git a/GitIntegration.Test/Hosting/GitHubProviderTests.cs b/GitIntegration.Test/Hosting/GitHubProviderTests.cs index e27d013..9b820d7 100644 --- a/GitIntegration.Test/Hosting/GitHubProviderTests.cs +++ b/GitIntegration.Test/Hosting/GitHubProviderTests.cs @@ -758,5 +758,101 @@ public async Task LeavesAnInjectedHandlerUndisposedAndUsableForASecondCallAsync( Assert.AreEqual("example-repo-1".As(), second[0].Name); } + [TestMethod] + public async Task EnumeratesAnOrganisationThroughTheOrgsRoute() + { + // The route is a documented part of this provider's contract, not an Octokit detail: + // GET /orgs/{org}/repos is the only one of the three that reports an organisation's private + // repositories to a token that can see them. + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, Fixture("github-org-repositories.json"), ("Content-Type", "application/json")); + GitHubProvider provider = new() + { + Owner = "contoso".As(), + OwnerKind = GitHubOwnerKind.Organization, + Handler = handler, + }; + + IReadOnlyList repositories = + await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual("/orgs/contoso/repos", handler.Requests[0].Uri.AbsolutePath); + Assert.AreEqual(2, repositories.Count); + Assert.AreEqual("org-public-repo".As(), repositories[0].Name); + Assert.AreEqual("org-private-repo".As(), repositories[1].Name); + } + + [TestMethod] + public async Task EnumeratesTheAuthenticatedAccountThroughTheUserRoute() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, Fixture("github-org-repositories.json"), ("Content-Type", "application/json")); + GitHubProvider provider = new() + { + Owner = "contoso".As(), + OwnerKind = GitHubOwnerKind.AuthenticatedUser, + Handler = handler, + }; + + _ = await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual("/user/repos", handler.Requests[0].Uri.AbsolutePath); + // Octokit serializes the flags enum as "owner, organization_member" (comma-space), which + // escapes to "%2C%20" — verified against this build's actual request rather than assumed. + StringAssert.Contains(handler.Requests[0].Uri.Query, "affiliation=owner%2C%20organization_member"); + } + + [TestMethod] + public async Task DropsRepositoriesBelongingToAnotherOwnerWhenEnumeratingTheAuthenticatedAccount() + { + // GET /user/repos takes no owner parameter, so routing to it unfiltered would silently ignore + // a configured Owner. The filter is what keeps this method's contract "Owner's repositories". + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, Fixture("github-org-repositories.json"), ("Content-Type", "application/json")); + GitHubProvider provider = new() + { + Owner = "someone-else".As(), + OwnerKind = GitHubOwnerKind.AuthenticatedUser, + Handler = handler, + }; + + IReadOnlyList repositories = + await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual(0, repositories.Count); + } + + [TestMethod] + public async Task MatchesTheOwnerWithoutRegardToCase() + { + // GitHub treats logins as case-insensitive. An ordinal comparison would drop a caller's + // repositories over a capital letter the caller did not choose. + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, Fixture("github-org-repositories.json"), ("Content-Type", "application/json")); + GitHubProvider provider = new() + { + Owner = "CONTOSO".As(), + OwnerKind = GitHubOwnerKind.AuthenticatedUser, + Handler = handler, + }; + + IReadOnlyList repositories = + await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual(2, repositories.Count); + } + + [TestMethod] + public async Task DefaultsToTheUserRouteItAlwaysUsed() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, Fixture("github-repositories.json"), ("Content-Type", "application/json")); + GitHubProvider provider = new() { Owner = "contoso".As(), Handler = handler }; + + _ = await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual("/users/contoso/repos", handler.Requests[0].Uri.AbsolutePath); + } + public TestContext TestContext { get; set; } = null!; } diff --git a/GitIntegration/GitHubProvider.cs b/GitIntegration/GitHubProvider.cs index ade82c1..ea8c87a 100644 --- a/GitIntegration/GitHubProvider.cs +++ b/GitIntegration/GitHubProvider.cs @@ -42,6 +42,17 @@ public sealed class GitHubProvider : GitProvider /// public override GitProviderName Name => "GitHub".As(); + /// + /// Gets what kind of account is. + /// + /// + /// by default, which is the route and the coverage this + /// provider has always had. A caller needing an organisation's private repositories sets + /// ; one needing its own sets + /// . + /// + public GitHubOwnerKind OwnerKind { get; init; } = GitHubOwnerKind.User; + /// private protected override HttpMessageHandler DefaultHandler => SharedHandler; @@ -71,16 +82,23 @@ public sealed class GitHubProvider : GitProvider /// neither is a copy of the other. An edit that moves this explanation must leave that pointer /// aimed somewhere real. /// - /// Calls GitHub's GET /users/{login}/repos, which returns only 's - /// public repositories. Supplying a token does not widen this: that endpoint does not - /// honour authentication to reveal private repositories the way GET /user/repos would for - /// the token's own account, and switching to that endpoint would silently stop honouring - /// — it always describes the token's own repositories, regardless - /// of which owner was configured, which would break callers who name someone else's owner on - /// purpose. A caller that needs private repositories for a specific owner has no equivalent - /// through this provider today; do not assume this method's coverage matches an - /// AzureDevOpsProvider equivalent, whose token can see everything it has access to under - /// the same interface. + /// The route, and therefore the coverage, is decided by . + /// — the default — calls GET /users/{login}/repos, + /// which returns 's public repositories only; supplying a + /// token does not widen it, because that endpoint does not honour authentication to reveal + /// private repositories. calls + /// GET /orgs/{org}/repos, which does report private repositories the credential can see. + /// calls GET /user/repos with + /// affiliation=owner,organization_member. + /// + /// + /// GET /user/repos takes no owner parameter — it always describes the token's own + /// reachable repositories — so that route's results are filtered to + /// here, case-insensitively, since GitHub treats logins that way. + /// Without the filter, selecting that kind would silently ignore a configured owner, which is the + /// objection that kept this method on the user route in the first place. Filtering rather than + /// validating the token's login against costs no extra request + /// and still serves an organisation the token is merely a member of. /// /// public override async Task> GetRepositoriesAsync(CancellationToken cancellationToken = default) @@ -92,7 +110,15 @@ public override async Task> GetRepositoriesAsync(Ca try { - IReadOnlyList repositories = await client.Repository.GetAllForUser(Owner.WeakString).ConfigureAwait(false); + IReadOnlyList repositories = OwnerKind switch + { + GitHubOwnerKind.Organization => + await client.Repository.GetAllForOrg(Owner.WeakString).ConfigureAwait(false), + GitHubOwnerKind.AuthenticatedUser => + FilterToOwner(await client.Repository.GetAllForCurrent(AuthenticatedUserRequest).ConfigureAwait(false)), + _ => await client.Repository.GetAllForUser(Owner.WeakString).ConfigureAwait(false), + }; + return [.. repositories.Select(ToGitRepository)]; } catch (ApiException exception) @@ -101,6 +127,33 @@ public override async Task> GetRepositoriesAsync(Ca } } + /// + /// The request enumerates with. + /// + /// + /// The affiliation is stated explicitly rather than left to GitHub's default, for the reason the + /// pull request listing states its own filter: this provider's coverage is defined by this + /// library, not by restating whichever default a vendor happens to ship today. + /// + private static RepositoryRequest AuthenticatedUserRequest => new() + { + Affiliation = RepositoryAffiliation.OwnerAndOrganizationMember, + }; + + /// + /// Drops repositories belonging to anyone but . + /// + /// + /// Only needs this: the other two routes carry + /// the owner in the request path and cannot answer for anybody else. Compared with + /// because GitHub logins are case-insensitive. + /// + /// Everything the route reported. + /// The subset this provider's owner has. + private IReadOnlyList FilterToOwner(IReadOnlyList repositories) => + [.. repositories.Where(repository => + string.Equals(repository.Owner?.Login, Owner.WeakString, StringComparison.OrdinalIgnoreCase))]; + /// internal override async Task> GetPullRequestsCoreAsync(GitRepositoryAddress repositoryAddress, CancellationToken cancellationToken) { diff --git a/GitIntegration/Models/GitEnums.cs b/GitIntegration/Models/GitEnums.cs index 3b465a2..f95b595 100644 --- a/GitIntegration/Models/GitEnums.cs +++ b/GitIntegration/Models/GitEnums.cs @@ -174,3 +174,35 @@ public enum GitSubmodulePushCheck /// Push the submodules' commits and stop, leaving the superproject unpushed. Only, } + +/// +/// What kind of account a 's owner is, which decides the endpoint its +/// repository enumeration can use. +/// +/// +/// GitHub answers "which repositories does this owner have" at three different routes with three +/// different coverages, and no single one of them serves every caller. Stating the kind is what lets +/// this provider pick correctly without probing, and without a contract that depends on what GitHub +/// answers to a speculative request. +/// +public enum GitHubOwnerKind +{ + /// + /// A user account other than the token's own. Enumerates that user's public repositories + /// only, whatever credential is supplied. + /// + User, + + /// + /// An organisation. Enumerates the organisation's repositories, including private ones the + /// credential can see. + /// + Organization, + + /// + /// The account the credential belongs to. Enumerates every repository that account owns or can + /// reach through an organisation membership, narrowed to + /// . + /// + AuthenticatedUser, +} From 51ee46343aef14f69b4e16e2fe6e79772e884118 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Tue, 22 Sep 2026 10:44:08 +1000 Subject: [PATCH 09/16] feat: report the single sign-on authorisation url on a forbidden response Co-Authored-By: Claude Sonnet 5 --- .../Hosting/GitHubProviderTests.cs | 40 +++++++++++++ GitIntegration/GitHubProvider.cs | 59 ++++++++++++++++++- 2 files changed, 96 insertions(+), 3 deletions(-) diff --git a/GitIntegration.Test/Hosting/GitHubProviderTests.cs b/GitIntegration.Test/Hosting/GitHubProviderTests.cs index 9b820d7..3e9f17f 100644 --- a/GitIntegration.Test/Hosting/GitHubProviderTests.cs +++ b/GitIntegration.Test/Hosting/GitHubProviderTests.cs @@ -854,5 +854,45 @@ public async Task DefaultsToTheUserRouteItAlwaysUsed() Assert.AreEqual("/users/contoso/repos", handler.Requests[0].Uri.AbsolutePath); } + [TestMethod] + public async Task ReportsTheSingleSignOnAuthorisationUrlOnAForbiddenResponse() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond( + HttpStatusCode.Forbidden, + "{\"message\":\"Resource protected by organization SAML enforcement.\"}", + ("Content-Type", "application/json"), + ("X-GitHub-SSO", "required; url=https://github.com/orgs/contoso/sso?authorization_request=ABC123")); + GitHubProvider provider = new() + { + Owner = "contoso".As(), + OwnerKind = GitHubOwnerKind.Organization, + Handler = handler, + }; + + GitHostingAuthenticationException exception = + await Assert.ThrowsExactlyAsync( + async () => await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false)) + .ConfigureAwait(false); + + StringAssert.Contains(exception.Message, "https://github.com/orgs/contoso/sso?authorization_request=ABC123"); + } + + [TestMethod] + public async Task StillReportsAForbiddenResponseCarryingNoSingleSignOnHeader() + { + // The URL is an addition to the message, never a requirement for classifying the failure. + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond( + HttpStatusCode.Forbidden, + "{\"message\":\"Bad credentials\"}", + ("Content-Type", "application/json")); + GitHubProvider provider = new() { Owner = "contoso".As(), Handler = handler }; + + _ = await Assert.ThrowsExactlyAsync( + async () => await provider.GetRepositoriesAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false)) + .ConfigureAwait(false); + } + public TestContext TestContext { get; set; } = null!; } diff --git a/GitIntegration/GitHubProvider.cs b/GitIntegration/GitHubProvider.cs index ea8c87a..2a06804 100644 --- a/GitIntegration/GitHubProvider.cs +++ b/GitIntegration/GitHubProvider.cs @@ -89,7 +89,7 @@ public sealed class GitHubProvider : GitProvider /// private repositories. calls /// GET /orgs/{org}/repos, which does report private repositories the credential can see. /// calls GET /user/repos with - /// affiliation=owner,organization_member. + /// affiliation=owner, organization_member. /// /// /// GET /user/repos takes no owner parameter — it always describes the token's own @@ -481,14 +481,24 @@ private GitHostingException Translate(ApiException exception) // GitHostingException itself defaults to, rather than throwing while translating a throw. string responseBody = exception.HttpResponse?.Body as string ?? string.Empty; + // A token that is valid but unauthorised for an organisation's single sign-on arrives as a + // plain 403, indistinguishable in status and body from a bad credential. The header is the + // only thing carrying the URL that resolves it, and that URL is the whole remedy — without + // it, the two failures a caller most needs to tell apart read identically. + string? singleSignOnUrl = TryGetSingleSignOnUrl(exception); + string authenticationMessage = singleSignOnUrl is null + ? exception.Message + : $"{exception.Message} This organisation requires single sign-on authorisation for " + + $"this credential. Authorise it at: {singleSignOnUrl}"; + return exception switch { RateLimitExceededException rateLimit => new GitHostingRateLimitException(exception.Message, Name, exception.StatusCode, responseBody, rateLimit.Reset, exception), SecondaryRateLimitExceededException => new GitHostingRateLimitException(exception.Message, Name, exception.StatusCode, responseBody, resetsAt: null, exception), AbuseException abuse => new GitHostingRateLimitException(exception.Message, Name, exception.StatusCode, responseBody, ToResetTime(abuse.RetryAfterSeconds), exception), { StatusCode: HttpStatusCode.TooManyRequests } => new GitHostingRateLimitException(exception.Message, Name, exception.StatusCode, responseBody, ToResetTime(TryGetRetryAfterSeconds(exception)), exception), - AuthorizationException => new GitHostingAuthenticationException(exception.Message, Name, exception.StatusCode, responseBody, exception), - ForbiddenException => new GitHostingAuthenticationException(exception.Message, Name, exception.StatusCode, responseBody, exception), + AuthorizationException => new GitHostingAuthenticationException(authenticationMessage, Name, exception.StatusCode, responseBody, exception), + ForbiddenException => new GitHostingAuthenticationException(authenticationMessage, Name, exception.StatusCode, responseBody, exception), NotFoundException => new GitHostingNotFoundException(exception.Message, Name, exception.StatusCode, responseBody, exception), _ => new GitHostingRequestException(exception.Message, Name, exception.StatusCode, responseBody, exception), }; @@ -548,4 +558,47 @@ private GitHostingException Translate(ApiException exception) return null; } + + /// + /// Reads the authorisation URL from a failed response's single sign-on header, if it carries one. + /// + /// + /// Scanned case-insensitively rather than looked up by key, for the reason + /// gives: HTTP header names are case-insensitive and + /// Octokit's header dictionary compares them ordinally, so a keyed lookup would work only by + /// matching whatever casing Octokit happens to canonicalise this header to. + /// + /// The header's value is a parameter list, required; url=<uri>. Only the URL is read, + /// and anything else in the list is left alone: the caller's whole use for this is a link to open. + /// + /// + /// The failed response. + /// The authorisation URL, or when the header is absent or carries none. + private static string? TryGetSingleSignOnUrl(ApiException exception) + { + const string headerName = "X-GitHub-SSO"; + const string urlParameter = "url="; + + if (exception.HttpResponse?.Headers is not IReadOnlyDictionary headers) + { + return null; + } + + foreach (KeyValuePair header in headers.Where( + candidate => candidate.Key.Equals(headerName, StringComparison.OrdinalIgnoreCase))) + { + foreach (string parameter in header.Value.Split(';')) + { + string trimmed = parameter.Trim(); + + if (trimmed.StartsWith(urlParameter, StringComparison.OrdinalIgnoreCase)) + { + string url = trimmed[urlParameter.Length..]; + return url.Length == 0 ? null : url; + } + } + } + + return null; + } } From 31d6857c58a5bbf27ac09ad5176089f593fdee70 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Tue, 22 Sep 2026 10:46:37 +1000 Subject: [PATCH 10/16] chore: map hostname-derived author addresses No user.email is configured on this machine, so git derives one from the hostname: commits land as matt@Mac.home or matt@MattBookPro.local rather than under an address .mailmap already knows. Both now resolve to the same identity as every other alias, which also cleans up the matt@MattBookPro.local commits already on main. Co-Authored-By: Claude Opus 5 (1M context) --- .mailmap | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.mailmap b/.mailmap index 7047021..99f93b9 100644 --- a/.mailmap +++ b/.mailmap @@ -2,6 +2,8 @@ matt-edmondson matt-edmondson matt-edmondson matt-edmondson +matt-edmondson +matt-edmondson dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> From 47d49182798aa82ca0716d626a512745c9c35c9e Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Tue, 22 Sep 2026 11:00:30 +1000 Subject: [PATCH 11/16] feat: obtain a GitHub credential through the OAuth device flow Co-Authored-By: Claude Sonnet 5 --- .../Hosting/GitHubDeviceFlowTests.cs | 134 ++++++ GitIntegration/Hosting/GitHubDeviceFlow.cs | 388 ++++++++++++++++++ .../SemanticTypes/GitProviderTypes.cs | 11 + ...-09-21-worktrees-and-github-auth-design.md | 6 + 4 files changed, 539 insertions(+) create mode 100644 GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs create mode 100644 GitIntegration/Hosting/GitHubDeviceFlow.cs diff --git a/GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs b/GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs new file mode 100644 index 0000000..53440e4 --- /dev/null +++ b/GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs @@ -0,0 +1,134 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration.Test; + +using System; +using System.Net; +using System.Threading.Tasks; + +using ktsu.Semantics.Strings; + +[TestClass] +public sealed class GitHubDeviceFlowTests +{ + public TestContext TestContext { get; set; } = null!; + + private const string DeviceCodeBody = + "{\"device_code\":\"dev-abc\",\"user_code\":\"WXYZ-1234\"," + + "\"verification_uri\":\"https://github.com/login/device\"," + + "\"expires_in\":900,\"interval\":5}"; + + private static GitHubDeviceFlow CreateFlow(FakeHttpMessageHandler handler) => + new("Iv1.0123456789abcdef".As(), ["repo", "read:org"]) { Handler = handler }; + + [TestMethod] + public async Task RequestsADeviceCodeAndReportsWhatTheUserNeeds() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + + GitHubDeviceCode code = await CreateFlow(handler) + .RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual("WXYZ-1234", code.UserCode); + Assert.AreEqual("dev-abc", code.DeviceCode); + Assert.AreEqual(new Uri("https://github.com/login/device"), code.VerificationUri); + Assert.AreEqual(TimeSpan.FromSeconds(900), code.ExpiresIn); + Assert.AreEqual(TimeSpan.FromSeconds(5), code.Interval); + } + + [TestMethod] + public async Task SendsTheRequestedScopes() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + + _ = await CreateFlow(handler) + .RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + StringAssert.Contains(handler.Requests[0].Body, "repo"); + StringAssert.Contains(handler.Requests[0].Body, "read:org"); + } + + [TestMethod] + public async Task ReturnsAHostNativeTokenOnSuccess() + { + // FromToken, not FromBearerToken: a GitHub OAuth token travels under Octokit's Token scheme. + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + _ = handler.Respond( + HttpStatusCode.OK, + "{\"access_token\":\"gho_realtoken\",\"token_type\":\"bearer\",\"scope\":\"repo,read:org\"}", + ("Content-Type", "application/json")); + + GitHubDeviceFlow flow = CreateFlow(handler); + GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + HostingCredential credential = await flow.WaitForTokenAsync(code, TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual(HostingCredentialKind.Token, credential.Kind); + Assert.AreEqual("gho_realtoken", credential.Token); + } + + [TestMethod] + public async Task ReportsARefusedAuthorisationAsAnAuthenticationFailure() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + _ = handler.Respond( + HttpStatusCode.OK, + "{\"error\":\"access_denied\",\"error_description\":\"The user denied the request.\"}", + ("Content-Type", "application/json")); + + GitHubDeviceFlow flow = CreateFlow(handler); + GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + GitHostingAuthenticationException exception = + await Assert.ThrowsExactlyAsync( + async () => await flow.WaitForTokenAsync(code, TestContext.CancellationTokenSource.Token).ConfigureAwait(false)) + .ConfigureAwait(false); + + StringAssert.Contains(exception.Message, "access_denied"); + } + + [TestMethod] + public async Task ReportsAnExpiredCodeAsAnAuthenticationFailure() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + _ = handler.Respond( + HttpStatusCode.OK, + "{\"error\":\"expired_token\",\"error_description\":\"The device code has expired.\"}", + ("Content-Type", "application/json")); + + GitHubDeviceFlow flow = CreateFlow(handler); + GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + _ = await Assert.ThrowsExactlyAsync( + async () => await flow.WaitForTokenAsync(code, TestContext.CancellationTokenSource.Token).ConfigureAwait(false)) + .ConfigureAwait(false); + } + + [TestMethod] + public async Task ReportsAnUnusableClientIdentifierAsARequestFailure() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond( + HttpStatusCode.NotFound, + "{\"error\":\"Not Found\"}", + ("Content-Type", "application/json")); + + _ = await Assert.ThrowsExactlyAsync( + async () => await CreateFlow(handler) + .RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false)) + .ConfigureAwait(false); + } + + [TestMethod] + public void RejectsANullClientIdentifier() => + _ = Assert.ThrowsExactly(() => new GitHubDeviceFlow(null!, ["repo"])); + + [TestMethod] + public void RejectsNullScopes() => + _ = Assert.ThrowsExactly( + () => new GitHubDeviceFlow("Iv1.0123456789abcdef".As(), null!)); +} diff --git a/GitIntegration/Hosting/GitHubDeviceFlow.cs b/GitIntegration/Hosting/GitHubDeviceFlow.cs new file mode 100644 index 0000000..3d220dc --- /dev/null +++ b/GitIntegration/Hosting/GitHubDeviceFlow.cs @@ -0,0 +1,388 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration; + +using System; +using System.Collections.Generic; +using System.Net.Http; +using System.Net.Http.Headers; +using System.Text; +using System.Text.Json; +using System.Text.Json.Serialization; +using System.Threading; +using System.Threading.Tasks; + +/// +/// What GitHub issues when a device flow begins: the code a human types, and the code the polling +/// request carries. +/// +/// +/// and are different values for different audiences. +/// The user code is short and is displayed, the device code is opaque and is what +/// sends. Showing the device code to a user, or +/// sending the user code in its place, both fail in ways that look like a broken sign-in. +/// +public sealed record GitHubDeviceCode +{ + /// Gets the code the user enters at . + public required string UserCode { get; init; } + + /// Gets the code the token request carries. Not shown to the user. + public required string DeviceCode { get; init; } + + /// Gets the page the user opens to enter . + public required Uri VerificationUri { get; init; } + + /// Gets how long this code remains usable. + /// Surfaced because a caller showing a countdown needs it and cannot derive it. + public required TimeSpan ExpiresIn { get; init; } + + /// Gets the minimum wait GitHub requires between token requests. + public required TimeSpan Interval { get; init; } +} + +/// +/// Obtains a GitHub credential through the OAuth device flow. +/// +/// +/// +/// Two calls rather than one. Device flow has an inherent pause in the middle: GitHub issues a +/// short code, the user types it into a browser, and only then does polling succeed, and that pause +/// is minutes long with the code on screen throughout. A single method taking a "here's the code" +/// callback would invoke it from whatever thread an HTTP continuation resumed on, leaving every +/// graphical caller to marshal the code back to a user interface thread from inside a callback it +/// does not control. Splitting the call puts the seam where the pause already is. +/// +/// +/// Separate from , whose every other method is one request and one +/// response. Nothing here resolves or applies a credential, this type produces one and +/// consumes one, through the credential cache or +/// . +/// +/// +/// Stores nothing. returns the credential and the caller decides +/// where it lives: +/// +/// +/// GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(cancellationToken); +/// // show code.UserCode and open code.VerificationUri +/// HostingCredential credential = await flow.WaitForTokenAsync(code, cancellationToken); +/// CredentialCache.Instance.AddOrReplace(persona, new CredentialWithToken { Token = credential.Token! }); +/// +/// +/// Built on a raw rather than on Octokit.IOauthClient, even though +/// Octokit exposes exactly this pair of calls. Two things about Octokit 14.0.0's own device-flow +/// implementation make it unusable here without changing what this type promises: +/// +/// +/// +/// OauthClient.InitiateDeviceFlow sends scope as a comma-joined, +/// form-url-encoded value, so a scope such as read:org travels as read%3Aorg. GitHub's +/// documented device-flow request accepts a JSON body, so this type sends one instead and the colon +/// survives unencoded. +/// +/// +/// OauthClient.CreateAccessTokenForDeviceFlow throws Octokit.ApiException for a body +/// carrying an OAuth error field (access_denied, expired_token, and so on) +/// rather than returning an OauthToken with Error populated, which collapses a refused +/// authorisation and an expired code into the same exception shape a transport failure gets. This +/// type reads the error field itself, so it can tell that apart as +/// rather than . +/// +/// +/// +/// Both were found empirically, against the actual package, while implementing this type. Reading +/// the response body by hand also means this type owns the poll loop: it waits +/// between attempts, widening it by five seconds whenever +/// GitHub answers slow_down, exactly as GitHub's documentation for the device flow describes. +/// +/// +/// The OAuth App's client identifier. +/// The scopes to request, such as repo and read:org. +public sealed class GitHubDeviceFlow(GitHubOAuthClientId clientId, IReadOnlyList scopes) +{ + /// GitHub's endpoint for beginning a device flow. + private static readonly Uri DeviceCodeEndpoint = new("https://github.com/login/device/code"); + + /// GitHub's endpoint for exchanging a device code for a token. + private static readonly Uri AccessTokenEndpoint = new("https://github.com/login/oauth/access_token"); + + /// The grant type GitHub's device-flow token exchange expects. + private const string DeviceCodeGrantType = "urn:ietf:params:oauth:grant-type:device_code"; + + /// How much longer to wait after GitHub answers slow_down. + private static readonly TimeSpan SlowDownIncrement = TimeSpan.FromSeconds(5); + + /// + /// The transport every flow shares when no was injected. + /// + /// + /// Its own instance rather than 's, matching that type's reasoning: + /// one handler for the process rather than one per call, and + /// is what keeps the settings from drifting apart. + /// + private static readonly SocketsHttpHandler SharedHandler = GitProvider.CreateDefaultHandler(); + + private readonly GitHubOAuthClientId _clientId = Ensure.NotNull(clientId); + private readonly IReadOnlyList _scopes = Ensure.NotNull(scopes); + + /// + /// Gets or initializes the transport this flow issues requests through, or + /// to use the shared one. + /// + /// + /// Internal rather than public, so no transport type appears in this library's public API, and + /// the test project injects a fake through InternalsVisibleTo, the same seam + /// provides. + /// + internal HttpMessageHandler? Handler { get; init; } + + /// + /// Asks GitHub to begin a device flow. + /// + /// Cancels the request. + /// The codes and timings the flow's second half needs. + /// GitHub refused or could not be reached. + public async Task RequestDeviceCodeAsync(CancellationToken cancellationToken = default) + { + cancellationToken.ThrowIfCancellationRequested(); + + GitHubDeviceCodeRequestBody requestBody = new() + { + ClientId = _clientId.WeakString, + Scope = string.Join(' ', _scopes), + }; + + using HttpClient client = CreateHttpClient(); + using HttpRequestMessage request = CreateJsonRequest( + DeviceCodeEndpoint, JsonSerializer.Serialize(requestBody, GitHubDeviceFlowJsonContext.Default.GitHubDeviceCodeRequestBody)); + + using HttpResponseMessage response = await client.SendAsync(request, cancellationToken).ConfigureAwait(false); + string body = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); + + if (!response.IsSuccessStatusCode) + { + throw new GitHostingRequestException( + $"GitHub refused to begin a device flow: {(int)response.StatusCode} {response.ReasonPhrase}. {body}".TrimEnd()); + } + + GitHubDeviceCodeResponseBody parsed = DeserializeOrThrow( + body, GitHubDeviceFlowJsonContext.Default.GitHubDeviceCodeResponseBody, "device code"); + + return new GitHubDeviceCode + { + UserCode = parsed.UserCode, + DeviceCode = parsed.DeviceCode, + VerificationUri = new Uri(parsed.VerificationUri), + // GitHub reports both as integer seconds, the conversion happens once, here. + ExpiresIn = TimeSpan.FromSeconds(parsed.ExpiresIn), + Interval = TimeSpan.FromSeconds(parsed.Interval), + }; + } + + /// + /// Waits for the user to authorise the flow, then returns the credential GitHub issues. + /// + /// + /// Polls until the user authorises, the code expires, or is + /// cancelled, so this may block for as long as . A + /// authorization_pending answer waits and tries + /// again; a slow_down answer widens that wait by first. + /// + /// What returned. + /// Abandons the wait. + /// A host-native token credential. + /// is . + /// The user refused, or the code expired. + /// GitHub refused the request or could not be reached. + public async Task WaitForTokenAsync(GitHubDeviceCode code, CancellationToken cancellationToken = default) + { + Ensure.NotNull(code); + cancellationToken.ThrowIfCancellationRequested(); + + using HttpClient client = CreateHttpClient(); + + GitHubAccessTokenRequestBody requestBody = new() + { + ClientId = _clientId.WeakString, + DeviceCode = code.DeviceCode, + GrantType = DeviceCodeGrantType, + }; + + string requestJson = JsonSerializer.Serialize(requestBody, GitHubDeviceFlowJsonContext.Default.GitHubAccessTokenRequestBody); + + TimeSpan interval = code.Interval; + + while (true) + { + cancellationToken.ThrowIfCancellationRequested(); + + using HttpRequestMessage request = CreateJsonRequest(AccessTokenEndpoint, requestJson); + using HttpResponseMessage response = await client.SendAsync(request, cancellationToken).ConfigureAwait(false); + string body = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); + + if (!response.IsSuccessStatusCode) + { + throw new GitHostingRequestException( + $"GitHub refused the device flow token request: {(int)response.StatusCode} {response.ReasonPhrase}. {body}".TrimEnd()); + } + + GitHubAccessTokenResponseBody parsed = DeserializeOrThrow( + body, GitHubDeviceFlowJsonContext.Default.GitHubAccessTokenResponseBody, "access token"); + + switch (parsed.Error) + { + case "authorization_pending": + await Task.Delay(interval, cancellationToken).ConfigureAwait(false); + continue; + + case "slow_down": + interval += SlowDownIncrement; + await Task.Delay(interval, cancellationToken).ConfigureAwait(false); + continue; + + case string error: + // GitHub reports a refusal and an expiry as a 200 carrying an error field + // rather than as a failure status, which is why this is read from the body + // rather than caught as a failed HttpResponseMessage above. + throw new GitHostingAuthenticationException( + $"GitHub did not issue a token: {error}. {parsed.ErrorDescription}".TrimEnd()); + + case null when string.IsNullOrEmpty(parsed.AccessToken): + throw new GitHostingRequestException("GitHub reported neither a token nor an error."); + + default: + // FromToken, not FromBearerToken: a GitHub OAuth token travels under Octokit's + // Token scheme, which is what FromToken means. FromBearerToken is for an Entra + // ID access token against Azure DevOps. + return HostingCredential.FromToken(parsed.AccessToken!); + } + } + } + + /// Creates the transport this flow's calls share. + /// + /// unconditionally: has to + /// outlive every call, and an injected belongs to whoever supplied it. + /// + /// The client, to be disposed once the call using it is done. + private HttpClient CreateHttpClient() => new(Handler ?? SharedHandler, disposeHandler: false); + + /// Builds a JSON POST request, asking GitHub to answer in JSON as well. + /// The endpoint to post to. + /// The already-serialized request body. + /// The request. + private static HttpRequestMessage CreateJsonRequest(Uri endpoint, string json) + { + HttpRequestMessage request = new(HttpMethod.Post, endpoint) + { + Content = new StringContent(json, Encoding.UTF8, "application/json"), + }; + + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); + + return request; + } + + /// Deserializes a response body, translating a malformed one into this library's own exception. + /// The shape the body is expected to have. + /// The response body, already read. + /// The source-generated metadata to deserialize with. + /// What the body was expected to describe, folded into the failure message. + /// The deserialized body. + /// The body is not valid JSON of the expected shape. + private static T DeserializeOrThrow(string body, System.Text.Json.Serialization.Metadata.JsonTypeInfo typeInfo, string what) + { + try + { + return JsonSerializer.Deserialize(body, typeInfo) + ?? throw new GitHostingRequestException($"GitHub reported success but returned an empty {what} body."); + } + catch (JsonException exception) + { + throw new GitHostingRequestException( + $"GitHub reported success but returned a {what} body that is not the expected JSON: {exception.Message}", + exception); + } + } +} + +/// The body sends. +internal sealed class GitHubDeviceCodeRequestBody +{ + /// Gets the OAuth App's client identifier. + [JsonPropertyName("client_id")] + public required string ClientId { get; init; } + + /// Gets the requested scopes, space-separated. + [JsonPropertyName("scope")] + public required string Scope { get; init; } +} + +/// The body GitHub's device code endpoint returns. +internal sealed class GitHubDeviceCodeResponseBody +{ + /// Gets the code the token request carries. + [JsonPropertyName("device_code")] + public required string DeviceCode { get; init; } + + /// Gets the code the user enters. + [JsonPropertyName("user_code")] + public required string UserCode { get; init; } + + /// Gets the page the user opens. + [JsonPropertyName("verification_uri")] + public required string VerificationUri { get; init; } + + /// Gets how long the codes remain usable, in seconds. + [JsonPropertyName("expires_in")] + public required int ExpiresIn { get; init; } + + /// Gets the minimum wait between token requests, in seconds. + [JsonPropertyName("interval")] + public required int Interval { get; init; } +} + +/// The body sends. +internal sealed class GitHubAccessTokenRequestBody +{ + /// Gets the OAuth App's client identifier. + [JsonPropertyName("client_id")] + public required string ClientId { get; init; } + + /// Gets the device code the earlier request obtained. + [JsonPropertyName("device_code")] + public required string DeviceCode { get; init; } + + /// Gets the grant type identifying this as a device-flow exchange. + [JsonPropertyName("grant_type")] + public required string GrantType { get; init; } +} + +/// The body GitHub's token endpoint returns. +internal sealed class GitHubAccessTokenResponseBody +{ + /// Gets the issued token, or when is set. + [JsonPropertyName("access_token")] + public string? AccessToken { get; init; } + + /// + /// Gets the OAuth error code, such as authorization_pending, slow_down, + /// access_denied, or expired_token, or on success. + /// + [JsonPropertyName("error")] + public string? Error { get; init; } + + /// Gets the human-readable detail accompanying , when GitHub sent one. + [JsonPropertyName("error_description")] + public string? ErrorDescription { get; init; } +} + +/// Source-generated JSON metadata for the device flow's request and response bodies. +[JsonSerializable(typeof(GitHubDeviceCodeRequestBody))] +[JsonSerializable(typeof(GitHubDeviceCodeResponseBody))] +[JsonSerializable(typeof(GitHubAccessTokenRequestBody))] +[JsonSerializable(typeof(GitHubAccessTokenResponseBody))] +internal sealed partial class GitHubDeviceFlowJsonContext : JsonSerializerContext +{ +} diff --git a/GitIntegration/SemanticTypes/GitProviderTypes.cs b/GitIntegration/SemanticTypes/GitProviderTypes.cs index e27771e..5d541cf 100644 --- a/GitIntegration/SemanticTypes/GitProviderTypes.cs +++ b/GitIntegration/SemanticTypes/GitProviderTypes.cs @@ -58,3 +58,14 @@ public sealed record GitPullRequestAuthor : SemanticString /// [HasNonWhitespaceContent] public sealed record GitPullRequestWebURI : SemanticString { } + +/// +/// The client identifier GitHub issues when an OAuth App is registered. +/// +/// +/// Not a secret. The device flow has no client secret precisely because a desktop binary cannot keep +/// one, so a consuming application may hold this in ordinary configuration. It is a value this +/// library takes rather than one it ships: the identifier belongs to whoever registered the app. +/// +[HasNonWhitespaceContent] +public sealed record GitHubOAuthClientId : SemanticString { } diff --git a/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md b/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md index a8ee9ac..077b6e2 100644 --- a/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md +++ b/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md @@ -261,6 +261,7 @@ already reads `Retry-After`, and for the same reason. public sealed record GitHubDeviceCode { public required string UserCode { get; init; } + public required string DeviceCode { get; init; } public required Uri VerificationUri { get; init; } public required TimeSpan ExpiresIn { get; init; } public required TimeSpan Interval { get; init; } @@ -273,6 +274,11 @@ public sealed class GitHubDeviceFlow(GitHubOAuthClientId clientId, IReadOnlyList } ``` +`DeviceCode` is the opaque code the polling request carries, distinct from `UserCode`, which is the +short string a human types at `VerificationUri`. Octokit's own device-flow exchange needs the device +code to resume, and showing one code where the other belongs fails in a way that looks like a broken +sign-in. + ### Why two calls rather than one Device flow has an inherent pause in the middle: GitHub issues a short user code, the user types it From 7660067690e3140e4f616da5a13b33c70f93dc14 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Tue, 22 Sep 2026 11:12:16 +1000 Subject: [PATCH 12/16] fix: enforce the device flow's expiry deadline and poll interval floor Address review findings on the OAuth device flow: WaitForTokenAsync now tracks an elapsed-time budget from a monotonic clock and throws once the device code's expiry passes, rather than polling forever against a server that never resolves. Every wait between polls is floored to GitHub's documented 5-second minimum, applied at the point the wait is issued so a hand-built GitHubDeviceCode is covered as well as a parsed one. Also stops echoing the token endpoint's response body into a failure message, and adds coverage for the pending/slow-down/expiry/floor/cancellation paths the poll loop previously left untested. Co-Authored-By: Claude Sonnet 5 --- .../Hosting/GitHubDeviceFlowTests.cs | 209 +++++++++++++++++- GitIntegration/Hosting/GitHubDeviceFlow.cs | 76 ++++++- ...-09-21-worktrees-and-github-auth-design.md | 5 +- 3 files changed, 280 insertions(+), 10 deletions(-) diff --git a/GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs b/GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs index 53440e4..cb2376e 100644 --- a/GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs +++ b/GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs @@ -3,7 +3,9 @@ namespace ktsu.GitIntegration.Test; using System; +using System.Collections.Generic; using System.Net; +using System.Threading; using System.Threading.Tasks; using ktsu.Semantics.Strings; @@ -18,8 +20,35 @@ public sealed class GitHubDeviceFlowTests "\"verification_uri\":\"https://github.com/login/device\"," + "\"expires_in\":900,\"interval\":5}"; - private static GitHubDeviceFlow CreateFlow(FakeHttpMessageHandler handler) => - new("Iv1.0123456789abcdef".As(), ["repo", "read:org"]) { Handler = handler }; + private const string DeviceCodeBodyWithZeroInterval = + "{\"device_code\":\"dev-abc\",\"user_code\":\"WXYZ-1234\"," + + "\"verification_uri\":\"https://github.com/login/device\"," + + "\"expires_in\":900,\"interval\":0}"; + + private const string SuccessTokenBody = + "{\"access_token\":\"gho_realtoken\",\"token_type\":\"bearer\",\"scope\":\"repo,read:org\"}"; + + private const string AuthorizationPendingBody = "{\"error\":\"authorization_pending\"}"; + + private const string SlowDownBody = "{\"error\":\"slow_down\"}"; + + /// + /// Creates a flow against , optionally replacing the wait and clock a + /// test needs to observe or fast-forward without a real wait. Omitting both keeps this identical + /// to the flow's own defaults ( and + /// ), which is what the tests already written against this + /// helper rely on. + /// + private static GitHubDeviceFlow CreateFlow( + FakeHttpMessageHandler handler, + Func? delay = null, + Func? nowTicks = null) => + new("Iv1.0123456789abcdef".As(), ["repo", "read:org"]) + { + Handler = handler, + Delay = delay ?? Task.Delay, + NowTicks = nowTicks ?? (() => Environment.TickCount64), + }; [TestMethod] public async Task RequestsADeviceCodeAndReportsWhatTheUserNeeds() @@ -46,8 +75,10 @@ public async Task SendsTheRequestedScopes() _ = await CreateFlow(handler) .RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); - StringAssert.Contains(handler.Requests[0].Body, "repo"); - StringAssert.Contains(handler.Requests[0].Body, "read:org"); + // Space-separated, not comma-joined: GitHub's device-flow request wants scopes + // space-separated, and this exact substring is what rules out a comma (or any other + // separator) sneaking back in. + StringAssert.Contains(handler.Requests[0].Body, "\"scope\":\"repo read:org\""); } [TestMethod] @@ -123,6 +154,176 @@ public async Task ReportsAnUnusableClientIdentifierAsARequestFailure() .ConfigureAwait(false); } + [TestMethod] + public async Task PollRequestCarriesTheDeviceCodeAndNotTheUserCodeAsync() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + _ = handler.Respond(HttpStatusCode.OK, SuccessTokenBody, ("Content-Type", "application/json")); + + GitHubDeviceFlow flow = CreateFlow(handler); + GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + _ = await flow.WaitForTokenAsync(code, TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual(new Uri("https://github.com/login/device/code"), handler.Requests[0].Uri); + Assert.AreEqual(new Uri("https://github.com/login/oauth/access_token"), handler.Requests[1].Uri); + StringAssert.Contains(handler.Requests[1].Body, "\"device_code\":\"dev-abc\""); + StringAssert.Contains(handler.Requests[1].Body, "\"grant_type\":\"urn:ietf:params:oauth:grant-type:device_code\""); + + // The exact confusion GitHubDeviceCode's own XML doc warns about: the poll request must + // never carry the code a human types, only the opaque one it was issued alongside. + Assert.IsFalse(handler.Requests[1].Body!.Contains("user_code", StringComparison.Ordinal)); + } + + [TestMethod] + public async Task ReportsAResponseWithNeitherTokenNorErrorAsARequestFailureAsync() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + _ = handler.Respond(HttpStatusCode.OK, "{\"token_type\":\"bearer\"}", ("Content-Type", "application/json")); + + GitHubDeviceFlow flow = CreateFlow(handler); + GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + _ = await Assert.ThrowsExactlyAsync( + async () => await flow.WaitForTokenAsync(code, TestContext.CancellationTokenSource.Token).ConfigureAwait(false)) + .ConfigureAwait(false); + } + + [TestMethod] + public async Task PollsThroughAuthorizationPendingToSuccessAsync() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + _ = handler.Respond(HttpStatusCode.OK, AuthorizationPendingBody, ("Content-Type", "application/json")); + _ = handler.Respond(HttpStatusCode.OK, SuccessTokenBody, ("Content-Type", "application/json")); + + // A no-op wait: this test is about the pending-then-success transition, not about timing, so + // the interval is never actually slept. + GitHubDeviceFlow flow = CreateFlow(handler, delay: (_, _) => Task.CompletedTask); + + GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + HostingCredential credential = await flow.WaitForTokenAsync(code, TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual("gho_realtoken", credential.Token); + Assert.AreEqual(3, handler.Requests.Count); + } + + [TestMethod] + public async Task WidensTheIntervalCumulativelyOnRepeatedSlowDownAsync() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + _ = handler.Respond(HttpStatusCode.OK, SlowDownBody, ("Content-Type", "application/json")); + _ = handler.Respond(HttpStatusCode.OK, SlowDownBody, ("Content-Type", "application/json")); + _ = handler.Respond(HttpStatusCode.OK, SuccessTokenBody, ("Content-Type", "application/json")); + + List delays = []; + + GitHubDeviceFlow flow = CreateFlow(handler, delay: (interval, _) => + { + delays.Add(interval); + return Task.CompletedTask; + }); + + GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + _ = await flow.WaitForTokenAsync(code, TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + // DeviceCodeBody's interval is 5 seconds. Two slow_down answers must widen it twice, to 10 + // then 15, not reset it back to 10 each time. + Assert.AreEqual(2, delays.Count); + Assert.AreEqual(TimeSpan.FromSeconds(10), delays[0]); + Assert.AreEqual(TimeSpan.FromSeconds(15), delays[1]); + } + + [TestMethod] + public async Task FloorsAZeroIntervalFromTheWireAsync() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBodyWithZeroInterval, ("Content-Type", "application/json")); + _ = handler.Respond(HttpStatusCode.OK, AuthorizationPendingBody, ("Content-Type", "application/json")); + _ = handler.Respond(HttpStatusCode.OK, SuccessTokenBody, ("Content-Type", "application/json")); + + List delays = []; + + GitHubDeviceFlow flow = CreateFlow(handler, delay: (interval, _) => + { + delays.Add(interval); + return Task.CompletedTask; + }); + + GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + Assert.AreEqual(TimeSpan.Zero, code.Interval); + + _ = await flow.WaitForTokenAsync(code, TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual(1, delays.Count); + Assert.AreEqual(TimeSpan.FromSeconds(5), delays[0]); + } + + [TestMethod] + public async Task ThrowsOnceTheDeviceCodeExpiresAsync() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + _ = handler.Respond(HttpStatusCode.OK, AuthorizationPendingBody, ("Content-Type", "application/json")); + + // The fake clock jumps past DeviceCodeBody's 900-second expiry the moment the first wait is + // asked for, simulating a server that answers authorization_pending forever without this + // test actually waiting 900 seconds for it. + long now = 0; + GitHubDeviceFlow flow = CreateFlow( + handler, + delay: (_, _) => + { + now += (long)TimeSpan.FromSeconds(900).TotalMilliseconds + 1_000; + return Task.CompletedTask; + }, + nowTicks: () => now); + + GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + GitHostingAuthenticationException exception = + await Assert.ThrowsExactlyAsync( + async () => await flow.WaitForTokenAsync(code, TestContext.CancellationTokenSource.Token).ConfigureAwait(false)) + .ConfigureAwait(false); + + StringAssert.Contains(exception.Message, "expired"); + + // Exactly the one poll that answered authorization_pending: the deadline is caught before a + // second poll is ever sent, not by a poll response saying so. + Assert.AreEqual(2, handler.Requests.Count); + } + + [TestMethod] + public async Task CancelsDuringTheWaitBetweenPollsAsync() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + _ = handler.Respond(HttpStatusCode.OK, AuthorizationPendingBody, ("Content-Type", "application/json")); + + using CancellationTokenSource cts = new(); + TaskCompletionSource waitStarted = new(TaskCreationOptions.RunContinuationsAsynchronously); + + // Never completes on its own: it only resolves when the token passed to it is cancelled, + // which is exactly the wait this test needs to cancel into rather than before. + GitHubDeviceFlow flow = CreateFlow(handler, delay: (_, cancellationToken) => + { + waitStarted.TrySetResult(); + return Task.Delay(Timeout.InfiniteTimeSpan, cancellationToken); + }); + + GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Task waitTask = flow.WaitForTokenAsync(code, cts.Token); + + await waitStarted.Task.ConfigureAwait(false); + await cts.CancelAsync().ConfigureAwait(false); + + _ = await Assert.ThrowsExactlyAsync( + async () => await waitTask.ConfigureAwait(false)).ConfigureAwait(false); + } + [TestMethod] public void RejectsANullClientIdentifier() => _ = Assert.ThrowsExactly(() => new GitHubDeviceFlow(null!, ["repo"])); diff --git a/GitIntegration/Hosting/GitHubDeviceFlow.cs b/GitIntegration/Hosting/GitHubDeviceFlow.cs index 3d220dc..90de9f8 100644 --- a/GitIntegration/Hosting/GitHubDeviceFlow.cs +++ b/GitIntegration/Hosting/GitHubDeviceFlow.cs @@ -96,6 +96,14 @@ public sealed record GitHubDeviceCode /// between attempts, widening it by five seconds whenever /// GitHub answers slow_down, exactly as GitHub's documentation for the device flow describes. /// +/// +/// Both endpoints are sent a JSON request body, with Accept: application/json, even though +/// GitHub's own published documentation shows the device flow using form-url-encoded requests. +/// GitHub's actual service accepts the JSON form in practice, which is what let +/// avoid the scope-encoding problem above, but this is +/// undocumented behaviour this library now depends on, and nothing in this library's test suite +/// verifies it against the real service, only against the fake transport. +/// /// /// The OAuth App's client identifier. /// The scopes to request, such as repo and read:org. @@ -113,6 +121,20 @@ public sealed class GitHubDeviceFlow(GitHubOAuthClientId clientId, IReadOnlyList /// How much longer to wait after GitHub answers slow_down. private static readonly TimeSpan SlowDownIncrement = TimeSpan.FromSeconds(5); + /// + /// The shortest wait this flow will ever use between polls, regardless of what + /// says. + /// + /// + /// GitHub's own device-flow documentation states five seconds as the minimum polling interval. + /// is public with an unvalidated + /// , so a value of zero, or one hand-built by a caller, + /// must not be able to produce an unthrottled loop against github.com. Applied at the point each + /// wait is issued, not when a wire response is parsed, so a hand-built + /// is covered exactly as a parsed one is. + /// + private static readonly TimeSpan MinimumPollInterval = TimeSpan.FromSeconds(5); + /// /// The transport every flow shares when no was injected. /// @@ -137,6 +159,29 @@ public sealed class GitHubDeviceFlow(GitHubOAuthClientId clientId, IReadOnlyList /// internal HttpMessageHandler? Handler { get; init; } + /// + /// Gets or initializes the wait performs between polls. + /// + /// + /// Defaults to . Internal for the same + /// reason as : this exists so a test can replace minutes of real waiting + /// with an instantaneous one while still exercising the interval, the widening, and the deadline + /// arithmetic that surround it, not so a caller can tune polling behaviour. + /// + internal Func Delay { get; init; } = Task.Delay; + + /// + /// Gets or initializes the monotonic clock reads to enforce + /// . + /// + /// + /// Defaults to , a monotonic source deliberately chosen over + /// or : neither is guaranteed + /// monotonic, and a clock adjustment during a wait that can last minutes must not extend or + /// collapse the window this flow enforces. Internal for the same reason is. + /// + internal Func NowTicks { get; init; } = () => Environment.TickCount64; + /// /// Asks GitHub to begin a device flow. /// @@ -188,6 +233,10 @@ public async Task RequestDeviceCodeAsync(CancellationToken can /// cancelled, so this may block for as long as . A /// authorization_pending answer waits and tries /// again; a slow_down answer widens that wait by first. + /// The deadline is enforced by this flow, not left to GitHub: a server that keeps answering + /// authorization_pending past would otherwise poll + /// forever, since nothing about that answer's shape distinguishes a slow user from a server that + /// never intends to resolve. /// /// What returned. /// Abandons the wait. @@ -213,18 +262,32 @@ public async Task WaitForTokenAsync(GitHubDeviceCode code, Ca TimeSpan interval = code.Interval; + // A monotonic elapsed-time budget rather than a fixed end-of-wall-clock instant: NowTicks + // wraps Environment.TickCount64 by default, which is what makes this immune to the machine's + // clock being changed mid-wait, forward or back. + long startTicks = NowTicks(); + long expiresInMilliseconds = (long)code.ExpiresIn.TotalMilliseconds; + while (true) { cancellationToken.ThrowIfCancellationRequested(); + if (NowTicks() - startTicks >= expiresInMilliseconds) + { + throw new GitHostingAuthenticationException("GitHub did not issue a token before the device code expired."); + } + using HttpRequestMessage request = CreateJsonRequest(AccessTokenEndpoint, requestJson); using HttpResponseMessage response = await client.SendAsync(request, cancellationToken).ConfigureAwait(false); string body = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); if (!response.IsSuccessStatusCode) { + // Status and reason phrase only, not the body: this is the one place in this type + // where a failure response comes from the endpoint that issues tokens, and a body + // echoed back into an exception message is a body that can end up in a log. throw new GitHostingRequestException( - $"GitHub refused the device flow token request: {(int)response.StatusCode} {response.ReasonPhrase}. {body}".TrimEnd()); + $"GitHub refused the device flow token request: {(int)response.StatusCode} {response.ReasonPhrase}".TrimEnd()); } GitHubAccessTokenResponseBody parsed = DeserializeOrThrow( @@ -233,12 +296,12 @@ public async Task WaitForTokenAsync(GitHubDeviceCode code, Ca switch (parsed.Error) { case "authorization_pending": - await Task.Delay(interval, cancellationToken).ConfigureAwait(false); + await WaitAsync(interval, cancellationToken).ConfigureAwait(false); continue; case "slow_down": interval += SlowDownIncrement; - await Task.Delay(interval, cancellationToken).ConfigureAwait(false); + await WaitAsync(interval, cancellationToken).ConfigureAwait(false); continue; case string error: @@ -260,6 +323,13 @@ public async Task WaitForTokenAsync(GitHubDeviceCode code, Ca } } + /// Waits between polls, never for less than . + /// The wait this flow would otherwise use. + /// Cancels the wait. + /// A task that completes once the (possibly floored) wait has elapsed. + private Task WaitAsync(TimeSpan interval, CancellationToken cancellationToken) => + Delay(interval < MinimumPollInterval ? MinimumPollInterval : interval, cancellationToken); + /// Creates the transport this flow's calls share. /// /// unconditionally: has to diff --git a/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md b/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md index 077b6e2..a2d8549 100644 --- a/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md +++ b/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md @@ -275,9 +275,8 @@ public sealed class GitHubDeviceFlow(GitHubOAuthClientId clientId, IReadOnlyList ``` `DeviceCode` is the opaque code the polling request carries, distinct from `UserCode`, which is the -short string a human types at `VerificationUri`. Octokit's own device-flow exchange needs the device -code to resume, and showing one code where the other belongs fails in a way that looks like a broken -sign-in. +short string a human types at `VerificationUri`. Showing one code where the other belongs fails in a +way that looks like a broken sign-in. ### Why two calls rather than one From 20236a924ffa8979709310ee0a2030ff12c4433f Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Tue, 22 Sep 2026 11:27:38 +1000 Subject: [PATCH 13/16] docs: document worktree verbs, owner kinds, and device flow [minor] Extends the README's Features bullets and adds three usage examples for the worktree verbs, GitHubProvider's OwnerKind routing, and the GitHubDeviceFlow sign-in. Also corrects the design doc's Transport and polling section, which still described the device flow as wrapping Octokit's OauthClient; it posts directly to GitHub's device and token endpoints instead, because Octokit percent-encodes the read:org scope's colon and collapses OAuth error bodies into ApiException. Co-Authored-By: Claude Sonnet 5 --- README.md | 93 +++++++++++++++++-- ...-09-21-worktrees-and-github-auth-design.md | 22 ++--- 2 files changed, 94 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index c2a16a8..dd0c608 100644 --- a/README.md +++ b/README.md @@ -38,12 +38,13 @@ builds and parses its requests by hand instead. `IsRepositoryAsync`, `OpenAsync`, `DiscoverAsync` — and creates new ones — `Init(...)`, `Clone(...)` — by delegating every invocation to `ktsu.RunCommand`. - **Fluent Verb Builders**: `GitRepository` exposes one builder per read-only verb — `Status()`, - `Log()`, `Diff()`, `Branches()`, `Tags()`, `Remotes()`, `Submodules()`, `RevParse(...)`, - `RevList(...)`, `Divergence(...)` — and one per mutating verb — `Add()`, `Commit(...)`, - `CreateBranch(...)`, `DeleteBranch(...)`, `CreateTag(...)`, `DeleteTag(...)`, `Checkout(...)`, - `AddRemote(...)`, `RemoveRemote(...)`, `SetRemoteUrl(...)`, `Fetch()`, `Pull()`, `Push()`, - `UpdateSubmodules()` — each configurable via chained method calls and run with `ExecuteAsync` or - the non-throwing `TryExecuteAsync`. + `Log()`, `Diff()`, `Branches()`, `Tags()`, `Remotes()`, `Submodules()`, `Worktrees()`, + `RevParse(...)`, `RevList(...)`, `Divergence(...)` — and one per mutating verb — `Add()`, + `Commit(...)`, `CreateBranch(...)`, `DeleteBranch(...)`, `CreateTag(...)`, `DeleteTag(...)`, + `Checkout(...)`, `AddRemote(...)`, `RemoveRemote(...)`, `SetRemoteUrl(...)`, `Fetch()`, `Pull()`, + `Push()`, `UpdateSubmodules()`, `AddWorktree(...)`, `RemoveWorktree(...)`, `PruneWorktrees()` — + each configurable via chained method calls and run with `ExecuteAsync` or the non-throwing + `TryExecuteAsync`. - **Tags and Submodules**: `Tags()`, `CreateTag(...)`, and `DeleteTag(...)` cover both lightweight and annotated tags; `Submodules()` reports each submodule's recorded gitlink alongside what is actually checked out, and `UpdateSubmodules()` checks the recorded commits out. @@ -53,9 +54,9 @@ builds and parses its requests by hand instead. `GitFetchResult`/`GitPushResult`, and a rejected push is the one place in this library where `ExecuteAsync` and `TryExecuteAsync` diverge in more than exception-versus-result. - **Strongly-Typed Results**: `GitStatus`, `GitCommit`, `GitBranch`, `GitRemote`, `GitDiffEntry`, - `GitVersion`, `GitInitResult`, `GitCompleted`, `GitFetchResult`, `GitPushResult`, and - `GitRefUpdate` records replace ad-hoc porcelain parsing with typed models — `GitCompleted` is the - shared result for mutating verbs whose only outcome is success. + `GitVersion`, `GitInitResult`, `GitCompleted`, `GitFetchResult`, `GitPushResult`, `GitRefUpdate`, + and `GitWorktree` records replace ad-hoc porcelain parsing with typed models — `GitCompleted` is + the shared result for mutating verbs whose only outcome is success. - **Reproducible Failures**: every command is scoped with `git -C ` instead of a process working directory, so a failing invocation's exact argument vector can be read off a `GitCommandException` and rerun verbatim. @@ -68,7 +69,12 @@ builds and parses its requests by hand instead. - **Hosting Provider Abstraction**: `IGitHostingProvider` defines a common contract for enumerating repositories, listing open pull requests, and creating a pull request — `GitHubProvider` implements it on top of Octokit, `AzureDevOpsProvider` on a raw `HttpClient` - against Azure DevOps's REST API. + against Azure DevOps's REST API. `GitHubProvider` routes repository enumeration by `OwnerKind`, + and only `Organization` and `AuthenticatedUser` report private repositories. The default, + `User`, is limited to public ones regardless of the credential supplied. +- **Interactive GitHub Sign-In**: `GitHubDeviceFlow` obtains a credential through GitHub's OAuth + device flow, split into two calls, `RequestDeviceCodeAsync` and `WaitForTokenAsync`, so a caller + can display the user code while the wait for authorisation runs. - **Credential Resolution**: hosting providers integrate with `ktsu.CredentialCache`, so credentials come from the host's native keyring rather than configuration files. `CredentialSource` is the escape hatch for a credential a keyring should not hold, such as a short-lived Entra ID access @@ -156,6 +162,73 @@ IReadOnlyList changes = await repository.Diff() .ExecuteAsync(); ``` +### One Worktree per Branch + +```csharp +using ktsu.GitIntegration; +using ktsu.Semantics.Paths; +using ktsu.Semantics.Strings; + +IReadOnlyList worktrees = await repository.Worktrees().ExecuteAsync(); + +GitBranchName branch = "feature/search".As(); + +if (!worktrees.Any(worktree => worktree.Branch == branch)) +{ + AbsoluteDirectoryPath destination = "/repos/project-feature-search".As(); + + await repository.AddWorktree(destination) + .CreatingBranch(branch) + .From("origin/main".As()) + .ExecuteAsync(); +} +``` + +The main working tree reports `IsMain`, which is how a caller refuses to remove the one that owns +the repository. It is positional: git emits it first, rather than an attribute git labels. + +### Signing In to GitHub + +```csharp +using ktsu.CredentialCache; +using ktsu.GitIntegration; +using ktsu.Semantics.Strings; + +PersonaGUID persona = CredentialCache.CreatePersonaGUID(); + +GitHubDeviceFlow flow = new("Iv1.0123456789abcdef".As(), ["repo", "read:org"]); + +GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(); +Console.WriteLine($"Open {code.VerificationUri} and enter {code.UserCode}"); + +HostingCredential credential = await flow.WaitForTokenAsync(code); +CredentialCache.Instance.AddOrReplace(persona, new CredentialWithToken { Token = credential.Token!.As() }); +``` + +The two calls are split so the user code can stay on screen for the minutes the wait may take. The +flow stores nothing: where the credential lives is the caller's decision. + +### Enumerating an Organisation's Private Repositories + +```csharp +using ktsu.GitIntegration; +using ktsu.Semantics.Strings; + +GitHubProvider provider = new() +{ + Owner = "contoso".As(), + OwnerKind = GitHubOwnerKind.Organization, + PersonaGUID = persona, +}; + +IReadOnlyList repositories = await provider.GetRepositoriesAsync(); +``` + +`OwnerKind` defaults to `GitHubOwnerKind.User`, which enumerates public repositories only: the route +this provider has always used. `Organization` and `AuthenticatedUser` report private repositories +the credential can see. A token that is valid but not authorised for an organisation's single +sign-on raises `GitHostingAuthenticationException` carrying the URL to authorise it at. + ### Initializing or Cloning a Repository `Init` probes the target path before running `git init`, so `GitInitResult.AlreadyExisted` can tell diff --git a/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md b/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md index a2d8549..8ea8cdb 100644 --- a/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md +++ b/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md @@ -323,17 +323,17 @@ travels under Octokit's `Token` scheme, which is what `FromToken` documents itse ### Transport and polling -Both calls go through Octokit's `OauthClient` — `InitiateDeviceFlow` and -`CreateAccessTokenForDeviceFlow` — for the same reason `GitHubProvider` uses Octokit: one client -library, one set of failure shapes to translate. `CreateAccessTokenForDeviceFlow` handles -`authorization_pending` and `slow_down` polling internally. - -**This rests on Octokit 14.0.0's actual surface, which is to be verified in the first implementation -step rather than assumed.** If either method is absent or does not poll, the fallback is two -`HttpClient` posts to `https://github.com/login/device/code` and -`https://github.com/login/oauth/access_token` with `Accept: application/json`, polling at `Interval` -and widening by five seconds on each `slow_down`. That fallback is a smaller amount of code than the -translation layer it would replace, so the risk here is low either way. +Built on a raw `HttpClient` rather than Octokit's `OauthClient`, even though Octokit exposes the same +pair of calls. Two problems turned up empirically against the real package: `InitiateDeviceFlow` +percent-encodes the scope's colon, so `read:org` travels as `read%3Aorg`, and +`CreateAccessTokenForDeviceFlow` throws `Octokit.ApiException` for an OAuth `error` body instead of +returning it, collapsing a refused authorisation and an expired code into the same exception shape a +transport failure gets. + +`GitHubDeviceFlow` instead posts JSON directly to `https://github.com/login/device/code` and +`https://github.com/login/oauth/access_token`, reading the `error` field itself so a refusal and an +expiry are told apart, and owns the poll loop: it waits `Interval` between attempts, widening by five +seconds on each `slow_down`. ### Failures From 9d058bbf5e245f476a6a7c084b2b3ef87e7b7ad1 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Tue, 22 Sep 2026 11:33:30 +1000 Subject: [PATCH 14/16] docs: cover worktree and device-flow types in the API reference Fixes the GitHubProvider reference entry, which still claimed GetRepositoriesAsync is public-only after OwnerKind made that conditional. Adds the missing worktree rows across the GitRepository, Verb Builders, and Result Models tables, a GitHubDeviceFlow reference section, and GitHubOAuthClientId to Semantic Types. Also corrects the non-compiling CredentialWithToken.Token assignment carried over into the design doc and GitHubDeviceFlow's own XML remarks. Co-Authored-By: Claude Sonnet 5 --- GitIntegration/Hosting/GitHubDeviceFlow.cs | 2 +- README.md | 35 +++++++++++++++++-- ...-09-21-worktrees-and-github-auth-design.md | 2 +- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/GitIntegration/Hosting/GitHubDeviceFlow.cs b/GitIntegration/Hosting/GitHubDeviceFlow.cs index 90de9f8..baa553b 100644 --- a/GitIntegration/Hosting/GitHubDeviceFlow.cs +++ b/GitIntegration/Hosting/GitHubDeviceFlow.cs @@ -67,7 +67,7 @@ public sealed record GitHubDeviceCode /// GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(cancellationToken); /// // show code.UserCode and open code.VerificationUri /// HostingCredential credential = await flow.WaitForTokenAsync(code, cancellationToken); -/// CredentialCache.Instance.AddOrReplace(persona, new CredentialWithToken { Token = credential.Token! }); +/// CredentialCache.Instance.AddOrReplace(persona, new CredentialWithToken { Token = credential.Token!.As<CredentialToken>() }); /// /// /// Built on a raw rather than on Octokit.IOauthClient, even though diff --git a/README.md b/README.md index dd0c608..719686a 100644 --- a/README.md +++ b/README.md @@ -857,6 +857,7 @@ Carries an optional `LocalPath` plus optional hosting metadata, and exposes one | `Tags()` | `IGitTagListBuilder` | Builds `git for-each-ref` over `refs/tags`. | | `Remotes()` | `IGitRemoteListBuilder` | Builds `git remote -v`. | | `Submodules()` | `IGitSubmoduleListBuilder` | Reads `git ls-files --stage -z` for the recorded gitlinks and `git submodule status` for what is checked out. | +| `Worktrees()` | `IGitWorktreeListBuilder` | Builds `git worktree list --porcelain`. | | `RevParse(GitRefName)` | `IGitRevParseBuilder` | Builds `git rev-parse --verify` for a revision. | | `RevList(GitRefName)` | `IGitRevListBuilder` | Builds `git rev-list --count`, counting commits without listing them. | | `Divergence(GitRefName, GitRefName)` | `IGitRevListDivergenceBuilder` | Builds `git rev-list --count --left-right`, reporting ahead and behind for any two revisions. | @@ -874,6 +875,9 @@ Carries an optional `LocalPath` plus optional hosting metadata, and exposes one | `Pull()` | `IGitPullBuilder` | Builds `git pull`. | | `Push()` | `IGitPushBuilder` | Builds `git push --porcelain`. | | `UpdateSubmodules()` | `IGitSubmoduleUpdateBuilder` | Builds `git submodule update`. | +| `AddWorktree(AbsoluteDirectoryPath)` | `IGitWorktreeAddBuilder` | Builds `git worktree add`. | +| `RemoveWorktree(AbsoluteDirectoryPath)` | `IGitWorktreeRemoveBuilder` | Builds `git worktree remove `. | +| `PruneWorktrees()` | `IGitWorktreePruneBuilder` | Builds `git worktree prune`. | | `IsClonedAsync(CancellationToken)` | `Task` | Decides whether `LocalPath` currently holds a git working tree. | | `OpenWebClient()` | `void` | Opens `WebURI` in the default browser, when it is an absolute `http`/`https` URI. | @@ -900,6 +904,7 @@ The shared contract every verb builder implements. A builder is single-use and n | `IGitTagListBuilder` | *(none)* | `IReadOnlyList` | | `IGitRemoteListBuilder` | *(none)* | `IReadOnlyList` | | `IGitSubmoduleListBuilder` | *(none)* | `IReadOnlyList` | +| `IGitWorktreeListBuilder` | *(none)* | `IReadOnlyList` | | `IGitRevParseBuilder` | *(none — revision supplied via `GitRepository.RevParse`)* | `GitCommitSha` | | `IGitRevListBuilder` | `FirstParentOnly()`, `ForPath(RelativeFilePath)` | `int` | | `IGitRevListDivergenceBuilder` | *(none — revisions supplied via `GitRepository.Divergence`)* | `GitDivergence` | @@ -919,6 +924,9 @@ The shared contract every verb builder implements. A builder is single-use and n | `IGitPullBuilder` | `FromRemote(GitRemoteName)`, `WithBranch(GitBranchName)`, `FastForwardOnly()`, `Rebase()`, `Merge()`, `Prune()`, `RecursingSubmodules(GitSubmoduleRecursion)`, `ReportingProgress(IProgress)` | `GitCompleted` | | `IGitPushBuilder` | `ToRemote(GitRemoteName)`, `WithBranch(GitBranchName)`, `SettingUpstream()`, `Force()`, `ForceWithLease()`, `DeletingRemoteBranch()`, `DryRun()`, `CheckingSubmodules(GitSubmodulePushCheck)`, `ReportingProgress(IProgress)` | `GitPushResult` | | `IGitSubmoduleUpdateBuilder` | `Initialise()`, `Recursive()`, `FromRemote()`, `Force()`, `WithDepth(int)`, `ReportingProgress(IProgress)` | `GitCompleted` | +| `IGitWorktreeAddBuilder` | `CheckingOut(GitBranchName)`, `CreatingBranch(GitBranchName)`, `CreatingOrResettingBranch(GitBranchName)`, `Detached()`, `From(GitRefName)`, `Force()`, `WithoutCheckout()` | `GitCompleted` | +| `IGitWorktreeRemoveBuilder` | `Force()` | `GitCompleted` | +| `IGitWorktreePruneBuilder` | *(none)* | `GitCompleted` | ### Result and Execution Models @@ -959,6 +967,7 @@ The shared contract every verb builder implements. A builder is single-use and n | `GitBranch` | `Name`, `Sha`, `Upstream`, `IsCurrent`, `IsRemote`. | | `GitRemote` | `Name`, `FetchUrl`, `PushUrl`. | | `GitDiffEntry` | `Kind`, `Path`, `OriginalPath`, `SimilarityPercent`. | +| `GitWorktree` | `Path`, `Head`, `Branch`, `IsMain`, `IsBare`, `IsDetached`, `IsLocked`, `LockReason`, `IsPrunable`, `PrunableReason` for one working tree. `IsMain` is positional: true only for the first record git emits, since git's porcelain has no attribute for it. | | `GitVersion` | `Major`, `Minor`, `Patch`, `Raw`, plus `AtLeast(major, minor)`. | | `GitFileState` | Enum: `Unmodified`, `Modified`, `Added`, `Deleted`, `Renamed`, `Copied`, `Untracked`, `Ignored`, `Unmerged`, `TypeChanged`. | | `GitChangeKind` | Enum: `Added`, `Copied`, `Deleted`, `Modified`, `Renamed`, `TypeChanged`, `Unmerged`, `Unknown`. | @@ -1012,9 +1021,28 @@ one connection pool rather than building and tearing down one per request. Neith ### `GitHubProvider` -`GitProvider` implementation backed by Octokit. `GetRepositoriesAsync` returns only `Owner`'s -**public** repositories — GitHub's `GET /users/{login}/repos` does not honour authentication to -reveal private ones, and supplying a token does not widen this. +`GitProvider` implementation backed by Octokit. `GetRepositoriesAsync` routes by `OwnerKind`, which +defaults to `GitHubOwnerKind.User`, so existing callers see no change. `User` calls +`GET /users/{login}/repos`, returning only `Owner`'s **public** repositories, and supplying a token +does not widen this. `Organization` calls `GET /orgs/{org}/repos` and `AuthenticatedUser` calls +`GET /user/repos`, and both report private repositories the credential can see. + +### `GitHubDeviceFlow` + +Obtains a GitHub credential through GitHub's OAuth device flow. The constructor takes a +`GitHubOAuthClientId` and the scopes to request. Split into two calls rather than one because the +pause between them is minutes long, and a caller displaying the user code needs to keep it on +screen while `WaitForTokenAsync` runs. Stores nothing: the caller decides where the resulting +credential lives. + +| Name | Return Type | Description | +|------|-------------|-------------| +| `RequestDeviceCodeAsync(CancellationToken)` | `Task` | Asks GitHub to begin a device flow. | +| `WaitForTokenAsync(GitHubDeviceCode, CancellationToken)` | `Task` | Polls until the user authorises, the code expires, or cancellation, returning the issued credential. | + +`GitHubDeviceCode` carries `UserCode`, `DeviceCode`, `VerificationUri`, `ExpiresIn`, and `Interval`. +`UserCode` is shown to the human at `VerificationUri`, and `DeviceCode` is sent in the poll instead. +The two are not interchangeable. ### `AzureDevOpsProvider` @@ -1069,6 +1097,7 @@ nor `AzureDevOpsProvider` has a constructor dependency a container could supply. | `GitBranchName` | Branch name | | `GitCommitMessage` | Commit message | | `GitCommitSha` | Commit object id (abbreviated or full, including SHA-256 repositories) | +| `GitHubOAuthClientId` | A GitHub OAuth App's client identifier, used to construct `GitHubDeviceFlow` | | `GitProviderName` | Hosting provider display name | | `GitProviderOwner` | Account or organization owning a repository | | `GitRefName` | A branch, tag, SHA, or revision expression | diff --git a/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md b/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md index 8ea8cdb..805e90f 100644 --- a/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md +++ b/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md @@ -310,7 +310,7 @@ connects them. ```csharp HostingCredential credential = await flow.WaitForTokenAsync(code, cancellationToken); -CredentialCache.Instance.AddOrReplace(persona, new CredentialWithToken { Token = credential.Token! }); +CredentialCache.Instance.AddOrReplace(persona, new CredentialWithToken { Token = credential.Token!.As() }); ``` Two lines at the call site, in exchange for a type that can be tested without a keyring and that does From a6d1cf88ff27e13769e5b18ac932deede281abf7 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Tue, 22 Sep 2026 11:50:05 +1000 Subject: [PATCH 15/16] fix: guard the verification URI and align error taxonomy with the spec RequestDeviceCodeAsync now validates verification_uri with Uri.TryCreate(..., UriKind.Absolute, ...) instead of letting a malformed value throw UriFormatException past this type's documented GitHostingRequestException-only failure surface. WaitForTokenAsync's error switch now distinguishes access_denied/expired_token (authentication failure) from incorrect_client_credentials/unsupported_grant_type/ device_flow_disabled and any unrecognised code (request failure, since none of those can be fixed by retrying sign-in), matching the design spec's Failures table, which is amended to document the device_flow_disabled and unrecognised-code additions. Also renames NowTicks to NowMilliseconds to name it by its unit rather than by the member it happens to default to, stops a future edit from reintroducing the token endpoint's response body into its failure message unnoticed, and fixes the org-repositories fixture's owner.type to Organization. Co-Authored-By: Claude Sonnet 5 --- .../Fixtures/github-org-repositories.json | 4 +- .../Hosting/GitHubDeviceFlowTests.cs | 114 +++++++++++++++++- GitIntegration/Hosting/GitHubDeviceFlow.cs | 88 +++++++++++--- ...-09-21-worktrees-and-github-auth-design.md | 13 +- 4 files changed, 198 insertions(+), 21 deletions(-) diff --git a/GitIntegration.Test/Fixtures/github-org-repositories.json b/GitIntegration.Test/Fixtures/github-org-repositories.json index 2aae3eb..09c40ee 100644 --- a/GitIntegration.Test/Fixtures/github-org-repositories.json +++ b/GitIntegration.Test/Fixtures/github-org-repositories.json @@ -22,7 +22,7 @@ "repos_url": "https://api.github.com/users/example-user/repos", "events_url": "https://api.github.com/users/example-user/events{/privacy}", "received_events_url": "https://api.github.com/users/example-user/received_events", - "type": "User", + "type": "Organization", "user_view_type": "public", "site_admin": false }, @@ -125,7 +125,7 @@ "repos_url": "https://api.github.com/users/example-user/repos", "events_url": "https://api.github.com/users/example-user/events{/privacy}", "received_events_url": "https://api.github.com/users/example-user/received_events", - "type": "User", + "type": "Organization", "user_view_type": "public", "site_admin": false }, diff --git a/GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs b/GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs index cb2376e..243d125 100644 --- a/GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs +++ b/GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs @@ -42,12 +42,12 @@ public sealed class GitHubDeviceFlowTests private static GitHubDeviceFlow CreateFlow( FakeHttpMessageHandler handler, Func? delay = null, - Func? nowTicks = null) => + Func? nowMilliseconds = null) => new("Iv1.0123456789abcdef".As(), ["repo", "read:org"]) { Handler = handler, Delay = delay ?? Task.Delay, - NowTicks = nowTicks ?? (() => Environment.TickCount64), + NowMilliseconds = nowMilliseconds ?? (() => Environment.TickCount64), }; [TestMethod] @@ -139,6 +139,114 @@ public async Task ReportsAnExpiredCodeAsAnAuthenticationFailure() .ConfigureAwait(false); } + [TestMethod] + [DataRow("incorrect_client_credentials")] + [DataRow("unsupported_grant_type")] + [DataRow("device_flow_disabled")] + public async Task ReportsAConfigurationFaultAsARequestFailureAsync(string errorCode) + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + _ = handler.Respond( + HttpStatusCode.OK, + $"{{\"error\":\"{errorCode}\",\"error_description\":\"unusable\"}}", + ("Content-Type", "application/json")); + + GitHubDeviceFlow flow = CreateFlow(handler); + GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + // Not GitHostingAuthenticationException: none of these three codes can be fixed by the user + // trying the sign-in again, so reporting them as an authentication failure would loop them + // through a flow that can never succeed. + GitHostingRequestException exception = + await Assert.ThrowsExactlyAsync( + async () => await flow.WaitForTokenAsync(code, TestContext.CancellationTokenSource.Token).ConfigureAwait(false)) + .ConfigureAwait(false); + + StringAssert.Contains(exception.Message, errorCode); + } + + [TestMethod] + public async Task ReportsAnUnrecognisedErrorCodeAsARequestFailureAsync() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + _ = handler.Respond( + HttpStatusCode.OK, + "{\"error\":\"some_future_github_error\",\"error_description\":\"not yet catalogued\"}", + ("Content-Type", "application/json")); + + GitHubDeviceFlow flow = CreateFlow(handler); + GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + // The deliberate default for a code this library was not taught: a request fault, not an + // authentication failure, so an unrecognised error never invites retrying a sign-in. + GitHostingRequestException exception = + await Assert.ThrowsExactlyAsync( + async () => await flow.WaitForTokenAsync(code, TestContext.CancellationTokenSource.Token).ConfigureAwait(false)) + .ConfigureAwait(false); + + StringAssert.Contains(exception.Message, "some_future_github_error"); + } + + [TestMethod] + public async Task DoesNotEchoTheTokenEndpointResponseBodyOnFailureAsync() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); + _ = handler.Respond( + HttpStatusCode.InternalServerError, + "{\"secret\":\"do-not-leak-me\"}", + ("Content-Type", "application/json")); + + GitHubDeviceFlow flow = CreateFlow(handler); + GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + // A deliberate omission, not an oversight: this asserts a future edit cannot add the body + // back "for symmetry" with RequestDeviceCodeAsync's own failure message without CI catching it. + GitHostingRequestException exception = + await Assert.ThrowsExactlyAsync( + async () => await flow.WaitForTokenAsync(code, TestContext.CancellationTokenSource.Token).ConfigureAwait(false)) + .ConfigureAwait(false); + + Assert.IsFalse(exception.Message.Contains("do-not-leak-me", StringComparison.Ordinal)); + } + + [TestMethod] + public async Task ReportsAnEmptyVerificationUriAsARequestFailureAsync() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond( + HttpStatusCode.OK, + "{\"device_code\":\"dev-abc\",\"user_code\":\"WXYZ-1234\"," + + "\"verification_uri\":\"\",\"expires_in\":900,\"interval\":5}", + ("Content-Type", "application/json")); + + _ = await Assert.ThrowsExactlyAsync( + async () => await CreateFlow(handler) + .RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false)) + .ConfigureAwait(false); + } + + [TestMethod] + public async Task ReportsANonAbsoluteVerificationUriAsARequestFailureAsync() + { + using FakeHttpMessageHandler handler = new(); + _ = handler.Respond( + HttpStatusCode.OK, + // Not a leading-slash path: Uri.TryCreate(..., UriKind.Absolute, ...) accepts one of + // those as an absolute file:// URI, which would defeat this test. A scheme-less + // authority-and-path string is what genuinely fails UriKind.Absolute parsing. + "{\"device_code\":\"dev-abc\",\"user_code\":\"WXYZ-1234\"," + + "\"verification_uri\":\"github.com/login/device\",\"expires_in\":900,\"interval\":5}", + ("Content-Type", "application/json")); + + _ = await Assert.ThrowsExactlyAsync( + async () => await CreateFlow(handler) + .RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false)) + .ConfigureAwait(false); + } + [TestMethod] public async Task ReportsAnUnusableClientIdentifierAsARequestFailure() { @@ -279,7 +387,7 @@ public async Task ThrowsOnceTheDeviceCodeExpiresAsync() now += (long)TimeSpan.FromSeconds(900).TotalMilliseconds + 1_000; return Task.CompletedTask; }, - nowTicks: () => now); + nowMilliseconds: () => now); GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); diff --git a/GitIntegration/Hosting/GitHubDeviceFlow.cs b/GitIntegration/Hosting/GitHubDeviceFlow.cs index baa553b..ffb6f47 100644 --- a/GitIntegration/Hosting/GitHubDeviceFlow.cs +++ b/GitIntegration/Hosting/GitHubDeviceFlow.cs @@ -171,16 +171,20 @@ public sealed class GitHubDeviceFlow(GitHubOAuthClientId clientId, IReadOnlyList internal Func Delay { get; init; } = Task.Delay; /// - /// Gets or initializes the monotonic clock reads to enforce - /// . + /// Gets or initializes the monotonic clock, in milliseconds, reads + /// to enforce . /// /// /// Defaults to , a monotonic source deliberately chosen over /// or : neither is guaranteed /// monotonic, and a clock adjustment during a wait that can last minutes must not extend or - /// collapse the window this flow enforces. Internal for the same reason is. + /// collapse the window this flow enforces. Named for its unit rather than for the .NET member it + /// defaults to: a future replacement built on, say, 's Ticks + /// (100-nanosecond units) would silently widen the deadline computed from it roughly ten-thousand + /// fold, and a name that already states "milliseconds" is what stops that substitution compiling + /// clean while quietly breaking the deadline. Internal for the same reason is. /// - internal Func NowTicks { get; init; } = () => Environment.TickCount64; + internal Func NowMilliseconds { get; init; } = () => Environment.TickCount64; /// /// Asks GitHub to begin a device flow. @@ -214,11 +218,23 @@ public async Task RequestDeviceCodeAsync(CancellationToken can GitHubDeviceCodeResponseBody parsed = DeserializeOrThrow( body, GitHubDeviceFlowJsonContext.Default.GitHubDeviceCodeResponseBody, "device code"); + // TryCreate, not `new Uri(...)`: `verification_uri` being present (required by the response + // shape) says nothing about its content. An empty string, a relative path, or a truncated + // body all reach here as a syntactically valid JSON string, and `new Uri` would throw + // UriFormatException straight past this method's documented failure surface, which is + // GitHostingRequestException alone, exactly the leak class DeserializeOrThrow already guards + // against for JsonException. + if (!Uri.TryCreate(parsed.VerificationUri, UriKind.Absolute, out Uri? verificationUri)) + { + throw new GitHostingRequestException( + $"GitHub reported success but returned a verification URI that is not an absolute URI: \"{parsed.VerificationUri}\"."); + } + return new GitHubDeviceCode { UserCode = parsed.UserCode, DeviceCode = parsed.DeviceCode, - VerificationUri = new Uri(parsed.VerificationUri), + VerificationUri = verificationUri, // GitHub reports both as integer seconds, the conversion happens once, here. ExpiresIn = TimeSpan.FromSeconds(parsed.ExpiresIn), Interval = TimeSpan.FromSeconds(parsed.Interval), @@ -229,6 +245,7 @@ public async Task RequestDeviceCodeAsync(CancellationToken can /// Waits for the user to authorise the flow, then returns the credential GitHub issues. /// /// + /// /// Polls until the user authorises, the code expires, or is /// cancelled, so this may block for as long as . A /// authorization_pending answer waits and tries @@ -237,13 +254,33 @@ public async Task RequestDeviceCodeAsync(CancellationToken can /// authorization_pending past would otherwise poll /// forever, since nothing about that answer's shape distinguishes a slow user from a server that /// never intends to resolve. + /// + /// + /// Every other OAuth error code is split by what a caller can do about it, not merely by whether + /// GitHub happened to send one. access_denied and expired_token are a fact about the + /// person authorising and become , since offering + /// the sign-in again is the caller's correct remedy for both. incorrect_client_credentials, + /// unsupported_grant_type, and device_flow_disabled are a fact about the caller's own + /// configuration and become instead: none of the three can + /// be fixed by the user trying again, so reporting them as an authentication failure would loop a + /// person through a sign-in that can never succeed. A code this method does not recognise is + /// deliberately treated as a as well, on the same + /// reasoning: an unrecognised code is either a GitHub error this library has not been taught yet or + /// something upstream of GitHub answering instead, and in both cases a caller is better served + /// being told something is wrong with the request than being invited to retry a sign-in for a + /// reason nobody has verified sign-in can fix. + /// /// /// What returned. /// Abandons the wait. /// A host-native token credential. /// is . /// The user refused, or the code expired. - /// GitHub refused the request or could not be reached. + /// + /// GitHub refused the request, could not be reached, reported a configuration fault + /// (incorrect_client_credentials, unsupported_grant_type, device_flow_disabled), + /// or reported an error code this method does not recognise. + /// public async Task WaitForTokenAsync(GitHubDeviceCode code, CancellationToken cancellationToken = default) { Ensure.NotNull(code); @@ -262,17 +299,17 @@ public async Task WaitForTokenAsync(GitHubDeviceCode code, Ca TimeSpan interval = code.Interval; - // A monotonic elapsed-time budget rather than a fixed end-of-wall-clock instant: NowTicks - // wraps Environment.TickCount64 by default, which is what makes this immune to the machine's - // clock being changed mid-wait, forward or back. - long startTicks = NowTicks(); + // A monotonic elapsed-time budget rather than a fixed end-of-wall-clock instant: + // NowMilliseconds wraps Environment.TickCount64 by default, which is what makes this immune + // to the machine's clock being changed mid-wait, forward or back. + long startMilliseconds = NowMilliseconds(); long expiresInMilliseconds = (long)code.ExpiresIn.TotalMilliseconds; while (true) { cancellationToken.ThrowIfCancellationRequested(); - if (NowTicks() - startTicks >= expiresInMilliseconds) + if (NowMilliseconds() - startMilliseconds >= expiresInMilliseconds) { throw new GitHostingAuthenticationException("GitHub did not issue a token before the device code expired."); } @@ -304,12 +341,35 @@ public async Task WaitForTokenAsync(GitHubDeviceCode code, Ca await WaitAsync(interval, cancellationToken).ConfigureAwait(false); continue; - case string error: + case "access_denied" or "expired_token": // GitHub reports a refusal and an expiry as a 200 carrying an error field // rather than as a failure status, which is why this is read from the body - // rather than caught as a failed HttpResponseMessage above. + // rather than caught as a failed HttpResponseMessage above. These two, and + // only these two, are a fact about the person authorising: they said no, or + // ran out of time, and the caller's own remedy is to offer the sign-in again. throw new GitHostingAuthenticationException( - $"GitHub did not issue a token: {error}. {parsed.ErrorDescription}".TrimEnd()); + $"GitHub did not issue a token: {parsed.Error}. {parsed.ErrorDescription}".TrimEnd()); + + case "incorrect_client_credentials" or "unsupported_grant_type" or "device_flow_disabled": + // A fact about this library's caller, not about the person authorising: the + // client identifier is wrong, revoked, or device flow was never enabled for it. + // Retrying the sign-in cannot fix any of the three, so these are a request + // fault rather than an authentication failure, matching the design's Failures + // table. Offering the user another sign-in attempt here would loop them through + // a flow that can never succeed. + throw new GitHostingRequestException( + $"GitHub refused the device flow token request: {parsed.Error}. {parsed.ErrorDescription}".TrimEnd()); + + case string unrecognisedError: + // A code this library does not recognise is treated as a request fault rather + // than an authentication failure, deliberately: the two known authentication + // codes above are enumerated explicitly, so anything else reaching here is + // either a new GitHub error this library has not been taught yet, or a + // misbehaving proxy or interstitial, and in both cases looping the user through + // another sign-in attempt is more likely to be wrong than treating it as + // something the caller or its configuration needs to look at. + throw new GitHostingRequestException( + $"GitHub reported an unrecognised device flow error: {unrecognisedError}. {parsed.ErrorDescription}".TrimEnd()); case null when string.IsNullOrEmpty(parsed.AccessToken): throw new GitHostingRequestException("GitHub reported neither a token nor an error."); diff --git a/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md b/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md index 805e90f..d41300a 100644 --- a/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md +++ b/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md @@ -343,13 +343,22 @@ No new exception family. Everything lands in the hosting hierarchy that already |---|---| | `access_denied` (user refused) | `GitHostingAuthenticationException` | | `expired_token` (code timed out) | `GitHostingAuthenticationException` | -| `incorrect_client_credentials`, `unsupported_grant_type` | `GitHostingRequestException` | -| transport failure, unparsable body | `GitHostingRequestException` | +| `incorrect_client_credentials`, `unsupported_grant_type`, `device_flow_disabled` | `GitHostingRequestException` | +| an error code this library does not recognise | `GitHostingRequestException` | +| transport failure, unparsable body, a non-absolute `verification_uri` | `GitHostingRequestException` | Denial and expiry share an exception and are distinguished by message. They are the same fact to a caller — no credential was obtained, offer to start again — and splitting them would add a type nobody switches on. +`device_flow_disabled` joins the request-fault row alongside the two the design first listed, for the +same reason as both: it is a fact about the OAuth App's configuration, not about the person +authorising, and no amount of retrying the sign-in fixes it. An error code this library has not been +taught falls into the same bucket by deliberate choice, not by falling through unhandled: only +`access_denied` and `expired_token` are enumerated as authentication failures, so anything else is +either a GitHub error introduced after this was written or a misbehaving intermediary, and looping a +person through another sign-in attempt is the worse guess of the two available. + ### The client identifier `GitHubOAuthClientId`, a new semantic string in `SemanticTypes/GitProviderTypes.cs`, supplied by the From 24d81adade5b038d1b061d33f515c258dc77064d Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Tue, 22 Sep 2026 14:23:52 +1000 Subject: [PATCH 16/16] refactor: address the code quality findings on the pull request Three findings from the code quality review. The device flow's `case null when string.IsNullOrEmpty(parsed.AccessToken)` tested a constant: the preceding `case string` arm consumes every non-null error, so the error is provably null by the time control reaches it. The null test is dropped and the token check moves into the default arm, which also lets the null-forgiveness operator go, since the compiler narrows AccessToken through the plain check where it could not through the pattern. The two loops in GitWorktreeParser.Parse and TryGetSingleSignOnUrl each remapped their iteration variable on the first line of the body. Both now map at the enumeration source instead. No behaviour changes. Co-Authored-By: Claude Opus 5 (1M context) --- GitIntegration/GitHubProvider.cs | 4 +--- GitIntegration/Hosting/GitHubDeviceFlow.cs | 13 +++++++++---- GitIntegration/Parsing/GitWorktreeParser.cs | 7 ++++--- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/GitIntegration/GitHubProvider.cs b/GitIntegration/GitHubProvider.cs index c4e8869..afb59e3 100644 --- a/GitIntegration/GitHubProvider.cs +++ b/GitIntegration/GitHubProvider.cs @@ -629,10 +629,8 @@ private GitHostingException Translate(ApiException exception) foreach (KeyValuePair header in headers.Where( candidate => candidate.Key.Equals(headerName, StringComparison.OrdinalIgnoreCase))) { - foreach (string parameter in header.Value.Split(';')) + foreach (string trimmed in header.Value.Split(';').Select(static parameter => parameter.Trim())) { - string trimmed = parameter.Trim(); - if (trimmed.StartsWith(urlParameter, StringComparison.OrdinalIgnoreCase)) { string url = trimmed[urlParameter.Length..]; diff --git a/GitIntegration/Hosting/GitHubDeviceFlow.cs b/GitIntegration/Hosting/GitHubDeviceFlow.cs index ffb6f47..50f4b7f 100644 --- a/GitIntegration/Hosting/GitHubDeviceFlow.cs +++ b/GitIntegration/Hosting/GitHubDeviceFlow.cs @@ -371,14 +371,19 @@ public async Task WaitForTokenAsync(GitHubDeviceCode code, Ca throw new GitHostingRequestException( $"GitHub reported an unrecognised device flow error: {unrecognisedError}. {parsed.ErrorDescription}".TrimEnd()); - case null when string.IsNullOrEmpty(parsed.AccessToken): - throw new GitHostingRequestException("GitHub reported neither a token nor an error."); - default: + // Every non-null error is handled above, so the error is null by the time control + // reaches here and testing it again would be testing a constant. The only question + // left is whether a token actually arrived alongside that absent error. + if (string.IsNullOrEmpty(parsed.AccessToken)) + { + throw new GitHostingRequestException("GitHub reported neither a token nor an error."); + } + // FromToken, not FromBearerToken: a GitHub OAuth token travels under Octokit's // Token scheme, which is what FromToken means. FromBearerToken is for an Entra // ID access token against Azure DevOps. - return HostingCredential.FromToken(parsed.AccessToken!); + return HostingCredential.FromToken(parsed.AccessToken); } } } diff --git a/GitIntegration/Parsing/GitWorktreeParser.cs b/GitIntegration/Parsing/GitWorktreeParser.cs index 396b44c..9649c4a 100644 --- a/GitIntegration/Parsing/GitWorktreeParser.cs +++ b/GitIntegration/Parsing/GitWorktreeParser.cs @@ -4,6 +4,7 @@ namespace ktsu.GitIntegration; using System; using System.Collections.Generic; +using System.Linq; /// /// Reads git worktree list --porcelain. @@ -31,10 +32,10 @@ internal static IReadOnlyList Parse(string output) List worktrees = []; List record = []; - foreach (string line in output.Split('\n')) + // Trimmed at the enumeration source rather than inside the loop: the carriage return is a + // line-ending artefact, not something any record's content means, so nothing below sees it. + foreach (string entry in output.Split('\n').Select(static line => line.TrimEnd('\r'))) { - string entry = line.TrimEnd('\r'); - if (entry.Length != 0) { record.Add(entry);