feat: add 'zen repo add' and first-use repo registration - #26
Conversation
Register a clone with zen in one shot: the short name comes from the clone's directory name, base_path from its parent directory, and full_name from the upstream remote (falling back to origin) so forks resolve to the repo the PRs live on. Adding the same repo twice is a no-op, and conflicting entries error instead of silently overwriting. zen work new and zen review now offer to register the surrounding clone on first use, prompting only on an interactive terminal. Config edits go through yaml.Node surgery so user comments and 2-space indentation in ~/.zen/config.yaml survive.
AddRepo previously truncated ~/.zen/config.yaml in place with a hardcoded 0644 mode, which could widen a user's 0600 config and leave a corrupt file behind an interrupted write. It now writes a temp file with the original permissions and renames it over the config. Also covers the remaining reachable paths: an empty 'repos:' key, an unparseable remote URL through Detect, a scheme-only remote URL, and identical re-adds with absolute base paths. internal/gitrepo is at 100% statement coverage.
406ab3d to
1f59486
Compare
|
Nice! |
mgreau
left a comment
mgreau
left a comment
There was a problem hiding this comment.
The YAML-preserving config edit and explicit duplicate checks are useful, and the full Go test suite and go vet passed locally. I reproduced one first-use registration bug that should be fixed before merging; details are inline.
The planned merge order is #23, #24, corrected #20, rebased #13, then this PR. Rebasing onto #20 requires retaining its ZEN_HOME-aware config directory while adding AddRepo. Keeping #13 before this PR ensures repositories inferred from upstream also fetch PR refs from the correct remote.
| fmt.Printf("%s is not registered with zen yet.\n", info.FullName) | ||
| fmt.Printf("Register it as %q (worktrees in %s)? [Y/n]: ", info.Short, ui.ShortenHome(info.BasePath, home)) | ||
| scanner := bufio.NewScanner(os.Stdin) | ||
| scanner.Scan() |
There was a problem hiding this comment.
[P2] Treat EOF as cancellation and use a real TTY check. stdinIsTerminal currently tests ModeCharDevice, which is also true for /dev/null. Then scanner.Scan() returns false at EOF, but its result is ignored and the empty answer is accepted as yes. I reproduced the built CLI modifying config.yaml when running zen work new example test --no-terminal </dev/null from an unregistered clone, without any answer. Please check whether stdin is actually a terminal and return false when Scan fails, rather than interpreting EOF as an empty submitted response. Cover /dev/null and EOF at an interactive prompt.
2 participants
Registering a repo with zen currently means the interactive
zen setupwalkthrough or hand-editing~/.zen/config.yaml. This addszen repo add— run from inside a clone, it infers the whole config entry — and makeszen work new/zen reviewoffer to register the surrounding clone on first use.What to review: the remote-URL inference in
internal/gitrepo/detect.go, and the YAML-preserving config edit inAddRepo(internal/config/config.go).Design and behavior
base_path/<short>, so this is the only correct choice — no--nameoverride),base_pathis the parent directory, andfull_namecomes from theupstreamremote when one exists, falling back toorigin, so forks resolve to the repo the PRs live on.zen work neworzen reviewinside an unregistered clone is the moment intent is observable — zen offers to register it there, prompting only when stdin is a TTY so scripted invocations never block. Agit cloneshell wrapper for clone-time registration is documented indocs/configuration.md.AddRepoedits the YAML document viayaml.Nodesurgery rather than a struct round-trip, so comments and 2-space indentation survive. The write goes through a temp file + rename with the config's original permissions, so an interrupted write can't truncate the file and a0600config stays0600. Re-adding an identical repo is a no-op; a short name or full name already configured with different values is an error, never a silent overwrite. Base paths under$HOMEare written in~/form. The daemon already hot-reloads config on every poll tick, so no restart is needed.zen repo addrefuses to run from a linked worktree (its directory name and parent would produce a bogus entry). Remote parsing handles scp-style,https://,ssh://, andgit://URLs.How it was verified
internal/gitrepo/detect_test.go); add, idempotent re-add in both~/absolute forms, both conflict shapes, missing and emptyrepos:sections, comment/~preservation, and file-mode preservation forAddRepo(internal/config/config_test.go).internal/gitrepoat 100% of statements;AddRepoand its helpers covered on every reachable path (remaining uncovered lines are filesystem/encoder fault branches).go vet, andgofmtpass. Thecmd/glue (prompt + hooks) follows this repo's pattern of keepingcmd/untested and logic ininternal/.$HOME: fresh add from a fork with both remotes, idempotent second run, and a hand-commented config file round-tripping with comments, inline comments, and indentation intact.