fix(compass-app): stop runStackUp/Down hanging on a fire-and-return stack (SEA-1685) - #227
Open
seal-agent wants to merge 2 commits into
Open
fix(compass-app): stop runStackUp/Down hanging on a fire-and-return stack (SEA-1685)#227seal-agent wants to merge 2 commits into
seal-agent wants to merge 2 commits into
Conversation
…tack (SEA-1685) runStackUp/runStackDown captured the compass-stack child's stderr into a bytes.Buffer. os/exec backs a non-*os.File stderr writer with an OS pipe whose copy goroutine Cmd.Wait blocks on until EOF. `compass-stack up` is fire-and-return: it exits 0 once the stack is Ready while its postgres/server/runner children keep running, and those children inherit the pipe's write-end — so EOF never arrives and Wait hangs for the children's whole lifetime. Embedded launch therefore never completed against a real stack (the T4.1/T4.2 unit tests passed only because their /bin/sh children leave no survivors). Capture stderr to a temp *os.File instead (extracted into captureStderr, shared by both seams): an *os.File is dup'd straight into the child (no pipe, no goroutine), so Wait returns the instant compass-stack itself exits regardless of lingering children, and the children write to a plain file that never EPIPEs, so the capture never signals the stack the app must keep alive. The failure-copy contract (surface the child's stderr on a non-zero exit) is preserved. Regression: TestRunStackUpReturnsWhileChildrenLinger drives /bin/sh that backgrounds a long sleep holding stderr then exits 0 — the fire-and-return shape. Pre-fix it hangs for the sleep's lifetime; the fix returns in milliseconds. Surfaced by the SEA-1685 T4.3 e2e integration test driving the real embedded composition root against a real compass-stack (up -> real WhoAmI resolved a live account id, DL-111). The full T4.3 e2e + its teardown assertions are gated on SEA-1880 (compass-stack down is a no-op across a process boundary) and land once Matt rules the teardown mechanism. Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
… read note, shorter test sleep (SEA-1685) Three low-severity review polish items (no high/medium findings; reviewer APPROVE): - captureStderr: name the returns (read/cleanup/err) so the signature self-documents; note that read's dropped ReadFile error is intentional best-effort degradation to the generic failure message. - TestRunStackUpReturnsWhileChildrenLinger: shrink the lingering grandchild from `sleep 60` to `sleep 5` — still far past the 1s regression bound, but a regressed run's leaked child self-reaps in seconds not a minute (shared-box hygiene under repeat/-count loops). No behavior change; union gate green (gofmt/vet/golangci 0-issues/-race) both tag sets. Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
runStackUp/runStackDown captured the compass-stack child's stderr into a
bytes.Buffer. os/exec backs a non-*os.File stderr writer with an OS pipe whose
copy goroutine Cmd.Wait blocks on until EOF.
compass-stack upisfire-and-return: it exits 0 once the stack is Ready while its
postgres/server/runner children keep running, and those children inherit the
pipe's write-end — so EOF never arrives and Wait hangs for the children's whole
lifetime. Embedded launch therefore never completed against a real stack (the
T4.1/T4.2 unit tests passed only because their /bin/sh children leave no
survivors).
Capture stderr to a temp *os.File instead (extracted into captureStderr, shared
by both seams): an *os.File is dup'd straight into the child (no pipe, no
goroutine), so Wait returns the instant compass-stack itself exits regardless of
lingering children, and the children write to a plain file that never EPIPEs, so
the capture never signals the stack the app must keep alive. The failure-copy
contract (surface the child's stderr on a non-zero exit) is preserved.
Regression: TestRunStackUpReturnsWhileChildrenLinger drives /bin/sh that
backgrounds a long sleep holding stderr then exits 0 — the fire-and-return
shape. Pre-fix it hangs for the sleep's lifetime; the fix returns in
milliseconds.
Surfaced by the SEA-1685 T4.3 e2e integration test driving the real embedded
composition root against a real compass-stack (up -> real WhoAmI resolved a live
account id, DL-111). The full T4.3 e2e + its teardown assertions are gated on
SEA-1880 (compass-stack down is a no-op across a process boundary) and land once
Matt rules the teardown mechanism.
Co-authored-by: Matt Wilkinson matt@sealedsecurity.com