fix(github): classify access, network and server failures; cancel a listing promptly #154

2026-11-10T21:16:08Z

james.maes commented

2026-11-10 21:16:08 +00:00

What

One mapping for GitHub failures (GitHubFailures), used by both listers. New: 403/451 → Forbidden with GitHub's own reason; HTTP 5xx, a network failure, and a timed-out request → the new GitHubAccessKind.Unavailable. A failure with no known meaning stays what it is.
CLI: Unavailable → exit code 4 (retry later).
A listing can be canceled during a request: each gateway call is awaited with the caller's token, so Ctrl+C no longer waits for the HTTP timeout of a hung request.
Nothing is reported after a repository's final state: OrgSyncEngine reports through RunProgress, which drops late progress from a git operation that was abandoned on cancel.

Why

Closes #153

Before this, a SAML-protected organization, a GitHub outage, a DNS failure, or a timeout ended a CLI run with exit code 70 and "unexpected error ... please report it", and the desktop app showed nothing for a timed-out organization lookup.

Behavior change for scripts

For those cases the exit code changes from 70 to 3 (access refused) or 4 (retry later). docs/CLI.md and README.md are updated; this needs a line in the 1.1.1 release notes.

How it was tested

GitHubListerTests: 403 on the first request and on a later one, 5xx, network failure, timeout, an unknown API error left alone, cancellation during a hung request (repositories, account-kind lookup, organizations), and the mapping itself arm by arm.
RunProgressTests and two OrgSyncEngineTests: late progress after Canceled is dropped; a second run reports the same repository again; a threaded race never puts progress after the final state.
CLI: Unavailable → 4 in sync, repos, orgs.
Local gates: locked zero-warning x64 build, 822 engine and 164 CLI tests, 100% line coverage on the three gated assemblies (branch: Engine 94.66%, ViewModels 94.55%, CLI 98.20%), dotnet format --verify-no-changes.

Notes for the reviewer

GitHubAccessKind.Unavailable is a new member of a public enum in the engine package.
The gateway (Octokit) still cannot cancel a request; a canceled listing abandons it and it ends on its own at the HTTP timeout.
A "cancel" control for a repository load in the desktop app is not part of this change (it belongs with the workspace state work).

Checklist

Commits are signed and signed off (git commit -S -s)
Zero build warnings; format and lint are clean
Tests added or updated; the 100% coverage gate still passes
Docs updated for user-visible changes; threat model updated if the attack surface changed (no change to the attack surface)
No secrets, credentials, or personal data
New dependencies are justified below, or none were added (none)
Substantial AI assistance, if any, is noted here: written with Claude Code, reviewed by an independent read-only agent before merge

🤖 Generated with Claude Code

## What - **One mapping for GitHub failures** (`GitHubFailures`), used by both listers. New: 403/451 → `Forbidden` with GitHub's own reason; HTTP 5xx, a network failure, and a timed-out request → the new `GitHubAccessKind.Unavailable`. A failure with no known meaning stays what it is. - **CLI**: `Unavailable` → exit code 4 (retry later). - **A listing can be canceled during a request**: each gateway call is awaited with the caller's token, so Ctrl+C no longer waits for the HTTP timeout of a hung request. - **Nothing is reported after a repository's final state**: `OrgSyncEngine` reports through `RunProgress`, which drops late progress from a git operation that was abandoned on cancel. ## Why Closes #153 Before this, a SAML-protected organization, a GitHub outage, a DNS failure, or a timeout ended a CLI run with exit code 70 and "unexpected error ... please report it", and the desktop app showed nothing for a timed-out organization lookup. ## Behavior change for scripts For those cases the exit code changes from **70** to **3** (access refused) or **4** (retry later). `docs/CLI.md` and `README.md` are updated; this needs a line in the 1.1.1 release notes. ## How it was tested - `GitHubListerTests`: 403 on the first request and on a later one, 5xx, network failure, timeout, an unknown API error left alone, cancellation during a hung request (repositories, account-kind lookup, organizations), and the mapping itself arm by arm. - `RunProgressTests` and two `OrgSyncEngineTests`: late progress after Canceled is dropped; a second run reports the same repository again; a threaded race never puts progress after the final state. - CLI: `Unavailable` → 4 in `sync`, `repos`, `orgs`. - Local gates: locked zero-warning x64 build, 822 engine and 164 CLI tests, 100% line coverage on the three gated assemblies (branch: Engine 94.66%, ViewModels 94.55%, CLI 98.20%), `dotnet format --verify-no-changes`. ## Notes for the reviewer - `GitHubAccessKind.Unavailable` is a new member of a public enum in the engine package. - The gateway (Octokit) still cannot cancel a request; a canceled listing abandons it and it ends on its own at the HTTP timeout. - A "cancel" control for a repository load in the desktop app is not part of this change (it belongs with the workspace state work). ## Checklist - [x] Commits are signed and signed off (`git commit -S -s`) - [x] Zero build warnings; format and lint are clean - [x] Tests added or updated; the 100% coverage gate still passes - [x] Docs updated for user-visible changes; threat model updated if the attack surface changed (no change to the attack surface) - [x] No secrets, credentials, or personal data - [x] New dependencies are justified below, or none were added (none) - [x] Substantial AI assistance, if any, is noted here: written with Claude Code, reviewed by an independent read-only agent before merge 🤖 Generated with [Claude Code](https://claude.com/claude-code)

james.maes added 2 commits 2026-11-10 21:38:55 +00:00

fix(github): classify access, network and server failures; cancel a listing promptly a19f741afc

The listers translated a rejected token, rate limits and "not found".
Everything else escaped as a raw exception: a refusal of access (403, an
organization that enforces SAML single sign-on, for example), an error on
GitHub's side (5xx), a network failure, and a request that timed out. The
CLI reported those as exit code 70, a bug in gclo, and the desktop app
took a timed-out organization lookup for a superseded one and showed
nothing.

- GitHubFailures is the one mapping both listers use. 403 (and 451) is
  Forbidden, with GitHub's own reason in the message; 5xx, a network
  failure and a timeout are the new GitHubAccessKind.Unavailable. A
  failure it does not know stays what it is.
- The CLI maps Unavailable to exit code 4 (retry later). For these cases
  the exit code changes from 70 to 3 or 4.
- Each gateway request is awaited with the caller's token, so Ctrl+C
  during a request that hangs no longer waits for the HTTP timeout. A
  cancellation the caller did not ask for is a timeout.
- OrgSyncEngine reports through RunProgress, which drops whatever arrives
  for a repository after its final state in the run. A git operation
  abandoned on cancel keeps reporting transfer progress; a row that said
  Canceled could go back to "Cloning 40%", and the CLI could print a
  progress line after the summary.

README.md listed a rejected token under exit code 2; its table now has
3, 4 and 70 as docs/CLI.md does.

Closes #153

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

fix(github): address the independent review of the failure mapping d6750699b6

The caller's cancellation is decided once; 429 is a rate limit and a
proxy's 407 is unavailable; messages take GitHub's reason only from a JSON
answer and describe a network failure by its kind, never by the platform's
text; an abandoned request's late failure is observed; a final state counts
only once the caller's progress took it; help texts and docs match.

Refs #153

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

james.maes force-pushed fix/153-failure-classification from e2289fdb63 to d6750699b6

2026-11-10 21:38:55 +00:00

Compare

james.maes commented

2026-11-10 21:38:55 +00:00

Independent review (read-only agent): merge after fixes

The reviewer ran the branch and fed the real Octokit Connection / HttpClientAdapter canned responses and loopback sockets (nothing left the machine). No regression in the listers' existing behavior or in the engine. Findings and what was done, in the second commit (the branch was also rebased onto main):

