feat(register): warn when a run/build appears in flight during defaul…#229
Open
TrevorBasinger wants to merge 3 commits into
Open
feat(register): warn when a run/build appears in flight during defaul…#229TrevorBasinger wants to merge 3 commits into
TrevorBasinger wants to merge 3 commits into
Conversation
…ted publish Bare `roar register` (no target) defaults to publishing the whole active session, but job rows are only written when a job completes and Session has no heartbeat/last-activity field — so the defaulted-active-session prompt had no way to tell a concurrent `roar run`/`roar build` in another terminal was still adding to that same session before it published. Add active-run markers (`.roar/active_runs/<pid>.json`, written for the lifetime of RunCoordinator.execute() and self-healed via os.kill(pid, 0) on read) mirroring the existing S3 proxy daemon liveness pattern, and surface a non-blocking warning line in confirm_defaulted_active_session_publish when another live run is found. Warn, never block, consistent with the rest of this prompt. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
`roar status` has the same blind spot register's defaulted-active-session prompt did: job rows only land in the DB when a job completes, so a concurrent `roar run`/`roar build` in another terminal is invisible to the session summary until it finishes. Move the marker-lookup + warning-formatting helpers from publish_intent.py into execution/runtime/active_runs.py (in_flight_run_warnings, format_elapsed) so both `register` and `status` share one implementation instead of duplicating it, and wire the warning into `roar status` (stderr, unconditional — a data-freshness risk, not a UX hint). Co-Authored-By: Claude Sonnet 5 <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
Bare
roar register(no target) defaults to publishing the whole activesession. Job rows are only written to the DB when a job completes
(
ExecutionJobRecorder.record()→record_job()fires once, afterduration_seconds/exit_codeare known), andsessionshas noheartbeat/last-activity column — only
created_at. So if terminal A is mid-roar runand terminal B runs a bareroar registerin the same repo, the newdefaulted-active-session confirmation prompt (#224) has nothing to look at:
it can only describe the scope of what's about to publish, not whether that
scope is still changing. Terminal B silently publishes a session that
terminal A is still adding to, with zero warning.
There was also no existing signal to repurpose for this:
Job.status's"running"enum value is defined in the schema but never set anywhere inapplication code (dead), and no lock/PID/heartbeat file exists for
roar run/roar build— the only PID-liveness pattern in the codebase(
.roar/proxy.json+os.kill(pid, 0)) tracks the S3 proxy daemon, not jobexecution.
.roar/active_runs/<pid>.jsonmarkers, written for the exact lifetimeof
RunCoordinator.execute()(active_run_marker()context manager,cleaned up on success, tracer error, exception, or signal-abort) and
self-healed on read via
os.kill(pid, 0)— same shape as the existingproxy-daemon liveness check, just scoped to run/build processes instead of
the proxy.
confirm_defaulted_active_session_publish()now checks for other livemarkers and prints a line like:
Warning: a roar run (pid 48213, started 2m14s ago:python train.py) appears to still be in progress — the active session may be incomplete.before the existing "Publish the whole active session?" prompt. Best-effort
and non-blocking — consistent with the rest of this prompt's warn-don't-block
style.
roar putwasn't touched — it always requires an explicit target, so itnever hits the defaulted-active-session ambiguity this fixes.