fix(slash): wrap sessions/resume handler in try/finally for robustness (#6224)

[webtecnica]

The /sessions and /resume slash intercept (from #6245) didn't wrap the handler body. If renderSessionList() throws (e.g. network error), the composer stays dirty and the dropdown remains open.

Fix: Wrap in try/finally — cleanup always runs.

Refs #6224

[greptile-apps]

Greptile Summary

This PR wraps the /sessions+/resume slash-command handler in messages.js inside a try/finally so the composer and dropdown are always cleaned up even when renderSessionList() rejects. It also ships a new markdown inline-preview feature (loadMarkdownInline in ui.js, supporting CSS in style.css, MIME entries in api/config.py, and session-media gating in api/routes.py).

• messages.js: The try/finally fix is correct — cleanup runs unconditionally and the return is correctly placed outside the finally block so exceptions still propagate.
• ui.js: loadMarkdownInline fetches the file, enforces a 256 KiB character limit, and renders via renderMd. The _tag() allowlist sanitises all HTML before DOM injection, so the rendered content is safe.
• api/config.py + api/routes.py: Adds .md/.mkd/.mkdn MIME types and gates markdown files behind the existing session-media token, consistent with the HTML/PDF/image pattern.

Confidence Score: 5/5

Safe to merge — the try/finally fix is mechanically correct and the markdown preview sanitises rendered HTML through the existing _tag() allowlist before DOM injection.

The messages.js change is a one-line structural fix with no logic risk; the return correctly sits outside the finally block. The markdown preview follows the same fetch-then-replace pattern as the existing HTML/PDF loaders and inherits the same sanitisation pipeline.

static/ui.js — the MD_MAX_SIZE character-count limit and the duplicate filename in the preview header are worth a second look.

Important Files Changed

Filename	Overview
static/messages.js	try/finally wraps the /sessions+/resume handler; return is correctly outside the finally block so exceptions propagate normally
static/ui.js	Adds loadMarkdownInline: fetches file, size-checks on string length (not bytes), renders via sanitising renderMd pipeline; header shows filename twice in title span and download link
api/routes.py	Adds text/markdown to _SESSION_MEDIA_TOKEN_TYPES, consistent with existing HTML/PDF/image gating pattern
api/config.py	Adds .md/.mkd/.mkdn MIME entries to MIME_MAP; straightforward extension of the existing map
static/style.css	New CSS classes for markdown inline preview; follows the pattern of existing html-preview-* classes

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant User
    participant SendFn as "send() in messages.js"
    participant RSL as "renderSessionList()"
    participant DOM

    User->>SendFn: types /sessions or /resume
    SendFn->>SendFn: try - open session browser sidebar
    SendFn->>RSL: await renderSessionList()
    alt success
        RSL-->>SendFn: resolves
        SendFn->>DOM: finally - clear composer, autoResize, hideCmdDropdown
        SendFn->>SendFn: return
    else throws or rejects
        RSL-->>SendFn: rejects
        SendFn->>DOM: finally - clear composer, autoResize, hideCmdDropdown
        SendFn->>SendFn: exception propagates as unhandled rejection
    end

    Note over DOM: loadMarkdownInline in ui.js
    DOM->>DOM: querySelectorAll .md-inline-load
    DOM->>DOM: setAttribute data-loaded 1
    DOM->>DOM: "fetch api/media?path=..."
    alt ok and within MD_MAX_SIZE chars
        DOM->>DOM: renderMd sanitises via _tag allowlist
        DOM->>DOM: "el.outerHTML = md-inline-wrap"
    else too large
        DOM->>DOM: "el.outerHTML = fallback download link"
    else fetch error
        DOM->>DOM: "el.outerHTML = error fallback"
    end

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant User
    participant SendFn as "send() in messages.js"
    participant RSL as "renderSessionList()"
    participant DOM

    User->>SendFn: types /sessions or /resume
    SendFn->>SendFn: try - open session browser sidebar
    SendFn->>RSL: await renderSessionList()
    alt success
        RSL-->>SendFn: resolves
        SendFn->>DOM: finally - clear composer, autoResize, hideCmdDropdown
        SendFn->>SendFn: return
    else throws or rejects
        RSL-->>SendFn: rejects
        SendFn->>DOM: finally - clear composer, autoResize, hideCmdDropdown
        SendFn->>SendFn: exception propagates as unhandled rejection
    end

    Note over DOM: loadMarkdownInline in ui.js
    DOM->>DOM: querySelectorAll .md-inline-load
    DOM->>DOM: setAttribute data-loaded 1
    DOM->>DOM: "fetch api/media?path=..."
    alt ok and within MD_MAX_SIZE chars
        DOM->>DOM: renderMd sanitises via _tag allowlist
        DOM->>DOM: "el.outerHTML = md-inline-wrap"
    else too large
        DOM->>DOM: "el.outerHTML = fallback download link"
    else fetch error
        DOM->>DOM: "el.outerHTML = error fallback"
    end

Loading

_{Reviews (2): Last reviewed commit: "fix: move return after finally to preser..." | Re-trigger Greptile}

[greptile-apps]

renderMd applies LLM-oriented heuristics to user-authored file content

renderMd runs two rounds of entity-decoding (lines 7090 and 7164–7165) before HTML-tag processing. This means a markdown file that legitimately contains <script> (to display the literal text <script>) will have the entity decoded to <script>, which is then stripped entirely by _tag(). The displayed text becomes empty instead of <script>. For LLM-produced chat messages this is the right trade-off, but for user-authored .md files it silently corrupts intentional escape sequences. A renderer without the double-decode pre-pass (or with a separate sanitise-only path) would be more faithful to the file's intent.

[greptile-apps]

PR bundles two unrelated logical changes

AGENTS.md states "Keep one logical change per PR; split unrelated refactors or cleanup." This PR's stated fix is wrapping the /sessions+/resume handler in try/finally (the one-line change in messages.js), but it also introduces a complete markdown inline-preview feature spanning api/config.py, api/routes.py, static/style.css, and a new loadMarkdownInline function in static/ui.js. These two changes are independent and would be easier to review, revert, and bisect if split into separate PRs.

Context Used: AGENTS.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[cutter-sh]

🎬 Cutter preview — PR #6278

/chat — Markdown attachments now render inline in chat with formatted headings, lists, code, and tables.

[nesquena-hermes]

Summary

The cleanup goal is correct, but the return inside this finally suppresses every exception raised while opening or refreshing the session browser. If _openProfileSwitchSessionBrowser, expandSidebar, or the awaited renderSessionList fails, send() now resolves successfully after clearing the composer, leaving the user with no drawer and no error signal.

Code references

• static/messages.js:1469-1478 wraps the browser-open path and ends the finally body with return.
• static/messages.js:1322-1327 starts the outer send() guard, while static/messages.js:1905 only has an outer finally that releases _sendInProgress; there is no outer catch that can recover an error hidden by the inner return.
• The existing profile-switch test contract at tests/test_profile_switch_ux.py:181-196 treats list refresh followed by opening the browser as the successful sequence. A failed refresh should not be converted into that same successful result.

In JavaScript, abrupt completion from a finally block overrides a pending throw. Keep cleanup in finally, but move the normal return after it:

try {
  // open the session browser and await renderSessionList()
} finally {
  document.getElementById('msg').value = '';
  autoResize();
  hideCmdDropdown();
}
return;

That preserves the requested composer/dropdown cleanup on both paths while allowing failures to propagate to whatever invokes send(). If the intended UX is to consume these failures, add an explicit catch that logs or shows a retryable error instead of consuming them implicitly in finally.

Suggested verification

Add a focused test with renderSessionList rejecting: the composer must be cleared and the dropdown hidden, but the send() promise must reject (or an explicit catch must surface user feedback). Add the corresponding success case to confirm the command returns without falling through to agent command lookup. I did not run code under this read-only triage policy; this finding is based on JavaScript try/finally completion semantics and the complete send() control flow.