feat(cloudflare): Instrument Flagship bindings in instrumentEnv#21244
feat(cloudflare): Instrument Flagship bindings in instrumentEnv#21244nehaprasad-dev wants to merge 2 commits into
Conversation
nehaprasad-dev
requested review from
JPeer264 and
andreiborza
and removed request for
a team
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 11bed6e. Configure here.
| const flagKey = args[0]; | ||
| if (typeof flagKey === 'string') { | ||
| recordFlagEvaluation(flagKey, result); | ||
| } |
There was a problem hiding this comment.
Missing boolean check before recording flag evaluations
High Severity
recordFlagEvaluation is called for all evaluation results without checking if the value is boolean. For the non-Details branch, a typeof result === 'boolean' check is missing before calling recordFlagEvaluation. For the Details branch, a typeof result.value === 'boolean' check is missing. The tests clearly expect only boolean values to be recorded (e.g., getStringValue and getStringDetails should NOT trigger recording), but the current code calls _INTERNAL_insertFlagToScope and _INTERNAL_addFeatureFlagToActiveSpan unconditionally — causing the test spies to register calls that the assertions expect not to happen.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 11bed6e. Configure here.
|
|
||
| expect(wrapped.appId).toBe('app-123'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Missing integration or E2E test for feat PR
Low Severity
Per the review rules, feat PRs require at least one integration or E2E test. This PR only includes a unit test for instrumentFlagship. No integration test exercises the full flow through instrumentEnv detecting a Flagship binding and producing the expected telemetry, and no E2E test exists in dev-packages.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 11bed6e. Configure here.
1 participant
Before submitting a pull request, please take a look at our Contributing guidelines and verify:
fix : #21184