Fix change_title reliability across providers#64
Open
happier-bot wants to merge 2 commits intohappier-dev:devfrom
Open
Fix change_title reliability across providers#64happier-bot wants to merge 2 commits intohappier-dev:devfrom
happier-bot wants to merge 2 commits intohappier-dev:devfrom
Conversation
WalkthroughCentralizes change-title tool name aliases into a shared protocol module, replaces ad-hoc string checks with the alias predicate across CLI and UI, adds a dynamic change-title instruction builder, and introduces a heuristic mapping change_title->Task in OpenCode transport with accompanying tests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryThis PR improves
Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| apps/cli/src/agent/runtime/changeTitleInstruction.ts | Instruction text updated to be provider-agnostic: no longer references non-existent functions.happier__change_title, instead uses generic phrasing with a preference for mcp__happier__change_title. Clear and well-documented. |
| apps/cli/src/agent/tools/normalization/index.ts | Extended canonicalizeToolNameV2 to normalize three additional change_title aliases (happier__change_title, happy__change_title, change_title) to the canonical change_title name. Uses existing lower variable for case-insensitive matching, consistent with surrounding code. |
| apps/cli/src/backends/opencode/acp/transport.ts | Added heuristic fallback in determineToolName to detect task-like change_title calls by checking for prompt/subagent_type fields in the absence of a title field. The heuristic is documented and tested. Minor concern: hasPrompt alone (without hasSubagentType) is sufficient to trigger Task classification, which is intentionally broad but could theoretically mis-classify edge cases. |
| apps/cli/src/backends/opencode/acp/transport.test.ts | Two new test cases added: one for the heuristic fallback (input with prompt + subagent_type but no title maps to Task) and one ensuring explicit title input stays as change_title. Good coverage of both positive and negative heuristic paths. |
| apps/ui/sources/components/tools/normalization/normalize/nameInference.ts | Extended canonicalizeToolNameNonV2 with three additional change_title aliases, matching the CLI normalization. Uses exact case comparison (pre-existing pattern in this function). |
| apps/ui/sources/sync/reducer/messageToEvent.ts | Added three new entries to CHANGE_TITLE_TOOL_NAMES Set: happier__change_title, happy__change_title, and change_title. Well-commented with provider context. |
| apps/ui/sources/sync/reducer/messageToEvent.test.ts | Test expanded from 2 tool name variants to 5, covering all CHANGE_TITLE_TOOL_NAMES entries. Refactored to loop-based assertion for cleaner coverage. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Tool call arrives with toolName"] --> B{"Is toolName a change_title variant?"}
B -- "No" --> C["Standard canonicalization pipeline"]
B -- "Yes (any of 5 aliases)" --> D["CLI/UI: Canonicalize to 'change_title'"]
D --> E{"OpenCode ACP transport?"}
E -- "No" --> F["Render as title change event"]
E -- "Yes" --> G{"Has _acp.title == 'task'?"}
G -- "Yes" --> H["Map to 'Task'"]
G -- "No" --> I{"Has prompt/subagent_type WITHOUT title?"}
I -- "Yes (heuristic)" --> H
I -- "No" --> F
style H fill:#4CAF50,color:#fff
style F fill:#2196F3,color:#fff
style D fill:#FF9800,color:#fff
Last reviewed commit: 7254216
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/cli/src/agent/tools/normalization/index.ts (1)
279-286: Indentation mismatch with surrounding code.The new block uses 2-space indentation; the rest of
canonicalizeToolNameV2(and the whole file) uses 4-space.♻️ Proposed fix
- if ( - lower === 'mcp__happier__change_title' || - lower === 'mcp__happy__change_title' || - lower === 'happier__change_title' || - lower === 'happy__change_title' || - lower === 'change_title' - ) - return 'change_title'; + if ( + lower === 'mcp__happier__change_title' || + lower === 'mcp__happy__change_title' || + lower === 'happier__change_title' || + lower === 'happy__change_title' || + lower === 'change_title' + ) return 'change_title';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/agent/tools/normalization/index.ts` around lines 279 - 286, The new conditional block in canonicalizeToolNameV2 is indented with 2 spaces which breaks consistency—update the block so all lines use 4-space indentation to match the surrounding code style (including the if (...) lines and the return 'change_title'; line), ensuring the conditional parentheses and the return statement align with other blocks in the function.apps/ui/sources/sync/reducer/messageToEvent.test.ts (1)
20-34: Considerit.eachfor per-variant failure attribution.With a
forloop, the first failingexpecthalts the test and leaves the remaining four variants untested. Switching toit.eachgives an independent result for each name, making regressions easier to pinpoint.♻️ Proposed refactor
- it('supports legacy + new + canonical change_title tool names', () => { - const events = [ - parseMessageAsEvent(makeToolCallMessage('mcp__happy__change_title')), - parseMessageAsEvent(makeToolCallMessage('mcp__happier__change_title')), - parseMessageAsEvent(makeToolCallMessage('happier__change_title')), - parseMessageAsEvent(makeToolCallMessage('happy__change_title')), - parseMessageAsEvent(makeToolCallMessage('change_title')), - ]; - - for (const event of events) { - expect(event).toEqual({ - type: 'message', - message: 'Title changed to "My title"', - }); - } - }); + it.each([ + 'mcp__happy__change_title', + 'mcp__happier__change_title', + 'happier__change_title', + 'happy__change_title', + 'change_title', + ])('recognises "%s" as a change_title tool', (toolName) => { + expect(parseMessageAsEvent(makeToolCallMessage(toolName))).toEqual({ + type: 'message', + message: 'Title changed to "My title"', + }); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/sources/sync/reducer/messageToEvent.test.ts` around lines 20 - 34, The test groups multiple tool-name variants into a single `for` loop which stops at the first failure; replace that loop with Jest's table-driven tests using `it.each` so each variant is its own test case. Iterate over the array of names (the values passed into `makeToolCallMessage`) with `it.each(names)(...)`, call `parseMessageAsEvent(makeToolCallMessage(name))` inside the test body, and assert the expected object (`type: 'message', message: 'Title changed to "My title"'`) for each; this preserves the same logic but gives per-variant failure attribution for easier debugging.apps/cli/src/backends/opencode/acp/transport.ts (1)
197-200: Unnecessaryas anycasts —inputis alreadyRecord<string, unknown>.
inputis typed asRecord<string, unknown>, soinput.prompt,input.subagent_type, andinput.titleare valid property accesses returningunknown. The(input as any)?.casts are redundant and violate the no-untyped-escapes guideline.♻️ Proposed fix
- const hasPrompt = typeof (input as any)?.prompt === 'string' && String((input as any).prompt).trim().length > 0; - const hasSubagentType = - typeof (input as any)?.subagent_type === 'string' && String((input as any).subagent_type).trim().length > 0; - const hasTitle = typeof (input as any)?.title === 'string' && String((input as any).title).trim().length > 0; + const hasPrompt = typeof input.prompt === 'string' && (input.prompt as string).trim().length > 0; + const hasSubagentType = + typeof input.subagent_type === 'string' && (input.subagent_type as string).trim().length > 0; + const hasTitle = typeof input.title === 'string' && (input.title as string).trim().length > 0;As per coding guidelines: "Broad
as anycasts are forbidden except in boundary fixtures with a one-line justification."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/backends/opencode/acp/transport.ts` around lines 197 - 200, The three boolean guards (hasPrompt, hasSubagentType, hasTitle) currently use redundant (input as any)?. casts; remove those casts and directly access the properties on input, relying on TypeScript's typeof narrowing (e.g. typeof input['prompt'] === 'string' && input['prompt'].trim().length > 0) for each check; update hasPrompt, hasSubagentType, and hasTitle to use input['prompt'], input['subagent_type'], and input['title'] (or input.prompt etc.) with typeof checks so no broad as any cast is needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/cli/src/agent/tools/normalization/index.ts`:
- Around line 279-286: The new conditional block in canonicalizeToolNameV2 is
indented with 2 spaces which breaks consistency—update the block so all lines
use 4-space indentation to match the surrounding code style (including the if
(...) lines and the return 'change_title'; line), ensuring the conditional
parentheses and the return statement align with other blocks in the function.
In `@apps/cli/src/backends/opencode/acp/transport.ts`:
- Around line 197-200: The three boolean guards (hasPrompt, hasSubagentType,
hasTitle) currently use redundant (input as any)?. casts; remove those casts and
directly access the properties on input, relying on TypeScript's typeof
narrowing (e.g. typeof input['prompt'] === 'string' &&
input['prompt'].trim().length > 0) for each check; update hasPrompt,
hasSubagentType, and hasTitle to use input['prompt'], input['subagent_type'],
and input['title'] (or input.prompt etc.) with typeof checks so no broad as any
cast is needed.
In `@apps/ui/sources/sync/reducer/messageToEvent.test.ts`:
- Around line 20-34: The test groups multiple tool-name variants into a single
`for` loop which stops at the first failure; replace that loop with Jest's
table-driven tests using `it.each` so each variant is its own test case. Iterate
over the array of names (the values passed into `makeToolCallMessage`) with
`it.each(names)(...)`, call `parseMessageAsEvent(makeToolCallMessage(name))`
inside the test body, and assert the expected object (`type: 'message', message:
'Title changed to "My title"'`) for each; this preserves the same logic but
gives per-variant failure attribution for easier debugging.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
apps/cli/src/backends/gemini/utils/permissionHandler.ts (1)
24-33: Spreading all 5 aliases into substring-match arrays creates redundant entries.Both
alwaysAutoApproveToolNameIncludesandalwaysAutoApproveToolCallIdIncludesare checked with.some((t) => lower.includes(t)). Because all longer aliases ('mcp__happier__change_title', etc.) contain'change_title'as a substring, the shortest alias already covers the entire alias list. The spread doesn't add correctness — only noise in the array.That said, the current behavior is not incorrect, and the comment about "permissive" intent is clear. Consider keeping just
'change_title'(plus itsisChangeTitleToolNameAliashelper if you want exact-match coverage elsewhere):♻️ Optional simplification
- this.alwaysAutoApproveToolNameIncludes = [ - ...CHANGE_TITLE_TOOL_NAME_ALIASES, - 'geminireasoning', - 'codexreasoning', - ]; - this.alwaysAutoApproveToolCallIdIncludes = [ - // Some transports only expose the tool in the toolCallId; keep it permissive. - ...CHANGE_TITLE_TOOL_NAME_ALIASES, - 'save_memory', - ]; + this.alwaysAutoApproveToolNameIncludes = [ + 'change_title', // substring covers all MCP/legacy aliases + 'geminireasoning', + 'codexreasoning', + ]; + this.alwaysAutoApproveToolCallIdIncludes = [ + // Some transports only expose the tool in the toolCallId; keep it permissive. + 'change_title', // substring covers all MCP/legacy aliases + 'save_memory', + ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/backends/gemini/utils/permissionHandler.ts` around lines 24 - 33, The arrays alwaysAutoApproveToolNameIncludes and alwaysAutoApproveToolCallIdIncludes redundantly spread CHANGE_TITLE_TOOL_NAME_ALIASES; reduce noise by replacing the spread with the minimal substring "change_title" (retain other explicit entries like 'geminireasoning', 'codexreasoning', 'save_memory'), and keep the isChangeTitleToolNameAlias helper (or the full CHANGE_TITLE_TOOL_NAME_ALIASES constant) for any places that need exact-match checks instead of substring matching.packages/protocol/src/tools/v2/aliases.ts (1)
27-29:isChangeTitleToolNameAliassilently returnsfalsefor mixed-case input — self-normalize or document the contract.All entries in
CHANGE_TITLE_TOOL_NAME_ALIASESare lowercase. Callers that forget to pre-normalize will get a silent false negative (e.g.,isChangeTitleToolNameAlias('Change_Title')→false). Current callers in this PR do normalize, but this is an invisible contract with no enforcement.The simplest fix is to fold the normalization into the function itself:
🛡️ Proposed fix
export function isChangeTitleToolNameAlias(name: string): boolean { - return (CHANGE_TITLE_TOOL_NAME_ALIASES as readonly string[]).includes(name); + return (CHANGE_TITLE_TOOL_NAME_ALIASES as readonly string[]).includes(name.toLowerCase()); }If keeping the current contract, add a JSDoc
@paramnote such as@param name - must be pre-normalized to lowercase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/protocol/src/tools/v2/aliases.ts` around lines 27 - 29, The function isChangeTitleToolNameAlias currently does a case-sensitive lookup against CHANGE_TITLE_TOOL_NAME_ALIASES (all lowercase) which causes silent false negatives for mixed-case input; update isChangeTitleToolNameAlias to normalize the input (e.g., use name.toLowerCase()) before checking includes (i.e., call (CHANGE_TITLE_TOOL_NAME_ALIASES as readonly string[]).includes(name.toLowerCase())), or alternatively add a JSDoc `@param` noting the caller must pass a lowercase name—prefer implementing the normalization inside isChangeTitleToolNameAlias for safety.apps/cli/src/agent/runtime/changeTitleInstruction.ts (1)
13-13: Hardcoded default'mcp__happier__change_title'could drift fromCHANGE_TITLE_TOOL_NAME_ALIASES.The fallback default on line 13 duplicates the string already present at
CHANGE_TITLE_TOOL_NAME_ALIASES[1]. If the alias list is ever reordered or the preferred MCP name changes, this default silently diverges.♻️ Suggested improvement
+// CHANGE_TITLE_TOOL_NAME_ALIASES[1] is the preferred MCP-prefixed name +const DEFAULT_PREFERRED_TOOL_NAME = CHANGE_TITLE_TOOL_NAME_ALIASES[1]; // 'mcp__happier__change_title' export const buildChangeTitleInstruction = (opts: ChangeTitleInstructionOptions = {}): string => { - const preferred = (opts.preferredToolName ?? 'mcp__happier__change_title').trim(); + const preferred = (opts.preferredToolName ?? DEFAULT_PREFERRED_TOOL_NAME).trim();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/agent/runtime/changeTitleInstruction.ts` at line 13, The default fallback for preferred tool name is hardcoded to 'mcp__happier__change_title' which duplicates an entry in CHANGE_TITLE_TOOL_NAME_ALIASES and may drift; change the fallback to use the alias array instead (use CHANGE_TITLE_TOOL_NAME_ALIASES[1] or a safe alias lookup) when computing preferred from opts.preferredToolName, and ensure you still call .trim() and handle the case where the alias array may be missing or empty (fall back to a sensible alias or throw a clear error).packages/protocol/src/tools/v2/index.ts (1)
20-20:ChangeTitleToolNameAliasSchemais not re-exported, leaving the runtime validator inaccessible through the public index.
aliases.tsexports both the schema (ChangeTitleToolNameAliasSchema) and the type (ChangeTitleToolNameAlias), but only the type is re-exported here. Any downstream consumer that needs to validate a value at runtime (.parse()/.safeParse()) is blocked from doing so through the standard package entry point.♻️ Proposed addition
-export { CHANGE_TITLE_TOOL_NAME_ALIASES, isChangeTitleToolNameAlias, type ChangeTitleToolNameAlias } from './aliases.js'; +export { + CHANGE_TITLE_TOOL_NAME_ALIASES, + ChangeTitleToolNameAliasSchema, + isChangeTitleToolNameAlias, + type ChangeTitleToolNameAlias, +} from './aliases.js';This also brings the export block in line with the multi-line style used by every other export group in the file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/protocol/src/tools/v2/index.ts` at line 20, The public index currently re-exports the type ChangeTitleToolNameAlias but not the runtime validator ChangeTitleToolNameAliasSchema; update the export group to also export ChangeTitleToolNameAliasSchema from './aliases.js' so downstream consumers can call .parse()/ .safeParse(); ensure the export statement includes CHANGE_TITLE_TOOL_NAME_ALIASES, isChangeTitleToolNameAlias, ChangeTitleToolNameAliasSchema, and type ChangeTitleToolNameAlias (matching the multi-line style used elsewhere).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/cli/src/agent/runtime/changeTitleInstruction.ts`:
- Around line 4-10: Replace the exported type alias
ChangeTitleInstructionOptions with an exported interface of the same name to
follow the coding guideline; keep the identical property signature
preferredToolName?: string and the JSDoc comment (including the default note),
and update any local references/usages to continue to refer to
ChangeTitleInstructionOptions (no other API changes required).
- Around line 12-24: Add a top-level JSDoc block describing this module's
responsibility (e.g., that it builds a localized instruction string for the
change-title tool and lists exported helpers) and place it before any
imports/exports; then convert the exported function declaration
buildChangeTitleInstruction to an exported arrow function expression (preserving
the same parameter name ChangeTitleInstructionOptions, defaulting behavior,
usages of preferred, fallbacks, fallbackPreview, and the trimIdent return) to
satisfy the arrow-function guideline.
In `@apps/cli/src/backends/gemini/runtime/geminiTurnMessageState.ts`:
- Line 23: The import statement for hasChangeTitleInstruction was inserted
mid-file between function definitions; move the import for
hasChangeTitleInstruction to the top of the module (with the other imports) so
all imports are declared before any code or function/class declarations (look
for hasChangeTitleInstruction and the surrounding functions/types in
geminiTurnMessageState.ts and relocate that import to the file header).
---
Nitpick comments:
In `@apps/cli/src/agent/runtime/changeTitleInstruction.ts`:
- Line 13: The default fallback for preferred tool name is hardcoded to
'mcp__happier__change_title' which duplicates an entry in
CHANGE_TITLE_TOOL_NAME_ALIASES and may drift; change the fallback to use the
alias array instead (use CHANGE_TITLE_TOOL_NAME_ALIASES[1] or a safe alias
lookup) when computing preferred from opts.preferredToolName, and ensure you
still call .trim() and handle the case where the alias array may be missing or
empty (fall back to a sensible alias or throw a clear error).
In `@apps/cli/src/backends/gemini/utils/permissionHandler.ts`:
- Around line 24-33: The arrays alwaysAutoApproveToolNameIncludes and
alwaysAutoApproveToolCallIdIncludes redundantly spread
CHANGE_TITLE_TOOL_NAME_ALIASES; reduce noise by replacing the spread with the
minimal substring "change_title" (retain other explicit entries like
'geminireasoning', 'codexreasoning', 'save_memory'), and keep the
isChangeTitleToolNameAlias helper (or the full CHANGE_TITLE_TOOL_NAME_ALIASES
constant) for any places that need exact-match checks instead of substring
matching.
In `@packages/protocol/src/tools/v2/aliases.ts`:
- Around line 27-29: The function isChangeTitleToolNameAlias currently does a
case-sensitive lookup against CHANGE_TITLE_TOOL_NAME_ALIASES (all lowercase)
which causes silent false negatives for mixed-case input; update
isChangeTitleToolNameAlias to normalize the input (e.g., use name.toLowerCase())
before checking includes (i.e., call (CHANGE_TITLE_TOOL_NAME_ALIASES as readonly
string[]).includes(name.toLowerCase())), or alternatively add a JSDoc `@param`
noting the caller must pass a lowercase name—prefer implementing the
normalization inside isChangeTitleToolNameAlias for safety.
In `@packages/protocol/src/tools/v2/index.ts`:
- Line 20: The public index currently re-exports the type
ChangeTitleToolNameAlias but not the runtime validator
ChangeTitleToolNameAliasSchema; update the export group to also export
ChangeTitleToolNameAliasSchema from './aliases.js' so downstream consumers can
call .parse()/ .safeParse(); ensure the export statement includes
CHANGE_TITLE_TOOL_NAME_ALIASES, isChangeTitleToolNameAlias,
ChangeTitleToolNameAliasSchema, and type ChangeTitleToolNameAlias (matching the
multi-line style used elsewhere).
Comment on lines
+4
to
+10
| export type ChangeTitleInstructionOptions = { | ||
| /** | ||
| * Preferred tool name to mention first. | ||
| * Defaults to `mcp__happier__change_title` (MCP convention). | ||
| */ | ||
| preferredToolName?: string; | ||
| }; |
There was a problem hiding this comment.
Use interface instead of type for ChangeTitleInstructionOptions.
As per coding guidelines, prefer interface over type for defining object shapes in TypeScript.
♻️ Proposed fix
-export type ChangeTitleInstructionOptions = {
- /**
- * Preferred tool name to mention first.
- * Defaults to `mcp__happier__change_title` (MCP convention).
- */
- preferredToolName?: string;
-};
+export interface ChangeTitleInstructionOptions {
+ /**
+ * Preferred tool name to mention first.
+ * Defaults to `mcp__happier__change_title` (MCP convention).
+ */
+ preferredToolName?: string;
+}As per coding guidelines: "Prefer interface over type for defining object shapes in TypeScript."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/cli/src/agent/runtime/changeTitleInstruction.ts` around lines 4 - 10,
Replace the exported type alias ChangeTitleInstructionOptions with an exported
interface of the same name to follow the coding guideline; keep the identical
property signature preferredToolName?: string and the JSDoc comment (including
the default note), and update any local references/usages to continue to refer
to ChangeTitleInstructionOptions (no other API changes required).
Comment on lines
+12
to
+24
| export function buildChangeTitleInstruction(opts: ChangeTitleInstructionOptions = {}): string { | ||
| const preferred = (opts.preferredToolName ?? 'mcp__happier__change_title').trim(); | ||
| const fallbacks = CHANGE_TITLE_TOOL_NAME_ALIASES.filter((n) => n !== preferred); | ||
| const fallbackPreview = fallbacks.slice(0, 3).join(', '); | ||
|
|
||
| return trimIdent( | ||
| `Based on the user's message, use the chat title tool to set (or update) a short, descriptive session title. | ||
|
|
||
| The tool may be exposed under different names depending on the provider. Prefer "${preferred}" when available; otherwise use an equivalent alias (for example: ${fallbackPreview}). | ||
|
|
||
| Call this tool again if the task changes significantly.` | ||
| ); | ||
| } |
There was a problem hiding this comment.
Missing file-level JSDoc header and export function should be an arrow function.
Two guideline violations in the new function:
-
File header JSDoc: The file has no top-level JSDoc block describing its responsibilities. The guidelines require: "Include comprehensive JSDoc comments as file header comments explaining file responsibilities."
-
Arrow function preferred:
export function buildChangeTitleInstructionis a function declaration. Guidelines say "Prefer arrow functions over function declarations."
♻️ Proposed fixes
Add a file-level JSDoc before the import:
+/**
+ * changeTitleInstruction
+ *
+ * Builds provider-agnostic prompts instructing the agent to call the
+ * change-title tool, with dynamic alias fallbacks. Exports both the
+ * builder and a pre-built default constant for backend injection.
+ */
import { CHANGE_TITLE_TOOL_NAME_ALIASES } from '@happier-dev/protocol/tools/v2';Convert the function declaration to an arrow function:
-export function buildChangeTitleInstruction(opts: ChangeTitleInstructionOptions = {}): string {
+export const buildChangeTitleInstruction = (opts: ChangeTitleInstructionOptions = {}): string => {
const preferred = (opts.preferredToolName ?? 'mcp__happier__change_title').trim();
const fallbacks = CHANGE_TITLE_TOOL_NAME_ALIASES.filter((n) => n !== preferred);
const fallbackPreview = fallbacks.slice(0, 3).join(', ');
return trimIdent(
`Based on the user's message, use the chat title tool to set (or update) a short, descriptive session title.
The tool may be exposed under different names depending on the provider. Prefer "${preferred}" when available; otherwise use an equivalent alias (for example: ${fallbackPreview}).
Call this tool again if the task changes significantly.`
);
-}
+};As per coding guidelines: "Include comprehensive JSDoc comments as file header comments explaining file responsibilities" and "Prefer arrow functions over function declarations."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/cli/src/agent/runtime/changeTitleInstruction.ts` around lines 12 - 24,
Add a top-level JSDoc block describing this module's responsibility (e.g., that
it builds a localized instruction string for the change-title tool and lists
exported helpers) and place it before any imports/exports; then convert the
exported function declaration buildChangeTitleInstruction to an exported arrow
function expression (preserving the same parameter name
ChangeTitleInstructionOptions, defaulting behavior, usages of preferred,
fallbacks, fallbackPreview, and the trimIdent return) to satisfy the
arrow-function guideline.
| }; | ||
| } | ||
|
|
||
| import { hasChangeTitleInstruction } from '../utils/promptUtils'; |
There was a problem hiding this comment.
Import placed mid-file — must be moved to the top.
The hasChangeTitleInstruction import sits between two function definitions (after line 21, before line 25), violating the explicit rule: "ALL imports must be at the top of the file - NEVER import modules mid-code."
🔧 Proposed fix
Move the import to the top of the file, before all declarations:
+import { hasChangeTitleInstruction } from '../utils/promptUtils';
+
export type GeminiTurnMessageState = {
thinking: boolean;
...
};
export function createGeminiTurnMessageState(): GeminiTurnMessageState { ... }
-import { hasChangeTitleInstruction } from '../utils/promptUtils';
-
export function resetGeminiTurnMessageStateForPrompt(As per coding guidelines: "ALL imports must be at the top of the file - NEVER import modules mid-code."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/cli/src/backends/gemini/runtime/geminiTurnMessageState.ts` at line 23,
The import statement for hasChangeTitleInstruction was inserted mid-file between
function definitions; move the import for hasChangeTitleInstruction to the top
of the module (with the other imports) so all imports are declared before any
code or function/class declarations (look for hasChangeTitleInstruction and the
surrounding functions/types in geminiTurnMessageState.ts and relocate that
import to the file header).
leeroybrun
added a commit
that referenced
this pull request
Mar 2, 2026
Ports and hardens the bot-generated PR changes against current dev worktree. - #60: clarify default model label + prune Claude model suggestions - #64: centralize change_title aliases + normalize across UI/CLI providers - #71: document skipping capability probe on Codex ACP resume - #78: stabilize iOS PathPicker header options while typing - #83: remove synthetic stop+explain follow-up; unify stop button copy/i18n
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #62.
Changes
CHANGE_TITLE_INSTRUCTIONprovider-robust (don’t reference non-existentfunctions.happier__change_title; prefermcp__happier__change_titlebut allow canonicalchange_title).change_titlealiases in CLI + UI (change_title,happier__change_title,happy__change_title, legacy MCP names).toolName=change_titleasTaskwhen the input shape looks like a task/subagent tool call (prevents tasks/subtasks being labeled "change title").Tests
yarn workspace @happier-dev/app test sources/sync/reducer/messageToEvent.test.tsyarn workspace @happier-dev/cli test:unitNote: in this container environment, two existing tests fail consistently due to process-tree termination behavior:
src/agent/acp/__tests__/killProcessTree.test.tssrc/agent/acp/__tests__/AcpBackend.dispose.killsProcessTree.test.tsThey appear unrelated to the changes in this PR.
Summary by CodeRabbit
New Features
Refactor
Tests