fix(jira): discard detail replies after closing an issue (#24835)

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>

config/scripts/check-changed-code-quality.mjs

@@ -13,6 +13,9 @@ const CASTING_DISABLE_PATTERN =
   /\/[/*]\s*(?:oxlint|eslint)-disable(?:-next-line|-line)?\s[^\n]*typescript\/consistent-type-assertions/
 const ANTI_SLOP_DISABLE_PATTERN =
   /\/[/*]\s*(?:oxlint|eslint)-disable(?:-next-line|-line)?\s[^\n]*\banti-slop\//
+const REACT_DOCTOR_DISABLE_PATTERN =
+  /^\s*\/[/*]\s*(?:oxlint|eslint)-disable(?:-next-line|-line)?\s+react-doctor\/[\w-]+(?:\s*,\s*react-doctor\/[\w-]+)*\s*(?:--(?:(?!\*\/).)*)?(?:\*\/)?\s*$/
+const EXPLICIT_DISABLE_RULE_PATTERN = /(?:-disable(?:-next-line|-line)?\s+|^)[\w-]+(?:\/[\w-]+)?/
 export const OXLINT_SCANS = [
   {
     // Why: no --config, so Oxlint keeps discovering nested configs. Pinning the root
@@ -41,7 +44,12 @@ export const OXLINT_SCANS = [
   },
   {
     label: 'React Doctor',
-    args: ['--config', 'config/oxlint-react-doctor.json']
+    args: [
+      '--config',
+      'config/oxlint-react-doctor.json',
+      '--report-unused-disable-directives-severity',
+      'warn'
+    ]
   },
   {
     // Why changed-lines only: the renderer carries ~4.7k pre-existing restyle/raw-color
@@ -348,16 +356,34 @@ export function isCastingDirectiveUnusedWarning(diagnostic, root) {
   )
 }
 
-// Why: the anti-slop rules live in a JS plugin that only config/oxlint-anti-slop.json loads, so
-// the root scan never sees those rule names and reports every anti-slop suppression as unused.
-// `audit:anti-slop` is the scan that enforces them.
-export function isAntiSlopDirectiveUnusedWarning(diagnostic, root) {
+// Unloaded plugin directives are checked by their owning scan.
+export function isUnloadedPluginDirectiveUnusedWarning(diagnostic, root, scanLabel) {
   if (!/^Unused (?:oxlint|eslint)-disable/.test(diagnostic.message ?? '')) {
     return false
   }
+  if (scanLabel === 'React Doctor') {
+    const labels = diagnostic.labels ?? []
+    return (
+      labels.length > 0 &&
+      labels.every(({ span }) => {
+        if (span.offset === undefined || span.length === undefined) {
+          return false
+        }
+        const file = path.isAbsolute(diagnostic.filename)
+          ? diagnostic.filename
+          : path.join(root, diagnostic.filename)
+        // Oxlint spans use UTF-8 byte offsets, including before non-ASCII comments.
+        const directive = readFileSync(file)
+          .subarray(span.offset, span.offset + span.length)
+          .toString('utf8')
+        const rules = directive.split('--')[0]
+        return EXPLICIT_DISABLE_RULE_PATTERN.test(rules) && !/\breact-doctor\//.test(rules)
+      })
+    )
+  }
   return (diagnostic.labels ?? []).some((label) =>
-    diagnosticHighlightedLines(root, diagnostic.filename, label.span).some((line) =>
-      ANTI_SLOP_DISABLE_PATTERN.test(line)
+    diagnosticHighlightedLines(root, diagnostic.filename, label.span).some(
+      (line) => ANTI_SLOP_DISABLE_PATTERN.test(line) || REACT_DOCTOR_DISABLE_PATTERN.test(line)
     )
   )
 }
@@ -436,7 +462,7 @@ export function main(
       (diagnostic) =>
         !isSuppressedDiagnostic(diagnostic, root) &&
         !isCastingDirectiveUnusedWarning(diagnostic, root) &&
-        !isAntiSlopDirectiveUnusedWarning(diagnostic, root) &&
+        !isUnloadedPluginDirectiveUnusedWarning(diagnostic, root, scan.label) &&
         diagnosticTouchesAddedLines(diagnostic, rangesByFile, root, baseBlocks)
     )
     for (const diagnostic of diagnostics) {

config/scripts/check-changed-code-quality.test.mjs

@@ -1,10 +1,12 @@
 import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
 import path from 'node:path'
 import { describe, expect, it } from 'vitest'
+import { runProcessSync } from '../../src/shared/child-process/run-process'
+import { resolveOxlintInvocation } from './oxlint-cli-invocation.mjs'
 import {
   OXLINT_SCANS,
   diagnosticTouchesAddedLines,
-  isAntiSlopDirectiveUnusedWarning,
+  isUnloadedPluginDirectiveUnusedWarning,
   isMovedCode,
   isRootCodeQualityPath,
   overlapsAddedLines,
@@ -124,7 +126,7 @@ describe('moved-code exemption', () => {
   })
 })
 
-describe('anti-slop directive unused warning', () => {
+describe('unloaded plugin directive unused warning', () => {
   const root = path.resolve(import.meta.dirname, '..', '..')
   // Assembled so no line here is itself a directive the gate would scan.
   const directive = (rule) => `/* oxlint-disable ${rule} -- reason */`
@@ -146,21 +148,149 @@ describe('anti-slop directive unused warning', () => {
 
   it('exempts a suppression the root scan cannot resolve', () => {
     withFixture(directive('anti-slop/no-module-mocking'), (diagnostic) => {
-      expect(isAntiSlopDirectiveUnusedWarning(diagnostic, root)).toBe(true)
+      expect(isUnloadedPluginDirectiveUnusedWarning(diagnostic, root, 'code quality')).toBe(true)
     })
   })
 
   it('still reports an unused directive for a rule the root scan does load', () => {
     withFixture(directive('unicorn/no-array-reduce'), (diagnostic) => {
-      expect(isAntiSlopDirectiveUnusedWarning(diagnostic, root)).toBe(false)
+      expect(isUnloadedPluginDirectiveUnusedWarning(diagnostic, root, 'code quality')).toBe(false)
     })
   })
 
   it('ignores diagnostics that are not unused-directive warnings', () => {
     withFixture(directive('anti-slop/no-module-mocking'), (diagnostic) => {
       expect(
-        isAntiSlopDirectiveUnusedWarning({ ...diagnostic, message: 'Unexpected any.' }, root)
+        isUnloadedPluginDirectiveUnusedWarning(
+          { ...diagnostic, message: 'Unexpected any.' },
+          root,
+          'code quality'
+        )
       ).toBe(false)
     })
   })
+
+  function scanFixture(label, file) {
+    const scan = OXLINT_SCANS.find((candidate) => candidate.label === label)
+    if (!scan) {
+      throw new Error(`Missing ${label} scan`)
+    }
+    const { command, prefixArgs } = resolveOxlintInvocation(root)
+    const result = runProcessSync({
+      program: command,
+      args: [...prefixArgs, ...scan.args, '--format', 'json', file],
+      cwd: root,
+      timeoutMs: 30_000,
+      maxOutputBytes: 4 * 1024 * 1024
+    })
+    return JSON.parse(result.stdout).diagnostics
+  }
+
+  it('accepts a used Doctor directive only through its loaded scan', () => {
+    const source = [
+      "import { useEffect, useState } from 'react'",
+      directive('react-doctor/no-derived-state-effect'),
+      'export function Title({ title }: { title: string }) {',
+      "  const [value, setValue] = useState('')",
+      '  useEffect(() => { setValue(title) }, [title])',
+      '  return value',
+      '}'
+    ].join('\n')
+    withFixture(source, ({ filename }) => {
+      const normal = scanFixture('code quality', filename)
+      const unused = normal.find((diagnostic) => diagnostic.message.startsWith('Unused '))
+      expect(unused).toBeDefined()
+      expect(isUnloadedPluginDirectiveUnusedWarning(unused, root, 'code quality')).toBe(true)
+      expect(scanFixture('React Doctor', filename)).toEqual([])
+    })
+  })
+
+  it('keeps an unused Doctor directive failing in its loaded scan', () => {
+    withFixture(directive('react-doctor/no-derived-state-effect'), ({ filename }) => {
+      const diagnostics = scanFixture('React Doctor', filename)
+      expect(diagnostics).toHaveLength(1)
+      expect(diagnostics[0].message).toMatch(/^Unused /)
+      expect(isUnloadedPluginDirectiveUnusedWarning(diagnostics[0], root, 'React Doctor')).toBe(
+        false
+      )
+    })
+  })
+
+  it('does not hide unused native rules in a mixed directive', () => {
+    withFixture(
+      directive('react-doctor/no-derived-state-effect, unicorn/no-array-reduce'),
+      (diagnostic) => {
+        expect(isUnloadedPluginDirectiveUnusedWarning(diagnostic, root, 'code quality')).toBe(false)
+      }
+    )
+  })
+
+  it('recognizes a standalone directive containing only Doctor rules', () => {
+    withFixture(
+      directive(
+        'react-doctor/no-derived-state-effect, react-doctor/no-adjust-state-on-prop-change'
+      ),
+      (diagnostic) => {
+        expect(isUnloadedPluginDirectiveUnusedWarning(diagnostic, root, 'code quality')).toBe(true)
+      }
+    )
+  })
+
+  it('keeps adjacent native directive warnings visible', () => {
+    const doctor = directive('react-doctor/no-derived-state-effect')
+    const native = directive('unicorn/no-array-reduce')
+    for (const source of [`${doctor} ${native}`, `${native} ${doctor}`]) {
+      withFixture(source, ({ filename }) => {
+        const diagnostic = scanFixture('code quality', filename).find((candidate) =>
+          candidate.labels.some((label) => label.span.offset === source.indexOf(native))
+        )
+        expect(diagnostic).toBeDefined()
+        expect(diagnostic.message).toMatch(/^Unused /)
+        expect(isUnloadedPluginDirectiveUnusedWarning(diagnostic, root, 'code quality')).toBe(false)
+      })
+    }
+  })
+
+  it('leaves used native directives to the scan that loads them', () => {
+    withFixture(
+      [
+        'export const banner = "λ"',
+        directive('typescript/no-explicit-any'),
+        'export const answer: any = 42'
+      ].join('\n'),
+      ({ filename }) => {
+        expect(scanFixture('code quality', filename)).toEqual([])
+        const diagnostics = scanFixture('React Doctor', filename)
+        expect(diagnostics).toHaveLength(1)
+        expect(isUnloadedPluginDirectiveUnusedWarning(diagnostics[0], root, 'React Doctor')).toBe(
+          true
+        )
+      }
+    )
+  })
+
+  it('does not exempt unused Doctor rules together with unloaded native rules', () => {
+    withFixture(
+      directive('react-doctor/no-derived-state-effect, typescript/no-explicit-any'),
+      ({ filename }) => {
+        const diagnostics = scanFixture('React Doctor', filename)
+        expect(diagnostics).toHaveLength(1)
+        expect(isUnloadedPluginDirectiveUnusedWarning(diagnostics[0], root, 'React Doctor')).toBe(
+          false
+        )
+      }
+    )
+  })
+
+  it('keeps blanket unused directives visible in the Doctor scan', () => {
+    for (const source of [directive(''), '// oxlint-disable-next-line -- reason']) {
+      withFixture(source, ({ filename }) => {
+        const diagnostics = scanFixture('React Doctor', filename)
+        expect(diagnostics).toHaveLength(1)
+        expect(isUnloadedPluginDirectiveUnusedWarning(diagnostics[0], root, 'React Doctor')).toBe(
+          false
+        )
+      })
+    }
+  })
 })

src/renderer/src/components/JiraIssueWorkspace.tsx

@@ -95,11 +95,14 @@ export default function JiraIssueWorkspace({
     [providerSettings]
   )
 
+  // oxlint-disable-next-line react-doctor/no-derived-state-effect -- Why: seeds editable issue drafts while IPC hydration runs and invalidates obsolete requests.
   useEffect(() => {
+    requestIdRef.current += 1
     if (!issue) {
       setFullIssue(null)
       setIssueLoading(false)
       setComments([])
+      setCommentsLoading(false)
       setCommentsError(null)
       setTransitions([])
       setPriorities([])
@@ -109,7 +112,6 @@ export default function JiraIssueWorkspace({
       return
     }
 
-    requestIdRef.current += 1
     const requestId = requestIdRef.current
     optimisticCommentsRef.current = []
     setFullIssue(issue)

src/renderer/src/components/jira-issue-workspace-request-lifetime.test.tsx

@@ -0,0 +1,296 @@
+// @vitest-environment happy-dom
+
+import { act, type ComponentProps, type ReactNode } from 'react'
+import { createRoot, type Root } from 'react-dom/client'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import type { JiraComment, JiraIssue } from '../../../shared/jira-types'
+import type { TaskSourceContext } from '../../../shared/task-source-context'
+import type { AppState } from '../store/types'
+import type {
+  JiraIssueCommentComposer,
+  JiraIssueWorkspaceContent
+} from './jira-issue-workspace-content'
+import JiraIssueWorkspace from './JiraIssueWorkspace'
+
+type PendingRead<T> = {
+  key: string
+  resolve: (value: T) => void
+  reject: (error: Error) => void
+}
+type ComposerProps = ComponentProps<typeof JiraIssueCommentComposer>
+type JiraStoreBoundary = Pick<AppState, 'patchJiraIssue'> & {
+  settings: Pick<NonNullable<AppState['settings']>, 'activeRuntimeEnvironmentId'>
+}
+
+const boundary = vi.hoisted(
+  (): {
+    issues: PendingRead<JiraIssue | null>[]
+    comments: PendingRead<JiraComment[]>[]
+    composer: ComposerProps | null
+  } => ({ issues: [], comments: [], composer: null })
+)
+
+vi.mock('@/store', () => {
+  const state: JiraStoreBoundary = {
+    settings: { activeRuntimeEnvironmentId: 'environment-1' },
+    patchJiraIssue: () => {}
+  }
+  return {
+    useAppStore: (select: (state: JiraStoreBoundary) => unknown) => select(state)
+  }
+})
+vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback }))
+vi.mock('@/runtime/runtime-jira-client', () => {
+  function request<T>(queue: PendingRead<T>[], key: string): Promise<T> {
+    return new Promise((resolve, reject) => queue.push({ key, resolve, reject }))
+  }
+  return {
+    jiraGetIssue: (_settings: unknown, key: string) => request(boundary.issues, key),
+    jiraIssueComments: (_settings: unknown, key: string) => request(boundary.comments, key),
+    jiraListTransitions: async () => [],
+    jiraListPriorities: async () => [],
+    jiraListAssignableUsers: async () => [],
+    jiraUpdateIssue: async () => ({ ok: true }),
+    jiraAddIssueComment: async () => ({ ok: true, id: 'added' })
+  }
+})
+vi.mock('@/components/ui/sheet', () => {
+  const Content = ({ children }: { children: ReactNode }) => <div>{children}</div>
+  return {
+    // Keep the owner mounted, including its closed content, so stale state is observable.
+    Sheet: ({ open, children }: { open: boolean; children: ReactNode }) => (
+      <div data-open={open}>{children}</div>
+    ),
+    SheetContent: Content,
+    SheetTitle: Content,
+    SheetDescription: Content
+  }
+})
+vi.mock('./jira-issue-workspace-chrome', () => ({
+  JiraIssueWorkspaceHeader: ({ issueLoading }: { issueLoading: boolean }) => (
+    <div data-testid="issue-loading">{String(issueLoading)}</div>
+  ),
+  JiraIssueMetadataBar: () => null
+}))
+vi.mock('./jira-issue-workspace-content', () => ({
+  JiraIssueWorkspaceContent: (props: ComponentProps<typeof JiraIssueWorkspaceContent>) => (
+    <div data-testid="detail">
+      <span data-testid="title">{props.displayed.title}</span>
+      <span data-testid="description">{props.displayed.description}</span>
+      <span data-testid="comments">{props.comments.map((comment) => comment.id).join(',')}</span>
+      <span data-testid="comments-loading">{String(props.commentsLoading)}</span>
+      <span data-testid="comments-error">{props.commentsError}</span>
+      <button type="button" onClick={props.retryComments}>
+        Retry
+      </button>
+    </div>
+  ),
+  JiraIssueCommentComposer: (props: ComposerProps) => (
+    <div
+      ref={(node) => {
+        boundary.composer = node ? props : null
+      }}
+    />
+  )
+}))
+
+Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true })
+const sourceContext: TaskSourceContext = {
+  kind: 'task-source',
+  provider: 'jira',
+  projectId: 'project-1',
+  hostId: 'runtime:environment-1'
+}
+let root: Root | null = null
+let container: HTMLDivElement
+
+function issue(key = 'JIR-1', title = key): JiraIssue {
+  return {
+    id: key,
+    key,
+    title,
+    siteId: 'site-1',
+    description: '',
+    url: `https://jira.invalid/browse/${key}`,
+    project: { id: 'project', key: 'PRJ', name: 'Project' },
+    issueType: { id: 'task', name: 'Task' },
+    status: { id: 'open', name: 'Open', categoryKey: 'new', categoryName: 'New' },
+    labels: [],
+    createdAt: '2026-10-02',
+    updatedAt: '2026-10-02'
+  }
+}
+
+function comment(id: string): JiraComment {
+  return { id, body: id, createdAt: '2026-10-02' }
+}
+
+async function render(selected: JiraIssue | null): Promise<void> {
+  if (!root) {
+    throw new Error('Missing React root')
+  }
+  const mountedRoot = root
+  await act(async () => {
+    mountedRoot.render(
+      <JiraIssueWorkspace
+        issue={selected}
+        sourceContext={sourceContext}
+        onUse={() => {}}
+        onClose={() => {}}
+      />
+    )
+  })
+}
+
+function take<T>(queue: PendingRead<T>[]): PendingRead<T> {
+  const pending = queue.shift()
+  if (!pending) {
+    throw new Error('Missing pending Jira read')
+  }
+  return pending
+}
+
+function text(id: string): string | null | undefined {
+  return container.querySelector(`[data-testid="${id}"]`)?.textContent
+}
+
+async function retryComments(): Promise<void> {
+  const button = container.querySelector('button')
+  if (!(button instanceof HTMLButtonElement)) {
+    throw new Error('Missing Retry button')
+  }
+  await act(async () => button.click())
+}
+
+beforeEach(() => {
+  boundary.issues.length = 0
+  boundary.comments.length = 0
+  container = document.createElement('div')
+  document.body.appendChild(container)
+  root = createRoot(container)
+})
+
+afterEach(async () => {
+  const mountedRoot = root
+  if (mountedRoot) {
+    await act(async () => mountedRoot.unmount())
+  }
+  root = null
+  await act(async () => {
+    for (const pending of [...boundary.issues, ...boundary.comments]) {
+      pending.reject(new Error('Test cleanup'))
+    }
+  })
+  boundary.issues.length = 0
+  boundary.comments.length = 0
+  boundary.composer = null
+  document.body.replaceChildren()
+})
+
+describe('Jira issue workspace request lifetime', () => {
+  it('discards late detail and comments after closing the mounted workspace', async () => {
+    await render(issue())
+    await render(null)
+    await act(async () => {
+      take(boundary.issues).resolve({ ...issue(), description: 'Screenshot data'.repeat(32_768) })
+      take(boundary.comments).resolve([{ ...comment('late'), body: 'Image data'.repeat(32_768) }])
+    })
+    expect(container.querySelector('[data-open]')?.getAttribute('data-open')).toBe('false')
+    expect(container.querySelector('[data-testid="detail"]')).toBeNull()
+    expect(boundary.composer).toBeNull()
+  })
+
+  it('clears an already hydrated issue and comments when it closes', async () => {
+    await render(issue())
+    await act(async () => {
+      take(boundary.issues).resolve(issue('JIR-1', 'Hydrated'))
+      take(boundary.comments).resolve([comment('loaded')])
+    })
+    expect(text('title')).toBe('Hydrated')
+    expect(text('comments')).toBe('loaded')
+    expect(text('issue-loading')).toBe('false')
+    expect(text('comments-loading')).toBe('false')
+    await render(null)
+    expect(container.querySelector('[data-testid="detail"]')).toBeNull()
+  })
+
+  it('ignores old replies and accepts the reopened issue replies', async () => {
+    await render(issue())
+    const oldIssue = take(boundary.issues)
+    const oldComments = take(boundary.comments)
+    await render(null)
+    await render(issue('JIR-2'))
+    await act(async () => {
+      oldIssue.resolve(issue('JIR-1', 'Late old issue'))
+      oldComments.resolve([comment('old')])
+    })
+    expect(text('title')).toBe('JIR-2')
+    expect(text('issue-loading')).toBe('true')
+    expect(text('comments-loading')).toBe('true')
+    await act(async () => {
+      take(boundary.issues).resolve(issue('JIR-2', 'New detail'))
+      take(boundary.comments).resolve([comment('new')])
+    })
+    expect(text('title')).toBe('New detail')
+    expect(text('comments')).toBe('new')
+    expect(text('comments-loading')).toBe('false')
+  })
+
+  it('discards a closed comments error together with late issue hydration', async () => {
+    await render(issue())
+    await render(null)
+    await act(async () => {
+      take(boundary.comments).reject(new Error('Late failure'))
+      take(boundary.issues).resolve(issue('JIR-1', 'Late detail'))
+    })
+    expect(container.querySelector('[data-testid="detail"]')).toBeNull()
+    await render(issue('JIR-2'))
+    expect(text('comments-error')).toBe('')
+    expect(text('comments-loading')).toBe('true')
+  })
+
+  it('keeps open comments errors retryable and clears loading on success', async () => {
+    await render(issue())
+    await act(async () => {
+      take(boundary.issues).resolve(issue())
+      take(boundary.comments).reject(new Error('Try again'))
+    })
+    expect(text('comments-error')).toBe('Try again')
+    expect(text('comments-loading')).toBe('false')
+    await retryComments()
+    expect(text('comments-loading')).toBe('true')
+    expect(text('comments-error')).toBe('')
+    await act(async () => take(boundary.comments).resolve([comment('retry')]))
+    expect(text('comments')).toBe('retry')
+    expect(text('comments-loading')).toBe('false')
+  })
+
+  it('preserves submitted comments across refresh and deduplicates returned IDs', async () => {
+    await render(issue())
+    await act(async () => {
+      take(boundary.issues).resolve(issue())
+      take(boundary.comments).resolve([comment('server')])
+    })
+    await act(async () => {
+      if (!boundary.composer) {
+        throw new Error('Missing comment composer')
+      }
+      boundary.composer.setCommentDraft('Added comment')
+    })
+    await act(async () => {
+      if (!boundary.composer) {
+        throw new Error('Missing comment composer')
+      }
+      boundary.composer.handleSubmitComment()
+    })
+    expect(text('comments')).toBe('server,added')
+    await retryComments()
+    await act(async () => take(boundary.comments).resolve([comment('server')]))
+    expect(text('comments')).toBe('server,added')
+    await retryComments()
+    await act(async () => take(boundary.comments).resolve([comment('server'), comment('added')]))
+    expect(text('comments')).toBe('server,added')
+    expect(boundary.composer?.commentDraft).toBe('')
+    expect(boundary.composer?.commentSubmitting).toBe(false)
+  })
+})