Skip to content

Commit 532c500

Browse files
unoheeunohee
authored
fix(review): map or fail closed before filing Linear issues without a project (INT-2619) (#273)
review --max --fix (and review --issues) filed autonomous follow-up issues with no Linear project when the repo had no openswarm.json mapping, only logging a warning that's easy to miss in unattended runs. Confirmed via Linear: 15 orphan audit issues across vega-agent, OpenSwarm, enzyme, audioman, pykiwoom-rest, ecount-mcp, whisper-diarization, NDrum, and CompanyDataEngine (backfilled to their correct projects; NDrum and CompanyDataEngine had no project at all, so those were created). - ensureProjectMapping() in reviewCommand.ts: when no repo->project mapping exists, map it interactively (same picker as openswarm add) on a TTY, or fail closed headless instead of silently filing a project-less issue. - Wired into runReviewCommand (--issues) and all three reviewMaxCommand.tsx filing paths (filePmSynthesizedIssues / createMasterAuditIssue / filePerAreaFollowups). - resolveLinearCredential() now also checks config.yaml linear.apiKey, the same source ensureTaskSource() uses — closes a gap where the mapping preflight could wrongly conclude Linear wasn't configured and let filing proceed without a project. - no-teams from the mapping picker (team lookup failure) now aborts instead of silently proceeding, since it isn't a user choice to skip mapping. - openswarm.json added for this repo and vega-agent (the two repos without one); enzyme, whisper-diarization, NDrum, CompanyDataEngine also mapped. 179 -> 186 tests passing, tsc clean, oxlint clean.
1 parent 241cfab commit 532c500

7 files changed

Lines changed: 399 additions & 46 deletions

‎openswarm.json‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"schemaVersion": 1,
3+
"projectName": "OpenSwarm",
4+
"linear": {
5+
"teamId": "49b7af95-3cac-4a56-adc7-f19d77dfbe9b",
6+
"teamKey": "INT",
7+
"projectId": "74a9d092-7b3c-4d4d-a998-a2c9a8f08e83",
8+
"projectName": "OpenSwarm"
9+
}
10+
}

‎src/cli/linearMapping.test.ts‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
2+
3+
const getProfileMock = vi.fn();
4+
vi.mock('../auth/index.js', () => ({
5+
AuthProfileStore: vi.fn().mockImplementation(function AuthProfileStore(this: unknown) {
6+
return { getProfile: getProfileMock };
7+
}),
8+
ensureValidToken: vi.fn(),
9+
}));
10+
11+
const loadConfigMock = vi.fn();
12+
vi.mock('../core/config.js', () => ({
13+
loadConfig: (...args: unknown[]) => loadConfigMock(...args),
14+
}));
15+
16+
const { resolveLinearCredential } = await import('./linearMapping.js');
17+
18+
describe('resolveLinearCredential (INT-2619)', () => {
19+
const originalEnv = process.env.LINEAR_API_KEY;
20+
21+
beforeEach(() => {
22+
vi.clearAllMocks();
23+
getProfileMock.mockReturnValue(undefined);
24+
loadConfigMock.mockReturnValue({ linearApiKey: '' });
25+
delete process.env.LINEAR_API_KEY;
26+
});
27+
28+
afterEach(() => {
29+
if (originalEnv === undefined) delete process.env.LINEAR_API_KEY;
30+
else process.env.LINEAR_API_KEY = originalEnv;
31+
});
32+
33+
it('returns null when no OAuth profile, env var, or config apiKey is present', async () => {
34+
expect(await resolveLinearCredential()).toBeNull();
35+
});
36+
37+
it('falls back to LINEAR_API_KEY when no OAuth profile is stored', async () => {
38+
process.env.LINEAR_API_KEY = 'env-key';
39+
expect(await resolveLinearCredential()).toEqual({ apiKey: 'env-key' });
40+
});
41+
42+
it('falls back to config.yaml `linear.apiKey` when neither OAuth profile nor env var is present (INT-2619)', async () => {
43+
// This is the exact gap the reviewer caught: ensureTaskSource() initializes
44+
// Linear from config.linearApiKey too, so the mapping preflight must check
45+
// the same source or it wrongly concludes "Linear isn't configured" and lets
46+
// filing proceed without a project.
47+
loadConfigMock.mockReturnValue({ linearApiKey: 'config-key' });
48+
expect(await resolveLinearCredential()).toEqual({ apiKey: 'config-key' });
49+
});
50+
51+
it('prefers the OAuth profile over env var and config apiKey', async () => {
52+
getProfileMock.mockReturnValue({ provider: 'linear' });
53+
process.env.LINEAR_API_KEY = 'env-key';
54+
loadConfigMock.mockReturnValue({ linearApiKey: 'config-key' });
55+
const { ensureValidToken } = await import('../auth/index.js');
56+
vi.mocked(ensureValidToken).mockResolvedValue('oauth-token');
57+
expect(await resolveLinearCredential()).toEqual({ accessToken: 'oauth-token' });
58+
});
59+
});

