diff --git a/openswarm.json b/openswarm.json new file mode 100644 index 0000000..719f324 --- /dev/null +++ b/openswarm.json @@ -0,0 +1,10 @@ +{ + "schemaVersion": 1, + "projectName": "OpenSwarm", + "linear": { + "teamId": "49b7af95-3cac-4a56-adc7-f19d77dfbe9b", + "teamKey": "INT", + "projectId": "74a9d092-7b3c-4d4d-a998-a2c9a8f08e83", + "projectName": "OpenSwarm" + } +} diff --git a/src/cli/linearMapping.test.ts b/src/cli/linearMapping.test.ts new file mode 100644 index 0000000..c783d67 --- /dev/null +++ b/src/cli/linearMapping.test.ts @@ -0,0 +1,59 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +const getProfileMock = vi.fn(); +vi.mock('../auth/index.js', () => ({ + AuthProfileStore: vi.fn().mockImplementation(function AuthProfileStore(this: unknown) { + return { getProfile: getProfileMock }; + }), + ensureValidToken: vi.fn(), +})); + +const loadConfigMock = vi.fn(); +vi.mock('../core/config.js', () => ({ + loadConfig: (...args: unknown[]) => loadConfigMock(...args), +})); + +const { resolveLinearCredential } = await import('./linearMapping.js'); + +describe('resolveLinearCredential (INT-2619)', () => { + const originalEnv = process.env.LINEAR_API_KEY; + + beforeEach(() => { + vi.clearAllMocks(); + getProfileMock.mockReturnValue(undefined); + loadConfigMock.mockReturnValue({ linearApiKey: '' }); + delete process.env.LINEAR_API_KEY; + }); + + afterEach(() => { + if (originalEnv === undefined) delete process.env.LINEAR_API_KEY; + else process.env.LINEAR_API_KEY = originalEnv; + }); + + it('returns null when no OAuth profile, env var, or config apiKey is present', async () => { + expect(await resolveLinearCredential()).toBeNull(); + }); + + it('falls back to LINEAR_API_KEY when no OAuth profile is stored', async () => { + process.env.LINEAR_API_KEY = 'env-key'; + expect(await resolveLinearCredential()).toEqual({ apiKey: 'env-key' }); + }); + + it('falls back to config.yaml `linear.apiKey` when neither OAuth profile nor env var is present (INT-2619)', async () => { + // This is the exact gap the reviewer caught: ensureTaskSource() initializes + // Linear from config.linearApiKey too, so the mapping preflight must check + // the same source or it wrongly concludes "Linear isn't configured" and lets + // filing proceed without a project. + loadConfigMock.mockReturnValue({ linearApiKey: 'config-key' }); + expect(await resolveLinearCredential()).toEqual({ apiKey: 'config-key' }); + }); + + it('prefers the OAuth profile over env var and config apiKey', async () => { + getProfileMock.mockReturnValue({ provider: 'linear' }); + process.env.LINEAR_API_KEY = 'env-key'; + loadConfigMock.mockReturnValue({ linearApiKey: 'config-key' }); + const { ensureValidToken } = await import('../auth/index.js'); + vi.mocked(ensureValidToken).mockResolvedValue('oauth-token'); + expect(await resolveLinearCredential()).toEqual({ accessToken: 'oauth-token' }); + }); +}); diff --git a/src/cli/linearMapping.ts b/src/cli/linearMapping.ts index d34b83c..f0ad6b0 100644 --- a/src/cli/linearMapping.ts +++ b/src/cli/linearMapping.ts @@ -16,8 +16,10 @@ import { AuthProfileStore, ensureValidToken } from '../auth/index.js'; /** * Resolve a Linear credential for non-interactive use (no auth prompt): * 1. linear:default OAuth profile (refreshed via ensureValidToken), then - * 2. LINEAR_API_KEY env var. - * Returns null when neither is present — the caller should hint at `auth login`. + * 2. LINEAR_API_KEY env var, then + * 3. config.yaml `linear.apiKey` — the same source ensureTaskSource() uses, so + * this stays in sync with what actually initializes Linear for filing. (INT-2619) + * Returns null when none is present — the caller should hint at `auth login`. */ export async function resolveLinearCredential(): Promise { try { @@ -31,6 +33,13 @@ export async function resolveLinearCredential(): Promise { }); }); +describe('ensureProjectMapping (INT-2599)', () => { + // A path with no openswarm.json, so resolveProjectId(cwd) always misses and + // each test's own stubs drive the rest of the decision tree. + const unmappedCwd = '/tmp/openswarm-test-ensure-project-mapping-unmapped'; + + it('short-circuits without touching Linear when an explicit parent is given', async () => { + const resolveCredential = vi.fn(async () => ({ apiKey: 'x' })); + const result = await ensureProjectMapping(unmappedCwd, 'INT-1', { resolveCredential }); + expect(result).toEqual({ projectId: undefined, abort: false }); + expect(resolveCredential).not.toHaveBeenCalled(); + }); + + it('proceeds without a project when Linear is not configured at all', async () => { + const result = await ensureProjectMapping(unmappedCwd, undefined, { + resolveCredential: async () => null, + }); + expect(result).toEqual({ projectId: undefined, abort: false }); + }); + + it('fails closed (no orphan issue) when unmapped and there is no terminal to prompt', async () => { + const logs: string[] = []; + const pickAndSave = vi.fn(); + const result = await ensureProjectMapping(unmappedCwd, undefined, { + resolveCredential: async () => ({ apiKey: 'x' }), + isTTY: false, + pickAndSave, + log: (l) => logs.push(l), + }); + expect(result).toEqual({ projectId: undefined, abort: true }); + expect(pickAndSave).not.toHaveBeenCalled(); + expect(logs.join('\n')).toMatch(/openswarm add/); + }); + + it('maps interactively on a TTY and returns the saved project id', async () => { + const pickAndSave = vi.fn(async () => ({ + kind: 'saved' as const, + teamId: 'team-1', + mapping: { teamId: 'team-1', projectId: 'proj-1', projectName: 'Demo' }, + })); + const result = await ensureProjectMapping(unmappedCwd, undefined, { + resolveCredential: async () => ({ apiKey: 'x' }), + isTTY: true, + pickAndSave, + }); + expect(result).toEqual({ projectId: 'proj-1', abort: false }); + expect(pickAndSave).toHaveBeenCalledWith(unmappedCwd, { apiKey: 'x' }); + }); + + it('fails closed when the Linear team lookup itself fails (no-teams is not a user choice) (INT-2619)', async () => { + const result = await ensureProjectMapping(unmappedCwd, undefined, { + resolveCredential: async () => ({ apiKey: 'x' }), + isTTY: true, + pickAndSave: async () => ({ kind: 'no-teams' as const }), + }); + expect(result).toEqual({ projectId: undefined, abort: true }); + }); + + it('proceeds without a project when the user actively skips the interactive picker', async () => { + const result = await ensureProjectMapping(unmappedCwd, undefined, { + resolveCredential: async () => ({ apiKey: 'x' }), + isTTY: true, + pickAndSave: async () => ({ kind: 'skipped' as const }), + }); + expect(result).toEqual({ projectId: undefined, abort: false }); + }); +}); + describe('runReviewCommand --issues branch inference (INT-1967)', () => { const approveWithFollowups = async () => ({ decision: 'approve', feedback: 'ok', recommendedActions: [{ type: 'test', title: 't' }] }) as ReviewResult; @@ -84,6 +157,7 @@ describe('runReviewCommand --issues branch inference (INT-1967)', () => { review: approveWithFollowups, getBranch: async () => 'main', fileFollowups: async () => 0, // e.g. Linear not configured + ensureProjectMapping: async () => ({ projectId: undefined, abort: false }), startProgress: () => null, log: (l) => logs.push(l), }, @@ -112,6 +186,7 @@ describe('runReviewCommand --issues branch inference (INT-1967)', () => { review: approveWithFollowups, getBranch: async () => 'main', fileFollowups, + ensureProjectMapping: async () => ({ projectId: undefined, abort: false }), startProgress: () => null, log: (l) => logs.push(l), }, diff --git a/src/cli/reviewCommand.ts b/src/cli/reviewCommand.ts index aed1132..41173e7 100644 --- a/src/cli/reviewCommand.ts +++ b/src/cli/reviewCommand.ts @@ -88,6 +88,71 @@ export async function resolveProjectId(cwd: string): Promise } } +export interface ProjectMappingResult { + /** Resolved Linear project id — undefined when none applies (explicit parent) or none is available. */ + projectId: string | undefined; + /** True when Linear filing should be skipped entirely rather than create a project-less (orphan) issue. */ + abort: boolean; +} + +/** + * Preflight before filing a standalone/master Linear issue (no explicit + * `--issues ` parent to inherit a project from): if this repo has no + * openswarm.json Linear mapping yet, map it now — interactively on a TTY + * (writes openswarm.json, same picker as `openswarm add`), or fail closed + * headless so an unattended run never silently creates an orphan issue. (INT-2599) + */ +export async function ensureProjectMapping( + cwd: string, + parentIssueId: string | undefined, + deps: { + resolveCredential?: () => Promise; + pickAndSave?: ( + repoPath: string, + cred: import('../linear/index.js').LinearCredential, + ) => Promise; + isTTY?: boolean; + log?: (line: string) => void; + } = {}, +): Promise { + // A sub-issue under an explicit parent inherits that parent's project. + if (parentIssueId) return { projectId: undefined, abort: false }; + + const existing = await resolveProjectId(cwd); + if (existing) return { projectId: existing, abort: false }; + + const log = deps.log ?? ((l: string) => console.log(l)); + const resolveCredential = + deps.resolveCredential ?? (async () => (await import('./linearMapping.js')).resolveLinearCredential()); + const cred = await resolveCredential(); + if (!cred) return { projectId: undefined, abort: false }; // Linear isn't configured — downstream reports that. + + const isTTY = deps.isTTY ?? !!process.stdin.isTTY; + if (!isTTY) { + log( + `No Linear project mapped for this repo (openswarm.json missing) and no terminal to map it interactively — ` + + `skipping issue filing to avoid an unassigned (orphan) issue. Run \`openswarm add ${cwd}\` once in a ` + + `terminal to map it, then re-run.`, + ); + return { projectId: undefined, abort: true }; + } + + log('This repo is not mapped to a Linear project yet — mapping it now:'); + const pickAndSave = + deps.pickAndSave ?? (async (p, c) => (await import('./linearMapping.js')).pickAndSaveLinearMapping(p, c)); + try { + const result = await pickAndSave(cwd, cred); + if (result.kind === 'saved') return { projectId: result.mapping.projectId, abort: false }; + // 'no-teams' means the Linear lookup itself failed or returned nothing — not a + // user choice — so fail closed rather than silently file a project-less issue. + if (result.kind === 'no-teams') return { projectId: undefined, abort: true }; + return { projectId: undefined, abort: false }; // user actively skipped the picker — not a silent gap. + } catch (err) { + if (err instanceof Error && err.name === 'ExitPromptError') return { projectId: undefined, abort: true }; + throw err; + } +} + /** * A Linear-backed task source for the standalone `review` CLI. The daemon * registers one at startup, but a bare `openswarm review` does not — so init @@ -151,6 +216,8 @@ export async function runReviewCommand( getChangedFiles?: (cwd: string) => Promise; review?: (wr: WorkerResult, cwd: string, onLog?: (line: string) => void) => Promise; fileFollowups?: (parentIssueId: string | undefined, review: ReviewResult) => Promise; + /** Override the project-mapping preflight (tests stub this to skip the interactive picker). */ + ensureProjectMapping?: (cwd: string, parentIssueId: string | undefined) => Promise; log?: (line: string) => void; /** Override the progress indicator (default: TTY-gated spinner). Tests pass a stub. */ startProgress?: () => { note: (line: string) => void; stop: () => void } | null; @@ -223,28 +290,34 @@ export async function runReviewCommand( if (parent) log(`Filing follow-ups under ${parent} (inferred from branch "${branch}").`); } // No parent → create top-level (standalone) issues rather than refusing. (INT-1968) - // Default path initializes a Linear task source itself (the daemon isn't - // running here), and files regardless of decision. (INT-1969) - const fileFollowups = - deps.fileFollowups ?? - (async (p: string | undefined, r: ReviewResult) => { - const { fileReviewerFollowups } = await import('../automation/runnerExecution.js'); - const source = await ensureTaskSource(); - if (!source) return 0; - const projectId = p ? undefined : await resolveProjectId(cwd); - return fileReviewerFollowups(source, p, r, { autoFile: true, projectId, requireApprove: false }); - }); - const filed = await fileFollowups(parent, result); - if (filed > 0) { - log( - parent - ? `Filed ${filed} follow-up sub-issue(s) under ${parent}.` - : `Filed ${filed} standalone follow-up issue(s) (pass \`--issues \` to nest them under an issue).`, - ); + // Before that, make sure this repo actually has a Linear project to land + // in — map it now (interactive) or bail rather than file an orphan. (INT-2599) + const mapping = await (deps.ensureProjectMapping ?? (async (c, p) => ensureProjectMapping(c, p, { log })))(cwd, parent); + if (mapping.abort) { + log('Skipped filing follow-ups — map this repo to a Linear project first (see above), then re-run.'); } else { - log( - `Could not file follow-ups (0 created). Is Linear connected? Run \`openswarm auth login --provider linear\` (or set linearApiKey in config).`, - ); + // Default path initializes a Linear task source itself (the daemon isn't + // running here), and files regardless of decision. (INT-1969) + const fileFollowups = + deps.fileFollowups ?? + (async (p: string | undefined, r: ReviewResult) => { + const { fileReviewerFollowups } = await import('../automation/runnerExecution.js'); + const source = await ensureTaskSource(); + if (!source) return 0; + return fileReviewerFollowups(source, p, r, { autoFile: true, projectId: mapping.projectId, requireApprove: false }); + }); + const filed = await fileFollowups(parent, result); + if (filed > 0) { + log( + parent + ? `Filed ${filed} follow-up sub-issue(s) under ${parent}.` + : `Filed ${filed} standalone follow-up issue(s) (pass \`--issues \` to nest them under an issue).`, + ); + } else { + log( + `Could not file follow-ups (0 created). Is Linear connected? Run \`openswarm auth login --provider linear\` (or set linearApiKey in config).`, + ); + } } } else if (followups) { // Suggestions were made but nothing was filed — make the flag discoverable. (INT-1966/1967) diff --git a/src/cli/reviewMaxCommand.test.ts b/src/cli/reviewMaxCommand.test.ts new file mode 100644 index 0000000..bfce8e2 --- /dev/null +++ b/src/cli/reviewMaxCommand.test.ts @@ -0,0 +1,119 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { ReviewResult } from '../agents/agentPair.js'; +import type { AuditRun, AuditSummary } from './reviewAudit.js'; + +const ensureTaskSourceMock = vi.fn(); +const ensureProjectMappingMock = vi.fn(); +const resolveIssueFromBranchMock = vi.fn(); +vi.mock('./reviewCommand.js', () => ({ + ensureTaskSource: (...args: unknown[]) => ensureTaskSourceMock(...args), + ensureProjectMapping: (...args: unknown[]) => ensureProjectMappingMock(...args), + resolveIssueFromBranch: (...args: unknown[]) => resolveIssueFromBranchMock(...args), + buildReviewWorkerResult: vi.fn(), +})); + +const synthesizeAuditIssuesMock = vi.fn(); +vi.mock('./auditPM.js', () => ({ + synthesizeAuditIssues: (...args: unknown[]) => synthesizeAuditIssuesMock(...args), +})); + +const fileReviewerFollowupsMock = vi.fn(); +vi.mock('../automation/runnerExecution.js', () => ({ + fileReviewerFollowups: (...args: unknown[]) => fileReviewerFollowupsMock(...args), +})); + +const { filePerAreaFollowups, filePmSynthesizedIssues } = await import('./reviewMaxCommand.js'); + +function makeRun(actions: ReviewResult['recommendedActions']): AuditRun { + const review: ReviewResult = { decision: 'revise', feedback: 'x', recommendedActions: actions }; + return { + results: [{ area: { label: 'src', dir: 'src', files: ['src/a.ts'] }, review }], + summary: { decision: 'revise', totalAreas: 1, completed: 1, failed: 0, areas: [], issues: [], recommendedActions: actions ?? [] } as AuditSummary, + }; +} + +beforeEach(() => { + vi.clearAllMocks(); + ensureTaskSourceMock.mockResolvedValue({ createTask: vi.fn(), createSubIssue: vi.fn() }); + resolveIssueFromBranchMock.mockReturnValue(undefined); +}); + +describe('filePerAreaFollowups (INT-2599)', () => { + it('skips filing entirely when the project-mapping preflight aborts', async () => { + ensureProjectMappingMock.mockResolvedValue({ projectId: undefined, abort: true }); + const run = makeRun([{ type: 'bug', title: 'fix it' }]); + + await filePerAreaFollowups('/repo', true, run); + + expect(fileReviewerFollowupsMock).not.toHaveBeenCalled(); + }); + + it('threads the resolved projectId into fileReviewerFollowups', async () => { + ensureProjectMappingMock.mockResolvedValue({ projectId: 'proj-123', abort: false }); + fileReviewerFollowupsMock.mockResolvedValue(1); + const run = makeRun([{ type: 'bug', title: 'fix it' }]); + + await filePerAreaFollowups('/repo', true, run); + + expect(fileReviewerFollowupsMock).toHaveBeenCalledWith( + expect.anything(), + undefined, + expect.anything(), + expect.objectContaining({ projectId: 'proj-123' }), + ); + }); +}); + +describe('filePmSynthesizedIssues (INT-2599)', () => { + it('skips creating the master issue and synthesized issues when the preflight aborts', async () => { + ensureProjectMappingMock.mockResolvedValue({ projectId: undefined, abort: true }); + const source = { createTask: vi.fn(), createSubIssue: vi.fn() }; + ensureTaskSourceMock.mockResolvedValue(source); + const summary: AuditSummary = { + decision: 'revise', + totalAreas: 1, + completed: 1, + failed: 0, + areas: [], + issues: [], + recommendedActions: [{ type: 'bug', title: 'fix it' }], + }; + + await filePmSynthesizedIssues('/repo', {}, summary, '# report', '2026-07-10T00-00-00'); + + expect(source.createTask).not.toHaveBeenCalled(); + expect(source.createSubIssue).not.toHaveBeenCalled(); + expect(synthesizeAuditIssuesMock).not.toHaveBeenCalled(); + }); + + it('passes the resolved projectId to the master issue and synthesized sub-issues', async () => { + ensureProjectMappingMock.mockResolvedValue({ projectId: 'proj-456', abort: false }); + const source = { + createTask: vi.fn().mockResolvedValue({ id: 'master-1', identifier: 'INT-1', title: 'master' }), + createSubIssue: vi.fn().mockResolvedValue({ id: 'sub-1', identifier: 'INT-2', title: 'sub' }), + }; + ensureTaskSourceMock.mockResolvedValue(source); + synthesizeAuditIssuesMock.mockResolvedValue([ + { title: 'grouped fix', priority: 2, items: ['fix it'], description: 'body' }, + ]); + const summary: AuditSummary = { + decision: 'revise', + totalAreas: 1, + completed: 1, + failed: 0, + areas: [], + issues: [], + recommendedActions: [{ type: 'bug', title: 'fix it' }], + }; + + await filePmSynthesizedIssues('/repo', {}, summary, '# report', '2026-07-10T00-00-00'); + + expect(source.createTask).toHaveBeenCalledWith(expect.any(String), '# report', 'proj-456'); + expect(source.createSubIssue).toHaveBeenCalledWith( + 'master-1', + 'grouped fix', + 'body', + expect.objectContaining({ projectId: 'proj-456' }), + ); + }); +}); diff --git a/src/cli/reviewMaxCommand.tsx b/src/cli/reviewMaxCommand.tsx index 469c0e5..4e8483c 100644 --- a/src/cli/reviewMaxCommand.tsx +++ b/src/cli/reviewMaxCommand.tsx @@ -29,7 +29,7 @@ import { type AuditSummary, } from './reviewAudit.js'; import { AuditBoard } from '../tui/components/AuditBoard.js'; -import { resolveIssueFromBranch, ensureTaskSource, resolveProjectId } from './reviewCommand.js'; +import { resolveIssueFromBranch, ensureTaskSource, ensureProjectMapping } from './reviewCommand.js'; import { synthesizeAuditIssues } from './auditPM.js'; import { c, status } from '../support/colors.js'; import type { AdapterName } from '../adapters/types.js'; @@ -102,7 +102,7 @@ async function confirmCost(areas: number, files: number, concurrency: number): P } /** File each area's recommendedActions as Linear follow-ups, one call per area (avoids the 10-action cap). */ -async function filePerAreaFollowups(cwd: string, fileIssue: string | boolean, run: AuditRun): Promise { +export async function filePerAreaFollowups(cwd: string, fileIssue: string | boolean, run: AuditRun): Promise { const withActions = run.results.filter((r) => r.review?.recommendedActions?.length); if (!withActions.length) { console.log('No follow-ups to file.'); @@ -130,12 +130,21 @@ async function filePerAreaFollowups(cwd: string, fileIssue: string | boolean, ru parent = resolveIssueFromBranch(branch); if (parent) console.log(`Filing follow-ups under ${parent} (inferred from branch "${branch}").`); } - const projectId = parent ? undefined : await resolveProjectId(cwd); + + const mapping = await ensureProjectMapping(cwd, parent); + if (mapping.abort) { + console.log('Skipped filing follow-ups — map this repo to a Linear project first, then re-run.'); + return; + } const { fileReviewerFollowups } = await import('../automation/runnerExecution.js'); let filed = 0; for (const r of withActions) { - filed += await fileReviewerFollowups(source, parent, r.review!, { autoFile: true, projectId, requireApprove: false }); + filed += await fileReviewerFollowups(source, parent, r.review!, { + autoFile: true, + projectId: mapping.projectId, + requireApprove: false, + }); } console.log( filed > 0 @@ -340,18 +349,17 @@ export async function runReviewMaxCommand(opts: ReviewMaxOptions = {}): Promise< * behavior). Returns the created issue's internal id (usable as a sub-issue * parent) or null on failure. (INT-2022 / INT-2225) */ -async function createMasterAuditIssue( - cwd: string, +export async function createMasterAuditIssue( summary: AuditSummary, report: string, ts: string, + projectId: string | undefined, ): Promise { const source = await ensureTaskSource(); if (!source) { console.log(status.warn('Linear not connected — report saved to file only. `openswarm auth login --provider linear` to enable.')); return null; } - const projectId = await resolveProjectId(cwd); const title = `chore(audit): codebase audit ${ts.slice(0, 10)} — review --max (${summary.recommendedActions.length} follow-ups)`; try { const res = await source.createTask(title, report, projectId); @@ -375,7 +383,7 @@ async function createMasterAuditIssue( * follow-ups, or the LLM output couldn't be parsed), the master issue alone still * captures everything. (INT-2225) */ -async function filePmSynthesizedIssues( +export async function filePmSynthesizedIssues( cwd: string, opts: ReviewMaxOptions, summary: AuditSummary, @@ -394,23 +402,23 @@ async function filePmSynthesizedIssues( ); return; } - const projectId = await resolveProjectId(cwd); - if (!projectId) { - // No /openswarm.json mapping → issues get filed without a project (and - // on a multi-team config, only the first team). Tell the user how to map it. (INT-2239) - console.warn( - status.warn( - 'No Linear project mapped for this repo (openswarm.json `linear.projectId` missing) — ' + - 'issues will not be linked to a project. Run `openswarm add` here to map it.', - ), - ); - } - // Resolve the parent: explicit --issues , else create the master report issue. - let parentId: string | undefined = + const explicitParentId: string | undefined = typeof opts.fileIssue === 'string' && opts.fileIssue ? opts.fileIssue : undefined; + + // No /openswarm.json mapping and no explicit parent to inherit a project + // from → map this repo now (interactive) or bail rather than file an orphan + // (no project) issue. (INT-2239 / INT-2599) + const mapping = await ensureProjectMapping(cwd, explicitParentId, { log: (l) => console.log(status.info(l)) }); + if (mapping.abort) { + console.log(status.warn('Skipped filing follow-ups — map this repo to a Linear project first, then re-run.')); + return; + } + const projectId = mapping.projectId; + + let parentId: string | undefined = explicitParentId; if (!parentId && !opts.noLinear) { - parentId = (await createMasterAuditIssue(cwd, summary, report, ts)) ?? undefined; + parentId = (await createMasterAuditIssue(summary, report, ts, projectId)) ?? undefined; } console.log(`${status.running('PM synthesis')} ${c.yellow(`${actions.length} follow-up(s)`)} ${c.dim('into cohesive issues')}`);