Implement file writing for OpenClaw plugin#5
Conversation
|
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 (5)
WalkthroughThis PR implements file-writing capabilities for the Grok Swarm plugin, enabling the model to extract and persist annotated code blocks directly to disk. The feature spans the documentation, Python bridge, Node.js CLI wrapper, TypeScript plugin layer, and configuration schema, adding optional Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Plugin as Grok Plugin
participant Bridge as Python Bridge
participant Model as LLM Model
participant FS as File System
User->>Plugin: Call with write_files=true
Plugin->>Bridge: Invoke with --write-files flag
Bridge->>Model: Send prompt + context
Model->>Bridge: Return response with annotated code blocks
Bridge->>Bridge: Parse response for lang:path or FILE: markers
rect rgba(100, 150, 200, 0.5)
Bridge->>Bridge: Validate each path (no traversal, within bounds)
Bridge->>FS: Create parent directories
Bridge->>FS: Write UTF-8 content to validated paths
end
Bridge->>Bridge: Generate file-by-file + aggregate summary
Bridge->>User: Print summary to stderr
Note over Bridge,User: If write_files=false, full response prints to stdout
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Explains the priority order: env vars > local config > OpenClaw profiles. Adds warning that env vars override config files. Includes example of how to unset env var to use config file.
There was a problem hiding this comment.
Pull request overview
Adds file-writing capability to the Grok Swarm OpenClaw plugin so Grok can emit annotated code blocks that get written to disk, returning a compact summary instead of a huge response.
Changes:
- Added
write_files/output_dirparameters to the OpenClaw tool schema and forwarded them through the TS plugin to the Python bridge. - Extended the Node bridge wrapper to accept/pass through
--write-filesand--output-dir. - Implemented response parsing + disk writes in
grok_bridge.py, plus README/docs updates and plugin config for a default output directory.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/bridge/grok_bridge.py | Implements --write-files / --output-dir and the core parsing + write-to-disk behavior. |
| src/bridge/index.js | Adds CLI arg parsing and forwards file-writing options to Python. |
| src/plugin/index.ts | Extends tool schema and forwards params; supports defaultOutputDir config fallback. |
| src/plugin/openclaw.plugin.json | Adds defaultOutputDir to plugin config schema. |
| README.md | Documents new file writing feature and updates usage/troubleshooting text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@KHAEntertainment I've opened a new pull request, #6, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: KHAEntertainment <43256680+KHAEntertainment@users.noreply.github.com>
Fix security, correctness, and docs issues in file-writing feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/bridge/grok_bridge.py (2)
190-204: Splitting on triple backticks is clever but has a subtle gotcha.Using
re.split(r'```', response_text)splits the response into alternating text/code segments. Odd-indexed parts are code block contents. However, the loop iterates over all parts without checking indices, so it might accidentally try to parse non-code segments (explanation text) if they happen to start with something that looks likelang:pathor contain// FILE:.In practice, this is unlikely to cause real issues since explanation text rarely starts with
typescript:src/..., but it's a potential edge case worth a quick note.🔧 Safer iteration over only code block contents
# Split into code blocks by ``` fences parts = re.split(r'```', response_text) - for part in parts: + # Odd indices are inside fenced blocks, even indices are outside + for i, part in enumerate(parts): + if i % 2 == 0: + continue # Skip non-code content + # Check for lang:path at start (language tag contains the path) lang_match = lang_path_pattern.match(part)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bridge/grok_bridge.py` around lines 190 - 204, The current split using parts = re.split(r'```', response_text) then iterating over every part can accidentally treat non-code text as code; update the loop to only process fenced code contents (the odd-indexed entries) by iterating with enumerate and skipping even indices (or replace the split approach with a regex that directly captures fenced blocks), then continue using lang_path_pattern, file_marker_pattern and _write_file as before to write detected files; ensure you only call _write_file for parts that are bona fide code block contents.
135-152: Path validation looks solid, but consider chaining the exception for better debugging.The
_safe_destfunction does a good job preventing path traversal attacks — it rejects absolute paths,..components, and verifies the resolved path stays within the output directory (a nice belt-and-suspenders approach).However, per the static analysis hint, when re-raising inside an
exceptclause, it's best practice to chain exceptions usingraise ... from excso the original traceback context is preserved.🔧 Proposed fix for exception chaining
try: dest.relative_to(resolved_root) except ValueError: - raise ValueError(f"Path escapes output directory: {file_path!r}") + raise ValueError(f"Path escapes output directory: {file_path!r}") from None return dest🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bridge/grok_bridge.py` around lines 135 - 152, The except block in _safe_dest currently re-raises a ValueError without chaining, losing original traceback; update the exception handling in the try/except that checks dest.relative_to(resolved_root) to re-raise the new ValueError using exception chaining (i.e., raise ValueError(f"Path escapes output directory: {file_path!r}") from exc) so the original exception context is preserved when relative_to fails; locate this in the _safe_dest function where dest.relative_to(resolved_root) is called and modify the except clause accordingly.src/bridge/index.js (1)
141-147: The default-skip optimization is safe but could be clearer.Line 145 skips passing
--output-dirwhen it matches the hardcoded default'./grok-output/'. This works because the Python bridge has the same default (confirmed in the context snippet). However, this creates a hidden coupling — if someone changes the Python default without updating this file, behavior diverges silently.For CLI-only usage this is fine, and the context snippets confirm that
src/plugin/index.tsbypasses this wrapper entirely when spawning Python directly. So this code path is really just for manual CLI invocations.Consider adding a brief comment to document the coupling:
📝 Add clarifying comment
if (opts.writeFiles) { pyArgs.push('--write-files'); } + // Only pass --output-dir if non-default (Python bridge defaults to same value) if (opts.outputDir && opts.outputDir !== './grok-output/') { pyArgs.push('--output-dir', opts.outputDir); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bridge/index.js` around lines 141 - 147, The code currently omits pushing '--output-dir' when opts.outputDir equals the hardcoded default './grok-output/', which creates a hidden coupling with the Python bridge default; add a short inline comment near the conditional referencing opts.outputDir and pyArgs that explains this intentional optimization and its dependency on the Python-side default (mentioning that CLI invocations bypass this wrapper via src/plugin/index.ts), so future maintainers know why the check exists and to update both sides if the default changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/bridge/grok_bridge.py`:
- Around line 190-204: The current split using parts = re.split(r'```',
response_text) then iterating over every part can accidentally treat non-code
text as code; update the loop to only process fenced code contents (the
odd-indexed entries) by iterating with enumerate and skipping even indices (or
replace the split approach with a regex that directly captures fenced blocks),
then continue using lang_path_pattern, file_marker_pattern and _write_file as
before to write detected files; ensure you only call _write_file for parts that
are bona fide code block contents.
- Around line 135-152: The except block in _safe_dest currently re-raises a
ValueError without chaining, losing original traceback; update the exception
handling in the try/except that checks dest.relative_to(resolved_root) to
re-raise the new ValueError using exception chaining (i.e., raise
ValueError(f"Path escapes output directory: {file_path!r}") from exc) so the
original exception context is preserved when relative_to fails; locate this in
the _safe_dest function where dest.relative_to(resolved_root) is called and
modify the except clause accordingly.
In `@src/bridge/index.js`:
- Around line 141-147: The code currently omits pushing '--output-dir' when
opts.outputDir equals the hardcoded default './grok-output/', which creates a
hidden coupling with the Python bridge default; add a short inline comment near
the conditional referencing opts.outputDir and pyArgs that explains this
intentional optimization and its dependency on the Python-side default
(mentioning that CLI invocations bypass this wrapper via src/plugin/index.ts),
so future maintainers know why the check exists and to update both sides if the
default changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 51d77e43-3840-445f-8af5-b07174cc1415
📒 Files selected for processing (5)
README.mdsrc/bridge/grok_bridge.pysrc/bridge/index.jssrc/plugin/index.tssrc/plugin/openclaw.plugin.json
|
✅ Merge conflicts resolved successfully! Resolved 1 conflict file(s). Commit: 5 file operation(s)
View agent analysis |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@KHAEntertainment I've opened a new pull request, #7, to work on those changes. Once the pull request is ready, I'll request review from you. |
Resolved conflicts in: - README.md (content) Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Fix byte counting, no-files-found output, and stdout/stderr mixing in grok_bridge.py
Summary
Implements Traycer's recommendation from issue #2 to add file writing capability to the OpenClaw Grok Swarm plugin.
Changes
Files Modified
Features
Supported File Path Patterns
Usage
OpenClaw agent calls:
const result = await tools.grok_swarm({
prompt: Refactor the auth module,
mode: refactor,
files: [src/auth/*.ts],
write_files: true,
output_dir: ./grok-output/
});
Result: Orchestrator receives compact summary, not 350K token flood.
Closes
Fixes #2
Summary by CodeRabbit
New Features
write_filestoggle (default false) andoutput_dirconfiguration option (default./grok-output/)Documentation