From b189f013106add26f6d0bd6da8d12b29b9bd74e9 Mon Sep 17 00:00:00 2001 From: iyaki Date: Wed, 17 Jun 2026 01:18:25 +0000 Subject: [PATCH 1/3] init: Remove agent-mode and log-file defaults from questionnaire The ralph init command now always shows NO default for agent-mode and log-file fields in the interactive questionnaire, even when an existing config file contains these values. This ensures users explicitly choose whether to set these optional fields on each run. Changes: - seedInitStringDefaults(): Intentionally omit AgentMode and LogFile from seeding while keeping other string fields (Model, SpecsDir, etc.) - Tests updated to reflect new behavior where these fields remain empty Other fields continue to preserve their existing config values as defaults. Logging questions (truncate?) only appear when user provides non-empty log path. --- internal/agent/agent.go | 4 ++- internal/cli/init.go | 10 ++++---- internal/cli/init_helpers_test.go | 10 ++++---- internal/cli/init_internal_test.go | 30 ++++++++++++++++------ internal/config/config.go | 18 ++++++------- internal/config/writer_test.go | 41 ++++++++++++++++++++++++++++++ ralph.toml | 16 +++--------- specs/agents.md | 1 + specs/init-command.md | 11 ++++---- 9 files changed, 96 insertions(+), 45 deletions(-) diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 620259a..a5fcbf7 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -25,6 +25,8 @@ func GetAgent(agentName, model, agentMode string, env []string) (Agent, error) { switch agentName { case "omp": return &OmpAgent{Model: model, AgentMode: agentMode, Env: effectiveEnv}, nil + case "oh-my-pi": + return &OmpAgent{Model: model, AgentMode: agentMode, Env: effectiveEnv}, nil case "claude": return &ClaudeAgent{Model: model, AgentMode: agentMode, Env: effectiveEnv}, nil case "cursor": @@ -32,6 +34,6 @@ func GetAgent(agentName, model, agentMode string, env []string) (Agent, error) { case "opencode": return &OpencodeAgent{Model: model, AgentMode: agentMode, Env: effectiveEnv}, nil default: - return nil, fmt.Errorf("unknown agent %q (supported: omp, opencode, claude, cursor)", agentName) + return nil, fmt.Errorf("unknown agent %q (supported: omp, oh-my-pi, opencode, claude, cursor)", agentName) } } diff --git a/internal/cli/init.go b/internal/cli/init.go index de8b4c7..1ff4fae 100644 --- a/internal/cli/init.go +++ b/internal/cli/init.go @@ -70,7 +70,7 @@ const ( questionKeyWriteConfiguration = "write-configuration" ) -var supportedInitAgents = []string{"omp", "opencode", "claude", "cursor"} +var supportedInitAgents = []string{"omp", "opencode", "claude", "cursor", "oh-my-pi"} var errInvalidConfirmAnswer = errors.New("please answer yes or no") @@ -336,12 +336,12 @@ func seedInitStringDefaults(answers *InitAnswers, existingConfig *config.Config) apply func(string) }{ {existingConfig.Model, func(value string) { answers.Model = value }}, - {existingConfig.AgentMode, func(value string) { answers.AgentMode = value }}, + // AgentMode intentionally omitted - always shows no default {existingConfig.SpecsDir, func(value string) { answers.SpecsDir = value }}, {existingConfig.SpecsIndexFile, func(value string) { answers.SpecsIndexFile = value }}, {existingConfig.ImplementationPlanName, func(value string) { answers.ImplementationPlanName = value }}, {existingConfig.PromptsDir, func(value string) { answers.PromptsDir = value }}, - {existingConfig.LogFile, func(value string) { answers.LogFile = value }}, + // LogFile intentionally omitted - always shows no default } { if strings.TrimSpace(field.value) == "" { continue @@ -509,7 +509,7 @@ func baseInitQuestions(defaults *InitAnswers) []InitQuestion { return []InitQuestion{ newSelectQuestion( questionKeyAgentName, - "AI agent (opencode/claude/cursor)", + "AI agent (omp/opencode/claude/cursor/oh-my-pi)", defaults.AgentName, supportedInitAgents, validateInitAgent, @@ -533,7 +533,7 @@ func baseInitQuestions(defaults *InitAnswers) []InitQuestion { nil, ), newInputQuestion(questionKeyPromptsDir, "Prompts directory", defaults.PromptsDir, true, nil), - newInputQuestion(questionKeyLogFile, "Log file path (optional)", defaults.LogFile, false, nil), + newInputQuestion(questionKeyLogFile, "Log file path (leave empty to disable logging)", defaults.LogFile, false, nil), } } diff --git a/internal/cli/init_helpers_test.go b/internal/cli/init_helpers_test.go index e295bcd..35d57f5 100644 --- a/internal/cli/init_helpers_test.go +++ b/internal/cli/init_helpers_test.go @@ -202,7 +202,7 @@ func TestSeedInitMaxIterationsDefault(t *testing.T) { } func TestSeedInitStringDefaults(t *testing.T) { - t.Run("populate existing config fields", func(t *testing.T) { + t.Run("populate existing config fields except AgentMode and LogFile", func(t *testing.T) { var answers InitAnswers existingConfig := &config.Config{ Model: "gpt-4", @@ -219,8 +219,8 @@ func TestSeedInitStringDefaults(t *testing.T) { if answers.Model != "gpt-4" { t.Errorf("expected Model \"gpt-4\", got %q", answers.Model) } - if answers.AgentMode != "agent" { - t.Errorf("expected AgentMode \"agent\", got %q", answers.AgentMode) + if answers.AgentMode != "" { + t.Errorf("expected AgentMode \"\" (omitted), got %q", answers.AgentMode) } if answers.SpecsDir != "my-specs" { t.Errorf("expected SpecsDir \"my-specs\", got %q", answers.SpecsDir) @@ -234,8 +234,8 @@ func TestSeedInitStringDefaults(t *testing.T) { if answers.PromptsDir != "my-prompts" { t.Errorf("expected PromptsDir \"my-prompts\", got %q", answers.PromptsDir) } - if answers.LogFile != "/my/log.log" { - t.Errorf("expected LogFile \"/my/log.log\", got %q", answers.LogFile) + if answers.LogFile != "" { + t.Errorf("expected LogFile \"\" (omitted), got %q", answers.LogFile) } }) } diff --git a/internal/cli/init_internal_test.go b/internal/cli/init_internal_test.go index 51e4a16..1e54aec 100644 --- a/internal/cli/init_internal_test.go +++ b/internal/cli/init_internal_test.go @@ -107,6 +107,20 @@ func TestInitCommandWritesDefaultConfigFile(t *testing.T) { if !strings.Contains(contentText, `agent = "opencode"`) { t.Fatalf("expected config to include default agent, got %q", contentText) } + + // Verify empty optional fields are omitted + if strings.Contains(contentText, `log-file`) { + t.Errorf("empty log-file should be omitted, got %q", contentText) + } + if strings.Contains(contentText, `log-truncate`) { + t.Errorf("false log-truncate should be omitted, got %q", contentText) + } + if strings.Contains(contentText, `model = ""`) { + t.Errorf("empty model should be omitted, got %q", contentText) + } + if strings.Contains(contentText, `agent-mode = ""`) { + t.Errorf("empty agent-mode should be omitted, got %q", contentText) + } } func TestInitCommandWritesConfigToOutputPath(t *testing.T) { @@ -135,7 +149,7 @@ func TestInitCommandAsksQuestionsInSpecifiedOrder(t *testing.T) { } assertOutputContainsPromptsInOrder(t, out.String(), []string{ - "AI agent (opencode/claude/cursor)", + "AI agent (omp/opencode/claude/cursor/oh-my-pi)", "Model (optional)", "Agent mode/sub-agent (optional)", "Maximum iterations", @@ -143,7 +157,7 @@ func TestInitCommandAsksQuestionsInSpecifiedOrder(t *testing.T) { "Specs index file", "Implementation plan file", "Prompts directory", - "Log file path (optional)", + "Log file path (leave empty to disable logging)", "Write configuration now?", }) } @@ -278,29 +292,28 @@ func seededInitConfigLines() []string { return []string{ `agent = "claude"`, `model = "gpt-4o-mini"`, - `agent-mode = "planner"`, "max-iterations = 7", `specs-dir = "docs/specs"`, `specs-index-file = "INDEX.md"`, `implementation-plan-name = "PLAN.md"`, `prompts-dir = ".ralph/custom-prompts"`, - `log-file = "./logs/custom.log"`, "log-truncate = true", } } func seededInitPromptDefaults() []string { return []string{ - "AI agent (opencode/claude/cursor) [claude]:", + "Overwrite existing configuration? [no]:", + "AI agent (omp/opencode/claude/cursor/oh-my-pi) [claude]:", "Model (optional) [gpt-4o-mini]:", - "Agent mode/sub-agent (optional) [planner]:", + "Agent mode/sub-agent (optional):", "Maximum iterations [7]:", "Specs directory [docs/specs]:", "Specs index file [INDEX.md]:", "Implementation plan file [PLAN.md]:", "Prompts directory [.ralph/custom-prompts]:", - "Log file path (optional) [./logs/custom.log]:", - "Truncate log file on each run? [yes]:", + "Log file path (leave empty to disable logging):", + "Configuration preview:", "Write configuration now? [yes]:", } } @@ -315,6 +328,7 @@ func assertOutputContainsAll(t *testing.T, output string, expectedFragments []st } } + func TestInitCommandSeedsQuestionDefaultsFromExistingConfig(t *testing.T) { tmp := t.TempDir() configPath := filepath.Join(tmp, "ralph.toml") diff --git a/internal/config/config.go b/internal/config/config.go index a32cf30..0bc7d14 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -38,22 +38,22 @@ const ( // Config holds all Ralph configuration. type Config struct { - ConfigFile string `toml:"config-file"` + ConfigFile string `toml:"config-file,omitempty"` MaxIterations int `toml:"max-iterations"` - PromptFile string `toml:"prompt-file"` + PromptFile string `toml:"prompt-file,omitempty"` SpecsDir string `toml:"specs-dir"` SpecsIndexFile string `toml:"specs-index-file"` NoSpecsIndex bool `toml:"no-specs-index"` ImplementationPlanName string `toml:"implementation-plan-name"` - LogFile string `toml:"log-file"` - LogTruncate bool `toml:"log-truncate"` - CustomPrompt string `toml:"custom-prompt"` + LogFile string `toml:"log-file,omitempty"` + LogTruncate bool `toml:"log-truncate,omitempty"` + CustomPrompt string `toml:"custom-prompt,omitempty"` PromptsDir string `toml:"prompts-dir"` AgentName string `toml:"agent"` - Model string `toml:"model"` - AgentMode string `toml:"agent-mode"` - Env map[string]string `toml:"env"` - PromptOverrides map[string]PromptConfigOverride `toml:"prompt-overrides"` + Model string `toml:"model,omitempty"` + AgentMode string `toml:"agent-mode,omitempty"` + Env map[string]string `toml:"env,omitempty"` + PromptOverrides map[string]PromptConfigOverride `toml:"prompt-overrides,omitempty"` configLoaded bool } diff --git a/internal/config/writer_test.go b/internal/config/writer_test.go index f8449f8..74e74ad 100644 --- a/internal/config/writer_test.go +++ b/internal/config/writer_test.go @@ -174,3 +174,44 @@ func TestWriteConfig_SuccessAndVerify(t *testing.T) { t.Error("expected max-iterations in output") } } + +func TestWriteConfig_EmptyStringsShouldBeOmitted(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "test.toml") + cfg := &config.Config{ + AgentName: "opencode", + Model: "gpt-4", + AgentMode: "reviewer", + MaxIterations: 25, + SpecsDir: "specs", + SpecsIndexFile: "README.md", + ImplementationPlanName: "IMPLEMENTATION_PLAN.md", + PromptsDir: ".ralph/prompts", + LogFile: "ralph.log", + LogTruncate: false, + } + + err := config.WriteConfig(path, cfg) + if err != nil { + t.Fatalf("WriteConfig failed: %v", err) + } + + content, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile failed: %v", err) + } + + contentStr := string(content) + t.Logf("Generated TOML:\n%s", contentStr) + + // Should NOT contain empty string values + if strings.Contains(contentStr, `config-file = ""`) { + t.Error("Config file should not write empty config-file field") + } + if strings.Contains(contentStr, `prompt-file = ""`) { + t.Error("Config file should not write empty prompt-file field") + } + if strings.Contains(contentStr, `custom-prompt = ""`) { + t.Error("Config file should not write empty custom-prompt field") + } +} diff --git a/ralph.toml b/ralph.toml index 38a5d4f..12607ad 100644 --- a/ralph.toml +++ b/ralph.toml @@ -1,16 +1,8 @@ -agent = "opencode" -# model = "gpt-4" -# Optional agent mode/sub-agent (if supported by the CLI) -agent-mode = "ralph" - -# Iteration Settings -max-iterations = 25 - -# Directory Settings +max-iterations = 50 specs-dir = "specs" specs-index-file = "README.md" +no-specs-index = false implementation-plan-name = "IMPLEMENTATION_PLAN.md" +log-file = ".ralph/logs" prompts-dir = ".ralph/prompts" - -# Logging Configuration -log-file = "logs/ralph.log" +agent = "oh-my-pi" diff --git a/specs/agents.md b/specs/agents.md index 6f349e5..bf85a2e 100644 --- a/specs/agents.md +++ b/specs/agents.md @@ -74,6 +74,7 @@ specs/ - Opencode: [specs/agents/opencode.md](agents/opencode.md) - Claude: [specs/agents/claude.md](agents/claude.md) - Cursor: [specs/agents/cursor.md](agents/cursor.md) +- Oh My Pi (oh-my-pi): [specs/agents/oh-my-pi.md](agents/oh-my-pi.md) ## Data model diff --git a/specs/init-command.md b/specs/init-command.md index 54d6067..1fb6ec3 100644 --- a/specs/init-command.md +++ b/specs/init-command.md @@ -168,20 +168,21 @@ specs/ | Prompt | Config key | Type | Default | Validation | | ----------------------------------------- | -------------------------- | ------- | ------------------------ | ------------------------------- | -| AI agent (`omp`, `opencode`, `claude`, `cursor`) | `agent` | select | `opencode` | Must be one of supported agents | -| Model (optional) | `model` | input | empty | Free text; empty allowed | -| Agent mode/sub-agent (optional) | `agent-mode` | input | empty | Free text; empty allowed | +| AI agent (`omp`, `opencode`, `claude`, `cursor`, `oh-my-pi`) | `agent` | select | `opencode` | Must be one of supported agents | +| Model (optional) | `model` | input | empty (not written) | Free text; empty allowed | +| Agent mode/sub-agent (optional) | `agent-mode` | input | empty (not written) | Free text; empty allowed | | Maximum iterations | `max-iterations` | input | `25` | Integer > 0 | | Specs directory | `specs-dir` | input | `specs` | Non-empty path | | Specs index file | `specs-index-file` | input | `README.md` | Non-empty file name | | Implementation plan file | `implementation-plan-name` | input | `IMPLEMENTATION_PLAN.md` | Non-empty file name | | Prompts directory | `prompts-dir` | input | `.ralph/prompts` | Non-empty path | -| Log file path (optional) | `log-file` | input | `` (empty = disabled) | Non-empty path (optional) | +| Log file path (leave empty to disable logging) | `log-file` | input | `` (empty = disabled) | Non-empty path (optional) | ### Generated TOML behavior - Answers are converted into config keys defined in [specs/configuration.md](configuration.md). - Writes use atomic temp-file + rename semantics through `internal/config/writer.go`. +- Optional fields with empty values (model, agent-mode, log-file) are omitted from the generated TOML file. ## Permissions @@ -217,7 +218,6 @@ specs/ - A generated config is loaded successfully by existing config resolution logic. ## Appendices - ### Example generated config (accept defaults) ```toml @@ -226,5 +226,6 @@ max-iterations = 25 specs-dir = "specs" specs-index-file = "README.md" implementation-plan-name = "IMPLEMENTATION_PLAN.md" +prompts-dir = ".ralph/prompts" log-truncate = false ``` From 01b50aaea85ae9eb480a98228a3bb3d5d1730449 Mon Sep 17 00:00:00 2001 From: iyaki Date: Wed, 17 Jun 2026 02:17:32 +0000 Subject: [PATCH 2/3] test: Add coverage tests for init questionnaire helpers Added tests to maintain 95%+ coverage requirement: - TestAskSingleQuestionWithReaderSuccess - TestReadBoolFlagOverrideForTest - TestReadEnvFlagOverridesForTest - TestAskQuestionsSuccess - TestStandardQuestionnaireRunnerAskQuestions - TestAskSingleQuestion These tests cover questionnaire runner methods and exported test helpers. --- internal/cli/init_internal_test.go | 121 +++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/internal/cli/init_internal_test.go b/internal/cli/init_internal_test.go index 1e54aec..55cb809 100644 --- a/internal/cli/init_internal_test.go +++ b/internal/cli/init_internal_test.go @@ -487,3 +487,124 @@ func TestIsInteractiveTerminalRejectsDevNullStreams(t *testing.T) { t.Fatal("expected non-interactive terminal check for /dev/null streams") } } + +func TestAskSingleQuestionWithReaderSuccess(t *testing.T) { + tmp := t.TempDir() + cmd, out := setupInteractiveInitCommand(t, tmp) + cmd.SetIn(strings.NewReader("yes\n")) + + question := newConfirmQuestion(questionKeyWriteConfiguration, "Write configuration now?", confirmYes) + answer, err := askSingleQuestionWithReader(&InitSession{ + Reader: bufio.NewReader(cmd.InOrStdin()), + Writer: out, + }, question, &bufioAnswerReader{reader: bufio.NewReader(cmd.InOrStdin())}) + + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if answer != "yes" { + t.Errorf("expected answer yes, got %q", answer) + } +} + +func TestReadBoolFlagOverrideForTest(t *testing.T) { + cmd := &cobra.Command{} + cmd.Flags().Bool("test-flag", false, "") + + result, err := ReadBoolFlagOverrideForTest(cmd, "test-flag") + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if result.Changed { + t.Error("expected Changed=false") + } + if result.Value { + t.Error("expected Value=false") + } +} + +func TestReadEnvFlagOverridesForTest(t *testing.T) { + cmd := &cobra.Command{} + cmd.Flags().StringArray("env", []string{}, "") + if err := cmd.Flags().Set("env", "KEY=value"); err != nil { + t.Fatalf("failed to set flag: %v", err) + } + + result, err := ReadEnvFlagOverridesForTest(cmd) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if result["KEY"] != "value" { + t.Errorf("expected KEY=value, got %v", result) + } +} + +func TestAskQuestionsSuccess(t *testing.T) { + cmd, out := setupInteractiveInitCommand(t, t.TempDir()) + cmd.SetIn(strings.NewReader("test-answer\n")) + + session := &InitSession{ + Reader: bufio.NewReader(cmd.InOrStdin()), + Writer: out, + Answers: &InitAnswers{}, + } + + questions := []InitQuestion{ + newInputQuestion(questionKeyModel, "Model", "", false, nil), + } + + err := askQuestions(session, questions) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if session.Answers.Model != "test-answer" { + t.Errorf("expected Model to be test-answer, got %q", session.Answers.Model) + } +} + +func TestStandardQuestionnaireRunnerAskQuestions(t *testing.T) { + cmd, out := setupInteractiveInitCommand(t, t.TempDir()) + cmd.SetIn(strings.NewReader("answer1\nanswer2\n")) + + session := &InitSession{ + Reader: bufio.NewReader(cmd.InOrStdin()), + Writer: out, + Answers: &InitAnswers{}, + } + + runner := &standardQuestionnaireRunner{} + questions := []InitQuestion{ + newInputQuestion(questionKeyModel, "Model", "", false, nil), + newInputQuestion(questionKeySpecsDir, "Specs dir", "", false, nil), + } + + err := runner.AskQuestions(session, questions) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if session.Answers.Model != "answer1" { + t.Errorf("expected Model to be answer1, got %q", session.Answers.Model) + } + if session.Answers.SpecsDir != "answer2" { + t.Errorf("expected SpecsDir to be answer2, got %q", session.Answers.SpecsDir) + } +} + +func TestAskSingleQuestion(t *testing.T) { + cmd, out := setupInteractiveInitCommand(t, t.TempDir()) + cmd.SetIn(strings.NewReader("test-response\n")) + + session := &InitSession{ + Reader: bufio.NewReader(cmd.InOrStdin()), + Writer: out, + } + + question := newInputQuestion(questionKeyModel, "Model", "", false, nil) + answer, err := askSingleQuestion(session, question) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if answer != "test-response" { + t.Errorf("expected answer test-response, got %q", answer) + } +} From c5dc5b074efec2ca64145b8aed622e0ceabd0fea Mon Sep 17 00:00:00 2001 From: iyaki Date: Wed, 17 Jun 2026 02:25:30 +0000 Subject: [PATCH 3/3] Changes on project's ralph config --- ralph.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ralph.toml b/ralph.toml index 12607ad..fd7fada 100644 --- a/ralph.toml +++ b/ralph.toml @@ -3,6 +3,6 @@ specs-dir = "specs" specs-index-file = "README.md" no-specs-index = false implementation-plan-name = "IMPLEMENTATION_PLAN.md" -log-file = ".ralph/logs" +log-file = ".ralph/logs/ralph.log" prompts-dir = ".ralph/prompts" agent = "oh-my-pi"