Skip to content

fix: prevent skill load failure when data-designer CLI is not installed#501

Merged
johnnygreco merged 6 commits intomainfrom
johnny/fix/skill-get-data-designer-command-failure
Apr 7, 2026
Merged

fix: prevent skill load failure when data-designer CLI is not installed#501
johnnygreco merged 6 commits intomainfrom
johnny/fix/skill-get-data-designer-command-failure

Conversation

@johnnygreco
Copy link
Copy Markdown
Contributor

@johnnygreco johnnygreco commented Apr 7, 2026

Summary

  • Removes the ! backtick command substitution from SKILL.md that resolved the data-designer CLI path at skill load time — if the CLI wasn't installed, this could cause the skill to fail to load entirely
  • Adds a Resolve CLI command step as step 1 in both workflows (interactive and autopilot) where the agent runs the lookup command itself and evaluates the result
  • Uses a CLI_NOT_FOUND sentinel (via || echo CLI_NOT_FOUND) so the command always exits 0 — this prevents the agent from going into error-fixing mode and instead keeps it following the workflow instructions
  • Updates the Troubleshooting section to ask the user for permission before creating a venv or installing packages, instead of attempting it automatically

Append `|| true` to the shell command that resolves the data-designer
path so it always exits 0. Without this, the skill fails to load
entirely when the CLI is missing, and the "If blank, see
Troubleshooting" fallback is never reached.
@johnnygreco johnnygreco requested a review from a team as a code owner April 7, 2026 15:30
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 7, 2026

Greptile Summary

Moves data-designer CLI resolution from skill-load time (backtick substitution in SKILL.md) to workflow runtime, preventing the skill from failing to load entirely when the CLI is not installed. Both workflow files gain an explicit first step with a CLI_NOT_FOUND sentinel that ensures the shell command always exits 0, guiding the agent to prompt the user for permission rather than attempting autonomous installation.

Confidence Score: 5/5

Safe to merge — the fix correctly defers CLI resolution to runtime and all three files are updated consistently.

No P0 or P1 issues found. The shell command logic is verified correct: command -v exits 1 (not producing stderr) when the command is absent, the OR-chain falls through to the .venv check, and the final || echo CLI_NOT_FOUND guarantees exit 0 and a clear sentinel. The troubleshooting section now properly gates any installation on explicit user consent. All files are updated in sync.

No files require special attention.

Important Files Changed

Filename Overview
skills/data-designer/SKILL.md Removes backtick CLI substitution at skill-load time; Troubleshooting updated to ask user permission before creating a venv or installing packages
skills/data-designer/workflows/autopilot.md Adds step 1 to resolve CLI path at runtime with CLI_NOT_FOUND sentinel; all subsequent steps renumbered
skills/data-designer/workflows/interactive.md Adds identical CLI-resolution step 1 as autopilot.md; subsequent steps renumbered consistently

Sequence Diagram

sequenceDiagram
    participant Agent
    participant Shell
    participant User

    Agent->>Shell: command -v data-designer 2>/dev/null
    alt Found in system PATH
        Shell-->>Agent: /usr/local/bin/data-designer
        Agent->>Shell: data-designer agent context
    else Not in PATH
        Shell-->>Agent: (empty, exit 1)
        Agent->>Shell: test -x .venv/bin/data-designer && realpath ...
        alt Found in .venv
            Shell-->>Agent: .venv/bin/data-designer (resolved path)
            Agent->>Shell: .venv/bin/data-designer agent context
        else Not found anywhere
            Shell-->>Agent: CLI_NOT_FOUND
            Agent->>User: CLI not installed — would you like me to set up a venv?
        end
    end
Loading

Reviews (6): Last reviewed commit: "Merge branch 'main' into johnny/fix/skil..." | Re-trigger Greptile

Replace `|| true` (blank output) with `|| echo NOT_FOUND` so the agent
sees a clear signal. Update the instruction to bold/imperative so it
actually gets followed.
nabinchha
nabinchha previously approved these changes Apr 7, 2026
Remove the \!`command` substitution from SKILL.md and add a "Resolve CLI
command" step to both workflows. The agent now runs the lookup itself
and uses the result as the data-designer executable for all subsequent
commands. If the command fails, the agent stops and follows
Troubleshooting.
The resolve command now always exits 0 and outputs CLI_NOT_FOUND when
the executable is missing, so the agent evaluates a value rather than
reacting to a shell error.
nabinchha
nabinchha previously approved these changes Apr 7, 2026
Update Troubleshooting to ask the user before creating a venv or
installing packages, instead of attempting it automatically.
@johnnygreco
Copy link
Copy Markdown
Contributor Author

ran the benchmarks to confirm everything looks okay

context_tokens tool_usage wall_clock

@johnnygreco johnnygreco merged commit d3209e8 into main Apr 7, 2026
47 checks passed
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.

2 participants