diff --git a/cmd/gitcontribute/main.go b/cmd/gitcontribute/main.go index 8db507a..d65accf 100644 --- a/cmd/gitcontribute/main.go +++ b/cmd/gitcontribute/main.go @@ -22,12 +22,16 @@ import ( var version = "dev" func main() { + os.Exit(run()) +} + +func run() int { if len(os.Args) > 1 && os.Args[1] == "runtime-contract" { if err := runRuntimeContract(os.Args[2:], os.Stdout); err != nil { fmt.Fprintln(os.Stderr, err) - os.Exit(ExitGeneral) + return ExitGeneral } - return + return 0 } ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() @@ -45,7 +49,7 @@ func main() { if err != nil { logger.ErrorContext(ctx, "failed to initialize application", "error", err) fmt.Fprintln(os.Stderr, err) - os.Exit(ExitGeneral) + return ExitGeneral } defer func() { if err := svc.Close(); err != nil { @@ -57,10 +61,11 @@ func main() { c.SetLogger(logger.With("component", "cli")) c.SetTUIRunner(tui.NewRunner(svc, os.Stdin, os.Stdout)) if err := c.Run(ctx, os.Args[1:]); err != nil { - os.Exit(reportCommandError(ctx, logger, os.Stderr, traceID, err)) + return reportCommandError(ctx, logger, os.Stderr, traceID, err) } logger.InfoContext(ctx, "command completed", "trace_id", traceID) + return 0 } func runRuntimeContract(args []string, output io.Writer) error { diff --git a/internal/app/contribution.go b/internal/app/contribution.go index c353148..f7f07ad 100644 --- a/internal/app/contribution.go +++ b/internal/app/contribution.go @@ -396,13 +396,6 @@ func evidenceReviewPriority(e *evidence.Evidence) (int, string) { } } -func diffMatchesOpportunity(diff *WorkspaceDiffResult, inv *investigation.Investigation, opp *investigation.Opportunity) bool { - if diff == nil || inv == nil || opp == nil { - return false - } - return diff.Repo.Owner == inv.Repo.Owner && diff.Repo.Repo == inv.Repo.Repo -} - func draftResult(kind, opportunityID, title, body string, renderedAt time.Time, manifestID string) *cli.DraftResult { return &cli.DraftResult{ OpportunityID: opportunityID, diff --git a/internal/app/mcp_v1.go b/internal/app/mcp_v1.go index 53a40a8..f72e4c9 100644 --- a/internal/app/mcp_v1.go +++ b/internal/app/mcp_v1.go @@ -51,27 +51,6 @@ func (r *MCPReader) SearchRepositories(ctx context.Context, in mcpserver.SearchR return mcpserver.SearchRepositoriesOutput{Query: in.Query, Total: res.Total, Matches: matches, NextCursor: res.NextCursor}, nil } -func repositoryToMCPOutput(repo *corpus.Repository) mcpserver.RepositoryOutput { - return mcpserver.RepositoryOutput{ - Owner: repo.Owner, - Repo: repo.Name, - UpdatedAt: formatTime(repo.SourceUpdatedAt), - Fields: map[string]any{ - "description": repo.Description, - "default_branch": repo.DefaultBranch, - "language": repo.Language, - "license": repo.License, - "topics": repo.Topics, - "stars": repo.Stars, - "watchers": repo.Watchers, - "forks": repo.Forks, - "open_issues": repo.OpenIssues, - "archived": repo.Archived, - "fork": repo.Fork, - }, - } -} - // ThreadByNumber reads an issue or pull request by repository and number only. func (r *MCPReader) ThreadByNumber(ctx context.Context, in mcpserver.ThreadByNumberInput) (mcpserver.ThreadOutput, error) { ref := domain.RepoRef{Owner: in.Owner, Repo: in.Repo} diff --git a/internal/app/upgrade.go b/internal/app/upgrade.go index 5c683e2..d9c9951 100644 --- a/internal/app/upgrade.go +++ b/internal/app/upgrade.go @@ -624,10 +624,12 @@ func readClaudeCommand(path string) (string, []string, error) { return "", nil, errors.New("gitcontribute args are missing from claude config") } args := make([]string, 0, len(argsIn)) - for _, a := range argsIn { - if s, ok := a.(string); ok { - args = append(args, s) + for i, a := range argsIn { + s, ok := a.(string) + if !ok { + return "", nil, fmt.Errorf("gitcontribute args[%d] must be a string", i) } + args = append(args, s) } return command, args, nil } diff --git a/internal/app/upgrade_config_test.go b/internal/app/upgrade_config_test.go new file mode 100644 index 0000000..4bb3080 --- /dev/null +++ b/internal/app/upgrade_config_test.go @@ -0,0 +1,19 @@ +package app + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestReadClaudeCommandRejectsNonStringArguments(t *testing.T) { + path := filepath.Join(t.TempDir(), ".claude.json") + data := `{"mcpServers":{"gitcontribute":{"command":"node","args":["mcp",123]}}}` + if err := os.WriteFile(path, []byte(data), 0600); err != nil { + t.Fatal(err) + } + if _, _, err := readClaudeCommand(path); err == nil || !strings.Contains(err.Error(), "args[1]") { + t.Fatalf("readClaudeCommand error = %v, want indexed non-string argument error", err) + } +} diff --git a/internal/codeindex/codeindex.go b/internal/codeindex/codeindex.go index 161c290..ba9e8e6 100644 --- a/internal/codeindex/codeindex.go +++ b/internal/codeindex/codeindex.go @@ -225,6 +225,14 @@ func normalizeOptions(opts Options) (Options, error) { if opts.MaxTotalBytes > hardMaxTotalBytes { return Options{}, fmt.Errorf("max total bytes exceeds hard limit %d", hardMaxTotalBytes) } + for _, pattern := range opts.Exclusions { + if pattern == "" { + continue + } + if _, err := path.Match(pattern, ""); err != nil { + return Options{}, fmt.Errorf("invalid exclusion pattern %q: %w", pattern, err) + } + } if opts.MaxFiles == 0 { opts.MaxFiles = defaultMaxFiles } diff --git a/internal/codeindex/codeindex_test.go b/internal/codeindex/codeindex_test.go index f76c391..074e4b2 100644 --- a/internal/codeindex/codeindex_test.go +++ b/internal/codeindex/codeindex_test.go @@ -345,6 +345,16 @@ func TestIndexExclusions(t *testing.T) { } } +func TestIndexRejectsMalformedExclusionPattern(t *testing.T) { + repo := newRepo(t) + writeFile(t, repo, "keep.go", "package main\n") + commitAll(t, repo, "initial") + + if _, err := Index(context.Background(), repo, Options{Exclusions: []string{"["}}); err == nil { + t.Fatal("Index accepted malformed exclusion pattern") + } +} + func TestIndexCancellation(t *testing.T) { repo := newRepo(t) writeFile(t, repo, "a.txt", "a\n") diff --git a/internal/corpus/jobs.go b/internal/corpus/jobs.go index e37ab97..a2598b5 100644 --- a/internal/corpus/jobs.go +++ b/internal/corpus/jobs.go @@ -9,6 +9,7 @@ import ( "time" "github.com/google/uuid" + "modernc.org/sqlite" ) // ErrJobCancelled is returned when a terminal transition is blocked because a @@ -304,7 +305,7 @@ func (c *Corpus) ReconcileInterruptedJobs(ctx context.Context, leaseTimeout time } defer conn.Close() - if _, err := conn.ExecContext(ctx, "BEGIN IMMEDIATE"); err != nil { + if err := beginReconcileTransaction(ctx, conn); err != nil { return fmt.Errorf("begin reconcile jobs: %w", err) } committed := false @@ -389,6 +390,42 @@ func (c *Corpus) ReconcileInterruptedJobs(ctx context.Context, leaseTimeout time return nil } +const ( + reconcileBusyTimeout = 100 * time.Millisecond + reconcileBeginTries = 20 + reconcileRetryDelay = 25 * time.Millisecond +) + +func beginReconcileTransaction(ctx context.Context, conn *sql.Conn) error { + if _, err := conn.ExecContext(ctx, fmt.Sprintf("PRAGMA busy_timeout = %d", reconcileBusyTimeout.Milliseconds())); err != nil { + return fmt.Errorf("configure reconcile busy timeout: %w", err) + } + + var lastErr error + for attempt := 0; attempt < reconcileBeginTries; attempt++ { + _, err := conn.ExecContext(ctx, "BEGIN IMMEDIATE") + if err == nil { + return nil + } + lastErr = err + if !isSQLiteBusy(err) { + return err + } + + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(reconcileRetryDelay): + } + } + return lastErr +} + +func isSQLiteBusy(err error) bool { + var sqliteErr *sqlite.Error + return errors.As(err, &sqliteErr) && sqliteErr.Code()&0xff == 5 +} + const jobSelect = ` SELECT id, kind, status, request, result, error, progress, statistics, created_at, started_at, completed_at, updated_at, cancelled_at diff --git a/internal/evidence/mcp_runner.go b/internal/evidence/mcp_runner.go index 167a55f..c478f65 100644 --- a/internal/evidence/mcp_runner.go +++ b/internal/evidence/mcp_runner.go @@ -58,7 +58,9 @@ func (r *MCPStdioRunner) Run(ctx context.Context, req RunRequest) (*RunResult, e // #nosec G204 -- argv is a stored shell-free validation command whose execution requires explicit authorization. cmd := exec.CommandContext(ctx, req.Args[0], req.Args[1:]...) cmd.Dir = req.Dir - cmd.Env = req.Env + if req.Env != nil { + cmd.Env = req.Env + } cmd.Stderr = stderr configureCommandCancellation(cmd) transport := &mcp.CommandTransport{Command: cmd, TerminateDuration: mcpShutdownGrace} @@ -144,7 +146,7 @@ func finishProtocolResult( if closer != nil { shutdownTimedOut, closeErr = closeProtocol(ctx, closer, cmd) } - if closeErr != nil && runErr == nil { + if closeErr != nil && runErr == nil && !isExpectedMCPShutdownError(closeErr) { runErr = fmt.Errorf("close MCP session: %w", closeErr) classification = RunClassificationError failurePhase = "shutdown" @@ -172,6 +174,11 @@ func finishProtocolResult( } } +func isExpectedMCPShutdownError(err error) bool { + var exitErr *exec.ExitError + return errors.As(err, &exitErr) && exitErr.ProcessState != nil && exitErr.ExitCode() == -1 +} + type protocolCloser interface { Close() error } diff --git a/internal/evidence/mcp_runner_test.go b/internal/evidence/mcp_runner_test.go index acf1b27..9ff2c0d 100644 --- a/internal/evidence/mcp_runner_test.go +++ b/internal/evidence/mcp_runner_test.go @@ -35,11 +35,12 @@ func TestMCPStdioHelper(_ *testing.T) { } func TestMCPStdioRunnerRecordsSDKMilestones(t *testing.T) { + t.Setenv("GITCONTRIBUTE_MCP_HELPER", "1") ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() result, err := NewMCPStdioRunner().Run(ctx, RunRequest{ Args: []string{os.Args[0], "-test.run=^TestMCPStdioHelper$"}, Dir: t.TempDir(), - Env: []string{"GITCONTRIBUTE_MCP_HELPER=1"}, MaxOutputBytes: 4096, + Env: nil, MaxOutputBytes: 4096, ReadinessTimeout: time.Second, SampleInterval: 10 * time.Millisecond, }) if err != nil { @@ -56,6 +57,18 @@ func TestMCPStdioRunnerRecordsSDKMilestones(t *testing.T) { } } +func TestExpectedMCPShutdownSignalIsNonFatal(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("signal exit status is POSIX-specific") + } + cmd := exec.Command("sh", "-c", "kill -TERM $$") + if err := cmd.Run(); err == nil { + t.Fatal("signal helper exited successfully") + } else if !isExpectedMCPShutdownError(err) { + t.Fatalf("shutdown error = %v, want expected signal shutdown", err) + } +} + func TestMCPStdioRunnerClassifiesReadinessDeadline(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("shell fixture is POSIX-only") diff --git a/internal/evidence/runner_test.go b/internal/evidence/runner_test.go index 452071f..4160aa8 100644 --- a/internal/evidence/runner_test.go +++ b/internal/evidence/runner_test.go @@ -144,9 +144,11 @@ func TestExecRunnerCapturesProcessTreeTelemetryAndCleanup(t *testing.T) { } sh := findOrSkip(t, "sh") r := NewExecRunner() - ctx, cancel := context.WithTimeout(context.Background(), 250*time.Millisecond) + ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) defer cancel() - res, err := r.Run(ctx, RunRequest{Args: []string{sh, "-c", "sleep 10 & wait"}, Dir: t.TempDir()}) + res, err := r.Run(ctx, RunRequest{ + Args: []string{sh, "-c", "sleep 10 & wait"}, Dir: t.TempDir(), SampleInterval: 10 * time.Millisecond, + }) if err != nil { t.Fatal(err) } diff --git a/internal/evidence/service.go b/internal/evidence/service.go index 7571fc4..a5f2dbd 100644 --- a/internal/evidence/service.go +++ b/internal/evidence/service.go @@ -178,6 +178,9 @@ func normalizeEnvironmentAllowlist(names []string) ([]string, error) { if len(names) > maxEnvironmentVariables { return nil, fmt.Errorf("%w: at most %d variable names are allowed", ErrInvalidEnvironment, maxEnvironmentVariables) } + if len(names) == 0 { + return nil, nil + } seen := make(map[string]struct{}, len(names)) out := make([]string, 0, len(names)) for _, name := range names { @@ -194,6 +197,9 @@ func normalizeEnvironmentAllowlist(names []string) ([]string, error) { } func resolveEnvironment(names []string) []string { + if len(names) == 0 { + return nil + } env := make([]string, 0, len(names)) for _, name := range names { if value, exists := os.LookupEnv(name); exists { diff --git a/internal/evidence/service_test.go b/internal/evidence/service_test.go index 0e2fe76..686daeb 100644 --- a/internal/evidence/service_test.go +++ b/internal/evidence/service_test.go @@ -383,6 +383,22 @@ func TestRunValidationResolvesEnvironmentAllowlistAtExecution(t *testing.T) { } } +func TestRunValidationInheritsEnvironmentWithEmptyAllowlist(t *testing.T) { + repo := newFakeRepo() + runner := &capturingRunner{result: &RunResult{Classification: RunClassificationPassing}} + svc := NewService(repo, runner) + def := &ValidationDefinition{ID: "def", Command: []string{"test"}, WorkingDir: "/tmp/ws"} + if err := svc.DefineValidation(context.Background(), def); err != nil { + t.Fatal(err) + } + if _, err := svc.RunValidation(context.Background(), def.ID, RunKindBase); err != nil { + t.Fatal(err) + } + if runner.request.Env != nil { + t.Fatalf("execution environment = %#v, want nil for inherited environment", runner.request.Env) + } +} + func TestRunValidationPassesOutputBound(t *testing.T) { repo := newFakeRepo() runner := &capturingRunner{result: &RunResult{Classification: RunClassificationPassing}} diff --git a/internal/github/client.go b/internal/github/client.go index 58ea7bf..8bf5aa1 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -195,9 +195,7 @@ func (c *Client) ListIssueTimeline(ctx context.Context, owner, name string, numb event.SourceIsPullRequest = issue.IsPullRequest() if repository := issue.GetRepository(); repository != nil { event.SourceRepository = repository.GetName() - if repository.Owner != nil { - event.SourceOwner = repository.Owner.GetLogin() - } + event.SourceOwner = userLogin(repository.Owner) } } } @@ -461,34 +459,6 @@ func pageInfo(resp *gh.Response) PageInfo { return p } -func stringVal(p *string) string { - if p == nil { - return "" - } - return *p -} - -func intVal(p *int) int { - if p == nil { - return 0 - } - return *p -} - -func int64Val(p *int64) int64 { - if p == nil { - return 0 - } - return *p -} - -func boolVal(p *bool) bool { - if p == nil { - return false - } - return *p -} - func timeVal(t *gh.Timestamp) time.Time { if t == nil { return time.Time{} @@ -530,6 +500,27 @@ func userLogins(users []*gh.User) []string { return out } +func userLogin(user *gh.User) string { + if user == nil { + return "" + } + return user.GetLogin() +} + +func milestoneTitle(milestone *gh.Milestone) string { + if milestone == nil { + return "" + } + return milestone.GetTitle() +} + +func licenseName(license *gh.License) string { + if license == nil { + return "" + } + return license.GetName() +} + func convertRepository(r *gh.Repository) Repository { if r == nil { return Repository{} @@ -541,7 +532,7 @@ func convertRepository(r *gh.Repository) Repository { return Repository{ ID: r.GetID(), NodeID: r.GetNodeID(), - Owner: r.Owner.GetLogin(), + Owner: userLogin(r.Owner), Name: r.GetName(), FullName: r.GetFullName(), Description: r.GetDescription(), @@ -556,7 +547,7 @@ func convertRepository(r *gh.Repository) Repository { Forks: r.GetForksCount(), OpenIssues: openIssues, Language: r.GetLanguage(), - License: r.License.GetName(), + License: licenseName(r.License), Topics: r.Topics, CreatedAt: timeVal(r.CreatedAt), UpdatedAt: timeVal(r.UpdatedAt), @@ -588,11 +579,11 @@ func convertIssue(i *gh.Issue) Issue { StateReason: i.GetStateReason(), Draft: i.GetDraft(), Locked: i.GetLocked(), - Author: i.User.GetLogin(), + Author: userLogin(i.User), AuthorAssociation: i.GetAuthorAssociation(), Labels: labelNames(i.Labels), Assignees: userLogins(i.Assignees), - Milestone: i.Milestone.GetTitle(), + Milestone: milestoneTitle(i.Milestone), CommentsCount: i.GetComments(), CreatedAt: timeVal(i.CreatedAt), UpdatedAt: timeVal(i.UpdatedAt), @@ -618,7 +609,7 @@ func convertIssueComment(c *gh.IssueComment) IssueComment { ID: c.GetID(), NodeID: c.GetNodeID(), Body: c.GetBody(), - Author: c.User.GetLogin(), + Author: userLogin(c.User), AuthorAssociation: c.GetAuthorAssociation(), CreatedAt: timeVal(c.CreatedAt), UpdatedAt: timeVal(c.UpdatedAt), @@ -650,11 +641,11 @@ func convertPullRequestDetails(pr *gh.PullRequest) PullRequestDetails { Body: pr.GetBody(), Draft: pr.GetDraft(), Locked: pr.GetLocked(), - Author: pr.User.GetLogin(), + Author: userLogin(pr.User), AuthorAssociation: pr.GetAuthorAssociation(), Labels: labelNames(pr.Labels), Assignees: userLogins(pr.Assignees), - Milestone: pr.Milestone.GetTitle(), + Milestone: milestoneTitle(pr.Milestone), CreatedAt: timeVal(pr.CreatedAt), UpdatedAt: timeVal(pr.UpdatedAt), ClosedAt: timePtr(pr.ClosedAt), @@ -684,7 +675,7 @@ func convertReview(r *gh.PullRequestReview) Review { NodeID: r.GetNodeID(), State: r.GetState(), Body: r.GetBody(), - Author: r.User.GetLogin(), + Author: userLogin(r.User), AuthorAssociation: r.GetAuthorAssociation(), CommitID: r.GetCommitID(), SubmittedAt: timeVal(r.SubmittedAt), @@ -704,7 +695,7 @@ func convertReviewComment(c *gh.PullRequestComment) ReviewComment { Body: c.GetBody(), Path: c.GetPath(), DiffHunk: c.GetDiffHunk(), - Author: c.User.GetLogin(), + Author: userLogin(c.User), AuthorAssociation: c.GetAuthorAssociation(), CommitID: c.GetCommitID(), OriginalCommitID: c.GetOriginalCommitID(), diff --git a/internal/github/conversion_test.go b/internal/github/conversion_test.go new file mode 100644 index 0000000..357c216 --- /dev/null +++ b/internal/github/conversion_test.go @@ -0,0 +1,28 @@ +package github + +import ( + "testing" + + gh "github.com/google/go-github/v89/github" +) + +func TestConversionsHandleMissingNestedModels(t *testing.T) { + if got := convertRepository(&gh.Repository{}); got.Owner != "" || got.License != "" { + t.Fatalf("repository = %+v, want empty nested fields", got) + } + if got := convertIssue(&gh.Issue{}); got.Author != "" || got.Milestone != "" { + t.Fatalf("issue = %+v, want empty nested fields", got) + } + if got := convertIssueComment(&gh.IssueComment{}); got.Author != "" { + t.Fatalf("issue comment = %+v, want empty author", got) + } + if got := convertPullRequestDetails(&gh.PullRequest{}); got.Author != "" || got.Milestone != "" { + t.Fatalf("pull request = %+v, want empty nested fields", got) + } + if got := convertReview(&gh.PullRequestReview{}); got.Author != "" { + t.Fatalf("review = %+v, want empty author", got) + } + if got := convertReviewComment(&gh.PullRequestComment{}); got.Author != "" { + t.Fatalf("review comment = %+v, want empty author", got) + } +} diff --git a/internal/mcpserver/server.go b/internal/mcpserver/server.go index b6e8dd4..6e0e059 100644 --- a/internal/mcpserver/server.go +++ b/internal/mcpserver/server.go @@ -613,28 +613,6 @@ func (s *Server) register() { func boolPtr(v bool) *bool { return &v } -func (s *Server) repository(ctx context.Context, _ *mcp.CallToolRequest, in RepoInput) (*mcp.CallToolResult, RepositoryOutput, error) { - if err := validateRepo(in); err != nil { - return nil, RepositoryOutput{}, err - } - out, err := s.reader.Repository(ctx, in) - return nil, out, err -} - -func (s *Server) thread(ctx context.Context, _ *mcp.CallToolRequest, in ThreadInput) (*mcp.CallToolResult, ThreadOutput, error) { - if err := validateRepo(RepoInput{Owner: in.Owner, Repo: in.Repo}); err != nil { - return nil, ThreadOutput{}, err - } - if in.Kind != "issue" && in.Kind != "pull_request" { - return nil, ThreadOutput{}, InvalidArgument("kind", "must be issue or pull_request", map[string]any{"kind": "issue"}) - } - if in.Number < 1 { - return nil, ThreadOutput{}, InvalidArgument("number", "must be positive", map[string]any{"number": 1}) - } - out, err := s.reader.Thread(ctx, in) - return nil, out, err -} - func (s *Server) searchCode(ctx context.Context, _ *mcp.CallToolRequest, in SearchCodeInput) (*mcp.CallToolResult, SearchCodeOutput, error) { if in.Query == "" { return nil, SearchCodeOutput{}, InvalidArgument("query", "is required", map[string]any{"query": "MIDI"}) diff --git a/internal/mcpserver/v1.go b/internal/mcpserver/v1.go index ec1a55b..ce87333 100644 --- a/internal/mcpserver/v1.go +++ b/internal/mcpserver/v1.go @@ -3,7 +3,6 @@ package mcpserver import ( "context" "errors" - "strconv" "strings" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -451,16 +450,6 @@ func (s *Server) explainMatch(ctx context.Context, _ *mcp.CallToolRequest, in Ex return nil, out, err } -func (s *Server) getJob(ctx context.Context, _ *mcp.CallToolRequest, in GetJobInput) (*mcp.CallToolResult, GetJobOutput, error) { - id, err := normalizeID("id", in.ID) - if err != nil { - return nil, GetJobOutput{}, err - } - in.ID = id - out, err := s.reader.GetJob(ctx, in) - return nil, out, err -} - func (s *Server) buildRepositoryDossier(ctx context.Context, _ *mcp.CallToolRequest, in BuildRepositoryDossierInput) (*mcp.CallToolResult, JobReference, error) { if err := validateRepo(RepoInput(in)); err != nil { return nil, JobReference{}, err @@ -589,14 +578,3 @@ func (s *Server) cancelJob(ctx context.Context, _ *mcp.CallToolRequest, in Cance out, err := operator.CancelJobs(ctx, in) return nil, out, err } - -func parsePositiveNumber(parts []string, idx int) (int, error) { - if len(parts) <= idx { - return 0, errors.New("missing path segment") - } - n, err := strconv.Atoi(parts[idx]) - if err != nil || n < 1 { - return 0, errors.New("invalid positive number") - } - return n, nil -} diff --git a/internal/setup/setup.go b/internal/setup/setup.go index b708bc1..9bcb6a0 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -144,9 +144,22 @@ func CheckRegistration(client Client, home string) (bool, string, error) { if err := json.Unmarshal(data, &root); err != nil { return false, path, err } - servers, _ := root["mcpServers"].(map[string]any) - _, present := servers[serverName] - return present, path, nil + rawServers, present := root["mcpServers"] + if !present { + return false, path, nil + } + servers, ok := rawServers.(map[string]any) + if !ok { + return false, path, errors.New("mcpServers must be an object in claude config") + } + rawServer, present := servers[serverName] + if !present { + return false, path, nil + } + if _, ok := rawServer.(map[string]any); !ok { + return false, path, errors.New("gitcontribute server must be an object in claude config") + } + return true, path, nil default: return false, "", fmt.Errorf("unsupported setup client %q", client) } diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 9609ce9..d23d62e 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -185,6 +185,27 @@ func TestDetect(t *testing.T) { } } +func TestCheckRegistrationRejectsMalformedClaudeShapes(t *testing.T) { + for _, tt := range []struct { + name string + data string + }{ + {name: "servers array", data: `{"mcpServers": []}`}, + {name: "server scalar", data: `{"mcpServers": {"gitcontribute": 123}}`}, + } { + t.Run(tt.name, func(t *testing.T) { + home := t.TempDir() + if err := os.WriteFile(filepath.Join(home, ".claude.json"), []byte(tt.data), 0600); err != nil { + t.Fatal(err) + } + registered, _, err := CheckRegistration(Claude, home) + if err == nil || registered { + t.Fatalf("registration = %t, error = %v; want invalid registration", registered, err) + } + }) + } +} + func TestMalformedClientConfigFailsWithoutOverwrite(t *testing.T) { home := t.TempDir() path := filepath.Join(home, ".codex", "config.toml")