Skip to content

[Bug]: Parent-cancellation test can accept a late completion before closure #1660

Description

@ben-ranford

Summary

The Linux race smoke job failed TestStaveTerminalParentCancellationPublishesIndeterminateOutcome because a late completion event returned nil instead of session.ErrSessionClosed. This differs from #1638's refresh-start timeout and #1637's resize-frame assertion.

Observed run: https://github.com/ben-ranford/lopper/actions/runs/34948806644/job/104314716672. PR1650 modifies suppression scanners and tests; it does not change Stave or this test.

Reproduction steps

  1. Run the Linux race suite with the dev build-channel linker setting used by the CI smoke job.
  2. In TestStaveTerminalParentCancellationPublishesIndeterminateOutcome, cancel the parent with a cause and finish the terminal run.
  3. Verify the cancelled effect and shutdown events, then inspect the model's indeterminate-outcome state.
  4. Send a late completed effect for the cancelled call and require ErrSessionClosed.

CI provides the observed failure. Three bounded targeted macOS race repetitions on the same PR head passed; no deterministic local schedule has been established.

Expected behavior

The shutdown contract defines when the session is closed. A late completion must not overwrite the cancelled/indeterminate outcome. The test must synchronize with that defined lifecycle state before making a closure assertion.

Actual behavior

The late send returned nil in CI after the preceding shutdown/state assertions passed.

Logs and output

--- FAIL: TestStaveTerminalParentCancellationPublishesIndeterminateOutcome (0.04s)
    stave_terminal_parent_cancel_test.go:86: late completion error = <nil>, want closed session
FAIL github.com/ben-ranford/lopper/internal/ui

Local targeted evidence: go test -race -ldflags '-X github.com/ben-ranford/lopper/internal/version.buildChannel=dev' ./internal/ui -run '^TestStaveTerminalParentCancellationPublishesIndeterminateOutcome$' -count=3 -timeout=90s -v passed all3 in1.678s.

Diagnosis and acceptance criteria

The cause is not proved. sendLopperEvent sends an event then waits for a sequence advance; test ordering, shutdown acknowledgement and eventual session closure may differ. Do not assume that every sequence change proves closure, and do not weaken the late-completion/outcome contract to make the test pass.

  • Trace or control shutdown publication, reducer acknowledgement, session closure and late completion delivery to establish the failing ordering.
  • Determine whether the defect is test synchronization, the bridge wait contract or production shutdown ordering; fix the demonstrated cause.
  • Ensure late results cannot overwrite the indeterminate cancellation outcome.
  • Preserve cancellation cause, backend cancellation, effect/shutdown ordering and error assertions.
  • Add deterministic lifecycle synchronization and meaningful diagnostics; no arbitrary sleep, skip or discarded closure assertion.
  • Repeat targeted normal/race tests on Linux and macOS, then run the repository CI gate.

Scope and scheduling

Targetv1.8.9, a focused cancellation lifecycle/test stability fix owned by the Stave work. No Stave implementation change is included in PR1650.

Version / commit

ed664c2.

Environment

GitHub Actions ubuntu-latest race smoke suite, Go1.27.1. Local macOS targeted checks use the same dev-channel linker setting. Other environments are not yet verified.

Activity

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

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingtarget-series:1.8.xRelease series targeting label

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions