From c0755f3391296b43f0a0730ea21fa99d57c2d318 Mon Sep 17 00:00:00 2001 From: Adarsh Prashar Date: Sun, 31 May 2026 20:25:15 +0530 Subject: [PATCH] fix(runs): persist run status on loop/time-budget halt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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.) --- CHANGELOG.md | 7 +++++++ internal/runs/manager.go | 20 +++++++++++++++++++- internal/runs/manager_test.go | 27 +++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d4f3c7..a003683 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,13 @@ surface is governed by [`COMPATIBILITY.md`](COMPATIBILITY.md). ## [Unreleased] +### Fixed +- **Loop/time-budget halts now persist the run status.** A run halted on its loop + or wall-clock budget is enforced in `BeginStep`/`CanProceed`, which returned + before writing through — so `runs list`, `audit`, and `GET /v1/runs/{id}` still + showed it as `running`. The halt (and its reason) is now persisted on that path, + matching the token/dollar halt behavior. ([#34](https://github.com/prashar32/riskkernel/issues/34)) + ## [0.1.1] - 2026-05-31 A fast follow-up to v0.1.0: makes the Python SDK installable from a build, and diff --git a/internal/runs/manager.go b/internal/runs/manager.go index 0912a41..5695ccc 100644 --- a/internal/runs/manager.go +++ b/internal/runs/manager.go @@ -69,6 +69,11 @@ type View struct { // start another step. func (r *Run) BeginStep() (int32, error) { if err := r.gov.PreStep(); err != nil { + // PreStep flipped the governor to halted (loop or time budget). Persist that + // terminal state — the gateway's 402 path returns here without recording a + // call, so without this the run would still read "running" in + // runs list / audit / GET. (#34) + r.persistHalt() return 0, err } now := time.Now() @@ -83,7 +88,20 @@ func (r *Run) BeginStep() (int32, error) { } // CanProceed enforces the hard ceiling immediately before a model/tool call. -func (r *Run) CanProceed() error { return r.gov.CanProceed() } +func (r *Run) CanProceed() error { + if err := r.gov.CanProceed(); err != nil { + r.persistHalt() // token/dollar/time ceiling tripped pre-call → persist it (#34) + return err + } + return nil +} + +// persistHalt writes the run's now-halted state through to the store. Best-effort +// and idempotent; a no-op for the in-memory-only manager (nil store). +func (r *Run) persistHalt() { + r.touch() + r.mgr.persistRun(r) +} // RecordCall meters a completed model call in the governor and writes through to // the cost ledger + step + run rows. Returns a *governor.HaltError if this call diff --git a/internal/runs/manager_test.go b/internal/runs/manager_test.go index 9a547bc..1fbd84e 100644 --- a/internal/runs/manager_test.go +++ b/internal/runs/manager_test.go @@ -197,3 +197,30 @@ func TestManager_HaltPersistsStatus(t *testing.T) { t.Fatalf("halted step not persisted: %+v", steps) } } + +// A loop-budget halt happens in BeginStep/PreStep, which returns before recording +// a call — so the run's persisted status must be updated there too, not only on the +// RecordCall (token/dollar) path. (#34) +func TestManager_LoopHaltPersistsStatus(t *testing.T) { + store, err := storage.OpenSQLite(filepath.Join(t.TempDir(), "loophalt.db")) + if err != nil { + t.Fatal(err) + } + defer store.Close() + + m := NewManager(governor.Budget{Loops: 1}). + WithStore(store, slog.New(slog.NewTextHandler(noopWriter{}, nil))) + r := m.Create(CreateOptions{ID: "run-l"}) + + if _, err := r.BeginStep(); err != nil { // step 1: allowed (loops 0→1) + t.Fatalf("first BeginStep: %v", err) + } + if _, err := r.BeginStep(); err == nil { // step 2: 1+1 > 1 → loop-budget halt + t.Fatal("expected loop-budget halt on second BeginStep") + } + + got, _ := store.GetRun(context.Background(), "run-l") + if got.Status != "halted" || got.HaltReason != string(governor.HaltLoopBudget) { + t.Fatalf("loop halt not persisted: status=%q reason=%q", got.Status, got.HaltReason) + } +}