Skip to content

feat: add execute file statement for in-process WFL page execution - #539

Merged
logbie merged 6 commits into
mainfrom
claude/cool-fermat-u520fm
Jun 12, 2026
Merged

feat: add execute file statement for in-process WFL page execution#539
logbie merged 6 commits into
mainfrom
claude/cool-fermat-u520fm

Conversation

@logbie

@logbie logbie commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds the ability for a WFL program (typically a web server) to execute another .wfl file in-process and capture its display output, enabling PHP-style dynamic pages:

execute [wfl] file at <path> [with <request>] [and read output as <variable>]

Example: serving dynamic WFL pages

listen on port 8080 as web_server
main loop:
    wait for request comes in on web_server as req
    try:
        execute wfl file at "pages/home.wfl" with req and read output as page_output
        respond to req with page_output and content_type "text/html"
    when file not found:
        respond to req with "Page not found" and status 404
    when error:
        respond to req with "Server error" and status 500
    end try
end loop

The executed page is a normal WFL program. With with req it sees the same request variables a server sees (method, path, client_ip, body, headers), and everything it displays is captured into the output variable instead of printed.

Implementation

  • New ExecuteFileStatement parsed entirely from existing keywords (no new tokens); syntax follows WFL's natural-language principles, mirroring open file at ... and read content as ...
  • Nested interpreter per execution with a fresh environment and stdlib, inheriting the parent's config; paths resolve relative to the executing file's directory (like load module)
  • Output capture via a new thread-local capture stack (src/interpreter/io_capture.rs) that display/print now route through; capture nests correctly so pages can execute layout/partial pages
  • Depth guard (4 levels) prevents circular execution; the limit is stack-bound (verified empirically — deeper nesting overflows the thread stack in debug builds before a larger guard could fire)
  • Catchable errors: missing file, parse errors, and runtime errors in the executed page become catchable parent errors with the page path in the message; when file not found matches missing pages, so one broken page cannot crash the server
  • Request object enrichment: request objects from wait for request now also carry method, path, client_ip, body, headers properties (additive and backward compatible; respond to is unchanged)

Testing (TDD)

Tests were written first and confirmed failing before implementation:

  • 23 Rust tests in tests/execute_file_test.rs: parse shapes, output capture, nested capture, request context, error catchability, depth guard, relative path resolution, and a true end-to-end test that starts a WFL server and asserts a real HTTP response contains the executed page's output
  • TestPrograms/execute_wfl_file.test.wfl: WFL-native test framework suite (3 tests, green under wfl --test)
  • TestPrograms/execute_wfl_file.wfl + TestPrograms/execute_pages/: end-to-end program for the integration suite
  • Full suite: 1019 passed / 0 failed; cargo fmt and cargo clippy -D warnings clean
  • Integration script failures (21) were verified pre-existing by rebuilding plain main and getting the identical list
  • Docs examples validated live against the release binary, including a multi-request server session exercising the 404 path

Docs

  • Docs/04-advanced-features/web-servers.md: new "Serving Dynamic WFL Pages" section
  • Docs/04-advanced-features/subprocess-execution.md: "Executing WFL Files In-Process" section
  • Docs/reference/syntax-reference.md: statement forms
  • CHANGELOG.md: Unreleased entry

Notes (pre-existing issues spotted, not addressed here)

  • Older examples in web-servers.md use as server, which fails to parse (server is a reserved keyword)
  • scripts/run_web_tests.sh aborts immediately due to set -e + ((counter++))

https://claude.ai/code/session_011z74V37zumkdS8tUwohjhV


Generated by Claude Code

Summary by CodeRabbit

  • New Features

    • In-process WFL file execution with optional request-context passing
    • Capture displayed output from executed files into variables
    • Catchable execution errors (including missing files) and a nesting-depth guard
  • Documentation

    • New advanced docs for subprocess execution and dynamic WFL page serving
    • Syntax reference examples added; web-server examples updated to use content_type
  • Tests

    • Comprehensive tests and example programs covering execution, capture, errors, nesting, and server flow

Adds the ability for a WFL program (typically a web server) to execute
another .wfl file in-process and capture its display output, enabling
PHP-style dynamic pages:

    execute [wfl] file at <path> [with <request>] [and read output as <var>]

- New ExecuteFileStatement parsed from existing keywords (no new tokens)
- Nested interpreter per execution with a fresh environment and stdlib,
  inheriting the parent's config; depth guard (4 levels) prevents
  circular execution
- Thread-local output-capture stack (src/interpreter/io_capture.rs)
  routes display/print output into a buffer with correct nesting
- Optional `with <request>` passes HTTP request context: the executed
  file sees method, path, client_ip, body and headers like a server does
- Request objects from `wait for request` now also carry those
  properties (backward compatible; respond still uses _response_sender)
- Child errors (missing file, parse, runtime) surface as catchable
  parent errors with the child path in the message; `when file not
  found` matches missing pages
- Paths resolve relative to the executing file's directory, like
  load module
- Tests: 23 Rust tests incl. reqwest end-to-end web serving,
  TestPrograms e2e + WFL test framework suite; docs + CHANGELOG updated

https://claude.ai/code/session_011z74V37zumkdS8tUwohjhV
Copilot AI review requested due to automatic review settings June 12, 2026 11:02
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@logbie, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 40 minutes and 4 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0357daed-ba7d-4547-ac5d-4877db6f9053

📥 Commits

Reviewing files that changed from the base of the PR and between 1c4119e and 3f27ff8.

📒 Files selected for processing (2)
  • src/interpreter/mod.rs
  • tests/execute_file_test.rs
📝 Walkthrough

Walkthrough

Adds an in-process execute file statement with optional request-context passing and output capture, thread-local nested output capture, interpreter depth guard, analyzer/typechecker support, transpiler rejection, extensive tests, and documentation/examples for dynamic .wfl page serving.

Changes

Execute File Feature Implementation

Layer / File(s) Summary
AST and Parsing
src/parser/ast.rs, src/parser/mod.rs, src/parser/stmt/processes.rs
New ExecuteFileStatement AST variant and parser for execute [wfl] file at <path> [with <request>] [and read output as <variable>].
JS Transpiler Rejection
src/transpiler/javascript.rs
Transpiler rejects ExecuteFileStatement with a TranspileError (includes source location).
Thread-Local Output Capture Stack
src/interpreter/io_capture.rs
RAII-guarded thread-local capture stack (push_capture, CaptureGuard, emit_line) with unit tests for nesting semantics.
Output Routing Through Capture
src/interpreter/mod.rs, src/stdlib/core.rs
Route native_display, DisplayStatement, and native_print through io_capture::emit_line instead of printing to stdout.
Interpreter Execution & Request Plumbing
src/interpreter/mod.rs
Depth-guarded nested-interpreter execution: resolve/canonicalize child path, clone/inject request properties, seed semantic analysis for injected globals, run child in fresh interpreter with optional capture, wrap child errors to parent site; refactor WaitForRequestStatement to construct headers once and inject request variables before registering response sender.
Analyzer, TypeChecker, Static Analyzer
src/analyzer/mod.rs, src/typechecker/mod.rs, src/analyzer/static_analyzer.rs
Analyzer defines captured-output variable; type checker validates path expression and sets captured variable type Text; unused-variable analysis marks ExecuteFileStatement usages.
WFL Test Programs and Rust Test Suite
TestPrograms/execute_pages/hello_page.wfl, TestPrograms/execute_pages/dynamic_page.wfl, TestPrograms/execute_wfl_file.test.wfl, TestPrograms/execute_wfl_file.wfl, tests/execute_file_test.rs
WFL example pages and comprehensive Rust tests covering parsing, capture, nesting, request context, error handling, depth limits, path resolution, pass-through output, and an end-to-end web-server test.
Documentation and CHANGELOG
CHANGELOG.md, Docs/04-advanced-features/subprocess-execution.md, Docs/04-advanced-features/web-servers.md, Docs/reference/syntax-reference.md
CHANGELOG entry and docs: in-process execution docs, web-server dynamic page serving section (with execute ... with req and read output as ...), syntax reference examples, and content_type keyword fixes in examples.

Sequence Diagram

