From c6dfbe69a1ab7e174c2946eef3770a073c63df86 Mon Sep 17 00:00:00 2001 From: Amir Date: Tue, 16 Jun 2026 12:04:42 +0200 Subject: [PATCH] fix(refs): tighten inbox/saved thread URL routing Only classify `t/{thread}` with an optional `c/{comment}` suffix after inbox/saved; extra trailing segments stay unclassified so malformed URLs aren't misrouted (e.g. as a conversation). Groups the branch by route prefix and adds saved-thread + strictness test coverage. Addresses review on #30. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/commands/view.test.ts | 12 +++++++++++ src/lib/refs.test.ts | 45 ++++++++++++++++++++++++++++++++++++--- src/lib/refs.ts | 41 +++++++++++++++++++++++++++++------ 3 files changed, 89 insertions(+), 9 deletions(-) diff --git a/src/commands/view.test.ts b/src/commands/view.test.ts index 59dfa06..b1e5a1c 100644 --- a/src/commands/view.test.ts +++ b/src/commands/view.test.ts @@ -95,6 +95,18 @@ describe('tdc view routing', () => { ).rejects.toThrow('Not a recognized Comms URL') }) + it('throws for malformed inbox thread URL with message-like suffix', async () => { + const program = createProgram() + await expect( + program.parseAsync([ + 'node', + 'tdc', + 'view', + 'https://comms.todoist.com/20/inbox/t/TH1/msg/CV1', + ]), + ).rejects.toThrow('Not a recognized Comms URL') + }) + it('throws for non-Comms URL', async () => { const program = createProgram() await expect( diff --git a/src/lib/refs.test.ts b/src/lib/refs.test.ts index 9fef551..c5ee912 100644 --- a/src/lib/refs.test.ts +++ b/src/lib/refs.test.ts @@ -145,6 +145,16 @@ describe('parseCommsUrl', () => { 'https://comms.todoist.com/a/12345/ch/CH1/t/TH1/c/CM1', { workspaceId: 12345, channelId: 'CH1', threadId: 'TH1', commentId: 'CM1' }, ], + [ + 'people URL as workspace-only', + 'https://comms.todoist.com/12345/people/u/678', + { workspaceId: 12345 }, + ], + ])('parses %s', (_description, url, expected) => { + expect(parseCommsUrl(url)).toEqual(expected) + }) + + it.each([ [ 'inbox thread URL', 'https://comms.todoist.com/12345/inbox/t/TH1/', @@ -161,14 +171,33 @@ describe('parseCommsUrl', () => { { workspaceId: 12345, threadId: 'TH1' }, ], [ - 'people URL as workspace-only', - 'https://comms.todoist.com/12345/people/u/678', - { workspaceId: 12345 }, + 'saved thread with comment URL', + 'https://comms.todoist.com/12345/saved/t/TH1/c/CM1', + { workspaceId: 12345, threadId: 'TH1', commentId: 'CM1' }, ], ])('parses %s', (_description, url, expected) => { expect(parseCommsUrl(url)).toEqual(expected) }) + it.each([ + ['inbox root URL', 'https://comms.todoist.com/12345/inbox'], + ['inbox done URL', 'https://comms.todoist.com/12345/inbox/done'], + ['inbox done thread-like URL', 'https://comms.todoist.com/12345/inbox/done/t/TH1'], + ['missing thread id', 'https://comms.todoist.com/12345/inbox/t'], + ['missing comment id', 'https://comms.todoist.com/12345/inbox/t/TH1/c'], + ['comment-only path', 'https://comms.todoist.com/12345/inbox/c/CM1'], + ['wrong marker after thread id', 'https://comms.todoist.com/12345/inbox/t/TH1/x/CM1'], + ['extra segment after thread id', 'https://comms.todoist.com/12345/inbox/t/TH1/extra'], + [ + 'extra segment after comment id', + 'https://comms.todoist.com/12345/inbox/t/TH1/c/CM1/extra', + ], + ['msg suffix after thread id', 'https://comms.todoist.com/12345/inbox/t/TH1/msg/CV1'], + ['saved URL with extra segment', 'https://comms.todoist.com/12345/saved/t/TH1/extra'], + ])('leaves %s workspace-only', (_description, url) => { + expect(parseCommsUrl(url)).toEqual({ workspaceId: 12345 }) + }) + it('parses conversation URL', () => { const result = parseCommsUrl('https://comms.todoist.com/a/12345/msg/CV1') expect(result).toEqual({ workspaceId: 12345, conversationId: 'CV1' }) @@ -293,6 +322,11 @@ describe('resolveThreadId', () => { 'inbox thread URL with comment suffix', 'https://comms.todoist.com/12345/inbox/t/TH1/c/CM1', ], + ['saved thread URL', 'https://comms.todoist.com/12345/saved/t/TH1'], + [ + 'saved thread URL with comment suffix', + 'https://comms.todoist.com/12345/saved/t/TH1/c/CM1', + ], ])('resolves %s', (_description, url) => { expect(resolveThreadId(url)).toBe('TH1') }) @@ -550,6 +584,8 @@ describe('classifyCommsUrl', () => { ['thread+comment URL', 'https://comms.todoist.com/a/20/ch/CH1/t/TH1/c/CM1', 'comment'], ['inbox thread URL', 'https://comms.todoist.com/20/inbox/t/TH1/', 'thread'], ['inbox thread+comment URL', 'https://comms.todoist.com/20/inbox/t/TH1/c/CM1', 'comment'], + ['saved thread URL', 'https://comms.todoist.com/20/saved/t/TH1', 'thread'], + ['saved thread+comment URL', 'https://comms.todoist.com/20/saved/t/TH1/c/CM1', 'comment'], ['conversation URL', 'https://comms.todoist.com/a/20/msg/CV1', 'conversation'], ['short conversation URL', 'https://comms.todoist.com/20/msg/CV1', 'conversation'], ['message URL', 'https://comms.todoist.com/a/20/msg/CV1/m/MS1', 'message'], @@ -561,6 +597,9 @@ describe('classifyCommsUrl', () => { ['inbox root URL', 'https://comms.todoist.com/20/inbox'], ['inbox done URL', 'https://comms.todoist.com/20/inbox/done'], ['inbox done thread-like URL', 'https://comms.todoist.com/20/inbox/done/t/TH1'], + ['inbox thread with extra segment', 'https://comms.todoist.com/20/inbox/t/TH1/extra'], + ['inbox thread with msg suffix', 'https://comms.todoist.com/20/inbox/t/TH1/msg/CV1'], + ['saved thread with extra segment', 'https://comms.todoist.com/20/saved/t/TH1/extra'], ['workspace-only URL', 'https://comms.todoist.com/a/20'], ['channel-only URL', 'https://comms.todoist.com/a/20/ch/CH1'], ['malformed account URL', 'https://comms.todoist.com/a/ch/CH1/t/TH1'], diff --git a/src/lib/refs.ts b/src/lib/refs.ts index efedac0..e2d8cb2 100644 --- a/src/lib/refs.ts +++ b/src/lib/refs.ts @@ -89,6 +89,26 @@ export interface ParsedCommsUrl { messageId?: string } +function parseInboxOrSavedThreadRoute( + routeSegments: readonly string[], +): Pick | null { + const [threadMarker, threadId, commentMarker, commentId, ...extraSegments] = routeSegments + + if (threadMarker !== 't' || !threadId || extraSegments.length > 0) { + return null + } + + if (routeSegments.length === 2) { + return { threadId } + } + + if (routeSegments.length === 4 && commentMarker === 'c' && commentId) { + return { threadId, commentId } + } + + return null +} + export function parseCommsUrl(url: string): ParsedCommsUrl | null { try { const parsed = new URL(url) @@ -135,12 +155,21 @@ export function parseCommsUrl(url: string): ParsedCommsUrl | null { } } - if ( - (segments[routeStart] === 'inbox' || segments[routeStart] === 'saved') && - segments[routeStart + 1] === 't' - ) { - parseRoutePairs(routeStart + 1) - } else if (segments[routeStart] !== 'inbox' && segments[routeStart] !== 'saved') { + const route = segments[routeStart] + if (route === 'inbox' || route === 'saved') { + // Inbox/saved routes only accept: + // t/{thread} + // t/{thread}/c/{comment} + // Other inbox/saved paths stay workspace-only so malformed URLs don't get + // misrouted as thread, comment, or conversation refs. + const threadRoute = parseInboxOrSavedThreadRoute(segments.slice(routeStart + 1)) + if (threadRoute) { + result.threadId = threadRoute.threadId + if (threadRoute.commentId) { + result.commentId = threadRoute.commentId + } + } + } else { parseRoutePairs(routeStart) }