Add strip_fences option and fix exec() scoping for nested functions
- Add strip_fences field to PromptStep for removing markdown code fences - Fix exec() to use same dict for globals and locals, allowing nested functions to access module-level imports - Add GUI checkbox for strip_fences option - Update CLAUDE.md with improved documentation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
33b807d545
commit
f980fe05f1
58
CLAUDE.md
58
CLAUDE.md
|
|
@ -12,11 +12,24 @@ CmdForge is a lightweight personal tool builder for AI-powered CLI commands. It
|
||||||
# Install for development
|
# Install for development
|
||||||
pip install -e ".[dev]"
|
pip install -e ".[dev]"
|
||||||
|
|
||||||
# Run tests
|
# Run all unit tests (excluding integration tests that need a server)
|
||||||
pytest
|
pytest tests/ -m "not integration"
|
||||||
|
|
||||||
# Run a single test
|
# Run all tests with verbose output
|
||||||
pytest tests/test.py::test_name
|
pytest tests/ -v
|
||||||
|
|
||||||
|
# Run a specific test file
|
||||||
|
pytest tests/test_runner.py -v
|
||||||
|
|
||||||
|
# Run a specific test class
|
||||||
|
pytest tests/test_runner.py::TestSubstituteVariables -v
|
||||||
|
|
||||||
|
# Run with coverage
|
||||||
|
pytest tests/ --cov=cmdforge --cov-report=html
|
||||||
|
|
||||||
|
# Run integration tests (requires local registry server at localhost:5000)
|
||||||
|
python -m cmdforge.registry.app # Start server first
|
||||||
|
pytest tests/test_registry_integration.py -v -m integration
|
||||||
|
|
||||||
# Run the CLI
|
# Run the CLI
|
||||||
python -m cmdforge.cli
|
python -m cmdforge.cli
|
||||||
|
|
@ -29,14 +42,19 @@ cmdforge
|
||||||
|
|
||||||
### Core Modules (`src/cmdforge/`)
|
### Core Modules (`src/cmdforge/`)
|
||||||
|
|
||||||
- **cli/**: CLI commands entry points (`cmdforge` command). Routes subcommands: list, create, edit, delete, test, run, refresh, collections
|
- **cli/**: CLI commands entry points (`cmdforge` command). Routes subcommands: list, create, edit, delete, test, run, refresh, collections, registry, settings, system-deps
|
||||||
- **tool.py**: Tool definition dataclasses (`Tool`, `ToolArgument`, `PromptStep`, `CodeStep`), YAML config loading/saving, wrapper script generation
|
- **tool.py**: Tool definition dataclasses (`Tool`, `ToolArgument`, `PromptStep`, `CodeStep`, `ToolStep`), YAML config loading/saving, wrapper script generation
|
||||||
|
- **runner.py**: Execution engine. Runs tool steps sequentially, handles variable substitution (`{input}`, `{varname}`), executes Python code steps via `exec()`, handles nested tool calls with depth limit (MAX_TOOL_DEPTH=10)
|
||||||
|
- **resolver.py**: Tool resolution. `resolve_tool()` searches: project manifest → local tools → owner/name → registry. Returns `ResolvedTool` with path info
|
||||||
- **collection.py**: Collection management (`Collection` dataclass, `resolve_tool_references()`, `classify_tool_reference()`), local collection storage in `~/.cmdforge/collections/`
|
- **collection.py**: Collection management (`Collection` dataclass, `resolve_tool_references()`, `classify_tool_reference()`), local collection storage in `~/.cmdforge/collections/`
|
||||||
- **runner.py**: Execution engine. Runs tool steps sequentially, handles variable substitution (`{input}`, `{varname}`), executes Python code steps via `exec()`
|
|
||||||
- **providers.py**: Provider abstraction. Calls AI CLI tools via subprocess, reads provider configs from `~/.cmdforge/providers.yaml`
|
- **providers.py**: Provider abstraction. Calls AI CLI tools via subprocess, reads provider configs from `~/.cmdforge/providers.yaml`
|
||||||
|
- **profiles.py**: AI persona profiles with system prompts, stored in `~/.cmdforge/profiles/`
|
||||||
|
- **manifest.py**: Project manifest (`cmdforge.yaml`) for declaring tool dependencies with version constraints
|
||||||
|
- **lockfile.py**: Lock file support for reproducible installs (`cmdforge.lock`)
|
||||||
|
- **registry_client.py**: Client for registry API (search, publish, download, authentication)
|
||||||
- **gui/**: PySide6 desktop GUI
|
- **gui/**: PySide6 desktop GUI
|
||||||
- **main_window.py**: Main application window with sidebar navigation
|
- **main_window.py**: Main application window with sidebar navigation
|
||||||
- **pages/**: Tools page, Tool Builder, Registry browser, Collections page, Providers management
|
- **pages/**: Tools page, Tool Builder, Registry browser, Collections page, Providers management, Profiles
|
||||||
- **dialogs/**: Step editors, Argument editor, Provider dialog, Connect/Publish dialogs
|
- **dialogs/**: Step editors, Argument editor, Provider dialog, Connect/Publish dialogs
|
||||||
|
|
||||||
### Key Paths
|
### Key Paths
|
||||||
|
|
@ -58,8 +76,9 @@ Tools are YAML configs with:
|
||||||
|
|
||||||
### Step Types
|
### Step Types
|
||||||
|
|
||||||
1. **Prompt Step**: Calls AI provider with template, stores result in `output_var`
|
1. **Prompt Step**: Calls AI provider with template, stores result in `output_var`. Supports `profile` for AI personas and `strip_fences` for markdown cleanup
|
||||||
2. **Code Step**: Executes Python code via `exec()`, captures specified variables
|
2. **Code Step**: Executes Python code via `exec()`, captures specified variables (comma-separated for multiple outputs)
|
||||||
|
3. **Tool Step**: Calls another tool (meta-tools), supports `args` dict and `provider` override. Dependencies resolved via `resolver.py`
|
||||||
|
|
||||||
### Variable Flow
|
### Variable Flow
|
||||||
|
|
||||||
|
|
@ -102,8 +121,10 @@ CmdForge includes a web interface and tool registry:
|
||||||
|
|
||||||
### Registry Modules (`src/cmdforge/registry/`)
|
### Registry Modules (`src/cmdforge/registry/`)
|
||||||
|
|
||||||
- **app.py**: Registry API (tool publishing, search, downloads)
|
- **app.py**: Flask-based Registry API (tool publishing, search, downloads, authentication, rate limiting)
|
||||||
- **db.py**: SQLite schema and queries
|
- **db.py**: SQLite schema and queries (`connect_db()`, `query_one()`, `query_all()`)
|
||||||
|
- **embeddings.py**: Semantic search with vector embeddings for tool discovery
|
||||||
|
- **sync.py**: Git-based tool sync from Gitea repository
|
||||||
|
|
||||||
### Key URLs
|
### Key URLs
|
||||||
|
|
||||||
|
|
@ -122,6 +143,19 @@ python -m cmdforge.web.app
|
||||||
CMDFORGE_REGISTRY_DB=/path/to/db PORT=5050 python -m cmdforge.web.app
|
CMDFORGE_REGISTRY_DB=/path/to/db PORT=5050 python -m cmdforge.web.app
|
||||||
```
|
```
|
||||||
|
|
||||||
|
## Testing Conventions
|
||||||
|
|
||||||
|
Tests use `pytest` with common fixtures:
|
||||||
|
- `temp_tools_dir(tmp_path)`: Redirects `TOOLS_DIR` and `BIN_DIR` to temp directory
|
||||||
|
- `temp_providers_file(tmp_path)`: Redirects `providers.yaml` to temp directory
|
||||||
|
|
||||||
|
Mocking strategy:
|
||||||
|
- **File system**: Use `tmp_path` fixture and patch module-level paths
|
||||||
|
- **Subprocess calls**: Mock `subprocess.run` and `shutil.which`
|
||||||
|
- **Provider calls**: Mock `call_provider` to return `ProviderResult`
|
||||||
|
|
||||||
|
Integration tests are marked with `@pytest.mark.integration` and require a running registry server.
|
||||||
|
|
||||||
## Infrastructure Documentation
|
## Infrastructure Documentation
|
||||||
|
|
||||||
For deployment, server details, and operations, see the `docs/` folder:
|
For deployment, server details, and operations, see the `docs/` folder:
|
||||||
|
|
|
||||||
|
|
@ -5,7 +5,8 @@ import ast
|
||||||
from PySide6.QtWidgets import (
|
from PySide6.QtWidgets import (
|
||||||
QDialog, QVBoxLayout, QFormLayout, QLineEdit,
|
QDialog, QVBoxLayout, QFormLayout, QLineEdit,
|
||||||
QComboBox, QPushButton, QHBoxLayout, QLabel,
|
QComboBox, QPushButton, QHBoxLayout, QLabel,
|
||||||
QPlainTextEdit, QSplitter, QGroupBox, QTextEdit, QMessageBox
|
QPlainTextEdit, QSplitter, QGroupBox, QTextEdit, QMessageBox,
|
||||||
|
QCheckBox
|
||||||
)
|
)
|
||||||
from PySide6.QtCore import Qt, QThread, Signal
|
from PySide6.QtCore import Qt, QThread, Signal
|
||||||
|
|
||||||
|
|
@ -68,6 +69,10 @@ class PromptStepDialog(QDialog):
|
||||||
self.output_input.setText("response")
|
self.output_input.setText("response")
|
||||||
form.addRow("Output variable:", self.output_input)
|
form.addRow("Output variable:", self.output_input)
|
||||||
|
|
||||||
|
# Strip fences checkbox
|
||||||
|
self.strip_fences_check = QCheckBox("Strip markdown code fences from output")
|
||||||
|
form.addRow("", self.strip_fences_check)
|
||||||
|
|
||||||
layout.addLayout(form)
|
layout.addLayout(form)
|
||||||
|
|
||||||
# Prompt text
|
# Prompt text
|
||||||
|
|
@ -118,6 +123,7 @@ class PromptStepDialog(QDialog):
|
||||||
|
|
||||||
self.output_input.setText(step.output_var)
|
self.output_input.setText(step.output_var)
|
||||||
self.prompt_input.setPlainText(step.prompt)
|
self.prompt_input.setPlainText(step.prompt)
|
||||||
|
self.strip_fences_check.setChecked(step.strip_fences)
|
||||||
|
|
||||||
def _validate_and_accept(self):
|
def _validate_and_accept(self):
|
||||||
"""Validate and accept."""
|
"""Validate and accept."""
|
||||||
|
|
@ -146,7 +152,8 @@ class PromptStepDialog(QDialog):
|
||||||
provider=self.provider_combo.currentText(),
|
provider=self.provider_combo.currentText(),
|
||||||
output_var=self.output_input.text().strip(),
|
output_var=self.output_input.text().strip(),
|
||||||
profile=profile,
|
profile=profile,
|
||||||
name=name
|
name=name,
|
||||||
|
strip_fences=self.strip_fences_check.isChecked()
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -221,7 +221,13 @@ def execute_prompt_step(
|
||||||
print(f"Error in prompt step: {result.error}", file=sys.stderr)
|
print(f"Error in prompt step: {result.error}", file=sys.stderr)
|
||||||
return "", False
|
return "", False
|
||||||
|
|
||||||
return result.text, True
|
text = result.text
|
||||||
|
if step.strip_fences:
|
||||||
|
import re
|
||||||
|
text = re.sub(r'^```\w*\n', '', text.strip())
|
||||||
|
text = re.sub(r'\n```\s*$', '', text)
|
||||||
|
|
||||||
|
return text, True
|
||||||
|
|
||||||
|
|
||||||
def execute_code_step(
|
def execute_code_step(
|
||||||
|
|
@ -245,11 +251,15 @@ def execute_code_step(
|
||||||
code = substitute_variables(step.code, variables, warn_non_scalar=verbose)
|
code = substitute_variables(step.code, variables, warn_non_scalar=verbose)
|
||||||
|
|
||||||
# Create execution environment with variables
|
# Create execution environment with variables
|
||||||
local_vars = dict(variables)
|
# IMPORTANT: Use the same dict for both globals and locals.
|
||||||
|
# When exec() gets separate dicts, nested functions can't access
|
||||||
|
# module-level imports (they look in globals, not the exec's locals).
|
||||||
|
local_vars = {"__builtins__": __builtins__}
|
||||||
|
local_vars.update(variables)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
# Execute the code with substituted variables
|
# Execute the code with substituted variables
|
||||||
exec(code, {"__builtins__": __builtins__}, local_vars)
|
exec(code, local_vars, local_vars)
|
||||||
|
|
||||||
# Support comma-separated output vars (e.g., "a, b, c")
|
# Support comma-separated output vars (e.g., "a, b, c")
|
||||||
output_vars = [v.strip() for v in step.output_var.split(',')]
|
output_vars = [v.strip() for v in step.output_var.split(',')]
|
||||||
|
|
|
||||||
|
|
@ -100,6 +100,7 @@ class PromptStep:
|
||||||
prompt_file: Optional[str] = None # Optional filename for external prompt
|
prompt_file: Optional[str] = None # Optional filename for external prompt
|
||||||
profile: Optional[str] = None # Optional AI persona profile name
|
profile: Optional[str] = None # Optional AI persona profile name
|
||||||
name: Optional[str] = None # Optional display name for the step
|
name: Optional[str] = None # Optional display name for the step
|
||||||
|
strip_fences: bool = False # Strip markdown code fences from output
|
||||||
|
|
||||||
def to_dict(self) -> dict:
|
def to_dict(self) -> dict:
|
||||||
d = {
|
d = {
|
||||||
|
|
@ -114,6 +115,8 @@ class PromptStep:
|
||||||
d["profile"] = self.profile
|
d["profile"] = self.profile
|
||||||
if self.name:
|
if self.name:
|
||||||
d["name"] = self.name
|
d["name"] = self.name
|
||||||
|
if self.strip_fences:
|
||||||
|
d["strip_fences"] = self.strip_fences
|
||||||
return d
|
return d
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
|
|
@ -124,7 +127,8 @@ class PromptStep:
|
||||||
output_var=data["output_var"],
|
output_var=data["output_var"],
|
||||||
prompt_file=data.get("prompt_file"),
|
prompt_file=data.get("prompt_file"),
|
||||||
profile=data.get("profile"),
|
profile=data.get("profile"),
|
||||||
name=data.get("name")
|
name=data.get("name"),
|
||||||
|
strip_fences=data.get("strip_fences", False)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -296,6 +296,54 @@ class TestExecuteCodeStep:
|
||||||
assert success is True
|
assert success is True
|
||||||
assert outputs2["total"] == 15
|
assert outputs2["total"] == 15
|
||||||
|
|
||||||
|
def test_nested_function_can_access_imports(self):
|
||||||
|
"""Nested functions should be able to access module-level imports.
|
||||||
|
|
||||||
|
This tests a fix for a scoping bug where exec() with separate
|
||||||
|
globals/locals dicts caused nested functions to fail when accessing
|
||||||
|
imported modules. The fix uses the same dict for both globals and
|
||||||
|
locals so imports are visible to nested function closures.
|
||||||
|
"""
|
||||||
|
step = CodeStep(
|
||||||
|
code="""
|
||||||
|
import re
|
||||||
|
|
||||||
|
def parse_numbers(text):
|
||||||
|
# This should be able to access 're' from the outer scope
|
||||||
|
matches = re.findall(r'\\d+', text)
|
||||||
|
return [int(m) for m in matches]
|
||||||
|
|
||||||
|
result = parse_numbers(input)
|
||||||
|
""",
|
||||||
|
output_var="result"
|
||||||
|
)
|
||||||
|
variables = {"input": "abc 123 def 456 ghi"}
|
||||||
|
|
||||||
|
outputs, success = execute_code_step(step, variables)
|
||||||
|
|
||||||
|
assert success is True
|
||||||
|
assert outputs["result"] == [123, 456]
|
||||||
|
|
||||||
|
def test_nested_function_can_access_outer_variables(self):
|
||||||
|
"""Nested functions should access variables defined in outer scope."""
|
||||||
|
step = CodeStep(
|
||||||
|
code="""
|
||||||
|
multiplier = 10
|
||||||
|
|
||||||
|
def multiply_all(numbers):
|
||||||
|
return [n * multiplier for n in numbers]
|
||||||
|
|
||||||
|
result = multiply_all([1, 2, 3])
|
||||||
|
""",
|
||||||
|
output_var="result"
|
||||||
|
)
|
||||||
|
variables = {"input": ""}
|
||||||
|
|
||||||
|
outputs, success = execute_code_step(step, variables)
|
||||||
|
|
||||||
|
assert success is True
|
||||||
|
assert outputs["result"] == [10, 20, 30]
|
||||||
|
|
||||||
|
|
||||||
class TestRunTool:
|
class TestRunTool:
|
||||||
"""Tests for run_tool function."""
|
"""Tests for run_tool function."""
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue