feat(codex): support file mentions#774
Conversation
There was a problem hiding this comment.
Findings
- [Major] File mentions break for paths containing spaces — the autocomplete now inserts raw
@${file.fullPath}, butbuildUserInputFromMessage()parses mentions with@([^\s]+), so selecting a valid repo path likedocs/My File.mdsends Codex a mention for onlydocs/Myand leavesFile.mdas plain text. The file-search endpoint returns rawrg --filespaths, so spaces are valid here. Evidence:web/src/router.tsx:374,cli/src/codex/utils/appServerConfig.ts:97.
Suggested fix:const mentionText = /\s/.test(file.fullPath) ? `@"${file.fullPath.replace(/(["\\])/g, '\\$1')}"` : `@${file.fullPath}` return { key: mentionText, text: mentionText, label: `@${file.fileName}`, description: file.filePath || file.fullPath }
const mentionPattern = /(^|\s)@(?:"((?:\\.|[^"\\])*)"|([^\s]+))/g; const rawPath = match[2] ?? match[3] ?? ''; const path = rawPath.replace(/\\(["\\])/g, '$1'); // ... lastIndex = mentionPattern.lastIndex;
Questions
- None.
Summary
- Review mode: initial
- One issue found in the new file mention path handling. Residual risk: punctuation and manually typed mentions still have limited parser coverage.
Testing
- Not run:
bun test cli/src/codex/utils/appServerConfig.test.tsfailed becausebunis not installed in this runner.
HAPI Bot
| if (!response.success || !response.files) return [] | ||
| return response.files.map((file) => ({ | ||
| key: `@${file.fullPath}`, | ||
| text: `@${file.fullPath}`, |
There was a problem hiding this comment.
[MAJOR] File mentions break for paths containing spaces. This inserts raw @${file.fullPath}, but the Codex-side parser uses @([^\s]+), so selecting docs/My File.md sends a mention for only docs/My and leaves the rest as text. Quote whitespace-containing paths here and teach buildUserInputFromMessage() to decode quoted mentions.
Suggested fix:
const mentionText = /\s/.test(file.fullPath)
? `@"${file.fullPath.replace(/(["\\])/g, '\\$1')}"`
: `@${file.fullPath}`
return {
key: mentionText,
text: mentionText,
label: `@${file.fileName}`,
description: file.filePath || file.fullPath
}const mentionPattern = /(^|\s)@(?:"((?:\\.|[^"\\])*)"|([^\s]+))/g;
const rawPath = match[2] ?? match[3] ?? '';
const path = rawPath.replace(/\\(["\\])/g, '$1');
// ...
lastIndex = mentionPattern.lastIndex;|
Thanks, fixed in Verified:
|
There was a problem hiding this comment.
Findings
- [Minor] Unquoted mentions include trailing sentence punctuation —
buildUserInputFromMessage()uses[^\s]+for the unquoted path, so natural text likeplease inspect @src/index.ts.sends Codex a mention withpath: 'src/index.ts.'instead of keeping the period as text. That makes a valid file mention fail whenever the user ends the sentence with punctuation. Evidence:cli/src/codex/utils/appServerConfig.ts:97.
Suggested fix:const quotedPath = match[2]; const unquotedPath = match[3]; const rawPath = quotedPath ?? unquotedPath ?? ''; const pathText = quotedPath === undefined ? rawPath.replace(/[),.;!?]+$/, '') : rawPath; const path = pathText.replace(/\\(["\\])/g, '$1'); if (!path) continue; // ... lastIndex = mentionPattern.lastIndex - (rawPath.length - pathText.length);
Questions
- None.
Summary
Review mode: follow-up after new commits
- Prior whitespace-path finding is addressed. One remaining parser edge case can still turn common sentence punctuation into part of the file path.
Testing
- Not run:
bunis not installed in this runner.
HAPI Bot
|
|
||
| export function buildUserInputFromMessage(message: string): UserInput[] { | ||
| const inputs: UserInput[] = []; | ||
| const mentionPattern = /(^|\s)@(?:"((?:\\.|[^"\\])*)"|([^\s]+))/g; |
There was a problem hiding this comment.
[MINOR] Unquoted mentions include trailing sentence punctuation. Because this branch consumes every non-whitespace character, please inspect @src/index.ts. sends Codex a mention with path: 'src/index.ts.' instead of preserving the period as text, so a valid file mention fails in normal sentence text.
Suggested fix:
const quotedPath = match[2];
const unquotedPath = match[3];
const rawPath = quotedPath ?? unquotedPath ?? '';
const pathText = quotedPath === undefined
? rawPath.replace(/[),.;!?]+$/, '')
: rawPath;
const path = pathText.replace(/\\(["\\])/g, '$1');
if (!path) continue;
// ...
lastIndex = mentionPattern.lastIndex - (rawPath.length - pathText.length);|
Thanks, fixed the remaining parser edge case in Verified:
|
There was a problem hiding this comment.
Findings
- None.
Questions
- None.
Summary
- Review mode: follow-up after new commits
- Reviewed the full current diff plus prior bot review and compare context. No high-confidence issues found in changed lines.
- Residual risk: no web-specific test coverage for
@autocomplete insertion; CLI parser tests cover the changed mention-token cases.
Testing
- Not run (automation):
bunis not installed in this runner, sobun typecheckandcd cli && bun test src/codex/utils/appServerConfig.test.tscould not execute.
HAPI Bot
There was a problem hiding this comment.
Findings
- [Major] Literal
@tokens are now reinterpreted as file mentions —buildTurnStartParamsruns every remote Codex message throughbuildUserInputFromMessage, and that parser treats any whitespace-delimited@...token as{ type: 'mention' }. That changes normal prompts such asupgrade @types/nodeorexplain @hapi/protocolinto file mentions with paths liketypes/node, losing the literal package/scope text before Codex sees it. Evidence:cli/src/codex/utils/appServerConfig.ts:97.
Suggested fix:and make the web autocomplete always insert the quoted form:const mentionPattern = /(^|\s)@"((?:\\.|[^"\\])*)"/g;
Add a regression test thatconst mentionText = `@"${file.fullPath.replace(/(["\\])/g, '\\$1')}"`;
buildUserInputFromMessage('upgrade @types/node')returns a single text input.
Questions
- None.
Summary
- Review mode: follow-up after new commits
- Reviewed the full current diff, prior bot review, and compare context. One correctness regression found in the new mention parser.
Testing
- Not run (automation):
bunis not installed in this runner, sobun typecheckandcd cli && bun test src/codex/utils/appServerConfig.test.tscould not execute.
HAPI Bot
|
|
||
| export function buildUserInputFromMessage(message: string): UserInput[] { | ||
| const inputs: UserInput[] = []; | ||
| const mentionPattern = /(^|\s)@(?:"((?:\\.|[^"\\])*)"|([^\s]+))/g; |
There was a problem hiding this comment.
[MAJOR] This regex converts every whitespace-delimited @... token into a Codex file mention. That changes normal prompts such as upgrade @types/node or explain @hapi/protocol into mention inputs with paths like types/node, so the literal package/scope text is no longer sent as text.
Suggested fix: reserve mention parsing for the quoted syntax emitted by autocomplete, and quote every autocomplete insertion:
const mentionPattern = /(^|\s)@"((?:\\.|[^"\\])*)"/g;const mentionText = `@"${file.fullPath.replace(/(["\\])/g, '\\$1')}"`;Also add a regression test:
expect(buildUserInputFromMessage('upgrade @types/node')).toEqual([
{ type: 'text', text: 'upgrade @types/node' }
]);There was a problem hiding this comment.
Findings
- [Major] File-mention autocomplete is exposed to non-Codex sessions — this callback is passed to every
SessionChat, but the new@branch does not checkagentType. Selecting a file in a Claude/Gemini/Cursor/OpenCode session inserts@"path"; only the Codex app-server path added in this PR converts that token into a structured{ type: 'mention' }, while non-Codex launchers forward the text unchanged, so the UI advertises a file mention that those agents receive as literal syntax instead of attached context. Evidence:web/src/router.tsx:367.
Suggested fix:Also addif (query.startsWith('@')) { if (agentType !== 'codex') return [] if (!api || !sessionId) return [] const search = query.slice(1) const response = await api.searchSessionFiles(sessionId, search, 50) if (!response.success || !response.files) return [] return response.files.map((file) => { const mentionText = `@"${file.fullPath.replace(/(["\\])/g, '\\$1')}"` return { key: mentionText, text: mentionText, label: `@${file.fileName}`, description: file.filePath || file.fullPath } }) }
agentTypeto the callback dependency list.
Questions
- None.
Summary
- Review mode: follow-up after new commits
- Reviewed the full current diff, prior HAPI Bot review, and compare context. The previous literal-
@parser regression is addressed, but the web autocomplete now exposes Codex-only mention syntax in every agent session.
Testing
- Not run (automation):
bunis not installed in this runner, sobun typecheckandcd cli && bun test src/codex/utils/appServerConfig.test.tscould not execute.
HAPI Bot
| } = useSkills(api, sessionId) | ||
|
|
||
| const getAutocompleteSuggestions = useCallback(async (query: string) => { | ||
| if (query.startsWith('@')) { |
There was a problem hiding this comment.
[MAJOR] This @ branch runs for every session because the same autocompleteSuggestions callback is passed to SessionChat regardless of session.metadata.flavor. Selecting a suggestion in a Claude/Gemini/Cursor/OpenCode session inserts @"path", but only the Codex app-server turn builder added in this PR converts that syntax into a structured mention; the other launchers forward the composer text unchanged. That makes the UI advertise file mention support where the agent only receives a literal quoted token, not file context.
Suggested fix: gate file mentions to Codex sessions and include agentType in the dependency list:
if (query.startsWith('@')) {
if (agentType !== 'codex') return []
if (!api || !sessionId) return []
const search = query.slice(1)
const response = await api.searchSessionFiles(sessionId, search, 50)
if (!response.success || !response.files) return []
return response.files.map((file) => {
const mentionText = `@"${file.fullPath.replace(/(["\\])/g, '\\$1')}"`
return {
key: mentionText,
text: mentionText,
label: `@${file.fileName}`,
description: file.filePath || file.fullPath
}
})
}}, [api, sessionId, agentType, getSkillSuggestions, getSlashSuggestions])There was a problem hiding this comment.
Findings
- No issues found.
Questions
- None.
Summary
- Review mode: follow-up after new commits
- Reviewed the full current diff plus the prior HAPI Bot review and compare diff. The previous Codex-only autocomplete gating issue is addressed; no new high-confidence correctness, security, data-loss, performance, or maintainability issues found in the modified lines.
Testing
- Not run (automation):
bunis not installed in this runner, so I could not executebun typecheckorcd cli && bun test src/codex/utils/appServerConfig.test.ts. - Residual gap: no web test coverage for the new Codex-only
@autocomplete branch.
HAPI Bot
Summary
mentionuser input support@pathtokens into mention inputs when starting Codex turns@autocomplete suggestions from session file search in the web composerTests
bun typecheckcd cli && bun test src/codex/utils/appServerConfig.test.ts