Skip to content

fix(msteams): bound SSO API JSON response reads to prevent OOM#97781

Closed
Alix-007 wants to merge 19 commits into
openclaw:mainfrom
Alix-007:fix/bound-msteams-sso-api
Closed

fix(msteams): bound SSO API JSON response reads to prevent OOM#97781
Alix-007 wants to merge 19 commits into
openclaw:mainfrom
Alix-007:fix/bound-msteams-sso-api

Conversation

@Alix-007

@Alix-007 Alix-007 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Routes the live Microsoft Teams signin.token-exchange and signin.verify-state routes through OpenClaw's bounded SSO User Token helper instead of the Teams SDK's unbounded axios res.data path.
  • Keeps the existing sender-policy gate and preserves token persistence for both activity.from.id and activity.from.aadObjectId aliases.
  • Preserves the SDK cloud-specific Bot Framework User Token service URL, so USGov/USGovDoD/China SSO routes keep using the same token host as the SDK route they replace.
  • Keeps the 16 MiB successful-response cap and existing non-OK error handling for Bot Framework User Token service calls.
  • Adds production-route tests proving normal, oversized, and non-public-cloud User Token responses through the registered sign-in routes.

Linked context

No linked issue; this is a narrow follow-up to the bounded-response-read hardening series for Microsoft Teams SSO.

Real behavior proof

Behavior addressed: The previous PR version bounded only the direct SSO helper path. The production monitor-registered signin.token-exchange and signin.verify-state routes still delegated to the Teams SDK handlers, whose User Token client reads axios res.data without a response-size cap. This update moves those live routes onto OpenClaw's bounded helper while preserving the SDK cloud token-service URL via app.cloud.tokenServiceUrl.

Real environment tested: Linux x86-64, Node 22.22.0, local OpenClaw worktree with installed repo dependencies, real Node/Web Response streams, and the msteams extension test runtime.

Exact steps or command run after this patch:

node scripts/run-vitest.mjs extensions/msteams/src/sso.test.ts extensions/msteams/src/monitor-handler.sso.test.ts extensions/msteams/src/monitor.lifecycle.test.ts extensions/msteams/src/sdk.test.ts -- --run
./node_modules/.bin/oxfmt --check extensions/msteams/src/monitor.ts extensions/msteams/src/monitor.lifecycle.test.ts extensions/msteams/src/sdk.ts extensions/msteams/src/sso.ts extensions/msteams/src/sso.test.ts extensions/msteams/src/monitor-handler.sso.test.ts extensions/msteams/src/sdk.test.ts
node scripts/run-oxlint.mjs extensions/msteams/src/monitor.ts extensions/msteams/src/monitor.lifecycle.test.ts extensions/msteams/src/sdk.ts extensions/msteams/src/sso.ts extensions/msteams/src/sso.test.ts extensions/msteams/src/monitor-handler.sso.test.ts extensions/msteams/src/sdk.test.ts

Evidence after fix:

Test Files  4 passed (4)
Tests  41 passed (41)

The new monitor lifecycle coverage calls the actual registered signin.token-exchange and signin.verify-state route callbacks. The success case verifies both routes call fetch through the bounded OpenClaw helper, persist both Teams user-id aliases, and do not call app.onTokenExchange / app.onVerifyState. The oversized case streams >16 MiB responses through those same registered routes and verifies they fail closed without saving a token or delegating to the SDK handlers. The USGov case verifies those same registered routes call https://tokengcch.botframework.azure.us/api/usertoken/..., proving the bounded replacement preserves SDK cloud token routing instead of falling back to the public Bot Framework token host.

Observed result after fix: Oversized successful Bot Framework User Token responses are capped before JSON parsing on the live SSO route path. Normal token exchange and verify-state responses still return { status: 200 } and persist tokens for later OpenClaw use. Non-public Teams cloud SSO keeps the SDK's configured User Token service host.

What was not tested: A live Microsoft Teams tenant / Bot Framework OAuth connection. No tenant credentials are available in this environment.

Tests and validation

  • node scripts/run-vitest.mjs extensions/msteams/src/sso.test.ts extensions/msteams/src/monitor-handler.sso.test.ts extensions/msteams/src/monitor.lifecycle.test.ts extensions/msteams/src/sdk.test.ts -- --run - 4 files, 41 tests passed.
  • ./node_modules/.bin/oxfmt --check ... - all matched files use the correct format.
  • node scripts/run-oxlint.mjs ... - passed.

Risk checklist

  • Did user-visible behavior change? Only for oversized successful Bot Framework User Token responses: they now fail closed at 16 MiB instead of being read unbounded.
  • Did config, environment, or migration behavior change? No.
  • Did security, auth, secrets, network, or tool execution behavior change? Yes, intentionally narrows the live SSO network read boundary; endpoint paths, auth headers, cloud token hosts, and token persistence semantics are preserved.
  • What is the highest-risk area? Compatibility of replacing the SDK sign-in route delegate with the local bounded implementation.
  • How is that risk mitigated? Tests exercise the registered production route callbacks for token-exchange and verify-state success, oversized-response failure, and USGov cloud token-host preservation; dependency inspection shows the SDK handlers perform the same User Token calls through unbounded axios res.data.

Current review state

  • Next action: ClawSweeper re-review for the previous cloud token-service URL blocker.
  • Waiting on: CI for commit b770b4b30122caed9624b6366d4ad8880dc6dc04.

@openclaw-barnacle openclaw-barnacle Bot added channel: msteams Channel integration: msteams size: S triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 29, 2026
@clawsweeper

clawsweeper Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed July 12, 2026, 11:51 PM ET / July 13, 2026, 03:51 UTC.

Summary
Routes the two live Microsoft Teams sign-in invoke handlers through a 16 MiB-bounded User Token client while preserving cloud-specific endpoints and both persisted Teams user aliases.

PR surface: Source +428, Tests +444. Total +872 across 6 files.

Reproducibility: yes. from source inspection: the live SDK handlers read the User Token success body without OpenClaw's byte limit, and the branch's registered-route tests exercise the replacement path. A live tenant-level OOM reproduction was not attempted.

Review metrics: 1 noteworthy metric.

  • Live SSO routes replaced: 2 production invoke routes. Both token-exchange and verify-state now bypass SDK handlers, so response-status parity must be demonstrated across the full authentication fallback surface.

Root-cause cluster
Relationship: canonical
Canonical: #97781
Summary: This PR is the broad viable route-level fix; several closed PRs addressed only the already-existing helper read and did not harden both production-registered invoke routes.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Preserve the dependency's distinct invoke statuses and bodies, with 412 limited to regular-login fallback.
  • Resolve or conclusively dismiss the exact-head CodeQL invocation-of-non-function alert.
  • Rerun the four focused Microsoft Teams suites against the repaired exact head.

Risk before merge

  • [P2] The replacement handler can return the regular-login fallback status for User Token failures that the SDK would expose as a distinct malformed-request or service failure, changing authentication recovery behavior for existing Teams deployments.
  • [P1] The current head has an unresolved high-precision CodeQL invocation-of-non-function alert after conflict resolution; if reachable, the affected SSO path can throw instead of returning an invoke response.
  • [P1] A live Microsoft Teams tenant was not exercised, so exact client behavior for each non-200 Bot Framework response remains inferred from the dependency contract and route-level proof.

Maintainer options:

  1. Restore exact SDK response semantics (recommended)
    Preserve each Bot Framework invoke status and body, use 412 only for the documented regular-login fallback, resolve the CodeQL alert, and add registered-route coverage before merge.
  2. Retain the SDK delegates
    If exact behavioral parity cannot be maintained locally, keep the SDK sign-in handlers and pursue the response bound through an upstream or narrower injectable client seam.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Preserve the pinned Teams SDK status/body behavior for token exchange and verify-state failures, using 412 only for regular-login fallback; resolve the current invocation-of-non-function alert and add focused registered-route regression tests for 400, 404, 412, and 5xx outcomes.

Next step before merge

  • [P2] The remaining blocker is a narrow mechanical response-mapping repair plus resolution of a concrete static-analysis alert; no product decision is required.

Security
Cleared: The patch narrows an external-response memory boundary without adding dependencies, broadening credentials, logging tokens, or weakening authorization checks; the remaining blockers are correctness and authentication compatibility concerns.

Review findings

  • [P1] Preserve non-fallback User Token HTTP statuses — extensions/msteams/src/monitor.ts:137
Review details

Best possible solution:

Keep the bounded local handlers, but make their invoke status and response body exactly match the pinned Teams/Bot Framework contract for success, fallback, malformed input, missing token, and service failure; resolve the non-callable CodeQL path and prove all cases through the registered routes.

Do we have a high-confidence way to reproduce the issue?

Yes from source inspection: the live SDK handlers read the User Token success body without OpenClaw's byte limit, and the branch's registered-route tests exercise the replacement path. A live tenant-level OOM reproduction was not attempted.

Is this the best way to solve the issue?

Not yet. Moving both live routes onto the bounded helper is the correct ownership boundary, but it is only the best fix once the replacement preserves the SDK's complete invoke-response contract and the latest CodeQL alert is resolved.

Full review comments:

  • [P1] Preserve non-fallback User Token HTTP statuses — extensions/msteams/src/monitor.ts:137
    The local route mapping must not turn every unsuccessful User Token call into the 412 regular-login fallback. The Bot Framework contract reserves 412 for token-exchange fallback; malformed requests, missing-token results, and service failures retain distinct 400/404/500-class responses. Collapsing those cases changes Teams client recovery and masks real auth outages. This was the prior review blocker, and the later listed commits contain only merges or conflict/build repairs rather than a targeted semantic fix.
    Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 6dfa4c27a3ad.

Label changes

Label justifications:

  • P2: The PR fixes a bounded Microsoft Teams availability risk but still has a contained authentication compatibility blocker rather than a currently demonstrated widespread outage.
  • merge-risk: 🚨 compatibility: Replacing the SDK handlers can change existing Teams clients' fallback and error behavior when User Token requests fail.
  • merge-risk: 🚨 auth-provider: The diff directly changes Bot Framework token exchange, cloud token-service routing, invoke statuses, and delegated-token persistence.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR provides after-fix live-output evidence using real streamed Response bodies and the actual registered sign-in callbacks for normal, oversized, alias-persistence, and USGov-host cases; live tenant coverage remains a stated limitation.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR provides after-fix live-output evidence using real streamed Response bodies and the actual registered sign-in callbacks for normal, oversized, alias-persistence, and USGov-host cases; live tenant coverage remains a stated limitation.
Evidence reviewed

PR surface:

Source +428, Tests +444. Total +872 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 4 455 27 +428
Tests 2 492 48 +444
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 6 947 75 +872

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs extensions/msteams/src/sso.test.ts extensions/msteams/src/monitor-handler.sso.test.ts extensions/msteams/src/monitor.lifecycle.test.ts extensions/msteams/src/sdk.test.ts -- --run.
  • [P1] node scripts/run-oxlint.mjs extensions/msteams/src/monitor.ts extensions/msteams/src/monitor.lifecycle.test.ts extensions/msteams/src/sdk.ts extensions/msteams/src/sso.ts extensions/msteams/src/sso.test.ts extensions/msteams/src/monitor-handler.sso.test.ts extensions/msteams/src/sdk.test.ts.
  • [P1] pnpm check:changed -- extensions/msteams/src/monitor.ts extensions/msteams/src/monitor.lifecycle.test.ts extensions/msteams/src/sdk.ts extensions/msteams/src/sso.ts extensions/msteams/src/sso.test.ts extensions/msteams/src/monitor-handler.sso.test.ts extensions/msteams/src/sdk.test.ts.

What I checked:

  • Production-path hardening: The branch replaces the SDK delegates for both signin.token-exchange and signin.verify-state with the local bounded helper, so oversized successful User Token responses are rejected before unbounded JSON parsing. (extensions/msteams/src/monitor.ts:44, aed5da7d6f7d)
  • Bounded response implementation: The new SSO client uses the shared response-limit runtime with a 16 MiB cap and retains bounded non-OK error extraction. (extensions/msteams/src/sso.ts:23, aed5da7d6f7d)
  • Prior blocker remains unaddressed: The July 12 review found that non-fallback User Token failures were not preserving the SDK invoke-response statuses. Every later branch commit listed in the supplied history is a main merge or conflict/build repair; none identifies or demonstrates a status-mapping correction. (extensions/msteams/src/monitor.ts:137, 904b13da26df)
  • Dependency response contract: The Bot Framework token-exchange contract uses HTTP 412 specifically to request regular-login fallback, while malformed requests and other service failures retain distinct 400/404/500-class outcomes; collapsing those outcomes changes Teams authentication behavior. (extensions/msteams/src/monitor.ts:137, aed5da7d6f7d)
  • Latest-head correctness alert: GitHub Advanced Security reported a high-precision invocation-of-non-function alert against the current head after the latest conflict-resolution sequence; it must be resolved or conclusively dismissed against the exact head before merge. (extensions/msteams/src/monitor.ts:137, aed5da7d6f7d)
  • After-fix behavior evidence: The PR body reports 41 passing focused tests across four Microsoft Teams suites, including registered production-route callbacks, oversized streamed responses, alias persistence, and the USGov token-service host. (extensions/msteams/src/monitor.lifecycle.test.ts:280, aed5da7d6f7d)

