[codex] Fix Kiro ACP image attachment errors#2793
Conversation
…-review Add Kiro active prompt steering and appearance settings
Fix Kiro ACP stop and icon polish
…ering [codex] fix running composer steering send
|
@codex review |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 657f253. Configure here.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![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: 657f253c1d
ℹ️ 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".
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd6e413d24
ℹ️ 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".
|
@codex review |
| export function browserApiCorsPreflightHeadersForOrigin(origin: string | undefined) { | ||
| if (!isBrowserApiCorsOriginAllowed(origin)) { | ||
| return {}; | ||
| } | ||
|
|
||
| return { | ||
| ...browserApiCorsHeadersForOrigin(origin), | ||
| "access-control-max-age": String(browserApiCorsMaxAgeSeconds), | ||
| } as const; | ||
| } |
There was a problem hiding this comment.
🟡 Medium src/httpCors.ts:66
When origin is not allowed, browserApiCorsPreflightHeadersForOrigin returns {} (no CORS headers), causing preflight requests to fail with CORS errors. However, browserApiCorsHeadersForOrigin returns permissive headers with "access-control-allow-origin": "*" for the same non-allowed origins on actual requests. This asymmetry allows simple requests from untrusted origins to succeed while blocking complex requests that require preflight.
export function browserApiCorsPreflightHeadersForOrigin(origin: string | undefined) {
if (!isBrowserApiCorsOriginAllowed(origin)) {
- return {};
+ return browserApiCorsHeadersForOrigin(origin);
}
return {
...browserApiCorsHeadersForOrigin(origin),
"access-control-max-age": String(browserApiCorsMaxAgeSeconds),
- } as const;
+ };
}🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/src/httpCors.ts around lines 66-75:
When `origin` is not allowed, `browserApiCorsPreflightHeadersForOrigin` returns `{}` (no CORS headers), causing preflight requests to fail with CORS errors. However, `browserApiCorsHeadersForOrigin` returns permissive headers with `"access-control-allow-origin": "*"` for the same non-allowed origins on actual requests. This asymmetry allows simple requests from untrusted origins to succeed while blocking complex requests that require preflight.
Evidence trail:
apps/server/src/httpCors.ts lines 16-20 (browserApiCorsHeaders with wildcard), lines 53-64 (browserApiCorsHeadersForOrigin returning wildcard for non-allowed), lines 66-75 (browserApiCorsPreflightHeadersForOrigin returning {} for non-allowed). apps/server/src/http.ts lines 46-69 (middleware using both functions for OPTIONS vs actual requests).
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Closing this cross-repo PR. Replacement PR targets declancowen/t3code:main: declancowen#6 |
1 participant
Summary
datawhen providers return response errors so invalid image failures surface as provider request errors instead of Effect decode defects.Validation
bun run test src/client.test.ts src/protocol.test.tsfrompackages/effect-acpbun run test src/provider/acp/StandardAcpAdapter.test.tsfromapps/serverbun fmtbun lint(passes with existing unrelated warnings)bun typecheckNotes
The fix keeps native ACP image content blocks enabled for Kiro. The root issue was error-channel classification: provider JSON-RPC errors were decoded as defects and displayed as low-level decode stacks.
Note
High Risk
High risk: changes authentication-adjacent CORS behavior, adds a new provider driver/runtime (Kiro) and modifies ACP error/permission handling, plus updates release automation and desktop update install flow.
Overview
Adds end-to-end Kiro CLI provider support (driver, ACP runtime, adapter, provider snapshot/model discovery, home/env handling) and wires Kiro-specific ACP behaviors like
session/set_model, active-prompt steering via_message/send, image MIME allowlisting, and continuation keying.Hardens server credentialed CORS by replacing wildcard CORS middleware with per-request origin handling: only loopback + known hosted app origins receive
Access-Control-Allow-Credentials, and auth routes now emit origin-scoped CORS headers; includes new unit/integration coverage.Improves desktop release/update reliability: introduces a dedicated
desktop-release.ymlworkflow for stable tags (tag/ref resolution, multi-platform artifacts, strict macOS signing requirement), limits the existingrelease.ymlto nightly tags, and updatesDesktopUpdates.installto stop the backend beforequitAndInstall, prevent double-installs, and restart the backend on install-handoff failures (with added tests and updated menu “no updates” dialog copy).Reviewed by Cursor Bugbot for commit 7d1ec6d. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix Kiro ACP image attachment errors and add full Kiro provider support
KiroAcpSupport), text generation, settings schema, UI icon, and sidebar entry with a 'new' badgemakeKiroAdapterand sending image content via the_message/sendmethodmakeAcpPatchedProtocoltoolCalltitle when kind is omitted (e.g. 'editing:' →edit, 'running:' →execute) inparsePermissionRequest/settings/appearancewith theme mode control, font, contrast, motion, and translucent sidebar options driven by CSS variables anddata-*attributes#ff5c5c(red) and dark mode background changes to#101827; existing color-dependent tests or screenshots will differMacroscope summarized 7d1ec6d.