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

[aaight]

Summary

Replaces 3 hardcoded agentType === 'review' checks with profile-based alternatives, enabling custom agent types with review-like behavior to work correctly without requiring the hardcoded string 'review'.

• sidecarManager.ts — Use profile.finishHooks.requiresReview instead of agentType === 'review' for review sidecar creation
• github-token-resolver.ts — Use getPersonaForAgentType() from personas.ts to delegate to AGENT_PERSONA_MAP for reviewer token selection
• tracking.ts — Add loopAdviceProfile field to TrackingContext, derived from finishHooks.requiresReview, so loop messages are profile-driven rather than hardcoded
• personas.ts — Add JSDoc to AGENT_PERSONA_MAP documenting it as the registration point for custom agent personas
• llmist/index.ts — Wire loopAdviceProfile from profile into createTrackingContext()

Test plan

• Updated sidecarManager.test.ts: new test verifies custom agent with requiresReview: true gets review sidecar
• Updated github-token-resolver.test.ts: mocked getPersonaForAgentType, verified persona-based token selection for both reviewer and implementer custom agents
• Updated tracking.test.ts: new tests verify loopAdviceProfile: 'review' yields review loop advice even for non-'review' agentType, and loopAdviceProfile: 'default' overrides agentType: 'review'
• Updated adapter.test.ts and llmist.test.ts to use profile-based finishHooks in mock profiles
• All 7274 unit tests pass, TypeScript clean, lint clean

Key decisions

• _agentType parameter kept in createCompletionArtifacts signature for backward compatibility with all callsites (only renamed to avoid unused-variable TS error)
• loopAdviceProfile uses explicit 'review' | 'default' union rather than a boolean to be self-documenting and extensible
• getPersonaForAgentType stays synchronous — AGENT_PERSONA_MAP remains the single source of truth

Trello card: https://trello.com/c/vSEx24UJ/580-13-hardcoded-agenttype-review-checks-scattered-across-source-found-in-src-backends-sidecarmanagerts37-src-router-github-token-re

🤖 Generated with Claude Code

🕵️ claude-code · claude-sonnet-4-6 · run details

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[nhopeatall]

Summary

LGTM — Clean, well-scoped refactor that replaces 3 hardcoded agentType === 'review' checks with profile-driven alternatives. All CI checks pass. The changes are backward-compatible and the test coverage is thorough.

Notes

• sidecarManager.ts: Correctly switches from agentType === 'review' to profile.finishHooks.requiresReview. The _agentType parameter rename is a reasonable choice for backward compatibility with callsites — removing it would be cleaner but would require updating secretOrchestrator.ts and all test callsites.

• github-token-resolver.ts: Delegating to getPersonaForAgentType() is the right move — it centralizes the agent→persona mapping in AGENT_PERSONA_MAP rather than spreading hardcoded checks. The behavior is identical for existing agent types.

• tracking.ts: The loopAdviceProfile field with explicit 'review' | 'default' union is a good design choice — more self-documenting than a boolean and extensible if more profiles are needed later. The precedence logic (loopAdviceProfile overrides agentType when defined) is correct with clean fallback.

• llmist/index.ts: The derivation profile.finishHooks.requiresReview ? 'review' : 'default' correctly handles the undefined case (falsy → 'default').

• personas.ts: The added JSDoc clearly documents AGENT_PERSONA_MAP as the registration point for custom agent personas. Good guidance for future contributors.

• Tests: New tests properly verify both the positive case (custom agent with requiresReview: true gets review behavior) and the override case (loopAdviceProfile takes precedence over agentType). Existing tests are updated with appropriate finishHooks in mock profiles.

🕵️ claude-code · claude-opus-4-6 · run details