fix(judge): guard judge configure on a project and back-fill the .nanotune/.gitignore - #131
Open
addyCooks wants to merge 3 commits into
Open
Conversation
…itignore `judge configure` never checked for a project, so with no .nanotune/ the 0600 exclusive create in saveJudgeConfig died with ENOENT — and because the save sat inside the connection-test try block, the user was told "Connection test failed: ENOENT..." after the API call had already succeeded. They debug their network or their key; the key they typed is gone. Gate the command on configExists() before the first prompt, using the same message every other command uses, and move the save out of the connection-test try so a write failure reports itself as a write failure. The second half is the .gitignore that shields judge.json. It was written only by initializeProjectDirs, only from `nanotune init`, and only when the file did not already exist — so a project keeps whatever .gitignore it was initialised with forever. One from 1.3.x or earlier has no judge.json line at all, and one from 1.4.0-1.6.x has a bare `judge.json` that misses the judge.json.tmp an interrupted save leaves behind. Both hold a literal API key that `git add .` would commit, which is what the careful 0600 write was protecting against in the first place. Back-fill missing entries into an existing .gitignore instead of skipping the write, and call initializeProjectDirs from saveJudgeConfig so writing the key is what repairs the file. Closes Nano-Collective#127
The command-level guard test pulled src/commands/judge.tsx into the coverage report for the first time. At 409 lines and 28.85% covered it dragged the project total from 71.35% to 69.40% — under the floor that pr-checks.yml pins at 71, and a regression besides, since fail-on-drop ratchets that number. Cover the command properly instead of backing the guard test out. The form is driven by watching the rendered frame rather than by counting keystrokes, because @inkjs/ui swallows the first keypress that lands on a freshly mounted TextInput. Two of these earn their place beyond the coverage arithmetic: against a stub judge on localhost, a 400 reports "Connection test failed" and writes nothing, while a judge that answers and a judge.json path that cannot be renamed onto reports "Failed to save judge config". That is the split this branch introduced, pinned from the outside. judge.tsx 0 -> 79.7%, judge.ts 71.93 -> 89.53%, config.ts 90.4 -> 96.46%; project total 71.35% -> 73.01%.
…f git Neither page said `judge configure` needs a project, and nothing documented that judge.json is gitignored — which is the whole point of writing it 0600, and the part a user storing a literal key most needs to know.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #127.
judge configurenever checked that it was inside a project, and the.gitignorethat shieldsjudge.jsonwas never back-filled into projects created before that entry existed.Case A, no project.
saveJudgeConfigwritesjudge.json.tmpwith flagwxstraight intogetProjectDir(). With no.nanotune/the write throwsENOENT, and because the save sat inside the connection-testtryblock it was caught by the connection-failure handler and shown asConnection test failed: ENOENT...after the API call had already succeeded. The user goes off debugging their network or their key, and the key they typed is gone.Case B, pre-1.4.0 project.
.nanotune/.gitignoreis written only byinitializeProjectDirs(), only fromnanotune init, and onlyif (!existsSync(gitignorePath)). A project keeps whatever.gitignoreit was initialised with forever, so one made with 1.3.x or earlier has nojudge.jsonline at all, leaving a file that may hold a literal API key sitting wheregit add .will take it. That defeats the point of the careful0600O_CREAT|O_EXCLwrite.Same back-fill also closes the loose end noted when #100 was closed: projects made with 1.4.0–1.6.x have a bare
judge.jsonline that does not cover thejudge.json.tmpan interrupted save leaves behind. Verified before/after withgit check-ignore.Changes
src/commands/judge.tsx, gate onconfigExists()before the first prompt, with the message every other command uses. Move the save out of the connection-testtryso a write failure reports itself as a write failure.src/lib/config.ts, new privatewriteProjectGitignore()merges missing entries into an existing.gitignoreinstead of skipping the write. Trim-compared so it is idempotent, trailing-newline aware so an appended block cannot run onto a hand-written last line, and append-only so it never removes or reorders what is already there.src/lib/judge.ts,saveJudgeConfigcallsinitializeProjectDirs()first, so writing the key is what repairs the file.Type of Change
Testing
Automated Tests
pnpm test:allcompletes successfully)Manual Testing
nanotune initnanotune datacommands (add/import/list/validate)nanotune trainnanotune exportnanotune benchmarkChecklist
pnpm format)