Always surface the Nix eval error, never a stale warning by domenkozar · Pull Request #2849 · cachix/devenv

domenkozar · 2026-05-19T17:32:07Z

Summary

Fixes #2820: when Nix emitted a warning at error verbosity (e.g. Ignoring the client-specified setting 'system' for untrusted users), devenv selected that log line as the error message and dropped the real error returned by eval. Syntax errors and actionable suggestions like devenv inputs add … were hidden behind the warning.

Root cause (thanks @sandydoo): the warning's color changed, looks_like_warning stopped matching, and the fallback treated the warning line as the main error.

What changed

The error returned by eval is now always rendered, in full and in Nix's natural order: --show-trace frames first, the actionable error: … paragraph last, at the bottom of the terminal where the cursor lands.
Dropped the brittle log-line heuristics entirely: select_raw_error, looks_like_warning, and peek_pre_repl_errors are gone. There is no path left where a captured log line replaces the eval error, and no color matching that can rot.
The only remaining text processing is dedenting the uniform left margin the C bindings add.

An earlier revision of this PR reordered the output to put the actionable paragraph above the trace; that was reverted per review since it forced scrolling up past the trace to find it.

Tests

Unit tests on format_eval_error: eval error always rendered (warning-shadow regression), natural trace-then-error order preserved, bare syntax error, no-error:-line fallback, dedenting.
New tests/missing-input-error/ integration test: the devenv inputs add suggestion is printed below the trace frames, and the headline never carries the stale warning.
tests/syntax-error/.test.sh gains an assertion that the warning does not shadow the syntax error.

Test plan

cargo nextest run -p devenv-nix-backend error::
devenv-run-tests run --only syntax-error --only missing-input-error tests

🤖 Generated with Claude Code

cloudflare-workers-and-pages · 2026-05-19T17:32:24Z

Deploying devenv with Cloudflare Pages

Latest commit:	`c0174f8`
Status:	✅ Deploy successful!
Preview URL:	https://1fe5a2b9.devenv.pages.dev
Branch Preview URL:	https://fix-2820-actionable-error.devenv.pages.dev

View logs

github-actions · 2026-05-20T13:55:05Z

🔍 Suggested Reviewers

Based on git blame analysis of the changed lines, the following contributors have significant experience with the modified code:

@sandydoo - 100.0% of changed lines (56 lines)

Please consider reviewing this PR as you have authored significant portions of the code being modified. Your expertise would be valuable! 🙏

This comment was automatically generated by git-blame-auto-reviewer

Last updated: 2026-05-29T22:29:44.550Z

sandydoo · 2026-05-29T13:05:45Z

+- Fixed import path validation rejecting valid input-relative imports on macOS, where `TMPDIR` resolves via the `/var → /private/var` symlink (`Import path '<name>/...' resolves outside the git repository`). `validate_within_root` now canonicalizes the longest existing ancestor of the path before comparing against the root.
+- Fixed `devenv inputs add` from a subdirectory writing to a stray `devenv.yaml` in the subdir instead of the enclosing project. It now walks up to find `devenv.nix` the same way `devenv shell` does, so the input is added where the rest of devenv reads it.


Looks like some other project dir fixes got bundled in

Good catch — split out into separate PRs. The devenv inputs add subdir fix is now #2883, and the macOS import-path validation fix is #2884. This PR is now just the error-surfacing work for #2820.

I split them into separate PRs

sandydoo · 2026-05-29T13:11:52Z

+/// `devenv.nix` so the command operates on the enclosing project, matching
+/// where `devenv shell` would run. Best-effort — a no-op if discovery or the
+/// chdir fails.
+fn enter_discovered_project_root() {


Worth adding some traces here for the failure modes.

I'd also think about whether we might want to resolve the project root earlier in the flow. Maybe devenv allow also wants the project root? And do we even want to chdir at all?

Added debug/warn traces for the three failure modes (unreadable cwd, no enclosing devenv.nix, and a found-but-failed chdir, which is the one that actually leaves inputs add writing to the wrong place). Since this devenv inputs add change is unrelated to error surfacing, I moved it to #2883 — the traces live there now.

On the design questions: agreed that resolving the project root once, earlier in the flow, would be cleaner than the two ad-hoc discovery sites, and devenv allow probably wants it too (incl. whether we should chdir at all vs. threading the root through). I'd rather not expand the split-out PR's scope, so I captured it as a follow-up note on #2883.

sandydoo

👍

A Nix eval error with `show-trace=true` (always on in devenv) rendered as a single line headline `Failed to get shell attribute from devenv:` followed by ~120 lines of `lib/modules.nix` frames, with the actionable text (the thrown assertion, e.g. `$ devenv inputs add git-hooks ...`) buried at the very bottom. The follow up comments on #2820 reported the suggestion as "swallowed". `format_eval_error` now extracts the trailing `error: ...` paragraph from the FFI return value and uses it as the diagnostic headline; the preceding frames move to miette's `help:` section. When no `error:` paragraph is present (malformed input, future format change), the original text is rendered as one block (graceful fallback). Vestigial code removed: `select_raw_error`, `looks_like_warning`, `peek_pre_repl_errors`. The C bindings now return the full error via FFI rather than the log callback, so the log buffer merge no longer contributes anything. `take_pre_repl_errors` stays because `launch_repl` uses it to replay TUI swallowed log lines. Tests: * 5 new unit tests on `format_eval_error` covering the happy path, the bare syntax error shape, warning shadow regression, graceful degradation, and indent stripping (no FFI required, all under 40ms). * `tests/missing-input-error/` new integration test asserting the `devenv inputs add ...` suggestion appears alongside the headline. * `tests/syntax-error/.test.sh` gains a warning shadow assertion on the headline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

`split_trailing_error` matched any line whose stripped prefix was `error:`. That misfired on user-authored multi-line `throw`s whose message body contained a line like `error: my note`: my parser picked the user's literal text as the boundary, leaving the real Nix `error:` keyword and the leading lines of the throw in the demoted trace. Reproduction: env.GREET = throw '' Step 1: do this error: but this is the actual problem Step 3: then this ''; Before: headline `× ... error: but this is the actual problem` plus `Step 3: then this`, with `Step 1: do this` buried in `help:`. After: headline starts with Nix's real `error:` keyword and carries the full user throw body. Discriminator: Nix's real `error:` paragraphs are always separated from preceding content by at least one blank line. User content inside a `throw` sits on consecutive lines. Require the boundary line to be preceded by a blank line (or start of input). Tests: * New `split_trailing_error_ignores_error_keyword_inside_user_throw_body` locks in the regression. * `split_trailing_error_picks_last_error_when_multiple_present` and `split_trailing_error_handles_ansi_prefixed_error_line` get an explicit blank line in their fixtures so they model what Nix actually emits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

With no leading trace and 4+ `error:` paragraphs, the early-return guard fired at the third paragraph and leaked later paragraphs into the headline. Match on `error_start` instead so "last error wins" actually holds for the no-leading-trace case while still cutting at the first boundary when leading trace is present. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

`split_trailing_error` used an empty `&str` to signal "no leading trace", forcing callers to `is_empty()`-check a value that otherwise reads like real text. Return `Option<&str>` so the absence of a trace lives in the type and `format_eval_error` matches on it directly. A trace that trims down to empty (input opens with `error:`, or only blank lines precede the first `error:`) now maps to `None` rather than `Some("")`, via a small `split_at` helper shared by both exit paths. No behaviour change for callers. Addresses review feedback on #2849. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

`validate_within_root` rejected valid input-relative imports on macOS, where `TMPDIR` resolves via the `/var -> /private/var` symlink and the import path's leaf does not exist yet (`Import path '<name>/...' resolves outside the git repository`). Resolve the longest existing ancestor with `canonicalize` and append the remaining components lexically, so symlinks anywhere up the chain are followed even when the leaf is absent. Split out of #2849. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

`Inputs::Add` is dispatched before `resolve()`, so the parent-directory discovery that `devenv shell` benefits from never ran for it — running `devenv inputs add` from a subdirectory silently created a stray `devenv.yaml` in the subdir that no other command would read. Apply the same chdir-up-to-`devenv.nix` step in the dispatch arm so the input lands in the enclosing project's config, and trace the discovery's failure modes so the best-effort behaviour is observable. Also compare against the physical path in the cli-subdir-discovery test so it passes on macOS, where `TMPDIR` resolves via the `/var -> /private/var` symlink. Split out of #2849. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

`Inputs::Add` is dispatched before `resolve()`, so the parent-directory discovery that `devenv shell` benefits from never ran for it — running `devenv inputs add` from a subdirectory silently created a stray `devenv.yaml` in the subdir that no other command would read. Apply the same chdir-up-to-`devenv.nix` step in the dispatch arm so the input lands in the enclosing project's config, and trace the discovery's failure modes so the best-effort behaviour is observable. Also compare against the physical path in the cli-subdir-discovery test so it passes on macOS, where `TMPDIR` resolves via the `/var -> /private/var` symlink. Split out of cachix#2849. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

sandydoo · 2026-06-11T13:02:29Z

I've figured out the cause for #2820.

The color of the warning in the logs changed, causing looks_like_warning to return false.
We then defaulted to treating the warning line as an error and selected it as the main error, ignoring the real error returned by eval (the getInput suggestion).
This particularly affects untrusted users because Nix will reliably print out a bunch of warnings in that cause.

We need to:

Always print out return eval errors (those returned by calling eval)
Avoid defaulting to error level if parsing fails

As for this PR, it currently prints the suggestion above the trace, requiring users to scroll up to find it. What we had before was better.

Review feedback on #2849: placing the actionable error paragraph above the --show-trace frames forced users to scroll up past ~120 lines to find it. Render the eval-returned error as-is (modulo dedenting) so the actionable message stays at the bottom where the cursor lands. The actual fix for #2820 is unchanged: always render the error returned by eval instead of a log line captured during evaluation, so a stale warning arriving at error verbosity can never shadow the real error. This drops split_trailing_error and the headline/help reshaping, which no longer have a purpose. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

domenkozar · 2026-06-11T17:13:20Z

Addressed in c0174f8:

Always render the eval-returned error: already the case since the first commit dropped select_raw_error/looks_like_warning/peek_pre_repl_errors, so no captured log line can replace the eval error anymore, and no color matching is left to rot.
No defaulting to error level on parse failure: the only remaining color-based check is is_nix_error() in the log bridge, and it fails toward Debug (demoting), not Error, so a format change there can't promote a warning into an error.
Ordering reverted: the error is rendered as-is in Nix's natural order again, trace frames first, the actionable error: … paragraph last at the bottom of the output. The headline/help: reshaping and split_trailing_error are gone. The only text processing left is dedenting the uniform left margin from the C bindings.

The integration tests now assert the suggestion appears below the trace frames and that the stale warning never shadows the real error.

domenkozar mentioned this pull request May 19, 2026

Unfree license error should suggest allow_unfree: true in devenv.yaml #2850

Open

domenkozar force-pushed the fix/2820-actionable-error branch from e063888 to d02361d Compare May 20, 2026 13:54

domenkozar force-pushed the fix/2820-actionable-error branch 2 times, most recently from 28d78b9 to 4115710 Compare May 25, 2026 13:30

sandydoo reviewed May 29, 2026

View reviewed changes

domenkozar and others added 4 commits May 29, 2026 18:24

domenkozar force-pushed the fix/2820-actionable-error branch from 158484f to d4770c2 Compare May 29, 2026 22:29

This was referenced May 29, 2026

cli: walk up to project root for devenv inputs add #2883

Merged

config: canonicalize existing ancestor for import-path validation on macOS #2884

Open

domenkozar changed the title ~~Surface actionable Nix error above show-trace frames~~ Always surface the Nix eval error, never a stale warning Jun 11, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Always surface the Nix eval error, never a stale warning#2849

Always surface the Nix eval error, never a stale warning#2849
domenkozar wants to merge 5 commits into
mainfrom
fix/2820-actionable-error

domenkozar commented May 19, 2026 •

edited

Loading

Uh oh!

cloudflare-workers-and-pages Bot commented May 19, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 20, 2026 •

edited

Loading

Uh oh!

sandydoo May 29, 2026

Uh oh!

domenkozar May 29, 2026

Uh oh!

domenkozar May 30, 2026

Uh oh!

sandydoo May 29, 2026

Uh oh!

domenkozar May 29, 2026

Uh oh!

Uh oh!

sandydoo left a comment

Uh oh!

sandydoo commented Jun 11, 2026

Uh oh!

domenkozar commented Jun 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		- Fixed import path validation rejecting valid input-relative imports on macOS, where `TMPDIR` resolves via the `/var → /private/var` symlink (`Import path '<name>/...' resolves outside the git repository`). `validate_within_root` now canonicalizes the longest existing ancestor of the path before comparing against the root.
		- Fixed `devenv inputs add` from a subdirectory writing to a stray `devenv.yaml` in the subdir instead of the enclosing project. It now walks up to find `devenv.nix` the same way `devenv shell` does, so the input is added where the rest of devenv reads it.

Conversation

domenkozar commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Tests

Test plan

Uh oh!

cloudflare-workers-and-pages Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying devenv with Cloudflare Pages

Uh oh!

github-actions Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔍 Suggested Reviewers

Uh oh!

sandydoo May 29, 2026

Choose a reason for hiding this comment

Uh oh!

domenkozar May 29, 2026

Choose a reason for hiding this comment

Uh oh!

domenkozar May 30, 2026

Choose a reason for hiding this comment

Uh oh!

sandydoo May 29, 2026

Choose a reason for hiding this comment

Uh oh!

domenkozar May 29, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sandydoo left a comment

Choose a reason for hiding this comment

Uh oh!

sandydoo commented Jun 11, 2026

Uh oh!

domenkozar commented Jun 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

domenkozar commented May 19, 2026 •

edited

Loading

cloudflare-workers-and-pages Bot commented May 19, 2026 •

edited

Loading

github-actions Bot commented May 20, 2026 •

edited

Loading