marvinmees/GCM
1
0
Fork 0
Code Issues Pull requests 1 Projects Releases 1 Packages Wiki Activity Actions

fix/apply-reliability-and-auto-detect #5

Open
marvinmees wants to merge 5 commits from fix/apply-reliability-and-auto-detect into main
pull from: fix/apply-reliability-and-auto-detect
merge into: marvinmees:main
Conversation 0 Commits 5 Files changed 12 +480 -106
marvinmees commented 2026-08-01 18:41:20 +00:00
Owner
Copy link
No description provided. 
Tracks the fixes identified during a review of gcm apply/auto: a
crash on standard SSH remotes, silently-swallowed git config errors,
and an argument-injection issue that could corrupt config values.
git@host:path (the form GitHub/GitLab/Bitbucket all tell you to use
for SSH) is not a valid absolute URI, so `new Uri(url).Host` threw a
UriFormatException for it - crashing `gcm auto` for the most common
SSH remote form. The regex matchers already captured the hostname in
a group but that capture was discarded in favor of re-parsing the raw
"<url> (fetch)" string with Uri, which also never worked correctly.

Normalize scp-like syntax to ssh:// before handing it to Uri, strip
the trailing " (fetch/push/pull)" annotation first, and use
Uri.TryCreate generically instead of two hand-rolled gating regexes.
Also fall back to the literal address for IP-literal hosts, since
slicing "the label before the TLD" doesn't make sense for those.
GitConfigApplier.Apply redirected git's stdout/stderr but never read
either stream and never checked the exit code, so a failing `git
config` call (bad key, permissions, etc.) was silently discarded and
Program.cs printed "Applied N entries" regardless. It also built the
process Arguments as one interpolated string, so a key or value
containing a space could be re-split into extra argv tokens by git -
e.g. a custom key of "user.name --global" silently wrote the literal
value "--global" into user.name instead of the intended value.

Switch to ArgumentList (removes the argument-splitting risk
entirely), check the exit code per entry, and capture stderr so
failures can be reported. Apply is now ApplyAsync, returning a
GitConfigApplyResult per entry instead of void; Program.cs reports
per-entry failures and only claims success when every entry actually
applied.

Also fixes IsGitRepo's console leak: the `git status` pre-flight
check didn't redirect output, so its raw porcelain text (branch name,
working tree status, etc.) printed straight to the terminal ahead of
GCM's own UI on every `apply`/`auto` run. Replaced with `git rev-parse
--is-inside-work-tree`, redirected, which is also the more standard
scripting-safe way to test this (the old approach also treated any
other fatal git error that happened to exit 128 as "not a repo").

Rounds out the exit-code story: Main now returns real process exit
codes (0/1) instead of always exiting 0, so callers/scripts can
detect failure; a missing git executable now produces a friendly
message via a top-level Win32Exception catch instead of a raw stack
trace; and the dead `return Directory.Exists(...)` code left over
from an earlier refactor (flagged by a CS0162 warning) is removed
along with the CS8602 nullable-dereference warning it was next to.
The "Custom key" editor accepted any text as a key with only a
whitespace check, which is how malformed keys like "user.name
--global" reached GitConfigApplier in the first place. Add
GitConfigKeyValidator (checks the section[.subsection].name grammar
git itself expects) and reject an invalid key inline in the TUI
before it's committed, with an error shown under the prompt until the
user fixes it or cancels.
fix(core): correct Zdiff3 merge conflict style label
All checks were successful
Continous Integration / build-and-test (pull_request) Successful in 26s
Details
a41c1d3328
 
The merge.conflictstyle choice for value "zdiff3" was labeled "Zdsc3"
in the TUI - a typo. The stored value was already correct, so this
only affects the displayed label.
All checks were successful
Continous Integration / build-and-test (pull_request) Successful in 26s
Details
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/apply-reliability-and-auto-detect:fix/apply-reliability-and-auto-detect
git switch fix/apply-reliability-and-auto-detect

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff fix/apply-reliability-and-auto-detect
git switch fix/apply-reliability-and-auto-detect
git rebase main
git switch main
git merge --ff-only fix/apply-reliability-and-auto-detect
git switch fix/apply-reliability-and-auto-detect
git rebase main
git switch main
git merge --no-ff fix/apply-reliability-and-auto-detect
git switch main
git merge --squash fix/apply-reliability-and-auto-detect
git switch main
git merge --ff-only fix/apply-reliability-and-auto-detect
git switch main
git merge fix/apply-reliability-and-auto-detect
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
marvinmees/GCM!5
No description provided.