fix(tui): deliver pasted media in /skill args and Ctrl-S steer#1588
Conversation
…file tags Skill and plugin command args are a plain-text RPC channel, so pasted image/video placeholders never reached the model. Rewrite matched placeholders into <image|video path="..."> tags pointing at cache-dir copies (the same convention pasted videos already use), and gate them on model media capabilities like the normal prompt path.
Ctrl-S steer dropped media: queued messages were reduced to their text and the editor draft never went through placeholder extraction, so session.steer only ever received placeholder strings. Carry the queued items' extracted parts, extract the editor draft on the spot (with the same capability gate as a normal submit), and merge everything into a part list when any item has media.
🦋 Changeset detectedLatest commit: ba77fc4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![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: 3581fc689e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The '\n\n' item separator could land as a standalone whitespace-only text part between two media parts, which normalizePromptInput rejects — failing the steer after the queue and editor draft were already cleared. Only append the separator onto a trailing text part so it always merges.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 035c181cf8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The separator was only appended onto trailing text parts, so a media item followed by a plain-text steer item lost the historical '\n\n' separation. Always append it — it merges into the adjacent text part — except between two touching media parts, where a standalone whitespace-only text part would be rejected by normalizePromptInput.
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 456b7b2aa8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Skill args are XML-escaped on render (renderSkillAttributes + expandSkillParameters), which mangled the <image|video path> tags into escaped text. Add a 'plain' reference style (Attached image file: <path> (open it with ReadMediaFile)) with no tag/attribute boundary characters and use it for the skill channel; plugin command args expand verbatim and keep the tag form.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10ac69059d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Video cache names carry the original label, which permits <>&"; in the plain (/skill) reference style those chars get XML-escaped in args and the reference no longer matches the file on disk. Strip them from the cache name in plain mode; the tag channel is unchanged (its attribute escaping already handles them).
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 973ac9b60e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The cache copy inside media placeholder rewriting can throw (unwritable cache dir, vanished video source) before any RPC .catch is installed, escaping the fire-and-forget slash-command dispatcher as an unhandled rejection. Catch it in sendSkillActivation, activatePluginCommand, and the Ctrl-S draft extraction, show an error, and leave queue/draft/session state untouched.
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
1 participant
Related Issue
No linked issue — the problem is explained below.
Problem
Pasted images only reached the model on the plain message path. Two other input paths silently dropped them:
/skill:foo <pasted image>(and plugin command arguments): command args are a plain-text RPC channel, so the[image #N …]placeholder was sent as literal text — the image bytes never left the TUI process, and the model only saw a dead placeholder.session.steeronly ever received placeholder strings even though the steer RPC accepts full prompt parts.What changed
<image|video path="…">tags pointing at cache-dir copies — the same convention pasted videos already use — so the model can open them withReadMediaFile. Args are gated on the model's media capabilities, matching the normal prompt path.'\n\n'-joined string payload.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update. (Bug fix restoring expected behavior; no user-facing docs change needed.)