Likely related people:

  • steipete: Verified and landed the closely related Microsoft Teams Bot Framework attachment bounded-read implementation, including exact-head runtime and CI proof, making them the strongest available routing candidate for the shared hardening pattern. (role: adjacent bounded-response verifier and merger; confidence: high; commits: 6bc661ce14c8; files: extensions/msteams/src/attachments/bot-framework.ts, extensions/msteams/src/attachments/bot-framework.test.ts)
  • NianJiuZst: Reviewed this PR's current Microsoft Teams SSO test and merge-base behavior and reproduced an earlier concrete integration failure on the merge ref. (role: recent reviewer; confidence: medium; files: extensions/msteams/src/sso.test.ts, extensions/msteams/src/monitor-handler.sso.test.ts, extensions/msteams/src/sso-token-store.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (11 earlier review cycles; latest 8 shown)
  • reviewed 2026-07-06T03:40:00.662Z sha 1cec41f :: needs real behavior proof before merge. :: [P1] Harden the live SDK SSO route
  • reviewed 2026-07-06T04:16:43.592Z sha 5b93c35 :: needs real behavior proof before merge. :: [P1] Harden the live SDK SSO route
  • reviewed 2026-07-06T06:30:19.798Z sha fce078f :: needs real behavior proof before merge. :: [P1] Harden the live SDK SSO route
  • reviewed 2026-07-06T12:59:30.293Z sha 4c073d6 :: needs changes before merge. :: [P1] Preserve the Teams cloud token endpoint
  • reviewed 2026-07-06T16:52:26.125Z sha efffccc :: needs maintainer review before merge. :: none
  • reviewed 2026-07-06T17:45:02.096Z sha efffccc :: needs maintainer review before merge. :: none
  • reviewed 2026-07-09T06:46:32.146Z sha acf5d11 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-12T10:13:50.995Z sha 904b13d :: needs changes before merge. :: [P1] Preserve non-fallback User Token HTTP statuses

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 29, 2026
@Alix-007
Alix-007 force-pushed the fix/bound-msteams-sso-api branch from 25ab029 to fb04443 Compare June 29, 2026 13:48
@Alix-007

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Addressed the blocking review feedback:

  • rebased onto latest main;
  • replaced the missing createMSTeamsSsoTokenStoreMemory test import with a local in-memory token store;
  • focused validation passes: sso.test.ts + monitor-handler.sso.test.ts 7/7 and oxlint 0 problems.

I also attempted the extension test type graph directly; the previous missing-export error is gone, and the remaining failures are unrelated existing acpx/qa-lab type/dependency errors outside this PR's touched files.

@clawsweeper

clawsweeper Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 29, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 29, 2026

@NianJiuZst NianJiuZst 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.

This change moves the successful Microsoft Teams SSO User Token response read onto the shared bounded response reader. That is the right owner boundary for the OOM guard, and the 16 MiB cap matches the existing provider JSON cap pattern, but the submitted test does not pass against the current base.

LOC: +176/-1 (2 files)

Findings: one blocking test/base compatibility issue, inline.

