[repository-quality] 🎯 Repository Quality Improvement Report - Interface Contract Completeness & Compile-Time Safety Gap

[github-actions[bot]]

Analysis Date: 2026-07-09
Focus Area: Interface Contract Completeness & Compile-Time Safety Gap
Strategy Type: Custom
Custom Area: Yes — gh-aw defines 20 public interfaces across pkg/workflow and pkg/cli, including a composite CodingAgentEngine interface with 8 sub-interfaces. Only 1/9 concrete engine types has a var _ CodingAgentEngine = ... compile-time assertion. Silent interface drift on engine structs is the single largest hidden refactoring hazard in this codebase.

Executive Summary

gh-aw defines 20 public interfaces, including the composite CodingAgentEngine (which embeds 8 sub-interfaces: Engine, CapabilityProvider, WorkflowExecutor, MCPConfigProvider, LogParser, SecurityProvider, ModelEnvVarProvider, ConfigRenderer). Nine concrete engine structs implement this interface — but only PiEngine has a compile-time var _ CodingAgentEngine = NewPiEngine() assertion. The other 8 engines (AntigravityEngine, ClaudeEngine, CopilotEngine, CodexEngine, GeminiEngine, OpenCodeEngine, CrushEngine, UniversalLLMConsumerEngine) are silently assumed to comply. Similarly, optional interfaces (HarnessProvider, AgentFileProvider, LLMProviderResolver) and structural interfaces (ConditionNode, ToolConfig, ValidatableTool) have zero compile-time checks at their implementation sites.

Without var _ assertions, adding or modifying an interface method can silently break implementors. The Go compiler only reports the error when the concrete type is assigned to the interface at a call site — often deep in tests or at runtime — rather than at the definition site. With 9 engine implementations and an 8-sub-interface composite, the risk surface is significant: a single missing method rename propagates to 9 compilation errors only discovered at assignment.

The pattern is partially understood: pkg/cli/log_aggregation_test.go has correct var _ LogAnalysis = (*DomainAnalysis)(nil) checks for LogAnalysis and MutableLogAnalysis. These assertions should be replicated for all 20 interfaces at their canonical implementation sites.

Full Analysis Report

Focus Area: Interface Contract Completeness & Compile-Time Safety Gap

Current State Assessment

Metrics Collected:

Metric	Value	Status
Public interfaces defined	20	⚠️
Concrete engine implementors (CodingAgentEngine)	9	⚠️
var _ CodingAgentEngine = ... assertions	1 (PiEngine only)	❌
var _ HarnessProvider = ... assertions	1 (CopilotEngine, inside test func)	⚠️
var _ AgentFileProvider = ... assertions	0	❌
var _ LLMProviderResolver = ... assertions	0	❌
var _ ConditionNode = ... assertions	0 (10 implementors)	❌
var _ ToolConfig = ... assertions	0	❌
var _ ValidatableTool = ... assertions	0	❌
var _ SHAResolver = ... assertions	0	❌
var _ FileCreationTracker = ... assertions	0	❌
var _ CommandProvider = ... assertions	0	❌
var _ LogAnalysis/MutableLogAnalysis = ... in tests	4 ✅	✅

Findings

Strengths

• pkg/cli/log_aggregation_test.go demonstrates the correct pattern with 4 var _ assertions for LogAnalysis and MutableLogAnalysis across DomainAnalysis and FirewallAnalysis.
• pkg/workflow/pi_engine_test.go:304 has var _ CodingAgentEngine = NewPiEngine() as a dedicated test function TestPiEngine_ImplementsCodingAgentEngine.
• pkg/workflow/copilot_engine_test.go:2670 has var _ HarnessProvider = engine inside an existing test case.
• EngineRegistry.Register validates at runtime that all engines implement CodingAgentEngine (checked via type assertion during engine registration in agentic_engine.go), providing a second safety net.

Areas for Improvement

