fix(runs): mark job/step runs cancelled when a run is cancelled - #145
Merged
Conversation
Cancelling a run only ever updated workflow_runs.status; the nested job_runs/step_runs rows the run detail page actually renders stayed stuck at running/pending forever. Also guard set_run_status/ set_job_status/set_step_status against overwriting an already-terminal row, so a background job process finishing after the fact can't silently flip a cancelled run back to succeeded/failed.
A step failing with PowerShell's CLIXML "Cannot find drive" error came back with failure_hint: null, since diagnose() only recognized missing-command patterns. Give it a name and point at the runner log instead of leaving cargo build step failed as the only visible signal.
A failed run showed the job's first step starting nearly 9 minutes after the job itself started, with nothing in between to say why. Log elapsed time for checkout and shard/sandbox setup so the next occurrence shows where the time actually went instead of leaving another unexplained gap to guess at.
Adds a lifecycle_events table plus RunClient/RCP plumbing (start_phase/finish_phase) so a shell can record when each phase of the event pipeline starts and ends, even though it has no direct database access of its own. finished_at stays null until a phase actually ends, so a hung phase is directly queryable instead of only inferable after the fact from a job's total duration.
…/step Wraps bucket_create, rcp_listener_bind (dispatch.rs), the shell capability probe and RCP handshake (shell_run.rs), and checkout, shard_create, step_exec (executor.rs) with start_phase/finish_phase calls. Each phase's row is written before the work starts, so a hang in any of them shows up as finished_at IS NULL immediately, rather than only being inferable from a job's total duration after the fact.
Adds atk_bucket::PhaseRecorder, a sync callback create_job_shard/ exec_step invoke around AppContainer profile creation, each ACL grant (including one per ancestor directory, since a single missing grant anywhere in that chain is what actually breaks PowerShell's working directory resolution), the extra-mount grants, and the sandboxed process's own spawn/wait. Existing call sites are unaffected: the plain create_job_shard/exec_step now delegate to *_with_recorder variants with a no-op recorder, so no existing test needed to change behavior, only its call site's arity. RunClientPhaseRecorder in core bridges these sync callbacks into RunClient's async start_phase/finish_phase, fire-and-forget the same way exec_step's on_line callback already reaches a RunClient from a blocking-pool thread. Also switches start_phase to a caller-supplied id (matching shards::create's convention) instead of a server-generated one returned from the call: a sync callback deep inside blocking OS code has no way to await a response before handing a token back to its own caller, so the id has to already be known before the write is fired. Linux is left uninstrumented for now: this crate's Linux backend can't be exercised or verified from a Windows dev machine, and the bug this was built to diagnose is Windows-specific anyway.
RunTopology (GET /runs/:id/topology) now includes every recorded phase for the run, oldest first. The UI's exportRunReport already nests this response under backend.topology, so the downloadable failed-run report gains a phases[] timeline for free: bucket_create, rcp_listener_bind, checkout, shard_create (and its AppContainer/ACL sub-phases), step_exec (and its process spawn/wait sub-phases), each with started_at/finished_at so a gap between two phases, or a phase with no finished_at at all, is readable directly instead of inferred from job/step durations after the fact.
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
workflow_runs.status; the nestedjob_runs/step_runsrows the run detail page renders (job/step tree, GitHub-style) stayed stuck at running/pending forever instead of showing cancelled/skipped/failed.set_run_status/set_job_status/set_step_statusagainst overwriting an already-terminal row, so a background job process finishing after the fact can't silently flip a cancelled run back to succeeded/failed.Test plan
cargo test -p atk-db --lib queries::runs::passescargo check -p atk-db -p actions-toolkit-backendpasses