fix(gateway): translate inbound cache paths to container paths for Docker backend - #19610
Closed
pvdb2178 wants to merge 1 commit into
Closed
fix(gateway): translate inbound cache paths to container paths for Docker backend#19610pvdb2178 wants to merge 1 commit into
pvdb2178 wants to merge 1 commit into
Conversation
…cker backend Inbound documents (.docx/.pdf/etc.) from messaging platforms are saved to host paths under ~/.hermes/cache/documents/. Under the Docker terminal backend those directories are bind-mounted at /root/.hermes/cache/documents/ via get_cache_directory_mounts(), but the gateway was injecting the raw host path into the agent's prompt when a doc was received. The agent runs inside the container and cannot open the host path, so reads of user-sent docs failed silently. Add to_agent_visible_cache_path() in tools/credential_files.py which maps host cache paths to their container-side mount points, gated on backend == "docker". Call it from the doc-injection sites in _prepare_inbound_message_text. Pass-through for non-docker backends keeps existing behavior unchanged. Mirrors the pattern from NousResearch#14990 (vision sandbox path resolution). Fixes NousResearch#18787
Collaborator
Author
|
Closing as duplicate of #19048 — thanks @alt-glitch for the catch. I missed it because I only searched merged PRs, not open ones. @ambition0802's PR landed ~22h before mine and implements the same fix (same helper name, location, call site, scope). Their backend gate via I've offered the unit tests + docs note from this branch to #19048 as a follow-up if @ambition0802 wants them — happy to either push directly (with collaborator access on their fork) or they can cherry-pick from |
Author
|
Duplicate of #19048. |
19 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
When
terminal.backend: docker, inbound documents (.docx/.pdf/etc.) from messaging platforms are saved at host paths under~/.hermes/cache/documents/and_prepare_inbound_message_textinjects those raw host paths into the agent's prompt:The agent runs inside the Docker container where that host path doesn't exist; the cache directories are bind-mounted at
/root/.hermes/cache/documents/viaget_cache_directory_mounts()(added in #4846), but the path translation step was missing — so reads of user-sent docs fail.This adds
to_agent_visible_cache_path()intools/credential_files.pynext toget_cache_directory_mounts, and calls it at the two doc-injection sites ingateway/run.py. Pass-through for non-Docker backends keeps existing behavior unchanged. Mirrors the pattern from #14990 (vision sandbox path resolution).Out of scope (intentionally, to keep the diff focused):
_enrich_message_with_transcriptioninjects the transcript text, not the file path.backendarg so they can opt in later if needed.Related Issue
Fixes #18787
Type of Change
Changes Made
tools/credential_files.py: addto_agent_visible_cache_path(host_path, *, backend, container_base='/root/.hermes')that maps host cache paths to their container-side mount points by walking_CACHE_DIRS. Idempotent (already-translated paths pass through). Gated onbackend == "docker"(case/space-insensitive).gateway/run.py: call the helper inside theevent.message_type == MessageType.DOCUMENTblock before injecting the path into both the text-document and binary-document context notes. Backend resolved once viaos.getenv("TERMINAL_ENV", "local").tests/tools/test_credential_files.py: addTestToAgentVisibleCachePath(9 tests) — translation for each cache subdir, nested subpaths, non-docker pass-through, paths outside cache, idempotence, case/space-insensitive backend, custom container_base, empty input.website/docs/user-guide/docker.md: brief note in "Skills and credential files" about the cache bind-mount and path rewrite, where the analogous skills/credential-file mount behavior is already documented.How to Test
Repro (before fix):
terminal.backend: dockerand run the gateway..docxor.pdfto the bot from Telegram (or any messaging platform with document support).read_file/terminal catwith the host path it was given (e.g./home/<user>/.hermes/cache/documents/...) and getsNo such file or directoryfrom inside the container.After fix:
/root/.hermes/cache/documents/...), which is the bind-mount target, and the read succeeds.Unit-test verification:
All 38 tests pass (9 new + 29 existing). Re-ran
tests/tools/andtests/gateway/for regression — no new failures from this change. There are 14 pre-existing failures in unrelated platform adapters (dingtalk/feishu/teams/whatsapp) that fail onmainwithout this patch as well.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass — partial: rantests/tools/andtests/gateway/(4322 passed, 14 unrelated pre-existing failures in dingtalk/feishu/teams/whatsapp, confirmed on cleanmain).Documentation & Housekeeping
website/docs/user-guide/docker.md)cli-config.yaml.exampleif I added/changed config keys — N/A (no new config)CONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/Apathlib.Path.relative_to/.resolve()which are cross-platform; container paths are forward-slash POSIX (correct for the Linux container target regardless of host OS).Open considerations (from #18787 triage questions)
backend/dockerlabel. Helper signature takes abackendkwarg so other backends can opt in without breaking callers.tools/credential_files.pynext toget_cache_directory_mounts— same_CACHE_DIRSconstant, same module that owns the mount mapping. Open to moving it togateway/platforms/base.pyif maintainers prefer the platform layer._prepare_inbound_message_text(the only place doc host paths get injected into prompt text). Did not translate_build_media_placeholder(line 646), since queued events re-enter_prepare_inbound_message_texton requeue and get translated there.Path.resolve()+relative_to, which canonicalise on each platform. Container paths are emitted as forward-slash POSIX strings (as_posix()) since the container target is always Linux.