Thread context through Provider.Execute and stop reporting read failures as unknown

Finding 6 of REVIEW-2026-07-30.md. runWithTimeout could not cancel anything,
because Provider.Execute took no context: the goroutine ran on to the HTTP
client's 60s timeout, outliving the 30s capability timeout. Provider.Execute
now takes a context carrying that timeout, and the workspace provider issues
its tool call with http.NewRequestWithContext, so a timed-out execution
actually tears the request down.

A read-only capability whose provider call times out now resolves to failed
rather than unknown. Spec §4.3 reserves unknown for executions whose side
effect may or may not have landed, and never retries them — which made read
failures both unretryable and indistinguishable from genuinely ambiguous
mutations, for calls that by definition have no side effect. Mutating
capabilities still resolve to unknown.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uea55zaiWuEByEDC4UBSdd
This commit is contained in:
kami
2026-07-30 23:39:50 +04:00
parent 47be24c4cc
commit be08938f1f
5 changed files with 129 additions and 32 deletions
+72 -6
View File
@@ -1,6 +1,7 @@
package execution_test
import (
"context"
"errors"
"path/filepath"
"testing"
@@ -16,13 +17,29 @@ type fakeProvider struct {
name string
delay time.Duration
err error
// returned closes once Execute has actually returned, letting a test
// prove the provider call was cancelled rather than left running.
returned chan struct{}
// ctxErr records the context error observed by Execute, if any.
ctxErr error
}
func (p *fakeProvider) Name() string { return p.name }
func (p *fakeProvider) Execute(capability *domain.Capability, req *domain.ExecuteRequest) (map[string]any, error) {
func (p *fakeProvider) Execute(ctx context.Context, capability *domain.Capability, req *domain.ExecuteRequest) (map[string]any, error) {
if p.returned != nil {
defer close(p.returned)
}
if p.delay > 0 {
time.Sleep(p.delay)
// A well-behaved provider aborts as soon as its context is done —
// this stands in for tearing down an in-flight HTTP request.
select {
case <-time.After(p.delay):
case <-ctx.Done():
p.ctxErr = ctx.Err()
return nil, ctx.Err()
}
}
if p.err != nil {
return nil, p.err
@@ -286,8 +303,57 @@ func TestExecute_TimeoutYieldsUnknownNotFailed(t *testing.T) {
if res.Execution.Status != domain.ExecutionUnknown {
t.Fatalf("expected status unknown on timeout, got %s", res.Execution.Status)
}
// Let the background provider goroutine finish so t.Cleanup can close
// the store without a dangling write racing it.
time.Sleep(1600 * time.Millisecond)
}
// A read_only capability has no side effect, so a timeout is an unambiguous
// failure: retryable, and distinguishable from a mutation that may or may not
// have landed. ECOSYSTEM-SPEC.md §4.3's "unknown" outcome must not be used
// for reads.
func TestExecute_ReadOnlyTimeoutYieldsFailedNotUnknown(t *testing.T) {
prov := &fakeProvider{name: "slow", delay: 1500 * time.Millisecond}
eng, store := newTestEngine(t, prov)
cap := mustCreateCapability(t, store, func(c *domain.Capability) {
c.TimeoutSeconds = 1
c.Provider = "slow"
c.Risk = "read"
c.ReadOnly = true
})
res, err := eng.Execute(&domain.ExecuteRequest{CapabilityID: cap.ID, TargetEntityID: "ent_x"})
if err != nil {
t.Fatalf("execute: %v", err)
}
if res.Execution.Status != domain.ExecutionFailed {
t.Fatalf("expected status failed on read-only timeout, got %s", res.Execution.Status)
}
if res.Execution.Error == "" {
t.Fatal("expected a timeout error message on the execution")
}
}
// The capability timeout must actually cancel the provider call. Before the
// context was threaded through Provider.Execute, the goroutine ran on to the
// provider's own (much longer) client timeout; here it must observe
// cancellation and return promptly after the 1s capability timeout, well
// before its nominal 30s delay.
func TestExecute_TimeoutCancelsProviderCall(t *testing.T) {
prov := &fakeProvider{name: "slow", delay: 30 * time.Second, returned: make(chan struct{})}
eng, store := newTestEngine(t, prov)
cap := mustCreateCapability(t, store, func(c *domain.Capability) {
c.TimeoutSeconds = 1
c.Provider = "slow"
})
if _, err := eng.Execute(&domain.ExecuteRequest{CapabilityID: cap.ID, TargetEntityID: "ent_x"}); err != nil {
t.Fatalf("execute: %v", err)
}
select {
case <-prov.returned:
case <-time.After(5 * time.Second):
t.Fatal("provider call was not cancelled: it outlived the capability timeout")
}
if !errors.Is(prov.ctxErr, context.DeadlineExceeded) {
t.Fatalf("expected provider to observe context.DeadlineExceeded, got %v", prov.ctxErr)
}
}