feat: simplify to two modes (tag and agent) for v1.0

BREAKING CHANGES:
- Remove review mode entirely - now handled via slash commands in agent mode
- Remove all deprecated backward compatibility fields (mode, anthropic_model, override_prompt, direct_prompt)
- Simplify mode detection: prompt overrides everything, then @claude mentions trigger tag mode, default is agent mode
- Remove slash command resolution from GitHub Action - Claude Code handles natively
- Remove variable substitution - prompts passed through as-is

Architecture changes:
- Only two modes now: tag (for @claude mentions) and agent (everything else)
- Agent mode is the default for all events including PRs
- Users configure behavior via prompts/slash commands (e.g. /review)
- GitHub Action is now a thin wrapper that passes prompts to Claude Code
- Mode names changed: 'experimental-review' → removed entirely

This aligns with the philosophy that the GitHub Action should do minimal work and delegate to Claude Code for all intelligent behavior.
This commit is contained in:
km-anthropic
2025-08-07 11:07:50 -07:00
parent da182b6afb
commit acbef8d08c
18 changed files with 125 additions and 728 deletions

View File

@@ -302,34 +302,7 @@ describe("generatePrompt", () => {
);
});
test("should include direct prompt when provided", async () => {
const envVars: PreparedContext = {
repository: "owner/repo",
claudeCommentId: "12345",
triggerPhrase: "@claude",
directPrompt: "Fix the bug in the login form",
eventData: {
eventName: "issues",
eventAction: "opened",
isPR: false,
issueNumber: "789",
baseBranch: "main",
claudeBranch: "claude/issue-789-20240101-1200",
},
};
const prompt = generateDefaultPrompt(envVars, mockGitHubData, false);
expect(prompt).toContain("<direct_prompt>");
expect(prompt).toContain(
"IMPORTANT: The following are direct instructions",
);
expect(prompt).toContain("Fix the bug in the login form");
expect(prompt).toContain("</direct_prompt>");
expect(prompt).toContain(
"CRITICAL: Direct user instructions were provided in the <direct_prompt> tag above. These are HIGH PRIORITY instructions that OVERRIDE all other context and MUST be followed exactly as written.",
);
});
// Removed test - direct_prompt field no longer supported in v1.0
test("should generate prompt for pull_request event", async () => {
const envVars: PreparedContext = {
@@ -389,7 +362,7 @@ describe("generatePrompt", () => {
repository: "owner/repo",
claudeCommentId: "12345",
triggerPhrase: "@claude",
overridePrompt: "Simple prompt for $REPOSITORY PR #$PR_NUMBER",
prompt: "Simple prompt for reviewing PR",
eventData: {
eventName: "pull_request",
eventAction: "opened",
@@ -405,17 +378,18 @@ describe("generatePrompt", () => {
mockTagMode,
);
expect(prompt).toBe("Simple prompt for owner/repo PR #123");
// v1.0: Prompt is passed through as-is
expect(prompt).toBe("Simple prompt for reviewing PR");
expect(prompt).not.toContain("You are Claude, an AI assistant");
});
test("should substitute all variables in override_prompt", async () => {
test("should pass through prompt without variable substitution", async () => {
const envVars: PreparedContext = {
repository: "test/repo",
claudeCommentId: "12345",
triggerPhrase: "@claude",
triggerUsername: "john-doe",
overridePrompt: `Repository: $REPOSITORY
prompt: `Repository: $REPOSITORY
PR: $PR_NUMBER
Title: $PR_TITLE
Body: $PR_BODY
@@ -445,19 +419,15 @@ describe("generatePrompt", () => {
mockTagMode,
);
expect(prompt).toContain("Repository: test/repo");
expect(prompt).toContain("PR: 456");
expect(prompt).toContain("Title: Test PR");
expect(prompt).toContain("Body: This is a test PR");
expect(prompt).toContain("Comments: ");
expect(prompt).toContain("Review Comments: ");
expect(prompt).toContain("Changed Files: ");
expect(prompt).toContain("Trigger Comment: Please review this code");
expect(prompt).toContain("Username: john-doe");
expect(prompt).toContain("Branch: feature-branch");
expect(prompt).toContain("Base: main");
expect(prompt).toContain("Event: pull_request_review_comment");
expect(prompt).toContain("Is PR: true");
// v1.0: Variables are NOT substituted - prompt is passed as-is to Claude Code
expect(prompt).toContain("Repository: $REPOSITORY");
expect(prompt).toContain("PR: $PR_NUMBER");
expect(prompt).toContain("Title: $PR_TITLE");
expect(prompt).toContain("Body: $PR_BODY");
expect(prompt).toContain("Branch: $BRANCH_NAME");
expect(prompt).toContain("Base: $BASE_BRANCH");
expect(prompt).toContain("Username: $TRIGGER_USERNAME");
expect(prompt).toContain("Comment: $TRIGGER_COMMENT");
});
test("should handle override_prompt for issues", async () => {
@@ -465,7 +435,7 @@ describe("generatePrompt", () => {
repository: "owner/repo",
claudeCommentId: "12345",
triggerPhrase: "@claude",
overridePrompt: "Issue #$ISSUE_NUMBER: $ISSUE_TITLE in $REPOSITORY",
prompt: "Review issue and provide feedback",
eventData: {
eventName: "issues",
eventAction: "opened",
@@ -497,16 +467,16 @@ describe("generatePrompt", () => {
mockTagMode,
);
expect(prompt).toBe("Issue #789: Bug: Login form broken in owner/repo");
// v1.0: Prompt is passed through as-is
expect(prompt).toBe("Review issue and provide feedback");
});
test("should handle empty values in override_prompt substitution", async () => {
test("should handle prompt without substitution", async () => {
const envVars: PreparedContext = {
repository: "owner/repo",
claudeCommentId: "12345",
triggerPhrase: "@claude",
overridePrompt:
"PR: $PR_NUMBER, Issue: $ISSUE_NUMBER, Comment: $TRIGGER_COMMENT",
prompt: "PR: $PR_NUMBER, Issue: $ISSUE_NUMBER, Comment: $TRIGGER_COMMENT",
eventData: {
eventName: "pull_request",
eventAction: "opened",
@@ -522,7 +492,10 @@ describe("generatePrompt", () => {
mockTagMode,
);
expect(prompt).toBe("PR: 123, Issue: , Comment: ");
// v1.0: No substitution - passed as-is
expect(prompt).toBe(
"PR: $PR_NUMBER, Issue: $ISSUE_NUMBER, Comment: $TRIGGER_COMMENT",
);
});
test("should not substitute variables when override_prompt is not provided", async () => {
@@ -1005,7 +978,7 @@ describe("getEventTypeAndContext", () => {
repository: "owner/repo",
claudeCommentId: "12345",
triggerPhrase: "@claude",
directPrompt: "Please assess this issue",
prompt: "Please assess this issue",
eventData: {
eventName: "issues",
eventAction: "assigned",
@@ -1013,7 +986,7 @@ describe("getEventTypeAndContext", () => {
issueNumber: "999",
baseBranch: "main",
claudeBranch: "claude/issue-999-20240101-1200",
// No assigneeTrigger when using directPrompt
// No assigneeTrigger when using prompt
},
};

View File

@@ -24,7 +24,6 @@ describe("prepareMcpConfig", () => {
entityNumber: 123,
isPR: false,
inputs: {
mode: "tag",
prompt: "",
triggerPhrase: "@claude",
assigneeTrigger: "",
@@ -32,8 +31,6 @@ describe("prepareMcpConfig", () => {
allowedTools: [],
disallowedTools: [],
customInstructions: "",
directPrompt: "",
overridePrompt: "",
branchPrefix: "",
useStickyComment: false,
additionalPermissions: new Map(),

View File

@@ -11,7 +11,6 @@ import type {
} from "@octokit/webhooks-types";
const defaultInputs = {
mode: "tag" as const,
prompt: "",
triggerPhrase: "/claude",
assigneeTrigger: "",
@@ -20,8 +19,6 @@ const defaultInputs = {
allowedTools: [] as string[],
disallowedTools: [] as string[],
customInstructions: "",
directPrompt: "",
overridePrompt: "",
useBedrock: false,
useVertex: false,
timeoutMinutes: 30,

View File

@@ -1,7 +1,7 @@
import { describe, test, expect } from "bun:test";
import { getMode, isValidMode } from "../../src/modes/registry";
import { agentMode } from "../../src/modes/agent";
import { reviewMode } from "../../src/modes/review";
import { tagMode } from "../../src/modes/tag";
import { createMockContext, createMockAutomationContext } from "../mockContext";
describe("Mode Registry", () => {
@@ -36,11 +36,7 @@ describe("Mode Registry", () => {
expect(mode.name).toBe("agent");
});
test("getMode can use explicit mode override for review", () => {
const mode = getMode(mockContext, "review");
expect(mode).toBe(reviewMode);
expect(mode.name).toBe("experimental-review");
});
// Removed test - explicit mode override no longer supported in v1.0
test("getMode auto-detects agent for workflow_dispatch", () => {
const mode = getMode(mockWorkflowDispatchContext);
@@ -54,37 +50,64 @@ describe("Mode Registry", () => {
expect(mode.name).toBe("agent");
});
test("getMode supports legacy experimental-review mode name", () => {
const mode = getMode(mockContext, "experimental-review");
expect(mode).toBe(reviewMode);
expect(mode.name).toBe("experimental-review");
});
// Removed test - legacy mode names no longer supported in v1.0
test("getMode auto-detects review mode for PR opened", () => {
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(reviewMode);
expect(mode.name).toBe("experimental-review");
});
test("getMode falls back to auto-detection for invalid mode override", () => {
const mode = getMode(mockContext, "invalid");
// Should fall back to auto-detection, which returns agent for issue_comment without trigger
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", () => {
const contextWithMention = createMockContext({
eventName: "issue_comment",
payload: {
action: "created",
comment: {
body: "@claude please help",
},
} as any,
inputs: {
triggerPhrase: "@claude",
} as any,
});
const mode = getMode(contextWithMention);
expect(mode).toBe(tagMode);
expect(mode.name).toBe("tag");
});
// 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);
expect(isValidMode("experimental-review")).toBe(true);
});
test("isValidMode returns false for invalid mode", () => {
expect(isValidMode("invalid")).toBe(false);
expect(isValidMode("review")).toBe(false);
});
});

View File

@@ -60,7 +60,6 @@ describe("checkWritePermissions", () => {
entityNumber: 1,
isPR: false,
inputs: {
mode: "tag",
prompt: "",
triggerPhrase: "@claude",
assigneeTrigger: "",
@@ -68,8 +67,6 @@ describe("checkWritePermissions", () => {
allowedTools: [],
disallowedTools: [],
customInstructions: "",
directPrompt: "",
overridePrompt: "",
branchPrefix: "claude/",
useStickyComment: false,
additionalPermissions: new Map(),

View File

@@ -220,13 +220,13 @@ describe("parseEnvVarsWithContext", () => {
).toThrow("BASE_BRANCH is required for issues event");
});
test("should allow issue assigned event with direct_prompt and no assigneeTrigger", () => {
test("should allow issue assigned event with prompt and no assigneeTrigger", () => {
const contextWithDirectPrompt = createMockContext({
...mockIssueAssignedContext,
inputs: {
...mockIssueAssignedContext.inputs,
assigneeTrigger: "", // No assignee trigger
directPrompt: "Please assess this issue", // But direct prompt is provided
prompt: "Please assess this issue", // But prompt is provided
},
});
@@ -239,7 +239,7 @@ describe("parseEnvVarsWithContext", () => {
expect(result.eventData.eventName).toBe("issues");
expect(result.eventData.isPR).toBe(false);
expect(result.directPrompt).toBe("Please assess this issue");
expect(result.prompt).toBe("Please assess this issue");
if (
result.eventData.eventName === "issues" &&
result.eventData.eventAction === "assigned"
@@ -249,13 +249,13 @@ describe("parseEnvVarsWithContext", () => {
}
});
test("should throw error when neither assigneeTrigger nor directPrompt provided for issue assigned event", () => {
test("should throw error when neither assigneeTrigger nor prompt provided for issue assigned event", () => {
const contextWithoutTriggers = createMockContext({
...mockIssueAssignedContext,
inputs: {
...mockIssueAssignedContext.inputs,
assigneeTrigger: "", // No assignee trigger
directPrompt: "", // No direct prompt
prompt: "", // No prompt
},
});

View File

@@ -22,19 +22,16 @@ import type {
import type { ParsedGitHubContext } from "../src/github/context";
describe("checkContainsTrigger", () => {
describe("direct prompt trigger", () => {
it("should return true when direct prompt is provided", () => {
describe("prompt trigger", () => {
it("should return true when prompt is provided", () => {
const context = createMockContext({
eventName: "issues",
eventAction: "opened",
inputs: {
mode: "tag",
prompt: "",
prompt: "Fix the bug in the login form",
triggerPhrase: "/claude",
assigneeTrigger: "",
labelTrigger: "",
directPrompt: "Fix the bug in the login form",
overridePrompt: "",
allowedTools: [],
disallowedTools: [],
customInstructions: "",
@@ -47,7 +44,7 @@ describe("checkContainsTrigger", () => {
expect(checkContainsTrigger(context)).toBe(true);
});
it("should return false when direct prompt is empty", () => {
it("should return false when prompt is empty", () => {
const context = createMockContext({
eventName: "issues",
eventAction: "opened",
@@ -62,13 +59,10 @@ describe("checkContainsTrigger", () => {
},
} as IssuesEvent,
inputs: {
mode: "tag",
prompt: "",
triggerPhrase: "/claude",
assigneeTrigger: "",
labelTrigger: "",
directPrompt: "",
overridePrompt: "",
allowedTools: [],
disallowedTools: [],
customInstructions: "",
@@ -280,13 +274,10 @@ describe("checkContainsTrigger", () => {
},
} as PullRequestEvent,
inputs: {
mode: "tag",
prompt: "",
triggerPhrase: "@claude",
assigneeTrigger: "",
labelTrigger: "",
directPrompt: "",
overridePrompt: "",
allowedTools: [],
disallowedTools: [],
customInstructions: "",
@@ -315,13 +306,10 @@ describe("checkContainsTrigger", () => {
},
} as PullRequestEvent,
inputs: {
mode: "tag",
prompt: "",
triggerPhrase: "@claude",
assigneeTrigger: "",
labelTrigger: "",
directPrompt: "",
overridePrompt: "",
allowedTools: [],
disallowedTools: [],
customInstructions: "",
@@ -350,13 +338,10 @@ describe("checkContainsTrigger", () => {
},
} as PullRequestEvent,
inputs: {
mode: "tag",
prompt: "",
triggerPhrase: "@claude",
assigneeTrigger: "",
labelTrigger: "",
directPrompt: "",
overridePrompt: "",
allowedTools: [],
disallowedTools: [],
customInstructions: "",