Conversation
github-actions
bot
added
tui
There was a problem hiding this comment.
Pull request overview
Adds a reusable path-normalization helper to reliably convert terminal drag-and-drop/pasted file references into attachable filesystem paths, and wires paste handling into the Check-ins answering composer to match other views.
Changes:
- Introduces
richtext.NormalizeDragPathto normalize quoted paths, shell-escaped paths,file://URLs, and~/expansion for drag-and-drop attachments. - Updates
Composer.ProcessPasteto useNormalizeDragPathinstead of inline tilde expansion. - Adds
tea.PasteMsghandling to the Check-ins view and expands unit tests for paste/attachment behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/tui/workspace/widget/composer.go | Routes pasted lines through richtext.NormalizeDragPath before attempting os.Stat for attachments. |
| internal/tui/workspace/widget/composer_test.go | Adds coverage for shell-escaped, quoted, and file:// pasted paths attaching correctly. |
| internal/tui/workspace/views/checkins.go | Handles tea.PasteMsg while answering by delegating to the composer’s paste pipeline. |
| internal/tui/workspace/views/checkins_test.go | Adds tests for paste behavior in answering vs non-answering modes (text vs attachment). |
| internal/richtext/dragpath.go | Implements drag/paste path normalization (currently !windows). |
| internal/richtext/dragpath_test.go | Adds unit tests for normalization cases (quotes, escapes, file URLs, tilde, cleaning). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b50d373468
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/richtext/dragpath.go">
<violation number="1" location="internal/richtext/dragpath.go:36">
P2: Double URL-decoding bug: `u.Path` from `url.Parse` is already percent-decoded, so the subsequent `url.PathUnescape` will incorrectly double-decode paths that contain literal `%` characters (e.g., a file named `100%25done.txt`). Use `u.Path` directly.</violation>
</file>
<file name="internal/richtext/dragpath_test.go">
<violation number="1" location="internal/richtext/dragpath_test.go:12">
P2: Silently discarding the error from `os.UserHomeDir()` can produce misleading test failures. If the home directory can't be resolved, `home` will be `""` and the `want` values for the tilde tests will be wrong (e.g., `"Documents/file.pdf"` instead of an absolute path). Use `t.Fatal` to fail fast with a clear message.</violation>
</file>
<file name="internal/tui/workspace/views/checkins_test.go">
<violation number="1" location="internal/tui/workspace/views/checkins_test.go:609">
P2: Unchecked error from `os.WriteFile` in test setup. If the file isn't created, the test will fail with a misleading assertion error instead of a clear setup failure. Use `require.NoError` to fail fast with a descriptive message.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
github-actions
bot
added
bug
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/tui/workspace/widget/composer.go">
<violation number="1" location="internal/tui/workspace/widget/composer.go:339">
P2: The composer text is cleared unconditionally after `AddAttachment`, but if `ValidateFile` inside `AddAttachment` rejects the file (e.g. too large or unsupported type), the attachment isn't actually added and the user's input is silently lost. Only clear the value when the attachment was successfully queued.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Dragging files from Finder into Terminal.app produces quoted, shell-escaped, tilde-prefixed, or file:// URL paths. NormalizeDragPath strips these layers so the composer can resolve them to actual files. Targets macOS/Linux only.
Three layers of detection for terminals without bracketed paste: 1. ProcessPaste: extracts file paths from paste events (iTerm2, etc.) 2. tryAutoAttachIfDrag: synchronous check after every keystroke detects paths starting with drag trigger characters (/, ', ", ~) 3. Submit-time interception: safety net that catches file paths on Enter when auto-detect hasn't fired yet, including when attachments already exist (e.g. user drags a second file before the first was detected) Add WithAttachmentsDisabled option for views where the API doesn't support file uploads (e.g. Campfire rich text lines reject non-mention attachments).
Route PasteMsg through the composer when answering a check-in, matching the pattern used by campfire and compose views. Use the composer's dynamic help bindings instead of hardcoded ones so the attach file hint appears.
The BC3 API rejects file-type bc-attachment SGIDs in campfire rich text lines (Chat::Lines::RichText permits only mentions and opengraph embeds). File uploads go through the web-only Chats::UploadsController which has no public API endpoint. Disable attachments in the campfire composer to avoid silent 422 failures.
- Only strip quotes and shell-unescape when input looks like a path (starts with /, ~/, or file://). Non-path inputs pass through unchanged. - Require absolute paths in detectFilePaths and tryAutoAttachIfDrag to avoid false positives from messages matching relative filenames. - Skip POSIX-specific tests on Windows.
github-actions
bot
added
enhancement
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
internal/tui/workspace/widget/composer.go:383
Submit()uploads attachments wheneverSGID == "", which includesAttachUploadingattachments that already have an in-flight async upload started byAddAttachment. If the user submits before the async upload finishes, this will trigger a second upload for the same file and can race with the eventualattachUploadedMsg.
Consider treating AttachUploading as a hard-block (return an error like "attachments still uploading") or only performing inline uploads for AttachPending/AttachFailed statuses (and leaving AttachUploading to complete via its existing cmd).
// Upload any pending attachments
for i := range attachments {
if attachments[i].SGID == "" && uploadFn != nil {
attachments[i].Status = AttachUploading
sgid, err := uploadFn(
ctx,
attachments[i].Path,
attachments[i].Filename,
attachments[i].ContentType,
)
if err != nil {
attachments[i].Status = AttachFailed
attachments[i].Err = err
return ComposerSubmitMsg{Err: fmt.Errorf("uploading %s: %w", attachments[i].Filename, err)}
}
attachments[i].SGID = sgid
attachments[i].Status = AttachUploaded
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
addAttachmentSync bypassed richtext.ValidateFile, allowing oversized or unreadable files through the submit-time drag path interception. Now all detected paths are validated before attaching — if any fail, the text is sent as markdown instead.
Summary
richtext.NormalizeDragPathto handle terminal-mangled file paths: shell-escaped (My\ Documents/file\ (1).pdf), quoted ('/path/to/file.pdf'),file://URLs (file:///path/to/my%20file.pdf), and tilde expansion — wired intoProcessPastereplacing the inline~/expansiontea.PasteMsghandling to the checkins view (was missing entirely), matching the pattern used by campfire and compose viewsTest plan
make checkpasses (fmt, vet, lint, unit tests, e2e)Summary by cubic
Normalize drag-and-drop file paths and auto-detect them in the composer so files dragged into terminals become attachments across composers (including check-ins). Attachments are disabled in Campfire where the API doesn’t support uploads.
New Features
richtext.NormalizeDragPathfor shell-escaped, quoted,file://, and~paths; only transforms inputs that look like paths; cleans absolute paths and leaves non-path text unchanged.WithAttachmentsDisabledand dynamic help.tea.PasteMsgto the composer when answering and uses composer help; Campfire usesWithAttachmentsDisabledand sends markdown only.Bug Fixes
Written for commit c46f982. Summary will update on new commits.