From 9d3db3c5865aaa2f0d3c0e0d3da3be68c14d7641 Mon Sep 17 00:00:00 2001 From: Cascade Bot Date: Mon, 11 May 2026 11:48:15 +0000 Subject: [PATCH 1/4] fix(pm): serialize inline checklist description updates --- CHANGELOG.md | 2 + src/integrations/README.md | 2 + src/pm/_shared/description-mutation-lock.ts | 134 ++++++++++++++++++ src/pm/_shared/inline-checklist.ts | 10 +- src/pm/jira/adapter.ts | 59 +++++--- src/pm/linear/adapter.ts | 52 +++++-- .../_shared/description-mutation-lock.test.ts | 107 ++++++++++++++ .../unit/pm/_shared/inline-checklist.test.ts | 40 ++++++ tests/unit/pm/jira/adapter.test.ts | 40 ++++++ tests/unit/pm/linear/adapter.test.ts | 34 +++++ 10 files changed, 448 insertions(+), 32 deletions(-) create mode 100644 src/pm/_shared/description-mutation-lock.ts create mode 100644 tests/unit/pm/_shared/description-mutation-lock.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ef512989..62555b99d 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 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). - **`cascade-tools scm create-pr-review` now accepts `--body-file ` and `--body-file -`.** This matches the generated CreatePRReview guidance and the existing CreatePR / PostPRComment file-input pattern for long Markdown bodies. See Trello card [7kmo42o6](https://trello.com/c/7kmo42o6/691-friction-tooling-low-createprreview-docs-advertise-body-file-but-cli-rejects-it). diff --git a/src/integrations/README.md b/src/integrations/README.md index a828507a1..a4137bce3 100644 --- a/src/integrations/README.md +++ b/src/integrations/README.md @@ -297,6 +297,8 @@ 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. + --- ## Image delivery contract diff --git a/src/pm/_shared/description-mutation-lock.ts b/src/pm/_shared/description-mutation-lock.ts new file mode 100644 index 000000000..c6572ae50 --- /dev/null +++ b/src/pm/_shared/description-mutation-lock.ts @@ -0,0 +1,134 @@ +import { randomUUID } from 'node:crypto'; +import { constants } from 'node:fs'; +import { mkdir, open, readFile, stat, unlink } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +export interface DescriptionMutationLockOptions { + lockDir?: string; + timeoutMs?: number; + staleMs?: number; + pollMs?: number; +} + +interface LockFileContents { + token: string; + createdAt: number; + pid: number; +} + +const DEFAULT_TIMEOUT_MS = 10_000; +const DEFAULT_STALE_MS = 120_000; +const DEFAULT_POLL_MS = 25; +const LOCK_DIR_ENV = 'CASCADE_DESCRIPTION_MUTATION_LOCK_DIR'; + +export async function withDescriptionMutationLock( + provider: string, + workItemId: string, + fn: () => Promise, + options: DescriptionMutationLockOptions = {}, +): Promise { + const settings = resolveOptions(options); + const lockPath = join( + settings.lockDir, + `${sanitizePathPart(provider)}-${sanitizePathPart(workItemId)}.lock`, + ); + const token = randomUUID(); + + await mkdir(settings.lockDir, { recursive: true }); + await acquireLock(lockPath, token, settings); + try { + return await fn(); + } finally { + await releaseLock(lockPath, token); + } +} + +function resolveOptions( + options: DescriptionMutationLockOptions, +): Required { + return { + lockDir: + options.lockDir ?? process.env[LOCK_DIR_ENV] ?? join(tmpdir(), 'cascade-description-locks'), + timeoutMs: options.timeoutMs ?? DEFAULT_TIMEOUT_MS, + staleMs: options.staleMs ?? DEFAULT_STALE_MS, + pollMs: options.pollMs ?? DEFAULT_POLL_MS, + }; +} + +async function acquireLock( + lockPath: string, + token: string, + options: Required, +): Promise { + const startedAt = Date.now(); + const deadline = startedAt + options.timeoutMs; + const contents: LockFileContents = { token, createdAt: startedAt, pid: process.pid }; + + while (true) { + try { + const file = await open(lockPath, constants.O_CREAT | constants.O_EXCL | constants.O_WRONLY); + try { + await file.writeFile(JSON.stringify(contents)); + } finally { + await file.close(); + } + return; + } catch (err) { + if (!isFileExistsError(err)) throw err; + } + + await removeStaleLock(lockPath, options.staleMs); + if (Date.now() >= deadline) { + throw new Error(`Timed out waiting for description mutation lock: ${lockPath}`); + } + await sleep(jitter(options.pollMs)); + } +} + +async function removeStaleLock(lockPath: string, staleMs: number): Promise { + try { + const lockStat = await stat(lockPath); + const ageMs = Date.now() - lockStat.mtimeMs; + if (ageMs <= staleMs) return; + await unlink(lockPath); + } catch (err) { + if (!isMissingFileError(err)) throw err; + } +} + +async function releaseLock(lockPath: string, token: string): Promise { + try { + const raw = await readFile(lockPath, 'utf8'); + const parsed = JSON.parse(raw) as Partial; + if (parsed.token !== token) return; + await unlink(lockPath); + } catch (err) { + if (!isMissingFileError(err)) throw err; + } +} + +function sanitizePathPart(value: string): string { + const sanitized = value.replace(/[^a-zA-Z0-9._-]+/g, '_').replace(/^_+|_+$/g, ''); + return sanitized || 'unknown'; +} + +function jitter(baseMs: number): number { + return baseMs + Math.floor(Math.random() * baseMs); +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +function isFileExistsError(err: unknown): boolean { + return isNodeError(err) && err.code === 'EEXIST'; +} + +function isMissingFileError(err: unknown): boolean { + return isNodeError(err) && err.code === 'ENOENT'; +} + +function isNodeError(err: unknown): err is NodeJS.ErrnoException { + return err instanceof Error && 'code' in err; +} diff --git a/src/pm/_shared/inline-checklist.ts b/src/pm/_shared/inline-checklist.ts index 0e452da3e..0ad43adf5 100644 --- a/src/pm/_shared/inline-checklist.ts +++ b/src/pm/_shared/inline-checklist.ts @@ -29,6 +29,7 @@ export function hashChecklistItemId(checklistName: string, itemText: string): st // --------------------------------------------------------------------------- const H3_REGEX = /^### (.+)$/; +const HEADING_REGEX = /^#{1,6}\s+/; const CHECKBOX_REGEX = /^- \[([ x])\] (.+)$/; export function parseInlineChecklists(description: string): ParsedChecklist[] { @@ -81,6 +82,8 @@ function classifyLine(line: string, current: { name: string } | null): LineClass const h3Match = line.match(H3_REGEX); if (h3Match) return { action: 'new-section', name: h3Match[1] }; + if (current && HEADING_REGEX.test(line)) return { action: 'end-section' }; + const cbMatch = line.match(CHECKBOX_REGEX); if (cbMatch && current) { const name = cbMatch[2].trim(); @@ -95,7 +98,7 @@ function classifyLine(line: string, current: { name: string } | null): LineClass } if (current && line.trim() === '') return { action: 'skip' }; - if (current) return { action: 'end-section' }; + if (current) return { action: 'skip' }; return { action: 'skip' }; } @@ -197,7 +200,7 @@ export function addItemToChecklist( if (inSection) { if (CHECKBOX_REGEX.test(lines[i])) { insertIdx = i; - } else if (lines[i].trim() !== '') { + } else if (HEADING_REGEX.test(lines[i])) { break; } } @@ -308,12 +311,11 @@ function scanSection(lines: string[], checklistName: string, targetItemName: str continue; } if (!inSection) continue; + if (HEADING_REGEX.test(lines[i])) break; const cbMatch = lines[i].match(CHECKBOX_REGEX); if (cbMatch) { itemCount++; if (cbMatch[2].trim() === targetItemName && targetLineIdx === -1) targetLineIdx = i; - } else if (lines[i].trim() !== '') { - break; } } diff --git a/src/pm/jira/adapter.ts b/src/pm/jira/adapter.ts index e3873e7c2..c31ccd370 100644 --- a/src/pm/jira/adapter.ts +++ b/src/pm/jira/adapter.ts @@ -6,6 +6,7 @@ import { jiraClient } from '../../jira/client.js'; import { logger } from '../../utils/logging.js'; +import { withDescriptionMutationLock } from '../_shared/description-mutation-lock.js'; import { addItemToChecklist, appendChecklistSection, @@ -327,33 +328,59 @@ export class JiraPMProvider implements PMProvider { } /** - * Read-modify-write the issue description as ADF round-trip. - * ADF → markdown → mutate → ADF. Retries once on conflict. + * Serialize and read-modify-write the issue description as ADF round-trip. + * ADF → markdown → mutate → ADF. Retries once on provider failure. */ private async updateDescription( issueKey: string, mutate: (desc: string) => string, ): Promise { - const apply = async () => { - const issue = await jiraClient.getIssue(issueKey); + await withDescriptionMutationLock('jira', issueKey, () => + this.updateDescriptionWithProviderRetry(issueKey, mutate), + ); + } + + private async updateDescriptionWithProviderRetry( + issueKey: string, + mutate: (desc: string) => string, + ): Promise { + for (let attempt = 0; attempt < 2; attempt++) { + let issue: Awaited>; + try { + issue = await jiraClient.getIssue(issueKey); + } catch (err) { + if (attempt === 0) { + this.logDescriptionRetry(issueKey, err); + continue; + } + throw err; + } + const adfDesc = (issue.fields as JiraSearchIssue['fields'])?.description; const markdown = adfDesc ? adfToPlainText(adfDesc) : ''; const newMarkdown = mutate(markdown); - await jiraClient.updateIssue(issueKey, { - description: markdownToAdf(newMarkdown), - }); - }; - try { - await apply(); - } catch (err) { - logger.warn('[JIRA] Description update failed; retrying once', { - issueKey, - error: String(err), - }); - await apply(); + try { + await jiraClient.updateIssue(issueKey, { + description: markdownToAdf(newMarkdown), + }); + return; + } catch (err) { + if (attempt === 0) { + this.logDescriptionRetry(issueKey, err); + continue; + } + throw err; + } } } + private logDescriptionRetry(issueKey: string, err: unknown): void { + logger.warn('[JIRA] Description provider update failed; retrying once', { + issueKey, + error: String(err), + }); + } + async getAttachments(workItemId: string): Promise { const issue = await jiraClient.getIssue(workItemId); const attachments = diff --git a/src/pm/linear/adapter.ts b/src/pm/linear/adapter.ts index 6f8a422ac..d68a44d88 100644 --- a/src/pm/linear/adapter.ts +++ b/src/pm/linear/adapter.ts @@ -11,6 +11,7 @@ import { resolveLabelId as sharedResolveLabelId } from '../../integrations/pm/_shared/label-id-resolver.js'; import { linearClient } from '../../linear/client.js'; import { logger } from '../../utils/logging.js'; +import { withDescriptionMutationLock } from '../_shared/description-mutation-lock.js'; import { addItemToChecklist, appendChecklistSection, @@ -256,28 +257,55 @@ export class LinearPMProvider implements PMProvider { } /** - * Read-modify-write the issue description with one retry on conflict. + * Serialize and read-modify-write the issue description with one retry on provider failure. * Used by all checklist mutation methods. */ private async updateDescription( issueId: string, mutate: (desc: string) => string, ): Promise { - try { - const issue = await linearClient.getIssue(issueId); - const newDesc = mutate(issue.description ?? ''); - await linearClient.updateIssue(issueId, { description: newDesc }); - } catch (err) { - logger.warn('[Linear] Description update failed; retrying once', { - issueId, - error: String(err), - }); - const issue = await linearClient.getIssue(issueId); + await withDescriptionMutationLock('linear', issueId, () => + this.updateDescriptionWithProviderRetry(issueId, mutate), + ); + } + + private async updateDescriptionWithProviderRetry( + issueId: string, + mutate: (desc: string) => string, + ): Promise { + for (let attempt = 0; attempt < 2; attempt++) { + let issue: Awaited>; + try { + issue = await linearClient.getIssue(issueId); + } catch (err) { + if (attempt === 0) { + this.logDescriptionRetry(issueId, err); + continue; + } + throw err; + } + const newDesc = mutate(issue.description ?? ''); - await linearClient.updateIssue(issueId, { description: newDesc }); + try { + await linearClient.updateIssue(issueId, { description: newDesc }); + return; + } catch (err) { + if (attempt === 0) { + this.logDescriptionRetry(issueId, err); + continue; + } + throw err; + } } } + private logDescriptionRetry(issueId: string, err: unknown): void { + logger.warn('[Linear] Description provider update failed; retrying once', { + issueId, + error: String(err), + }); + } + async getAttachments(workItemId: string): Promise { const attachments = await linearClient.getAttachments(workItemId); return attachments.map((a) => ({ diff --git a/tests/unit/pm/_shared/description-mutation-lock.test.ts b/tests/unit/pm/_shared/description-mutation-lock.test.ts new file mode 100644 index 000000000..eb22d0830 --- /dev/null +++ b/tests/unit/pm/_shared/description-mutation-lock.test.ts @@ -0,0 +1,107 @@ +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { readdir, unlink, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { withDescriptionMutationLock } from '../../../../src/pm/_shared/description-mutation-lock.js'; + +describe('withDescriptionMutationLock', () => { + let lockDir: string; + + beforeEach(() => { + lockDir = mkdtempSync(join(tmpdir(), 'cascade-description-lock-test-')); + }); + + afterEach(() => { + rmSync(lockDir, { recursive: true, force: true }); + }); + + it('serializes concurrent work for the same provider and work item', async () => { + const events: string[] = []; + + const first = withDescriptionMutationLock( + 'linear', + 'MNG-656', + async () => { + events.push('a:start'); + await sleep(20); + events.push('a:end'); + }, + { lockDir, pollMs: 1 }, + ); + while (!events.includes('a:start')) await sleep(1); + + await Promise.all([ + first, + withDescriptionMutationLock( + 'linear', + 'MNG-656', + async () => { + events.push('b:start'); + await sleep(1); + events.push('b:end'); + }, + { lockDir, pollMs: 1 }, + ), + ]); + + expect(events).toHaveLength(4); + expect(events.indexOf('a:end')).toBeLessThan(events.indexOf('b:start')); + }); + + it('removes stale lock files and continues', async () => { + const lockPath = join(lockDir, 'jira-PROJ-1.lock'); + writeFileSync(lockPath, JSON.stringify({ token: 'stale', createdAt: 1, pid: 1 })); + const oldTime = new Date(Date.now() - 10_000); + await import('node:fs/promises').then((fs) => fs.utimes(lockPath, oldTime, oldTime)); + + const result = await withDescriptionMutationLock('jira', 'PROJ-1', async () => 'ok', { + lockDir, + staleMs: 1, + pollMs: 1, + }); + + expect(result).toBe('ok'); + expect(await readdir(lockDir)).toEqual([]); + }); + + it('times out when a fresh lock remains held', async () => { + const lockPath = join(lockDir, 'linear-MNG-656.lock'); + writeFileSync(lockPath, JSON.stringify({ token: 'held', createdAt: Date.now(), pid: 1 })); + + await expect( + withDescriptionMutationLock('linear', 'MNG-656', async () => undefined, { + lockDir, + timeoutMs: 5, + staleMs: 60_000, + pollMs: 1, + }), + ).rejects.toThrow('Timed out waiting for description mutation lock'); + }); + + it('does not release a newer owner lock when the original token is gone', async () => { + let replacementPath = ''; + + await withDescriptionMutationLock( + 'linear', + 'MNG/656', + async () => { + const files = await readdir(lockDir); + replacementPath = join(lockDir, files[0]); + await unlink(replacementPath); + await writeFile( + replacementPath, + JSON.stringify({ token: 'new-owner', createdAt: Date.now(), pid: process.pid }), + ); + }, + { lockDir, pollMs: 1 }, + ); + + expect(existsSync(replacementPath)).toBe(true); + expect(JSON.parse(readFileSync(replacementPath, 'utf8')).token).toBe('new-owner'); + }); +}); + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} diff --git a/tests/unit/pm/_shared/inline-checklist.test.ts b/tests/unit/pm/_shared/inline-checklist.test.ts index 11bf13e93..86bf40e64 100644 --- a/tests/unit/pm/_shared/inline-checklist.test.ts +++ b/tests/unit/pm/_shared/inline-checklist.test.ts @@ -113,6 +113,24 @@ This is just a paragraph. expect(result).toHaveLength(1); expect(result[0].name).toBe('H3 heading'); }); + + it('keeps parsing checklist items separated by rich markdown detail lines', () => { + const desc = `### AC +- [ ] First + Indented detail for first item. + +Normal prose between rows. +- bullet detail that is not a checkbox +- [ ] Second + +### Other +- [x] Third`; + + const result = parseInlineChecklists(desc); + expect(result).toHaveLength(2); + expect(result[0].items.map((item) => item.name)).toEqual(['First', 'Second']); + expect(result[1].items.map((item) => item.name)).toEqual(['Third']); + }); }); // --------------------------------------------------------------------------- @@ -178,6 +196,14 @@ describe('addItemToChecklist', () => { const result = addItemToChecklist(desc, 'AC', 'Unchecked'); expect(result).toBe('### AC\n- [x] First\n- [ ] Unchecked'); }); + + it('adds after the last checkbox even when detail lines appear between rows', () => { + const desc = '### AC\n- [ ] First\n Detail\n- [ ] Second\n\n### Next\n- [ ] Other'; + const result = addItemToChecklist(desc, 'AC', 'Third'); + expect(result).toBe( + '### AC\n- [ ] First\n Detail\n- [ ] Second\n- [ ] Third\n\n### Next\n- [ ] Other', + ); + }); }); // --------------------------------------------------------------------------- @@ -214,6 +240,13 @@ describe('toggleChecklistItem', () => { expect(result).toContain('### CL-A\n- [ ] Same'); expect(result).toContain('### CL-B\n- [x] Same'); }); + + it('toggles items after prose in the same checklist section', () => { + const rich = '### AC\n- [ ] First\nPlain detail\n- [ ] Second'; + const checklists = parseInlineChecklists(rich); + const result = toggleChecklistItem(rich, checklists[0].items[1].id, true, checklists); + expect(result).toBe('### AC\n- [ ] First\nPlain detail\n- [x] Second'); + }); }); // --------------------------------------------------------------------------- @@ -242,6 +275,13 @@ describe('removeChecklistItem', () => { const checklists = parseInlineChecklists(desc); expect(() => removeChecklistItem(desc, 'cl-00000000', checklists)).toThrow(); }); + + it('removes items after prose in the same checklist section', () => { + const desc = '### AC\n- [ ] Keep\nSome detail\n- [ ] Remove\n\n### Next\n- [ ] Other'; + const checklists = parseInlineChecklists(desc); + const result = removeChecklistItem(desc, checklists[0].items[1].id, checklists); + expect(result).toBe('### AC\n- [ ] Keep\nSome detail\n\n### Next\n- [ ] Other'); + }); }); // --------------------------------------------------------------------------- diff --git a/tests/unit/pm/jira/adapter.test.ts b/tests/unit/pm/jira/adapter.test.ts index 70fc68592..793724030 100644 --- a/tests/unit/pm/jira/adapter.test.ts +++ b/tests/unit/pm/jira/adapter.test.ts @@ -691,6 +691,28 @@ describe('JiraPMProvider', () => { expect(mockJiraClient.transitionIssue).not.toHaveBeenCalled(); }); + + it('serializes concurrent ADF round-trips so all checklist rows are retained', async () => { + let description = '### ✅ AC\n- [ ] Item A\n- [ ] Item B\n- [ ] Item C'; + mockJiraClient.getIssue.mockImplementation(async () => ({ + fields: { description }, + })); + mockAdfToPlainText.mockImplementation((value) => String(value)); + mockMarkdownToAdf.mockImplementation((markdown) => markdown); + mockJiraClient.updateIssue.mockImplementation( + async (_issueKey: string, updates: { description?: unknown }) => { + await sleep(5); + description = String(updates.description ?? description); + }, + ); + + const checklists = await provider.getChecklists('PROJ-1'); + await Promise.all( + checklists[0].items.map((item) => provider.updateChecklistItem('PROJ-1', item.id, true)), + ); + + expect(description).toBe('### ✅ AC\n- [x] Item A\n- [x] Item B\n- [x] Item C'); + }); }); describe('deleteChecklistItem (inline)', () => { @@ -743,6 +765,20 @@ describe('JiraPMProvider', () => { expect(mockJiraClient.updateIssue).toHaveBeenCalledTimes(2); }); + + it('does not retry local checklist mutation errors', async () => { + mockJiraClient.getIssue.mockResolvedValue({ + fields: { description: { type: 'doc', content: [] } }, + }); + mockAdfToPlainText.mockReturnValue('### ✅ AC\n- [ ] Item'); + + await expect(provider.updateChecklistItem('PROJ-1', 'cl-00000000', true)).rejects.toThrow( + 'Checklist item not found', + ); + + expect(mockJiraClient.getIssue).toHaveBeenCalledTimes(1); + expect(mockJiraClient.updateIssue).not.toHaveBeenCalled(); + }); }); describe('getAttachments', () => { @@ -867,3 +903,7 @@ describe('JiraPMProvider', () => { }); }); }); + +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 daee419c8..2ea696312 100644 --- a/tests/unit/pm/linear/adapter.test.ts +++ b/tests/unit/pm/linear/adapter.test.ts @@ -558,6 +558,25 @@ describe('LinearPMProvider', () => { expect(mockUpdateIssueState).not.toHaveBeenCalled(); }); + + it('serializes concurrent description rewrites so all checklist rows are retained', async () => { + let description = '### ✅ AC\n- [ ] Item A\n- [ ] Item B\n- [ ] Item C'; + mockGetIssue.mockImplementation(async () => makeIssue({ description })); + mockUpdateIssue.mockImplementation(async (_id, updates: { description?: string }) => { + await sleep(5); + description = updates.description ?? description; + return makeIssue({ description }); + }); + + const checklists = await provider.getChecklists('issue-uuid'); + await Promise.all( + checklists[0].items.map((item) => + provider.updateChecklistItem('issue-uuid', item.id, true), + ), + ); + + expect(description).toBe('### ✅ AC\n- [x] Item A\n- [x] Item B\n- [x] Item C'); + }); }); describe('deleteChecklistItem (inline)', () => { @@ -588,6 +607,17 @@ describe('LinearPMProvider', () => { expect(mockUpdateIssue).toHaveBeenCalledTimes(2); }); + + it('does not retry local checklist mutation errors', async () => { + mockGetIssue.mockResolvedValue(makeIssue({ description: '### ✅ AC\n- [ ] Item' })); + + await expect(provider.updateChecklistItem('issue-uuid', 'cl-00000000', true)).rejects.toThrow( + 'Checklist item not found', + ); + + expect(mockGetIssue).toHaveBeenCalledTimes(1); + expect(mockUpdateIssue).not.toHaveBeenCalled(); + }); }); // ========================================================================= @@ -706,3 +736,7 @@ describe('LinearPMProvider', () => { }); }); }); + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} From 459c1c92a8f8e35b8452ba2e1adc8e5c43a3f5fa Mon Sep 17 00:00:00 2001 From: Cascade Bot Date: Mon, 11 May 2026 12:03:50 +0000 Subject: [PATCH 2/4] fix(pm): preserve trailing detail lines in addItemToChecklist and removeChecklistItem addItemToChecklist() anchored insertion at the last checkbox line, so trailing detail/prose lines that belong to that item got displaced under the newly-inserted checkbox. Fix: advance insertIdx past non-empty non-heading lines so the new item always lands after any trailing detail. removeChecklistItem() called removeSectionBlock with targetLineIdx (the checkbox line) even when the section held detail lines after the only checkbox, leaving those lines orphaned before the next heading. Fix: scanSection now tracks lastContentIdx (last non-empty line in section) and the single-item deletion path uses it as the section end. Adds regression tests for both scenarios per reviewer feedback. Co-Authored-By: Claude Sonnet 4.6 --- src/pm/_shared/inline-checklist.ts | 17 +++++++++++++++-- tests/unit/pm/_shared/inline-checklist.test.ts | 15 +++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/pm/_shared/inline-checklist.ts b/src/pm/_shared/inline-checklist.ts index 0ad43adf5..21147b50b 100644 --- a/src/pm/_shared/inline-checklist.ts +++ b/src/pm/_shared/inline-checklist.ts @@ -202,6 +202,10 @@ export function addItemToChecklist( insertIdx = i; } else if (HEADING_REGEX.test(lines[i])) { break; + } else if (lines[i].trim() !== '') { + // Non-empty detail/prose line — advance insertIdx so new items land + // after all trailing detail belonging to the previous item, not before it. + insertIdx = i; } } } @@ -248,7 +252,10 @@ export function removeChecklistItem( if (scan.targetLineIdx === -1) throw new Error(`Checklist item line not found: ${itemId}`); if (scan.itemCount === 1) { - removeSectionBlock(lines, scan.headingIdx, scan.targetLineIdx); + // Remove the entire section: use lastContentIdx so trailing detail lines + // after the only checkbox are included and not left orphaned. + const sectionEnd = scan.lastContentIdx !== -1 ? scan.lastContentIdx : scan.targetLineIdx; + removeSectionBlock(lines, scan.headingIdx, sectionEnd); } else { lines.splice(scan.targetLineIdx, 1); } @@ -295,6 +302,8 @@ interface SectionScan { headingIdx: number; targetLineIdx: number; itemCount: number; + /** Index of the last non-empty line in the section (may be a detail line after the last checkbox). */ + lastContentIdx: number; } function scanSection(lines: string[], checklistName: string, targetItemName: string): SectionScan { @@ -303,6 +312,7 @@ function scanSection(lines: string[], checklistName: string, targetItemName: str let targetLineIdx = -1; let inSection = false; let itemCount = 0; + let lastContentIdx = -1; for (let i = 0; i < lines.length; i++) { if (lines[i] === heading) { @@ -317,9 +327,12 @@ function scanSection(lines: string[], checklistName: string, targetItemName: str itemCount++; if (cbMatch[2].trim() === targetItemName && targetLineIdx === -1) targetLineIdx = i; } + if (lines[i].trim() !== '') { + lastContentIdx = i; + } } - return { headingIdx, targetLineIdx, itemCount }; + return { headingIdx, targetLineIdx, itemCount, lastContentIdx }; } function removeSectionBlock(lines: string[], headingIdx: number, lastItemIdx: number): void { diff --git a/tests/unit/pm/_shared/inline-checklist.test.ts b/tests/unit/pm/_shared/inline-checklist.test.ts index 86bf40e64..13c322bf8 100644 --- a/tests/unit/pm/_shared/inline-checklist.test.ts +++ b/tests/unit/pm/_shared/inline-checklist.test.ts @@ -204,6 +204,13 @@ describe('addItemToChecklist', () => { '### AC\n- [ ] First\n Detail\n- [ ] Second\n- [ ] Third\n\n### Next\n- [ ] Other', ); }); + + it('adds after trailing detail lines that follow the last checkbox', () => { + // Regression: detail after the last checkbox must not be displaced under the new item. + const desc = '### AC\n- [ ] First\n Detail line'; + const result = addItemToChecklist(desc, 'AC', 'Second'); + expect(result).toBe('### AC\n- [ ] First\n Detail line\n- [ ] Second'); + }); }); // --------------------------------------------------------------------------- @@ -282,6 +289,14 @@ describe('removeChecklistItem', () => { const result = removeChecklistItem(desc, checklists[0].items[1].id, checklists); expect(result).toBe('### AC\n- [ ] Keep\nSome detail\n\n### Next\n- [ ] Other'); }); + + it('removes the whole section including trailing detail when deleting the only item', () => { + // Regression: detail lines after the sole checkbox must not be left orphaned. + const desc = '### AC\n- [ ] Only\nDetail line\n\n### Next\n- [ ] Other'; + const checklists = parseInlineChecklists(desc); + const result = removeChecklistItem(desc, checklists[0].items[0].id, checklists); + expect(result).toBe('### Next\n- [ ] Other'); + }); }); // --------------------------------------------------------------------------- From 25eb09c79a92024702b6f0dd19598d0a4e87de16 Mon Sep 17 00:00:00 2001 From: Cascade Bot Date: Mon, 11 May 2026 12:16:45 +0000 Subject: [PATCH 3/4] fix(pm): remove detail lines with deleted checkbox in multi-item sections When deleting a checklist item in a multi-item section, also remove any non-checkbox, non-heading, non-blank detail/prose lines that immediately follow the deleted checkbox line. Previously only the checkbox line was spliced out, leaving trailing detail orphaned (e.g. ` Detail for remove` appearing above the next item or attached to the previous one). The single-item section path already used `lastContentIdx` to cover this case; the multi-item `else` branch now scans forward from the target line and removes the whole item block in a single splice. Co-Authored-By: Claude Sonnet 4.6 --- src/pm/_shared/inline-checklist.ts | 11 ++++++++++- tests/unit/pm/_shared/inline-checklist.test.ts | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/pm/_shared/inline-checklist.ts b/src/pm/_shared/inline-checklist.ts index 21147b50b..f8063a711 100644 --- a/src/pm/_shared/inline-checklist.ts +++ b/src/pm/_shared/inline-checklist.ts @@ -257,7 +257,16 @@ export function removeChecklistItem( const sectionEnd = scan.lastContentIdx !== -1 ? scan.lastContentIdx : scan.targetLineIdx; removeSectionBlock(lines, scan.headingIdx, sectionEnd); } else { - lines.splice(scan.targetLineIdx, 1); + // Also remove detail/prose lines immediately following the deleted checkbox + // (up to the next checkbox, heading, or blank line) so they aren't orphaned. + let deleteEnd = scan.targetLineIdx; + for (let i = scan.targetLineIdx + 1; i < lines.length; i++) { + if (HEADING_REGEX.test(lines[i]) || CHECKBOX_REGEX.test(lines[i]) || lines[i].trim() === '') { + break; + } + deleteEnd = i; + } + lines.splice(scan.targetLineIdx, deleteEnd - scan.targetLineIdx + 1); } return lines.join('\n').trimEnd(); diff --git a/tests/unit/pm/_shared/inline-checklist.test.ts b/tests/unit/pm/_shared/inline-checklist.test.ts index 13c322bf8..aa59276cf 100644 --- a/tests/unit/pm/_shared/inline-checklist.test.ts +++ b/tests/unit/pm/_shared/inline-checklist.test.ts @@ -297,6 +297,22 @@ describe('removeChecklistItem', () => { const result = removeChecklistItem(desc, checklists[0].items[0].id, checklists); expect(result).toBe('### Next\n- [ ] Other'); }); + + it('removes trailing detail lines when deleting a non-last item in a multi-item section', () => { + // Regression: detail after a deleted checkbox in a multi-item section must not be left orphaned. + const desc = '### AC\n- [ ] Remove\n Detail for remove\n- [ ] Keep'; + const checklists = parseInlineChecklists(desc); + const result = removeChecklistItem(desc, checklists[0].items[0].id, checklists); + expect(result).toBe('### AC\n- [ ] Keep'); + }); + + it('removes trailing detail lines when deleting the last item in a multi-item section', () => { + // Regression: trailing detail after the last checkbox must not become attached to the previous item. + const desc = '### AC\n- [ ] First\n- [ ] Last\n Trailing detail'; + const checklists = parseInlineChecklists(desc); + const result = removeChecklistItem(desc, checklists[0].items[1].id, checklists); + expect(result).toBe('### AC\n- [ ] First'); + }); }); // --------------------------------------------------------------------------- From c7bfc928fd5b3692a2cade39d349870f88553309 Mon Sep 17 00:00:00 2001 From: aaight Date: Mon, 11 May 2026 15:56:58 +0200 Subject: [PATCH 4/4] fix(sentry): handle REST issue event formatting (#1342) * fix(sentry): handle REST issue event formatting * fix(sentry): handle REST request query as tuple pairs, record, or string Widens SentryRequest with a `query` field (tuple pairs | record | string) to cover Sentry's Retrieve-an-Issue-Event REST shape where query params are returned under `data.query` rather than `queryString`. Adds normalizeRequestQuery() helper that falls back from query_string/queryString to the tuple-pairs/record/string shape, and regression tests for each variant plus the precedence rule. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Cascade Bot Co-authored-by: Claude Sonnet 4.6 --- docs/architecture/07-gadgets.md | 4 +- src/gadgets/sentry/core/format.ts | 201 +++++++++++++++-- src/sentry/types.ts | 63 +++++- tests/unit/gadgets/sentry/format.test.ts | 207 ++++++++++++++++++ .../sentry/getSentryEventDetail.test.ts | 81 +++++++ 5 files changed, 524 insertions(+), 32 deletions(-) create mode 100644 tests/unit/gadgets/sentry/format.test.ts create mode 100644 tests/unit/gadgets/sentry/getSentryEventDetail.test.ts diff --git a/docs/architecture/07-gadgets.md b/docs/architecture/07-gadgets.md index 0225854f3..9875ff4bd 100644 --- a/docs/architecture/07-gadgets.md +++ b/docs/architecture/07-gadgets.md @@ -104,9 +104,11 @@ cascade-tools pm report-friction \ | Gadget | Capability | Purpose | |--------|-----------|---------| | `GetAlertingIssue` | `alerting:read` | Fetch Sentry issue details | -| `GetAlertingEventDetail` | `alerting:read` | Fetch specific event with stacktrace | +| `GetAlertingEventDetail` | `alerting:read` | Fetch Sentry issue-event details with stacktrace, tags, breadcrumbs, request data, and context | | `ListAlertingEvents` | `alerting:read` | List recent events for an issue | +`GetAlertingEventDetail` accepts Sentry's issue-event response shape, including REST aliases from the [Retrieve an Issue Event API](https://docs.sentry.io/api/events/retrieve-an-issue-event/). + ## cascade-tools CLI `src/cli/` — the `cascade-tools` binary diff --git a/src/gadgets/sentry/core/format.ts b/src/gadgets/sentry/core/format.ts index 4a2b8cb4d..eb8c12bcb 100644 --- a/src/gadgets/sentry/core/format.ts +++ b/src/gadgets/sentry/core/format.ts @@ -8,6 +8,7 @@ import type { SentryEvent, SentryException, SentryIssue, + SentryRequest, SentryStackFrame, } from '../../../sentry/types.js'; @@ -51,24 +52,28 @@ export function formatSentryIssue(issue: SentryIssue): string { function formatStackFrame(frame: SentryStackFrame, index: number): string { const lines: string[] = []; - const location = [frame.filename ?? frame.abs_path, frame.lineno].filter(Boolean).join(':'); + const lineno = frame.lineno ?? frame.lineNo; + const location = [frame.filename ?? frame.abs_path ?? frame.absPath, lineno] + .filter(Boolean) + .join(':'); const fn = frame.function ?? ''; - const inApp = frame.in_app ? ' [in_app]' : ''; + const inApp = (frame.in_app ?? frame.inApp) ? ' [in_app]' : ''; lines.push(` Frame ${index}: ${fn}${inApp}`); if (location) lines.push(` at ${location}`); // Source context - if (frame.pre_context?.length) { - for (const line of frame.pre_context) { + const sourceContext = normalizeFrameSourceContext(frame); + if (sourceContext.pre.length) { + for (const line of sourceContext.pre) { lines.push(` | ${line}`); } } - if (frame.context_line !== undefined) { - lines.push(` > | ${frame.context_line} ← error here`); + if (sourceContext.current !== undefined) { + lines.push(` > | ${sourceContext.current} ← error here`); } - if (frame.post_context?.length) { - for (const line of frame.post_context) { + if (sourceContext.post.length) { + for (const line of sourceContext.post) { lines.push(` | ${line}`); } } @@ -81,6 +86,39 @@ function formatStackFrame(frame: SentryStackFrame, index: number): string { return lines.join('\n'); } +function normalizeFrameSourceContext(frame: SentryStackFrame): { + pre: string[]; + current?: string; + post: string[]; +} { + if (frame.pre_context?.length || frame.context_line !== undefined || frame.post_context?.length) { + return { + pre: frame.pre_context ?? [], + current: frame.context_line, + post: frame.post_context ?? [], + }; + } + + if (!frame.context?.length) { + return { pre: [], post: [] }; + } + + const lineNo = frame.lineno ?? frame.lineNo; + const currentIndex = + lineNo === undefined + ? -1 + : frame.context.findIndex(([contextLineNo]) => contextLineNo === lineNo); + if (currentIndex < 0) { + return { pre: frame.context.map(([, line]) => line), post: [] }; + } + + return { + pre: frame.context.slice(0, currentIndex).map(([, line]) => line), + current: frame.context[currentIndex][1], + post: frame.context.slice(currentIndex + 1).map(([, line]) => line), + }; +} + function formatException(exc: SentryException): string { const lines: string[] = []; const header = [exc.type, exc.value].filter(Boolean).join(': '); @@ -128,32 +166,54 @@ function formatBreadcrumbs(breadcrumbs: SentryBreadcrumb[]): string { // ============================================================================ function appendEventMeta(lines: string[], event: SentryEvent): void { - if (event.event_id) lines.push(`Event ID: ${event.event_id}`); - if (event.timestamp) lines.push(`Timestamp: ${event.timestamp}`); + const eventId = getEventId(event); + const timestamp = getEventTimestamp(event); + const release = getReleaseValue(event.release); + if (eventId) lines.push(`Event ID: ${eventId}`); + if (timestamp) lines.push(`Timestamp: ${timestamp}`); if (event.environment) lines.push(`Environment: ${event.environment}`); - if (event.release) lines.push(`Release: ${event.release}`); + if (release) lines.push(`Release: ${release}`); if (event.platform) lines.push(`Platform: ${event.platform}`); if (event.transaction) lines.push(`Transaction: ${event.transaction}`); if (event.level) lines.push(`Level: ${event.level}`); } function appendEventTags(lines: string[], event: SentryEvent): void { - const tags = event.tags; - if (!tags) return; - const tagPairs = Array.isArray(tags) - ? tags.map(([k, v]) => `${k}=${v}`) - : Object.entries(tags).map(([k, v]) => `${k}=${v}`); + const tagPairs = normalizeTagPairs(event.tags).map(([key, value]) => `${key}=${value}`); if (tagPairs.length > 0) { lines.push(`Tags: ${tagPairs.join(', ')}`); } } +function normalizeRequestQuery(request: SentryRequest): string | undefined { + // Prefer the already-serialized query-string aliases + const qs = request.query_string ?? request.queryString; + if (qs) return qs; + + // REST issue-event shape: `query` can be tuple pairs, a plain string, or a record + const q = request.query; + if (!q) return undefined; + if (typeof q === 'string') return q; + if (Array.isArray(q)) { + const pairs = q.map(([k, v]) => `${k}=${v}`).join('&'); + return pairs || undefined; + } + // Record + const pairs = Object.entries(q) + .map(([k, v]) => `${k}=${v}`) + .join('&'); + return pairs || undefined; +} + function appendEventRequest(lines: string[], event: SentryEvent): void { - if (!event.request?.url) return; + const request = getEventRequest(event); + if (!request?.url) return; lines.push(''); lines.push('## Request'); - lines.push(`${event.request.method ?? 'GET'} ${event.request.url}`); - if (event.request.query_string) lines.push(`Query: ${event.request.query_string}`); + lines.push(`${request.method ?? 'GET'} ${request.url}`); + const query = normalizeRequestQuery(request); + if (query) lines.push(`Query: ${query}`); + if (request.data !== undefined) lines.push(`Data: ${formatCompactValue(request.data)}`); } function appendEventUser(lines: string[], event: SentryEvent): void { @@ -168,7 +228,7 @@ function appendEventUser(lines: string[], event: SentryEvent): void { } function appendEventStacktrace(lines: string[], event: SentryEvent): void { - const exceptions = event.exception?.values; + const exceptions = getEventExceptions(event); if (exceptions?.length) { lines.push(''); lines.push('## Exception'); @@ -188,6 +248,99 @@ function appendEventStacktrace(lines: string[], event: SentryEvent): void { } } +function appendEventContext(lines: string[], event: SentryEvent): void { + const contextLines: string[] = []; + for (const [key, value] of Object.entries(event.context ?? {})) { + contextLines.push(`${key}: ${formatCompactValue(value)}`); + } + for (const [key, value] of Object.entries(event.contexts ?? {})) { + if (value === undefined || value === null) continue; + contextLines.push(`${key}: ${formatCompactValue(value)}`); + } + if (contextLines.length === 0) return; + + lines.push(''); + lines.push('## Context'); + for (const line of contextLines.slice(0, 20)) { + lines.push(line); + } +} + +function normalizeTagPairs( + tags: SentryEvent['tags'], +): Array<[string, string | number | boolean | null]> { + if (!tags) return []; + if (!Array.isArray(tags)) { + return Object.entries(tags).filter(([key, value]) => key && value !== undefined); + } + + const pairs: Array<[string, string | number | boolean | null]> = []; + for (const tag of tags) { + if (Array.isArray(tag)) { + const [key, value] = tag; + if (key && value !== undefined) pairs.push([key, value]); + continue; + } + if (tag && typeof tag === 'object' && tag.key && tag.value !== undefined) { + pairs.push([tag.key, tag.value]); + } + } + return pairs; +} + +function getEventId(event: SentryEvent): string | undefined { + return event.event_id ?? event.eventID ?? event.id; +} + +function getEventTimestamp(event: SentryEvent): string | undefined { + return event.timestamp ?? event.dateCreated ?? event.dateReceived ?? event.received; +} + +function getReleaseValue(release: SentryEvent['release']): string | undefined { + if (!release) return undefined; + if (typeof release === 'string') return release; + if (typeof release === 'object') { + const record = release as Record; + const value = record.version ?? record.shortVersion ?? record.package; + return typeof value === 'string' ? value : undefined; + } + return undefined; +} + +function findEntryData(event: SentryEvent, type: string): T | undefined { + const entry = event.entries?.find((candidate) => candidate.type === type); + return entry?.data as T | undefined; +} + +function getEventExceptions(event: SentryEvent): SentryException[] | undefined { + return ( + event.exception?.values ?? + findEntryData<{ values?: SentryException[] }>(event, 'exception')?.values + ); +} + +function getEventBreadcrumbs(event: SentryEvent): SentryBreadcrumb[] | undefined { + return ( + event.breadcrumbs?.values ?? + findEntryData<{ values?: SentryBreadcrumb[] }>(event, 'breadcrumbs')?.values + ); +} + +function getEventRequest(event: SentryEvent): SentryRequest | undefined { + return event.request ?? findEntryData(event, 'request'); +} + +function formatCompactValue(value: unknown): string { + if (typeof value === 'string') return value.slice(0, 200); + if (typeof value === 'number' || typeof value === 'boolean' || value === null) + return String(value); + try { + return JSON.stringify(value, null, 0).slice(0, 200); + } catch { + return String(value).slice(0, 200); + } +} + export function formatSentryEvent(event: SentryEvent): string { const lines: string[] = []; @@ -201,12 +354,13 @@ export function formatSentryEvent(event: SentryEvent): string { appendEventUser(lines, event); appendEventStacktrace(lines, event); - const breadcrumbs = event.breadcrumbs?.values; + const breadcrumbs = getEventBreadcrumbs(event); if (breadcrumbs?.length) { lines.push(''); lines.push('## Breadcrumbs'); lines.push(formatBreadcrumbs(breadcrumbs)); } + appendEventContext(lines, event); if (event.web_url) { lines.push(''); @@ -225,8 +379,9 @@ export function formatSentryEventList(events: SentryEvent[]): string { const lines: string[] = [`${events.length} event(s):`]; for (const e of events) { - const ts = e.timestamp ?? e.received ?? '(unknown time)'; - const id = e.event_id ? e.event_id.slice(0, 8) : '(no id)'; + const ts = getEventTimestamp(e) ?? '(unknown time)'; + const eventId = getEventId(e); + const id = eventId ? eventId.slice(0, 8) : '(no id)'; const tx = e.transaction ? ` — ${e.transaction}` : ''; lines.push(` [${id}] ${ts}${tx}`); } diff --git a/src/sentry/types.ts b/src/sentry/types.ts index 672d1f55a..dd370fc7f 100644 --- a/src/sentry/types.ts +++ b/src/sentry/types.ts @@ -30,6 +30,11 @@ export interface SentryStackFrame { vars?: Record; abs_path?: string; module?: string; + lineNo?: number; + colNo?: number; + inApp?: boolean; + absPath?: string; + context?: Array<[number, string]>; } export interface SentryStackTrace { @@ -56,27 +61,72 @@ export interface SentryBreadcrumb { data?: Record; } +export interface SentryTag { + key?: string; + value?: string | number | boolean | null; +} + +export interface SentryRequest { + url?: string; + method?: string; + headers?: Record; + query_string?: string; + queryString?: string; + /** REST issue-event shape: tuple pairs, a plain string, or a record */ + query?: string | Array<[string, string]> | Record; + data?: unknown; + cookies?: unknown; + env?: Record; +} + +export type SentryEventEntry = + | { + type: 'exception'; + data?: { + values?: SentryException[]; + }; + } + | { + type: 'breadcrumbs'; + data?: { + values?: SentryBreadcrumb[]; + }; + } + | { + type: 'request'; + data?: SentryRequest; + } + | { + type?: string; + data?: unknown; + }; + // ============================================================================ // Sentry event (included in issue alert payloads and REST API responses) // ============================================================================ export interface SentryEvent { + id?: string; event_id?: string; + eventID?: string; url?: string; web_url?: string; issue_id?: string; issue_url?: string; project?: string; - release?: string; + release?: string | Record; environment?: string; platform?: string; message?: string; title?: string; culprit?: string; timestamp?: string; + dateCreated?: string; + dateReceived?: string; received?: string; level?: string; transaction?: string; + metadata?: Record; exception?: { values?: SentryException[]; }; @@ -84,14 +134,11 @@ export interface SentryEvent { breadcrumbs?: { values?: SentryBreadcrumb[]; }; - tags?: Array<[string, string]> | Record; + entries?: SentryEventEntry[]; + tags?: Array<[string, string] | SentryTag> | Record; + context?: Record; contexts?: Record; - request?: { - url?: string; - method?: string; - headers?: Record; - query_string?: string; - }; + request?: SentryRequest; user?: { id?: string; email?: string; diff --git a/tests/unit/gadgets/sentry/format.test.ts b/tests/unit/gadgets/sentry/format.test.ts new file mode 100644 index 000000000..f0608e4d9 --- /dev/null +++ b/tests/unit/gadgets/sentry/format.test.ts @@ -0,0 +1,207 @@ +import { describe, expect, it } from 'vitest'; + +import { + formatSentryEvent, + formatSentryEventList, +} from '../../../../src/gadgets/sentry/core/format.js'; +import type { SentryEvent } from '../../../../src/sentry/types.js'; + +function makeRestEvent(): SentryEvent { + return { + id: 'numeric-event-id', + eventID: 'abcdef1234567890', + title: 'TypeError: object is not iterable', + dateCreated: '2026-05-11T12:00:00Z', + environment: 'production', + release: { version: 'api@1.2.3' }, + platform: 'javascript', + transaction: '/api/items', + level: 'error', + tags: [ + { key: 'environment', value: 'production' }, + { key: 'runtime', value: 'node' }, + { key: undefined, value: 'skipped' }, + ], + entries: [ + { + type: 'exception', + data: { + values: [ + { + type: 'TypeError', + value: 'object is not iterable', + stacktrace: { + frames: [ + { + filename: 'src/worker.ts', + function: 'runWorker', + lineNo: 42, + colNo: 7, + inApp: true, + absPath: '/app/src/worker.ts', + context: [ + [40, 'const tags = event.tags;'], + [41, 'for (const tag of tags) {'], + [42, ' const [key, value] = tag;'], + [43, ' render(key, value);'], + ], + vars: { issueId: '119054737' }, + }, + ], + }, + }, + ], + }, + }, + { + type: 'breadcrumbs', + data: { + values: [ + { + timestamp: '2026-05-11T11:59:59Z', + level: 'info', + category: 'http', + message: 'GET /api/items', + }, + ], + }, + }, + { + type: 'request', + data: { + method: 'POST', + url: 'https://example.com/api/items', + queryString: 'debug=true', + data: { filter: 'open' }, + }, + }, + ], + context: { + runtime: { name: 'node', version: '20.0.0' }, + }, + contexts: { + os: { name: 'Linux' }, + trace: { trace_id: 'trace-1' }, + }, + }; +} + +describe('formatSentryEvent', () => { + it('formats Sentry REST issue-event responses without throwing', () => { + const result = formatSentryEvent(makeRestEvent()); + + expect(result).toContain('Event ID: abcdef1234567890'); + expect(result).toContain('Timestamp: 2026-05-11T12:00:00Z'); + expect(result).toContain('Release: api@1.2.3'); + expect(result).toContain('Tags: environment=production, runtime=node'); + expect(result).toContain('## Exception'); + expect(result).toContain('Exception: TypeError: object is not iterable'); + expect(result).toContain('Stacktrace'); + expect(result).toContain('Frame 0: runWorker [in_app]'); + expect(result).toContain('at src/worker.ts:42'); + expect(result).toContain('const [key, value] = tag;'); + expect(result).toContain('error here'); + expect(result).toContain('## Breadcrumbs'); + expect(result).toContain('GET /api/items'); + expect(result).toContain('## Request'); + expect(result).toContain('POST https://example.com/api/items'); + expect(result).toContain('Query: debug=true'); + expect(result).toContain('## Context'); + expect(result).toContain('runtime: {"name":"node","version":"20.0.0"}'); + expect(result).toContain('os: {"name":"Linux"}'); + }); + + it('formats tuple-array tags', () => { + const result = formatSentryEvent({ + title: 'Tuple tags', + tags: [ + ['environment', 'staging'], + ['release', 'abc123'], + ], + }); + + expect(result).toContain('Tags: environment=staging, release=abc123'); + }); + + it('formats object-map tags', () => { + const result = formatSentryEvent({ + title: 'Object tags', + tags: { + environment: 'production', + handled: false, + }, + }); + + expect(result).toContain('Tags: environment=production, handled=false'); + }); + + it('formats REST request query as tuple pairs', () => { + const result = formatSentryEvent({ + title: 'REST query tuples', + entries: [ + { + type: 'request', + data: { + method: 'GET', + url: 'https://example.com/api/items', + query: [ + ['page', '2'], + ['limit', '50'], + ], + }, + }, + ], + }); + + expect(result).toContain('## Request'); + expect(result).toContain('GET https://example.com/api/items'); + expect(result).toContain('Query: page=2&limit=50'); + }); + + it('formats REST request query as a record', () => { + const result = formatSentryEvent({ + title: 'REST query record', + entries: [ + { + type: 'request', + data: { + method: 'GET', + url: 'https://example.com/api/search', + query: { q: 'TypeError', sort: 'newest' }, + }, + }, + ], + }); + + expect(result).toContain('Query: q=TypeError&sort=newest'); + }); + + it('prefers query_string over query tuple pairs', () => { + const result = formatSentryEvent({ + title: 'query_string wins', + request: { + method: 'GET', + url: 'https://example.com/api', + query_string: 'already=serialized', + query: [['should', 'be-ignored']], + }, + }); + + expect(result).toContain('Query: already=serialized'); + expect(result).not.toContain('be-ignored'); + }); +}); + +describe('formatSentryEventList', () => { + it('uses event ID aliases', () => { + const result = formatSentryEventList([ + { eventID: 'eventid-alias', dateCreated: '2026-05-11T12:00:00Z' }, + { event_id: 'event_id-alias', timestamp: '2026-05-11T12:01:00Z' }, + { id: 'id-alias', received: '2026-05-11T12:02:00Z' }, + ]); + + expect(result).toContain('[eventid-] 2026-05-11T12:00:00Z'); + expect(result).toContain('[event_id] 2026-05-11T12:01:00Z'); + expect(result).toContain('[id-alias] 2026-05-11T12:02:00Z'); + }); +}); diff --git a/tests/unit/gadgets/sentry/getSentryEventDetail.test.ts b/tests/unit/gadgets/sentry/getSentryEventDetail.test.ts new file mode 100644 index 000000000..573f230d2 --- /dev/null +++ b/tests/unit/gadgets/sentry/getSentryEventDetail.test.ts @@ -0,0 +1,81 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { SentryEvent } from '../../../../src/sentry/types.js'; + +const { mockGetIssueEvent } = vi.hoisted(() => ({ + mockGetIssueEvent: vi.fn(), +})); + +vi.mock('../../../../src/sentry/client.js', () => ({ + getSentryClient: vi.fn(() => ({ + getIssueEvent: mockGetIssueEvent, + })), +})); + +import { getSentryEventDetail } from '../../../../src/gadgets/sentry/core/getSentryEventDetail.js'; + +function makeRestEvent(): SentryEvent { + return { + eventID: 'abcdef1234567890', + title: 'TypeError: object is not iterable', + dateCreated: '2026-05-11T12:00:00Z', + tags: [{ key: 'environment', value: 'production' }], + entries: [ + { + type: 'exception', + data: { + values: [ + { + type: 'TypeError', + value: 'object is not iterable', + stacktrace: { + frames: [ + { + filename: 'src/worker.ts', + function: 'runWorker', + lineNo: 42, + inApp: true, + context: [ + [41, 'for (const tag of tags) {'], + [42, ' const [key, value] = tag;'], + ], + }, + ], + }, + }, + ], + }, + }, + ], + contexts: { + runtime: { name: 'node' }, + }, + }; +} + +describe('getSentryEventDetail', () => { + beforeEach(() => { + mockGetIssueEvent.mockReset(); + }); + + it('formats REST-shaped issue-event responses', async () => { + mockGetIssueEvent.mockResolvedValueOnce(makeRestEvent()); + + const result = await getSentryEventDetail('mongrel', '119054737', 'latest'); + + expect(mockGetIssueEvent).toHaveBeenCalledWith('mongrel', '119054737', 'latest'); + expect(result).toContain('Event ID: abcdef1234567890'); + expect(result).toContain('Tags: environment=production'); + expect(result).toContain('## Exception'); + expect(result).toContain('Stacktrace'); + expect(result).toContain('## Context'); + }); + + it('defaults eventId to latest', async () => { + mockGetIssueEvent.mockResolvedValueOnce(makeRestEvent()); + + await getSentryEventDetail('mongrel', '119054737'); + + expect(mockGetIssueEvent).toHaveBeenCalledWith('mongrel', '119054737', 'latest'); + }); +});