Skip to content

ci(nightly): re-enable nix-flake job & fix sandbox failures - #2648

Merged
max-sixty merged 4 commits into
mainfrom
nix
May 8, 2026
Merged

max-sixty merged 4 commits into
mainfrom
nix

Conversation

@max-sixty

@max-sixty max-sixty commented May 8, 2026 •

Copy link
Copy Markdown
Owner

Summary

Restore the nix-flake nightly job (disabled in #2647) and fix the
sandbox-specific test failures it surfaces. With this merged, the
nightly cron run is back to gating against packaging-environment bugs.

What's in here

  • Build-mode snapshot redaction: 49 snapshots hardcoded
    target/debug/wt in "Invoked as:" and diagnostic output, which broke
    under crane's release builds. Collapse both modes to
    target/[BUILD_MODE]/wt in the shared insta filter.
  • Nix sandbox env: add pkgs.python3 and pkgs.procps to
    worktrunk-tests' native build inputs (needed by argv-quoting,
    post-start, and pgid-invariant tests). Replace
    #!/usr/bin/env python3 with the absolute python path resolved at
    script-write time, since /usr/bin/env doesn't exist on NixOS.
  • PTY filter ordering: extend add_pty_binary_path_filters'
    alternation to match the [BUILD_MODE] placeholder so PTY snapshots
    still collapse to [BIN] after the prelude rewrite runs first
    (caught by worktrunk-bot review).
  • One inherited-CWD test fix: test_config_init_already_exists
    branched on whether the inherited CWD had a project config — pin
    it to a no-config tempdir so the snapshot is deterministic across
    cargo and nix.

Test plan

  • nix-flake job passes in CI (1633 / 1633, was 54 failing)
  • All required ci checks pass
  • Auto-reviewer approved

🤖 Generated with Claude Code

@max-sixty max-sixty added the nightly Trigger the nightly workflow on this PR label May 8, 2026
max-sixty and others added 2 commits May 8, 2026 12:30
Snapshots hardcoded `target/debug/wt` in "Invoked as:" and diagnostic
output, which broke under crane's release builds in the nightly
nix-flake sandbox. Collapse both build modes to `target/[BUILD_MODE]/wt`
in the shared insta filter chain so a single set of snapshots covers
both cargo and crane test runs.

Adjacent: pin `test_config_init_already_exists` to a no-project-config
CWD. The hint text branches on whether a project config exists, and
bare `wt_command()` was inheriting the worktrunk repo's CWD locally
(project config present → "view both") while the nix sandbox saw none
("create a project config"). Setting `current_dir(temp_home)` makes
the test deterministic across both environments.

Co-Authored-By: Claude <noreply@anthropic.com>
Three integration tests fail in the nix-flake sandbox for sandbox-only
reasons: `for_each_preserves_argv_quoting` and
`post_create_script_reads_json` need `python3` on PATH; the latter also
relies on `#!/usr/bin/env python3`, which fails on NixOS where
`/usr/bin/env` doesn't exist (the kernel runs the literal interpreter
path, so PATH lookup can't apply); `alias_child_shares_parent_pgroup`
shells out to `ps -o pgid= -p $$` and writes empty content when `ps`
isn't found, surfacing as `cannot parse integer from empty string`.

Add `pkgs.python3` and `pkgs.procps` to `worktrunk-tests`' native build
inputs, and resolve `python3` at script-write time so the post-create
fixture's shebang is the real interpreter path.

Co-Authored-By: Claude <noreply@anthropic.com>

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new build-mode filter conflicts with add_pty_binary_path_filters (line 1211). setup_snapshot_settings runs first and rewrites /target/debug/wt to /target/[BUILD_MODE]/wt; afterwards the PTY [BIN] filter ((?:debug|release)/wt) no longer matches, so PTY snapshots that expect [BIN] now see [PROJECT_ROOT]/target/[BUILD_MODE]/wt.

The affected tests (linux, advisory) run is failing on test_approval_prompt_accept, test_approval_prompt_decline, and test_approval_prompt_mixed_approved_unapproved_accept for exactly this reason — the snapshots in tests/snapshots/integration__integration_tests__approval_pty__*.snap and …__bare_repository__* still use [BIN]. test (linux) will likely fail too once it gets there.

Suggested fix: extend the alternation in add_pty_binary_path_filters to also match the placeholder, so it still collapses to [BIN] regardless of which filter ran first:

// tests/common/mod.rs:1211
settings.add_filter(
    r"[^\s]+/target/(?:[^/\s]+/)*(?:debug|release|\[BUILD_MODE\])/wt",
    "[BIN]",
);

Happy to push that as a fixup if you'd like.

Comment thread tests/common/mod.rs
The prelude filter rewrites `target/(debug|release)/wt` to
`target/[BUILD_MODE]/wt` before per-test settings run. PTY tests then
expect the more specific `[BIN]` placeholder, but its regex still
anchored on the literal build-mode segment and silently no-op'd when
the prelude had already collapsed it. Snapshots in
`integration__integration_tests__approval_pty__*` and
`integration__integration_tests__bare_repository__*` saw
`[PROJECT_ROOT]/target/[BUILD_MODE]/wt` instead of `[BIN]`.

Extend the alternation to match the placeholder too, so the filter
collapses to `[BIN]` regardless of which rewrite landed first.

Spotted by worktrunk-bot review on #2648.

Co-Authored-By: Claude <noreply@anthropic.com>
@max-sixty
max-sixty merged commit 1fe346b into main May 8, 2026
36 checks passed
@max-sixty
max-sixty deleted the nix branch May 8, 2026 23:22
max-sixty added a commit that referenced this pull request May 9, 2026
## Summary

Bare `wt_command()` (free function in `src/testing/mod.rs`) previously
didn't set `current_dir`, so spawned `wt` test subprocesses inherited
the test process's CWD — which is the worktrunk repo root, with its own
`.config/wt.toml` and git history. PR #2648 patched one such case
(`test_config_init_already_exists`) per-test after the nix sandbox
surfaced it.

This change sets a sane default at the constructor: a process-scoped
empty `LazyLock<TempDir>` (outside any git repo, no project config). The
inherited CWD is never the implicit answer.

## Why structural over per-test

I audited the 330 bare `wt_command()` callsites in
`tests/integration_tests/`:

- **297 already set `current_dir(...)` later** in their chain —
`Command::current_dir` overwrites, so the new default is a no-op for
them.
- **33 SUSPECT** sites don't set `current_dir`. None deliberately rely
on inherited CWD — they're all CWD-independent in practice today (`wt
--help`, `wt --version`, `wt config plugins opencode install`, `wt
config shell init <shell>`, clap routing of unknown subcommands, etc.)
but precariously so. A future change to any of those code paths that
starts reading project config would silently couple them to the
worktrunk repo's `.config/wt.toml` again.

Setting the default once removes the latent surprise; patching 33 sites
would not.

## What's in here

- `src/testing/mod.rs`: add `isolated_test_cwd()` helper backed by
`LazyLock<TempDir>`; call `cmd.current_dir(...)` from `wt_command()`.
- `tests/CLAUDE.md`: document the new default contract.
- `tests/integration_tests/config_init.rs`: remove the now-redundant
per-test override added in #2648.

## Test plan

- [x] `cargo check --tests` clean
- [x] All 64 SUSPECT-class tests pass with the new default (help,
completion, custom subcommand routing, opencode plugin
install/uninstall, config_init, config_show_theme)
- [x] Full integration suite has the same pre-existing flakes as `main`
(test_quickstart_merge, test_docs_merge_pre_merge_hook,
test_readme_example_complex, ci_status::* — verified by stashing and
running on main)
- [x] All 1737 unit tests pass
- [x] `pre-commit run --all-files` clean

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude <noreply@anthropic.com>
This was referenced May 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

nightly Trigger the nightly workflow on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants