amadan
Close the stub's timeout gap, and test the wiring the stub cannot
The keymail stub rebuilt app.flow.Keymail without the HTTP field, so every exchange in the tests ran on the library's 15s default client rather than the app's bounded one. A stub test would therefore have passed against an app whose timeout wiring had been deleted. The reviewer named it; this closes it. The stub now bounds its own client the same way — but that still cannot prove newFlow wires one, because the stub replaces the builder. So there is a direct test of the real thing: the classifier and the keymail client each carry classifierTimeout, and RedirectPath is the app's callback rather than signin's /auth/callback default, which this app does not serve. It matters because both clients reach hosts chosen by whoever typed the address. Verified by removing HTTP from newFlow's keymail builder: the test fails with "the keymail client is unbounded".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 files changed,
+31
−0
internal/home/keymail_test.go+27 −0internal/home/keymailstub_test.go+4 −0
CI passed — run details
Intent
Attributed to this commit: 42.9 g CO₂e (5271174 tokens). This is an attribution, not a measurement — usage counted from this commit's own marker forward to the next one on step2/seal-and-secrets, never backward from it. A turn that produced more than one commit books all of its tokens to the last one, and a concurrent session on this branch can add to the same figure. Some of it was priced using the shared default rate, because no model-specific factor row exists yet for at least one model involved. The Carbon view has the repo total, which is never derived from this attribution.
diff --git a/internal/home/keymail_test.go b/internal/home/keymail_test.go| index 4f46782..ece31cc 100644 |
| --- a/internal/home/keymail_test.go |
| +++ b/internal/home/keymail_test.go |
| @@ -166,3 +166,30 @@ func TestCallbackWithWrongStateMintsNothing(t *testing.T) { |
| t.Fatalf("a mismatched state created %d sessions", n) |
| } |
| } |
| + |
| +// The stub replaces app.flow.Keymail, so it cannot prove that newFlow |
| +// wires a bounded client. This does, directly: the classifier and the |
| +// exchange both reach hosts chosen by whoever typed the address, and an |
| +// unbounded client there is a way to hold a handler open on a host the |
| +// submitter picked. |
| +func TestFlowUsesBoundedHTTPClients(t *testing.T) { |
| + app := newTestApp(t) |
| + fresh := newFlow(app.secrets, app.links) |
| + |
| + if fresh.Classifier.HTTP == nil { |
| + t.Error("the classifier has no HTTP client, so it uses an unbounded default") |
| + } else if fresh.Classifier.HTTP.Timeout != classifierTimeout { |
| + t.Errorf("classifier timeout is %v, want %v", fresh.Classifier.HTTP.Timeout, classifierTimeout) |
| + } |
| + |
| + km := fresh.Keymail("example.invalid") |
| + if km.HTTP == nil { |
| + t.Error("the keymail client is unbounded") |
| + } else if km.HTTP.Timeout != classifierTimeout { |
| + t.Errorf("keymail timeout is %v, want %v", km.HTTP.Timeout, classifierTimeout) |
| + } |
| + if km.RedirectPath != callbackPath { |
| + t.Errorf("keymail RedirectPath is %q, want %q — signin's default is /auth/callback, "+ |
| + "which this app does not serve", km.RedirectPath, callbackPath) |
| + } |
| +} |
diff --git a/internal/home/keymailstub_test.go b/internal/home/keymailstub_test.go| index 90d7d4d..bc28754 100644 |
| --- a/internal/home/keymailstub_test.go |
| +++ b/internal/home/keymailstub_test.go |
| @@ -112,6 +112,10 @@ func newKeymailStub(t *testing.T, app *testApp, address string) *keymailStub { |
| Base: "http://" + server, |
| Origin: Origin, |
| RedirectPath: callbackPath, |
| + // Bounded, like the real one. Omitting this left exchanges |
| + // on the library's 15s default client, so a stub test would |
| + // have passed against an app whose timeout wiring was gone. |
| + HTTP: &http.Client{Timeout: classifierTimeout}, |
| } |
| } |
| return k |