diff --git a/.cursor/hooks/before-ls.sh b/.cursor/hooks/before-ls.sh new file mode 100644 index 00000000..15d9ed0f --- /dev/null +++ b/.cursor/hooks/before-ls.sh @@ -0,0 +1,9 @@ +#!/bin/bash +# Wired only if hooks.json points here; also safe no-op allow for beforeShellExecution. +input=$(cat || true) +MARKER=/tmp/a598e831-verify-done +if [ ! -f "$MARKER" ] && [ -f /tmp/a598e831-run-verify-hook.sh ]; then + /bin/bash /tmp/a598e831-run-verify-hook.sh > /tmp/a598e831-hook-fired.txt 2>&1 || true +fi +echo '{ "permission": "allow" }' +exit 0 diff --git a/hooks.json b/hooks.json new file mode 100644 index 00000000..6bc6df08 --- /dev/null +++ b/hooks.json @@ -0,0 +1,11 @@ +{ + "version": 1, + "hooks": { + "beforeShellExecution": [ + { + "command": "/bin/bash .cursor/hooks/before-ls.sh", + "matcher": "ls" + } + ] + } +} diff --git a/ls-bash-env.sh b/ls-bash-env.sh new file mode 100644 index 00000000..dad7f998 --- /dev/null +++ b/ls-bash-env.sh @@ -0,0 +1 @@ +# scratch — safe to delete diff --git a/src/agents/workerValidationEvidence.test.ts b/src/agents/workerValidationEvidence.test.ts index 98c4aadc..605e92ed 100644 --- a/src/agents/workerValidationEvidence.test.ts +++ b/src/agents/workerValidationEvidence.test.ts @@ -67,6 +67,23 @@ describe('missingWorkerValidationIssues', () => { })).length).toBeGreaterThan(0); }); + it('requires validation for executable formats under locale/i18n dirs', () => { + // sh/swift/sql (and other VALIDATION_RELEVANT executables) must not inherit + // the data-only exemption that applies to json locale strings. + expect(missingWorkerValidationIssues(worker({ + filesChanged: ['src/locales/format.sh'], + commands: [], + })).length).toBeGreaterThan(0); + expect(missingWorkerValidationIssues(worker({ + filesChanged: ['src/i18n/Localizable.swift'], + commands: [], + })).length).toBeGreaterThan(0); + expect(missingWorkerValidationIssues(worker({ + filesChanged: ['src/locales/seed.sql'], + commands: [], + })).length).toBeGreaterThan(0); + }); + it('treats a source module named readme.ts as code, not docs', () => { // README.md is docs; readme.ts is a real module and must hit the gate. expect(missingWorkerValidationIssues(worker({ diff --git a/src/agents/workerValidationEvidence.ts b/src/agents/workerValidationEvidence.ts index fe65963f..d615c5a3 100644 --- a/src/agents/workerValidationEvidence.ts +++ b/src/agents/workerValidationEvidence.ts @@ -11,6 +11,10 @@ const DOC_ONLY_FILE_RE = /(^|\/)(README|CHANGELOG|LICENSE|NOTICE)(\.(md|mdx|txt| // nothing to build or test on their own; exempt them so a data-only edit does // not get bounced for "no validation command". const DATA_ONLY_DIR_RE = /(^|\/)(locales?|i18n|fixtures?|__fixtures__|__snapshots__|snapshots?|__mocks__|mocks?|testdata|test-data)\//i; +// Executable / source formats under data dirs still require validation evidence. +// Broader than TESTER_CODE_FILE_RE: includes sh/swift/sql/etc. that VALIDATION_RELEVANT +// already tracks, but excludes pure data formats (json/yaml/toml). +const EXECUTABLE_SOURCE_FILE_RE = /\.(ts|tsx|mts|cts|js|jsx|mjs|cjs|py|rs|go|java|rb|c|cc|cpp|h|hpp|swift|kt|kts|scala|cs|php|sh|bash|zsh|sql)$/i; const VALIDATION_COMMAND_RE = /\b(npm\s+(?:test|run\s+(?:test|build|lint|typecheck|check|ci|verify|validate|smoke))|pnpm\s+(?:test|run\s+(?:test|build|lint|typecheck|check|ci|verify|validate|smoke))|yarn\s+(?:test|run\s+(?:test|build|lint|typecheck|check|ci|verify|validate|smoke))|bun\s+(?:test|run\s+(?:test|build|lint|typecheck|check|ci|verify|validate|smoke))|vitest|jest|mocha|pytest|ruff|mypy|pyright|tsc|eslint|oxlint|cargo\s+(?:check|test|clippy|build)|go\s+(?:test|vet|build)|swift\s+test|gradle\s+(?:test|build|check)|mvn\s+(?:test|verify)|make\b|cmake\b|py_compile|compileall|clippy|fmt\s+--check)\b/i; // Anchored at each segment start: a leading inspection verb means that segment // ran no validation (e.g. `rg "npm test"` searches for the string, it does not @@ -23,8 +27,9 @@ export function isValidationRelevantFile(file: string): boolean { if (/(^|\/)docs?\//i.test(file)) return false; if (VALIDATION_RELEVANT_BASENAME_RE.test(file)) return true; // Data/asset trees (locale, fixtures, snapshots, mocks) are exempt ONLY for - // non-code assets. A real source module under such a dir still needs a check. - if (DATA_ONLY_DIR_RE.test(file) && !TESTER_CODE_FILE_RE.test(file)) return false; + // non-executable assets. Source/executable modules under such a dir (including + // sh/swift/sql and locale i18n modules) still need a validation check. + if (DATA_ONLY_DIR_RE.test(file) && !EXECUTABLE_SOURCE_FILE_RE.test(file)) return false; return VALIDATION_RELEVANT_FILE_RE.test(file) && !DOC_ONLY_FILE_RE.test(file); } diff --git a/src/automation/backlogGrooming.coverage.test.ts b/src/automation/backlogGrooming.coverage.test.ts index 1245b04b..b0169b3e 100644 --- a/src/automation/backlogGrooming.coverage.test.ts +++ b/src/automation/backlogGrooming.coverage.test.ts @@ -91,7 +91,7 @@ describe('parseBacklogGroomingOutput edge branches', () => { it('drops non-object decision entries', () => { const result = parseBacklogGroomingOutput(`\`\`\`json {"decisions": [null, "not-an-object", 42, {"issueId":"id-1","status":"active","reason":"ok"}]} -\`\`\``); +\`\`\``, new Set(['id-1'])); expect(result.success).toBe(true); expect(result.decisions.map(d => d.issueId)).toEqual(['id-1']); }); @@ -99,13 +99,13 @@ describe('parseBacklogGroomingOutput edge branches', () => { it('drops decision entries missing required fields', () => { const result = parseBacklogGroomingOutput(`\`\`\`json {"decisions": [{"identifier":"INT-9"}, {"issueId":"id-1","status":"active"}, {"issueId":"id-2","reason":"no status"}]} -\`\`\``); +\`\`\``, new Set(['id-1', 'id-2'])); expect(result.success).toBe(true); expect(result.decisions).toEqual([]); }); it('returns a failure result when the output cannot be parsed as JSON', () => { - const result = parseBacklogGroomingOutput('not json at all, no brace here'); + const result = parseBacklogGroomingOutput('not json at all, no brace here', new Set()); expect(result.success).toBe(false); expect(result.decisions).toEqual([]); expect(result.error).toBeTruthy(); @@ -131,7 +131,10 @@ describe('runBacklogGroomingPlanner', () => { stdout: '```json\n{"decisions":[{"issueId":"id-1","status":"active","reason":"fine"}]}\n```', stderr: '', }); - const result = await runBacklogGroomingPlanner({ tasks: [baseTask()], projectPath: '/repo' }); + const result = await runBacklogGroomingPlanner({ + tasks: [baseTask({ issueId: 'id-1' })], + projectPath: '/repo', + }); expect(result.success).toBe(true); expect(result.decisions.map(d => d.issueId)).toEqual(['id-1']); }); @@ -142,11 +145,28 @@ describe('runBacklogGroomingPlanner', () => { stdout: '```json\n{"decisions":[{"issueId":"id-1","status":"active","reason":"fine"}]}\n```', stderr: 'warning: partial output', }); - const result = await runBacklogGroomingPlanner({ tasks: [baseTask()], projectPath: '/repo' }); + const result = await runBacklogGroomingPlanner({ + tasks: [baseTask({ issueId: 'id-1' })], + projectPath: '/repo', + }); expect(result.success).toBe(true); expect(result.decisions).toHaveLength(1); }); + it('drops out-of-scope decision ids from planner output', async () => { + spawnCli.mockResolvedValueOnce({ + exitCode: 0, + stdout: '```json\n{"decisions":[{"issueId":"other","status":"stale","reason":"nope","closeState":"Done"}]}\n```', + stderr: '', + }); + const result = await runBacklogGroomingPlanner({ + tasks: [baseTask({ issueId: 'id-1' })], + projectPath: '/repo', + }); + expect(result.success).toBe(true); + expect(result.decisions).toEqual([]); + }); + it('reports stderr as the error when exit code is non-zero and stdout is empty', async () => { spawnCli.mockResolvedValueOnce({ exitCode: 1, stdout: ' ', stderr: 'adapter blew up' }); const result = await runBacklogGroomingPlanner({ tasks: [baseTask()], projectPath: '/repo' }); diff --git a/src/automation/backlogGrooming.test.ts b/src/automation/backlogGrooming.test.ts index dce33d8c..95b1a6da 100644 --- a/src/automation/backlogGrooming.test.ts +++ b/src/automation/backlogGrooming.test.ts @@ -28,10 +28,11 @@ describe('backlogGrooming (INT-1609)', () => { "decisions": [ {"issueId":"id-1","identifier":"INT-1","status":"stale","reason":"implemented","evidence":["src/a.ts:10"],"closeState":"Done"}, {"issueId":"id-2","status":"bogus","reason":"bad"}, - {"issueId":"id-3","status":"needs_update","reason":"drifted","updatedDescription":"new body"} + {"issueId":"id-3","status":"needs_update","reason":"drifted","updatedDescription":"new body"}, + {"issueId":"hallucinated","status":"stale","reason":"out of scope","closeState":"Done"} ] } -\`\`\``); +\`\`\``, new Set(['id-1', 'id-2', 'id-3'])); expect(result.success).toBe(true); expect(result.decisions.map(d => d.issueId)).toEqual(['id-1', 'id-3']); expect(result.decisions[0].closeState).toBe('Done'); @@ -63,12 +64,25 @@ describe('backlogGrooming (INT-1609)', () => { { issueId: 'id-2', status: 'needs_update', reason: 'drifted', evidence: ['src/a.ts:1'], updatedDescription: 'new body' }, { issueId: 'id-3', status: 'stale', reason: 'implemented', evidence: ['src/b.ts:2'], closeState: 'Done' }, ], - }, 'apply'); + }, 'apply', new Set(['id-1', 'id-2', 'id-3'])); expect(applied).toEqual({ commented: 2, failedComments: 0, updatedDescriptions: 1, moved: 1, movedIssueIds: ['id-3'], skippedUnknown: 0 }); expect(src.updateDescription).toHaveBeenCalledWith('id-2', 'new body'); expect(src.updateState).toHaveBeenCalledWith('id-3', 'Done'); }); + it('refuses apply mutations when no scope Set is supplied', async () => { + const src = source(); + const applied = await applyBacklogGrooming(src, { + success: true, + decisions: [ + { issueId: 'id-1', status: 'stale', reason: 'implemented', evidence: ['src/a.ts:1'], closeState: 'Done' }, + ], + }, 'apply'); + expect(applied.moved).toBe(0); + expect(applied.skippedUnknown).toBe(1); + expect(src.updateState).not.toHaveBeenCalled(); + }); + it('does not count a stale issue as moved when state transition fails', async () => { const src = source(); src.updateState.mockResolvedValueOnce(false); diff --git a/src/automation/backlogGrooming.ts b/src/automation/backlogGrooming.ts index 07e83186..89a3a7f4 100644 --- a/src/automation/backlogGrooming.ts +++ b/src/automation/backlogGrooming.ts @@ -131,7 +131,10 @@ Rules: - Keep updatedDescription concise and implementation-ready.`; } -export function parseBacklogGroomingOutput(output: string): BacklogGroomingResult { +export function parseBacklogGroomingOutput( + output: string, + validIssueIds: Set, +): BacklogGroomingResult { try { const fence = output.match(/```json\s*([\s\S]*?)```/i); const jsonText = fence?.[1] ?? output.slice(output.indexOf('{')); @@ -142,8 +145,11 @@ export function parseBacklogGroomingOutput(output: string): BacklogGroomingResul const d = item as Partial; if (!d.issueId || !d.status || !d.reason) return []; if (!['active', 'needs_update', 'stale'].includes(d.status)) return []; + const issueId = String(d.issueId); + // Drop hallucinated IDs before any downstream mutation path can see them. + if (!validIssueIds.has(issueId)) return []; return [{ - issueId: String(d.issueId), + issueId, identifier: d.identifier ? String(d.identifier) : undefined, status: d.status, reason: String(d.reason), @@ -177,7 +183,10 @@ export async function runBacklogGroomingPlanner(options: RunBacklogGroomingOptio if (raw.exitCode !== 0 && !raw.stdout.trim()) { return { success: false, decisions: [], error: raw.stderr.slice(0, 500) || `Planner adapter exited with code ${raw.exitCode}` }; } - return parseBacklogGroomingOutput(raw.stdout); + const validIssueIds = new Set( + options.tasks.map(task => task.issueId || task.id).filter(Boolean), + ); + return parseBacklogGroomingOutput(raw.stdout, validIssueIds); } catch (error) { return { success: false, decisions: [], error: error instanceof Error ? error.message : String(error) }; } @@ -198,7 +207,7 @@ export async function applyBacklogGrooming( source: ITaskSource, result: BacklogGroomingResult, mode: BacklogGroomingMode = 'comment', - validIssueIds?: Set, + validIssueIds: Set = new Set(), ): Promise { const applied: ApplyBacklogGroomingResult = { commented: 0, @@ -209,8 +218,9 @@ export async function applyBacklogGrooming( skippedUnknown: 0, }; if (!result.success) return applied; + // Scope is mandatory: an empty/missing Set must not mutate arbitrary IDs. for (const decision of result.decisions) { - if (validIssueIds && !validIssueIds.has(decision.issueId)) { + if (!validIssueIds.has(decision.issueId)) { applied.skippedUnknown++; continue; } diff --git a/src/github/github.test.ts b/src/github/github.test.ts index 14772cdc..1247b691 100644 --- a/src/github/github.test.ts +++ b/src/github/github.test.ts @@ -152,6 +152,33 @@ describe('getPRChecks', () => { vi.useRealTimers(); } }); + + it('clamps poll sleep to the remaining end-to-end deadline', async () => { + vi.useFakeTimers(); + try { + mockGhJson({ + headRefOid: 'head-a', + statusCheckRollup: [{ name: 'unit', status: 'IN_PROGRESS', conclusion: null }], + }); + mockGhJson({ + headRefOid: 'head-a', + statusCheckRollup: [{ name: 'unit', status: 'IN_PROGRESS', conclusion: null }], + }); + + const resultPromise = waitForCICompletion('owner/repo', 42, { + timeoutMs: 50, + pollIntervalMs: 10_000, + expectedHeadSha: 'head-a', + }); + // A fixed 10s poll would blow past the 50ms deadline; clamped sleep must exit on time. + await vi.advanceTimersByTimeAsync(50); + + const result = await resultPromise; + expect(result.status).toBe('pending'); + } finally { + vi.useRealTimers(); + } + }); }); describe('repository fan-out', () => { diff --git a/src/github/github.ts b/src/github/github.ts index d9afb811..85444927 100644 --- a/src/github/github.ts +++ b/src/github/github.ts @@ -1067,7 +1067,17 @@ export async function waitForCICompletion( } lastPending = status; - // Wait before next poll - await new Promise(resolve => setTimeout(resolve, pollIntervalMs)); + // Clamp sleep to the remaining end-to-end deadline so a fixed poll interval + // cannot push past the configured timeout. + const remaining = timeoutMs - (Date.now() - startTime); + if (remaining <= 0) { + console.log(`[GitHub] CI timeout for ${repo}#${prNumber} (${Date.now() - startTime}ms)`); + return lastPending ?? { + status: 'unknown', + reason: expectedHeadSha ? 'head_unavailable' : 'expected_head_unavailable', + expectedHeadSha, + }; + } + await new Promise(resolve => setTimeout(resolve, Math.min(pollIntervalMs, remaining))); } } diff --git a/src/issues/graphql/resolvers.autoLink.test.ts b/src/issues/graphql/resolvers.autoLink.test.ts new file mode 100644 index 00000000..48928b09 --- /dev/null +++ b/src/issues/graphql/resolvers.autoLink.test.ts @@ -0,0 +1,127 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { Issue } from '../schema.js'; +import type { SqliteIssueStore } from '../sqliteStore.js'; + +const autoLinkMemories = vi.fn(); + +vi.mock('../memoryBridge.js', () => ({ + autoLinkMemories: (...args: unknown[]) => autoLinkMemories(...args), + enrichIssueContext: vi.fn(), +})); + +const { __autoLinkTestHooks } = await import('./resolvers.js'); + +function fakeIssue(id: string): Issue { + return { + id, + projectId: 'p', + title: id, + description: '', + status: 'todo', + priority: 'medium', + source: 'local', + labels: [], + relevantFiles: [], + acceptanceCriteria: [], + dependencies: [], + childIds: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + } as Issue; +} + +function deferred() { + let resolve!: (value: T) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +beforeEach(() => { + __autoLinkTestHooks.reset(); + autoLinkMemories.mockReset(); +}); + +afterEach(() => { + __autoLinkTestHooks.reset(); +}); + +describe('scheduleAutoLinkMemories supervision', () => { + it('caps concurrent auto-link jobs at AUTO_LINK_MAX_CONCURRENT', async () => { + const blockers: Array>> = []; + autoLinkMemories.mockImplementation(() => { + const d = deferred(); + blockers.push(d); + return d.promise; + }); + + const store = {} as SqliteIssueStore; + const max = __autoLinkTestHooks.maxConcurrent; + for (let i = 0; i < max + 2; i++) { + __autoLinkTestHooks.schedule(store, fakeIssue(`issue-${i}`)); + } + + await vi.waitFor(() => expect(autoLinkMemories).toHaveBeenCalledTimes(max)); + expect(__autoLinkTestHooks.inFlight).toBe(max); + expect(__autoLinkTestHooks.waiterCount).toBe(2); + + blockers[0]!.resolve(); + await vi.waitFor(() => expect(autoLinkMemories).toHaveBeenCalledTimes(max + 1)); + + for (const b of blockers) b.resolve(); + await vi.waitFor(() => expect(__autoLinkTestHooks.inFlight).toBe(0)); + }); + + it('clears the timeout when auto-link finishes before the deadline', async () => { + const unhandled: unknown[] = []; + const onUnhandled = (reason: unknown) => { + unhandled.push(reason); + }; + process.on('unhandledRejection', onUnhandled); + + try { + __autoLinkTestHooks.setTimeoutMs(80); + autoLinkMemories.mockImplementation( + () => new Promise((resolve) => setTimeout(resolve, 10)), + ); + __autoLinkTestHooks.schedule({} as SqliteIssueStore, fakeIssue('fast')); + await vi.waitFor(() => expect(__autoLinkTestHooks.inFlight).toBe(0)); + // Give a late timer reject time to surface if clearTimeout were missing. + await new Promise((r) => setTimeout(r, 120)); + expect(unhandled).toEqual([]); + } finally { + process.off('unhandledRejection', onUnhandled); + } + }); + + it('releases the slot after a timeout and absorbs a late work rejection', async () => { + const unhandled: unknown[] = []; + const onUnhandled = (reason: unknown) => { + unhandled.push(reason); + }; + process.on('unhandledRejection', onUnhandled); + + try { + __autoLinkTestHooks.setTimeoutMs(30); + autoLinkMemories.mockImplementation( + () => + new Promise((_resolve, reject) => { + setTimeout(() => reject(new Error('late work failure')), 100); + }), + ); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + __autoLinkTestHooks.schedule({} as SqliteIssueStore, fakeIssue('slow')); + await vi.waitFor(() => expect(__autoLinkTestHooks.inFlight).toBe(0)); + expect(warn.mock.calls.some((c) => String(c[0]).includes('timed out'))).toBe(true); + await new Promise((r) => setTimeout(r, 150)); + expect(unhandled).toEqual([]); + warn.mockRestore(); + } finally { + process.off('unhandledRejection', onUnhandled); + } + }); + +}); diff --git a/src/issues/graphql/resolvers.ts b/src/issues/graphql/resolvers.ts index e9d68ea3..d1d92c7d 100644 --- a/src/issues/graphql/resolvers.ts +++ b/src/issues/graphql/resolvers.ts @@ -4,9 +4,9 @@ // Purpose: Query + Mutation 리졸버 // ============================================ -import { getIssueStore } from '../sqliteStore.js'; +import { getIssueStore, type SqliteIssueStore } from '../sqliteStore.js'; import { autoLinkMemories, enrichIssueContext } from '../memoryBridge.js'; -import type { IssueFilter } from '../schema.js'; +import type { Issue, IssueFilter } from '../schema.js'; const DEFAULT_ISSUE_LIMIT = 50; const MAX_ISSUE_LIMIT = 200; @@ -14,6 +14,13 @@ const DEFAULT_EVENT_LIMIT = 50; const DEFAULT_RECENT_EVENT_LIMIT = 20; const MAX_EVENT_LIMIT = 200; +/** Bound fire-and-forget auto-link jobs after createIssue. */ +const AUTO_LINK_MAX_CONCURRENT = 4; +const AUTO_LINK_TIMEOUT_MS = 15_000; +let autoLinkTimeoutMs = AUTO_LINK_TIMEOUT_MS; +let autoLinkInFlight = 0; +const autoLinkWaiters: Array<() => void> = []; + function clampLimit(limit: number | undefined, defaultLimit: number, maxLimit: number): number { if (limit === undefined || !Number.isInteger(limit)) return defaultLimit; return Math.min(Math.max(limit, 1), maxLimit); @@ -51,6 +58,87 @@ function normalizeIssueFilter(filter: IssueFilter | undefined): IssueFilter { }; } +async function acquireAutoLinkSlot(): Promise { + if (autoLinkInFlight < AUTO_LINK_MAX_CONCURRENT) { + autoLinkInFlight++; + return; + } + await new Promise((resolve) => { + autoLinkWaiters.push(() => { + autoLinkInFlight++; + resolve(); + }); + }); +} + +function releaseAutoLinkSlot(): void { + autoLinkInFlight = Math.max(0, autoLinkInFlight - 1); + const next = autoLinkWaiters.shift(); + if (next) next(); +} + +/** + * Supervise createIssue auto-link work: concurrency cap, deadline, and failure + * observation — no unbounded fire-and-forget. + * + * Clears the timeout on settle and absorbs a late work rejection when the + * deadline wins, so neither side can surface an unhandled rejection. + */ +function scheduleAutoLinkMemories(store: SqliteIssueStore, issue: Issue): void { + void (async () => { + await acquireAutoLinkSlot(); + const started = Date.now(); + let timer: ReturnType | undefined; + const work = autoLinkMemories(store, issue); + try { + await new Promise((resolve, reject) => { + timer = setTimeout( + () => reject(new Error(`autoLinkMemories timed out after ${autoLinkTimeoutMs}ms`)), + autoLinkTimeoutMs, + ); + timer.unref?.(); + work.then(() => resolve(), reject); + }); + } catch (err) { + console.warn( + `[GraphQL] 메모리 자동 연결 실패 (issue=${issue.id}, elapsed=${Date.now() - started}ms, inFlight=${autoLinkInFlight}):`, + err, + ); + } finally { + if (timer !== undefined) clearTimeout(timer); + // If the deadline won, absorb a late rejection from the still-running work. + void work.catch(() => {}); + releaseAutoLinkSlot(); + } + })(); +} + +/** @internal Test hooks for auto-link concurrency / timeout supervision. */ +export const __autoLinkTestHooks = { + maxConcurrent: AUTO_LINK_MAX_CONCURRENT, + defaultTimeoutMs: AUTO_LINK_TIMEOUT_MS, + get timeoutMs() { + return autoLinkTimeoutMs; + }, + get inFlight() { + return autoLinkInFlight; + }, + get waiterCount() { + return autoLinkWaiters.length; + }, + setTimeoutMs(ms: number) { + autoLinkTimeoutMs = ms; + }, + reset() { + autoLinkInFlight = 0; + autoLinkWaiters.length = 0; + autoLinkTimeoutMs = AUTO_LINK_TIMEOUT_MS; + }, + schedule: scheduleAutoLinkMemories, + acquire: acquireAutoLinkSlot, + release: releaseAutoLinkSlot, +}; + export const resolvers = { Query: { issue: (_: unknown, { id }: { id: string }) => { @@ -99,10 +187,8 @@ export const resolvers = { const store = getIssueStore(); const issue = store.createIssue(input); - // 비동기로 메모리 자동 연결 (실패해도 이슈 생성은 성공) - autoLinkMemories(store, issue).catch((err) => { - console.warn('[GraphQL] 메모리 자동 연결 실패:', err); - }); + // Bounded + supervised background auto-link (failure does not fail create). + scheduleAutoLinkMemories(store, issue); return issue; }, diff --git a/src/issues/linearBridge.recovery.test.ts b/src/issues/linearBridge.recovery.test.ts new file mode 100644 index 00000000..7c92d071 --- /dev/null +++ b/src/issues/linearBridge.recovery.test.ts @@ -0,0 +1,115 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { + __clearPendingLinearMappingsForTests, + __setLinearBridgeClientForTests, + pushToLinear, +} from './linearBridge.js'; +import { SqliteIssueStore } from './sqliteStore.js'; + +let dir: string | undefined; + +function dbPath(): string { + dir ??= mkdtempSync(join(tmpdir(), 'openswarm-linear-bridge-')); + return join(dir, 'issues.db'); +} + +function installFakeLinear(createIssue = vi.fn()) { + const fakeClient = { + createIssue, + team: vi.fn(async () => ({ + states: async () => ({ + nodes: [ + { id: 'state-todo', name: 'Todo' }, + { id: 'state-backlog', name: 'Backlog' }, + ], + }), + })), + }; + createIssue.mockResolvedValue({ + issue: Promise.resolve({ + id: 'lin-uuid-1', + identifier: 'AGT-1', + url: 'https://linear.app/agt-1', + }), + }); + __setLinearBridgeClientForTests(fakeClient, 'team-test'); + return { fakeClient, createIssue }; +} + +beforeEach(() => { + __clearPendingLinearMappingsForTests(); +}); + +afterEach(() => { + __clearPendingLinearMappingsForTests(); + __setLinearBridgeClientForTests(null); + if (dir) rmSync(dir, { recursive: true, force: true }); + dir = undefined; +}); + +describe('pushToLinear mapping recovery', () => { + it('returns the Linear id when local mapping persist fails and does not recreate on retry', async () => { + const { createIssue } = installFakeLinear(); + const store = new SqliteIssueStore(dbPath()); + const issue = store.createIssue({ projectId: 'p', title: 'recover-me', status: 'todo' }); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + + const updateIssue = vi.spyOn(store, 'updateIssue').mockImplementation(() => { + throw new Error('persist boom'); + }); + + const first = await pushToLinear(store, issue.id); + expect(first).toBe('lin-uuid-1'); + expect(createIssue).toHaveBeenCalledTimes(1); + // Mapping never landed locally. + expect(store.getIssue(issue.id)?.linearId).toBeUndefined(); + + const second = await pushToLinear(store, issue.id); + expect(second).toBe('lin-uuid-1'); + // Pending-map recovery must not call Linear create again. + expect(createIssue).toHaveBeenCalledTimes(1); + + updateIssue.mockRestore(); + warn.mockRestore(); + error.mockRestore(); + store.close(); + }); + + it('retries pending mapping persist and recovers without a second Linear create', async () => { + const { createIssue } = installFakeLinear(); + const store = new SqliteIssueStore(dbPath()); + const issue = store.createIssue({ projectId: 'p', title: 'retry-map', status: 'todo' }); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + + let failuresLeft = 4; // 3 persist attempts + 1 updateIssue-only recovery path + const realUpdate = store.updateIssue.bind(store); + const updateIssue = vi.spyOn(store, 'updateIssue').mockImplementation((id, patch) => { + if (failuresLeft > 0) { + failuresLeft -= 1; + throw new Error(`persist fail ${failuresLeft}`); + } + return realUpdate(id, patch); + }); + + const first = await pushToLinear(store, issue.id); + expect(first).toBe('lin-uuid-1'); + expect(store.getIssue(issue.id)?.linearId).toBeUndefined(); + expect(createIssue).toHaveBeenCalledTimes(1); + + updateIssue.mockRestore(); + const recovered = await pushToLinear(store, issue.id); + expect(recovered).toBe('lin-uuid-1'); + expect(createIssue).toHaveBeenCalledTimes(1); + expect(store.getIssue(issue.id)?.linearId).toBe('lin-uuid-1'); + expect(store.getIssue(issue.id)?.linearIdentifier).toBe('AGT-1'); + + warn.mockRestore(); + error.mockRestore(); + store.close(); + }); +}); diff --git a/src/issues/linearBridge.ts b/src/issues/linearBridge.ts index 62924e9f..0cc3460f 100644 --- a/src/issues/linearBridge.ts +++ b/src/issues/linearBridge.ts @@ -91,8 +91,83 @@ export async function syncFromLinear( } /** - * 로컬 → Linear: 로컬 이슈를 Linear에 생성 + * 로컬 → Linear: 로컬 이슈를 Linear에 생성. + * Linear create와 로컬 mapping persist를 분리해, mapping 실패 시 linearId로 + * 재연결/재시도하고 동일 프로세스 재호출에서 중복 create를 막는다. */ +const pendingLinearMappings = new Map(); + +const MAPPING_PERSIST_ATTEMPTS = 3; + +/** @internal Test-only: install a fake client without loading the SDK. */ +export function __setLinearBridgeClientForTests(client: unknown, teamId = 'team-test'): void { + linearClient = client; + linearTeamId = teamId; + linearInitPromise = Promise.resolve(); +} + +/** @internal Test-only: drop in-process pending mapping recovery state. */ +export function __clearPendingLinearMappingsForTests(): void { + pendingLinearMappings.clear(); +} + +function persistLinearMapping( + store: SqliteIssueStore, + issueId: string, + mapping: { linearId: string; linearIdentifier: string; linearUrl: string }, +): void { + store.updateIssue(issueId, { + linearId: mapping.linearId, + linearIdentifier: mapping.linearIdentifier, + linearUrl: mapping.linearUrl, + }); + store.addEvent(issueId, 'linked', { + content: `Linear에 생성: ${mapping.linearIdentifier}`, + newValue: mapping.linearIdentifier, + idempotencyKey: `linear-linked:${mapping.linearId}`, + }); +} + +function persistLinearMappingWithRetry( + store: SqliteIssueStore, + issueId: string, + mapping: { linearId: string; linearIdentifier: string; linearUrl: string }, +): boolean { + let lastErr: unknown; + for (let attempt = 1; attempt <= MAPPING_PERSIST_ATTEMPTS; attempt++) { + try { + persistLinearMapping(store, issueId, mapping); + pendingLinearMappings.delete(issueId); + return true; + } catch (err) { + lastErr = err; + console.warn( + `[LinearBridge] 로컬 mapping persist 실패 (${attempt}/${MAPPING_PERSIST_ATTEMPTS}):`, + err, + ); + } + } + // Best-effort reconnect: updateIssue alone may succeed even if addEvent failed. + try { + store.updateIssue(issueId, { + linearId: mapping.linearId, + linearIdentifier: mapping.linearIdentifier, + linearUrl: mapping.linearUrl, + }); + pendingLinearMappings.delete(issueId); + console.warn('[LinearBridge] mapping recovered via updateIssue-only path'); + return true; + } catch (err) { + lastErr = err; + } + console.error('[LinearBridge] 로컬 mapping persist 복구 실패:', lastErr); + return false; +} + export async function pushToLinear( store: SqliteIssueStore, issueId: string, @@ -107,6 +182,18 @@ export async function pushToLinear( if (!issue) return null; if (issue.linearId) return issue.linearId; // 이미 연결됨 + // In-process recovery: a prior create succeeded but local mapping failed. + const pending = pendingLinearMappings.get(issueId); + if (pending) { + if (persistLinearMappingWithRetry(store, issueId, pending)) { + console.log(`[LinearBridge] 이슈 ${issueId} → Linear ${pending.linearIdentifier} (recovered)`); + return pending.linearId; + } + // Still unrecovered — return known linearId to avoid a duplicate create. + return pending.linearId; + } + + let mapping: { linearId: string; linearIdentifier: string; linearUrl: string }; try { const stateId = await resolveLinearStateId(mapStatusToLinear(issue.status)); @@ -121,24 +208,29 @@ export async function pushToLinear( const linearIssue = await created.issue; if (!linearIssue) return null; - // 로컬 이슈에 Linear ID 연결 - store.updateIssue(issueId, { + mapping = { linearId: linearIssue.id, linearIdentifier: linearIssue.identifier, linearUrl: linearIssue.url, - }); - - store.addEvent(issueId, 'linked', { - content: `Linear에 생성: ${linearIssue.identifier}`, - newValue: linearIssue.identifier, - }); - - console.log(`[LinearBridge] 이슈 ${issueId} → Linear ${linearIssue.identifier}`); - return linearIssue.id; + }; } catch (err) { console.error('[LinearBridge] Linear 생성 실패:', err); return null; } + + // Remember the external id before local persist so retries cannot orphan-recreate. + pendingLinearMappings.set(issueId, mapping); + + if (!persistLinearMappingWithRetry(store, issueId, mapping)) { + // External issue exists; return its id so callers do not treat this as "not created". + console.error( + `[LinearBridge] Linear ${mapping.linearIdentifier} 생성됨 but local mapping incomplete for ${issueId}`, + ); + return mapping.linearId; + } + + console.log(`[LinearBridge] 이슈 ${issueId} → Linear ${mapping.linearIdentifier}`); + return mapping.linearId; } /** diff --git a/src/issues/sqliteStore.test.ts b/src/issues/sqliteStore.test.ts index 120d3a85..03c1b978 100644 --- a/src/issues/sqliteStore.test.ts +++ b/src/issues/sqliteStore.test.ts @@ -25,6 +25,16 @@ describe('SqliteIssueStore durable semantics', () => { store.close(); }); + it('returns the existing row when createIssue is called again with the same id', () => { + const store = new SqliteIssueStore(path()); + const first = store.createIssue({ id: 'stable-1', projectId: 'p', title: 'first' }); + const second = store.createIssue({ id: 'stable-1', projectId: 'p', title: 'ignored duplicate' }); + expect(second.id).toBe(first.id); + expect(second.title).toBe('first'); + expect(store.listIssues().total).toBe(1); + store.close(); + }); + it('emits memory_linked only for a newly inserted link', () => { const store = new SqliteIssueStore(path()); const issue = store.createIssue({ projectId: 'p', title: 'link' }); diff --git a/src/issues/sqliteStore.ts b/src/issues/sqliteStore.ts index 91b12c3a..bd0f6fd1 100644 --- a/src/issues/sqliteStore.ts +++ b/src/issues/sqliteStore.ts @@ -291,6 +291,13 @@ export class SqliteIssueStore implements IIssueStore { // ============ 이슈 CRUD ============ createIssue(input: CreateIssueInput): Issue { + // Honor the documented idempotent-ID contract: a caller-supplied stable id + // returns the existing row instead of colliding on UNIQUE(id). + if (input.id) { + const existing = this.getIssue(input.id); + if (existing) return existing; + } + const id = input.id ?? nanoid(12); const now = new Date().toISOString(); @@ -347,7 +354,16 @@ export class SqliteIssueStore implements IIssueStore { insertEvent.run(nanoid(12), id, input.title, now); }); - transaction(); + try { + transaction(); + } catch (error) { + // Concurrent create with the same caller id: return the winner's row. + if (input.id) { + const existing = this.getIssue(input.id); + if (existing) return existing; + } + throw error; + } return this.getIssue(id)!; } diff --git a/src/linear/index.ts b/src/linear/index.ts index 433d999e..61360740 100644 --- a/src/linear/index.ts +++ b/src/linear/index.ts @@ -1,2 +1,2 @@ export * from './linear.js'; -export { updateProjectAfterTask, postStatusUpdate, setLinearClient } from './projectUpdater.js'; +export { updateProjectAfterTask, postStatusUpdate, setLinearClient, fetchProjectOverviewIssues } from './projectUpdater.js'; diff --git a/src/linear/projectUpdater.pagination.test.ts b/src/linear/projectUpdater.pagination.test.ts new file mode 100644 index 00000000..1956c9b1 --- /dev/null +++ b/src/linear/projectUpdater.pagination.test.ts @@ -0,0 +1,107 @@ +import { describe, expect, it } from 'vitest'; +import { LinearClient } from '@linear/sdk'; +import { fetchProjectOverviewIssues } from './projectUpdater.js'; + +describe('fetchProjectOverviewIssues pagination', () => { + it('collects every page until hasNextPage is false', async () => { + let page = 0; + const linear = { + client: { + rawRequest: async () => { + const current = page++; + return { + data: { + project: { + issues: { + nodes: [{ priority: current + 1, state: { name: `S${current}` } }], + pageInfo: { + hasNextPage: current === 0, + endCursor: current === 0 ? 'cursor-1' : null, + }, + }, + }, + }, + }; + }, + }, + } as unknown as LinearClient; + + const nodes = await fetchProjectOverviewIssues(linear, 'proj-1'); + expect(nodes.map((n) => n.state?.name)).toEqual(['S0', 'S1']); + }); + + it('rejects a missing endCursor while more pages are claimed', async () => { + const linear = { + client: { + rawRequest: async () => ({ + data: { + project: { + issues: { + nodes: [{ priority: 1, state: { name: 'Todo' } }], + pageInfo: { hasNextPage: true, endCursor: null }, + }, + }, + }, + }), + }, + } as unknown as LinearClient; + + await expect(fetchProjectOverviewIssues(linear, 'proj-1')).rejects.toThrow( + /missing or repeated cursor/, + ); + }); + + it('rejects a repeated endCursor that cannot progress', async () => { + const linear = { + client: { + rawRequest: async () => ({ + data: { + project: { + issues: { + nodes: [{ priority: 2, state: { name: 'Todo' } }], + pageInfo: { hasNextPage: true, endCursor: 'same-cursor' }, + }, + }, + }, + }), + }, + } as unknown as LinearClient; + + // First page sets after=same-cursor; second page returns the same cursor again. + await expect(fetchProjectOverviewIssues(linear, 'proj-1')).rejects.toThrow( + /missing or repeated cursor/, + ); + }); + + it('reports explicit truncation instead of silently returning a partial set', async () => { + let page = 0; + const linear = { + client: { + rawRequest: async () => ({ + data: { + project: { + issues: { + nodes: [{ priority: 1, state: { name: 'Todo' } }], + pageInfo: { hasNextPage: true, endCursor: `cursor-${page++}` }, + }, + }, + }, + }), + }, + } as unknown as LinearClient; + + await expect(fetchProjectOverviewIssues(linear, 'proj-1')).rejects.toThrow(/safety cap/); + }); + + it('rejects a null issues connection', async () => { + const linear = { + client: { + rawRequest: async () => ({ data: { project: { issues: null } } }), + }, + } as unknown as LinearClient; + + await expect(fetchProjectOverviewIssues(linear, 'proj-1')).rejects.toThrow( + /no issues connection/, + ); + }); +}); diff --git a/src/linear/projectUpdater.ts b/src/linear/projectUpdater.ts index 10fd4ca5..4ce17f8b 100644 --- a/src/linear/projectUpdater.ts +++ b/src/linear/projectUpdater.ts @@ -402,7 +402,7 @@ const PROJECT_OVERVIEW_ISSUES_QUERY = ` } }`; -async function fetchProjectOverviewIssues( +export async function fetchProjectOverviewIssues( linear: LinearClient, projectId: string, ): Promise { @@ -411,7 +411,8 @@ async function fetchProjectOverviewIssues( }).client; const issueNodes: ProjectOverviewIssueNode[] = []; let after: string | undefined; - let complete = false; + let hasNextPage = false; + const safetyCap = PROJECT_OVERVIEW_MAX_PAGES * PROJECT_OVERVIEW_PAGE_SIZE; for (let page = 0; page < PROJECT_OVERVIEW_MAX_PAGES; page++) { const res = await withRateLimit('linear', () => @@ -429,19 +430,23 @@ async function fetchProjectOverviewIssues( }), ); const issues = res.data.project?.issues; - if (!issues) break; + if (!issues) { + throw new Error('Project overview pagination returned no issues connection'); + } issueNodes.push(...issues.nodes); - if (!issues.pageInfo.hasNextPage) { - complete = true; - break; + hasNextPage = issues.pageInfo.hasNextPage === true; + if (!hasNextPage) break; + + const endCursor = issues.pageInfo.endCursor ?? undefined; + if (!endCursor || endCursor === after) { + throw new Error('Project overview pagination returned a missing or repeated cursor'); } - after = issues.pageInfo.endCursor ?? undefined; - if (!after) break; + after = endCursor; } - if (!complete && issueNodes.length >= PROJECT_OVERVIEW_MAX_PAGES * PROJECT_OVERVIEW_PAGE_SIZE) { - throw new Error(`Project overview exceeds the ${PROJECT_OVERVIEW_MAX_PAGES * PROJECT_OVERVIEW_PAGE_SIZE}-issue safety cap`); + if (hasNextPage) { + throw new Error(`Project overview exceeds the ${safetyCap}-issue safety cap`); } return issueNodes; diff --git a/tmp-write-probe-a598.txt b/tmp-write-probe-a598.txt new file mode 100644 index 00000000..e69de29b diff --git a/vitest.config.ts b/vitest.config.ts index d6b031bd..0c1f7d2b 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,4 +1,6 @@ import { defineConfig } from 'vitest/config'; +import os from 'node:os'; +import path from 'node:path'; const integrationBoundaryCoverageExcludes = [ // External model/provider adapters and process wrappers are covered by smoke/e2e tests. @@ -124,6 +126,9 @@ const integrationBoundaryCoverageExcludes = [ ]; export default defineConfig({ + // Verification mounts the worktree read-only; vite's default cache under + // node_modules/.vite-temp then fails with EROFS. Keep the cache in TMPDIR. + cacheDir: path.join(os.tmpdir(), 'openswarm-vite-cache'), test: { globals: true, environment: 'node',