Skip to content

Fix Agent ID header#315

Merged
miles-kt-inkeep merged 1 commit intomainfrom
fix/delegation-headers
Sep 25, 2025
Merged

Fix Agent ID header#315
miles-kt-inkeep merged 1 commit intomainfrom
fix/delegation-headers

Conversation

@miles-kt-inkeep
Copy link
Copy Markdown
Contributor

No description provided.

@vercel
Copy link
Copy Markdown

vercel Bot commented Sep 25, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
agents-manage-api Ready Ready Preview Comment Sep 25, 2025 7:26pm
agents-manage-ui Ready Ready Preview Comment Sep 25, 2025 7:26pm
agents-run-api Ready Ready Preview Comment Sep 25, 2025 7:26pm
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
agents-docs Skipped Skipped Sep 25, 2025 7:26pm

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Sep 25, 2025

Claude finished @miles-kt-inkeep's task —— View job


Code Review Complete

Todo List:

  • Read and understand the changed file
  • Research related code patterns and agent ID handling
  • Analyze the change for potential issues and improvements
  • Provide comprehensive code review feedback

Summary

This PR addresses a consistency issue in the API key authentication middleware. The change ensures that the agentId from the x-inkeep-agent-id header is properly assigned to the execution context in all authentication flows, fixing a missing assignment in the development/test environment path.

The Change

File: agents-run-api/src/middleware/api-key-auth.ts

  • Added: executionContext.agentId = agentId; on line 41 (in the development/test API key success branch)
  • Import reordering: Minor import reorganization for consistency

Analysis

What's Done Well

  • Fixes a legitimate inconsistency: The production paths (lines 119, 145) already assign agentId, but the dev/test path was missing it
  • Maintains existing patterns: Follows the same approach used elsewhere in the middleware
  • Preserves behavior: No breaking changes to existing functionality
  • Small, focused change: Minimal risk of introducing side effects

🔍 Areas for Consideration

1. Missing Test Coverage for Agent ID Assignment (Medium Priority)

The existing tests in api-key-auth.test.ts don't verify that agentId is properly assigned to the execution context when the x-inkeep-agent-id header is present.

// Test should verify this case:
const res = await app.request('/', {
  headers: {
    Authorization: 'Bearer sk_test_...',
    'x-inkeep-agent-id': 'test-agent-123'
  }
});
expect(body.agentId).toBe('test-agent-123'); // Currently not tested

2. Potential for Further Consolidation (Low Priority)

There's repetition of executionContext.agentId = agentId; in three locations (lines 41, 119, 145). Consider extracting this into a helper function if this pattern grows:

const applyAgentIdToContext = (context: ExecutionContext, agentId: string | undefined) => {
  if (agentId) context.agentId = agentId;
  return context;
};

3. Header Validation Consistency (Low Priority)

The agentId is extracted from headers but there's no validation that it matches expected patterns (unlike tenant/project/graph IDs which have validation in bypass mode). Consider if agent ID format validation would be beneficial.

🎯 System Design Considerations

Agent-to-Agent (A2A) Communication Context

This fix is critical for proper A2A delegation flows. The x-inkeep-agent-id header is used in:

  • Delegation calls (relationTools.ts:228) where agents delegate tasks to other agents
  • Execution routing (executionHandler.ts) for directing requests to specific agents

Without this assignment in dev/test mode, delegation chains could break or lose agent context.

Security Implications

The change is security-neutral - it only assigns a header value that's already being extracted and used in production flows. The header is used for internal routing, not authorization.

Recommendations

Immediate Actions

The PR is ready to merge - it fixes a legitimate bug without introducing risks.

Follow-up Improvements 📋

  1. Add test coverage for agent ID assignment in all auth flows
  2. Consider consolidation if this pattern repeats further
  3. Add integration test verifying delegation works correctly in dev/test environments

Architectural Impact

  • No breaking changes
  • Maintains API compatibility
  • Fixes development environment inconsistency
  • Supports proper A2A delegation flows

Verdict: LGTM - This is a solid bug fix that maintains consistency across authentication flows and ensures proper agent context propagation in development/test environments.

@miles-kt-inkeep miles-kt-inkeep added this pull request to the merge queue Sep 25, 2025
Merged via the queue into main with commit 15c8b70 Sep 25, 2025
7 checks passed
@miles-kt-inkeep miles-kt-inkeep deleted the fix/delegation-headers branch September 25, 2025 19:35
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