fix: Add null checks in local-memory.ts to prevent role crash#438
Merged
Conversation
Contributor
Author
|
Added comprehensive test suite for Test Coverage: 81% on local-memory.ts
All tests pass. 🎉 |
corinagum
reviewed
Feb 12, 2026
corinagum
left a comment
corinagum
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the PR! LGTM. Do you mind rebasing with latest? Otherwise my review will be disregarded.
Collaborator
|
@copilot rebase |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds defensive null/bounds checks to LocalMemory trimming/collapse logic to prevent crashes when message arrays are empty or when boundary scanning moves past the end.
Changes:
- Guarded trim loop against empty arrays and added optional chaining for
messages[0]access - Added bounds checking to collapse “end boundary” scan to prevent out-of-range access
- Added Jest test suite covering empty memory, trimming, and collapse edge cases
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/ai/src/local-memory.ts | Adds null/bounds guards in trimming and collapse boundary scanning to prevent crashes. |
| packages/ai/src/local-memory.spec.ts | Introduces Jest coverage for the reported crash scenarios and related edge cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes microsoft/teams-sdk#2500 Fixes microsoft/teams-sdk#2503 - Add bounds check (len > 0) before accessing messages[0] - Add optional chaining for messages[0]?.role access - Add null guard and bounds check in collapse() loop - Prevents 'Cannot read properties of null (reading role)' crash Root cause: Array access without bounds checking when: 1. Messages array becomes empty during collapse 2. end index exceeds array bounds in collapse loop Co-authored-by: claude-swarm-agent <swarm@local>
- Tests for constructor, get, push, pop, values, where methods - Tests for collapse functionality with different strategies - Specific tests for null safety fixes: - Empty array access prevention - Optional chaining for messages[0].role - Bounds checking in collapse boundary detection All 21 tests pass with 81% coverage on local-memory.ts
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
corinagum
force-pushed
the
fix/null-role-crash
branch
from
May 13, 2026 19:08
ba3fa9d to
34ebf2b
Compare
singhk97
approved these changes
May 13, 2026
7 tasks
corinagum
added a commit
that referenced
this pull request
May 27, 2026
## Summary
Brings `origin/main` into `release` for the **2.0.12** release. **No
carve-outs this time** — Quoted Replies is included.
`version.json`: `2.0.12-preview.{height}` → `2.0.12`.
Aligns with teams.py 2.0.12 (PR #442 already on PyPI / pending publish).
## What's in this release (delta from 2.0.11)
**Features**
- Quoted Replies + new quotes features — previously held back via the
2.0.11 carve-out (#576)
- SuggestedActionSubmitActivity for `suggestedAction/submit` invoke
(#591)
- Default targeted replies for targeted inbound messages (#592)
- Filter colliding keys before `Object.assign` in `ActivityContext`
(#596)
- Prompt Preview support (#536)
- A2A sample (#584)
- AI/MCPClient (#572)
- Allow passing custom HTTP client via `AppOptions`-equivalent
- Reactions API marked GA (#575), Rename `ReactionClient.remove()` →
`delete()` (#567)
**Security & fixes**
- Security hardening: tighten cross-origin policies (#595)
- Lock JsonWebToken trust-boundary contract (#586)
- `fix(deps)`: npm audit fix — clears all high-severity advisories
(#599)
- `fix(apps)`: log inbound activities at info, warn on missing
Authorization (#568)
- `fix`: App user-agent merging (#573)
- `fix`: Add null checks in `local-memory.ts` to prevent role crash
(#438)
- `fix`: `@microsoft/teams.client` import error in webpack 5 (#566)
- Switch to named imports without subpaths (#561)
**Deprecations / package changes**
- Deprecate `DevtoolsPlugin` in favor of Microsoft 365 Agents Playground
(#593)
- Remove in-repo Teams CLI package (#580) — moved to
`teams-sdk/packages/cli`
- Deprecate AI Libraries (#588)
- Correct imports + return types in misc. packages (#589)
**Dependency bumps**
- turbo 2.8.11 → 2.9.14 (#587)
- qs 6.15.0 → 6.15.2 (#594)
- hono 4.12.14 → 4.12.16 (#562)
- npm audit fix bundle (#599): axios 1.13.5 → 1.16.1, fast-uri 3.1.0 →
3.1.2, ws 8.19.0 → 8.21.0, @azure/msal-node hoisted 5.2.2, uuid 11.1.0 →
11.1.1, @azure/identity 4.13.0 → 4.13.1, express-rate-limit, ip-address,
brace-expansion, etc.
## Quoted Replies inclusion notes
- Teams client rendering is in-sync with the wire format as of
2026-05-06.
- APX QR rollout completed: Public 2026-04-10, GCCH/DoD/Gallatin
2026-04-14.
- No SDK-side carve-outs needed; this matches the teams.py 2.0.12 (PR
#442) decision.
## Branch structure note
`prep-release/2.0.12` is reset to `origin/main` + a one-line
`version.json` bump. Diff against `release` (this PR) shows the full
delta from 2.0.11. We deliberately did not use a 2-parent merge commit —
the auto-merge tried to interleave non-overlapping hunks from main's QR
additions with release's QR carve-outs in `activity.ts`, producing a
semantically broken file. Reset-to-main + version bump is the clean way
to express "release should equal main."
## Test plan
- [x] `npm install` clean (post-audit-fix lockfile)
- [x] `npm run build` — 33/33 targets clean
- [x] `npm test` — 14/14 task targets, 279+ tests pass
- [x] E2E echo bot smoke test against `cg-test-bot-py` via canary ABS —
done on `cg/audit-deps-ts` (now merged); covers axios + msal-node code
paths
- [ ] Pipeline Build + Test stages green on `release` after merge
- [ ] Publish pipeline run with **Public** → ESRP → npm
`@microsoft/teams.*@2.0.12`
- [ ] Smoke install: `npm view @microsoft/teams[email protected]` after
publish
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
Adds defensive null checks to prevent crashes when accessing message properties on empty or boundary arrays.
Changes
len > 0guard before array access in trim loop?.) formessages[0].roleaccesslast &&guard and bounds check in slice operationTesting
Fixes #2500, #2503