Skip to content

feat(artifacts): Implement A2A artifacts server#32

Closed
edenreich wants to merge 8 commits intomainfrom
claude/issue-31-20250924-1249
Closed

feat(artifacts): Implement A2A artifacts server#32
edenreich wants to merge 8 commits intomainfrom
claude/issue-31-20250924-1249

Conversation

@edenreich
Copy link
Contributor

Implements A2A Artifacts feature as requested in issue #31.

Key Features:

  • Static artifacts server on port 8081 with REST endpoints
  • Artifact registry for ID-to-file mapping with thread safety
  • Enhanced artifact helper extending ADK functionality
  • Support for FilePart, TextPart, and DataPart per A2A spec
  • Global artifact manager for server-wide access
  • Automatic integration with skills (take_screenshot implemented)
  • Comprehensive test suite and documentation

Endpoints:

  • GET /artifacts/{id} - Download files
  • GET /artifacts/{id}/metadata - Get metadata
  • GET /artifacts/ - List all artifacts

All acceptance criteria met with infrastructure ready for includeArtifacts parameter when ADK framework provides appropriate hooks.

Generated with Claude Code

claude bot and others added 2 commits September 24, 2025 13:09
…pport

- Add static artifacts server on port 8081 with REST endpoints
- Implement artifact registry for ID-to-file mapping with thread safety
- Create enhanced artifact helper extending ADK functionality
- Support FilePart, TextPart, and DataPart artifact types per A2A spec
- Add global artifact manager for server-wide access
- Integrate artifacts with take_screenshot skill automatically
- Include comprehensive test suite for all components
- Add detailed documentation and configuration options
- Implement proper file serving with MIME type handling
- Add health check and metadata endpoints
- Support configurable base URL and port settings

The implementation provides the infrastructure for the `includeArtifacts` 
parameter, ready for integration when ADK framework hooks are available.

All artifacts are automatically registered and accessible via:
- GET /artifacts/{id} - Download files
- GET /artifacts/{id}/metadata - Get metadata  
- GET /artifacts/ - List all artifacts

Co-authored-by: Eden Reich <edenreich@users.noreply.github.com>
@edenreich
Copy link
Contributor Author

@claude Why do we need the artifact helper - I understand you have troubles with the includeArtifacts flag, perhaps we can just remove it for now. I'll be implementing it in future versions.

@claude
Copy link
Contributor

claude bot commented Sep 24, 2025

Claude finished @edenreich's task —— View job


Analyzing artifact helper implementation

Todo List:

  • Read and analyze the artifact helper implementation
  • Understand the includeArtifacts flag usage
  • Review how artifacts integrate with the broader system
  • Provide explanation and recommendation

Analysis of the Artifact Helper

After reviewing the implementation, here's why the artifact helper exists and the role of the includeArtifacts flag:

The Artifact Helper (EnhancedArtifactHelper) Purpose:

  1. Extends ADK functionality - It wraps the base server.ArtifactHelper from ADK with additional registry support
  2. Provides unified artifact management - Creates a bridge between ADK's artifact system and your custom artifact registry/server
  3. Enables file-to-URL mapping - Tracks artifacts in a registry that maps IDs to file paths, enabling the REST endpoints
  4. Automatic registration - When artifacts are created, they're automatically registered in your custom registry for serving via HTTP endpoints

The includeArtifacts Flag:

The flag is currently implemented in middleware (internal/artifacts/middleware.go) but is not fully integrated with the ADK framework. It checks for:

  • Query parameter: ?includeArtifacts=true
  • Request body parameter: "includeArtifacts": true

However, this middleware approach has limitations because it can't access the deeper ADK task handling system.

Recommendation:

You're right to consider removing the includeArtifacts flag for now. Here's why:

  1. Incomplete integration - The current middleware approach can't properly integrate with ADK's internal task response formatting
  2. Artifact helper still valuable - The helper provides essential functionality beyond just the flag:
    • Registry management for your REST endpoints
    • Enhanced artifact creation with metadata
    • URL generation for accessing artifacts via your server
  3. Future-ready architecture - The infrastructure is well-designed for when ADK provides proper hooks

Suggested approach:

  • Keep the artifact helper and registry system (they provide real value)
  • Remove the middleware and includeArtifacts logic
  • Focus on the core artifact serving functionality that's working well

The artifact helper is essential for your artifact server to function - without it, there's no way to map artifact IDs to files for the REST endpoints.

@edenreich
Copy link
Contributor Author

@claude Let's simplify it, we can remove includeArtifacts for now and rely more on the ADK rather then creating more utilities - less code is better.

@claude
Copy link
Contributor

claude bot commented Sep 24, 2025

Claude finished @edenreich's task —— View job


Simplifying artifacts implementation ✅

I've successfully simplified the artifacts implementation by removing the includeArtifacts functionality and reducing custom utilities in favor of relying more on ADK.

Todo List:

  • Analyze current artifacts implementation to understand simplification opportunities
  • Remove includeArtifacts middleware functionality
  • Simplify artifact helper to rely more on ADK
  • Update main.go to remove middleware registration
  • Update and fix tests after changes
  • Run linting and tests to ensure everything works
  • Update documentation to reflect changes

Summary of Changes

Files Removed:

  • internal/artifacts/middleware.go (168 lines) - Completely removed the includeArtifacts middleware functionality

Files Simplified:

  • internal/artifacts/helper.go - Renamed EnhancedArtifactHelper to ArtifactHelper, removed baseURL dependencies, streamlined the implementation to focus on essential registry integration for REST endpoints
  • internal/artifacts/manager.go - Removed baseURL parameter from InitializeGlobalManager
  • main.go - Updated call to InitializeGlobalManager to remove baseURL parameter
  • Tests - Updated all tests to work with simplified interface, fixed race condition in server test
  • Documentation - Updated to reflect the simpler architecture and removal of includeArtifacts functionality

Key Improvements:

  • Reduced code complexity - Removed 168+ lines of middleware code
  • Better ADK integration - Now relies more on ADK's built-in artifact handling
  • Simplified configuration - No more baseURL configuration needed
  • Cleaner architecture - Focus on core artifact serving functionality
  • All tests passing - Full test suite passes with 0 linting issues
  • Maintained functionality - Core artifact server and REST endpoints still work perfectly

The implementation now follows the "less code is better" principle while maintaining all essential functionality for artifact creation, storage, and serving via REST endpoints. The artifact registry is still essential for the REST endpoints to function, but the implementation is much simpler and relies more on ADK's capabilities.


claude bot and others added 2 commits September 24, 2025 13:37
…mentation

Co-authored-by: Eden Reich <edenreich@users.noreply.github.com>
… middleware

- Rename EnhancedArtifactHelper to ArtifactHelper and simplify implementation
- Remove baseURL parameter dependencies  
- Focus on core artifact serving functionality with minimal custom utilities
- Rely more on ADK's built-in artifact handling
- Update tests and documentation to reflect simplifications
- All tests passing with no linting issues

Co-authored-by: Eden Reich <edenreich@users.noreply.github.com>
@edenreich
Copy link
Contributor Author

@claude To simplify it, let's remove the GET /artifacts/ - list all artifacts endpoint. Clients should request only the artifacts associated with the submitted task.

@claude
Copy link
Contributor

claude bot commented Sep 24, 2025

Claude finished @edenreich's task —— View job


Successfully removed GET /artifacts/ endpoint for simplification ✅

Todo List:

  • Analyze current artifacts server implementation to identify the list endpoint
  • Remove the GET /artifacts/ endpoint from server code
  • Update tests to remove tests for the deleted endpoint
  • Update documentation to reflect the removed endpoint
  • Run tests to ensure everything still works
  • Commit changes to the PR branch

Summary

Successfully removed the GET /artifacts/ list endpoint as requested to simplify the artifacts implementation. The changes include:

Code Changes:

  • Removed endpoint: GET /artifacts/ route handler and listArtifacts function from server.go
  • Removed registry method: ListArtifacts() method no longer needed
  • Updated tests: Modified all test files to avoid using the removed listing functionality
  • Updated docs: Removed documentation for the deleted endpoint

Key Benefits:

  • Simplified API: Clients now focus on task-specific artifact access only
  • Reduced complexity: Fewer endpoints to maintain and test
  • Better separation of concerns: Registry only handles direct artifact lookup
  • All tests pass: Full test suite continues to work with 0 linting issues

The remaining endpoints (GET /artifacts/{id} for download and GET /artifacts/{id}/metadata for metadata) continue to work perfectly for accessing specific artifacts generated during task execution.


claude bot and others added 2 commits September 24, 2025 22:48
…lementation

- Removes the `GET /artifacts/` endpoint and associated `listArtifacts` handler
- Removes unused `ListArtifacts()` method from ArtifactRegistry
- Updates tests to remove dependencies on listing functionality
- Updates documentation to reflect removed endpoint
- Maintains focus on task-specific artifact access as requested

Clients should request only artifacts associated with submitted tasks rather than listing all artifacts.

Co-authored-by: Eden Reich <edenreich@users.noreply.github.com>
Signed-off-by: Eden Reich <eden.reich@gmail.com>
@edenreich edenreich changed the title feat(artifacts): Implement A2A artifacts server with comprehensive support feat(artifacts): Implement A2A artifacts server Sep 26, 2025
…ntation

Signed-off-by: Eden Reich <eden.reich@gmail.com>
Signed-off-by: Eden Reich <eden.reich@gmail.com>
@edenreich edenreich closed this Oct 1, 2025
@edenreich edenreich deleted the claude/issue-31-20250924-1249 branch October 6, 2025 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant