diff --git a/CHANGELOG.md b/CHANGELOG.md index 62555b99..46cc1947 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ All notable user-visible changes to CASCADE are documented here. The format is l ### Fixed +- **Linear inline checklist creation now waits for description visibility before releasing its mutation lock.** Linear can briefly return stale issue descriptions after accepting `updateIssue()`, so CASCADE now polls `getIssue()` under the existing per-issue description lock until the new markdown is visible. Parallel `AddChecklist` flows no longer fail with `Checklist not found in description` when one flow creates a checklist heading immediately before another appends items. See Linear issue [MNG-685](https://linear.app/issue/MNG-685). + - **Linear and JIRA inline checklist updates no longer lose sibling checklist rows during concurrent updates.** Both providers rewrite the whole issue description for checklist mutations, so their read/mutate/write path is now serialized per provider/work item with a stale-safe temp-file lock and provider-only retry semantics. The shared inline checklist parser also keeps scanning through prose, indented detail lines, and bullet detail lines until the next heading, so `ReadWorkItem` reports every visible checkbox row under a checklist heading. See Linear issue [MNG-656](https://linear.app/issue/MNG-656). - **`ReadWorkItem` examples now render PM IDs as runnable bare CLI values.** Native-tool prompt guidance and `cascade-tools pm read-work-item --help` now show `--workItemId abc123` instead of JSON-string-literal forms like `--workItemId '"abc123"'`. The CLI also strips one accidental outer quote layer for `ReadWorkItem` IDs only, so a copied bad example no longer sends literal quote characters to the PM provider. See Trello card [M5f9T1D7](https://trello.com/c/M5f9T1D7/673-frictionlow-readworkitem-example-quoting-produced-trello-card-id-with-literal-quotes). diff --git a/src/integrations/README.md b/src/integrations/README.md index a4137bce..228b251b 100644 --- a/src/integrations/README.md +++ b/src/integrations/README.md @@ -297,7 +297,7 @@ Different PM providers have different native concepts of "checklist". The `PMPro The shared engine that parses, appends, toggles, and removes inline checklist items lives at `src/pm/_shared/inline-checklist.ts` and is consumed by both the Linear and JIRA adapters. -Because Linear and JIRA checklist mutations rewrite the whole description, their adapters serialize the full read/mutate/write operation with `withDescriptionMutationLock(provider, workItemId, fn)` from `src/pm/_shared/description-mutation-lock.ts`. Keep future inline-description mutations inside that guard; otherwise concurrent `cascade-tools pm update-checklist-item` processes can overwrite each other's description snapshots without a provider-side conflict error. +Because Linear and JIRA checklist mutations rewrite the whole description, their adapters serialize the full read/mutate/write operation with `withDescriptionMutationLock(provider, workItemId, fn)` from `src/pm/_shared/description-mutation-lock.ts`. Keep future inline-description mutations inside that guard; otherwise concurrent `cascade-tools pm update-checklist-item` processes can overwrite each other's description snapshots without a provider-side conflict error. Linear mutations must also keep the lock held until `linearClient.getIssue()` observes the markdown just written by `linearClient.updateIssue()`, because Linear can briefly serve stale descriptions after accepting a write. Releasing the lock before read-after-write convergence lets the next queued checklist mutation start from an old snapshot and miss a newly created `###` checklist heading. --- diff --git a/src/pm/linear/adapter.ts b/src/pm/linear/adapter.ts index d68a44d8..e56913ca 100644 --- a/src/pm/linear/adapter.ts +++ b/src/pm/linear/adapter.ts @@ -36,6 +36,9 @@ import type { WorkItemLabel, } from '../types.js'; +const DESCRIPTION_VISIBILITY_TIMEOUT_MS = 1_000; +const DESCRIPTION_VISIBILITY_POLL_MS = 25; + export class LinearPMProvider implements PMProvider { readonly type = 'linear' as const; @@ -288,6 +291,7 @@ export class LinearPMProvider implements PMProvider { const newDesc = mutate(issue.description ?? ''); try { await linearClient.updateIssue(issueId, { description: newDesc }); + await this.waitForDescriptionVisibility(issueId, newDesc); return; } catch (err) { if (attempt === 0) { @@ -299,6 +303,30 @@ export class LinearPMProvider implements PMProvider { } } + private async waitForDescriptionVisibility( + issueId: string, + expectedDescription: string, + ): Promise { + const startedAt = Date.now(); + const deadline = startedAt + DESCRIPTION_VISIBILITY_TIMEOUT_MS; + const expected = normalizeDescriptionForVisibility(expectedDescription); + + while (true) { + const issue = await linearClient.getIssue(issueId); + if (normalizeDescriptionForVisibility(issue.description ?? '') === expected) { + return; + } + + if (Date.now() >= deadline) { + throw new Error( + `Linear description visibility timed out for issue ${issueId} after ${DESCRIPTION_VISIBILITY_TIMEOUT_MS}ms`, + ); + } + + await sleep(Math.min(DESCRIPTION_VISIBILITY_POLL_MS, Math.max(0, deadline - Date.now()))); + } + } + private logDescriptionRetry(issueId: string, err: unknown): void { logger.warn('[Linear] Description provider update failed; retrying once', { issueId, @@ -374,3 +402,11 @@ export class LinearPMProvider implements PMProvider { }; } } + +function normalizeDescriptionForVisibility(description: string): string { + return description.replace(/\r\n/g, '\n').replace(/\r/g, '\n'); +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} diff --git a/tests/unit/pm/linear/adapter.test.ts b/tests/unit/pm/linear/adapter.test.ts index 2ea69631..b2b9f6e1 100644 --- a/tests/unit/pm/linear/adapter.test.ts +++ b/tests/unit/pm/linear/adapter.test.ts @@ -76,6 +76,20 @@ function makeIssue(overrides: Record = {}) { }; } +function mockIssueDescription(initialDescription: string | null) { + let description = initialDescription; + mockGetIssue.mockImplementation(async () => makeIssue({ description })); + mockUpdateIssue.mockImplementation(async (_id, updates: { description?: string }) => { + description = updates.description ?? description; + return makeIssue({ description }); + }); + return { + get description() { + return description; + }, + }; +} + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -472,8 +486,7 @@ describe('LinearPMProvider', () => { describe('createChecklist (inline)', () => { it('appends new checklist section to description and returns Checklist', async () => { - mockGetIssue.mockResolvedValue(makeIssue({ description: 'Existing.' })); - mockUpdateIssue.mockResolvedValue(makeIssue()); + mockIssueDescription('Existing.'); const result = await provider.createChecklist('issue-uuid', '✅ AC'); @@ -486,13 +499,72 @@ describe('LinearPMProvider', () => { expect(result.id).toMatch(/^inline-issue-uuid-[0-9a-f]{8}$/); expect(result.items).toEqual([]); }); + + it('waits for Linear read-after-write visibility before the next checklist append', async () => { + let description = 'Existing.'; + let staleDescription: string | null = null; + mockGetIssue.mockImplementation(async () => { + if (staleDescription !== null) { + const value = staleDescription; + staleDescription = null; + return makeIssue({ description: value }); + } + return makeIssue({ description }); + }); + mockUpdateIssue.mockImplementation(async (_id, updates: { description?: string }) => { + staleDescription = description; + description = updates.description ?? description; + return makeIssue({ description }); + }); + + const checklist = await provider.createChecklist('issue-uuid', '✅ AC'); + await provider.addChecklistItem(checklist.id, 'First item'); + + expect(description).toBe('Existing.\n\n### ✅ AC\n- [ ] First item'); + }); + + it('preserves multiple concurrently-created checklist sections despite stale Linear reads', async () => { + let description = 'Existing.'; + let staleDescription: string | null = null; + mockGetIssue.mockImplementation(async () => { + if (staleDescription !== null) { + const value = staleDescription; + staleDescription = null; + return makeIssue({ description: value }); + } + return makeIssue({ description }); + }); + mockUpdateIssue.mockImplementation(async (_id, updates: { description?: string }) => { + await sleep(5); + staleDescription = description; + description = updates.description ?? description; + return makeIssue({ description }); + }); + + const results = await Promise.allSettled([ + provider + .createChecklist('issue-uuid', '✅ Acceptance Criteria') + .then((checklist) => provider.addChecklistItem(checklist.id, 'Ready to ship')), + provider + .createChecklist('issue-uuid', '🔗 Dependencies') + .then((checklist) => provider.addChecklistItem(checklist.id, 'External API key')), + ]); + + expect(results).toEqual([ + { status: 'fulfilled', value: undefined }, + { status: 'fulfilled', value: undefined }, + ]); + expect(description).toContain('### ✅ Acceptance Criteria'); + expect(description).toContain('- [ ] Ready to ship'); + expect(description).toContain('### 🔗 Dependencies'); + expect(description).toContain('- [ ] External API key'); + }); }); describe('addChecklistItem (inline)', () => { it('appends a markdown checkbox to the description', async () => { // Pre-existing checklist section in description - mockGetIssue.mockResolvedValue(makeIssue({ description: '### ✅ AC\n- [ ] Existing' })); - mockUpdateIssue.mockResolvedValue(makeIssue()); + mockIssueDescription('### ✅ AC\n- [ ] Existing'); // Build the checklistId for this checklist (without calling createChecklist) const checklist = await provider.createChecklist('issue-uuid', '✅ AC'); @@ -503,8 +575,7 @@ describe('LinearPMProvider', () => { }); it('does NOT call createIssue (no sub-issue creation)', async () => { - mockGetIssue.mockResolvedValue(makeIssue({ description: '### ✅ AC\n- [ ] Existing' })); - mockUpdateIssue.mockResolvedValue(makeIssue()); + mockIssueDescription('### ✅ AC\n- [ ] Existing'); const checklist = await provider.createChecklist('issue-uuid', '✅ AC'); await provider.addChecklistItem(checklist.id, 'Item'); @@ -519,8 +590,7 @@ describe('LinearPMProvider', () => { }); it('supports checked=true', async () => { - mockGetIssue.mockResolvedValue(makeIssue({ description: '### ✅ AC\n- [ ] First' })); - mockUpdateIssue.mockResolvedValue(makeIssue()); + mockIssueDescription('### ✅ AC\n- [ ] First'); const checklist = await provider.createChecklist('issue-uuid', '✅ AC'); await provider.addChecklistItem(checklist.id, 'Done item', true); @@ -533,8 +603,7 @@ describe('LinearPMProvider', () => { describe('updateChecklistItem (inline)', () => { it('toggles a checkbox in the description', async () => { const desc = '### ✅ AC\n- [ ] Item A'; - mockGetIssue.mockResolvedValue(makeIssue({ description: desc })); - mockUpdateIssue.mockResolvedValue(makeIssue()); + mockIssueDescription(desc); const checklists = await provider.getChecklists('issue-uuid'); const itemId = checklists[0].items[0].id; @@ -549,8 +618,7 @@ describe('LinearPMProvider', () => { it('does NOT call updateIssueState (no transition)', async () => { const desc = '### ✅ AC\n- [ ] Item A'; - mockGetIssue.mockResolvedValue(makeIssue({ description: desc })); - mockUpdateIssue.mockResolvedValue(makeIssue()); + mockIssueDescription(desc); const checklists = await provider.getChecklists('issue-uuid'); const itemId = checklists[0].items[0].id; @@ -582,8 +650,7 @@ describe('LinearPMProvider', () => { describe('deleteChecklistItem (inline)', () => { it('removes the item line from the description', async () => { const desc = '### ✅ AC\n- [ ] Keep\n- [ ] Remove'; - mockGetIssue.mockResolvedValue(makeIssue({ description: desc })); - mockUpdateIssue.mockResolvedValue(makeIssue()); + mockIssueDescription(desc); const checklists = await provider.getChecklists('issue-uuid'); const removeId = checklists[0].items[1].id; @@ -599,8 +666,14 @@ describe('LinearPMProvider', () => { describe('checklist update retry on conflict', () => { it('retries description update once on failure', async () => { const desc = '### ✅ AC\n- [ ] Item'; - mockGetIssue.mockResolvedValue(makeIssue({ description: desc })); - mockUpdateIssue.mockRejectedValueOnce(new Error('stale')).mockResolvedValueOnce(makeIssue()); + let description = desc; + mockGetIssue.mockImplementation(async () => makeIssue({ description })); + mockUpdateIssue + .mockRejectedValueOnce(new Error('stale')) + .mockImplementation(async (_id, updates: { description?: string }) => { + description = updates.description ?? description; + return makeIssue({ description }); + }); const checklists = await provider.getChecklists('issue-uuid'); await provider.updateChecklistItem('issue-uuid', checklists[0].items[0].id, true);