diff --git a/README.md b/README.md index 7d48cf8..030576c 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,8 @@ Or if you already have `curl` you can run the following script to detect OS and $ curl -fsSL https://commit.jaw.dev/install.sh | bash ``` -On the first run, Commit asks only for your OpenRouter API key. It then +On the first run, Commit asks for your OpenRouter API key and preferred model. It +uses `openrouter/free` when you press Enter at the model prompt, then creates `~/.config/commit/config.json` with private permissions automatically: ```bash @@ -46,7 +47,8 @@ The generated configuration looks like this: ```json { - "api_key": "YOUR_OPENROUTER_API_KEY" + "api_key": "YOUR_OPENROUTER_API_KEY", + "model": "openrouter/free" } ``` @@ -57,8 +59,9 @@ again at any time: $ curl -fsSL https://commit.jaw.dev/ | bash -s -- --setup ``` -Set `OPENROUTER_API_KEY` to avoid saving a key locally. Advanced users can set -`COMMIT_MODEL` to override the default model. Browse valid model IDs at +Set `OPENROUTER_API_KEY` to avoid saving a key locally. `--model` overrides +`COMMIT_MODEL`, which overrides the model saved in the configuration. Browse +valid model IDs at [openrouter.ai/models](https://openrouter.ai/models), or list them from the API: ```bash @@ -77,7 +80,7 @@ $ curl -fsSL https://commit.jaw.dev/ | bash - `--dry-run` Run the script without making any changes - `-y`, `--yes` Accept the generated message without confirmation - `-v`, `--verbose` Enable verbose logging -- `--setup` Create or update the saved configuration +- `--setup` Configure the saved API key and model - `-h`, `--help` Display this help message ### Example Commands @@ -96,8 +99,8 @@ The configuration path follows `$XDG_CONFIG_HOME` when set and defaults to default model is `openrouter/free`, which randomly selects an available free model. Free models have lower rate limits and may be less consistent. Pass a model ID exactly as OpenRouter displays it, for example `openrouter/auto`, to -override the default. Diffs larger than 1 MiB are rejected before an API request -is made. +override the default. Model IDs must not contain whitespace. Diffs larger than +1 MiB are rejected before an API request is made. # Docs diff --git a/assets/sh/commit.sh b/assets/sh/commit.sh index c19b738..402ac78 100755 --- a/assets/sh/commit.sh +++ b/assets/sh/commit.sh @@ -13,6 +13,7 @@ API_KEY="" API_URL="https://openrouter.ai/api/v1/chat/completions" AI_MODEL="" CONFIG_API_KEY="" +CONFIG_MODEL="" AUTH_HEADER_FILE="" MAX_DIFF_BYTES=1048576 CONFIG_DIR_MANAGED=true @@ -112,6 +113,11 @@ format_changed_files() { }' } +is_valid_model() { + local candidate="$1" + [ -n "$candidate" ] && [[ "$candidate" != *[[:space:]]* ]] +} + show_help() { local status="${1:-0}" log_verbose "Displaying help message" @@ -122,7 +128,7 @@ show_help() { printf " ${GREEN}%-22s${NC} %s\n" "-y, --yes" "Accept the generated message without confirmation" printf " ${GREEN}%-22s${NC} %s\n" "-m, --model" "Override the OpenRouter model" printf " ${GREEN}%-22s${NC} %s\n" "-v, --verbose" "Enable verbose logging" - printf " ${GREEN}%-22s${NC} %s\n" "--setup" "Create or update the saved configuration" + printf " ${GREEN}%-22s${NC} %s\n" "--setup" "Configure the saved API key and model" printf " ${GREEN}%-22s${NC} %s\n" "-h, --help" "Display this help message" printf "\n" printf "${YELLOW}Configuration:${NC}\n" @@ -169,12 +175,20 @@ load_config() { exit 1 fi + if ! jq -e '(.model == null) or ((.model | type) == "string" and (.model | length) > 0 and (.model | test("\\s") | not))' "$CONFIG_FILE" >/dev/null 2>&1; then + printf "${RED}Invalid model in %s. Use a non-empty model ID without whitespace.${NC}\n" "$CONFIG_FILE" + exit 1 + fi + CONFIG_API_KEY=$(jq -r '.api_key // empty' "$CONFIG_FILE") + CONFIG_MODEL=$(jq -r '.model // empty' "$CONFIG_FILE") } setup_config() { local api_key local existing_api_key="$CONFIG_API_KEY" + local existing_model="${CONFIG_MODEL:-openrouter/free}" + local model local config_dir local config_dir_existed=false local temp_file @@ -204,6 +218,22 @@ setup_config() { CONFIG_API_KEY="$api_key" API_KEY="$api_key" + + while true; do + printf "Model [%s]: " "$existing_model" >> "$TTY_OUTPUT" + if ! read -r model <&3; then + exec 3<&- + return 1 + fi + if [ -z "$model" ]; then + model="$existing_model" + fi + if is_valid_model "$model"; then + break + fi + printf "${RED}Enter a non-empty model ID without whitespace.${NC}\n" >> "$TTY_OUTPUT" + done + CONFIG_MODEL="$model" exec 3<&- config_dir=$(dirname "$CONFIG_FILE") @@ -218,9 +248,11 @@ setup_config() { temp_file=$(mktemp "$CONFIG_FILE.tmp.XXXXXX") || return 1 if ! jq -n \ - --arg api_key "$CONFIG_API_KEY" ' + --arg api_key "$CONFIG_API_KEY" \ + --arg model "$CONFIG_MODEL" ' { - api_key: $api_key + api_key: $api_key, + model: $model } | with_entries(select(.value != ""))' > "$temp_file"; then rm -f "$temp_file" return 1 @@ -238,7 +270,11 @@ setup_config() { } configure_openrouter() { - AI_MODEL="${AI_MODEL:-${COMMIT_MODEL:-openrouter/free}}" + AI_MODEL="${AI_MODEL:-${COMMIT_MODEL:-${CONFIG_MODEL:-openrouter/free}}}" + if ! is_valid_model "$AI_MODEL"; then + printf "${RED}Invalid OpenRouter model. Use a non-empty model ID without whitespace.${NC}\n" + exit 1 + fi if [ -z "$API_KEY" ]; then API_KEY="${OPENROUTER_API_KEY:-$CONFIG_API_KEY}" fi diff --git a/assets/templates/index.html b/assets/templates/index.html index 72632c4..64c9cf2 100644 --- a/assets/templates/index.html +++ b/assets/templates/index.html @@ -7,7 +7,10 @@