• [HIGH] 8/9 CodingAgentEngine concrete implementations lack top-level compile-time assertions; engine method drift is only caught at registration call sites, not at definition.
• [HIGH] HarnessProvider (ClaudeEngine, CodexEngine) and AgentFileProvider (8 engines) lack assertions; both are cast via type assertions in safe_outputs_config_runtime.go, engine_config_dir.go — silent failures surface as wrong behaviour, not compile errors.
• [MEDIUM] ConditionNode — 10 concrete structs (ExpressionNode, AndNode, OrNode, NotNode, DisjunctionNode, FunctionCallNode, PropertyAccessNode, StringLiteralNode, BooleanLiteralNode, ComparisonNode) with no assertions; adding a required method to the interface would silently leave all 10 non-compliant.
• [MEDIUM] ToolConfig / MapToolConfig and ValidatableTool / GitHubToolConfig lack assertions; both are widely used in permissions validation.
• [LOW] SHAResolver / ActionResolver, FileCreationTracker, and CommandProvider lack assertions; lower risk due to single implementor each but still useful as documentation.

Detailed Analysis

CodingAgentEngine interface structure:

The composite interface embeds 8 sub-interfaces. Each sub-interface has 1–5 methods. Total method surface: ~25 methods. Any engine added in the future without all 25 will cause a runtime panic in EngineRegistry.Register — not a compile error. The existing var _ check in pi_engine_test.go is the only test that catches this at compile time (and only for PiEngine). The 8 unchecked engines are:

Engine	File
AntigravityEngine	pkg/workflow/antigravity_engine.go
ClaudeEngine	pkg/workflow/claude_engine.go
CopilotEngine	pkg/workflow/copilot_engine.go
CodexEngine	pkg/workflow/codex_engine.go
GeminiEngine	pkg/workflow/gemini_engine.go
OpenCodeEngine	pkg/workflow/opencode_engine.go
CrushEngine	pkg/workflow/crush_engine.go
UniversalLLMConsumerEngine	pkg/workflow/universal_llm_consumer_engine.go

Optional interfaces (HarnessProvider, AgentFileProvider, LLMProviderResolver):

These are accessed via type assertion (engine.(AgentFileProvider)), which silently returns false if the assertion fails. A var _ assertion in the engine's own file documents the intentional implementation and catches regressions if a method is removed from the struct.

• HarnessProvider implementors: ClaudeEngine, CopilotEngine, CodexEngine
• AgentFileProvider implementors: AntigravityEngine, ClaudeEngine, CopilotEngine, CodexEngine, GeminiEngine, PiEngine, OpenCodeEngine, CrushEngine (plus BaseEngine default)
• LLMProviderResolver implementors: ClaudeEngine, PiEngine, CopilotEngine, CodexEngine

Correct placement: var _ assertions should live in the non-test source file of the concrete type (e.g., var _ CodingAgentEngine = (*ClaudeEngine)(nil) at the top of claude_engine.go, near the struct definition). Using (*T)(nil) avoids allocating an instance. The 4 assertions in log_aggregation_test.go use this pattern correctly.

---

🤖 Tasks for Copilot Agent

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

Improvement Tasks

Task 1: Add var _ CodingAgentEngine assertions for all 8 unchecked engine implementations

Priority: High
Estimated Effort: Small
Focus Area: Interface Contract Completeness

Description: Add compile-time interface satisfaction checks for all concrete engine types that implement CodingAgentEngine but currently lack a var _ assertion. Each assertion should be placed in the engine's primary source file, after the struct definition, using the (*T)(nil) nil-pointer pattern.

Acceptance Criteria:

