From b399ebe3918e1931fae1f384b31ff6864db3c9b9 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 26 Sep 2026 16:17:23 +0000 Subject: [PATCH] Test deadline behavior without context identity --- docs/roadmap/remediation.md | 24 ++++++++++++++++++------ internal/adapter/http/handler_test.go | 25 ++++++++++++++++--------- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index c965689..1f01a59 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -3,18 +3,18 @@ ## Scope And Status Review baseline: `e177d7c`, covering implementation commits after `5c3b0cf`. -This document records an outstanding test-contract issue; it does not authorize +This document records a resolved test-contract issue; it does not authorize unrelated implementation changes. No runtime defect or missing planned feature was identified in this review. Release preparation remains separate work. ## R1: Remove The Unrequested Context-Identity Invariant -Priority: P3 (low). Status: Open. +Priority: P3 (low). Status: Complete. -Location: `internal/adapter/http/handler_test.go:334–349`, +Original location: `internal/adapter/http/handler_test.go:334–349`, `TestHandlerRunTimeoutValidationAndContextMapping`, introduced in `7de2b02`. -The test's `child` field and `(ctx != parent) != tc.child` assertion require +The test's `child` field and `(ctx != parent) != tc.child` assertion required the exact parent context object for omitted/null/zero timeouts and a distinct object for a positive timeout. The requested contract concerns effective deadlines and cancellation propagation, not Go interface identity. In @@ -22,7 +22,7 @@ particular, a harmless derived context that preserves the parent's deadline, values, and cancellation would fail the omission cases despite satisfying the application contract. -This elevates an incidental implementation choice into a test-enforced +This elevated an incidental implementation choice into a test-enforced invariant, contrary to the [agent policy](../policy/agents.md) and [testing policy](../policy/testing.md). The current handler's context behavior is correct; this is a maintainability issue, not a runtime cancellation bug. @@ -48,7 +48,19 @@ refactor solely to change this test. is a different object. - Existing timeout validation and HTTP outcome coverage remains intact. -## Review Verification +### Resolution And Validation + +Removed the identity-only table field and context comparisons. The test now +checks effective deadlines, including omitted/null/zero requests with no +parent deadline, and explicitly checks that parent cancellation reaches the +runner. Existing positive-deadline integration tests and live-context +provider-timeout mapping remain intact. No production code changed. + +Focused HTTP timeout tests, `go test ./...`, `go test -race ./...`, +`go vet ./...`, a native build outside the repository, and `git diff --check` +all passed after remediation. + +## Original Review Verification - `go test -race -count=1 ./...` passed, including retry, shared-capacity, deadline, report, definition-conformance, and architecture-boundary tests. diff --git a/internal/adapter/http/handler_test.go b/internal/adapter/http/handler_test.go index 11c4691..f4b095e 100644 --- a/internal/adapter/http/handler_test.go +++ b/internal/adapter/http/handler_test.go @@ -332,26 +332,30 @@ func TestHandlerRunTimeoutValidationAndContextMapping(t *testing.T) { defer cancel() parentDeadline, _ := parent.Deadline() for _, tc := range []struct { - name, body string - child bool + name, body string + parent context.Context + wantDeadline time.Time }{ - {name: "omitted", body: `{"prompt_id":"p"}`}, - {name: "null", body: `{"prompt_id":"p","run_timeout_seconds":null}`}, - {name: "zero", body: `{"prompt_id":"p","run_timeout_seconds":0}`}, - {name: "positive", body: `{"prompt_id":"p","run_timeout_seconds":10}`, child: true}, + {name: "omitted inherits deadline", body: `{"prompt_id":"p"}`, parent: parent, wantDeadline: parentDeadline}, + {name: "null inherits deadline", body: `{"prompt_id":"p","run_timeout_seconds":null}`, parent: parent, wantDeadline: parentDeadline}, + {name: "zero inherits deadline", body: `{"prompt_id":"p","run_timeout_seconds":0}`, parent: parent, wantDeadline: parentDeadline}, + {name: "positive preserves earlier deadline", body: `{"prompt_id":"p","run_timeout_seconds":10}`, parent: parent, wantDeadline: parentDeadline}, + {name: "omitted adds no deadline", body: `{"prompt_id":"p"}`, parent: context.Background()}, + {name: "null adds no deadline", body: `{"prompt_id":"p","run_timeout_seconds":null}`, parent: context.Background()}, + {name: "zero adds no deadline", body: `{"prompt_id":"p","run_timeout_seconds":0}`, parent: context.Background()}, } { t.Run(tc.name, func(t *testing.T) { called := false h := NewHandler(runnerFunc(func(ctx context.Context, req promptkit.RunRequest) (*promptkit.RunResult, error) { called = true deadline, ok := ctx.Deadline() - if !ok || !deadline.Equal(parentDeadline) || (ctx != parent) != tc.child { - t.Fatalf("unexpected execution context: deadline=%v child=%t", deadline, ctx != parent) + if ok != !tc.wantDeadline.IsZero() || (ok && !deadline.Equal(tc.wantDeadline)) { + t.Fatalf("execution deadline: got %v (present=%t), want %v", deadline, ok, tc.wantDeadline) } return &promptkit.RunResult{}, nil })) w := httptest.NewRecorder() - h.ServeHTTP(w, httptest.NewRequest(http.MethodPost, "/v1/runs", strings.NewReader(tc.body)).WithContext(parent)) + h.ServeHTTP(w, httptest.NewRequest(http.MethodPost, "/v1/runs", strings.NewReader(tc.body)).WithContext(tc.parent)) if !called || w.Code != http.StatusOK { t.Fatalf("context mapping: called=%t status=%d body=%s", called, w.Code, w.Body.String()) } @@ -361,6 +365,9 @@ func TestHandlerRunTimeoutValidationAndContextMapping(t *testing.T) { canceled, cancelParent := context.WithCancel(context.Background()) cancelParent() h := NewHandler(runnerFunc(func(ctx context.Context, req promptkit.RunRequest) (*promptkit.RunResult, error) { + if ctx.Err() != context.Canceled { + t.Fatalf("parent cancellation did not reach execution: %v", ctx.Err()) + } return nil, ctx.Err() })) w := httptest.NewRecorder()