🤖 Commit

Configure

-

The first run asks only for your OpenRouter API key, then securely saves it.

+

+ The first run asks for your OpenRouter API key and preferred model, + then securely saves both. Press Enter to use openrouter/free. +

$ curl -fsSL {{.Domain}} | bash
 $ curl -fsSL {{.Domain}} | bash -s -- --setup
@@ -55,7 +58,7 @@

Options

Show command help.
--setup
-
Create or update the saved configuration.
+
Configure the saved OpenRouter API key and model.
diff --git a/cmd/script_test.go b/cmd/script_test.go index 3abe5b2..b2973bd 100644 --- a/cmd/script_test.go +++ b/cmd/script_test.go @@ -197,7 +197,7 @@ func TestCommitScriptRunsFirstSetupAndCallsOpenRouter(t *testing.T) { configPath := filepath.Join(configDir, "config.json") setupInputPath := filepath.Join(root, "setup-input") setupOutputPath := filepath.Join(root, "setup-output") - if err := os.WriteFile(setupInputPath, []byte("openrouter-secret\n"), 0o600); err != nil { + if err := os.WriteFile(setupInputPath, []byte("openrouter-secret\n\n"), 0o600); err != nil { t.Fatal(err) } @@ -260,7 +260,7 @@ printf '{"choices":[{"message":{"content":"feat: test openrouter"}}]}\n200' if err := json.Unmarshal(configData, &config); err != nil { t.Fatal(err) } - if config["api_key"] != "openrouter-secret" || len(config) != 1 { + if config["api_key"] != "openrouter-secret" || config["model"] != "openrouter/free" || len(config) != 2 { t.Errorf("unexpected generated config: %#v", config) } configInfo, err := os.Stat(configPath) @@ -338,6 +338,86 @@ printf '{"choices":[{"message":{"content":"feat: test openrouter"}}]}\n200' } } +func TestCommitScriptModelPrecedence(t *testing.T) { + script, err := assets.Embeddedfiles.ReadFile("sh/commit.sh") + if err != nil { + t.Fatal(err) + } + + root := t.TempDir() + repo := filepath.Join(root, "repo") + configDir := filepath.Join(root, "config", "commit") + binDir := filepath.Join(root, "bin") + for _, dir := range []string{repo, configDir, binDir} { + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + } + config := `{"api_key":"test-key","model":"saved/model"}` + if err := os.WriteFile(filepath.Join(configDir, "config.json"), []byte(config), 0o600); err != nil { + t.Fatal(err) + } + requestPath := filepath.Join(root, "request.json") + fakeCurl := `#!/bin/bash +cat > "$CAPTURE_REQUEST" +printf '{"choices":[{"message":{"content":"test: verify model precedence"}}]}\n200' +` + if err := os.WriteFile(filepath.Join(binDir, "curl"), []byte(fakeCurl), 0o755); err != nil { + t.Fatal(err) + } + runGit(t, repo, "init", "-q") + if err := os.WriteFile(filepath.Join(repo, "feature.txt"), []byte("model test\n"), 0o600); err != nil { + t.Fatal(err) + } + runGit(t, repo, "add", "feature.txt") + + tests := []struct { + name string + envModel string + args []string + want string + }{ + {name: "saved config", want: "saved/model"}, + {name: "environment", envModel: "environment/model", want: "environment/model"}, + {name: "flag", envModel: "environment/model", args: []string{"--model", "flag/model"}, want: "flag/model"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + args := []string{"-s", "--", "--dry-run"} + args = append(args, tt.args...) + cmd := exec.Command("bash", args...) + cmd.Dir = repo + cmd.Stdin = bytes.NewReader(script) + cmd.Env = append(os.Environ(), + "PATH="+binDir+":"+os.Getenv("PATH"), + "XDG_CONFIG_HOME="+filepath.Join(root, "config"), + "OPENROUTER_API_KEY=", + "COMMIT_MODEL="+tt.envModel, + "TMPDIR="+root, + "CAPTURE_REQUEST="+requestPath, + ) + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("commit script failed: %v\n%s", err, output) + } + + requestData, err := os.ReadFile(requestPath) + if err != nil { + t.Fatal(err) + } + var request struct { + Model string `json:"model"` + } + if err := json.Unmarshal(requestData, &request); err != nil { + t.Fatal(err) + } + if request.Model != tt.want { + t.Errorf("model = %q, want %q", request.Model, tt.want) + } + }) + } +} + func TestCommitScriptRejectsLooseConfigPermissions(t *testing.T) { script, err := assets.Embeddedfiles.ReadFile("sh/commit.sh") if err != nil { @@ -369,7 +449,48 @@ func TestCommitScriptRejectsLooseConfigPermissions(t *testing.T) { } } -func TestCommitScriptSetupKeepsExistingKey(t *testing.T) { +func TestCommitScriptRejectsInvalidSavedModels(t *testing.T) { + script, err := assets.Embeddedfiles.ReadFile("sh/commit.sh") + if err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + model string + }{ + {name: "empty", model: `""`}, + {name: "whitespace", model: `"bad model"`}, + {name: "non-string", model: `{}`}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + configRoot := t.TempDir() + configDir := filepath.Join(configRoot, "commit") + if err := os.MkdirAll(configDir, 0o700); err != nil { + t.Fatal(err) + } + config := `{"api_key":"test-key","model":` + tt.model + `}` + if err := os.WriteFile(filepath.Join(configDir, "config.json"), []byte(config), 0o600); err != nil { + t.Fatal(err) + } + + cmd := exec.Command("bash", "-s", "--", "--dry-run") + cmd.Stdin = bytes.NewReader(script) + cmd.Env = append(os.Environ(), "XDG_CONFIG_HOME="+configRoot) + output, err := cmd.CombinedOutput() + if err == nil { + t.Fatal("script accepted an invalid saved model") + } + if !strings.Contains(string(output), "Invalid model") { + t.Fatalf("unexpected output:\n%s", output) + } + }) + } +} + +func TestCommitScriptSetupKeepsExistingKeyAndChangesModel(t *testing.T) { script, err := assets.Embeddedfiles.ReadFile("sh/commit.sh") if err != nil { t.Fatal(err) @@ -381,13 +502,13 @@ func TestCommitScriptSetupKeepsExistingKey(t *testing.T) { t.Fatal(err) } configPath := filepath.Join(configDir, "config.json") - initialConfig := `{"api_key":"saved-key"}` + initialConfig := `{"api_key":"saved-key","model":"openrouter/auto"}` if err := os.WriteFile(configPath, []byte(initialConfig), 0o600); err != nil { t.Fatal(err) } inputPath := filepath.Join(root, "setup-input") outputPath := filepath.Join(root, "setup-output") - if err := os.WriteFile(inputPath, []byte("\n"), 0o600); err != nil { + if err := os.WriteFile(inputPath, []byte("\nbad model\ncustom/model\n"), 0o600); err != nil { t.Fatal(err) } @@ -402,6 +523,13 @@ func TestCommitScriptSetupKeepsExistingKey(t *testing.T) { if output, err := cmd.CombinedOutput(); err != nil { t.Fatalf("setup failed: %v\n%s", err, output) } + setupOutput, err := os.ReadFile(outputPath) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(setupOutput), "without whitespace") { + t.Fatalf("setup did not reject the invalid model:\n%s", setupOutput) + } configData, err := os.ReadFile(configPath) if err != nil { @@ -411,7 +539,7 @@ func TestCommitScriptSetupKeepsExistingKey(t *testing.T) { if err := json.Unmarshal(configData, &config); err != nil { t.Fatal(err) } - if config["api_key"] != "saved-key" || len(config) != 1 { + if config["api_key"] != "saved-key" || config["model"] != "custom/model" || len(config) != 2 { t.Errorf("unexpected updated config: %#v", config) } } @@ -472,7 +600,7 @@ func TestCommitScriptDoesNotChangeExistingConfigDirectoryMode(t *testing.T) { } inputPath := filepath.Join(root, "setup-input") outputPath := filepath.Join(root, "setup-output") - if err := os.WriteFile(inputPath, []byte("test-key\n"), 0o600); err != nil { + if err := os.WriteFile(inputPath, []byte("test-key\n\n"), 0o600); err != nil { t.Fatal(err) } @@ -513,7 +641,7 @@ func TestCommitScriptRemovesConfigTempFileWhenMoveFails(t *testing.T) { } inputPath := filepath.Join(root, "setup-input") outputPath := filepath.Join(root, "setup-output") - if err := os.WriteFile(inputPath, []byte("test-key\n"), 0o600); err != nil { + if err := os.WriteFile(inputPath, []byte("test-key\n\n"), 0o600); err != nil { t.Fatal(err) } diff --git a/docs/manual-qa.md b/docs/manual-qa.md index 7e32c2b..601d541 100644 --- a/docs/manual-qa.md +++ b/docs/manual-qa.md @@ -43,7 +43,7 @@ Create or replace the configuration without putting the key in shell history: ```bash $ ./assets/sh/commit.sh --setup $ stat -c '%a' ~/.config/commit ~/.config/commit/config.json -$ jq -e 'keys == ["api_key"] and (.api_key | type == "string" and length > 0)' ~/.config/commit/config.json >/dev/null +$ jq -e 'keys == ["api_key", "model"] and (.api_key | type == "string" and length > 0) and (.model | type == "string" and length > 0)' ~/.config/commit/config.json >/dev/null ``` On macOS, use `stat -f '%Lp'` for the permission checks. Expected directory and @@ -77,7 +77,7 @@ Expected: - OpenRouter returns a one-line Conventional Commit message. - The output lists `qa.txt` and does not create a commit. -- The default request uses `openrouter/free`. +- Pressing Enter during setup saves `openrouter/free`, and the request uses it. - The key and complete request body are never printed without `--verbose`. Repeat once with an advanced model override: @@ -119,6 +119,8 @@ Verify these cases without using a real repository: - `--yes` is the explicit exception and accepts without confirmation. - Git commit hooks are always skipped by the Commit client. - Missing configuration starts setup instead of sending a request. +- Model precedence is `--model`, `COMMIT_MODEL`, saved config, then `openrouter/free`. +- Empty, whitespace-containing, and non-string saved models are rejected. - Diffs larger than 1 MiB are rejected before any API request. - Invalid JSON is rejected before any request. - Config mode `644` is rejected with the `chmod 600` instruction.