fix(mcp): name chats in list_chats the way the UI does, not an 8-char id slice (#614) - #626
Open
edspencer wants to merge 1 commit into
Open
fix(mcp): name chats in list_chats the way the UI does, not an 8-char id slice (#614)#626edspencer wants to merge 1 commit into
edspencer wants to merge 1 commit into
Conversation
… id slice (#614) The MCP projection resolved a chat's name as `customName ?? autoName ?? sessionId.slice(0, 8)`, omitting the `preview` step the REST DTO has (`chat-dto.ts:241`). So for the same chat the web UI showed prose and MCP showed a hash. `autoName` cannot cover the gap: core derives it only from `type:"summary"` records, and current Claude Code writes `ai-title` instead. Across the 1,704 transcripts on this instance, 1,649 carry an `ai-title` and 0 carry a `summary` — so the chain effectively ran `customName ?? slice(0,8)` and 39 of 96 listed chats (41%) came back named after their own sessionId. That is worse than ugly. The stub LOOKS like an id, and feeding one back to `read_chat` returns a successful empty result rather than an error, so a caller can conclude "empty chat" about a conversation it never opened. It also silently broke a documented instruction: the Night-Watch reviewer is told to skip chats whose name contains "night-watch", and the prior run's actual title is "Night-Watch nightly quality review system" — the filter could not fire, two nights running, and it burned a read re-identifying the chat by content instead. - Extract the preload/attachment preview recovery out of `buildProjectChats` into a shared `recoverPreview`, so the REST DTO and the MCP list run the same logic and cannot drift on this again. - Use the full chain in `listChats`, including the raw-preview fallback, so MCP and the UI agree case for case — the truncated-wrapper wart included, which is pinned by a test rather than quietly "improved" on one surface. - Only polluted previews pay for the extra head-of-file read; everything else returns without I/O, so `list_chats` stays cheap. This is half of #614. The other half — teaching `autoName` to read `ai-title` — lives in `@herdctl/core` and needs a coordinated release, so it is deliberately not attempted here. Verified the three behavioural tests fail against the old one-liner rather than passing vacuously. Full server suite 1447 passed; typecheck clean. Co-Authored-By: Claude <noreply@anthropic.com>
Deploying paddock with
|
| Latest commit: |
612c4e3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ffffff1e.paddock-7u2.pages.dev |
| Branch Preview URL: | https://fix-614-list-chats-preview-n.paddock-7u2.pages.dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Half of #614 — the half that doesn't need a cross-repo release.
The problem
The MCP projection resolved a chat's name as
customName ?? autoName ?? sessionId.slice(0, 8)(management-ops.ts:199), omitting thepreviewstep the REST DTO has (chat-dto.ts:241). For the same chat, the web UI showed prose and MCP showed a hash.autoNamecan't cover the gap. Core derives it only fromtype: "summary"records, and current Claude Code writesai-titleinstead. Measured across all 1,704 transcripts on this instance:ai-titlerecordsummaryrecordSo the chain effectively ran
customName ?? slice(0,8), and 39 of 96 (41%) chats in a reallist_chats {}response came back named after their own sessionId — 18 paddock, 9 coderabbit, 8 herdctl, 2 edspencer-net, 1 hushpod, 1 eightsleep.This is worse than ugly. The stub looks like an id, and feeding one back to
read_chatreturns a successful empty result rather than an error — so a caller can conclude "empty chat" about a conversation it never opened.It also silently broke a documented instruction. The Night-Watch reviewer is told to skip chats whose name contains "night-watch". The prior run's actual title is "Night-Watch nightly quality review system" — the filter could not fire, two nights running, and it spent a
read_chatre-identifying the chat by content instead. Both reports say so out loud: "hash-named, evades the name filter" / "hash-named, content-matched".The change
buildProjectChatsinto a sharedrecoverPreview, so the REST DTO and the MCP list run the same logic and can't drift on this again. (The drift is the bug — a second copy of the chain would just re-introduce it later.)listChats, including the raw-preview fallback, so MCP and the UI agree case for case.list_chatsstays cheap as advertised.One deliberate non-improvement
When a preload wrapper is truncated before the
My request:marker and the untruncated message can't be read from disk, there's nothing to recover and both surfaces fall through to the raw<project-context>…preview. That's a wart — but the point of #614 is that the two surfaces must not disagree, so I matched it and pinned it with a test rather than quietly making one surface "nicer". Worth fixing separately if you want it.What this does NOT do
Teaching
autoNameto readai-titleis the other half. It lives in@herdctl/coreand needs a herdctl PR, a publish and a bump here — a coordinated release I didn't want to bundle into this. So chats with no title and no preview still fall back to the slice; that's correct behaviour for a genuinely nameless chat. #614 stays open for theai-titlework.Also not attempted: the extra triage fields (
messageCount,errorCount) from the issue.errorCountmeans parsing every transcript per list call — a performance design question, not a code change.Verification
management-ops-chat-name.test.ts: 7 passedexpected 'cccccccc' to be 'Audit the Night-Watch run against the…') rather than passing vacuouslychat-dtoextraction breaks nothingnpm run typecheck: clean (server + web)Companions: #613 (the
read_chatfidelity defect) and #625 (descriptions, CI green).