Skip to content

Commit 2250748

Browse files
author
bgagent
committed
fix(jira): validate repo onboarding at map time (#369)
bgagent jira map (and linear onboard-project) accepted and persisted a project->repo mapping for a repo with no active Blueprint. Every label trigger then failed at task creation with 422 REPO_NOT_ONBOARDED, with no visible operator feedback. Add a shared checkRepoOnboarding helper (cli/src/repo-onboarding.ts) that reads the RepoTable via the RepoTableName stack output and reports whether the repo has a status='active' row -- mirroring the runtime gate in repo-config.ts. Both map commands now refuse (with actionable guidance) to persist a mapping for a non-onboarded repo, unless --skip-onboarding-check is passed. An inconclusive check (missing output / IAM gap) warns and proceeds rather than blocking a valid mapping. Linear's onboard-project gets the same guard since it shares the create-task-core onboarding gate. Docs: JIRA_SETUP_GUIDE.md and LINEAR_SETUP_GUIDE.md state the Blueprint-onboarding prerequisite and document --skip-onboarding-check; Starlight mirrors regenerated. Tests: new repo-onboarding.test.ts plus map-guard cases in jira/linear command tests (423 pass).
1 parent 0e2806a commit 2250748

10 files changed

Lines changed: 494 additions & 8 deletions

File tree

cli/src/commands/jira.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import {
4444
StoredJiraOauthToken,
4545
} from '../jira-oauth';
4646
import { awaitOauthCallback, CALLBACK_URL } from '../oauth-callback-server';
47+
import { checkRepoOnboarding, notOnboardedGuidance } from '../repo-onboarding';
4748

4849
/** Default label that triggers an ABCA task when applied to a Jira issue. */
4950
const DEFAULT_LABEL_FILTER = 'bgagent';
@@ -652,6 +653,7 @@ export function makeJiraCommand(): Command {
652653
.option('--label <label>', `Label that triggers a task (default: ${DEFAULT_LABEL_FILTER})`, DEFAULT_LABEL_FILTER)
653654
.option('--region <region>', 'AWS region (defaults to configured region)')
654655
.option('--stack-name <name>', 'CloudFormation stack name', 'backgroundagent-dev')
656+
.option('--skip-onboarding-check', 'Persist the mapping even if the repo has no active Blueprint (the mapping cannot trigger until one is deployed)')
655657
.action(async (cloudId: string, projectKey: string, opts) => {
656658
const config = loadConfig();
657659
const region = opts.region || config.region;
@@ -673,6 +675,23 @@ export function makeJiraCommand(): Command {
673675
process.exit(1);
674676
}
675677

678+
// Onboarding gate: refuse to persist a mapping for a repo that has
679+
// no active Blueprint, since every label trigger would fail at
680+
// task-creation with 422 REPO_NOT_ONBOARDED — and the failure is
681+
// nearly invisible to the operator. An inconclusive check (no
682+
// RepoTable output, IAM gap) warns and proceeds rather than blocks.
683+
if (!opts.skipOnboardingCheck) {
684+
const onboarding = await checkRepoOnboarding({ region, stackName: opts.stackName, repo: opts.repo });
685+
if (onboarding.kind === 'not-onboarded') {
686+
for (const line of notOnboardedGuidance(opts.repo, onboarding)) {
687+
console.error(line);
688+
}
689+
process.exit(1);
690+
} else if (onboarding.kind === 'unverifiable') {
691+
console.error(`⚠ Could not verify repo onboarding (${onboarding.detail}); proceeding without the check.`);
692+
}
693+
}
694+
676695
const now = new Date().toISOString();
677696
const ddb = DynamoDBDocumentClient.from(new DynamoDBClient({ region }));
678697
await ddb.send(new PutCommand({

cli/src/commands/linear.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import {
4545
StoredLinearOauthToken,
4646
} from '../linear-oauth';
4747
import { awaitOauthCallback, CALLBACK_URL } from '../oauth-callback-server';
48+
import { checkRepoOnboarding, notOnboardedGuidance } from '../repo-onboarding';
4849

4950
/** Default label that triggers an ABCA task when applied to a Linear issue. */
5051
const DEFAULT_LABEL_FILTER = 'bgagent';
@@ -1314,6 +1315,7 @@ export function makeLinearCommand(): Command {
13141315
.option('--team-id <id>', 'Optional Linear team UUID for the project (stored for debug)')
13151316
.option('--region <region>', 'AWS region (defaults to configured region)')
13161317
.option('--stack-name <name>', 'CloudFormation stack name', 'backgroundagent-dev')
1318+
.option('--skip-onboarding-check', 'Persist the mapping even if the repo has no active Blueprint (the mapping cannot trigger until one is deployed)')
13171319
.action(async (projectId: string, opts) => {
13181320
const config = loadConfig();
13191321
const region = opts.region || config.region;
@@ -1341,6 +1343,23 @@ export function makeLinearCommand(): Command {
13411343
process.exit(1);
13421344
}
13431345

1346+
// Onboarding gate: refuse to persist a mapping for a repo that has
1347+
// no active Blueprint, since every label trigger would fail at
1348+
// task-creation with 422 REPO_NOT_ONBOARDED (Linear shares the same
1349+
// create-task-core gate as Jira). An inconclusive check warns and
1350+
// proceeds rather than blocks.
1351+
if (!opts.skipOnboardingCheck) {
1352+
const onboarding = await checkRepoOnboarding({ region, stackName: opts.stackName, repo: opts.repo });
1353+
if (onboarding.kind === 'not-onboarded') {
1354+
for (const line of notOnboardedGuidance(opts.repo, onboarding)) {
1355+
console.error(line);
1356+
}
1357+
process.exit(1);
1358+
} else if (onboarding.kind === 'unverifiable') {
1359+
console.error(`⚠ Could not verify repo onboarding (${onboarding.detail}); proceeding without the check.`);
1360+
}
1361+
}
1362+
13441363
const now = new Date().toISOString();
13451364
const ddb = DynamoDBDocumentClient.from(new DynamoDBClient({ region }));
13461365
await ddb.send(new PutCommand({

cli/src/repo-onboarding.ts

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
/**
2+
* MIT No Attribution
3+
*
4+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
5+
*
6+
* Permission is hereby granted, free of charge, to any person obtaining a copy of
7+
* the Software without restriction, including without limitation the rights to
8+
* use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
9+
* the Software, and to permit persons to whom the Software is furnished to do so.
10+
*
11+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
12+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
13+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
14+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
15+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
16+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
17+
* SOFTWARE.
18+
*/
19+
20+
import { CloudFormationClient, DescribeStacksCommand } from '@aws-sdk/client-cloudformation';
21+
import { DynamoDBClient } from '@aws-sdk/client-dynamodb';
22+
import { DynamoDBDocumentClient, GetCommand } from '@aws-sdk/lib-dynamodb';
23+
24+
/**
25+
* Stack-output key that exposes the name of the RepoTable — the DynamoDB
26+
* table the Blueprint construct writes a `status='active'` row into for
27+
* every onboarded repository. Mirrors the `CfnOutput` id in
28+
* `cdk/src/stacks/agent.ts`.
29+
*/
30+
export const REPO_TABLE_OUTPUT_KEY = 'RepoTableName';
31+
32+
/**
33+
* Result of checking whether a repo has a deployed Blueprint (i.e. an
34+
* active RepoTable row). Mirrors the runtime onboarding gate in
35+
* `cdk/src/handlers/shared/repo-config.ts` (`lookupRepo`):
36+
*
37+
* - `onboarded` — an `active` row exists; the repo will trigger.
38+
* - `not-onboarded` (`missing`) — no row at all for this `owner/repo`.
39+
* - `not-onboarded` (`inactive`) — a row exists but `status != 'active'`
40+
* (e.g. soft-removed); the runtime gate treats this as not onboarded.
41+
* - `unverifiable` — the check could not run (RepoTable output absent, or
42+
* an IAM / read error). The caller should warn and proceed rather than
43+
* block on an inconclusive signal, since the misconfiguration is the
44+
* check's, not the mapping's.
45+
*/
46+
export type RepoOnboardingResult =
47+
| { readonly kind: 'onboarded' }
48+
| { readonly kind: 'not-onboarded'; readonly reason: 'missing' | 'inactive'; readonly status?: string }
49+
| { readonly kind: 'unverifiable'; readonly detail: string };
50+
51+
export interface CheckRepoOnboardingOptions {
52+
readonly region: string;
53+
readonly stackName: string;
54+
/** The `owner/repo` string the operator is about to map. */
55+
readonly repo: string;
56+
}
57+
58+
/**
59+
* Read a single stack output. Returns null when the stack or the output
60+
* does not exist, so callers can distinguish "not deployed" from a real
61+
* AWS error (which is rethrown).
62+
*/
63+
async function getStackOutput(region: string, stackName: string, outputKey: string): Promise<string | null> {
64+
const cfn = new CloudFormationClient({ region });
65+
try {
66+
const result = await cfn.send(new DescribeStacksCommand({ StackName: stackName }));
67+
const output = (result.Stacks?.[0]?.Outputs ?? []).find((o) => o.OutputKey === outputKey);
68+
return output?.OutputValue ?? null;
69+
} catch (err) {
70+
const name = (err as Error)?.name ?? '';
71+
const message = (err as Error)?.message ?? '';
72+
if (name === 'ValidationError' && /does not exist/i.test(message)) {
73+
return null;
74+
}
75+
throw err;
76+
}
77+
}
78+
79+
/**
80+
* Check whether `opts.repo` is onboarded — i.e. has an `active` row in the
81+
* deployed RepoTable, the same condition the task-submit gate enforces at
82+
* trigger time (`422 REPO_NOT_ONBOARDED`). Run this at `map`/`onboard`
83+
* time so an operator learns immediately that a mapping can never fire,
84+
* rather than discovering it deep in the processor when a label is added.
85+
*
86+
* Never throws for the "not onboarded" case — that is a normal verdict the
87+
* caller turns into actionable guidance. A genuinely inconclusive check
88+
* (missing output, IAM gap) returns `unverifiable` with detail so the
89+
* caller can warn-and-proceed instead of falsely blocking a valid mapping.
90+
*/
91+
export async function checkRepoOnboarding(opts: CheckRepoOnboardingOptions): Promise<RepoOnboardingResult> {
92+
let repoTableName: string | null;
93+
try {
94+
repoTableName = await getStackOutput(opts.region, opts.stackName, REPO_TABLE_OUTPUT_KEY);
95+
} catch (err) {
96+
return { kind: 'unverifiable', detail: `could not read stack outputs: ${err instanceof Error ? err.message : String(err)}` };
97+
}
98+
if (!repoTableName) {
99+
return {
100+
kind: 'unverifiable',
101+
detail: `stack '${opts.stackName}' has no ${REPO_TABLE_OUTPUT_KEY} output (deploy the latest CDK stack to enable the onboarding check)`,
102+
};
103+
}
104+
105+
const ddb = DynamoDBDocumentClient.from(new DynamoDBClient({ region: opts.region }));
106+
let item: Record<string, unknown> | undefined;
107+
try {
108+
const result = await ddb.send(new GetCommand({
109+
TableName: repoTableName,
110+
Key: { repo: opts.repo },
111+
}));
112+
item = result.Item;
113+
} catch (err) {
114+
return { kind: 'unverifiable', detail: `could not read RepoTable '${repoTableName}': ${err instanceof Error ? err.message : String(err)}` };
115+
}
116+
117+
if (!item) {
118+
return { kind: 'not-onboarded', reason: 'missing' };
119+
}
120+
const status = typeof item.status === 'string' ? item.status : undefined;
121+
if (status !== 'active') {
122+
return { kind: 'not-onboarded', reason: 'inactive', status };
123+
}
124+
return { kind: 'onboarded' };
125+
}
126+
127+
/**
128+
* Build the multi-line operator guidance printed when a repo is not
129+
* onboarded. Shared by the Jira and Linear map commands so the remediation
130+
* steps stay identical. `repo` is the offending `owner/repo`.
131+
*/
132+
export function notOnboardedGuidance(repo: string, result: Extract<RepoOnboardingResult, { kind: 'not-onboarded' }>): string[] {
133+
const lead = result.reason === 'inactive'
134+
? `Repository '${repo}' has a RepoTable row but its status is '${result.status ?? 'unknown'}' (not 'active'), so the task-submit gate will reject every trigger with 422 REPO_NOT_ONBOARDED.`
135+
: `Repository '${repo}' is not onboarded — it has no active Blueprint, so the task-submit gate will reject every trigger with 422 REPO_NOT_ONBOARDED.`;
136+
return [
137+
lead,
138+
'',
139+
'Onboard the repo first by deploying a Blueprint for it, then re-run this command:',
140+
'',
141+
' 1. Add (or point) a Blueprint construct at this repo in cdk/src/stacks/agent.ts,',
142+
' e.g. set BLUEPRINT_REPO / the `blueprintRepo` CDK context, or instantiate',
143+
` new Blueprint(this, 'MyRepoBlueprint', { repo: '${repo}', repoTable: repoTable.table });`,
144+
' 2. Deploy: MISE_EXPERIMENTAL=1 mise //cdk:deploy',
145+
' 3. Re-run this map command.',
146+
'',
147+
'See docs/guides/QUICK_START.md (“Onboard a repository / Blueprint”) for the full steps.',
148+
'To map anyway (e.g. you are deploying the Blueprint momentarily), pass --skip-onboarding-check.',
149+
];
150+
}

cli/test/commands/jira.test.ts

Lines changed: 65 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
PutSecretValueCommand,
2323
ResourceExistsException,
2424
} from '@aws-sdk/client-secrets-manager';
25+
import { GetCommand } from '@aws-sdk/lib-dynamodb';
2526
import { ApiClient } from '../../src/api-client';
2627
import {
2728
isWebhookSecretConfigured,
@@ -365,9 +366,22 @@ describe('jira map action', () => {
365366
let exitSpy: jest.SpiedFunction<typeof process.exit>;
366367

367368
beforeEach(() => {
368-
ddbSend.mockReset().mockResolvedValue({});
369+
// The `map` action issues a GetItem (onboarding check) then a PutItem
370+
// (the mapping). Differentiate by command type: GetItem → an active
371+
// RepoTable row (onboarded), PutItem → ack.
372+
ddbSend.mockReset().mockImplementation((cmd: unknown) => {
373+
if (cmd instanceof GetCommand) {
374+
return Promise.resolve({ Item: { repo: 'owner/repo', status: 'active' } });
375+
}
376+
return Promise.resolve({});
377+
});
369378
cfnSend.mockReset().mockResolvedValue({
370-
Stacks: [{ Outputs: [{ OutputKey: 'JiraProjectMappingTableName', OutputValue: 'ProjMapTable' }] }],
379+
Stacks: [{
380+
Outputs: [
381+
{ OutputKey: 'JiraProjectMappingTableName', OutputValue: 'ProjMapTable' },
382+
{ OutputKey: 'RepoTableName', OutputValue: 'RepoTable' },
383+
],
384+
}],
371385
});
372386
loadConfigSpy = jest.spyOn(config, 'loadConfig').mockReturnValue({ region: 'us-west-2' } as ReturnType<typeof config.loadConfig>);
373387
consoleLogSpy = jest.spyOn(console, 'log').mockImplementation();
@@ -390,10 +404,16 @@ describe('jira map action', () => {
390404
await program.parseAsync(['node', 'bgagent', 'map', ...args]);
391405
}
392406

407+
/** The PutCommand input from the mapping write (skips the GetItem check). */
408+
function putMappingInput(): { TableName: string; Item: Record<string, unknown> } {
409+
const call = ddbSend.mock.calls.find((c) => !(c[0] instanceof GetCommand));
410+
if (!call) throw new Error('no PutCommand was issued');
411+
return call[0].input;
412+
}
413+
393414
test('writes an active mapping row with the resolved label on the happy path', async () => {
394415
await runMap(['cloud-123', 'ENG', '--repo', 'owner/repo', '--label', 'agentme']);
395-
expect(ddbSend).toHaveBeenCalledTimes(1);
396-
const putInput = ddbSend.mock.calls[0][0].input;
416+
const putInput = putMappingInput();
397417
expect(putInput.TableName).toBe('ProjMapTable');
398418
expect(putInput.Item).toMatchObject({
399419
jira_project_identity: 'cloud-123#ENG',
@@ -407,7 +427,7 @@ describe('jira map action', () => {
407427

408428
test('defaults the trigger label to bgagent when --label is omitted', async () => {
409429
await runMap(['cloud-123', 'ENG', '--repo', 'owner/repo']);
410-
expect(ddbSend.mock.calls[0][0].input.Item.label_filter).toBe('bgagent');
430+
expect(putMappingInput().Item.label_filter).toBe('bgagent');
411431
});
412432

413433
test('rejects an invalid --repo value before writing', async () => {
@@ -425,6 +445,46 @@ describe('jira map action', () => {
425445
await expect(runMap(['cloud-123', 'ENG', '--repo', 'owner/repo'])).rejects.toThrow('process.exit:1');
426446
expect(ddbSend).not.toHaveBeenCalled();
427447
});
448+
449+
test('rejects a repo with no RepoTable row (not onboarded) before writing the mapping', async () => {
450+
ddbSend.mockImplementation((cmd: unknown) => {
451+
if (cmd instanceof GetCommand) return Promise.resolve({}); // no Item
452+
return Promise.resolve({});
453+
});
454+
await expect(runMap(['cloud-123', 'ENG', '--repo', 'owner/repo'])).rejects.toThrow('process.exit:1');
455+
// GetItem ran; no PutItem.
456+
expect(ddbSend).toHaveBeenCalledTimes(1);
457+
expect(ddbSend.mock.calls[0][0]).toBeInstanceOf(GetCommand);
458+
expect(consoleErrorSpy.mock.calls.flat().join('\n')).toContain('REPO_NOT_ONBOARDED');
459+
});
460+
461+
test('rejects a repo whose RepoTable row is not active (soft-removed)', async () => {
462+
ddbSend.mockImplementation((cmd: unknown) => {
463+
if (cmd instanceof GetCommand) return Promise.resolve({ Item: { repo: 'owner/repo', status: 'removed' } });
464+
return Promise.resolve({});
465+
});
466+
await expect(runMap(['cloud-123', 'ENG', '--repo', 'owner/repo'])).rejects.toThrow('process.exit:1');
467+
expect(consoleErrorSpy.mock.calls.flat().join('\n')).toContain("status is 'removed'");
468+
});
469+
470+
test('--skip-onboarding-check persists the mapping without a RepoTable read', async () => {
471+
ddbSend.mockImplementation((cmd: unknown) => {
472+
if (cmd instanceof GetCommand) throw new Error('onboarding check should be skipped');
473+
return Promise.resolve({});
474+
});
475+
await runMap(['cloud-123', 'ENG', '--repo', 'owner/repo', '--skip-onboarding-check']);
476+
expect(ddbSend.mock.calls.some((c) => c[0] instanceof GetCommand)).toBe(false);
477+
expect(putMappingInput().Item).toMatchObject({ repo: 'owner/repo', status: 'active' });
478+
});
479+
480+
test('warns and proceeds when onboarding cannot be verified (no RepoTable output)', async () => {
481+
cfnSend.mockResolvedValue({
482+
Stacks: [{ Outputs: [{ OutputKey: 'JiraProjectMappingTableName', OutputValue: 'ProjMapTable' }] }],
483+
});
484+
await runMap(['cloud-123', 'ENG', '--repo', 'owner/repo']);
485+
expect(consoleErrorSpy.mock.calls.flat().join('\n')).toContain('Could not verify repo onboarding');
486+
expect(putMappingInput().Item).toMatchObject({ repo: 'owner/repo' });
487+
});
428488
});
429489

430490
describe('renderJiraAppTemplate', () => {

0 commit comments

Comments
 (0)