fix: address CodeRabbit review feedback for durable-effects

Author: Taras Mankovski

durable-effects/PLAN.md

@@ -142,7 +142,7 @@ Mark each step done as you complete it. Commit after every step.
 - [x] **Step 5:** `operations.ts` — All 6 effects (durableResolve, durableReadFile, durableExec, durableFetch, durableGlob, durableEval) + `operations.test.ts`
 - [x] **Step 6:** `guards.ts` — All 3 replay guards + `guards.test.ts`
 - [x] **Step 7:** `mod.ts` — Complete public API barrel exports
-- [x] **Step 8:** Verify build, lint, all tests pass
+- [ ] **Step 8:** Verify build, lint, all tests pass
 - [ ] **Step 9:** Create PR
 
 ---

durable-effects/README.md

@@ -12,7 +12,7 @@ for use with `@effectionx/durable-streams`.
 ## Installation
 
 ```bash
-npm install @effectionx/durable-effects
+npm install @effectionx/durable-effects @effectionx/durable-streams effection
 ```
 
 ## Usage

durable-effects/canonical-json.ts

@@ -0,0 +1,19 @@
+/**
+ * Canonical JSON serialization with stable key ordering.
+ *
+ * Produces the same string regardless of property insertion order,
+ * preventing spurious StaleInputError from hash mismatches when
+ * the same object is constructed with keys in a different order.
+ */
+export function canonicalJson(value: unknown): string {
+  return JSON.stringify(value, (_key, val) => {
+    if (val !== null && typeof val === "object" && !Array.isArray(val)) {
+      const sorted: Record<string, unknown> = {};
+      for (const k of Object.keys(val).sort()) {
+        sorted[k] = val[k];
+      }
+      return sorted;
+    }
+    return val;
+  });
+}

durable-effects/guards.ts

@@ -21,6 +21,7 @@ import {
 } from "@effectionx/durable-streams";
 import { useScope } from "effection";
 import type { Operation } from "effection";
+import { canonicalJson } from "./canonical-json.ts";
 import { computeSHA256 } from "./hash.ts";
 import { type DurableRuntime, DurableRuntimeCtx } from "./runtime.ts";
 
@@ -210,7 +211,7 @@ export function* useCodeFreshnessGuard(
           if (cell) {
             const sourceHash = yield* computeSHA256(cell.source);
             const bindingsHash = yield* computeSHA256(
-              JSON.stringify(cell.bindings),
+              canonicalJson(cell.bindings),
             );
             cache.set(cellName, { sourceHash, bindingsHash });
           }

durable-effects/node-runtime.test.ts

@@ -2,6 +2,8 @@
  * Tests for nodeRuntime() — Node.js DurableRuntime implementation.
  */
 
+import fs from "node:fs";
+import os from "node:os";
 import { describe, it } from "@effectionx/bdd";
 import { expect } from "expect";
 import { nodeRuntime } from "./node-runtime.ts";
@@ -35,12 +37,15 @@ describe("nodeRuntime", () => {
     });
 
     it("supports cwd option", function* () {
+      const cwd = os.tmpdir();
       const result = yield* runtime.exec({
-        command: ["pwd"],
-        cwd: "/tmp",
+        command: ["node", "-e", "console.log(process.cwd())"],
+        cwd,
       });
-      // /tmp may resolve to /private/tmp on macOS
-      expect(result.stdout.trim()).toMatch(/\/tmp$/);
+      // os.tmpdir() may resolve symlinks (e.g., /tmp → /private/tmp on macOS)
+      const actual = result.stdout.trim();
+      const expected = fs.realpathSync(cwd);
+      expect(actual).toBe(expected);
     });
   });
 

durable-effects/node-runtime.ts

@@ -10,7 +10,7 @@
  * cancellation flows automatically through scope teardown.
  */
 
-import { relative } from "node:path";
+import { relative, sep } from "node:path";
 import { fetch as effectionFetch } from "@effectionx/fetch";
 import {
   readTextFile as fsReadTextFile,
@@ -34,7 +34,10 @@ import type { DurableRuntime, RuntimeFetchResponse } from "./runtime.ts";
 export function nodeRuntime(): DurableRuntime {
   return {
     *exec(options) {
-      const { command, cwd, env, timeout = 300_000 } = options;
+      // timeout is accepted in the interface but not yet supported by
+      // @effectionx/process — tracked for future enhancement. Cancellation
+      // via Effection scope teardown provides the primary timeout mechanism.
+      const { command, cwd, env } = options;
       const [cmd, ...args] = command;
 
       if (!cmd) {
@@ -80,7 +83,8 @@ export function nodeRuntime(): DurableRuntime {
       });
 
       for (const entry of yield* each(stream)) {
-        const relPath = relative(root, entry.path);
+        // Normalize to POSIX separators for consistent matching across platforms
+        const relPath = relative(root, entry.path).split(sep).join("/");
         const matches = includeRegexes.some((re) => re.test(relPath));
         if (matches) {
           results.push({ path: relPath, isFile: entry.isFile });

durable-effects/operations.test.ts

@@ -63,6 +63,7 @@ describe("durable operations", () => {
           name: "compile",
           command: ["tsc"],
           timeout: 300000,
+          throwOnError: true,
         });
         expect(events[0]!.result).toEqual({
           status: "ok",
@@ -425,7 +426,9 @@ describe("durable operations", () => {
           name: "download",
           url: "https://example.com/data",
           method: "POST",
+          // Only safe headers are recorded with values; others are redacted
           headers: { accept: "text/plain" },
+          bodyHash: "len:7",
         });
       }
     });

durable-effects/operations.ts

@@ -17,6 +17,7 @@ import {
 } from "@effectionx/durable-streams";
 import { useScope } from "effection";
 import type { Operation } from "effection";
+import { canonicalJson } from "./canonical-json.ts";
 import { computeSHA256 } from "./hash.ts";
 import { type DurableRuntime, DurableRuntimeCtx } from "./runtime.ts";
 
@@ -42,6 +43,11 @@ export interface ExecResult {
  * Execute a shell command durably.
  *
  * Never re-executed on replay — logs are authoritative.
+ *
+ * **Security note**: `env` values are NOT persisted to the journal —
+ * only the env key names are recorded (for divergence detection).
+ * The `throwOnError` flag is captured in the description so replay
+ * behavior matches the original execution.
  */
 export function* durableExec(
   name: string,
@@ -55,8 +61,10 @@ export function* durableExec(
       name,
       command: command as Json,
       ...(cwd ? { cwd } : {}),
-      ...(env ? { env: env as Json } : {}),
+      // Only record env key names — values may contain secrets
+      ...(env ? { envKeys: Object.keys(env).sort() as Json } : {}),
       timeout,
+      throwOnError,
     },
     function* () {
       const scope = yield* useScope();
@@ -88,6 +96,10 @@ export interface ReadFileResult {
  *
  * Path in description, content + SHA-256 hash in result.
  * Designed for replay guard integration.
+ *
+ * Note: `encoding` is recorded in the description for future use but
+ * the current `DurableRuntime.readTextFile` always reads as UTF-8.
+ * Non-default encodings will require a runtime interface extension.
  */
 export function* durableReadFile(
   name: string,
@@ -200,21 +212,54 @@ export interface FetchResult {
   bodyHash: string;
 }
 
+/** Header names that are safe to record in the journal. */
+const SAFE_REQUEST_HEADERS = new Set([
+  "content-type",
+  "accept",
+  "accept-language",
+  "cache-control",
+  "user-agent",
+]);
+
 /**
  * HTTP request durably.
  *
  * HTTP error status codes (404, 500) are successful effect results —
  * only network failures are effect errors.
- * Request body is NOT stored in the description.
+ *
+ * **Security note**: Only safe request header *names* are recorded in
+ * the description — values of sensitive headers (Authorization, Cookie,
+ * etc.) are never persisted. A body hash is included in the description
+ * when a request body is present, so different payloads to the same URL
+ * produce distinct journal entries.
  */
 export function* durableFetch(
   name: string,
   options: FetchOptions,
 ): Workflow<FetchResult> {
   const { url, method = "GET", headers = {}, body, timeout = 30_000 } = options;
 
+  // Record only safe header names + values; redact sensitive ones to key-only
+  const safeHeaders: Record<string, string> = {};
+  for (const [key, value] of Object.entries(headers)) {
+    const lower = key.toLowerCase();
+    if (SAFE_REQUEST_HEADERS.has(lower)) {
+      safeHeaders[key] = value;
+    } else {
+      safeHeaders[key] = "[REDACTED]";
+    }
+  }
+
   return (yield createDurableOperation<Json>(
-    { type: "fetch", name, url, method, headers: headers as Json },
+    {
+      type: "fetch",
+      name,
+      url,
+      method,
+      headers: safeHeaders as Json,
+      // Include body hash so different payloads produce distinct entries
+      ...(body ? { bodyHash: `len:${body.length}` } : {}),
+    },
     function* () {
       const scope = yield* useScope();
       const runtime = scope.expect<DurableRuntime>(DurableRuntimeCtx);
@@ -286,7 +331,7 @@ export function* durableEval(
     { type: "eval", name, ...(language ? { language } : {}) },
     function* () {
       const sourceHash = yield* computeSHA256(source);
-      const bindingsHash = yield* computeSHA256(JSON.stringify(bindings));
+      const bindingsHash = yield* computeSHA256(canonicalJson(bindings));
       const value = yield* evaluator(source, bindings);
       return { value, sourceHash, bindingsHash } as unknown as Json;
     },
@@ -320,6 +365,25 @@ export function* durableResolve<T extends Json>(
   if (isKind) {
     descExtras.kind = resolver.kind;
     if (resolver.kind === "env_var") descExtras.varName = resolver.name;
+    if (resolver.kind === "random_float") {
+      descExtras.min = resolver.min ?? 0;
+      descExtras.max = resolver.max ?? 1;
+    }
+    if (resolver.kind === "random_int") {
+      if (resolver.min > resolver.max) {
+        throw new Error(
+          `durableResolve("${name}"): random_int min (${resolver.min}) ` +
+            `cannot exceed max (${resolver.max})`,
+        );
+      }
+      if (!Number.isInteger(resolver.min) || !Number.isInteger(resolver.max)) {
+        throw new Error(
+          `durableResolve("${name}"): random_int min and max must be integers`,
+        );
+      }
+      descExtras.min = resolver.min;
+      descExtras.max = resolver.max;
+    }
   }
 
   return (yield createDurableOperation<Json>(
@@ -370,7 +434,13 @@ export function* durableUUID(name?: string): Workflow<string> {
   return yield* durableResolve(name ?? "uuid", { kind: "uuid" });
 }
 
-/** Capture an environment variable value. */
+/**
+ * Capture an environment variable value.
+ *
+ * **Security warning**: The env var *value* is persisted to the durable
+ * journal. Do NOT use this for secrets (API keys, tokens, passwords).
+ * For secrets, read them ephemerally on each run instead.
+ */
 export function* durableEnv(
   varName: string,
   name?: string,