Skip to content

ui(config-form): fix rendering of secret input fields with union types#40388

Open
ArvinDevel wants to merge 1 commit intoopenclaw:mainfrom
ArvinDevel:fix/secret-input-rendering
Open

ui(config-form): fix rendering of secret input fields with union types#40388
ArvinDevel wants to merge 1 commit intoopenclaw:mainfrom
ArvinDevel:fix/secret-input-rendering

Conversation

@ArvinDevel
Copy link
Copy Markdown

Summary

  • Problem: Config UI fails to render fields using buildSecretInputSchema() with error "Unsupported type: . Use Raw mode."
  • Why it matters: Users cannot configure channel secrets (e.g., appSecret, botToken) through the UI; forced to edit config files manually.
  • What changed: Added handling in config-form.node.ts for JSON schema unions mixing primitive types with objects. Renders a textarea accepting both plain strings and JSON objects.
  • What did NOT change: No changes to config schema definitions, secret storage, or channel runtime behavior. Purely a UI rendering fix.

Change Type (select all)

  • Bug fix

Scope (select all touched areas)

  • UI / DX

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:

  • Plain string: my-secret-token
  • JSON object: {"source":"env","provider":"1password","id":"op://..."}

Security Impact

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No - UI only; storage/processing unchanged
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No

Repro + Verification

Environment

  • OS: macOS (any)
  • Runtime: Node 22+
  • Repository: openclaw/openclaw (ui branch)

Steps

  1. Run OpenClaw and open config UI
  2. Navigate to channel config with secret fields (Feishu, Zalo, etc.)
  3. Observe fields now render correctly

Expected

  • appSecret, botToken, etc. fields show a textarea input

Actual

  • Fixed: textarea appears, accepts both string and JSON input

Evidence

  • File changed: ui/src/ui/views/config-form.node.ts
  • Added anyOf handler for primitive + object unions (lines ~444-512)

Human Verification (required)

What I personally verified:

  • Locally inspected the code change in context
  • Verified the new branch covers the secret input union pattern
  • Checked that similar anyOf patterns for pure primitives and literals remain unaffected

Edge cases checked:

  • Empty input handling
  • Default value display for both string and object formats
  • JSON parsing errors fall back to string
  • Reset button behavior

What you did NOT verify:

  • Full UI build/test (requires dependency installation)
  • End-to-end UI interaction (would require running the gateway and UI)

Compatibility / Migration

  • Backward compatible? Yes - existing configs work unchanged
  • Config/env changes? No
  • Migration needed? No

Failure Recovery

  • How to disable/revert: Revert this PR; UI will fall back to error message for these fields (worst case: manual config editing)
  • Files/config to restore: ui/src/ui/views/config-form.node.ts to pre-fix version
  • Bad symptoms to watch for:
    • Secret fields not rendering or rejecting valid input
    • JSON parsing errors in console when editing these fields

Risks and Mitigations

  • Risk: Textarea UX is less friendly than separate Simple/Advanced modes.
    • Mitigation: This unblocks UI usage; follow-up can add a nicer component with mode toggle if desired. The current solution is functionally correct and consistent with the form's fallback approach for complex schemas.
  • Risk: JSON.parse() might throw on edge case strings.
    • Mitigation: Caught and treated as raw string, which is valid for secret input (string is always accepted).

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 9, 2026

Greptile Summary

This PR fixes a UI rendering bug where config fields using buildSecretInputSchema() (which produces a JSON Schema anyOf union of a primitive type and an object type) would fall through all existing union handlers and display an "Unsupported type" error. The fix adds a new branch in config-form.node.ts that detects mixed primitive+object unions and renders a textarea that accepts both plain strings and JSON objects, with graceful JSON-parse fallback.

Key points:

  • The new branch correctly slots in after the existing pure-literal and pure-primitive union handlers, so none of those code paths are disrupted.
  • Value serialization (JSON.stringify with 2-space indent) and deserialization (JSON.parse with string fallback) are handled correctly.
  • The reset button contains a redundant if/else where both branches call onPatch(path, def) — the condition is dead code and should be collapsed.

Confidence Score: 4/5

  • This PR is safe to merge; it is a UI-only fix with no impact on secret storage, schema definitions, or runtime behavior.
  • The change is well-scoped to a single function branch, is backward compatible, and the logic for handling mixed primitive+object unions is correct. The only issue found is a trivial redundant if-else in the reset button handler, which has no functional impact.
  • No files require special attention.

Last reviewed commit: ff18453

Comment on lines +495 to +502
@click=${() => {
const def = schema.default;
if (typeof def === "string") {
onPatch(path, def);
} else {
onPatch(path, def);
}
}}
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.

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.

Suggested change
@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.

Copy link
Copy Markdown

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

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: 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".

Comment on lines +481 to +482
const parsed = JSON.parse(raw);
onPatch(path, parsed);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

config-form: secret input fields fail to render with 'Unsupported type' error

1 participant