‎src/cli/linearMapping.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@ import { AuthProfileStore, ensureValidToken } from '../auth/index.js';
1616
/**
1717
* Resolve a Linear credential for non-interactive use (no auth prompt):
1818
* 1. linear:default OAuth profile (refreshed via ensureValidToken), then
19-
* 2. LINEAR_API_KEY env var.
20-
* Returns null when neither is present — the caller should hint at `auth login`.
19+
* 2. LINEAR_API_KEY env var, then
20+
* 3. config.yaml `linear.apiKey` — the same source ensureTaskSource() uses, so
21+
* this stays in sync with what actually initializes Linear for filing. (INT-2619)
22+
* Returns null when none is present — the caller should hint at `auth login`.
2123
*/
2224
export async function resolveLinearCredential(): Promise<LinearCredential | null> {
2325
try {
@@ -31,6 +33,13 @@ export async function resolveLinearCredential(): Promise<LinearCredential | null
3133
}
3234
const apiKey = process.env.LINEAR_API_KEY?.trim();
3335
if (apiKey) return { apiKey };
36+
try {
37+
const { loadConfig } = await import('../core/config.js');
38+
const configApiKey = loadConfig().linearApiKey?.trim();
39+
if (configApiKey) return { apiKey: configApiKey };
40+
} catch { // cxt-ignore: error_swallow,exception_hiding — no usable config → no credential
41+
/* fall through to null */
42+
}
3443
return null;
3544
}
3645

‎src/cli/reviewCommand.test.ts‎

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
import { describe, it, expect, vi } from 'vitest';
2-
import { buildReviewWorkerResult, formatReviewOutput, runReviewCommand, resolveIssueFromBranch } from './reviewCommand.js';
2+
import {
3+
buildReviewWorkerResult,
4+
formatReviewOutput,
5+
runReviewCommand,
6+
resolveIssueFromBranch,
7+
ensureProjectMapping,
8+
} from './reviewCommand.js';
39
import type { ReviewResult } from '../agents/agentPair.js';
410

511
// Only exercised by tests that do NOT override deps.getChangedFiles — every
@@ -53,6 +59,73 @@ describe('resolveIssueFromBranch (INT-1967)', () => {
5359
});
5460
});
5561

62+
describe('ensureProjectMapping (INT-2599)', () => {
63+
// A path with no openswarm.json, so resolveProjectId(cwd) always misses and
64+
// each test's own stubs drive the rest of the decision tree.
65+
const unmappedCwd = '/tmp/openswarm-test-ensure-project-mapping-unmapped';
66+
67+
it('short-circuits without touching Linear when an explicit parent is given', async () => {
68+
const resolveCredential = vi.fn(async () => ({ apiKey: 'x' }));
69+
const result = await ensureProjectMapping(unmappedCwd, 'INT-1', { resolveCredential });
70+
expect(result).toEqual({ projectId: undefined, abort: false });
71+
expect(resolveCredential).not.toHaveBeenCalled();
72+
});
73+
74+
it('proceeds without a project when Linear is not configured at all', async () => {
75+
const result = await ensureProjectMapping(unmappedCwd, undefined, {
76+
resolveCredential: async () => null,
77+
});
78+
expect(result).toEqual({ projectId: undefined, abort: false });
79+
});
80+
81+
it('fails closed (no orphan issue) when unmapped and there is no terminal to prompt', async () => {
82+
const logs: string[] = [];
83+
const pickAndSave = vi.fn();
84+
const result = await ensureProjectMapping(unmappedCwd, undefined, {
85+
resolveCredential: async () => ({ apiKey: 'x' }),
86+
isTTY: false,
87+
pickAndSave,
88+
log: (l) => logs.push(l),
89+
});
90+
expect(result).toEqual({ projectId: undefined, abort: true });
91+
expect(pickAndSave).not.toHaveBeenCalled();
92+
expect(logs.join('\n')).toMatch(/openswarm add/);
93+
});
94+
95+
it('maps interactively on a TTY and returns the saved project id', async () => {
96+
const pickAndSave = vi.fn(async () => ({
97+
kind: 'saved' as const,
98+
teamId: 'team-1',
99+
mapping: { teamId: 'team-1', projectId: 'proj-1', projectName: 'Demo' },
100+
}));
101+
const result = await ensureProjectMapping(unmappedCwd, undefined, {
102+
resolveCredential: async () => ({ apiKey: 'x' }),
103+
isTTY: true,
104+
pickAndSave,
105+
});
106+
expect(result).toEqual({ projectId: 'proj-1', abort: false });
107+
expect(pickAndSave).toHaveBeenCalledWith(unmappedCwd, { apiKey: 'x' });
108+
});
109+
110+
it('fails closed when the Linear team lookup itself fails (no-teams is not a user choice) (INT-2619)', async () => {
111+
const result = await ensureProjectMapping(unmappedCwd, undefined, {
112+
resolveCredential: async () => ({ apiKey: 'x' }),
113+
isTTY: true,
114+
pickAndSave: async () => ({ kind: 'no-teams' as const }),
115+
});
116+
expect(result).toEqual({ projectId: undefined, abort: true });
117+
});
118+
119+
it('proceeds without a project when the user actively skips the interactive picker', async () => {
120+
const result = await ensureProjectMapping(unmappedCwd, undefined, {
121+
resolveCredential: async () => ({ apiKey: 'x' }),
122+
isTTY: true,
123+
pickAndSave: async () => ({ kind: 'skipped' as const }),
124+
});
125+
expect(result).toEqual({ projectId: undefined, abort: false });
126+
});
127+
});
128+
56129
describe('runReviewCommand --issues branch inference (INT-1967)', () => {
57130
const approveWithFollowups = async () =>
58131
({ decision: 'approve', feedback: 'ok', recommendedActions: [{ type: 'test', title: 't' }] }) as ReviewResult;
@@ -84,6 +157,7 @@ describe('runReviewCommand --issues branch inference (INT-1967)', () => {
84157
review: approveWithFollowups,
85158
getBranch: async () => 'main',
86159
fileFollowups: async () => 0, // e.g. Linear not configured
160+
ensureProjectMapping: async () => ({ projectId: undefined, abort: false }),
87161
startProgress: () => null,
88162
log: (l) => logs.push(l),
89163
},
@@ -112,6 +186,7 @@ describe('runReviewCommand --issues branch inference (INT-1967)', () => {
112186
review: approveWithFollowups,
113187
getBranch: async () => 'main',
114188
fileFollowups,
189+
ensureProjectMapping: async () => ({ projectId: undefined, abort: false }),
115190
startProgress: () => null,
116191
log: (l) => logs.push(l),
117192
},

‎src/cli/reviewCommand.ts‎

Lines changed: 94 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,71 @@ export async function resolveProjectId(cwd: string): Promise<string | undefined>
8888
}
8989
}
9090

91+
export interface ProjectMappingResult {
92+
/** Resolved Linear project id — undefined when none applies (explicit parent) or none is available. */
93+
projectId: string | undefined;
94+
/** True when Linear filing should be skipped entirely rather than create a project-less (orphan) issue. */
95+
abort: boolean;
96+
}
97+
98+
/**
99+
* Preflight before filing a standalone/master Linear issue (no explicit
100+
* `--issues <id>` parent to inherit a project from): if this repo has no
101+
* openswarm.json Linear mapping yet, map it now — interactively on a TTY
102+
* (writes openswarm.json, same picker as `openswarm add`), or fail closed
103+
* headless so an unattended run never silently creates an orphan issue. (INT-2599)
104+
*/
105+
export async function ensureProjectMapping(
106+
cwd: string,
107+
parentIssueId: string | undefined,
108+
deps: {
109+
resolveCredential?: () => Promise<import('../linear/index.js').LinearCredential | null>;
110+
pickAndSave?: (
111+
repoPath: string,
112+
cred: import('../linear/index.js').LinearCredential,
113+
) => Promise<import('./linearMapping.js').MappingPickResult>;
114+
isTTY?: boolean;
115+
log?: (line: string) => void;
116+
} = {},
117+
): Promise<ProjectMappingResult> {
118+
// A sub-issue under an explicit parent inherits that parent's project.
119+
if (parentIssueId) return { projectId: undefined, abort: false };
120+
121+
const existing = await resolveProjectId(cwd);
122+
if (existing) return { projectId: existing, abort: false };
123+
124+
const log = deps.log ?? ((l: string) => console.log(l));
125+
const resolveCredential =
126+
deps.resolveCredential ?? (async () => (await import('./linearMapping.js')).resolveLinearCredential());
127+
const cred = await resolveCredential();
128+
if (!cred) return { projectId: undefined, abort: false }; // Linear isn't configured — downstream reports that.
129+
130+
const isTTY = deps.isTTY ?? !!process.stdin.isTTY;
131+
if (!isTTY) {
132+
log(
133+
`No Linear project mapped for this repo (openswarm.json missing) and no terminal to map it interactively — ` +
134+
`skipping issue filing to avoid an unassigned (orphan) issue. Run \`openswarm add ${cwd}\` once in a ` +
135+
`terminal to map it, then re-run.`,
136+
);
137+
return { projectId: undefined, abort: true };
138+
}
139+
140+
log('This repo is not mapped to a Linear project yet — mapping it now:');
141+
const pickAndSave =
142+
deps.pickAndSave ?? (async (p, c) => (await import('./linearMapping.js')).pickAndSaveLinearMapping(p, c));
143+
try {
144+
const result = await pickAndSave(cwd, cred);
145+
if (result.kind === 'saved') return { projectId: result.mapping.projectId, abort: false };
146+
// 'no-teams' means the Linear lookup itself failed or returned nothing — not a
147+
// user choice — so fail closed rather than silently file a project-less issue.
148+
if (result.kind === 'no-teams') return { projectId: undefined, abort: true };
149+
return { projectId: undefined, abort: false }; // user actively skipped the picker — not a silent gap.
150+
} catch (err) {
151+
if (err instanceof Error && err.name === 'ExitPromptError') return { projectId: undefined, abort: true };
152+
throw err;
153+
}
154+
}
155+
91156
/**
92157
* A Linear-backed task source for the standalone `review` CLI. The daemon
93158
* registers one at startup, but a bare `openswarm review` does not — so init
@@ -151,6 +216,8 @@ export async function runReviewCommand(
151216
getChangedFiles?: (cwd: string) => Promise<string[]>;
152217
review?: (wr: WorkerResult, cwd: string, onLog?: (line: string) => void) => Promise<ReviewResult>;
153218
fileFollowups?: (parentIssueId: string | undefined, review: ReviewResult) => Promise<number>;
219+
/** Override the project-mapping preflight (tests stub this to skip the interactive picker). */
220+
ensureProjectMapping?: (cwd: string, parentIssueId: string | undefined) => Promise<ProjectMappingResult>;
154221
log?: (line: string) => void;
155222
/** Override the progress indicator (default: TTY-gated spinner). Tests pass a stub. */
156223
startProgress?: () => { note: (line: string) => void; stop: () => void } | null;
@@ -223,28 +290,34 @@ export async function runReviewCommand(
223290
if (parent) log(`Filing follow-ups under ${parent} (inferred from branch "${branch}").`);
224291
}
225292
// No parent → create top-level (standalone) issues rather than refusing. (INT-1968)
226-
// Default path initializes a Linear task source itself (the daemon isn't
227-
// running here), and files regardless of decision. (INT-1969)
228-
const fileFollowups =
229-
deps.fileFollowups ??
230-
(async (p: string | undefined, r: ReviewResult) => {
231-
const { fileReviewerFollowups } = await import('../automation/runnerExecution.js');
232-
const source = await ensureTaskSource();
233-
if (!source) return 0;
234-
const projectId = p ? undefined : await resolveProjectId(cwd);
235-
return fileReviewerFollowups(source, p, r, { autoFile: true, projectId, requireApprove: false });
236-
});
237-
const filed = await fileFollowups(parent, result);
238-
if (filed > 0) {
239-
log(
240-
parent
241-
? `Filed ${filed} follow-up sub-issue(s) under ${parent}.`
242-
: `Filed ${filed} standalone follow-up issue(s) (pass \`--issues <id>\` to nest them under an issue).`,
243-
);
293+
// Before that, make sure this repo actually has a Linear project to land
294+
// in — map it now (interactive) or bail rather than file an orphan. (INT-2599)
295+
const mapping = await (deps.ensureProjectMapping ?? (async (c, p) => ensureProjectMapping(c, p, { log })))(cwd, parent);
296+
if (mapping.abort) {
297+
log('Skipped filing follow-ups — map this repo to a Linear project first (see above), then re-run.');
244298
} else {
245-
log(
246-
`Could not file follow-ups (0 created). Is Linear connected? Run \`openswarm auth login --provider linear\` (or set linearApiKey in config).`,
247-
);
299+
// Default path initializes a Linear task source itself (the daemon isn't
300+
// running here), and files regardless of decision. (INT-1969)
301+
const fileFollowups =
302+
deps.fileFollowups ??
303+
(async (p: string | undefined, r: ReviewResult) => {
304+
const { fileReviewerFollowups } = await import('../automation/runnerExecution.js');
305+
const source = await ensureTaskSource();
306+
if (!source) return 0;
307+
return fileReviewerFollowups(source, p, r, { autoFile: true, projectId: mapping.projectId, requireApprove: false });
308+
});
309+
const filed = await fileFollowups(parent, result);
310+
if (filed > 0) {
311+
log(
312+
parent
313+
? `Filed ${filed} follow-up sub-issue(s) under ${parent}.`
314+
: `Filed ${filed} standalone follow-up issue(s) (pass \`--issues <id>\` to nest them under an issue).`,
315+
);
316+
} else {
317+
log(
318+
`Could not file follow-ups (0 created). Is Linear connected? Run \`openswarm auth login --provider linear\` (or set linearApiKey in config).`,
319+
);
320+
}
248321
}
249322
} else if (followups) {
250323
// Suggestions were made but nothing was filed — make the flag discoverable. (INT-1966/1967)

0 commit comments

Comments
 (0)