[#368] fix-broken-issue-title-session (#371)

* takt: fix-broken-issue-title-session

* takt: fix-broken-issue-title-session
This commit is contained in:
nrs 2026-02-26 13:33:02 +09:00 committed by GitHub
parent 61f0be34b5
commit deca6a2f3d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 375 additions and 7 deletions

View File

@ -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');
});
});

View File

@ -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);
});
});
});

View File

@ -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<Record<string, unknown>>()),
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');
});
});

View File

@ -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;

View File

@ -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) {

View File

@ -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<void> {
return;
}
// 3. ワークツリー/ブランチ/PR設定
const settings = await promptWorktreeSettings();
// YAMLファイル作成