fix(mt#4985): Make the timeout classification name the error that actually arrived

[minsky-ai]

Summary

packages/domain/src/ai/embedding-service-openai.test.ts AT1 asserted
expect(isRequestTimeoutError(err)).toBe(true). That fails with Expected: true, Received: false
and says nothing about what actually rejected. On 2026-09-04 it failed once under the full gated
suite and passed 3/3 in isolation, blocking a session_pr_create on unrelated work — and the only
record it left was those two words, so the cause could not be determined afterwards and the flake is
too rare to reproduce on demand.

This ships the diagnostic, not a fix. timeoutVerdict(err) returns "TimeoutError" on the passing
path and "<name>: <message>" otherwise, so the same failure names its own cause.

It does not make the flake less likely. It makes the next occurrence self-describing. A green run
after this change is therefore consistent with the flake still being present — AT4 states that bound
rather than reading green as fixed.

The task's own premise was falsified during planning

The spec asserted a SLOW path — "the wall-clock budget … can be crossed by scheduling delay." The
failure line it pasted reads [5.56ms] against a TIMEOUT_MS of 150: the request rejected ~27x
faster than the timeout it should have hit, so the abort never fired and nothing crossed a
budget. The falsifier was inside the spec's own transcribed quote. The cause is now deliberately
UNSTATED and the scope is written to finding it.

Both fixes the spec originally proposed were also unsound and were dropped: injecting a clock does
not apply (the timeout is a real AbortSignal on a real socket), and asserting on a constructed
error contradicts the suite's own docblock — "These deliberately do NOT mock fetch: … a mocked
rejection would prove nothing about whether the fix works."

Key changes

• timeoutVerdict(err) — reads only the error the test already caught. No clock dependency, no
  mock, real stalled server preserved.
• Applied to both real-socket assertions: AT1 and its batch sibling AT3. Class, not instance —
  the file's other two isRequestTimeoutError sites construct a DOMException directly, are
  deterministic, and are not in this class, so they are untouched.
• A deterministic test pinning what the diagnostic reports.

Testing

Execution evidence:

AT1 — the instrumentation's own check, with the classification forced to fail. Deterministic; it
does not wait on the flake to recur:

(pass) OpenAIEmbeddingService request timeout (mt#3444) > mt#4985: the timeout verdict names the
       error that actually arrived [0.15ms]

AT3 — passes in isolation (23 pass, up from 22 with the new test):

 23 pass
 0 fail
 57 expect() calls
Ran 23 tests across 1 file. [26.36s]

AT4 — the full main suite, the partition the incident occurred in:

 17776 pass
 10 skip
 0 fail
 50981 expect() calls
Ran 17786 tests across 1148 files. [346.55s]

(pass) mt#4985: the timeout verdict names the error that actually arrived [0.12ms]
(pass) AT1: a stalled single request rejects within the bound instead of hanging [155.12ms]
(pass) AT3: a stalled BATCH request rejects within the bound too [153.50ms]

Note the change-scoped runner is NOT sufficient evidence here: run-tests-gated.ts selected 1 of
1148 files for this diff, so it never exercises the load condition. The run above is
run-tests-main.ts unscoped.

AT2 — negative control: AT1 pointed at a dead port (http://127.0.0.1:1/v1), stalled server
left running so only the failure mode changes, and the test observed FAILING.

expect(timeoutVerdict(err)).toBe("TimeoutError");
Expected: "TimeoutError"
Received: "Error: Unable to connect. Is the computer able to access the url?"
(fail) AT1: a stalled single request rejects within the bound instead of hanging [4.01ms]

The control corroborated the direction rather than just proving the probe can fail. Two findings
the planning pass could only hypothesize:

1. Timing matches the incident. The control failed in 2.22ms and 4.01ms across two runs,
   against the incident's 5.56ms — same band, ~40x below the 150ms bound. The healthy path in
   those same runs sits at 152–155ms, i.e. exactly at the bound. A connection-shaped failure
   reproduces the incident's timing signature; a slow path cannot.
2. The connection error's name is plain Error, not TypeError — which is exactly why
   isRequestTimeoutError (a name === "TimeoutError" check) rejects it silently. The deterministic
   test now asserts this VERBATIM observed string rather than an invented one.

What the control does not buy. It proves a connection failure produces this shape and this
timing. It does not prove that is what happened on 2026-09-04 — that run was not reproduced. The
instrumentation is what settles it, on the next occurrence, for free. Fix restored and re-verified
after the control.

Typecheck clean across 8 projects; lint clean over 4,399 files; prettier clean.

Deploy verification

isDeploySurfaceFile returns true for the changed file — run as the predicate over the actual
changed-file list, not recalled from a pattern list:

true packages/domain/src/ai/embedding-service-openai.test.ts

That is a surprising result for a test file and is precisely why the tag was not written. No
[no-deploy-impact] claim is made anywhere in this PR or its commit message. Post-merge I will
wait on the deployment bound to this merge (notBefore = merge time, expectCommitSha = merge SHA)
and assert the health body's service identity rather than the status code.

Parallel work

No collision. git_log --path over the changed file and over request-resilience.ts for 7 days:
pathMatched: true, no commits. Two open PRs were real candidates and both were read by
changed-file list rather than title: PR #3590 (mt#3575, test-suite order-independence and
randomization) touches 15 files, none under packages/domain/src/ai/; PR #3412 (mt#4639,
614-site log conversion) touches 313 files — enumerated across all four pages, since a single
100-item page is a truncated list whose "no hit" is worth nothing. Six of its files are under
packages/domain/src/ai/, and the changed file is not among them.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TPNyYTreM4F7PRbvh4xA5b

[minsky-reviewer]

Minsky Reviewer Status

Verdict: APPROVED — no blocking findings
Review: View review
Model: openai/gpt-5 | Tokens: 120K prompt, 3K completion | Duration: 58s
Mode: test-only

Commands

• /review — request a fresh review

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Overall, the change is tight and scoped to tests. The new timeoutVerdict helper makes timeout-related failures self-describing without altering timing or test mechanics, and both real-socket assertions are routed through it. Acceptance criteria around self-diagnosing failures and preserving the real socket are satisfied from the code. I left two non-blocking notes: (1) timeoutVerdict is exported from a test file despite having no external consumers — consider making it file-local or moving it to a shared test helper if reuse is intended; (2) the comment promising non-Error values won’t show as "[object Object]" overstates current behavior for plain objects — either adjust the formatter or the comment. No blocking issues found; approving.

Findings

• [NON-BLOCKING] packages/domain/src/ai/embedding-service-openai.test.ts:370 — Helper timeoutVerdict is exported from a test file without external consumers
  export function timeoutVerdict(err: unknown): string { … } is declared at module scope and exported from a test file. This leaks a symbol that can be imported elsewhere unintentionally and complicates future refactors. Since it is only used locally within this file, prefer making it file-local (remove export) to keep the test surface minimal. If you intend it to be reused across tests, consider moving it to a dedicated test helper module (e.g., __tests__/helpers/ai.ts) and documenting that contract.
• [NON-BLOCKING] packages/domain/src/ai/embedding-service-openai.test.ts:164 — Helper timeoutVerdict is exported from a test file without external consumers
  export function timeoutVerdict(err: unknown): string { … } is declared at module scope and exported from a test file. This unnecessarily expands the test module's API surface and can encourage unintended reuse. Since it is only used locally within this file, prefer making it file-local (remove export). If reuse across tests is desired, move it to a dedicated test helper module and document its contract.
• [NON-BLOCKING] packages/domain/src/ai/embedding-service-openai.test.ts:138 — Comment over-promises about avoiding "[object Object]" for non-Error rejections
  The docstring above timeoutVerdict claims non-Error rejections won't render as "[object Object]" or empty, but the implementation falls back to String(err) when message is absent: const message = … ? e.message : String(err); (packages/domain/src/ai/embedding-service-openai.test.ts:171). For an object like {}, this will yield "(no name): [object Object]". Consider adjusting the comment to match behavior or updating the formatter to handle plain objects (e.g., JSON.stringify with a length cap) and add a test case to pin it.

Spec verification

Criterion	Status	Evidence
The failure names its own cause. When the assertion fails, the output carries the actual error's name and message rather than only Expected: true, Received: false.	Met	Implemented via timeoutVerdict(err) at packages/domain/src/ai/embedding-service-openai.test.ts:164-177, and both real-socket assertions now assert expect(timeoutVerdict(err)).toBe("TimeoutError") (:214-218, :247-251), ensuring a non-timeout error prints <name>: <message>.
The instrumentation costs nothing on the passing path and cannot itself fail under load — it reports what the error WAS, and adds no new timing dependency.	Met	timeoutVerdict(err) is a pure formatter that checks isRequestTimeoutError and otherwise derives name/message from the caught value (packages/domain/src/ai/embedding-service-openai.test.ts:164-177). It introduces no async, I/O, or timing.
The test still FAILS when the timeout path is genuinely broken. A change that makes it pass by loosening a bound until nothing can fail is the can't-fail-probe failure (mem#704) — record a negative control showing the assertion still catches a real regression.	Unverifiable	This criterion depends on execution evidence (a deliberately induced failure) that is not present in the diff. The code adds a deterministic test that exercises timeoutVerdict on constructed errors (packages/domain/src/ai/embedding-service-openai.test.ts:190-212), but whether the main timeout assertions still fail in a genuine broken-path scenario is not verifiable from static code alone.
The real socket and the stalled server are PRESERVED. The suite's docblock states the design reason: "These deliberately do NOT mock fetch: the defect was that the real fetch never self-times-out, so a mocked rejection would prove nothing about whether the fix works." Constructing the error to make the assertion deterministic is explicitly out.	Met	The existing real-socket tests remain and still use Bun.serve({ port: 0, fetch() { return new Promise(() => {}); } }) (packages/domain/src/ai/embedding-service-openai.test.ts:184-202, 220-231, 255-293). No mocking of fetch was added to those tests; only a separate deterministic unit test was introduced.
Verified under the condition that produced the failure, not only in isolation: bun scripts/run-tests-gated.ts passes with it.	Unverifiable	This requires a live gated run outside the diff. No such run artifacts are part of the code change, so this cannot be verified from repository contents.
If a later failing run shows the cause IS the runner's per-test budget rather than anything in this test, say so and hand off to mt#4321 rather than patching here.	N/A	No code paths in this diff relate to runner budget detection or handoff mechanics; the change is limited to test assertions/diagnostics in packages/domain/src/ai/embedding-service-openai.test.ts.

Adoption sweep

Symbol	Kind	Consumers found	Classification	Notes
packages/domain/src/ai/embedding-service-openai.test.ts:timeoutVerdict	function	—	Missing consumers	Exported from a test file; intended for local use within the file. Recommend removing export or moving to a shared test helper if cross-file reuse is desired.

Recommendation: file a follow-up adoption task to wire 1 missing consumer.

Documentation impact

• no-update-needed — Test-only change: adds a diagnostic helper and updates assertions within packages/domain/src/ai/embedding-service-openai.test.ts. No user-facing APIs, commands, or behavior changed; no documentation references this test internals. I did not find any docs covering this test file specifically.

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Re-verification complete. The follow-up commit addresses the prior non-blocking observations: timeoutVerdict is now test-local (no unused export), and the comment correctly scopes the fallback behavior. The new timeoutVerdict(err) helper is pure and only affects assertion messaging; it introduces no timing or load dependencies. Both real-socket tests (AT1/AT3) retain their behavior and now surface the actual error on failure, while the added deterministic test validates the diagnostic without stubbing around the production path. I see no vacuous assertions, no masking of the exercised behavior, and no added flakiness. Test deletions did not occur. Verdict: APPROVE.

Spec verification

Criterion	Status	Evidence
The failure names its own cause. When the assertion fails, the output carries the actual error's name and message rather than only Expected: true, Received: false.	Met	packages/domain/src/ai/embedding-service-openai.test.ts:468-472,502-505 — assertions now route through timeoutVerdict(err) and compare to "TimeoutError"; helper returns "<name>: <message>" for non-timeouts (lines 370-388), ensuring failures print the arriving error's name/message.
The instrumentation costs nothing on the passing path and cannot itself fail under load — it reports what the error WAS, and adds no new timing dependency.	Met	packages/domain/src/ai/embedding-service-openai.test.ts:370-388 — timeoutVerdict(err) is a pure classifier over the caught error; no timers/awaits. It is only used to format assertions (lines 468-472, 502-505), leaving request timing unchanged.
The test still FAILS when the timeout path is genuinely broken. A change that makes it pass by loosening a bound until nothing can fail is the can't-fail-probe failure (mem#704) — record a negative control showing the assertion still catches a real regression.	Unverifiable	Negative-control execution evidence is described in the PR body, not codified in-repo. This diff adds a deterministic unit test for timeoutVerdict (packages/domain/src/ai/embedding-service-openai.test.ts:420-446) but does not introduce a failing-path harness scenario; verifying the recorded failing output requires out-of-repo runs.
The real socket and the stalled server are PRESERVED. The suite's docblock states the design reason: "These deliberately do NOT mock fetch: … a mocked rejection would prove nothing about whether the fix works." Constructing the error to make the assertion deterministic is explicitly out.	Met	packages/domain/src/ai/embedding-service-openai.test.ts:447-498 — AT1/AT3 still start a real stalled server via startStalledServer() and perform real requests; only the new mt#4985 unit test (lines 420-446) constructs errors to verify the diagnostic itself, without replacing the real-socket tests.
Verified under the condition that produced the failure, not only in isolation: bun scripts/run-tests-gated.ts passes with it.	Unverifiable	Pass results for the full gated suite are reported in the PR description; they cannot be verified from the repository diff alone.
If a later failing run shows the cause IS the runner's per-test budget rather than anything in this test, say so and hand off to mt#4321 rather than patching here.	N/A	No change in this diff relates to runner budget or hand-off; this is a future conditional action outside the scope of the code changes here.

Documentation impact

• no-update-needed — Test-only change in packages/domain/src/ai/embedding-service-openai.test.ts: adds a helper and a deterministic test, and routes existing assertions through it. No production behavior, public API, CLI, or user-facing contract changed; no docs reference these internal test assertions.

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Re-verification complete. The change is confined to tests and introduces a local timeoutVerdict(err) helper plus a deterministic unit that validates the diagnostic formatting. Both real-socket timeout tests (AT1/AT3) continue to exercise the same behavior and now surface the arriving error’s name/message on failure; there’s no stubbing that masks the production path, no vacuous assertions, and no added timing dependencies. No tests were deleted. I find no new critical defects introduced by this fix and the prior non-blocking observations are addressed. Verdict: APPROVE.

Spec verification

Criterion	Status	Evidence
The failure names its own cause. When the assertion fails, the output carries the actual error's name and message rather than only Expected: true, Received: false.	Met	packages/domain/src/ai/embedding-service-openai.test.ts:352-389 adds timeoutVerdict(err), which returns "TimeoutError" on classification pass and "<name>: <message>" otherwise. Assertions in AT1/AT3 are routed through it at :470-473 and :502-505, so failures now print the arriving error's name/message.
The instrumentation costs nothing on the passing path and cannot itself fail under load — it reports what the error WAS, and adds no new timing dependency.	Met	timeoutVerdict(err) (packages/domain/src/ai/embedding-service-openai.test.ts:376-389) is a pure stringifier over the already-caught err, using isRequestTimeoutError(err) and simple property reads. It introduces no async, clocks, or network I/O. Passing-path assertion equivalence is maintained by returning exactly "TimeoutError" when the predicate is true.
The test still FAILS when the timeout path is genuinely broken. A change that makes it pass by loosening a bound until nothing can fail is the can't-fail-probe failure (mem#704) — record a negative control showing the assertion still catches a real regression.	Unverifiable	The PR description includes run output for a negative control (dead port) showing Received: "Error: Unable to connect…", but this evidence is not part of the repository diff. No code changes in-repo simulate or assert the negative-control run beyond the deterministic unit for timeoutVerdict.
The real socket and the stalled server are PRESERVED. The suite's docblock states the design reason: "These deliberately do NOT mock fetch: … a mocked rejection would prove nothing …" Constructing the error to make the assertion deterministic is explicitly out.	Met	Real-socket tests remain intact: startStalledServer() is still called in AT1 at :416 and AT3 at :488; only the assertion changed to use timeoutVerdict. No fetch mocking or timing alteration was introduced; the deterministic test (:418-452) is scoped to timeoutVerdict only and does not replace AT1/AT3.
Verified under the condition that produced the failure, not only in isolation: bun scripts/run-tests-gated.ts passes with it.	Unverifiable	Pass/fail evidence for the full gated run is provided in the PR description prose, not as an artifact in the diff. The repository changes are test-only and do not include CI logs or recorded outputs.
If a later failing run shows the cause IS the runner's per-test budget rather than anything in this test, say so and hand off to mt#4321 rather than patching here.	N/A	This is a future conditional handoff requirement; no code change in this PR could satisfy or violate it. No such later failing run is part of this diff.

Documentation impact

• no-update-needed — Test-only change in packages/domain/src/ai/embedding-service-openai.test.ts adding a local helper and adjusting assertions. No user-facing behavior, APIs, or commands changed; no docs reference this test. No documentation additions or updates are required.