Skip to content

mcpx tests: port the spawn-marker assertion into TestEnsureDaemonCancelledWhileWaiting (nix copy has it, this copy doesn't) #361

Closed

Description

@dezren39

Problem

The mcpx dedup audit (2026-10-19, comparing this repo's pkgs/mcpx against the vendored copy in dezren39/nix at nix e5c6407 / code e04b519) found the two trees' production code behaviorally identical — the only real divergence is one test assertion the nix copy has and this copy lacks.

In dezren39/nix pkgs/mcpx/internal/cli/ensuredaemon_test.go, TestEnsureDaemonCancelWhilePolling (lines 70–103) asserts, after the prompt context.Canceled return, that the daemon spawn actually happened: it waits up to 5s for the fake daemon's marker file and fails with "expected the daemon spawn to have happened before cancellation" if it never appears.

This repo's counterpart, TestEnsureDaemonCancelledWhileWaiting (pkgs/mcpx/internal/cli/ensuredaemon_test.go:62), discards the marker (c, _ := ensureTestClient(t, "30s")). It would pass even if EnsureDaemon returned context.Canceled without ever spawning — e.g. if a future change moved the cancellation check ahead of the spawn in the polling path, silently turning "cancel while waiting" into "never start".

Why the wait-for-marker shape

cmd.Start returns once the process exists, before the child has run any code, and the spawned child is Setsid + Released, so its marker write can legitimately land after EnsureDaemon returns. An instant marker assertion races child scheduling (this flake got the nix copy's test skipped in the darwin build, dezren39/darwin-nix#428, and is why TestEnsureDaemonNeverReadyStillTimesOut in this repo already uses the wait-for-marker pattern, citing dezren39/darwin-nix#445). The port must use the same bounded-wait shape.

Acceptance criteria

  • TestEnsureDaemonCancelledWhileWaiting keeps the returned marker and, after the existing error/promptness assertions, waits up to 5s for the marker to appear, failing if the daemon was never spawned.
  • The ported test fails if EnsureDaemon is made to return context.Canceled before spawning (ablation-checked), and passes unmodified code.
  • go test ./internal/cli/ passes.
  • No production-code changes: the audit verified client.go differs between the copies only in comments.

Context: this is the last migrate item before dezren39/nix deletes its vendored copy and pins this repo as the single mcpx source (its extraction plan, planning/plans/mcpx-extraction.md there; consumption decision tracked in #308).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions