From 26d6ecc65d1db9599b0bc830a290d4a8ddd250ce Mon Sep 17 00:00:00 2001 From: km-anthropic Date: Tue, 29 Jul 2025 10:15:22 -0700 Subject: [PATCH] simplify: remove workflow_dispatch/schedule from create-prompt - Remove workflow_dispatch and schedule event handling from create-prompt since agent mode doesn't use the standard prompt generation flow - Enforce mode compatibility at selection time in the registry instead of runtime validation in tag mode - Add explanatory comment in agent mode about why prompt file is needed - Update tests to reflect simplified event handling This reduces code duplication and makes the separation between tag mode (entity-based events) and agent mode (automation events) clearer. --- src/create-prompt/index.ts | 30 -------------- src/create-prompt/types.ts | 18 +-------- src/entrypoints/prepare.ts | 2 +- src/modes/agent/index.ts | 5 ++- src/modes/registry.ts | 20 ++++++++-- src/modes/tag/index.ts | 11 ------ test/modes/registry.test.ts | 45 +++++++++++++++++++-- test/prepare-context.test.ts | 77 ------------------------------------ 8 files changed, 64 insertions(+), 144 deletions(-) diff --git a/src/create-prompt/index.ts b/src/create-prompt/index.ts index 14a7220..e72118c 100644 --- a/src/create-prompt/index.ts +++ b/src/create-prompt/index.ts @@ -350,24 +350,6 @@ export function prepareContext( }; break; - case "workflow_dispatch": - eventData = { - eventName: "workflow_dispatch", - isPR: false, - ...(baseBranch && { baseBranch }), - ...(claudeBranch && { claudeBranch }), - }; - break; - - case "schedule": - eventData = { - eventName: "schedule", - isPR: false, - ...(baseBranch && { baseBranch }), - ...(claudeBranch && { claudeBranch }), - }; - break; - default: throw new Error(`Unsupported event type: ${eventName}`); } @@ -430,18 +412,6 @@ export function getEventTypeAndContext(envVars: PreparedContext): { : `pull request event`, }; - case "workflow_dispatch": - return { - eventType: "WORKFLOW_DISPATCH", - triggerContext: `workflow dispatch event`, - }; - - case "schedule": - return { - eventType: "SCHEDULE", - triggerContext: `scheduled automation event`, - }; - default: throw new Error(`Unexpected event type`); } diff --git a/src/create-prompt/types.ts b/src/create-prompt/types.ts index ea9e4a2..bbf4103 100644 --- a/src/create-prompt/types.ts +++ b/src/create-prompt/types.ts @@ -88,20 +88,6 @@ type PullRequestEvent = { baseBranch?: string; }; -type WorkflowDispatchEvent = { - eventName: "workflow_dispatch"; - isPR?: false; - baseBranch?: string; - claudeBranch?: string; -}; - -type ScheduleEvent = { - eventName: "schedule"; - isPR?: false; - baseBranch?: string; - claudeBranch?: string; -}; - // Union type for all possible event types export type EventData = | PullRequestReviewCommentEvent @@ -111,9 +97,7 @@ export type EventData = | IssueOpenedEvent | IssueAssignedEvent | IssueLabeledEvent - | PullRequestEvent - | WorkflowDispatchEvent - | ScheduleEvent; + | PullRequestEvent; // Combined type with separate eventData field export type PreparedContext = CommonFields & { diff --git a/src/entrypoints/prepare.ts b/src/entrypoints/prepare.ts index 7b80c6b..0523ff1 100644 --- a/src/entrypoints/prepare.ts +++ b/src/entrypoints/prepare.ts @@ -34,7 +34,7 @@ async function run() { } // Step 4: Get mode and check trigger conditions - const mode = getMode(context.inputs.mode); + const mode = getMode(context.inputs.mode, context); const containsTrigger = mode.shouldTrigger(context); // Set output for action.yml to check diff --git a/src/modes/agent/index.ts b/src/modes/agent/index.ts index 4d67c37..caa9435 100644 --- a/src/modes/agent/index.ts +++ b/src/modes/agent/index.ts @@ -53,7 +53,10 @@ export const agentMode: Mode = { recursive: true, }); - // Write the prompt - either override_prompt, direct_prompt, or a minimal default + // Write the prompt file - the base action requires a prompt_file parameter, + // so we must create this file even though agent mode typically uses + // override_prompt or direct_prompt. If neither is provided, we write + // a minimal prompt with just the repository information. const promptContent = context.inputs.overridePrompt || context.inputs.directPrompt || diff --git a/src/modes/registry.ts b/src/modes/registry.ts index 043137a..70d6c7e 100644 --- a/src/modes/registry.ts +++ b/src/modes/registry.ts @@ -13,6 +13,7 @@ import type { Mode, ModeName } from "./types"; import { tagMode } from "./tag"; import { agentMode } from "./agent"; +import type { ParsedGitHubContext } from "../github/context"; export const DEFAULT_MODE = "tag" as const; export const VALID_MODES = ["tag", "agent"] as const; @@ -27,12 +28,13 @@ const modes = { } as const satisfies Record; /** - * Retrieves a mode by name. + * Retrieves a mode by name and validates it can handle the event type. * @param name The mode name to retrieve + * @param context The GitHub context to validate against * @returns The requested mode - * @throws Error if the mode is not found + * @throws Error if the mode is not found or cannot handle the event */ -export function getMode(name: ModeName): Mode { +export function getMode(name: ModeName, context: ParsedGitHubContext): Mode { const mode = modes[name]; if (!mode) { const validModes = VALID_MODES.join("', '"); @@ -40,6 +42,18 @@ export function getMode(name: ModeName): Mode { `Invalid mode '${name}'. Valid modes are: '${validModes}'. Please check your workflow configuration.`, ); } + + // Validate mode can handle the event type + if ( + name === "tag" && + (context.eventName === "workflow_dispatch" || + context.eventName === "schedule") + ) { + throw new Error( + `Tag mode cannot handle ${context.eventName} events. Use 'agent' mode for automation events.`, + ); + } + return mode; } diff --git a/src/modes/tag/index.ts b/src/modes/tag/index.ts index 40ecd71..5731c94 100644 --- a/src/modes/tag/index.ts +++ b/src/modes/tag/index.ts @@ -53,17 +53,6 @@ export const tagMode: Mode = { }: ModeOptions): Promise { // Tag mode handles entity-based events (issues, PRs, comments) - // Validate this mode can handle the event - if ( - context.eventName === "workflow_dispatch" || - context.eventName === "schedule" - ) { - throw new Error( - `Tag mode cannot handle ${context.eventName} events. ` + - `Use 'agent' mode for automation events.`, - ); - } - // Check if actor is human await checkHumanActor(octokit.rest, context); diff --git a/test/modes/registry.test.ts b/test/modes/registry.test.ts index 2e7b011..82e4915 100644 --- a/test/modes/registry.test.ts +++ b/test/modes/registry.test.ts @@ -3,23 +3,60 @@ 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"; describe("Mode Registry", () => { - test("getMode returns tag mode by default", () => { - const mode = getMode("tag"); + const mockContext = createMockContext({ + eventName: "issue_comment", + }); + + const mockWorkflowDispatchContext = createMockContext({ + eventName: "workflow_dispatch", + }); + + const mockScheduleContext = createMockContext({ + eventName: "schedule", + }); + + test("getMode returns tag mode for standard events", () => { + const mode = getMode("tag", mockContext); expect(mode).toBe(tagMode); expect(mode.name).toBe("tag"); }); test("getMode returns agent mode", () => { - const mode = getMode("agent"); + const mode = getMode("agent", mockContext); + expect(mode).toBe(agentMode); + expect(mode.name).toBe("agent"); + }); + + test("getMode throws error for tag mode with workflow_dispatch event", () => { + expect(() => getMode("tag", mockWorkflowDispatchContext)).toThrow( + "Tag mode cannot handle workflow_dispatch events. Use 'agent' mode for automation events.", + ); + }); + + test("getMode throws error for tag mode with schedule event", () => { + expect(() => getMode("tag", mockScheduleContext)).toThrow( + "Tag mode cannot handle schedule events. Use 'agent' mode for automation events.", + ); + }); + + test("getMode allows agent mode for workflow_dispatch event", () => { + const mode = getMode("agent", mockWorkflowDispatchContext); + expect(mode).toBe(agentMode); + expect(mode.name).toBe("agent"); + }); + + test("getMode allows agent mode for schedule event", () => { + const mode = getMode("agent", mockScheduleContext); expect(mode).toBe(agentMode); expect(mode.name).toBe("agent"); }); test("getMode throws error for invalid mode", () => { const invalidMode = "invalid" as unknown as ModeName; - expect(() => getMode(invalidMode)).toThrow( + expect(() => getMode(invalidMode, mockContext)).toThrow( "Invalid mode 'invalid'. Valid modes are: 'tag', 'agent'. Please check your workflow configuration.", ); }); diff --git a/test/prepare-context.test.ts b/test/prepare-context.test.ts index 74aabc3..fb2e9d0 100644 --- a/test/prepare-context.test.ts +++ b/test/prepare-context.test.ts @@ -29,83 +29,6 @@ describe("parseEnvVarsWithContext", () => { process.env = originalEnv; }); - describe("workflow_dispatch event", () => { - beforeEach(() => { - process.env = { - ...BASE_ENV, - BASE_BRANCH: "main", - }; - }); - - test("should parse workflow_dispatch event correctly", () => { - const mockWorkflowDispatchContext = createMockContext({ - eventName: "workflow_dispatch", - payload: { - inputs: { - task: "Run automated task", - }, - repository: { - name: "test-repo", - owner: { - login: "test-owner", - }, - }, - sender: { - login: "test-user", - }, - workflow: "test.yml", - }, - }); - - const result = prepareContext( - mockWorkflowDispatchContext, - "", - "main", - undefined, - ); - - expect(result.repository).toBe("test-owner/test-repo"); - expect(result.eventData.eventName).toBe("workflow_dispatch"); - expect(result.eventData.isPR).toBe(false); - expect(result.eventData.baseBranch).toBe("main"); - // triggerUsername is not extracted for workflow_dispatch events - expect(result.triggerUsername).toBeUndefined(); - }); - }); - - describe("schedule event", () => { - beforeEach(() => { - process.env = { - ...BASE_ENV, - BASE_BRANCH: "main", - }; - }); - - test("should parse schedule event correctly", () => { - const mockScheduleContext = createMockContext({ - eventName: "schedule", - payload: { - schedule: "0 0 * * *", - repository: { - name: "test-repo", - owner: { - login: "test-owner", - }, - }, - }, - }); - - const result = prepareContext(mockScheduleContext, "", "main", undefined); - - expect(result.repository).toBe("test-owner/test-repo"); - expect(result.eventData.eventName).toBe("schedule"); - expect(result.eventData.isPR).toBe(false); - expect(result.eventData.baseBranch).toBe("main"); - // triggerUsername is not extracted for schedule events - expect(result.triggerUsername).toBeUndefined(); - }); - }); - describe("issue_comment event", () => { describe("on issue", () => { beforeEach(() => {