Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
Adds a new “hooks-first” onboarding flow centered around codemap setup, aiming to make initial project configuration + Claude hook installation automated and idempotent.
Changes:
- Introduces
codemap setupto create.codemap/config.json(if missing) and install codemap Claude hooks into project-local or global Claude settings. - Refactors config initialization internals for reuse (shared
initProjectConfigresult/error handling) and adds tests for settings merge/dedup behavior. - Updates docs/README and adds helper onboarding scripts for macOS/Linux and Windows.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/onboard.sh | Bash helper to install codemap (via Homebrew) and run codemap setup for a target project. |
| scripts/onboard.ps1 | PowerShell helper to install codemap (Scoop/Winget) and run codemap setup for a target project. |
| main.go | Adds early handling for the new setup subcommand and updates --help output. |
| docs/HOOKS.md | Updates hook docs to emphasize codemap setup as the recommended path and refreshes install commands. |
| cmd/setup.go | Implements RunSetup + Claude settings merge/dedup logic for hook installation. |
| cmd/setup_test.go | Adds tests for creating settings, preserving fields, avoiding duplicates, and rejecting invalid JSON. |
| cmd/config.go | Refactors config init into reusable initProjectConfig for use by setup. |
| README.md | Reworks onboarding docs to make codemap setup the primary recommended setup flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cmd/setup.go
Outdated
| hooksRaw, err := json.Marshal(hooksByEvent) | ||
| if err != nil { | ||
| return result, fmt.Errorf("encode hooks: %w", err) | ||
| } | ||
| root["hooks"] = hooksRaw |
There was a problem hiding this comment.
root is a map[string]json.RawMessage and you assign root["hooks"] = hooksRaw where hooksRaw was produced by json.Marshal(...). When you later json.MarshalIndent(root, ...), json.RawMessage is emitted verbatim, so the hooks object will stay compact/minified (one long line) even though the rest of the file is indented. If the goal is a readable settings file, consider representing the top-level settings as map[string]any (or a struct) and assigning the structured hooksByEvent value so MarshalIndent can format it normally.
| if err := os.WriteFile(settingsPath, out, 0644); err != nil { | ||
| return result, fmt.Errorf("write %s: %w", settingsPath, err) | ||
| } |
There was a problem hiding this comment.
ensureClaudeHooks writes settingsPath unconditionally, even when AddedHooks == 0 (i.e., already configured). That makes the operation non-no-op (touches mtime / may reorder JSON keys) despite printing "already configured". Consider short-circuiting before marshalling/writing when no changes are needed (while still creating the file when it doesn't exist).
cmd/setup.go
Outdated
| target := strings.TrimSpace(command) | ||
| for _, entry := range entries { |
There was a problem hiding this comment.
hasHookCommand dedupes purely on (type==command && command string), ignoring the hook entry's matcher. For the recommended PreToolUse/PostToolUse hooks the matcher (Edit|Write) is part of the behavior; if a user already has codemap hook pre-edit without the matcher (or with a different matcher), ensureClaudeHooks will treat it as present and won't install the recommended matcher-scoped hook. Consider including matcher in the match criteria for specs that require it, or updating existing entries to add/fix the matcher when the command matches.
| target := strings.TrimSpace(command) | |
| for _, entry := range entries { | |
| target := strings.TrimSpace(command) | |
| // Some recommended hooks (e.g. PreToolUse/PostToolUse) require a specific matcher | |
| // as part of their intended behavior. For those commands, only treat an existing | |
| // hook as present if both the command and the entry's matcher match the | |
| // recommended values. For all other commands, continue to ignore the matcher. | |
| requiredMatcher := "" | |
| switch target { | |
| case "codemap hook pre-edit", "codemap hook post-edit": | |
| requiredMatcher = "Edit|Write" | |
| } | |
| for _, entry := range entries { | |
| if requiredMatcher != "" && !strings.EqualFold(strings.TrimSpace(entry.Matcher), requiredMatcher) { | |
| // This entry doesn't have the required matcher; don't let it | |
| // suppress installation of the matcher-scoped recommended hook. | |
| continue | |
| } |
cmd/setup.go
Outdated
| fs := flag.NewFlagSet("setup", flag.ExitOnError) | ||
| useGlobalHooks := fs.Bool("global", false, "Install hooks into ~/.claude/settings.json instead of project-local .claude/settings.local.json") | ||
| skipConfig := fs.Bool("no-config", false, "Skip creating .codemap/config.json") | ||
| skipHooks := fs.Bool("no-hooks", false, "Skip writing Claude hook settings") | ||
| if err := fs.Parse(args); err != nil { | ||
| fmt.Fprintf(os.Stderr, "Error: %v\n", err) | ||
| os.Exit(1) |
There was a problem hiding this comment.
RunSetup uses flag.NewFlagSet("setup", flag.ExitOnError). With ExitOnError, fs.Parse calls os.Exit(2) on parse errors/help and never returns a non-nil error, so the if err := fs.Parse(args); err != nil { ... } block is effectively dead code and you lose control over exit codes/usage text. Consider switching to flag.ContinueOnError and handling -h/-help + parse errors explicitly (or remove the manual error check and rely on the default behavior).
| fs := flag.NewFlagSet("setup", flag.ExitOnError) | |
| useGlobalHooks := fs.Bool("global", false, "Install hooks into ~/.claude/settings.json instead of project-local .claude/settings.local.json") | |
| skipConfig := fs.Bool("no-config", false, "Skip creating .codemap/config.json") | |
| skipHooks := fs.Bool("no-hooks", false, "Skip writing Claude hook settings") | |
| if err := fs.Parse(args); err != nil { | |
| fmt.Fprintf(os.Stderr, "Error: %v\n", err) | |
| os.Exit(1) | |
| fs := flag.NewFlagSet("setup", flag.ContinueOnError) | |
| fs.SetOutput(os.Stderr) | |
| useGlobalHooks := fs.Bool("global", false, "Install hooks into ~/.claude/settings.json instead of project-local .claude/settings.local.json") | |
| skipConfig := fs.Bool("no-config", false, "Skip creating .codemap/config.json") | |
| skipHooks := fs.Bool("no-hooks", false, "Skip writing Claude hook settings") | |
| if err := fs.Parse(args); err != nil { | |
| if errors.Is(err, flag.ErrHelp) { | |
| fmt.Fprintln(os.Stderr, "Usage: codemap setup [--global] [--no-config] [--no-hooks] [path]") | |
| } else { | |
| fmt.Fprintf(os.Stderr, "Error: %v\n", err) | |
| } | |
| os.Exit(2) |
Summary
codemap setupas the single recommended onboarding path.codemap/config.json(if missing) and install codemap hooks into Claude settingscodemap setup(no repo clone required)scripts/onboard.sh,scripts/onboard.ps1)Validation