Skip to content

feat: resolve agent-home-relative file references in get_content#444

Open
andrii-novikov wants to merge 3 commits into
developmentfrom
fix/appdir-relative-file-refs
Open

feat: resolve agent-home-relative file references in get_content#444
andrii-novikov wants to merge 3 commits into
developmentfrom
fix/appdir-relative-file-refs

Conversation

@andrii-novikov

@andrii-novikov andrii-novikov commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Applicable issues

Description of changes

Agents routinely hold agent-home-relative paths (e.g. reports/img.png) — that is the convention the internal_file_* tools speak. But internal_attachments_get_content rejected such references as unsupported, forcing the model to guess the absolute files/... form it has never seen.

This PR makes the reference round-trip work:

  • UrlScheme.DIAL_APPDIR_RELATIVEclassify_url classifies a schemeless path as the agent-home-relative convention instead of UNSUPPORTED. Detection is owned by the canonical public predicate is_appdir_relative so the grammar cannot drift between consumers:
    • file form (file.md, dir1/file.md; extension optional, so write_file-created names like notes round-trip) or dir form with a trailing slash (some_dir/, dir_1/dir2/)
    • no .. / . / empty segments, no segment ending with a dot (DIAL Core rule)
    • rejects the DIAL UI forbidden characters : ; , = { } % & \ " and control chars (ui-kit NOT_ALLOWED_SYMBOLS / NOT_ALLOWED_SPACES)
    • DIAL UI length limits: ≤ 255 UTF-8 bytes per segment, ≤ 1024 bytes total
  • internal_attachments_get_content resolves such references under the agent home and delivers the resolved files/... url on the attachment, while the payload echoes the reference as passed. Rejection messages enumerate the accepted forms. Traversal attempts (../x.png) now fail at classification, before the resolver.
  • shared/home_path/HomePathResolver — the private dial_files_tooling/_HomePathResolver moves to a public shared package with its own HomePathModule (spliced into shared_module), so consumers outside the file family can inject it without reaching into another module's privates. validate_relative_path moved with it (the resolver was its only caller), so shared/ no longer imports from a feature module. Rationale: _GetContentTool lives outside dial_files_tooling; injecting a _-private class across module boundaries would violate the DI conventions. The move is mechanical (class rename + module binding), covered by the relocated resolver tests.
  • MCP dial_url parameters keep rejecting the new scheme explicitly (a home-relative path is not a shareable DIAL file there) — behavior is unchanged; resolving instead of rejecting is tracked in Support agent-home-relative file references in MCP dial_url parameters #445 (TODO(#445) at the branch).

Tests

  • classify_url / is_appdir_relative: valid file/dir forms (incl. extensionless and names with spaces); invalid traversal, forbidden characters, trailing-dot segments, and UTF-8 byte-length limits (incl. multibyte case).
  • get_content: home-relative success (attachment carries resolved url, payload echoes reference, no promotion), resolution failure surfaces the resolver message, traversal rejected before the resolver, MIME rejection on resolved content.
  • MCP: home-relative dial_url parameter still rejected, no permission grant.
  • Resolver tests moved to tests/unit_tests/shared/ unchanged in substance.

All 1643 unit tests pass; make lint clean.

Checklist

  • Title of the pull request follows Conventional Commits specification
  • [ ] Design documented is updated/created and approved by the team (if applicable) (no design doc for this change)
  • [ ] Documentation is updated/created (if applicable) (internal change; no docs affected)
  • Changes are tested on review environment
  • [ ] App schema changes are backward compatible, or breaking changes are documented with a migration guide (no schema change)
  • Integration tests pass

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Classify schemeless, hostless paths (e.g. reports/img.png) as a new
UrlScheme.DIAL_APPDIR_RELATIVE — the agent-home-relative convention the
internal_file_* tools speak — instead of lumping them into UNSUPPORTED.
internal_attachments_get_content now resolves such references under the
agent home and delivers the resolved files/ url on the attachment, so the
model can hand a file-tool path straight to get_content.

To make the resolver injectable outside the file family, _HomePathResolver
moves from dial_files_tooling to a public shared/home_path/HomePathResolver
bound by a HomePathModule spliced into shared_module; dial_files_tooling
drops its inline binding and injects the shared type.

MCP dial_url parameters keep rejecting the new scheme explicitly (it is not
a shareable DIAL file there), preserving current behavior.
andrii-novikov and others added 2 commits July 14, 2026 09:45
Address PR #444 review comments:

- add is_appdir_relative to common/url_classification.py as the canonical
  detection predicate: DIAL UI forbidden chars (:;,={}%&\\" and control
  chars), no ..//./empty/trailing-dot segments, dir form via trailing '/',
  extensionless files allowed, UI length limits (255 bytes/segment, 1024
  total); classify_url delegates its schemeless branch to it
- move validate_relative_path from dial_files_tooling/_utils.py into
  shared/home_path/home_path_resolver.py (its only caller) so shared/ no
  longer imports from a feature module
- drop leftover binding breadcrumb comment in dial_files_tooling_module
- TODO(#445) at the MCP dial_url rejection branch for future
  appdir-relative resolution
- rework classification tests to the new grammar; cover traversal,
  forbidden chars, and UTF-8 byte-length limits; get_content traversal
  references now fail at classification, before the resolver

@VinShurik VinShurik 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.

Small nit adjustments:
Update documentation files
pass_attachments_to_orchestrator.md
agent.md
dial_files_tools.md
to mention new shared/home_path module and it's functionality

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

get_content cannot load files referenced by agent-home-relative paths

2 participants