test(cloudflare): Add e2e test app for @cloudflare/think - #24660
Merged
Merged
Conversation
Contributor
size-limit report 📦
|
RulaKhaled
marked this pull request as ready for review
September 24, 2026 08:51
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Comment on lines
+59
to
+60
| execute: async (_input: { reason: string }): Promise<string> => { | ||
| throw new Error('Think tool failed on purpose'); |
Contributor
There was a problem hiding this comment.
Bug: The runTurn method can return a result where continuation is undefined due to a race condition. This value is used without a check, leading to an incorrect API response.
Severity: MEDIUM
Suggested Fix
Add a check to ensure result and result.continuation are not undefined before returning the response. If result.continuation is missing, consider throwing an error or returning a more explicit error response to the client instead of an empty object. For example: if (!result?.continuation) { throw new Error('Failed to get continuation from agent turn.'); } return Response.json({ continuation: result.continuation });.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: dev-packages/e2e-tests/test-applications/cloudflare-think/src/index.ts#L59-L60
Potential issue: In
`dev-packages/e2e-tests/test-applications/cloudflare-think/src/index.ts`, the
`onRequest` method calls `runTurn` and then directly accesses `result.continuation` to
build the JSON response. Due to a known race condition in the `@cloudflare/think`
library, concurrent agent turns can cause `runTurn` to resolve with a result where the
`continuation` property is `undefined`. When this happens, `Response.json({
continuation: undefined })` serializes to an empty object `{}`, breaking the API
contract and causing silent failures for clients that expect the `continuation` field.
This race condition is reproducible in the e2e test environment.
RulaKhaled
added a commit
that referenced
this pull request
Sep 25, 2026
…ort (#24700) The Vite transform freed an exported class's binding for the wrapper by renaming the declaration to `__SENTRY_ORIGINAL_<Name>__`. That also renames `Function.prototype.name`, and libraries read it: `@cloudflare/think` passes `this.constructor.name` to the AI SDK as the `functionId`, so a zero-config agent span came out as `invoke_agent __SENTRY_ORIGINAL_ThinkAgent__`, and `agents` puts the same string in `gen_ai.agent.name`. Only the export has to be the wrapper, so the declaration now stays exactly as written and the export is aliased to a wrapper binding. That is the shape the transform already emits for classes re-exported from another module and the shape the docs show for manual wrapping, so local and cross-module classes now share one code path. A default export of an already-wrapped class is re-pointed at that binding. The one observable change is that in-module references to the class see the original rather than the wrapper. That matches manual wrapping, and Durable Objects, Agents, Workflows and entrypoints are constructed by the runtime through the export. Verified against a real worker with the `cloudflare-autoinstrument` e2e app, where `gen_ai.agent.name` is now an exact match. The Think span-name assertions land with #24660. Fixes #24696 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Records what a Think agent produces with the current SDK, before any Think-specific code exists. The worker imports no Sentry API at all: `sentryCloudflareVitePlugin()` detects `class ThinkAgent extends Think` and wraps the export at build time, and `ai` publishes the telemetry `vercelAIIntegration` already consumes. Covers the same five things the other AI apps do: the gen_ai hierarchy, a tool throw becoming an issue with an errored span, a manual span nesting under the tool span, the provider request staying inside the turn, and `dataloader` proving the orchestrion transform reaches a bundled worker. Runs on `ai` v7 by default with a v6 lane as an optional variant, because Think's peer range spans both and the two produce different trace shapes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`getPlaywrightConfig` already defaults to `workers: 1` and a 30s timeout. The app overrode both, so five tests each driving a live OpenRouter turn ran in parallel under a cap meant for mocked models. That came from copying `cloudflare-vercelai-v7`, which mocks its model; the two apps that call a real provider, `cloudflare-mastra` and `node-flue`, keep the serial default and raise the timeout to 90s instead. Also corrects the comment above the first test, which still claimed the worker imports no Sentry API. That stopped being true when the manual-span test was added, and the worker now imports `startSpan` for exactly that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…name Depends on #24700, which stops the Vite transform from renaming the wrapped class. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
RulaKhaled
force-pushed
the
feat/cloudflare-think-e2e
branch
from
September 25, 2026 08:57
e52780f to
9cdc5b9
Compare
JPeer264
approved these changes
Sep 28, 2026
Comment on lines
+1
to
+8
| import type { CloudflareOptions } from '@sentry/cloudflare'; | ||
|
|
||
| export default (env: Env): CloudflareOptions => ({ | ||
| dsn: env.E2E_TEST_DSN, | ||
| environment: 'qa', | ||
| tunnel: 'http://localhost:3031/', | ||
| tracesSampleRate: 1.0, | ||
| }); |
Member
There was a problem hiding this comment.
l: We should use defineCloudflareOptions instead
Suggested change
| import type { CloudflareOptions } from '@sentry/cloudflare'; | |
| export default (env: Env): CloudflareOptions => ({ | |
| dsn: env.E2E_TEST_DSN, | |
| environment: 'qa', | |
| tunnel: 'http://localhost:3031/', | |
| tracesSampleRate: 1.0, | |
| }); | |
| import { defineCloudflareOptions } from '@sentry/cloudflare'; | |
| export default defineCloudflareOptions((env) => ({ | |
| dsn: env.E2E_TEST_DSN, | |
| environment: 'qa', | |
| tunnel: 'http://localhost:3031/', | |
| tracesSampleRate: 1.0, | |
| })); |
| * into `instrumentAgentWithSentry(...)` at build time. Wrapping it here would prove nothing about | ||
| * the zero-config path. | ||
| */ | ||
| export class ThinkAgent extends Think<Env> { |
Member
There was a problem hiding this comment.
m: Since auto wrapping is super tough within Vite it would be better if this class would be defined in another file and then re-exported. So in the future we'd know if this would break.
export { ThinkAgent } from './think-agent.ts';| "forceConsistentCasingInFileNames": true, | ||
| "strict": true, | ||
| "skipLibCheck": true, | ||
| "types": ["@cloudflare/workers-types/experimental"] |
Member
There was a problem hiding this comment.
l:
Suggested change
| "types": ["@cloudflare/workers-types/experimental"] | |
| "types": ["@cloudflare/workers-types"] |
Review follow-ups on #24660. The agent class now lives in `src/think-agent.ts` and the worker entry only re-exports it. That is the harder case for the Vite plugin: the entry never binds the class, so detection has to follow the re-export and the wrapper has to import the class under a private name. The built worker still wraps it with `instrumentAgentWithSentry` and keeps the class name, so the `invoke_agent ThinkAgent` assertion holds. Also use `defineCloudflareOptions` in `instrument.server.ts`, which is the documented shape for the plugin's options module, and switch `tsconfig.json` to the stable `@cloudflare/workers-types` entry. Suite is green on both the default lane and the `ai v6` variant. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
RulaKhaled
enabled auto-merge (squash)
September 28, 2026 10:01
RulaKhaled
disabled auto-merge
September 28, 2026 10:33
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5994ebc. Configure here.
| const invokeAgent = spans.find(span => getSpanOp(span) === 'gen_ai.invoke_agent'); | ||
| const parent = spans.find(span => span.span_id === providerRequest?.parent_span_id); | ||
|
|
||
| expect(providerRequest?.trace_id).toBe(invokeAgent?.trace_id); |
There was a problem hiding this comment.
Test drops late invoke_agent span
Medium Severity
The provider HTTP test returns from collectStreamedSpans once generate_content and the OpenRouter http.client span exist, then reads invoke_agent for the shared-trace check and, on the v6 lane, as the HTTP parent. That agent span is the parent that stays open and flushes in a later envelope, so it is often still missing when the predicate succeeds and those assertions fail.
Reviewed by Cursor Bugbot for commit 5994ebc. Configure here.
JPeer264
approved these changes
Sep 28, 2026
47 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
cloudflare-thinke2e app that records what a Think agent produces with the SDK as it stands today, before any Think-specific code exists. Same shape asnode-eveandcloudflare-mastra.The setup is the whole point: the worker imports no Sentry API at all.
sentryCloudflareVitePlugin()detectsextends Thinkand wraps the export at build time, andaipublishes the telemetryvercelAIIntegrationalready consumes. Options come fromsrc/instrument.server.tsby convention.What works today
Five checks, the same set the other AI apps cover:
gen_ai.invoke_agent, onegen_ai.generate_contentper step, andgen_ai.execute_tool, correctly nested and all with originauto.vercelai.channel. Token usage is right per step and rolls up onto the agent span; prompts, tool arguments and results, tool definitions and finish reasons are all present.auto.vercelai.channeland marks only its own span errored, leaving the model callsok.Sentry.startSpanraised inside a tool nests under that tool's span, so Think runs tools inside the async context the SDK opened.dataloaderspans land in the same trace, so the orchestrion module transform reaches a bundled Think worker with no--importbootstrap, unlike the Node apps.What does not
The provider HTTP call nests under
gen_ai.generate_contentonaiv7 but is a sibling of it on v4 to v6. That difference is ours: only v7's native channel binds the model-call span into async context. Asserted per lane so the gap is recorded rather than hidden.The agent span is named from
this.constructor.name, which the Vite plugin's class rename currently mangles intoinvoke_agent __SENTRY_ORIGINAL_ThinkAgent__. Not asserted here. A follow-up fixes it and adds the assertion.There is no
gen_ai.agent.name, and none of thecloudflare.agents.turn.*context Think supplies (agent id, Durable Object conversation id, turn trigger, continuation) reaches a span. Think sends all of it, asruntimeContexton v7 andexperimental_telemetry.metadataon v6, and neither subscriber reads either field.Notes
sentryTest.optional: truekeeps this in the optional matrix likenode-eve, since it makes real OpenRouter calls. The v6 lane rides along as anoptionalVariantsentry rather than a second app.E2E_OPENROUTER_API_KEYis already wired inbuild.ymlfrom the eve PR, so CI needs no change.ref JS-3568
🤖 Generated with Claude Code