Best-fix verdict: acceptable mitigation once the test helper reference is fixed and proof is refreshed.
Alternatives considered: readProviderJsonResponse would also bound the body but would require reshaping this helper's existing error strings; keeping readResponseWithLimit local to extensions/msteams/src/sso.ts is fine. Re-exporting a production memory store just for this test is less clean than reusing the local in-memory-store pattern already in extensions/msteams/src/monitor-handler.sso.test.ts, unless the helper is intentionally promoted for multiple tests.
Code read: extensions/msteams/src/sso.ts, extensions/msteams/src/sso-token-store.ts, extensions/msteams/src/monitor-handler.sso.test.ts, extensions/msteams/src/http-error.ts, src/agents/provider-http-errors.ts, packages/media-core/src/read-response-with-limit.ts, packages/media-core/src/read-response-with-limit.test.ts, extensions/exa/src/exa-web-search-provider.runtime.ts, extensions/AGENTS.md.
Evidence: git fetch upstream pull/97781/merge:refs/remotes/upstream/pr/97781-merge; OPENCLAW_VITEST_FS_MODULE_CACHE_PATH=/tmp/openclaw-pr-97781-merge.XyNbsk/.artifacts/vitest-cache node scripts/run-vitest.mjs extensions/msteams/src/sso.test.ts extensions/msteams/src/monitor-handler.sso.test.ts fails on the merge ref with TypeError: createMSTeamsSsoTokenStoreMemory is not a function; GitHub check-test-types also fails on the same missing export. git diff --check upstream/main...refs/remotes/upstream/pr/97781-merge is clean.
Remaining uncertainty: I did not run a live Teams SSO flow; the runtime path review is based on source, sibling tests, the shared bounded-reader contract, and the merge-ref loopback proof.

Comment thread extensions/msteams/src/sso.test.ts Outdated
// but it would NOT surface the "msteams.sso" label in the error message.
import http from "node:http";
import { afterEach, describe, expect, it, vi } from "vitest";
import { createMSTeamsSsoTokenStoreMemory } from "./sso-token-store.js";

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.

This test imports createMSTeamsSsoTokenStoreMemory, but that helper is not exported on the current base. The PR head was cut from an older merge-base where the helper existed; once GitHub merges it with current main, extensions/msteams/src/sso-token-store.ts only exports makeMSTeamsSsoTokenStoreKey and createMSTeamsSsoTokenStoreFs, so the new loopback tests fail before they exercise the bounded-read path. I reproduced this on refs/pull/97781/merge: node scripts/run-vitest.mjs extensions/msteams/src/sso.test.ts extensions/msteams/src/monitor-handler.sso.test.ts fails with TypeError: createMSTeamsSsoTokenStoreMemory is not a function, and CI check-test-types reports TS2724 for this import. Please inline the same small memory-store pattern already used in extensions/msteams/src/monitor-handler.sso.test.ts or intentionally add/export a current test helper before relying on this import.

@Alix-007
Alix-007 force-pushed the fix/bound-msteams-sso-api branch from fb04443 to b393217 Compare June 30, 2026 16:36
@Alix-007

Copy link
Copy Markdown
Contributor Author

Rebased onto latest upstream/main to refresh the stale Real behavior proof check. Local validation remains green:\n- node scripts/run-vitest.mjs extensions/msteams/src/sso.test.ts extensions/msteams/src/monitor-handler.sso.test.ts -- --run\n- oxlint extensions/msteams/src/sso.ts extensions/msteams/src/sso.test.ts extensions/msteams/src/monitor-handler.sso.test.ts\n- node scripts/generate-docs-map.mjs --check

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 2, 2026
@Alix-007

Alix-007 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Current head is clean and the previous missing createMSTeamsSsoTokenStoreMemory import blocker is fixed by using a local in-memory token store in the test. There are no failing status checks on the PR right now.

Focused validation passed locally:

  • node scripts/run-vitest.mjs extensions/msteams/src/sso.test.ts extensions/msteams/src/monitor-handler.sso.test.ts -- --run (7/7)
  • oxlint extensions/msteams/src/sso.ts extensions/msteams/src/sso.test.ts extensions/msteams/src/monitor-handler.sso.test.ts
  • node scripts/generate-docs-map.mjs --check

The PR body/test includes real loopback-server proof for the bounded 16 MiB User Token service response path.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 6, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 12, 2026
Alix-007 added 2 commits July 12, 2026 22:16
…lict

# Conflicts:
#	extensions/msteams/src/monitor.ts
#	extensions/msteams/src/sso.ts
Comment thread extensions/msteams/src/sso.ts Fixed
@Alix-007

Copy link
Copy Markdown
Contributor Author

Closing this contribution because the change expanded from a bounded response read into replacing two production SSO invoke routes. The latest review still identifies authentication response-compatibility and static-analysis concerns, and completing the change would require broader tenant-level validation than is appropriate for this narrow hardening PR.

The branch remains available for reference. A future attempt should either use an upstream or injectable SDK transport seam, or preserve the pinned SDK status and response-body contract with focused route-level and live tenant proof.

@Alix-007 Alix-007 closed this Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: msteams Channel integration: msteams merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: XL status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants