Known-issues sweep: verified review findings (#30, #31, #32), update progress (#39), full action logging (#40) #58
Merged
KofTwentyTwo
merged 10 commits from 2026-10-09 12:46:51 +00:00
dev into main
10
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 James MaesandClaude Fable 5.1
|
7a596c2407 |
Release: gate the optional publish steps on step outputs, not the secrets context
GitHub does not expose the secrets context in a step's if:, so the file failed validation. A dedicated step (the only one besides the two publishers that receives the secrets) turns their presence into outputs the if: conditions use. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> |
||
|
James MaesandClaude Fable 5.1
|
fa6317d4fb |
CI: remove duplicated cache keys from the setup-dotnet steps (workflow files failed to validate)
The lock-file batch inserted the cache: / cache-dependency-path: pair twice into every setup-dotnet 'with:' block (the batch script ran twice); GitHub rejects duplicate mapping keys and every workflow on dev reported a startup failure. One pair each now. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> |
||
|
James MaesandClaude Fable 5.1
|
d8e2e49eef |
XAML + test quality: adaptive floor, backdrop fallback, theme-correct brushes, shared connection form, lifecycle tests (#31)
XAML / WinUI
- MinWindowWidth 700 -> 900: the pane (240) + paddings (64) + fixed columns
(400) left the Name column at zero width at the old floor.
- Backdrop chosen in code: Mica where supported, acrylic where only that is,
else the themed page background — the declared MicaBackdrop was silently
ignored on the Windows 10 floor the app targets, leaving the layered card
brushes on an unthemed surface.
- Status brushes and badges resolve against the theme actually shown
(StatusFormat.CurrentTheme, read from the matching theme dictionary); on a
theme change (Settings, or an OS flip in System mode) every row re-evaluates
via RefreshPresentation and the title bar follows (TitleBar.PreferredTheme).
Previously an in-session switch to Dark kept light-theme status colors on
dark cards until restart.
- The active strip scrolls (three whole rows visible) instead of clipping the
third row to a sliver at default parallelism.
- The connect form (token, org, target, subfolder, preview) is one
ConnectionFormControl hosted by the connect card and the chip's Edit flyout
with an AutomationIdPrefix each ("Connect…", "Edit…"), ending the two
diverging copies; it owns the no-token-redisplay placeholder logic.
- Table column widths are GridLength resources referenced by both the header
grid and the row template; the wizard's step host has a MinHeight so the
dialog keeps one footprint across steps; About renders attributions from a
data list through one template.
Test quality
- WorkspaceViewModel.Dispose is idempotent (a second call threw
ObjectDisposedException — the shell disposes on eviction and again on
close) and has direct tests: dispose mid-lookup cancels the lookup's token
and a late result cannot land; double dispose is a no-op.
- WorkspaceViewModelTests is IDisposable: every view model it creates is
disposed and its temp folder removed on every path (27 inline success-only
deletes gone).
- The one wall-clock test (debounce) uses a 1.5 s window instead of 120 ms.
- LibGit2GitClient gains an internal post-fetch hook so the offline suite
exercises real cancellation: a clone canceled after its fetch throws
OperationCanceledException and removes the directory; a pull canceled after
its fetch merges nothing and a later pull fast-forwards.
- UI smoke tests anchor their negative assertions to positive oracles: the
wizard test waits for the step-1 validation message, and the dialog-
collision test waits for the shell's "Account wizard not opened" log line
(DialogGuard.IsDialogOpen is checked before logging) instead of sleeping.
392 Engine tests, coverage 100%, format clean, FlaUI e2e 8/8.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
||
|
James MaesandClaude Fable 5.1
|
437d497e63 |
Security + release engineering: SHA-pinned actions, scoped secrets, pinned tooling, provenance, no token redisplay, startup update check, lock files, UI diagnostics (#32, #31)
Workflows - Every `uses:` is pinned to a full commit SHA with the version as a comment (checkout v7.0.1, setup-dotnet v6.0.0, upload-artifact v7.0.2, codeql-action v4, dependency-review-action v5.0.0, attest-build-provenance v4); Dependabot keeps the pins current. - release.yml: NUGET_API_KEY and WINGET_TOKEN are no longer job-level env — only the push and winget steps receive them. vpk is installed at a pinned version (1.2.161, matching the Velopack package) and wingetcreate is downloaded from a pinned release asset and verified against its committed SHA-256 before it runs. A build-provenance attestation is published for the CLI zip and every Velopack asset (verify with `gh attestation verify <file> -R KofTwentyTwo/gclo`). gclo.Engine is packed with ContinuousIntegrationBuild and a .snupkg. - packages.lock.json committed for all seven projects (RestorePackagesWithLockFile); CI builds and the format restore run in locked mode, every setup-dotnet step caches the NuGet packages keyed on the lock files, and the NuGet-updates workflow refreshes them after a bump. - UI e2e job: TRX logger, and on failure an artifact with the TRX, a screenshot of the screen at session end, and the app's activity log (AppSession captures both when GCLO_UITEST_ARTIFACTS is set). App - The stored token is never redisplayed: the account wizard's token box opens empty with "Leave empty to keep the stored token" and fetches the vault token only for step 2's validation call when the box is empty (never into the Token property); the connect card and chip flyout no longer mirror an account's or the default token into a PasswordBox. - A background update check on every launch of an installed build offers a newer release in the update bar with an "Update and restart…" action; silent when up to date or when the check fails. - gclo.Engine.csproj: PackageProjectUrl, PackageTags, SourceLink to the public repo, embedded untracked sources, symbol package. Docs - SECURITY.md: no longer "pre-1.0"; the token section states what is persisted (Credential Manager only), the same-user trust boundary, and that the UI never redisplays a stored token; a new "How releases are trusted" section covers the tag/gates/attestation chain and the open signing item (#57). - CONTRIBUTING.md: honest branch model (checks advisory on dev, enforced at main) and the lock-file refresh step. README: repos/dry-run/accounts examples. #57 opened as needs-owner for Authenticode signing. Tests: wizard token handling (empty box validates with the stored token without exposing it; no stored token blocks step 2 until one is typed). 387 Engine tests, coverage 100%, format clean, locked-mode build clean, FlaUI e2e 8/8 with the artifact capture exercised locally. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> |
||
|
James MaesandClaude Fable 5.1
|
54afbb0d7a |
CLI: selection filters, dry-run, NDJSON, full JSON contract, account lifecycle, exit-code taxonomy (#32)
The review's CLI findings, each re-checked against the code and all confirmed:
- --json carries what it used to lose: "sanitized":[{repo,renamed,skipped,
skippedPaths}] for repositories checked out with renames/skips (an incomplete
tree a mirror pipeline must know about) and failures[].invalidPaths with the
FULL offending-path list; "type":"summary" marks the document. The
'Sanitized' stderr note and path-detail lines are suppressed in the JSON modes.
- --json-lines: one {"type":"progress",repo,status,error} object per status
transition on stdout (NDJSON), then the summary object.
- gclo repos --org <name> lists what a sync would see (name, default branch,
archived; --json), and sync/repos share --include/--exclude globs (whole-name,
'*' and '?', case-insensitive, repeatable) and --skip-archived; sync gains
--dry-run (would clone / would update; JSON array with --json). sync now lists
first and calls the engine's subset overload, so '--include one-repo
--sanitize-paths' gives targeted path recovery.
- Exit codes: 3 = token rejected or access denied (never retry as is), 4 = rate
limited or throttled (retry later), 70 = unexpected error (type printed, full
exception written to the activity log with its path shown). Both were 2
before. The engine's listers now throw GitHubAccessException (derives from
InvalidOperationException, carries a Kind) so the mapping needs no message
parsing. Documented: fatal exits print no JSON document.
- gclo accounts add / edit / remove manage accounts without the desktop app
(token via the usual stdin/env/file options, never an argument);
'accounts --json' adds id, description, createOrgSubfolder, maxConcurrency,
lastSyncSummary.
- --parallel is clamped to the app's 1..64 range with a clear message.
- 'gclo help sync' dispatches to the command's help; unknown names are usage
errors.
docs/CLI.md rewritten for all of the above (commands, selection, JSON and
NDJSON schemas, accounts subcommands, exit-code table with what to do).
Tests: 61 new CLI tests (filters, dry-run text/JSON, NDJSON stream, sanitized
and invalidPaths JSON, parallel range, json/json-lines conflict, exit-code
mapping for sync/orgs/repos, repos listing and filters, RepoFilterSpec glob
rules, accounts add/edit/remove and their errors, ProgressPrinter modes).
386 Engine + 126 CLI tests, coverage 100% on all three libraries, format clean.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
||
|
James MaesandClaude Fable 5.1
|
95887f8eda |
Engine: close the verified engine findings of the unverified review (#31)
Each claim was re-checked against the current code before acting; all ten held.
- 404 fallback: an organization the token cannot see used to fall through to
/users/{org}/repos and sync only its public repositories with a green
result. The gateway now resolves the account kind first; an invisible
organization is an actionable error (grant the PAT / authorize SSO), an
unknown login gets a spelling hint, and only real user accounts take the
user-repos path.
- Default-branch rename: fetches prune, and when the tracked branch vanished
from origin the client follows origin's current default (remote HEAD
symref), checks it out with upstream wired, and removes the orphaned local
branch — only if every local commit is contained in the new default; a
branch with its own commits fails loudly instead. Previously the frozen
tracking ref made every later sync "up to date" forever.
- Cancellation: the engine awaits git operations with WaitAsync(token), so a
transport wedged in native code (no callbacks, no cancellation) cannot hold
the whole run; the orphan finishes on its own thread.
- Secondary rate limit / abuse / login-attempt 403s are no longer degraded
into "no organizations" (which read as a missing read:org scope); both
listers translate them with a retry hint.
- Recovery file: written atomically (temp + move) and loaded with a typed,
actionable error on corruption instead of a raw JsonException on every pull.
- Recovery materialization keeps a manifest of what it wrote and deletes
files that left the tree (pruning emptied directories), and records the
materialized tip so an unchanged tip no longer rewrites the whole tree on
every sync.
- The fresh-clone checkout is cancellable like every other checkout.
- A repository whose name is a Windows device name (aux, con, ...) fails its
own row with a plain reason before any git or disk access.
- The credentials handler refuses a third request for the same token with
the same 401 guidance the listers give, instead of libgit2's
"authentication replays" loop per repository.
Tests: 13 new (org-invisible and not-found fallbacks, secondary/abuse/login
throttles in both listers, branch rename followed and refused, prompt cancel
of a token-ignoring operation, device-name rows, deletion pass, no-rewrite at
tip, corrupt file, atomic save with manifest). 386 Engine tests, coverage
100%, format clean.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
||
|
James MaesandClaude Fable 5.1
|
05f0fc48be |
Updates: inline progress for check/download (#39); log every user and app action (#40)
#39 — Help > Check for updates now shows every phase in an InfoBar under the menu bar: "Checking for updates…" with an indeterminate bar (menu item disabled for the duration, re-entry guarded), the outcome inline (up to date with version, or the failure), and after the user confirms, "Downloading gclo vX… NN%" with a determinate bar driven by Velopack's progress callback until the restart. Only "update available" still asks with a dialog, because it is a decision. Failures no longer need a modal. #40 — The activity log is now a complete session trail. New entries, none of which ever contain a token: - App lifecycle: version + build hash at startup, "closing" at shutdown. - Connection: token entered/cleared (length only), organization lookup started/finished/failed, load (already logged). - Sync: retry count; path recovery requested/canceled/applied/failed; folder opened from the results bar; Sync all requested/cancel requested and the queued account list. - Accounts: added/updated (with or without a new token), deleted (with or without a token), sync result recorded; wizard opened (add/edit/save Quick Sync), canceled, token validating/accepted/rejected; delete canceled. - Settings: opened, closed without saving, saved with the exact field diff (theme, default folder, parallelism, splash on/off and duration, default token stored/removed). - Updates: check started (installed version), up to date, update available, declined, downloading, downloaded/restarting, failed. - Navigation/dialogs: workspace shown, activity log window, About, GitHub link. The duplicate "account deleted" entry moved from the shell into the store. Tests: ActionLoggingTests (6) pin the view-model-layer entries and assert no message contains the token; RecordingActivityLog added to the shared fakes. 373 Engine tests, coverage 100%, FlaUI e2e 8/8 (two timing flakes on the first post-build run passed on two clean reruns). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> |
||
|
James MaesandClaude Fable 5.1
|
6c5a968534 |
Shell + dialogs: real account edit path, save-as-account, wizard step errors, recovery pre-flight, idle page release (#30)
- The connection chip's Edit flyout on an account workspace no longer shows disabled fields and a "(next update)" note: it explains where the settings live and offers "Edit account…", which opens the edit wizard (the pane's right-click menu remains as the secondary path). - Quick Sync's chip gains "Save as account…": the add wizard opens seeded with the working token, organization, folder, subfolder preference, and parallelism (AccountWizardSeed), so saving a connection is a matter of naming it. The connect card mentions this. - Account wizard steps 3 and 4 show (and announce) why Next/Save did nothing: "Choose an organization…" / "Choose a target folder." - Path-recovery dialog: rows the validator has no safe rename for start as Skip (the CLI's default), and "Apply and check out" pre-flights the choices through the new PathRecoveryPlanner (unchanged-and-not-skipped rows, invalid replacements, two rows onto one destination) and keeps the dialog open listing the problems instead of closing, failing in the engine, and reopening with a fresh list. - MainWindow caches view models unconditionally but releases a workspace's page when the user navigates away and nothing is running in it; the page is rebuilt from the view model on return. WorkspacePage.Detach unhooks the page from the view model's events so it can actually be collected, and 'Sync all' warms unvisited workspaces with a view model only. Tests: PathRecoveryPlanner (sound plan, unchanged row, invalid replacement, duplicate destination, duplicate rows, null). 367 Engine tests, coverage 100%, FlaUI e2e 8/8. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> |
||
|
James MaesandClaude Fable 5.1
|
9479a76cd8 |
Engine + wizard: actionable pull errors, diff-only path validation, Octokit reuse, wizard step errors (#30)
- FetchAndPull translates NonFastForwardException into an InvalidOperationException that names the branch, says gclo only fast-forwards and never merges, and gives the ways out (push, reset, or re-clone); the detached-HEAD message gained the same treatment. The raw libgit2 text was the most common pull failure a user of this tool sees. - WindowsPathValidator.ValidateIncoming validates only what a pull introduces (added/copied/renamed/type-changed paths from a tree diff with rename detection) plus case-only collisions against what stays in the index, instead of re-walking the whole incoming tree on every pull with new commits. The full walk remains for clones and never-checked-out repositories. - OctokitGatewayCache keeps one Octokit client per token for the process so successive org lookups and loads reuse the warm HTTP connection instead of a fresh DNS/TCP/TLS handshake per action. - AccountWizardViewModel: steps 3 and 4 now set OrganizationError / TargetError instead of silently refusing to advance, and a new seeded constructor (AccountWizardSeed) carries a Quick Sync connection into the wizard so it can be saved as an account; the seed record redacts its token. Tests: pull-error message and inner exception; six ValidateIncoming cases (only introduced paths reported, modified-only is empty, case collision against existing, case change via delete+add allowed, detected rename releases the old spelling, null args); wizard step 3/4 messages and the seeded wizard end to end. 361 Engine tests, coverage 100%. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> |
||
|
James MaesandClaude Fable 5.1
|
64814d1411 |
Workspace: error surfaces, stale-table guard, visible-row selection, O(1) progress updates (#30)
Verified UX findings: - Load and organization-lookup failures now open a red, dismissable InfoBar with a Retry button (LoadErrorTitle/Message/Open), outside both page states so a failed first load and a failed refresh look the same. StatusText still carries the message for the live region. - Editing the organization after a load marks the table stale (LoadedOrganization/IsTableStale/StaleTableMessage): a warning bar with "Reload repositories" appears and SyncCommand is blocked until a reload, so the chip can no longer promise one org while the rows (and the clone) are another's. - A load that returns nothing shows an empty state naming the organization instead of a blank table with headers. - The header checkbox acts on the VISIBLE rows and reflects them, so "filter by name, then select what you see" is the subset workflow; hidden rows keep their selection and still sync. - Saving Settings no longer resets Quick Sync's parallelism unless the default itself changed (ApplySettings takes an explicit flag). Verified performance findings: - FilteredRepos is maintained incrementally on status transitions: one Insert or RemoveAt at the row's table position (binary search over a row->index map) instead of an O(n) rebuild with a Reset per report. Full rebuilds remain for filter, load, sort, and run start/end. - Per-repo failure logging moved off the UI thread into a progress wrapper that logs on the engine's worker thread before marshaling. - Header select-all and Retry-failed batch their selection changes: one recount and one CanExecute re-query per batch instead of per row. - Sort drops the O(n^2) IndexOf/Move choreography (nothing binds Repos) for a plain O(n) reorder plus the one rebuild of the bound list. Tests: 9 new WorkspaceViewModel tests (error surface open/close, lookup failure, stale/unstale by edit and by reload, visible-row header semantics, one CanExecute per batch, in-place membership updates with no Reset, sorted insertion after a sort, worker-thread failure logging). 353 Engine tests, coverage 100%, FlaUI e2e 8/8. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> |