fix(runs): persist run status on loop/time-budget halt (#34) - #41
Merged
Conversation
A run halted on its loop or wall-clock budget is enforced in BeginStep/CanProceed
(PreStep), which returned the HaltError before writing anything through to the
store. The governor was halted in memory (context cancelled, further calls
correctly refused with 402), but runs list / audit / GET /v1/runs/{id} kept
reporting "running" with an empty halt reason.
Persist the run on both halt paths (mirroring the token/dollar RecordCall path,
which already did). Best-effort and idempotent; no-op for the in-memory-only
manager. Adds TestManager_LoopHaltPersistsStatus covering the loop-budget path.
Addresses the halt-persistence half of #34. (The secondary note — a normally
completed run also stays "running" because there's no run-finished signal — is a
separate lifecycle design item, left for a follow-up.)
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.
Problem
A run halted on its loop or wall-clock budget kept
status: "running"(emptyhaltReason) in the store — visible inruns list,audit, andGET /v1/runs/{id}— even though it was terminally halted in memory (context cancelled, further calls correctly refused with402). Only token/dollar halts (which go throughRecordCall→persistCall) persisted.Caught while reviewing the observability story: a run the governor killed showing as
runningdirectly contradicts the demo narrative and undercuts the "observability done right" pitch — worth fixing before promo.Cause
Loop/time halts happen in
governor.PreStep(viaRun.BeginStep) and the pre-callCanProceed. Both flip the governor to halted and return theHaltErrorbefore anything is written through — the gateway's 402 path returns there without recording a call.Fix
Persist the run on both halt paths (
persistHalt→touch+persistRun). Best-effort, idempotent, and a no-op for the in-memory-only manager. Mirrors the existing token/dollar behavior.Tests
TestManager_LoopHaltPersistsStatus— loop budget = 1; the 2ndBeginStephalts; asserts the persisted run ishalted/loop_budget_exceeded.TestManager_HaltPersistsStatus(token path) still passes.go test ./internal/runs,go vet, gofmt clean.Scope
Addresses the halt-persistence half of #34. The secondary note — a normally completed run also stays
running(there's no run-finished signal from the SDK/governed_run) — is a separate lifecycle design item; leaving #34 open for that follow-up.