fix(cli): exit codes, option handling and Ctrl+C for accounts and dry runs; the log follows GCLO_DATA_DIR #160

2026-11-10T22:34:48Z

james.maes commented

2026-11-10 22:34:48 +00:00

What

A batch of small command-line fixes, each with a regression test:

Dry run canceled while listing: says "Canceled.", prints no plan, exits 1 (was: empty plan, exit 0).
gclo accounts failures of the accounts file or Credential Manager: exit 2 with the reason, or 4 when another gclo process was changing the accounts (was: exit 70, "please report it").
gclo accounts -h is help; --description= is an empty description.
gclo accounts edit refuses a blank --rename, --org, --target.
A relative --target is stored as a full path.
The CLI's activity log follows GCLO_DATA_DIR, like accounts and settings (GcloPaths.LogDirectory).
Ctrl+C is held back only where something acts on it: sync, repos, orgs, and not while waiting at the token prompt (CancelKeyPolicy).

Why

Closes #159

How it was tested

AccountsCommandTests: -h; empty description in both forms, on add and edit; blank values refused; relative target stored as a full path; a target that is not a path; file-write and credential-store failures → exit 2 with the cause attached and nothing half-changed; busy accounts file → exit 4 for every subcommand; an unrelated failure stays what it is.
SyncCommandTests: canceled dry run, text and JSON.
CancelKeyPolicyTests, OptionReaderTests, TokenOptionsTests (the process says it is waiting for input while the token is read, and not afterwards), GcloPathsTests (log folder under a moved data folder).
Local gates: locked zero-warning x64 build, 912 engine and 194 CLI tests, 100% line coverage on the three gated assemblies, dotnet format --verify-no-changes.

Behavior changes for scripts

gclo sync --dry-run interrupted during the listing now exits 1, not 0.
gclo accounts add|edit|remove now exits 2 (or 4) for storage failures, not 70.
An account's target is stored absolute; gclo accounts --json shows the full path.
With GCLO_DATA_DIR set, the CLI writes its log under that folder.

Notes for the reviewer

The Ctrl+C handler itself lives in Program (the composition root, excluded from coverage); the decision is in CancelKeyPolicy, which is tested.
Not in this change: AccountWizardViewModel.SaveAsync throwing synchronously (it belongs with the view-model work).
ITokenVault gains a member in #158; the test double added here (BrokenVault) will need it once both are merged.

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 A batch of small command-line fixes, each with a regression test: - **Dry run canceled while listing**: says "Canceled.", prints no plan, exits 1 (was: empty plan, exit 0). - **`gclo accounts` failures of the accounts file or Credential Manager**: exit 2 with the reason, or 4 when another gclo process was changing the accounts (was: exit 70, "please report it"). - **`gclo accounts -h`** is help; **`--description=`** is an empty description. - **`gclo accounts edit`** refuses a blank `--rename`, `--org`, `--target`. - **A relative `--target`** is stored as a full path. - **The CLI's activity log follows `GCLO_DATA_DIR`**, like accounts and settings (`GcloPaths.LogDirectory`). - **Ctrl+C** is held back only where something acts on it: `sync`, `repos`, `orgs`, and not while waiting at the token prompt (`CancelKeyPolicy`). ## Why Closes #159 ## How it was tested - `AccountsCommandTests`: `-h`; empty description in both forms, on add and edit; blank values refused; relative target stored as a full path; a target that is not a path; file-write and credential-store failures → exit 2 with the cause attached and nothing half-changed; busy accounts file → exit 4 for every subcommand; an unrelated failure stays what it is. - `SyncCommandTests`: canceled dry run, text and JSON. - `CancelKeyPolicyTests`, `OptionReaderTests`, `TokenOptionsTests` (the process says it is waiting for input while the token is read, and not afterwards), `GcloPathsTests` (log folder under a moved data folder). - Local gates: locked zero-warning x64 build, 912 engine and 194 CLI tests, 100% line coverage on the three gated assemblies, `dotnet format --verify-no-changes`. ## Behavior changes for scripts - `gclo sync --dry-run` interrupted during the listing now exits 1, not 0. - `gclo accounts add|edit|remove` now exits 2 (or 4) for storage failures, not 70. - An account's target is stored absolute; `gclo accounts --json` shows the full path. - With `GCLO_DATA_DIR` set, the CLI writes its log under that folder. ## Notes for the reviewer - The Ctrl+C handler itself lives in `Program` (the composition root, excluded from coverage); the decision is in `CancelKeyPolicy`, which is tested. - Not in this change: `AccountWizardViewModel.SaveAsync` throwing synchronously (it belongs with the view-model work). - `ITokenVault` gains a member in #158; the test double added here (`BrokenVault`) will need it once both are merged. ## 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 1 commit 2026-11-10 22:34:48 +00:00

fix(cli): exit codes, option handling and Ctrl+C for accounts and dry runs; the log follows GCLO_DATA_DIR 7334ee21fc

- `gclo sync --dry-run` canceled while the repositories were being
  listed exited 0 with an empty plan. It now says it was canceled, prints
  no plan, and exits 1.
- `gclo accounts`: a failure of the accounts file or of Windows Credential
  Manager ended with exit code 70 ("please report it") where the help
  promises 2. It is now a fatal error with its reason (2), or "retry
  later" (4) when another gclo process was changing the accounts.
- `gclo accounts -h` was an unknown subcommand. `--description=` was
  rejected although `--description ""` works.
- `gclo accounts edit` accepted a blank --rename, --org, or --target.
- A relative --target was stored as typed, so an account meant a
  different folder depending on where it was used from. It is stored as a
  full path.
- The CLI's activity log ignored GCLO_DATA_DIR while accounts and settings
  followed it. The log folder is now "logs" under the data folder for the
  app and the CLI alike (GcloPaths.LogDirectory).
- Ctrl+C: the first one was held back ("Canceling...") also for commands
  that never look at the request (`accounts`) and while waiting at the
  token prompt, and then nothing happened. It is held back only where
  something acts on it (CancelKeyPolicy).

Closes #159

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

james.maes added 1 commit 2026-11-10 22:53:03 +00:00

fix(cli): address the independent review of the command-line fixes 7d55b24906

An edit of an account removed meanwhile, and credential-store or accounts
file failures in 'sync --account', are fatal errors with their reason, not
unexpected ones. A change to the accounts is let through to its end on
Ctrl+C. The target folder is checked before the token is read, and what
cannot be a folder is refused. A credential-store failure says what its
number means. Help and docs match.

Refs #159

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

james.maes commented

2026-11-10 22:53:03 +00:00

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

The reviewer drove the command cores through the test seams with its own probes (no real vault, no network) and ran the CLI from source for the help and usage paths. Nothing regressed; two failures in this change's own theme still exited 70. What was done, in the second commit:

#	Finding	Resolution
M1	`accounts edit` of an account another gclo process removed meanwhile exited 70.	It is a fatal error (2) with the store's reason. Test added.
M2	`sync --account` did not get the storage-failure treatment: an unusable Credential Manager exited 70.	The mapping is shared (`CliErrorException.IsStorageFailure` / `FromStorage`) and applied where the account is opened and its token read. Tests added.
L1	One Ctrl+C could end `accounts add/edit/remove` between the credential store write and the file write.	For `accounts` the first Ctrl+C lets the change finish (with a notice that says so); at the token prompt, where nothing has started, it still ends the process.
L2	A bad `--target` was reported only after the token had been read.	Resolved and checked before anything is read or opened. The test asserts the token line is still unread.
L3	The target check accepted almost anything (`a\|b`, `con`, `~`, `%USERPROFILE%\src`).	Refused with a reason each: characters no folder name can have, a device name, unexpanded `~` / `%NAME%`, a colon anywhere but after a drive letter. Drive-relative and UNC paths still work.
L4	The credential-store message carried only a number.	What Windows says the number means is appended; the test double now mimics the real vault's message.
L5	Help and docs gaps (accounts exit codes, `--target`, canceled `--dry-run --json`, the development edition's log).	Updated.
L6	The "busy" test threw from a place the real store never does.	One test now takes the real path: the lock is held, the store waits, the command exits 4.

Checked and found sound by the reviewer: catch order and single log line per failure in all four accounts sites; -h and unknown-option parsing; RequireValueOrEmpty; blank-value checks; the canceled dry run with --account records no sync result; no new path prints or logs a token; the CLI's default log location is unchanged when GCLO_DATA_DIR is not set; the CLI test assembly does not run in parallel, so the console and working-directory tests cannot race.

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

Noted for other work: accounts list with an unreadable accounts file says "No accounts yet" (exit 0); the desktop wizard stores a relative target as typed; a canceled dry run after the listing still prints its plan.

## Independent review (read-only agent): merge after fixes The reviewer drove the command cores through the test seams with its own probes (no real vault, no network) and ran the CLI from source for the help and usage paths. Nothing regressed; two failures in this change's own theme still exited 70. What was done, in the second commit: | # | Finding | Resolution | | --- | --- | --- | | M1 | `accounts edit` of an account another gclo process removed meanwhile exited 70. | It is a fatal error (2) with the store's reason. Test added. | | M2 | `sync --account` did not get the storage-failure treatment: an unusable Credential Manager exited 70. | The mapping is shared (`CliErrorException.IsStorageFailure` / `FromStorage`) and applied where the account is opened and its token read. Tests added. | | L1 | One Ctrl+C could end `accounts add/edit/remove` between the credential store write and the file write. | For `accounts` the first Ctrl+C lets the change finish (with a notice that says so); at the token prompt, where nothing has started, it still ends the process. | | L2 | A bad `--target` was reported only after the token had been read. | Resolved and checked before anything is read or opened. The test asserts the token line is still unread. | | L3 | The target check accepted almost anything (`a\|b`, `con`, `~`, `%USERPROFILE%\src`). | Refused with a reason each: characters no folder name can have, a device name, unexpanded `~` / `%NAME%`, a colon anywhere but after a drive letter. Drive-relative and UNC paths still work. | | L4 | The credential-store message carried only a number. | What Windows says the number means is appended; the test double now mimics the real vault's message. | | L5 | Help and docs gaps (accounts exit codes, `--target`, canceled `--dry-run --json`, the development edition's log). | Updated. | | L6 | The "busy" test threw from a place the real store never does. | One test now takes the real path: the lock is held, the store waits, the command exits 4. | Checked and found sound by the reviewer: catch order and single log line per failure in all four `accounts` sites; `-h` and unknown-option parsing; `RequireValueOrEmpty`; blank-value checks; the canceled dry run with `--account` records no sync result; no new path prints or logs a token; the CLI's default log location is unchanged when `GCLO_DATA_DIR` is not set; the CLI test assembly does not run in parallel, so the console and working-directory tests cannot race. Gates after the fixes: 912 engine and 211 CLI tests, 100% line coverage on the three gated assemblies, zero warnings, format clean. Noted for other work: `accounts list` with an unreadable accounts file says "No accounts yet" (exit 0); the desktop wizard stores a relative target as typed; a canceled dry run after the listing still prints its plan.

james.maes merged commit d518504e47 into main

2026-11-10 22:53:06 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/gclo#160