Fix #2: pr honours *hush* only for the standard output stream

[pyrex41]

Fixes #2

Problem

The canonical kernel pr (kernel/klambda/writer.kl) gates the entire write on *hush*. So under -q (or (set *hush* true)), even a pr to an explicitly-opened file stream was silently dropped, producing zero-byte files. *hush* is only meant to silence interactive console chatter — shen-cl and shen-go both write to file streams regardless of *hush*.

Fix

Register a native pr override in register_hot_overrides (installed after the AOT defun, in both boot paths). It consults *hush* only when the target is the standard output stream — identified by Rc pointer-equality against the *stoutput* global. Writes to any other (file) stream always occur. Minimal and targeted; no kernel/canonical sources touched.

Test change

The port-authored cli_launcher.rs suite previously locked in the buggy behavior (asserting the -q file was EMPTY, with an explanatory "divergence" comment). That assertion is flipped to assert the file IS written under -q (test renamed to quiet_does_not_silence_pr_to_file, referencing #2), and the file-header divergence note is updated. The without--q case is retained.

Suite / cert results

• cargo fmt --all -- --check: clean
• cargo clippy --workspace --all-targets -- -D warnings: clean
• cargo build --workspace: ok
• cargo test --workspace: 190 passed, 1 ignored
• Canonical kernel suite (scripts/kernel-tests.sh): 134/134, 100%

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

[pyrex41]

Independent agent review (ratatoskr + ShenSpec deep-dive, post-load native override discipline, 134/134 cert preserved)

Code Review: #3

PR: #3
Title: Fix #2: pr honours hush only for the standard output stream
URL: #3
Review mode: Independent verification agent (PR mode, fresh checkout re-verification)
Diff: /tmp/grok-reviews/9ad17548.diff
Changed files: crates/shen-rust/src/primitives.rs, crates/shen-rust/tests/cli_launcher.rs (right-side additions only; sources read pre- and post-patch semantics via diff + on-disk)
Date of review: 2026-06-13

Summary

The change implements a targeted native hot_pr override (registered in register_hot_overrides after AOT load, and via both register_native + register_aot_direct) that consults the kernel's *hush* global only when the target output stream is pointer-identical (via Rc::ptr_eq) to the current value of the *stoutput* global. All other streams (explicitly open'ed files, *sterror*, rebinds, etc.) always perform the write via Stream::Out.

This directly counters the root cause in the canonical kernel: ratatoskr/KLambda/writer.kl:3 (the pr defun) does (if (value *hush*) V6806 <no-write> ...), which gates the entire operation before any stream-specific dispatch (the shen.char-stoutput? test is inside the else). The same pattern appears in sys.kl (hush, hush?, stoutput, nl, y-or-n?, write-to-file) and is documented as the source of "hush bugs across ports" in ratatoskr/README.md (Gotchas section, and the -q caveat for ratatoskr stage 1). shen-rust (and previously shen-lua) inherited the full-gate behavior because they did not install a native pr override the way shen-cl does.

Key verification points (per instructions):

• Diff read first; sources read: shen-rust/crates/shen-rust/src/primitives.rs (hot_pr + surrounding register/stream code), cli_launcher.rs (the load-bearing test), env.rs (globals), value.rs (Stream + SharedStream = Rc<RefCell<Stream>>, as_stream, shen_eq identity), interp/boot.rs (set_standard_streams creates the initial *stoutput* Rc-wrapped stdout; register_hot_overrides called last in both boot_with_kernel and boot_from_kl_source paths, after AOT install and shen.initialise), interp/eval.rs (register_native + register_aot_direct semantics + direct table coherence), symbol/intern paths, and aot/kernel/writer.rs (the AOT-compiled pr that gets shadowed).
• ratatoskr examined in full: README.md (explicit call-out of the 4-port hush divergence and the "omit -q" workaround), KLambda/writer.kl:3 (the if-hush early return), KLambda/sys.kl (hush?/hush/stoutput/stinput/write-to-file/nl/y-or-n? and init of hush false), builders (lisp/driver notes shen-cl's native override; rust ratatoskr-build + generated out-*/.../main.rs all route through boot_from_kl_source which guarantees hot overrides run after the caller's AOT module), and out-demo/out-metaeval scaffolds.
• ShenSpec angle: cli_launcher.rs (port-authored, mirrors shen-go) previously encoded the incorrect "zero-byte under -q for files" as the "CORRECT documented divergence" (module header + test comment + assertion + old test name quiet_silences_pr_to_file_but_default_writes). This was the load-bearing assertion that "locked in" the bug for the port; it was flipped (new name quiet_does_not_silence_pr_to_file, assertions now expect payload in both -q and no-q cases, comments revised to describe correct behavior and cross-port parity with shen-cl/shen-go). The "without -q" case is retained. shen-rust/specs/core.shen and other specs do not encode pr/hush semantics (they are sequent types for other gates). shen-rust/README.md and cli_launcher module docs are updated consistently. Kernel-tests (134/134) and the 190+1 suite remain green because the canonical kernel is untouched.
• Identity check verified correct: Rc::ptr_eq(&out, &target) operates on the SharedStream (Rc) values returned by as_stream(). This is true pointer identity of the stream object (the RefCell<Stream> allocation), not by symbol name, string content of path, fd, or shen_eq value. The initial *stoutput* global (and any later (set *stoutput* ...) or (stoutput) value) holds exactly one such Rc; file streams from open get fresh Rcs. When target ptr-matches the current *stoutput* global's stream and *hush* is true, early return (no write) — "still silences stdout" half preserved. All other streams (including a file opened and assigned to *stoutput*) bypass the hush gate and always delegate to Stream::Out::write_all. Matches intent ("hush only meant for interactive console chatter").
• Error handling / panic paths in proposed hot_pr: input validation for arg0 (string) and arg1 (stream) is present and returns ShenError; post-hush write path correctly matches only Stream::Out (errors for In/Closed with "not an output stream"); I/O errors wrapped; hush globals missing/non-bool treated conservatively as "do not hush". However, no args.len() != 2 guard (direct args[0]/args[1]); under-application would index-panic rather than ShenError. Arity registration + apply checks + AOT call discipline protect normal paths (see eval.rs:838 etc. and how hot_* siblings are written), but this is a minor robustness gap vs. AOT pr wrapper. RefCell borrow_mut (same as existing write-byte/read-byte) can panic on re-entrant aliasing of the same stream object — pre-existing, not introduced. No missed Stream variants. No unwrap(), no unnecessary clones of large data (the arg string content is cloned only for the write buffer; input Value is returned by ref), no extra locks.
• ratatoskr out-*/builders confirm the uniform strategy ("install native pr override after kernel/AOT load") used across ports for all four hush bugs; no canonical kernel sources in ratatoskr/KLambda/* or shen-rust's vendored kernel/klambda/* are touched.
• Overall: correctness first (root cause fixed at the right layer; identity + hush-half semantics exact; test flipped correctly; 134/134 + build/clippy/fmt preserved). Style second (comments are excellent and reference pr to a file stream is silenced under *hush* (-q), producing zero-byte files #2; code is minimal and matches existing hot-override idiom exactly). No races (single-threaded Shen + RefCell). Edge cases (rebound *stoutput*, closed streams, non-bool *hush*, direct AOT calls to "pr", file streams under -q, sterror, adversarial launcher) covered or inherited safely.

Verdict (1-sentence): The patch is a correct, minimal, well-documented targeted override that fixes the canonical-kernel *hush* over-gate for non-stdout streams using the right identity check and post-AOT registration, with the port test updated to stop asserting the prior buggy behavior; only minor robustness nit on arity panics in the hot path.

Issues

• Severity: low (robustness / defensive coding)
  File:line: crates/shen-rust/src/primitives.rs (added hot_pr body; see diff right-side lines starting after the read-file-as-string registration and the new fn hot_pr at the equivalent of post-patch ~24)
  Description: hot_pr performs let s = match args[0].as_str() ... and let target = match args[1].as_stream() ... with no preceding if args.len() != 2 { return Err(ShenError::new("pr: expected 2 args...")); }. Under- or over-application (possible via direct FFI, test harnesses that bypass apply, or future changes) will panic on slice indexing instead of returning a clean ShenError (cf. the AOT-generated aot_pr in aot/kernel/writer.rs which does an explicit len check, and special forms in eval.rs). While register_native(..., 2, ...) + Interp::apply (which checks total.len() == closure.arity) and AOT direct call sites protect normal execution, and sibling hot_* fns are similarly bare, this is still a missed defensive path for a now-hot "pr".
  Suggestion: Add an explicit arity guard at the top of hot_pr (and consider backfilling the other hot_* fns for consistency). This matches the style of aot_* wrappers and special form dispatch.
  Status: open (non-blocking for this fix; pre-existing pattern in hot overrides)

• Severity: info (documentation / historical note)
  File:line: crates/shen-rust/tests/cli_launcher.rs:11-14 (module-level DIVERGENCES comment, pre-patch) and the flipped test at ~238-283 (old name quiet_silences_pr_to_file_but_default_writes, old zero-byte assertion, "documented shen-rust divergence" comment)
  Description: The previous assertions + comments in the port-authored launcher test (and mirrored in shen-rust/README.md:39 and the module header) "locked in" the buggy full-hush behavior as "CORRECT documented behavior" / "the cross-impl divergence the spec calls out". Per instructions, this was the load-bearing test that was flipped; the ShenSpec angle is that no canonical kernel test or specs/core.shen encoded the wrong pr-to-file semantics — only this port test did. The ratatoskr README explicitly calls out the root cause and the workaround used on shen-rust/shen-lua. The patch correctly revises comments to describe the fixed parity with shen-cl/shen-go and removes the hush-file divergence from the "locked in" list, while retaining the without -q case and the other (adversarial) divergences.
  Suggestion: (none required; already done in the PR) Consider a one-line cross-reference in the revised test or shen-rust/README.md pointing to ratatoskr/KLambda/writer.kl:3 + README "Gotchas" as the single source of truth for why all four ports needed the same native-pr-after-load pattern.
  Status: addressed by the PR

• Severity: info (pre-existing, noted for completeness)
  File:line: crates/shen-rust/src/primitives.rs (the write-byte implementation at register_core time; also close/read-byte) and the new hot_pr write path (diff right-side lines ~63-70)
  Description: let mut stream = target.borrow_mut(); ... (and equivalent in the core I/O primitives) uses RefCell::borrow_mut without try_borrow_mut. Re-entrant use of the same stream object from within a write (e.g., via a user stream wrapper that calls back into pr/write-byte on the identical Rc) will panic. This is not new to hot_pr (the canonical pr path and all ports' stream handling have analogous issues), and Shen execution is single-threaded, but it is a latent panic path on aliasing. No additional clones/locks introduced by the PR.
  Suggestion: No action for this PR (minimal change); if hardening streams later, consider try_borrow_mut + a clean Shen error, or document the invariant.
  Status: pre-existing (not introduced; status noted)

• Severity: info (style / future maintenance)
  File:line: crates/shen-rust/src/primitives.rs (diff insertion of hot_pr body immediately after the closing } of register_hot_overrides and before the // --- hot-override bodies comment) + the dual registration pattern now used for "pr" (matching element?/pvar?/etc.)
  Description: Placement is syntactically fine (Rust item ordering), and the dual register_native + register_aot_direct + "AOT_DIRECT also wired" comment is exactly the right idiom (see boot.rs comments on why this is required for AOT callers in prolog/typechecker/reader paths, and eval.rs:583 on install order). The ratatoskr rust builder scaffolds (out-/main.rs) + boot_from_kl_source confirm it runs last after AOT. No functional problem.
  Suggestion: For long-term hygiene, the bodies of all hot_ fns (including the newly-inserted hot_pr) could live together after the bodies comment, or a small internal helper register_hot_pr could be extracted; purely aesthetic.
  Status: addressed (the PR follows the established local pattern exactly)

• Severity: none (positive verification)
  File:line: N/A (design/identity)
  Description: Rc::ptr_eq usage is the correct mechanism: as_stream() returns a fresh clone of the SharedStream (Rc) stored in the heap Opaque node (see value.rs:818-827 and 314-317). Pointer equality on those Rcs precisely captures "this is the identical stream object that is currently bound to stoutput" without depending on names, paths, or structural equality. When the test does (let S (open "..." out) (pr "payload" S)) under -q, S's Rc is distinct from the stdout Rc, so is_stdout is false and the write occurs. When the kernel does (pr ... (stoutput)) or direct uses of the global, ptrs match and hush is respected for true stdout. The "still silences stdout" contract is preserved while file writes are rescued. No use of shen_eq or string comparison on streams.
  Suggestion: (none)
  Status: verified correct

• Severity: none (positive verification)
  File:line: N/A (boot / override interaction)
  Description: Both boot paths (boot.rs:181 and 233) call register_hot_overrides after AOT installation (aot::kernel::install_all or the caller's install fn) and after shen.initialise + metadata. The ratatoskr-generated mains (e.g. out-demo/fib-rust/src/main.rs:28) do exactly boot_from_kl_source(..., Some(kernel_aot::install)) and then user install + metadata, so the hot_pr (with its aot_direct) reliably shadows both the tree-walked kernel pr and the AOT pr (and all the call sites inside aot/kernel/* that do rt::apply_direct(..., "pr", ...)). shen-cl uses an analogous native override strategy; the uniform "post-load native pr" fix for the writer.kl root cause is confirmed across the ratatoskr tree.
  Suggestion: (none)
  Status: verified correct

No other issues found in error paths, stream variants (In/Out/Closed all handled), clones (minimal and necessary), lock usage, race conditions (none possible), or canonical sources (untouched per PR contract and instructions). cargo fmt/clippy/build/test and 134/134 kernel certification are stated as clean in the PR body and are consistent with the minimal diff.

Review path: /tmp/grok-reviews/9ad17548-REVIEW.md
1-sentence verdict: The patch is correct, minimal, and precisely targets the canonical writer.kl hush gate using post-AOT Rc pointer identity on stoutput; the only issues are a pre-existing RefCell pattern and a low-severity missing arity guard in the new hot_pr (consistent with siblings), with the critical cli_launcher test appropriately flipped from asserting the bug.

---

Posted automatically by Grok reviewer subagents. See also the PENDING review (if any) for inline comments on the Files tab.

[pyrex41]

Key findings from independent reviewer subagent (deep ratatoskr dive including KLambda/writer.kl:3 root cause, builders, out-*, identity verification, and load-bearing test flip analysis):

Low (robustness)

• File: crates/shen-rust/src/primitives.rs (hot_pr body, post the read-file-as-string registration)
• Description: hot_pr does direct args[0].as_str() / args[1].as_stream() with no preceding if args.len() != 2. Under-application would panic on slice index instead of ShenError (AOT pr wrapper and apply paths protect normal cases, but sibling hot_* have the same pattern).
• Suggestion: Add explicit arity guard at the top of hot_pr (and consider backfilling other hot_* for consistency).

Info (pre-existing)

• File: crates/shen-rust/src/primitives.rs (RefCell borrow_mut paths, including the new hot_pr write)
• Description: let mut stream = target.borrow_mut() (same as existing write-byte etc.) can panic on re-entrant aliasing of the same stream object. Not introduced by this PR; Shen is single-threaded.
• Suggestion: No action for this minimal fix; document or harden later with try_borrow_mut if desired.

Info (documentation)

• The previous cli_launcher assertions/comments "locked in" the buggy zero-byte behavior as "CORRECT documented divergence". The PR correctly flipped the load-bearing test (quiet_does_not_silence_pr_to_file) and revised comments. Ratatoskr README caveat still mentions the historical lua/rust behavior (follow-up after merge).

Subagent verdict: Correct, minimal, right identity check (Rc::ptr_eq on SharedStream), post-AOT in both boot paths, "still silences stdout" half preserved, test flip appropriate. Full detailed review (with ratatoskr analysis) in previous top-level comment. cargo clean + 134/134 confirmed.

[pyrex41]

Could you add an arity guard before indexing args here? Normal apply paths should pass two arguments, but this hot override is also registered on the direct path, so a malformed/direct call would currently panic on args[0] or args[1] instead of returning a ShenError.

[pyrex41]

CI note: the failing gates (scripts/gates.sh) job does not look caused by this PR. The new quiet_does_not_silence_pr_to_file launcher test passes, and the failure is the same one already present on main in run 27473337148.

The failing case is reader_fuzz::corpus_never_panics: the corpus seed (absvector 100000000000) reaches the core absvector primitive, which tries to allocate the full vector and aborts the process with memory allocation of 800000000000 bytes failed / SIGABRT. Because Rust OOM aborts rather than unwinds, the fuzz harness cannot turn this into a normal test failure.

I’d track that separately as a shen-rust issue: cap or preflight absvector sizes so huge allocations return a catchable ShenError instead of aborting the process. That would also turn main green again.