PR #641 follow-up: remaining lifecycle, concurrency, and compatibility blockers

[logbie]

Scope

This tracks the remaining findings from a re-review of #641 at exact head b25aed57ea50697c596796446d1f47466668773d (69 files, +10,978 / -303).

The exact-head gate is genuinely green: CI run 30098113534 passed Rust tests/formatting/all-features Clippy, the documented integration scripts on Linux and Windows, Windows docs validation (18/18), and Windows web tests (3/3). Most earlier review findings are closed. Green CI does not cover the cases below, though, and I would not merge that exact head until the P1 items are fixed.

1. P1 — request-local failures can still stop the concurrent server

execute_concurrent_main_loop sends every non-Cancelled handler error and every panic through one global 256-consecutive-failure breaker (src/interpreter/mod.rs:4402-4428). That includes request-local upstream/network failures, request-dependent response errors, and an ordinary wait for request ... with timeout expiry (:9040-9058). After enough consecutive instances, the entire loop stops even though the listener and unrelated handlers are healthy.

There is also a concrete disconnect path that still reaches this breaker:

1. A handler dequeues a request and parks its sender in pending_responses.
2. A sibling wait for request globally prunes closed senders (:9166-9178).
3. The owning handler later reaches respond (:9236-9268) or start streaming response (:9489-9518).
4. The pending entry is missing, so both paths return ErrorKind::General, not Cancelled.
5. That expected client disconnect is counted as a structural failure.

The new burst test is false-green for this race:

• it fires only 270 clients with concurrency 40 (tests/concurrent_disconnect_paths_burst_test.rs:24-26), leaving roughly the final batch on the correctly classified direct-send path;
• it waits only three seconds before /ping (:104-106);
• the General-failure backoff (src/interpreter/mod.rs:4444-4455) takes about 11.5 seconds to consume 256 failures, so /ping can be handled and reset the counter before the buggy results are drained;
• its fired counter is incremented but never asserted.

Acceptance criteria:

• Distinguish structural/pre-request loop failures from expected or request-local outcomes. A handler that accepted a request must not be able to tear down the whole server because that request failed.
• Treat “pending entry missing while this handler still owns the request” as cancellation/timeout; retain the duplicate-response error when the handler no longer owns it.
• Repeated finite wait for request timeouts must not terminate an otherwise healthy server.
• Add real-socket regressions for >256 buffered-response disconnects and disconnect-before-streaming-head-send. The current streaming test reads the head first and exercises only the later write path.
• Ensure the test proves every intended handler result was consumed before probing /ping (or make the breaker threshold injectable and small in tests).

2. P1 — the outbound hard-lifetime reaper has an ownership race and leaks timer tasks

The new reaper removes a handle once at its deadline (src/interpreter/mod.rs:2018-2035). A body read removes that handle from the shared map for the duration of the await (:2044-2053), then put_stream blindly reinserts it (:2056-2062; called by next_chunk/next_line).

If expiry lands while a read owns the handle, the one-shot removal is a no-op. A ready chunk can win the read/timeout race, and the read can reinsert the expired handle after the reaper has exited. With no subsequent read, the live upstream can outlast the documented real-time hard cap.

Related lifecycle problems:

• every open stream spawns a timer that remains parked until the cap even after EOF, error, or explicit close, so rapid open/close cycles accumulate roughly request-rate × cap sleeping tasks;
• when the reaper does remove a parked handle, the next read reports “unknown/already closed” (General) rather than the promised typed timeout;
• arbitrary u64 config values reach Instant::now() + Duration::from_secs(secs) (:1943-1946), which can panic for extreme values.

Acceptance criteria:

• Use stable shared per-handle state/cancellation/tombstones, or otherwise make expiry and read ownership one atomic lifecycle.
• Refuse reinsertion at/after the deadline and preserve the terminal reason as Timeout.
• Cancel/remove the timer when the stream reaches EOF, errors, or closes; resource usage must remain bounded under rapid open/close.
• Validate/safely clamp the configured duration so config cannot panic.
• Add a near-deadline active-read race regression and a rapid open/close resource regression. outbound_stream_open_expiry_test currently covers only an unread handle that stays in the map.

3. P1 — ambiguous write line|chunk checking is still unsound

The aligned analyzer reports the ambiguous lead only when neither candidate exists (src/analyzer/mod.rs:3760-3769). The typechecker then selects a concrete branch, but unresolved variables infer as Unknown without another diagnostic (src/typechecker/mod.rs:3076-3140). That lets the selected runtime branch contain an undefined name.

Concrete text-target regression:

store value as "x"
write line value to "/tmp/out"

The stream lead value exists, so analysis passes. The text target makes runtime select the classic branch and evaluate the missing variable line value. Before this PR, that classic file write was rejected statically. The inverse occurs with a concrete ResponseStream when only line value exists. PropertyAccess is also absent from the aligned analyzer walker, so write line missing.field to "/tmp/out" can evade the “neither lead exists” check.

There are two further branch-selection holes:

• for a gradual target, src/typechecker/mod.rs:1083-1112 accepts when either speculative branch is valid, although runtime may select the invalid branch;
• analyzer loop scopes are discarded, and the typechecker only marks a streaming-response binding when a surviving symbol already exists (:1045-1050). Inside the canonical main loop, out can therefore remain Unknown; a valid file fallback can mask an invalid payload on the actual stream branch.

Example of the latter:

main loop:
    wait for request comes in on s as req
    start streaming response to req with status 200 as out
    store items as [1 and 2]
    store line items as "legacy"
    write line items to out
end loop

At runtime out is definitely a response stream and the list payload is invalid, but the gradual/file branch can make this pass static checking.

Acceptance criteria:

• Validate definedness and payload type for the concrete branch runtime will select.
• Preserve/recreate the ResponseStream binding while typechecking main-loop and action bodies.
• For a gradual target, conservatively validate every viable runtime branch rather than accepting any one valid branch.
• Cover one-sided undefined leads for both concrete targets, property access, gradual targets, and stream payloads in real handler scope.

4. P1 — merged operands do not compose like normal WFL expressions

parse_write_value_from_lead delegates postfix handling to parse_trailing_postfix (src/parser/stmt/io.rs:22-29). That helper handles brackets and dot/method access, but omits two existing WFL indexing forms handled by the ordinary primary parser: direct integer indexing (src/parser/expr/primary.rs:1301-1341) and at indexing (:1414-1424).

Previously valid classic code now parse-fails:

store line values as ["first" and "second"]
write line values at 0 to "/tmp/out"

write line values 0 to ... is broken similarly.

The same limited helper is used for merged content type and headers operands (src/parser/stmt/web.rs:449-500). The documented <e> clauses therefore reject ordinary call/operator continuations such as:

start streaming response to req with status 200 and content type mime_type of path as out

Acceptance criteria:

• Reuse a shared continuation parser so merged-command operands support the same postfix/call/operator grammar and precedence as ordinary expressions.
• Add compatibility tests for [], .field, .method(), at, direct-integer indexing, of, and operator continuations across write, content type, and headers.

5. P1 — flush compatibility preserves only one old behavior

Before this PR, a merged bare name such as flush cache was an ordinary expression statement. The new runtime fallback (src/interpreter/mod.rs:9879-9903) preserves a Function or an Overloaded value containing a zero-argument overload, then otherwise falls through to stream flushing.

This changes previously legal behavior:

store flush cache as 1
flush cache

The base branch evaluates that variable and completes. This head instead tries to flush an undefined cache. An overloaded flush cache action with no zero-argument overload likewise changed from ordinary bare-expression evaluation/no-op to a stream error.

Acceptance criteria:

• Preserve the complete old expression-statement fallback when the full merged name resolves, not only the zero-argument action happy path.
• Keep analyzer, typechecker, and runtime resolution rules aligned.
• Add regressions for a non-callable full-name binding and an overloaded action with no zero-argument overload.

R3 test-strength follow-ups

These are not substitutes for the correctness fixes above, but they are needed to make the Green evidence meaningful:

• response_stream_backpressure_test must assert the expected timeout/error and a reasonable lower timing bound; it currently discards interpret() and passes on an unrelated immediate exit.
• outbound_stream_open_expiry_test must assert successful setup/the expected terminal reason and a lower bound; it also currently ignores interpret().
• Exercise the dropped-run pending-request 500 branch. dropped_interpret_server_cleanup_test starts a streaming response, so it proves body closure but has already consumed the pending sender.
• Assert that all disconnect clients actually connected, sent, and reached the intended lifecycle point.

Verification note

I could not run new ad-hoc Rust regressions locally because this review environment has no cargo. The existing exact-head Actions run is fully green, but it does not contain these cases. Per the repository’s R3 policy, each behavior change above should land as a Red test-only commit before its Green fix.

[logbie]

Warden triage — verdict: confirmed, well-scoped merge-blocker tracking issue for #641 (not a defect in main).

The streaming feature isn't on main yet, so nothing here affects shipped behavior — this is correctly framed as the gating checklist that must go green before #641 lands. Every item is grounded in concrete code with exact line refs, and the "green exact-head gate doesn't cover these" framing is right: §1–§3 are concurrency races and branch-selection holes that the unit/integration CI on a fixed head structurally can't surface.

One thing to flag up front: this was filed against head b25aed57, but #641 has moved on — current head is a543650f, now 131 commits / +27,620 / −7,533 across 87 files, mergeable_state = unstable. Re-walk the acceptance boxes against the current head before closing any of them; some may already be partially addressed.

Priority read of the four items:

• §4 is the one that touches backward compatibility, which this repo treats as non-negotiable — so I'd rank it alongside the P1s as a hard blocker. write line values at 0 to "/tmp/out" and the bare write line values 0 to ... form were valid classic code and now parse-fail. I verified the mechanism on current head: src/parser/stmt/io.rs:148 parse_write_value_from_lead delegates postfix handling to parse_trailing_postfix (io.rs:42), while the ordinary primary parser in src/parser/expr/primary.rs is what implements at indexing (Token::KeywordAt → IndexAccess, :315) and direct integer indexing (:187/:300/:322/:1413). The merged-operand helper doesn't reach those forms, so classic programs that used them regress. Per the repo's rule, that parse regression alone is sufficient reason to keep Add streaming response support: client, server, and concurrent handlers #641 out of main. The fix shape you propose — reuse a shared continuation parser so merged write / content type / headers operands get the same postfix/call/operator grammar and precedence — is the right one.

• §1 / §2 / §3 (P1) are legitimate robustness/soundness blockers. §1: a request-local failure or an ordinary wait for request … with timeout expiry feeding the global 256-consecutive-failure breaker means one client's disconnect can tear down a healthy listener — the respond/start streaming response → ErrorKind::General (not Cancelled) disconnect path is the concrete hole, and you're right that the current burst test is false-green (270 clients @ 40, 3s pre-/ping, fired never asserted). §2: the one-shot reaper vs. put_stream reinsertion is a classic read/expiry ownership race, and the per-stream timer that stays parked until the cap is an unbounded-task leak under rapid open/close. §3: selecting a concrete write line|chunk branch while unresolved names infer as Unknown (typechecker/mod.rs:3076-3140) with the analyzer only flagging when neither lead exists (analyzer/mod.rs:3760-3769) is genuinely unsound, and the missing PropertyAccess walker case widens it.

Module map for whoever picks this up: concurrent server loop + failure breaker + outbound reaper → src/interpreter/mod.rs; ambiguous write-lead definedness/payload checking → src/analyzer/mod.rs + src/typechecker/mod.rs; merged-operand grammar → src/parser/stmt/io.rs + src/parser/stmt/web.rs (sharing the postfix/index grammar in src/parser/expr/primary.rs).

Next steps (aligned to the repo's mandatory-TDD rule — land each box as a failing regression first):

• Make the 256-consecutive-failure breaker threshold injectable so a small deterministic test can prove request-local/disconnect misclassification without the ~11.5s drain window that hides it today.
• Add the near-deadline active-read race + rapid open/close resource regressions for §2, and preserve the terminal reason as Timeout on post-deadline reads.
• For §3, cover one-sided undefined leads across concrete targets, PropertyAccess, gradual targets, and stream payloads in real handler scope.
• For §4, add a TestPrograms/ regression (must pass under the release build) plus parser unit tests for both … at 0 … and bare … 0 …, since that's the backward-compat surface.

Suggested labels (I can't apply them myself): blocker, area: interpreter, area: parser, backward-compat, tracks #641.

Net: keep #641 unmerged until §4 and the three P1s are green with the added regressions; because the branch has advanced past the head cited here, re-verify each box against a543650f first.

Posted by the WFL repo warden (automated triage pass).

[logbie]

Exact-head re-review update — 690be0af (2026-07-25)

CI run #1779 is green, and most previously reported lifecycle defects have been fixed. The remaining work is concentrated rather than “everything is broken.”

Fix before releasing concurrent web serving:

• Make execute file ... and read output capture handler-local; the current thread-local stack can cross-wire captured output between concurrent requests.
• Evaluate request timeout expressions before locking the shared receiver; an awaiting expression can otherwise stall every request handler.
• Make module/include source and loading context handler-local; concurrent loads can falsely report cycles or resolve a relative path against another handler.
• Enforce response-stream ownership/cancellation so a handler cannot close a stream while another handler still has a pending write that later succeeds.

Track here as follow-up correctness/compatibility:

• Preserve live recursion depth in nested concurrent handlers.
• Restore classic write line|chunk continuations for starts, ends, and :.
• Preserve both legacy expression side effects for exact flush call ... / flush (...) forms.
• Align try/finally error-alias scope, repeat-until/backedge flow, and WebSocket binding shadowing.
• Harden docs validation and add packaged-binary E2E coverage for the new streaming/concurrency journeys.

Practical disposition: ordinary non-concurrent WFL use is unlikely to fail merely because this head lands. It should not, however, be treated as production-ready for untrusted concurrent web traffic until the first group is fixed. The Section 17 exception is inactive/stale for this executable SHA, so the repository's current testing policy still treats the known failures as merge-blocking. If this is integrated before those fixes, treat it as a development-only, non-release merge and keep the concurrent server path experimental.

[logbie]

Warden re-triage — the premise of this issue changed: streaming is now on main

My 07-25 assessment above opened with "the streaming feature isn't on main yet, so nothing here affects shipped behavior." That is no longer true, and it's worth correcting explicitly:

• Add streaming response support: client, server, and concurrent handlers #641 merged 2026-07-25T11:58Z (62a1b302) → v26.7.48
• Fix concurrent handler isolation for #642 (capture, modules, streams, depth) #643 merged 2026-07-25T15:47Z (b7b540e8) → v26.7.49, current main head 8d6aaed2

So every item still unchecked here now describes shipped behavior, not a merge gate. The priority framing shifts accordingly — §4 (classic write line|chunk continuations) is a backward-compatibility regression in a released version, not a blocker on an unmerged branch.

But the tree suggests this is largely done and just not ticked off

#643's description states it "Completes the remaining work from issue #642 — four release-blockers ..., the follow-up correctness/compatibility items, and the PR-review feedback rounds." All 23 acceptance checkboxes here are still - [ ], yet main now carries a regression test that maps to nearly every one:

Acceptance area	Regression on main
§1 >256 buffered-response disconnects / prune race	tests/concurrent_disconnect_burst_test.rs, concurrent_disconnect_paths_burst_test.rs, concurrent_prehead_prune_race_test.rs
§1 disconnect before streaming head send	tests/outbound_stream_head_disconnect_test.rs, wait_line_pre_response_disconnect_test.rs
§2 near-deadline active-read race / hard cap	tests/outbound_stream_reaper_race_test.rs, outbound_stream_absolute_lifetime_test.rs, outbound_stream_open_expiry_test.rs
§2 ownership, deadline, disconnect	tests/outbound_stream_ownership_test.rs, outbound_stream_deadline_test.rs, outbound_stream_disconnect_test.rs
§3 branch selection / typeflow	tests/stream_handle_type_test.rs, typechecker_response_stream_scope_test.rs, typechecker_response_stream_join_test.rs
§4 classic-form back-compat	tests/write_line_backcompat_test.rs, flush_action_backcompat_test.rs, write_web_postfix_test.rs, TestPrograms/parser_streaming_compat_regression.test.wfl
Isolation blockers	tests/concurrent_execute_capture_test.rs, concurrent_module_loading_test.rs, concurrent_stream_ownership_test.rs, concurrent_timeout_eval_lock_test.rs, concurrent_recursion_depth_test.rs

Verdict: probably complete, definitely stale. The most useful next action is a walk of the 23 boxes against 8d6aaed2, ticking what #643 closed and leaving only genuine remainders — then closing this. If anything survives (the §2 timer-cancellation-on-EOF resource bound and the §3 PropertyAccess gap in the aligned analyzer walker are the two I'd re-check first), it deserves a fresh, narrow issue rather than staying inside a 23-box tracker that reads as fully open.

Honest limitation: I verified those files exist and what they claim to cover; I did not execute them. This triage environment is aarch64 Linux with no Rust toolchain, so I can't run cargo test or reproduce the §3/§4 snippets.

One consequence you'll want to know about

The combined test surface from #641 + #643 — 26 new files in tests/, now 112 targets — took the nightly build red. cargo test --release in nightly.yml compiles every one of those binaries in release mode with [profile.release] debug = true; the step went from ~27 min at v26.7.47 to >70 min without finishing, and run 30190411454 was cancelled at the 90-minute job cap. No nightly release published for 07-26. ci.yml stayed green because its integration lanes compile those same binaries in debug. Tracked with a timeout fix in #645.

Suggested labels: tracking, concurrency, needs-verification.

Posted by the WFL repo warden (automated triage pass).