fix(song-ref): don't misparse NetEase collection URLs / trailing-punct ids (#90 follow-up)

Corner-case review of the #90 play-by-id parser found two reachable issues:

- A NetEase playlist/album/artist/toplist/djradio share URL (which reuses ?id=)
  was matched as a SONG id, so pasting one into !play called getSongDetail() on
  a collection id and returned a confusing 'No song found' instead of falling
  back to a normal search. Guard the id= branch to exclude collection pages.
- The id: prefix captured trailing punctuation from a chat paste ('id:12345.' ->
  '12345.'), which then failed to resolve. Strip trailing .,;)] from the id.

Both fall back to safe behavior (plain search / clean id). Tests added for
collection URLs and pasted ids with punctuation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
saopig1andClaude Opus 4.8 committed 2026-06-16 22:06:17 +08:00
1 parent 64328d7bdf
commit 6d56f1f371
2 files changed
+25 -3

No files matched your search

+17
View File
@@ -15,6 +15,23 @@ describe("parseSongRef (#90 exact-song selection)", () => {
expect(parseSongRef("ID: 004Z8Ihr0JIu5s")).toEqual({ id: "004Z8Ihr0JIu5s", platform: null }); expect(parseSongRef("ID: 004Z8Ihr0JIu5s")).toEqual({ id: "004Z8Ihr0JIu5s", platform: null });
}); });
it("strips trailing punctuation from a pasted id:", () => {
expect(parseSongRef("id:185868.")).toEqual({ id: "185868", platform: null });
expect(parseSongRef("id:185868)")).toEqual({ id: "185868", platform: null });
expect(parseSongRef("id:185868,")).toEqual({ id: "185868", platform: null });
});
it("does NOT treat NetEase collection (playlist/album/artist) URLs as a song id", () => {
// These reuse ?id= but are not songs — they should fall through to search,
// not misresolve to getSongDetail(collectionId) and error "no song".
expect(parseSongRef("https://music.163.com/playlist?id=123456")).toBeNull();
expect(parseSongRef("https://music.163.com/#/playlist?id=123456")).toBeNull();
expect(parseSongRef("https://music.163.com/album?id=123456")).toBeNull();
expect(parseSongRef("https://music.163.com/artist?id=185858")).toBeNull();
// A genuine song URL is still parsed.
expect(parseSongRef("https://music.163.com/song?id=185868")).toEqual({ id: "185868", platform: "netease" });
});
it("parses NetEase song URLs", () => { it("parses NetEase song URLs", () => {
expect(parseSongRef("https://music.163.com/song?id=185868")).toEqual({ id: "185868", platform: "netease" }); expect(parseSongRef("https://music.163.com/song?id=185868")).toEqual({ id: "185868", platform: "netease" });
expect(parseSongRef("https://music.163.com/#/song?id=185868&userid=1")).toEqual({ id: "185868", platform: "netease" }); expect(parseSongRef("https://music.163.com/#/song?id=185868&userid=1")).toEqual({ id: "185868", platform: "netease" });
+8 -3
View File
@@ -31,8 +31,10 @@ export function parseSongRef(raw: string): SongRef | null {
if (!q) return null; if (!q) return null;
// Explicit "id:<id>" — platform decided by the command's flags/default. // Explicit "id:<id>" — platform decided by the command's flags/default.
// Strip trailing punctuation that tags along from a chat paste ("id:12345."
// / "id:12345)") — no supported id (numeric / BVID / mid) ends in those.
const idPrefix = /^id:\s*(\S+)$/i.exec(q); const idPrefix = /^id:\s*(\S+)$/i.exec(q);
if (idPrefix) return { id: idPrefix[1], platform: null }; if (idPrefix) return { id: idPrefix[1].replace(/[.,;)\]]+$/, ""), platform: null };
// BiliBili BV id, bare or inside a bilibili URL (NetEase ids are numeric, so // BiliBili BV id, bare or inside a bilibili URL (NetEase ids are numeric, so
// a "BV..." token never collides with them). // a "BV..." token never collides with them).
@@ -41,8 +43,11 @@ export function parseSongRef(raw: string): SongRef | null {
return { id: bv[0], platform: "bilibili" }; return { id: bv[0], platform: "bilibili" };
} }
// NetEase song URL. // NetEase song URL. Only treat `id=N` as a SONG id when the URL is not a
if (/music\.163\.com/i.test(q)) { // collection page (playlist/album/artist/toplist/djradio) — those reuse the
// same `id=` param but are NOT songs; getSongDetail() would 404 them into a
// confusing "no song" error instead of falling back to a normal search.
if (/music\.163\.com/i.test(q) && !/(playlist|album|artist|toplist|djradio)/i.test(q)) {
const m = /[?&#/]id=(\d+)/.exec(q) ?? /\/song\/(\d+)/.exec(q); const m = /[?&#/]id=(\d+)/.exec(q) ?? /\/song\/(\d+)/.exec(q);
if (m) return { id: m[1], platform: "netease" }; if (m) return { id: m[1], platform: "netease" };
} }