From deca6a2f3dff4b50657a07f89a146ed30264fb96 Mon Sep 17 00:00:00 2001 From: nrs <38722970+nrslib@users.noreply.github.com> Date: Thu, 26 Feb 2026 13:33:02 +0900 Subject: [PATCH] [#368] fix-broken-issue-title-session (#371) * takt: fix-broken-issue-title-session * takt: fix-broken-issue-title-session --- src/__tests__/conversationLoop-resume.test.ts | 35 ++++ src/__tests__/createIssueFromTask.test.ts | 147 ++++++++++++++++ src/__tests__/quietMode-session.test.ts | 160 ++++++++++++++++++ src/features/interactive/conversationLoop.ts | 6 +- src/features/interactive/quietMode.ts | 3 +- src/features/tasks/add/index.ts | 31 +++- 6 files changed, 375 insertions(+), 7 deletions(-) create mode 100644 src/__tests__/quietMode-session.test.ts diff --git a/src/__tests__/conversationLoop-resume.test.ts b/src/__tests__/conversationLoop-resume.test.ts index 5d15111..54b2b9b 100644 --- a/src/__tests__/conversationLoop-resume.test.ts +++ b/src/__tests__/conversationLoop-resume.test.ts @@ -226,3 +226,38 @@ describe('/resume command', () => { expect(result.action).toBe('cancel'); }); }); + +// ================================================================= +// /go command: summary AI session isolation +// ================================================================= +describe('/go command', () => { + it('should pass sessionId as undefined to summary AI even when conversation has an active session', async () => { + // Given: send message (AI responds with sessionId) → /go triggers summary + setupRawStdin(toRawInputs(['hello', '/go'])); + + const { provider, capture } = createScenarioProvider([ + // Call 0: user message → AI responds and sets sessionId + { content: 'AI response', sessionId: 'session-abc' }, + // Call 1: /go summary → should NOT inherit sessionId + { content: '## Fix broken title\nDetails here' }, + ]); + + const ctx: SessionContext = { + provider: provider as SessionContext['provider'], + providerType: 'mock' as SessionContext['providerType'], + model: undefined, + lang: 'en', + personaName: 'interactive', + sessionId: undefined, + }; + + // When + const result = await runConversationLoop('/test', ctx, defaultStrategy, undefined, undefined); + + // Then: first AI call had no session (initial state) + expect(capture.sessionIds[0]).toBeUndefined(); + // Then: summary call must NOT inherit the conversation session + expect(capture.sessionIds[1]).toBeUndefined(); + expect(result.action).toBe('execute'); + }); +}); diff --git a/src/__tests__/createIssueFromTask.test.ts b/src/__tests__/createIssueFromTask.test.ts index 7f022be..e079092 100644 --- a/src/__tests__/createIssueFromTask.test.ts +++ b/src/__tests__/createIssueFromTask.test.ts @@ -34,6 +34,7 @@ vi.mock('../shared/utils/index.js', async (importOriginal) => ({ import { success, error } from '../shared/ui/index.js'; import { createIssueFromTask } from '../features/tasks/index.js'; +import { extractTitle } from '../features/tasks/add/index.js'; const mockSuccess = vi.mocked(success); const mockError = vi.mocked(error); @@ -229,3 +230,149 @@ describe('createIssueFromTask', () => { }); }); }); + +describe('extractTitle', () => { + describe('Markdown heading extraction', () => { + it('should strip # and return text for h1 heading', () => { + // Given + const task = '# Fix broken title\nDetails here'; + + // When + const result = extractTitle(task); + + // Then + expect(result).toBe('Fix broken title'); + }); + + it('should strip ## and return text for h2 heading', () => { + // Given + const task = '## Fix broken title\nDetails here'; + + // When + const result = extractTitle(task); + + // Then + expect(result).toBe('Fix broken title'); + }); + + it('should strip ### and return text for h3 heading', () => { + // Given + const task = '### Fix broken title\nDetails here'; + + // When + const result = extractTitle(task); + + // Then + expect(result).toBe('Fix broken title'); + }); + + it('should prefer first Markdown heading over preceding plain text', () => { + // Given: AI preamble followed by heading + const task = '失礼しました。修正します。\n## Fix broken title\nDetails here'; + + // When + const result = extractTitle(task); + + // Then: heading wins over first line + expect(result).toBe('Fix broken title'); + }); + + it('should find heading even when multiple empty lines precede it', () => { + // Given + const task = '\n\n## Fix broken title\nDetails here'; + + // When + const result = extractTitle(task); + + // Then + expect(result).toBe('Fix broken title'); + }); + }); + + describe('fallback to first non-empty line', () => { + it('should return first non-empty line when no Markdown heading exists', () => { + // Given: plain text without heading + const task = 'Fix broken title\nSecond line details'; + + // When + const result = extractTitle(task); + + // Then + expect(result).toBe('Fix broken title'); + }); + + it('should skip leading empty lines when no heading exists', () => { + // Given: leading blank lines + const task = '\n\nFix broken title\nDetails here'; + + // When + const result = extractTitle(task); + + // Then + expect(result).toBe('Fix broken title'); + }); + + it('should not treat h4+ headings as Markdown headings', () => { + // Given: #### is not matched (only h1-h3 are recognized) + const task = '#### h4 heading\nDetails here'; + + // When + const result = extractTitle(task); + + // Then: falls back to first non-empty line as-is + expect(result).toBe('#### h4 heading'); + }); + + it('should not treat heading without space after hash as Markdown heading', () => { + // Given: #Title has no space, so not recognized as heading + const task = '#NoSpace\nDetails here'; + + // When + const result = extractTitle(task); + + // Then: falls back to first non-empty line + expect(result).toBe('#NoSpace'); + }); + }); + + describe('title truncation', () => { + it('should truncate heading title to 97 chars + ellipsis when over 100 chars', () => { + // Given: heading text over 100 characters + const longTitle = 'a'.repeat(102); + const task = `## ${longTitle}\nDetails here`; + + // When + const result = extractTitle(task); + + // Then: truncated to 97 + "..." + expect(result).toBe(`${'a'.repeat(97)}...`); + expect(result).toHaveLength(100); + }); + + it('should truncate plain text title to 97 chars + ellipsis when over 100 chars', () => { + // Given: plain text over 100 characters + const longTitle = 'b'.repeat(102); + const task = `${longTitle}\nDetails here`; + + // When + const result = extractTitle(task); + + // Then: truncated to 97 + "..." + expect(result).toBe(`${'b'.repeat(97)}...`); + expect(result).toHaveLength(100); + }); + + it('should not truncate title of exactly 100 characters', () => { + // Given: title exactly 100 chars + const title100 = 'c'.repeat(100); + const task = `## ${title100}\nDetails here`; + + // When + const result = extractTitle(task); + + // Then: not truncated + expect(result).toBe(title100); + expect(result).toHaveLength(100); + }); + }); +}); diff --git a/src/__tests__/quietMode-session.test.ts b/src/__tests__/quietMode-session.test.ts new file mode 100644 index 0000000..1301da2 --- /dev/null +++ b/src/__tests__/quietMode-session.test.ts @@ -0,0 +1,160 @@ +/** + * Tests for quietMode summary AI session isolation. + * + * Verifies that the summary AI call in quietMode does not inherit the + * conversation session (sessionId must be undefined), even when ctx + * carries an active sessionId. This matches the fix already applied to + * conversationLoop.ts's /go command. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// ── Mocks ───────────────────────────────────────────────────────────── + +vi.mock('../features/interactive/conversationLoop.js', () => ({ + initializeSession: vi.fn(), + callAIWithRetry: vi.fn(), +})); + +vi.mock('../features/interactive/interactive.js', () => ({ + DEFAULT_INTERACTIVE_TOOLS: ['Read'], + buildSummaryPrompt: vi.fn(), + selectPostSummaryAction: vi.fn(), +})); + +vi.mock('../shared/utils/index.js', async (importOriginal) => ({ + ...(await importOriginal>()), + createLogger: () => ({ info: vi.fn(), debug: vi.fn(), error: vi.fn() }), +})); + +vi.mock('../shared/ui/index.js', () => ({ + info: vi.fn(), + error: vi.fn(), + blankLine: vi.fn(), +})); + +vi.mock('../shared/i18n/index.js', () => ({ + getLabel: vi.fn((_key: string, _lang: string) => 'Mock label'), + getLabelObject: vi.fn(() => ({ + intro: 'Intro', + proposed: 'Proposed:', + cancelled: 'Cancelled', + })), +})); + +// ── Imports (after mocks) ────────────────────────────────────────────── + +import { quietMode } from '../features/interactive/quietMode.js'; +import { initializeSession, callAIWithRetry } from '../features/interactive/conversationLoop.js'; +import { buildSummaryPrompt, selectPostSummaryAction } from '../features/interactive/interactive.js'; +import type { SessionContext } from '../features/interactive/conversationLoop.js'; + +const mockInitializeSession = vi.mocked(initializeSession); +const mockCallAIWithRetry = vi.mocked(callAIWithRetry); +const mockBuildSummaryPrompt = vi.mocked(buildSummaryPrompt); +const mockSelectPostSummaryAction = vi.mocked(selectPostSummaryAction); + +// ── Helpers ─────────────────────────────────────────────────────────── + +function createMockSessionContext(sessionId: string | undefined): SessionContext { + return { + provider: {} as SessionContext['provider'], + providerType: 'mock' as SessionContext['providerType'], + model: undefined, + lang: 'en', + personaName: 'interactive', + sessionId, + }; +} + +// ── Setup ───────────────────────────────────────────────────────────── + +beforeEach(() => { + vi.clearAllMocks(); +}); + +// ================================================================= +// quietMode: summary AI session isolation +// ================================================================= + +describe('quietMode: summary AI session isolation', () => { + it('should pass sessionId as undefined to callAIWithRetry even when ctx carries an active sessionId', async () => { + // Given: initializeSession returns a ctx with an active session + const ctxWithSession = createMockSessionContext('active-session-123'); + mockInitializeSession.mockReturnValue(ctxWithSession); + + // Given: buildSummaryPrompt returns a non-null prompt + mockBuildSummaryPrompt.mockReturnValue('Summary prompt for task'); + + // Given: callAIWithRetry returns a successful result + mockCallAIWithRetry.mockResolvedValue({ + result: { content: '## Fix the bug\nDetails here', success: true, sessionId: undefined }, + sessionId: undefined, + }); + + // Given: user selects execute action + mockSelectPostSummaryAction.mockResolvedValue('execute'); + + // When + const result = await quietMode('/test/cwd', 'fix the bug'); + + // Then: callAIWithRetry was called exactly once + expect(mockCallAIWithRetry).toHaveBeenCalledOnce(); + + // Then: 5th argument (ctx) must NOT inherit the conversation sessionId + const calledCtx = mockCallAIWithRetry.mock.calls[0]![4] as SessionContext; + expect(calledCtx.sessionId).toBeUndefined(); + + // Then: result is as expected + expect(result.action).toBe('execute'); + }); + + it('should preserve other ctx fields while clearing sessionId', async () => { + // Given: ctx with active session and specific lang + const ctxWithSession = createMockSessionContext('session-xyz'); + ctxWithSession.lang = 'ja'; + mockInitializeSession.mockReturnValue(ctxWithSession); + mockBuildSummaryPrompt.mockReturnValue('要約プロンプト'); + mockCallAIWithRetry.mockResolvedValue({ + result: { content: '## タスク\n詳細', success: true, sessionId: undefined }, + sessionId: undefined, + }); + mockSelectPostSummaryAction.mockResolvedValue('execute'); + + // When + await quietMode('/test/cwd', 'バグを修正する'); + + // Then: sessionId is cleared but other fields are preserved + const calledCtx = mockCallAIWithRetry.mock.calls[0]![4] as SessionContext; + expect(calledCtx.sessionId).toBeUndefined(); + expect(calledCtx.lang).toBe('ja'); + expect(calledCtx.personaName).toBe('interactive'); + }); + + it('should return cancel when callAIWithRetry returns null result', async () => { + // Given + mockInitializeSession.mockReturnValue(createMockSessionContext('session-abc')); + mockBuildSummaryPrompt.mockReturnValue('Summary prompt'); + mockCallAIWithRetry.mockResolvedValue({ result: null, sessionId: undefined }); + + // When + const result = await quietMode('/test/cwd', 'some input'); + + // Then + expect(result.action).toBe('cancel'); + expect(result.task).toBe(''); + }); + + it('should return cancel when buildSummaryPrompt returns null', async () => { + // Given: no conversation history leads to null prompt + mockInitializeSession.mockReturnValue(createMockSessionContext(undefined)); + mockBuildSummaryPrompt.mockReturnValue(null); + + // When + const result = await quietMode('/test/cwd', 'some input'); + + // Then: short-circuits before callAIWithRetry + expect(mockCallAIWithRetry).not.toHaveBeenCalled(); + expect(result.action).toBe('cancel'); + }); +}); diff --git a/src/features/interactive/conversationLoop.ts b/src/features/interactive/conversationLoop.ts index 488bc4d..2a1fa2f 100644 --- a/src/features/interactive/conversationLoop.ts +++ b/src/features/interactive/conversationLoop.ts @@ -192,7 +192,11 @@ export async function runConversationLoop( if (userNote) { summaryPrompt = `${summaryPrompt}\n\nUser Note:\n${userNote}`; } - const summaryResult = await doCallAI(summaryPrompt, summaryPrompt, strategy.allowedTools); + // Summary AI must not inherit the conversation session to avoid chat-mode behavior. + const { result: summaryResult } = await callAIWithRetry( + summaryPrompt, summaryPrompt, strategy.allowedTools, cwd, + { ...ctx, sessionId: undefined }, + ); if (!summaryResult) { info(ui.summarizeFailed); continue; diff --git a/src/features/interactive/quietMode.ts b/src/features/interactive/quietMode.ts index 13a8093..1e6f3a6 100644 --- a/src/features/interactive/quietMode.ts +++ b/src/features/interactive/quietMode.ts @@ -85,7 +85,8 @@ export async function quietMode( } const { result } = await callAIWithRetry( - summaryPrompt, summaryPrompt, DEFAULT_INTERACTIVE_TOOLS, cwd, ctx, + summaryPrompt, summaryPrompt, DEFAULT_INTERACTIVE_TOOLS, cwd, + { ...ctx, sessionId: undefined }, ); if (!result) { diff --git a/src/features/tasks/add/index.ts b/src/features/tasks/add/index.ts index 8ca639d..59baa4f 100644 --- a/src/features/tasks/add/index.ts +++ b/src/features/tasks/add/index.ts @@ -70,16 +70,38 @@ export async function saveTaskFile( return { taskName: created.name, tasksFile }; } +const TITLE_MAX_LENGTH = 100; +const TITLE_TRUNCATE_LENGTH = 97; +const MARKDOWN_HEADING_PATTERN = /^#{1,3}\s+\S/; + +/** + * Extract a clean title from a task description. + * + * Prefers the first Markdown heading (h1-h3) if present. + * Falls back to the first non-empty line otherwise. + * Truncates to 100 characters (97 + "...") when exceeded. + */ +export function extractTitle(task: string): string { + const lines = task.split('\n'); + const headingLine = lines.find((l) => MARKDOWN_HEADING_PATTERN.test(l)); + const titleLine = headingLine + ? headingLine.replace(/^#{1,3}\s+/, '') + : (lines.find((l) => l.trim().length > 0) ?? task); + return titleLine.length > TITLE_MAX_LENGTH + ? `${titleLine.slice(0, TITLE_TRUNCATE_LENGTH)}...` + : titleLine; +} + /** * Create a GitHub Issue from a task description. * - * Extracts the first line as the issue title (truncated to 100 chars), - * uses the full task as the body, and displays success/error messages. + * Extracts the first Markdown heading (h1-h3) as the issue title, + * falling back to the first non-empty line. Truncates to 100 chars. + * Uses the full task as the body, and displays success/error messages. */ export function createIssueFromTask(task: string, options?: { labels?: string[] }): number | undefined { info('Creating GitHub Issue...'); - const titleLine = task.split('\n')[0] || task; - const title = titleLine.length > 100 ? `${titleLine.slice(0, 97)}...` : titleLine; + const title = extractTitle(task); const effectiveLabels = options?.labels?.filter((l) => l.length > 0) ?? []; const labels = effectiveLabels.length > 0 ? effectiveLabels : undefined; @@ -259,7 +281,6 @@ export async function addTask(cwd: string, task?: string): Promise { return; } - // 3. ワークツリー/ブランチ/PR設定 const settings = await promptWorktreeSettings(); // YAMLファイル作成