Add interruption reason to job_should_exit? and interrupted notification#683
Merged
Conversation
SomalianIvan
force-pushed
the
timb/add-interruption-reason
branch
2 times, most recently
from
080106b to
3ecf059
Compare
`job_should_exit?` now returns a reason string instead of bare `true`: - "max_job_runtime_exceeded" when runtime limit is hit - "interrupted" when the interruption adapter signals exit - Custom overrides can return their own reason strings The reason is threaded through to the "interrupted.iteration" ActiveSupport::Notifications event and logged by LogSubscriber. This is backwards compatible — existing overrides returning `true` still work (reason shows as "unknown"), and the `reason` key in the notification payload is additive. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
SomalianIvan
force-pushed
the
timb/add-interruption-reason
branch
from
3ecf059 to
a09c0e7
Compare
freeze_time prevented any time from elapsing, so the check `(Time.now.utc - start_time) > 0` was never true with max_job_runtime=0. Without freeze_time, real time elapses between iterations, triggering the interruption as expected. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use a slow job class that travels time during each iteration, matching the pattern used in iteration_test.rb. This avoids flakiness from relying on real wall time to exceed max_job_runtime. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Mangara
reviewed
Mar 6, 2026
If the adapter returns a string, use it as the interruption reason. Otherwise fall back to "interrupted". Also remove unnecessary || false from job_should_exit?. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
SomalianIvan
force-pushed
the
timb/add-interruption-reason
branch
5 times, most recently
from
e9f6894 to
a18e92d
Compare
All existing interruption adapters return booleans, so keep it simple and just use "interrupted" as the reason. Custom adapter reasons can be added when there's a real need. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
SomalianIvan
force-pushed
the
timb/add-interruption-reason
branch
from
a18e92d to
3200574
Compare
Mangara
approved these changes
Mar 17, 2026
Mangara
left a comment
Mangara
left a comment
Contributor
There was a problem hiding this comment.
Let's add this to the change log, then it's ready.
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
2 participants
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
job_should_exit?now returns a reason string ("max_job_runtime_exceeded","interrupted") instead of baretrue"interrupted.iteration"ActiveSupport::Notifications payload asreason:LogSubscriber#interruptedlogs the reason when presenttruestill work (reason shows as"unknown")Motivation
It's useful to know why a job was interrupted, not just that it was. Currently there's no way to distinguish between max runtime, adapter signal, or custom exit logic without duplicating the checks.
Test plan
"interrupted.iteration"notification includesreasonin payloadreason=when job is interrupted🤖 Generated with Claude Code