diff --git a/src/create-prompt/index.ts b/src/create-prompt/index.ts index 884e36b..f5efeba 100644 --- a/src/create-prompt/index.ts +++ b/src/create-prompt/index.ts @@ -125,10 +125,8 @@ export function prepareContext( const isPR = context.isPR; // Get PR/Issue number from entityNumber - const prNumber = - isPR && context.entityNumber ? context.entityNumber.toString() : undefined; - const issueNumber = - !isPR && context.entityNumber ? context.entityNumber.toString() : undefined; + const prNumber = isPR ? context.entityNumber.toString() : undefined; + const issueNumber = !isPR ? context.entityNumber.toString() : undefined; // Extract trigger username and comment data based on event type let triggerUsername: string | undefined; diff --git a/src/entrypoints/prepare.ts b/src/entrypoints/prepare.ts index 0523ff1..6120de8 100644 --- a/src/entrypoints/prepare.ts +++ b/src/entrypoints/prepare.ts @@ -9,7 +9,7 @@ import * as core from "@actions/core"; import { setupGitHubToken } from "../github/token"; import { checkWritePermissions } from "../github/validation/permissions"; import { createOctokit } from "../github/api/client"; -import { parseGitHubContext } from "../github/context"; +import { parseGitHubContext, isEntityContext } from "../github/context"; import { getMode } from "../modes/registry"; import { prepare } from "../prepare"; @@ -22,15 +22,17 @@ async function run() { // Step 2: Parse GitHub context (once for all operations) const context = parseGitHubContext(); - // Step 3: Check write permissions - const hasWritePermissions = await checkWritePermissions( - octokit.rest, - context, - ); - if (!hasWritePermissions) { - throw new Error( - "Actor does not have write permissions to the repository", + // Step 3: Check write permissions (only for entity contexts) + if (isEntityContext(context)) { + const hasWritePermissions = await checkWritePermissions( + octokit.rest, + context, ); + if (!hasWritePermissions) { + throw new Error( + "Actor does not have write permissions to the repository", + ); + } } // Step 4: Get mode and check trigger conditions diff --git a/src/entrypoints/update-comment-link.ts b/src/entrypoints/update-comment-link.ts index 6586931..3a14e66 100644 --- a/src/entrypoints/update-comment-link.ts +++ b/src/entrypoints/update-comment-link.ts @@ -9,6 +9,7 @@ import { import { parseGitHubContext, isPullRequestReviewCommentEvent, + isEntityContext, } from "../github/context"; import { GITHUB_SERVER_URL } from "../github/api/config"; import { checkAndCommitOrDeleteBranch } from "../github/operations/branch-cleanup"; @@ -23,13 +24,13 @@ async function run() { const triggerUsername = process.env.TRIGGER_USERNAME; const context = parseGitHubContext(); - const { owner, repo } = context.repository; // This script is only called for entity-based events - if (!context.entityNumber) { - throw new Error("update-comment-link requires an entity number"); + if (!isEntityContext(context)) { + throw new Error("update-comment-link requires an entity context"); } - const entityNumber = context.entityNumber; + + const { owner, repo } = context.repository; const octokit = createOctokit(githubToken); @@ -80,7 +81,7 @@ async function run() { const { data: pr } = await octokit.rest.pulls.get({ owner, repo, - pull_number: entityNumber, + pull_number: context.entityNumber, }); console.log(`PR state: ${pr.state}`); console.log(`PR comments count: ${pr.comments}`); diff --git a/src/github/context.ts b/src/github/context.ts index c1cf5ef..58ae761 100644 --- a/src/github/context.ts +++ b/src/github/context.ts @@ -37,9 +37,24 @@ export type ScheduleEvent = { import type { ModeName } from "../modes/types"; import { DEFAULT_MODE, isValidMode } from "../modes/registry"; -export type ParsedGitHubContext = { +// Event name constants for better maintainability +const ENTITY_EVENT_NAMES = [ + "issues", + "issue_comment", + "pull_request", + "pull_request_review", + "pull_request_review_comment", +] as const; + +const AUTOMATION_EVENT_NAMES = ["workflow_dispatch", "schedule"] as const; + +// Derive types from constants for better maintainability +type EntityEventName = (typeof ENTITY_EVENT_NAMES)[number]; +type AutomationEventName = (typeof AUTOMATION_EVENT_NAMES)[number]; + +// Common fields shared by all context types +type BaseContext = { runId: string; - eventName: string; eventAction?: string; repository: { owner: string; @@ -47,16 +62,6 @@ export type ParsedGitHubContext = { full_name: string; }; actor: string; - payload: - | IssuesEvent - | IssueCommentEvent - | PullRequestEvent - | PullRequestReviewEvent - | PullRequestReviewCommentEvent - | WorkflowDispatchEvent - | ScheduleEvent; - entityNumber?: number; - isPR?: boolean; inputs: { mode: ModeName; triggerPhrase: string; @@ -75,7 +80,29 @@ export type ParsedGitHubContext = { }; }; -export function parseGitHubContext(): ParsedGitHubContext { +// Context for entity-based events (issues, PRs, comments) +export type ParsedGitHubContext = BaseContext & { + eventName: EntityEventName; + payload: + | IssuesEvent + | IssueCommentEvent + | PullRequestEvent + | PullRequestReviewEvent + | PullRequestReviewCommentEvent; + entityNumber: number; + isPR: boolean; +}; + +// Context for automation events (workflow_dispatch, schedule) +export type AutomationContext = BaseContext & { + eventName: AutomationEventName; + payload: WorkflowDispatchEvent | ScheduleEvent; +}; + +// Union type for all contexts +export type GitHubContext = ParsedGitHubContext | AutomationContext; + +export function parseGitHubContext(): GitHubContext { const context = github.context; const modeInput = process.env.MODE ?? DEFAULT_MODE; @@ -85,7 +112,6 @@ export function parseGitHubContext(): ParsedGitHubContext { const commonFields = { runId: process.env.GITHUB_RUN_ID!, - eventName: context.eventName, eventAction: context.payload.action, repository: { owner: context.repo.owner, @@ -115,61 +141,67 @@ export function parseGitHubContext(): ParsedGitHubContext { switch (context.eventName) { case "issues": { + const payload = context.payload as IssuesEvent; return { ...commonFields, - payload: context.payload as IssuesEvent, - entityNumber: (context.payload as IssuesEvent).issue.number, + eventName: "issues", + payload, + entityNumber: payload.issue.number, isPR: false, }; } case "issue_comment": { + const payload = context.payload as IssueCommentEvent; return { ...commonFields, - payload: context.payload as IssueCommentEvent, - entityNumber: (context.payload as IssueCommentEvent).issue.number, - isPR: Boolean( - (context.payload as IssueCommentEvent).issue.pull_request, - ), + eventName: "issue_comment", + payload, + entityNumber: payload.issue.number, + isPR: Boolean(payload.issue.pull_request), }; } case "pull_request": { + const payload = context.payload as PullRequestEvent; return { ...commonFields, - payload: context.payload as PullRequestEvent, - entityNumber: (context.payload as PullRequestEvent).pull_request.number, + eventName: "pull_request", + payload, + entityNumber: payload.pull_request.number, isPR: true, }; } case "pull_request_review": { + const payload = context.payload as PullRequestReviewEvent; return { ...commonFields, - payload: context.payload as PullRequestReviewEvent, - entityNumber: (context.payload as PullRequestReviewEvent).pull_request - .number, + eventName: "pull_request_review", + payload, + entityNumber: payload.pull_request.number, isPR: true, }; } case "pull_request_review_comment": { + const payload = context.payload as PullRequestReviewCommentEvent; return { ...commonFields, - payload: context.payload as PullRequestReviewCommentEvent, - entityNumber: (context.payload as PullRequestReviewCommentEvent) - .pull_request.number, + eventName: "pull_request_review_comment", + payload, + entityNumber: payload.pull_request.number, isPR: true, }; } case "workflow_dispatch": { return { ...commonFields, + eventName: "workflow_dispatch", payload: context.payload as unknown as WorkflowDispatchEvent, - // No entityNumber or isPR for workflow_dispatch }; } case "schedule": { return { ...commonFields, + eventName: "schedule", payload: context.payload as unknown as ScheduleEvent, - // No entityNumber or isPR for schedule }; } default: @@ -205,37 +237,53 @@ export function parseAdditionalPermissions(s: string): Map { } export function isIssuesEvent( - context: ParsedGitHubContext, + context: GitHubContext, ): context is ParsedGitHubContext & { payload: IssuesEvent } { return context.eventName === "issues"; } export function isIssueCommentEvent( - context: ParsedGitHubContext, + context: GitHubContext, ): context is ParsedGitHubContext & { payload: IssueCommentEvent } { return context.eventName === "issue_comment"; } export function isPullRequestEvent( - context: ParsedGitHubContext, + context: GitHubContext, ): context is ParsedGitHubContext & { payload: PullRequestEvent } { return context.eventName === "pull_request"; } export function isPullRequestReviewEvent( - context: ParsedGitHubContext, + context: GitHubContext, ): context is ParsedGitHubContext & { payload: PullRequestReviewEvent } { return context.eventName === "pull_request_review"; } export function isPullRequestReviewCommentEvent( - context: ParsedGitHubContext, + context: GitHubContext, ): context is ParsedGitHubContext & { payload: PullRequestReviewCommentEvent } { return context.eventName === "pull_request_review_comment"; } export function isIssuesAssignedEvent( - context: ParsedGitHubContext, + context: GitHubContext, ): context is ParsedGitHubContext & { payload: IssuesAssignedEvent } { return isIssuesEvent(context) && context.eventAction === "assigned"; } + +// Type guard to check if context is an entity context (has entityNumber and isPR) +export function isEntityContext( + context: GitHubContext, +): context is ParsedGitHubContext { + return ENTITY_EVENT_NAMES.includes(context.eventName as EntityEventName); +} + +// Type guard to check if context is an automation context +export function isAutomationContext( + context: GitHubContext, +): context is AutomationContext { + return AUTOMATION_EVENT_NAMES.includes( + context.eventName as AutomationEventName, + ); +} diff --git a/src/github/operations/comments/create-initial.ts b/src/github/operations/comments/create-initial.ts index 72fd378..1243035 100644 --- a/src/github/operations/comments/create-initial.ts +++ b/src/github/operations/comments/create-initial.ts @@ -22,12 +22,6 @@ export async function createInitialComment( ) { const { owner, repo } = context.repository; - // This function is only called for entity-based events - if (!context.entityNumber) { - throw new Error("createInitialComment requires an entity number"); - } - const entityNumber = context.entityNumber; - const jobRunLink = createJobRunLink(owner, repo, context.runId); const initialBody = createCommentBody(jobRunLink); @@ -42,7 +36,7 @@ export async function createInitialComment( const comments = await octokit.rest.issues.listComments({ owner, repo, - issue_number: entityNumber, + issue_number: context.entityNumber, }); const existingComment = comments.data.find((comment) => { const idMatch = comment.user?.id === CLAUDE_APP_BOT_ID; @@ -65,7 +59,7 @@ export async function createInitialComment( response = await octokit.rest.issues.createComment({ owner, repo, - issue_number: entityNumber, + issue_number: context.entityNumber, body: initialBody, }); } @@ -74,7 +68,7 @@ export async function createInitialComment( response = await octokit.rest.pulls.createReplyForReviewComment({ owner, repo, - pull_number: entityNumber, + pull_number: context.entityNumber, comment_id: context.payload.comment.id, body: initialBody, }); @@ -83,7 +77,7 @@ export async function createInitialComment( response = await octokit.rest.issues.createComment({ owner, repo, - issue_number: entityNumber, + issue_number: context.entityNumber, body: initialBody, }); } @@ -101,7 +95,7 @@ export async function createInitialComment( const response = await octokit.rest.issues.createComment({ owner, repo, - issue_number: entityNumber, + issue_number: context.entityNumber, body: initialBody, }); diff --git a/src/modes/agent/index.ts b/src/modes/agent/index.ts index a32260a..15a8d0c 100644 --- a/src/modes/agent/index.ts +++ b/src/modes/agent/index.ts @@ -1,6 +1,7 @@ import * as core from "@actions/core"; import { mkdir, writeFile } from "fs/promises"; import type { Mode, ModeOptions, ModeResult } from "../types"; +import { isAutomationContext } from "../../github/context"; /** * Agent mode implementation. @@ -15,10 +16,7 @@ export const agentMode: Mode = { shouldTrigger(context) { // Only trigger for automation events - return ( - context.eventName === "workflow_dispatch" || - context.eventName === "schedule" - ); + return isAutomationContext(context); }, prepareContext(context) { diff --git a/src/modes/registry.ts b/src/modes/registry.ts index 70d6c7e..83ce7ab 100644 --- a/src/modes/registry.ts +++ b/src/modes/registry.ts @@ -13,7 +13,8 @@ import type { Mode, ModeName } from "./types"; import { tagMode } from "./tag"; import { agentMode } from "./agent"; -import type { ParsedGitHubContext } from "../github/context"; +import type { GitHubContext } from "../github/context"; +import { isAutomationContext } from "../github/context"; export const DEFAULT_MODE = "tag" as const; export const VALID_MODES = ["tag", "agent"] as const; @@ -34,7 +35,7 @@ const modes = { * @returns The requested mode * @throws Error if the mode is not found or cannot handle the event */ -export function getMode(name: ModeName, context: ParsedGitHubContext): Mode { +export function getMode(name: ModeName, context: GitHubContext): Mode { const mode = modes[name]; if (!mode) { const validModes = VALID_MODES.join("', '"); @@ -44,11 +45,7 @@ export function getMode(name: ModeName, context: ParsedGitHubContext): Mode { } // Validate mode can handle the event type - if ( - name === "tag" && - (context.eventName === "workflow_dispatch" || - context.eventName === "schedule") - ) { + if (name === "tag" && isAutomationContext(context)) { throw new Error( `Tag mode cannot handle ${context.eventName} events. Use 'agent' mode for automation events.`, ); diff --git a/src/modes/tag/index.ts b/src/modes/tag/index.ts index 9f4ef45..7d4b57a 100644 --- a/src/modes/tag/index.ts +++ b/src/modes/tag/index.ts @@ -8,6 +8,7 @@ import { configureGitAuth } from "../../github/operations/git-config"; import { prepareMcpConfig } from "../../mcp/install-mcp-server"; import { fetchGitHubData } from "../../github/data/fetcher"; import { createPrompt } from "../../create-prompt"; +import { isEntityContext } from "../../github/context"; /** * Tag mode implementation. @@ -21,6 +22,10 @@ export const tagMode: Mode = { description: "Traditional implementation mode triggered by @claude mentions", shouldTrigger(context) { + // Tag mode only handles entity events + if (!isEntityContext(context)) { + return false; + } return checkContainsTrigger(context); }, @@ -51,7 +56,10 @@ export const tagMode: Mode = { octokit, githubToken, }: ModeOptions): Promise { - // Tag mode handles entity-based events (issues, PRs, comments) + // Tag mode only handles entity-based events + if (!isEntityContext(context)) { + throw new Error("Tag mode requires entity context"); + } // Check if actor is human await checkHumanActor(octokit.rest, context); @@ -60,11 +68,6 @@ export const tagMode: Mode = { const commentData = await createInitialComment(octokit.rest, context); const commentId = commentData.id; - // Fetch GitHub data - entity events always have entityNumber and isPR - if (!context.entityNumber || context.isPR === undefined) { - throw new Error("Entity events must have entityNumber and isPR defined"); - } - const githubData = await fetchGitHubData({ octokits: octokit, repository: `${context.repository.owner}/${context.repository.repo}`, diff --git a/src/modes/types.ts b/src/modes/types.ts index d8a0ae9..777e9a5 100644 --- a/src/modes/types.ts +++ b/src/modes/types.ts @@ -1,10 +1,10 @@ -import type { ParsedGitHubContext } from "../github/context"; +import type { GitHubContext } from "../github/context"; export type ModeName = "tag" | "agent"; export type ModeContext = { mode: ModeName; - githubContext: ParsedGitHubContext; + githubContext: GitHubContext; commentId?: number; baseBranch?: string; claudeBranch?: string; @@ -32,12 +32,12 @@ export type Mode = { /** * Determines if this mode should trigger based on the GitHub context */ - shouldTrigger(context: ParsedGitHubContext): boolean; + shouldTrigger(context: GitHubContext): boolean; /** * Prepares the mode context with any additional data needed for prompt generation */ - prepareContext(context: ParsedGitHubContext, data?: ModeData): ModeContext; + prepareContext(context: GitHubContext, data?: ModeData): ModeContext; /** * Returns the list of tools that should be allowed for this mode @@ -64,7 +64,7 @@ export type Mode = { // Define types for mode prepare method to avoid circular dependencies export type ModeOptions = { - context: ParsedGitHubContext; + context: GitHubContext; octokit: any; // We'll use any to avoid circular dependency with Octokits githubToken: string; }; diff --git a/src/prepare/types.ts b/src/prepare/types.ts index 5fa8c19..c064275 100644 --- a/src/prepare/types.ts +++ b/src/prepare/types.ts @@ -1,4 +1,4 @@ -import type { ParsedGitHubContext } from "../github/context"; +import type { GitHubContext } from "../github/context"; import type { Octokits } from "../github/api/client"; import type { Mode } from "../modes/types"; @@ -13,7 +13,7 @@ export type PrepareResult = { }; export type PrepareOptions = { - context: ParsedGitHubContext; + context: GitHubContext; octokit: Octokits; mode: Mode; githubToken: string; diff --git a/test/mockContext.ts b/test/mockContext.ts index 7d00f13..2005a9a 100644 --- a/test/mockContext.ts +++ b/test/mockContext.ts @@ -1,4 +1,7 @@ -import type { ParsedGitHubContext } from "../src/github/context"; +import type { + ParsedGitHubContext, + AutomationContext, +} from "../src/github/context"; import type { IssuesEvent, IssueCommentEvent, @@ -38,7 +41,7 @@ export const createMockContext = ( ): ParsedGitHubContext => { const baseContext: ParsedGitHubContext = { runId: "1234567890", - eventName: "", + eventName: "issue_comment", // Default to a valid entity event eventAction: "", repository: defaultRepository, actor: "test-actor", @@ -55,6 +58,22 @@ export const createMockContext = ( return { ...baseContext, ...overrides }; }; +export const createMockAutomationContext = ( + overrides: Partial = {}, +): AutomationContext => { + const baseContext: AutomationContext = { + runId: "1234567890", + eventName: "workflow_dispatch", + eventAction: undefined, + repository: defaultRepository, + actor: "test-actor", + payload: {} as any, + inputs: defaultInputs, + }; + + return { ...baseContext, ...overrides }; +}; + export const mockIssueOpenedContext: ParsedGitHubContext = { runId: "1234567890", eventName: "issues", diff --git a/test/modes/agent.test.ts b/test/modes/agent.test.ts index 9302790..2daf068 100644 --- a/test/modes/agent.test.ts +++ b/test/modes/agent.test.ts @@ -1,15 +1,14 @@ import { describe, test, expect, beforeEach } from "bun:test"; import { agentMode } from "../../src/modes/agent"; -import type { ParsedGitHubContext } from "../../src/github/context"; -import { createMockContext } from "../mockContext"; +import type { GitHubContext } from "../../src/github/context"; +import { createMockContext, createMockAutomationContext } from "../mockContext"; describe("Agent Mode", () => { - let mockContext: ParsedGitHubContext; + let mockContext: GitHubContext; beforeEach(() => { - mockContext = createMockContext({ + mockContext = createMockAutomationContext({ eventName: "workflow_dispatch", - isPR: false, }); }); @@ -34,30 +33,26 @@ describe("Agent Mode", () => { test("agent mode only triggers for workflow_dispatch and schedule events", () => { // Should trigger for automation events - const workflowDispatchContext = createMockContext({ + const workflowDispatchContext = createMockAutomationContext({ eventName: "workflow_dispatch", - isPR: false, }); expect(agentMode.shouldTrigger(workflowDispatchContext)).toBe(true); - const scheduleContext = createMockContext({ + const scheduleContext = createMockAutomationContext({ eventName: "schedule", - isPR: false, }); expect(agentMode.shouldTrigger(scheduleContext)).toBe(true); - // Should NOT trigger for other events - const otherEvents = [ - "push", - "repository_dispatch", + // Should NOT trigger for entity events + const entityEvents = [ "issue_comment", "pull_request", "pull_request_review", "issues", - ]; + ] as const; - otherEvents.forEach((eventName) => { - const context = createMockContext({ eventName, isPR: false }); + entityEvents.forEach((eventName) => { + const context = createMockContext({ eventName }); expect(agentMode.shouldTrigger(context)).toBe(false); }); }); diff --git a/test/modes/registry.test.ts b/test/modes/registry.test.ts index 82e4915..a014861 100644 --- a/test/modes/registry.test.ts +++ b/test/modes/registry.test.ts @@ -3,18 +3,18 @@ import { getMode, isValidMode } from "../../src/modes/registry"; import type { ModeName } from "../../src/modes/types"; import { tagMode } from "../../src/modes/tag"; import { agentMode } from "../../src/modes/agent"; -import { createMockContext } from "../mockContext"; +import { createMockContext, createMockAutomationContext } from "../mockContext"; describe("Mode Registry", () => { const mockContext = createMockContext({ eventName: "issue_comment", }); - const mockWorkflowDispatchContext = createMockContext({ + const mockWorkflowDispatchContext = createMockAutomationContext({ eventName: "workflow_dispatch", }); - const mockScheduleContext = createMockContext({ + const mockScheduleContext = createMockAutomationContext({ eventName: "schedule", }); diff --git a/test/prepare-context.test.ts b/test/prepare-context.test.ts index fb2e9d0..cf2b7a2 100644 --- a/test/prepare-context.test.ts +++ b/test/prepare-context.test.ts @@ -299,15 +299,4 @@ describe("parseEnvVarsWithContext", () => { expect(result.allowedTools).toBe("Tool1,Tool2"); }); }); - - test("should throw error for unsupported event type", () => { - process.env = BASE_ENV; - const unsupportedContext = createMockContext({ - eventName: "unsupported_event", - eventAction: "whatever", - }); - expect(() => prepareContext(unsupportedContext, "12345")).toThrow( - "Unsupported event type: unsupported_event", - ); - }); }); diff --git a/test/trigger-validation.test.ts b/test/trigger-validation.test.ts index 6d3ca3c..ec1f6af 100644 --- a/test/trigger-validation.test.ts +++ b/test/trigger-validation.test.ts @@ -474,17 +474,6 @@ describe("checkContainsTrigger", () => { }); }); }); - - describe("non-matching events", () => { - it("should return false for non-matching event type", () => { - const context = createMockContext({ - eventName: "push", - eventAction: "created", - payload: {} as any, - }); - expect(checkContainsTrigger(context)).toBe(false); - }); - }); }); describe("escapeRegExp", () => {