Fix concurrent handler isolation for #642 (capture, modules, streams, depth)

[logbie]

Summary

Completes the remaining work from issue #642 (re-review of the merged #641 streaming/concurrency head): four release-blockers for concurrent web serving, the follow-up correctness/compatibility items, and the PR-review feedback rounds. Every behavioral change landed as a Red test-only commit that is an ancestor of its Green fix (testing.md §6.2 path 1); pairs are listed under Test evidence below.

Key Changes

Runtime isolation (concurrent handler safety)

1. Handler-local output capture (src/interpreter/io_capture.rs, src/interpreter/mod.rs) — the execute file ... and read output capture stack joins the per-handler RunState swap; handlers inherit a clone of the ambient stack so output still reaches an enclosing capture; CaptureGuard removes its buffer by Rc identity instead of a blind pop. Regression: tests/concurrent_execute_capture_test.rs.
2. Handler-local module loading (src/interpreter/mod.rs) — current_source_file and loading_stack join the RunState swap, so relative include/load module paths resolve against the handler's own context and a sibling's in-flight load is never misreported as a cycle; ModuleLoadGuard restores/removes by identity. Regression: tests/concurrent_module_loading_test.rs.
3. Response-stream ownership (src/interpreter/mod.rs) — write/flush/close on a live stream require it to be in the current handler's open list, so a handler holding a sibling's handle (e.g. via a shared global) cannot inject into, flush, or truncate the sibling's response. A closed stream keeps the closed-stream error for write/flush; close on a stale handle stays an idempotent no-op (nothing live left to protect). Regression: tests/concurrent_stream_ownership_test.rs.
4. Timeout evaluation ordering (src/interpreter/mod.rs) — wait for request ... with timeout <expr> evaluates the expression before locking the shared receiver, so one handler's awaiting expression no longer stalls every sibling's dequeue. Each concurrent slot evaluates its own operand at iteration start (documented). Regression: tests/concurrent_timeout_eval_lock_test.rs.
5. Live recursion depth (src/interpreter/mod.rs) — handlers seed call_depth from the live depth at loop entry (mirroring execute file child seeding), so enclosing action frames stay counted; an early test variant reproduced a real native stack overflow. IsolatedHandler::drop now drops the handler future with the handler's own run state installed, so RAII guards in a future dropped mid-suspend unwind against the right context. Regression: tests/concurrent_recursion_depth_test.rs.

Parser/typechecker/analyzer compatibility

6. Classic write line|chunk continuations (src/parser/stmt/io.rs) — the bare-marker guard now includes starts/ends/: (the arms it was missing relative to parse_binary_continuation_inner), so write line starts with "/" to f and write line : to f keep their pre-streaming file-write meaning. Regression: tests/write_web_postfix_test.rs.
7. Exact flush (…) / flush call … (src/interpreter/mod.rs, src/analyzer/mod.rs, src/typechecker/mod.rs) — these were two legacy statements (bare flush + the operand); the fallback now evaluates both in order when the bare flush binding exists, and all three passes validate the operand. Regression: tests/flush_action_backcompat_test.rs.
8. repeat until runtime-order typechecking (src/typechecker/mod.rs) — body checked first under a backedge fixed point, then the condition against post-body state; bodies containing an early exit (break/exit/return) soften the condition to the header⊔post-body join, since runtime skips the condition on those paths and type errors are fatal in load module. Regressions: tests/typechecker_repeat_until_backedge_test.rs.
9. WebSocket handler binding (src/typechecker/mod.rs) — the binding symbol is recreated (Type::Unknown) in the handler's checker scope, shadowing an outer same-named symbol the way runtime define_direct does. Regression: tests/typechecker_websocket_binding_scope_test.rs.

Docs & process

• Docs/04-advanced-features/web-servers.md: what concurrently isolates per handler; response-stream ownership semantics (including exact close-after-close behavior); per-slot timeout-expression evaluation guidance.
• Dev diary/2026-07-25-issue-642-remaining-blockers.md: full Red→Green map and rationale.
• Deferred (maintainer ruling requested): try/finally error-alias scope. Runtime keeps the alias live through finally; analyzer/typechecker deliberately resolve the outer binding there, and each direction is locked in by existing tests — aligning either way is a breaking decision (details in the Dev Diary).

Test evidence

• Risk class: R3 (concurrency, lifecycle, streaming, backward compatibility).
• Red evidence (§6.2 path 1) — test-only Red commit → Green fix, all on this branch (base 62a1b30):
  • timeout/lock ordering 01ee863 → 4556a65
  • execute-file capture 91ffee1 → 952e270
  • module loading 822778d → cfd3440
  • stream ownership 3ba71e7 → 1f71935
  • recursion depth bd183ad → 0320d24
  • write continuations dff4f3b → c4f74bc
  • flush legacy forms cd7bef4 → daca8c7
  • repeat-until + ws binding f2e28d9 → 59a71cd
  • repeat-until break-path (review round) c6e719c → 7553a6a
  • (History was rewritten once pre-push for committer identity only; commit messages cite pre-rewrite red hashes — the Dev Diary maps them.)
• Layers run locally (Linux x86-64): cargo fmt --all -- --check; cargo clippy --all-targets --all-features -- -D warnings; cargo test --workspace (all green); ./scripts/run_integration_tests.sh — TestPrograms walk 111 passed / 0 failed / 24 documented skips (backward-compat gate §11.6); ./scripts/run_web_tests.sh — 3/3; python scripts/validate_docs_examples.py --force — 18/18.
• Platforms: Linux locally; Windows via this PR's CI matrix.
• Not applicable: transpiler changes (streaming statements already rejected with a clear error); benches.
• Residual risk: per-slot timeout-expression evaluation is burstier on idle servers than the old lock-serialized accident (documented; eval-once caching offered as follow-up if preferred). Test-harness duplication across the five new concurrency tests follows the existing per-file convention (a shared tests/common helper would emit dead-code warnings in the other suites that include common).
• Rollback: revert this branch's commits; no schema/config/persisted-state changes.

https://claude.ai/code/session_01DPs45HiGQ4RQzDdihoti4K

---

<img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open in Devin Review">

Summary by CodeRabbit

• New Features
  • Improved concurrent web-server handling with isolated request contexts, non-blocking timeout evaluation, and safer module loading.
  • Added ownership protection for streaming responses; unauthorized handlers can no longer write, flush, or close another handler’s stream.
• Bug Fixes
  • Preserved legacy flush behavior and improved classic write syntax compatibility.
  • Corrected loop type checking, WebSocket handler scoping, recursion limits, and concurrent output capture.
• Documentation
  • Expanded guidance on concurrent request handling and streaming response ownership.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 084b0bc3-e4e1-4c39-ac6c-741eda531d40

📥 Commits

Reviewing files that changed from the base of the PR and between 0163817 and e0985c5.

📒 Files selected for processing (4)

• Docs/04-advanced-features/web-servers.md
• src/interpreter/mod.rs
• src/typechecker/mod.rs
• tests/typechecker_repeat_until_backedge_test.rs

🚧 Files skipped from review as they are similar to previous changes (3)

• Docs/04-advanced-features/web-servers.md
• src/interpreter/mod.rs
• src/typechecker/mod.rs

---

📝 Walkthrough

Walkthrough

The PR isolates concurrent handler execution state, enforces response-stream ownership, moves timeout evaluation outside receiver locking, preserves legacy IO behavior, aligns typechecking with runtime semantics, and adds documentation and regression tests.

Changes

Lifecycle and concurrency

Layer / File(s)	Summary
Handler-local execution context src/interpreter/io_capture.rs, src/interpreter/mod.rs, tests/concurrent_execute_capture_test.rs, tests/concurrent_module_loading_test.rs, tests/concurrent_recursion_depth_test.rs	Capture stacks, module-loading context, source paths, and recursion depth are isolated per handler; suspended futures restore their handler state during cleanup.
Request timing and response ownership src/interpreter/mod.rs, Docs/04-advanced-features/web-servers.md, tests/concurrent_timeout_eval_lock_test.rs, tests/concurrent_stream_ownership_test.rs	Timeout expressions are evaluated before receiver locking, while stream operations reject non-owner handlers.
Legacy IO compatibility src/analyzer/mod.rs, src/parser/stmt/io.rs, src/interpreter/mod.rs, tests/flush_action_backcompat_test.rs, tests/write_web_postfix_test.rs	Legacy flush operand evaluation and classic write continuations preserve side effects and parsing behavior.
Runtime-aligned typechecking src/typechecker/mod.rs, tests/typechecker_repeat_until_backedge_test.rs, tests/typechecker_websocket_binding_scope_test.rs	repeat until conditions observe stabilized post-body types, and WebSocket bindings shadow outer symbols in handler scope.
Issue 642 change record Dev diary/...	The development diary records the concurrent-server blockers, compatibility fixes, deferred scope decision, and unchanged behaviors.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related issues

• PR #641 follow-up: remaining lifecycle, concurrency, and compatibility blockers #642 — The PR directly implements the issue’s concurrency, lifecycle, compatibility, and typechecking blockers.

Possibly related PRs

• WebFirstLanguage/wfl#539 — Added the capture mechanics extended here for concurrent handler execution.
• WebFirstLanguage/wfl#593 — Introduced the WebSocket handler scoping work updated by this PR.
• WebFirstLanguage/wfl#641 — Shares the concurrent request and streaming machinery hardened here.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant RequestHandler
  participant RunState
  participant ResponseStream
  Client->>RequestHandler: send concurrent request
  RequestHandler->>RunState: install handler-local context
  RequestHandler->>ResponseStream: write, flush, or close
  ResponseStream-->>RequestHandler: allow owner or return ownership error
  RequestHandler-->>Client: return response

Loading

🚥 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 summarizes the main change: concurrent handler isolation fixes for capture, modules, streams, and recursion depth.
Docstring Coverage	✅ Passed	Docstring coverage is 86.67% 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.

✨ 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 claude/github-issue-642-4q0tfb

---

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.}

[Copilot]

Pull request overview

This PR addresses remaining correctness and backward-compatibility issues related to issue #642, focusing on isolating concurrent handler execution under main loop concurrently: and restoring pre-streaming parsing/typechecking behaviors where concurrency/streaming changes regressed legacy programs.

Changes:

• Hardened concurrent-handler isolation in the interpreter (handler-local capture/module-load context, explicit handler-future drop ordering, response-stream ownership checks, timeout evaluation ordering, recursion depth seeding).
• Fixed parser/typechecker/analyzer alignment for legacy-compatible forms (write line|chunk …, exact flush (…) / flush call …, repeat until runtime order, WebSocket handler binding shadowing).
• Added targeted Red→Green regressions plus updated docs and a dev diary entry documenting the fixes.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/write_web_postfix_test.rs	Adds regression coverage for classic `write line
tests/typechecker_websocket_binding_scope_test.rs	Regression to ensure WebSocket handler binding shadows outer symbols in typechecking.
tests/typechecker_repeat_until_backedge_test.rs	Regression for repeat until body-first typechecking with correct backedge behavior.
tests/flush_action_backcompat_test.rs	Regression ensuring exact legacy flush (…) / flush call … still evaluates operand side effects.
tests/concurrent_timeout_eval_lock_test.rs	Regression ensuring timeout expressions don’t stall sibling handlers by holding the receiver lock.
tests/concurrent_stream_ownership_test.rs	Regression ensuring response streams are handler-owned (no sibling write/flush/close).
tests/concurrent_recursion_depth_test.rs	Regression ensuring handler recursion budget includes enclosing action frames.
tests/concurrent_module_loading_test.rs	Regression ensuring handler-local module loading context (no false cycles / wrong relative base).
tests/concurrent_execute_capture_test.rs	Regression ensuring output capture is handler-local (no cross-wired capture buffers).
src/typechecker/mod.rs	Implements repeat until body-first fixed-point checking; restores legacy flush operand checks; fixes WS handler binding recreation.
src/parser/stmt/io.rs	Extends classic-continuation guard tokens for `write line
src/interpreter/mod.rs	Adds per-handler run-state swapping for capture/module-load context, explicit handler drop ordering, response-stream ownership enforcement, timeout eval ordering, live-depth seeding.
src/interpreter/io_capture.rs	Makes capture guards identity-based; adds stack swap/snapshot for per-handler capture isolation.
src/analyzer/mod.rs	Analyzes legacy flush operand expression for exact forms to preserve name validation.
Docs/04-advanced-features/web-servers.md	Documents concurrency isolation details and response-stream ownership semantics.
Dev diary/2026-07-25-issue-642-remaining-blockers.md	Records Red→Green evidence and rationale for the #642 follow-up fixes.

---

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

[logbie]

Addressed by documenting the actual guarantee rather than adding post-close ownership tracking (627f679 for the user docs, 7553a6a for the owns_response_stream/close-site comments — this thread and the two sibling comments were all the same nuance).

Rationale for not tracking previously-owned/closed ids: the enforced invariant is that a live response can't be injected into, flushed, or truncated by a non-owner — once the stream is closed there is no response left to protect, so a stale-handle close is a harmless idempotent no-op for any holder. Per-handler closed-id tracking would add unbounded per-handler state and would break the serial loop's existing idempotent re-close across iterations (the per-iteration drain clears the open list, so iteration N+1's re-close of an iteration-N stream would start erroring). write/flush on a closed stream still fail with the closed-stream error.

---

Generated by Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0163817603

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

[chatgpt-codex-connector]

Avoid eagerly evaluating every handler timeout

When main loop concurrently begins with wait ... with timeout call f, all 256 prefilled handler futures can reach this evaluation before any request is dequeued, so an idle server now invokes f once per handler slot. This bypasses the first-wait invariant documented in src/analyzer/mod.rs:1241-1260 to prevent speculative pre-request side effects; for example, a timeout action that increments state or performs external I/O fires roughly 256 times instead of once before the first request. Avoid holding the receiver lock across a slow evaluation without allowing every queued handler to evaluate the timeout eagerly.

AGENTS.md reference: AGENTS.md:L145-L148

Useful? React with 👍 / 👎.

[logbie]

Acknowledged, but keeping the current behavior deliberately — the two goals are in genuine tension. Any scheme that serializes timeout-expression evaluation (the old accidental behavior, or a separate eval mutex) recreates the P1 this fixes: one handler's awaiting expression stops every sibling from dequeuing, which was a merge-blocker in #642. The eval must therefore happen outside any shared lock, and each of the N handler iterations evaluates its own operand when it starts — the burst is inherent to N concurrent iterations, not to the lock reorder (each iteration always evaluated it eventually; only the timing/parallelism changed).

The first-wait analyzer invariant still holds for its stated purpose (no user statements run per-slot before a request); the timeout operand is part of the wait itself. Documented in 7553a6a: web-servers.md now states each slot evaluates its own timeout expression at iteration start and that timeout expressions should stay side-effect free. If eval-once-per-loop-entry caching semantics are preferred for the operand, that's a language-semantics decision for the maintainer — happy to implement it as a follow-up if wanted.

---

Generated by Claude Code

[chatgpt-codex-connector]

Exclude break-only states from repeat conditions

When a repeat until body retypes a binding and then executes break, the runtime exits at the break without evaluating the condition, but this unconditional post-body state makes the checker validate the condition against that retyped binding. For example, a body that starts a response stream as out and then breaks is runtime-valid even if the condition compares the original numeric out, yet it now produces a type error; such errors are fatal while loading modules. Track only condition-reaching body paths for condition inference while retaining break-path state for code after the loop.

AGENTS.md reference: AGENTS.md:L19-L22

Useful? React with 👍 / 👎.

[logbie]

Confirmed and fixed — load module does treat type errors as fatal, so this was a real compat hazard, not just imprecision. As of 7553a6a (red test in c6e719c): when the body subtree contains an early exit (break/exit/return, including inside if/try/nested blocks), the condition — and post-loop state — is checked against the join of the stabilized header and the post-body state, so a retype-then-break body typechecks cleanly again. Straight-line bodies keep the precise post-body check (the original #642 soundness fix). The scan is deliberately conservative: a nested loop's break also softens, which can only under-report, never falsely reject.

---

Generated by Claude Code

[devin-ai-integration]

✅ Devin Review: No Issues Found

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

[coderabbitai]

🧹 Nitpick comments (1)

tests/concurrent_module_loading_test.rs (1)

22-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Move the duplicated server harness into tests/common. All five new concurrency tests copy the same start_server_thread / wait_for_server / shutdown trio, and each already declares mod common for free_tcp_port — so the shared module is the obvious home. Parameterizing stack size and optional source file covers every current variant.

• tests/concurrent_module_loading_test.rs#L22-L65: move start_server_thread (with its Option<PathBuf> source-file parameter), wait_for_server, shutdown, and wfl_path into tests/common.
• tests/concurrent_recursion_depth_test.rs#L26-L73: drop the local copies and call the shared helper, passing the 16 MiB stack size (keep the stack-size rationale comment at the call site).
• tests/concurrent_execute_capture_test.rs#L26-L62: drop the local copies in favor of the shared helpers; keep write_child local.
• tests/concurrent_stream_ownership_test.rs#L23-L59: drop the local copies in favor of the shared helpers.
• tests/concurrent_timeout_eval_lock_test.rs#L26-L68: drop the local copies in favor of the shared helpers; the doc comments on readiness/shutdown can move with them.

As per coding guidelines, "Place Rust unit and integration tests under tests/, using feature-oriented names such as *_test.rs" — a shared common module keeps the feature-named test files focused on their scenario.

🤖 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/concurrent_module_loading_test.rs` around lines 22 - 65, Move the
duplicated start_server_thread, wait_for_server, shutdown, and wfl_path helpers
into tests/common, preserving optional source-file handling and parameterizing
stack size as needed. In tests/concurrent_module_loading_test.rs:22-65, relocate
the helpers; in tests/concurrent_recursion_depth_test.rs:26-73,
tests/concurrent_execute_capture_test.rs:26-62,
tests/concurrent_stream_ownership_test.rs:23-59, and
tests/concurrent_timeout_eval_lock_test.rs:26-68, remove local copies and use
the shared helpers, retaining the recursion stack-size rationale, write_child,
and applicable readiness/shutdown documentation at their respective sites.

Source: Coding guidelines

🤖 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.

Nitpick comments:
In `@tests/concurrent_module_loading_test.rs`:
- Around line 22-65: Move the duplicated start_server_thread, wait_for_server,
shutdown, and wfl_path helpers into tests/common, preserving optional
source-file handling and parameterizing stack size as needed. In
tests/concurrent_module_loading_test.rs:22-65, relocate the helpers; in
tests/concurrent_recursion_depth_test.rs:26-73,
tests/concurrent_execute_capture_test.rs:26-62,
tests/concurrent_stream_ownership_test.rs:23-59, and
tests/concurrent_timeout_eval_lock_test.rs:26-68, remove local copies and use
the shared helpers, retaining the recursion stack-size rationale, write_child,
and applicable readiness/shutdown documentation at their respective sites.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 21209f93-f2fd-4135-8f03-5ad1de32b480

📥 Commits

Reviewing files that changed from the base of the PR and between b6ccb49 and 0163817.

📒 Files selected for processing (16)

• Dev diary/2026-07-25-issue-642-remaining-blockers.md
• Docs/04-advanced-features/web-servers.md
• src/analyzer/mod.rs
• src/interpreter/io_capture.rs
• src/interpreter/mod.rs
• src/parser/stmt/io.rs
• src/typechecker/mod.rs
• tests/concurrent_execute_capture_test.rs
• tests/concurrent_module_loading_test.rs
• tests/concurrent_recursion_depth_test.rs
• tests/concurrent_stream_ownership_test.rs
• tests/concurrent_timeout_eval_lock_test.rs
• tests/flush_action_backcompat_test.rs
• tests/typechecker_repeat_until_backedge_test.rs
• tests/typechecker_websocket_binding_scope_test.rs
• tests/write_web_postfix_test.rs

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[logbie]

Fixed in eb02330 (red test in 0b3c6c3). The walker now stops descending for break at nested loop boundaries — a nested loop's body is scanned only for exit loop/return, which the runtime re-raises through every enclosing loop's control-flow dispatch (ControlFlow::Exit/Return), while break is absorbed by the nearest loop. Two new regressions pin both directions: a nested-loop break keeps the precise post-body condition check (the retyped binding is reported again), and a nested exit loop still softens.

---

Generated by Claude Code

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.