fix: restore run TUI elapsed timer across retry, failure, cancel, and remote paths#1
Open
MatheusBBarni wants to merge 4 commits into
Open
fix: restore run TUI elapsed timer across retry, failure, cancel, and remote paths#1MatheusBBarni wants to merge 4 commits into
MatheusBBarni wants to merge 4 commits into
Conversation
Thread the executor's authoritative JobCompletedPayload.DurationMs through jobFinishedMsg into handleJobFinished, which previously derived elapsed time from a startedAt that the retry flow never seeds (retry attempts emit no JobStarted; a remote tab can bootstrap a job mid-retry). The sidebar timer no longer stays blank after a retried job completes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e jobs Complete the retry-timer fix across every terminal outcome. Add DurationMs to JobFailedPayload/JobCancelledPayload and populate it in the executor (markGiveUp/markCanceled, zero-guarded when no attempt started), thread it through the failure and cancel UI translators, and add DurationMs to the RunJobSummary contract populated by the daemon snapshot builder (authoritative payload value with a JobStarted->terminal timestamp fallback for historical journals). The remote bootstrap threads it into every terminal arm, so a tab attaching to a job that finished before attach shows the real elapsed time instead of a bogus ~00:00. No persistence/schema migration required. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Addresses PR review: rename the TestJobLifecycleTerminalEventsCarryDuration and TestRunSnapshotJobResolveDurationMs subtests to the repo's Should-prefixed convention and mark the independent lifecycle subtests t.Parallel(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses the remaining CodeRabbit review comment: the two TestTranslateEvent*ThreadsDuration cases now run inside a Should-prefixed t.Run subtest, matching the ui package's dominant convention. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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.
Summary
Fixes the run TUI bug where the elapsed timer disappears after a retried job succeeds — and completes the fix so the timer is correct for every terminal outcome (success, failure, cancel) and for remote tabs that attach after a job already finished.
Root cause: the UI derived a job's elapsed time from a locally-tracked
startedAtthat the retry flow never seeds (retry attempts emit no freshJobStarted, and a remote tab can bootstrap a job mid-retry), while discarding the authoritative duration the executor already computes.What changed
Core fix (
e6d1214) — thread the executor's authoritativeJobCompletedPayload.DurationMsinto the UI:jobFinishedMsggainsDurationMs;translateJobCompletedEventcarries it;handleJobFinishedprefers it and backfillsstartedAtfor triple-coherence.Completion across all terminal outcomes (
4cbce91) — no persistence/schema migration required:DurationMsadded toJobFailedPayload/JobCancelledPayload, populated inexecutor/lifecycle.govia a zero-guardedelapsedMs()(guard matters:markGiveUp/markCanceledcan fire before any attempt starts), threaded through the failure and cancel UI translators.DurationMsadded to theRunJobSummarycontract; the daemon snapshot builder populates it from the authoritative payload with aJobStarted → terminaltimestamp fallback for historical journals;remote.gothreads it into all three terminal bootstrap arms.Why this altitude
Duration was only carried on the live
JobCompletedevent. Generalizing it to all terminal payloads + the remote summary contract fixes the whole class (blank timer on retry→failure/cancel; ~00:00on remote pre-attach) instead of special-casing the success arm.Testing
gofmt -s: clean ·go build ./...: OK ·golangci-lint(5 packages): 0 issuesgo test -race:pkg/compozy/events/kinds,internal/core/run/ui,internal/core/run/executor,internal/daemon,internal/api/contract— all passresolveDurationMs(payload-wins / timestamp-fallback / no-start), plus a translator guard and reproduction test in the core fix.🤖 Generated with Claude Code