fix(msteams): default alwaysFetchGraphMessage=true on the Boon fork#39
Conversation
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.
|
/cr |
Dependency GuardThis PR changes dependency-related files. Maintainers should confirm these changes are intentional. Changed files:
Maintainer follow-up:
|
Dependency graph changes notedThis PR includes dependency graph changes. The dependency guard is informational because the PR author is a repository admin or a member of
Security review is still recommended before merge when the dependency graph change is intentional. |
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."
…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.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
…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.
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.
![gandalfboon[bot]](https://avatars.githubusercontent.com/in/3087119?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed — approving. No blocking issues.
Logic. resolveMSTeamsAlwaysFetchGraphMessage(configured) returns configured ?? BOON_FORK_ALWAYS_FETCH_GRAPH_MESSAGE_DEFAULT. The nullish-coalesce is the right call: an explicit false opts out per host, and only an unset (undefined) flag falls through to the fork default true. A || here would have silently clobbered the opt-out — it doesn't. Correct.
Seam. The default is resolved in message-handler.ts before calling resolveMSTeamsInboundMedia, and the callee still treats undefined as off. That keeps the callee contract upstream-neutral, and the retitled inbound-media.test.ts:231 now asserts the callee boundary rather than a (misleading) "upstream default" — good catch in round 3.
Fork hygiene. Upstream-mirrored docs/channels/msteams.md documents only the upstream false default with no fork-override leak; the fork default + opt-out live in CHANGELOG.md and internal JSDoc. Config-contract peers (types.msteams.ts, zod-schema.providers-core.ts) are aligned. Zero upstream schema changes. Version bump + npm-shrinkwrap.json are in sync at 2026.6.11-boon.2.
Verification (pulled from source, not the description):
- Test:
graph-fallback-default.test.tscovers all three branches (unset→true, explicit true, explicit false→false). CIchecks-fast-contracts-channels-a+channel-runtime-boundarygreen. - Checks: 120 pass / 0 fail / 0 pending. The
channel-import-guardrailsregression from the round-1 JSDoc is resolved (final commit reworded to reference the plugin publicly). - Review threads: 1, resolved + outdated (cubic-dev-ai P2 — correctly rejected; upstream docs shouldn't carry the fork override).
- Commits: all 5 verified-signed.
mergeStateStatus: CLEAN.
Ships. Retiring the host-local SP-PATCH in favor of a fork default is the right structural move.
2 participants
What Problem This Solves
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 withoutreference-typed entries even with RSC consent granted, so attachments silently vanish unless the Graph re-fetch path fires. arguijo hit this on 2026-07-06 shortly after the fleet moved to2026.6.11-boon.1— Teams broke while Slack kept working.Historically the fleet worked around this with a host-local msteams SP-PATCH baked in via gandalf-manager
scripts/fleet-msteams5.22-bump.sh— a Python edit of the compiled bundle on every Teams host — withENG-14431 will retire thisin the patch comment. This PR is that retirement.Why This Change Was Made
Upstream 6.11 already ships the fix as an opt-in config:
channels.msteams.alwaysFetchGraphMessage(default off). Upstream keeps it off because tenants where Bot Framework delivers the<attachment>stub correctly should not pay per-channel-message Graph latency.The Boon fork does not have such tenants — every Teams host in the fleet was already carrying the SP-PATCH, i.e. was already paying the round-trip. Flipping the fork default from
undefined→truegives the same behavior as SP-PATCH without host-local bundle edits, and centralizes future changes to the flag in the fork rather than in a shell script.The change is scoped to the fork consume-site (
extensions/msteams/src/monitor-handler/message-handler.ts) via a small resolver — the upstream schema/type/inbound-media contracts are unchanged. Operators can still setchannels.msteams.alwaysFetchGraphMessage: falseper host to opt out.User Impact
openclawupgrades, no drift.alwaysFetchGraphMessage: falsestill honored.2026.6.11-boon.1→2026.6.11-boon.2.Evidence
getboon/gandalf-managerscripts/fleet-msteams5.22-bump.sh— the two edits it applied on every Teams host.extensions/msteams/src/attachments/graph.ts:85— no fork work needed.alwaysFetchGraphMessageopt-in config is the same code path; this PR flips the default only.#project-rohan/p1783349412), agreed with Victor + team.extensions/msteams/src/monitor-handler/graph-fallback-default.test.ts— asserts fork default istrue, explicitfalseopts out, explicittruestill-on.2026.6.11-boon.1 (807986a)(SSM probe 2026-07-06).Rollout after merge:
2026.6.11-boon.2fork release → fleet-sweep Teams hosts to pick up the tarball → per-host changedoc entry stating the SP-PATCH bump script is retired.🤖 Generated with Claude Code
Summary by cubic
Default
channels.msteams.alwaysFetchGraphMessageto true on the Boon fork so Teams channel attachments resolve reliably without per-host bundle patches. Retires the SP-PATCH workaround while keeping a per-host opt‑out.Bug Fixes
resolveMSTeamsAlwaysFetchGraphMessagein the@openclaw/msteamsmessage handler; honors explicit true/false and leaves callee contracts unchanged; tests added.2026.6.11-boon.2with refreshednpm-shrinkwrap.json.Migration
channels.msteams.alwaysFetchGraphMessage: falseon a host.Written for commit e528f64. Summary will update on new commits.