Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions src/agents/prompts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,12 +189,12 @@ export interface TaskPromptInput {
workItemId?: string;
prNumber?: number;
prBranch?: string;
// PM comment trigger fields
triggerCommentText?: string;
triggerCommentAuthor?: string;
// PR comment trigger fields
// Comment trigger fields (unified for PM and SCM comment-mention triggers)
triggerCommentBody?: string;
triggerCommentPath?: string;
triggerCommentAuthor?: string;
/** @deprecated Use triggerCommentBody. Kept as backward-compatible alias. */
triggerCommentText?: string;
// Allow extra fields for future extensibility
[key: string]: unknown;
}
Expand All @@ -210,9 +210,9 @@ export function buildTaskPromptContext(input: TaskPromptInput): TaskPromptContex
workItemId: input.workItemId,
prNumber: input.prNumber,
prBranch: input.prBranch,
commentText: input.triggerCommentText,
commentText: input.triggerCommentBody ?? input.triggerCommentText,
commentAuthor: input.triggerCommentAuthor,
commentBody: input.triggerCommentBody,
commentBody: input.triggerCommentBody ?? input.triggerCommentText,
commentPath: input.triggerCommentPath,
};
}
Expand Down
3 changes: 3 additions & 0 deletions src/router/acknowledgments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,15 @@ export async function deleteLinearAck(
// for backward compatibility with pm/ integrations and router/trello.ts.
// ---------------------------------------------------------------------------

export type { JiraBotIdentity, TrelloBotIdentity } from './bot-identity-resolvers.js';
export {
_resetJiraBotCache,
_resetLinearBotCache,
_resetTrelloBotCache,
resolveJiraBotAccountId,
resolveJiraBotIdentity,
resolveLinearBotUserId,
resolveTrelloBotIdentity,
resolveTrelloBotMemberId,
} from './bot-identity-resolvers.js';

Expand Down
52 changes: 42 additions & 10 deletions src/router/bot-identity-resolvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,18 @@ import {
// JIRA bot identity
// ---------------------------------------------------------------------------

const jiraBotIdentityCache = new BotIdentityCache<string>('accountId');
export interface JiraBotIdentity {
accountId: string;
displayName: string;
}

const jiraBotIdentityCache = new BotIdentityCache<JiraBotIdentity>('identity');

/**
* Resolve the JIRA account ID for the bot credentials linked to a project.
* Resolve the JIRA bot identity (accountId + displayName) for a project.
* Cached per-project with 60s TTL. Returns null on any failure.
*/
export async function resolveJiraBotAccountId(projectId: string): Promise<string | null> {
export async function resolveJiraBotIdentity(projectId: string): Promise<JiraBotIdentity | null> {
return jiraBotIdentityCache.resolve(projectId, async () => {
const creds = await resolveJiraCredentials(projectId);
if (!creds) return null;
Expand All @@ -35,11 +40,21 @@ export async function resolveJiraBotAccountId(projectId: string): Promise<string
});
if (!response.ok) return null;

const data = (await response.json()) as { accountId?: string };
return data.accountId ?? null;
const data = (await response.json()) as { accountId?: string; displayName?: string };
if (!data.accountId) return null;
return { accountId: data.accountId, displayName: data.displayName ?? '' };
});
}

/**
* Resolve the JIRA account ID for the bot credentials linked to a project.
* Cached per-project with 60s TTL. Returns null on any failure.
*/
export async function resolveJiraBotAccountId(projectId: string): Promise<string | null> {
const identity = await resolveJiraBotIdentity(projectId);
return identity?.accountId ?? null;
}

/** @internal Visible for testing only */
export function _resetJiraBotCache(): void {
jiraBotIdentityCache._reset();
Expand All @@ -49,13 +64,20 @@ export function _resetJiraBotCache(): void {
// Trello bot identity
// ---------------------------------------------------------------------------

const trelloBotIdentityCache = new BotIdentityCache<string>('memberId');
export interface TrelloBotIdentity {
id: string;
username: string;
}

const trelloBotIdentityCache = new BotIdentityCache<TrelloBotIdentity>('identity');

/**
* Resolve the Trello member ID for the bot credentials linked to a project.
* Resolve the Trello bot identity (id + username) for a project.
* Cached per-project with 60s TTL. Returns null on any failure.
*/
export async function resolveTrelloBotMemberId(projectId: string): Promise<string | null> {
export async function resolveTrelloBotIdentity(
projectId: string,
): Promise<TrelloBotIdentity | null> {
return trelloBotIdentityCache.resolve(projectId, async () => {
const creds = await resolveTrelloCredentials(projectId);
if (!creds) return null;
Expand All @@ -66,11 +88,21 @@ export async function resolveTrelloBotMemberId(projectId: string): Promise<strin
);
if (!response.ok) return null;

const data = (await response.json()) as { id?: string };
return data.id ?? null;
const data = (await response.json()) as { id?: string; username?: string };
if (!data.id) return null;
return { id: data.id, username: data.username ?? '' };
});
}

/**
* Resolve the Trello member ID for the bot credentials linked to a project.
* Cached per-project with 60s TTL. Returns null on any failure.
*/
export async function resolveTrelloBotMemberId(projectId: string): Promise<string | null> {
const identity = await resolveTrelloBotIdentity(projectId);
return identity?.id ?? null;
}

/** @internal Visible for testing only */
export function _resetTrelloBotCache(): void {
trelloBotIdentityCache._reset();
Expand Down
5 changes: 4 additions & 1 deletion src/triggers/github/check-suite-failure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { logger } from '../../utils/logging.js';
import { parseRepoFullName } from '../../utils/repo.js';
import { checkTriggerEnabled } from '../shared/trigger-check.js';
import { type GitHubCheckSuitePayload, isGitHubCheckSuitePayload } from './types.js';
import { parsePrNumberFromRef, resolveWorkItemId } from './utils.js';
import { parsePrNumberFromRef, resolveWorkItemDisplayData, resolveWorkItemId } from './utils.js';

/**
* Resolve a PR number from a check_suite payload.
Expand Down Expand Up @@ -121,6 +121,7 @@ export class CheckSuiteFailureTrigger implements TriggerHandler {

// Resolve work item from DB
const workItemId = await resolveWorkItemId(ctx.project.id, prNumber);
const { workItemUrl, workItemTitle } = await resolveWorkItemDisplayData(workItemId);

// Get ALL check runs for this commit to verify they're all complete
const checkStatus = await githubClient.getCheckSuiteStatus(owner, repo, headSha);
Expand Down Expand Up @@ -205,6 +206,8 @@ export class CheckSuiteFailureTrigger implements TriggerHandler {
prUrl: prDetails.htmlUrl,
prTitle: prDetails.title,
workItemId,
workItemUrl,
workItemTitle,
};
}
}
10 changes: 9 additions & 1 deletion src/triggers/github/check-suite-success.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ import {
releaseReviewDispatch,
} from './review-dispatch-dedup.js';
import { type GitHubCheckSuitePayload, isGitHubCheckSuitePayload } from './types.js';
import { evaluateAuthorMode, parsePrNumberFromRef, resolveWorkItemId } from './utils.js';
import {
evaluateAuthorMode,
parsePrNumberFromRef,
resolveWorkItemDisplayData,
resolveWorkItemId,
} from './utils.js';

const MAX_RETRIES = 12;
const RETRY_DELAY_MS = 10_000;
Expand Down Expand Up @@ -156,6 +161,7 @@ export class CheckSuiteSuccessTrigger implements TriggerHandler {

// Resolve work item from DB
const workItemId = await resolveWorkItemId(ctx.project.id, prNumber);
const { workItemUrl, workItemTitle } = await resolveWorkItemDisplayData(workItemId);

// Skip if the reviewer persona's latest review already covers the current HEAD SHA
const reviews = await githubClient.getPRReviews(owner, repo, prNumber);
Expand Down Expand Up @@ -224,6 +230,8 @@ export class CheckSuiteSuccessTrigger implements TriggerHandler {
prUrl: prDetails.htmlUrl,
prTitle: prDetails.title,
workItemId,
workItemUrl,
workItemTitle,
waitForChecks: true,
onBlocked: () => releaseReviewDispatch(dedupKey),
};
Expand Down
5 changes: 4 additions & 1 deletion src/triggers/github/pr-comment-mention.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { logger } from '../../utils/logging.js';
import { parseRepoFullName } from '../../utils/repo.js';
import { checkTriggerEnabled } from '../shared/trigger-check.js';
import { isGitHubIssueCommentPayload, isGitHubPRReviewCommentPayload } from './types.js';
import { resolveWorkItemId } from './utils.js';
import { resolveWorkItemDisplayData, resolveWorkItemId } from './utils.js';

/**
* Trigger that fires when someone @mentions the reviewer bot in a PR comment.
Expand Down Expand Up @@ -118,6 +118,7 @@ export class PRCommentMentionTrigger implements TriggerHandler {

// Resolve work item from DB
const workItemId = await resolveWorkItemId(ctx.project.id, prNumber);
const { workItemUrl, workItemTitle } = await resolveWorkItemDisplayData(workItemId);

logger.info('PR comment @mention detected, triggering respond-to-pr-comment agent', {
prNumber,
Expand All @@ -144,6 +145,8 @@ export class PRCommentMentionTrigger implements TriggerHandler {
prUrl,
prTitle,
workItemId,
workItemUrl,
workItemTitle,
};
}
}
5 changes: 4 additions & 1 deletion src/triggers/github/pr-review-submitted.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { TriggerContext, TriggerHandler, TriggerResult } from '../../types/
import { logger } from '../../utils/logging.js';
import { checkTriggerEnabled } from '../shared/trigger-check.js';
import { type GitHubPullRequestReviewPayload, isGitHubPullRequestReviewPayload } from './types.js';
import { resolveWorkItemId } from './utils.js';
import { resolveWorkItemDisplayData, resolveWorkItemId } from './utils.js';

export class PRReviewSubmittedTrigger implements TriggerHandler {
name = 'pr-review-submitted';
Expand Down Expand Up @@ -59,6 +59,7 @@ export class PRReviewSubmittedTrigger implements TriggerHandler {

// Resolve work item from DB
const workItemId = await resolveWorkItemId(ctx.project.id, prNumber);
const { workItemUrl, workItemTitle } = await resolveWorkItemDisplayData(workItemId);

logger.info('PR review submitted, triggering review agent', {
prNumber,
Expand All @@ -83,6 +84,8 @@ export class PRReviewSubmittedTrigger implements TriggerHandler {
prUrl: reviewPayload.pull_request.html_url,
prTitle: reviewPayload.pull_request.title,
workItemId,
workItemUrl,
workItemTitle,
};
}
}
31 changes: 31 additions & 0 deletions src/triggers/github/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { lookupWorkItemForPR } from '../../db/repositories/prWorkItemsRepository.js';
import type { PersonaIdentities } from '../../github/personas.js';
import { getPMProviderOrNull } from '../../pm/context.js';
import type { ProjectConfig } from '../../types/index.js';
import { logger } from '../../utils/logging.js';

Expand Down Expand Up @@ -129,3 +130,33 @@ export async function resolveWorkItemId(

return undefined;
}

/**
* Fetch work item display data (URL and title) from the active PM provider.
*
* Best-effort: returns an empty object on any error so callers can safely
* spread the result without checking for failure. Requires a PM provider
* to be in scope (set up by `withPMScope`).
*
* @param workItemId - The work item ID to look up (Trello card ID, JIRA issue key, etc.)
*/
export async function resolveWorkItemDisplayData(
workItemId: string | undefined,
): Promise<{ workItemUrl?: string; workItemTitle?: string }> {
if (!workItemId) return {};
try {
const provider = getPMProviderOrNull();
if (!provider) return {};
const workItem = await provider.getWorkItem(workItemId);
return {
workItemUrl: workItem.url ?? undefined,
workItemTitle: workItem.title ?? undefined,
};
} catch (err) {
logger.debug('Could not resolve work item display data (best-effort)', {
workItemId,
error: String(err),
});
return {};
}
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SHOULD_FIX] This new utility has no unit tests. It handles three distinct branches (no workItemId, no PM provider in scope, API error) plus the happy path. Since it's invoked by 4 trigger handlers and does an API call, basic test coverage would catch regressions.

Suggested test scenarios:

  1. Returns {} when workItemId is undefined
  2. Returns {} when getPMProviderOrNull() returns null
  3. Returns { workItemUrl, workItemTitle } when provider returns a valid work item
  4. Returns {} and logs debug on provider error

Loading
Loading