refactor: stabilize Go tests and adapters

.golangci.yml

@@ -90,25 +90,6 @@ linters:
       - LICENSES
       - third_party$
     rules:
-      - path: _test\.go$
-        linters:
-          - funlen
-          - gocognit
-          - dupl
-          - errcheck
-          - noctx
-          - contextcheck
-          - gosec
-          - errname
-          - errorlint
-          - bodyclose
-          - cyclop
-          - nilnil
-          - perfsprint
-          - prealloc
-          - revive
-          - staticcheck
-          - unconvert
       # These boundaries intentionally create/close context-owning services;
       # contextcheck cannot model their lifecycle contracts.
       - path: ^cmd/gitcontribute/main\.go$|^internal/app/(app|discovery|hydration|jobs)\.go$
@@ -133,3 +114,104 @@ linters:
       - path: ^internal/corpus/(frontier|jobs|tracking)\.go$
         linters:
           - sqlclosecheck
+      # These tests intentionally model raw RoundTripper responses and use
+      # repeated status-specific fixtures; the production transport owns the
+      # response lifecycle and the duplicated fixtures keep each rate-limit
+      # contract readable.
+      - path: ^internal/github/(retry|retry_replay)_test\.go$
+        linters:
+          - bodyclose
+          - dupl
+          - noctx
+      # MCP subprocess fixtures own their service lifetime inside the test
+      # process and deliberately use the convenience constructor.
+      - path: ^internal/app/mcp_stdio_e2e_test\.go$
+        linters:
+          - contextcheck
+      # These test helpers intentionally use the project convenience
+      # constructor and close services at the test boundary.
+      - path: ^internal/app/read_boundary_test\.go$
+        linters:
+          - contextcheck
+      # These are subprocess and filesystem boundary harnesses. Their command
+      # paths and permissions are test-controlled, while the production
+      # adapters remain subject to the security linters.
+      - path: ^internal/app/mcp_stdio_e2e_test\.go$|^internal/app/tui_capture_test\.go$|^internal/app/app_test\.go$|^internal/acquire/acquire_test\.go$|^internal/codeindex/codeindex_test\.go$|^internal/workspace/workspace_test\.go$|^internal/corpus/lifecycle_test\.go$|^internal/app/setup_verification_test\.go$|^internal/managedbinary/install_test\.go$|^internal/tui/snapshot_test\.go$
+        linters:
+          - gosec
+          - noctx
+      # These compact fakes and HTTP fixtures intentionally omit unrelated
+      # arguments or return zero values for capabilities under test elsewhere.
+      - path: ^internal/app/hydration_test\.go$|^internal/app/job_executor_reconciliation_test\.go$|^internal/discovery/gharchive_fetcher_test\.go$|^internal/discovery/search_test\.go$|^internal/github/client_test\.go$|^internal/tui/tui_test\.go$
+        linters:
+          - revive
+          - errcheck
+          - dupl
+      - path: ^internal/app/job_executor_test\.go$|^internal/github/retry_test\.go$
+        linters:
+          - revive
+      - path: ^internal/app/job_executor_test\.go$|^internal/cli/cli_test\.go$
+        linters:
+          - nilnil
+      # These local test servers and teardown-only writes are intentionally
+      # best effort; their assertions cover the observable operation.
+      - path: ^internal/app/app_test\.go$|^internal/app/control_test\.go$|^internal/app/corpus_lifecycle_test\.go$|^internal/app/discovery_test\.go$|^internal/app/mcp_stdio_e2e_test\.go$|^internal/app/setup_verification_test\.go$|^internal/buflimit/buflimit_test\.go$|^internal/cli/setup_prompt_internal_test\.go$|^internal/corpus/lifecycle_test\.go$|^internal/corpus/tracking_test\.go$|^internal/discovery/gharchive_fetcher_test\.go$|^internal/discovery/gharchive_test\.go$|^internal/github/pull_request_workflows_test\.go$|^internal/log/log_test\.go$
+        linters:
+          - errcheck
+      # These test handlers and nested JSON assertions are fixture plumbing;
+      # their surrounding behavior is asserted by the test cases.
+      - path: ^internal/app/guidance_test\.go$|^internal/app/mcp_github_acquisition_test\.go$|^internal/app/read_boundary_test\.go$|^internal/app/setup_test\.go$|^internal/tracking/sanitize_test\.go$
+        linters:
+          - errcheck
+      - path: ^internal/app/commitplan_test\.go$|^internal/app/control_test\.go$|^internal/tui/snapshot_test\.go$
+        linters:
+          - gosec
+      - path: ^internal/app/setup_test\.go$|^internal/buflimit/buflimit_test\.go$|^internal/cli/surfaces_test\.go$|^internal/discovery/gharchive_fetcher_test\.go$|^internal/log/log_test\.go$
+        linters:
+          - gosec
+      - path: ^internal/app/tui_test\.go$|^internal/tui/actions_test\.go$
+        linters:
+          - errcheck
+      - path: ^internal/tui/snapshot_test\.go$
+        linters:
+          - errcheck
+      - path: ^internal/cli/extended_test\.go$
+        linters:
+          - revive
+      - path: ^internal/app/sync_metadata_test\.go$
+        linters:
+          - revive
+      # These final fixture-only writes use executable or world-readable
+      # permissions to emulate installed artifacts and serialized outputs.
+      - path: ^internal/app/discovery_test\.go$|^internal/app/surfaces_test\.go$|^internal/app/upgrade_activation_test\.go$|^internal/app/upgrade_registration_test\.go$|^internal/github/client_test\.go$
+        linters:
+          - gosec
+      # Type assertions in these protocol fixtures are the assertions under
+      # test; failure is reported by the surrounding test assertion.
+      - path: ^cmd/gitcontribute/main_test\.go$|^internal/mcpserver/server_test\.go$|^internal/setup/setup_test\.go$
+        linters:
+          - errcheck
+      - path: ^internal/app/guidance_test\.go$|^internal/app/upgrade_setup_test\.go$|^internal/app/upgrade_test\.go$|^internal/corpus/tracking_test\.go$
+        linters:
+          - gosec
+      - path: ^internal/cli/surfaces_test\.go$
+        linters:
+          - revive
+      - path: ^internal/app/mcp_pr_workflows_test\.go$|^internal/app/mcp_thread_facets_test\.go$
+        linters:
+          - errcheck
+      - path: ^internal/app/mcp_github_acquisition_test\.go$
+        linters:
+          - gosec
+      - path: ^internal/cli/cli_test\.go$
+        linters:
+          - revive
+      - path: ^internal/app/surfaces_test\.go$
+        linters:
+          - errcheck
+      - path: ^internal/discovery/gharchive_test\.go$
+        linters:
+          - gosec
+      - path: ^internal/app/upgrade_test\.go$|^internal/evidence/mcp_runner_test\.go$
+        linters:
+          - noctx

internal/acquire/acquire_test.go

@@ -218,7 +218,7 @@ func TestWriteMetadataAtomicallyWithPrivatePermissions(t *testing.T) {
 
 func runGit(t *testing.T, dir string, args ...string) string {
 	t.Helper()
-	cmd := exec.Command("git", append([]string{"--no-pager"}, args...)...)
+	cmd := exec.CommandContext(context.Background(), "git", append([]string{"--no-pager"}, args...)...)
 	cmd.Dir = dir
 	cmd.Env = append(os.Environ(),
 		"GIT_TERMINAL_PROMPT=0",
@@ -295,7 +295,7 @@ func TestAcquireMirrorLockCancelsWhileHeld(t *testing.T) {
 	if err != nil || !ok {
 		t.Fatalf("failed to hold test lock: ok=%v err=%v", ok, err)
 	}
-	defer fl.Close()
+	defer func() { _ = fl.Close() }()
 
 	mgr, err := NewManager(root, nil)
 	if err != nil {

internal/app/app_test.go

@@ -690,7 +690,7 @@ func setupAppGitRemote(t *testing.T) (remoteURL, baseSHA, candidateSHA string) {
 
 func runGitApp(t *testing.T, dir string, args ...string) string {
 	t.Helper()
-	cmd := exec.Command("git", append([]string{"--no-pager"}, args...)...)
+	cmd := exec.CommandContext(context.Background(), "git", append([]string{"--no-pager"}, args...)...)
 	cmd.Dir = dir
 	cmd.Env = append(os.Environ(),
 		"GIT_TERMINAL_PROMPT=0",

internal/app/control_test.go

@@ -345,6 +345,7 @@ func TestDoctorReportsOlderRegistrationWhenNewerPrivateRuntimeIsInstalled(t *tes
 	if err != nil {
 		t.Fatal(err)
 	}
+	var registeredPath string
 	for _, version := range []string{"0.15.0", "0.16.0"} {
 		path, err := managedbinary.Destination(dataDir, version)
 		if err != nil {
@@ -356,8 +357,11 @@ func TestDoctorReportsOlderRegistrationWhenNewerPrivateRuntimeIsInstalled(t *tes
 		if err := os.WriteFile(path, []byte(version), 0o755); err != nil {
 			t.Fatal(err)
 		}
+		if version == "0.15.0" {
+			registeredPath = path
+		}
 	}
-	writeCodexConfig(t, home, filepath.Join(dataDir, "bin", "0.15.0", "gitcontribute"))
+	writeCodexConfig(t, home, registeredPath)
 
 	result, err := svc.Doctor(context.Background())
 	if err != nil {

internal/app/corpus_lifecycle_test.go

@@ -94,10 +94,10 @@ func TestSetupFailsFastForUnmarkedCorpusInDryRunAndRealModes(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if _, err := db.Exec("CREATE TABLE goose_db_version (id INTEGER PRIMARY KEY, version_id INTEGER)"); err != nil {
+	if _, err := db.ExecContext(context.Background(), "CREATE TABLE goose_db_version (id INTEGER PRIMARY KEY, version_id INTEGER)"); err != nil {
 		t.Fatal(err)
 	}
-	if _, err := db.Exec("INSERT INTO goose_db_version (id, version_id) VALUES (1, 9999)"); err != nil {
+	if _, err := db.ExecContext(context.Background(), "INSERT INTO goose_db_version (id, version_id) VALUES (1, 9999)"); err != nil {
 		t.Fatal(err)
 	}
 	if err := db.Close(); err != nil {

internal/app/hydration_test.go

@@ -41,15 +41,15 @@ func (f *fakeHydrationReader) ListIssueTimeline(_ context.Context, _, _ string,
 	return github.ListResult[github.IssueTimelineEvent]{Items: f.issueTimelinePages[idx], Page: page}, nil
 }
 
-func (f *fakeHydrationReader) GetRepository(ctx context.Context, owner, name string) (github.Repository, github.RateInfo, error) {
+func (f *fakeHydrationReader) GetRepository(_ context.Context, owner, name string) (github.Repository, github.RateInfo, error) {
 	return github.Repository{Owner: owner, Name: name, NodeID: "R_1", UpdatedAt: time.Now()}, github.RateInfo{}, nil
 }
 
-func (f *fakeHydrationReader) ListIssues(ctx context.Context, owner, name string, opts github.ListIssueOptions) (github.ListResult[github.Issue], error) {
+func (f *fakeHydrationReader) ListIssues(_ context.Context, owner, name string, opts github.ListIssueOptions) (github.ListResult[github.Issue], error) {
 	return github.ListResult[github.Issue]{}, nil
 }
 
-func (f *fakeHydrationReader) ListIssueComments(ctx context.Context, owner, name string, issueNumber int, opts github.PageOptions) (github.ListResult[github.IssueComment], error) {
+func (f *fakeHydrationReader) ListIssueComments(_ context.Context, owner, name string, issueNumber int, opts github.PageOptions) (github.ListResult[github.IssueComment], error) {
 	if f.failWith != nil && f.issueCommentsCalls >= f.failAfterIssueCalls {
 		return github.ListResult[github.IssueComment]{}, f.failWith
 	}
@@ -66,14 +66,14 @@ func (f *fakeHydrationReader) ListIssueComments(ctx context.Context, owner, name
 	return github.ListResult[github.IssueComment]{Items: f.issueCommentsPages[idx], Page: page}, nil
 }
 
-func (f *fakeHydrationReader) GetPullRequestDetails(ctx context.Context, owner, name string, number int) (github.PullRequestDetails, github.RateInfo, error) {
+func (f *fakeHydrationReader) GetPullRequestDetails(_ context.Context, owner, name string, number int) (github.PullRequestDetails, github.RateInfo, error) {
 	if f.failWith != nil {
 		return github.PullRequestDetails{}, github.RateInfo{}, f.failWith
 	}
 	return f.prDetails, github.RateInfo{}, nil
 }
 
-func (f *fakeHydrationReader) ListPullRequestReviews(ctx context.Context, owner, name string, number int, opts github.PageOptions) (github.ListResult[github.Review], error) {
+func (f *fakeHydrationReader) ListPullRequestReviews(_ context.Context, owner, name string, number int, opts github.PageOptions) (github.ListResult[github.Review], error) {
 	if f.failWith != nil && f.prReviewsCalls >= f.failAfterIssueCalls {
 		return github.ListResult[github.Review]{}, f.failWith
 	}
@@ -90,7 +90,7 @@ func (f *fakeHydrationReader) ListPullRequestReviews(ctx context.Context, owner,
 	return github.ListResult[github.Review]{Items: f.prReviewsPages[idx], Page: page}, nil
 }
 
-func (f *fakeHydrationReader) ListPullRequestComments(ctx context.Context, owner, name string, number int, opts github.PageOptions) (github.ListResult[github.ReviewComment], error) {
+func (f *fakeHydrationReader) ListPullRequestComments(_ context.Context, owner, name string, number int, opts github.PageOptions) (github.ListResult[github.ReviewComment], error) {
 	if f.failWith != nil && f.prReviewCommentsCalls >= f.failAfterIssueCalls {
 		return github.ListResult[github.ReviewComment]{}, f.failWith
 	}
@@ -365,7 +365,7 @@ func TestHydrateBoundsPagination(t *testing.T) {
 	repo, thread := seedRepoAndThread(t, svc, corpus.ThreadKindIssue, 1)
 	pages := make([][]github.IssueComment, 10)
 	for i := range pages {
-		pages[i] = []github.IssueComment{{ID: int64(i + 1), UpdatedAt: time.Date(2024, 1, 1, 0, 0, int(i), 0, time.UTC)}}
+		pages[i] = []github.IssueComment{{ID: int64(i + 1), UpdatedAt: time.Date(2024, 1, 1, 0, 0, i, 0, time.UTC)}}
 	}
 	reader := &fakeHydrationReader{issueCommentsPages: pages}
 	svc.SetGitHubReader(reader)

internal/app/job_executor_lifecycle_test.go

@@ -35,7 +35,7 @@ func TestJobExecutorCloseCancelsAndWaits(t *testing.T) {
 	}
 
 	started := make(chan struct{})
-	id, err := jobs.Submit(ctx, "block", nil, func(ctx context.Context, report func(progress, statistics string) error) (any, error) {
+	id, err := jobs.Submit(ctx, "block", nil, func(ctx context.Context, _ func(progress, statistics string) error) (any, error) {
 		close(started)
 		<-ctx.Done()
 		return nil, ctx.Err()

internal/app/job_executor_test.go

@@ -271,7 +271,7 @@ func TestJobCancellation(t *testing.T) {
 	}
 
 	blocked := make(chan struct{})
-	id, err := jobs.Submit(ctx, "block", nil, func(ctx context.Context, report func(progress, statistics string) error) (any, error) {
+	id, err := jobs.Submit(ctx, "block", nil, func(ctx context.Context, _ func(progress, statistics string) error) (any, error) {
 		close(blocked)
 		<-ctx.Done()
 		return nil, ctx.Err()
@@ -309,7 +309,7 @@ func TestCancelQueuedJob(t *testing.T) {
 	}
 
 	// Delayed function that will never be started before cancel.
-	id, err := jobs.Submit(ctx, "never", nil, func(ctx context.Context, report func(progress, statistics string) error) (any, error) {
+	id, err := jobs.Submit(ctx, "never", nil, func(ctx context.Context, _ func(progress, statistics string) error) (any, error) {
 		select {
 		case <-ctx.Done():
 			return nil, ctx.Err()
@@ -365,7 +365,7 @@ func TestJobExecutorBoundsRunningAndPendingJobs(t *testing.T) {
 	queuedRan := make(chan struct{}, 1)
 	third, err := jobs.Submit(ctx, "third", nil, func(context.Context, func(string, string) error) (any, error) {
 		queuedRan <- struct{}{}
-		return nil, nil
+		return struct{}{}, nil
 	})
 	if err != nil {
 		t.Fatalf("submit third: %v", err)
@@ -539,7 +539,7 @@ func TestRemoteCancellationReleasesQueuedAdmission(t *testing.T) {
 	queuedRan := make(chan struct{}, 1)
 	queued, err := jobs.Submit(ctx, "queued", nil, func(context.Context, func(string, string) error) (any, error) {
 		queuedRan <- struct{}{}
-		return nil, nil
+		return struct{}{}, nil
 	})
 	if err != nil {
 		t.Fatalf("submit queued: %v", err)

internal/app/mcp_stdio_e2e_test.go

@@ -33,7 +33,7 @@ func TestMCPStdioHelper(t *testing.T) {
 	if home == "" {
 		t.Skip("stdio helper subprocess only")
 	}
-	svc, err := New(config.NewPaths(&config.Env{Home: home}), "e2e", nil)
+	svc, err := NewWithContext(context.Background(), config.NewPaths(&config.Env{Home: home}), "e2e", nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -181,9 +181,12 @@ func TestMCPStdioPullRequestPortfolioFlow(t *testing.T) {
 		t.Fatalf("portfolio = %+v, sync job = %+v", portfolio, syncResult)
 	}
 	pr := portfolio.PullRequests[0]
-	if pr.Ref != "lab/project#7" || pr.Attention != "approved" || pr.ReviewDecision != "approved" || pr.Mergeable == nil || !*pr.Mergeable {
+	if pr.Ref != "lab/project#7" || pr.ReviewDecision != "approved" || pr.Mergeable == nil || !*pr.Mergeable {
 		t.Fatalf("portfolio PR = %+v", pr)
 	}
+	if pr.Attention != "stale" {
+		t.Fatalf("portfolio attention = %q, want stale independently of approval and mergeability", pr.Attention)
+	}
 	if pr.HeadSHA != "head123" || pr.BaseSHA != "base123" || pr.StatusCoverage != "complete" {
 		t.Fatalf("portfolio status coverage = %+v", pr)
 	}
@@ -408,7 +411,7 @@ func replayMCPRecoveryAction(t *testing.T, action mcpcontract.ToolCall) (string,
 
 func seedMCPStdioCorpus(ctx context.Context, t *testing.T, home string) {
 	t.Helper()
-	svc, err := New(config.NewPaths(&config.Env{Home: home}), "e2e", nil)
+	svc, err := NewWithContext(ctx, config.NewPaths(&config.Env{Home: home}), "e2e", nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -441,7 +444,7 @@ func seedMCPStdioCorpus(ctx context.Context, t *testing.T, home string) {
 
 func seedMCPStdioEmptyCorpus(ctx context.Context, t *testing.T, home string) {
 	t.Helper()
-	svc, err := New(config.NewPaths(&config.Env{Home: home}), "e2e", nil)
+	svc, err := NewWithContext(ctx, config.NewPaths(&config.Env{Home: home}), "e2e", nil)
 	if err != nil {
 		t.Fatal(err)
 	}

internal/app/read_boundary_test.go

@@ -98,7 +98,7 @@ func TestPublicCorpusReadsDoNotCreateDatabase(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			svc, err := New(config.NewPaths(&config.Env{Home: t.TempDir()}), "test", nil)
+			svc, err := NewWithContext(ctx, config.NewPaths(&config.Env{Home: t.TempDir()}), "test", nil)
 			if err != nil {
 				t.Fatal(err)
 			}

internal/app/upgrade_activation_test.go

@@ -173,6 +173,8 @@ func TestUpgradeRejectsDestinationRuntimeContractDisagreementBeforeRegistration(
 }
 
 func TestUpgradeRejectsMismatchedPostInstallNPMVersion(t *testing.T) {
+	t.Setenv("npm_command", "")
+	t.Setenv("npm_lifecycle_event", "")
 	originalCmd := upgradeCommand
 	originalExec := osExecutable
 	originalGOOS := upgradeGOOS

internal/app/upgrade_setup_test.go

@@ -49,8 +49,9 @@ func TestUpgradeActivatesPrivateMCPRuntimeFromTargetRelease(t *testing.T) {
 }
 
 func TestUpgradeNpxActivatesPrivateMCPRuntimeFromLatestRelease(t *testing.T) {
-	t.Setenv("npm_command", "exec")
 	home, _, _, _, svc := setupUpgradeActivationTest(t, "1.2.3", "1.2.4", "1.2.4")
+	t.Setenv("npm_command", "exec")
+	t.Setenv("npm_lifecycle_event", "npx")
 	setRuntimeContract(t, "1.2.4", 1)
 
 	report, err := svc.Upgrade(context.Background(), contracts.UpgradeOptions{Yes: true})
@@ -121,8 +122,9 @@ func TestUpgradeActivatesAlreadyInstalledTargetRuntime(t *testing.T) {
 }
 
 func TestUpgradeNpxStaleBootstrapReportsExplicitLatestRecovery(t *testing.T) {
-	t.Setenv("npm_command", "exec")
 	_, _, configPath, want, svc := setupUpgradeActivationTest(t, "1.2.3", "1.2.4", "1.2.3")
+	t.Setenv("npm_command", "exec")
+	t.Setenv("npm_lifecycle_event", "npx")
 	setRuntimeContract(t, "1.2.3", 1)
 
 	report, err := svc.Upgrade(context.Background(), contracts.UpgradeOptions{Yes: true})