Exit ephemeral runners on broker acknowledge job-not-found#4540
Open
rentziass wants to merge 5 commits into
Open
Exit ephemeral runners on broker acknowledge job-not-found#4540rentziass wants to merge 5 commits into
rentziass wants to merge 5 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds explicit classification for broker acknowledge “404 / Job not found” responses and uses that classification to let single-use runners (ephemeral / --once) exit cleanly instead of continuing to poll in a terminal condition.
Changes:
- Add a new
RunnerRequestJobNotFoundExceptionand throw it fromBrokerHttpClient.AcknowledgeRunnerRequestAsyncwhen the broker returns a “Job not found” 404. - Update the listener runner loop to exit early for single-use runners when acknowledge hits the job-not-found terminal condition, while preserving best-effort behavior for persistent runners.
- Add L0 coverage for the new broker classification and the single-use vs persistent runner behaviors.
Show a summary per file
| File | Description |
|---|---|
| src/Sdk/WebApi/WebApi/BrokerHttpClient.cs | Adds job-not-found detection and throws a dedicated exception on acknowledge. |
| src/Sdk/WebApi/WebApi/Exceptions/RunnerRequestJobNotFoundException.cs | Introduces a new exception type representing the acknowledge terminal condition. |
| src/Runner.Listener/Runner.cs | Exits single-use runners on job-not-found acknowledge and ensures ephemeral cleanup triggers. |
| src/Test/L0/Sdk/RSWebApi/BrokerHttpClientL0.cs | Adds unit tests validating acknowledge error classification behavior. |
| src/Test/L0/Listener/RunnerL0.cs | Adds runner loop tests for single-use exit vs persistent continuation on acknowledge job-not-found. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 3
- Review effort level: Low
rentziass
changed the title
rentziass
changed the title
rentziass
changed the title
rentziass
changed the title
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
TingluoHuang
reviewed
Jul 6, 2026
Comment on lines
+260
to
+263
| if (IsAcknowledgeJobNotFound(result.StatusCode, brokerError)) | ||
| { | ||
| throw new RunnerRequestJobNotFoundException(brokerError.Message); | ||
| } |
Member
There was a problem hiding this comment.
should this merge with the switch above somehow? since they are both kind of checking the brokererror and throw exception.
ericsciple
reviewed
Jul 6, 2026
| return statusCode == HttpStatusCode.NotFound && | ||
| brokerError?.StatusCode == (int)HttpStatusCode.NotFound && | ||
| !string.Equals(brokerError.ErrorKind, BrokerErrorKind.RunnerNotFound, StringComparison.Ordinal) && | ||
| string.Equals(brokerError.Message, "Job not found", StringComparison.OrdinalIgnoreCase); |
Collaborator
There was a problem hiding this comment.
Can you help me understand more about this?
Is there an error kind we could rely on, instead of message?
I'm worried relying on the error message might be fragile. That's why error kind exists.
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
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
Scope
Testing
Fixes #4312