fix(modal): surface the reason that ended the worker by aviggiano · Pull Request #335 · monad-developers/ultrafuzz

aviggiano · 2026-08-07T15:51:18Z

Closes #307.

PR #309 tried the one-line fix and was correctly closed: the reason is destroyed upstream, so printing at :619 alone emits only OperationalDispositionError: worker operation failed.

This retains a bounded, sanitized stderr tail in runChecked and attaches it as cause before touching the print — reversing that order dumps up to 32 MB of workspace diff. Also routes public-worker.ts:928-933 (which embedded 4000 raw stderr bytes) through the same redactor, and adds a frame-only stack plus a String(error) fallback so a null rejection no longer prints a bare worker terminated: .

Known limitation: the worker.ts half cannot be unit-tested — it reads /data, /opt/ultrafuzz and requiredEnv at module scope. The public-worker.ts half is covered.

Greptile Summary

This PR retains and sanitizes bounded child stderr, attaches it to worker failure causes, and emits bounded cause-chain and stack diagnostics when a Modal worker terminates.

Adds shared worker diagnostic sanitization, bounded stderr retention, cause rendering, and frame-only stack rendering.
Routes public-worker command failures through the shared sanitizer.
Adds coverage for stderr retention, redaction, cause chains, rejection fallbacks, and stack diagnostics.

Confidence Score: 4/5

The PR appears safe to merge with non-blocking follow-up recommended for exact credential redaction and Unicode-safe diagnostic truncation.

The failure-reporting flow is bounded and generally sanitized, but opaque judge credentials are not supplied to the exact-value redactor and long Unicode diagnostics can be malformed at byte boundaries.

Files Needing Attention: packages/modal/src/worker.ts, packages/modal/src/worker-diagnostics.ts

Security Review

The new private-worker stderr path does not provide the injected judge credential as an exact forbidden value, so an opaque credential echoed by a failed scoring child can bypass pattern-based redaction and reach terminal stderr.

Important Files Changed

Filename	Overview
packages/modal/src/worker-diagnostics.ts	Adds bounded stderr collection and diagnostic rendering, but truncates UTF-8 at arbitrary byte boundaries.
packages/modal/src/worker.ts	Surfaces subprocess causes and stack frames; the private scoring path omits exact redaction of its injected credential.
packages/modal/src/public-worker.ts	Reuses the shared sanitizer for public diagnostics and failed-command stderr while supplying known forbidden values.
packages/modal/test/worker-diagnostics.test.ts	Thoroughly covers bounded retention, redaction, cause rendering, payload avoidance, and stack extraction.
packages/modal/test/public-worker.test.ts	Adds focused coverage that public-worker failure causes redact secret-like stderr content.

Sequence Diagram

sequenceDiagram
  participant Worker as worker.ts
  participant Child as eval subprocess
  participant Tail as Bounded stderr tail
  participant Diagnostic as Diagnostic sanitizer
  participant Terminal as Worker terminal
  Worker->>Child: spawn with merged environment
  Child-->>Tail: stderr chunks
  Child-->>Worker: non-zero exit
  Worker->>Diagnostic: construct cause from stderr tail
  Diagnostic-->>Worker: bounded, pattern-redacted reason
  Worker->>Terminal: print cause chain and stack frames

Prompt To Fix All With AI

### Issue 1
packages/modal/src/worker.ts:533
**Injected credential lacks exact redaction**

If the scoring subprocess writes an opaque `ULTRAFUZZ_EVAL_JUDGE_API_KEY` value to stderr, `childExitFailureCause` sanitizes it without the injected credential in `forbiddenSecretValues`, so pattern-based redaction can leave the credential visible in the terminal diagnostic.

**How this was verified:** The judge credential flows through `options.env` into the child, while the resulting stderr cause is constructed without passing that value to the exact-value redactor.

### Issue 2
packages/modal/src/worker-diagnostics.ts:47-49
**Truncation splits UTF-8 characters**

Slicing the encoded buffer at an arbitrary byte offset can split a multibyte character, causing long termination reasons or stack diagnostics to contain replacement characters and lose text at the boundary.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "fix(modal): keep the failing frames the ..." | Re-trigger Greptile}

Greptile also left 2 inline comments on this PR.

Context used:

Knowledge Base — Modal remote benchmark execution

The worker's top-level catch discarded the rejection value, so every failure outside the two classes `diagnosticCodeForError` recognises produced the literal string "worker terminated" and exit code 1. Binding the error alone would not have helped: `runChecked` drains child stderr into a no-op and throws `OperationalDispositionError` with no `cause`, so the reason was already gone two layers earlier. Retain a bounded, redacted stderr tail in `runChecked` and attach it as `cause`, then render the whole `cause` chain at the top-level catch. The chain is rendered by reading only `name`, `code` and `message` at each link — never by inspecting the error object, because an `execFileSync` ENOBUFS `SystemError` carries `output`/`stdout`/`stderr` holding up to the whole captured workspace diff. The mirror leak in `public-worker.ts` — 4000 raw stderr bytes embedded as a `cause`, hidden today only because `.stack` does not render it — now goes through the same sanitizer, which is `public-worker.ts`'s own redaction path lifted into a shared module. Refs #307. Supersedes #309. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…arry `describeWorkerTermination` reads only `name`, `code` and `message`, so the class of failure #307 is actually chasing -- an unanticipated error in the worker's own code -- prints "TypeError: Cannot read properties of undefined" and names no file and no line. The stated reason for dropping the stack does not hold: `Error.prototype.stack` is a plain string of `name: message` plus frames and never carries an error's own enumerable properties, so an `execFileSync` ENOBUFS `SystemError`'s `output`/`stdout`/`stderr` cannot reach it. That is now measured by a test rather than assumed. Add `workerTerminationStack`, which redacts and bounds those frames the same way every other diagnostic string in this module is bounded, and print it from the top-level catch. Also make a rejection with `undefined` or `null` name itself instead of collapsing to the bare prefix the issue is about. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

greptile-apps · 2026-08-07T15:54:29Z

    await appendGenericLog("operation-failed");
-    throw new OperationalDispositionError(failureCategory(options));
+    throw new OperationalDispositionError(failureCategory(options), {
+      cause: childExitFailureCause(options.label, exitCode, stderrTail)


Injected credential lacks exact redaction

If the scoring subprocess writes an opaque ULTRAFUZZ_EVAL_JUDGE_API_KEY value to stderr, childExitFailureCause sanitizes it without the injected credential in forbiddenSecretValues, so pattern-based redaction can leave the credential visible in the terminal diagnostic.

How this was verified: The judge credential flows through options.env into the child, while the resulting stderr cause is constructed without passing that value to the exact-value redactor.

Knowledge Base Used: Modal remote benchmark execution

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/modal/src/worker.ts Line: 533 Comment: **Injected credential lacks exact redaction** If the scoring subprocess writes an opaque `ULTRAFUZZ_EVAL_JUDGE_API_KEY` value to stderr, `childExitFailureCause` sanitizes it without the injected credential in `forbiddenSecretValues`, so pattern-based redaction can leave the credential visible in the terminal diagnostic. **How this was verified:** The judge credential flows through `options.env` into the child, while the resulting stderr cause is constructed without passing that value to the exact-value redactor. **Knowledge Base Used:** [Modal remote benchmark execution](https://app.greptile.com/monad-foudnation/-/custom-context/knowledge-base/monad-developers/ultrafuzz/-/docs/modal.md) --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

greptile-apps · 2026-08-07T15:54:29Z

+  return (options.keep === "head" ? bytes.subarray(0, maxBytes) : bytes.subarray(bytes.length - maxBytes)).toString(
+    "utf8"
+  );


Truncation splits UTF-8 characters

Slicing the encoded buffer at an arbitrary byte offset can split a multibyte character, causing long termination reasons or stack diagnostics to contain replacement characters and lose text at the boundary.

Knowledge Base Used: Modal remote benchmark execution

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/modal/src/worker-diagnostics.ts Line: 47-49 Comment: **Truncation splits UTF-8 characters** Slicing the encoded buffer at an arbitrary byte offset can split a multibyte character, causing long termination reasons or stack diagnostics to contain replacement characters and lose text at the boundary. **Knowledge Base Used:** [Modal remote benchmark execution](https://app.greptile.com/monad-foudnation/-/custom-context/knowledge-base/monad-developers/ultrafuzz/-/docs/modal.md) --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Four files conflicted; every resolution keeps both sides' behaviour. workflow.tsx -- both destructuring imports were unions, not choices. Kept #344's `checkInvariantSourcePinned` / `invariantPinnedSourceRefExists` and #342's `validateWorkspacePatchCapture` alongside #188's cloud-selected-task, finding-provenance and dynamic-topology symbols. The rest of the file merged without overlap: #334's `invariantSuiteGitPaths` still bounds all five invariant-discovery enumerations, #342's single ancestor-wide suite budget and depth bound are intact, #343's per-path publisher resolution still fails closed on unordered siblings, and #188's dynamic `taskSpecs` reassignment happens before any of those helpers read it, so they see the expanded node set. public-worker.ts -- kept #188's `REFERENCE_GITHUB_TOKEN_ENV` import and dropped `redactSecretsInText`, whose only consumer (`sanitizePublicDiagnosticMessage`) #335 deleted when it routed the stderr tail through `sanitizeWorkerDiagnosticMessage`. The two compose: the reference token #188 adds to `publicBenchmarkWorkerSecretValues` is now redacted from the bounded stderr tail as well. node-provider.test.ts -- unioned `logicalDispatchFingerprint` and `verificationMarker` into one options type, and moved #339's three new verification-marker tests onto #188's two-argument `createResultArchive` signature so their bundles carry a real `logical_dispatch_fingerprint` instead of bypassing the provenance check. topology-transform.test.ts -- kept both new tests. #342's invariant-only exclusion test now reads the `REPOSITORY_ROOT` constant #188 hoisted rather than recomputing the path, so the file keeps one spelling of the repository root. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Ubuntu and others added 2 commits August 7, 2026 14:30

aviggiano merged commit 810852e into main Aug 7, 2026
8 checks passed

aviggiano deleted the fix/worker-failure-reason branch August 7, 2026 15:53

greptile-apps Bot reviewed Aug 7, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix(modal): surface the reason that ended the worker - #335

fix(modal): surface the reason that ended the worker#335
aviggiano merged 2 commits into
mainfrom
fix/worker-failure-reason

aviggiano commented Aug 7, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

Uh oh!

greptile-apps Bot Aug 7, 2026

Uh oh!

greptile-apps Bot Aug 7, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

aviggiano commented Aug 7, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Security Review

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

greptile-apps Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aviggiano commented Aug 7, 2026 •

edited by greptile-apps Bot

Loading