• var _ CodingAgentEngine = (*AntigravityEngine)(nil) added to pkg/workflow/antigravity_engine.go
• var _ CodingAgentEngine = (*ClaudeEngine)(nil) added to pkg/workflow/claude_engine.go
• var _ CodingAgentEngine = (*CopilotEngine)(nil) added to pkg/workflow/copilot_engine.go
• var _ CodingAgentEngine = (*CodexEngine)(nil) added to pkg/workflow/codex_engine.go
• var _ CodingAgentEngine = (*GeminiEngine)(nil) added to pkg/workflow/gemini_engine.go
• var _ CodingAgentEngine = (*OpenCodeEngine)(nil) added to pkg/workflow/opencode_engine.go
• var _ CodingAgentEngine = (*CrushEngine)(nil) added to pkg/workflow/crush_engine.go
• var _ CodingAgentEngine = (*UniversalLLMConsumerEngine)(nil) added (or omitted with a comment if UniversalLLMConsumerEngine is intentionally not a full CodingAgentEngine)
• go build ./pkg/workflow/... passes

Code Region: pkg/workflow/*_engine.go (all engine definition files)

In pkg/workflow/antigravity_engine.go, claude_engine.go, copilot_engine.go, codex_engine.go, gemini_engine.go, opencode_engine.go, and crush_engine.go, add a blank-identifier compile-time interface check immediately after the struct type declaration. Use the nil-pointer pattern:

  var _ CodingAgentEngine = (*AntigravityEngine)(nil)

Place each assertion in the file that defines the struct (e.g., the `var _ CodingAgentEngine = (*ClaudeEngine)(nil)` line goes in `claude_engine.go` right after `type ClaudeEngine struct { ... }`).

For UniversalLLMConsumerEngine, first verify whether it is registered as a CodingAgentEngine in the EngineRegistry (check pkg/workflow/agentic_engine.go defaultEngines list). If it is registered, add the assertion; if not, add a comment documenting that it is a helper base type, not a full CodingAgentEngine.

Do NOT use `var _ CodingAgentEngine = NewClaudeEngine()` (which allocates) — use `(*ClaudeEngine)(nil)`.
Run `make build` to confirm all assertions pass without compilation errors.

---

Task 2: Add var _ assertions for HarnessProvider, AgentFileProvider, and LLMProviderResolver

Priority: High
Estimated Effort: Small
Focus Area: Optional Interface Compliance Documentation

Description: Three optional interfaces are implemented by subsets of engines and accessed via type assertions (engine.(AgentFileProvider)). Adding var _ assertions documents the intentional implementation and catches regressions when a method is removed.

Acceptance Criteria:

• var _ HarnessProvider = (*ClaudeEngine)(nil) in pkg/workflow/claude_engine.go
• var _ HarnessProvider = (*CopilotEngine)(nil) in pkg/workflow/copilot_engine.go (currently only in test)
• var _ HarnessProvider = (*CodexEngine)(nil) in pkg/workflow/codex_engine.go
• var _ AgentFileProvider = (*AntigravityEngine)(nil), (*ClaudeEngine)(nil), (*CopilotEngine)(nil), (*CodexEngine)(nil), (*GeminiEngine)(nil), (*PiEngine)(nil), (*OpenCodeEngine)(nil), (*CrushEngine)(nil) — each in their respective file
• var _ LLMProviderResolver = (*ClaudeEngine)(nil), (*PiEngine)(nil), (*CopilotEngine)(nil), (*CodexEngine)(nil) — each in their respective file
• go build ./pkg/workflow/... passes with no errors

Code Region: pkg/workflow/claude_engine.go, pkg/workflow/copilot_engine.go, pkg/workflow/codex_engine.go, pkg/workflow/antigravity_engine.go, pkg/workflow/gemini_engine.go, pkg/workflow/pi_engine.go, pkg/workflow/opencode_engine.go, pkg/workflow/crush_engine.go

For each engine file listed in the acceptance criteria, add the appropriate `var _` assertions immediately after the struct definition. Group them together:

Example for pkg/workflow/claude_engine.go:
  var _ CodingAgentEngine = (*ClaudeEngine)(nil)
  var _ HarnessProvider   = (*ClaudeEngine)(nil)
  var _ AgentFileProvider = (*ClaudeEngine)(nil)
  var _ LLMProviderResolver = (*ClaudeEngine)(nil)

Check the existing implementation methods in each engine file to confirm which optional interfaces are implemented before adding assertions. For example, only add `var _ HarnessProvider` for engines that have a `GetHarnessScriptName()` method. Run `go build ./pkg/workflow/...` to verify all assertions compile without errors.

---

Task 3: Add var _ assertions for ConditionNode and ToolConfig implementors

Priority: Medium
Estimated Effort: Small
Focus Area: Expression Tree & Tool Configuration Interface Safety

Description: ConditionNode has 10 concrete implementors (ExpressionNode, AndNode, OrNode, NotNode, DisjunctionNode, FunctionCallNode, PropertyAccessNode, StringLiteralNode, BooleanLiteralNode, ComparisonNode) — none have compile-time checks. ToolConfig has MapToolConfig as its key implementor. Both are structural interfaces with simple method sets but high refactoring risk.

Acceptance Criteria:

• var _ ConditionNode = (*ExpressionNode)(nil) and assertions for all 10 ConditionNode implementors added to pkg/workflow/expression_nodes.go
• var _ ToolConfig = (MapToolConfig)(nil) (or MapToolConfig{}) added to pkg/workflow/mcp_config_types.go
• var _ ValidatableTool = (*GitHubToolConfig)(nil) added to pkg/workflow/permissions_toolset_data.go
• go build ./pkg/workflow/... passes

Code Region: pkg/workflow/expression_nodes.go, pkg/workflow/mcp_config_types.go, pkg/workflow/permissions_toolset_data.go

In pkg/workflow/expression_nodes.go, add a block of var _ ConditionNode assertions after all struct definitions:

  var (
    _ ConditionNode = (*ExpressionNode)(nil)
    _ ConditionNode = (*AndNode)(nil)
    _ ConditionNode = (*OrNode)(nil)
    _ ConditionNode = (*NotNode)(nil)
    _ ConditionNode = (*DisjunctionNode)(nil)
    _ ConditionNode = (*FunctionCallNode)(nil)
    _ ConditionNode = (*PropertyAccessNode)(nil)
    _ ConditionNode = (*StringLiteralNode)(nil)
    _ ConditionNode = (*BooleanLiteralNode)(nil)
    _ ConditionNode = (*ComparisonNode)(nil)
  )

Note: DisjunctionNode also has a RenderMultiline() method that is not part of ConditionNode. The assertion checks only that Render() is present, which is the interface contract.

In pkg/workflow/mcp_config_types.go, add after the MapToolConfig methods:
  var _ ToolConfig = (MapToolConfig)(nil)

In pkg/workflow/permissions_toolset_data.go, add after GitHubToolConfig methods:
  var _ ValidatableTool = (*GitHubToolConfig)(nil)

Run `go build ./pkg/workflow/...` to confirm.

---

Task 4: Add var _ assertions for SHAResolver, FileCreationTracker, and CommandProvider

Priority: Low
Estimated Effort: Small
Focus Area: Single-Implementor Interface Documentation

Description: Three interfaces each have a single concrete implementor. While the risk of undetected drift is lower (a compile error will appear at assignment sites when the interface is broken), adding var _ assertions serves as documentation of intentional interface compliance and co-locates the contract with the implementation.

Acceptance Criteria:

• var _ SHAResolver = (*ActionResolver)(nil) added to pkg/workflow/action_resolver.go (note: SHAResolver in pkg/workflow is a type alias for actionpins.SHAResolver — verify the assertion should be against actionpins.SHAResolver)
• var _ FileCreationTracker = ... added for the concrete type that implements it (check compiler_types.go callsites to find the implementor)
• var _ CommandProvider = (*cobra.Command)(nil) added to pkg/cli/interfaces.go as documentation that cobra.Command satisfies the interface (import github.com/spf13/cobra)
• go build ./... passes

Code Region: pkg/workflow/action_resolver.go, pkg/workflow/compiler_types.go, pkg/cli/interfaces.go

For pkg/workflow/action_resolver.go:
- ActionResolver.ResolveSHA takes ctx, repo, version and matches actionpins.SHAResolver.
- Add: var _ SHAResolver = (*ActionResolver)(nil)
  (SHAResolver is a type alias in pkg/workflow/action_pins.go for actionpins.SHAResolver)

For FileCreationTracker: search for the concrete type that gets passed to Compiler.SetFileTracker() in tests or production code, then add var _ FileCreationTracker = (*ConcreteType)(nil) in that type's file.

For pkg/cli/interfaces.go:
- The interface is documented as "satisfied by *cobra.Command". Formalise this:
  import "github.com/spf13/cobra"
  var _ CommandProvider = (*cobra.Command)(nil)
  This turns the documentation comment into an enforced contract.

Run `go build ./...` to verify everything compiles.

---

📊 Historical Context

Previous Focus Areas

Date	Focus Area	Type	Custom	Key Outcomes
2026-07-08	Deprecated API Alias Proliferation & Zombie Callsite Cleanup	Custom	Y	3 GetGeminiAPITarget callsites; zombie validateImportedStepsNoAgenticSecrets; 4 always-true guards
2026-07-07	Environment Variable Access Fragmentation	Custom	Y	45 bare nolint:osgetenvlibrary suppressions; 6 env vars duplicated raw
2026-07-06	Context-Blind Sleep Gap	Custom	Y	4 functions with 7 time.Sleep calls invisible to linter
2026-07-03	ADR-42150 Implementation Gap	Custom	Y	yaml tag, handler registry, error messages still use underscore form
2026-07-02	Remote Fetch API Context Gap	Custom	Y	8 public API wrappers strip caller context

---

🎯 Recommendations

Immediate Actions (This Week)

1. Add var _ CodingAgentEngine for 8 engines — Priority: High (Task 1 above). Prevents silent interface drift on the core CodingAgentEngine contract.
2. Add optional interface assertions (HarnessProvider, AgentFileProvider, LLMProviderResolver) — Priority: High (Task 2). Documents intentional compliance and prevents regression when methods are moved.

Short-term Actions (This Month)

1. Add var _ for ConditionNode (10 implementors) and ToolConfig — Priority: Medium (Task 3). Expression tree refactoring risk.
2. Enforce via custom linter — Consider adding a pkg/linters/ analyzer that flags type T struct declarations in pkg/workflow/ with an // implements X comment but no corresponding var _ X = (*T)(nil) in the same file.

Long-term Actions (This Quarter)

1. Move var _ assertions from test files to production files — The 4 assertions in log_aggregation_test.go belong in log_aggregation.go itself. This ensures they catch interface drift even without running tests. Priority: Low.

---

📈 Success Metrics

• var _ assertion coverage: 6/~45 implementors (13%) → 45/45 (100%)
• Unchecked CodingAgentEngine implementors: 8 → 0
• Optional interface assertion coverage: 0/~15 implementors → 15/15

---

Next Steps

1. Review and prioritise the 4 tasks above
2. Assign Task 1 and Task 2 to Copilot coding agent via planner agent (small, self-contained changes)
3. Task 3 and 4 can be batched into a single PR
4. Re-evaluate this focus area in 2 weeks after Task 1–2 are merged

---

References:

• §29022468281

Generated by Repository Quality Improvement Agent

Generated by ⚡ Repository Quality Improvement Agent · 94.7 AIC · ⌖ 6.63 AIC · ⊞ 6.6K · ◷

• expires on Jul 10, 2026, 5:58 AM UTC-08:00