fix(#335): implement run <plugin> <resource> <action> one-shot command - #365
Open
javimosch wants to merge 1 commit into
Open
fix(#335): implement run <plugin> <resource> <action> one-shot command#365javimosch wants to merge 1 commit into
run <plugin> <resource> <action> one-shot command#365javimosch wants to merge 1 commit into
Conversation
Implements `supercli run <plugin> <resource> <action>`, which syncs the plugin catalog from GitHub, installs the plugin if it is not already registered, and executes the requested command in a single invocation. Key details: - Adds a 1-hour freshness check for the local remote catalog, skipping the network fetch when the catalog is current. - Adds fuzzy plugin suggestions when a requested plugin is not found. - Defends `supercli.js` against a missing `dotenv` dependency so `npx` works. - Surfaces the new `run` command in help and help-json output. - Expands `__tests__/run-command.test.js` to cover validation, fast-path, install, execute, and not-found paths. Fixes #335 Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThe ChangesRun command
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant RunCommand
participant Catalog
participant PluginInstaller
participant PluginExecutor
User->>RunCommand: Request plugin resource action
RunCommand->>Catalog: Check and update catalog when stale
RunCommand->>PluginInstaller: Install plugin when absent
RunCommand->>PluginExecutor: Execute action with configuration
PluginExecutor-->>User: Return result or resource error
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/run.js (1)
90-112: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not report all setup failures as plugin absence.
This
tryblock catchesloadConfigfailures and allinstallPluginfailures. A malformed config or install conflict is then reported asresource_not_foundwith code92.Only generate plugin suggestions for the installer’s actual not-found error. Preserve the original code, type, message, and suggestions for configuration and installation failures.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/run.js` around lines 90 - 112, Update the error handling around loadConfig and installPlugin so only the installer’s actual plugin-not-found error enters the findSuggestions/outputError resource_not_found path. Preserve the original error code, type, message, and suggestions for configuration failures and other installation errors, using the existing error symbols or properties exposed by the installer to distinguish not-found failures.
🧹 Nitpick comments (1)
__tests__/run-command.test.js (1)
189-192: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert configuration forwarding to
execute.The test only verifies that
executewas called. It does not verify the newconfigfield passed bycli/run.jsLine 162, so removal of that field would pass this test.Store the mocked configuration in a variable. Assert that the third
executeargument contains that exactconfigobject.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@__tests__/run-command.test.js` around lines 189 - 192, Update the test around the execute expectation to retain the mocked configuration in a variable and assert that execute’s third argument includes that exact config object. Keep the existing call-count and other interaction assertions unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli/help-json.js`:
- Line 12: Update the run help usage to invoke the required sc-zig default
executor instead of supercli in cli/help-json.js (line 12), cli/help.js (line
23), and cli/help.js (line 79). Keep the command syntax and descriptions
consistent across all machine-readable, structured JSON, and console help
entries.
In `@cli/run.js`:
- Around line 32-36: Update findSuggestions in cli/run.js (lines 32-36) to
filter name-only results by normalized exact name equality before returning;
only perform the fuzzy lookup when no exact match remains. Update
__tests__/run-command.test.js (lines 147-152) to mock empty exact results and
fuzzy results, then assert both listRegistryPlugins calls.
- Around line 25-26: Update the catalog freshness check to require a nonnegative
age in addition to age being below CATALOG_FRESH_MS, so future mtimeMs values
are treated as stale. Add a test covering a future catalog timestamp and verify
it does not suppress synchronization.
- Around line 74-75: Update the successful no-op path around updatePlugins so an
unchanged remote catalog still persists the local catalog or its
successful-check timestamp before marking the run updated. Preserve the existing
behavior for catalogs with changes, and add a regression test that runs twice
with an unchanged stale catalog and verifies the second run does not repeat
synchronization.
In `@cli/supercli.js`:
- Around line 5-9: Update the dotenv initialization around the top-level require
so only a missing-module error from require("dotenv") is handled as an optional
dependency; rethrow any other require failure, retain the loaded module, and
invoke its config({ quiet: true }) method outside the catch so configuration
errors propagate.
---
Outside diff comments:
In `@cli/run.js`:
- Around line 90-112: Update the error handling around loadConfig and
installPlugin so only the installer’s actual plugin-not-found error enters the
findSuggestions/outputError resource_not_found path. Preserve the original error
code, type, message, and suggestions for configuration failures and other
installation errors, using the existing error symbols or properties exposed by
the installer to distinguish not-found failures.
---
Nitpick comments:
In `@__tests__/run-command.test.js`:
- Around line 189-192: Update the test around the execute expectation to retain
the mocked configuration in a variable and assert that execute’s third argument
includes that exact config object. Keep the existing call-count and other
interaction assertions unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c8efc35b-6acb-4463-8447-4c910547dae1
📒 Files selected for processing (5)
__tests__/run-command.test.jscli/help-json.jscli/help.jscli/run.jscli/supercli.js
| mcp: { subcommands: ["list", "add", "tools", "call", "bind", "doctor", "remove"], description: "Manage local MCP server registry and invoke MCP tools" }, | ||
| commands: { description: "List all commands" }, | ||
| inspect: { description: "Inspect command details", usage: "supercli inspect <ns> <res> <act>" }, | ||
| run: { description: "Sync catalog, install plugin, and execute a command", usage: "supercli run <plugin> <resource> <action> [--args]" }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the required default command executor.
These entries direct users to execute plugins with supercli run. Update them to use the required sc-zig default invocation.
cli/help-json.js#L12-L12: update the machine-readablerunusage.cli/help.js#L23-L23: update structured JSON help.cli/help.js#L79-L79: update console help.
As per coding guidelines, use sc-zig by default for plugin discovery and command execution.
📍 Affects 2 files
cli/help-json.js#L12-L12(this comment)cli/help.js#L23-L23cli/help.js#L79-L79
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/help-json.js` at line 12, Update the run help usage to invoke the
required sc-zig default executor instead of supercli in cli/help-json.js (line
12), cli/help.js (line 23), and cli/help.js (line 79). Keep the command syntax
and descriptions consistent across all machine-readable, structured JSON, and
console help entries.
Source: Coding guidelines
Comment on lines
+25
to
+26
| const age = now - st.mtimeMs; | ||
| return age < CATALOG_FRESH_MS; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Treat future catalog timestamps as stale.
A future mtimeMs produces a negative age, which passes the current check. Clock skew or restored metadata can then suppress synchronization until the system clock passes that timestamp.
Require age >= 0 before accepting the catalog as fresh. Add a future-timestamp test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/run.js` around lines 25 - 26, Update the catalog freshness check to
require a nonnegative age in addition to age being below CATALOG_FRESH_MS, so
future mtimeMs values are treated as stale. Add a test covering a future catalog
timestamp and verify it does not suppress synchronization.
Comment on lines
+32
to
+36
| function findSuggestions(pluginName) { | ||
| const exact = listRegistryPlugins({ name: pluginName, nameOnly: true }).slice(0, 5); | ||
| if (exact.length > 0) return exact.map((p) => p.name); | ||
| const fuzzy = listRegistryPlugins({ name: pluginName }).slice(0, 5); | ||
| return fuzzy.map((p) => p.name); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use equality before the fuzzy lookup.
listRegistryPlugins({ name, nameOnly: true }) uses substring matching. A query such as git returns github-* entries in the first lookup, so the fuzzy lookup never runs despite no exact plugin name.
cli/run.js#L32-L36: filter name-only results for normalized equality before returning them, then run the fuzzy lookup when no equal name exists.__tests__/run-command.test.js#L147-L152: mock an empty first lookup and fuzzy results in the second lookup. Assert both filter calls.
📍 Affects 2 files
cli/run.js#L32-L36(this comment)__tests__/run-command.test.js#L147-L152
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/run.js` around lines 32 - 36, Update findSuggestions in cli/run.js (lines
32-36) to filter name-only results by normalized exact name equality before
returning; only perform the fuzzy lookup when no exact match remains. Update
__tests__/run-command.test.js (lines 147-152) to mock empty exact results and
fuzzy results, then assert both listRegistryPlugins calls.
Comment on lines
+74
to
+75
| const updateResult = await updatePlugins({ check: false }); | ||
| updated = true; |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Persist successful no-op catalog checks.
updatePlugins returns before writeLocalCatalog(remoteCatalog) when the remote catalog has no changes. The catalog mtime then remains stale, so every later run repeats the remote synchronization.
Persist a successful check timestamp, or rewrite/touch the local catalog after a successful no-op update. Add a regression test that runs twice after an unchanged stale-catalog sync.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/run.js` around lines 74 - 75, Update the successful no-op path around
updatePlugins so an unchanged remote catalog still persists the local catalog or
its successful-check timestamp before marking the run updated. Preserve the
existing behavior for catalogs with changes, and add a regression test that runs
twice with an unchanged stale catalog and verifies the second run does not
repeat synchronization.
Comment on lines
+5
to
+9
| try { | ||
| require("dotenv").config({ quiet: true }); | ||
| } catch (e) { | ||
| // dotenv not installed, proceed without .env loading | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files matching supercli =="
fd -a 'supercli\.js$' . || true
echo "== target file excerpt =="
if [ -f cli/supercli.js ]; then
sed -n '1,80p' cli/supercli.js | cat -n
fi
echo "== dotenv references =="
rg -n "dotenv|config\\(\\{|MODULE_NOT_FOUND" -S . --glob '!node_modules' --glob '!dist' --glob '!build' || true
echo "== package metadata =="
for f in package.json cli/package.json package-lock.json npm-shrinkwrap.json yarn.lock pnpm-lock.yaml; do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,120p' "$f"
fi
done
echo "== behavioral probe: catch behavior for require/config failures =="
node - <<'JS'
const cases = [
"missing package require",
"missing package config",
"sync require failing before export",
".config() throwing",
];
for (const name of cases) {
const errors = [];
const handlers = [];
const fn = () => {
handlers.push("loaded");
(function () {
if (name.startsWith(".config")) throw new Error("config failed");
if (name === "missing package require") throw Object.assign(new Error(), { code: "MODULE_NOT_FOUND", message: "Cannot find module 'dotenv'" });
if (name === "missing package config") throw Object.assign(new Error(), { code: "MODULE_NOT_FOUND", message: "Cannot find module 'dotenv'" });
if (name.includes("synchronous") || name === "sync require failing before export") throw new TypeError("require failed");
handlers.push("configured");
})();
throw new Error("post-config");
});
try { fn(); } catch (e) { errors.push(String(typeof e === 'object' && e.code ? e.code : e.message), e.code); }
console.log(name, JSON.stringify({ errors, handlers }));
}
JSRepository: javimosch/supercli
Length of output: 41592
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== behavioral probe: Node catch behavior for chained require/config errors =="
node - <<'JS'
function simulateMissingPackage() {
const errors = [];
const handlers = [];
let dotenv;
try {
handlers.push("before require");
try {
dotenv = require("dotenv");
} catch (error) {
handlers.push("require caught");
if (error?.code !== "MODULE_NOT_FOUND" || !String(error.message).includes("'dotenv'")) throw error;
}
dotenv?.config({ quiet: true });
handlers.push("configure complete");
} catch (error) {
errors.push(String(typeof error === "object" && error.code ? error.code : error.message));
}
return { errors, handlers };
}
function simulateConfigFailure() {
const errors = [];
const handlers = [];
let dotenv;
const fakeModule = {
config: function () {
handlers.push("configure entered");
throw new Error("dot configuration failed");
}
};
try {
handlers.push("before require");
try {
dotenv = fakeModule;
} catch (error) {
handlers.push("require caught");
if (error?.code !== "MODULE_NOT_FOUND" || !String(error.message).includes("'dotenv'")) throw error;
}
dotenv?.config({ quiet: true });
handlers.push("configure complete");
} catch (error) {
errors.push(String(typeof error === "object" && error.code ? error.code : error.message));
}
return { errors, handlers };
}
console.log("missing-package path:", JSON.stringify(simulateMissingPackage()));
console.log("config failure path:", JSON.stringify(simulateConfigFailure()));
JS
echo "== read-only check: catch shape in cli/supercli.js =="
python3 - <<'PY'
from pathlib import Path
text = Path("cli/supercli.js").read_text()
start = text.index("try {\n require(\"dotenv\").config({ quiet: true });\n} catch (e) {")
end = text.index("}", start) + 1
print("first catch block:", text[start:end])
PYRepository: javimosch/supercli
Length of output: 571
Do not swallow all dotenv initialization errors.
This catch covers both require("dotenv") and .config(), so any thrown dotenv error is treated as a missing package and ignored. Catch the missing-module case from require("dotenv") separately, then call .config({ quiet: true }) outside that block.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/supercli.js` around lines 5 - 9, Update the dotenv initialization around
the top-level require so only a missing-module error from require("dotenv") is
handled as an optional dependency; rethrow any other require failure, retain the
loaded module, and invoke its config({ quiet: true }) method outside the catch
so configuration errors propagate.
This was referenced Aug 3, 2026
This was referenced Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated maintenance run by automaintainer.
Focus: ISSUE FIX OBJECTIVE (GitHub Issue #335: "feat:
sc run <plugin> <action>— one-shot discover+install+execute")ISSUE DESCRIPTION:
Problem
Promoting SuperCLI on social media needs a tweetable one-liner. Currently, discovering and running a plugin for the first time requires 3 chained commands:
Thats ~240 characters — too long for a tweet, and too verbose for a first-time user to remember.
Desired UX
A single
run(orgo,x,!) command that:plugins update— syncs the latest plugin catalog from GitHub master (so new plugins are discoverable even on stale npm releases)plugins install <plugin>— registers the plugin commands if not already installed (idempotent — no-op if already installed)<resource> <action>with given argsRequirements
npx: must not require global install —npx superacli run <plugin> <action>should be the entry pointExample
Why This Matters
A 240-char shell pipeline wont fit in a tweet, wont get retweeted, and wont convert casual users. A single
runcommand at ~80 chars will.Real use case: promoting
claude-session-optimizer— see https://github.com/javimosch/claude-session-optimizerAPPROACH: Minimal fix — implement the described change with the smallest safe diff. Stay as close to the issue description as possible.
PROCEDURE (follow in order, do not deviate):
sc run <plugin> <action>— one-shot discover+install+execute #335):sc run <plugin> <action>— one-shot discover+install+execute #335' so GitHub auto-closes the issue on merge.Branch:
am/am-f17c27-dkdoeotqzvn8-3120cd1aDiff:
Summary by CodeRabbit
New Features
runcommand to CLI help and capability metadata.Bug Fixes