diff --git a/internal/agents/backward_compat_test.go b/internal/agents/backward_compat_test.go new file mode 100644 index 0000000..176b6ab --- /dev/null +++ b/internal/agents/backward_compat_test.go @@ -0,0 +1,107 @@ +package agents + +import ( + "context" + "testing" + "time" +) + +// Backward-compatibility tests for the agents package. +// These tests verify that the Agent interface, ExecuteOptions, and +// ExecuteResult structs maintain their public API surface. + +// TestBackwardCompat_DefaultTimeout verifies the default timeout constant +// hasn't changed, since scripts and configs may rely on the 30-minute default. +func TestBackwardCompat_DefaultTimeout(t *testing.T) { + expected := 30 * time.Minute + if DefaultTimeout != expected { + t.Errorf("DefaultTimeout = %v, want %v — changing the default timeout breaks existing behavior", + DefaultTimeout, expected) + } +} + +// TestBackwardCompat_ExecuteOptionsFields verifies that ExecuteOptions +// can be constructed with all expected fields. A compilation failure here +// means a field was removed or renamed. +func TestBackwardCompat_ExecuteOptionsFields(t *testing.T) { + opts := ExecuteOptions{ + Prompt: "test prompt", + WorkDir: "/tmp/test", + Files: []string{"file1.go", "file2.go"}, + Timeout: 10 * time.Minute, + } + + if opts.Prompt != "test prompt" { + t.Error("ExecuteOptions.Prompt field not working") + } + if opts.WorkDir != "/tmp/test" { + t.Error("ExecuteOptions.WorkDir field not working") + } + if len(opts.Files) != 2 { + t.Error("ExecuteOptions.Files field not working") + } + if opts.Timeout != 10*time.Minute { + t.Error("ExecuteOptions.Timeout field not working") + } +} + +// TestBackwardCompat_ExecuteResultFields verifies that ExecuteResult +// can be constructed with all expected fields and that IsSuccess() +// works as expected. +func TestBackwardCompat_ExecuteResultFields(t *testing.T) { + // Successful result + success := &ExecuteResult{ + Output: "task completed", + JSON: []byte(`{"ok":true}`), + ExitCode: 0, + Duration: 5 * time.Second, + Error: "", + } + + if !success.IsSuccess() { + t.Error("IsSuccess() should return true for exit code 0 and no error") + } + + // Failed result (non-zero exit code) + failedExit := &ExecuteResult{ + Output: "", + ExitCode: 1, + Error: "", + } + + if failedExit.IsSuccess() { + t.Error("IsSuccess() should return false for non-zero exit code") + } + + // Failed result (error message) + failedError := &ExecuteResult{ + Output: "", + ExitCode: 0, + Error: "something went wrong", + } + + if failedError.IsSuccess() { + t.Error("IsSuccess() should return false when Error is set") + } +} + +// testAgent implements the Agent interface for compile-time verification. +type testAgent struct{} + +func (a *testAgent) Name() string { return "test" } +func (a *testAgent) Execute(_ context.Context, _ ExecuteOptions) (*ExecuteResult, error) { + return &ExecuteResult{}, nil +} + +// TestBackwardCompat_AgentInterfaceCompiles verifies that the Agent +// interface has the expected method signatures. This is a compile-time +// check — if the interface changes, this test won't compile. +func TestBackwardCompat_AgentInterfaceCompiles(t *testing.T) { + // This is a compile-time assertion that the interface shape is preserved. + // The testAgent type above implements Agent, and if Agent's methods change, + // this assignment will fail to compile. + var _ Agent = (*testAgent)(nil) + + // Verify the interface can't be satisfied with just Name() — Execute() is required + t.Log("Agent interface requires both Name() and Execute() methods") +} diff --git a/internal/config/backward_compat_test.go b/internal/config/backward_compat_test.go index 9042660..5137d61 100644 --- a/internal/config/backward_compat_test.go +++ b/internal/config/backward_compat_test.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "testing" + "time" ) // TestBackwardCompat_OldConfigLoadsWithNewDefaults verifies that config files @@ -334,6 +335,358 @@ budget: } } +// TestBackwardCompat_CopilotProviderConfig verifies that configs including +// copilot provider settings (added in v0.3.2) load correctly alongside +// older claude/codex settings. +func TestBackwardCompat_CopilotProviderConfig(t *testing.T) { + tmpDir := t.TempDir() + + // Config that includes all three providers (v0.3.2+ format) + configWithCopilot := ` +providers: + claude: + enabled: true + data_path: ~/.claude + dangerously_skip_permissions: false + codex: + enabled: true + data_path: ~/.codex + dangerously_bypass_approvals_and_sandbox: false + copilot: + enabled: true + data_path: ~/.copilot + dangerously_skip_permissions: false + preference: + - claude + - codex + - copilot +` + configPath := filepath.Join(tmpDir, "nightshift.yaml") + if err := os.WriteFile(configPath, []byte(configWithCopilot), 0644); err != nil { + t.Fatal(err) + } + + cfg, err := LoadFromPaths(tmpDir, configPath) + if err != nil { + t.Fatalf("LoadFromPaths error: %v", err) + } + + if !cfg.Providers.Copilot.Enabled { + t.Error("Copilot.Enabled should be true") + } + if cfg.Providers.Copilot.DataPath != "~/.copilot" { + t.Errorf("Copilot.DataPath = %q, want ~/.copilot", cfg.Providers.Copilot.DataPath) + } + if cfg.Providers.Copilot.DangerouslySkipPermissions { + t.Error("Copilot.DangerouslySkipPermissions should be false") + } + if len(cfg.Providers.Preference) != 3 { + t.Errorf("Providers.Preference length = %d, want 3", len(cfg.Providers.Preference)) + } +} + +// TestBackwardCompat_PreCopilotConfigStillWorks verifies that configs from +// before v0.3.2 (that don't mention copilot at all) still load correctly +// and get appropriate defaults for the copilot provider. +func TestBackwardCompat_PreCopilotConfigStillWorks(t *testing.T) { + tmpDir := t.TempDir() + + // Config from before copilot was added — no copilot section at all + oldConfig := ` +providers: + claude: + enabled: true + data_path: ~/.claude + codex: + enabled: true + data_path: ~/.codex +budget: + mode: daily +` + configPath := filepath.Join(tmpDir, "nightshift.yaml") + if err := os.WriteFile(configPath, []byte(oldConfig), 0644); err != nil { + t.Fatal(err) + } + + cfg, err := LoadFromPaths(tmpDir, configPath) + if err != nil { + t.Fatalf("LoadFromPaths error: %v", err) + } + + // Copilot should get defaults even when not mentioned in config + if !cfg.Providers.Copilot.Enabled { + t.Error("Copilot.Enabled should default to true") + } + if cfg.Providers.Copilot.DangerouslySkipPermissions { + t.Error("Copilot.DangerouslySkipPermissions should default to false") + } +} + +// TestBackwardCompat_ConfigFieldsStable verifies that all expected fields +// exist on the Config struct by constructing a full config. A compilation +// failure here means a field was removed or renamed. +func TestBackwardCompat_ConfigFieldsStable(t *testing.T) { + // This test verifies the Config struct shape via compilation. + // If any field is removed or renamed, this won't compile. + cfg := Config{ + Schedule: ScheduleConfig{ + Cron: "0 2 * * *", + Interval: "", + Window: nil, + MaxProjects: 3, + MaxTasks: 2, + }, + Budget: BudgetConfig{ + Mode: "daily", + MaxPercent: 75, + AggressiveEndOfWeek: true, + ReservePercent: 5, + WeeklyTokens: 700000, + PerProvider: map[string]int{"claude": 500000}, + BillingMode: "subscription", + CalibrateEnabled: true, + SnapshotInterval: "30m", + SnapshotRetentionDays: 90, + WeekStartDay: "monday", + DBPath: "/tmp/test.db", + }, + Providers: ProvidersConfig{ + Claude: ProviderConfig{ + Enabled: true, + DataPath: "~/.claude", + DangerouslySkipPermissions: false, + DangerouslyBypassApprovalsAndSandbox: false, + }, + Codex: ProviderConfig{ + Enabled: true, + DataPath: "~/.codex", + }, + Copilot: ProviderConfig{ + Enabled: true, + DataPath: "~/.copilot", + }, + Preference: []string{"claude", "codex", "copilot"}, + }, + Projects: []ProjectConfig{ + { + Path: "/tmp/project", + Priority: 1, + Tasks: []string{"lint-fix"}, + Config: "", + Pattern: "", + Exclude: nil, + }, + }, + Tasks: TasksConfig{ + Enabled: []string{"lint-fix"}, + Priorities: map[string]int{"lint-fix": 1}, + Disabled: nil, + Intervals: map[string]string{"lint-fix": "24h"}, + Custom: []CustomTaskConfig{ + { + Type: "my-task", + Name: "My Task", + Description: "test", + Category: "pr", + CostTier: "low", + RiskLevel: "low", + Interval: "48h", + }, + }, + }, + Integrations: IntegrationsConfig{ + ClaudeMD: true, + AgentsMD: true, + TaskSources: []TaskSourceEntry{ + {GithubIssues: true}, + }, + }, + Logging: LoggingConfig{ + Level: "info", + Path: "/tmp/logs", + Format: "json", + }, + Reporting: ReportingConfig{ + MorningSummary: true, + }, + } + + // Sanity check that it validates + if err := Validate(&cfg); err != nil { + t.Fatalf("Full config validation failed: %v", err) + } +} + +// TestBackwardCompat_DefaultConstants verifies that default constant values +// haven't changed, since they affect behavior for users with minimal configs. +func TestBackwardCompat_DefaultConstants(t *testing.T) { + checks := []struct { + name string + got interface{} + want interface{} + }{ + {"DefaultBudgetMode", DefaultBudgetMode, "daily"}, + {"DefaultMaxPercent", DefaultMaxPercent, 75}, + {"DefaultReservePercent", DefaultReservePercent, 5}, + {"DefaultWeeklyTokens", DefaultWeeklyTokens, 700000}, + {"DefaultBillingMode", DefaultBillingMode, "subscription"}, + {"DefaultSnapshotInterval", DefaultSnapshotInterval, "30m"}, + {"DefaultSnapshotRetention", DefaultSnapshotRetention, 90}, + {"DefaultWeekStartDay", DefaultWeekStartDay, "monday"}, + {"DefaultLogLevel", DefaultLogLevel, "info"}, + {"DefaultLogFormat", DefaultLogFormat, "json"}, + {"DefaultClaudeDataPath", DefaultClaudeDataPath, "~/.claude"}, + {"DefaultCodexDataPath", DefaultCodexDataPath, "~/.codex"}, + {"DefaultCopilotDataPath", DefaultCopilotDataPath, "~/.copilot"}, + {"ProjectConfigName", ProjectConfigName, "nightshift.yaml"}, + } + + for _, c := range checks { + if c.got != c.want { + t.Errorf("%s = %v, want %v — changing defaults breaks existing minimal configs", + c.name, c.got, c.want) + } + } +} + +// TestBackwardCompat_CustomTaskValidation verifies that custom task +// validation rules are stable. +func TestBackwardCompat_CustomTaskValidation(t *testing.T) { + // Valid custom task must pass + validCfg := &Config{ + Tasks: TasksConfig{ + Custom: []CustomTaskConfig{ + { + Type: "my-review", + Name: "My Review", + Description: "Custom review task", + Category: "pr", + CostTier: "medium", + RiskLevel: "low", + Interval: "48h", + }, + }, + }, + } + if err := Validate(validCfg); err != nil { + t.Errorf("Valid custom task should pass: %v", err) + } + + // Invalid category must fail + invalidCat := &Config{ + Tasks: TasksConfig{ + Custom: []CustomTaskConfig{ + {Type: "test", Name: "Test", Description: "test", Category: "invalid"}, + }, + }, + } + if err := Validate(invalidCat); err == nil { + t.Error("Invalid category should fail validation") + } + + // Missing required fields must fail + missingType := &Config{ + Tasks: TasksConfig{ + Custom: []CustomTaskConfig{ + {Name: "Test", Description: "test"}, + }, + }, + } + if err := Validate(missingType); err == nil { + t.Error("Missing type should fail validation") + } +} + +// TestBackwardCompat_HelperMethodsStable verifies that Config helper +// methods work as expected with various inputs. +func TestBackwardCompat_HelperMethodsStable(t *testing.T) { + cfg := &Config{ + Budget: BudgetConfig{ + WeeklyTokens: 700000, + PerProvider: map[string]int{"claude": 500000}, + }, + Tasks: TasksConfig{ + Enabled: []string{"lint-fix", "bug-finder"}, + Disabled: []string{"dead-code"}, + Priorities: map[string]int{"lint-fix": 10}, + Intervals: map[string]string{"lint-fix": "24h"}, + }, + } + + // GetProviderBudget: per-provider override + if got := cfg.GetProviderBudget("claude"); got != 500000 { + t.Errorf("GetProviderBudget(claude) = %d, want 500000", got) + } + // GetProviderBudget: fallback to default + if got := cfg.GetProviderBudget("codex"); got != 700000 { + t.Errorf("GetProviderBudget(codex) = %d, want 700000", got) + } + + // IsTaskEnabled: in enabled list + if !cfg.IsTaskEnabled("lint-fix") { + t.Error("lint-fix should be enabled") + } + // IsTaskEnabled: not in enabled list + if cfg.IsTaskEnabled("perf-profile") { + t.Error("perf-profile should not be enabled (not in list)") + } + // IsTaskEnabled: explicitly disabled + if cfg.IsTaskEnabled("dead-code") { + t.Error("dead-code should be disabled") + } + + // GetTaskPriority + if got := cfg.GetTaskPriority("lint-fix"); got != 10 { + t.Errorf("GetTaskPriority(lint-fix) = %d, want 10", got) + } + if got := cfg.GetTaskPriority("unknown"); got != 0 { + t.Errorf("GetTaskPriority(unknown) = %d, want 0", got) + } + + // GetTaskInterval + if got := cfg.GetTaskInterval("lint-fix"); got != 24*time.Hour { + t.Errorf("GetTaskInterval(lint-fix) = %v, want 24h", got) + } + if got := cfg.GetTaskInterval("unknown"); got != 0 { + t.Errorf("GetTaskInterval(unknown) = %v, want 0", got) + } +} + +// TestBackwardCompat_ValidationErrorsStable verifies that validation +// error sentinel values haven't changed. +func TestBackwardCompat_ValidationErrorsStable(t *testing.T) { + // These error values are part of the public API since users may + // check for specific validation errors. + errors := []struct { + name string + err error + }{ + {"ErrCronAndInterval", ErrCronAndInterval}, + {"ErrInvalidBudgetMode", ErrInvalidBudgetMode}, + {"ErrInvalidBillingMode", ErrInvalidBillingMode}, + {"ErrInvalidWeekStartDay", ErrInvalidWeekStartDay}, + {"ErrInvalidMaxPercent", ErrInvalidMaxPercent}, + {"ErrInvalidReservePercent", ErrInvalidReservePercent}, + {"ErrInvalidSnapshotRetention", ErrInvalidSnapshotRetention}, + {"ErrInvalidLogLevel", ErrInvalidLogLevel}, + {"ErrInvalidLogFormat", ErrInvalidLogFormat}, + {"ErrCustomTaskMissingType", ErrCustomTaskMissingType}, + {"ErrCustomTaskMissingName", ErrCustomTaskMissingName}, + {"ErrCustomTaskMissingDescription", ErrCustomTaskMissingDescription}, + {"ErrCustomTaskInvalidType", ErrCustomTaskInvalidType}, + {"ErrCustomTaskInvalidCategory", ErrCustomTaskInvalidCategory}, + {"ErrCustomTaskInvalidCostTier", ErrCustomTaskInvalidCostTier}, + {"ErrCustomTaskInvalidRiskLevel", ErrCustomTaskInvalidRiskLevel}, + {"ErrCustomTaskDuplicateType", ErrCustomTaskDuplicateType}, + } + + for _, e := range errors { + if e.err == nil { + t.Errorf("%s is nil — sentinel errors must not be removed", e.name) + } + } +} + // TestBackwardCompat_ProviderPathExpansion verifies that provider // path expansion still works correctly. func TestBackwardCompat_ProviderPathExpansion(t *testing.T) { diff --git a/internal/orchestrator/backward_compat_test.go b/internal/orchestrator/backward_compat_test.go new file mode 100644 index 0000000..46bc737 --- /dev/null +++ b/internal/orchestrator/backward_compat_test.go @@ -0,0 +1,167 @@ +package orchestrator + +import ( + "encoding/json" + "testing" + "time" +) + +// Backward-compatibility tests for the orchestrator package. +// These tests verify that the output structs serialize to JSON correctly, +// since external tools and reports parse these structures. + +// TestBackwardCompat_TaskStatusValues verifies that TaskStatus string +// values are stable. These appear in logs, reports, and state database. +func TestBackwardCompat_TaskStatusValues(t *testing.T) { + statuses := map[TaskStatus]string{ + StatusPending: "pending", + StatusPlanning: "planning", + StatusExecuting: "executing", + StatusReviewing: "reviewing", + StatusCompleted: "completed", + StatusFailed: "failed", + StatusAbandoned: "abandoned", + } + + for status, want := range statuses { + if string(status) != want { + t.Errorf("TaskStatus %q has value %q, want %q — status values appear in logs and database", + want, string(status), want) + } + } +} + +// TestBackwardCompat_TaskResultJSONKeys verifies that TaskResult serializes +// to JSON with the expected key names. External tools may parse this output. +func TestBackwardCompat_TaskResultJSONKeys(t *testing.T) { + result := TaskResult{ + TaskID: "test-123", + Status: StatusCompleted, + Iterations: 2, + Plan: &PlanOutput{ + Steps: []string{"step1", "step2"}, + Files: []string{"file.go"}, + Description: "test plan", + }, + Output: "done", + OutputType: "PR", + OutputRef: "https://github.com/test/repo/pull/1", + Error: "", + Duration: 5 * time.Minute, + Logs: []LogEntry{{Message: "started"}}, + } + + data, err := json.Marshal(result) + if err != nil { + t.Fatalf("json.Marshal(TaskResult): %v", err) + } + + // Unmarshal into generic map to check key names + var m map[string]interface{} + if err := json.Unmarshal(data, &m); err != nil { + t.Fatalf("json.Unmarshal: %v", err) + } + + requiredKeys := []string{ + "task_id", "status", "iterations", "plan", + "output", "output_type", "output_ref", "duration", "logs", + } + for _, key := range requiredKeys { + if _, ok := m[key]; !ok { + t.Errorf("TaskResult JSON missing key %q — this breaks external consumers", key) + } + } + + // Verify status value is correct + if m["status"] != "completed" { + t.Errorf("TaskResult.status = %v, want completed", m["status"]) + } +} + +// TestBackwardCompat_PlanOutputJSONKeys verifies PlanOutput serialization. +func TestBackwardCompat_PlanOutputJSONKeys(t *testing.T) { + plan := PlanOutput{ + Steps: []string{"analyze", "implement", "test"}, + Files: []string{"main.go", "main_test.go"}, + Description: "fix the bug", + Raw: "raw plan output", + } + + data, err := json.Marshal(plan) + if err != nil { + t.Fatalf("json.Marshal(PlanOutput): %v", err) + } + + var m map[string]interface{} + if err := json.Unmarshal(data, &m); err != nil { + t.Fatalf("json.Unmarshal: %v", err) + } + + for _, key := range []string{"steps", "files", "description"} { + if _, ok := m[key]; !ok { + t.Errorf("PlanOutput JSON missing key %q", key) + } + } +} + +// TestBackwardCompat_ImplementOutputJSONKeys verifies ImplementOutput serialization. +func TestBackwardCompat_ImplementOutputJSONKeys(t *testing.T) { + impl := ImplementOutput{ + FilesModified: []string{"main.go"}, + Summary: "fixed the bug", + Raw: "raw output", + } + + data, err := json.Marshal(impl) + if err != nil { + t.Fatalf("json.Marshal(ImplementOutput): %v", err) + } + + var m map[string]interface{} + if err := json.Unmarshal(data, &m); err != nil { + t.Fatalf("json.Unmarshal: %v", err) + } + + for _, key := range []string{"files_modified", "summary"} { + if _, ok := m[key]; !ok { + t.Errorf("ImplementOutput JSON missing key %q", key) + } + } +} + +// TestBackwardCompat_ReviewOutputJSONKeys verifies ReviewOutput serialization. +func TestBackwardCompat_ReviewOutputJSONKeys(t *testing.T) { + review := ReviewOutput{ + Passed: true, + Feedback: "looks good", + Issues: []string{}, + Raw: "raw review", + } + + data, err := json.Marshal(review) + if err != nil { + t.Fatalf("json.Marshal(ReviewOutput): %v", err) + } + + var m map[string]interface{} + if err := json.Unmarshal(data, &m); err != nil { + t.Fatalf("json.Unmarshal: %v", err) + } + + for _, key := range []string{"passed", "feedback"} { + if _, ok := m[key]; !ok { + t.Errorf("ReviewOutput JSON missing key %q", key) + } + } +} + +// TestBackwardCompat_OrchestratorConstants verifies that orchestration +// constants haven't changed unexpectedly. +func TestBackwardCompat_OrchestratorConstants(t *testing.T) { + if DefaultMaxIterations != 3 { + t.Errorf("DefaultMaxIterations = %d, want 3", DefaultMaxIterations) + } + if DefaultAgentTimeout != 30*time.Minute { + t.Errorf("DefaultAgentTimeout = %v, want 30m", DefaultAgentTimeout) + } +} diff --git a/internal/providers/backward_compat_test.go b/internal/providers/backward_compat_test.go new file mode 100644 index 0000000..9501486 --- /dev/null +++ b/internal/providers/backward_compat_test.go @@ -0,0 +1,323 @@ +package providers + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "testing" +) + +// Backward-compatibility tests for the providers package. +// These tests verify that provider interfaces, struct shapes, and +// JSON serialization are stable across versions. + +// TestBackwardCompat_ProviderInterfaceShape verifies that all three +// providers (Claude, Codex, Copilot) satisfy the Provider interface. +// If the interface changes, this test won't compile. +func TestBackwardCompat_ProviderInterfaceShape(t *testing.T) { + var _ Provider = (*Claude)(nil) + var _ Provider = (*Codex)(nil) + var _ Provider = (*Copilot)(nil) +} + +// TestBackwardCompat_ProviderNames verifies that provider Name() methods +// return stable values. These are used in config files, database records, +// and log entries. +func TestBackwardCompat_ProviderNames(t *testing.T) { + tests := []struct { + provider Provider + wantName string + }{ + {NewClaude(), "claude"}, + {NewCodex(), "codex"}, + {NewCopilot(), "copilot"}, + } + + for _, tt := range tests { + if got := tt.provider.Name(); got != tt.wantName { + t.Errorf("%T.Name() = %q, want %q — provider names are referenced in configs and database", + tt.provider, got, tt.wantName) + } + } +} + +// TestBackwardCompat_ClaudeWithCustomPath verifies that NewClaudeWithPath +// works and that DataPath() returns the configured path. +func TestBackwardCompat_ClaudeWithCustomPath(t *testing.T) { + c := NewClaudeWithPath("/custom/claude") + if c.DataPath() != "/custom/claude" { + t.Errorf("Claude.DataPath() = %q, want /custom/claude", c.DataPath()) + } +} + +// TestBackwardCompat_CodexWithCustomPath verifies NewCodexWithPath. +func TestBackwardCompat_CodexWithCustomPath(t *testing.T) { + c := NewCodexWithPath("/custom/codex") + if c.DataPath() != "/custom/codex" { + t.Errorf("Codex.DataPath() = %q, want /custom/codex", c.DataPath()) + } +} + +// TestBackwardCompat_CopilotWithCustomPath verifies NewCopilotWithPath. +func TestBackwardCompat_CopilotWithCustomPath(t *testing.T) { + c := NewCopilotWithPath("/custom/copilot") + if c.DataPath() != "/custom/copilot" { + t.Errorf("Copilot.DataPath() = %q, want /custom/copilot", c.DataPath()) + } +} + +// TestBackwardCompat_ProviderCostMethod verifies that Cost() returns +// values for all providers (even if zero for Copilot). +func TestBackwardCompat_ProviderCostMethod(t *testing.T) { + // Claude should have non-zero cost (token-based billing) + claudeIn, claudeOut := NewClaude().Cost() + if claudeIn <= 0 || claudeOut <= 0 { + t.Errorf("Claude.Cost() = (%d, %d), expected positive values", claudeIn, claudeOut) + } + + // Codex should have non-zero cost (token-based billing) + codexIn, codexOut := NewCodex().Cost() + if codexIn <= 0 || codexOut <= 0 { + t.Errorf("Codex.Cost() = (%d, %d), expected positive values", codexIn, codexOut) + } + + // Copilot returns 0 (request-based, not token-based) + copilotIn, copilotOut := NewCopilot().Cost() + if copilotIn != 0 || copilotOut != 0 { + t.Errorf("Copilot.Cost() = (%d, %d), expected (0, 0) for request-based billing", + copilotIn, copilotOut) + } +} + +// TestBackwardCompat_StatsCacheJSONParsing verifies that the StatsCache +// JSON structure matches Claude Code's stats-cache.json format. +func TestBackwardCompat_StatsCacheJSONParsing(t *testing.T) { + // Simulate a stats-cache.json from Claude Code + jsonData := `{ + "version": 1, + "dailyActivity": [ + {"date": "2026-04-10", "messageCount": 42, "sessionCount": 3, "toolCallCount": 100} + ], + "dailyModelTokens": [ + {"date": "2026-04-10", "tokensByModel": {"claude-opus-4-6": 50000, "claude-sonnet-4-6": 30000}} + ] + }` + + var stats StatsCache + if err := json.Unmarshal([]byte(jsonData), &stats); err != nil { + t.Fatalf("Failed to parse StatsCache JSON: %v", err) + } + + if stats.Version != 1 { + t.Errorf("Version = %d, want 1", stats.Version) + } + if len(stats.DailyActivity) != 1 { + t.Fatalf("DailyActivity length = %d, want 1", len(stats.DailyActivity)) + } + if stats.DailyActivity[0].Date != "2026-04-10" { + t.Errorf("DailyActivity[0].Date = %q, want 2026-04-10", stats.DailyActivity[0].Date) + } + if stats.DailyActivity[0].MessageCount != 42 { + t.Errorf("MessageCount = %d, want 42", stats.DailyActivity[0].MessageCount) + } + + // TokensByDate should sum across models + byDate := stats.TokensByDate() + if byDate["2026-04-10"] != 80000 { + t.Errorf("TokensByDate[2026-04-10] = %d, want 80000", byDate["2026-04-10"]) + } + + // GetDailyStat should return combined data + stat := stats.GetDailyStat("2026-04-10") + if stat == nil { + t.Fatal("GetDailyStat returned nil for existing date") + } + if stat.MessageCount != 42 { + t.Errorf("DailyStat.MessageCount = %d, want 42", stat.MessageCount) + } +} + +// TestBackwardCompat_TokenUsageTotalTokens verifies that TotalTokens() +// sums all four token fields correctly. +func TestBackwardCompat_TokenUsageTotalTokens(t *testing.T) { + usage := &TokenUsage{ + InputTokens: 1000, + OutputTokens: 2000, + CacheReadInputTokens: 500, + CacheCreationInputTokens: 300, + } + + total := usage.TotalTokens() + expected := int64(3800) + if total != expected { + t.Errorf("TotalTokens() = %d, want %d", total, expected) + } +} + +// TestBackwardCompat_ParseStatsCacheEmptyFile verifies that a missing +// stats-cache.json file returns an empty StatsCache (not an error). +func TestBackwardCompat_ParseStatsCacheEmptyFile(t *testing.T) { + stats, err := ParseStatsCache("/nonexistent/path/stats-cache.json") + if err != nil { + t.Fatalf("ParseStatsCache for missing file should not error: %v", err) + } + if stats == nil { + t.Fatal("ParseStatsCache should return empty StatsCache, not nil") + } + if len(stats.DailyActivity) != 0 { + t.Error("Empty StatsCache should have no DailyActivity") + } +} + +// TestBackwardCompat_CopilotUsageDataJSON verifies that the Copilot +// usage tracking file format is stable. +func TestBackwardCompat_CopilotUsageDataJSON(t *testing.T) { + data := CopilotUsageData{ + RequestCount: 42, + Month: "2026-04", + } + + jsonBytes, err := json.Marshal(data) + if err != nil { + t.Fatalf("json.Marshal(CopilotUsageData): %v", err) + } + + var m map[string]interface{} + if err := json.Unmarshal(jsonBytes, &m); err != nil { + t.Fatalf("json.Unmarshal: %v", err) + } + + for _, key := range []string{"request_count", "last_reset", "month"} { + if _, ok := m[key]; !ok { + t.Errorf("CopilotUsageData JSON missing key %q — this breaks usage file compatibility", key) + } + } +} + +// TestBackwardCompat_CopilotUsageDataRoundTrip verifies that usage data +// can be written and read back correctly (file format stability). +func TestBackwardCompat_CopilotUsageDataRoundTrip(t *testing.T) { + tmpDir := t.TempDir() + c := NewCopilotWithPath(tmpDir) + + // Load should return default data when file doesn't exist + data, err := c.LoadUsageData() + if err != nil { + t.Fatalf("LoadUsageData for new install: %v", err) + } + if data.RequestCount != 0 { + t.Errorf("New install RequestCount = %d, want 0", data.RequestCount) + } + + // Save and reload + data.RequestCount = 10 + if err := c.SaveUsageData(data); err != nil { + t.Fatalf("SaveUsageData: %v", err) + } + + reloaded, err := c.LoadUsageData() + if err != nil { + t.Fatalf("LoadUsageData after save: %v", err) + } + if reloaded.RequestCount != 10 { + t.Errorf("After reload RequestCount = %d, want 10", reloaded.RequestCount) + } +} + +// TestBackwardCompat_CodexRateLimitsJSON verifies that Codex rate limit +// JSON parsing is stable. +func TestBackwardCompat_CodexRateLimitsJSON(t *testing.T) { + jsonData := `{ + "primary": {"used_percent": 45.5, "window_minutes": 300, "resets_at": 1712700000}, + "secondary": {"used_percent": 12.0, "window_minutes": 10080, "resets_at": 1713000000} + }` + + var limits CodexRateLimits + if err := json.Unmarshal([]byte(jsonData), &limits); err != nil { + t.Fatalf("Failed to parse CodexRateLimits: %v", err) + } + + if limits.Primary == nil { + t.Fatal("Primary rate limit should not be nil") + } + if limits.Primary.UsedPercent != 45.5 { + t.Errorf("Primary.UsedPercent = %f, want 45.5", limits.Primary.UsedPercent) + } + if limits.Primary.WindowMinutes != 300 { + t.Errorf("Primary.WindowMinutes = %d, want 300", limits.Primary.WindowMinutes) + } + if limits.Secondary == nil { + t.Fatal("Secondary rate limit should not be nil") + } + if limits.Secondary.UsedPercent != 12.0 { + t.Errorf("Secondary.UsedPercent = %f, want 12.0", limits.Secondary.UsedPercent) + } +} + +// TestBackwardCompat_CodexSessionJSONLParsing verifies that Codex session +// JSONL file parsing handles the expected format. +func TestBackwardCompat_CodexSessionJSONLParsing(t *testing.T) { + tmpDir := t.TempDir() + sessionPath := filepath.Join(tmpDir, "session.jsonl") + + // Write a minimal Codex session JSONL + lines := `{"type":"event_msg","payload":{"type":"token_count","info":{"total_token_usage":{"input_tokens":1000,"cached_input_tokens":200,"output_tokens":500,"reasoning_output_tokens":100,"total_tokens":1400},"last_token_usage":null},"rate_limits":{"primary":{"used_percent":10.0,"window_minutes":300,"resets_at":1712700000},"secondary":{"used_percent":5.0,"window_minutes":10080,"resets_at":1713000000}}}} +{"type":"event_msg","payload":{"type":"token_count","info":{"total_token_usage":{"input_tokens":2000,"cached_input_tokens":400,"output_tokens":1000,"reasoning_output_tokens":200,"total_tokens":2800},"last_token_usage":null},"rate_limits":{"primary":{"used_percent":20.0,"window_minutes":300,"resets_at":1712700000},"secondary":{"used_percent":10.0,"window_minutes":10080,"resets_at":1713000000}}}} +` + if err := os.WriteFile(sessionPath, []byte(lines), 0644); err != nil { + t.Fatal(err) + } + + c := NewCodexWithPath(tmpDir) + limits, err := c.ParseSessionJSONL(sessionPath) + if err != nil { + t.Fatalf("ParseSessionJSONL: %v", err) + } + + // Should get the LAST entry's rate limits + if limits == nil { + t.Fatal("Expected rate limits from session") + } + if limits.Primary.UsedPercent != 20.0 { + t.Errorf("Primary.UsedPercent = %f, want 20.0 (last entry)", limits.Primary.UsedPercent) + } + + // Token usage parsing + usage, err := c.ParseSessionTokenUsage(sessionPath) + if err != nil { + t.Fatalf("ParseSessionTokenUsage: %v", err) + } + if usage == nil { + t.Fatal("Expected token usage from session") + } + + // Delta: last - first + // Input: 2000-1000=1000, Cached: 400-200=200, Output: 1000-500=500, Reasoning: 200-100=100 + // Billable = (1000-200) + 500 + 100 = 1200 + if usage.InputTokens != 1000 { + t.Errorf("InputTokens = %d, want 1000 (delta)", usage.InputTokens) + } + if usage.OutputTokens != 500 { + t.Errorf("OutputTokens = %d, want 500 (delta)", usage.OutputTokens) + } +} + +// TestBackwardCompat_ProviderExecuteSignature verifies that all providers +// accept the same Execute(ctx, Task) signature. A compilation check. +func TestBackwardCompat_ProviderExecuteSignature(t *testing.T) { + ctx := context.Background() + task := Task{} + + // These calls verify the method signatures haven't changed. + // They will return empty results since providers aren't configured, + // but we're testing the interface, not the implementation. + claude := NewClaudeWithPath(t.TempDir()) + _, _ = claude.Execute(ctx, task) + + codex := NewCodexWithPath(t.TempDir()) + _, _ = codex.Execute(ctx, task) + + copilot := NewCopilotWithPath(t.TempDir()) + _, _ = copilot.Execute(ctx, task) +} diff --git a/internal/tasks/backward_compat_test.go b/internal/tasks/backward_compat_test.go new file mode 100644 index 0000000..9d69ddd --- /dev/null +++ b/internal/tasks/backward_compat_test.go @@ -0,0 +1,305 @@ +package tasks + +import ( + "testing" + "time" +) + +// Backward-compatibility tests for the task registry. +// These tests pin the public API surface so that accidental changes +// to task type strings, categories, cost tiers, or risk levels are +// caught before release. Any change here is a breaking change for +// users who reference task types in config files or scripts. + +// TestBackwardCompat_TaskTypeStringsStable verifies that all 59 built-in +// task type string values have not changed. Users reference these in +// config YAML (tasks.enabled, tasks.disabled, tasks.intervals, etc.), +// so renaming a task type is a breaking change. +func TestBackwardCompat_TaskTypeStringsStable(t *testing.T) { + expected := map[TaskType]string{ + // Category 1: PR + TaskLintFix: "lint-fix", + TaskBugFinder: "bug-finder", + TaskAutoDRY: "auto-dry", + TaskSkillGroom: "skill-groom", + TaskAPIContractVerify: "api-contract-verify", + TaskBackwardCompat: "backward-compat", + TaskBuildOptimize: "build-optimize", + TaskDocsBackfill: "docs-backfill", + TaskCommitNormalize: "commit-normalize", + TaskChangelogSynth: "changelog-synth", + TaskReleaseNotes: "release-notes", + TaskADRDraft: "adr-draft", + TaskTDReview: "td-review", + // Category 2: Analysis + TaskDocDrift: "doc-drift", + TaskSemanticDiff: "semantic-diff", + TaskDeadCode: "dead-code", + TaskDependencyRisk: "dependency-risk", + TaskTestGap: "test-gap", + TaskTestFlakiness: "test-flakiness", + TaskLoggingAudit: "logging-audit", + TaskMetricsCoverage: "metrics-coverage", + TaskPerfRegression: "perf-regression", + TaskCostAttribution: "cost-attribution", + TaskSecurityFootgun: "security-footgun", + TaskPIIScanner: "pii-scanner", + TaskPrivacyPolicy: "privacy-policy", + TaskSchemaEvolution: "schema-evolution", + TaskEventTaxonomy: "event-taxonomy", + TaskRoadmapEntropy: "roadmap-entropy", + TaskBusFactor: "bus-factor", + TaskKnowledgeSilo: "knowledge-silo", + // Category 3: Options + TaskGroomer: "task-groomer", + TaskGuideImprover: "guide-improver", + TaskIdeaGenerator: "idea-generator", + TaskTechDebtClassify: "tech-debt-classify", + TaskWhyAnnotator: "why-annotator", + TaskEdgeCaseEnum: "edge-case-enum", + TaskErrorMsgImprove: "error-msg-improve", + TaskSLOSuggester: "slo-suggester", + TaskUXCopySharpener: "ux-copy-sharpener", + TaskA11yLint: "a11y-lint", + TaskServiceAdvisor: "service-advisor", + TaskOwnershipBoundary: "ownership-boundary", + TaskOncallEstimator: "oncall-estimator", + // Category 4: Safe + TaskMigrationRehearsal: "migration-rehearsal", + TaskContractFuzzer: "contract-fuzzer", + TaskGoldenPath: "golden-path", + TaskPerfProfile: "perf-profile", + TaskAllocationProfile: "allocation-profile", + // Category 5: Map + TaskVisibilityInstrument: "visibility-instrument", + TaskRepoTopology: "repo-topology", + TaskPermissionsMapper: "permissions-mapper", + TaskDataLifecycle: "data-lifecycle", + TaskFeatureFlagMonitor: "feature-flag-monitor", + TaskCISignalNoise: "ci-signal-noise", + TaskHistoricalContext: "historical-context", + // Category 6: Emergency + TaskRunbookGen: "runbook-gen", + TaskRollbackPlan: "rollback-plan", + TaskPostmortemGen: "postmortem-gen", + } + + for taskConst, wantStr := range expected { + if string(taskConst) != wantStr { + t.Errorf("TaskType constant %q has value %q, want %q — renaming task types breaks user configs", + wantStr, string(taskConst), wantStr) + } + } + + // Verify count: if a new task is added, the count increases. + // A decrease means a task was removed (breaking change). + allTypes := AllTaskTypes() + if len(allTypes) < len(expected) { + t.Errorf("AllTaskTypes() returned %d types, expected at least %d — a built-in task type was removed", + len(allTypes), len(expected)) + } +} + +// TestBackwardCompat_TaskCategoryAssignments verifies that each task +// remains in its original category. Changing a task's category affects +// budget allocation (different categories have different default intervals +// and cost expectations). +func TestBackwardCompat_TaskCategoryAssignments(t *testing.T) { + categoryChecks := map[TaskType]TaskCategory{ + TaskLintFix: CategoryPR, + TaskBugFinder: CategoryPR, + TaskAutoDRY: CategoryPR, + TaskSkillGroom: CategoryPR, + TaskAPIContractVerify: CategoryPR, + TaskBackwardCompat: CategoryPR, + TaskBuildOptimize: CategoryPR, + TaskDocsBackfill: CategoryPR, + TaskDocDrift: CategoryAnalysis, + TaskSemanticDiff: CategoryAnalysis, + TaskDeadCode: CategoryAnalysis, + TaskTestGap: CategoryAnalysis, + TaskSecurityFootgun: CategoryAnalysis, + TaskPIIScanner: CategoryAnalysis, + TaskBusFactor: CategoryAnalysis, + TaskGroomer: CategoryOptions, + TaskGuideImprover: CategoryOptions, + TaskA11yLint: CategoryOptions, + TaskMigrationRehearsal: CategorySafe, + TaskContractFuzzer: CategorySafe, + TaskGoldenPath: CategorySafe, + TaskRepoTopology: CategoryMap, + TaskPermissionsMapper: CategoryMap, + TaskVisibilityInstrument: CategoryMap, + TaskRunbookGen: CategoryEmergency, + TaskRollbackPlan: CategoryEmergency, + TaskPostmortemGen: CategoryEmergency, + } + + for taskType, wantCat := range categoryChecks { + def, err := GetDefinition(taskType) + if err != nil { + t.Errorf("GetDefinition(%q) error: %v — task was removed from registry", taskType, err) + continue + } + if def.Category != wantCat { + t.Errorf("Task %q category = %d (%s), want %d (%s) — changing category breaks budget behavior", + taskType, def.Category, def.Category.String(), wantCat, wantCat.String()) + } + } +} + +// TestBackwardCompat_CostTierAssignments verifies that key tasks maintain +// their cost tier. Changing a cost tier affects budget allocation calculations. +func TestBackwardCompat_CostTierAssignments(t *testing.T) { + costChecks := map[TaskType]CostTier{ + TaskLintFix: CostLow, + TaskCommitNormalize: CostLow, + TaskBugFinder: CostHigh, + TaskAutoDRY: CostHigh, + TaskMigrationRehearsal: CostVeryHigh, + TaskContractFuzzer: CostVeryHigh, + } + + for taskType, wantTier := range costChecks { + def, err := GetDefinition(taskType) + if err != nil { + t.Errorf("GetDefinition(%q) error: %v", taskType, err) + continue + } + if def.CostTier != wantTier { + t.Errorf("Task %q CostTier = %s, want %s — changing cost tier affects budget calculations", + taskType, def.CostTier.String(), wantTier.String()) + } + } +} + +// TestBackwardCompat_CostTierTokenRangesStable verifies that the token +// ranges for each cost tier haven't changed. These are used in budget +// allocation and user-facing displays. +func TestBackwardCompat_CostTierTokenRangesStable(t *testing.T) { + ranges := []struct { + tier CostTier + wantMin int + wantMax int + }{ + {CostLow, 10_000, 50_000}, + {CostMedium, 50_000, 150_000}, + {CostHigh, 150_000, 500_000}, + {CostVeryHigh, 500_000, 1_000_000}, + } + + for _, r := range ranges { + min, max := r.tier.TokenRange() + if min != r.wantMin || max != r.wantMax { + t.Errorf("CostTier %s TokenRange() = (%d, %d), want (%d, %d) — token ranges must not change", + r.tier.String(), min, max, r.wantMin, r.wantMax) + } + } +} + +// TestBackwardCompat_CategoryDefaultIntervals verifies that default +// intervals per category haven't changed. Users depend on these for +// scheduling expectations. +func TestBackwardCompat_CategoryDefaultIntervals(t *testing.T) { + intervals := map[TaskCategory]time.Duration{ + CategoryPR: 168 * time.Hour, // 7 days + CategoryAnalysis: 72 * time.Hour, // 3 days + CategoryOptions: 168 * time.Hour, // 7 days + CategorySafe: 336 * time.Hour, // 14 days + CategoryMap: 168 * time.Hour, // 7 days + CategoryEmergency: 720 * time.Hour, // 30 days + } + + for cat, want := range intervals { + got := DefaultIntervalForCategory(cat) + if got != want { + t.Errorf("DefaultIntervalForCategory(%s) = %v, want %v — changing default intervals breaks scheduling", + cat.String(), got, want) + } + } +} + +// TestBackwardCompat_TDReviewDisabledByDefault verifies that td-review +// remains disabled by default, since it requires explicit opt-in. +func TestBackwardCompat_TDReviewDisabledByDefault(t *testing.T) { + def, err := GetDefinition(TaskTDReview) + if err != nil { + t.Fatalf("GetDefinition(td-review): %v", err) + } + if !def.DisabledByDefault { + t.Error("td-review must remain DisabledByDefault — enabling it by default would surprise users") + } +} + +// TestBackwardCompat_CustomTaskRegistration verifies that the custom task +// registration API works the same way as in v0.3.0+. +func TestBackwardCompat_CustomTaskRegistration(t *testing.T) { + t.Cleanup(func() { UnregisterCustom("compat-custom") }) + + def := TaskDefinition{ + Type: "compat-custom", + Category: CategoryAnalysis, + Name: "Compat Test Task", + Description: "Tests backward compat of custom task registration", + CostTier: CostMedium, + RiskLevel: RiskLow, + DefaultInterval: 48 * time.Hour, + } + + // Register must succeed + if err := RegisterCustom(def); err != nil { + t.Fatalf("RegisterCustom() error: %v", err) + } + + // Must be retrievable + got, err := GetDefinition("compat-custom") + if err != nil { + t.Fatalf("GetDefinition() after register: %v", err) + } + if got.Type != def.Type || got.Category != def.Category || got.CostTier != def.CostTier { + t.Errorf("Retrieved definition doesn't match registered: got type=%q cat=%d tier=%d", + got.Type, got.Category, got.CostTier) + } + + // Must appear in AllDefinitions + found := false + for _, d := range AllDefinitions() { + if d.Type == "compat-custom" { + found = true + break + } + } + if !found { + t.Error("Custom task not found in AllDefinitions()") + } + + // IsCustom must return true + if !IsCustom("compat-custom") { + t.Error("IsCustom() should return true for registered custom task") + } + + // Re-registration must fail (duplicate) + if err := RegisterCustom(def); err == nil { + t.Error("RegisterCustom() should fail for duplicate type") + } + + // Collision with built-in must fail + builtIn := TaskDefinition{ + Type: TaskLintFix, + Category: CategoryPR, + Name: "Collision", + Description: "should fail", + CostTier: CostLow, + RiskLevel: RiskLow, + DefaultInterval: 24 * time.Hour, + } + if err := RegisterCustom(builtIn); err == nil { + t.Error("RegisterCustom() should fail when colliding with built-in type") + } + + // Unregister must work + UnregisterCustom("compat-custom") + if IsCustom("compat-custom") { + t.Error("IsCustom() should return false after UnregisterCustom()") + } +}