Skip to content

fix: npx/global bin does nothing (0.1.1 hotfix) - #6

Merged
dvd90 merged 1 commit into
mainfrom
fix/bin-symlink-entry
Jul 11, 2026
Merged

fix: npx/global bin does nothing (0.1.1 hotfix)#6
dvd90 merged 1 commit into
mainfrom
fix/bin-symlink-entry

Conversation

@dvd90

@dvd90 dvd90 commented Jul 11, 2026

Copy link
Copy Markdown
Owner

Critical: published 0.1.0 is dead on its headline command

npx gitgotchi (and npm i -g gitgotchigitgotchi) exits 0 and prints nothing.

Cause: the entry-point guard compared import.meta.url to process.argv[1] directly. npm runs a package's bin through a symlink (node_modules/.bin/gitgotchidist/cli.js), so argv[1] is the symlink path while import.meta.url is the real file — they never match, the run block is skipped, and the process does nothing.

Verified locally: node dist/cli.js printed the pet; node <symlink-to-cli.js> printed nothing.

Fix: resolve argv[1] to its real path before comparing. Extracted isDirectRun(metaUrl, argv1) and added a test that builds a real file + a symlink to it and asserts both resolve as the entry point (the regression this guards).

  • src/cli.ts: realpathSync(argv[1]) in the entry check
  • test/cli.test.ts: symlink entry-point test
  • version 0.1.00.1.1

131 tests green, typecheck + lint clean. After merge this needs a npm publish of 0.1.1 (0.1.0 should be deprecated).

🤖 Generated with Claude Code

The entry-point guard compared import.meta.url to argv[1] directly, but npm
runs the bin through a symlink (node_modules/.bin/gitgotchi), so the paths
never matched and `npx gitgotchi` exited 0 doing nothing. Resolve argv[1]'s
real path before comparing. Extracted isDirectRun() and covered the symlink
case with a test. Bumps 0.1.0 → 0.1.1.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown

Claude finished @dvd90's task in 2m 1s —— View job


Review: fix/bin-symlink-entry

  • Gather context (diff, CLAUDE.md conventions)
  • Review src/cli.ts changes (entry-point guard fix)
  • Review test/cli.test.ts changes (TDD compliance)
  • Check engine purity / collectors-never-throw / product principles applicability
  • Post final review

Verdict: looks good, approving the approach. This is a correct, well-scoped hotfix.

Root cause & fix
The diagnosis is right and the fix is the officially-documented Node.js ESM idiom for this exact problem: comparing import.meta.url to pathToFileURL(realpathSync(argv[1])).href instead of a raw path compare (src/cli.ts:117-124). Node resolves symlinks when setting import.meta.url for the entry module by default, so a raw argv[1] compare was always going to miss the node_modules/.bin/gitgotchi symlink case — this matches expectations.

TDD / CLAUDE.md conventions

  • Extracting isDirectRun() as a pure, exported, testable function instead of leaving the logic inline in the guard is the right call — this couldn't have been unit-tested otherwise.
  • test/cli.test.ts:47-61 covers the actual regression (symlink → true) plus solid boundary cases: real file, unrelated/nonexistent file (exercises the catch branch), and undefined argv1. That's good boundary coverage per the "exact thresholds, null inputs" guidance, even though this is a hotfix rather than a milestone-doc-driven feature.
  • Single commit makes red→green ordering unverifiable from history, but the PR description states the test was written to capture the regression first, and the diff is coherent with that.
  • Commit message follows conventional commits (fix: ...).
  • The inline comment on isDirectRun explains a genuinely non-obvious constraint (why the realpath resolution is required) rather than restating what the code does — consistent with the repo's comment policy.

Product principles

  • Directly restores principle M0: scaffold and core type contracts #1 (zero-config first run via npx gitgotchi) which was fully broken in 0.1.0 — appropriately treated as critical.
  • Doesn't touch src/engine/** or src/collectors/**, so purity/never-throw rules aren't in play here.
  • Version bump 0.1.0 → 0.1.1 in package.json/package-lock.json is consistent and appropriate for a hotfix; PR body correctly flags that 0.1.0 should be deprecated on npm post-merge.

Minor, non-blocking note

  • Nothing I'd hold up the merge for. One nit: the catch branch in isDirectRun is exercised implicitly (via the nonexistent other.js path) rather than an explicit dedicated case — fine as-is, just noting it for anyone re-reading later.

Verification note: I was not able to run npm test/npm run typecheck/npm run lint locally in this sandbox (npm ci requires approval that isn't available in this context), so I relied on manual review of the diff and cross-checked the fix against Node's documented ESM entry-point detection pattern. The PR description states 131 tests green, typecheck + lint clean — worth confirming via CI before merge.

@dvd90
dvd90 merged commit 5c0cf96 into main Jul 11, 2026
5 checks passed
@dvd90
dvd90 deleted the fix/bin-symlink-entry branch July 11, 2026 21:17
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