Skip to content

fix: namespace user IDs by channel-type prefix, not bare colon#2591

Open
mmahmed wants to merge 2 commits into
nanocoai:mainfrom
mmahmed:fix/user-id-channel-prefix
Open

fix: namespace user IDs by channel-type prefix, not bare colon#2591
mmahmed wants to merge 2 commits into
nanocoai:mainfrom
mmahmed:fix/user-id-channel-prefix

Conversation

@mmahmed
Copy link
Copy Markdown
Contributor

@mmahmed mmahmed commented May 22, 2026

Type of Change

  • Feature skill - adds a channel or integration (source code changes + SKILL.md)
  • Utility skill - adds a standalone tool (code files in .claude/skills/<name>/, no source changes)
  • Operational/container skill - adds a workflow or agent skill (SKILL.md only, no source changes)
  • Fix - bug fix or security fix to source code
  • Simplification - reduces or simplifies source code
  • Documentation - docs, README, or CONTRIBUTING changes only

Description

What

Detect already-namespaced user IDs by checking for the <channelType>: prefix instead of any bare colon.

Why

Teams Bot Framework user IDs natively look like 29:1abc.... The previous rawHandle.includes(':') check treated those as already namespaced and skipped the channel-type prefix, so the resolver returned 29:1abc... while the owner row created by init-first-agent.ts was keyed teams:29:1abc.... The IDs never matched, so:

  • every Teams DM from the owner was dropped as MESSAGE DROPPED — unknown sender (approval requested) with accessReason="not_member"
  • the resulting unknown-sender approval card delivered to that same owner had its click rejected as unauthorized clicker (the click handler had the same broken namespacing)
  • a duplicate users row was being created on every inbound (one prefixed, one not), keeping the auth check permanently out of sync

Net effect: Teams users were locked out of their own agents immediately after /init-first-agent succeeded.

How

  • src/modules/permissions/index.ts — fix the sender resolver and both approval click handlers
  • container/agent-runner/src/formatter.ts — fix the matching agent-side resolver
  • src/modules/permissions/sender-approval.test.ts — the existing fixture (senderId: 'tg:stranger') was passing under the broken behavior because the colon bypassed namespacing; switch to a colonless raw id so the test actually exercises the namespacing path

How it was tested

  • pnpm test: 328/328 host tests pass (after fixing the fixture)
  • bun test in container/agent-runner: 89/89 pass
  • End-to-end on a live install: Teams DM that was previously being dropped now routes to the owner's agent and the agent's reply is delivered back through Teams

For Skills

  • SKILL.md contains instructions, not inline code (code goes in separate files)
  • SKILL.md is under 500 lines
  • I tested this skill on a fresh clone

- Teams Bot Framework user IDs natively contain a colon (`29:1abc...`),
  so `rawHandle.includes(':')` treated them as already-namespaced and
  skipped the channel-type prefix
- Owner rows keyed `teams:29:...` never matched the resolved `29:...`,
  so Teams DMs dropped as unknown_sender and approval clicks were
  rejected as unauthorized
- Match by `<channelType>:` prefix instead, in both the host
  permissions module (sender resolver + approval click handlers) and
  the container-side formatter
- Update sender-approval test fixture: the previous `senderId:
  'tg:stranger'` passed under the broken behavior because the colon
  bypassed namespacing; switch to a colonless id so the test
  exercises the actual code path
@github-actions github-actions Bot added follows-guidelines PR was created using the current contributing template PR: Fix Bug fix labels May 22, 2026
@mmahmed
Copy link
Copy Markdown
Contributor Author

mmahmed commented May 25, 2026

Hi @gavrielc @gabi-simons - please review this change. teams integration is broken without this change as teams user ids have colon in them.

mmahmed added a commit to mmahmed/nanoclaw that referenced this pull request May 25, 2026
- Records what's stacked on top of origin/main (PRs nanocoai#2591, nanocoai#2617, multi-bot Teams, bootstrap)
- Documents how to refresh when upstream consumes a PR, how to add a new carry, how to bump deps
- Captures the non-standard remote convention (origin = upstream, fork = our fork)
mmahmed added a commit to mmahmed/nanoclaw that referenced this pull request May 25, 2026
- Records what's stacked on top of origin/main (PRs nanocoai#2591, nanocoai#2617, multi-bot Teams, bootstrap)
- Documents how to refresh when upstream consumes a PR, how to add a new carry, how to bump deps
- Captures the non-standard remote convention (origin = upstream, fork = our fork)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

follows-guidelines PR was created using the current contributing template PR: Fix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant