feat: env var tokens, SIGINT cleanup, non-TTY spinner

[nrjdalal]

Summary

Three small, practical improvements for CI/scripting use cases.

1. Environment variable token support (#8)

Private repos without embedding tokens in URLs:

export GITHUB_TOKEN=ghp_xxxx    # or GH_TOKEN
export GITLAB_TOKEN=glpat-xxxx
export BITBUCKET_TOKEN=xxxx

gitpick owner/private-repo  # token picked up automatically

URL-embedded tokens (https://token@github.com/...) still take precedence.

2. SIGINT/SIGTERM temp dir cleanup (#14)

Registers signal handlers that clean up active temp directories when the process is killed. No more orphaned dirs in /tmp.

3. Non-TTY spinner suppression (#15)

Spinner animation is now completely suppressed when stdout is not a TTY (CI logs, piped output). Only the success line is printed:

# before (in CI)
⠋ Picking tree from repository...
✔ Picked tree from repository in 1.08 seconds.

# after (in CI)
✔ Picked tree from repository in 1.08 seconds.

Test plan

• All 125 existing tests pass
• GITHUB_TOKEN=fake_token gitpick owner/repo --dry-run works
• GH_TOKEN fallback works
• Piped output shows no spinner frames
• Manual SIGINT test — no orphaned temp dirs

🤖 Generated with Claude Code

[nrjdalal]

Score: 8.5/10

No blocking findings in the diff. The implementation across bin/utils/transform-url.ts, bin/utils/clone-action.ts, and bin/external/yocto-spinner.ts looks reasonable, and I re-ran the current suite locally; bun test passes with 125/125 tests.

The main follow-up I would recommend before or soon after merge is adding regression coverage for the new behavior introduced here, since this PR changes user-visible and lifecycle-sensitive paths but does not add tests for them yet. In particular:

• env token precedence and host selection (GITHUB_TOKEN / GH_TOKEN, GITLAB_TOKEN, BITBUCKET_TOKEN)
• non-TTY spinner suppression so CI/piped output does not emit spinner frames
• SIGINT/SIGTERM temp-dir cleanup behavior, ideally with a focused integration test or a narrower unit seam around the cleanup registration/handler

So this looks mergeable to me, but not quite at a 10/10 without coverage for the new code paths.

[nrjdalal]

Updated review after the latest push:

Score: 9.5/10

This is in much better shape now. The new tests cover the non-TTY spinner path and the SIGINT temp-dir cleanup path, and I re-ran the suite locally after commit c3bc146; bun test passes with 130/130 tests. I do not have any blocking issues in the implementation diff.

One small follow-up note: the first env-token test (GITHUB_TOKEN is used for shorthand URLs) does not actually set GITHUB_TOKEN, so it currently validates the baseline public shorthand flow more than the env-token path itself. The GH_TOKEN and URL-precedence cases are stronger, so this is minor, but if you want that section to be fully explicit it would be worth setting GITHUB_TOKEN in that test the same way the fallback/precedence tests do.

[nrjdalal]

Updated review after the latest push:

Score: 9.5/10

The env-token coverage is in better shape now: the GITHUB_TOKEN case is explicit, the fallback and precedence cases are covered, and I re-ran bun test locally after commit 4394d9e; it passes with 129/129 tests. I still do not have any blocking concerns in the implementation diff.

The only reason I am still holding this just below 10 is that the SIGINT/SIGTERM cleanup path remains untested in the final PR state after the signal test was removed. I understand why that test may have been flaky across environments, so this is not a blocker, just the remaining bit of residual risk in an otherwise solid PR.

[nrjdalal]

Updated review after the latest push:

Score: 10/10

This closes out the last meaningful gap from my earlier reviews. The PR now has coverage for env-token resolution, non-TTY spinner suppression, and the SIGINT temp-dir cleanup path, and I re-ran bun test locally after commit 6150961; it passes with 130/130 tests.

At this point I do not have any blocking comments left.