Skip to content

fix: sanitize branch name and update cli tests#7

Merged
MusicMaster4 merged 2 commits into
mainfrom
testing
Jul 11, 2026
Merged

fix: sanitize branch name and update cli tests#7
MusicMaster4 merged 2 commits into
mainfrom
testing

Conversation

@MusicMaster4

Copy link
Copy Markdown
Owner

Summary

  • Fixed multiple issues with branch name normalization and sanitization
  • Added sanitizeBranchName function to lib/branch-name.ts
  • Implemented doctor command to check environment health
  • Added checks for Node.js version, Git presence, repository, branch, and upstream
  • Improved error handling and provided helpful tips for issues

Test plan

  • Run gitgen doctor in a clean project directory
  • Verify all checks pass and no errors occur
  • Test with different Node.js versions (e.g., 16, 17, 18) to ensure version check works
  • Run gitgen doctor in a directory without Git installed to test Git check failure
  • Test with detached HEAD and verify branch check warns about it
  • Test with a non-existent upstream branch and verify upstream check fails

@greptile-apps

greptile-apps Bot commented Jul 11, 2026

Copy link
Copy Markdown

Greptile Summary

This PR introduces a gitgen doctor command (lib/doctor.ts) that runs a suite of environment health checks (Node.js version, Git, repo, branch, upstream, API key, config file, GitHub CLI, and OpenRouter connectivity), wires it into the CLI, and adds a new pull command that defaults to --rebase. A test suite covers the core check logic with injected command runners.

  • All four issues raised in the previous review cycle are resolved: checkUpstream now returns null in detached HEAD, checkConfigPath uses existsSync, checkApiKey no longer has an unused parameter, and checkGhAuth accepts a pre-computed ghAvailable boolean instead of re-running gh --version.
  • The new pull command checks for upstream before pulling but does not check for detached HEAD independently, so it can surface a misleading "push first" error when the user is in detached HEAD state — the same class of fix applied to checkUpstream was not mirrored here.

Confidence Score: 5/5

Safe to merge — the doctor command and pull command are additive, the previously flagged issues are all addressed, and the new code is well-isolated behind a dependency-injection interface.

All four issues from the prior review cycle are fixed in this revision, the new test coverage exercises the injection seams correctly, and the one remaining gap (misleading error wording in the pull command for detached HEAD) is a UX-only concern that does not affect correctness or data safety.

scripts/cli.ts — the pull command's detached HEAD path could give misleading guidance

Important Files Changed

Filename Overview
lib/doctor.ts New file implementing all doctor health checks. All four issues from previous review threads are addressed: detached HEAD is handled in checkUpstream (returns null), checkConfigPath now uses existsSync, checkApiKey has no configPath parameter, and checkGhAuth accepts ghAvailable:boolean instead of re-running gh --version.
scripts/cli.ts Adds doctor and pull commands. Pull command correctly uses --rebase by default and checks for upstream before pulling, but does not guard against detached HEAD separately — giving a misleading error message in that state.
test/doctor.test.ts New test file covering parseNodeMajor, summarizeDoctorChecks, and three runDoctorChecks scenarios via injected command runners. Tests correctly cover the detached HEAD/upstream skip, OpenRouter 401 fail, and missing-git-repo paths.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[gitgen doctor] --> B[runDoctorChecks]
    B --> C[checkNode]
    B --> D[checkGit]
    B --> E[checkRepo]
    E -->|inside work tree| F[checkBranch]
    F -->|named branch| G[checkUpstream]
    F -->|detached HEAD| H[null — skip upstream]
    G -->|upstream set| I[ok]
    G -->|no upstream| J[warn]
    B --> K[checkApiKey]
    B --> L[checkConfigPath]
    B --> M[checkGh]
    M -->|gh available| N[checkGhAuth]
    M -->|gh missing| O[skip gh auth]
    K -->|valid OR key| P[checkOpenRouter via fetch]
    K -->|no/invalid key| Q[skip network check]
    B --> R[summarizeDoctorChecks]
    R --> S{any fail?}
    S -->|yes| T[exitCode = 1]
    S -->|no| U[exitCode = 0]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[gitgen doctor] --> B[runDoctorChecks]
    B --> C[checkNode]
    B --> D[checkGit]
    B --> E[checkRepo]
    E -->|inside work tree| F[checkBranch]
    F -->|named branch| G[checkUpstream]
    F -->|detached HEAD| H[null — skip upstream]
    G -->|upstream set| I[ok]
    G -->|no upstream| J[warn]
    B --> K[checkApiKey]
    B --> L[checkConfigPath]
    B --> M[checkGh]
    M -->|gh available| N[checkGhAuth]
    M -->|gh missing| O[skip gh auth]
    K -->|valid OR key| P[checkOpenRouter via fetch]
    K -->|no/invalid key| Q[skip network check]
    B --> R[summarizeDoctorChecks]
    R --> S{any fail?}
    S -->|yes| T[exitCode = 1]
    S -->|no| U[exitCode = 0]
Loading

Reviews (2): Last reviewed commit: "fix: skip upstream check in detached HEA..." | Re-trigger Greptile

Comment thread lib/doctor.ts
Comment thread lib/doctor.ts Outdated
Comment thread lib/doctor.ts
Comment thread lib/doctor.ts Outdated
@MusicMaster4 MusicMaster4 merged commit 1e81158 into main Jul 11, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant