v3.2.0: scoped config setters, one identity key - #17
Conversation
Every setting is now settable at either scope through a single `setValue` helper, with the default chosen to match what people mean by the bare command: sync and privacy are about this repo, identity and api-url about the user. Each command prints the file it wrote. `sync:disable` previously wrote the global config, silencing every repo at once; it now silences only the current one, with --global to restore the old behavior. `playAsUsername` and `githubUsername` were one concept under two names, which is why identity needed a bespoke resolver instead of the normal repo-overrides-global merge. Collapsed into `githubUsername`, with a one-shot idempotent migration on read. Also fixes init suppressing the org-credit prompt under privacy mode (the reason a private org repo had to be hand-edited) and init reading the obfuscated private/private remote for its own local decisions. Closes #13 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review round 1 on #17. Repo config assumed `cwd/.git`, which only resolves at the repo root of an ordinary checkout. That was survivable while repo scope was reserved for init-time settings; as of this branch it is the default for sync and privacy, so it became a setting the CLI writes and then can't find: from a subdirectory `getRepoConfig()` returned `{}` and the merge fell back to the global defaults, so a repo with sync disabled synced and a private repo sent its real owner and name. In a worktree or submodule `.git` is a file, so `hasRepoConfigTarget()` passed and the write failed with ENOTDIR. Resolves via `git rev-parse --absolute-git-dir`, cached per cwd, behind a fast path for the root case that the post-commit hook always hits. `init` seeded `usePrivacyMode` from nothing, so a re-run with privacy already on took the public branch, printed "✓ Public repository confirmed" and claimed the repo URL, owner and name were sent while getRepoInfo() was still sending private/private. Also wraps `username:set` in the error handler the other commands have — its `--repo` path could surface an unhandled rejection. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review round 1 — all 4 findings fixed (
|
Review round 2 on #17. Round 1 resolved the git dir with `--absolute-git-dir`, which is per-worktree. Per-repo settings are properties of the repository, not of a checkout — and hooks live in the common dir, so a hook installed from the main checkout fires in every worktree. Splitting the config reintroduced round 1's own bug: `privacy:on` at the main root wrote a config the worktree couldn't see, so the merge fell back to the global defaults and the private repo sent its real owner and name. Now `--git-common-dir`, resolved against cwd because git returns it relative from inside an ordinary checkout and absolute from a linked worktree. The round 1 test only asserted where the write landed, so it encoded the split rather than catching it. It now sets privacy in the main checkout and reads it from the worktree, and sync the other way. init's hookPath had the same `cwd/.git` assumption and this branch made the crash newly reachable: in a worktree the write threw ENOTDIR after init had already prompted for and persisted the privacy answer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review round 2 — both findings fixed (
|
Review round 3 on #17. `username:set <name> --repo` is new in this branch and lets a repo be credited to any name. Plays resolve their token through that name, so setting one you hold no token for makes every commit's sync fail — and the failure is invisible: play.ts swallows the error, and its "not authenticated" notice is gated off --small, which is the post-commit hook's only mode. It now warns where the choice is made, and the README documents the login step alongside the --repo example. init's credit prompt keyed on owner-vs-personal, which pre-3.2 was the only way an override could exist. With --repo it isn't: in `than/my-app` overridden to `broomfitters`, init skipped the question, authenticated as than, and left plays credited to broomfitters. The candidate list now includes the existing override as its own option. That list moved to utils/credit.ts to be testable — init.ts imports chalk, which is ESM-only and can't be loaded by this repo's jest transform. isPrivateRepo() is merged now, so init's "already enabled for this repo" could be reporting the global default; it says which. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review round 3 — all 3 findings fixed (
|
|
Review round 4 — one real hole, plus small stuff Rounds 1–3 landed well. One finding worth fixing before merge.
Same shape as round 3's finding — a hole
const globalUsername = getGlobalConfig().githubUsername;
const persist =
!globalUsername || globalUsername.toLowerCase() === githubUsername.toLowerCase();The
Following that instruction hits Suggested fix — narrow the escape hatch so it does not cover the one name this repo already routes elsewhere: const globalUsername = getGlobalConfig().githubUsername;
const repoOverride = getPlayAsUsername();
const persist = globalUsername
? globalUsername.toLowerCase() === githubUsername.toLowerCase()
: repoOverride?.toLowerCase() !== githubUsername.toLowerCase();"First login establishes you" survives for every other case. Logging in as the name this repo is explicitly credited to stores the token and leaves the global identity unset — which Lower priority
The changelog overstates the output. "All of them print which file they wrote" —
On round 1 deferred corrupt-JSON item — still fine to leave, but the calculus shifted. Nothing else: no I could not run |
Review round 4 on #17, from the GitHub Actions reviewer. The `!globalUsername` escape hatch in login was safe through 3.1.1 because init was the only writer of a per-repo override and it sets the global identity first, so "an override exists but no global identity does" was unreachable. `username:set <name> --repo` is standalone and writes only the repo config, so on a machine that has never run init, following the "log in as <name>" hint this branch prints adopted the org globally — every other repo crediting its plays to it, and a bare logout targeting it. The hijack two releases have been closing, reached through the remedy for round 3's finding. The rule is now a tested function: an established identity still decides, a first login still establishes you, except for the one name this repo already routes elsewhere. Also from that review: config:set rejects unrecognized booleans rather than coercing to false (`private-repo yes` turned privacy off); init resolves the git dir before its first prompt, so a failure can't leave a saved privacy answer with no hook, and its action has the try/catch every other command has (program.parse() isn't awaited, so a throw was an unhandled rejection); `username:set --global` names its scope; the changelog no longer claims commands print the file they wrote when they print the scope. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review round 4 — fixed (
|
|
Review round 5 — no blockers; the round-4 fix is correct
Re-verified the rest of the round-4 items against the tree rather than the description:
Also spot-checked the invariants that matter outside the diff: Everything below is minor. The changelog claim moved rather than went away.
type ScopedKey = 'githubUsername' | 'apiUrl' | 'syncEnabled' | 'privateRepo';
function setValue<K extends ScopedKey>(key: K, value: Config[K], scope: Scope): voidAll four current callers already pass one of those, so it is a type-only change. Value validation runs after the repo-target check.
Residual on the round-4 hole, and I think it is acceptable as-is. The narrowing keys on this repo's override, so the hijack is still reachable one directory over: fresh machine, no global identity, Same caveat as round 4: no network in this sandbox, so |
Review round 5 on #17 — no blockers, five minor items, all taken. whoami printed scopes while the changelog and README claimed it named files. Rather than downgrade the wording, it prints both config paths — worth having now that repo config resolves to the *common* git dir, which in a worktree is not the .git next to you. setValue took all of `keyof Config`, so only convention kept a token out of a repo config. A `ScopedKey` union makes the compiler hold the line the issue drew: a token belongs to an identity, not a directory. parseBoolean runs before the repo-target check, so `sync-enabled maybe` outside a repo reports the argument you got wrong rather than the directory and then a second error once you fix it. `username:set <name> --global` in an overridden repo said nothing, while its token warning keyed on a name plays here never resolve to — so setting a name you already hold a token for printed nothing at all. It now prints the note auth.ts already prints after a non-adopting login. On the residual round-4 hole the reviewer judged acceptable (login from a *different* directory can still adopt an org set with --repo elsewhere): taken the cheap end they suggested — `username:set --repo` with no global identity says to set a personal one first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review round 5 — all five minor items taken (
|
|
Review round 6 — no blockers; one gap the branch’s own hint walks into The round-5 items all landed, verified against the tree rather than the description:
Also re-checked the invariants outside the diff: One finding. Nothing can remove a per-repo override, and the new note recommends creating a permanent one
That suggestion does not restore inheritance — it writes a repo And
So that pin is unreachable from the CLI; hand-editing Cheapest fix is at the hint: point at the clear path instead of the pin. But since Credit where due: Nits
Pre-existing, worth its own issue
Nothing else. The scope model reads well, the Same sandbox caveat as rounds 4–5: no network here, so |
Review round 6 on #17 — no blockers, one gap worth the fix. `clearPlayAsUsername()` had exactly one caller: init's credit prompt. That was proportionate while init was also the only creator of an override. This branch made `username:set <name> --repo` a standalone creator without a standalone remover, and init cannot always undo one — it exits when there is no GitHub remote, and skips the prompt entirely when the candidates dedupe to one, which is exactly what a redundant pin produces. Hand-editing .git/slot-machine-config.json was the only way out: the state #13 opened on. Worse, round 5's own note walked people into it. `username:set than --global` in an overridden repo suggested `username:set than --repo`, which does not restore inheritance — it pins the repo to today's global name, so changing the global identity later silently leaves the repo behind and fires the same note again recommending the same pin. It now points at the clear. Nits from the same review: init's credit prompt re-asks on unparseable input instead of falling through to choice 1, which is the branch that *clears* an override — survivable with two options, less so now that owner plus override can make three. Empty input still takes the default, so a closed stdin terminates. And `config:set api-url --global --repo` reports the contradiction rather than the narrower global-only rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review round 6 — fixed (
|
|
Review round 7 — no blockers; one gap in the new remover Round 6's items landed, verified against the tree:
One finding.
|
Closes #13. Plan:
~/.claude/plans/git-slot-machine-3.2.0-config-scopes.mdBehavior change to call out
sync:disablenow silences only the current repo. It previously wrote the global config, silencing every repo at once — the symptom that opened #13.sync:disable --globalrestores the old behavior. Intended fix, but it will surprise anyone who relied on the old default.What changed
syncglobal-only,privateReporepo-only). All five now route through a singlesetValue(key, value, scope)over the existing load/save pairs — no new IO paths.sync:*andprivacy:*default to repo,username:setandconfig:set api-urldefault to global; every one takes--global/--repoand prints the file it wrote.playAsUsernameandgithubUsernamewere the same concept under two names, which is whygetGitHubUsername()needed a bespoke resolver instead of the merge that already handles every other key. Collapsed intogithubUsername, resolved throughgetConfig(). Repo configs migrate on first read — idempotent, no write when there's nothing to migrate (this runs on every post-commit play), and the in-memory result still stands if the write-back fails.api-urlstays global-only and rejects--repo.getApiUrl()reads global config only so a.git/slot-machine-config.jsoncan't redirect an authenticated sync; a repo-scoped value would be silently inert, so writing one would be a lie.privateReporeads through the merge, so a globalprivateRepo: truemeans "default all my repos to private". No existing config sets it globally — no-op on upgrade.privacy:on/privacy:off. Falls out of the scoped setter for free. Privacy mode was only settable duringinit, so re-runninginitwas the only way to change it.whoamimarks which file owns each setting, and names the global value when a repo overrides it.Fixes found along the way
initsuppressed the org-credit prompt under privacy mode (!usePrivacyModein the guard) — the reason~/Broomfitters/houseneededplayAsUsernamehand-written into its repo config. Privacy hides the repo; the username is sent either way, asinitalready tells the user.initwas reading the obfuscated remote for its own local decisions. With privacy mode already on,getRepoInfo()returnsprivate/private— so a re-run queriedapi.github.com/repos/private/privateand would have offered to credit an org named "private". AddedgetRemoteRepoInfo()for local-only decisions; anything server-bound still goes throughgetRepoInfo().ENOENT. Now: "not a git repository — use --global".clearPlayAsUsername()deletes both keys. The plan didn't mention the clear path; leaving it on the legacy key would have silently regressed the 3.1.1 hijack fix, since init's "personal credit" branch is the only way back from an org override.Verification
86 tests (was 71),
tsc --noEmitclean,pnpm buildclean. 15 new tests cover the repo migration (including the idempotent no-write path, the write-failure path, and token resolution through a migrated identity), scope isolation in both directions, and the clear path.Live-run against the real configs (backed up first, then restored):
~/Broomfitters/house—playAsUsername: "broomfitters"becamegithubUsername: "broomfitters"on first read;whoamioutput unchanged.sync:disablein house wrote only.git/slot-machine-config.json; global untouched, other repos still enabled.sync:disable --globalwrote global and every repo reported disabled.than, house plays asbroomfitters.--global --repotogether,api-url --repo.Not run interactively: the
initorg-credit prompt under privacy mode (plan verification step 6).initcalls the GitHub API andauthLoginCommand, so a scripted run would create real leaderboard state. The change is the one-line guard atinit.tsplus the raw-remote switch; verified by code review and covered indirectly by the migration tests, not by an interactive run.Out of scope
hasRepoConfigTarget()returns true when.gitis a file (worktrees, submodules), wheresaveRepoConfigthen fails withENOTDIR. Pre-existing —setPrivateRepoalways had it — not a regression from this change.🤖 Generated with Claude Code