[repository-quality] 🎯 Repository Quality Improvement — Import Pipeline Reliability: Silent Unmarshal Errors (2026-06-24)

[github-actions[bot]]

🎯 Repository Quality Improvement Report - Import Pipeline Reliability

Analysis Date: 2026-06-24
Focus Area: Import Pipeline Reliability — Silent Unmarshal Errors & Serialization Overhead
Strategy Type: Custom (random=44, 0–59 tier)
Custom Area: Yes — YAML-string-as-IPC is unique to this codebase

Executive Summary

The workflow compilation pipeline stores five step categories as raw YAML strings in ImportsResult, forcing up to 10 YAML operations per compile (Unmarshal+SliceToSteps+StepsToSlice+Marshal per category). More critically, 31 production sites use the silent if err := yaml.Unmarshal(...); err == nil pattern — malformed imported YAML silently produces empty step lists with no error to the user.

The highest-risk sites are workflow_builder.go (5 instances) and compiler_orchestrator_engine.go (5 instances). Fixing these to propagate errors is low-effort and high-impact. A companion linter prevents recurrence.

Full Analysis

Current State

Metric	Value	Status
Silent if err := yaml.Unmarshal(...); err == nil in production	31	❌
Silent patterns in workflow_builder.go	5	❌
Silent patterns in compiler_orchestrator_engine.go	5	❌
Total YAML marshal/unmarshal in pkg/workflow	76	⚠️
YAML ops per compilation (step merge, 5 categories × 2)	10	⚠️
ImportsResult step fields using YAML string transport	5	⚠️
init() eager JSON loads in pkg/workflow	3	⚠️

Strengths

• SliceToSteps/MapToStep avoid JSON round-trips via direct map[string]any extraction
• Compiler benchmarks use b.ReportAllocs() and -benchmem
• Regex patterns pre-compiled at package level (no in-function compile overhead)

Silent Unmarshal Pattern

// pkg/workflow/workflow_builder.go:416
if err := yaml.Unmarshal([]byte(importsResult.MergedSteps), &target); err == nil {
    // use target — silently empty if YAML is malformed
}

Same pattern ×5 in compiler_orchestrator_engine.go drops engine/runtime config silently.

YAML Round-Trip in ImportsResult

Each of 5 step fields (MergedSteps, CopilotSetupSteps, MergedPreSteps, MergedPreAgentSteps, MergedPostSteps) traverses: string → Unmarshal → []any → SliceToSteps → []*WorkflowStep → applyPins → StepsToSlice → []any → Marshal → string. Replacing those string fields with []map[string]any eliminates all 10 YAML operations.

Startup Eager Loads

Three init() in pkg/workflow unmarshal JSON at package import: domains.go (~30 KB), gh_cli_permissions.go (~50 KB + map build), github_tool_to_toolset.go (~10 KB). Non-compile commands (gh aw env, gh aw audit) pay this cost unnecessarily.

---

🤖 Tasks for Copilot Agent

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

Task 1: Propagate unmarshal errors in workflow_builder.go

Priority: High | Effort: Small

Description: 5 if err := yaml.Unmarshal(...); err == nil blocks in processAndMergeSteps silently discard steps on parse failure.

Acceptance Criteria:

• All 5 silent patterns replaced with return fmt.Errorf("failed to parse <field>: %w", err)
• New test: malformed MergedSteps YAML returns an error from processAndMergeSteps
• make test passes

Code Region: pkg/workflow/workflow_builder.go

In processAndMergeSteps, change each:
  if err := yaml.Unmarshal([]byte(f), &t); err == nil {
to:
  if err := yaml.Unmarshal([]byte(f), &t); err != nil {
      return fmt.Errorf("failed to parse <field>: %w", err)
  }
Fields: MergedSteps, CustomSteps, mainPreStepsYAML, mainPreAgentStepsYAML, mainPostStepsYAML.
Add a test in workflow_builder_test.go asserting malformed YAML returns an error.

---

Task 2: Propagate unmarshal errors in compiler_orchestrator_engine.go

Priority: High | Effort: Small

Description: 5 silent if err == nil unmarshal patterns in compiler_orchestrator_engine.go silently drop engine/runtime configuration.

Acceptance Criteria:

• All 5 patterns replaced with error-returning paths
• New test verifies malformed engine YAML returns an error
• make test passes

Code Region: pkg/workflow/compiler_orchestrator_engine.go

Find all 5 occurrences of:
  if err := yaml.Unmarshal([]byte(...), &t); err == nil {
Replace with:
  if err := yaml.Unmarshal([]byte(...), &t); err != nil {
      return <zero>, fmt.Errorf("failed to parse <section>: %w", err)
  }
Add a test verifying malformed YAML returns an error from the orchestrator.

---

Task 3: Add silentyamlunmarshal linter

Priority: Medium | Effort: Medium

Description: Linter to flag if err := yaml.Unmarshal(...); err == nil anti-pattern. Register in cmd/linters/main.go and spec_test.go.

Acceptance Criteria:

• Analyzer in pkg/linters/silentyamlunmarshal/ with analysistest tests
• Registered in cmd/linters/main.go and cmd/linters/spec_test.go
• Entry added to pkg/linters/doc.go
• Intentional sites suppressed with //nolint:silentyamlunmarshal + justification
• make lint passes

Code Region: pkg/linters/silentyamlunmarshal/, cmd/linters/main.go, pkg/linters/doc.go

Create pkg/linters/silentyamlunmarshal/ following pkg/linters/errorfwrapv/ pattern.
Detect: if-stmt where init is err := yaml.Unmarshal(...) and condition is err == nil.
Report: "yaml.Unmarshal error silently swallowed; use err != nil to propagate"
Register in cmd/linters/main.go and spec_test.go. Add to doc.go.
Suppress intentional sites (runs_on_snippet.go, permissions_parser.go,
known_action_credentials.go) with (nolint/redacted):silentyamlunmarshal + justification comment.
Run make lint to confirm zero violations.

---

Task 4: Replace ImportsResult step YAML strings with typed slices

Priority: Medium | Effort: Large

Description: Change 5 string fields in ImportsResult to []map[string]any, eliminating 10 YAML operations per compilation.

Acceptance Criteria:

• 5 fields changed to []map[string]any in pkg/parser/import_processor.go
• Import processor assigns decoded slices directly (no yaml.Marshal for these fields)
• workflow_builder.go:processAndMergeSteps skips initial yaml.Unmarshal for these fields
• BenchmarkStepMerging with b.ReportAllocs() added and shows improvement
• make test passes; golden tests unchanged

Code Region: pkg/parser/import_processor.go, pkg/workflow/workflow_builder.go

In pkg/parser/import_processor.go, change MergedSteps, CopilotSetupSteps,
MergedPreSteps, MergedPreAgentSteps, MergedPostSteps from string to []map[string]any.
Update all sites that populate these fields to assign decoded slices directly.
In workflow_builder.go:processAndMergeSteps, remove yaml.Unmarshal for these fields.
Add BenchmarkStepMerging with b.ReportAllocs() to workflow_builder_test.go.
Run make test and verify golden tests produce identical output.

---

Task 5: Convert workflow init() to sync.Once lazy init

Priority: Low | Effort: Small

Description: Convert 3 eager init() in pkg/workflow to sync.Once so non-compile commands avoid startup cost.

Acceptance Criteria:

• init() removed from domains.go, gh_cli_permissions.go, github_tool_to_toolset.go
• Each replaced with sync.Once + getter accessed lazily from call sites
• make test passes

Code Region: pkg/workflow/domains.go, pkg/workflow/gh_cli_permissions.go, pkg/workflow/github_tool_to_toolset.go

For each file, replace func init() with a sync.Once + getter:
  var (<data> <Type>; <data>Once sync.Once)
  func get<Data>() <Type> { <data>Once.Do(func() { /* original body */ }); return <data> }
Update call sites to use the getter. Run make test.

---

📊 Historical Context

Last 5 Focus Areas

Date	Focus Area	Custom	Outcomes
2026-06-19	Compilation Idempotence — Nested Map Key Ordering	Y	OrderMapFields shallow
2026-06-22	Context Propagation (reuse)	N	execcommandwithoutcontext linter added
2026-06-23	AIC Attribution Completeness	Y	TokenClassWeights silently dropped

---

📈 Success Metrics

• Silent YAML unmarshal patterns: 31 → 0 unintentional
• YAML ops per compilation (step merge): 10 → 0
• Startup init() eager loads: 3 → 0

References: [§28102213814]

Generated by Repository Quality Improvement Agent

Generated by ⚡ Repository Quality Improvement Agent · 130.3 AIC · ⌖ 14.3 AIC · ⊞ 6.7K · ◷

• expires on Jun 25, 2026, 5:47 AM UTC-08:00