Fetch Definition requiredToFetch property

[miles-kt-inkeep]

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: aae9aaa18dab2b38c02494c0912c8bf3a9e29a16

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages

Name	Type
@inkeep/agents-cli	Patch
@inkeep/agents-core	Patch
@inkeep/agents-manage-api	Patch
@inkeep/agents-manage-ui	Patch
@inkeep/agents-run-api	Patch
@inkeep/agents-sdk	Patch
@inkeep/create-agents	Patch
@inkeep/ai-sdk-provider	Patch
@inkeep/agents-manage-mcp	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel[bot]]

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

Project	Deployment	Preview	Comments	Updated (UTC)
agents-docs	Ready	Preview	Comment	Dec 2, 2025 8:59pm
agents-manage-api	Ready	Preview	Comment	Dec 2, 2025 8:59pm
agents-manage-ui	Ready	Preview	Comment	Dec 2, 2025 8:59pm
agents-run-api	Ready	Preview	Comment	Dec 2, 2025 8:59pm

[claude[bot]]

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

---

TypeScript Staff Engineer Code Review

I've completed a comprehensive review of the requiredToFetch property addition. This feature adds validation to ensure required context variables are present before making fetch requests, which is a valuable addition to the context fetching system.

Summary of Changes

• Type Definition: Added requiredToFetch?: Array<string> to ContextFetchDefinition['fetchConfig'] in packages/agents-core/src/types/utility.ts:132
• Implementation: Added validation logic in ContextFetcher.resolveTemplateVariables() (lines 200-212)
• Error Handling: Created MissingRequiredVariableError class for specific error cases
• Test Coverage: Added comprehensive test suite covering various validation scenarios (207 new test lines)

Critical Issues Requiring Immediate Attention

🚨 1. Missing Validation Schema Update (High Priority)

Location: packages/agents-core/src/validation/schemas.ts:714-728

The FetchConfigSchema validation schema is missing the new requiredToFetch property, creating a significant disconnect between the TypeScript types and runtime validation:

export const FetchConfigSchema = z
  .object({
    url: z.string().min(1, 'URL is required'),
    method: z.enum(['GET', 'POST', 'PUT', 'DELETE', 'PATCH']).optional().default('GET'),
    headers: z.record(z.string(), z.string()).optional(),
    body: z.record(z.string(), z.unknown()).optional(),
    transform: z.string().optional(),
    timeout: z.number().min(0).optional().default(CONTEXT_FETCHER_HTTP_TIMEOUT_MS_DEFAULT).optional(),
    // Missing: requiredToFetch: z.array(z.string()).optional(),
  })

Impact:

• API requests with requiredToFetch will be rejected by Zod validation
• Users can't actually use this feature through the API
• Type/runtime validation inconsistency

Recommendation: Add the field to FetchConfigSchema immediately.

🚨 2. Builder Pattern Inconsistency (Medium Priority)

Location: packages/agents-core/src/context/ContextConfig.ts:53-68

The builderFetchDefinition type in the ContextConfig builder doesn't include requiredToFetch, meaning users can't configure this property through the builder pattern:

export type builderFetchDefinition<R extends z.ZodTypeAny> = {
  id: string;
  name?: string;
  trigger: 'initialization' | 'invocation';
  fetchConfig: {
    url: string;
    method?: string;
    headers?: Record<string, string>;
    body?: Record<string, unknown>;
    transform?: string;
    timeout?: number;
    // Missing: requiredToFetch?: Array<string>;
  };
  // ...
};

Impact: Incomplete feature - users can't configure requiredToFetch through the recommended builder pattern.

Design & Implementation Analysis

✅ Strengths

1. Clear Intent: The feature addresses a real need - preventing wasteful HTTP requests when required context is missing

2. Robust Error Handling: The MissingRequiredVariableError provides specific error messaging with the problematic variable name

3. Comprehensive Test Coverage: Excellent test scenarios covering:

   • Single/multiple required variables
   • Empty string validation
   • Nested variable paths
   • Edge cases (empty arrays, missing arrays)

4. Non-Breaking Change: The optional field maintains backward compatibility

5. Fail-Fast Behavior: Early validation prevents unnecessary network requests

🔍 Design Considerations

1. Template String Validation Logic (Line 208-210):

   if (resolvedVariable === '' || resolvedVariable === variable) {
     throw new MissingRequiredVariableError(variable);
   }
   

   Concern: This logic treats unresolved templates (where resolvedVariable === variable) as missing. While this makes sense for template variables, it could create confusion for literal strings.

   Example Edge Case: If someone accidentally puts a literal string like "user_id_123" in requiredToFetch instead of "{{user.id}}", it would be considered "missing" even though it's a valid string.

   Recommendation: Consider adding a check to distinguish between template variables and literal values, or document this behavior clearly.

2. Early Validation Placement: The validation occurs before credential injection and URL resolution. This is good for performance but could potentially miss some edge cases where credentials affect the context.

🛠 Implementation Quality

1. Error Handling: Good use of custom error class for specific error scenarios

2. Logging: Appropriate logging is missing for the validation failures - consider adding debug logs when validation fails

3. Performance: Validation is efficient and fails fast, preventing unnecessary processing

System Architecture Impact

👍 Positive Impacts

• Reduces unnecessary API calls and associated costs
• Provides better debugging experience with specific error messages
• Maintains system responsiveness by avoiding doomed requests

⚠️ Potential Concerns

• Additional validation overhead (minimal, but worth noting)
• Could make configuration more complex for users who need to understand template resolution order

Recommendations

Immediate Actions Required

1. Update Validation Schema: Add requiredToFetch to FetchConfigSchema

   export const FetchConfigSchema = z.object({
     // ... existing fields
     requiredToFetch: z.array(z.string()).optional(),
   });
   

2. Update Builder Pattern: Add requiredToFetch to builderFetchDefinition type

3. Add Documentation: Include clear examples and edge case documentation

Nice-to-Have Improvements

1. Enhanced Logging: Add debug logs for validation failures
2. Template Validation: Consider validating that requiredToFetch items are actually template strings
3. Builder Helper: Consider adding type-safe template helpers in the builder pattern

Testing

The test coverage is excellent and comprehensive. No additional test scenarios are needed.

Overall Assessment

This is a well-implemented feature that adds valuable functionality to the context fetching system. The core implementation is solid with good error handling and comprehensive tests. However, the missing validation schema updates are critical blockers that prevent the feature from being used in practice.

Recommendation: Fix the validation schema issues immediately, then this PR will be ready to merge.

---