ui(config-form): fix rendering of secret input fields with union types#40388
ui(config-form): fix rendering of secret input fields with union types#40388ArvinDevel wants to merge 1 commit intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR fixes a UI rendering bug where config fields using Key points:
Confidence Score: 4/5
Last reviewed commit: ff18453 |
| @click=${() => { | ||
| const def = schema.default; | ||
| if (typeof def === "string") { | ||
| onPatch(path, def); | ||
| } else { | ||
| onPatch(path, def); | ||
| } | ||
| }} |
There was a problem hiding this comment.
Redundant if-else in reset handler
Both branches of the if/else here call onPatch(path, def) identically — the condition on typeof def === "string" has no effect. This appears to be an incomplete refactor (perhaps the else branch was originally meant to call onPatch(path, JSON.stringify(def)) so the textarea shows a JSON string, but that ended up not being needed since displayStr already handles the display-side conversion). Either way, the current code is equivalent to simply calling onPatch(path, def) directly.
| @click=${() => { | |
| const def = schema.default; | |
| if (typeof def === "string") { | |
| onPatch(path, def); | |
| } else { | |
| onPatch(path, def); | |
| } | |
| }} | |
| @click=${() => { | |
| onPatch(path, schema.default); | |
| }} |
Prompt To Fix With AI
This is a comment left during a code review.
Path: ui/src/ui/views/config-form.node.ts
Line: 495-502
Comment:
**Redundant if-else in reset handler**
Both branches of the `if/else` here call `onPatch(path, def)` identically — the condition on `typeof def === "string"` has no effect. This appears to be an incomplete refactor (perhaps the `else` branch was originally meant to call `onPatch(path, JSON.stringify(def))` so the textarea shows a JSON string, but that ended up not being needed since `displayStr` already handles the display-side conversion). Either way, the current code is equivalent to simply calling `onPatch(path, def)` directly.
```suggestion
@click=${() => {
onPatch(path, schema.default);
}}
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff18453949
ℹ️ 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".
| const parsed = JSON.parse(raw); | ||
| onPatch(path, parsed); |
There was a problem hiding this comment.
Preserve plain-string values in mixed object unions
This change parses every non-empty textarea value with JSON.parse, which coerces valid plain strings like 123456 or true into number/boolean before patching. For schemas that intentionally allow string | object (for example secret inputs), those values should remain strings; coercion can make otherwise valid secrets fail schema validation or be saved with the wrong type. Parse should only be attempted for object-like JSON input (e.g. starts with {) or behind an explicit JSON mode.
Useful? React with 👍 / 👎.
Summary
buildSecretInputSchema()with error "Unsupported type: . Use Raw mode."appSecret,botToken) through the UI; forced to edit config files manually.config-form.node.tsfor JSON schema unions mixing primitive types with objects. Renders a textarea accepting both plain strings and JSON objects.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Closes #39869
User-visible / Behavior Changes
Before: Secret input fields show an error message and are unusable in the config UI.
After: Secret input fields render as a textarea with helpful placeholder text. Accepts:
my-secret-token{"source":"env","provider":"1password","id":"op://..."}Security Impact
Repro + Verification
Environment
Steps
Expected
appSecret,botToken, etc. fields show a textarea inputActual
Evidence
ui/src/ui/views/config-form.node.tsprimitive + objectunions (lines ~444-512)Human Verification (required)
What I personally verified:
Edge cases checked:
What you did NOT verify:
Compatibility / Migration
Failure Recovery
ui/src/ui/views/config-form.node.tsto pre-fix versionRisks and Mitigations