mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
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 <neil@stably.ai>
This commit is contained in:
@@ -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'
|
||||
|
||||
@@ -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<number, string>()
|
||||
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
|
||||
])
|
||||
}
|
||||
@@ -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<typeof linearClient>()),
|
||||
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<string, unknown> }[] = []
|
||||
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<void>((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<void>((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', '<pre>\n- literal\n</pre>', '<pre>\n* literal\n</pre>'],
|
||||
['HTML comment', '<!--\n- literal\n-->', '<!--\n* literal\n-->'],
|
||||
['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 = '<pre>\n- literal\n</pre>'
|
||||
rewrite = () => '<pre>\n* literal\n</pre>'
|
||||
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' })
|
||||
})
|
||||
@@ -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<boolean>
|
||||
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', () => {
|
||||
|
||||
@@ -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.'
|
||||
|
||||
@@ -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<Awaited<ReturnType<typeof getLinearIssueByUuidForAgent>>>,
|
||||
fields: LinearIssueUpdate
|
||||
): Promise<boolean> {
|
||||
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<Awaited<ReturnType<typeof getLinearIssueByUuidForAgent>>>,
|
||||
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
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user