Skip to content

feat(codex): support file mentions#774

Open
NightWatcher314 wants to merge 6 commits into
tiann:mainfrom
NightWatcher314:feat/codex-file-mentions
Open

feat(codex): support file mentions#774
NightWatcher314 wants to merge 6 commits into
tiann:mainfrom
NightWatcher314:feat/codex-file-mentions

Conversation

@NightWatcher314

Copy link
Copy Markdown
Contributor

Summary

  • add Codex app-server mention user input support
  • parse @path tokens into mention inputs when starting Codex turns
  • surface @ autocomplete suggestions from session file search in the web composer

Tests

  • bun typecheck
  • cd cli && bun test src/codex/utils/appServerConfig.test.ts

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Major] File mentions break for paths containing spaces — the autocomplete now inserts raw @${file.fullPath}, but buildUserInputFromMessage() parses mentions with @([^\s]+), so selecting a valid repo path like docs/My File.md sends Codex a mention for only docs/My and leaves File.md as plain text. The file-search endpoint returns raw rg --files paths, 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.ts failed because bun is not installed in this runner.

HAPI Bot

Comment thread web/src/router.tsx Outdated
if (!response.success || !response.files) return []
return response.files.map((file) => ({
key: `@${file.fullPath}`,
text: `@${file.fullPath}`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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;

@NightWatcher314

Copy link
Copy Markdown
Contributor Author

Thanks, fixed in 6c130f4: autocomplete now quotes paths containing whitespace and the Codex input parser supports quoted/escaped file mentions.

Verified:

  • bun typecheck
  • cd cli && bun test src/codex/utils/appServerConfig.test.ts

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Minor] Unquoted mentions include trailing sentence punctuation — buildUserInputFromMessage() uses [^\s]+ for the unquoted path, so natural text like please inspect @src/index.ts. sends Codex a mention with path: '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: bun is not installed in this runner.

HAPI Bot

Comment thread cli/src/codex/utils/appServerConfig.ts Outdated

export function buildUserInputFromMessage(message: string): UserInput[] {
const inputs: UserInput[] = [];
const mentionPattern = /(^|\s)@(?:"((?:\\.|[^"\\])*)"|([^\s]+))/g;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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);

@NightWatcher314

Copy link
Copy Markdown
Contributor Author

Thanks, fixed the remaining parser edge case in 728fbe7: unquoted mentions now keep trailing sentence punctuation outside the file path, while quoted mentions remain unchanged.

Verified:

  • bun typecheck
  • cd cli && bun test src/codex/utils/appServerConfig.test.ts

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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): bun is not installed in this runner, so bun typecheck and cd cli && bun test src/codex/utils/appServerConfig.test.ts could not execute.

HAPI Bot

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Major] Literal @ tokens are now reinterpreted as file mentions — buildTurnStartParams runs every remote Codex message through buildUserInputFromMessage, and that parser treats any whitespace-delimited @... token as { type: 'mention' }. That changes normal prompts such as upgrade @types/node or explain @hapi/protocol into file mentions with paths like types/node, losing the literal package/scope text before Codex sees it. Evidence: cli/src/codex/utils/appServerConfig.ts:97.
    Suggested fix:
    const mentionPattern = /(^|\s)@"((?:\\.|[^"\\])*)"/g;
    and make the web autocomplete always insert the quoted form:
    const mentionText = `@"${file.fullPath.replace(/(["\\])/g, '\\$1')}"`;
    Add a regression test that 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): bun is not installed in this runner, so bun typecheck and cd cli && bun test src/codex/utils/appServerConfig.test.ts could not execute.

HAPI Bot

Comment thread cli/src/codex/utils/appServerConfig.ts Outdated

export function buildUserInputFromMessage(message: string): UserInput[] {
const inputs: UserInput[] = [];
const mentionPattern = /(^|\s)@(?:"((?:\\.|[^"\\])*)"|([^\s]+))/g;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Major] File-mention autocomplete is exposed to non-Codex sessions — this callback is passed to every SessionChat, but the new @ branch does not check agentType. 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:
    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
            }
        })
    }
    Also add agentType to 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): bun is not installed in this runner, so bun typecheck and cd cli && bun test src/codex/utils/appServerConfig.test.ts could not execute.

HAPI Bot

Comment thread web/src/router.tsx
} = useSkills(api, sessionId)

const getAutocompleteSuggestions = useCallback(async (query: string) => {
if (query.startsWith('@')) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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])

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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): bun is not installed in this runner, so I could not execute bun typecheck or cd cli && bun test src/codex/utils/appServerConfig.test.ts.
  • Residual gap: no web test coverage for the new Codex-only @ autocomplete branch.

HAPI Bot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant