From f0a399d143dd4aa031513265a99d76b32b70eea2 Mon Sep 17 00:00:00 2001 From: Pr1p <48266347+Pr1p@users.noreply.github.com> Date: Sat, 24 Oct 2026 15:34:01 +0800 Subject: [PATCH] fix(linear): accept normalized Markdown save confirmations (#30076) * fix(linear): tolerate documented Markdown normalization * fix(linear): preserve fenced Markdown literals * docs(linear): describe Markdown fence matching * fix(linear): close nested Markdown fences * fix(linear): tolerate documented Markdown normalization * fix(linear): preserve fenced Markdown literals * docs(linear): describe Markdown fence matching * fix(linear): close nested Markdown fences * fix(linear): compare only parsed list marker rewrites * style(linear): satisfy save confirmation quality gates * fix(linear): preserve list structure and load parser on demand * fix(linear): bundle save Markdown comparator for every runtime The comparator resolved unified/remark at runtime via createRequire, which orcad's esbuild bundle does not ship. Import them statically from a lazily imported module so Electron and orcad both inline the parser. --------- Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> Co-authored-by: Neil --- electron.vite.config.ts | 4 + .../linear-issue-description-comparison.ts | 44 ++++ src/main/runtime/linear-save-markdown.test.ts | 225 ++++++++++++++++++ .../rpc/methods/linear-agent-access.test.ts | 12 +- .../runtime/runtime-linear-save-commands.ts | 4 +- .../runtime-linear-save-fields-commands.ts | 25 +- 6 files changed, 303 insertions(+), 11 deletions(-) create mode 100644 src/main/runtime/linear-issue-description-comparison.ts create mode 100644 src/main/runtime/linear-save-markdown.test.ts diff --git a/electron.vite.config.ts b/electron.vite.config.ts index 19362ba664d..708b8031358 100644 --- a/electron.vite.config.ts +++ b/electron.vite.config.ts @@ -21,6 +21,10 @@ const BUNDLED_MAIN_DEPENDENCIES = new Set([ 'tldts', 'smol-toml', 'fflate', + // Linear save confirmation; bundled like orcad so packaging needs no extra node_modules. + 'unified', + 'remark-parse', + 'remark-gfm', // Why: Windows NSIS deploys app.asar before external resources; bootstrap must // not race the later resources/node_modules copy. 'zod' diff --git a/src/main/runtime/linear-issue-description-comparison.ts b/src/main/runtime/linear-issue-description-comparison.ts new file mode 100644 index 00000000000..0084f5c509b --- /dev/null +++ b/src/main/runtime/linear-issue-description-comparison.ts @@ -0,0 +1,44 @@ +import type { Nodes } from 'mdast' +import remarkGfm from 'remark-gfm' +import remarkParse from 'remark-parse' +import { unified } from 'unified' + +const markdown = unified().use(remarkParse).use(remarkGfm) + +export function linearIssueDescriptionComparisonKey(description: string): string { + const replacements = new Map() + const structure: string[] = [] + const pending: Nodes[] = [markdown.parse(description)] + while (pending.length > 0) { + const node = pending.pop() + if (!node) { + continue + } + structure.push(`${node.type}:${'children' in node ? node.children.length : 0}`) + if (node.type === 'list' && !node.ordered) { + for (const item of node.children) { + const offset = item.position?.start.offset + if (offset === undefined) { + continue + } + const source = description.slice(offset, item.position?.end.offset) + if (!/^[-*] (?=\S)/.test(source)) { + continue + } + replacements.set(offset, '*') + const taskNumber = /^[-*] \[[ xX]\] \d+(\\)\.(?=\s)/.exec(source) + if (item.checked !== null && item.checked !== undefined && taskNumber) { + replacements.set(offset + taskNumber[0].length - 2, '') + } + } + } + if ('children' in node) { + pending.push(...node.children) + } + } + // Preserve block structure too: changing list markers can join or separate adjacent lists. + return JSON.stringify([ + description.replace(/[-*\\]/g, (character, offset) => replacements.get(offset) ?? character), + structure + ]) +} diff --git a/src/main/runtime/linear-save-markdown.test.ts b/src/main/runtime/linear-save-markdown.test.ts new file mode 100644 index 00000000000..1b2ac2bb094 --- /dev/null +++ b/src/main/runtime/linear-save-markdown.test.ts @@ -0,0 +1,225 @@ +import { createServer, type Server } from 'node:http' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import type * as linearClient from '../linear/client' +import { RuntimeLinearSaveCommands } from './runtime-linear-save-commands' +import { loadLinearSdk } from '../linear/linear-sdk' + +const boundary = vi.hoisted(() => ({ clients: vi.fn(), status: vi.fn(), signing: vi.fn() })) +vi.mock('../linear/client', async (importOriginal) => ({ + ...(await importOriginal()), + getClients: boundary.clients, + getStatus: boundary.status, + getPublicFileUrlClient: boundary.signing +})) + +const workspace = { + id: 'workspace-proof', + organizationId: 'org-proof', + organizationName: 'Local proof', + organizationUrlKey: 'local-proof', + name: 'Proof', + email: 'proof@example.invalid', + viewerId: 'viewer-proof', + displayName: 'Proof' +} +const baseIssue = { + id: '00000000-0000-4000-8000-000000000001', + identifier: 'PROOF-1', + title: 'Existing title', + description: 'Existing description', + url: 'https://linear.app/local-proof/issue/PROOF-1', + team: { id: 'team-proof', key: 'PROOF', name: 'Proof' }, + state: { id: 'state-proof', name: 'Todo' }, + parent: null, + project: null, + assignee: null, + priority: 0, + estimate: null, + dueDate: null, + labelIds: [], + labels: { nodes: [] } +} +const nativeFetch = globalThis.fetch +let server: Server +let stored = { ...baseIssue } +let sent: { query: string; variables: Record }[] = [] +let rewrite: (description: string) => string + +beforeEach(async () => { + stored = { ...baseIssue } + sent = [] + rewrite = (description) => description + server = createServer((request, response) => { + let body = '' + request.on('data', (chunk) => (body += chunk.toString())) + request.on('end', () => { + const payload = JSON.parse(body) + sent.push(payload) + response.writeHead(200, { 'content-type': 'application/json' }) + if (payload.query.includes('mutation')) { + if (!payload.query.includes('issueUpdate')) { + response.end(JSON.stringify({ errors: [{ message: 'Unexpected mutation' }] })) + return + } + const update = payload.variables.input + stored = { + ...stored, + ...update, + ...(typeof update.description === 'string' + ? { description: rewrite(update.description) } + : {}) + } + response.end(JSON.stringify({ data: { issueUpdate: { success: true, issue: stored } } })) + } else if (/query (?:OrcaAgentLinearIssue|OrcaLinearIssueByUuid)\(/.test(payload.query)) { + response.end(JSON.stringify({ data: { issue: stored } })) + } else { + response.end(JSON.stringify({ errors: [{ message: 'Unexpected query' }] })) + } + }) + }) + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) + const address = server.address() + if (!address || typeof address === 'string') { + throw new Error('No localhost address') + } + const apiUrl = `http://127.0.0.1:${address.port}/graphql` + vi.spyOn(globalThis, 'fetch').mockImplementation((_input, init) => nativeFetch(apiUrl, init)) + const createClient = (signal?: AbortSignal) => + new (loadLinearSdk().LinearClient)({ apiKey: 'local-proof-only', signal }) + boundary.clients.mockImplementation((_workspace, signal) => [ + { workspace, apiKey: 'local-proof-only', client: createClient(signal) } + ]) + boundary.signing.mockImplementation((_entry, signal) => createClient(signal)) + boundary.status.mockReturnValue({ connected: true, workspaces: [workspace] }) +}) + +afterEach(async () => { + vi.restoreAllMocks() + const closed = new Promise((resolve, reject) => + server.close((error) => (error ? reject(error) : resolve())) + ) + server.closeAllConnections() + await closed +}) + +function commands() { + return new RuntimeLinearSaveCommands({ + runtimeAvailable: () => true, + showTerminal: async () => { + throw new Error('No terminal in HTTP proof') + }, + resolveWorktreeSelector: async () => { + throw new Error('No workspace resolution in HTTP proof') + }, + listResolvedWorktrees: async () => [], + setWorktreeMeta: () => { + throw new Error('Unexpected workspace metadata write') + }, + emitClientEvent: () => { + throw new Error('Unexpected event') + } + }) +} + +it.each([ + { + name: 'unordered bullets', + requested: '## Notes\n\n- first item\n- second item', + saved: '## Notes\n\n* first item\n* second item' + }, + { + name: 'task number escape', + requested: '- [ ] 1. Share `outcome`: ...', + saved: '- [ ] 1\\. Share `outcome`: ...' + } +])('confirms $name and makes an identical retry a no-op', async ({ requested, saved }) => { + rewrite = (description) => (description === requested ? saved : description) + const runtime = commands() + const request = { input: stored.identifier, description: requested, workspaceId: workspace.id } + await expect(runtime.linearSaveIssue(request)).resolves.toMatchObject({ + issue: { description: saved }, + meta: { created: false, workspaceId: workspace.id } + }) + await expect(runtime.linearSaveIssue(request)).resolves.toMatchObject({ + issue: { description: saved } + }) + const mutations = sent.filter(({ query }) => query.includes('mutation')) + expect(stored.description).toBe(saved) + expect(mutations).toHaveLength(1) + expect(mutations[0]?.variables).toEqual({ id: stored.id, input: { description: requested } }) + expect(sent.map(({ query }) => query.match(/(?:query|mutation)\s+(\w+)/)?.[1])).toEqual([ + 'OrcaAgentLinearIssue', + 'OrcaLinearIssueByUuid', + 'updateIssue', + 'OrcaLinearIssueByUuid', + 'OrcaAgentLinearIssue', + 'OrcaLinearIssueByUuid' + ]) +}) + +it.each([ + ['adjacent list grouping', '- first\n* second', '* first\n* second'], + ['separating lists', '* first\n* second', '- first\n* second'], + ['fenced code', '```text\n- literal\n```', '```text\n* literal\n```'], + ['unterminated code', '```text\n- literal', '```text\n* literal'], + ['list-contained code', '- ```text\n - literal\n ```', '- ```text\n * literal\n ```'], + [ + 'nested code', + '- parent\n - ```text\n - literal\n ```', + '- parent\n - ```text\n * literal\n ```' + ], + ['quoted code', '> ```text\n> - literal\n> ```', '> ```text\n> * literal\n> ```'], + ['indented code', ' - literal', ' * literal'], + ['tab-indented code', '\t- literal', '\t* literal'], + ['nested indented code', '- parent\n\n - literal', '- parent\n\n * literal'], + ['raw HTML', '
\n- literal\n
', '
\n* literal\n
'], + ['HTML comment', '', ''], + ['thematic break', '- - -', '* - -'], + ['code task', '```text\n- [ ] 1. literal\n```', '```text\n- [ ] 1\\. literal\n```'], + ['inline code', '- `literal - value`', '- `literal * value`'], + ['content', '- first', '* second'], + ['unordered marker outside observed pair', '+ first', '- first'], + ['ordered delimiter', '1. first', '1) first'], + ['line endings', '- first\r\n- second', '* first\n* second'] +])('sends meaningful %s edits instead of skipping the write', async (_name, current, requested) => { + stored.description = current + await expect( + commands().linearSaveIssue({ + input: stored.identifier, + description: requested, + workspaceId: workspace.id + }) + ).resolves.toMatchObject({ issue: { description: requested } }) + const mutations = sent.filter(({ query }) => query.includes('mutation')) + expect(mutations).toHaveLength(1) + expect(mutations[0]?.variables.input).toEqual({ description: requested }) + expect(stored.description).toBe(requested) +}) + +it('rejects a literal code change in the read-back after a successful mutation', async () => { + const requested = '
\n- literal\n
' + rewrite = () => '
\n* literal\n
' + await expect( + commands().linearSaveIssue({ + input: stored.identifier, + description: requested, + workspaceId: workspace.id + }) + ).rejects.toMatchObject({ code: 'linear_write_unconfirmed' }) + expect(sent.filter(({ query }) => query.includes('mutation'))).toHaveLength(1) +}) + +it('still writes other fields when the description already matches after normalization', async () => { + stored.description = '* first' + await expect( + commands().linearSaveIssue({ + input: stored.identifier, + title: 'Changed title', + description: '- first', + workspaceId: workspace.id + }) + ).resolves.toMatchObject({ issue: { title: 'Changed title', description: '- first' } }) + const mutations = sent.filter(({ query }) => query.includes('mutation')) + expect(mutations).toHaveLength(1) + expect(mutations[0]?.variables.input).toEqual({ title: 'Changed title', description: '- first' }) +}) diff --git a/src/main/runtime/rpc/methods/linear-agent-access.test.ts b/src/main/runtime/rpc/methods/linear-agent-access.test.ts index 40b10fe549d..313f0ae98fc 100644 --- a/src/main/runtime/rpc/methods/linear-agent-access.test.ts +++ b/src/main/runtime/rpc/methods/linear-agent-access.test.ts @@ -359,7 +359,7 @@ type LinearUnconfirmedBuilder = { ): Error & { data?: { cause?: string; nextSteps?: string[] } } resolveLinearAgentState(input: string, states: unknown[]): unknown linearCreatedIssueMatchesIntent(issue: unknown, intent: unknown): boolean - linearSavedIssueMatchesIntent(issue: unknown, intent: unknown): boolean + linearSavedIssueMatchesIntent(issue: unknown, intent: unknown): Promise notifyLinearLinkedIssueUpdated( workspaceId: string, identifier: string | readonly string[] @@ -584,7 +584,7 @@ describe('Linear agent write recovery helpers', () => { ).toBe(false) }) - it('confirms save-issue updates including explicit relationship clears', () => { + it('confirms save-issue updates including explicit relationship clears', async () => { const runtime = new OrcaRuntimeService() const builder = runtime as unknown as LinearUnconfirmedBuilder const issue = { @@ -604,7 +604,7 @@ describe('Linear agent write recovery helpers', () => { labelIds: ['label-1'] } - expect( + await expect( builder.linearSavedIssueMatchesIntent(issue, { title: 'Updated title', description: 'Updated description', @@ -617,8 +617,10 @@ describe('Linear agent write recovery helpers', () => { dueDate: null, labelIds: ['label-1'] }) - ).toBe(true) - expect(builder.linearSavedIssueMatchesIntent(issue, { projectId: 'project-2' })).toBe(false) + ).resolves.toBe(true) + await expect( + builder.linearSavedIssueMatchesIntent(issue, { projectId: 'project-2' }) + ).resolves.toBe(false) }) it('resolves workflow states by UUID or case-insensitive exact name', () => { diff --git a/src/main/runtime/runtime-linear-save-commands.ts b/src/main/runtime/runtime-linear-save-commands.ts index a707ed1ba96..3bb15f07065 100644 --- a/src/main/runtime/runtime-linear-save-commands.ts +++ b/src/main/runtime/runtime-linear-save-commands.ts @@ -51,7 +51,7 @@ export class RuntimeLinearSaveCommands extends RuntimeLinearCommentCommands { if (Object.keys(fields).length === 0) { throw linearError('linear_write_failed', 'No issue fields were provided to save.') } - const alreadySet = this.linearSavedIssueMatchesIntent(current, fields) + const alreadySet = await this.linearSavedIssueMatchesIntent(current, fields) const updated = alreadySet ? current : await this.runLinearAgentWrite( @@ -62,7 +62,7 @@ export class RuntimeLinearSaveCommands extends RuntimeLinearCommentCommands { target.workspaceId, { signal } ) - if (!this.linearSavedIssueMatchesIntent(saved, fields)) { + if (!(await this.linearSavedIssueMatchesIntent(saved, fields))) { throw new LinearWriteFailure( 'unconfirmed', 'Linear issue save could not be confirmed.' diff --git a/src/main/runtime/runtime-linear-save-fields-commands.ts b/src/main/runtime/runtime-linear-save-fields-commands.ts index c07cd8f4c05..6676702788e 100644 --- a/src/main/runtime/runtime-linear-save-fields-commands.ts +++ b/src/main/runtime/runtime-linear-save-fields-commands.ts @@ -8,16 +8,33 @@ import { linearError } from '../linear/issue-context/errors' import { RuntimeLinearTaskFieldCommands } from './runtime-linear-task-fields-commands' export class RuntimeLinearSaveFieldCommands extends RuntimeLinearTaskFieldCommands { - public linearSavedIssueMatchesIntent( + public async linearSavedIssueMatchesIntent( + issue: NonNullable>>, + fields: LinearIssueUpdate + ): Promise { + if (!this.linearSavedIssueFieldsMatchIntent(issue, fields)) { + return false + } + const description = issue.description ?? '' + if (fields.description === undefined || description === fields.description) { + return true + } + // Why dynamic: keeps the Markdown parser off startup while every bundle (Electron, orcad) inlines it. + const { linearIssueDescriptionComparisonKey } = + await import('./linear-issue-description-comparison') + return ( + linearIssueDescriptionComparisonKey(description) === + linearIssueDescriptionComparisonKey(fields.description) + ) + } + + private linearSavedIssueFieldsMatchIntent( issue: NonNullable>>, fields: LinearIssueUpdate ): boolean { if (fields.title !== undefined && issue.title !== fields.title) { return false } - if (fields.description !== undefined && (issue.description ?? '') !== fields.description) { - return false - } if (fields.parentId !== undefined && (issue.parent?.id ?? null) !== fields.parentId) { return false }