[repository-quality] Repository Quality: Concurrency Safety & Race Detection

[github-actions[bot]]

🎯 Repository Quality Improvement Report - Concurrency Safety & Race Detection

Analysis Date: 2026-10-09
Focus Area: Concurrency safety & race detection
Strategy Type: Custom
Custom Area: Yes — prior runs covered errors, contexts, function size and test isolation; concurrency/race coverage is not yet examined.

Executive Summary

The repo has solid concurrency discipline: 37 non-test go statements, all inspected ones recover from panics, and custom linters exist (goroutinemissingrecover, wgdonenotdeferred, timeafterleak, manualmutexunlock). The biggest gap is that no -race run is configured in the Makefile or ci.yml, despite ~4,100 t.Parallel()/RunParallel usages and shared package-level caches.

Secondary items: ~15 package-level sync.Map caches (pkg/cli/update_workflows.go:25-27, pkg/workflow/yaml.go:108, pkg/workflow/behavior_defined_engine.go:806,824) with no documented lifecycle or reset for tests; ~87 package-level maps/sync vars that are safe only by "read-only by convention"; and two time.After uses in select statements (pkg/cli/mcp_inspect.go:139, pkg/cli/mcp_inspect_inspector.go:243).

Full Analysis Report

Current State Assessment

Metric	Value	Status
-race in Makefile/CI	0 occurrences	❌
Non-test go statements	37	✅
Goroutines with recover (sampled)	5/5	✅
Concurrency linters	4 present	✅
Package-level sync.Map caches	~10	⚠️
Non-test time.After	2 (in select)	⚠️
Parallel test markers	~4,111	⚠️ without race detection

Strengths

• Panic-recovery and defer wg.Done() enforced by linters.
• errgroup and semaphores used for fan-out (audit_analysis_fanout.go, forecast_compute.go).

Areas for Improvement

• High: no race detector in any test target.
• Medium: global sync.Map caches without invalidation/test-reset helpers.
• Low: time.After in mcp_inspect*.go; read-only global maps not enforced (pkg/cli/outcome_eval.go:78, pkg/workflow/safe_outputs_handler_registry_issues.go:4).

---

🤖 Tasks for Copilot Agent

NOTE TO PLANNER AGENT: Split the following tasks into individual work items.

Improvement Tasks

Task 1: Add race-detector test target

Priority: High
Estimated Effort: Small
Focus Area: Concurrency

Description: Add make test-race running go test -race on concurrency-heavy packages (./pkg/cli, ./pkg/console, ./pkg/syncutil, ./pkg/parser) and wire it into CI (scheduled or non-blocking first).

Acceptance Criteria:

• make test-race exists and is documented
• CI job runs it
• Any detected races are listed or fixed

Code Region: Makefile, .github/workflows/ci.yml

Add a `test-race` Makefile target using `go test -race -count=1` for pkg/cli, pkg/console, pkg/syncutil, pkg/parser, pkg/workflow. Add a CI job. Fix or report any races found.

---

Task 2: Consolidate global sync.Map caches

Priority: Medium
Estimated Effort: Medium
Focus Area: Concurrency

Description: Replace raw package-level sync.Map caches with typed wrappers (generic cache in pkg/syncutil) that provide typed access and a Reset() for tests.

Acceptance Criteria:

• Typed cache helper with tests
• update_workflows.go, yaml.go, behavior_defined_engine.go caches migrated
• Tests use Reset rather than leaking state

Code Region: pkg/cli/update_workflows.go, pkg/workflow/yaml.go, pkg/workflow/behavior_defined_engine.go

Add a generic typed cache to pkg/syncutil with Load/Store/Reset and migrate the package-level sync.Map vars listed. Keep behavior identical; run make test-unit.

---

Task 3: Replace time.After in MCP inspect selects

Priority: Low
Estimated Effort: Small
Focus Area: Concurrency

Description: Use time.NewTimer + defer Stop() in mcp_inspect.go:139 and mcp_inspect_inspector.go:243, or justify with (nolint/redacted):timeafterleak.

Acceptance Criteria:

• No unexplained time.After in non-test code
• Tests pass

Code Region: pkg/cli/mcp_inspect.go, pkg/cli/mcp_inspect_inspector.go

Replace the time.After calls with timers that are stopped on exit from the select.

---

Task 4: Freeze read-only global lookup maps

Priority: Low
Estimated Effort: Medium
Focus Area: Concurrency

Description: Convert package-level lookup maps (e.g. outcomeEvaluators, issueHandlerRegistry, jsonToolToToolset) to accessor functions or sync.OnceValue builders so mutation cannot happen from parallel tests; confirm packagelevelmutableslicemap linter covers them.

Acceptance Criteria:

• Listed maps not mutated anywhere (verified by grep)
• Linter coverage confirmed or extended

Code Region: pkg/cli/outcome_eval.go, pkg/workflow/safe_outputs_handler_registry_issues.go, pkg/cli/jsonworkflow_to_markdown.go

Audit writes to these maps; where none exist, add a linter exemption comment or convert to accessor functions.

---

📊 Historical Context

Previous Focus Areas

Date	Focus Area	Type	Custom	Key Outcomes
2026-10-05	Error-handling contract consistency	Custom	Y	4 tasks
2026-10-06	Context propagation & cancellation hygiene	Custom	Y	5 tasks
2026-10-07	Function size & file cohesion	Custom	Y	4 tasks
2026-10-08	Test isolation & hermeticity	Custom	Y	4 tasks

---

🎯 Recommendations

Immediate Actions (This Week)

1. Add make test-race and CI job — Priority: High

Short-term Actions (This Month)

1. Typed cache wrapper — Priority: Medium

Long-term Actions (This Quarter)

1. Harden global maps — Priority: Low

---

📈 Success Metrics

• Race-checked packages: 0 → 5
• Raw package-level sync.Map: ~10 → 0

---

Next Steps

1. Review and prioritise the tasks above
2. Assign tasks to Copilot coding agent via planner agent
3. Track progress
4. Re-evaluate in 1 month

---

Generated by Repository Quality Improvement Agent
Next analysis: 2026-10-10 — Focus area selected by diversity algorithm

Generated by ⚡ Repository Quality Improvement Agent · copilot · auto · 19.5 AIC · ⌖ 0.656 AIC · ⊞ 9.8K · ◷

• expires on Oct 10, 2026, 5:07 AM UTC-08:00