Skip to content

fix(slack): preserve complete executor summaries safely#92

Merged
mingyooagi merged 4 commits into
mainfrom
fix/slack-summary-truncation
Jul 14, 2026
Merged

fix(slack): preserve complete executor summaries safely#92
mingyooagi merged 4 commits into
mainfrom
fix/slack-summary-truncation

Conversation

@gabiIsCoding

@gabiIsCoding gabiIsCoding commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Problem

Slack final replies could silently discard most executor output: only the first paragraph was selected, the remaining text was capped at 360 characters, and shared whitespace normalization collapsed line breaks.

Fix

  • Preserve the complete final summary, including paragraph breaks.
  • Prefer the dedicated What changed section when the executor returns a structured summary.
  • Normalize CRLF and lone CR without flattening multiline output.
  • Convert Markdown to Slack mrkdwn before applying a 2500-character summary cap, leaving room under Slack's 3000-character section limit.
  • Truncate on Unicode code-point boundaries so emoji cannot leave lone surrogate characters.
  • Keep shared compact fields such as next actions on one line.
  • Mark provider-rendered callback bodies as Slack mrkdwn so chat.postMessage and chat.update do not encode entities and links twice.
  • Escape plain Slack fallbacks for doctor summaries and source-thread status without changing their blocks.

Regression coverage

  • Multi-paragraph summaries and blank lines are preserved.
  • Escaping expansion remains within the Slack section limit.
  • Emoji boundary truncation produces well-formed Unicode.
  • Compact one-line fields still collapse whitespace.
  • The complete presentation-to-callback path preserves entities and Slack links for both post and update payloads.
  • Doctor and source-thread Slack fallbacks escape entities and convert Markdown links while other providers stay plain.

Verification

  • pnpm build
  • pnpm lint
  • pnpm typecheck
  • pnpm test — 119 test files, 1517 tests passed
  • Focused Slack renderer and dispatcher callback suites — 82 tests passed
  • git diff --check

Files changed

  • packages/slack/src/render.ts
  • packages/slack/test/render.test.ts
  • packages/dispatcher/src/callbacks.ts
  • packages/dispatcher/test/callbacks.test.ts
  • packages/dispatcher/src/presentation.ts
  • packages/dispatcher/test/presentation.test.ts

Summary by CodeRabbit

  • Bug Fixes
    • Improved Slack “Finished”/final summary rendering: consistent newline normalization, better selection of the “What changed” section, cleaner header handling, and safer mrkdwn truncation within Slack limits.
    • Prevented double-encoding by introducing an explicit textFormat option for Slack message payloads (and setting it to mrkdwn in the callback flow).
    • Updated Slack doctor/source-thread presentations to render bodies as Slack-compatible MRKDWN when supported.
  • Tests
    • Added/expanded Vitest coverage for Slack Block Kit compliance, CRLF/CR newline behavior, Unicode-safe ellipsis truncation, compact “next action” formatting, and mrkdwn escaping in both top-level text and blocks.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Slack final summaries now normalize multiline content, select compact sections, truncate rendered mrkdwn to 2500 characters, and use explicit mrkdwn handling for Slack callbacks and Slack presentation fallbacks.

Changes

Slack rendering and callback delivery

Layer / File(s) Summary
Slack text and summary formatting
packages/slack/src/render.ts
Compact summaries normalize content, select the What changed section when available, remove leading headers, and truncate rendered mrkdwn to 2500 characters for string and Block Kit final summaries.
Explicit mrkdwn payload handling
packages/slack/src/render.ts, packages/dispatcher/src/callbacks.ts
Post and update payload builders accept textFormat; Slack callback delivery marks pre-rendered content as mrkdwn to avoid double conversion.
Slack presentation fallback formatting
packages/dispatcher/src/presentation.ts
Doctor and source-thread Slack fallback bodies are converted to Slack-compatible mrkdwn while other providers retain their existing output behavior.
Rendering and callback validation
packages/slack/test/render.test.ts, packages/dispatcher/test/callbacks.test.ts, packages/dispatcher/test/presentation.test.ts
Tests cover escaping, Slack length limits, newline normalization, Unicode-safe truncation, compact next-action text, provider-specific output, and correctly encoded callback payloads.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

Suggested reviewers: mingyooagi

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is concise and accurately reflects the main change: safely preserving complete Slack executor summaries.
Description check ✅ Passed The description includes the needed problem, fix, validation, and notes-style details, though it uses different headings than the template.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/slack-summary-truncation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the Slack text rendering logic by changing how text is normalized and increasing the summary truncation limit from 360 to 2800 characters. Feedback suggests lowering the truncation limit to 2500 to safely accommodate character escaping and prepended text within Slack's 3000-character limit. Additionally, it is recommended to update the line ending normalization regex to handle classic Mac OS line endings as well.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread packages/slack/src/render.ts Outdated
Comment thread packages/slack/src/render.ts Outdated
gabiIsCoding and others added 2 commits July 13, 2026 13:51
Slack presentation bodies are already provider-rendered, so callback delivery now marks them as mrkdwn instead of encoding entities and links a second time. Final-summary normalization is separated from shared one-line truncation, and the only length cap now runs after mrkdwn rendering on code-point boundaries.

Constraint: Slack section summaries must stay below the 3000-character provider limit

Rejected: Re-encode callback bodies in payload builders | corrupts escaped entities and Slack link syntax

Rejected: Truncate source summaries before mrkdwn rendering | can split Unicode and undercount escaped output

Confidence: high

Scope-risk: narrow

Reversibility: clean

Directive: Keep raw Markdown payload callers on the default format and mark provider-rendered callback bodies as mrkdwn

Tested: full build, lint, typecheck, 1516 tests, focused Slack post/update payload regressions

Not-tested: live Slack API delivery
@mingyooagi mingyooagi changed the title fix(slack): show full executor output instead of 360-char truncated first paragraph fix(slack): preserve complete executor summaries safely Jul 14, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/dispatcher/src/callbacks.ts`:
- Around line 462-469: Escape the fallback Slack text before passing
message.body to createSlackPostMessagePayload with textFormat "mrkdwn". Update
the relevant callback path, including doctor_summary and source_thread_status
flows, to apply the existing Slack mrkdwn escaping utility while preserving the
current blocks and message structure.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 40ace511-90d2-4da8-a581-2d06fbd03b57

📥 Commits

Reviewing files that changed from the base of the PR and between 824a6b3 and f0c442a.

📒 Files selected for processing (4)
  • packages/dispatcher/src/callbacks.ts
  • packages/dispatcher/test/callbacks.test.ts
  • packages/slack/src/render.ts
  • packages/slack/test/render.test.ts

Comment thread packages/dispatcher/src/callbacks.ts
Doctor and source-thread status presentations still supplied raw plain text as
their Slack fallback while other callback bodies were already rendered as
mrkdwn. Convert only those two provider-specific fallbacks at the presentation
boundary so the shared callback sink can preserve its existing transport
contract.

Constraint: The Slack callback sink receives both raw semantic fallbacks and already-rendered mrkdwn bodies.
Rejected: Escape every message.body in createSlackCallbackSink | this would double-encode final, action, approval, and run-status callbacks.
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep Slack mrkdwn ownership in provider presentation renderers; do not globally re-encode callback bodies.
Tested: 82 focused tests; dispatcher build and lint; full typecheck; 119 files and 1517 full tests.
Not-tested: Live Slack API delivery.
@mingyooagi
mingyooagi merged commit 03a12a7 into main Jul 14, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants