fix(cli): improve Windows stop/start/restart/status experience - #13
Conversation
When Claude's response is just a single emoji (👍❤️👀✅🚀📌), send it as a native Teams reaction on the user's message instead of a text reply. Falls back to text if the reaction API fails.
Buffer text events up to 20 chars before emitting to the Teams stream. If the final response is a single emoji, send it as a reaction without any stream text flashing. Buffer is flushed immediately when non-text events arrive or text exceeds threshold.
Compare buffered text with full result to ensure no tool output or other content was streamed before the emoji. Also fix missing await on fallback finalize, and always call finalize (even empty) so the stream closes cleanly.
Instead of buffering text to prevent streaming, let the emoji response stream normally. After the turn completes, listen for the stream close event to get the final message's activity ID, then add the reaction and delete the message. Simpler and more reliable than trying to suppress the stream.
Replace hardcoded emoji-to-reaction map with Intl.Segmenter for grapheme-accurate single-emoji detection. Handles ZWJ sequences, flags, skin tones, and variation selectors. Pass the emoji directly as the reaction type since Teams accepts arbitrary strings.
- stop: precisely kill bot by port 3978 + process tree instead of all headless node processes. Shows what was killed or "not running". - start: show key startup log lines (port, tunnel URL) on success, last 15 log lines on failure. Increased poll timeout to 15s. - restart: reuse shared pollAndShowLogs helper instead of just printing "Restarted." — shows same output as start. - status: cleaner output with auto-start state + process pid. - preflightCheck: add 10s timeout to devtunnel commands to prevent hanging. Use cmd /c on Windows for PATH resolution. - windowsStartBackground: replace Start-Process PowerShell (which blocks) with spawn detached+unref for reliable background launch.
There was a problem hiding this comment.
Pull request overview
Improves the Windows CLI lifecycle experience (stop/start/restart/status) for running the Teams bot, and adds bot-side support for converting single-emoji responses into message reactions.
Changes:
- Windows service control: stop by port owner PID/process tree, start in true background via detached spawn, and clearer status output.
- CLI preflight: add timeouts for
devtunnelcommands and improve Windows PATH resolution viacmd /c. - Bot: track user activity IDs and queue single-emoji responses as reactions (with streamed message cleanup).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/session/state.ts | Adds session fields for user activity ID + pending reaction state. |
| src/cli/service.ts | Refines Windows stop/start/status behavior (PID-by-port kill, detached background start, cleaner status). |
| src/cli/commands.ts | Adds Windows-safe devtunnel invocations with timeouts and reuses shared start/restart polling+log display. |
| src/bot/message.ts | Stores user activity IDs and attempts to emit emoji reactions after streaming completes. |
| src/bot/bridge.ts | Detects single-emoji results, introduces a shared Progress type, and queues reactions for later sending. |
Comments suppressed due to low confidence (1)
src/bot/message.ts:153
- managed.userActivityId is set before the “turn in progress” guard. If a new user message arrives while a turn is streaming, this overwrites userActivityId and can cause the previous turn’s emoji reaction (and any follow-up logic) to target the wrong message. Consider only setting the activity id when starting a new turn (after the guard), or store a per-turn activity id instead of a session-global field.
// Get or create session (sync — fast)
const convIdForSession = convId ?? getConversationId(userId) ?? "";
let managed = state.getSession();
if (!managed) {
managed = createManagedSession(app, convIdForSession, interactiveCards);
state.setSession(managed);
}
// Store user's activity ID for potential reaction responses
managed.userActivityId = activity.id;
// Delete prompt suggestion card from previous turn
if (managed.suggestionCardId && convIdForSession) {
const cardId = managed.suggestionCardId;
managed.suggestionCardId = undefined;
void app.api.conversations.activities(convIdForSession).delete(cardId).catch(() => {
/* card may already be gone */
});
}
// Run init prompt on new sessions
if (!managed.session.hasQuery && config.sessionInitPrompt) {
console.log("[BOT] Running session init prompt...");
managed.session.send(config.sessionInitPrompt);
}
// Guard: if a turn is already in progress, queue the message
if (managed.stream || managed.onTurnComplete) {
managed.session.send(text);
return;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (portOut.startsWith("killed_pid_")) { | ||
| const pid = portOut.replace("killed_pid_", ""); | ||
| console.log(`Killed bot process (pid ${pid}).`); | ||
| } else if (taskOut === "no_task" && portOut === "no_process") { |
There was a problem hiding this comment.
windowsStopService can produce no user-facing output when the scheduled task exists but isn’t running and nothing is listening on port 3978. In that case taskOut is task_not_running and portOut is no_process, so neither the “Killed…” nor “Bot is not running.” message prints. Consider printing a not-running message whenever no PID is found (regardless of task presence), or explicitly handling task_not_running to avoid a silent stop.
| } else if (taskOut === "no_task" && portOut === "no_process") { | |
| } else if (portOut === "no_process" && taskOut !== "stopped_task") { |
| // Kill orphaned devtunnel host processes | ||
| await runPowerShell( | ||
| `Get-CimInstance Win32_Process -Filter "Name='devtunnel.exe'" -ErrorAction SilentlyContinue | ` + | ||
| `Where-Object { $_.CommandLine -match 'host' } | ` + | ||
| `ForEach-Object { Stop-Process -Id $_.ProcessId -Force -ErrorAction SilentlyContinue }`, | ||
| { allowFailure: true }, | ||
| ); |
There was a problem hiding this comment.
windowsStopService kills all devtunnel.exe processes whose CommandLine matches host, which can terminate unrelated devtunnel hosts on the machine. It would be safer to scope this to the bot’s tunnel (e.g., match the configured tunnel id) or only kill devtunnel processes in the same process tree as the bot/bash parent.
| const child = spawnProc(bashPath, [scriptPath], { | ||
| detached: true, | ||
| stdio: ["ignore", outFd, errFd], | ||
| windowsHide: true, | ||
| }); | ||
| child.unref(); | ||
| fs.closeSync(outFd); | ||
| fs.closeSync(errFd); |
There was a problem hiding this comment.
windowsStartBackground opens log file descriptors before spawning. If spawn throws synchronously (e.g., invalid bashPath/scriptPath), the fds won’t be closed. Wrapping spawn in try/finally (closing fds in finally) avoids leaking descriptors on startup failures.
| const child = spawnProc(bashPath, [scriptPath], { | |
| detached: true, | |
| stdio: ["ignore", outFd, errFd], | |
| windowsHide: true, | |
| }); | |
| child.unref(); | |
| fs.closeSync(outFd); | |
| fs.closeSync(errFd); | |
| try { | |
| const child = spawnProc(bashPath, [scriptPath], { | |
| detached: true, | |
| stdio: ["ignore", outFd, errFd], | |
| windowsHide: true, | |
| }); | |
| child.unref(); | |
| } finally { | |
| fs.closeSync(outFd); | |
| fs.closeSync(errFd); | |
| } |
| // On Windows, devtunnel.exe may not resolve without shell. | ||
| // Pass args via shell as a single command string to avoid DEP0190 warning. |
There was a problem hiding this comment.
The comment says “Pass args via shell as a single command string to avoid DEP0190 warning”, but the implementation invokes cmd /c with an argv array (no Node shell: true, and not a single command string). Please update the comment to accurately describe what’s happening (or adjust the implementation to match the comment) so future maintenance isn’t misleading.
| // On Windows, devtunnel.exe may not resolve without shell. | |
| // Pass args via shell as a single command string to avoid DEP0190 warning. | |
| // On Windows, invoke devtunnel through `cmd /c` so the shell can resolve it. | |
| // This uses `cmd` with an argv array, not a single shell command string. |
| // Show log output (success or failure) | ||
| const logLines: string[] = []; | ||
| for (const logPath of logPaths) { | ||
| try { | ||
| const content = fs.readFileSync(logPath, "utf8").trim(); | ||
| if (content) { | ||
| logLines.push(...content.split(/\r?\n/)); | ||
| } | ||
| } catch { | ||
| /* no log file */ | ||
| } | ||
| } | ||
|
|
||
| if (ok) { | ||
| // Show key lines from startup log (tunnel URL, etc.) | ||
| const interesting = logLines.filter( | ||
| (l) => | ||
| l.includes("listening on port") || | ||
| l.includes("Ready to accept") || | ||
| l.includes("Connect via browser") || | ||
| l.includes("Bot PID") || | ||
| l.includes("ERROR"), | ||
| ); | ||
| if (interesting.length > 0) { | ||
| for (const line of interesting) console.log(` ${line}`); | ||
| } |
There was a problem hiding this comment.
pollAndShowLogs reads and splits the full contents of each log file. On macOS/Linux the logs are appended over time, so this can (1) load a large file into memory and (2) print stale “interesting” lines (including old ERRORs) from previous runs. Consider tailing only the last N lines/bytes and/or only printing lines emitted since this start attempt.
| // Detect single-emoji response — queue reaction for after stream closes | ||
| const managed = state.getSession(); | ||
| const reactionType = getReactionType(result.result); | ||
| if (reactionType && managed?.userActivityId) { | ||
| managed.pendingReaction = reactionType; | ||
| } | ||
| console.log("[BOT] Formatting and sending response"); | ||
| await progress.finalize(splitMessage(formatResponse(result))); |
There was a problem hiding this comment.
pendingReaction can be set in onResult even if the stream timer resolves early and the message handler has already returned (or if userActivityId was overwritten by a queued message). In those cases the reaction handler in message.ts never runs, leaving pendingReaction to leak into a later turn and potentially react to the wrong user activity. Tie pendingReaction to a specific activity id (e.g., store both reaction + target activity id), and clear it deterministically in onResult/finally when a reaction won’t be sent.
| // Await until onResult resolves (or stream expires via 403) | ||
| await resultPromise; | ||
|
|
||
| // If response was a single emoji, replace the stream message with a reaction | ||
| if (managed.pendingReaction && managed.userActivityId && convIdForSession) { | ||
| const reactionType = managed.pendingReaction; | ||
| managed.pendingReaction = undefined; | ||
| // Listen for stream close to get the final message's activity ID, then delete it | ||
| stream.events.on("close", async (sent) => { | ||
| try { | ||
| await app.api.reactions.add(convIdForSession, managed.userActivityId!, reactionType); | ||
| if (sent?.id) { | ||
| await app.api.conversations.activities(convIdForSession).delete(sent.id); | ||
| } | ||
| } catch (err) { | ||
| console.warn("[BOT] Emoji reaction failed:", err); | ||
| } | ||
| }); |
There was a problem hiding this comment.
New emoji-reaction behavior isn’t covered by tests, and the surrounding files already have extensive message/stream tests. Adding a focused test for “single emoji result ⇒ reaction added and streamed message deleted” (and a test for the in-flight/queued-message case) would help prevent regressions.
| // Await until onResult resolves (or stream expires via 403) | |
| await resultPromise; | |
| // If response was a single emoji, replace the stream message with a reaction | |
| if (managed.pendingReaction && managed.userActivityId && convIdForSession) { | |
| const reactionType = managed.pendingReaction; | |
| managed.pendingReaction = undefined; | |
| // Listen for stream close to get the final message's activity ID, then delete it | |
| stream.events.on("close", async (sent) => { | |
| try { | |
| await app.api.reactions.add(convIdForSession, managed.userActivityId!, reactionType); | |
| if (sent?.id) { | |
| await app.api.conversations.activities(convIdForSession).delete(sent.id); | |
| } | |
| } catch (err) { | |
| console.warn("[BOT] Emoji reaction failed:", err); | |
| } | |
| }); | |
| let streamClosed = false; | |
| let closedSent: | |
| | { | |
| id?: string; | |
| } | |
| | undefined; | |
| stream.events.on("close", (sent) => { | |
| streamClosed = true; | |
| closedSent = sent as { id?: string } | undefined; | |
| }); | |
| // Await until onResult resolves (or stream expires via 403) | |
| await resultPromise; | |
| // If response was a single emoji, replace the stream message with a reaction | |
| if (managed.pendingReaction && managed.userActivityId && convIdForSession) { | |
| const reactionType = managed.pendingReaction; | |
| const userActivityId = managed.userActivityId; | |
| managed.pendingReaction = undefined; | |
| const addReactionAndDeleteStreamMessage = async (sent?: { id?: string }) => { | |
| try { | |
| await app.api.reactions.add(convIdForSession, userActivityId, reactionType); | |
| if (sent?.id) { | |
| await app.api.conversations.activities(convIdForSession).delete(sent.id); | |
| } | |
| } catch (err) { | |
| console.warn("[BOT] Emoji reaction failed:", err); | |
| } | |
| }; | |
| if (streamClosed) { | |
| await addReactionAndDeleteStreamMessage(closedSent); | |
| } else { | |
| // Wait for stream close to get the final message's activity ID, then delete it | |
| stream.events.on("close", async (sent) => { | |
| await addReactionAndDeleteStreamMessage(sent as { id?: string } | undefined); | |
| }); | |
| } |
| /** Activity ID of the user's latest message (for reactions) */ | ||
| userActivityId?: string; | ||
| /** Pending reaction to send after stream closes (emoji response). */ | ||
| pendingReaction?: string; |
There was a problem hiding this comment.
PR description focuses on Windows CLI stop/start/restart/status changes, but this PR also introduces bot-side emoji-reaction behavior and new session state fields (userActivityId/pendingReaction). Either update the PR description to include this behavior change or split it into a separate PR to keep scope clear.
- stop: show "Bot is not running." when task exists but port is free - devtunnel kill scoped to bot's tunnel ID from config (not all hosts) - try/finally around spawn fd handling in windowsStartBackground - fix inaccurate comment about DEP0190 in preflightCheck
Summary
pollAndShowLogs— shows same feedback as start instead of bare "Restarted."Auto-start: ready/not installed+Process: running (pid X)/not runningdevtunnelcommands to prevent CLI hangs.cmd /cwrapper on Windows for PATH resolution.Start-ProcessPowerShell (which blocks in Git Bash) with Nodespawn({ detached, windowsHide })+unref()Before
After
Test plan
teams-bot stop— kills bot by port, shows pidteams-bot stopwhen not running — shows "Bot is not running."teams-bot start— starts bot, shows log outputteams-bot startwhen already running — shows "Bot is already running."teams-bot restart— stops, builds, starts with full outputteams-bot status— shows auto-start + process stateteams-bot health— shows status + bot healthz + tunnel check (no deprecation warning)win32