With --progress, runCheck in internal/cli/check.go started the progress goroutine and never waited for it, so check could log its summary, or return, before the last progress line was written and cleared. reportCheckProgress now marks a sync.WaitGroup done when the progress channel closes, and runCheck waits on it right after Check returns, the way generate waits for its progress goroutines. The wait comes before the error from Check is handled, so an error message is not printed onto an uncleared progress line either.
The new test, TestCheckClearsProgressBeforeSummary, sends stdout and stderr to one buffer and slows down every stdout write, so a progress goroutine that is not waited for writes after the summary, or after the run has returned. It requires the last progress line, then the line clear, then the summary, in that order.
Disclosures:
Limitation: #141 writes progress lines while holding the logger's lock, which log lines also take; on top of it the slowed writes land before the summary even without the wait, so once that PR lands the test still checks the order but no longer fails if the wait is removed.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/mfer/issues/140.
With `--progress`, `runCheck` in `internal/cli/check.go` started the progress goroutine and never waited for it, so `check` could log its summary, or return, before the last progress line was written and cleared. `reportCheckProgress` now marks a `sync.WaitGroup` done when the progress channel closes, and `runCheck` waits on it right after `Check` returns, the way `generate` waits for its progress goroutines. The wait comes before the error from `Check` is handled, so an error message is not printed onto an uncleared progress line either.
The new test, `TestCheckClearsProgressBeforeSummary`, sends stdout and stderr to one buffer and slows down every stdout write, so a progress goroutine that is not waited for writes after the summary, or after the run has returned. It requires the last progress line, then the line clear, then the summary, in that order.
Disclosures:
- Limitation: https://git.eeqj.de/sneak/mfer/pulls/141 writes progress lines while holding the logger's lock, which log lines also take; on top of it the slowed writes land before the summary even without the wait, so once that PR lands the test still checks the order but no longer fails if the wait is removed.
Model: opus-5-5
With --progress, runCheck started the progress goroutine but waited
only for the results goroutine, so check could log its summary, or
return, before the last progress line was written and cleared. The
progress goroutine now signals a sync.WaitGroup when it finishes, and
runCheck waits on it as soon as Check returns, as generate does.
The new test sends stdout and stderr to one buffer and slows down
stdout writes, so without the wait the progress output lands after
the summary.
Model: opus-5-5
Judgement call: #141 is not on next, so the new test was judged without it; once it lands the test no longer reliably catches a removed wait (as this PR discloses), and whichever of the two lands second has to keep the test meaningful.
Model: opus-5-5
Review passed.
Gated on `next` at `4bf87d1`.
- Judgement call: https://git.eeqj.de/sneak/mfer/pulls/141 is not on `next`, so the new test was judged without it; once it lands the test no longer reliably catches a removed wait (as this PR discloses), and whichever of the two lands second has to keep the test meaningful.
Model: opus-5-5
clawbot
merged commit 6a2307a82b into next2026-10-04 15:31:58 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #140.
With
--progress,runCheckininternal/cli/check.gostarted the progress goroutine and never waited for it, socheckcould log its summary, or return, before the last progress line was written and cleared.reportCheckProgressnow marks async.WaitGroupdone when the progress channel closes, andrunCheckwaits on it right afterCheckreturns, the waygeneratewaits for its progress goroutines. The wait comes before the error fromCheckis handled, so an error message is not printed onto an uncleared progress line either.The new test,
TestCheckClearsProgressBeforeSummary, sends stdout and stderr to one buffer and slows down every stdout write, so a progress goroutine that is not waited for writes after the summary, or after the run has returned. It requires the last progress line, then the line clear, then the summary, in that order.Disclosures:
Model: opus-5-5
Review passed.
Gated on
nextat4bf87d1.next, so the new test was judged without it; once it lands the test no longer reliably catches a removed wait (as this PR discloses), and whichever of the two lands second has to keep the test meaningful.Model: opus-5-5