Conversation
Improves the accuracy of unused variable detection in two key scenarios. First, the analyzer now correctly tracks variable usage within the initializers of nested variable declarations, preventing false positives. Second, custom counter variables in count loops are now always marked as used, resolving cases where they were incorrectly flagged if not referenced within the loop body. Adds comprehensive tests to cover these fixes and prevent regressions.
Replaces the `Start-Process` and `WaitForExit` logic with the `Start-Job` cmdlet for executing test programs. This change provides a more robust and idiomatic PowerShell method for handling process timeouts and ensures cleaner termination of hanging tests.
Replaces `Box` with `Rc` for parent scope references in the semantic analyzer. This change addresses a performance bottleneck where creating nested scopes required expensive, recursive clones, leading to quadratic complexity in deeply nested code. Using reference counting allows child scopes to share an immutable reference to their parent, making the creation of new scopes a cheap, constant-time operation. This significantly improves analysis speed for complex control flow structures. Adds comprehensive correctness and performance test suites to validate the new implementation and prevent regressions.
Replaces the `catch` keyword with the more descriptive `when error`. This also standardizes the caught exception variable from `error_message` to `error`, improving consistency.
Applies standard formatting to Rust source files. This change enhances code consistency and readability without altering any underlying logic.
Refactors the crypto module to implement a more robust and performant stateful design for the WflHash algorithm. This change introduces a proper sponge construction using an internal buffer, allowing for the correct and efficient processing of streaming data. The core permutation function is heavily optimized by removing `black_box` calls, and its mixing step is corrected to ensure invertibility. Additionally, rotation constants are improved for better diffusion. Padding and finalization logic is now integrated directly into the state machine, simplifying the hashing process.
(re)Implements a 100MB input size limit on hashing functions to prevent resource exhaustion from overly large inputs. Exposes a new, more direct function for hashing raw binary data, bypassing the standard value type system.
Updates subprocess and interpreter tests to correctly execute shell commands on Windows by using `cmd /C`. Consolidates the logic for locating the test binary into a single helper function across multiple test files. This refactoring eliminates code duplication and improves maintainability. The new helper also checks for the debug binary, making it easier to run tests during development.
Introduces a new native function, 'input', to read a line of text from standard input. This enables scripts to receive data from the user at runtime and can optionally display a prompt. The function's type signature is registered with the static analyzer for type safety. An example script is also added to demonstrate its usage.
Prints a warning to standard error when any of the custom `wfl` hash or MAC functions are used. This ensures users are aware that these functions are experimental, provide no security guarantees, and should not be used in security-sensitive contexts.
Introduces a frozen, immutable specification for the WFLHASH1 cryptographic algorithm. This document incorporates critical corrections to the algorithm's logic, including MAC key parameter mixing, salt zero-padding, and standard HKDF usage. It provides a definitive reference for implementation and includes a comprehensive JSON test suite with 20 valid test vectors. Additionally, updates local development settings to support the WFL language server.
Introduces a Model Context Protocol (MCP) server mode to the WFL language server, enabling AI assistants like Claude to analyze and interact with WFL codebases. The language server can now be launched with a `--mcp` flag to expose a suite of tools for code analysis (parsing, diagnostics, completions) and resources for workspace-level context (files, symbols, configuration). The server architecture is refactored to use a shared core for analysis logic, ensuring consistency between the existing LSP and the new MCP modes. Adds extensive documentation, including user guides, API references, architecture details, and example clients to support the new feature.
Adds a comprehensive test report for the WFL MCP server implementation. The report validates the functionality of all tools, resources, and error handling mechanisms against a real workspace. It confirms that the server is feature-complete, backward compatible with the existing LSP, and production-ready.
|
Claude finished @logbie's task —— View job Code Review for MCP Server Integration PRI've completed a comprehensive review of this substantial PR that adds Model Context Protocol (MCP) server support to WFL. Here's my detailed analysis: Review Tasks:
🔍 Overall AssessmentThis is a high-quality, well-engineered PR that adds significant value to the WFL ecosystem. The code demonstrates excellent security practices, comprehensive testing, and thorough documentation. ✅ Strengths1. MCP Server Implementation (
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a Model Context Protocol (MCP) JSON-RPC server and shared analysis core, refactors analyzer scopes to use Rc, adds function-call parsing, introduces an Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCP as "WFL MCP Server\n(wfl-lsp --mcp)"
participant Core as "WflLanguageCore\n(Parser/Analyzer/TypeChecker)"
participant FS as "FileSystem"
Client->>MCP: initialize (JSON-RPC)
MCP-->>Client: initialize response (capabilities)
Client->>MCP: tools/list
MCP-->>Client: list of tools
Client->>MCP: parse_wfl {source}
MCP->>Core: lex & parse source
Core-->>MCP: AST / parse result
MCP-->>Client: parse result
Client->>MCP: analyze_wfl {source}
MCP->>Core: analyze & typecheck
Core-->>MCP: diagnostics
MCP-->>Client: diagnostics
Client->>MCP: resources/list
MCP->>FS: enumerate workspace files
FS-->>MCP: file list
MCP-->>Client: resources list
sequenceDiagram
participant Parser
participant Primary as "Primary Expr Parser"
participant Args as "Argument List Parser"
participant AST
Parser->>Primary: parse_primary_expression()
Primary->>Primary: parse base expression
alt next token is LeftParen
Primary->>Args: consume '('
Args->>Args: parse zero-or-more expressions separated by ','
Args->>Args: require ')'
Args-->>Primary: Vec<args>
Primary->>AST: emit FunctionCall(base, args) with base position
Primary->>Primary: continue postfix loop with FunctionCall as base
else other postfix (., [)
Primary->>Primary: handle property/indexing
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{rs,toml}📄 CodeRabbit inference engine (AGENTS.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (4)
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 |
There was a problem hiding this comment.
Pull request overview
This PR introduces a Model Context Protocol (MCP) server to enable AI assistants like Claude to interact with WFL codebases, alongside several performance improvements and language updates.
Key changes:
- Adds MCP server implementation with 6 tools and 5 workspace resources for AI-powered development assistance
- Optimizes analyzer performance by switching to reference-counted scopes (Rc)
- Overhauls crypto library with secure buffered sponge construction and formal specification
Reviewed changes
Copilot reviewed 41 out of 42 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| wflhash/wflhashspec.md | Adds frozen specification for WFLHASH1 algorithm with test vectors |
| wfl-lsp/src/mcp_server.rs | Implements MCP server with JSON-RPC 2.0 tools and resources |
| wfl-lsp/src/main.rs | Adds --mcp flag routing to enable MCP server mode |
| wfl-lsp/src/lib.rs | Refactors to use shared WflLanguageCore |
| wfl-lsp/src/core.rs | Extracts shared analysis logic for LSP and MCP |
| src/stdlib/typechecker.rs | Registers new input() function type signature |
| src/stdlib/crypto.rs | Rewrites wflhash with buffered sponge construction |
| src/stdlib/core.rs | Implements input() function for console input |
| src/builtins.rs | Adds input to builtin function list |
| src/analyzer/static_analyzer.rs | Fixes unused variable detection for nested contexts |
| src/analyzer/mod.rs | Changes scope parent to Rc for performance |
| tests/*.rs | Updates test infrastructure for cross-platform compatibility |
| Docs/guides/*.md | Adds comprehensive MCP documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| /// Run the MCP server on stdin/stdout | ||
| pub async fn run_server() -> Result<(), Box<dyn std::error::Error>> { |
There was a problem hiding this comment.
The MCP server entry point function lacks test coverage. Consider adding integration tests that spawn the server and verify the complete request/response flow, similar to the unit tests for individual handlers.
Improves code readability and maintainability by applying consistent formatting across the codebase. This mainly involves breaking up long lines in function calls, macros, and declarations.
![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.
ℹ️ 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".
| "print" => 1, | ||
| "typeof" | "type_of" => 1, | ||
| "isnothing" | "is_nothing" => 1, | ||
| "input" => 1, |
There was a problem hiding this comment.
native_input was added to read user input and explicitly allows zero or one argument, but get_function_arity still reports that input requires exactly one parameter. The type checker uses this arity when inferring builtin function types, so a call like input() (no prompt) now fails type checking even though the runtime implementation supports it. This makes the new builtin unusable without a dummy argument.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/interpreter/mod.rs (3)
6115-6128: Cross‑platform cmd/args selection looks good; fix rustfmt failureThe conditional
(cmd, args)setup for Windows vs non‑Windows and passinguse_shell = falseexercises the safe argument‑based path throughIoClient::execute_command, which aligns with the subprocess‑sanitization and Tokio async guidelines. The CI failure fromcargo fmt --checkpoints at this region, so please runcargo fmt --allto normalize attribute placement/indentation before merging.As per coding guidelines, this should be rustfmt‑clean.
6209-6221: Update comment to match new arg‑based execution intest_capture_process_outputThe comment still refers to “no args = shell execution”, but this test now passes explicit
cmdandargs, which goes through the non‑shell code path inspawn_process. Suggest updating the comment to avoid confusion with the interpreter’suse_shell/sanitizer behavior.Proposed comment tweak
- // Use shell command that works cross-platform (no args = shell execution) + // Use a simple echo command that works cross-platform via explicit cmd/args
6239-6251: Keeptest_wait_for_process_completioncomment consistent with arg‑based spawnSame as above: this test now uses explicit
cmdandargswithuse_shell = false, so the existing “no args = shell execution” wording is stale. Aligning the comment with the actual behavior will make the intent clearer to future maintainers.Proposed comment tweak
- // Use shell command that works cross-platform (no args = shell execution) + // Use a simple echo command that works cross-platform via explicit cmd/argstests/modulo_operator_test.rs (1)
159-159: Update to new error handling syntax.Line 159 uses the deprecated
catch:syntax instead of the newwhen error:syntax. Per the PR objectives, the language has replacedcatch:withwhen error:to standardize the exception variable. While this is test code, maintaining consistency with the language update improves test maintainability.🔎 Proposed fix
try: store r as 5 % 0 change result to "FAIL: No error raised" -catch: +when error: change result to "PASS" end trytests/subprocess_security_test.rs (2)
128-144: Fix formatting violations flagged by pipeline.The pipeline reports
cargo fmt --checkfailures on the assertion at line 144 (reported as line 141 pre-change). The assertion message is too long and needs formatting.🔎 Apply cargo fmt to fix
Run the following command to automatically fix formatting:
cargo fmt --allOr manually format line 144 to comply with rustfmt rules (likely needs line breaking for the long assertion message).
As per coding guidelines: Format Rust code using
cargo fmt --all.
157-172: Fix formatting violations flagged by pipeline.The pipeline reports
cargo fmt --checkfailures on the assertion at line 172 (reported as line 169 pre-change). The assertion message is too long and needs formatting.🔎 Apply cargo fmt to fix
Run the following command to automatically fix formatting:
cargo fmt --allOr manually format line 172 to comply with rustfmt rules (likely needs line breaking for the long assertion message).
As per coding guidelines: Format Rust code using
cargo fmt --all.
🧹 Nitpick comments (14)
scripts/run_integration_tests.ps1 (1)
139-145: Consider displaying test output on failure for easier debugging.The job captures test output (line 141) but never displays it. When a test fails, no diagnostic information is shown, making it harder to debug failures. Consider returning and displaying the output for failed tests.
🔎 Optional enhancement to display output on failure
Modify the script block to return both exit code and output:
# Run with timeout using background job $job = Start-Job -ScriptBlock { param($binaryPath, $testFile) $output = & $binaryPath $testFile 2>&1 $exitCode = $LASTEXITCODE - # Return only the exit code as a structured object - return @{ ExitCode = $exitCode } + # Return exit code and output + return @{ ExitCode = $exitCode; Output = ($output | Out-String) } } -ArgumentList (Resolve-Path $BinaryPath).Path, $wflFile.FullNameThen display output on failure:
if ($exitCode -eq 0) { Write-Host "[SUCCESS] PASS $($wflFile.Name)" -ForegroundColor Green } else { Write-Host "[ERROR] FAIL $($wflFile.Name) (exit code: $exitCode)" -ForegroundColor Red + if ($result.Output) { + Write-Host " Output:" -ForegroundColor Gray + Write-Host $result.Output -ForegroundColor Gray + } $failedPrograms++ }wfl-lsp/examples/README.md (2)
79-111: Consider adding resource cleanup and error handling to examples.The Python example spawns a subprocess but doesn't demonstrate proper cleanup (process termination) or error handling. While the note on line 203 mentions these concerns, the examples should at least hint at production patterns.
🔎 Enhanced Python example with cleanup
import subprocess import json # Start MCP server proc = subprocess.Popen( ['wfl-lsp', '--mcp'], stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True ) try: # Send request request = { "jsonrpc": "2.0", "id": 1, "method": "tools/call", "params": { "name": "parse_wfl", "arguments": { "source": "store x as 5" } } } proc.stdin.write(json.dumps(request) + '\n') proc.stdin.flush() # Read response response = json.loads(proc.stdout.readline()) print(response) finally: # Clean up proc.terminate() proc.wait(timeout=5)
113-140: Consider adding error handling to Node.js example.Similar to the Python example, the Node.js code doesn't show cleanup or error handling patterns.
🔎 Enhanced Node.js example
const { spawn } = require('child_process'); const server = spawn('wfl-lsp', ['--mcp']); // Handle errors server.on('error', (err) => { console.error('Failed to start server:', err); }); // Send request const request = { jsonrpc: '2.0', id: 1, method: 'tools/call', params: { name: 'parse_wfl', arguments: { source: 'store x as 5' } } }; server.stdin.write(JSON.stringify(request) + '\n'); // Read response server.stdout.on('data', (data) => { const response = JSON.parse(data.toString()); console.log(response); // Clean up after receiving response server.kill(); }); // Timeout safety setTimeout(() => { server.kill(); }, 5000);generate_hash.wfl (1)
5-6: Minor: Fix comment numbering.Both comment lines are numbered "1." Consider using sequential numbering or removing the first comment since "Get user input" more accurately describes the following code section.
🔎 Proposed fix
-// 1. Generate the hash // 1. Get user inputOr use sequential numbering:
-// 1. Generate the hash -// 1. Get user input +// 1. Get user inputREADME.md (1)
250-290: Consider: Potential content duplication in README.The AI-generated summary indicates that the MCP section content appears in two places within the README, which could create maintenance burden. Consider consolidating into a single section or using references to avoid duplication.
Based on coding guidelines requiring README updates with significant changes.
Docs/technical/wfl-mcp-architecture.md (2)
30-61: Add language specifier to fenced code block.The ASCII architecture diagram should specify a language (e.g.,
textorplaintext) to satisfy markdownlint MD040 and improve accessibility for screen readers.🔎 Proposed fix
-``` +```text ┌──────────────────────────────────────────────────────────────┐ │ wfl-lsp Binary │
309-319: Add language specifier to directory structure code block.Similar to the ASCII diagram, this directory structure block should specify a language to satisfy MD040.
🔎 Proposed fix
-``` +```text wfl-lsp/ ├── src/ │ ├── main.rs # Entry point, mode selection (50 lines)src/analyzer/mod.rs (1)
500-505: Defensive fallback to empty scope is unusual but safe.The
Scope::new()fallback whenthen_scope.parentisNoneshould be unreachable given the code flow (scope is created with a parent on line 491). Consider adding a debug assertion or comment clarifying this is a safety fallback.🔎 Suggested clarification
let outer_scope = if let Some(parent_rc) = &then_scope.parent { Rc::try_unwrap(parent_rc.clone()).unwrap_or_else(|rc| (*rc).clone()) } else { - Scope::new() // Shouldn't happen, but provide fallback + // Unreachable: scope created with parent on line 491 + debug_assert!(false, "then_scope should always have a parent"); + Scope::new() };wfl-lsp/src/core.rs (1)
167-190: Hardcoded URI in related diagnostic information.The URI
file:///document.wflis hardcoded for all related diagnostic notes. While functional, this could be confusing when analyzing files with different names. Consider passing the actual document URI or using a placeholder that indicates it's a synthetic location.wfl-lsp/examples/test_mcp_server.sh (1)
14-22: Test helper suppresses stderr which may hide diagnostics.The
2>/dev/nullredirect suppresses all stderr output including MCP server startup messages and potential error information. Consider using a flag or environment variable to optionally show stderr for debugging.🔎 Suggested enhancement
# Function to send JSON-RPC request and pretty-print response send_request() { local name=$1 local request=$2 echo -e "${BLUE}[Test] $name${NC}" - echo "$request" | wfl-lsp --mcp 2>/dev/null + if [ "${DEBUG:-}" = "1" ]; then + echo "$request" | wfl-lsp --mcp + else + echo "$request" | wfl-lsp --mcp 2>/dev/null + fi echo "" }Usage:
DEBUG=1 ./test_mcp_server.shto see stderr output.Docs/guides/wfl-mcp-guide.md (1)
258-265: Add language specifier to fenced code block.The code block showing workspace config output should have a language specifier for proper syntax highlighting.
🔎 Proposed fix
-**Returns:** -``` -timeout_seconds = 60 -logging_enabled = false -debug_report_enabled = true -log_level = info -``` +**Returns:** +```ini +timeout_seconds = 60 +logging_enabled = false +debug_report_enabled = true +log_level = info +```Based on coding guidelines, this aligns with markdown best practices.
wfl-lsp/examples/simple_mcp_client.rs (1)
8-17: Consider adding child process cleanup.The spawned
wfl-lspprocess is not explicitly terminated or waited for. While the OS will clean it up when the parent process exits, it's better practice to explicitly manage the child process lifecycle.🔎 Suggested enhancement
Add cleanup at the end of main:
println!("========================================"); println!("All examples completed successfully!"); println!("========================================"); // Clean up child process drop(stdin); drop(reader); let _ = child.wait(); Ok(())src/stdlib/crypto.rs (2)
380-380: Consider a less noisy warning mechanism.Emitting a warning to stderr on every crypto function call will flood logs and may mask other important messages. Consider alternatives:
- Warn once per process using a static flag
- Use the
logcrate withwarn!level so users can control verbosity- Document the experimental status prominently instead
🔎 Proposed refactor: Warn once using std::sync::Once
+use std::sync::Once; + +static CRYPTO_WARNING: Once = Once::new(); + +fn warn_experimental_crypto() { + CRYPTO_WARNING.call_once(|| { + eprintln!( + "WARNING: WFL crypto functions are experimental \ + and provide no security guarantees. USE AT OWN RISK." + ); + }); +} + pub fn native_wflhash256(args: Vec<Value>) -> Result<Value, RuntimeError> { - eprintln!("WARNING: WFL crypto functions are experimental and provide no security guarantees. USE AT OWN RISK."); + warn_experimental_crypto(); // ... rest unchanged }Also applies to: 405-405, 424-424, 447-447
398-402: Binary hashing function doesn't emit security warning.
native_wflhash256_binaryis the only function that doesn't emit the experimental warning. For consistency, consider adding the warning here as well, or documenting why it's exempt (e.g., internal use only).🔎 Proposed fix: Add warning for consistency
pub fn native_wflhash256_binary(input: &[u8]) -> Result<String, RuntimeError> { + warn_experimental_crypto(); // If using the Once-based helper let params = WflHashParams::new(32); let hash = wflhash_core(input, ¶ms)?; Ok(bytes_to_hex(&hash)) }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (41)
.claude/settings.local.jsonCLAUDE.mdDocs/guides/claude-desktop-integration.mdDocs/guides/wfl-mcp-api-reference.mdDocs/guides/wfl-mcp-guide.mdDocs/technical/wfl-mcp-architecture.mdREADME.mdTestPrograms/complex_expression_catch_test.wflTestPrograms/unicode_catch_test.wflgenerate_hash.wflhash_output.txtscripts/run_integration_tests.ps1src/analyzer/mod.rssrc/analyzer/static_analyzer.rssrc/builtins.rssrc/interpreter/mod.rssrc/main.rssrc/stdlib/core.rssrc/stdlib/crypto.rssrc/stdlib/typechecker.rstests/analyzer_scope_correctness_test.rstests/analyzer_scope_performance_test.rstests/file_io_windows_sync_errors_test.rstests/modulo_operator_test.rstests/split_functionality.rstests/string_escape_sequences.rstests/subprocess_cleanup_test.rstests/subprocess_security_test.rstests/subprocess_test.rstests/zero_arg_action_error_propagation_test.rswfl-lsp/Cargo.tomlwfl-lsp/MCP_TEST_REPORT.mdwfl-lsp/examples/README.mdwfl-lsp/examples/simple_mcp_client.rswfl-lsp/examples/test_mcp_server.ps1wfl-lsp/examples/test_mcp_server.shwfl-lsp/src/core.rswfl-lsp/src/lib.rswfl-lsp/src/main.rswfl-lsp/src/mcp_server.rswflhash/wflhashspec.md
🧰 Additional context used
📓 Path-based instructions (15)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingcargo fmt --all(see.rustfmt.toml)
Lint clean: runcargo clippy --all-targets --all-features -- -D warningswith no warnings
Usesnake_casefor function and file names in Rust
UseCamelCasefor types and traits in Rust
UseSCREAMING_SNAKE_CASEfor constants in Rust
ReviewSECURITY.md; avoid logging secrets and prefer zeroization for sensitive data in Rust code
**/*.rs: Use Rust Edition 2024 for all Rust code in this project
Format all Rust code with 'cargo fmt --all'
Run 'cargo clippy --all-targets --all-features -- -D warnings' to enforce linting rules
Files:
src/stdlib/typechecker.rstests/string_escape_sequences.rssrc/main.rstests/subprocess_cleanup_test.rstests/subprocess_test.rssrc/builtins.rstests/file_io_windows_sync_errors_test.rstests/modulo_operator_test.rssrc/stdlib/core.rstests/subprocess_security_test.rstests/analyzer_scope_correctness_test.rstests/split_functionality.rswfl-lsp/src/lib.rstests/analyzer_scope_performance_test.rswfl-lsp/src/main.rssrc/analyzer/static_analyzer.rswfl-lsp/examples/simple_mcp_client.rssrc/stdlib/crypto.rssrc/interpreter/mod.rstests/zero_arg_action_error_propagation_test.rssrc/analyzer/mod.rswfl-lsp/src/core.rswfl-lsp/src/mcp_server.rs
src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Component documentation required for all major modules
Files:
src/stdlib/typechecker.rssrc/main.rssrc/builtins.rssrc/stdlib/core.rssrc/analyzer/static_analyzer.rssrc/stdlib/crypto.rssrc/interpreter/mod.rssrc/analyzer/mod.rs
src/stdlib/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use WFLHASH custom cryptographic hash function for memory and cryptographic operations
Files:
src/stdlib/typechecker.rssrc/stdlib/core.rssrc/stdlib/crypto.rs
**/*.wfl
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.wfl: Natural language syntax in WFL: use 'store name as "value"' and 'check if x is greater than 5'
Use static typing with intelligent type inference in WFL programs
Use comprehensive error handling with try/when/otherwise blocks in WFL
WFL code must pass linting with 'wfl --lint program.wfl'
Files:
generate_hash.wflTestPrograms/unicode_catch_test.wflTestPrograms/complex_expression_catch_test.wfl
Docs/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Keep documentation current in
Docs/directory and update relevant indexes when adding features; major changes warrant a Dev Diary note
Files:
Docs/guides/claude-desktop-integration.mdDocs/technical/wfl-mcp-architecture.mdDocs/guides/wfl-mcp-api-reference.mdDocs/guides/wfl-mcp-guide.md
Docs/**
📄 CodeRabbit inference engine (CLAUDE.md)
Update documentation in Docs/ folder with significant changes
Files:
Docs/guides/claude-desktop-integration.mdDocs/technical/wfl-mcp-architecture.mdDocs/guides/wfl-mcp-api-reference.mdDocs/guides/wfl-mcp-guide.md
**/tests/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Integration tests require
cargo build --releaseand must use the provided scripts (run_integration_tests.ps1|.sh)
Files:
tests/string_escape_sequences.rstests/subprocess_cleanup_test.rstests/subprocess_test.rstests/file_io_windows_sync_errors_test.rstests/modulo_operator_test.rstests/subprocess_security_test.rstests/analyzer_scope_correctness_test.rstests/split_functionality.rstests/analyzer_scope_performance_test.rstests/zero_arg_action_error_propagation_test.rs
**/{tests,TestPrograms}/**
📄 CodeRabbit inference engine (CLAUDE.md)
**/{tests,TestPrograms}/**: Write failing tests FIRST for any feature or bug fix before implementing the solution
Never modify tests to make them pass - fix the implementation instead
Files:
tests/string_escape_sequences.rstests/subprocess_cleanup_test.rstests/subprocess_test.rstests/file_io_windows_sync_errors_test.rstests/modulo_operator_test.rstests/subprocess_security_test.rstests/analyzer_scope_correctness_test.rsTestPrograms/unicode_catch_test.wfltests/split_functionality.rstests/analyzer_scope_performance_test.rsTestPrograms/complex_expression_catch_test.wfltests/zero_arg_action_error_propagation_test.rs
tests/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.rs: Always run 'cargo build --release' before running integration tests
Integration tests in Rust must use the release binary (target/release/wfl.exe on Windows, target/release/wfl on Unix)
Files:
tests/string_escape_sequences.rstests/subprocess_cleanup_test.rstests/subprocess_test.rstests/file_io_windows_sync_errors_test.rstests/modulo_operator_test.rstests/subprocess_security_test.rstests/analyzer_scope_correctness_test.rstests/split_functionality.rstests/analyzer_scope_performance_test.rstests/zero_arg_action_error_propagation_test.rs
**/tests/**/*_test.rs
📄 CodeRabbit inference engine (AGENTS.md)
Write failing tests first (TDD approach); feature-oriented test names (e.g.,
*_test.rs)
Files:
tests/subprocess_cleanup_test.rstests/subprocess_test.rstests/file_io_windows_sync_errors_test.rstests/modulo_operator_test.rstests/subprocess_security_test.rstests/analyzer_scope_correctness_test.rstests/analyzer_scope_performance_test.rstests/zero_arg_action_error_propagation_test.rs
TestPrograms/*.wfl
📄 CodeRabbit inference engine (CLAUDE.md)
All TestPrograms/*.wfl files MUST pass after any change
Files:
TestPrograms/unicode_catch_test.wflTestPrograms/complex_expression_catch_test.wfl
wfl-lsp/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Implement LSP server support in wfl-lsp workspace member using tower-lsp
Files:
wfl-lsp/src/lib.rswfl-lsp/src/main.rswfl-lsp/examples/simple_mcp_client.rswfl-lsp/src/core.rswfl-lsp/src/mcp_server.rs
src/interpreter/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
src/interpreter/**/*.rs: Interpreter must be async-capable using Tokio runtime
Interpreter subprocess handling must include security sanitization for commands
Files:
src/interpreter/mod.rs
{README.md,readme.md}
📄 CodeRabbit inference engine (.cursor/rules/wfl-rules.mdc)
Make sure we update readme.md with any new information
Files:
README.md
README.md
📄 CodeRabbit inference engine (CLAUDE.md)
Update README.md with significant changes to the project
Files:
README.md
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.961Z
Learning: Applies to wfl-lsp/**/*.rs : Implement LSP server support in wfl-lsp workspace member using tower-lsp
📚 Learning: 2025-12-08T16:25:57.961Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.961Z
Learning: Applies to Cargo.toml : Configure workspace with Cargo and support workspace members (wfl-lsp, vscode-extension)
Applied to files:
wfl-lsp/Cargo.toml
📚 Learning: 2025-12-01T18:28:13.642Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T18:28:13.642Z
Learning: LSP crate development: build with `cargo build -p wfl-lsp` and run with `cargo run -p wfl-lsp`
Applied to files:
wfl-lsp/Cargo.tomltests/string_escape_sequences.rstests/subprocess_cleanup_test.rstests/subprocess_security_test.rstests/split_functionality.rswfl-lsp/src/lib.rsCLAUDE.mdwfl-lsp/src/main.rswfl-lsp/examples/simple_mcp_client.rstests/zero_arg_action_error_propagation_test.rswfl-lsp/src/core.rs
📚 Learning: 2025-12-01T18:28:13.642Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T18:28:13.642Z
Learning: Ensure LSP features integration by building release binary: `cargo build --release` provides `target/release/wfl`
Applied to files:
wfl-lsp/Cargo.tomltests/string_escape_sequences.rstests/subprocess_cleanup_test.rstests/file_io_windows_sync_errors_test.rstests/modulo_operator_test.rstests/subprocess_security_test.rstests/split_functionality.rsCLAUDE.mdwfl-lsp/src/main.rswfl-lsp/examples/simple_mcp_client.rstests/zero_arg_action_error_propagation_test.rs
📚 Learning: 2025-12-08T16:25:57.961Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.961Z
Learning: Use Rust 1.75+ as minimum version, currently developing with 1.91.1+
Applied to files:
wfl-lsp/Cargo.toml
📚 Learning: 2025-12-08T16:25:57.961Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.961Z
Learning: Applies to README.md : Update README.md with significant changes to the project
Applied to files:
wfl-lsp/examples/README.mdREADME.md
📚 Learning: 2025-12-08T16:25:57.961Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.961Z
Learning: Applies to wfl-lsp/**/*.rs : Implement LSP server support in wfl-lsp workspace member using tower-lsp
Applied to files:
wfl-lsp/examples/README.mdDocs/technical/wfl-mcp-architecture.mdwfl-lsp/src/lib.rsCLAUDE.mdwfl-lsp/src/main.rsDocs/guides/wfl-mcp-api-reference.mdwfl-lsp/examples/simple_mcp_client.rsDocs/guides/wfl-mcp-guide.mdwfl-lsp/src/core.rswfl-lsp/src/mcp_server.rs
📚 Learning: 2025-12-08T16:25:57.961Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.961Z
Learning: Applies to src/stdlib/**/*.rs : Use WFLHASH custom cryptographic hash function for memory and cryptographic operations
Applied to files:
generate_hash.wfltests/string_escape_sequences.rstests/subprocess_cleanup_test.rstests/file_io_windows_sync_errors_test.rstests/modulo_operator_test.rstests/subprocess_security_test.rstests/split_functionality.rswflhash/wflhashspec.mdsrc/stdlib/crypto.rstests/zero_arg_action_error_propagation_test.rs
📚 Learning: 2025-12-08T16:25:57.961Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.961Z
Learning: Applies to tests/**/*.rs : Integration tests in Rust must use the release binary (target/release/wfl.exe on Windows, target/release/wfl on Unix)
Applied to files:
tests/string_escape_sequences.rstests/subprocess_cleanup_test.rstests/subprocess_test.rstests/file_io_windows_sync_errors_test.rstests/modulo_operator_test.rstests/subprocess_security_test.rsscripts/run_integration_tests.ps1tests/split_functionality.rssrc/stdlib/crypto.rssrc/interpreter/mod.rstests/zero_arg_action_error_propagation_test.rs
📚 Learning: 2025-08-12T17:35:32.346Z
Learnt from: logbie
Repo: WebFirstLanguage/wfl PR: 150
File: tests/file_io_modes_test.rs:0-0
Timestamp: 2025-08-12T17:35:32.346Z
Learning: In the WFL project tests, file_io_execution_test.rs contains tests that actually execute WFL code through the interpreter and create real files, while file_io_modes_test.rs contains parser-only tests that don't execute code or create files during parsing.
Applied to files:
tests/string_escape_sequences.rstests/subprocess_cleanup_test.rstests/subprocess_test.rstests/file_io_windows_sync_errors_test.rstests/modulo_operator_test.rstests/subprocess_security_test.rstests/split_functionality.rstests/analyzer_scope_performance_test.rssrc/stdlib/crypto.rssrc/interpreter/mod.rstests/zero_arg_action_error_propagation_test.rswfl-lsp/src/core.rswfl-lsp/src/mcp_server.rs
📚 Learning: 2025-12-01T18:28:13.642Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T18:28:13.642Z
Learning: Do not break existing WFL programs; ensure backward compatibility and run all `TestPrograms/` before committing
Applied to files:
tests/subprocess_cleanup_test.rstests/split_functionality.rsTestPrograms/complex_expression_catch_test.wfl
📚 Learning: 2025-12-01T18:28:13.642Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T18:28:13.642Z
Learning: Applies to **/tests/**/*.rs : Integration tests require `cargo build --release` and must use the provided scripts (`run_integration_tests.ps1|.sh`)
Applied to files:
tests/subprocess_test.rstests/file_io_windows_sync_errors_test.rstests/modulo_operator_test.rsscripts/run_integration_tests.ps1src/interpreter/mod.rstests/zero_arg_action_error_propagation_test.rs
📚 Learning: 2025-12-08T16:25:57.961Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.961Z
Learning: Applies to src/interpreter/**/*.rs : Interpreter must be async-capable using Tokio runtime
Applied to files:
tests/subprocess_test.rssrc/interpreter/mod.rs
📚 Learning: 2025-12-08T16:25:57.961Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.961Z
Learning: Applies to tests/**/*.rs : Always run 'cargo build --release' before running integration tests
Applied to files:
tests/modulo_operator_test.rstests/zero_arg_action_error_propagation_test.rs
📚 Learning: 2025-12-08T16:25:57.961Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.961Z
Learning: Applies to TestPrograms/*.wfl : All TestPrograms/*.wfl files MUST pass after any change
Applied to files:
scripts/run_integration_tests.ps1TestPrograms/unicode_catch_test.wflTestPrograms/complex_expression_catch_test.wfl
📚 Learning: 2025-08-11T05:10:43.202Z
Learnt from: logbie
Repo: WebFirstLanguage/wfl PR: 137
File: TestPrograms/test.wfl:1-2
Timestamp: 2025-08-11T05:10:43.202Z
Learning: Some test scripts in TestPrograms/ are deliberately designed to trigger errors as negative test cases to validate error handling and detection mechanisms. The file TestPrograms/test.wfl specifically tests variable redefinition error detection by attempting to use `store` twice on the same variable.
Applied to files:
tests/analyzer_scope_correctness_test.rsTestPrograms/unicode_catch_test.wflTestPrograms/complex_expression_catch_test.wfltests/zero_arg_action_error_propagation_test.rs
📚 Learning: 2025-12-08T16:25:57.961Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.961Z
Learning: Applies to **/*.wfl : Use comprehensive error handling with try/when/otherwise blocks in WFL
Applied to files:
TestPrograms/unicode_catch_test.wflTestPrograms/complex_expression_catch_test.wfl
📚 Learning: 2025-12-01T18:28:13.642Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T18:28:13.642Z
Learning: Enable LSP trace logs with `RUST_LOG=trace cargo run -p wfl-lsp`
Applied to files:
wfl-lsp/src/main.rswfl-lsp/examples/simple_mcp_client.rstests/zero_arg_action_error_propagation_test.rs
📚 Learning: 2025-12-01T18:28:13.642Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T18:28:13.642Z
Learning: Applies to **/*.rs : Review `SECURITY.md`; avoid logging secrets and prefer zeroization for sensitive data in Rust code
Applied to files:
src/stdlib/crypto.rs
📚 Learning: 2025-08-12T09:39:16.504Z
Learnt from: logbie
Repo: WebFirstLanguage/wfl PR: 148
File: .claude/settings.local.json:41-44
Timestamp: 2025-08-12T09:39:16.504Z
Learning: The .claude/settings.local.json file is specifically formatted for Claude AI assistant usage and should not be reformatted to standard bash command syntax. The existing formatting is intentional and optimized for Claude's permission system.
Applied to files:
.claude/settings.local.json
📚 Learning: 2025-12-08T16:25:57.961Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.961Z
Learning: Applies to src/interpreter/**/*.rs : Interpreter subprocess handling must include security sanitization for commands
Applied to files:
src/interpreter/mod.rs
📚 Learning: 2025-12-08T16:25:57.961Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.961Z
Learning: Applies to src/**/*.rs : Component documentation required for all major modules
Applied to files:
wfl-lsp/src/core.rs
🧬 Code graph analysis (8)
src/main.rs (1)
src/stdlib/typechecker.rs (1)
register_stdlib_types(4-46)
tests/file_io_windows_sync_errors_test.rs (6)
tests/split_functionality.rs (2)
path(21-23)get_wfl_binary_path(28-51)tests/string_escape_sequences.rs (2)
path(21-23)get_wfl_binary_path(26-49)tests/subprocess_cleanup_test.rs (2)
path(21-23)get_wfl_binary_path(26-49)tests/subprocess_security_test.rs (2)
path(21-23)get_wfl_binary_path(26-49)tests/modulo_operator_test.rs (1)
get_wfl_binary_path(9-32)tests/zero_arg_action_error_propagation_test.rs (1)
get_wfl_binary_path(11-34)
tests/analyzer_scope_correctness_test.rs (2)
src/lexer/mod.rs (1)
lex_wfl_with_positions(88-226)src/analyzer/mod.rs (3)
new(79-84)new(144-150)new(180-325)
tests/split_functionality.rs (2)
tests/file_io_windows_sync_errors_test.rs (1)
get_wfl_binary_path(13-36)tests/modulo_operator_test.rs (1)
get_wfl_binary_path(9-32)
wfl-lsp/src/lib.rs (2)
wfl-lsp/src/core.rs (1)
new(40-45)wfl-lsp/src/mcp_server.rs (1)
new(51-58)
wfl-lsp/src/main.rs (3)
wfl-lsp/src/core.rs (1)
new(40-45)wfl-lsp/src/lib.rs (1)
new(21-27)wfl-lsp/src/mcp_server.rs (2)
new(51-58)run_server(1171-1231)
wfl-lsp/examples/simple_mcp_client.rs (4)
wfl-lsp/src/main.rs (1)
main(5-25)wfl-lsp/src/lib.rs (1)
new(21-27)wfl-lsp/src/mcp_server.rs (1)
new(51-58)wfl-lsp/examples/test_mcp_server.sh (1)
send_request(15-22)
tests/zero_arg_action_error_propagation_test.rs (6)
tests/split_functionality.rs (2)
path(21-23)get_wfl_binary_path(28-51)tests/string_escape_sequences.rs (2)
path(21-23)get_wfl_binary_path(26-49)tests/subprocess_cleanup_test.rs (2)
path(21-23)get_wfl_binary_path(26-49)tests/subprocess_security_test.rs (2)
path(21-23)get_wfl_binary_path(26-49)tests/file_io_windows_sync_errors_test.rs (1)
get_wfl_binary_path(13-36)tests/modulo_operator_test.rs (1)
get_wfl_binary_path(9-32)
🪛 GitHub Actions: CI
src/stdlib/core.rs
[error] 75-75: cargo fmt --check detected formatting changes. Run 'cargo fmt' to fix code style issues.
tests/subprocess_security_test.rs
[error] 141-141: cargo fmt --check formatting changes in assertion formatting.
[error] 169-169: cargo fmt --check formatting changes in assertion formatting.
wfl-lsp/examples/simple_mcp_client.rs
[error] 1-1: cargo fmt --check formatting changes: import ordering in example.
[error] 47-47: cargo fmt --check formatting changes: adjusted println! usage formatting.
[error] 82-82: cargo fmt --check formatting changes: adjusted as_str() usage formatting.
src/stdlib/crypto.rs
[error] 377-377: cargo fmt --check formatting changes: multiple lines wrapping the warning string.
[error] 402-402: cargo fmt --check formatting changes: multi-line eprintln! macro formatting.
[error] 421-421: cargo fmt --check formatting changes: multi-line eprintln! macro formatting.
[error] 444-444: cargo fmt --check formatting changes: multi-line eprintln! macro formatting.
src/interpreter/mod.rs
[error] 6123-6123: cargo fmt --check detected formatting changes. Run 'cargo fmt' to fix code style issues.
[error] 6123-6123: Formatting diff detected by cargo fmt --check; apply rustfmt.
wfl-lsp/src/core.rs
[error] 1-1: cargo fmt --check formatting changes: re-export formatting for lsp_types imports.
wfl-lsp/src/mcp_server.rs
[error] 205-205: cargo fmt --check formatting changes: single-line vs multi-line formatting in code.
[error] 635-635: cargo fmt --check formatting changes: indentation/line-wrapping adjustments.
[error] 642-642: cargo fmt --check formatting changes: keyword list formatting adjusted.
[error] 712-712: cargo fmt --check formatting changes: argument formatting in function call.
🪛 LanguageTool
Docs/technical/wfl-mcp-architecture.md
[grammar] ~287-~287: Ensure spelling is correct
Context: ...gn - Performance: Analysis is fast (<100ms typically) - Reliability: No concur...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
Docs/guides/claude-desktop-integration.md
30-30: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
309-309: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
wfl-lsp/MCP_TEST_REPORT.md
247-247: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
Docs/technical/wfl-mcp-architecture.md
30-30: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
309-309: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
wflhash/wflhashspec.md
30-30: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
Docs/guides/wfl-mcp-guide.md
30-30: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
309-309: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
README.md
278-278: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: claude-review
- GitHub Check: config-lint
| ```json | ||
| // Good | ||
| "cwd": "G:/Projects/my-wfl-app" | ||
|
|
||
| // Bad | ||
| "cwd": "../my-wfl-app" | ||
| ``` |
There was a problem hiding this comment.
Fix language identifier for JSON with comments.
The code block contains JavaScript-style comments (//) which are not valid in standard JSON. Use jsonc (JSON with Comments) or javascript as the language identifier instead of the missing identifier.
🔎 Proposed fix
+```jsonc
// Good
"cwd": "G:/Projects/my-wfl-app"
// Bad
"cwd": "../my-wfl-app"
+```Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Docs/guides/claude-desktop-integration.md around lines 301 to 307, the fenced
code block uses JSON but contains JavaScript-style comments which are invalid in
standard JSON; change the code fence language identifier to "jsonc" (or
"javascript") so comments are allowed and ensure the fenced block has a proper
closing ``` fence; do not alter the snippet content, only update the opening
fence language and verify the block is correctly closed.
| # Get exit code from job result | ||
| $result = Receive-Job -Job $job | ||
| $exitCode = if ($result -and $result.ExitCode -ne $null) { $result.ExitCode } else { 0 } | ||
| Remove-Job -Job $job -Force |
There was a problem hiding this comment.
Dangerous fallback: defaulting to success on job failure.
Line 159 defaults $exitCode to 0 (success) when the job result is null or doesn't contain an ExitCode. This could mask critical failures if the job crashes, throws an exception, or fails to return the expected structure. A test that genuinely fails could be reported as passing.
🔎 Proposed fix to treat missing exit codes as failures
# Get exit code from job result
$result = Receive-Job -Job $job
- $exitCode = if ($result -and $result.ExitCode -ne $null) { $result.ExitCode } else { 0 }
+ $exitCode = if ($result -and $result.ExitCode -ne $null) { $result.ExitCode } else { 1 }
Remove-Job -Job $job -Force
if ($exitCode -eq 0) {Alternatively, add explicit error handling:
# Get exit code from job result
$result = Receive-Job -Job $job
- $exitCode = if ($result -and $result.ExitCode -ne $null) { $result.ExitCode } else { 0 }
+ if (-not $result -or $result.ExitCode -eq $null) {
+ Write-Host "[ERROR] FAIL $($wflFile.Name) (job did not return exit code)" -ForegroundColor Red
+ $exitCode = 1
+ } else {
+ $exitCode = $result.ExitCode
+ }
Remove-Job -Job $job -Force📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Get exit code from job result | |
| $result = Receive-Job -Job $job | |
| $exitCode = if ($result -and $result.ExitCode -ne $null) { $result.ExitCode } else { 0 } | |
| Remove-Job -Job $job -Force | |
| # Get exit code from job result | |
| $result = Receive-Job -Job $job | |
| $exitCode = if ($result -and $result.ExitCode -ne $null) { $result.ExitCode } else { 1 } | |
| Remove-Job -Job $job -Force |
🤖 Prompt for AI Agents
In scripts/run_integration_tests.ps1 around lines 157 to 160, the code currently
defaults $exitCode to 0 when the job result is null or missing an ExitCode which
hides failures; update the logic to treat missing or null results as a failure:
if Receive-Job returns null or lacks ExitCode set $exitCode to a non‑zero value
(e.g. 1), and add an explicit error/log message including job state or exception
details before removing the job so failures are visible and the script returns
non‑zero on unexpected job termination.
| let mut stdin = child.stdin.take().expect("Failed to open stdin"); | ||
| let stdout = child.stdout.take().expect("Failed to open stdout"); | ||
| let reader = BufReader::new(stdout); |
There was a problem hiding this comment.
Reader needs to be mutable for the closure.
The reader variable needs to be declared as mutable since it's captured by the closure and by_ref() requires a mutable reference.
🔎 Proposed fix
let mut stdin = child.stdin.take().expect("Failed to open stdin");
let stdout = child.stdout.take().expect("Failed to open stdout");
-let reader = BufReader::new(stdout);
+let mut reader = BufReader::new(stdout);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let mut stdin = child.stdin.take().expect("Failed to open stdin"); | |
| let stdout = child.stdout.take().expect("Failed to open stdout"); | |
| let reader = BufReader::new(stdout); | |
| let mut stdin = child.stdin.take().expect("Failed to open stdin"); | |
| let stdout = child.stdout.take().expect("Failed to open stdout"); | |
| let mut reader = BufReader::new(stdout); |
🤖 Prompt for AI Agents
In wfl-lsp/examples/simple_mcp_client.rs around lines 19 to 21, the BufReader
instance is not declared mutable but it's later captured by a closure that calls
by_ref(), which requires a &mut reference; change the declaration to create a
mutable reader (e.g., let mut reader = BufReader::new(stdout);) so the closure
can borrow it mutably, and update any subsequent uses to accept &mut reader as
needed.
| ``` | ||
| timeout_seconds = 60 | ||
| logging_enabled = false | ||
| debug_report_enabled = true | ||
| log_level = info | ||
| ``` |
There was a problem hiding this comment.
Fix missing language identifier for configuration block.
The configuration content should have a language identifier. Use ini since this is a .wflcfg configuration file format.
🔎 Proposed fix
-```
+```ini
timeout_seconds = 60
logging_enabled = false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` | |
| timeout_seconds = 60 | |
| logging_enabled = false | |
| debug_report_enabled = true | |
| log_level = info | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
247-247: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In wfl-lsp/MCP_TEST_REPORT.md around lines 247 to 252, the fenced code block
containing the .wflcfg configuration lacks a language identifier; update the
opening fence to use ```ini so the block is treated as INI syntax (ensure the
opening fence becomes ```ini and keep the existing closing ``` intact).
| /// Handle file:///{path} resource | ||
| fn handle_file_resource(&self, id: Option<Value>, uri: &str) -> JsonRpcResponse { | ||
| // Extract path from file:/// URI | ||
| let path_str = uri.strip_prefix("file:///").unwrap_or(uri); | ||
| let path = Path::new(path_str); | ||
|
|
||
| match fs::read_to_string(path) { | ||
| Ok(content) => JsonRpcResponse { | ||
| jsonrpc: "2.0".to_string(), | ||
| id, | ||
| result: Some(json!({ | ||
| "contents": [{ | ||
| "uri": uri, | ||
| "mimeType": "text/x-wfl", | ||
| "text": content | ||
| }] | ||
| })), | ||
| error: None, | ||
| }, | ||
| Err(e) => JsonRpcResponse { | ||
| jsonrpc: "2.0".to_string(), | ||
| id, | ||
| result: None, | ||
| error: Some(JsonRpcError { | ||
| code: -32603, | ||
| message: format!("Failed to read file: {}", e), | ||
| data: None, | ||
| }), | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential path traversal vulnerability in file resource handler.
The handle_file_resource method strips file:/// from the URI and reads the file directly without validating the path is within the workspace. A malicious client could potentially read arbitrary files (e.g., file:///etc/passwd or file:///C:/Windows/System32/config/SAM).
🔎 Suggested fix: validate path is within workspace
fn handle_file_resource(&self, id: Option<Value>, uri: &str) -> JsonRpcResponse {
// Extract path from file:/// URI
let path_str = uri.strip_prefix("file:///").unwrap_or(uri);
let path = Path::new(path_str);
+ // Security: Validate path is within workspace to prevent path traversal
+ if let Some(workspace_root) = &self.workspace_root {
+ let canonical_path = match path.canonicalize() {
+ Ok(p) => p,
+ Err(e) => {
+ return JsonRpcResponse {
+ jsonrpc: "2.0".to_string(),
+ id,
+ result: None,
+ error: Some(JsonRpcError {
+ code: -32603,
+ message: format!("Invalid path: {}", e),
+ data: None,
+ }),
+ };
+ }
+ };
+ let canonical_root = workspace_root.canonicalize().unwrap_or(workspace_root.clone());
+ if !canonical_path.starts_with(&canonical_root) {
+ return JsonRpcResponse {
+ jsonrpc: "2.0".to_string(),
+ id,
+ result: None,
+ error: Some(JsonRpcError {
+ code: -32603,
+ message: "Access denied: path outside workspace".to_string(),
+ data: None,
+ }),
+ };
+ }
+ }
+
match fs::read_to_string(path) {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In wfl-lsp/src/mcp_server.rs around lines 912 to 942, the file:// handler
currently strips the prefix and reads the path directly which allows path
traversal (e.g., file:///etc/passwd); fix by resolving and validating the path
against the workspace root before reading: canonicalize the requested path (and
handle failures), canonicalize the workspace root once, ensure the requested
path starts with the workspace canonical path (taking care with path separators
and Windows drive letters), and if it does not return a JsonRpc error (e.g.,
permission/invalid request) instead of reading the file; only call
fs::read_to_string when validation succeeds.
- Add comprehensive Git Workflow with Worktrees section - Document worktree creation, usage, and cleanup procedures - Establish mandatory worktree usage for all WFL development - Include best practices and common commands reference - Fix minor code fence formatting in README.md 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
- Add allowed_bots: '*' to claude-code-review workflow - Fixes error: "Workflow initiated by non-human actor" - Enables automated reviews from all GitHub bots including google-labs-jules 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (9)
src/stdlib/core.rs (1)
62-85: Verify that the formatting issue flagged in a previous review has been resolved.A prior CI run detected formatting changes at line 75. Please ensure
cargo fmt --allhas been run and the formatting check now passes.#!/bin/bash # Check if formatting is correct for this file cargo fmt --all -- --checkwfl-lsp/examples/simple_mcp_client.rs (4)
40-93: Consider more robust JSON field access.Multiple
.unwrap()calls (lines 52, 64, 88-90) could panic if the server returns unexpected JSON structure. While acceptable for an example, consider using safer access patterns with?operator orunwrap_or_default()for better robustness.For instance, line 52 could use:
init_response.get("result") .and_then(|r| r.get("serverInfo")) .and_then(|s| s.get("version")) .and_then(|v| v.as_str()) .unwrap_or("unknown")Note: CI also reports a formatting issue at line 47.
95-173: Examples demonstrate MCP protocol correctly.The example interactions properly demonstrate the MCP JSON-RPC protocol. The
.unwrap()usage follows the same pattern noted earlier. CI reports a formatting issue at line 82 that should be addressed withcargo fmt --all.
1-6: Address cargo fmt pipeline failures.The CI pipeline reports formatting issues including import ordering at line 1. Run
cargo fmt --allto fix these formatting inconsistencies.As per coding guidelines, all Rust code must be formatted with
cargo fmt --all.
19-21: Reader needs to be mutable for the closure.The
readervariable needs to be declared as mutable since it's captured by the closure andby_ref()requires a mutable reference.🔎 Proposed fix
let mut stdin = child.stdin.take().expect("Failed to open stdin"); let stdout = child.stdout.take().expect("Failed to open stdout"); -let reader = BufReader::new(stdout); +let mut reader = BufReader::new(stdout);wfl-lsp/src/core.rs (1)
1-11: Formatting appears resolved.The import formatting in the current code matches rustfmt standards (multi-line imports with proper grouping). If CI is still flagging formatting issues, run
cargo fmt --allto ensure consistency, but the current state appears correct.wfl-lsp/src/mcp_server.rs (3)
1-14: Verify formatting with cargo fmt.Past review indicated formatting issues at multiple locations. Run
cargo fmt --allto ensure all imports and formatting comply with rustfmt standards. As per coding guidelines, all Rust code should be formatted consistently.
1197-1258: Add integration tests for the MCP server entry point.The
run_server()function lacks test coverage. Consider adding integration tests that spawn the server and verify complete request/response flows, including error handling for malformed JSON and stdin/stdout communication.Example test structure:
- Mock stdin with test JSON-RPC requests
- Capture stdout responses
- Verify correct JSON-RPC protocol handling
- Test error cases (invalid JSON, unknown methods, etc.)
939-969: CRITICAL: Path traversal vulnerability in file resource handler.The
handle_file_resourcemethod stripsfile:///and reads the file directly without validating that the path is within the workspace. A malicious MCP client could read arbitrary files on the system (e.g.,file:///etc/passwd,file:///C:/Windows/System32/config/SAM, or any sensitive configuration files).🔎 Proposed fix: validate path is within workspace
fn handle_file_resource(&self, id: Option<Value>, uri: &str) -> JsonRpcResponse { // Extract path from file:/// URI let path_str = uri.strip_prefix("file:///").unwrap_or(uri); let path = Path::new(path_str); + // Security: Validate path is within workspace to prevent path traversal + if let Some(workspace_root) = &self.workspace_root { + let canonical_path = match path.canonicalize() { + Ok(p) => p, + Err(e) => { + return JsonRpcResponse { + jsonrpc: "2.0".to_string(), + id, + result: None, + error: Some(JsonRpcError { + code: -32603, + message: format!("Invalid path: {}", e), + data: None, + }), + }; + } + }; + let canonical_root = workspace_root.canonicalize().unwrap_or_else(|_| workspace_root.clone()); + if !canonical_path.starts_with(&canonical_root) { + return JsonRpcResponse { + jsonrpc: "2.0".to_string(), + id, + result: None, + error: Some(JsonRpcError { + code: -32603, + message: "Access denied: path outside workspace".to_string(), + data: None, + }), + }; + } + } else { + // If no workspace is configured, deny all file access + return JsonRpcResponse { + jsonrpc: "2.0".to_string(), + id, + result: None, + error: Some(JsonRpcError { + code: -32603, + message: "No workspace configured - file access denied".to_string(), + data: None, + }), + }; + } match fs::read_to_string(path) {Based on learnings, review
SECURITY.mdfor additional guidance on secure file handling.
🧹 Nitpick comments (6)
src/stdlib/core.rs (1)
71-75: Consider moving the import to the top and handling flush failure more explicitly.The
use std::io::Write;inside the if-block is valid but unconventional. Additionally, silently discarding the flush result means the prompt may not appear before blocking on input if flush fails.🔎 Suggested refactor
Move the import to the module level (top of file) and optionally log or propagate flush errors:
use crate::interpreter::environment::Environment; use crate::interpreter::error::RuntimeError; use crate::interpreter::value::Value; +use std::io::Write; use std::rc::Rc;Then simplify the prompt block:
if let Some(prompt) = args.first() { print!("{}", prompt); - use std::io::Write; - let _ = std::io::stdout().flush(); + std::io::stdout().flush().ok(); // or propagate error if desired }wfl-lsp/src/mcp_server.rs (1)
989-1012: Duplicate code pattern for workspace scanning.The logic for scanning
.wflfiles in the workspace is duplicated acrosshandle_workspace_symbols(lines 992-1012) andhandle_workspace_diagnostics(lines 1095-1120). Both use similar patterns: read directory, filter for.wflfiles, read content, parse, and collect results.🔎 Extract common workspace scanning logic
Create a helper method that scans the workspace and applies a callback:
fn scan_workspace_files<F>(&self, mut callback: F) -> Result<(), String> where F: FnMut(&Path, &str) -> Option<Value>, { let workspace_root = self.workspace_root.as_ref() .ok_or("No workspace root configured")?; for entry in fs::read_dir(workspace_root).map_err(|e| e.to_string())? { let entry = entry.map_err(|e| e.to_string())?; if entry.file_type().map_err(|e| e.to_string())?.is_file() { let path = entry.path(); if path.extension().and_then(|s| s.to_str()) == Some("wfl") { if let Ok(content) = fs::read_to_string(&path) { callback(&path, &content); } } } } Ok(()) }Then use it in both handlers to reduce duplication.
src/stdlib/crypto.rs (3)
291-291: Remove meta-commentary that describes PR changes.This comment describes how the code changed rather than documenting its behavior. Such comments become stale and confusing over time.
🔎 Proposed fix
-// ... Params struct remains mostly the same, ensuring zeroize ... #[derive(Clone, Debug)] struct WflHashParams {
440-444: Inconsistent error messages: "Invalid arg type" vs "Invalid argument type".Some functions use the abbreviated "Invalid arg type" while others use "Invalid argument type". Consider standardizing for consistent error reporting.
🔎 Proposed fix: Standardize to full form
- _ => return Err(RuntimeError::new("Invalid arg type".to_string(), 0, 0)), + _ => return Err(RuntimeError::new("Invalid argument type".to_string(), 0, 0)),Also applies to: 465-469
495-512: Consider logging or handling define failures.The
let _ = env.define(...)pattern silently discards any errors from registration. While failures here are unlikely (and may indicate a bug elsewhere), consider at least debug-logging registration failures for easier troubleshooting.CLAUDE.md (1)
122-126: Reconsider the mandatory worktree requirement.While git worktrees are useful for parallel development, declaring them "MANDATORY" for all WFL development may be unnecessarily restrictive. This requirement:
- Is not mentioned in the PR objectives or commit messages
- May create barriers for new or occasional contributors
- May be unnecessary for simple changes (documentation, small bugfixes)
- Adds workflow complexity without clear justification
Consider softening this to "RECOMMENDED" for complex features or parallel work, while allowing developers flexibility for simpler changes.
🔎 Suggested revision
## Git Workflow with Worktrees -**MANDATORY: Always use git worktrees when working on WFL tasks.** +**RECOMMENDED: Use git worktrees for complex features and parallel development.** Git worktrees allow you to work on multiple branches simultaneously without switching branches in your main working directory. This is the required workflow for all WFL development. +This workflow is especially helpful when working on multiple features simultaneously or when you need to maintain build isolation.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/claude-code-review.ymlCLAUDE.mdREADME.mdsrc/interpreter/mod.rssrc/stdlib/core.rssrc/stdlib/crypto.rstests/subprocess_security_test.rswfl-lsp/examples/simple_mcp_client.rswfl-lsp/src/core.rswfl-lsp/src/mcp_server.rs
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/subprocess_security_test.rs
- src/interpreter/mod.rs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingcargo fmt --all(see.rustfmt.toml)
Lint clean: runcargo clippy --all-targets --all-features -- -D warningswith no warnings
Usesnake_casefor function and file names in Rust
UseCamelCasefor types and traits in Rust
UseSCREAMING_SNAKE_CASEfor constants in Rust
ReviewSECURITY.md; avoid logging secrets and prefer zeroization for sensitive data in Rust code
**/*.rs: Use Rust Edition 2024 for all Rust code in this project
Format all Rust code with 'cargo fmt --all'
Run 'cargo clippy --all-targets --all-features -- -D warnings' to enforce linting rules
Files:
src/stdlib/core.rswfl-lsp/examples/simple_mcp_client.rswfl-lsp/src/core.rssrc/stdlib/crypto.rswfl-lsp/src/mcp_server.rs
src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Component documentation required for all major modules
Files:
src/stdlib/core.rssrc/stdlib/crypto.rs
src/stdlib/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use WFLHASH custom cryptographic hash function for memory and cryptographic operations
Files:
src/stdlib/core.rssrc/stdlib/crypto.rs
wfl-lsp/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Implement LSP server support in wfl-lsp workspace member using tower-lsp
Files:
wfl-lsp/examples/simple_mcp_client.rswfl-lsp/src/core.rswfl-lsp/src/mcp_server.rs
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.972Z
Learning: Applies to wfl-lsp/**/*.rs : Implement LSP server support in wfl-lsp workspace member using tower-lsp
📚 Learning: 2025-12-08T16:25:57.972Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.972Z
Learning: Applies to wfl-lsp/**/*.rs : Implement LSP server support in wfl-lsp workspace member using tower-lsp
Applied to files:
wfl-lsp/examples/simple_mcp_client.rsCLAUDE.mdwfl-lsp/src/core.rswfl-lsp/src/mcp_server.rs
📚 Learning: 2025-12-01T18:28:13.642Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T18:28:13.642Z
Learning: Enable LSP trace logs with `RUST_LOG=trace cargo run -p wfl-lsp`
Applied to files:
wfl-lsp/examples/simple_mcp_client.rsCLAUDE.mdwfl-lsp/src/core.rs
📚 Learning: 2025-12-01T18:28:13.642Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T18:28:13.642Z
Learning: LSP crate development: build with `cargo build -p wfl-lsp` and run with `cargo run -p wfl-lsp`
Applied to files:
wfl-lsp/examples/simple_mcp_client.rsCLAUDE.mdwfl-lsp/src/core.rs
📚 Learning: 2025-12-08T16:25:57.972Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.972Z
Learning: Applies to **/*.rs : Format all Rust code with 'cargo fmt --all'
Applied to files:
wfl-lsp/examples/simple_mcp_client.rswfl-lsp/src/core.rswfl-lsp/src/mcp_server.rs
📚 Learning: 2025-12-01T18:28:13.642Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T18:28:13.642Z
Learning: Applies to **/*.rs : Format Rust code using `cargo fmt --all` (see `.rustfmt.toml`)
Applied to files:
wfl-lsp/examples/simple_mcp_client.rswfl-lsp/src/core.rs
📚 Learning: 2025-12-08T16:25:57.972Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.972Z
Learning: Applies to **/*.rs : Run 'cargo clippy --all-targets --all-features -- -D warnings' to enforce linting rules
Applied to files:
wfl-lsp/examples/simple_mcp_client.rswfl-lsp/src/core.rs
📚 Learning: 2025-12-01T18:28:13.642Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T18:28:13.642Z
Learning: Applies to **/*.rs : Lint clean: run `cargo clippy --all-targets --all-features -- -D warnings` with no warnings
Applied to files:
wfl-lsp/examples/simple_mcp_client.rswfl-lsp/src/core.rs
📚 Learning: 2025-12-01T18:28:13.642Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T18:28:13.642Z
Learning: Ensure LSP features integration by building release binary: `cargo build --release` provides `target/release/wfl`
Applied to files:
CLAUDE.md
📚 Learning: 2025-08-12T17:35:32.346Z
Learnt from: logbie
Repo: WebFirstLanguage/wfl PR: 150
File: tests/file_io_modes_test.rs:0-0
Timestamp: 2025-08-12T17:35:32.346Z
Learning: In the WFL project tests, file_io_execution_test.rs contains tests that actually execute WFL code through the interpreter and create real files, while file_io_modes_test.rs contains parser-only tests that don't execute code or create files during parsing.
Applied to files:
CLAUDE.mdwfl-lsp/src/core.rssrc/stdlib/crypto.rswfl-lsp/src/mcp_server.rs
📚 Learning: 2025-12-08T16:25:57.972Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.972Z
Learning: Applies to **/*.wfl : WFL code must pass linting with 'wfl --lint program.wfl'
Applied to files:
CLAUDE.md
📚 Learning: 2025-12-08T16:25:57.972Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.972Z
Learning: Applies to Cargo.toml : Configure workspace with Cargo and support workspace members (wfl-lsp, vscode-extension)
Applied to files:
CLAUDE.mdwfl-lsp/src/core.rs
📚 Learning: 2025-12-08T16:25:57.972Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.972Z
Learning: Applies to **/*.rs : Use Rust Edition 2024 for all Rust code in this project
Applied to files:
wfl-lsp/src/core.rs
📚 Learning: 2025-12-01T18:28:13.642Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T18:28:13.642Z
Learning: Applies to **/*.rs : Use `CamelCase` for types and traits in Rust
Applied to files:
wfl-lsp/src/core.rs
📚 Learning: 2025-12-08T16:25:57.972Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.972Z
Learning: Applies to src/stdlib/**/*.rs : Use WFLHASH custom cryptographic hash function for memory and cryptographic operations
Applied to files:
src/stdlib/crypto.rs
📚 Learning: 2025-12-01T18:28:13.642Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T18:28:13.642Z
Learning: Applies to **/*.rs : Review `SECURITY.md`; avoid logging secrets and prefer zeroization for sensitive data in Rust code
Applied to files:
src/stdlib/crypto.rswfl-lsp/src/mcp_server.rs
📚 Learning: 2025-12-08T16:25:57.972Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.972Z
Learning: Applies to tests/**/*.rs : Integration tests in Rust must use the release binary (target/release/wfl.exe on Windows, target/release/wfl on Unix)
Applied to files:
src/stdlib/crypto.rs
📚 Learning: 2025-12-08T16:25:57.972Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.972Z
Learning: Applies to src/interpreter/**/*.rs : Interpreter subprocess handling must include security sanitization for commands
Applied to files:
wfl-lsp/src/mcp_server.rs
🧬 Code graph analysis (2)
wfl-lsp/examples/simple_mcp_client.rs (4)
wfl-lsp/src/main.rs (1)
main(5-25)wfl-lsp/src/mcp_server.rs (1)
new(51-58)wfl-lsp/src/lib.rs (1)
new(21-27)wfl-lsp/examples/test_mcp_server.sh (1)
send_request(15-22)
wfl-lsp/src/core.rs (2)
src/lexer/mod.rs (1)
lex_wfl_with_positions(88-226)src/diagnostics/mod.rs (1)
from(18-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Integration Tests (windows-latest)
- GitHub Check: Build, Test, Clippy
🔇 Additional comments (15)
src/stdlib/core.rs (1)
59-60: LGTM!The registration follows the established pattern used for other native functions in this module.
wfl-lsp/examples/simple_mcp_client.rs (3)
8-17: LGTM!The process spawning logic is correct, with appropriate stdio configuration for JSON-RPC communication over pipes.
23-38: LGTM with dependency on reader mutability.The closure correctly implements JSON-RPC request/response handling. Once the
readermutability issue (line 21) is fixed, this will work as intended.
175-179: LGTM!The completion message and overall example structure effectively demonstrate MCP client usage.
wfl-lsp/src/core.rs (2)
253-315: LGTM! Comprehensive test coverage.The test suite covers core functionality including creation, workspace handling, document lifecycle, and basic analysis scenarios. Good validation of both valid and invalid code paths.
211-231: The code is correct;offset_to_line_colguarantees 1-based coordinates orNoneon error.The function explicitly adds 1 to both line and column (lines 242-243) and only returns
Some((line, column))when both values are ≥ 1. It returnsNonefor invalid inputs or out-of-bounds offsets. Therefore,saturating_sub(1)safely converts from 1-based to 0-based indexing with no risk of 0 remaining as a coordinate.Likely an incorrect or invalid review comment.
wfl-lsp/src/mcp_server.rs (1)
1260-1335: Good test coverage for core MCP functionality.The test suite validates server creation, initialization, tools listing, and parse tool behavior for both valid and invalid code. Well-structured tests that cover the main success and error paths.
src/stdlib/crypto.rs (6)
1-70: LGTM: Constants and imports are well-structured.The DoS protection constant
MAX_INPUT_SIZEis appropriately public for external validation, and the cryptographic constants are properly documented with their derivation (cube roots of primes).
72-97: LGTM: State structure with proper zeroization.The buffered sponge state correctly tracks input length with
u128for large inputs, and theDropimplementation properly zeroizes both state and buffer to prevent sensitive data leakage. Based on learnings, this follows the guideline to prefer zeroization for sensitive data.
99-158: LGTM: Initialization and permutation logic.The parameter mixing into the IV provides proper domain separation, and the MAC mode correctly absorbs the derived key after the initial permutation. The column-then-row G-function application follows standard sponge permutation patterns.
160-186: LGTM: G-function implementation.The ARX quarter-round follows ChaCha/BLAKE structure with appropriate 64-bit rotation constants. The sequential mixing step (Feistel-like) maintains invertibility as documented. The
#[inline(always)]is appropriate for this performance-critical inner loop.
400-404: Missing experimental warning innative_wflhash256_binary.Unlike the other native hash functions, this function doesn't emit the experimental crypto warning. If this is intentional (e.g., for internal use), consider adding a brief comment explaining why. Otherwise, add the warning for consistency.
477-493: LGTM: Constant-time MAC verification.The use of
subtle::ConstantTimeEqfor MAC comparison correctly prevents timing attacks. ReturningOk(false)for invalid MAC length is reasonable behavior for a verification function.CLAUDE.md (2)
117-120: No issues found. All referenced MCP documentation files exist with correct paths:
- Docs/guides/wfl-mcp-guide.md ✓
- Docs/guides/wfl-mcp-api-reference.md ✓
- Docs/guides/claude-desktop-integration.md ✓
- Docs/technical/wfl-mcp-architecture.md ✓
81-86: All referenced files exist in the repository.Verification confirms:
wfl-lsp/examples/test_mcp_server.ps1✓wfl-lsp/examples/test_mcp_server.sh✓wfl-lsp/examples/simple_mcp_client.rs✓The documentation at lines 81-86 is accurate.
| # Allow all bots to trigger Claude Code reviews | ||
| allowed_bots: '*' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat .github/workflows/claude-code-review.ymlRepository: WebFirstLanguage/wfl
Length of output: 3167
🌐 Web query:
anthropics/claude-code-action allowed_bots parameter
💡 Result:
The Claude Code Action supports an allowed_bots input (env var ALLOWED_BOTS / input name allowed_bots) that controls which GitHub bot accounts may trigger the action. By default it's empty (no bots allowed). You can set a comma-separated list of bot usernames (e.g., "dependabot[bot],renovate[bot]") or "*" to allow all bots. [1][2]
Example (workflow with input):
- allowed_bots: "dependabot[bot],renovate[bot]"
Sources: Anthropic docs (Claude Code Action config) and action config/readme. [1][2]
Restrict allowed_bots from wildcard to specific trusted bots.
The allowed_bots: '*' parameter allows any bot account to trigger Claude Code reviews, creating unnecessary exposure to unauthorized or malicious bots. Since the default behavior is to allow no bots, using a wildcard significantly increases the security surface and API usage costs. Restrict this to specific trusted bots that legitimately need to trigger reviews:
🔎 Proposed fix
- # Allow all bots to trigger Claude Code reviews
- allowed_bots: '*'
+ # Allow specific trusted bots to trigger Claude Code reviews
+ allowed_bots: 'dependabot[bot],renovate[bot]'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Allow all bots to trigger Claude Code reviews | |
| allowed_bots: '*' | |
| # Allow specific trusted bots to trigger Claude Code reviews | |
| allowed_bots: 'dependabot[bot],renovate[bot]' |
🤖 Prompt for AI Agents
.github/workflows/claude-code-review.yml around lines 40-41: the workflow
currently uses allowed_bots: '*' which permits any bot to trigger Claude Code
reviews; change this to an explicit list of trusted bot accounts (e.g., CI and
automation bot names) by replacing the wildcard with an array of those bot
usernames or GitHub app identifiers, include only the minimum required bots, and
add a brief comment listing the owners/justification for auditability before
committing.
| fn squeeze(&mut self, output_bytes: usize) -> Vec<u8> { | ||
| let mut output = Vec::new(); | ||
| let mut output = Vec::with_capacity(output_bytes); | ||
|
|
||
| while output.len() < output_bytes { | ||
| // Extract rate portion | ||
| let rate = self.extract_rate(); | ||
|
|
||
| // Convert to bytes | ||
| for &word in &rate { | ||
| let bytes = word.to_le_bytes(); | ||
| output.extend_from_slice(&bytes); | ||
|
|
||
| let rate_words = [ | ||
| self.state[0][0], | ||
| self.state[0][1], | ||
| self.state[0][2], | ||
| self.state[0][3], | ||
| self.state[1][0], | ||
| self.state[1][1], | ||
| self.state[1][2], | ||
| self.state[1][3], | ||
| ]; | ||
|
|
||
| for &word in &rate_words { | ||
| output.extend_from_slice(&word.to_le_bytes()); | ||
| if output.len() >= output_bytes { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // Apply permutation for next block | ||
| if output.len() < output_bytes { | ||
| self.permute(); | ||
| } | ||
| } | ||
|
|
||
| output.truncate(output_bytes); | ||
| output | ||
| } |
There was a problem hiding this comment.
Output length is not truncated to exact output_bytes.
The squeeze function may return more bytes than requested. When breaking from the inner loop after extend_from_slice, the output can contain up to 7 extra bytes beyond output_bytes. This could cause issues for callers expecting exact-length output.
🔎 Proposed fix: Truncate output before returning
if output.len() < output_bytes {
self.permute();
}
}
+ output.truncate(output_bytes);
output
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn squeeze(&mut self, output_bytes: usize) -> Vec<u8> { | |
| let mut output = Vec::new(); | |
| let mut output = Vec::with_capacity(output_bytes); | |
| while output.len() < output_bytes { | |
| // Extract rate portion | |
| let rate = self.extract_rate(); | |
| // Convert to bytes | |
| for &word in &rate { | |
| let bytes = word.to_le_bytes(); | |
| output.extend_from_slice(&bytes); | |
| let rate_words = [ | |
| self.state[0][0], | |
| self.state[0][1], | |
| self.state[0][2], | |
| self.state[0][3], | |
| self.state[1][0], | |
| self.state[1][1], | |
| self.state[1][2], | |
| self.state[1][3], | |
| ]; | |
| for &word in &rate_words { | |
| output.extend_from_slice(&word.to_le_bytes()); | |
| if output.len() >= output_bytes { | |
| break; | |
| } | |
| } | |
| // Apply permutation for next block | |
| if output.len() < output_bytes { | |
| self.permute(); | |
| } | |
| } | |
| output.truncate(output_bytes); | |
| output | |
| } | |
| fn squeeze(&mut self, output_bytes: usize) -> Vec<u8> { | |
| let mut output = Vec::with_capacity(output_bytes); | |
| while output.len() < output_bytes { | |
| let rate_words = [ | |
| self.state[0][0], | |
| self.state[0][1], | |
| self.state[0][2], | |
| self.state[0][3], | |
| self.state[1][0], | |
| self.state[1][1], | |
| self.state[1][2], | |
| self.state[1][3], | |
| ]; | |
| for &word in &rate_words { | |
| output.extend_from_slice(&word.to_le_bytes()); | |
| if output.len() >= output_bytes { | |
| break; | |
| } | |
| } | |
| if output.len() < output_bytes { | |
| self.permute(); | |
| } | |
| } | |
| output.truncate(output_bytes); | |
| output | |
| } |
🤖 Prompt for AI Agents
In src/stdlib/crypto.rs around lines 261 to 288, the squeeze method can append
full 8-byte words and then break, producing up to 7 extra bytes beyond
output_bytes; change the logic to ensure exactly output_bytes are produced by
only appending the needed number of bytes from each word (or continue to append
but truncate the output to output_bytes before returning). Update the inner loop
to compute remaining = output_bytes - output.len() and append only
min(remaining, 8) bytes from word.to_le_bytes(), and as a safe final step always
truncate output.truncate(output_bytes) before returning.
| if !wfl_diag.notes.is_empty() { | ||
| let related = wfl_diag | ||
| .notes | ||
| .iter() | ||
| .map(|note| DiagnosticRelatedInformation { | ||
| location: Location { | ||
| uri: Url::parse("file:///document.wfl").unwrap(), | ||
| range: Range { | ||
| start: Position { | ||
| line: 0, | ||
| character: 0, | ||
| }, | ||
| end: Position { | ||
| line: 0, | ||
| character: 0, | ||
| }, | ||
| }, | ||
| }, | ||
| message: note.clone(), | ||
| }) | ||
| .collect(); | ||
| related_information = Some(related); | ||
| } |
There was a problem hiding this comment.
Hardcoded file URI in related diagnostic information.
The related diagnostic information uses a hardcoded URI "file:///document.wfl" and zero positions for all notes. This means related information won't correctly link to actual file locations, making it less useful for clients.
🔎 Consider using the actual document URI
If the document URI is available in the context, pass it as a parameter to convert_to_lsp_diagnostic and use it here. Alternatively, if related information lacks proper location data, consider omitting the location field entirely or setting it to point to the main diagnostic's location.
- location: Location {
- uri: Url::parse("file:///document.wfl").unwrap(),
- range: Range {
- start: Position {
- line: 0,
- character: 0,
- },
- end: Position {
- line: 0,
- character: 0,
- },
- },
- },
+ location: Location {
+ uri: Url::parse("file:///document.wfl").unwrap(), // TODO: use actual document URI
+ range: range, // Use the main diagnostic's range
+ },Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In wfl-lsp/src/core.rs around lines 176 to 198, the related diagnostic
information currently uses a hardcoded URI ("file:///document.wfl") and zeroed
positions; update convert_to_lsp_diagnostic to accept the actual document URI
(or the main diagnostic's Location) and use that URI for related items, and
populate each related item's Range/Position from the note's real location data
if available; if no location data exists for a note, either omit the related
DiagnosticRelatedInformation.location field or set it to the main diagnostic's
location so clients can link correctly.
| let mut wfl_files = Vec::new(); | ||
| if let Ok(entries) = fs::read_dir(workspace_root) { | ||
| for entry in entries.flatten() { | ||
| if let Ok(file_type) = entry.file_type() { | ||
| if file_type.is_file() { | ||
| if let Some(path) = entry.path().to_str() { | ||
| if path.ends_with(".wfl") { | ||
| let file_name = entry.file_name(); | ||
| wfl_files.push(json!({ | ||
| "uri": format!("file:///{}", path.replace("\\", "/")), | ||
| "name": file_name.to_string_lossy(), | ||
| "mimeType": "text/x-wfl" | ||
| })); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Non-recursive workspace file listing.
The handle_workspace_files function only scans the top-level workspace directory (fs::read_dir) without recursing into subdirectories. If WFL projects typically organize code in subdirectories, this will miss nested .wfl files.
🔎 Consider recursive directory traversal
Use a recursive directory walker (e.g., walkdir crate) to find all .wfl files:
// Add to Cargo.toml: walkdir = "2"
use walkdir::WalkDir;
// In handle_workspace_files:
for entry in WalkDir::new(workspace_root)
.into_iter()
.filter_map(|e| e.ok())
.filter(|e| e.path().extension().and_then(|s| s.to_str()) == Some("wfl"))
{
// Process entry...
}🤖 Prompt for AI Agents
In wfl-lsp/src/mcp_server.rs around lines 902 to 920, the current code only
reads the top-level directory with fs::read_dir so nested .wfl files are missed;
replace the non-recursive loop with a recursive directory walker (add walkdir =
"2" to Cargo.toml), iterate WalkDir::new(workspace_root) filtering out Err
entries and keeping only files whose extension is "wfl", then for each matching
entry build the same JSON object (normalizing path separators and creating the
file:/// URI) and push it to wfl_files; ensure you skip unreadable entries
rather than panicking and preserve the existing name/mimeType fields.
| let config_path = workspace_root.join(".wflcfg"); | ||
| let config_content = if config_path.exists() { | ||
| fs::read_to_string(&config_path).unwrap_or_else(|_| "{}".to_string()) | ||
| } else { | ||
| json!({ | ||
| "message": "No .wflcfg file found in workspace", | ||
| "using_defaults": true | ||
| }) | ||
| .to_string() | ||
| }; |
There was a problem hiding this comment.
Config file read without validation.
The .wflcfg file is read and returned as-is without validating that it contains valid JSON or expected configuration structure. If the file contains malformed JSON, the client will receive invalid data.
🔎 Validate configuration file format
let config_path = workspace_root.join(".wflcfg");
let config_content = if config_path.exists() {
- fs::read_to_string(&config_path).unwrap_or_else(|_| "{}".to_string())
+ match fs::read_to_string(&config_path) {
+ Ok(content) => {
+ // Validate it's valid JSON
+ match serde_json::from_str::<Value>(&content) {
+ Ok(_) => content,
+ Err(e) => {
+ json!({
+ "error": format!("Invalid .wflcfg format: {}", e),
+ "using_defaults": true
+ }).to_string()
+ }
+ }
+ }
+ Err(e) => json!({
+ "error": format!("Failed to read .wflcfg: {}", e),
+ "using_defaults": true
+ }).to_string()
+ }
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let config_path = workspace_root.join(".wflcfg"); | |
| let config_content = if config_path.exists() { | |
| fs::read_to_string(&config_path).unwrap_or_else(|_| "{}".to_string()) | |
| } else { | |
| json!({ | |
| "message": "No .wflcfg file found in workspace", | |
| "using_defaults": true | |
| }) | |
| .to_string() | |
| }; | |
| let config_path = workspace_root.join(".wflcfg"); | |
| let config_content = if config_path.exists() { | |
| match fs::read_to_string(&config_path) { | |
| Ok(content) => { | |
| // Validate it's valid JSON | |
| match serde_json::from_str::<Value>(&content) { | |
| Ok(_) => content, | |
| Err(e) => { | |
| json!({ | |
| "error": format!("Invalid .wflcfg format: {}", e), | |
| "using_defaults": true | |
| }).to_string() | |
| } | |
| } | |
| } | |
| Err(e) => json!({ | |
| "error": format!("Failed to read .wflcfg: {}", e), | |
| "using_defaults": true | |
| }).to_string() | |
| } | |
| } else { | |
| json!({ | |
| "message": "No .wflcfg file found in workspace", | |
| "using_defaults": true | |
| }) | |
| .to_string() | |
| }; |
Enhances the static analyzer to recognize and track variables declared in `for each` and `count` loops. Previously, these loop-specific variables were not registered, preventing accurate unused variable analysis. This change updates the variable collection process to correctly identify these declarations. This allows the analyzer to properly validate their usage within the loop body and report them as unused when appropriate. Tests are added to confirm the correct behavior.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 42 out of 43 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Introduces support for the conventional `function(arg1, arg2)` syntax for calling functions, complementing the existing space-separated argument style. This change allows for explicit function calls, including those with zero arguments like `random()`. The interpreter's evaluation logic is updated to handle this new syntax, ensuring that a function reference is resolved correctly before it's called with the provided arguments. A new test case is added to verify this behavior with a zero-argument native function.
Adjusts code indentation and line breaks across several files to improve readability and maintain a uniform style. This change is purely stylistic and does not alter program logic.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/parser/expr/primary.rs (1)
1117-1175: Well-implemented function call parsing with parentheses.The postfix handling correctly parses function calls, handles empty argument lists, supports chained calls (via the while loop), and provides clear error messages. Good use of
expect_tokenfor the closing paren.Minor inconsistency (optional): The expression types matched for line/column extraction (lines 1155-1165) differ from the
IntLiteralindex access case (lines 990-1004). Consider aligning them for consistency—e.g., addingIndexAccessandMethodCallhere if those are valid bases for a call expression:🔎 Suggested alignment (optional)
let (base_line, base_col) = match &expr { Expression::Variable(_, line, col) | Expression::FunctionCall { line, column: col, .. } | Expression::PropertyAccess { line, column: col, .. } | Expression::StaticMemberAccess { line, column: col, .. } + | Expression::IndexAccess { + line, column: col, .. + } + | Expression::MethodCall { + line, column: col, .. + } => (*line, *col), _ => (token.line, token.column), };src/interpreter/mod.rs (2)
6224-6231: Update comment or intent now thatspawn_processis argument‑basedThis test now always passes explicit args with
use_shell = false, so the “no args = shell execution” part of the preceding comment is stale. Either update the comment to describe the argument‑based path, or, if you meant to exercise the shell‑execution branch here, switch touse_shell = trueand/or the no‑args form.
6254-6261: Same comment drift intest_wait_for_process_completionHere too, you’re exercising direct, argument‑based
spawn_processon all platforms, but the comment still talks about “no args = shell execution”. Consider aligning the comment with the current behavior (or adjusting the test if shell execution was the original intent).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.claude/settings.local.jsonsrc/interpreter/mod.rssrc/interpreter/tests.rssrc/parser/expr/primary.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- .claude/settings.local.json
🧰 Additional context used
📓 Path-based instructions (4)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingcargo fmt --all(see.rustfmt.toml)
Lint clean: runcargo clippy --all-targets --all-features -- -D warningswith no warnings
Usesnake_casefor function and file names in Rust
UseCamelCasefor types and traits in Rust
UseSCREAMING_SNAKE_CASEfor constants in Rust
ReviewSECURITY.md; avoid logging secrets and prefer zeroization for sensitive data in Rust code
**/*.rs: Use Rust Edition 2024 for all Rust code in this project
Format all Rust code with 'cargo fmt --all'
Run 'cargo clippy --all-targets --all-features -- -D warnings' to enforce linting rules
Files:
src/interpreter/tests.rssrc/parser/expr/primary.rssrc/interpreter/mod.rs
src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Component documentation required for all major modules
Files:
src/interpreter/tests.rssrc/parser/expr/primary.rssrc/interpreter/mod.rs
src/interpreter/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
src/interpreter/**/*.rs: Interpreter must be async-capable using Tokio runtime
Interpreter subprocess handling must include security sanitization for commands
Files:
src/interpreter/tests.rssrc/interpreter/mod.rs
src/parser/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
src/parser/**/*.rs: Update bytecode when modifying the parser
Parser must use recursive descent parsing with natural language constructs and error recovery
Files:
src/parser/expr/primary.rs
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.972Z
Learning: Applies to wfl-lsp/**/*.rs : Implement LSP server support in wfl-lsp workspace member using tower-lsp
📚 Learning: 2025-12-08T16:25:57.972Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T16:25:57.972Z
Learning: Applies to src/interpreter/**/*.rs : Interpreter must be async-capable using Tokio runtime
Applied to files:
src/interpreter/tests.rssrc/interpreter/mod.rs
🧬 Code graph analysis (1)
src/interpreter/tests.rs (2)
src/parser/mod.rs (1)
new(27-32)src/lexer/mod.rs (1)
lex_wfl_with_positions(88-226)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Integration Tests (windows-latest)
- GitHub Check: Build, Test, Clippy
🔇 Additional comments (3)
src/interpreter/tests.rs (1)
229-250: LGTM! Well-structured test for zero-argument native function calls.The test correctly validates the new explicit parentheses syntax for zero-argument native functions (
random()). It follows the established testing patterns in the file, uses proper async handling with Tokio, and includes appropriate range assertions and error messages.src/interpreter/mod.rs (2)
4897-4909: Variable fast‑path inFunctionCallcorrectly bypasses zero‑arg auto‑callDirectly resolving
Expression::Variableviaenv.borrow().get(name)here cleanly sidesteps the auto‑call behavior ofExpression::Variable, sofoo()now works for zero‑arg native and user‑defined functions without double invocation, while preserving existing behavior for non‑variable call targets and improving the error message span. Looks good.
6133-6138: Cross‑platformexecute_commandtest setup looks solidThe cfg‑split between
"cmd" /C echo helloon Windows andecho helloelsewhere, combined withuse_shell = falseand explicit args, correctly exercises the argument‑based safe path inIoClient::execute_commandacross platforms.
Resolved merge conflicts by: - Using main's disable_auto_call implementation for function calls (prevents state leakage) - Keeping all test cases from both branches (testing native and user-defined zero-arg functions) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
The test was for functionality that doesn't exist in main yet.
Fixes compiler warning about unreachable pattern that resulted from merge.
Fixes issue where builtin functions with multiple signatures (like 'length' for both Text and List) were only seeing the first registered signature. The type checker now checks if a function is builtin BEFORE looking up its symbol, and returns Type::Any parameters to accept any compatible type. This fixes the split_functionality integration tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 43 out of 45 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| The following **frozen specification** for WFLHASH1 incorporates all required corrections: explicit MAC key parameter mixing, derived key absorption into the message length, salt zero-padding, and standard HKDF usage. | ||
|
|
||
| Following the specification is the corrected **JSON Test Suite**, where all repeated input patterns have been expanded into valid hex strings for direct conformance testing. |
There was a problem hiding this comment.
The opening paragraph mentions 'all required corrections' and 'corrected JSON Test Suite' without explaining what was previously incorrect. For a frozen specification document, this phrasing suggests iterative fixes rather than a canonical reference. Consider removing references to corrections and presenting this as the authoritative specification from the start.
| The following **frozen specification** for WFLHASH1 incorporates all required corrections: explicit MAC key parameter mixing, derived key absorption into the message length, salt zero-padding, and standard HKDF usage. | |
| Following the specification is the corrected **JSON Test Suite**, where all repeated input patterns have been expanded into valid hex strings for direct conformance testing. | |
| The following **frozen specification** for WFLHASH1 defines explicit MAC key parameter mixing, derived key absorption into the message length, salt zero-padding, and standard HKDF usage. | |
| Following the specification is the canonical **JSON Test Suite**, where all repeated input patterns have been expanded into valid hex strings for direct conformance testing. |
| - [WFL MCP User Guide](../../Docs/guides/wfl-mcp-guide.md) | ||
| - [WFL MCP API Reference](../../Docs/guides/wfl-mcp-api-reference.md) |
There was a problem hiding this comment.
The relative paths to documentation files use inconsistent casing - 'Docs' with capital 'D'. Verify that this matches the actual directory structure. Some systems are case-sensitive and this could lead to broken links.
| - [WFL MCP User Guide](../../Docs/guides/wfl-mcp-guide.md) | |
| - [WFL MCP API Reference](../../Docs/guides/wfl-mcp-api-reference.md) | |
| - [WFL MCP User Guide](../../docs/guides/wfl-mcp-guide.md) | |
| - [WFL MCP API Reference](../../docs/guides/wfl-mcp-api-reference.md) |
Removes unnecessary newlines to simplify the assignment of a builtin function's return type.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/split_functionality.rs (1)
28-51: Duplicated helper function.This
get_wfl_binary_path()helper is duplicated across at least five test files (file_io_windows_sync_errors_test.rs, modulo_operator_test.rs, zero_arg_action_error_propagation_test.rs, subprocess_cleanup_test.rs, and this file). Extract it to a shared test utility module (e.g.,tests/common.rsortests/utils/mod.rs) to eliminate duplication and simplify maintenance.🤖 Prompt for AI Agents
Create tests/common.rs with: pub use std::env; pub use std::path::PathBuf; pub fn get_wfl_binary_path() -> PathBuf { let current_dir = env::current_dir().unwrap(); let release_path = if cfg!(target_os = "windows") { current_dir.join("target/release/wfl.exe") } else { current_dir.join("target/release/wfl") }; if release_path.exists() { return release_path; } let debug_path = if cfg!(target_os = "windows") { current_dir.join("target/debug/wfl.exe") } else { current_dir.join("target/debug/wfl") }; if debug_path.exists() { return debug_path; } panic!("WFL binary not found. Run 'cargo build' or 'cargo build --release' first."); } Then in each test file (split_functionality.rs, file_io_windows_sync_errors_test.rs, modulo_operator_test.rs, zero_arg_action_error_propagation_test.rs, subprocess_cleanup_test.rs): - Remove the local get_wfl_binary_path() function - Add at the top: mod common; - Replace calls with: common::get_wfl_binary_path() - Remove std::env and std::path::PathBuf imports if only used for this helper
🧹 Nitpick comments (2)
src/typechecker/mod.rs (1)
2159-2195: Remove redundant builtin function checks.The early-return path at lines 2166-2168 already handles all builtin functions, so the subsequent checks for builtins at lines 2175 and 2181-2183 are unreachable. This creates dead code and unnecessary complexity.
🔎 Proposed simplification
Expression::ActionCall { name, arguments, line: _line, column: _column, } => { // For builtin functions, use special handling (variadic support, etc.) if Analyzer::is_builtin_function(name) { return self.get_builtin_function_type(name, arguments.len()); } let symbol_opt = self.analyzer.get_symbol(name); if symbol_opt.is_none() { - // Check if this is an action parameter, builtin function, or special function name before reporting it as undefined + // Check if this is an action parameter or special function name before reporting it as undefined if self.analyzer.get_action_parameters().contains(name) - || Analyzer::is_builtin_function(name) || name == "helper_function" || name == "nested_function" { - // It's an action parameter or a special function name, so don't report an error - // For builtin functions, return their proper type - if Analyzer::is_builtin_function(name) { - return self.get_builtin_function_type(name, arguments.len()); - } + // It's an action parameter or a special function name, so don't report an error return Type::Unknown; } else { self.type_error(src/analyzer/mod.rs (1)
500-505: Refactor to avoid unnecessary cloning in scope comparisons.The pattern
Rc::try_unwrap(parent_rc.clone())will always fail to unwrap becauseparent_rc.clone()creates a temporary reference, forcing a clone of the innerScope. Since the outer scope is only used for read-only comparison (callingresolve()), you can avoid this clone by borrowing the Rc directly:🔎 Proposed refactor
- // Recover the outer scope Rc for variable tracking - let outer_scope = if let Some(parent_rc) = &then_scope.parent { - Rc::try_unwrap(parent_rc.clone()).unwrap_or_else(|rc| (*rc).clone()) - } else { - Scope::new() // Shouldn't happen, but provide fallback - }; - - for (name, symbol) in &then_scope.symbols { - if outer_scope.resolve(name).is_none() { + // Check symbols against parent scope (read-only) + for (name, symbol) in &then_scope.symbols { + if let Some(parent_rc) = &then_scope.parent { + if parent_rc.resolve(name).is_none() { + defined_in_then.push((name.clone(), symbol.clone())); + } + } else { defined_in_then.push((name.clone(), symbol.clone())); } }Apply the same pattern for the else block on lines 530-535.
Also applies to: 530-535
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
CLAUDE.mdsrc/analyzer/mod.rssrc/interpreter/tests.rssrc/typechecker/mod.rstests/split_functionality.rstests/subprocess_cleanup_test.rs
✅ Files skipped from review due to trivial changes (1)
- src/interpreter/tests.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/subprocess_cleanup_test.rs
- CLAUDE.md
🧰 Additional context used
📓 Path-based instructions (4)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code according to cargo fmt standards
Ensure Rust code passes clippy linter with no warnings (cargo clippy --all-targets --all-features -- -D warnings)
**/*.rs: Lint clean: runcargo clippy --all-targets --all-features -- -D warningswith no violations
Usesnake_casefor function and file names
UseCamelCasefor types and traits
UseSCREAMING_SNAKE_CASEfor constants
Files:
tests/split_functionality.rssrc/analyzer/mod.rssrc/typechecker/mod.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (AGENTS.md)
Format Rust code with
cargo fmt --allfollowing Rust 2024 edition conventions (see.rustfmt.toml)
Files:
tests/split_functionality.rssrc/analyzer/mod.rssrc/typechecker/mod.rs
tests/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Write failing tests first (TDD is mandatory); place unit/integration tests in
tests/directory
Files:
tests/split_functionality.rs
src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.rs: Update bytecode implementation when modifying parser features
Use zeroize and subtle crates for cryptographic security operations
Use hkdf and sha2 crates for key derivation in crypto operations
Files:
src/analyzer/mod.rssrc/typechecker/mod.rs
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T14:20:06.311Z
Learning: Applies to **/*.wfl : Use comprehensive try/when/otherwise error handling in WFL programs
📚 Learning: 2025-08-12T17:35:32.346Z
Learnt from: logbie
Repo: WebFirstLanguage/wfl PR: 150
File: tests/file_io_modes_test.rs:0-0
Timestamp: 2025-08-12T17:35:32.346Z
Learning: In the WFL project tests, file_io_execution_test.rs contains tests that actually execute WFL code through the interpreter and create real files, while file_io_modes_test.rs contains parser-only tests that don't execute code or create files during parsing.
Applied to files:
tests/split_functionality.rs
📚 Learning: 2026-01-02T14:20:06.311Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T14:20:06.311Z
Learning: Applies to src/interpreter/**/*.rs : Utilize Tokio runtime for built-in async/await support in WFL
Applied to files:
tests/split_functionality.rs
📚 Learning: 2026-01-02T14:20:28.752Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T14:20:28.752Z
Learning: Maintain backward compatibility: do not break existing WFL programs; run all `TestPrograms/` before merging
Applied to files:
tests/split_functionality.rs
📚 Learning: 2026-01-02T14:20:28.752Z
Learnt from: CR
Repo: WebFirstLanguage/wfl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T14:20:28.752Z
Learning: Applies to tests/**/*_test.rs : Use feature-oriented test file names following the pattern `*_test.rs`
Applied to files:
tests/split_functionality.rs
🧬 Code graph analysis (2)
tests/split_functionality.rs (5)
tests/subprocess_cleanup_test.rs (4)
path(21-23)get_wfl_binary_path(26-49)run_wfl(51-69)new(14-19)tests/pattern_replace_test.rs (3)
path(19-21)run_wfl(24-46)new(12-17)tests/file_io_windows_sync_errors_test.rs (1)
get_wfl_binary_path(13-36)tests/modulo_operator_test.rs (1)
get_wfl_binary_path(9-32)tests/zero_arg_action_error_propagation_test.rs (1)
get_wfl_binary_path(11-34)
src/typechecker/mod.rs (2)
src/analyzer/mod.rs (4)
is_builtin_function(327-329)new(79-84)new(144-150)new(180-325)src/builtins.rs (2)
is_builtin_function(204-206)get_function_arity(215-312)
🪛 GitHub Actions: CI
src/typechecker/mod.rs
[error] 1526-1529: Cargo fmt check failed. Expected formatting changes: convert multi-line return_type into a single-line expression. Run 'cargo fmt' to fix formatting.
🔇 Additional comments (8)
tests/split_functionality.rs (1)
58-60: Good improvement to use dynamic path resolution.Replacing the hardcoded binary path with
get_wfl_binary_path()makes the test more flexible and robust across debug/release builds. Once the helper is extracted to a shared module (see comment on lines 28-51), this usage will be cleaner with a simplecommon::get_wfl_binary_path()call.src/typechecker/mod.rs (1)
1518-1560: LGTM: Centralized builtin function handling.The refactoring correctly consolidates builtin function type resolution into a dedicated early-return path (lines 1523-1533), preventing conflicts with symbol table lookups and properly handling overloaded builtins like
lengthby usingType::Anyfor parameters. The special-casing ofloopcounterandcount(lines 1519-1521) is appropriate and avoids unnecessary symbol lookups.src/analyzer/mod.rs (6)
4-4: Well-documented performance optimization.The migration from
Box<Scope>toRc<Scope>is well-motivated and clearly documented. The O(N²) to O(N) complexity improvement for deeply nested scopes is a solid optimization.Also applies to: 60-70, 86-90
566-598: LGTM: Correct Rc-based scope management.The scope management in
SingleLineIf,ForEachLoop,CountLoop, andWhileLoopcorrectly uses the Rc pattern: wrap in Rc, create child scope, restore parent viaRc::try_unwrap. The fallback to clone is appropriate for these constructs.Also applies to: 600-638, 640-687, 689-706
742-793: LGTM: Correct scope propagation in WaitForStatement.The WaitForStatement correctly creates a child scope, analyzes the inner statement, and propagates symbols (like file handles) back to the parent scope using Rc-based management.
800-863: LGTM: Correct Rc-based scope management in TryStatement.The try/when/otherwise blocks correctly create isolated child scopes using Rc parents and restore the outer scope appropriately.
1462-1511: LGTM: Correct Rc-based scope management in action bodies.The action body analysis correctly creates an isolated scope for parameters and body statements, and properly restores the outer scope after analysis.
1606-1615: LGTM: Clean abstraction for scope management.The
push_scopeandpop_scopehelper methods provide a clean, consistent interface for Rc-based scope management, used by container method analysis.
Collapse nested if-let statements using && chaining as suggested by clippy. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 43 out of 45 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| eprintln!( | ||
| "WARNING: WFL crypto functions are experimental and provide no security guarantees. USE AT OWN RISK." | ||
| ); |
There was a problem hiding this comment.
The warning message is printed to stderr on every call to native_wflhash256. This could spam stderr during normal usage. Consider logging this warning once during initialization, using a static flag, or documenting this behavior in the function's documentation rather than printing on every invocation.
| } | ||
|
|
||
| /// Run the MCP server on stdin/stdout | ||
| pub async fn run_server() -> Result<(), Box<dyn std::error::Error>> { |
There was a problem hiding this comment.
The function is marked as async but doesn't perform any async operations. The entire implementation is synchronous (reading from stdin, processing, writing to stdout). Consider removing the async keyword and adjusting the caller in main.rs accordingly to simplify the code.
| pub async fn run_server() -> Result<(), Box<dyn std::error::Error>> { | |
| pub fn run_server() -> Result<(), Box<dyn std::error::Error>> { |
| let reader = BufReader::new(stdout); | ||
|
|
||
| // Helper function to send request and read response | ||
| let mut send_request = |request: Value| -> Result<Value, Box<dyn std::error::Error>> { |
There was a problem hiding this comment.
The closure captures reader by reference but attempts to iterate over it with for line in reader.by_ref().lines(). This will fail because reader is moved into the closure on first call and cannot be reused. Each call to send_request will only work once. Consider refactoring to pass reader as a mutable parameter or restructure the client to handle the response reading differently.
| eprintln!( | ||
| "WARNING: WFL crypto functions are experimental and provide no security guarantees. USE AT OWN RISK." | ||
| ); |
There was a problem hiding this comment.
Similar to Comment 2, this warning is printed on every call to native_wflhash512. Consider consolidating warning emissions or using a once_cell/lazy_static pattern to warn once per session.
| # WFL MCP Server Test Report | ||
|
|
||
| **Date:** January 2, 2026 | ||
| **Version:** wfl-lsp v0.1.0 |
There was a problem hiding this comment.
The version number should be verified to match the actual version in Cargo.toml. Consider using a build-time constant or script to ensure version consistency between documentation and code.
| **Version:** wfl-lsp v0.1.0 | |
| **Version:** wfl-lsp (see Cargo.toml) |
This introduces a Model Context Protocol (MCP) server to enable AI assistants, like Claude, to interact with WFL codebases. The
wfl-lspbinary can now be run with a--mcpflag to expose a suite of tools and workspace resources over a JSON-RPC interface.Key Features & Improvements
Rc). This improves performance on deeply nested code by avoiding expensive cloning and preventing quadratic complexity.wflhashimplementation with a secure, buffered sponge construction, adds DoS protection, and formalizes the algorithm in a new specification document.input()function to get user input from the console.catch:to the more explicitwhen error:.countloops.Additionally, the integration test runner has been refactored to use PowerShell Jobs for more reliable timeout handling.
Summary by CodeRabbit
New Features
Bug Fixes
catch:towhen error:Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.