fix(runners): preserve stopped status when Run returns error

[cursor]

Bug and impact

User-stopped runner tasks could be reported as failed instead of stopped.

When a user stops a running task, the server moves it to stopping/stopped and the runner's checkNewJobs loop applies that status locally and kills the executor process. Executor.Run() then returns a non-nil error (e.g. killed subprocess). Commit 2a156b30 (fix(runners): logs) moved the IsFinished() early-return into the err == nil branch only, so the error path overwrote an already-finished stopped status with failed.

This is user-facing breakage: stopped tasks appear failed in history, trigger failure notifications/workflows, and pollute metrics.

Root cause

In services/runners/job_pool.go, post-Run() handling only checked getStatus().IsFinished() on the success path. The error path unconditionally set TaskFailStatus unless the status was still TaskStoppingStatus — missing the case where the status had already advanced to TaskStoppedStatus.

Fix and validation

• Extract handleJobRunComplete and restore the IsFinished() guard before branching on err, so a terminal status set by a concurrent stop/kill is never overwritten.
• Keep the logging improvement from 2a156b30 (error log only on genuine launch failures, debug log on normal completion).
• Added regression tests covering: stopped+error preserved, stopping+error → stopped, running+error → failed, running+success → success.

go test ./services/runners/... -run TestHandleJobRunComplete -v

  

[cursor]

Security review

Outcome: No medium, high, or critical vulnerabilities found.

This PR refactors runner job completion into handleJobRunComplete and adds an early return when the job status is already terminal (stopped, success, or error). The change fixes a correctness bug where a legitimately stopped task could be overwritten as error when Executor.Run returned a non-nil error (e.g. process killed after user stop).

Reviewed attack paths (none exploitable):

• Status spoofing / alert bypass: Terminal status is only preserved when the runner-side status is already finished before handleJobRunComplete runs. That state is set by authorized server stop/kill flows (checkNewJobs, applyTerminatedJobs) or executor kill handling—not by unauthenticated input in this diff. A compromised runner could already report arbitrary statuses via sendProgress; this change does not widen that trust boundary.
• Authz bypass: Server-side progress handling (api/runners/runners.go) still validates runner token/assignment, rejects invalid statuses, and ignores progress for tasks already terminal on the server.
• Injection / secret leakage: No new attacker-controlled sinks. Logging uses existing task_id fields and internal Run() errors; user-facing logs remain a static message.

No new inline findings.

  

_{Sent by Cursor Automation: Find vulnerabilities}