Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ec466b539
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // Unicode path segments and symlinks; individual handlers still enforce | ||
| // file-vs-directory checks, size caps, hidden directory skips and binary | ||
| // detection where appropriate. | ||
| return { realPath: real, readOnly: true }; |
There was a problem hiding this comment.
Restrict preview reads to trusted roots
When the renderer invokes the allowlisted file:readText, file:readBinary, file:listDir, or file:listTree IPC channels, this read-mode fallback now accepts any real local path instead of the previous app/resource roots. In a chat-renderer context, any injected renderer code or malicious content that reaches the preload invoke API can read/list arbitrary files the user account can access (subject only to size caps), so the preview IPC is no longer sandboxed despite the preload comment. Please keep a read allowlist or require an explicit user-granted path before returning success here.
Useful? React with 👍 / 👎.
| const revealOnly = supportsRichDocumentPreview(file.ext); | ||
| if (revealOnly) { |
There was a problem hiding this comment.
Let rich generated files open the preview panel
For PDF/XLSX files generated by a run, this branch bypasses onOpen and only reveals the file in the system file manager, so the primary generated-file pill cannot use the new in-app PDF/Excel preview. ArtifactPanel already redirects rich documents opened through openChanges to the preview tab, so sending these cards through onOpen(file) would preserve the intended preview flow instead of forcing users out of the app.
Useful? React with 👍 / 👎.
| if (selectedNode.contentType === 'snapshot') { | ||
| return <ImageViewer filePath={selectedNode.absPath} fileName={selectedNode.name} />; | ||
| } | ||
| if (isPdfPreviewExt(selectedNode.ext)) { |
There was a problem hiding this comment.
Check rich-file size state before mounting viewers
When a workspace PDF or spreadsheet is larger than RICH_PREVIEW_MAX_BYTES, the selection effect sets fileState to tooLarge, but renderBody returns the rich viewer before it checks fileState.status. In that scenario the direct-open fallback UI below is unreachable; the viewer simply calls readBinaryFile, gets tooLarge, and shows a dead-end message without the open-with-system-app action. Check fileState.status before these rich-preview returns, or gate the viewer on fileState.status === 'ready'.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d156a976c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| function stripAssistantMediaTags(text: string): string { | ||
| if (!text) return text; | ||
| const exts = 'png|jpe?g|gif|webp|bmp|avif|svg|pdf|docx?|xlsx?|pptx?|txt|csv|md|rtf|epub|zip|tar|gz|rar|7z|mp3|wav|ogg|aac|flac|m4a|mp4|mov|avi|mkv|webm|m4v'; | ||
| const tagged = new RegExp(`(^|[\\s(\\[{>])(?:MEDIA|media):(?:\\/|~\\/)[^\\s\\n"'()\\[\\],<>]*?\\.(?:${exts})`, 'g'); |
There was a problem hiding this comment.
Strip Windows MEDIA artifact markers
When the runtime emits a Windows artifact marker such as MEDIA:C:\Users\me\report.xlsx, extractRawFilePaths in src/stores/chat/helpers.ts still promotes the drive-letter path to an attachment via its winRegex, but this strip regex only matches MEDIA: followed by / or ~/. On Windows those generated PDF/XLSX artifacts will show both the file card and the raw MEDIA:C:\... marker in the assistant bubble instead of hiding the marker as intended; include drive-letter paths in this matcher as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7eee922398
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // Unix absolute paths (/... or ~/...) — lookbehind rejects mid-token slashes | ||
| // (e.g. "path/to/file.mp4", "https://example.com/file.mp4") | ||
| const unixRegex = new RegExp(`(?<![\\w./:])((?:\\/|~\\/)[^\\s\\n"'()\\[\\],<>]*?\\.(?:${exts}))`, 'gi'); | ||
| const unixRegex = new RegExp(`(?<![\\w./:])((?:\\/|~\\/)[^\\s\\n"'()\`\\[\\],<>]*?\\.(?:${exts}))`, 'gi'); |
There was a problem hiding this comment.
Handle MEDIA-prefixed paths in the active chat store
When history/tool-result text contains a runtime artifact marker like MEDIA:/Users/me/report.xlsx, this active @/stores/chat extractor still only runs the generic Unix/Windows regexes; the Unix regex's lookbehind rejects the / because it follows MEDIA:, so no _attachedFiles card is created after reload. The tagged MEDIA: handling added in src/stores/chat/helpers.ts is not used by the Chat page (which imports this file), so generated PDF/XLSX artifacts emitted with these markers can disappear instead of opening in the new preview flow.
Useful? React with 👍 / 👎.
No description provided.