fix: handle OpenAI tool content and stable secret ids#1374
Conversation
OpenAI rejects assistant messages whose content is null, including tool-call messages. Preserve an empty string when the normalized message has no content so the request body stays valid.
Allow user-script secret options to provide an optional id for SecretStorage references. This keeps saved secrets stable across visible setting label changes while retaining generated IDs as the fallback.
Deploying quickadd with
|
| Latest commit: |
0b26d76
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://414b91fa.quickadd.pages.dev |
| Branch Preview URL: | https://fix-ai-tool-content-secret-i.quickadd.pages.dev |
![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: 0b26d7655e
ℹ️ 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".
| for (const settingName of secretOptionNames) { | ||
| for (const [settingName, option] of secretOptionEntries) { | ||
| const value = command.settings?.[settingName]; | ||
| if (isUserScriptSecretRef(value)) continue; |
There was a problem hiding this comment.
When a script adds a non-label id to a secret option that users have already saved, value is already a UserScriptSecretRef pointing at the old label-derived key, so this early continue prevents it from ever being moved to the option-derived stable key. The modal save path also preserves that existing ref, so a later label rename using the documented id will not find the user's saved secret and will require re-entry. Consider migrating/copying refs whose secretRef differs from buildUserScriptSecretId(command, settingName, option) when the old secret can be read.
Useful? React with 👍 / 👎.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds an optional ChangesStable Secret Option ID Support
OpenAI Assistant Message Content Normalization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
1 participant
Fix two related runtime issues from the latest user-script secret work.
The first commit fixes the reported OpenAI 400 response:
OpenAI assistant messages with tool calls still need string content, so the OpenAI provider mapping now sends an empty string when normalized assistant content is absent instead of
null.The second commit adds optional
idsupport for user-script secret options. Script authors can keep SecretStorage references stable across visible label changes, while existing scripts continue to fall back to the option label. The migration path also reattaches an existing id-backed secret under a renamed option label.Verification:
pnpm run test -- src/ai/tools/providerToolMapping.test.ts src/utils/userScriptSecrets.test.ts src/gui/MacroGUIs/UserScriptSettingsModal.test.ts src/engine/MacroChoiceEngine.entry.test.ts src/engine/SingleMacroEngine.member-access.test.ts(ran full Vitest suite: 225 passed, 5 skipped)pnpm run build-with-lintgit diff --check master...HEADSummary by CodeRabbit
Documentation
idfield, which keeps stored secrets stable when renaming user-visible setting labels.Tests