-
Notifications
You must be signed in to change notification settings - Fork 0
Add NeuroRift package skeleton, skill system, runtime/model checks, and hardened tool execution #37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
6558efb
f4393f6
7736c15
b362984
fcd7962
d5e4cea
d6fc44a
6c97451
b8b8e8d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,99 @@ | ||||||||||||||||||||||||||
| """Ollama model capability verification for autonomous NeuroRift agent mode.""" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| from __future__ import annotations | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| import json | ||||||||||||||||||||||||||
| import subprocess | ||||||||||||||||||||||||||
| from typing import Any | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| CAPABILITY_PROMPT = """Evaluate whether you can reliably perform the following tasks: | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| * structured tool invocation | ||||||||||||||||||||||||||
| * Linux command generation | ||||||||||||||||||||||||||
| * file creation and modification | ||||||||||||||||||||||||||
| * interpreting tool outputs | ||||||||||||||||||||||||||
| * multi-step reasoning for autonomous agents | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Return a JSON response: | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| "tool_usage": true/false, | ||||||||||||||||||||||||||
| "command_generation": true/false, | ||||||||||||||||||||||||||
| "filesystem_operations": true/false, | ||||||||||||||||||||||||||
| "multi_step_reasoning": true/false, | ||||||||||||||||||||||||||
| "agent_ready": true/false | ||||||||||||||||||||||||||
| }""" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def _extract_json(raw_text: str) -> dict[str, Any]: | ||||||||||||||||||||||||||
| raw_text = (raw_text or "").strip() | ||||||||||||||||||||||||||
| 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]) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def verify_model_capabilities(model_name: str) -> dict[str, Any]: | ||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||
| proc = subprocess.run( | ||||||||||||||||||||||||||
| ["ollama", "run", model_name, CAPABILITY_PROMPT], | ||||||||||||||||||||||||||
| capture_output=True, | ||||||||||||||||||||||||||
| text=True, | ||||||||||||||||||||||||||
| timeout=120, | ||||||||||||||||||||||||||
| check=False, | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| except FileNotFoundError: | ||||||||||||||||||||||||||
| return {"ok": False, "error": "ollama_missing", "agent_ready": False} | ||||||||||||||||||||||||||
| except Exception as exc: # defensive for unstable environments | ||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||
| "ok": False, | ||||||||||||||||||||||||||
| "error": f"ollama_exec_error:{type(exc).__name__}", | ||||||||||||||||||||||||||
| "agent_ready": False, | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if proc.returncode != 0: | ||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||
| "ok": False, | ||||||||||||||||||||||||||
| "error": f"ollama_returned_{proc.returncode}", | ||||||||||||||||||||||||||
| "stderr": proc.stderr, | ||||||||||||||||||||||||||
| "agent_ready": False, | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||
| parsed = _extract_json(proc.stdout) | ||||||||||||||||||||||||||
| except Exception as exc: | ||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||
| "ok": False, | ||||||||||||||||||||||||||
| "error": f"invalid_capability_json:{type(exc).__name__}", | ||||||||||||||||||||||||||
| "raw": proc.stdout[:1000], | ||||||||||||||||||||||||||
| "agent_ready": False, | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| required = { | ||||||||||||||||||||||||||
| "tool_usage", | ||||||||||||||||||||||||||
| "command_generation", | ||||||||||||||||||||||||||
| "filesystem_operations", | ||||||||||||||||||||||||||
| "multi_step_reasoning", | ||||||||||||||||||||||||||
| "agent_ready", | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| 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 | ||||||||||||||||||||||||||
|
Comment on lines
+72
to
+73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: Converting Severity Level: Critical 🚨- ❌ Non-ready models can pass autonomous safety gate.
- ❌ run-agent may execute with invalid capability contract.
Suggested change
Steps of Reproduction ✅1. Invoke autonomous flow via `run-agent` (parser wiring at `neurorift_main.py:937-940`,
execution gate at `neurorift_main.py:1006-1021`).
2. In `verify_model_capabilities()` (`model_capability_check.py:67-83`), only key presence
is checked (`required.issubset(...)` at lines 67-75), not field types.
3. When model returns JSON like `"agent_ready": "false"` (string), line 82 computes
`bool("false") == True`, so `ok` is incorrectly marked true.
4. `neurorift_main.py:1018` tests `if not capability.get("agent_ready")`; string `"false"`
is truthy, so the rejection branch is skipped and orchestrated agent mode proceeds
(`neurorift_main.py:1066+`) even though readiness value is semantically false.Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** model_capability_check.py
**Line:** 82:83
**Comment:**
*Type Error: Converting `agent_ready` with `bool(...)` treats non-empty strings like `"false"` as `True`, which can incorrectly mark an incapable model as ready. Require `agent_ready` to be a real boolean and then copy that value directly.
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 __name__ == "__main__": | ||||||||||||||||||||||||||
| import argparse | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| parser = argparse.ArgumentParser( | ||||||||||||||||||||||||||
| description="Check Ollama model capability for NeuroRift agent mode" | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| parser.add_argument("--model", required=True) | ||||||||||||||||||||||||||
| args = parser.parse_args() | ||||||||||||||||||||||||||
| print(json.dumps(verify_model_capabilities(args.model), indent=2)) | ||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| class ChannelRouter: | ||
| def route(self, msg: dict): | ||
| return msg |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| class Channel: | ||
| def connect(self): | ||
| return True | ||
|
|
||
| def receive_message(self, payload=None): | ||
| return payload or {} | ||
|
|
||
| def send_message(self, message): | ||
| return message | ||
|
|
||
| def close(self): | ||
| return True |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| class Channel: | ||
| def connect(self): | ||
| return True | ||
|
|
||
| def receive_message(self, payload=None): | ||
| return payload or {} | ||
|
|
||
| def send_message(self, message): | ||
| return message | ||
|
|
||
| def close(self): | ||
| return True |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| class Channel: | ||
| def connect(self): | ||
| return True | ||
|
|
||
| def receive_message(self, payload=None): | ||
| return payload or {} | ||
|
|
||
| def send_message(self, message): | ||
| return message | ||
|
|
||
| def close(self): | ||
| return True |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| class Channel: | ||
| def connect(self): | ||
| return True | ||
|
|
||
| def receive_message(self, payload=None): | ||
| return payload or {} | ||
|
|
||
| def send_message(self, message): | ||
| return message | ||
|
|
||
| def close(self): | ||
| return True |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| class ClawHubAPI: | ||
| base_url = "https://clawhub.example/api" |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,18 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from pathlib import Path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import shutil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not src.exists(): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| raise FileNotFoundError(skill_name) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dst = self.cache_dir / skill_name | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if dst.exists(): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| shutil.rmtree(dst) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| shutil.copytree(src, dst) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+14
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎.
Comment on lines
+14
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: The destination path is built from untrusted Severity Level: Critical 🚨- ❌ rmtree can delete ~/.neurorift/skills/installed unexpectedly.
- ❌ copytree can overwrite directories outside cache root.
Suggested change
Steps of Reproduction ✅1. Install any skill once via `neurorift --clawhub recon_scanner` so
`~/.neurorift/skills/installed` exists (`SkillInstaller` paths created at
`neurorift/skills/installer.py:8-10`).
2. Run `neurorift --clawhub ../installed`; argument is accepted at `neurorift_main.py:914`
and passed into `SkillManager.install_clawhub()` at `neurorift_main.py:981-983`.
3. `fetch_skill()` computes `dst = self.cache_dir / skill_name`
(`neurorift/clawhub/clawhub_client.py:8`), so `../installed` targets sibling directory
outside cache.
4. Because line 9 executes `shutil.rmtree(dst)` and line 10 executes `shutil.copytree(src,
dst)`, paths outside `cache_dir` can be deleted/overwritten.Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** neurorift/clawhub/clawhub_client.py
**Line:** 8:10
**Comment:**
*Security: The destination path is built from untrusted `skill_name` and then passed to `rmtree`/`copytree`, which allows path traversal to delete or overwrite directories outside the cache. Resolve and validate the destination path is contained within `cache_dir` before any filesystem mutation.
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.
Comment on lines
+10
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prevent path traversal/arbitrary delete via
🔒 Suggested hardening patch def fetch_skill(self, skill_name: str, source_dir: Path) -> Path:
- src = source_dir / skill_name
- if not src.exists():
+ if not skill_name or Path(skill_name).name != skill_name:
+ raise ValueError(f"Invalid skill name: {skill_name!r}")
+
+ source_root = source_dir.resolve()
+ cache_root = self.cache_dir.resolve()
+ src = (source_root / skill_name).resolve()
+ dst = (cache_root / skill_name).resolve()
+
+ if source_root not in src.parents:
+ raise ValueError(f"Skill path escapes source_dir: {skill_name!r}")
+ if cache_root not in dst.parents:
+ raise ValueError(f"Skill path escapes cache_dir: {skill_name!r}")
+
+ if not src.is_dir():
raise FileNotFoundError(skill_name)
- dst = self.cache_dir / skill_name📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return dst | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| class ClawHubResolver: | ||
| def resolve(self, skill_name: str) -> str: | ||
| return f"https://clawhub.example/skills/{skill_name}" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| """Package-level CLI helper that delegates to global entrypoint.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from neurorift_cli import build_parser, install_global_wrapper, main | ||
|
|
||
| __all__ = ["build_parser", "install_global_wrapper", "main"] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| CHANNELS = ["cli", "websocket", "discord", "telegram", "api"] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| MODEL_PROVIDERS = ["deepseek", "mistral", "llama", "openai"] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| from dataclasses import dataclass | ||
| from pathlib import Path | ||
|
|
||
|
|
||
| @dataclass | ||
| class Settings: | ||
| data_dir: Path = Path.home() / ".neurorift" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| class AgentLoop: | ||
| async def handle_message(self, session, message: str) -> dict: | ||
| return {"success": True, "response": f"Session {session.session_id}: {message}"} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| from dataclasses import dataclass | ||
| from neurorift.core.agent_loop import AgentLoop | ||
| from neurorift.sessions.session_manager import SessionManager | ||
|
|
||
|
|
||
| @dataclass | ||
| class AgentHandle: | ||
| session_id: str | ||
| user_id: str | ||
| channel: str | ||
|
|
||
|
|
||
| class AgentManager: | ||
| def __init__(self, session_manager: SessionManager, agent_loop: AgentLoop): | ||
| self.session_manager = session_manager | ||
| self.agent_loop = agent_loop | ||
| self.handles: dict[str, AgentHandle] = {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| from dataclasses import dataclass | ||
|
|
||
|
|
||
| @dataclass | ||
| class PlannedStep: | ||
| order: int | ||
| action: str | ||
|
|
||
|
|
||
| class Planner: | ||
| def create_plan(self, objective: str) -> list[PlannedStep]: | ||
| return ( | ||
| [PlannedStep(order=1, action=f"Investigate: {objective}")] | ||
| if objective | ||
| else [] | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| class TaskRouter: | ||
| def route(self, task: str) -> dict: | ||
| return {"task": task, "route": "agent_loop"} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import subprocess | ||
|
|
||
|
|
||
| class CommandRunner: | ||
| def run(self, cmd: list[str]): | ||
| p = subprocess.run(cmd, capture_output=True, text=True, check=False) | ||
| return { | ||
| "success": p.returncode == 0, | ||
| "stdout": p.stdout, | ||
| "stderr": p.stderr, | ||
| "exit_code": p.returncode, | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| from dataclasses import dataclass | ||
|
|
||
|
|
||
| @dataclass | ||
| class ResourceLimits: | ||
| timeout_seconds: int = 30 | ||
| memory_limit_mb: int = 512 | ||
| cpu_seconds: int = 20 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| import time | ||
|
|
||
|
|
||
| class RetryManager: | ||
| def __init__(self, retries=2): | ||
| self.retries = retries | ||
|
|
||
| def wait(self, attempt): | ||
| time.sleep(2**attempt) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| from neurorift.execution.command_runner import CommandRunner | ||
|
|
||
|
|
||
| class SandboxRunner(CommandRunner): | ||
| pass |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| class APIServer: | ||
| def start(self): | ||
| return True |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| class AuthManager: | ||
| def validate(self, token: str): | ||
| return bool(token) | ||
|
Comment on lines
+1
to
+3
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fail-closed auth is required; current token check is bypassable.
🔒 Proposed safe placeholder until real verification is implemented class AuthManager:
- def validate(self, token: str):
- return bool(token)
+ def validate(self, token: str) -> bool:
+ raise NotImplementedError(
+ "Token verification is not implemented. Wire a real verifier before enabling auth."
+ )🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| class WebSocketGateway: | ||
| def start(self): | ||
| return True |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| class LongTermMemory: | ||
| def __init__(self): | ||
| self.data = {} | ||
|
|
||
| def store(self, user_id: str, key: str, value): | ||
| self.data.setdefault(user_id, {})[key] = value |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| class MemoryCompaction: | ||
| def compact(self, messages: list[str]): | ||
| return {"summary": " ".join(messages[:10]), "kept": messages[-10:]} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| from collections import defaultdict, deque | ||
|
|
||
|
|
||
| class ShortTermMemory: | ||
| def __init__(self, limit: int = 30): | ||
| self.buffers = defaultdict(lambda: deque(maxlen=limit)) | ||
|
|
||
| def add(self, sid: str, msg: str): | ||
| self.buffers[sid].append(msg) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| class VectorMemory: | ||
| def semantic_search(self, user_id: str, query: str, top_k: int = 5): | ||
| return [] | ||
|
|
||
| def time_based_retrieval(self, user_id: str, limit: int = 10): | ||
| return [] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| class ContextBuilder: | ||
| def build(self, session, memories): | ||
| return {"session": session.session_id, "memories": memories} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| class ModelFailover: | ||
| def pick(self, providers): | ||
| return providers[0] if providers else "fallback" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| class ModelRouter: | ||
| async def generate(self, prompt: str): | ||
| return { | ||
| "response": prompt, | ||
| "model": "openai", | ||
| "tokens": len(prompt.split()), | ||
| "cost": 0.0, | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| class PromptBuilder: | ||
| def build(self, message: str, context: dict): | ||
| return f"{context}\n{message}" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
|
|
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,29 @@ | ||||||||||||||
| from dataclasses import dataclass, field | ||||||||||||||
| from enum import Enum | ||||||||||||||
| from datetime import datetime, timezone | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| class SessionState(str, Enum): | ||||||||||||||
| CREATED = "CREATED" | ||||||||||||||
| ACTIVE = "ACTIVE" | ||||||||||||||
| IDLE = "IDLE" | ||||||||||||||
| PAUSED = "PAUSED" | ||||||||||||||
| CLOSED = "CLOSED" | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| @dataclass | ||||||||||||||
| class SessionContext: | ||||||||||||||
| session_id: str | ||||||||||||||
| user_id: str | ||||||||||||||
| channel: str | ||||||||||||||
| state: SessionState = SessionState.CREATED | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: Severity Level: Major
|
||||||||||||||
| state: SessionState = SessionState.CREATED | |
| state: SessionState = SessionState.CREATED | |
| def __post_init__(self): | |
| if not isinstance(self.state, SessionState): | |
| self.state = SessionState(self.state) |
Steps of Reproduction ✅
1. Create a persisted session through `SessionManager.create_session()` at
`neurorift/sessions/session_manager.py:10-12`; it stores JSON via `SessionStore.save()` at
`neurorift/sessions/session_store.py:9`.
2. Load that same session through `SessionManager.get_session()` at
`neurorift/sessions/session_manager.py:13`, which calls `SessionStore.load()` at
`neurorift/sessions/session_store.py:10-12`.
3. `SessionStore.load()` reconstructs with `SessionContext(**json.loads(...))`
(`neurorift/sessions/session_store.py:12`), so `state` is passed as plain JSON string.
4. `SessionContext` currently has no `__post_init__` validation
(`neurorift/sessions/session_context.py:8-19`), so loaded objects can carry `state` as
`str` instead of `SessionState`, violating the dataclass contract immediately after
deserialization.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** neurorift/sessions/session_context.py
**Line:** 13:13
**Comment:**
*Type Error: `state` is annotated as an enum, but sessions loaded from JSON pass a plain string into this dataclass, so the object can carry an invalid/non-enum state and break enum-based logic (for example code expecting enum behavior). Coerce and validate `state` in `__post_init__` so deserialized sessions always have a real `SessionState`.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: The JSON extraction uses the first
{and last}in the whole response, so any extra brace text before/after the actual JSON object can make parsing fail even when the model returned valid JSON. Parse the first decodable JSON object instead of slicing by global brace positions. [logic error]Severity Level: Major⚠️
Steps of Reproduction ✅
Prompt for AI Agent 🤖