Skip to content

Fix app crash when run commands contain non-string values#3098

Open
denjalonso wants to merge 1 commit into
superset-sh:mainfrom
denjalonso:fix/workspace-run-button-command-trim-crash
Open

Fix app crash when run commands contain non-string values#3098
denjalonso wants to merge 1 commit into
superset-sh:mainfrom
denjalonso:fix/workspace-run-button-command-trim-crash

Conversation

@denjalonso
Copy link
Copy Markdown

@denjalonso denjalonso commented Apr 1, 2026

Summary

  • Fixes TypeError: command.trim is not a function crash in WorkspaceRunButton when .superset/config.json contains objects instead of plain strings in the run array
  • Filters config.run entries in getResolvedRunCommands to only include non-empty strings, consistent with how hasConfiguredScripts already handles this

Fixes #3095

Root Cause

getResolvedRunCommands returned config.run directly without validation:

return { commands: config?.run ?? [] };

When a user writes their config as:

{ "run": [{ "name": "Dev", "command": "npm run dev" }] }

The renderer receives objects instead of strings, and WorkspaceRunButton crashes calling .trim() on them — making the entire app unusable (black screen).

Fix

Filter to only valid string entries before returning, matching the existing pattern in hasConfiguredScripts:

const commands = Array.isArray(config?.run)
    ? config.run.filter(
        (s): s is string => typeof s === "string" && s.trim().length > 0,
    )
    : [];
return { commands };

Test plan

  • Create .superset/config.json with object-format run commands ({ name, command }) — app should no longer crash
  • Create config with valid string run commands — run button should work as before
  • Create config with empty run: [] — no crash, run button shows "Set Run"
  • Create config with mixed valid/invalid entries — only valid strings are used

🤖 Generated with Claude Code


Summary by cubic

Fixes an app crash when .superset/config.json contains non-string entries in run by returning only non-empty string commands from getResolvedRunCommands, preventing .trim() errors in the Workspace Run button.

  • Bug Fixes
    • Filter config.run to non-empty strings; ignore objects and blanks.
    • Aligns with existing hasConfiguredScripts validation and preserves normal run button behavior.

Written for commit 68cf238. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced validation and handling of run command configuration to ensure proper processing and improve system stability.

The `getResolvedRunCommands` procedure passed `config.run` directly to
the renderer without validating that entries are strings. When users
configure run commands as objects (e.g. `{ name, command }`) instead of
plain strings, `WorkspaceRunButton` crashes calling `.trim()` on a
non-string value, rendering the entire app unusable.

Filter `config.run` to only include non-empty strings before returning,
consistent with how `hasConfiguredScripts` already handles this.

Fixes superset-sh#3095

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 1, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 58b1c047-be0f-4581-b086-804a55ea06e2

📥 Commits

Reviewing files that changed from the base of the PR and between 30d6833 and 68cf238.

📒 Files selected for processing (1)
  • apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts

📝 Walkthrough

Walkthrough

The getResolvedRunCommands procedure has been updated to validate and sanitize config.run by enforcing array type, filtering non-string entries, and removing empty or whitespace-only strings. This ensures the returned commands array contains only valid, non-empty strings.

Changes

Cohort / File(s) Summary
Run Commands Validation
apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts
Added validation logic to filter config.run to only include strings and exclude empty/whitespace-only values, preventing downstream crashes when non-string values are present in the configuration.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A crash once came when strings turned strange,
Objects sneaking in to cause dismay,
But now we filter, trim, and range,
Only true strings shall save the day,
Validation guards the startup play! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and clearly describes the main fix: preventing app crashes caused by non-string values in run commands.
Description check ✅ Passed The description is comprehensive with clear sections covering Summary, Root Cause, Fix, and Test Plan, aligned with the repository template requirements.
Linked Issues check ✅ Passed The PR successfully addresses issue #3095 by adding validation to filter config.run to only non-empty strings, preventing the TypeError crash and matching hasConfiguredScripts behavior.
Out of Scope Changes check ✅ Passed The changes are narrowly focused on fixing the getResolvedRunCommands validation logic without introducing unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

@denjalonso
Copy link
Copy Markdown
Author

Hey @kirankunigiri, could you take a look at this? The crash originates from the getResolvedRunCommands procedure you added in #2511 — it passes config.run to the renderer without filtering for strings, so non-string entries (like objects) cause WorkspaceRunButton to crash on .trim().

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

App crashes with 'command.trim is not a function' when config uses object format for run commands

1 participant