fix(runtime): surface local runtime failures#141
Conversation
Greptile SummaryThis PR surfaces more local runtime and session cleanup failures. The main changes are:
Confidence Score: 3/5This should be fixed before merging.
Focus on
|
| Filename | Overview |
|---|---|
| internal/localruntime/conversions.go | Tightens local runtime result conversion, but breaks legacy allowed-only responses. |
| internal/run/run.go | Surfaces more runtime and cleanup errors, including one unredacted user-facing teardown error. |
Reviews (1): Last reviewed commit: "fix(runtime): surface local runtime fail..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| decision, ok := hook.NormalizeDecision(result.Decision) | ||
| if !ok { | ||
| decision = resultFromBool(result.Allowed).Decision | ||
| return hook.Result{ | ||
| Decision: hook.DecisionDeny, | ||
| Reason: fmt.Sprintf("invalid local runtime decision %q", result.Decision), | ||
| } |
There was a problem hiding this comment.
EvaluateResult.Decision is still omitempty on the wire, and both the CLI hook path and local runtime client pass socket responses through this converter. When an older or hand-written local runtime sends a valid legacy response like {"allowed":true} without decision, this branch now returns a deny instead of preserving the previous allow behavior. That can block operations after only the client side is upgraded.
| if err := client.EndSession(endCtx, sessionID); err != nil { | ||
| fmt.Fprintf(out, "\n! Failed to end session (%s): %v\n", truncateID(sessionID), err) | ||
| return | ||
| } |
There was a problem hiding this comment.
This prints the raw EndSession error directly to stderr, while diagnostic logging goes through the redactor. If the backend or transport error includes a request URL, authorization detail, cookie, or token-shaped parameter, teardown failures can leak that value into terminal or CI logs even when verbose diagnostics are off.
| if err := client.EndSession(endCtx, sessionID); err != nil { | |
| fmt.Fprintf(out, "\n! Failed to end session (%s): %v\n", truncateID(sessionID), err) | |
| return | |
| } | |
| if err := client.EndSession(endCtx, sessionID); err != nil { | |
| fmt.Fprintf(out, "\n! Failed to end session (%s): %s\n", truncateID(sessionID), diagnostic.Redact(err.Error())) | |
| return | |
| } |
1 participant
Summary
This cleans up
kontext startlocal runtime handling by surfacing real failures instead of silently ignoring them.Before this,
kontext startand local runtime startup/cleanup ignored several errors (host/cwd/executable lookup, socket removal/close, session end/close).ResultFromEvaluateResultalso fell back toallowedwhendecisionwas invalid, which is a fail-open bug.Now there is one stricter path:
internal/run/run.gosurfaces and logs lifecycle failures.internal/localruntime/service.gofail-fast on unexpected socket cleanup failures.internal/localruntime/conversions.godenies on invalid decision or decision/allowed mismatch (explicit reason).internal/guard/judge/llama_server_test.gomakes the early-exit timing assertion stable under-race.Why
This gives kontext-cli a cleaner maintenance path for local runtime correctness:
kontext start-> localruntime service (socket)
-> runtimecore session lifecycle
-> hook decision conversion
-> explicit failure surfaced
This PR does not broaden behavior beyond the cleanup scope.
What changed
Removed silent fallbacks in local runtime conversions
Surfaced ignored errors in
kontext startruntime lifecycleMade local runtime socket cleanup strict
Stabilized judge early-exit timing under race
Verification
go test ./...go vet ./...go test -race ./...