Repository navigation
fix: address Cursor plugin public-release blockers - #6
Conversation
LFE-16866 Fix public-release blockers in Cursor observability plugin
MotivationPrepare langfuse/cursor-observability-plugin for a public launch by fixing the five P1 findings from the pre-publication review. Confirmed behaviorUsing the shipped bundle and a localhost receiver with fake credentials: a project-only baseUrl override receives global credentials; captureFileContent=false still uploads Read tool results; captureToolOutput=false still persists raw results locally; configured keys leak through observation status messages; the npm binary has no Node shebang and fails when executed on Unix. The committed bundle also redistributes OpenTelemetry and Langfuse dependencies without their license/notice texts. Scope and intended changes
Keep the committed self-contained dist bundle: Cursor's Git installation requires it. Update documentation and regenerate dist. Validation and rolloutAdd focused regression tests and local OTLP receiver checks; run formatting, typecheck, build, full tests, reproducible-bundle checks, and packed-package smoke tests. Publish one draft PR; do not merge, release a package, or change repository visibility. Non-goalsThe review's P2 findings (retry/persistence, duplicate stops, attached-trace IDs, concurrency, setup safety and file permissions) remain separate follow-ups. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c01fb88284
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const chunk of Object.values(bundle)) { | ||
| if (chunk.type !== "chunk") continue; | ||
| for (const id of Object.keys(chunk.modules)) { | ||
| if (!id.includes(`${path.sep}node_modules${path.sep}`) || !fs.existsSync(id)) continue; |
There was a problem hiding this comment.
Accept normalized module IDs on Windows
When pnpm build or prepack runs on Windows, Rolldown supplies normalized module IDs with / separators, while path.sep is \. This check therefore skips every bundled dependency, leaves entries empty, and later fails the build with No bundled dependency licenses found. Match both separator styles (or normalize id) so Windows contributors can rebuild and package the plugin.
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Fixes the five P1 findings blocking the Cursor plugin's public launch:
Keeps the committed, self-contained
dist/installation and regenerates it. P2 findings from the review are outside this PR.Validation: formatting and typecheck pass; all 79 tests pass, including localhost OTLP capture/redaction regressions and a packed-binary smoke test. A real npm installation into a temporary prefix executes successfully. Rebuilding produces no
dist/changes.Fixes https://linear.app/clickhouse/issue/LFE-16866/fix-public-release-blockers-in-cursor-observability-plugin