refactor: replace hardcoded agentType === 'review' checks with profile-driven alternatives

src/agents/utils/tracking.ts

@@ -72,6 +72,12 @@ export interface TrackingContext {
 	loopDetection: LoopDetectionState;
 	/** Agent type for role-aware loop messages */
 	agentType?: string;
+	/**
+	 * Loop advice profile to use for role-aware messages.
+	 * When set, takes precedence over the agentType-based lookup.
+	 * Derived from `profile.finishHooks.requiresReview`.
+	 */
+	loopAdviceProfile?: 'review' | 'default';
 }
 
 /**
@@ -92,12 +98,16 @@ export function createLoopDetectionState(): LoopDetectionState {
 /**
  * Create a new tracking context with zero metrics.
  */
-export function createTrackingContext(agentType?: string): TrackingContext {
+export function createTrackingContext(
+	agentType?: string,
+	loopAdviceProfile?: 'review' | 'default',
+): TrackingContext {
 	return {
 		metrics: { llmIterations: 0, gadgetCalls: 0 },
 		syntheticInvocationIds: new Set(),
 		loopDetection: createLoopDetectionState(),
 		agentType,
+		loopAdviceProfile,
 	};
 }
 
@@ -215,8 +225,13 @@ const LOOP_ADVICE = {
 	},
 } as const;
 
-function getAdvice(agentType?: string): (typeof LOOP_ADVICE)[keyof typeof LOOP_ADVICE] {
-	return agentType === 'review' ? LOOP_ADVICE.review : LOOP_ADVICE.default;
+function getAdvice(
+	agentType?: string,
+	loopAdviceProfile?: 'review' | 'default',
+): (typeof LOOP_ADVICE)[keyof typeof LOOP_ADVICE] {
+	const isReview =
+		loopAdviceProfile !== undefined ? loopAdviceProfile === 'review' : agentType === 'review';
+	return isReview ? LOOP_ADVICE.review : LOOP_ADVICE.default;
 }
 
 /**
@@ -236,13 +251,14 @@ function generateLoopWarning(
 	repeatCount: number,
 	repeatedPattern: string,
 	agentType?: string,
+	loopAdviceProfile?: 'review' | 'default',
 ): string {
 	const urgency = repeatCount >= 3 ? '🚨' : '⚠️';
 	return `[System] ${urgency} LOOP DETECTED (×${repeatCount})
 
 Pattern: ${repeatedPattern}
 
-${getAdvice(agentType).exact}`;
+${getAdvice(agentType, loopAdviceProfile).exact}`;
 }
 
 /**
@@ -252,9 +268,11 @@ function generateNameOnlyLoopAction(
 	repeatCount: number,
 	pattern: string,
 	agentType?: string,
+	loopAdviceProfile?: 'review' | 'default',
 ): LoopAction | null {
-	const isReview = agentType === 'review';
-	const advice = getAdvice(agentType);
+	const isReview =
+		loopAdviceProfile !== undefined ? loopAdviceProfile === 'review' : agentType === 'review';
+	const advice = getAdvice(agentType, loopAdviceProfile);
 
 	if (repeatCount >= LOOP_THRESHOLDS.HARD_STOP) {
 		return {
@@ -324,6 +342,7 @@ export function checkForLoopAndAdvance(context: TrackingContext): boolean {
 			state.repeatCount,
 			state.repeatedPattern,
 			context.agentType,
+			context.loopAdviceProfile,
 		);
 	} else {
 		state.repeatCount = 1;
@@ -346,6 +365,7 @@ export function checkForLoopAndAdvance(context: TrackingContext): boolean {
 				state.nameOnlyRepeatCount,
 				formatCallsForDisplay(state.currentIterationCalls),
 				context.agentType,
+				context.loopAdviceProfile,
 			);
 		} else {
 			state.nameOnlyRepeatCount = 1;

src/backends/llmist/index.ts

@@ -65,7 +65,10 @@ export class LlmistEngine implements AgentEngine {
 
 		// Create per-execution llmist logger and tracking state
 		const llmistLogger = createLogger({ minLevel: getLogLevel() });
-		const trackingContext = createTrackingContext(agentType);
+		const trackingContext = createTrackingContext(
+			agentType,
+			profile.finishHooks.requiresReview ? 'review' : 'default',
+		);
 		const llmCallAccumulator: AccumulatedLlmCall[] = [];
 
 		// Create a LLM call logger for raw request/response file logging.

src/backends/sidecarManager.ts

@@ -23,7 +23,7 @@ import type { AgentEngineResult } from './types.js';
  */
 export function createCompletionArtifacts(
 	profile: Awaited<ReturnType<typeof getAgentProfile>>,
-	agentType: string,
+	_agentType: string,
 	needsNativeToolRuntime: boolean,
 	input: AgentInput,
 	projectSecrets: Record<string, string>,
@@ -33,10 +33,9 @@ export function createCompletionArtifacts(
 	reviewSidecarPath: string | undefined;
 	pmWriteSidecarPath: string | undefined;
 } {
-	const reviewSidecarPath =
-		agentType === 'review'
-			? join(tmpdir(), `cascade-review-sidecar-${process.pid}-${Date.now()}.json`)
-			: undefined;
+	const reviewSidecarPath = profile.finishHooks.requiresReview
+		? join(tmpdir(), `cascade-review-sidecar-${process.pid}-${Date.now()}.json`)
+		: undefined;
 	if (reviewSidecarPath) {
 		projectSecrets[REVIEW_SIDECAR_ENV_VAR] = reviewSidecarPath;
 	}

src/github/personas.ts

@@ -17,6 +17,20 @@ export interface PersonaIdentities {
 // Agent → Persona Mapping
 // ============================================================================
 
+/**
+ * Maps agent types to their GitHub personas.
+ *
+ * This is the canonical registration point for agent persona assignments.
+ * - `'implementer'` — uses the implementer GitHub token for all SCM operations
+ * - `'reviewer'`    — uses the reviewer GitHub token, appropriate for agents
+ *   that submit PR reviews (e.g. the built-in `review` agent)
+ *
+ * To add a custom agent with reviewer behaviour, add an entry here:
+ * ```ts
+ * 'my-custom-reviewer': 'reviewer',
+ * ```
+ * Any agent type not listed here defaults to `'implementer'`.
+ */
 const AGENT_PERSONA_MAP: Record<string, GitHubPersona> = {
 	splitting: 'implementer',
 	planning: 'implementer',

src/router/github-token-resolver.ts

@@ -9,6 +9,7 @@
 
 import { getProjectGitHubToken } from '../config/projects.js';
 import { findProjectByRepo, getIntegrationCredential } from '../config/provider.js';
+import { getPersonaForAgentType } from '../github/personas.js';
 import type { ProjectConfig } from '../types/index.js';
 import { logger } from '../utils/logging.js';
 
@@ -57,7 +58,8 @@ export async function resolveGitHubTokenForAckByAgent(
 	if (!resolvedProject) return null;
 
 	try {
-		if (agentType === 'review') {
+		const persona = getPersonaForAgentType(agentType);
+		if (persona === 'reviewer') {
 			const token = await getIntegrationCredential(resolvedProject.id, 'scm', 'reviewer_token');
 			return { token, project: resolvedProject };
 		}

tests/unit/agents/utils/tracking.test.ts

@@ -519,4 +519,51 @@ describe('loop detection', () => {
 			expect(action?.message).toContain('delete the failing test');
 		});
 	});
+
+	describe('loopAdviceProfile overrides agentType', () => {
+		it('uses review loop advice when loopAdviceProfile is "review" and agentType is not "review"', () => {
+			const ctx = createTrackingContext('implementation', 'review');
+
+			// Create exact-match loop to trigger warning
+			recordGadgetCallForLoop(ctx, 'ReadFile', { filePath: '/foo.ts' });
+			checkForLoopAndAdvance(ctx);
+			recordGadgetCallForLoop(ctx, 'ReadFile', { filePath: '/foo.ts' });
+			checkForLoopAndAdvance(ctx);
+
+			const warning = consumeLoopWarning(ctx);
+			expect(warning).toContain('CreatePRReview');
+			expect(warning).not.toContain('COMPLETELY DIFFERENT APPROACH');
+		});
+
+		it('uses default loop advice when loopAdviceProfile is "default" and agentType is "review"', () => {
+			const ctx = createTrackingContext('review', 'default');
+
+			// Create exact-match loop to trigger warning
+			recordGadgetCallForLoop(ctx, 'ReadFile', { filePath: '/foo.ts' });
+			checkForLoopAndAdvance(ctx);
+			recordGadgetCallForLoop(ctx, 'ReadFile', { filePath: '/foo.ts' });
+			checkForLoopAndAdvance(ctx);
+
+			const warning = consumeLoopWarning(ctx);
+			expect(warning).toContain('COMPLETELY DIFFERENT APPROACH');
+			expect(warning).not.toContain('CreatePRReview');
+		});
+
+		it('uses review name-only loop action when loopAdviceProfile is "review" and agentType is not "review"', () => {
+			const ctx = createTrackingContext('implementation', 'review');
+
+			for (let i = 0; i < LOOP_THRESHOLDS.WARNING; i++) {
+				recordGadgetCallForLoop(ctx, 'FileSearchAndReplace', {
+					filePath: '/foo.ts',
+					search: `v${i}`,
+				});
+				checkForLoopAndAdvance(ctx);
+			}
+
+			const action = consumeLoopAction(ctx);
+			expect(action).not.toBeNull();
+			expect(action?.message).toContain('CreatePRReview');
+			expect(action?.message).not.toContain('delete the failing test');
+		});
+	});
 });

tests/unit/backends/adapter.test.ts

@@ -854,6 +854,9 @@ describe('executeWithEngine', () => {
 
 		it('calls recordReviewSubmission when sidecar exists for review agent', async () => {
 			setupMocks();
+			mockGetAgentProfile.mockReturnValue(
+				makeMockProfile({ finishHooks: { requiresReview: true } }),
+			);
 			const engine = makeMockBackend();
 			writeSidecarAtInjectedPath(engine, {
 				reviewUrl: 'https://github.com/o/r/pull/1#pullrequestreview-99',
@@ -873,6 +876,9 @@ describe('executeWithEngine', () => {
 
 		it('injects CASCADE_REVIEW_SIDECAR_PATH into projectSecrets for review agent', async () => {
 			setupMocks();
+			mockGetAgentProfile.mockReturnValue(
+				makeMockProfile({ finishHooks: { requiresReview: true } }),
+			);
 			const engine = makeMockBackend();
 			const input = makeInput();
 
@@ -951,6 +957,9 @@ describe('executeWithEngine', () => {
 
 		it('clears initialCommentId when sidecar has ackCommentDeleted: true', async () => {
 			setupMocks();
+			mockGetAgentProfile.mockReturnValue(
+				makeMockProfile({ finishHooks: { requiresReview: true } }),
+			);
 			const engine = makeMockBackend();
 			writeSidecarAtInjectedPath(engine, {
 				reviewUrl: 'https://github.com/o/r/pull/1#pullrequestreview-42',

tests/unit/backends/llmist.test.ts

@@ -22,6 +22,7 @@ vi.mock('../../../src/agents/definitions/index.js', () => ({
 vi.mock('../../../src/agents/definitions/profiles.js', () => ({
 	getAgentProfile: vi.fn(() => ({
 		getLlmistGadgets: vi.fn(() => []),
+		finishHooks: {},
 	})),
 }));
 
@@ -363,6 +364,7 @@ describe('LlmistEngine.execute', () => {
 		const mockGetLlmistGadgets = vi.fn().mockReturnValue([]);
 		mockGetAgentProfile.mockReturnValue({
 			getLlmistGadgets: mockGetLlmistGadgets,
+			finishHooks: {},
 		} as ReturnType<typeof getAgentProfile>);
 
 		const engine = new LlmistEngine();

tests/unit/backends/sidecarManager.test.ts

@@ -59,8 +59,8 @@ function makeSidecarPath(name: string): string {
 }
 
 describe('createCompletionArtifacts', () => {
-	it('creates a review sidecar path for review agent type', () => {
-		const profile = makeProfile();
+	it('creates a review sidecar path when profile.finishHooks.requiresReview is true', () => {
+		const profile = makeProfile({ finishHooks: { requiresReview: true } });
 		const projectSecrets: Record<string, string> = {};
 
 		const result = createCompletionArtifacts(
@@ -91,6 +91,38 @@ describe('createCompletionArtifacts', () => {
 		expect(projectSecrets.CASCADE_REVIEW_SIDECAR_PATH).toBeUndefined();
 	});
 
+	it('creates review sidecar for custom agent with requiresReview: true', () => {
+		const profile = makeProfile({ finishHooks: { requiresReview: true } });
+		const projectSecrets: Record<string, string> = {};
+
+		const result = createCompletionArtifacts(
+			profile,
+			'custom-reviewer',
+			false,
+			{} as AgentInput,
+			projectSecrets,
+		);
+
+		expect(result.reviewSidecarPath).toMatch(/cascade-review-sidecar-\d+-\d+\.json$/);
+		expect(projectSecrets.CASCADE_REVIEW_SIDECAR_PATH).toBe(result.reviewSidecarPath);
+	});
+
+	it('does not create review sidecar when requiresReview is not set', () => {
+		const profile = makeProfile({ finishHooks: {} });
+		const projectSecrets: Record<string, string> = {};
+
+		const result = createCompletionArtifacts(
+			profile,
+			'implementation',
+			false,
+			{} as AgentInput,
+			projectSecrets,
+		);
+
+		expect(result.reviewSidecarPath).toBeUndefined();
+		expect(projectSecrets.CASCADE_REVIEW_SIDECAR_PATH).toBeUndefined();
+	});
+
 	it('creates a PR sidecar path when requiresPR and needsNativeToolRuntime', () => {
 		const profile = makeProfile({ finishHooks: { requiresPR: true } });
 		const projectSecrets: Record<string, string> = {};

tests/unit/router/github-token-resolver.test.ts

@@ -11,6 +11,13 @@ vi.mock('../../../src/config/projects.js', () => ({
 	getProjectGitHubToken: vi.fn(),
 }));
 
+// Mock getPersonaForAgentType for persona-based token selection tests
+vi.mock('../../../src/github/personas.js', () => ({
+	getPersonaForAgentType: vi.fn((agentType: string) =>
+		agentType === 'review' ? 'reviewer' : 'implementer',
+	),
+}));
+
 // Mock config cache (imported transitively)
 vi.mock('../../../src/config/configCache.js', () => ({
 	configCache: {
@@ -36,11 +43,14 @@ vi.mock('../../../src/utils/logging.js', () => ({
 
 import { getProjectGitHubToken } from '../../../src/config/projects.js';
 import { findProjectByRepo, getIntegrationCredential } from '../../../src/config/provider.js';
+import { getPersonaForAgentType } from '../../../src/github/personas.js';
 import {
 	resolveGitHubTokenForAck,
 	resolveGitHubTokenForAckByAgent,
 } from '../../../src/router/github-token-resolver.js';
 
+const mockGetPersonaForAgentType = vi.mocked(getPersonaForAgentType);
+
 const mockGetIntegrationCredential = vi.mocked(getIntegrationCredential);
 const mockGetProjectGitHubToken = vi.mocked(getProjectGitHubToken);
 const mockFindProjectByRepo = vi.mocked(findProjectByRepo);
@@ -184,4 +194,29 @@ describe('resolveGitHubTokenForAckByAgent', () => {
 		expect(mockFindProjectByRepo).not.toHaveBeenCalled();
 		expect(mockGetProjectGitHubToken).toHaveBeenCalledWith(preResolvedProject);
 	});
+
+	it('delegates to getPersonaForAgentType for reviewer token selection', async () => {
+		mockGetPersonaForAgentType.mockReturnValueOnce('reviewer');
+		mockGetIntegrationCredential.mockImplementation(async (_projectId, category, role) => {
+			if (category === 'scm' && role === 'reviewer_token') return 'custom-reviewer-token';
+			throw new Error(`Credential '${category}/${role}' not found`);
+		});
+
+		const result = await resolveGitHubTokenForAckByAgent('owner/repo', 'custom-review-agent');
+
+		expect(result).not.toBeNull();
+		expect(result?.token).toBe('custom-reviewer-token');
+		expect(mockGetPersonaForAgentType).toHaveBeenCalledWith('custom-review-agent');
+	});
+
+	it('delegates to getPersonaForAgentType for implementer token selection', async () => {
+		mockGetPersonaForAgentType.mockReturnValueOnce('implementer');
+
+		const result = await resolveGitHubTokenForAckByAgent('owner/repo', 'custom-impl-agent');
+
+		expect(result).not.toBeNull();
+		expect(result?.token).toBe('test-github-token');
+		expect(mockGetPersonaForAgentType).toHaveBeenCalledWith('custom-impl-agent');
+		expect(mockGetProjectGitHubToken).toHaveBeenCalled();
+	});
 });