From 031410a97aea7212b0b662a3dcfa324f8e38d804 Mon Sep 17 00:00:00 2001 From: Hinne Stolzenberg Date: Fri, 8 May 2026 07:55:45 +0200 Subject: [PATCH] feat: per-person App default + shell-aware export snippet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two ergonomic changes to setup: 1. Default to one-App-per-person. The setup flow now points at https://github.com/settings/apps/new (personal account) and suggests `-claude` as the App name, so PR comments are attributed to e.g. `hinne-claude[bot]` per developer instead of a shared org bot. Org-owned Apps remain a documented alternative for teams that want a single shared identity. 2. Detect \$SHELL and emit the right rc-file path + export syntax: - bash → ~/.bashrc with `export FOO=bar` - zsh → ~/.zshrc with `export FOO=bar` - fish → ~/.config/fish/config.fish with `set -x FOO bar` and fish-style command substitution `(cmd)` instead of `\$(cmd)` - unknown → bash-style snippet with broader rc-file hint Also wraps command-substitution values in double quotes for bash/zsh so the substitution defers to read-time, not profile-load time. README updated to lead with the per-person pattern and document the trade-offs vs a shared org bot. --- README.md | 17 ++++++++- internal/cli/setup.go | 28 +++++++++----- internal/cli/shell.go | 77 ++++++++++++++++++++++++++++++++++++++ internal/cli/shell_test.go | 77 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 188 insertions(+), 11 deletions(-) create mode 100644 internal/cli/shell.go create mode 100644 internal/cli/shell_test.go diff --git a/README.md b/README.md index c627743..abd6a20 100644 --- a/README.md +++ b/README.md @@ -63,10 +63,25 @@ export GH_AS_BOT_PRIVATE_KEY="$(security find-generic-password -s gh-as-bot -w)" export GH_AS_BOT_PRIVATE_KEY="$(op read 'op://Private/gh-as-bot/private-key')" ``` +## One App per person (recommended) + +`gh as-bot` is designed for the **per-person** pattern: each developer creates their own GitHub App under their personal account, with a name like `-claude`. PR comments are then attributed to e.g. `hinne-claude[bot]` vs `marcus-claude[bot]`, so it's always clear whose Claude Code session produced a review. + +Why per-person: + +- **Attribution.** Reviews link back to the person whose context produced them — important when bot output is shaped by individual project memory and prompts. +- **Blast radius.** Revoking one developer's App or rotating their key affects only them. +- **Self-service.** No org admin coordination needed to onboard a new dev. +- **No shared private key.** Each dev controls and stashes their own `.pem`. + +Trade-off: N Apps to manage instead of one shared bot. For a small team this is barely visible; for a large team you may prefer a shared org-owned App, in which case use `https://github.com/organizations//settings/apps/new` instead — `gh as-bot` itself works identically either way. + ## GitHub App reference `gh as-bot setup` covers this interactively. For reference, the App needs: +- **Owner**: your personal account (per-person) or your org (shared bot). +- **"Where can this GitHub App be installed?"**: Any account — so personal Apps can still install on org repos. - **Repository permissions**: - `Pull requests: Read & write` — post reviews and review comments - `Contents: Read` — `gh` needs this for most read paths @@ -75,7 +90,7 @@ export GH_AS_BOT_PRIVATE_KEY="$(op read 'op://Private/gh-as-bot/private-key')" - **Private key**: generate from App settings → "Private keys". The `.pem` is shown once; setup helps you stash it. - **Install** the App on the repos that should accept bot reviews. -The bot identity that posts reviews will be `[bot]` (e.g. `negsoft-claude[bot]`). +The bot identity that posts reviews will be `[bot]` (e.g. `hinne-claude[bot]`). ## Why not just `gh auth switch`? diff --git a/internal/cli/setup.go b/internal/cli/setup.go index f83aace..26e5d9e 100644 --- a/internal/cli/setup.go +++ b/internal/cli/setup.go @@ -57,9 +57,15 @@ func (s *setup) run() int { // Step 1 _, _ = fmt.Fprintln(s.out, "") - _, _ = fmt.Fprintln(s.out, "Step 1/4 — Create the GitHub App") - _, _ = fmt.Fprintln(s.out, " Open this URL (replace YOUR-ORG):") - _, _ = fmt.Fprintln(s.out, " https://github.com/organizations/YOUR-ORG/settings/apps/new") + _, _ = fmt.Fprintln(s.out, "Step 1/4 — Create your GitHub App (one App per person, recommended)") + _, _ = fmt.Fprintln(s.out, " Per-person Apps give each developer their own bot identity") + _, _ = fmt.Fprintln(s.out, " (e.g. `-claude[bot]`), so PR comments are attributed") + _, _ = fmt.Fprintln(s.out, " to whoever's session produced them. No shared private keys.") + _, _ = fmt.Fprintln(s.out, "") + _, _ = fmt.Fprintln(s.out, " Open this URL (creates the App under YOUR account):") + _, _ = fmt.Fprintln(s.out, " https://github.com/settings/apps/new") + _, _ = fmt.Fprintln(s.out, "") + _, _ = fmt.Fprintln(s.out, " Suggested name: -claude") _, _ = fmt.Fprintln(s.out, "") _, _ = fmt.Fprintln(s.out, " Required Repository permissions:") _, _ = fmt.Fprintln(s.out, " - Pull requests: Read & write (post reviews / review comments)") @@ -67,6 +73,7 @@ func (s *setup) run() int { _, _ = fmt.Fprintln(s.out, " - Issues: Read & write (post issue / PR conversation comments)") _, _ = fmt.Fprintln(s.out, "") _, _ = fmt.Fprintln(s.out, " Webhook: uncheck \"Active\" — we don't use webhooks.") + _, _ = fmt.Fprintln(s.out, " \"Where can this GitHub App be installed?\": Any account.") _, _ = fmt.Fprintln(s.out, "") _, _ = fmt.Fprintln(s.out, " After saving the App:") _, _ = fmt.Fprintln(s.out, " a) Generate a private key (.pem) and download it.") @@ -74,6 +81,10 @@ func (s *setup) run() int { _, _ = fmt.Fprintln(s.out, " c) Note the App ID (App settings header) and installation ID") _, _ = fmt.Fprintln(s.out, " (visible in the URL after install: .../installations//...)") _, _ = fmt.Fprintln(s.out, "") + _, _ = fmt.Fprintln(s.out, " (Want a shared org-owned bot instead? Use") + _, _ = fmt.Fprintln(s.out, " https://github.com/organizations//settings/apps/new — but") + _, _ = fmt.Fprintln(s.out, " you lose per-person attribution and share one private key.)") + _, _ = fmt.Fprintln(s.out, "") appID, err := s.requirePrompt("App ID (numeric)") if err != nil { @@ -139,14 +150,11 @@ func (s *setup) run() int { // Step 4 _, _ = fmt.Fprintln(s.out, "") _, _ = fmt.Fprintln(s.out, "Step 4/4 — Shell configuration") - _, _ = fmt.Fprintln(s.out, " Add this to your shell profile (~/.zshrc or ~/.bashrc):") + shell := detectShell() + _, _ = fmt.Fprintf(s.out, " Add this to your shell profile (%s):\n", shell.rcDescription) _, _ = fmt.Fprintln(s.out, "") - _, _ = fmt.Fprintf(s.out, " export GH_AS_BOT_APP_ID=%s\n", appID) - _, _ = fmt.Fprintf(s.out, " export GH_AS_BOT_INSTALLATION_ID=%s\n", instID) - if strings.HasPrefix(keyRef, "$(") { - _, _ = fmt.Fprintf(s.out, " export GH_AS_BOT_PRIVATE_KEY=\"%s\"\n", keyRef) - } else { - _, _ = fmt.Fprintf(s.out, " export GH_AS_BOT_PRIVATE_KEY=%s\n", keyRef) + for _, line := range shell.exportLines(appID, instID, keyRef) { + _, _ = fmt.Fprintf(s.out, " %s\n", line) } _, _ = fmt.Fprintln(s.out, "") _, _ = fmt.Fprintln(s.out, " Open a fresh shell (or `source` your profile), then verify:") diff --git a/internal/cli/shell.go b/internal/cli/shell.go new file mode 100644 index 0000000..8a42dc7 --- /dev/null +++ b/internal/cli/shell.go @@ -0,0 +1,77 @@ +package cli + +import ( + "fmt" + "os" + "path/filepath" + "strings" +) + +// shellProfile describes how to render the env-export snippet for a +// detected shell. Bash and zsh share the same `export` syntax; fish uses +// `set -x`. Unknown shells fall back to the bash/zsh snippet with a +// generic rc-file description, which is correct for ~99% of dev setups. +type shellProfile struct { + // rcDescription is the human-readable file path shown in the prompt + // (e.g. "~/.bashrc"). For unknown shells we widen this to a list. + rcDescription string + + // quotedAssign is the function that renders `KEY=value` (or its + // equivalent) for this shell, given a value that may contain a + // command substitution. + render func(key, value string, isCommandSub bool) string +} + +func (p shellProfile) exportLines(appID, instID, keyRef string) []string { + keyIsSub := strings.HasPrefix(keyRef, "$(") + return []string{ + p.render("GH_AS_BOT_APP_ID", appID, false), + p.render("GH_AS_BOT_INSTALLATION_ID", instID, false), + p.render("GH_AS_BOT_PRIVATE_KEY", keyRef, keyIsSub), + } +} + +// detectShell inspects $SHELL and returns a profile describing how to +// emit the export snippet. We deliberately keep the matching loose +// (basename suffix only) — exotic shell paths (/opt/homebrew/bin/zsh, +// custom builds, etc.) all resolve correctly that way. +func detectShell() shellProfile { + switch shellBasename() { + case "zsh": + return shellProfile{rcDescription: "~/.zshrc", render: bashStyleExport} + case "bash": + return shellProfile{rcDescription: "~/.bashrc", render: bashStyleExport} + case "fish": + return shellProfile{rcDescription: "~/.config/fish/config.fish", render: fishStyleExport} + default: + return shellProfile{rcDescription: "~/.zshrc, ~/.bashrc, or equivalent", render: bashStyleExport} + } +} + +func shellBasename() string { + sh := os.Getenv("SHELL") + if sh == "" { + return "" + } + return filepath.Base(sh) +} + +func bashStyleExport(key, value string, isCommandSub bool) string { + if isCommandSub { + // Quote command substitutions so the shell defers evaluation + // until the variable is read, not at profile load. + return fmt.Sprintf(`export %s="%s"`, key, value) + } + return fmt.Sprintf("export %s=%s", key, value) +} + +func fishStyleExport(key, value string, isCommandSub bool) string { + // fish uses `set -x` for exported vars. Command substitution in + // fish is `(cmd)` not `$(cmd)`, so we rewrite if we see one. + if isCommandSub { + v := strings.TrimPrefix(value, "$(") + v = strings.TrimSuffix(v, ")") + return fmt.Sprintf("set -x %s (%s)", key, v) + } + return fmt.Sprintf("set -x %s %s", key, value) +} diff --git a/internal/cli/shell_test.go b/internal/cli/shell_test.go new file mode 100644 index 0000000..ec9c3cf --- /dev/null +++ b/internal/cli/shell_test.go @@ -0,0 +1,77 @@ +package cli + +import ( + "strings" + "testing" +) + +func TestDetectShell_Bash(t *testing.T) { + t.Setenv("SHELL", "/bin/bash") + p := detectShell() + if p.rcDescription != "~/.bashrc" { + t.Errorf("rcDescription = %q, want ~/.bashrc", p.rcDescription) + } + lines := p.exportLines("1", "2", "/path/to/key.pem") + if !strings.HasPrefix(lines[0], "export ") { + t.Errorf("bash should emit `export`; got %q", lines[0]) + } +} + +func TestDetectShell_Zsh(t *testing.T) { + t.Setenv("SHELL", "/usr/bin/zsh") + p := detectShell() + if p.rcDescription != "~/.zshrc" { + t.Errorf("rcDescription = %q, want ~/.zshrc", p.rcDescription) + } +} + +func TestDetectShell_Fish(t *testing.T) { + t.Setenv("SHELL", "/opt/homebrew/bin/fish") + p := detectShell() + if p.rcDescription != "~/.config/fish/config.fish" { + t.Errorf("rcDescription = %q", p.rcDescription) + } + lines := p.exportLines("1", "2", "$(security find-generic-password -s gh-as-bot -w)") + keyLine := lines[2] + if !strings.HasPrefix(keyLine, "set -x ") { + t.Errorf("fish should emit `set -x`; got %q", keyLine) + } + if strings.Contains(keyLine, "$(") { + t.Errorf("fish command substitution should be (...) not $(...); got %q", keyLine) + } + if !strings.Contains(keyLine, "(security find-generic-password -s gh-as-bot -w)") { + t.Errorf("fish should keep the substitution body; got %q", keyLine) + } +} + +func TestDetectShell_UnknownFallsBackToBashStyle(t *testing.T) { + t.Setenv("SHELL", "/usr/local/bin/elvish") + p := detectShell() + if !strings.Contains(p.rcDescription, "~/.zshrc") || !strings.Contains(p.rcDescription, "~/.bashrc") { + t.Errorf("unknown shell should mention both rc files; got %q", p.rcDescription) + } + lines := p.exportLines("1", "2", "/path/key.pem") + if !strings.HasPrefix(lines[0], "export ") { + t.Errorf("fallback should use bash-style export; got %q", lines[0]) + } +} + +func TestExportLines_QuotesCommandSubstitution(t *testing.T) { + t.Setenv("SHELL", "/bin/bash") + p := detectShell() + lines := p.exportLines("1", "2", "$(security find-generic-password -s gh-as-bot -w)") + keyLine := lines[2] + if !strings.Contains(keyLine, `"$(security`) { + t.Errorf("bash command sub should be wrapped in double quotes; got %q", keyLine) + } +} + +func TestExportLines_LiteralPathUnquoted(t *testing.T) { + t.Setenv("SHELL", "/bin/bash") + p := detectShell() + lines := p.exportLines("1", "2", "/abs/path/key.pem") + keyLine := lines[2] + if strings.Contains(keyLine, `"`) { + t.Errorf("literal path should not be quoted; got %q", keyLine) + } +}