feat(sentry-monitor): bundle Sentry error monitoring into the fork#3
Closed
insomnius wants to merge 2 commits into
Closed
feat(sentry-monitor): bundle Sentry error monitoring into the fork#3insomnius wants to merge 2 commits into
insomnius wants to merge 2 commits into
Conversation
Adds a bundled, default-on (DSN-gated) plugin that forwards every error-bearing OpenClaw lifecycle event to Sentry: model_call_ended, agent_end, after_tool_call, message_sent, subagent_ended, cron_changed, and abnormal session_end, plus node-level uncaught/unhandled rejections. Inactive (safe no-op) until BOON_SENTRY_DSN or plugin-config dsn is set. Vendored from @getboon/openclaw-monitor and adapted for in-fork bundling: ships in core dist, @sentry/node mirrored at root as an internalized bundled-plugin runtime dependency. Strictly typed: pure per-hook capture builders return a discriminated SentryCapture union; dispatch is typed against a minimal structural client; hook payloads use the real openclaw/plugin-sdk/types so core field changes surface at compile time. Extensive unit tests cover every trigger boundary, dispatch routing, and the register() activation paths. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
- Register lifecycle hooks via the typed api.on(...) API instead of raw api.registerHook(...), per the bundled-plugin boundary contract; add the registration file + its hook names to the contract allowlist. This also removes the cast/generics (api.on supplies per-hook typed events). - Drop the redundant String() coercion in pruneTags (values are already typed string) and the now-moot coercion test; use mock.lastCall to avoid an unnecessary non-null assertion in dispatch tests — clears 5 oxlint errors. - Ignore @sentry/node in both knip bundled-dependency lists (root + extension), matching the @homebridge/ciao bundled-at-root precedent. - Add the blank line markdownlint MD031 requires around the config fence. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Author
|
Superseded by #4, which recreates this work as a single GPG-signed commit ( |
insomnius
added a commit
that referenced
this pull request
Jul 6, 2026
…lee-level test Round-3 review findings on PR #39: - `docs/channels/msteams.md` "Set to \`false\` explicitly on any deployment that overrides this default and wants to opt out." leaked the existence of a fork override to the upstream docs mirror while sitting next to "Default: false.". Drop the trailer; the default sentence is enough for upstream readers, and the fork `CHANGELOG.md` still documents the fork override + opt-out. - `extensions/msteams/src/monitor-handler/inbound-media.test.ts:231` test title said "(upstream default)", but the assertion is at the callee boundary and stays valid regardless of the caller's default. The message handler applies the resolver upstream, so this test proves the callee contract, not the caller default. Retitle + rewrite the leading comment to describe the contract, not the (now-misleading) upstream behavior. Round-3 finding #3 (PLAUSIBLE, not filed): no message-handler-level end-to-end test that exercises the fork default. Accepted as-is — resolver unit tests + explicit callee-level tests + comment coverage give enough confidence for a 1-line default flip. Full message-handler test scaffolding is heavy and out of scope for this PR.
insomnius
added a commit
that referenced
this pull request
Jul 7, 2026
) * fix(msteams): default alwaysFetchGraphMessage=true on the Boon fork Every Teams tenant in the Boon fleet observed the ENG-14349 Bot Framework stub-stripping regression - inbound activities arrive without the `<attachment id=...>` HTML stub and without `reference`-typed entries even with RSC consent granted, so attachments silently vanish unless the Graph re-fetch path fires. Upstream 6.11 already ships the fix as opt-in config `channels.msteams.alwaysFetchGraphMessage` (default off, kept off upstream so tenants where Bot Framework delivers the stub correctly don't pay per-message Graph latency). The Boon fleet does not have such tenants - every Teams host was previously relying on a host-local msteams SP-PATCH baked in via gandalf-manager `scripts/fleet-msteams5.22-bump.sh`. Flip the fork default via a small resolver so an explicit `false` still opts out per host, and the change is unit-testable without wiring through the full message-handler. Version bump: 2026.6.11-boon.1 -> 2026.6.11-boon.2. * fix(msteams): align config docs + shrinkwrap with fork default flip Round-1 review findings on PR #39: - `npm-shrinkwrap.json` still pinned `2026.6.11-boon.1`; regen via `scripts/generate-npm-shrinkwrap.mjs` so release-check version parity (`scripts/check-openclaw-package-tarball.mjs:243`) passes. - Config-contract docs at `src/config/types.msteams.ts:164` and `src/config/zod-schema.providers-core.ts:1651` still asserted "Default false." — now note upstream=false / fork=true so the schema/type peer stays aligned with the runtime resolver, per root `CLAUDE.md` "Config contract: exported types, schema/help, metadata, baselines, docs aligned." * docs(msteams): document alwaysFetchGraphMessage + sync inbound-media JSDoc Round-2 review findings on PR #39: - `extensions/msteams/src/monitor-handler/inbound-media.ts:37` — the sibling JSDoc on `resolveMSTeamsInboundMedia`'s param still said "Default false (upstream behavior)." and is what editors surface at the message-handler call site. Note that the caller resolves the fork default before calling in. - `docs/channels/msteams.md` — `alwaysFetchGraphMessage` was never listed in the user-facing Configuration section even upstream; add it so the flag is discoverable and the CHANGELOG's opt-out (`false`) is findable via docs. Doc source publishes to the openclaw/docs upstream mirror, so the entry stays upstream-neutral and describes upstream behavior; the fork-specific default is called out only in the fork `CHANGELOG.md` and in the internal-facing schema/type JSDoc. * docs+test(msteams): tighten alwaysFetchGraphMessage doc + retitle callee-level test Round-3 review findings on PR #39: - `docs/channels/msteams.md` "Set to \`false\` explicitly on any deployment that overrides this default and wants to opt out." leaked the existence of a fork override to the upstream docs mirror while sitting next to "Default: false.". Drop the trailer; the default sentence is enough for upstream readers, and the fork `CHANGELOG.md` still documents the fork override + opt-out. - `extensions/msteams/src/monitor-handler/inbound-media.test.ts:231` test title said "(upstream default)", but the assertion is at the callee boundary and stays valid regardless of the caller's default. The message handler applies the resolver upstream, so this test proves the callee contract, not the caller default. Retitle + rewrite the leading comment to describe the contract, not the (now-misleading) upstream behavior. Round-3 finding #3 (PLAUSIBLE, not filed): no message-handler-level end-to-end test that exercises the fork default. Accepted as-is — resolver unit tests + explicit callee-level tests + comment coverage give enough confidence for a 1-line default flip. Full message-handler test scaffolding is heavy and out of scope for this PR. * fix(msteams): drop extensions/ path from core config JSDoc CI `checks-fast-contracts-channels-a` failure: the round-1 doc updates inserted the plugin-private path `extensions/msteams/src/monitor-handler/graph-fallback-default.ts` into core `src/config/types.msteams.ts` and `src/config/zod-schema.providers-core.ts` JSDoc, which triggered `src/channels/plugins/contracts/channel-import-guardrails.test.ts` ("keeps core production files off plugin-private src imports"). Reword the doc so it names the fork consume-site by plugin (`@openclaw/msteams`) and describes the runtime seam (resolver applied before `resolveMSTeamsInboundMedia`), rather than pointing at a plugin-private source path. Verified locally: - `channel-import-guardrails.test.ts`: 148 tests passing. - `graph-fallback-default.test.ts` + `inbound-media.test.ts`: 16 tests passing.
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
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.
Summary
Bundles Sentry error monitoring directly into the fork as a default-on (DSN-gated) plugin, instead of the separate git-installed
@getboon/openclaw-monitor. It forwards every error-bearing OpenClaw lifecycle event to Sentry and stays a clean plugin tapping hooks (no Sentry code in coresrc/**), honoring the plugin-agnostic-core rule.extensions/sentry-monitor/—enabledByDefault: true,activation.onStartup, but inactive (safe no-op) untilBOON_SENTRY_DSNor plugin-configdsnis set.model_call_ended(error),agent_end(fail),after_tool_call(error),message_sent(fail),subagent_ended(error/timeout/killed/reset/deleted),cron_changed(run/delivery error),session_end(unknown), plus node uncaught/unhandled rejections. Errors only; no prompt/message/tool content.package.jsonfilesexclude list);@sentry/[email protected]mirrored at root as an internalized bundled-plugin runtime dep (same pattern as bonjour/@homebridge/ciao).docs/gateway/sentry-monitor.md(+ nav),plugin: sentry-monitorlabeler entry.Design / typing
SentryCaptureunion;dispatchtyped against a minimal structuralSentryCaptureClient(no cast, mockable).openclaw/plugin-sdk/types(PluginHook*Event) rather than local mirrors, so a core field rename/removal is a compile error rather than a silently-undefined Sentry field.coerceHookEventboundary instead of scatteredas unknown as.Tests
44 unit tests across
format,captures,dispatch, andregister: every hook trigger/no-trigger boundary, dispatch routing (exception vs message, null no-op), and theregister()activation paths (no-DSN inactive → no init/no hooks; env + config DSN → init + all 7 hooks + flush lifecycle).Verification
Real behavior proof
Behavior addressed: Sentry error reporting ships with the fork by default (DSN-gated), as a bundled plugin rather than a separate git install.
Real environment tested: local build + unit/type verification in a worktree off
boon. Confirmed the plugin compiles intodist/extensions/sentry-monitor/and is discovered by the bundled-plugin build pipeline.Exact steps or command run after this patch: the five commands above (vitest, two tsgo lanes, pnpm build, dependency-ownership --check).
Evidence after fix: 44/44 tests pass; both tsgo lanes exit 0; full
pnpm buildclean withdist/extensions/sentry-monitor/{index.js,index.d.ts,openclaw.plugin.json,package.json}emitted; dep auditok.Observed result after fix: bundled plugin builds, typechecks, and is wired with default-on + DSN-gated activation.
What was not tested: no live gateway run with a real
BOON_SENTRY_DSNdelivering an actual event to a Sentry project; no end-to-end assertion that a triggered error appears in Sentry (logic is covered by unit tests against a mocked client). Channel connect/disconnect capture is intentionally out of scope (needs a new corechannel_connection_changedhook — follow-up).🤖 Generated with Claude Code
Summary by cubic
Bundle a Sentry error monitor as a built‑in plugin. It forwards error-only OpenClaw lifecycle events to Sentry without touching core code.
New Features
extensions/sentry-monitor/with startup activation, shipped in core dist.@sentry/[email protected]as a bundled-plugin runtime dep; docs atdocs/gateway/sentry-monitor.md. Knip updated to ignore@sentry/node.api.on(...)per bundled-plugin boundary; added to contracts allowlist and fixed docs lint.Migration
BOON_SENTRY_DSNor plugin configdsnto enable.environment(defaults to hostname),tracesSampleRate(defaults to 0).Written for commit 26ab8f6. Summary will update on new commits.