sequenceDiagram
  participant ParentInterpreter
  participant FS as FileSystem
  participant CaptureStack
  participant ChildInterpreter
  ParentInterpreter->>ParentInterpreter: evaluate path & request expr
  ParentInterpreter->>FS: canonicalize path (relative to parent)
  ParentInterpreter->>CaptureStack: push_capture(buffer) [if var provided]
  ParentInterpreter->>ChildInterpreter: create child with injected request vars, increment execute_depth
  ParentInterpreter->>ChildInterpreter: parse & interpret child file
  ChildInterpreter->>CaptureStack: emit_line (display/print -> buffer or stdout)
  ChildInterpreter-->>ParentInterpreter: exit (or error)
  ParentInterpreter->>CaptureStack: pop_capture (via CaptureGuard drop)
  ParentInterpreter->>ParentInterpreter: define captured variable (if any) or pass-through output
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • WebFirstLanguage/wfl#262: Related to JavaScript transpiler infrastructure and prior transpiler handling referenced by this change.

"I'm a rabbit in the code, hopping through the night,
I nest your prints in buffers snug and tight.
Requests arrive, I pass them through with care,
Pages render cleanly from an in-process lair.
Hooray for captured output and webs that light the air!"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main feature added: an execute file statement that enables in-process WFL page execution, which aligns with the primary changeset objective.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/cool-fermat-u520fm

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an in-process execute file / execute wfl file statement to the WFL interpreter, enabling PHP-style dynamic page execution with optional HTTP request context injection and captured display/print output.

Changes:

  • Introduces ExecuteFileStatement across AST/parser/analyzer/typechecker/interpreter, including depth guarding and relative-path resolution.
  • Adds a thread-local output capture stack so display/print can be redirected into a variable (with correct nesting).
  • Expands web-server request objects to include method, path, client_ip, body, and headers, plus adds docs and end-to-end tests.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/parser/ast.rs Adds ExecuteFileStatement to the AST.
