runner: session-scoped upload route with session-end cleanup by mattieruth · Pull Request #5830 · pipecat-ai/pipecat

mattieruth · 2026-09-17T19:57:00Z

Part 4 of File Support work
Part 1
Part 2
Part 3
Part 4

Corresponding Client-JS changes
New File Input Example

Add POST /sessions/{session_id}/files, mirroring Pipecat Cloud's session-scoped routes; it validates the session against the active session store and tracks each upload so the session's files are deleted when its bot session ends. The flat POST /files remains for transports whose clients never learn a session id (the websocket flow returns a sessionId from /start but the bot mints a different one at connect time); files uploaded there are only removed by the storage backend's own retention. The uploads routes are registered before the WebRTC routes so they win over the catch-all /sessions/{session_id}/{path:path} proxy, which matches later in registration order.

Bot sessions started from the /start flows (daily, livekit, moq) and from the WebRTC offer now run through _run_bot_session, which on completion removes the session from the active session store — fixing entries never being released — and best-effort deletes its uploads. The daily, livekit, and moq flows now register their sessions in the store in the first place; previously only webrtc did, so their session routes always 404ed.

LocalFileStorage.delete ignores an already-missing file: an upload consumed by delete_after_load is deleted again by session-end cleanup.

kompfner · 2026-09-21T19:40:55Z

Just going to leave all feedback here rather than strewn throughout the PR stack, if you don't mind.

kompfner · 2026-09-21T19:44:13Z

On the client: looks like sendFile can take an input which can be FileId. Is this a holdover from when we were thinking that an id by itself could be used instead of a URL when the client uploaded the file to the bot's /file endpoint and the bot was using LocalFileStorage? If I'm understanding correctly, isn't this superseded by "pipecat:"-prefixed URLs?

kompfner · 2026-09-21T19:51:52Z

On the client: is uploadFile public in order to support developers calling it ahead of time as an optimization, to avoid the just-in-time cost of uploading when invoking sendFile?

If so:

Shouldn't uploadFile be able to take the same types of files as sendFile (i.e. URLs in addition to Files), instead of only Files?
The app's code doesn't know whether a file exceeds the message limit and thus whether it'd benefit from ahead-of-time uploading. Should we expose a mechanism for them to be able to make that decision? Alternatively...should we consider adjusting this API? For example, we could make it clear that it's meant as an optimization and make it not always run, like preUploadFileIfNeeded (name obviously just a "thinking out loud" placeholder).

kompfner · 2026-09-21T20:03:23Z

Re BaseLLMAdapter.resolve_file_items:

The work of replacing file-URL items with file-contents items should happen automatically as part of the "main" adapter logic, which runs in get_llm_invocation_params. This is the home of all logic responsible for "translating" (adapting) what's in an LLMContext into whatever the LLMService needs to invoke its LLM, and is already guaranteed to run on every LLM invocation. Otherwise (as implemented) the file logic must be triggered through a secondary call (LLMService.resolve_context_files) in the LLM service implementations themselves, which is more fragile. (I think we may already have a bug where file messages aren't resolved as part of run_inference calls.) If we make this change, I think we can get rid of LLMService.resolve_context_files entirely. We can pass in FileResolver into get_llm_invocation_params if needed; services already pass whatever they need into that method.
I don’t love that we’re using “secret” fields on LLMContext message items themselves as a per-provider cache of downloaded files; we should be leaving LLMContext intact as a “canonical” representation of the context. Instead, let's just write the downloaded files to an explicit in-memory cache, in per-provider entries. The adapters can replace the “canonical” file messages with the downloaded content practically for free on each future run of get_llm_invocation_params.

kompfner · 2026-09-21T20:05:30Z

Overall, this looks really neat! I mean "neat" in both senses of the word—cool, and tidy.

codecov · 2026-10-01T22:41:14Z

Codecov Report

❌ Patch coverage is 70.88608% with 23 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/pipecat/runner/run.py	69.73%	23 Missing ⚠️

Files with missing lines	Coverage Δ
src/pipecat/runner/types.py	`97.22% <100.00%> (+0.05%)`	⬆️
src/pipecat/services/llm_service.py	`91.71% <ø> (ø)`
src/pipecat/utils/file_resolver.py	`96.19% <ø> (ø)`
src/pipecat/utils/file_storage.py	`83.33% <100.00%> (-3.04%)`	⬇️
src/pipecat/runner/run.py	`45.57% <69.73%> (+2.35%)`	⬆️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

mattieruth · 2026-10-01T22:49:44Z

Re BaseLLMAdapter.resolve_file_items:

The work of replacing file-URL items with file-contents items should happen automatically as part of the "main" adapter logic, which runs in get_llm_invocation_params. This is the home of all logic responsible for "translating" (adapting) what's in an LLMContext into whatever the LLMService needs to invoke its LLM, and is already guaranteed to run on every LLM invocation. Otherwise (as implemented) the file logic must be triggered through a secondary call (LLMService.resolve_context_files) in the LLM service implementations themselves, which is more fragile. (I think we may already have a bug where file messages aren't resolved as part of run_inference calls.) If we make this change, I think we can get rid of LLMService.resolve_context_files entirely. We can pass in FileResolver into get_llm_invocation_params if needed; services already pass whatever they need into that method.

I don’t love that we’re using “secret” fields on LLMContext message items themselves as a per-provider cache of downloaded files; we should be leaving LLMContext intact as a “canonical” representation of the context. Instead, let's just write the downloaded files to an explicit in-memory cache, in per-provider entries. The adapters can replace the “canonical” file messages with the downloaded content practically for free on each future run of get_llm_invocation_params.

This triggered a pretty major update to how files get resolved. See this commit. Caching now lives in the FileResolver itself: downloaded bytes (plus the provider-shaped forms and whether it's reachable) are cached there, keyed by URL, and the adapters swap them in at conversion. With this, LLMContext stays intact, holding only the URL, as you suggested. A nice side effect for dynamic LLM swaps: services sharing a resolver reuse each other's downloads while each provider still makes its own pass-through decision against the original URL.

For your first point, resolution and conversion are now fused into a single prepare_llm_invocation_params that every service calls on every invocation (resolve_context_files is gone). And you were right about run_inference: it was skipping resolution. This fix also surfaced the same bug in the logging-path conversions and Sarvam's pre-flight validation. I went that route rather than building it into get_llm_invocation_params because that method is synchronous and implemented by every adapter — the downloads' await can't live inside it without async-ifying that whole surface — so the sync conversion just reads the caches instead. Relatedly, the resolver became adapter state (adapter.file_resolver, set once by the service) rather than a parameter, since both conversion entry points — invocation params and get_messages_for_logging — need the same caches.

mattieruth · 2026-10-06T02:38:21Z

On the client: looks like sendFile can take an input which can be FileId. Is this a holdover from when we were thinking that an id by itself could be used instead of a URL when the client uploaded the file to the bot's /file endpoint and the bot was using LocalFileStorage? If I'm understanding correctly, isn't this superseded by "pipecat:"-prefixed URLs?

This is holdover. I removed the ID option from pipecat but hadn't updated the client yet.

On the client: is uploadFile public in order to support developers calling it ahead of time as an optimization, to avoid the just-in-time cost of uploading when invoking sendFile?

I re-worked the way we set the upload endpoint and really uploadFile is just an internal helper and probably shouldn't be made public. If you want to do something truly unique, then you'll have to do your own POST.

kompfner · 2026-10-06T15:08:04Z

Thanks for the fixes! Will go ahead and approve all individual PRs related to this project.

filipi87 · 2026-10-06T16:53:56Z

+        # Where to POST file uploads for this session, when uploads are
+        # enabled. A path relative to the /start endpoint's origin (or an
+        # absolute URL from backends that know theirs). Omitted when uploads
+        # are disabled.
+        fileUploadUrl: str


Is this something we will need to implement inside PCC as well, or will the client try to fall back to a URL that just works there ? Like this one:

/sessions/{session_id}/files

it would be something we need to implement in PCC.

filipi87 · 2026-10-06T16:57:53Z

            "format": media_type,
        }

+    @app.post("/sessions/{session_id}/files")


Are we also going to create routes to allow retrieving or deleting a file ? Do we need it ?

it's not needed for this feature, no.

filipi87 · 2026-10-06T16:59:40Z

+            raise HTTPException(404, "Invalid or not-yet-ready session_id")
+        return await handle_upload(file, session_id)
+
+    @app.post("/files")


I might be missing something, but I am not totally sure why we still need this one. What would be a case where we wouldn't have a session ?

looks like the main reason it's there is for websocket.

Its /start returns a sessionId that's never registered with the runner — the bot mints its own session id when the client connects to /ws-client — so the session-scoped route would reject those clients with a 404. /start only advertises the flat route for that transport; everything else gets /sessions/{id}/files. If we later fix the websocket session-id mismatch (it predates this PR), the flat route can go away.

Even for the WebSocket, in theory they should have invoked /start first, which would return a session ID. So, if they are using our Web SDK, that should just work.

I think we could accept the limitation that file uploads would not work unless they have created a session first. So they would need to invoke /start in order to be able to upload a file, and we could remove this /files endpoint.

i'd like to do this as a separate PR. claude is claiming it has to fix pre-existing issues in order to remove /files and not break websocket. I don't fully follow, but I don't want this to hold up merging this feature.

Yep, let's do that in a different PR. 👍

filipi87

With this approach, it should already be possible to make it work both locally and on Pipecat Cloud. 🙇

Nice feature, @mattieruth. Approved all the PRs. 🚀

Add POST /sessions/{session_id}/files, mirroring Pipecat Cloud's session-scoped routes; it validates the session against the active session store and tracks each upload so the session's files are deleted when its bot session ends. The flat POST /files remains for transports whose clients never learn a session id (the websocket flow returns a sessionId from /start but the bot mints a different one at connect time); files uploaded there are only removed by the storage backend's own retention. The uploads routes are registered before the WebRTC routes so they win over the catch-all /sessions/{session_id}/{path:path} proxy, which matches later in registration order. Bot sessions started from the /start flows (daily, livekit, moq) and from the WebRTC offer now run through _run_bot_session, which on completion removes the session from the active session store — fixing entries never being released — and best-effort deletes its uploads. The daily, livekit, and moq flows now register their sessions in the store in the first place; previously only webrtc did, so their session routes always 404ed. LocalFileStorage.delete ignores an already-missing file: an upload consumed by delete_after_load is deleted again by session-end cleanup.

…the client to use

Replace set_runner_file_storage()/runner_file_storage() — module globals only the development runner's main() honored — with a host-agnostic convention: the bot module optionally defines create_file_storage() -> FileStorage | None alongside bot(), and whichever host runs the bot discovers it, serves its upload endpoints from the returned backend, and injects it into the bot as runner_args.file_storage. A defined factory wins over -u/--uploads-folder; without one the flag backs uploads with local disk. This keeps bot code identical whether the development runner or a cloud platform's base image hosts it.

mattieruth · 2026-10-07T15:21:51Z

trying to merge this stack just as github goes down. feels like...

github-actions · 2026-10-07T15:26:32Z

Docs automation failed for this PR — its documentation is missing.

The update-docs workflow did not produce a docs PR: run log

Re-run it once the cause is addressed:

gh workflow run update-docs.yml -f pr_number=5830 --repo pipecat-ai/pipecat

If this PR genuinely needs no documentation, remove the
docs-automation-failed label.

mattieruth added this pull request to stack #5828 September 17, 2026 19:57

mattieruth requested review from aconchillo, filipi87, kompfner and markbackman September 17, 2026 20:12

mattieruth force-pushed the mrkb/file-support-part-4 branch from f0f1aa0 to 59e0831 Compare October 1, 2026 22:33

mattieruth force-pushed the mrkb/file-support-part-4 branch from 59e0831 to 2f79467 Compare October 1, 2026 22:51

mattieruth force-pushed the mrkb/file-support-part-4 branch from 2f79467 to 5c90031 Compare October 2, 2026 14:21

mattieruth force-pushed the mrkb/file-support-part-4 branch from 5c90031 to d0a2077 Compare October 6, 2026 13:57

kompfner approved these changes Oct 6, 2026

View reviewed changes

filipi87 reviewed Oct 6, 2026

View reviewed changes

filipi87 approved these changes Oct 7, 2026

View reviewed changes

mattieruth force-pushed the mrkb/file-support-part-4 branch from cf4c770 to 9744052 Compare October 7, 2026 14:11

mattieruth added 2 commits October 7, 2026 10:50

Updated runner start endpoint to return the file upload endpoint for …

2e66786

…the client to use

mattieruth force-pushed the mrkb/file-support-part-4 branch from 9744052 to 491e742 Compare October 7, 2026 14:50

mattieruth merged commit 562db68 into main Oct 7, 2026
6 checks passed

mattieruth deleted the mrkb/file-support-part-4 branch October 7, 2026 15:25

github-actions Bot added the docs-automation-failed The update-docs workflow failed for this PR label Oct 7, 2026

mattieruth restored the mrkb/file-support-part-4 branch October 7, 2026 15:58

pipecat-docs-bot Bot mentioned this pull request Oct 7, 2026

docs: update for pipecat PR #5830 pipecat-ai/docs#1443

Closed

mattieruth deleted the mrkb/file-support-part-4 branch October 7, 2026 19:23

Conversation

mattieruth commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kompfner commented Sep 21, 2026

Uh oh!

kompfner commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kompfner commented Sep 21, 2026

Uh oh!

kompfner commented Sep 21, 2026

Uh oh!

kompfner commented Sep 21, 2026

Uh oh!

codecov Bot commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mattieruth commented Oct 1, 2026

Uh oh!

mattieruth commented Oct 6, 2026

Uh oh!

kompfner commented Oct 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

filipi87 left a comment

Choose a reason for hiding this comment

Uh oh!

mattieruth commented Oct 7, 2026

Uh oh!

Uh oh!

github-actions Bot commented Oct 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mattieruth commented Sep 17, 2026 •

edited

Loading

kompfner commented Sep 21, 2026 •

edited

Loading

codecov Bot commented Oct 1, 2026 •

edited

Loading