Conversation
|
@BugBot run |
Summary of ChangesHello @azizmejri1, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the agent's capabilities by introducing an AI-powered image generation tool. This allows the agent to create visual assets directly, enhancing its ability to fulfill user requests for custom images, illustrations, and graphics. The changes include the core tool logic, a dedicated UI component for displaying generation progress, and updated agent instructions to guide its usage, providing a more comprehensive and interactive experience for visual content creation. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@BugBot run |
🔍 Dyadbot Code Review SummaryVerdict: 🤔 NOT SURE - Potential issues Reviewed by 3 independent agents: Correctness Expert, Code Health Expert, UX Wizard. Issues Summary
🟢 Low Priority Notes (4 items)
🚫 Dropped False Positives (7 items)
Generated by Dyadbot multi-agent code review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
github-actions
bot
added
the
needs-human:review-issue
|
@BugBot run |
🤖 Claude Code Review SummaryPR Confidence: 4/5All review comments have been addressed with code changes; confidence is high but image generation is a new feature that would benefit from manual testing. Unresolved Threads
No unresolved threads Resolved Threads
Product Principle SuggestionsNo suggestions — principles were clear enough for all decisions. 🤖 Generated by Claude Code |
|
@BugBot run |
🔍 Dyadbot Code Review SummaryVerdict: ✅ YES - Ready to merge Reviewed by 3 independent agents: Correctness Expert, Code Health Expert, UX Wizard. ✅ No confirmed issues found by multi-agent review. The code is well-structured and follows existing codebase patterns. Highlights:
🟢 Low Priority Notes (2 items)
🚫 Dropped False Positives (11 items)
Generated by Dyadbot multi-agent code review |
Add a new `generate_image` tool that calls the Dyad engine's `/images/generations` endpoint to create AI-generated images. The tool saves images to `.dyad/media` and instructs the agent to use `copy_file` to place them in the project codebase. - New tool implementation with engineFetch, Pro-only gating - UI card component (DyadImageGeneration) with violet accent - System prompt guidelines encouraging image generation over placeholders - E2E test fixture and spec - Mock endpoint in fake LLM server - Updated prompt snapshot https://claude.ai/code/session_01J6tXYHo4RvQguFJF6UkHuk
- Add proper type interface for DyadImageGeneration node prop instead of any - Convert sync fs operations (mkdirSync, writeFileSync) to async (fs/promises) - Add modifiesState: true to generate_image tool for plan mode consistency - Add try-catch error handling to prevent stuck "Generating..." UI state Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
azizmejri1
force-pushed
the
image-generation-tool
branch
from
aab19a2 to
d131075
Compare
|
@BugBot run |
🔍 Dyadbot Code Review SummaryVerdict: ✅ YES - Ready to merge Reviewed by 3 independent agents: Correctness Expert, Code Health Expert, UX Wizard. No new HIGH or MEDIUM issues found beyond what existing reviewers have already flagged. The existing review comments (error handling gap, missing Issues SummaryNo new HIGH or MEDIUM issues to report. 🟢 Low Priority Notes (4 items)
🚫 Dropped False Positives (11 items)
Generated by Dyadbot multi-agent code review |
🔍 Dyadbot Code Review SummaryVerdict: ✅ YES - Ready to merge Reviewed by 3 independent agents: Correctness Expert, Code Health Expert, UX Wizard. Issues Summary
🟢 Low Priority Notes (2 items)
🚫 Dropped False Positives (14 items)
Generated by Dyadbot multi-agent code review |
| <DyadStateIndicator state="aborted" abortedLabel="Did not finish" /> | ||
| )} | ||
| <div className="ml-auto"> | ||
| <DyadExpandIcon isExpanded={isExpanded} /> |
There was a problem hiding this comment.
🟡 MEDIUM | missing-state-feedback
No success indicator when image generation completes
When image generation finishes successfully, the card shows no visual confirmation. The spinner disappears with no replacement feedback, making it unclear whether the operation succeeded. This is especially noticeable because image generation is a slow operation where the user is actively watching for completion.
Other similar tools in the codebase (e.g., DyadCopy) display a green checkmark with a completion label via DyadStateIndicator.
💡 Suggestion: Add a finished state indicator:
| <DyadExpandIcon isExpanded={isExpanded} /> | |
| {aborted && ( | |
| <DyadStateIndicator state="aborted" abortedLabel="Did not finish" /> | |
| )} | |
| {state === "finished" && ( | |
| <DyadStateIndicator state="finished" finishedLabel="Generated" /> | |
| )} |
wwwillchen
left a comment
wwwillchen
left a comment
There was a problem hiding this comment.
Great job! It's creating very nice-looking UIs :)
Main comment is to just enable the tool to "always" (see inline comment).
Once the tests are passing, feel free to merge.
Follow-ups:
- One nice follow-up item (can do in this PR or follow-up PR) is to show some kind of preview of the generated image in the chat .
- Also, I ran into an error while using this feature (but it's not specific to this feature so it can be in a separate PR: #2804)
| name: "generate_image", | ||
| description: DESCRIPTION, | ||
| inputSchema: generateImageSchema, | ||
| defaultConsent: "ask", |
There was a problem hiding this comment.
i think we should just set this to "always" - i can't really think of a risk with image generation (e.g. it can't really cause data exfiltration) besides generating images costs money, but this tool shouldn't be called that often and users can disable tools in the settings if they really want to.
github-actions
bot
added
needs-human:final-check
|
@BugBot run |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| await po.agentConsent.waitForAgentConsentBanner(); | ||
| await po.agentConsent.clickAgentConsentAlwaysAllow(); |
There was a problem hiding this comment.
test will timeout here because generate_image tool has defaultConsent: "always", so no consent banner appears
| await po.agentConsent.waitForAgentConsentBanner(); | |
| await po.agentConsent.clickAgentConsentAlwaysAllow(); | |
| // No consent banner needed - generate_image has defaultConsent: "always" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: e2e-tests/local_agent_generate_image.spec.ts
Line: 17-18
Comment:
test will timeout here because `generate_image` tool has `defaultConsent: "always"`, so no consent banner appears
```suggestion
// No consent banner needed - generate_image has defaultConsent: "always"
```
How can I resolve this? If you propose a fix, please make it concise.
🔍 Dyadbot Code Review SummaryVerdict: ✅ YES - Ready to merge Reviewed by 3 independent agents: Correctness Expert, Code Health Expert, UX Wizard. Issues Summary
🟢 Low Priority Notes (5 items)
🚫 Dropped False Positives (4 items)
Generated by Dyadbot multi-agent code review |
| )} | ||
| {children && <div className="mt-0.5 text-foreground">{children}</div>} | ||
| </div> | ||
| </DyadCardContent> |
There was a problem hiding this comment.
🟡 MEDIUM | missing-image-preview
Expanded card shows only a file path, not the actual generated image
When a user expands the card after image generation completes, they see the prompt text and a raw file path like .dyad/media/generated-1234-abc.png, but never the actual image. For an image generation feature, not showing the generated image is a significant UX miss — the user has to navigate to the file manually to verify the output.
💡 Suggestion: Render an inline image preview in the expanded content when the image path is available and generation is complete. For example:
{imagePath && !inProgress && (
<img src={resolvedImagePath} alt={prompt} className="rounded-md max-h-48 mt-2" />
)}| throw new Error(`Failed to download generated image: ${response.status}`); | ||
| } | ||
| const arrayBuffer = await response.arrayBuffer(); | ||
| await fs.writeFile(filePath, Buffer.from(arrayBuffer)); |
There was a problem hiding this comment.
🟡 MEDIUM | security
URL fetch has no validation — SSRF defense-in-depth concern
When the API returns a URL instead of base64, the code fetches it with the global fetch and no URL validation. While the URL comes from a trusted API (Dyad engine), as a defense-in-depth measure, consider either:
- Always requesting
b64_jsonformat by addingresponse_format: 'b64_json'to the API request body (eliminating the URL path entirely) - Validating the URL scheme (HTTPS only) and hostname (no private/loopback ranges) before fetching
This also bypasses any proxy settings or request tracking that engineFetch provides — a comment explaining why raw fetch is appropriate here would help future maintainers.
github-actions
bot
added
needs-human:review-issue
|
@BugBot run |
🔍 Dyadbot Code Review SummaryVerdict: ✅ YES - Ready to merge Reviewed by 3 independent agents: Correctness Expert, Code Health Expert, UX Wizard. Issues Summary
🟢 Low Priority Notes (1 item)
🚫 Dropped False Positives (8 items)
Generated by Dyadbot multi-agent code review |
| const timestamp = Date.now(); | ||
| const fileName = `generated-${timestamp}-${hash}.png`; | ||
| const filePath = path.join(mediaDir, fileName); | ||
| const relativePath = path.join(DYAD_MEDIA_DIR_NAME, fileName); |
There was a problem hiding this comment.
🟡 MEDIUM | data-integrity
relativePath uses platform-specific path separator, breaking on Windows
path.join(DYAD_MEDIA_DIR_NAME, fileName) uses the OS-native path separator. On Windows this produces .dyad\media\generated-...png (backslashes). This path is:
- Returned to the LLM in the tool result string (line 154)
- Embedded in the XML
pathattribute shown in the UI - Used by the LLM with
copy_fileand ultimately in web asset references like<img src="...">
Web paths require forward slashes. The E2E test explicitly skips Windows (testSkipIfWindows), confirming this path is untested on that platform.
💡 Suggestion: Use forward-slash joining explicitly:
const relativePath = DYAD_MEDIA_DIR_NAME + '/' + fileName;
// or: path.posix.join(DYAD_MEDIA_DIR_NAME, fileName)| ctx.onXmlComplete( | ||
| `<dyad-image-generation prompt="${escapeXmlAttr(args.prompt)}"></dyad-image-generation>`, | ||
| ); | ||
| throw error; |
There was a problem hiding this comment.
🟡 MEDIUM | error-handling
Error path emits empty card with no error details in UI
When execute() throws, the catch block calls ctx.onXmlComplete() with an empty <dyad-image-generation> tag (no path, no error info), then re-throws. Two issues:
-
Dual cards on error: The outer
buildAgentToolSetcatch also callsctx.onXmlComplete()with a<dyad-output type="error">tag. Both are persisted, producing an empty image-generation card AND a separate error card — unlike other tools which show only the error card. -
No error info in the image card: The
DyadImageGenerationcomponent has no error state rendering. The card will appear as if it finished with no output (no path, no error message), sincestatewill befinished(notaborted) because the tag is closed.
💡 Suggestion: Either remove the try/catch and let errors propagate naturally (other tools' pattern), or pass an error attribute: <dyad-image-generation prompt="..." error="Content policy violation"> and render it in the component.
| )} | ||
| {aborted && ( | ||
| <DyadStateIndicator state="aborted" abortedLabel="Did not finish" /> | ||
| )} |
There was a problem hiding this comment.
🟡 MEDIUM | consistency
No success state indicator after image generation completes
The component shows Generating... (pending) and Did not finish (aborted), but has no visual confirmation when generation succeeds. Other card components like DyadCopy show a green checkmark via DyadStateIndicator with state='finished' and a finishedLabel. After waiting through what could be a long generation, the user gets no visual feedback that it worked.
💡 Suggestion: Add a finished state indicator:
{!inProgress && !aborted && state === 'finished' && (
<DyadStateIndicator state="finished" finishedLabel="Generated" />
)}
|
@BugBot run |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fdbb29deb2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const buffer = Buffer.from(imageData.b64_json, "base64"); | ||
| await fs.writeFile(filePath, buffer); | ||
| } else if (imageData.url) { | ||
| const response = await fetch(imageData.url); |
There was a problem hiding this comment.
The saveGeneratedImage path downloads imageData.url directly without any protocol or host validation, so if the engine response is misconfigured or compromised to return a URL like http://127.0.0.1/... or another internal address, the Electron main process will perform an SSRF request and persist the response into the workspace. Because local-agent tools can then read/copy that file, this creates a practical internal-data exfiltration path; enforce strict URL validation (at minimum http/https plus private-network blocking or an allowlist) before calling fetch.
Useful? React with 👍 / 👎.
🔍 Dyadbot Code Review SummaryVerdict: ✅ YES - Ready to merge Reviewed by 3 independent agents: Correctness Expert, Code Health Expert, UX Wizard. Issues SummaryNo HIGH or MEDIUM issues found beyond what existing PR comments already cover. This PR is well-structured, follows established codebase patterns (DyadCard primitives, escapeXmlAttr/escapeXmlContent, engineFetch, Zod schemas), and includes E2E tests with a fake server endpoint. 🟢 Low Priority Notes (4 items)
🚫 Dropped False Positives (10 items)
Generated by Dyadbot multi-agent code review |
github-actions
bot
added
needs-human:final-check
🎭 Playwright Test Results✅ All tests passed!
Total: 727 tests passed (7 flaky) (234 skipped)
|
3 participants
Uh oh!
There was an error while loading. Please reload this page.