fix: pass loaded WflConfig to Interpreter (not just timeout)

[logbie]

Summary

Interpreter::with_timeout(config.timeout_seconds) in src/main.rs constructs a fresh
WflConfig with only the timeout field copied across — every other setting falls back
to WflConfig::default(). This means .wflcfg is parsed correctly, but almost
nothing from it actually reaches the interpreter.

The most user-visible symptom is that web_server_bind_address is silently ignored:
no matter what operators put in .wflcfg, listen on port N as ... binds 127.0.0.1
because that's the hard-coded default. WFL web servers are effectively loopback-only
in any production deployment that relies on .wflcfg to change the bind address.

Interpreter::with_config(Arc<WflConfig>) already exists — it's just not being called
from main.rs in the script-run path. This PR switches to it.

Patch

- let mut interpreter = Interpreter::with_timeout(config.timeout_seconds);
+ let mut interpreter = Interpreter::with_config(std::sync::Arc::new(config.clone()));

The .clone() is required because config is also used later in main.rs at
if config.logging_enabled (around line 1297). Without it you get E0382: use of moved value.

Repro

Before this change:

echo "web_server_bind_address = 0.0.0.0" > .wflcfg
cat > server.wfl <<'EOF'
listen on port 8080 as s
main loop:
    wait for request comes in on s as r
    respond to r with "hi" and content_type "text/plain"
end loop
EOF
cargo run --release -- server.wfl &
ss -tlnp | grep :8080
# LISTEN 0 128 127.0.0.1:8080  <-- wrong; should be 0.0.0.0:8080

After this change the same repro binds 0.0.0.0:8080 as configured.

Context

Found while deploying WFL 26.4.1 to host a public splash page on a cloud VPS — the
server daemon insisted on loopback-only and the config file had no effect. Traced to
with_timeout discarding everything. Patched locally, site came up, PR'd upstream.

Notes for review

• with_timeout still exists and is still used elsewhere (REPL, tests, etc.), so
  no API surface changes.
• Performance cost of the .clone() is negligible — WflConfig is a small struct
  and this happens once at startup.
• Existing unit tests should all still pass; this doesn't touch any code paths other
  than "which config the interpreter runs with".

---

Summary by CodeRabbit

• Chores
  • Internal configuration handling improvements to enhance system reliability and maintainability.

[coderabbitai]

📝 Walkthrough

Walkthrough

The interpreter initialization in script execution was refactored to pass the full Arc-wrapped configuration object to Interpreter::with_config(...) instead of constructing it solely with the timeout value. Subsequent interpreter setup and execution flow remain unchanged.

Changes

Cohort / File(s)	Summary
Interpreter Configuration src/main.rs	Refactored interpreter instantiation to accept full WflConfig via Interpreter::with_config() instead of only passing config.timeout_seconds to with_timeout().

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

• feat: make web server bind address configurable via .wflcfg #253: Depends on the full config object being available to the interpreter; reads config.web_server_bind_address from WflConfig for web server initialization.

Poem

🐰 A config once hidden, now flows full and true,
From timeout alone to the whole stew,
The interpreter smiles, it has all it needs,
More power to bloom from these generous seeds! 🌱

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: passing the full WflConfig to Interpreter instead of just the timeout value, which directly addresses the bug fix.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/pass-full-config-to-interpreter

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

[coderabbitai]

🧹 Nitpick comments (2)

src/main.rs (2)

1183-1183: Nit: import Arc instead of fully qualifying.

Minor readability tweak — the rest of the file uses plain imports at the top, so bringing Arc into scope would be more consistent.

♻️ Proposed refactor

@@ -1,6 +1,7 @@
 use std::env;
 use std::fs;
 use std::io::{self, Write};
 use std::path::Path;
 use std::process;
+use std::sync::Arc;
 use std::time::Instant;

-                let mut interpreter = Interpreter::with_config(std::sync::Arc::new(config.clone()));
+                let mut interpreter = Interpreter::with_config(Arc::new(config.clone()));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main.rs` at line 1183, The line creating the interpreter uses a fully
qualified std::sync::Arc; import Arc at the top instead and replace
std::sync::Arc::new(config.clone()) with Arc::new(config.clone()) to match the
file's import style—update the top-of-file use statements to include Arc and
keep the Interpreter::with_config(...) call otherwise unchanged.

---

1183-1183: LGTM — full config now reaches the Interpreter.

The fix correctly replaces with_timeout(config.timeout_seconds) with with_config(Arc::new(config.clone())), so fields like web_server_bind_address, subprocess_config, and shell_execution_mode from .wflcfg are now honored on the script-execution path. The clone() is necessary because config is read later in this function.

One behavior change to note: the old with_timeout path clamped timeout_seconds to 300s, whereas with_config uses the configured value verbatim. This impact is isolated to script execution (the REPL at src/repl.rs:42 still uses with_timeout and retains the 300s cap). This is the intended outcome—the cap was a side effect of the limited with_timeout shortcut.

Optional: Import Arc from std::sync at the top of the file rather than using the fully qualified path.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main.rs` at line 1183, The Interpreter is now constructed with
Interpreter::with_config(std::sync::Arc::new(config.clone())), which is correct;
to tidy imports, add use std::sync::Arc at the top and change the construction
to Interpreter::with_config(Arc::new(config.clone())); keep the clone() as
config is used later and do not reintroduce the old with_timeout call or the
300s cap.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/main.rs`:
- Line 1183: The line creating the interpreter uses a fully qualified
std::sync::Arc; import Arc at the top instead and replace
std::sync::Arc::new(config.clone()) with Arc::new(config.clone()) to match the
file's import style—update the top-of-file use statements to include Arc and
keep the Interpreter::with_config(...) call otherwise unchanged.
- Line 1183: The Interpreter is now constructed with
Interpreter::with_config(std::sync::Arc::new(config.clone())), which is correct;
to tidy imports, add use std::sync::Arc at the top and change the construction
to Interpreter::with_config(Arc::new(config.clone())); keep the clone() as
config is used later and do not reintroduce the old with_timeout call or the
300s cap.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5ce7bc92-cb6d-49cd-8b21-cd9c5237958f

📥 Commits

Reviewing files that changed from the base of the PR and between 8d8ab44 and 29481be.

📒 Files selected for processing (1)

• src/main.rs

[Copilot]

Pull request overview

Fixes the script-run startup path to pass the fully loaded .wflcfg (WflConfig) into the interpreter, so runtime settings like web_server_bind_address are actually honored during execution.

Changes:

• Replace Interpreter::with_timeout(config.timeout_seconds) with Interpreter::with_config(Arc<WflConfig>) in the main script execution path.

---

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

[Copilot]

Switching from Interpreter::with_timeout(...) to Interpreter::with_config(...) changes timeout semantics: with_timeout clamps the timeout to a max of 300s, but with_config uses config.timeout_seconds as-is. If the 300s cap is intentional (e.g., to prevent extremely long-running scripts), consider preserving it here by clamping config.timeout_seconds before constructing the interpreter or by moving the cap into config loading/validation so both constructors behave consistently.

Suggested change

	let mut interpreter = Interpreter::with_config(std::sync::Arc::new(config.clone()));
	let mut interpreter_config = config.clone();
	interpreter_config.timeout_seconds = interpreter_config.timeout_seconds.min(300);
	let mut interpreter =
	Interpreter::with_config(std::sync::Arc::new(interpreter_config));