Skip to content

📎 refactor: Move the Attachment Row Into @librechat/chat Behind the Files Host - #17801

Open
berry-13 wants to merge 6 commits into
devfrom
refactor/chat-composer-filerow
Open

berry-13 wants to merge 6 commits into
devfrom
refactor/chat-composer-filerow

Conversation

@berry-13

@berry-13 berry-13 commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

The staged attachment row in the composer (FileRow, with its image previews, file cards, upload progress and pasted-text actions) still lives in client/src/components/Chat/Input/Files/FileRow.tsx, and it reaches into the app for the delete mutation and the cached local previews. A consumer of @librechat/chat could not render the composer's attachments.

This PR moves the row to packages/chat/src/components/composer/files/FileRow.tsx and exports FileRow and FileRowWrapper from @librechat/chat/components. The two app capabilities become FilesHost members: useDeleteFile (scoped by agent, assistant, tool resource and index) and getPreviewUrl. The client fills them in FilesHostContext.tsx with the same delete mutation and useFileDeletion call the row made before; without a host, the package falls back to removing the file from the staged map. The old client path re-exports the package row.

Stacked on #17799.

Type of change

  • Refactor
  • Tests / tooling / CI

Testing

Tested environments/configuration:

  • Node 22, jsdom (packages/chat and client jest)

Automated tests:

  • FileRow.spec.tsx moved to packages/chat/src/components/composer/files/__tests__ and now runs against a fake FilesHost. New FileRowFallback.spec.tsx covers the row with no host, and FilesHostContext.spec.tsx covers the app's delete scope and preview lookup.
  • packages/chat jest (composer/files): 66/66. Client jest (FilesHostContext, useFileDeletion, Input/Files): 37/37. npx tsc --noEmit in packages/chat: clean. ESLint on touched files: clean.

Screenshots / recordings

No user-facing change. The row's markup and classes are unchanged.

Risk / compatibility

FilesHost gains two required members, so any other host must supply them. The only host in the repo is the client's.

Checklist

  • I reviewed my own changes
  • Relevant tests have been added or updated
  • Existing relevant tests pass
  • The change does not introduce new warnings or errors
  • Required documentation PR: N/A

@berry-13 berry-13 added 🔨 refactor Code cleanup without new features 🧪 tests Unit/integration test updates labels Nov 4, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-06T16:37:34.912933Z 04b6cde New commits
🔒 Security Review ✅ Completed 2026-11-04T14:07:21.169707Z c19ea17 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c19ea17464

ℹ️ 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".

Comment on lines +23 to +25
jest.mock('~/hooks/Files', () => ({
useFileDeletion: (args: unknown) => mockUseFileDeletion(args),
}));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Test the files host with the real deletion hooks

The new host-adapter suite mocks both useFileDeletion and useDeleteFilesMutation, contrary to the repository's explicit testing rule to mock only external HTTP APIs or uncontrollable services. Consequently, these tests verify argument forwarding but never exercise the new adapter together with the actual deletion lifecycle; the separate deletion tests also supply their own host rather than appFilesHost. Render the real app host with the required providers and mock the HTTP boundary so the extracted wiring is covered without replacing its internal collaborators.

AGENTS.md reference: AGENTS.md:L211-L214

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 60d6ec603a

ℹ️ 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".

Comment on lines +13 to +15
jest.mock('../FileContainer', () => ({
__esModule: true,
default: function MockFileContainer({

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Exercise the real file card in the fallback deletion test

The new fallback suite replaces FileContainer with a synthetic button, contrary to the repository's explicit rule to mock only external HTTP APIs or uncontrollable services. This means the no-host deletion test can pass even if the real attachment card fails to expose or invoke its remove action. Render the real FileContainer and interact with its remove control so the test covers the user-facing fallback path rather than only callback forwarding.

AGENTS.md reference: AGENTS.md:L211-L214

Useful? React with 👍 / 👎.

@berry-13
berry-13 added this pull request to stack #17848 November 5, 2026 21:36
Base automatically changed from refactor/chat-composer-usage to dev November 5, 2026 21:36
@berry-13
berry-13 force-pushed the refactor/chat-composer-filerow branch from eb6aa46 to 38fb656 Compare November 5, 2026 21:36
Copilot AI balanced review requested due to automatic review settings November 6, 2026 16:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

This branch has not been deployed

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

Labels

🔨 refactor Code cleanup without new features 🧪 tests Unit/integration test updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants