Skip to content

[TASK] Remove handler duplication between A2AServerImpl and DefaultA2AProtocolHandler #100

@claude

Description

@claude

Problem Statement

There is unnecessary duplication and dead code in task handler management between A2AServerImpl and DefaultA2AProtocolHandler, creating maintenance burden and synchronization issues.

Current State Analysis

Architecture Issues

  1. Dead Code: backgroundTaskHandler field in DefaultA2AProtocolHandler (line 348) is never used
  2. Duplication: streamingTaskHandler stored in both server and protocol handler
  3. Inconsistent Usage Pattern:
    • Background tasks: Server uses s.backgroundTaskHandler directly (server.go:596)
    • Streaming tasks: Protocol handler uses its copy h.streamingTaskHandler (task_handler.go:610)
  4. Synchronization Bug: SetStreamingTaskHandler() only updates server copy, leaving protocol handler with stale reference

Code Evidence

// DefaultA2AProtocolHandler - lines 348-349
type DefaultA2AProtocolHandler struct {
    backgroundTaskHandler TaskHandler        // DEAD CODE - never referenced
    streamingTaskHandler  StreamableTaskHandler // Only used in HandleMessageStream
}

// A2AServerImpl stores the same handlers
type A2AServerImpl struct {
    backgroundTaskHandler TaskHandler        // Used in ProcessTask (line 596)
    streamingTaskHandler  StreamableTaskHandler // Never used directly by server
}

Proposed Solutions

Option 1: Method Parameter Injection (Recommended)

Approach: Protocol handler methods receive handlers as parameters from server

  • ✅ Eliminates duplication completely
  • ✅ Single source of truth (server only)
  • ✅ No synchronization issues
  • ✅ Clean dependency injection
  • ✅ Improved testability
// Protocol handler interface change
type A2AProtocolHandler interface {
    HandleMessageStream(c *gin.Context, req types.JSONRPCRequest, handler StreamableTaskHandler)
    // background tasks already use server's handler via task manager
}

// Server calls protocol handler
s.protocolHandler.HandleMessageStream(c, req, s.streamingTaskHandler)

Impact Assessment

Benefits

  • 🗑️ Eliminate Dead Code: Remove unused backgroundTaskHandler from protocol handler
  • 🎯 Single Source of Truth: All handlers managed by server only
  • 🔄 Fix Synchronization: No more stale handler references
  • 🏗️ Cleaner Architecture: Reduce coupling and improve maintainability
  • Better Testability: Easier to mock and test handler interactions

Risk Analysis

  • Low Risk: Only affects internal server architecture
  • No Breaking Changes: Public API remains unchanged
  • Improved Maintainability: Cleaner code structure

Acceptance Criteria

  • Remove unused backgroundTaskHandler field from DefaultA2AProtocolHandler
  • Eliminate streamingTaskHandler duplication between server and protocol handler
  • Protocol handler gets streaming handler dynamically (not stored copy)
  • Single source of truth for all task handlers (server only)
  • No synchronization issues when SetStreamingTaskHandler() is called
  • All existing tests continue to pass
  • No breaking changes to public API
  • Code follows clean architecture principles (dependency injection over service locator)

Implementation Notes

Files to Modify

  • server/task_handler.go (lines 342-369) - Protocol handler structure and constructor
  • server/server.go (lines 254-275) - Server handler management
  • server/server_builder.go (lines 304-310) - Builder pattern updates
  • Test files using protocol handler

Testing Strategy

  • Update existing protocol handler tests to use dependency injection
  • Verify handler synchronization works correctly
  • Test both background and streaming task processing flows
  • Ensure no regression in task processing functionality

Definition of Done

  • All linting checks pass (task lint)
  • All tests pass (task test)
  • Code review approved
  • Documentation updated if needed

Priority: Medium
Complexity: Low
Category: Technical Debt / Refactoring

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    Status

    Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions