fix: support image paste attachments#791
Open
nrslib wants to merge 2 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
💤 Files with no reviewable changes (3)
📝 WalkthroughWalkthrough中断された running タスクは起動時に pending に戻すのではなく failed にマークされます。対話モードはクリップボード画像の貼り付けとネイティブ画像転送に対応し、Claude/Codex 向けの画像付きプロンプトを組み立てます。孤立クローンは base ブランチを refs/takt/base にフェッチします。 ChangesInterrupted Task Cleanup: Failed Status Instead of Recovery
Clipboard / Native Image Input and Interactive Changes
Provider implementations and transports
Tests and test infrastructure
Clone Infrastructure: Base Branch Fetch
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/infra/providers/kiro.ts (1)
9-24: ⚡ Quick winAdd warning for unsupported
imageAttachmentsto match the established pattern.The
toKiroOptionsfunction logs warnings for unsupported features (allowedTools,mcpServers,maxTurns,outputSchema,model), but does not warn whenoptions.imageAttachmentsis provided. For consistency and better user feedback, add a similar check.📝 Suggested addition
if (options.model) { log.info('Kiro provider does not support model CLI flag; ignoring'); } + if (options.imageAttachments && options.imageAttachments.length > 0) { + log.info('Kiro provider does not support imageAttachments; ignoring'); + } return {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/infra/providers/kiro.ts` around lines 9 - 24, The toKiroOptions function currently logs when several unsupported ProviderCallOptions fields are used but omits imageAttachments; add a check in toKiroOptions that detects options.imageAttachments (truthy length or presence) and calls log.info('Kiro provider does not support imageAttachments; ignoring') to match the established pattern and provide consistent user feedback.src/infra/claude/image-input.ts (1)
25-43: Clarify error surfacing for image attachment file reads
readFile(attachment.path)insrc/infra/claude/image-input.tsthrows without local try/catch, but failures occurring during async prompt generation are wrapped bysrc/infra/claude/executor.ts’s outertry/catch(around thefor awaitloop) and returned asClaudeResult.errorviagetErrorMessage(error)(using the underlyingError.message, which for fs failures typically includes the missing/unreadable path).
Optional: wrapreadFileto rethrow with attachment context (e.g., includeattachment.path) so diagnostics stay clear even if the fs message is less descriptive.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/infra/claude/image-input.ts` around lines 25 - 43, In buildContentBlocks, wrap the await readFile(attachment.path) call in a try/catch so FS errors are rethrown with attachment context; catch the error, augment it (or throw a new Error) that includes the attachment.path (and optionally formatImageAttachmentPathReference(attachment) or attachment.id) before rethrowing so executor.ts/getErrorMessage surfaces a clear, contextual message; keep the rest of the block (image payload creation using inferMediaType and base64) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/features/interactive/imageAttachments.ts`:
- Around line 124-126: The current filter uses
prompt.includes(attachment.placeholder) which allows substring matches (e.g.,
"[Image `#1`]" matches "[Image `#10`]"); change the matching to exact token matching
by checking for whole-token equality—either split the prompt into tokens (e.g.,
prompt.split(/\s+/).includes(attachment.placeholder)) or build an escaped-regex
that enforces boundaries (e.g., new
RegExp(`(?<!\\S)${escapeRegex(attachment.placeholder)}(?!\\S)`).test(prompt));
update the filter in the attachments.filter(...) expression to use that
exact-match logic and add/implement an escapeRegex helper if using regex.
---
Nitpick comments:
In `@src/infra/claude/image-input.ts`:
- Around line 25-43: In buildContentBlocks, wrap the await
readFile(attachment.path) call in a try/catch so FS errors are rethrown with
attachment context; catch the error, augment it (or throw a new Error) that
includes the attachment.path (and optionally
formatImageAttachmentPathReference(attachment) or attachment.id) before
rethrowing so executor.ts/getErrorMessage surfaces a clear, contextual message;
keep the rest of the block (image payload creation using inferMediaType and
base64) unchanged.
In `@src/infra/providers/kiro.ts`:
- Around line 9-24: The toKiroOptions function currently logs when several
unsupported ProviderCallOptions fields are used but omits imageAttachments; add
a check in toKiroOptions that detects options.imageAttachments (truthy length or
presence) and calls log.info('Kiro provider does not support imageAttachments;
ignoring') to match the established pattern and provide consistent user
feedback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1252e22a-6d37-49f9-a86d-7ebb43f7937c
📒 Files selected for processing (65)
docs/task-management.ja.mddocs/task-management.mddocs/testing/e2e.mde2e/specs/run-recovery.e2e.tssrc/__tests__/clone.test.tssrc/__tests__/codex-client-retry.test.tssrc/__tests__/commandMatcher.test.tssrc/__tests__/conversationLoop-resume.test.tssrc/__tests__/helpers/stdinSimulator.tssrc/__tests__/imageAttachments.test.tssrc/__tests__/interactiveInput.test.tssrc/__tests__/lineEditor.test.tssrc/__tests__/provider-capabilities.test.tssrc/__tests__/provider-image-attachments.test.tssrc/__tests__/provider-structured-output.test.tssrc/__tests__/runAllTasks-concurrency.test.tssrc/__tests__/saveTaskFile.test.tssrc/__tests__/selectAndExecute-skipTaskList.test.tssrc/__tests__/slashCommandRegistry.test.tssrc/__tests__/task.test.tssrc/__tests__/taskSpecContext.test.tssrc/__tests__/watchTasks.test.tssrc/features/interactive/aiCaller.tssrc/features/interactive/clipboardImage.tssrc/features/interactive/clipboardImageFeedback.tssrc/features/interactive/conversationLoop.tssrc/features/interactive/imageAttachments.tssrc/features/interactive/interactive.tssrc/features/interactive/interactiveInput.tssrc/features/interactive/lineEditor.tssrc/features/interactive/passthroughMode.tssrc/features/interactive/quietMode.tssrc/features/interactive/slashCommandRegistry.tssrc/features/tasks/attachments.tssrc/features/tasks/execute/runAllTasks.tssrc/features/tasks/execute/selectAndExecute.tssrc/features/tasks/execute/taskSpecContext.tssrc/features/tasks/watch/index.tssrc/infra/claude/client.tssrc/infra/claude/executor.tssrc/infra/claude/image-input.tssrc/infra/claude/types.tssrc/infra/codex/client.tssrc/infra/codex/types.tssrc/infra/providers/claude-headless.tssrc/infra/providers/claude-terminal.tssrc/infra/providers/claude.tssrc/infra/providers/codex.tssrc/infra/providers/copilot.tssrc/infra/providers/cursor.tssrc/infra/providers/imageAttachmentPrompt.tssrc/infra/providers/kiro.tssrc/infra/providers/mock.tssrc/infra/providers/opencode.tssrc/infra/providers/provider-capabilities.tssrc/infra/providers/types.tssrc/infra/task/clone-exec.tssrc/infra/task/clone.tssrc/infra/task/runner.tssrc/infra/task/taskLifecycleService.tssrc/infra/task/taskRecordMutations.tssrc/shared/constants.tssrc/shared/i18n/labels_en.yamlsrc/shared/i18n/labels_ja.yamlvitest.config.e2e.mock.ts
💤 Files with no reviewable changes (1)
- src/infra/task/taskRecordMutations.ts
Owner
Author
Summaryタスク指示書タスク概要PR #791 のレビューコメントを含めて対応する。Source Context に含まれる PR #791 のレビューコメントを事実情報として参照し、現在のコードに照合したうえで、まだ有効な指摘を実装・テスト・検証まで完了する。 対象 PR: 優先度: 高
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Tests
Review
Summary by CodeRabbit
New Features
Behavior Changes
Improvements