#	Finding	Resolution
F1	The decision "was this cancellation the caller's" was made twice; a timeout that coincided with a cancel threw an internal `ArgumentException` (reproduced: 13,318 of 200,000 racing rounds).	Decided once, in the exception filter; `Translate` no longer looks at the token. Test added.
F2	HTTP 429 (GitHub answers rate limits with 403 or 429) was unmapped: exit 70.	429 → rate limited (exit 4). A proxy's 407 → unavailable. Tests added.
F3	The four `--help` texts and two doc examples still described exit code 4 as rate limiting only.	Updated, with a migration line in `docs/CLI.md`.
F4	Messages embedded raw text: a TLS failure lost its reason, a web filter's 403 page was embedded whole (3 KB), and the platform's proxy error can carry the proxy's user name and password.	A network failure is described by its kind (`HttpRequestError`), never by the platform's text. GitHub's reason is used only when it is the `message` of a JSON answer, on one line, at most 300 characters. Tests for each, including one that the proxy password does not appear.
F5	Punctuation after `?` and `!`.	Fixed in the same helper.
F6	An abandoned request that failed later raised `UnobservedTaskException`.	`GitHubFailures.WaitFor` looks at the abandoned task's failure.
F7	`RunProgress` marked a repository finished before the caller's progress accepted the report.	Marked only after the report was taken. Test added.
F8	The 403 test did not assert GitHub's reason; no tests for F1 and F2.	Added.

Checked and found sound by the reviewer: the switch order against the real Octokit 14 hierarchy, real timeouts and DNS/connection failures through HttpClientAdapter, that a caller's cancellation cannot become "unavailable", the organization lister's degraded mode, the repository lister's not-found paths, every exit-code path, lock ordering and contention in RunProgress, and the public surface (one enum member added).

Gates after the fixes: 881 engine and 164 CLI tests, 100% line coverage on the three gated assemblies, zero warnings, format clean.

Not part of this change, noted for later: a 200 answer with a non-JSON body makes the listing return an empty list (and /user throw); a cancel control for a repository load in the desktop app.

## Independent review (read-only agent): merge after fixes The reviewer ran the branch and fed the real Octokit `Connection` / `HttpClientAdapter` canned responses and loopback sockets (nothing left the machine). No regression in the listers' existing behavior or in the engine. Findings and what was done, in the second commit (the branch was also rebased onto `main`): | # | Finding | Resolution | | --- | --- | --- | | F1 | The decision "was this cancellation the caller's" was made twice; a timeout that coincided with a cancel threw an internal `ArgumentException` (reproduced: 13,318 of 200,000 racing rounds). | Decided once, in the exception filter; `Translate` no longer looks at the token. Test added. | | F2 | HTTP 429 (GitHub answers rate limits with 403 or 429) was unmapped: exit 70. | 429 → rate limited (exit 4). A proxy's 407 → unavailable. Tests added. | | F3 | The four `--help` texts and two doc examples still described exit code 4 as rate limiting only. | Updated, with a migration line in `docs/CLI.md`. | | F4 | Messages embedded raw text: a TLS failure lost its reason, a web filter's 403 page was embedded whole (3 KB), and the platform's proxy error can carry the proxy's user name and password. | A network failure is described by its kind (`HttpRequestError`), never by the platform's text. GitHub's reason is used only when it is the `message` of a JSON answer, on one line, at most 300 characters. Tests for each, including one that the proxy password does not appear. | | F5 | Punctuation after `?` and `!`. | Fixed in the same helper. | | F6 | An abandoned request that failed later raised `UnobservedTaskException`. | `GitHubFailures.WaitFor` looks at the abandoned task's failure. | | F7 | `RunProgress` marked a repository finished before the caller's progress accepted the report. | Marked only after the report was taken. Test added. | | F8 | The 403 test did not assert GitHub's reason; no tests for F1 and F2. | Added. | Checked and found sound by the reviewer: the switch order against the real Octokit 14 hierarchy, real timeouts and DNS/connection failures through `HttpClientAdapter`, that a caller's cancellation cannot become "unavailable", the organization lister's degraded mode, the repository lister's not-found paths, every exit-code path, lock ordering and contention in `RunProgress`, and the public surface (one enum member added). Gates after the fixes: 881 engine and 164 CLI tests, 100% line coverage on the three gated assemblies, zero warnings, format clean. Not part of this change, noted for later: a 200 answer with a non-JSON body makes the listing return an empty list (and `/user` throw); a cancel control for a repository load in the desktop app.

james.maes merged commit cc3aeecdf4 into main

2026-11-10 21:39:19 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/gclo#154