Conversation
mike-inkeep
commented
Feb 5, 2026
- Add image handling support (without persistence to conversation history)
- Tweak image URL schema and tighten up tests
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
|
Claude finished @mike-inkeep's task in 4m 57s —— View job PR Review Summary5 Key Findings | Risk: Medium 🟠🔶
|
| Location | Issue | Reason Excluded |
|---|---|---|
message-parts.ts:5-17 |
No size limit on base64 data URIs (unlike schemas.ts which has 1MB limit) |
Different use case - request-level validation vs storage; AI providers handle large payloads |
imageUrlSchema |
No internal IP blocklist for SSRF protection | URLs are passed to AI providers who fetch them; SSRF is provider's responsibility |
Agent.ts:3290 |
new URL(file.uri) could throw if URI is malformed |
Covered by upstream validation in route handlers |
chat.ts:343-348 |
Images not persisted to conversation history | Intentional per PR description |
chatDataStream.ts:68 |
Vercel schema uses text field for image URL (unconventional naming) |
Matches Vercel AI SDK v5 convention |
Discarded as invalid or not applicable
| Location | Issue | Reason Excluded |
|---|---|---|
Agent.ts:3288-3291 |
file.bytes access not narrowed after union check |
TypeScript narrowing via 'uri' in file is correct; the discriminated union ensures mutual exclusivity |
| Route handlers | Missing rate limiting for image uploads | Pre-existing concern not introduced by this PR |
generateTaskHandler.ts |
No image count limits | Reasonable for MVP; can be added if abuse occurs |
| export const isImageContentItem = ( | ||
| item: ContentItem | ||
| ): item is { type: 'image_url'; image_url: { url: string; detail?: 'auto' | 'low' | 'high' } } => { | ||
| return ( | ||
| item.type === 'image_url' && | ||
| 'image_url' in item && | ||
| imageUrlSchema.safeParse(item.image_url?.url).success | ||
| ); | ||
| }; |
There was a problem hiding this comment.
MAJOR Type guard uses inline type instead of imported ImageContentItem
The type predicate re-declares the type inline instead of using the imported ImageContentItem type from ../types/chat. This creates maintenance risk if the type changes.
| export const isImageContentItem = ( | |
| item: ContentItem | |
| ): item is { type: 'image_url'; image_url: { url: string; detail?: 'auto' | 'low' | 'high' } } => { | |
| return ( | |
| item.type === 'image_url' && | |
| 'image_url' in item && | |
| imageUrlSchema.safeParse(item.image_url?.url).success | |
| ); | |
| }; | |
| export const isImageContentItem = ( | |
| item: ContentItem | |
| ): item is ImageContentItem => { | |
| return ( | |
| item.type === 'image_url' && | |
| 'image_url' in item && | |
| item.image_url != null && | |
| imageUrlSchema.safeParse(item.image_url.url).success | |
| ); | |
| }; |
| .string() | ||
| .regex( | ||
| /^data:image\/(png|jpeg|jpg|webp);base64,/, | ||
| 'Image must be PNG, JPEG, or WebP format (GIF not supported by all providers)' |
There was a problem hiding this comment.
can you check this? i believe we can support the gif file format. but it's just that only a single frame will work, it cannot understand animated gifs. and that is ok
There was a problem hiding this comment.
claude and chatgpt both support GIF, but Gemini only supports these: https://ai.google.dev/gemini-api/docs/image-understanding#supported-formats
Gemini supports the following image format MIME types:
PNG - image/png
JPEG - image/jpeg
WEBP - image/webp
HEIC - image/heic
HEIF - image/heif
Alternatively, we can accept GIF and let the downstream provider error bubble up if we hit this case
There was a problem hiding this comment.
I think skip gif for now until we figure out how to properly expose agent capabilities to the front end for client side validation.
|
Claude finished @mike-inkeep's task in 5m 34s —— View job PR Review Summary1 Key Finding | Risk: Low This is a follow-up review after the PR was synchronized with commit ✅ Previous Feedback AddressedThe latest commit successfully addressed the following items from the previous review:
🟠🔶
|
| Location | Issue | Reason Excluded |
|---|---|---|
chatDataStream.ts:58 |
content: z.any() is permissive |
Intentional for backwards compatibility; actual parts are validated separately |
Agent.ts:3296 |
Fallback mimeType: 'image/*' for URL-based images |
Acceptable — providers handle MIME type detection from URL |
Discarded as invalid or not applicable
| Location | Issue | Reason Excluded |
|---|---|---|
message-parts.ts:8-20 |
No size limit on base64 data URIs | Different use case than storage validation; AI providers handle payload limits |
Previous: buildFilePart URL validation |
Was flagged as missing | ✅ Now addressed in latest commit |
| Previous: Type guard using inline type | Was flagged as maintenance risk | ✅ Now uses imported ImageContentItem |
| }; | ||
|
|
||
| export type ImageContentItem = { | ||
| type: 'image_url'; |
There was a problem hiding this comment.
Are we redefining types and schemas?
I think it's a mistake made throughout our codebase, but we try to reduce schemas and types were we can.
You can infer types from zod schemas. https://zod.dev/basics?id=inferring-types
|
|
||
| export type UserInput = | ||
| | string | ||
| | { |
There was a problem hiding this comment.
I think at this point we can follow the A2A protocol for parts.
I don't want to introduce too many intermediate types in the message handling.
I think we support: Vercel Streaming, A2A, OpenAI Completions, but our primary pipeline (our widget to our agent framework) does Vercel Streaming (Client Widget to Agents Server) to A2A (Agent Server internal messaging)
There was a problem hiding this comment.
I suggest defining types int he packages where we define A2A types or just re-using the existing by scoping down/omitting properties that we are not supporting in the A2A at this time.
| type: string; | ||
| text?: string; | ||
| image?: string | URL; | ||
| experimental_providerMetadata?: { openai?: { imageDetail?: 'auto' | 'low' | 'high' } }; |
There was a problem hiding this comment.
Even just the enum component can make a useful extraction as a schema so we don't have to redefine the enum set.
Pattern extracted from PR #1737 human reviewer feedback (amikofalvy): - Types should derive from Zod schemas using z.infer<typeof schema> - Use Pick/Omit/Partial instead of manually redefining type subsets - Extract shared enum/union schemas instead of inline string literals Changes: - pr-review-types.md: New anti-pattern + analysis step 6 with detection patterns - pr-review-consistency.md: Extended "Reuse" section to cover types This demonstrates the closed-pr-review-auto-improver output — these are the exact changes the agent proposed when run against PR #1737. Co-Authored-By: Claude <noreply@anthropic.com>
Patterns extracted from human reviewer feedback: - Type Definition Discipline: use z.infer<> instead of manual type definitions - Type Composition Safety: discriminated unions for mutually exclusive states Co-Authored-By: Claude <noreply@anthropic.com>
- Add manual trigger with pr_number input - Add Get PR Metadata step to fetch data via API (works for both triggers) - Update all PR references to use the new metadata outputs - Enables testing against historical PRs like #1737 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* feat: Add closed-pr-review-auto-improver agent Automated system that analyzes human reviewer feedback after PRs are merged to identify generalizable improvements for the pr-review-* subagent system. - Workflow triggers on merged PRs, extracts human/bot comments - Agent applies 4-criteria generalizability test - Creates draft PRs with improvements to pr-review-*.md files Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat: Add context-gathering phase for deeper comment analysis - Include diffHunk in GraphQL query (shows code each comment is on) - Add Phase 2 "Deep-Dive on Promising Comments" with explicit guidance: - Read the full file to understand broader context - Grep for schemas/types/patterns mentioned in comments - Understand the anti-pattern before judging generalizability - Update Tool Policy to emphasize context gathering - Renumber phases (now 6 phases total) The agent now actively investigates each comment rather than judging based on comment text alone. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: Apply write-agent best practices Based on write-agent skill guidance: 1. Add near-miss example (questions/discussions ≠ reviewer feedback) 2. Strengthen Role & Mission - describe what "excellence looks like" 3. Failure modes now use contrastive examples (❌ vs ✅) 4. Phase 2 now checklist format with stop condition 5. Example shows completed checklist, not just steps Key insight: "Stop here if you can't articulate a clear principle" prevents vague improvements from polluting reviewers. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat: Add git time-travel for progressive context gathering - Phase 2 now uses git rev-list + git show to see code at comment time - Progressive gathering: diffHunk → full file → PR diff → other files - GraphQL query now includes createdAt for all comment types - Added git rev-list and git show to allowedTools This ensures the agent sees what the human reviewer saw, not the final merged state which may have fixes applied. Co-Authored-By: Claude <noreply@anthropic.com> * feat: Add explicit stop conditions for context gathering Two exit paths at each level: - EXIT A: Not generalizable (repo-specific, one-off bug, style preference) - EXIT B: Pattern found (can articulate anti-pattern + universal principle) Includes decision flow diagram and two contrasting examples showing early exit (repo-specific DateUtils) vs pattern discovery (type/schema DRY). Co-Authored-By: Claude <noreply@anthropic.com> * refactor: Strengthen agent per write-agent best practices - Role & Mission: Add "what the best human analyst would do" section - Failure modes: Add "Asserting when uncertain" with contrastive example - Generalizability: Add confidence calibration guidance - Add explicit conservative default: "when torn, choose lower confidence" Per write-agent skill review: personality should describe best human behavior, failure modes should include asserting when uncertain (relevant for classification tasks). Co-Authored-By: Claude <noreply@anthropic.com> * pr-review: Add Schema-Type Derivation Discipline learnings Pattern extracted from PR #1737 human reviewer feedback (amikofalvy): - Types should derive from Zod schemas using z.infer<typeof schema> - Use Pick/Omit/Partial instead of manually redefining type subsets - Extract shared enum/union schemas instead of inline string literals Changes: - pr-review-types.md: New anti-pattern + analysis step 6 with detection patterns - pr-review-consistency.md: Extended "Reuse" section to cover types This demonstrates the closed-pr-review-auto-improver output — these are the exact changes the agent proposed when run against PR #1737. Co-Authored-By: Claude <noreply@anthropic.com> * pr-review: Expand type derivation sources Extended "Schema-Type Derivation Discipline" to cover full spectrum: - Zod/validation schemas (z.infer) - Database schemas (Prisma, Drizzle generated types) - Internal packages (@inkeep/*, shared types) - External packages/SDKs (OpenAI, Vercel AI SDK) - Function signatures (Parameters<>, ReturnType<>) - Existing domain types (Pick, Omit, Partial) Added table format for clarity and comprehensive detection patterns. Co-Authored-By: Claude <noreply@anthropic.com> * pr-review: Add advanced type derivation patterns from codebase research Expanded type derivation guidance based on actual patterns found in agents repo: - Awaited<ReturnType<>> for async function returns - keyof typeof for constants-derived types - interface extends and intersection (&) for composition - Discriminated unions with type guards - satisfies operator for type-safe constants - Re-exports for API surface boundaries - Type duplication detection signals Patterns sourced from agents-api codebase analysis including: - env.ts, middleware/*, types/app.ts, domains/run/* Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * pr-review: Add Zod schema composition patterns Added guidance for Zod schema extension/derivation patterns based on codebase research (packages/agents-core/src/validation/schemas.ts): - .extend() for adding/overriding fields - .pick()/.omit() for field subsetting - .partial() for Insert → Update schema derivation - .extend().refine() for cross-field validation - Anti-patterns: parallel schemas, duplicated fields Examples from codebase: - SubAgentInsertSchema.extend({ id: ResourceIdSchema }) - SubAgentUpdateSchema = SubAgentInsertSchema.partial() - StopWhenSchema.pick({ transferCountIs: true }) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * pr-review: Rationalize scope between types and consistency agents Clear separation of concerns: - pr-review-types: Illegal states, invariants, unsafe narrowing - pr-review-consistency: DRY, schema reuse, convention conformance Moved to consistency: - Zod schema composition patterns (.extend, .pick, .partial) - Type derivation detection signals - satisfies operator, re-exports conventions Kept in types (type safety focus): - Discriminated unions vs optional fields (prevents illegal states) - Type guards vs unsafe `as` assertions - Detection of union types without discriminants Added cross-reference note in types agent pointing to consistency for derivation/DRY concerns. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * closed-pr-review-auto-improver: Add exit states, skills integration, and Phase 5.5 - Add skills: pr-review-subagents-available, pr-review-subagents-guidelines, find-similar-patterns - Add proper exit states at Phase 1, 2, and 4 (embedded in workflow, not separate section) - Add Phase 5 step 2: "Find examples of the pattern" with judgment guidance - Add Phase 5.5: Full file review & integration planning (scope fit, duplication check) - Update output contract with detailed JSON structure and exit examples - Add reviewer tagging to close the feedback loop Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * closed-pr-review-auto-improver: Add "keep agents standalone" guidance Agents should be self-contained without cross-references to other agents. This prevents coupling and ensures agents work correctly when read in isolation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Recover lost skills: find-similar-patterns, pr-review-subagents-* These skills were created in the previous session but never committed. Recovered from conversation history. - find-similar-patterns: Methodology for finding similar code patterns - pr-review-subagents-available: Catalog of pr-review-* agents with scope boundaries - pr-review-subagents-guidelines: Best practices for writing/improving reviewers Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Remove pr-review-* changes (moved to separate PR #1759) The pr-review-consistency.md and pr-review-types.md improvements belong in PR #1759, not this auto-improver feature branch. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: Move auto-improver to private plugin repo Move agent and skills to inkeep/internal-cc-plugins for CI/CD-only loading: - Removed: .claude/agents/closed-pr-review-auto-improver.md - Removed: .agents/skills/{find-similar-patterns,pr-review-subagents-available,pr-review-subagents-guidelines}/ Updated workflow: - Added step to clone inkeep/internal-cc-plugins - Added --plugin-dir flag to load agent from plugin Prerequisites before merging: 1. Create private repo: inkeep/internal-cc-plugins 2. Push plugin content to new repo 3. Add GH_PAT_PLUGINS secret to inkeep/agents Co-Authored-By: Claude <noreply@anthropic.com> * Switch from PAT to GitHub App for cross-repo auth GitHub Apps provide better security and maintainability: - 8-hour token lifetime (vs days/infinite for PATs) - No user account dependency (survives personnel changes) - Zero manual rotation (tokens generated fresh each run) - Scales to N plugins without additional credentials Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add workflow_dispatch trigger for testing historical PRs - Add manual trigger with pr_number input - Add Get PR Metadata step to fetch data via API (works for both triggers) - Update all PR references to use the new metadata outputs - Enables testing against historical PRs like #1737 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address PR review feedback: robustness improvements Fixes from Claude Code review: 1. Add merge validation for workflow_dispatch (Major) - Prevents analyzing unmerged PRs via manual trigger - Validates PR is merged before proceeding 2. Use unique HEREDOC delimiters (Major) - Prevents collision if PR body/comments contain "EOF" - Uses unique suffixes like __BODY_DELIM_7f3a9b2c__ 3. Pin claude-code-action to SHA (Major) - Aligns with claude-code-review.yml for consistency - Tracks issue #892 for AJV validation bug 4. Add concurrency control (Minor) - Prevents race conditions on concurrent runs - Groups by PR number, doesn't cancel in-progress 5. Add shell error handling (Minor) - set -eo pipefail in all shell blocks - Fail fast on command errors Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add debug artifact upload for troubleshooting Uploads execution logs when workflow fails, matching pattern from claude-code-review.yml. 7-day retention. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
* pr-review: Add type derivation and composition patterns from PR #1737 Patterns extracted from human reviewer feedback: - Type Definition Discipline: use z.infer<> instead of manual type definitions - Type Composition Safety: discriminated unions for mutually exclusive states Co-Authored-By: Claude <noreply@anthropic.com> * fix: Remove cross-agent references to keep agents standalone Agents should be self-contained without referencing what other agents do. - pr-review-consistency: Remove "→ see pr-review-types" reference - pr-review-types: Change "reviewed by pr-review-consistency" to "out of scope" Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>