src/parser/mod.rs Dispatches execute to either execute file or existing execute command.
src/parser/stmt/processes.rs Implements parsing for execute [wfl] file at ... [with ...] [and read output as ...].
src/analyzer/mod.rs Analyzes ExecuteFileStatement and defines the capture output variable as Text when present.
src/typechecker/mod.rs Adds basic type checking for execute-file path/request expressions.
src/interpreter/io_capture.rs Implements thread-local nested output capture for program output.
src/interpreter/mod.rs Routes output through capture, enriches request objects, and executes nested interpreters for execute file.
src/stdlib/core.rs Routes native print output through the capture mechanism.
src/transpiler/javascript.rs Rejects ExecuteFileStatement during JS transpilation with a clear error.
tests/execute_file_test.rs Adds Rust-level TDD tests including nested capture, request context, errors, and an HTTP end-to-end test.
TestPrograms/execute_wfl_file.wfl Adds WFL end-to-end program exercising execute-file behavior.
TestPrograms/execute_wfl_file.test.wfl Adds WFL test-framework coverage for execute-file behavior.
TestPrograms/execute_pages/hello_page.wfl Adds a simple executed page used by tests.
TestPrograms/execute_pages/dynamic_page.wfl Adds a request-context-using page used by tests.
Docs/reference/syntax-reference.md Documents the new statement forms.
Docs/04-advanced-features/web-servers.md Documents serving dynamic WFL pages via execute file.
Docs/04-advanced-features/subprocess-execution.md Documents in-process WFL execution vs execute command.
CHANGELOG.md Adds an Unreleased entry describing the new capability.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/interpreter/mod.rs Outdated
Comment thread src/interpreter/mod.rs Outdated
["method", "path", "client_ip", "body", "headers"]
.iter()
.filter_map(|key| {
props.get(*key).map(|v| ((*key).to_string(), v.clone()))
Comment thread tests/execute_file_test.rs Outdated
)
.expect("Failed to write page file");

let port = 8123;
Comment thread tests/execute_file_test.rs Outdated
Comment on lines +461 to +465
tokio::time::sleep(std::time::Duration::from_millis(500)).await;

let response = reqwest::get(format!("http://127.0.0.1:{port}/welcome"))
.await
.expect("Failed to send request");

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a49d4206de

ℹ️ 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".

self.expect_token(Token::KeywordFile, "Expected 'file' after 'execute'")?;
self.expect_token(Token::KeywordAt, "Expected 'at' after 'execute file'")?;

let path = self.parse_primary_expression()?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Parse full path expressions before request context

Because the path is parsed as only a primary expression, any inline dynamic path that uses WFL's with concatenation (for example execute wfl file at "pages/" with page_name and read output as out) is split as path "pages/" plus request context page_name. At runtime this then tries to execute the wrong path or rejects the string as a non-request object, even though the new syntax and type checker model the path as an expression. Please parse the path up to an unambiguous request-context delimiter so dynamic page paths work.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentional, matching how other WFL statements treat their clause delimiters (respond to parses its content as a primary expression so and status ... stays unambiguous). A with directly after the path is the request-context delimiter; inline concatenation there would make with irrecoverably ambiguous since both forms accept arbitrary expressions. Dynamic paths use the standard pattern of building the path into a variable first:

store page_path as "pages/" with page_name with ".wfl"
execute wfl file at page_path with req and read output as page_output

Documented this in web-servers.md and added a regression test (test_execute_file_dynamic_path_via_variable) in 98f8574.


Generated by Claude Code

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/interpreter/mod.rs (1)

4937-5014: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid registering the pending response before all injected request variables are known to succeed.

Line 4940 stores the oneshot sender before Lines 4980-5012 finish defining request_name, method, path, client_ip, body, and headers. If any of those define(...) calls fails, the interpreter returns with the request still parked in pending_responses, so the HTTP handler has no response path and the client hangs. Preflight the full name set first, or roll back pending_responses on every failure path.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/interpreter/mod.rs` around lines 4937 - 5014, The pending response sender
is inserted into pending_responses (pending_responses.insert at the top) before
defining request variables (env.define calls for request_name, "method", "path",
"client_ip", "body", "headers"), so if any define fails the request remains
parked and the client hangs; fix by delaying the insert until after all
env.define calls succeed (i.e., construct headers_object and request_object and
call env.define for request_name and each property, then insert into
pending_responses), or if you prefer to keep the current order, ensure you
remove the entry from pending_responses on every error path (call
pending_responses.remove(request.id) before returning
Err(RuntimeError::new(...))) so the oneshot sender is never left unhandled.
🧹 Nitpick comments (3)
tests/execute_file_test.rs (2)

432-432: 💤 Low value

Hardcoded port may cause conflicts in parallel test execution.

Port 8123 is hardcoded, which could cause test failures if multiple test runs execute concurrently (e.g., in CI or with multiple developers). While WFL's listen on port statement requires a specific port number, consider using a less common high-numbered port (e.g., 58123) to reduce collision likelihood.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/execute_file_test.rs` at line 432, The test hardcodes port = 8123 which
can conflict when tests run concurrently; update the declaration of the port
variable (the let port binding in tests/execute_file_test.rs) to use a less
common high-numbered port (e.g., 58123) or otherwise pick an ephemeral/high port
to reduce collisions, then run the test suite to verify no port conflicts occur.

461-461: ⚡ Quick win

Fixed 500ms sleep may cause flaky test failures.

The test sleeps for a fixed 500ms before sending the HTTP request, assuming the server will be ready. On slow CI systems or under load, the server might take longer to start, causing connection-refused failures.

Consider implementing a retry loop with a reasonable timeout:

♻️ Suggested improvement with retry logic
-    tokio::time::sleep(std::time::Duration::from_millis(500)).await;
+    // Wait for server to be ready with retry logic
+    let mut retries = 0;
+    loop {
+        tokio::time::sleep(std::time::Duration::from_millis(100)).await;
+        if reqwest::get(format!("http://127.0.0.1:{port}/"))
+            .await
+            .is_ok()
+        {
+            break;
+        }
+        retries += 1;
+        if retries > 20 {
+            panic!("Server failed to start within 2 seconds");
+        }
+    }
 
     let response = reqwest::get(format!("http://127.0.0.1:{port}/welcome"))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/execute_file_test.rs` at line 461, The fixed
tokio::time::sleep(std::time::Duration::from_millis(500)).await is brittle and
can cause flaky failures; replace it with a bounded retry loop that attempts to
connect to the server (e.g., using reqwest::Client::get or TcpStream connect)
until success or a timeout (e.g., total 5–10s) with short backoff between
attempts; update the test to stop sleeping once a probe succeeds and proceed to
send the HTTP request, referencing the existing tokio::time::sleep and
std::time::Duration::from_millis(500) locations to find and replace the
behaviour.
src/interpreter/io_capture.rs (1)

40-54: 💤 Low value

Consider cloning the Rc to clarify borrow lifetimes.

The current pattern stack.borrow().last() relies on temporary lifetime extension to keep the Ref guard alive while buffer is used inside the if let block. While correct (the borrow checker validates it), cloning the Rc makes the lifetime semantics explicit and improves readability.

♻️ Clearer alternative
 pub(crate) fn emit_line(line: &str) {
-    let captured = CAPTURE_STACK.with(|stack| {
-        if let Some(buffer) = stack.borrow().last() {
-            let mut buffer = buffer.borrow_mut();
-            buffer.push_str(line);
-            buffer.push('\n');
-            true
-        } else {
-            false
-        }
-    });
-    if !captured {
+    let buffer_opt = CAPTURE_STACK.with(|stack| stack.borrow().last().cloned());
+    if let Some(buffer) = buffer_opt {
+        let mut buf = buffer.borrow_mut();
+        buf.push_str(line);
+        buf.push('\n');
+    } else {
         println!("{line}");
     }
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/interpreter/io_capture.rs` around lines 40 - 54, The borrow of the
thread-local CAPTURE_STACK in emit_line uses stack.borrow().last() which keeps a
temporary Ref alive and relies on subtle lifetime extension; change this to
clone the Rc before mutably borrowing to make lifetimes explicit: use
stack.borrow().last().cloned() (or clone the returned Rc into a buffer_rc
variable) and then call buffer_rc.borrow_mut() to push_str and push('\n'),
preserving the existing println fallback when captured is false.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Docs/04-advanced-features/web-servers.md`:
- Line 323: The example uses the incorrect underscore form `content_type` in the
snippet "respond to req with page_output and content_type \"text/html\""; update
this to the documented, consistent phrase "content type" so it reads "respond to
req with page_output and content type \"text/html\"" to match other examples
(e.g., the usage around lines 204-210).

In `@src/interpreter/mod.rs`:
- Around line 5538-5545: The code inserts
pending_responses.insert(request.id.clone(), request.response_sender) before
calling env_mut.define(...) in the WaitForRequestStatement handling, which can
leave the oneshot sender alive if any define(...) fails and cause the warp
handler to hang; fix by moving the pending_responses.insert(...) to after all
env_mut.define(...) calls succeed (or, alternatively, add an error-path guard
that removes the entry—pending_responses.remove(&request.id) or drops the
sender—before returning Err) so the sender is always removed/dropped on failure;
reference symbols: pending_responses, WaitForRequestStatement handling,
request.id / request.response_sender, and env_mut.define.

In `@src/typechecker/mod.rs`:
- Around line 1049-1071: The ExecuteFileStatement branch currently checks the
path/request types but never assigns the captured variable's type; after
handling request inference (inside the Statement::ExecuteFileStatement arm) set
the variable named by variable_name to Type::Text in the type environment (same
approach used by HttpGetStatement, ReadFileStatement, WaitForProcessStatement)
so downstream type checking can resolve it; locate the code handling
Statement::ExecuteFileStatement and set the symbol for variable_name in the type
context/stack to Type::Text (avoid changing existing error handling for
path/request).

---

Outside diff comments:
In `@src/interpreter/mod.rs`:
- Around line 4937-5014: The pending response sender is inserted into
pending_responses (pending_responses.insert at the top) before defining request
variables (env.define calls for request_name, "method", "path", "client_ip",
"body", "headers"), so if any define fails the request remains parked and the
client hangs; fix by delaying the insert until after all env.define calls
succeed (i.e., construct headers_object and request_object and call env.define
for request_name and each property, then insert into pending_responses), or if
you prefer to keep the current order, ensure you remove the entry from
pending_responses on every error path (call pending_responses.remove(request.id)
before returning Err(RuntimeError::new(...))) so the oneshot sender is never
left unhandled.

---

Nitpick comments:
In `@src/interpreter/io_capture.rs`:
- Around line 40-54: The borrow of the thread-local CAPTURE_STACK in emit_line
uses stack.borrow().last() which keeps a temporary Ref alive and relies on
subtle lifetime extension; change this to clone the Rc before mutably borrowing
to make lifetimes explicit: use stack.borrow().last().cloned() (or clone the
returned Rc into a buffer_rc variable) and then call buffer_rc.borrow_mut() to
push_str and push('\n'), preserving the existing println fallback when captured
is false.

In `@tests/execute_file_test.rs`:
- Line 432: The test hardcodes port = 8123 which can conflict when tests run
concurrently; update the declaration of the port variable (the let port binding
in tests/execute_file_test.rs) to use a less common high-numbered port (e.g.,
58123) or otherwise pick an ephemeral/high port to reduce collisions, then run
the test suite to verify no port conflicts occur.
- Line 461: The fixed
tokio::time::sleep(std::time::Duration::from_millis(500)).await is brittle and
can cause flaky failures; replace it with a bounded retry loop that attempts to
connect to the server (e.g., using reqwest::Client::get or TcpStream connect)
until success or a timeout (e.g., total 5–10s) with short backoff between
attempts; update the test to stop sleeping once a probe succeeds and proceed to
send the HTTP request, referencing the existing tokio::time::sleep and
std::time::Duration::from_millis(500) locations to find and replace the
behaviour.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 09188bfa-a02f-405a-83b4-f0cefc73c618

📥 Commits

Reviewing files that changed from the base of the PR and between 96ea493 and a49d420.

📒 Files selected for processing (18)
  • CHANGELOG.md
  • Docs/04-advanced-features/subprocess-execution.md
  • Docs/04-advanced-features/web-servers.md
  • Docs/reference/syntax-reference.md
  • TestPrograms/execute_pages/dynamic_page.wfl
  • TestPrograms/execute_pages/hello_page.wfl
  • TestPrograms/execute_wfl_file.test.wfl
  • TestPrograms/execute_wfl_file.wfl
  • src/analyzer/mod.rs
  • src/interpreter/io_capture.rs
  • src/interpreter/mod.rs
  • src/parser/ast.rs
  • src/parser/mod.rs
  • src/parser/stmt/processes.rs
  • src/stdlib/core.rs
  • src/transpiler/javascript.rs
  • src/typechecker/mod.rs
  • tests/execute_file_test.rs

Comment thread Docs/04-advanced-features/web-servers.md
Comment thread src/interpreter/mod.rs
Comment thread src/typechecker/mod.rs
claude added 2 commits June 12, 2026 11:25
- Map PermissionDenied I/O errors in execute file so `when permission
  denied` handlers work (Copilot)
- Deep-clone request context values injected into the child interpreter
  so the executed page cannot share mutable state with the parent's
  request object (Copilot)
- Register pending HTTP responses only after all request variables are
  defined, so a define failure can no longer leave the client hanging
  (CodeRabbit)
- Assign Type::Text to the captured output variable in the typechecker,
  matching HttpGet/ReadFile/WaitForProcess (CodeRabbit)
- Clone the Rc in io_capture::emit_line for explicit borrow lifetimes
  (CodeRabbit)
- Use uncommon port 58123 and a TCP readiness probe instead of a fixed
  500ms sleep in the web e2e test (Copilot/CodeRabbit)
- Document and test the dynamic-path pattern: `with` after the path is
  the request-context delimiter, so dynamic paths are built into a
  variable first (Codex)
- Fix pre-existing broken `content type` examples in web-servers.md:
  the two-word form does not parse; the correct syntax is `content_type`
  (verified against the release binary)

https://claude.ai/code/session_011z74V37zumkdS8tUwohjhV
- Mark execute_pages/dynamic_page.wfl with CI-SKIP: the CI runner walks
  TestPrograms recursively and ran this page fragment standalone, where
  its request-context variables are undefined
- Use forward slashes when embedding the temp file path into WFL source
  in the pass-through test: backslashes in Windows paths were lexed as
  escape sequences and broke the Windows integration job

https://claude.ai/code/session_011z74V37zumkdS8tUwohjhV
Copilot AI review requested due to automatic review settings June 12, 2026 11:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

Comment thread src/analyzer/mod.rs
Comment on lines +1455 to +1465
Statement::ExecuteFileStatement {
path,
request,
variable_name,
line,
column,
} => {
self.analyze_expression(path);
if let Some(request_expr) = request {
self.analyze_expression(request_expr);
}
Comment thread src/interpreter/mod.rs
- Surface a RuntimeError when std::env::current_dir() fails during
  execute file path resolution instead of silently falling back to the
  bare relative path, consistent with resolve_module_path()
- Teach the static analyzer's unused-variable pass about
  ExecuteFileStatement so variables used in path/request expressions
  (e.g. execute wfl file at page_path) are not reported as unused

https://claude.ai/code/session_011z74V37zumkdS8tUwohjhV

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/interpreter/mod.rs (1)

5452-5467: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Validate the request-object shape before cloning it.

This currently treats any Value::Object as a valid request context and silently drops missing fields via filter_map. If the caller passes the wrong object, the child interpreter runs with partial request globals or fails later with unrelated undefined variable / header-type errors instead of failing at the execute file ... with <request> boundary. Check that method, path, client_ip, body, and headers all exist and have the expected types before building request_vars.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/interpreter/mod.rs` around lines 5452 - 5467, The code builds
request_vars from any Value::Object and silently drops missing keys; update the
request validation inside evaluate_expression handling (where request_vars is
constructed) to verify the object has all required keys
"method","path","client_ip","body","headers" and that each key has the expected
type (e.g., method/path/client_ip as String, headers as Object/Map, body as
appropriate Value) before performing the deep_clone and building request_vars;
if any key is missing or has the wrong type, return a clear error at the
execute-file-with-request boundary (the same area that currently matches
Value::Object and calls deep_clone) so the caller fails fast with a descriptive
message instead of producing partial globals or later undefined-variable errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/interpreter/mod.rs`:
- Around line 5452-5467: The code builds request_vars from any Value::Object and
silently drops missing keys; update the request validation inside
evaluate_expression handling (where request_vars is constructed) to verify the
object has all required keys "method","path","client_ip","body","headers" and
that each key has the expected type (e.g., method/path/client_ip as String,
headers as Object/Map, body as appropriate Value) before performing the
deep_clone and building request_vars; if any key is missing or has the wrong
type, return a clear error at the execute-file-with-request boundary (the same
area that currently matches Value::Object and calls deep_clone) so the caller
fails fast with a descriptive message instead of producing partial globals or
later undefined-variable errors.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 78ed6d38-22c3-43cb-a11f-51a60b359047

📥 Commits

Reviewing files that changed from the base of the PR and between a49d420 and 1c4119e.

📒 Files selected for processing (7)
  • Docs/04-advanced-features/web-servers.md
  • TestPrograms/execute_pages/dynamic_page.wfl
  • src/analyzer/static_analyzer.rs
  • src/interpreter/io_capture.rs
  • src/interpreter/mod.rs
  • src/typechecker/mod.rs
  • tests/execute_file_test.rs
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/typechecker/mod.rs
  • src/interpreter/io_capture.rs
  • tests/execute_file_test.rs

Per CodeRabbit review: a wrong object passed via `with <request>` was
accepted silently with missing fields dropped, surfacing later as
confusing undefined-variable errors inside the executed file. The
request context is now validated upfront - all five request fields
(method, path, client_ip, body, headers) must exist with the expected
types, and a missing or mistyped field produces a clear error naming
it at the execute statement.

https://claude.ai/code/session_011z74V37zumkdS8tUwohjhV
Copilot AI review requested due to automatic review settings June 12, 2026 12:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

Comment thread src/interpreter/mod.rs
Comment on lines +5518 to +5529
let program = parser.parse().map_err(|errors| {
let first_error = errors.first();
RuntimeError::new(
format!(
"Parse error in executed file '{}': {}",
resolved_path.display(),
first_error.map(|e| e.message.as_str()).unwrap_or("unknown")
),
*line,
*column,
)
})?;
Per Copilot review: parse errors from an executed file mentioned the
child path but dropped the child's line/column, making template parse
failures hard to debug. The wrapped message now includes the child
parse error position, matching how runtime errors already report it.

https://claude.ai/code/session_011z74V37zumkdS8tUwohjhV
@logbie
logbie merged commit a276249 into main Jun 12, 2026
14 checks passed
@logbie
logbie deleted the claude/cool-fermat-u520fm branch June 12, 2026 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants