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' }
]);
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