Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,25 @@ jobs:

- run: bun run build

- name: Upload sourcemaps to Sentry
if: startsWith(github.ref, 'refs/tags/v')
env:
SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }}
SENTRY_ORG: pickforge
SENTRY_PROJECT: picklab
run: |
if [ -z "$SENTRY_AUTH_TOKEN" ]; then
echo "SENTRY_AUTH_TOKEN not set; skipping"
exit 0
fi
VERSION=$(node -p "require('./packages/cli/package.json').version")
if ! {
npx --yes @sentry/cli sourcemaps inject packages/cli/dist &&
npx --yes @sentry/cli sourcemaps upload --release "picklab@$VERSION" packages/cli/dist
}; then
echo "::warning::Sentry sourcemap upload failed; continuing release"
fi

- name: Publish @pickforge/picklab
if: startsWith(github.ref, 'refs/tags/v')
working-directory: packages/cli
Expand Down
2 changes: 2 additions & 0 deletions INSTALL.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ command -v picklab && picklab --version

If `picklab` is not on PATH, the global bin dir (`npm prefix -g`/bin or `~/.bun/bin`) is missing from PATH — fix the user's shell profile or tell them, don't fall back to a local install.

On fatal errors the CLI and MCP server report the error message and stack trace — the message can reference the failing command and its output, with secrets redacted — plus OS, Node.js, and app versions to Sentry; nothing else is collected. Tell the user they can disable this with `PICKLAB_TELEMETRY=0`.

## 2. Register the MCP server with the agent the user uses

That is probably you. Built-in support:
Expand Down
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ Each session gets its own isolated display or emulator, so several agents and pr
<img src="https://raw.githubusercontent.com/pickforge/picklab/main/assets/brand/picklab-run-lab-mock.svg" alt="PICKLAB · RUN LAB — desktop session, Android emulator, live screenshots, logs, and agent terminal" width="900">
</p>

## Telemetry

When the `picklab` CLI or `picklab-mcp` server hits a fatal error, it reports the error message and stack trace — the message can reference the failing command and its output, with secrets redacted — plus OS, Node.js, and app versions to Sentry so we can fix it. Nothing else is collected. Disable with `PICKLAB_TELEMETRY=0`.

## MCP setup for agents

Register the MCP server with your coding agent:
Expand Down
59 changes: 59 additions & 0 deletions bun.lock

Large diffs are not rendered by default.

10 changes: 6 additions & 4 deletions docs/releases/UNRELEASED.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,25 @@ then reset this file.

## User-facing changes

- None yet.
- Fatal errors from the `picklab` CLI and `picklab-mcp` server are now reported to Sentry (error message, stack trace, OS and app version only; breadcrumbs dropped, hostname stripped, messages run through `redactSecrets`). Opt out with `PICKLAB_TELEMETRY=0`; documented in README and INSTALL.

## Internal/release changes

- Added repo-local release tracking in `docs/releases/UNRELEASED.md`.
- CLI bundles now emit sourcemaps; the release workflow injects debug IDs and uploads sourcemaps to Sentry on tag builds (skips cleanly until `SENTRY_AUTH_TOKEN` secret exists).

## Validation

### Tested

- Reviewed the release tracking docs.
- `bun run typecheck`, `bun run test` (556 passed), `bun run build`.
- MCP stdio smoke: initialize handshake over the built `picklab-mcp` with telemetry enabled — stdout is pure JSON-RPC.

### Not tested yet

- CLI build.
- MCP server smoke test.
- npm publish flow.
- Sentry event delivery end-to-end (needs a real fatal in a released build).
- npm publish flow with the new sourcemap upload step.

### Release blockers

Expand Down
1 change: 1 addition & 0 deletions packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
},
"dependencies": {
"@modelcontextprotocol/sdk": "^1.17.0",
"@sentry/node": "^10.64.0",
"commander": "^14.0.0",
"zod": "^4.0.0"
},
Expand Down
15 changes: 13 additions & 2 deletions packages/cli/src/picklab-mcp.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
#!/usr/bin/env node
import { runMcpServe } from "./commands/mcp.js";
import { captureFatal, initTelemetry } from "./telemetry.js";

process.exitCode = await runMcpServe();
initTelemetry();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Initialize telemetry before loading server modules

Because ESM static imports are evaluated before the module body, any startup failure while loading ./commands/mcp.js or its MCP-server dependencies happens before this initTelemetry() call and before the surrounding try/catch exists. That leaves dependency/load-time startup failures unreported despite this change aiming to capture fatal MCP startup errors; move the server import behind telemetry initialization with a dynamic import or a tiny bootstrap.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9a392cb: both bins now dynamic-import their heavy entry module after initTelemetry() and inside the try block, so module-load-time failures are captured. Side effect worth noting: the no-sudo bundle scanner only followed static import specifiers, so it was extended to trace dynamic import("...") chunks — coverage of the MCP chunk graph is restored (verified by the control test failing before the scanner fix).


try {
const { runMcpServe } = await import("./commands/mcp.js");
process.exitCode = await runMcpServe();
} catch (error) {
await captureFatal(error);
console.error(
`error: ${error instanceof Error ? error.message : String(error)}`,
);
process.exitCode = 1;
}
10 changes: 8 additions & 2 deletions packages/cli/src/picklab.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
#!/usr/bin/env node
import { buildProgram } from "./program.js";
import { captureFatal, initTelemetry } from "./telemetry.js";

initTelemetry();

try {
const { buildProgram } = await import("./program.js");
await buildProgram().parseAsync();
} catch (error) {
console.error(`error: ${(error as Error).message}`);
await captureFatal(error);
console.error(
`error: ${error instanceof Error ? error.message : String(error)}`,
);
process.exitCode = 1;
}
74 changes: 74 additions & 0 deletions packages/cli/src/telemetry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { createRequire } from "node:module";
import * as Sentry from "@sentry/node";
import type { ErrorEvent } from "@sentry/node";
import { redactSecrets } from "@pickforge/picklab-core";

export type EnvLike = Record<string, string | undefined>;

const DSN =
"https://25cc6307aeca0d1d454e0af21bee5498@o4511699702317056.ingest.us.sentry.io/4511699813990400";

const DISABLE_VALUES = new Set(["0", "false", "off"]);

export function telemetryEnabled(env: EnvLike = process.env): boolean {
const value = env.PICKLAB_TELEMETRY?.trim();
if (value === undefined || value === "") {
return true;
}
return !DISABLE_VALUES.has(value.toLowerCase());
}

export function initTelemetry(env: EnvLike = process.env): void {
if (!telemetryEnabled(env)) {
return;
}
const require = createRequire(import.meta.url);
const { version } = require("../package.json") as { version: string };
Sentry.init({

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Disable Sentry's default context collectors

With the bare Sentry.init call here, the Node SDK keeps its default integrations; Sentry's own integrations table documents contextLinesIntegration as auto-enabled and adding source code to stack frames, and localVariablesIntegration as default for adding local variables (see the Node integrations docs). In fatal CLI paths this can attach source snippets and in-scope values such as parsed args/options even if the error message is sanitized, violating the new README/INSTALL promise that no file contents, commands, or arguments are collected. Filter these integrations out or explicitly turn stack-frame context/variables off.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9a392cb: includeLocalVariables: false and ContextLines filtered out of the default integrations. (localVariablesIntegration is opt-in via includeLocalVariables, but it's now explicitly off as a belt-and-braces measure.)

dsn: DSN,
release: `picklab@${version}`,
tracesSampleRate: 0,
defaultIntegrations: false,
integrations: [
Sentry.inboundFiltersIntegration(),
Sentry.functionToStringIntegration(),
Sentry.linkedErrorsIntegration(),
Sentry.dedupeIntegration(),
Sentry.onUncaughtExceptionIntegration(),
Sentry.onUnhandledRejectionIntegration({ mode: "strict" }),
Sentry.nodeContextIntegration(),
],
beforeBreadcrumb: dropBreadcrumb,
beforeSend: scrubEvent,
});
}

export function dropBreadcrumb(): null {
return null;
}

export function scrubEvent(event: ErrorEvent): ErrorEvent {
delete event.server_name;
delete event.modules;
if (event.contexts) {
for (const key of Object.keys(event.contexts)) {
if (key !== "os" && key !== "runtime") {
delete event.contexts[key];
}
}
}
if (typeof event.message === "string") {
event.message = redactSecrets(event.message);
}
for (const exception of event.exception?.values ?? []) {
if (exception.value !== undefined) {
exception.value = redactSecrets(exception.value);
}
}
return event;
}

export async function captureFatal(err: unknown): Promise<void> {
Sentry.captureException(err);
await Sentry.flush(2000);
}
111 changes: 111 additions & 0 deletions packages/cli/test/telemetry.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import { describe, expect, it } from "vitest";
import * as Sentry from "@sentry/node";
import type { ErrorEvent } from "@sentry/node";
import {
captureFatal,
dropBreadcrumb,
initTelemetry,
scrubEvent,
telemetryEnabled,
} from "../src/telemetry.js";

describe("telemetryEnabled", () => {
it("defaults to on when unset", () => {
expect(telemetryEnabled({})).toBe(true);
});

it("defaults to on when trimmed-empty", () => {
expect(telemetryEnabled({ PICKLAB_TELEMETRY: " " })).toBe(true);
});

it("defaults to on for unrelated values", () => {
expect(telemetryEnabled({ PICKLAB_TELEMETRY: "yes" })).toBe(true);
});

it("is off for '0'", () => {
expect(telemetryEnabled({ PICKLAB_TELEMETRY: "0" })).toBe(false);
});

it("is off for 'false'", () => {
expect(telemetryEnabled({ PICKLAB_TELEMETRY: "false" })).toBe(false);
});

it("is off for 'OFF' case-insensitively", () => {
expect(telemetryEnabled({ PICKLAB_TELEMETRY: "OFF" })).toBe(false);
});

it("is off for ' off ' with surrounding whitespace", () => {
expect(telemetryEnabled({ PICKLAB_TELEMETRY: " off " })).toBe(false);
});
});

describe("initTelemetry", () => {
it("does not initialize Sentry when opted out", () => {
initTelemetry({ PICKLAB_TELEMETRY: "0" });
expect(Sentry.isInitialized()).toBe(false);
});
});

describe("scrubEvent", () => {
it("deletes server_name and modules", () => {
const event: ErrorEvent = {
type: undefined,
server_name: "my-hostname",
modules: { commander: "14.0.0" },
};
const scrubbed = scrubEvent(event);
expect(scrubbed.server_name).toBeUndefined();
expect(scrubbed.modules).toBeUndefined();
});

it("prunes contexts down to os and runtime", () => {
const event: ErrorEvent = {
type: undefined,
contexts: {
os: { name: "linux" },
runtime: { name: "node", version: "v20" },
device: { arch: "x64" },
app: { app_start_time: "now" },
culture: { locale: "en-US" },
trace: { trace_id: "abc", span_id: "def" },
},
};
const scrubbed = scrubEvent(event);
expect(Object.keys(scrubbed.contexts ?? {}).sort()).toEqual([
"os",
"runtime",
]);
});

it("redacts secrets in the message and exception values", () => {
const event: ErrorEvent = {
type: undefined,
message: "failed with token=ghp_0123456789012345678901234567890abcde",
exception: {
values: [
{ type: "Error", value: "adb failed: API_KEY=super-secret-value" },
{ type: "Error" },
],
},
};
const scrubbed = scrubEvent(event);
expect(scrubbed.message).not.toContain("ghp_");
expect(scrubbed.exception?.values?.[0].value).toBe(
"adb failed: API_KEY=[REDACTED]",
);
expect(scrubbed.exception?.values?.[1].value).toBeUndefined();
});
});

describe("dropBreadcrumb", () => {
it("always returns null", () => {
expect(dropBreadcrumb()).toBeNull();
});
});

describe("captureFatal", () => {
it("resolves without throwing when Sentry is uninitialized", async () => {
expect(Sentry.isInitialized()).toBe(false);
await expect(captureFatal(new Error("boom"))).resolves.toBeUndefined();
});
});
1 change: 1 addition & 0 deletions packages/cli/tsup.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ export default defineConfig({
format: ["esm"],
platform: "node",
clean: true,
sourcemap: true,
noExternal: [/^@pickforge\//],
});
2 changes: 1 addition & 1 deletion test/security/security-no-sudo.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ describe("bundle: built picklab-mcp entrypoint", () => {
const content = fs.readFileSync(path.join(distDir, name), "utf8");
contents.set(name, content);
for (const match of content.matchAll(
/\bfrom\s*["'](\.\.?\/[^"']+)["']/g,
/(?:\bfrom\s*|\bimport\s*\(\s*)["'](\.\.?\/[^"']+)["']/g,
)) {
queue.push(path.normalize(path.join(path.dirname(name), match[1])));
}
Expand Down
Loading