fix: route tinfoil sends to the assigned enclave and cut GLM latency - #1170
Conversation
Semgrep Security ScanNo security issues found. |
|
Preview environment destroyed 🧹 Stack |
PR Metrics
Updated Fri, 31 Jul 2026 21:22:46 GMT · run #2539 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔭 thunder-deep-review (advisory)
Complements the other bots — surfaces only what they did not flag. Never approves, never requests changes, never gates merge.
head: 26e1d0f8f6a6 · mode: deep · deferred 0 item(s) already reported by other bots (best-effort dedup)
There was a problem hiding this comment.
🔭 thunder-deep-review (advisory)
Complements the other bots — surfaces only what they did not flag. Never approves, never requests changes, never gates merge.
head: b63bb1dd39e1 · mode: deep · deferred 0 item(s) already reported by other bots (best-effort dedup)
There was a problem hiding this comment.
🔭 thunder-deep-review (advisory)
Complements the other bots — surfaces only what they did not flag. Never approves, never requests changes, never gates merge.
head: b9bb61987575 · mode: deep · deferred 0 item(s) already reported by other bots (best-effort dedup)
There was a problem hiding this comment.
🔭 thunder-deep-review (advisory)
Complements the other bots — surfaces only what they did not flag. Never approves, never requests changes, never gates merge.
head: 9df015754cf3 · mode: deep · deferred 0 item(s) already reported by other bots (best-effort dedup)
ital0
changed the title
There was a problem hiding this comment.
🔭 thunder-deep-review (advisory)
Complements the other bots — surfaces only what they did not flag. Never approves, never requests changes, never gates merge.
head: b4b5cc82385f · mode: deep · deferred 0 item(s) already reported by other bots (best-effort dedup)
There was a problem hiding this comment.
🔭 thunder-deep-review (advisory)
Complements the other bots — surfaces only what they did not flag. Never approves, never requests changes, never gates merge.
head: f2c56f53890e · mode: deep · deferred 0 item(s) already reported by other bots (best-effort dedup)
There was a problem hiding this comment.
🔭 thunder-deep-review (advisory)
Complements the other bots — surfaces only what they did not flag. Never approves, never requests changes, never gates merge.
head: 1a853b887aeb · mode: deep · deferred 0 item(s) already reported by other bots (best-effort dedup)
There was a problem hiding this comment.
🔭 thunder-deep-review (advisory)
Complements the other bots — surfaces only what they did not flag. Never approves, never requests changes, never gates merge.
head: a9c164565811 · mode: deep · deferred 0 item(s) already reported by other bots (best-effort dedup)
There was a problem hiding this comment.
🔭 thunder-deep-review (advisory)
Complements the other bots — surfaces only what they did not flag. Never approves, never requests changes, never gates merge.
head: 2981c27c6d8b · mode: deep · deferred 0 item(s) already reported by other bots (best-effort dedup)
There was a problem hiding this comment.
🔭 thunder-deep-review (advisory)
Complements the other bots — surfaces only what they did not flag. Never approves, never requests changes, never gates merge.
head: 295d0ff598d0 · mode: deep · deferred 0 item(s) already reported by other bots (best-effort dedup)
| } | ||
|
|
||
| export type InferenceLogger = { | ||
| info: (context: InferenceUpstreamAttemptLog | object, message: string) => void |
There was a problem hiding this comment.
📐 Convention — InferenceUpstreamAttemptLog | object collapses to just object
The | object here swallows the typed member — the whole parameter widens to object, so the log-shape type buys you nothing and a malformed log entry wouldn't be caught. The sibling TinfoilProxyLogger.info is precisely typed, and this logger is actually called with two shapes (InferenceUpstreamAttemptLog and InferenceProxyLatencyLog). Could we type it as InferenceUpstreamAttemptLog | InferenceProxyLatencyLog instead?
| .use( | ||
| createInferenceRoutes({ | ||
| auth, | ||
| fetchFn: deps?.fetchFn, |
There was a problem hiding this comment.
📐 Convention — Inference gets raw deps?.fetchFn while every sibling gets the resolved fetchFn — load-bearing but undocumented
Every other route is handed the resolved fetchFn (deps?.fetchFn ?? globalThis.fetch), but inference is deliberately passed the raw deps?.fetchFn. That's intentional — the client-cache gate does if (client && !fetchFn) return cached, so passing a truthy globalThis.fetch in prod would silently disable client caching and rebuild the client every request. The problem is nothing says so, so a well-meaning refactor "normalizing" this to match the siblings would quietly regress performance. A one-line comment explaining why inference must receive undefined in prod would protect it.
|
|
||
| // Fire-and-forget: the wrapper no-ops (before any dynamic import) unless | ||
| // this is a Tinfoil system model, so the first send finds a warm client. | ||
| void (deps.prewarmSystemModel ?? prewarmSystemModel)(model) |
There was a problem hiding this comment.
📐 Convention — Fire-and-forget prewarm has no .catch, so a chunk-load failure becomes an unhandled rejection
I traced the wrapper: runSystemModelPrewarm swallows its own errors internally, so the only way prewarmSystemModel actually rejects is a failed dynamic await import('./tinfoil-client') — a real event after a deploy invalidates old chunks. This void (...)(model) attaches no handler, so that path surfaces as an unhandled promise rejection. The tell is that the new test has to pre-attach void prewarmFailure.catch(() => undefined) to suppress exactly the rejection production leaves dangling. Could we add .catch((error) => console.error(...)) here, or wrap the import() inside the wrapper so it can never reject?
1 participant
Fixes THU-758
GLM felt slow and unreliable. Live profiling on the preview environment (Firefox, real sends, instrumented end to end) turned that into numbers, and the numbers moved the target twice. This PR ships the fixes that came out of it, plus the instrumentation that made them findable.
The headline fix: requests were going to the wrong enclave
The Tinfoil ATC assigns an enclave per attestation (usually
inference.tinfoil.sh, sometimes a pool node likerouter.inf6.tinfoil.sh), returns that enclave's HPKE key, and the SDK seals the request body for it — telling us where to deliver viaX-Tinfoil-Enclave-Url. Our proxy ignored that header and always forwarded to the fixedTINFOIL_ENCLAVE_URL. When the assignment diverged, the sealed body landed on an enclave whose key didn't match:which cost a full eviction + re-attestation retry (~1.6–2.3s) on the first send. Measured at ~50% of new-chat sends in one run.
The proxy now forwards to the assigned enclave, allowlisted to HTTPS
tinfoil.shhosts (label-boundary check —evil-tinfoil.shandtinfoil.sh.evil.comare rejected), preserving the SSRF posture that motivated the fixed URL, and applies the API path prefix derived from the configured enclave URL so no route is lost. Validated live:router.inf6went 422 → 404 (my first attempt dropped/v1) → 200, and 30 consecutive sends now run without a single key-config failure.Also fixed
502 Bad Gatewayafter idle — the backend closed keep-alive connections at ~30s while the ALB held them for 60s, so the ALB periodically reused a dead socket. BackendidleTimeoutis now 120s (above the ALB's 60s, per AWS's rule) and both Pulumi ALBs pinidleTimeout: 60so the contract can't silently invert. Proven with a raw-TCP probe: server close 31.5s → 119.1s; reuse after 59s idle went from dead socket to OK.Instrumentation (how the above became findable)
pre(guard chain: auth + rate limiter),fetch,headers— in the structured log and in a CORS-exposedX-Proxy-Timingheader plus standardServer-Timing.Timing-Allow-Origin, resolved through the same origin allowlist as CORS, so the browser can decompose cross-origin Resource Timing.What the measurements say now
Median GLM time-to-first-token, warm connection, 10 sends:
Connection reuse works (9/10 sends, handshake only on the first, 54ms). The enclave returns headers in ~172ms — it is not the bottleneck, and neither is the database or the app. The remaining levers are the network leg (terminating closer to users) and the 178ms client-side pre-send; both are follow-up work, and the network figure should be confirmed against production before anyone invests in it.
Verification
bun run checkclean; frontend suite green; backend suite green apart from the known same-process WebSocket/socket-bind flakes.pre=81ms, backend total 413ms). Did not reproduce in 30 subsequent sends; instrumentation is now in place to catch the next one.