Skip to content

pack:verify: await the --web child's exit before removing its work dir #1826

Closed

Description

@cliffhall

Follow-up to #1801 (fixed in #1814). Same shape, lower risk — filed so it isn't rediscovered from scratch if it ever does bite.

Context

#1801 was smoke:tui failing intermittently with ENOTEMPTY: done() sent SIGTERM and then synchronously rmSync'd the work dir, without waiting for the child to exit. scripts/pack-and-verify.mjs has the same kill-then-remove pattern in two places.

The two sites

scripts/pack-and-verify.mjs:330 (success path) — the closer analogue. verifyWeb()'s finally { stop() } sends SIGTERM (line 377-380), control returns to the main flow, and rmSync(work, …) runs immediately with nothing awaiting the child's exit.

scripts/pack-and-verify.mjs:74-78 (fail()) — kills webChild and rmSyncs workDir back to back.

Why this is low priority, not a live flake

  • Unlike smoke-tui, the --web child's HOME is not redirected into the work dir — work is only its cwd and the location of the installed package it reads. In smoke:tui intermittently fails with ENOTEMPTY cleaning its temp dir #1801 the work dir was HOME, so the Ink app was actively writing config/storage into the directory being deleted, which is what made the race fire. Nothing here writes into work on shutdown, and on macOS a live cwd doesn't block removal.
  • fail() runs only when the verify has already failed and is exiting 1, so an ENOTEMPTY there cannot manufacture a spurious red — worst case it masks the real failure message with an rmSync stack and skips the "tarball retained at …" hint.
  • pack:verify is not in the local pre-push gate. It runs in CI on the publish path only (.github/workflows/main.yml:155), so a flake would be rare and loud rather than the learned-to-re-run red smoke:tui intermittently fails with ENOTEMPTY cleaning its temp dir #1801 became.

Suggested fix

Apply the #1814 treatment to stop(): make it await the child's exit event (escalating SIGTERM → SIGKILL after a grace period, then proceeding anyway with a warning) before the caller removes work, and wrap the rmSync calls so a leftover temp dir warns instead of throwing over the real failure message. See scripts/smoke-tui.mjs after #1814 for the pattern.

Activity

  1. self-assigned this
    on Jul 27, 2026
  2. added this to the v2.1.0 milestone on Jul 28, 2026
  3. removed their assignment
    on Jul 29, 2026
  4. cliffhall commented on Aug 1, 2026

    @cliffhall
    MemberAuthor

    Triage: Priority = Medium

    Scored 6/16 with the priority rubric added to AGENTS.md in #1891. This card is already approved and sitting in Todo — only Priority was set here, its Status is unchanged.

    Axis Score Reasoning
    Severity / impact 2/5 A kill-then-remove race in pack:verify that can produce an intermittent ENOTEMPTY. It affects a local/release-only script, not shipped behaviour, and the issue itself was filed defensively rather than in response to a failure.
    Urgency / staleness 3/5 v2.1.0 milestone, and pack:verify is the pre-publish gate for the release in #1876 — so if it does bite, it bites at the worst moment.
    Signal bonuses +1 milestone (v2.1.0) (+1)
    Total 6 Lands in the Medium band (6-8).

    Related issues. One connected issue:

    Scores are a starting point, not a verdict — the rubric is meant to be overruled when it's plainly wrong, provided the reason is written down.

  5. self-assigned this
    on Aug 1, 2026
  6. cliffhall commented on Aug 2, 2026

    @cliffhall
    MemberAuthor

    Closed by #1895, squash-merged into v2/main. (Auto-close doesn't fire here — Closes #N only closes on PRs targeting the default branch, and v2 PRs target v2/main.)

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

Metadata

Metadata

Assignees

Labels

v2Issues and PRs for v2

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions