Skip to content

review-gate: selftest-teardown suite signals process groups, so concurrent instances kill each other's probes (0/4 concurrent vs 4/4 for every other skill) #1506

Description

@bmethod

Summary

review-gate/tests/review-predicate-selftest-teardown.test.sh signals PROCESS GROUPS, so two
concurrent instances of the suite signal into each other and fail spuriously. It is not
temp-directory interference — the suite is properly isolated with its own mktemp -d.

Evidence (measured)

Running the vendored suites through a per-skill runner, 4 concurrent invocations of each skill,
each invocation with its own private TMPDIR:

skill concurrent result
size-ratchet 4/4 pass
growth-guards 4/4 pass
preflight 4/4 pass
review-gate 0/4 pass — all four fail on review-predicate-selftest-teardown.test.sh

Failure text:

FAIL: sigquit.test.sh: expected exit 130, got 99
FAIL: an interrupted run exited 0 rather than 130 — the interrupt never reached cleanup

Sequentially, and on an idle machine, the suite passes reliably (4/4 standalone runs, and 3/3
through the runner). It also passes under heavy CPU load alone (3/3 with 32 busy loops), so this
is interference between INSTANCES, not slowness.

Cause

The suite signals negative PIDs — process groups — to prove the wrapper's teardown owns its whole
replay tree:

kill "-$sig" "-$p" 2>/dev/null || kill "-$sig" "$p" 2>/dev/null || true

When two instances are launched from the same shell/session their replay processes can share a
process group, so one instance's kill -SIGQUIT -PGID reaches the other's probes. The exit 99
in the diagnostic is the fixture's own "signal never arrived" arm.

Impact

  • CI is unaffected where the suites run sequentially in one job.
  • It bites any consumer running the suites in parallel — which is the normal pattern in a repo
    worked from several git worktrees at once.

Suggested fix

Have each instance put its replay tree in its OWN process group (setsid, or set -m plus an
explicit group per replay leader) and signal only that group, so the blast radius is the
instance's own tree rather than whatever shares its group.

Metadata

Metadata

Assignees

No one assigned

    Labels

    ci-infraCI, review gates, runners, and repo tooling

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions