Fix #22: pr to a file stream is silenced under *hush* (-q)

[pyrex41]

Fixes #22

Problem

The canonical kernel pr (klambda/writer.kl) gates all output on *hush*:

(defun pr (S ST) (if (value *hush*) S (...write S to ST...)))

So under -q/*hush*, a pr to a file stream wrote nothing, producing a zero-byte file. This diverges from shen-cl, shen-go, and ShenScript, which write to files regardless of *hush*. *hush* is intended to suppress only the interactive output that goes to standard output.

Fix

Add a native pr override in install_native_stdlib (prims.lua) that consults *hush* only when the target stream is the standard output stream (*stoutput*). Writes to any other stream (files, stderr) always occur. The actual write delegates to the original compiled-KL pr (with *hush* temporarily cleared and restored), so the write path stays byte-identical and global state is not perturbed.

The canonical/vendored kernel sources (klambda/, the kernel cert suite, etc.) are untouched.

Test changes

• Updated the test/cli_spec.lua assertion + comment that locked in the old zero-byte-file behavior (commented "WITH -q the file is empty / documented divergence") to assert the corrected behavior: under -q, the file now receives the payload.
• Added focused regression checks referencing issue pr to a file stream is silenced under *hush* (-q), producing zero-byte files #22 that *hush* still silences pr to standard output (the gate itself is preserved; only file/non-stdout streams are exempt), distinguishing the pr write from the -e value echo by returning a distinct value.

Results

• make test: 447 pass / 0 fail across 11 port specs.
• make certify: canonical Shen kernel certification 100% (exit 0), no regression.

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: #23 (Fix #22: pr to a file stream is silenced under hush (-q))

Summary

This PR adds a targeted native override for the pr primitive inside install_native_stdlib (post kernel load) to fix the divergence where -q / *hush* (set via the launcher and extension-launcher) caused the canonical kernel pr (from klambda/writer.kl:3 and the identical ratatoskr/KLambda/writer.kl:3) to early-return for all streams:

(defun pr (V6806 V6807) (if (value *hush*) V6806 (if (shen.char-stoutput? V6807) ...write...)))

(See: ratatoskr/README.md:124-130 (hush caveat section calling out exactly that lua/rust needed -q dropped for ratatoskr runs), ratatoskr/KLambda/sys.kl:234 (hush?), writer.kl:3, klambda/sys.kl:207 (stoutput), klambda/writer.kl:3, klambda/init.kl: (set hush false), and shen-lua equivalents.)

The fix (only in prims.lua + cli_spec.lua; klambda/ and cert untouched) captures orig_pr = F["pr"] after compiled KL is loaded into F, then installs a wrapper:

• If *hush* and target st == GLOBALS["*stoutput*"] (reference identity on the exact table object registered at boot), return s early (preserves the gate for stdout).
• Else if *hush*, save *hush*, temporarily set false, pcall(orig_pr, s, st) (so kernel pr takes its write branch and delegates to write-chars/write-byte etc. with identical bytecode path), restore, re-raise or return.
• Else delegate normally.

Stream objects are Lua tables with private Stream metatable (prims.lua:373-384: mk_out_stream/mk_in_stream; shen.char-stoutput? and shen.char-stinput? are hard-wired false because this port uses byte streams, not char streams). *stoutput* (and friends) are populated in boot.lua:81-87 (and identically in bin/ratatoskr-build.lua:376-381) before kernel load, using the same P.mk_out_stream for the real stdout handle. stoutput (sys.kl:207) just does (value *stoutput*). Assignment to F via the local install helper also sets FA arity.

This matches the cross-port pattern (shen-cl has a native pr override; shen-go/ShenScript route around the gate). Lua == on tables is reference identity (no __eq metatable), exactly analogous to CL EQ / Rust ptr_eq / Rc identity on the registered *stoutput* object. The wrapper is installed late (boot.lua:404/415/422 after compile_kernel / cached load + chunks run; ratatoskr-build.lua:438-439 after kernel chunks and before (shen.initialise)), so it overrides the compiled-KL pr.

Tests: cli_spec.lua updated to assert payload is written under -q for file streams (flipped from the old "documented divergence" zero-byte assertion), plus a new "other half" regression (cli_spec.lua:210-229) that -q still silences pr to (stoutput) itself (using distinct marker + return value 99 to avoid -e echo confusion). 447/447 + cert 100% reported.

Correctness first assessment: The implementation is correct. The temporary clear/restore is exception-safe (restore before the if not ok re-raise). No kernel sources touched. The "stdout identity" test is robust for the documented use cases (distinct open streams never compare == to the boot-registered stdout table; live GLOBALS["*stoutput*"] lookup respects (set *stoutput* ...) rebinds). Direct GLOBALS mutation for the temp clear is consistent with bin/shen and test/interop_spec.lua's hushed helper.

Issues

• Severity: nit
  File: /Users/reuben/projects/shen/shen-lua/prims.lua:753
  Description: The st == GLOBALS["*stoutput*"] test (post-change line) uses raw Lua table reference equality. This is correct and intentional (streams are the table objects returned by mk_out_stream; P.Stream metatable has no __eq; std streams assigned once at boot.lua:85 and never recreated for the canonical stdout; open always produces a distinct table; stoutput returns the live GLOBALS value). It mirrors CL EQ usage in ratatoskr/Primitives/CL/char-stoutput.lsp:5 and the port's own char-stoutput? intent. However, there is no local helper or comment at the comparison site explaining why identity (vs. e.g. name field or pointer) is used and why it is safe here vs. user streams.
  Suggestion: Add a tiny helper or inline comment, e.g. local function is_stdout(st) return st == GLOBALS["*stoutput*"] end -- reference identity on the exact boot-registered Stream table (cf. CL EQ / Rust ptr_eq).
  Status: open

• Severity: suggestion
  File: /Users/reuben/projects/shen/shen-lua/prims.lua:758-761
  Description: Direct mutation GLOBALS["*hush*"] = false / restore (bypassing the set primitive at prims.lua:314-316 and the KL value path) is used inside the non-stdout hush case so that the captured orig_pr (the compiled writer.kl body) will observe falsy on its (value *hush*) test. This is consistent with launcher code (bin/shen:62,89) and the test helper (test/interop_spec.lua:28-32, which also does save/restore via F["set"] + pcall). No side effects or observers on hush are documented beyond the pr gate and hush?/hush in sys.kl, so it is safe. Still, it is an exception to the normal set/value protocol.
  Suggestion: Add an explicit comment at the save/restore block: -- Direct GLOBALS write for *hush* is intentional (matches bin/shen and interop hushed helper); kernel pr observes it via (value *hush*) inside the pcall.
  Status: open

• Severity: nit
  File: /Users/reuben/projects/shen/shen-lua/prims.lua:762
  Description: error(res, 0) on pcall failure re-raises the (already Shen-translated via TOEXCN in some paths) error object. This matches the style of other pcall-rethrow wrappers in the same file and in boot.lua (e.g. 704-705, 780). Level 0 avoids extra location prefix. No incorrectness, but the choice of level could be documented alongside the exception-safety claim in the big comment at 740-750.
  Suggestion: Extend the existing wrapper comment (lines 740-750) with one sentence on error path: "pcall + restore-before-re-raise keeps hush unperturbed even on exceptions from the delegated write."
  Status: open

• Severity: nit
  File: /Users/reuben/projects/shen/ratatoskr/README.md:124-130
  Description: The "hush caveat" section explicitly calls out the pre-fix behavior for shen-lua ("on shen-lua and shen-rust that silences the pr writes to the output files, producing zero-byte artifacts — omit -q on those two") and contrasts it with shen-cl (native pr override), shen-go and ShenScript. This was the "unifying root cause" reference (canonical writer.kl + ratatoskr/README). The current PR fixes only shen-lua (minimal port-only change); ratatoskr/README is correctly left untouched per scope.
  Suggestion: (For a later commit after the shen-rust counterpart lands): update the caveat to say "pre-Fix #22: pr to a file stream is silenced under *hush* (-q) #23 shen-lua" or "shen-rust (and historical shen-lua)" and note that -q is now safe on shen-lua.
  Status: open

• Severity: nit
  File: /Users/reuben/projects/shen/shen-lua/test/cli_spec.lua:216-219 (and 102-107 in the other half block)
  Description: The "other half" test (and its explanatory comment) correctly uses a distinct marker + trailing 99 return value under -e so that the only way the marker appears in captured stdout is via the actual pr write to (stoutput) (rather than the -e echo of the form's value). This is a good defensive choice. The test also verifies the positive case (without -q the marker is present). No issues in logic.
  Suggestion: (Minor polish) The marker name NOISE_22_MARKER is clear in context; consider a constant or slightly more unique string if the suite ever grows similar -e + stdout-silence tests, but current form is sufficient.
  Status: open

• Severity: suggestion
  File: /Users/reuben/projects/shen/shen-lua/prims.lua:751 (and boot.lua:293-298, ratatoskr-build.lua:434-439)
  Description: orig_pr capture + wrapper + install("pr", ...) happens inside install_native_stdlib, which is deliberately called after all kernel chunks have executed (so F["pr"] holds the compiled writer.kl version). The same ordering is reproduced in the ratatoskr single-file builder. This is correct and documented in the surrounding comments for the whole family of native overrides. No ordering bug.
  Suggestion: The pr-specific comment block (740-750) could cross-reference the call sites (boot.lua:298 and the builder) for future readers tracing "when does the override take effect vs. (shen.initialise)".
  Status: open

No correctness, edge-case, race, or error-handling bugs were identified. The pcall restore is before any re-raise (prims.lua:760 then 761-762). The "stdout gate preservation" half of the spec is present and passing by construction. No unwrap/clone/lock concerns (Lua port). Stream identity is the right mechanism here and is used consistently with how this port (and ratatoskr's CL primitives) identify the canonical stdout object.

Verdict

Review notes written to /tmp/grok-reviews/99900d2e-REVIEW.md. The PR is a clean, minimal, correct post-load native pr override that restores cross-port semantics for file-stream pr under *hush* while preserving the stdout silencing gate (and the byte-identical write path), with appropriately flipped + augmented CLI tests; only nits around comments and out-of-scope docs.

---

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

[pyrex41]

Notes from independent reviewer subagent (ratatoskr/KLambda/writer.kl + sys.kl root cause, boot/ratatoskr-build timing, stream identity, pcall safety, and "other half" test verification):

Nit

• File: shen-lua/prims.lua:753 (the st == GLOBALS["*stoutput*"] test)
• Description: Raw Lua table reference equality is correct (streams are the tables from mk_out_stream; no __eq; boot-registered once; open() always distinct; respects live GLOBALS value for rebinds). But no local helper or comment explaining why identity (vs name/field) and why safe.
• Suggestion: Add tiny helper or inline comment, e.g. local function is_stdout(st) return st == GLOBALS["*stoutput*"] end -- reference identity... (cf. CL EQ / Rust ptr_eq).

Suggestion

• File: shen-lua/prims.lua:758-761 (temp hush clear/restore)
• Description: Direct GLOBALS["*hush*"] = false / restore (bypassing the set primitive) is used so the captured orig_pr observes the kernel (value *hush*) test. Consistent with bin/shen and interop_spec.lua's hushed helper, but an exception to normal set/value protocol.
• Suggestion: Add explicit comment at the save/restore block explaining the intent and that it's pre-existing practice here.

Nit (out-of-scope but noted)

• File: ratatoskr/README.md:124-130 (hush caveat section)
• Description: Still documents the pre-fix zero-byte behavior for shen-lua (and shen-rust). This PR (and the rust counterpart) fix the root cause.
• Suggestion: After both land, update to "pre-Fix #22: pr to a file stream is silenced under *hush* (-q) #23 shen-lua / historical shen-rust" and note -q is now safe on shen-lua for ratatoskr runs.

Other nits around pcall error re-raise level, comment cross-refs to boot sites, and marker name polish. Core: pcall+restore is exception-safe, identity is robust, override timing correct (after kernel chunks), "other half" test (distinct marker + 99 return) correctly proves the stdout gate is preserved. Full review in previous comment. 447/447 + cert 100% confirmed.