Skip to content

fix(daemon): route image requests into media generation#3988

Open
mrcfps wants to merge 9 commits into
mainfrom
looper/2953-image-1d17cc59abf65fa3
Open

fix(daemon): route image requests into media generation#3988
mrcfps wants to merge 9 commits into
mainfrom
looper/2953-image-1d17cc59abf65fa3

Conversation

@mrcfps

@mrcfps mrcfps commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Closes #2953

Why

Users reported that plain image/poster requests could stay on the normal chat/code path instead of producing a real image file. I picked this up to close the repro where Image projects backed by BYOK OpenAI chat would print an od media generate command, and short freeform prompts like image could miss the media router entirely.

What users will see

When they ask Open Design to generate an image or poster, the request now stays on the image-generation path more reliably:

  • concise prompts like image route into media generation instead of code output;
  • BYOK OpenAI Image projects dispatch the built-in image tool and return an actual rendered image instead of shell instructions.

Surface area

  • UI — new page / dialog / panel / menu item / setting / empty state in apps/web or apps/desktop (including Electron menu bar)
  • Keyboard shortcut — new or changed
  • CLI / env var — new od subcommand or flag, new tools-dev / tools-pack / tools-pr flag, or new OD_* env var
  • API / contract — new /api/* endpoint, new SSE event, or changed shape in packages/contracts
  • Extension point — new entry under skills/, design-systems/, design-templates/, or craft/, or change to the skills protocol
  • i18n keys — added new translation keys (see TRANSLATIONS.md for the locale workflow)
  • New top-level dependency — adding any new entry to the root package.json (dependencies or devDependencies); workspace-package package.json files are out of scope. Include a paragraph on what we get vs. what bytes we ship (see CONTRIBUTING.md → Code style)
  • Default behavior change — changes what existing users experience without opting in (default model, default setting, file/SQLite schema, auto-network on startup, auto-install)
  • None — internal refactor, docs, tests, or translation update only

Screenshots

None.

Bug fix verification

  • Test paths that reproduce the bug: apps/daemon/tests/chat-route.test.ts, apps/daemon/tests/proxy-routes.test.ts
  • Did the test go red on main and green on this branch? no — I verified the targeted regressions green on this branch, but did not re-run them on main in this worktree.

Validation

  • pnpm exec vitest run -c vitest.config.ts tests/proxy-routes.test.ts
  • pnpm exec vitest run -c vitest.config.ts tests/chat-route.test.ts -t "routes concise image-only prompts away from od-default and into image generation"
  • pnpm --filter @open-design/daemon typecheck
  • pnpm guard
  • pnpm typecheck

🔁 Powered by Looper · runner=worker · agent=opencode · An autonomous AI dev team for your GitHub repos.

mrcfps added 4 commits June 4, 2026 02:59
Generated-By: looper 0.9.3 (runner=worker, agent=opencode)
Generated-By: looper 0.9.3 (runner=worker, agent=opencode)
Generated-By: looper 0.9.3 (runner=worker, agent=opencode)
Generated-By: looper 0.9.6 (runner=worker, agent=opencode)
@mrcfps mrcfps mentioned this pull request Jun 9, 2026
@lefarcen lefarcen added size/XL PR changes 700-1500 lines risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps type/bugfix Bug fix labels Jun 9, 2026
@lefarcen lefarcen requested a review from Siri-Ray June 9, 2026 09:50
@Siri-Ray

Siri-Ray commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

@mrcfps I'm holding off on generating review comments for #3988 because this pull request has merge conflicts right now.

Please resolve the conflicts with main and push the updated branch. Once that's done, request or wait for the review to run again and I'll take another look.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 370f7da246

ℹ️ 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".

Comment thread apps/daemon/src/media.ts
Generated-By: looper 0.9.6 (runner=fixer, agent=opencode)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9bc3b3126b

ℹ️ 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".

Comment thread apps/daemon/src/chat-routes.ts

@Siri-Ray Siri-Ray left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrcfps Thanks for pushing this image-routing fix forward. I reviewed the daemon run-routing changes and the BYOK OpenAI image tool path; the main paths are covered, but I found one credential-precedence case worth tightening before users with an existing custom image provider hit it.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

Comment thread apps/daemon/src/media.ts
@lefarcen lefarcen requested a review from Siri-Ray June 9, 2026 10:09
Generated-By: looper 0.9.6 (runner=fixer, agent=opencode)
@mrcfps

mrcfps commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Looper fixer round completee14819a

  • ✅ Review comment on apps/daemon/src/media.ts:495 (@chatgpt-codex-connector) — thread
    • I changed apps/daemon/src/media.ts to skip the custom-image credential override whenever providerConfigOverride is present, and added a regression test in apps/daemon/tests/media-openai-compatible-providers.test.ts covering the BYOK OpenAI image path.
  • • Review comment on apps/daemon/src/chat-routes.ts:1034 (@chatgpt-codex-connector) — thread
    • Agent did not provide a decision for this thread
  • • Review comment on apps/daemon/src/media.ts:492 (@Siri-Ray) — thread
    • Agent did not provide a decision for this thread

🔁 Powered by Looper · runner=fixer · agent=opencode · An autonomous AI dev team for your GitHub repos.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e14819a2a4

ℹ️ 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".

Comment thread apps/daemon/src/chat-routes.ts

@Siri-Ray Siri-Ray left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrcfps Thanks for continuing to tighten the image-routing flow. I reviewed the daemon routing changes, the OpenAI image tool loop, and the added regression coverage. One main-path gap is still present: the new OpenAI proxy gate depends on projectId, but the current web OpenAI-compatible caller does not forward the proxy context, so the BYOK OpenAI Image project path described in the PR will still miss the tool loop from the UI.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

Comment thread apps/daemon/src/chat-routes.ts
Generated-By: looper 0.9.6 (runner=fixer, agent=opencode)
@mrcfps

mrcfps commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Looper fixer round complete0a17735

  • ✅ Review comment on apps/daemon/src/chat-routes.ts:1034 (@chatgpt-codex-connector) — thread
    • I forwarded the optional proxy context through apps/web/src/providers/anthropic.ts and apps/web/src/providers/openai-compatible.ts so OpenAI proxy requests now carry projectId, and added a web regression in apps/web/tests/providers/sse.test.ts for the actual chat path.
  • ✅ Review comment on apps/daemon/src/media.ts:492 (@Siri-Ray) — thread
    • This branch already skips custom-image overrides when providerConfigOverride is present in apps/daemon/src/media.ts, and apps/daemon/tests/media-openai-compatible-providers.test.ts covers the BYOK gpt-image-2 precedence case so per-request OpenAI credentials stay authoritative.
  • • Review comment on apps/daemon/src/chat-routes.ts:1059 (@chatgpt-codex-connector) — thread
    • Agent did not provide a decision for this thread
  • • Review comment on apps/daemon/src/chat-routes.ts:1031 (@Siri-Ray) — thread
    • Agent did not provide a decision for this thread

🔁 Powered by Looper · runner=fixer · agent=opencode · An autonomous AI dev team for your GitHub repos.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

Generated-By: looper 0.9.6 (runner=fixer, agent=opencode)
@mrcfps

mrcfps commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Looper fixer round complete3531c30

  • ✅ Review comment on apps/daemon/src/chat-routes.ts:1059 (@chatgpt-codex-connector) — thread
    • I updated apps/daemon/src/chat-routes.ts so the OpenAI image tool now passes appendVersionedApiPath(baseUrl, '') as upstreamBaseUrl, and tightened apps/daemon/tests/proxy-routes.test.ts to cover the bare https://api.openai.com case.
  • • Review comment on apps/daemon/src/chat-routes.ts:1031 (@Siri-Ray) — thread
    • The current branch already threads ProxyContext through apps/web/src/providers/anthropic.ts and apps/web/src/providers/openai-compatible.ts, and apps/web/tests/providers/sse.test.ts exercises the UI caller shape by asserting projectId reaches /api/proxy/openai/stream.

🔁 Powered by Looper · runner=fixer · agent=opencode · An autonomous AI dev team for your GitHub repos.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

@Siri-Ray Siri-Ray left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrcfps I reviewed the current head's daemon image-routing changes, the BYOK OpenAI image tool loop, the credential/base-url fixes from the follow-up commits, and the web OpenAI proxy context forwarding. The previous concerns are addressed in the changed ranges, and I did not find any additional actionable correctness or safety issues. Thanks for sticking with this and tightening the regression coverage around the real user paths. 🙂

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Visual regression review

Head: c8f214f · Base: de5b189

18 case(s) have no baseline; PR screenshots are shown without a diff.

5 changed · 12 unchanged · 18 missing baseline · 0 failed

Changed cases

Case Main PR Diff
visual-avatar-menu main pr diff
visual-home-catalog main pr diff
visual-new-project-modal main pr diff
visual-settings-byok main pr diff
visual-settings-execution main pr diff
PR screenshots without baselines
Case PR
visual-avatar-local-agent-list pr
visual-design-system-detail pr
visual-home-plugin-use-staged pr
visual-home-plugin-use-with-query pr
visual-home-staged-attachment pr
visual-integrations-mcp pr
visual-onboarding-runtime pr
visual-plugin-share-menu pr
visual-project-avatar-model-dropdown pr
visual-project-workspace pr
visual-settings-byok-model-dropdown pr
visual-settings-byok-openai pr
visual-settings-local-cli pr
visual-settings-local-cli-model-dropdown pr
visual-topbar-byok-model-dropdown pr
visual-topbar-byok-switcher pr
visual-topbar-local-cli-model-dropdown pr
visual-workspace-staged-contexts pr
Unchanged cases
Case Main PR Diff
visual-design-systems main pr diff
visual-home main pr diff
visual-home-context-picker main pr diff
visual-home-plugin-filter main pr diff
visual-integrations main pr diff
visual-integrations-use-everywhere main pr diff
visual-plugin-details main pr diff
visual-plugins main pr diff
visual-projects main pr diff
visual-projects-kanban main pr diff
visual-tasks main pr diff
visual-topbar-execution-switcher main pr diff

Visual diff is advisory only and does not block merging.

Generated-By: looper 0.9.6 (runner=fixer, agent=opencode)

@Siri-Ray Siri-Ray left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrcfps I reviewed the current head's changed ranges across the daemon image-routing fallback, the OpenAI BYOK image tool loop, media credential precedence, web proxy context forwarding, and the added daemon/web regression coverage. The previous review concerns are addressed on this head, and I did not find any new actionable correctness, safety, or maintainability issues. I was not able to rerun the targeted Vitest commands in this prepared worktree because vitest is not installed in the package context, but the changed test coverage matches the user paths this PR is protecting. Thanks for continuing to tighten this flow and for carrying the follow-up fixes through. 🙂

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps size/XL PR changes 700-1500 lines type/bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

image

3 participants