Skip to content

Feat/privacy safe telemetry - #700

Merged
chrisdoc merged 10 commits into
mainfrom
feat/privacy-safe-telemetry
Jul 21, 2026
Merged

Feat/privacy safe telemetry#700
chrisdoc merged 10 commits into
mainfrom
feat/privacy-safe-telemetry

Conversation

@chrisdoc

@chrisdoc chrisdoc commented Jul 20, 2026

Copy link
Copy Markdown
Owner

Add privacy-safe MCP telemetry taxonomy, bounded tool outcomes, session compatibility signals, and dashboard guidance.

Primary changes

  • Adds a fixed Hevy/MCP telemetry taxonomy for feature, read/write kind, and operation dimensions.
  • Records bounded tool result-shape, workflow, API retry/error, and MCP session lifecycle signals.
  • Sanitizes MCP/Sentry metadata, disables Sentry MCP input/output capture, and flushes telemetry on startup failure.
  • Adds a telemetry data dictionary, dashboard guidance, and privacy-focused regression coverage.

Reviewer walkthrough

  • Start with src/utils/tool-taxonomy.ts, src/utils/telemetry-wrapper.ts, and src/utils/response-formatter.ts for taxonomy, bounded arguments/results, and outcome instrumentation.
  • Follow session and transport handling in src/utils/mcp-session-observability.ts, src/utils/stdio-observability.ts, src/index.ts, and src/utils/graceful-shutdown.ts.
  • Review docs/telemetry-data-dictionary.md and docs/telemetry-dashboards.md alongside the focused tests.

Correctness and invariants

  • Telemetry emits only fixed taxonomies, presence flags, bounded buckets, normalized endpoints, and allowlisted error diagnostics; raw MCP content, identifiers, dates, titles, notes, descriptions, and measurements are excluded.
  • Returned MCP errors and thrown tool failures remain distinct outcomes, and session termination is categorized without exposing session identifiers.
  • Sentry MCP input/output capture is explicitly disabled and prohibited MCP metadata is filtered before span export.

Testing and QA

  • Adds or updates unit coverage for tool registration, response/result telemetry, tool-wrapper outcomes, session/client metadata sanitization, stdio observability, graceful shutdown, API retry diagnostics, and Sentry privacy filtering.
  • Documents dashboard publication, retention, access, and privacy-regression checks in docs/telemetry-dashboards.md.

✨ PR Description

Purpose: Implement privacy-safe telemetry system with bounded taxonomy, result metrics, and sanitized client metadata to ensure no sensitive data exposure.

Main changes:

  • Added bounded tool taxonomy (feature/kind/operation), result bucketing, and session lifecycle tracking with MCP client metadata normalization
  • Implemented privacy-focused telemetry wrappers filtering arguments/results to structural presence/buckets while sanitizing Sentry MCP spans and API retry counts
  • Created graceful shutdown callbacks, telemetry flushing, and comprehensive documentation with regression guard tests for privacy compliance

Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Description using Guidelines Learn how

Summary by CodeRabbit

  • Privacy & Security
    • Added privacy-safe telemetry guidance, dashboard rules, and a contractual telemetry data dictionary (bounded dimensions, normalized endpoints, and redaction of sensitive fields).
    • Implemented Sentry span sanitization and stricter safe telemetry for tool arguments, results, and API errors.
  • Reliability
    • Improved telemetry coverage for MCP sessions, tool outcomes, retries, and structured result telemetry.
    • Telemetry is now flushed during shutdown and fatal startup/connect failures.
  • Documentation
    • Added telemetry dashboards guidance, retention/access policy, and publish checklists.
  • Tests
    • Expanded privacy, metrics boundary, retry, MCP session lifecycle, and graceful-shutdown observer coverage.

Refs #697
Refs #694
Refs #696
Refs #698

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jul 20, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Forced-exit awaits onComplete ✓ Resolved 🐞 Bug ☼ Reliability
Description
In installGracefulShutdown, the forced-exit fallback defers process termination until the optional
onComplete promise settles, so a hung completion observer can keep a SIGINT/SIGTERM-terminated
process alive indefinitely. This breaks the forced-exit timeout guarantee and can prevent reliable
termination under failure conditions.
Code

src/utils/graceful-shutdown.ts[R120-128]

  const forcedExitTimer = scheduleForcedExit(() => {
-			processLike.exit(processLike.exitCode ?? 0);
+			const completion = reportCompletion(false);
+			if (completion) {
+				void completion.finally(() =>
+					processLike.exit(processLike.exitCode ?? 0),
+				);
+			} else {
+				processLike.exit(processLike.exitCode ?? 0);
+			}
Relevance

⭐⭐⭐ High

PR #570 added forced-exit specifically to guarantee termination even if shutdown stalls; awaiting
observer breaks guarantee.

PR-#570

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The file documents the forced-exit timeout as a bound, but the new forced-exit callback waits for an
async completion observer to finish before calling exit, which can remove that bound if the observer
never settles. This is the same failure mode that prior graceful-shutdown work aimed to prevent by
ensuring the process terminates even when shutdown work stalls.

src/utils/graceful-shutdown.ts[44-46]
src/utils/graceful-shutdown.ts[120-129]
PR-#570

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`installGracefulShutdown`’s forced-exit timer is meant to *bound* shutdown time, but it now waits for `onComplete` (via `completion.finally(...)`) before calling `processLike.exit(...)`. If `onComplete` returns a promise that never resolves (or resolves very slowly), the process may never exit even after the forced-exit timeout.
### Issue Context
This library hook is generic (`onComplete?: (succeeded: boolean) => void | Promise<void>`), so it must be safe even if a future/custom observer hangs. The code comment explicitly describes the forced-exit behavior as a bound.
### Fix Focus Areas
- src/utils/graceful-shutdown.ts[111-170]
### Suggested fix
In the forced-exit callback, call `processLike.exit(...)` unconditionally when the timer fires. Invoke `reportCompletion(false)` in a best-effort, non-blocking way (or cap it with a very small timeout), but do **not** delay `exit()` on its completion.
Also add/adjust a regression test where `onComplete` returns a never-resolving promise and verify the forced-exit path still calls `process.exit` promptly when the forced-exit timer fires.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Forced-exit awaits onComplete ✓ Resolved 🐞 Bug ☼ Reliability
Description
In installGracefulShutdown, the forced-exit fallback defers process termination until the optional
onComplete promise settles, so a hung completion observer can keep a SIGINT/SIGTERM-terminated
process alive indefinitely. This breaks the forced-exit timeout guarantee and can prevent reliable
termination under failure conditions.
Code

src/utils/graceful-shutdown.ts[R120-128]

  	const forcedExitTimer = scheduleForcedExit(() => {
-			processLike.exit(processLike.exitCode ?? 0);
+			const completion = reportCompletion(false);
+			if (completion) {
+				void completion.finally(() =>
+					processLike.exit(processLike.exitCode ?? 0),
+				);
+			} else {
+				processLike.exit(processLike.exitCode ?? 0);
+			}
Relevance

⭐⭐⭐ High

PR #570 added forced-exit specifically to guarantee termination even if shutdown stalls; awaiting
observer breaks guarantee.

PR-#570

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The file documents the forced-exit timeout as a bound, but the new forced-exit callback waits for an
async completion observer to finish before calling exit, which can remove that bound if the observer
never settles. This is the same failure mode that prior graceful-shutdown work aimed to prevent by
ensuring the process terminates even when shutdown work stalls.

src/utils/graceful-shutdown.ts[44-46]
src/utils/graceful-shutdown.ts[120-129]
PR-#570

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`installGracefulShutdown`’s forced-exit timer is meant to *bound* shutdown time, but it now waits for `onComplete` (via `completion.finally(...)`) before calling `processLike.exit(...)`. If `onComplete` returns a promise that never resolves (or resolves very slowly), the process may never exit even after the forced-exit timeout.
### Issue Context
This library hook is generic (`onComplete?: (succeeded: boolean) => void | Promise<void>`), so it must be safe even if a future/custom observer hangs. The code comment explicitly describes the forced-exit behavior as a bound.
### Fix Focus Areas
- src/utils/graceful-shutdown.ts[111-170]
### Suggested fix
In the forced-exit callback, call `processLike.exit(...)` unconditionally when the timer fires. Invoke `reportCompletion(false)` in a best-effort, non-blocking way (or cap it with a very small timeout), but do **not** delay `exit()` on its completion.
Also add/adjust a regression test where `onComplete` returns a never-resolving promise and verify the forced-exit path still calls `process.exit` promptly when the forced-exit timer fires.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR introduces privacy-safe MCP telemetry with bounded tool taxonomy, sanitized arguments and results, session lifecycle metrics, retry observations, Sentry filtering, telemetry flushing, and dashboard/data-dictionary documentation.

Changes

Privacy-safe telemetry

Layer / File(s) Summary
Tool taxonomy and handler wiring
src/tools/*, src/utils/tool-taxonomy.ts, src/shared-server.ts, src/utils/error-handler.ts
Tool definitions provide bounded metadata, which generic wrappers pass into telemetry.
Bounded tool and result telemetry
src/utils/telemetry-wrapper.ts, src/utils/response-formatter.ts, src/utils/result-telemetry.ts, src/utils/sentry-privacy.ts, src/utils/telemetry.ts, src/utils/metrics.ts
Arguments, outcomes, result shapes, metrics, and Sentry spans use sanitized or bucketed fields.
Session, API, and shutdown lifecycle
src/utils/mcp-session-observability.ts, src/utils/stdio-observability.ts, src/utils/hevyClientKubb.ts, src/utils/hevy-client-observability.ts, src/utils/graceful-shutdown.ts, src/index.ts, src/cli.ts
Session metadata, termination categories, retry counts, normalized API diagnostics, shutdown callbacks, and telemetry flushing are added.
Telemetry contract and dashboard documentation
docs/telemetry-data-dictionary.md, docs/telemetry-dashboards.md, .changeset/privacy-safe-telemetry.md
Approved dimensions, prohibited fields, retention rules, dashboard panels, and publication checks are documented.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MCPClient
  participant ToolRuntime
  participant TelemetryWrapper
  participant Metrics
  participant Sentry
  MCPClient->>ToolRuntime: invoke typed tool
  ToolRuntime->>TelemetryWrapper: pass taxonomy metadata
  TelemetryWrapper->>Metrics: record bounded outcome and duration
  TelemetryWrapper->>Sentry: submit sanitized span
Loading

Possibly related issues

Possibly related PRs

Suggested labels: 30 min review

Poem

A rabbit hops through metrics bright,
Bucketing secrets out of sight.
Sessions start and sessions end,
Safe spans now their rules defend.
Flush the trail when errors call—
Privacy guards watch it all.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 13.16% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly captures the main theme of the PR: privacy-safe telemetry work.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/privacy-safe-telemetry

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

MCP tool token cost

Measured with o200k_base over the complete json-serialized mcp tools/list result payload: { tools }.
Targets are advisory and never fail CI.

Metric Current Target Status
Tools 25 ≤ 20 Above target
Total tokens 12667
Average tokens/tool 506.68 < 600 Within target

Change from baseline

Metric Baseline Current Delta
Tools 25 25 0
Total tokens 12667 12667 0
Average tokens/tool 506.68 506.68 0

Per-tool changes

Tool Baseline Current Delta
create-body-measurement 669 669 0
create-exercise-template 421 421 0
create-routine 564 564 0
create-routine-folder 166 166 0
create-workout 655 655 0
get-body-measurement 628 628 0
get-body-measurements 629 629 0
get-exercise-history 808 808 0
get-exercise-template 276 276 0
get-exercise-templates 308 308 0
get-routine 552 552 0
get-routine-folder 245 245 0
get-routine-folders 270 270 0
get-routines 586 586 0
get-training-summary 923 923 0
get-user-info 244 244 0
get-workout 522 522 0
get-workout-count 195 195 0
get-workout-events 670 670 0
get-workouts 540 540 0
search-exercise-templates 426 426 0
search-routines 465 465 0
update-body-measurement 667 667 0
update-routine 562 562 0
update-workout 672 672 0

Per-tool breakdown

Tool Tokens Share of total
get-training-summary 923 7.29%
get-exercise-history 808 6.38%
update-workout 672 5.31%
get-workout-events 670 5.29%
create-body-measurement 669 5.28%
update-body-measurement 667 5.27%
create-workout 655 5.17%
get-body-measurements 629 4.97%
get-body-measurement 628 4.96%
get-routines 586 4.63%
create-routine 564 4.45%
update-routine 562 4.44%
get-routine 552 4.36%
get-workouts 540 4.26%
get-workout 522 4.12%
search-routines 465 3.67%
search-exercise-templates 426 3.36%
create-exercise-template 421 3.32%
get-exercise-templates 308 2.43%
get-exercise-template 276 2.18%
get-routine-folders 270 2.13%
get-routine-folder 245 1.93%
get-user-info 244 1.93%
get-workout-count 195 1.54%
create-routine-folder 166 1.31%

Per-tool counts encode each complete tool object independently. The total encodes the complete { tools } envelope, so punctuation and separators mean the per-tool values need not sum exactly to the total.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Privacy-safe MCP telemetry: bounded taxonomy, outcomes, sessions, and dashboards

✨ Enhancement 🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Add privacy-reviewed telemetry contract (dictionary + dashboards) with bounded dimensions
• Emit bounded tool taxonomy/outcomes, session lifecycle metrics, and safe API retry diagnostics
• Harden Sentry/OTel export to fail-closed and avoid inspecting user-provided payloads
Diagram

graph TD
  A["Stdio transport"] --> B["Session observability"] --> C["Tool runtime"] --> D["Telemetry wrapper"] --> E["OTel metrics"] --> G{{"Collector/Honeycomb"}}
  D --> F{{"Sentry spans"}}
  H["Response formatter"] --> D
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Rely only on Sentry MCP wrapper (disable inputs/outputs) and skip custom metrics
  • ➕ Less custom code to maintain
  • ➕ Fewer moving parts across tool/runtime/response formatting
  • ➖ Still requires careful span filtering to avoid correlation/metadata leakage
  • ➖ Harder to build stable product metrics without explicit bounded dimensions
  • ➖ Less control over session lifecycle and structured outcome semantics
2. Schema-driven telemetry extraction from tool input/output schemas
  • ➕ Single source of truth for which fields are safe
  • ➕ Potentially reduces manual allowlists over time
  • ➖ Higher implementation complexity (schema traversal + safe-field annotations)
  • ➖ Greater risk of accidental payload inspection if the schema is too permissive
  • ➖ Harder to guarantee bounded cardinality without explicit bucketing rules

Recommendation: Keep the PR’s explicit allowlist + bucketing approach. It fails closed, avoids reading user-authored result fields by requiring opt-in result telemetry attachment, and adds regression tests + documentation as a privacy contract. The added plumbing (taxonomy metadata + session tracking + flush-on-exit) is justified by the privacy and operability requirements.

Files changed (42) +1487 / -308

Enhancement (21) +751 / -141
cli.tsFlush telemetry before fatal exit +7/-1

Flush telemetry before fatal exit

• Updates the CLI entrypoint to attempt a best-effort telemetry flush when server startup fails, without masking the original fatal exit behavior.

src/cli.ts

index.tsDisable Sentry MCP input/output capture and record session termination +23/-2

Disable Sentry MCP input/output capture and record session termination

• Configures Sentry MCP wrapper to not record inputs/outputs. Records session termination categories on connect/startup failures and on graceful shutdown completion, flushing telemetry on exit.

src/index.ts

body-measurements.tsAdd bounded telemetry taxonomy to body-measurement tools +8/-0

Add bounded telemetry taxonomy to body-measurement tools

• Annotates tool definitions with feature and operation metadata so telemetry can emit bounded product taxonomy dimensions.

src/tools/body-measurements.ts

define-tool.tsRequire tool telemetry metadata on definitions and pass into runtime wrapper +8/-3

Require tool telemetry metadata on definitions and pass into runtime wrapper

• Extends ToolDefinition to include bounded feature/operation metadata and forwards feature/kind/operation into the runtime wrapper for telemetry emission.

src/tools/define-tool.ts

folders.tsAdd bounded telemetry taxonomy to folder tools +6/-0

Add bounded telemetry taxonomy to folder tools

• Adds feature/operation metadata to folder tool definitions for bounded metrics/spans.

src/tools/folders.ts

routine-discovery.tsAdd workflow taxonomy metadata to routine discovery tool +2/-0

Add workflow taxonomy metadata to routine discovery tool

• Annotates the bounded search tool with feature/operation for workflow-level metrics.

src/tools/routine-discovery.ts

routines.tsAdd bounded telemetry taxonomy to routine tools +8/-0

Add bounded telemetry taxonomy to routine tools

• Adds feature and operation metadata across list/get/create/update routine tools for bounded telemetry dimensions.

src/tools/routines.ts

templates.tsAdd bounded telemetry taxonomy to template tools +10/-0

Add bounded telemetry taxonomy to template tools

• Annotates exercise template tools with bounded feature/operation metadata to support allowlisted telemetry.

src/tools/templates.ts

user.tsAdd bounded telemetry taxonomy to user info tool +2/-0

Add bounded telemetry taxonomy to user info tool

• Adds feature/operation metadata to the profile tool definition.

src/tools/user.ts

workflows.tsAdd bounded telemetry taxonomy to training summary workflow tool +2/-0

Add bounded telemetry taxonomy to training summary workflow tool

• Annotates workflow tool definition with feature/operation metadata for bounded telemetry.

src/tools/workflows.ts

workouts.tsAdd bounded telemetry taxonomy to workout tools +12/-0

Add bounded telemetry taxonomy to workout tools

• Adds feature and operation metadata across workout list/get/count/sync/create/update tools to support bounded telemetry dimensions.

src/tools/workouts.ts

graceful-shutdown.tsSupport async onComplete hook and report shutdown outcome once +29/-1

Support async onComplete hook and report shutdown outcome once

• Adds an optional onComplete callback (sync or async) that is invoked exactly once on shutdown completion or forced-exit fallback, with errors logged but not thrown.

src/utils/graceful-shutdown.ts

hevy-client-observability.tsRecord retry bucket and normalized error diagnostics for API telemetry +19/-2

Record retry bucket and normalized error diagnostics for API telemetry

• Adds retryCount bucketing to API spans/metrics and propagates only normalized error category/code attributes derived from createSafeErrorDiagnostic.

src/utils/hevy-client-observability.ts

mcp-session-observability.tsAdd session lifecycle tracking with bounded client metadata +156/-0

Add session lifecycle tracking with bounded client metadata

• Implements sanitized client/protocol extraction, tracks session duration/tool-call buckets, and emits started/ended metrics with bounded termination categories.

src/utils/mcp-session-observability.ts

metrics.tsAdd tool outcome and session counters +16/-2

Add tool outcome and session counters

• Introduces mcp.tool.outcomes plus session started/ended counters, and clarifies that tool errors represent thrown errors (not returned MCP errors).

src/utils/metrics.ts

observability-wrapper.tsPass taxonomy metadata through and bucket error context +7/-6

Pass taxonomy metadata through and bucket error context

• Extends withObservability to accept tool taxonomy metadata and forwards it into withTelemetry. Buckets argumentKeyCount for privacy-safe error context.

src/utils/observability-wrapper.ts

response-formatter.tsAttach explicit result-shape telemetry to tool responses +126/-7

Attach explicit result-shape telemetry to tool responses

• Extends response contracts to optionally attach bounded result-shape/workflow telemetry via a WeakMap. Adds bucketed telemetry for common responses and allowlisted workflow name mapping.

src/utils/response-formatter.ts

result-telemetry.tsAdd bounded result telemetry types and WeakMap attachment +40/-0

Add bounded result telemetry types and WeakMap attachment

• Defines count buckets and result telemetry structures, with attach/get helpers that avoid modifying response payloads or inspecting content.

src/utils/result-telemetry.ts

telemetry-wrapper.tsEmit privacy-safe tool telemetry (taxonomy, outcomes, args, results, client dims) +217/-116

Emit privacy-safe tool telemetry (taxonomy, outcomes, args, results, client dims)

• Replaces value-capturing arg telemetry with bounded presence/buckets, adds explicit outcome tracking (success/returned_error/thrown_error), includes session client metadata, and reads only attached result telemetry (never response text). Adds a dedicated mcp.tool.outcomes counter and expands duration metric dimensions safely.

src/utils/telemetry-wrapper.ts

telemetry.tsAdd Sentry span sanitizer and flushTelemetry helper +21/-1

Add Sentry span sanitizer and flushTelemetry helper

• Installs beforeSendSpan to sanitize MCP fields, persists the meter provider for later flushing, and adds flushTelemetry to force-flush traces/metrics and Sentry with a timeout.

src/utils/telemetry.ts

tool-taxonomy.tsDefine bounded tool taxonomy types (feature/kind/operation) +32/-0

Define bounded tool taxonomy types (feature/kind/operation)

• Introduces finite allowlists and TypeScript unions for tool feature, kind, and operation metadata used across tool definitions and telemetry.

src/utils/tool-taxonomy.ts

Bug fix (4) +77 / -18
error-handler.tsPreserve handler parameter types in error wrapper +5/-9

Preserve handler parameter types in error wrapper

• Adjusts withErrorHandling to return a handler typed as the original parameter type and normalizes nullish args internally for safer key-count calculation.

src/utils/error-handler.ts

hevyClientKubb.tsInclude retryCount in request observations and mark exhausted retries earlier +10/-5

Include retryCount in request observations and mark exhausted retries earlier

• Extends HevyRequestObservation with retryCount and ensures exhausted retry marking happens before emitting request-complete observations. Uses the marker consistently to decide when to throw.

src/utils/hevyClientKubb.ts

sentry-privacy.tsFilter prohibited MCP attributes from Sentry spans +39/-0

Filter prohibited MCP attributes from Sentry spans

• Adds a sanitizer that removes correlation identifiers and unbounded client/protocol fields from span data before export.

src/utils/sentry-privacy.ts

stdio-observability.tsAllowlist MCP methods and record session start on initialize +23/-4

Allowlist MCP methods and record session start on initialize

• Restricts mcp.method capture to a safe allowlist and records session start metadata only from initialize messages. Sets bounded client/protocol span attributes without capturing other params.

src/utils/stdio-observability.ts

Refactor (3) +28 / -12
shared-server.tsGeneralize tool handler wrapper type and default wrapper +8/-5

Generalize tool handler wrapper type and default wrapper

• Replaces a fixed error-handler wrapper type with a generic ToolHandlerWrapper and a default wrapper implementation. Aligns shared server options with the new wrapper contract.

src/shared-server.ts

register.tsExport hevyToolDefinitions for taxonomy validation +1/-1

Export hevyToolDefinitions for taxonomy validation

• Makes the tool definition list exportable to enable metadata allowlist tests.

src/tools/register.ts

tool-runtime.tsIntroduce ToolHandlerWrapper with optional telemetry metadata +19/-6

Introduce ToolHandlerWrapper with optional telemetry metadata

• Refactors runtime handler typing to preserve parameter types and allows wrappers to receive bounded telemetry metadata. Adds a default wrapper that delegates to existing error handling.

src/tools/tool-runtime.ts

Tests (11) +434 / -137
index.test.tsExpand entrypoint tests for Sentry MCP settings and shutdown hook +13/-3

Expand entrypoint tests for Sentry MCP settings and shutdown hook

• Mocks new metrics counters and asserts Sentry MCP wrapper is configured with input/output capture disabled. Updates graceful shutdown expectations to include an async completion hook.

src/index.test.ts

register.test.tsAssert every tool declares bounded feature/kind/operation metadata +26/-1

Assert every tool declares bounded feature/kind/operation metadata

• Exports tool definitions and adds a test ensuring each tool uses only the approved feature/kind/operation allowlists.

src/tools/register.test.ts

graceful-shutdown.test.tsAdd tests for async completion observers and failure reporting +30/-0

Add tests for async completion observers and failure reporting

• Verifies graceful shutdown awaits an async onComplete observer and reports failure exactly once when forced exit triggers.

src/utils/graceful-shutdown.test.ts

hevy-client-observability.test.tsUpdate tests for retry bucket and safe error attributes +12/-0

Update tests for retry bucket and safe error attributes

• Extends expectations to include retry_count_bucket and verifies safe error_category propagation into metrics without leaking secret strings.

src/utils/hevy-client-observability.test.ts

hevyClientKubb.test.tsAssert retryCount observations and exhausted-retry diagnostics +18/-2

Assert retryCount observations and exhausted-retry diagnostics

• Adds coverage ensuring onRequestComplete receives retryCount across retries and that exhausted GET retries are marked without exposing request details.

src/utils/hevyClientKubb.test.ts

mcp-session-observability.test.tsAdd tests for bounded client metadata and session lifecycle metrics +98/-0

Add tests for bounded client metadata and session lifecycle metrics

• Validates client/protocol metadata sanitization, bounded session started/ended metrics, and guards against including session IDs or user hashes.

src/utils/mcp-session-observability.test.ts

observability-wrapper.test.tsUpdate wrapper tests to bucket argument key counts +1/-1

Update wrapper tests to bucket argument key counts

• Adjusts Sentry context expectations to record bucketed argumentKeyCount instead of raw counts.

src/utils/observability-wrapper.test.ts

response-formatter.test.tsAdd regression test for null-field result telemetry inference +20/-0

Add regression test for null-field result telemetry inference

• Ensures result telemetry does not infer structural flags from null-valued fields in user-authored outputs.

src/utils/response-formatter.test.ts

stdio-observability.test.tsAdd test ensuring initialize client metadata is sanitized +38/-0

Add test ensuring initialize client metadata is sanitized

• Verifies only bounded initialize metadata is captured on stdio spans and that private fields are not serialized into telemetry.

src/utils/stdio-observability.test.ts

telemetry-wrapper.test.tsRewrite telemetry wrapper tests for bounded args, outcomes, and result telemetry +141/-129

Rewrite telemetry wrapper tests for bounded args, outcomes, and result telemetry

• Updates tests to validate bounded taxonomy attributes, presence/bucketed args, distinct outcomes for returned vs thrown errors, and explicit result telemetry attachment without leaking content.

src/utils/telemetry-wrapper.test.ts

telemetry.test.tsAssert Sentry PII disabled and MCP span sanitization hooked +37/-1

Assert Sentry PII disabled and MCP span sanitization hooked

• Ensures sendDefaultPii is false, Sentry.flush is mocked, and beforeSendSpan sanitizes MCP correlation/client metadata without mutating the original span object.

src/utils/telemetry.test.ts

Documentation (2) +192 / -0
telemetry-dashboards.mdDocument privacy-reviewed dashboard panels and publication checklist +104/-0

Document privacy-reviewed dashboard panels and publication checklist

• Adds recommended dashboard panels for usage, reliability, API health, workflows, and sessions using only allowlisted fields. Includes retention/access policy and a checklist tied to regression tests.

docs/telemetry-dashboards.md

telemetry-data-dictionary.mdDefine telemetry data dictionary and explicit prohibited fields +88/-0

Define telemetry data dictionary and explicit prohibited fields

• Adds the privacy contract for span/metric fields, bounded taxonomies, and sanitization rules. Documents client/session handling, Sentry MCP filtering expectations, and required regression guards.

docs/telemetry-data-dictionary.md

Other (1) +5 / -0
privacy-safe-telemetry.mdAdd changeset for privacy-safe telemetry patch release +5/-0

Add changeset for privacy-safe telemetry patch release

• Introduces a changeset describing the privacy-safe telemetry work and marks it for a patch release.

.changeset/privacy-safe-telemetry.md

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add privacy-safe MCP telemetry taxonomy, outcomes, and session metrics

✨ Enhancement 🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Add bounded, privacy-reviewed telemetry taxonomy for MCP tool and session signals.
• Harden telemetry to avoid capturing prompts/args/results and sanitize Sentry MCP span metadata.
• Add dashboards + data dictionary docs and expand tests to enforce privacy regressions.
Diagram

graph TD
  A["CLI (src/cli.ts)"] --> B["Server run (src/index.ts)"] --> C["Stdio observability"] --> D["Session observability"] --> E["OTel metrics/spans"] --> F{{"Sentry export"}}
  B --> G["Tool runtime/register"] --> H["Observability wrapper"] --> I["Telemetry wrapper"] --> E
  I --> D
  F --> J["Sentry MCP sanitizer"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. OTel attribute processor for sanitization
  • ➕ Centralizes attribute filtering without requiring every wrapper to cooperate
  • ➕ Can enforce fail-closed semantics at export boundaries
  • ➖ Harder to unit-test at the same granularity as wrapper-level invariants
  • ➖ Doesn’t address Sentry MCP wrapper capture settings; still need explicit config/tests
2. Emit only spans (no metrics) for tool outcome taxonomy
  • ➕ Simplifies metric taxonomy governance and dimensionality concerns
  • ➕ Avoids long-lived metric dimensions like tool_name
  • ➖ Makes product usage/reliability dashboards slower/harder (trace-based queries)
  • ➖ Loses cheap aggregate metrics for adoption/latency/outcome rates
3. Schema-driven telemetry declaration per tool (generated)
  • ➕ Guarantees every tool has telemetry metadata via compile-time generation
  • ➕ Can evolve taxonomy alongside tool registration automatically
  • ➖ More build-time complexity and coupling between tooling and runtime
  • ➖ Current explicit metadata + tests already enforce bounded values effectively

Recommendation: Current approach is a good tradeoff for a privacy-sensitive SDK: bounded taxonomy is enforced close to the emission site (tool wrapper/session observer), result shape telemetry is opt-in via a WeakMap (avoids inspecting user-authored text), and Sentry export is explicitly scrubbed. The main alternative (export-time processors) is viable but would be harder to prove with the same focused regression tests this PR adds.

Files changed (42) +1487 / -308

Enhancement (23) +807 / -145
index.tsDisable Sentry MCP I/O capture and record session termination +23/-2

Disable Sentry MCP I/O capture and record session termination

• Configures Sentry MCP wrapper to not record inputs/outputs. Adds session termination categorization, ensures telemetry flush on shutdown completion, and records startup/connect failure termination paths.

src/index.ts

body-measurements.tsAdd telemetry feature/operation metadata to body measurement tools +8/-0

Add telemetry feature/operation metadata to body measurement tools

• Annotates body measurement tool definitions with bounded 'feature' and 'operation' taxonomy values for consistent telemetry.

src/tools/body-measurements.ts

define-tool.tsRequire bounded telemetry metadata on ToolDefinition and pass into wrapper +8/-3

Require bounded telemetry metadata on ToolDefinition and pass into wrapper

• Extends tool definitions to include 'feature' and 'operation' fields (from the telemetry taxonomy) and passes full metadata into the runtime handler wrapper, enabling consistent telemetry emission per tool.

src/tools/define-tool.ts

folders.tsAdd telemetry feature/operation metadata to folder tools +6/-0

Add telemetry feature/operation metadata to folder tools

• Adds 'feature: folders' and per-tool 'operation' taxonomy values to folder tool definitions.

src/tools/folders.ts

register.tsExport hevyToolDefinitions for taxonomy validation +1/-1

Export hevyToolDefinitions for taxonomy validation

• Exports the combined 'hevyToolDefinitions' array to support taxonomy conformance tests.

src/tools/register.ts

routine-discovery.tsAdd workflow taxonomy metadata to routine discovery tool +2/-0

Add workflow taxonomy metadata to routine discovery tool

• Annotates 'search-routines' as 'feature: workflows' and 'operation: search' for bounded telemetry.

src/tools/routine-discovery.ts

routines.tsAdd telemetry feature/operation metadata to routine tools +8/-0

Add telemetry feature/operation metadata to routine tools

• Annotates routine tools with 'feature: routines' and appropriate operations (list/get/create/update).

src/tools/routines.ts

templates.tsAdd telemetry feature/operation metadata to template tools +10/-0

Add telemetry feature/operation metadata to template tools

• Adds bounded 'feature: templates' and per-tool operations across list/get/search/create template endpoints.

src/tools/templates.ts

user.tsAdd telemetry feature/operation metadata to user info tool +2/-0

Add telemetry feature/operation metadata to user info tool

• Annotates 'get-user-info' with 'feature: profile' and 'operation: get' for bounded telemetry.

src/tools/user.ts

workflows.tsAdd workflow taxonomy metadata to training summary tool +2/-0

Add workflow taxonomy metadata to training summary tool

• Annotates 'get-training-summary' as a workflows feature with 'operation: get' for bounded telemetry.

src/tools/workflows.ts

workouts.tsAdd telemetry feature/operation metadata to workout tools +12/-0

Add telemetry feature/operation metadata to workout tools

• Annotates all workout tools with 'feature: workouts' and appropriate operations (list/get/count/sync/create/update).

src/tools/workouts.ts

graceful-shutdown.tsAdd async onComplete hook and ensure it runs once +29/-1

Add async onComplete hook and ensure it runs once

• Extends graceful shutdown to accept an 'onComplete(succeeded)' observer that may be async. Ensures completion is reported once and is invoked on both normal shutdown and forced-exit fallback paths.

src/utils/graceful-shutdown.ts

hevy-client-observability.tsRecord retry-count buckets and normalized safe error diagnostics for API calls +19/-2

Record retry-count buckets and normalized safe error diagnostics for API calls

• Adds retry-count bucketing to API span/metric attributes and reuses 'createSafeErrorDiagnostic' normalization to emit only safe error category/code fields. Preserves debug logging while avoiding raw error values.

src/utils/hevy-client-observability.ts

mcp-session-observability.tsImplement bounded session start/end metrics with client metadata sanitization +156/-0

Implement bounded session start/end metrics with client metadata sanitization

• Adds session lifecycle tracking with allowlisted termination categories, duration buckets, and tool-call count buckets. Normalizes initialize client metadata with strict validation and emits only sanitized metric dimensions.

src/utils/mcp-session-observability.ts

metrics.tsAdd tool outcomes and session lifecycle counters +16/-2

Add tool outcomes and session lifecycle counters

• Introduces new counters for 'mcp.tool.outcomes', 'mcp.session.started', and 'mcp.session.ended', and updates metric descriptions to reflect bounded taxonomy usage.

src/utils/metrics.ts

observability-wrapper.tsPropagate tool taxonomy metadata into telemetry and bucket arg-key counts +7/-6

Propagate tool taxonomy metadata into telemetry and bucket arg-key counts

• Extends the observability wrapper to pass telemetry taxonomy metadata down into 'withTelemetry'. Replaces raw argument-key counts with bounded buckets for Sentry context to reduce leakage risk.

src/utils/observability-wrapper.ts

response-formatter.tsAttach bounded result-shape telemetry to responses without inspecting text +126/-7

Attach bounded result-shape telemetry to responses without inspecting text

• Adds optional telemetry producers to response contracts and attaches result telemetry via 'attachResultTelemetry'. Implements count bucketing for items/exercises/sets and allowlisted workflow telemetry, enabling safe downstream recording.

src/utils/response-formatter.ts

result-telemetry.tsIntroduce bucketed result telemetry and WeakMap attachment API +40/-0

Introduce bucketed result telemetry and WeakMap attachment API

• Adds bounded count buckets and a WeakMap-based mechanism to attach and retrieve result telemetry without serializing or inspecting response content.

src/utils/result-telemetry.ts

sentry-privacy.tsFilter prohibited MCP correlation/client span attributes before Sentry export +39/-0

Filter prohibited MCP correlation/client span attributes before Sentry export

• Introduces a sanitizer that removes a fixed set of MCP request/session/progress/prompt/protocol/client attributes from span data prior to Sentry export, preserving only safe keys.

src/utils/sentry-privacy.ts

stdio-observability.tsWhitelist safe MCP methods and record session start on initialize +23/-4

Whitelist safe MCP methods and record session start on initialize

• Restricts recorded 'mcp.method' values to a safe allowlist. On 'initialize', records sanitized client metadata via session observability and sets the corresponding span attributes.

src/utils/stdio-observability.ts

telemetry-wrapper.tsRewrite tool telemetry to be fail-closed and emit bounded outcomes +217/-116

Rewrite tool telemetry to be fail-closed and emit bounded outcomes

• Replaces argument value capture with structural-only presence/bucket attributes, adds bounded taxonomy attributes, and records a new tool outcome counter distinguishing success/returned_error/thrown_error. Pulls workflow/result-shape telemetry only from explicitly attached metadata and includes sanitized client dimensions from session tracking.

src/utils/telemetry-wrapper.ts

telemetry.tsAdd Sentry beforeSendSpan sanitizer and implement flushTelemetry() +21/-1

Add Sentry beforeSendSpan sanitizer and implement flushTelemetry()

• Wires Sentry MCP span sanitization via 'beforeSendSpan', persists the meter provider reference when configured, and adds a 'flushTelemetry()' helper that force-flushes tracing/metrics and Sentry with a bounded timeout.

src/utils/telemetry.ts

tool-taxonomy.tsDefine allowlisted tool feature/kind/operation telemetry taxonomy +32/-0

Define allowlisted tool feature/kind/operation telemetry taxonomy

• Introduces shared allowlists and types for 'hevy.feature', 'mcp.tool.kind', and 'mcp.tool.operation' to keep telemetry dimensions bounded and consistent across tools.

src/utils/tool-taxonomy.ts

Bug fix (2) +17 / -6
cli.tsFlush telemetry on fatal startup failure before exiting +7/-1

Flush telemetry on fatal startup failure before exiting

• Updates the CLI entrypoint to attempt 'flushTelemetry()' after logging a fatal error, while preserving the original exit behavior if flushing fails.

src/cli.ts

hevyClientKubb.tsInclude retryCount in request observations and tag exhausted retries earlier +10/-5

Include retryCount in request observations and tag exhausted retries earlier

• Extends request observations to include 'retryCount'. Ensures exhausted-retry diagnostics are applied before 'onRequestComplete' runs, and reuses 'hevyRetryExhausted' flag to control retry termination.

src/utils/hevyClientKubb.ts

Refactor (3) +32 / -20
shared-server.tsGeneralize tool handler wrapping via ToolHandlerWrapper +8/-5

Generalize tool handler wrapping via ToolHandlerWrapper

• Refactors shared server options to accept a 'ToolHandlerWrapper' rather than the older error-handler wrapper type, defaulting to a new 'defaultToolHandlerWrapper' for compatibility.

src/shared-server.ts

tool-runtime.tsIntroduce ToolHandlerWrapper type and default wrapper implementation +19/-6

Introduce ToolHandlerWrapper type and default wrapper implementation

• Defines a generic 'ToolHandler' type and a 'ToolHandlerWrapper' that can accept optional telemetry metadata. Provides 'defaultToolHandlerWrapper' that delegates to 'withErrorHandling', and updates runtime to use it by default.

src/tools/tool-runtime.ts

error-handler.tsPreserve handler parameter typing end-to-end in error wrapper +5/-9

Preserve handler parameter typing end-to-end in error wrapper

• Adjusts 'withErrorHandling' to return a handler typed as '(args: TParams)' and normalizes nullish args internally, reducing unsafe casting and improving wrapper composition.

src/utils/error-handler.ts

Tests (11) +434 / -137
index.test.tsExtend server entry tests for Sentry capture settings and shutdown hook +13/-3

Extend server entry tests for Sentry capture settings and shutdown hook

• Adds expectations that the MCP server is wrapped with Sentry using 'recordInputs: false' and 'recordOutputs: false'. Updates graceful shutdown assertions to include an 'onComplete' callback.

src/index.test.ts

register.test.tsAssert every tool declares bounded feature/kind/operation taxonomy +26/-1

Assert every tool declares bounded feature/kind/operation taxonomy

• Exports tool definitions and adds a test ensuring each tool’s telemetry metadata is within the approved allowlists for feature, kind, and operation.

src/tools/register.test.ts

graceful-shutdown.test.tsTest async completion observers and forced-exit completion semantics +30/-0

Test async completion observers and forced-exit completion semantics

• Adds coverage ensuring 'onComplete' can be async and is awaited before shutdown promise settlement. Verifies forced-exit path reports unsuccessful completion exactly once.

src/utils/graceful-shutdown.test.ts

hevy-client-observability.test.tsUpdate API observability tests for retry buckets and safe error attributes +12/-0

Update API observability tests for retry buckets and safe error attributes

• Adjusts expectations to include 'retryCount' in observations, 'hevy.api.retry_count_bucket' span attributes, and safe error-category propagation into API metrics without leaking secrets.

src/utils/hevy-client-observability.test.ts

hevyClientKubb.test.tsAssert retryCount observation hook and exhausted-retry diagnostics +18/-2

Assert retryCount observation hook and exhausted-retry diagnostics

• Adds tests that the injected 'onRequestComplete' receives retry counts per attempt, and that exhausted GET retries are marked with a dedicated error code without exposing request details.

src/utils/hevyClientKubb.test.ts

mcp-session-observability.test.tsAdd tests for bounded client metadata and session lifecycle metrics +98/-0

Add tests for bounded client metadata and session lifecycle metrics

• Introduces a new test suite validating client metadata normalization (bounded character set/length) and ensuring session metrics never include session IDs or user hashes.

src/utils/mcp-session-observability.test.ts

observability-wrapper.test.tsUpdate Sentry context assertion to use argument key-count buckets +1/-1

Update Sentry context assertion to use argument key-count buckets

• Adjusts the test to expect 'argumentKeyCountBucket' rather than a raw key count in Sentry context.

src/utils/observability-wrapper.test.ts

response-formatter.test.tsEnsure structural result telemetry ignores null fields +20/-0

Ensure structural result telemetry ignores null fields

• Adds a regression test ensuring result-shape telemetry is derived from structure/counts rather than inferring meaning from null-valued user fields.

src/utils/response-formatter.test.ts

stdio-observability.test.tsAdd test ensuring only sanitized initialize metadata is captured +38/-0

Add test ensuring only sanitized initialize metadata is captured

• Adds coverage that initialize messages record only bounded client name/version/protocol version and do not capture private fields from params.

src/utils/stdio-observability.test.ts

telemetry-wrapper.test.tsExpand telemetry wrapper tests for privacy-safe args, outcomes, and result shape +141/-129

Expand telemetry wrapper tests for privacy-safe args, outcomes, and result shape

• Updates tests to cover taxonomy attributes, session client metadata, bounded argument structure (presence/buckets only), distinct outcomes for returned vs thrown errors, and attached result-shape/workflow telemetry without inspecting response text.

src/utils/telemetry-wrapper.test.ts

telemetry.test.tsAssert Sentry PII disabled and MCP span sanitization is applied +37/-1

Assert Sentry PII disabled and MCP span sanitization is applied

• Adds tests ensuring 'sendDefaultPii: false', Sentry flush support is present, and the 'beforeSendSpan' hook removes prohibited MCP attributes while leaving safe ones intact.

src/utils/telemetry.test.ts

Documentation (2) +192 / -0
telemetry-dashboards.mdDocument privacy-reviewed dashboard panels and publication checklist +104/-0

Document privacy-reviewed dashboard panels and publication checklist

• Adds recommended dashboard panels for product usage, reliability, API health, workflow performance, and session lifecycle. Documents retention/access policy and a checklist of automated privacy guards to keep dashboards compliant.

docs/telemetry-dashboards.md

telemetry-data-dictionary.mdDefine telemetry data dictionary and explicit privacy contract +88/-0

Define telemetry data dictionary and explicit privacy contract

• Adds the allowlisted/bounded dimension dictionary for metrics and spans, including prohibited-field guidance. Describes sanitization rules for client metadata, session signals, and Sentry MCP filtering, and points to regression tests that enforce the contract.

docs/telemetry-data-dictionary.md

Other (1) +5 / -0
privacy-safe-telemetry.mdAdd changeset entry for privacy-safe telemetry patch +5/-0

Add changeset entry for privacy-safe telemetry patch

• Introduces a patch-level changeset describing the new privacy-safe telemetry taxonomy, tool outcomes, session signals, and dashboard guidance.

.changeset/privacy-safe-telemetry.md

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.29508% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.52%. Comparing base (f6612a3) to head (ab81cae).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/utils/mcp-session-observability.ts 80.95% 6 Missing and 2 partials ⚠️
src/utils/telemetry.ts 11.11% 8 Missing ⚠️
src/utils/graceful-shutdown.ts 69.23% 3 Missing and 1 partial ⚠️
src/utils/response-formatter.ts 94.33% 0 Missing and 3 partials ⚠️
src/index.ts 75.00% 2 Missing ⚠️
src/utils/result-telemetry.ts 77.77% 1 Missing and 1 partial ⚠️
src/utils/sentry-privacy.ts 71.42% 0 Missing and 2 partials ⚠️
src/utils/telemetry-wrapper.ts 97.26% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #700      +/-   ##
==========================================
- Coverage   95.45%   94.52%   -0.93%     
==========================================
  Files          47       50       +3     
  Lines        2090     2266     +176     
  Branches      575      639      +64     
==========================================
+ Hits         1995     2142     +147     
- Misses         32       51      +19     
- Partials       63       73      +10     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Unit Test Results

  1 files   50 suites   4s ⏱️
682 tests 682 ✅ 0 💤 0 ❌
686 runs  686 ✅ 0 💤 0 ❌

Results for commit ab81cae.

♻️ This comment has been updated with latest results.

@gitstream-cm gitstream-cm Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✨ PR Review

The PR introduces a comprehensive privacy-safe telemetry taxonomy with bounded tool outcomes, session lifecycle signals, and sanitized Sentry spans. The implementation is well-structured, but there is one concrete functional gap: the kind field required by ToolTelemetryMetadata is never populated in any tool definition, which breaks the taxonomy test added in the same PR.

1 issues detected:

🐞 Bug - `kind` is a required field in `ToolTelemetryMetadata` but is absent from every tool definition, causing the taxonomy test to fail and emitting `undefined` as a bounded metric dimension. 🛠️

Details: Every tool definition in this PR adds feature and operation fields but omits the kind field required by ToolTelemetryMetadata. The register.test.ts taxonomy test added in the same PR asserts expect(["read", "write"]).toContain(definition.kind) for every tool definition, which will fail for all tools because definition.kind is undefined. The taxonomyAttributes function in telemetry-wrapper.ts will emit "mcp.tool.kind": undefined, sending the literal string "undefined" as a span attribute instead of a bounded taxonomy value.

File: src/tools/workouts.ts (61-63)

🛠️ A suggested code correction is included in the review comments.

Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how

Comment thread src/tools/workouts.ts
@mergify

mergify Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a privacy-safe MCP telemetry taxonomy, bounded tool outcomes, session compatibility signals, and dashboard guidance. It refactors the telemetry and observability wrappers to capture only structural argument presence, bucketed counts, and safe result-shape metadata rather than raw arguments or result bodies. The review feedback highlights critical TypeScript compilation and contravariance type errors introduced by narrowing the parameter types of withErrorHandling, withObservability, withTelemetry, and ToolHandlerWrapper away from the generic Record<string, unknown> expected by the MCP SDK. Additionally, an issue was identified in the graceful shutdown watchdog timer where a timeout could cause a delayed exit and report an incorrect successful exit code instead of a hard failure.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/utils/error-handler.ts Outdated
Comment thread src/utils/observability-wrapper.ts Outdated
Comment thread src/utils/telemetry-wrapper.ts
Comment thread src/tools/tool-runtime.ts Outdated
Comment thread src/utils/graceful-shutdown.ts
@qodo-code-review

qodo-code-review Bot commented Jul 20, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 65 rules

Grey Divider


Action required

1. Forced-exit awaits onComplete ✓ Resolved 🐞 Bug ☼ Reliability
Description
In installGracefulShutdown, the forced-exit fallback defers process termination until the optional
onComplete promise settles, so a hung completion observer can keep a SIGINT/SIGTERM-terminated
process alive indefinitely. This breaks the forced-exit timeout guarantee and can prevent reliable
termination under failure conditions.
Code

src/utils/graceful-shutdown.ts[R120-128]

		const forcedExitTimer = scheduleForcedExit(() => {
-			processLike.exit(processLike.exitCode ?? 0);
+			const completion = reportCompletion(false);
+			if (completion) {
+				void completion.finally(() =>
+					processLike.exit(processLike.exitCode ?? 0),
+				);
+			} else {
+				processLike.exit(processLike.exitCode ?? 0);
+			}
Relevance

⭐⭐⭐ High

PR #570 added forced-exit specifically to guarantee termination even if shutdown stalls; awaiting
observer breaks guarantee.

PR-#570

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The file documents the forced-exit timeout as a bound, but the new forced-exit callback waits for an
async completion observer to finish before calling exit, which can remove that bound if the observer
never settles. This is the same failure mode that prior graceful-shutdown work aimed to prevent by
ensuring the process terminates even when shutdown work stalls.

src/utils/graceful-shutdown.ts[44-46]
src/utils/graceful-shutdown.ts[120-129]
PR-#570

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`installGracefulShutdown`’s forced-exit timer is meant to *bound* shutdown time, but it now waits for `onComplete` (via `completion.finally(...)`) before calling `processLike.exit(...)`. If `onComplete` returns a promise that never resolves (or resolves very slowly), the process may never exit even after the forced-exit timeout.

### Issue Context
This library hook is generic (`onComplete?: (succeeded: boolean) => void | Promise<void>`), so it must be safe even if a future/custom observer hangs. The code comment explicitly describes the forced-exit behavior as a bound.

### Fix Focus Areas
- src/utils/graceful-shutdown.ts[111-170]

### Suggested fix
In the forced-exit callback, call `processLike.exit(...)` unconditionally when the timer fires. Invoke `reportCompletion(false)` in a best-effort, non-blocking way (or cap it with a very small timeout), but do **not** delay `exit()` on its completion.

Also add/adjust a regression test where `onComplete` returns a never-resolving promise and verify the forced-exit path still calls `process.exit` promptly when the forced-exit timer fires.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread src/utils/graceful-shutdown.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/telemetry-dashboards.md`:
- Line 31: Update the Thrown-error rate metric around mcp.tool.errors so
error_type is either replaced with an approved bounded field or formally defined
as a finite, sanitized tool-error taxonomy in the data dictionary and
corresponding instrumentation. Keep the dashboard grouping and implementation
aligned with the chosen documented contract.

In `@docs/telemetry-data-dictionary.md`:
- Around line 29-30: Keep exact tool names debugging-only by updating
docs/telemetry-data-dictionary.md lines 29-30 to define the permitted
short-lived lifetime and access policy. In docs/telemetry-dashboards.md lines
14-15 and 30-32, remove tool_name from product-adoption and reliability
groupings, or replace it with approved bounded taxonomy fields; do not retain it
in dashboards unless they are explicitly short-lived and access-controlled.

In `@src/utils/mcp-session-observability.ts`:
- Around line 46-57: Update normalizeMetadata and SAFE_METADATA_PATTERN so
client-reported metadata cannot pass arbitrary free-text values such as “private
client metadata”; tighten the accepted character set, including disallowing
spaces, while preserving the existing missing, empty, oversized, and
unsafe-value fallback to UNKNOWN_METADATA.

In `@src/utils/telemetry-wrapper.ts`:
- Around line 92-116: Update setWorkflowAttributes to safely handle missing or
undefined workflow.pagination before iterating, and isolate any telemetry
attribute-setting failures so they cannot escape withTelemetry’s try block.
Preserve the successful tool result and avoid converting instrumentation errors
into thrown tool-call failures.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7bff7176-cc8e-4fcc-b6b3-3879e9b199cb

📥 Commits

Reviewing files that changed from the base of the PR and between b2cf50d and 0408dc9.

📒 Files selected for processing (42)
  • .changeset/privacy-safe-telemetry.md
  • docs/telemetry-dashboards.md
  • docs/telemetry-data-dictionary.md
  • src/cli.ts
  • src/index.test.ts
  • src/index.ts
  • src/shared-server.ts
  • src/tools/body-measurements.ts
  • src/tools/define-tool.ts
  • src/tools/folders.ts
  • src/tools/register.test.ts
  • src/tools/register.ts
  • src/tools/routine-discovery.ts
  • src/tools/routines.ts
  • src/tools/templates.ts
  • src/tools/tool-runtime.ts
  • src/tools/user.ts
  • src/tools/workflows.ts
  • src/tools/workouts.ts
  • src/utils/error-handler.ts
  • src/utils/graceful-shutdown.test.ts
  • src/utils/graceful-shutdown.ts
  • src/utils/hevy-client-observability.test.ts
  • src/utils/hevy-client-observability.ts
  • src/utils/hevyClientKubb.test.ts
  • src/utils/hevyClientKubb.ts
  • src/utils/mcp-session-observability.test.ts
  • src/utils/mcp-session-observability.ts
  • src/utils/metrics.ts
  • src/utils/observability-wrapper.test.ts
  • src/utils/observability-wrapper.ts
  • src/utils/response-formatter.test.ts
  • src/utils/response-formatter.ts
  • src/utils/result-telemetry.ts
  • src/utils/sentry-privacy.ts
  • src/utils/stdio-observability.test.ts
  • src/utils/stdio-observability.ts
  • src/utils/telemetry-wrapper.test.ts
  • src/utils/telemetry-wrapper.ts
  • src/utils/telemetry.test.ts
  • src/utils/telemetry.ts
  • src/utils/tool-taxonomy.ts

Comment thread docs/telemetry-dashboards.md Outdated
Comment thread docs/telemetry-data-dictionary.md Outdated
Comment thread src/utils/mcp-session-observability.ts
Comment thread src/utils/telemetry-wrapper.ts
Co-Authored-By: Oz <oz-agent@warp.dev>
@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Bundle Report

Changes will increase total bundle size by 14.35kB (9.09%) ⬆️⚠️, exceeding the configured threshold of 5%.

Bundle name Size Change
hevy-mcp-esm 172.24kB 14.35kB (9.09%) ⬆️⚠️

Affected Assets, Files, and Routes:

view changes for bundle: hevy-mcp-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
src-2kNor1UB.mjs (New) 169.98kB 169.98kB 100.0% 🚀
cli.mjs 72 bytes 927 bytes 8.42% ⚠️
src-86dub6ti.mjs (Deleted) -155.7kB 0 bytes -100.0% 🗑️

Files in src-2kNor1UB.mjs:

  • ./src/tools/workflows.ts → Total Size: 6.26kB

  • ./src/utils/metrics.ts → Total Size: 2.12kB

  • ./src/tools/workouts.ts → Total Size: 5.7kB

  • ./src/index.ts → Total Size: 6.04kB

  • ./src/tools/templates.ts → Total Size: 7.41kB

  • ./src/tools/user.ts → Total Size: 873 bytes

  • ./src/utils/hevyClientKubb.ts → Total Size: 9.79kB

  • ./src/tools/routines.ts → Total Size: 4.16kB

  • ./src/utils/error-handler.ts → Total Size: 1.91kB

  • ./src/utils/hevy-client-observability.ts → Total Size: 2.0kB

  • ./src/shared-server.ts → Total Size: 1.61kB

  • ./src/tools/folders.ts → Total Size: 2.94kB

  • ./src/tools/body-measurements.ts → Total Size: 4.69kB

  • ./src/tools/define-tool.ts → Total Size: 825 bytes

  • ./src/utils/graceful-shutdown.ts → Total Size: 2.76kB

  • ./src/tools/register.ts → Total Size: 524 bytes

  • ./src/tools/tool-runtime.ts → Total Size: 334 bytes

  • ./src/utils/mcp-session-observability.ts → Total Size: 3.06kB

  • ./src/tools/routine-discovery.ts → Total Size: 2.54kB

Files in cli.mjs:

  • ./src/cli.ts → Total Size: 213 bytes

@chrisdoc
chrisdoc merged commit cd3fbde into main Jul 21, 2026
22 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant