fix(teams): register the tunnel port individually before hosting

Confirmed against a live devtunnel run: "devtunnel host <id> -p <port>
--allow-anonymous" against a persistent (already-created via `devtunnel
create`) tunnel fails with: "Tunnel service error: Invalid arguments.
Batch update of ports is not supported. Add, update, or delete ports
individually instead."

DevTunnelProvider.start() now runs `devtunnel port create <id> -p <port>
--allow-anonymous` as its own short-lived process first (tolerating an
"already exists" failure from a prior partial run), then invokes `devtunnel
host <id>` with no -p at all, since the tunnel already has the port
registered individually. Ad-hoc/temporary sessions (no tunnelId) are
unaffected -- they still host directly with -p in one call, matching prior
behaviour.

Rewrote devtunnel.test.ts to model both subprocess phases (port create,
then host) with distinct fake children.

Author: roccoren

packages/messaging-gateway/src/adapters/teams/tunnel/devtunnel.test.ts

@@ -12,46 +12,121 @@ describe('parseTunnelUrl', () => {
   })
 })
 
-describe('DevTunnelProvider', () => {
-  it('resolves the public URL parsed from spawn stdout', async () => {
-    const fakeChild = Object.assign(new EventEmitter(), {
-      stdout: new EventEmitter(), stderr: new EventEmitter(), kill: () => {},
-    })
-    const spawnImpl = (() => fakeChild) as unknown as typeof import('node:child_process').spawn
-    const p = new DevTunnelProvider({ binPath: '/fake/devtunnel', tunnelId: 't1', spawnImpl })
+function fakeChild(): EventEmitter & { stdout: EventEmitter; stderr: EventEmitter; kill: () => void } {
+  return Object.assign(new EventEmitter(), { stdout: new EventEmitter(), stderr: new EventEmitter(), kill: () => {} })
+}
+
+describe('DevTunnelProvider — no tunnelId (ad-hoc/temporary session)', () => {
+  it('hosts directly with -p, no port-create step', async () => {
+    const child = fakeChild()
+    const calls: string[][] = []
+    const spawnImpl = ((_bin: string, args: string[]) => { calls.push(args); return child }) as unknown as typeof import('node:child_process').spawn
+    const p = new DevTunnelProvider({ binPath: '/fake/devtunnel', spawnImpl })
     const started = p.start(3978)
     setTimeout(() => {
-      fakeChild.stdout.emit('data', Buffer.from('Connect via browser: https://t1-3978.usw2.devtunnels.ms\n'))
+      child.stdout.emit('data', Buffer.from('Connect via browser: https://t1-3978.usw2.devtunnels.ms\n'))
     }, 10)
     const { publicUrl } = await started
     expect(publicUrl).toBe('https://t1-3978.usw2.devtunnels.ms')
+    expect(calls).toEqual([['host', '--allow-anonymous', '-p', '3978']])
     expect(p.isRunning()).toBe(true)
     await p.stop()
     expect(p.isRunning()).toBe(false)
   })
 
   it('rejects when the process exits before a URL is seen', async () => {
-    const fakeChild = Object.assign(new EventEmitter(), {
-      stdout: new EventEmitter(), stderr: new EventEmitter(), kill: () => {},
-    })
-    const spawnImpl = (() => fakeChild) as unknown as typeof import('node:child_process').spawn
+    const child = fakeChild()
+    const spawnImpl = (() => child) as unknown as typeof import('node:child_process').spawn
     const p = new DevTunnelProvider({ binPath: '/fake/devtunnel', spawnImpl })
     const started = p.start(3978)
-    setTimeout(() => { fakeChild.emit('exit', 1) }, 10)
+    setTimeout(() => { child.emit('exit', 1) }, 10)
     await expect(started).rejects.toThrow(/devtunnel exited/)
   })
 
   it('includes the real stderr text in the rejection instead of a hardcoded guess', async () => {
-    const fakeChild = Object.assign(new EventEmitter(), {
-      stdout: new EventEmitter(), stderr: new EventEmitter(), kill: () => {},
-    })
-    const spawnImpl = (() => fakeChild) as unknown as typeof import('node:child_process').spawn
-    const p = new DevTunnelProvider({ binPath: '/fake/devtunnel', tunnelId: 't1', spawnImpl })
+    const child = fakeChild()
+    const spawnImpl = (() => child) as unknown as typeof import('node:child_process').spawn
+    const p = new DevTunnelProvider({ binPath: '/fake/devtunnel', spawnImpl })
     const started = p.start(3978)
     setTimeout(() => {
-      fakeChild.stderr.emit('data', Buffer.from('Error: port 3978 is already forwarded on this tunnel\n'))
-      fakeChild.emit('exit', 1)
+      child.stderr.emit('data', Buffer.from('Error: port 3978 is already forwarded on this tunnel\n'))
+      child.emit('exit', 1)
     }, 10)
     await expect(started).rejects.toThrow(/port 3978 is already forwarded/)
   })
 })
+
+describe('DevTunnelProvider — persistent tunnelId', () => {
+  /**
+   * Real CLI behaviour (confirmed against a live devtunnel run): hosting a
+   * previously-created (persistent) tunnel with `-p <port>` combined in the
+   * same `host` invocation fails with "Batch update of ports is not
+   * supported. Add, update, or delete ports individually instead." The port
+   * must be registered via a separate `devtunnel port create` call first;
+   * `host` is then invoked with no `-p` at all.
+   */
+  function twoPhaseSpawn(): {
+    spawnImpl: typeof import('node:child_process').spawn
+    portCreateChild: ReturnType<typeof fakeChild>
+    hostChild: ReturnType<typeof fakeChild>
+    calls: string[][]
+  } {
+    const portCreateChild = fakeChild()
+    const hostChild = fakeChild()
+    const calls: string[][] = []
+    let call = 0
+    const spawnImpl = ((_bin: string, args: string[]) => {
+      calls.push(args)
+      call += 1
+      return call === 1 ? portCreateChild : hostChild
+    }) as unknown as typeof import('node:child_process').spawn
+    return { spawnImpl, portCreateChild, hostChild, calls }
+  }
+
+  it('registers the port individually before hosting, then hosts without -p', async () => {
+    const { spawnImpl, portCreateChild, hostChild, calls } = twoPhaseSpawn()
+    const p = new DevTunnelProvider({ binPath: '/fake/devtunnel', tunnelId: 't1', spawnImpl })
+    const started = p.start(3978)
+
+    await new Promise((r) => setTimeout(r, 5))
+    portCreateChild.emit('exit', 0)
+
+    await new Promise((r) => setTimeout(r, 5))
+    hostChild.stdout.emit('data', Buffer.from('Connect via browser: https://t1-3978.usw2.devtunnels.ms\n'))
+
+    const { publicUrl } = await started
+    expect(publicUrl).toBe('https://t1-3978.usw2.devtunnels.ms')
+    expect(calls[0]).toEqual(['port', 'create', 't1', '-p', '3978', '--allow-anonymous'])
+    expect(calls[1]).toEqual(['host', 't1'])
+  })
+
+  it('tolerates "port already exists" from a prior partial run and still hosts', async () => {
+    const { spawnImpl, portCreateChild, hostChild, calls } = twoPhaseSpawn()
+    const p = new DevTunnelProvider({ binPath: '/fake/devtunnel', tunnelId: 't1', spawnImpl })
+    const started = p.start(3978)
+
+    await new Promise((r) => setTimeout(r, 5))
+    portCreateChild.stderr.emit('data', Buffer.from('Error: a port with this number already exists\n'))
+    portCreateChild.emit('exit', 1)
+
+    await new Promise((r) => setTimeout(r, 5))
+    hostChild.stdout.emit('data', Buffer.from('Connect via browser: https://t1-3978.usw2.devtunnels.ms\n'))
+
+    const { publicUrl } = await started
+    expect(publicUrl).toBe('https://t1-3978.usw2.devtunnels.ms')
+    expect(calls[1]).toEqual(['host', 't1'])
+  })
+
+  it('rejects without ever hosting when port registration genuinely fails', async () => {
+    const { spawnImpl, portCreateChild, calls } = twoPhaseSpawn()
+    const p = new DevTunnelProvider({ binPath: '/fake/devtunnel', tunnelId: 't1', spawnImpl })
+    const started = p.start(3978)
+
+    await new Promise((r) => setTimeout(r, 5))
+    portCreateChild.stderr.emit('data', Buffer.from('Error: not logged in\n'))
+    portCreateChild.emit('exit', 1)
+
+    await expect(started).rejects.toThrow(/port create failed.*not logged in/s)
+    expect(calls.length).toBe(1) // host was never invoked
+  })
+})

packages/messaging-gateway/src/adapters/teams/tunnel/devtunnel.ts

@@ -2,6 +2,13 @@
  * DevTunnelProvider — hosts a persistent Azure Dev Tunnel so the public URL is
  * stable across restarts. Requires a prior one-time `devtunnel user login`.
  *
+ * Persistent (already-created) tunnels reject a `host <id> -p <port>` call in
+ * one shot with: "Tunnel service error: Invalid arguments. Batch update of
+ * ports is not supported. Add, update, or delete ports individually instead."
+ * (confirmed against a live devtunnel run). The port must be registered
+ * individually via `devtunnel port create` first; `host` is then invoked with
+ * no `-p` at all, since the tunnel already knows which port to forward.
+ *
  * NOTE: the exact `devtunnel host` stdout format varies by CLI version.
  * `parseTunnelUrl` targets the printed `https://<id>.<cluster>.devtunnels.ms`
  * connect URL; verify against the installed CLI before shipping.
@@ -36,10 +43,45 @@ export class DevTunnelProvider implements TunnelProvider {
     if (opts.onUrl) this.urlHandlers.add(opts.onUrl)
   }
 
-  start(localPort: number): Promise<{ publicUrl: string }> {
+  async start(localPort: number): Promise<{ publicUrl: string }> {
+    if (this.opts.tunnelId) {
+      await this.ensurePortRegistered(this.opts.tunnelId, localPort)
+    }
+    return this.runHost(localPort)
+  }
+
+  /**
+   * Register `localPort` on the persistent tunnel as its own operation. Runs
+   * to completion (short-lived) before `host` ever starts. Tolerates "already
+   * exists" so a reconnect after an earlier partial failure doesn't hard-fail
+   * on a port the tunnel already knows about.
+   */
+  private ensurePortRegistered(tunnelId: string, localPort: number): Promise<void> {
     const spawnImpl = this.opts.spawnImpl ?? nodeSpawn
-    const args = ['host', '--allow-anonymous', '-p', String(localPort)]
-    if (this.opts.tunnelId) args.push(this.opts.tunnelId)
+    const args = ['port', 'create', tunnelId, '-p', String(localPort), '--allow-anonymous']
+    const proc = spawnImpl(this.opts.binPath, args, { stdio: ['ignore', 'pipe', 'pipe'] })
+    return new Promise((resolve, reject) => {
+      let stderr = ''
+      proc.stderr?.on('data', (chunk: Buffer) => { stderr += chunk.toString('utf8') })
+      proc.on('error', reject)
+      proc.on('exit', (code) => {
+        if (code === 0) { resolve(); return }
+        if (/already exists|already defined/i.test(stderr)) { resolve(); return }
+        reject(new Error(`devtunnel port create failed (exit ${code}): ${stderr.trim() || 'no output'}`))
+      })
+    })
+  }
+
+  /** Start the long-running `devtunnel host` process and resolve once its
+   *  public URL appears in stdout. */
+  private runHost(localPort: number): Promise<{ publicUrl: string }> {
+    const spawnImpl = this.opts.spawnImpl ?? nodeSpawn
+    // A persistent tunnel already has its port registered (ensurePortRegistered
+    // above); passing -p again here is what triggers the "batch update" error.
+    // An ad-hoc/temporary session (no tunnelId) still needs -p at host time.
+    const args = this.opts.tunnelId
+      ? ['host', this.opts.tunnelId]
+      : ['host', '--allow-anonymous', '-p', String(localPort)]
     const proc = spawnImpl(this.opts.binPath, args, { stdio: ['ignore', 'pipe', 'pipe'] })
     this.proc = proc
 
@@ -87,3 +129,4 @@ export class DevTunnelProvider implements TunnelProvider {
   onUrlChange(cb: (publicUrl: string) => void): void { this.urlHandlers.add(cb) }
   isRunning(): boolean { return this.proc !== null }
 }
+