[Fix] Harden validation, configuration, and adapter boundaries

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 {

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,

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}

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
 }

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)
+	}
+}

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
 	}

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")

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

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
 }

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")

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)
 	}

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 {

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}}