[AI-122] - escalate a conversation to a human

[stevenarnold-ed-fi]

No description provided.

[Copilot]

Pull request overview

Adds the /fiona escalate capability and supporting infrastructure so Slack users can escalate a conversation to a human support channel, including optional on-call user-group mention and best-effort analytics persistence.

Changes:

• Wires new escalation configuration (ESCALATION_CHANNEL, ESCALATION_USERGROUP_ID) through Bicep, .env.sample, manifest usage hint, and PRD docs.
• Introduces escalation flow building blocks: a non-streaming summarizeForEscalation() helper and a shared postEscalation() helper that posts summary + transcript and records to Cosmos.
• Updates usage reporting to exclude escalation rows from feedback response-rate calculations, with unit test coverage.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
infra/fiona-slack-container/main.bicep	Adds parameters and container env vars for escalation channel + user group.
docs/superpowers/plans/2026-06-22-escalate-to-human-slash-command.md	Detailed implementation plan for the escalation slash command and helpers.
docs/superpowers/plans/2026-06-22-escalate-to-human-design.md	Design doc for escalate-to-human flow and future proactive escalation.
docs/fiona-skills-prd.md	Updates environment variable table to include escalation vars (ID-based).
apps/usage-report-function/lib/cosmos-queries.js	Filters feedback response-rate query to exclude escalation rows.
apps/usage-report-function/test/unit/cosmos-queries.test.js	Adds unit test asserting feedback allow-list filter is applied.
apps/fiona-slack/src/agent/llm-caller.js	Adds summarizeForEscalation() helper (non-streaming) for escalation summaries.
apps/fiona-slack/src/agent/feedback-store.js	Adds optional interactionType field to feedback docs.
apps/fiona-slack/src/agent/escalation.js	New postEscalation() helper: transcript fetch, summary, Slack posting, Cosmos recording.
apps/fiona-slack/src/agent/interaction-store.js	Updates JSDoc to include escalation interaction types.
apps/fiona-slack/src/listeners/commands/fiona.js	Adds escalate sub-command with rate limiting + DM detection + delegation.
apps/fiona-slack/manifest.json	Updates /fiona usage hint to include escalate.
apps/fiona-slack/.env.sample	Documents escalation-related env vars.
apps/fiona-slack/tests/agent/llm-caller.summarize.test.js	New unit tests for summarizeForEscalation() success/empty/error paths.
apps/fiona-slack/tests/agent/llm-caller.summarize-noclient.test.js	New unit test for summarize helper when LLM client is unconfigured.
apps/fiona-slack/tests/agent/feedback-store-cosmos.test.js	Adds tests for optional interactionType persistence behavior.
apps/fiona-slack/tests/agent/escalation.test.js	New tests for postEscalation() behavior (posting, pinging, recording, DM behavior).
apps/fiona-slack/tests/listeners/commands/fiona.test.js	Mocks escalation helper and adds /fiona escalate command tests.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[stevenarnold-ed-fi]

Team PR Review — AI-122 escalate a conversation to a human

Jira AI-122 · Reviewed across: Security · Functionality · Maintainability · Usability · Test Coverage

🔴 Critical

• [Functionality + Test] The core requirement — "a summary of recent conversation history from that thread" — is not genuinely met. Slash commands carry no thread_ts, so handleEscalate always passes threadTs: null (fiona.js:170). fetchTranscript then falls into the conversations.history branch (escalation.js:60), capturing the channel's recent top-level messages, not the thread where the user's Fiona conversation lives. The test that "proves" the thread link/transcript (escalation.test.js:467) passes only because it injects a synthetic threadTs: '999.000' the real slash path never produces — so the suite is green while the requirement is unmet.
  Fix: Re-scope to a message shortcut / "escalate this thread" action where thread_ts exists, or derive the conversation from another signal. At minimum, get explicit product-owner sign-off that channel-history-as-transcript is acceptable, and fix the tests to use real slash-path arguments.

🟠 High

• [Functionality] getPermalink is called with trigger_id as message_ts (escalation.js:120); it errors every time, is caught, and degrades to a bare <#channel> link — so AC GitHub Action workflow files and copy of the package.json #1's "link back to channel and thread" becomes channel-only, plus a guaranteed failed API call + misleading warning. Fix: Skip the permalink attempt when no real message ts exists, or resolve a genuine thread ts.
• [Usability] Confirmation hardcodes "#escalation" (fiona.js:30) but the channel is configured by ID and may be named anything / be invisible to the user. Fix: Drop the channel name or resolve the real one (needs product-owner agreement since AC [AI-84]Moves Fiona from Closed repo to OSS #2 specifies the exact string).
• [Usability] escalate is missing from HELP_TEXT (fiona.js:13-18), making the only working command undiscoverable via /fiona help. Fix: Add an /fiona escalate line.
• [Maintainability] fetchTranscript (escalation.js:53-76) reinvents the existing buildThreadHistory() in thread-history.js (which the design doc said to reuse). The two will drift — buildThreadHistory already filters system messages, truncates, and merges consecutive messages; fetchTranscript doesn't. Fix: Reuse a shared core for both.
• [Maintainability] Rate-limit handling (fiona.js:147-162) is copy-pasted from rate-limited-handler.js (exact ephemeral string, minutes formatting, error-record block). Fix: Extract a shared rateLimitMessage(retryAfterMs) helper.
• [Test] The largest piece of new logic — fetchTranscript — runs without a single assertion on its output. The conversations.history branch + .reverse(), the who-labeling, the mention-stripping regex, and the 2900-char truncation are all built but unasserted. Fix: Add a threadTs: null, isDm: false test asserting history is called, ordering is fixed, and transcript body content.

🟡 Medium

• [Security] Any user can copy a channel's recent messages (including other users') into the escalation channel and to Perplexity; DM transcripts likewise. The design doc acknowledges the DM case but not the channel-history case. This is a deliberate sensitive-data egress that should be documented, ideally consent-gated, and confirmed against Perplexity's data-retention terms. (Warrants a human + privacy sign-off.)
• [Functionality] No #escalation default (AC Add Slack CLI configuration: manifest.json, slack.json, .env.sample, and .slack directory guidance #5): unset ESCALATION_CHANNEL returns channel_not_configured (escalation.js:106-108). Documented design decision (store an ID) but a literal AC deviation — needs sign-off on the ticket.
• [Functionality/Test] DM-posts-to-channel behavior is implemented but untested.
• [Usability] Empty ack() then slow postEscalation (transcript fetch + non-streaming LLM + 2 posts) before confirmation → perceived hang / double-submit risk. Give immediate feedback before the slow work.
• [Usability] Silent no-op when hasRequiredFields fails (fiona.js:142-145) — user gets an empty ack then nothing. Send ESCALATE_ERROR_TEXT in that branch.
• [Usability] "When:" uses raw new Date().toISOString() (escalation.js:134). Use Slack's <!date^...> token for localized timestamps.
• [Maintainability] 'slash_escalate' / 'auto_escalation' and the feedback value taxonomy are magic literals duplicated across files and the hardcoded SQL allow-list in another app (cosmos-queries.js:150). Introduce shared constants.
• [Maintainability] escalation.js reads process.env inline (:105, :127) against the module-level-const convention.
• [Maintainability] Two large process-artifact docs committed (970 + 213 lines under docs/superpowers/plans/) embedding already-stale source snapshots. Move to PR description / local.
• [Test] getPermalink failure path untested; escalate "missing required fields → silent return" untested; rate-limit test mutates the shared module-level limiter Map with no cleanup (fragile).

🔵 Low

• [Security] Slack mrkdwn / mention injection via display name + transcript (escalation.js:132, :160) — a <!channel>/<!subteam^…> could mass-ping or spoof "Summary:" lines. Render user content as plain_text or sanitize.
• [Security] Prompt injection into the summarizer (raw transcript as user message). Bounded impact; wrap in delimiters, render result as plain_text.
• [Security] postEscalation isn't rate-limited at the helper level; the documented auto_escalation reuse path could spam. Note in JSDoc or guard inside.
• [Functionality] Split recording ownership (helper records success, handler records errors) is easy to break — document the contract.
• [Usability] Generic "could not escalate… reach out to the team directly" isn't actionable; name a concrete fallback. DM "Where:" falls back to an unopenable <#channel> link — label it "Direct message (transcript below)".
• [Maintainability] Best-effort return ''/return null chains mean a fully-degraded escalation still returns { ok: true } with only scattered warns.
• [Test] JSON.stringify(blocks).toContain(...) matches anywhere; the postMessage text fallback field is never asserted; auto_escalation source has zero coverage; process.env mutated without restore.

⚪ Nit

• DM detection is heuristic; group DMs (G/mpim) aren't handled (degrades safely).
• Mixed fetch limits: named HISTORY_LIMIT = 20 vs inline limit: 50.
• recordFeedback JSDoc value not updated for 'escalation'.
• /fiona escalate <anything> silently discards trailing text users may expect as a note.
• Logger-guard style inconsistency; emoji signifiers correctly paired with text (good a11y).

---

Specialist Verdicts

Reviewer	Verdict
Security	No Critical/High; conscious accept-or-mitigate item is the Medium privacy/data-egress.
Functionality	PARTIAL — plumbing/config/persistence/rate-limit/helper/dual-write are clean, but the central "summary + link from the user's thread" is unmet (slash commands expose no thread_ts).
Maintainability	Functionally clean but dented by two real duplications and cross-app magic-string coupling; agent-execution plan doc shouldn't live in the repo.
Usability	Solid flow with graceful degradation, but two High issues undermine trust when it matters most.
Test Coverage	ADEQUATE — all seven AC #9 items map to assertions, but fetchTranscript runs with no output assertions and one test leaks shared limiter state.

Summary

The escalation infrastructure (shared postEscalation helper, dual Cosmos write, rate limiting, graceful summary degradation, config wiring) is well-built and well-tested. The blocking issue is architectural: a slash command has no thread_ts, so the feature can't escalate "the conversation in that thread" — it falls back to unrelated channel history and a failed thread-permalink call, and the green tests mask this by injecting a synthetic threadTs. Before merge: decide whether to re-scope to a thread-aware trigger or get explicit product-owner acceptance of channel-history semantics; then fix the permalink call, add escalate to help text, reconsider the hardcoded "#escalation" confirmation, and back-fill fetchTranscript tests. The privacy/data-egress deserves a human + privacy sign-off.

---

🤖 This review was substantially AI-generated (team-pr-review fleet). Please have a human reviewer confirm the findings — especially the data-egress decision — before acting on them.