Add NeuroRift package scaffold, runtime & model checks, secure command runner, CLI and skill system#38
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (82)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
codeant-ai
Bot
added
the
size:XXL
Sequence DiagramThis PR introduces a new global CLI flow that routes commands into either the skill management pipeline or autonomous agent startup. For agent startup, it adds runtime and model readiness checks before launching the secured execution path. sequenceDiagram
participant User
participant CLI
participant Main
participant SkillManager
participant RuntimeCheck
participant ModelCheck
participant AgentRuntime
User->>CLI: Run neurorift command
CLI->>Main: Forward parsed arguments
Main->>SkillManager: Initialize skill manager
alt Skill command path
Main->>SkillManager: Install list run or uninstall skill
SkillManager-->>Main: Skill operation result
Main-->>User: Return skill response
else Run agent path
Main->>RuntimeCheck: Verify runtime tools and workspace
RuntimeCheck-->>Main: Runtime status
Main->>ModelCheck: Verify model capabilities
ModelCheck-->>Main: Agent readiness
Main->>AgentRuntime: Start autonomous mode with secure execution
AgentRuntime-->>User: Agent startup response
end
Generated by CodeAnt AI |
 |
|
Overall Grade Focus Area: Complexity |
Security  Reliability Complexity  Hygiene |
Feedback
- State-free methods masquerading as instance methods
- Many methods are bound to classes without using instance state, bloating class interfaces and increasing cognitive complexity. Converting them to staticmethods or module-level helpers and grouping stateless utilities will shrink class surface and simplify reasoning.
- Leftover refactor artifacts causing clutter
- Unused imports, shadowed names, dead variables, and unused args point to incomplete refactors that obscure intent and hide real issues. Systematic cleanup and stricter linting for unused symbols will prevent this noise from accumulating.
- Implicit environment trust causes security and fragility
- The code assumes safe temp paths, partial executable resolution, and reaches into protected internals, producing insecure temp usage, fragile process launches, and encapsulation breaches. Require explicit validation, secure-temp APIs, absolute executable resolution, and respect for private APIs to break the pattern.
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Python |  |
Mar 14, 2026 12:52p.m. | Review ↗ |
Nitpicks 🔍
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac3341c9c7
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| def main() -> int: | ||
| parser = build_parser() | ||
| ns = parser.parse_args() |
There was a problem hiding this comment.
Because setup.py switches the console entrypoint to neurorift_cli:main, this parse_args() call now gates all normal CLI invocations; options that belong to neurorift_main (for example neurorift --target example.com --orchestrated) are rejected as unrecognized before they are forwarded. In practice this breaks existing flag-based workflows unless users add a manual -- separator, so the wrapper should accept/pass through unknown args instead of hard-failing.
Useful? React with 👍 / 👎.
| if not stdout.strip() and not stderr.strip(): | ||
| validation_passed = False | ||
| error_type = error_type or "empty_output" |
There was a problem hiding this comment.
This logic treats any command with both output streams empty as a validation failure, even when the process exits with code 0. _run_command_secure then marks the run unsuccessful, which causes false negatives for legitimately silent commands such as go install ...; install_missing_tools will report those installs as failed even when they succeeded, breaking automated tool bootstrapping.
Useful? React with 👍 / 👎.
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| start = raw_text.find("{") | ||
| end = raw_text.rfind("}") | ||
| if start == -1 or end == -1 or end <= start: | ||
| raise ValueError("No JSON object found in model response") | ||
| return json.loads(raw_text[start : end + 1]) |
There was a problem hiding this comment.
Suggestion: JSON extraction is fragile because it slices from the first { to the last } in the whole response. If the model returns a valid JSON object plus any extra brace in trailing commentary, parsing fails and the model is incorrectly rejected. Parse the first valid JSON object instead of using a greedy substring. [logic error]
Severity Level: Major ⚠️
- ❌ run-agent startup fails on otherwise usable model output.
- ⚠️ Capability check becomes brittle to minor output noise.| start = raw_text.find("{") | |
| end = raw_text.rfind("}") | |
| if start == -1 or end == -1 or end <= start: | |
| raise ValueError("No JSON object found in model response") | |
| return json.loads(raw_text[start : end + 1]) | |
| decoder = json.JSONDecoder() | |
| for idx, ch in enumerate(raw_text): | |
| if ch != "{": | |
| continue | |
| try: | |
| obj, _ = decoder.raw_decode(raw_text[idx:]) | |
| except json.JSONDecodeError: | |
| continue | |
| if isinstance(obj, dict): | |
| return obj | |
| raise ValueError("No JSON object found in model response") |
Steps of Reproduction ✅
1. Launch autonomous mode through real CLI path: `neurorift_cli.py:51-52` forwards args to
`neurorift_main.main()`, and parser exposes `run-agent` at `neurorift_main.py:937`.
2. Run `neurorift run-agent --model test-model` so `neurorift_main.py:1007-1018` calls
`verify_model_capabilities()` from `model_capability_check.py:35`.
3. Have `ollama run` return text containing a valid JSON object followed by commentary
with an extra `}`; parser in `_extract_json` (`model_capability_check.py:28-32`) slices
from first `{` to last `}`, creating invalid JSON.
4. `json.loads` fails, `verify_model_capabilities` returns `invalid_capability_json` at
`model_capability_check.py:59-64`, and `run-agent` is blocked by readiness gate at
`neurorift_main.py:1019-1023`.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** model_capability_check.py
**Line:** 28:32
**Comment:**
*Logic Error: JSON extraction is fragile because it slices from the first `{` to the last `}` in the whole response. If the model returns a valid JSON object plus any extra brace in trailing commentary, parsing fails and the model is incorrectly rejected. Parse the first valid JSON object instead of using a greedy substring.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| if not required.issubset(parsed): | ||
| return { | ||
| "ok": False, | ||
| "error": "capability_fields_missing", | ||
| "parsed": parsed, | ||
| "agent_ready": False, | ||
| } | ||
|
|
||
| parsed["ok"] = bool(parsed.get("agent_ready")) | ||
| return parsed |
There was a problem hiding this comment.
Suggestion: Capability fields are only checked for presence, not type, so string values like "false" are treated as truthy by callers and can incorrectly allow non-ready models to pass. Enforce boolean types for all required fields and compute readiness from a real boolean. [type error]
Severity Level: Critical 🚨
- ❌ Non-ready models can pass autonomous readiness gate.
- ⚠️ Autonomous execution reliability drops with weak model capability.| if not required.issubset(parsed): | |
| return { | |
| "ok": False, | |
| "error": "capability_fields_missing", | |
| "parsed": parsed, | |
| "agent_ready": False, | |
| } | |
| parsed["ok"] = bool(parsed.get("agent_ready")) | |
| return parsed | |
| if not required.issubset(parsed): | |
| return { | |
| "ok": False, | |
| "error": "capability_fields_missing", | |
| "parsed": parsed, | |
| "agent_ready": False, | |
| } | |
| if not all(isinstance(parsed.get(field), bool) for field in required): | |
| return { | |
| "ok": False, | |
| "error": "capability_fields_invalid_type", | |
| "parsed": parsed, | |
| "agent_ready": False, | |
| } | |
| parsed["ok"] = parsed["agent_ready"] | |
| return parsed |
Steps of Reproduction ✅
1. Trigger autonomous startup via `neurorift_cli.py:51-52` into `run-agent` path in
`neurorift_main.py:1007`.
2. Execute with a model output where capability fields are strings (e.g., `"agent_ready":
"false"`), which is realistic for LLM formatting drift; `verify_model_capabilities` parses
it at `model_capability_check.py:57-58`.
3. Current code only checks key presence (`model_capability_check.py:74`) and then
computes `ok` using `bool(parsed.get("agent_ready"))` (`model_capability_check.py:82`), so
`"false"` is treated truthy.
4. Caller checks raw `capability.get("agent_ready")` at `neurorift_main.py:1019`;
non-empty string bypasses rejection, allowing autonomous run with a non-ready model.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** model_capability_check.py
**Line:** 74:83
**Comment:**
*Type Error: Capability fields are only checked for presence, not type, so string values like `"false"` are treated as truthy by callers and can incorrectly allow non-ready models to pass. Enforce boolean types for all required fields and compute readiness from a real boolean.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| @@ -0,0 +1,5 @@ | |||
| class Channel: | |||
| def connect(self): return True | |||
| def receive_message(self, payload=None): return payload or {} | |||
There was a problem hiding this comment.
Suggestion: Using payload or {} incorrectly rewrites valid falsy payloads (such as "", 0, [], or False) into an empty dict, which can break message handling and type expectations at runtime. Only substitute {} when the payload is actually None. [logic error]
Severity Level: Major ⚠️
- ⚠️ Channel receive path drops valid falsy message content.
- ⚠️ Telegram adapter returns wrong type for empty payload.
- ⚠️ Same bug duplicated in CLI/Discord/Web channels.| def receive_message(self, payload=None): return payload or {} | |
| def receive_message(self, payload=None): return {} if payload is None else payload |
Steps of Reproduction ✅
1. Open `neurorift/channels/telegram_channel.py:1-4` and instantiate `Channel` (class
defined at line 1), then call `receive_message("")` on it.
2. Execution reaches `receive_message` at `neurorift/channels/telegram_channel.py:3`,
which evaluates `payload or {}`.
3. Because `""` is falsy, method returns `{}` instead of the original payload, changing
both value and type.
4. This is reproducible in current code and is not isolated: identical logic exists in
`neurorift/channels/cli_channel.py:3`, `discord_channel.py:3`, and `web_channel.py:3`
(verified by search), so the same payload-rewrite behavior appears across all channel
stubs.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** neurorift/channels/telegram_channel.py
**Line:** 3:3
**Comment:**
*Logic Error: Using `payload or {}` incorrectly rewrites valid falsy payloads (such as `""`, `0`, `[]`, or `False`) into an empty dict, which can break message handling and type expectations at runtime. Only substitute `{}` when the payload is actually `None`.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| class ClawHubClient: | ||
| def __init__(self, cache_dir: Path): self.cache_dir=cache_dir; cache_dir.mkdir(parents=True, exist_ok=True) | ||
| def fetch_skill(self, skill_name: str, source_dir: Path) -> Path: | ||
| src = source_dir / skill_name |
There was a problem hiding this comment.
Suggestion: skill_name is directly joined into the source path without containment checks, so values like ../... can escape the intended skill directory and copy arbitrary local folders. Resolve the path and verify it still stays under source_dir before using it. [security]
Severity Level: Critical 🚨
- ⚠️ ClawHub install reads directories outside skill_store.
- ❌ Unauthorized local folder contents copied into cache.| src = source_dir / skill_name | |
| src = (source_dir / skill_name).resolve() | |
| if src.parent != source_dir.resolve(): | |
| raise ValueError(f"Invalid skill name: {skill_name}") |
Steps of Reproduction ✅
1. Run CLI install flow with user-controlled argument: `python3 neurorift_main.py
--clawhub ../../..`; `--clawhub` is accepted at `neurorift_main.py:914` and executed at
`neurorift_main.py:983`.
2. Call chain is `main()` (`neurorift_main.py:1472`) → `_async_main()`
(`neurorift_main.py:973`) → `SkillManager.install_clawhub()`
(`neurorift/skills/skill_manager.py:13-14`) → `ClawHubClient.fetch_skill()`
(`neurorift/clawhub/clawhub_client.py:5-6`).
3. At `clawhub_client.py:6`, `src = source_dir / skill_name` allows `..` traversal, so
`src` can resolve outside `self.examples` (`skill_manager.py:12`) without any containment
check.
4. `shutil.copytree(src, dst)` at `clawhub_client.py:10` then copies an out-of-scope local
directory into cache before validation, proving source-directory escape.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** neurorift/clawhub/clawhub_client.py
**Line:** 6:6
**Comment:**
*Security: `skill_name` is directly joined into the source path without containment checks, so values like `../...` can escape the intended skill directory and copy arbitrary local folders. Resolve the path and verify it still stays under `source_dir` before using it.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| def __init__(self, cache_dir: Path): self.cache_dir=cache_dir; cache_dir.mkdir(parents=True, exist_ok=True) | ||
| def fetch_skill(self, skill_name: str, source_dir: Path) -> Path: | ||
| src = source_dir / skill_name | ||
| if not src.exists(): raise FileNotFoundError(skill_name) |
There was a problem hiding this comment.
Suggestion: Checking only exists() lets regular files pass, and then shutil.copytree raises NotADirectoryError at runtime. Validate that the source is a directory before copying. [logic error]
Severity Level: Major ⚠️
- ❌ `--clawhub` command crashes on file-name input.
- ⚠️ Skill install UX returns traceback, not clean error.| if not src.exists(): raise FileNotFoundError(skill_name) | |
| if not src.is_dir(): raise FileNotFoundError(skill_name) |
Steps of Reproduction ✅
1. Confirm source store contains a file:
`/workspace/NeuroRift/neurorift/skill_store/installed/__init__.py` (listed by `LS`), not
only skill directories.
2. Run `python3 neurorift_main.py --clawhub __init__.py`; this reaches
`skill_manager.install_clawhub()` at `neurorift/skills/skill_manager.py:13-14`.
3. In `fetch_skill()` (`neurorift/clawhub/clawhub_client.py:7`), `src.exists()` passes for
that file, so no early rejection occurs.
4. `shutil.copytree(src, dst)` at `clawhub_client.py:10` raises `NotADirectoryError`;
there is no surrounding handler in `_async_main`/`main` (`neurorift_main.py:973`, `1472`,
`1521`), so CLI run fails with traceback.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** neurorift/clawhub/clawhub_client.py
**Line:** 7:7
**Comment:**
*Logic Error: Checking only `exists()` lets regular files pass, and then `shutil.copytree` raises `NotADirectoryError` at runtime. Validate that the source is a directory before copying.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| class SkillLoader: | ||
| def run(self, skill_dir: Path, **kwargs): | ||
| meta=json.loads((skill_dir/'skill.json').read_text(encoding='utf-8')) | ||
| entry=skill_dir/meta['entrypoint'] |
There was a problem hiding this comment.
Suggestion: Accessing required metadata keys with direct indexing will raise a runtime exception when a skill package omits entrypoint, which crashes skill execution instead of returning a controlled error response. [logic error]
Severity Level: Major ⚠️
- ❌ `--skill run` crashes on incomplete `skill.json`.
- ⚠️ Skill CLI reliability depends on perfect metadata.| entry=skill_dir/meta['entrypoint'] | |
| entrypoint = meta.get('entrypoint') | |
| if not entrypoint: | |
| return {'error': 'missing entrypoint in skill.json'} | |
| entry = skill_dir / entrypoint |
Steps of Reproduction ✅
1. Use CLI skill flow in `neurorift_main.py` (`_async_main`, lines 91-99 from Grep output)
by running `python3 neurorift_main.py --skill run recon_scanner`; this path calls
`SkillManager.run()` at `neurorift/skills/skill_manager.py:17-18`.
2. Ensure installed skill metadata is incomplete by removing `entrypoint` from
`~/.neurorift/skills/installed/recon_scanner/skill.json` (validator only checks file
presence at `neurorift/skills/skill_validator.py:3-5`, not required JSON keys).
3. Re-run `--skill run recon_scanner`; execution reaches `SkillLoader.run()` at
`neurorift/skills/loader.py:4-6`.
4. Observe crash: `meta['entrypoint']` at `loader.py:6` raises `KeyError`, and
`_async_main` has no try/except around `skill_manager.run` branch, so skill execution
terminates.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** neurorift/skills/loader.py
**Line:** 6:6
**Comment:**
*Logic Error: Accessing required metadata keys with direct indexing will raise a runtime exception when a skill package omits `entrypoint`, which crashes skill execution instead of returning a controlled error response.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| meta=json.loads((skill_dir/'skill.json').read_text(encoding='utf-8')) | ||
| entry=skill_dir/meta['entrypoint'] | ||
| spec=importlib.util.spec_from_file_location(f"skill_{meta['name']}", entry) | ||
| mod=importlib.util.module_from_spec(spec); spec.loader.exec_module(mod) |
There was a problem hiding this comment.
Suggestion: The import spec and loader are dereferenced without checking for None; invalid module specs will raise runtime exceptions during module loading. [null pointer]
Severity Level: Major ⚠️
- ❌ Invalid entrypoint crashes skill execution flow.
- ⚠️ Users receive traceback instead of structured error JSON.| mod=importlib.util.module_from_spec(spec); spec.loader.exec_module(mod) | |
| if spec is None or spec.loader is None: | |
| return {'error': 'invalid skill entrypoint module'} | |
| mod = importlib.util.module_from_spec(spec) | |
| spec.loader.exec_module(mod) |
Steps of Reproduction ✅
1. Run skill path `--skill run <name>` via `_async_main` (`neurorift_main.py:91-99`),
which always calls `SkillLoader.run` through `SkillManager.run`
(`neurorift/skills/skill_manager.py:18`).
2. Set skill `entrypoint` in `skill.json` to a directory-like target (for example `"."`),
producing a non-loadable module location.
3. At `neurorift/skills/loader.py:7`, `spec_from_file_location(...)` returns `None` for
directory paths (verified in runtime check), but code immediately continues.
4. `module_from_spec(spec)` at `loader.py:8` raises `AttributeError` (`NoneType` loader),
causing unhandled crash instead of controlled error.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** neurorift/skills/loader.py
**Line:** 8:8
**Comment:**
*Null Pointer: The import spec and loader are dereferenced without checking for `None`; invalid module specs will raise runtime exceptions during module loading.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| entry=skill_dir/meta['entrypoint'] | ||
| spec=importlib.util.spec_from_file_location(f"skill_{meta['name']}", entry) | ||
| mod=importlib.util.module_from_spec(spec); spec.loader.exec_module(mod) | ||
| return mod.run(**kwargs) if hasattr(mod, 'run') else {'error':'missing run()'} |
There was a problem hiding this comment.
Suggestion: Checking only attribute existence is insufficient because run may exist but be non-callable, which raises a TypeError when invoked. [type error]
Severity Level: Major ⚠️
- ❌ Broken skills crash `--skill run` command.
- ⚠️ No graceful validation for malformed skill modules.| return mod.run(**kwargs) if hasattr(mod, 'run') else {'error':'missing run()'} | |
| run_fn = getattr(mod, 'run', None) | |
| return run_fn(**kwargs) if callable(run_fn) else {'error': 'missing callable run()'} |
Steps of Reproduction ✅
1. Execute skill command path `python3 neurorift_main.py --skill run recon_scanner`
(`neurorift_main.py:91-99`), which calls `SkillLoader.run` via `SkillManager.run`
(`neurorift/skills/skill_manager.py:18`).
2. Make skill module define non-callable `run`, e.g., `run = "not_callable"` in installed
`skill.py`.
3. Loader condition at `neurorift/skills/loader.py:9` passes because `hasattr(mod, 'run')`
is true.
4. Call `mod.run(**kwargs)` throws `TypeError: 'str' object is not callable`, crashing CLI
skill invocation.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** neurorift/skills/loader.py
**Line:** 9:9
**Comment:**
*Type Error: Checking only attribute existence is insufficient because `run` may exist but be non-callable, which raises a `TypeError` when invoked.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| def install_clawhub(self, skill_name: str): | ||
| pkg=self.client.fetch_skill(skill_name, self.examples) | ||
| return self.installer.install(pkg) |
There was a problem hiding this comment.
Suggestion: install_clawhub calls fetch_skill directly and lets FileNotFoundError bubble up. When a user requests a non-existent skill, the CLI flow will crash instead of returning a structured failure result. Catch this specific exception and return an error payload. [possible bug]
Severity Level: Major ⚠️
- ❌ `--clawhub` install flow terminates on missing skill names.
- ⚠️ User gets traceback, not structured install error JSON.| def install_clawhub(self, skill_name: str): | |
| pkg=self.client.fetch_skill(skill_name, self.examples) | |
| return self.installer.install(pkg) | |
| def install_clawhub(self, skill_name: str): | |
| try: | |
| pkg = self.client.fetch_skill(skill_name, self.examples) | |
| except FileNotFoundError: | |
| return {"success": False, "error": f"skill_not_found:{skill_name}"} | |
| return self.installer.install(pkg) |
Steps of Reproduction ✅
1. Run CLI entrypoint with an unavailable skill name, e.g. `python3 neurorift_main.py
--clawhub does_not_exist`; this path is wired by `--clawhub` argument in
`neurorift_main.py:914`.
2. In `_async_main`, `args.clawhub` dispatches to
`skill_manager.install_clawhub(args.clawhub.strip())` at `neurorift_main.py:983`.
3. `SkillManager.install_clawhub()` calls `ClawHubClient.fetch_skill()` directly at
`neurorift/skills/skill_manager.py:14`; `fetch_skill()` raises `FileNotFoundError` when
source directory is missing at `neurorift/clawhub/clawhub_client.py:7`.
4. No local exception handling exists in `install_clawhub()` or `_async_main` around that
call, so execution aborts before printing the intended failure branch
(`result.get("success")` block at `neurorift_main.py:984-988`).Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** neurorift/skills/skill_manager.py
**Line:** 13:15
**Comment:**
*Possible Bug: `install_clawhub` calls `fetch_skill` directly and lets `FileNotFoundError` bubble up. When a user requests a non-existent skill, the CLI flow will crash instead of returning a structured failure result. Catch this specific exception and return an error payload.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| def run(self, skill_name: str, **kwargs): | ||
| return self.loader.run(self.installer.installed / skill_name, **kwargs) |
There was a problem hiding this comment.
Suggestion: run forwards execution to the loader without verifying the skill directory exists, so invoking an unknown skill raises file access errors from deep inside the loader. Add an existence check and return a controlled error response. [possible bug]
Severity Level: Major ⚠️
- ❌ `--skill run` crashes for uninstalled skill names.
- ⚠️ Skill execution CLI lacks graceful not-installed response.| def run(self, skill_name: str, **kwargs): | |
| return self.loader.run(self.installer.installed / skill_name, **kwargs) | |
| def run(self, skill_name: str, **kwargs): | |
| skill_dir = self.installer.installed / skill_name | |
| if not skill_dir.is_dir(): | |
| return {"error": f"skill_not_installed:{skill_name}"} | |
| return self.loader.run(skill_dir, **kwargs) |
Steps of Reproduction ✅
1. Execute `python3 neurorift_main.py --skill run missing_skill`; `--skill` command is
defined in `neurorift_main.py:915`.
2. `_async_main` routes `run` action to `skill_manager.run(skill_name, ...)` at
`neurorift_main.py:995-998`.
3. `SkillManager.run()` forwards directly to loader with computed path at
`neurorift/skills/skill_manager.py:18`, without checking existence.
4. `SkillLoader.run()` immediately reads `skill.json` from that directory at
`neurorift/skills/loader.py:5`; for unknown skill this raises file-access exception and
aborts CLI before JSON output line at `neurorift_main.py:998`.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** neurorift/skills/skill_manager.py
**Line:** 17:18
**Comment:**
*Possible Bug: `run` forwards execution to the loader without verifying the skill directory exists, so invoking an unknown skill raises file access errors from deep inside the loader. Add an existence check and return a controlled error response.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
|
CodeAnt AI finished reviewing your PR. |
1 participant
User description
Motivation
neuroriftpackage and CLI, add runtime and model capability checks, and harden tool execution for safer autonomous agent operation.Description
model_capability_check.pyto query an Ollama model with a capability prompt and parse a JSON readiness response, and addedruntime_environment_check.pyto validate required system tools and workspace.neuroriftpackage with many lightweight module stubs and implementations (channels, core, execution, gateway, memory, models, sessions, skills, storage, tools, utils) and an examplerecon_scannerskill underskill_store/installed.neurorift_main.pyto initialize the new skill manager, wire in model/runtime checks, and replace ad-hoc subprocess usage with a hardened_run_command_securerunner plus validation, sandboxing, resource limits, retries, and improved logging.neurorift_cli.pyas the global CLI entry wrapper,neurorift_launch.shfor guided startup, and updatedsetup.pyto include the new package and entry point adjustment.Testing
python3 runtime_environment_check.pywhich ran the automated tool probes and returned a JSON report of tool statuses successfully.python3 model_capability_check.py --model nonexistent_modelin an environment withoutollamaand observed the expectedollama_missingerror JSON response from the script.python3 neurorift_cli.py --helpto validate the CLI parser and confirmed the help output returned successfully.Codex Task
CodeAnt-AI Description
Add NeuroRift package scaffold, robust startup checks, secure command runner, CLI wrapper, and skill manager
What Changed
Impact
✅ Clearer startup checks (missing tools and model readiness reported)✅ Safer tool execution (argument validation, time/memory limits, retries)✅ Easier skill install and management from packaged examples💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.