refactor: simplify mode system by removing Mode interface and registry (#899)

Replace the over-engineered Mode interface/registry/detector pattern with
straightforward inline logic. There are only 2 modes (tag and agent) and
the complexity wasn't justified.

- Delete Mode interface, registry, and prepare pass-through modules
- Export prepareTagMode() and prepareAgentMode() as standalone functions
- Inline trigger checking and mode dispatch in run.ts/prepare.ts
- Change generatePrompt/createPrompt to take modeName string instead of Mode
- Remove dead code (extractGitHubContext, unused detector helpers)
- Update CLAUDE.md to reflect new architecture
This commit is contained in:
Ashwin Bhat
2026-02-05 17:22:30 -08:00
committed by GitHub
parent f09dc9a6a3
commit 7057f3318b
16 changed files with 390 additions and 1300 deletions
+12 -90
View File
@@ -7,22 +7,17 @@ import {
spyOn,
mock,
} from "bun:test";
import { agentMode } from "../../src/modes/agent";
import type { GitHubContext } from "../../src/github/context";
import { createMockContext, createMockAutomationContext } from "../mockContext";
import { prepareAgentMode } from "../../src/modes/agent";
import { createMockAutomationContext } from "../mockContext";
import * as core from "@actions/core";
import * as gitConfig from "../../src/github/operations/git-config";
describe("Agent Mode", () => {
let mockContext: GitHubContext;
let exportVariableSpy: any;
let setOutputSpy: any;
let configureGitAuthSpy: any;
beforeEach(() => {
mockContext = createMockAutomationContext({
eventName: "workflow_dispatch",
});
exportVariableSpy = spyOn(core, "exportVariable").mockImplementation(
() => {},
);
@@ -45,84 +40,11 @@ describe("Agent Mode", () => {
configureGitAuthSpy?.mockRestore();
});
test("agent mode has correct properties", () => {
expect(agentMode.name).toBe("agent");
expect(agentMode.description).toBe(
"Direct automation mode for explicit prompts",
);
expect(agentMode.shouldCreateTrackingComment()).toBe(false);
expect(agentMode.getAllowedTools()).toEqual([]);
expect(agentMode.getDisallowedTools()).toEqual([]);
test("prepareAgentMode is exported as a function", () => {
expect(typeof prepareAgentMode).toBe("function");
});
test("prepareContext returns minimal data", () => {
const context = agentMode.prepareContext(mockContext);
expect(context.mode).toBe("agent");
expect(context.githubContext).toBe(mockContext);
// Agent mode doesn't use comment tracking or branch management
expect(Object.keys(context)).toEqual(["mode", "githubContext"]);
});
test("agent mode only triggers when prompt is provided", () => {
// Should NOT trigger for automation events without prompt
const workflowDispatchContext = createMockAutomationContext({
eventName: "workflow_dispatch",
});
expect(agentMode.shouldTrigger(workflowDispatchContext)).toBe(false);
const scheduleContext = createMockAutomationContext({
eventName: "schedule",
});
expect(agentMode.shouldTrigger(scheduleContext)).toBe(false);
const repositoryDispatchContext = createMockAutomationContext({
eventName: "repository_dispatch",
});
expect(agentMode.shouldTrigger(repositoryDispatchContext)).toBe(false);
// Should NOT trigger for entity events without prompt
const entityEvents = [
"issue_comment",
"pull_request",
"pull_request_review",
"issues",
] as const;
entityEvents.forEach((eventName) => {
const contextNoPrompt = createMockContext({ eventName });
expect(agentMode.shouldTrigger(contextNoPrompt)).toBe(false);
});
// Should trigger for ANY event when prompt is provided
const allEvents = [
"workflow_dispatch",
"repository_dispatch",
"schedule",
"issue_comment",
"pull_request",
"pull_request_review",
"issues",
] as const;
allEvents.forEach((eventName) => {
const contextWithPrompt =
eventName === "workflow_dispatch" ||
eventName === "repository_dispatch" ||
eventName === "schedule"
? createMockAutomationContext({
eventName,
inputs: { prompt: "Do something" },
})
: createMockContext({
eventName,
inputs: { prompt: "Do something" },
});
expect(agentMode.shouldTrigger(contextWithPrompt)).toBe(true);
});
});
test("prepare method passes through claude_args", async () => {
test("prepare passes through claude_args", async () => {
// Clear any previous calls before this test
exportVariableSpy.mockClear();
setOutputSpy.mockClear();
@@ -156,7 +78,7 @@ describe("Agent Mode", () => {
},
},
} as any;
const result = await agentMode.prepare({
const result = await prepareAgentMode({
context: contextWithCustomArgs,
octokit: mockOctokit,
githubToken: "test-token",
@@ -186,7 +108,7 @@ describe("Agent Mode", () => {
process.env.GITHUB_REF_NAME = originalRefName;
});
test("prepare method rejects bot actors without allowed_bots", async () => {
test("prepare rejects bot actors without allowed_bots", async () => {
const contextWithPrompts = createMockAutomationContext({
eventName: "workflow_dispatch",
});
@@ -206,7 +128,7 @@ describe("Agent Mode", () => {
} as any;
await expect(
agentMode.prepare({
prepareAgentMode({
context: contextWithPrompts,
octokit: mockOctokit,
githubToken: "test-token",
@@ -216,7 +138,7 @@ describe("Agent Mode", () => {
);
});
test("prepare method allows bot actors when in allowed_bots list", async () => {
test("prepare allows bot actors when in allowed_bots list", async () => {
const contextWithPrompts = createMockAutomationContext({
eventName: "workflow_dispatch",
});
@@ -237,7 +159,7 @@ describe("Agent Mode", () => {
// Should not throw - bot is in allowed list
await expect(
agentMode.prepare({
prepareAgentMode({
context: contextWithPrompts,
octokit: mockOctokit,
githubToken: "test-token",
@@ -245,7 +167,7 @@ describe("Agent Mode", () => {
).resolves.toBeDefined();
});
test("prepare method creates prompt file with correct content", async () => {
test("prepare creates prompt file with correct content", async () => {
const contextWithPrompts = createMockAutomationContext({
eventName: "workflow_dispatch",
});
@@ -268,7 +190,7 @@ describe("Agent Mode", () => {
},
},
} as any;
const result = await agentMode.prepare({
const result = await prepareAgentMode({
context: contextWithPrompts,
octokit: mockOctokit,
githubToken: "test-token",
-155
View File
@@ -1,155 +0,0 @@
import { describe, test, expect } from "bun:test";
import { getMode, isValidMode } from "../../src/modes/registry";
import { agentMode } from "../../src/modes/agent";
import { tagMode } from "../../src/modes/tag";
import {
createMockContext,
createMockAutomationContext,
mockRepositoryDispatchContext,
} from "../mockContext";
describe("Mode Registry", () => {
const mockContext = createMockContext({
eventName: "issue_comment",
payload: {
action: "created",
comment: {
body: "Test comment without trigger",
},
} as any,
});
const mockWorkflowDispatchContext = createMockAutomationContext({
eventName: "workflow_dispatch",
});
const mockScheduleContext = createMockAutomationContext({
eventName: "schedule",
});
test("getMode auto-detects agent mode for issue_comment without trigger", () => {
const mode = getMode(mockContext);
// Agent mode is the default when no trigger is found
expect(mode).toBe(agentMode);
expect(mode.name).toBe("agent");
});
test("getMode auto-detects agent mode for workflow_dispatch", () => {
const mode = getMode(mockWorkflowDispatchContext);
expect(mode).toBe(agentMode);
expect(mode.name).toBe("agent");
});
// Removed test - explicit mode override no longer supported in v1.0
test("getMode auto-detects agent for workflow_dispatch", () => {
const mode = getMode(mockWorkflowDispatchContext);
expect(mode).toBe(agentMode);
expect(mode.name).toBe("agent");
});
test("getMode auto-detects agent for schedule event", () => {
const mode = getMode(mockScheduleContext);
expect(mode).toBe(agentMode);
expect(mode.name).toBe("agent");
});
test("getMode auto-detects agent for repository_dispatch event", () => {
const mode = getMode(mockRepositoryDispatchContext);
expect(mode).toBe(agentMode);
expect(mode.name).toBe("agent");
});
test("getMode auto-detects agent for repository_dispatch with client_payload", () => {
const contextWithPayload = createMockAutomationContext({
eventName: "repository_dispatch",
payload: {
action: "trigger-analysis",
client_payload: {
source: "external-system",
metadata: { priority: "high" },
},
repository: {
name: "test-repo",
owner: { login: "test-owner" },
},
sender: { login: "automation-user" },
},
});
const mode = getMode(contextWithPayload);
expect(mode).toBe(agentMode);
expect(mode.name).toBe("agent");
});
// Removed test - legacy mode names no longer supported in v1.0
test("getMode auto-detects agent mode for PR opened", () => {
const prContext = createMockContext({
eventName: "pull_request",
payload: { action: "opened" } as any,
isPR: true,
});
const mode = getMode(prContext);
expect(mode).toBe(agentMode);
expect(mode.name).toBe("agent");
});
test("getMode uses agent mode when prompt is provided, even with @claude mention", () => {
const contextWithPrompt = createMockContext({
eventName: "issue_comment",
payload: {
action: "created",
comment: {
body: "@claude please help",
},
} as any,
inputs: {
prompt: "/review",
} as any,
});
const mode = getMode(contextWithPrompt);
expect(mode).toBe(agentMode);
expect(mode.name).toBe("agent");
});
test("getMode uses tag mode for @claude mention without prompt", () => {
// Ensure PROMPT env var is not set (clean up from previous tests)
const originalPrompt = process.env.PROMPT;
delete process.env.PROMPT;
const contextWithMention = createMockContext({
eventName: "issue_comment",
payload: {
action: "created",
comment: {
body: "@claude please help",
},
} as any,
inputs: {
triggerPhrase: "@claude",
prompt: "",
} as any,
});
const mode = getMode(contextWithMention);
expect(mode).toBe(tagMode);
expect(mode.name).toBe("tag");
// Restore original value if it existed
if (originalPrompt !== undefined) {
process.env.PROMPT = originalPrompt;
}
});
// Removed test - explicit mode override no longer supported in v1.0
test("isValidMode returns true for all valid modes", () => {
expect(isValidMode("tag")).toBe(true);
expect(isValidMode("agent")).toBe(true);
});
test("isValidMode returns false for invalid mode", () => {
expect(isValidMode("invalid")).toBe(false);
expect(isValidMode("review")).toBe(false);
});
});
+4 -88
View File
@@ -1,92 +1,8 @@
import { describe, test, expect, beforeEach } from "bun:test";
import { tagMode } from "../../src/modes/tag";
import type { ParsedGitHubContext } from "../../src/github/context";
import type { IssueCommentEvent } from "@octokit/webhooks-types";
import { createMockContext } from "../mockContext";
import { describe, test, expect } from "bun:test";
import { prepareTagMode } from "../../src/modes/tag";
describe("Tag Mode", () => {
let mockContext: ParsedGitHubContext;
beforeEach(() => {
mockContext = createMockContext({
eventName: "issue_comment",
isPR: false,
});
});
test("tag mode has correct properties", () => {
expect(tagMode.name).toBe("tag");
expect(tagMode.description).toBe(
"Traditional implementation mode triggered by @claude mentions",
);
expect(tagMode.shouldCreateTrackingComment()).toBe(true);
});
test("shouldTrigger delegates to checkContainsTrigger", () => {
const contextWithTrigger = createMockContext({
eventName: "issue_comment",
isPR: false,
inputs: {
...createMockContext().inputs,
triggerPhrase: "@claude",
},
payload: {
comment: {
body: "Hey @claude, can you help?",
},
} as IssueCommentEvent,
});
expect(tagMode.shouldTrigger(contextWithTrigger)).toBe(true);
const contextWithoutTrigger = createMockContext({
eventName: "issue_comment",
isPR: false,
inputs: {
...createMockContext().inputs,
triggerPhrase: "@claude",
},
payload: {
comment: {
body: "This is just a regular comment",
},
} as IssueCommentEvent,
});
expect(tagMode.shouldTrigger(contextWithoutTrigger)).toBe(false);
});
test("prepareContext includes all required data", () => {
const data = {
commentId: 123,
baseBranch: "main",
claudeBranch: "claude/fix-bug",
};
const context = tagMode.prepareContext(mockContext, data);
expect(context.mode).toBe("tag");
expect(context.githubContext).toBe(mockContext);
expect(context.commentId).toBe(123);
expect(context.baseBranch).toBe("main");
expect(context.claudeBranch).toBe("claude/fix-bug");
});
test("prepareContext works without data", () => {
const context = tagMode.prepareContext(mockContext);
expect(context.mode).toBe("tag");
expect(context.githubContext).toBe(mockContext);
expect(context.commentId).toBeUndefined();
expect(context.baseBranch).toBeUndefined();
expect(context.claudeBranch).toBeUndefined();
});
test("getAllowedTools returns empty array", () => {
expect(tagMode.getAllowedTools()).toEqual([]);
});
test("getDisallowedTools returns empty array", () => {
expect(tagMode.getDisallowedTools()).toEqual([]);
test("prepareTagMode is exported as a function", () => {
expect(typeof prepareTagMode).toBe("function");
});
});