Skip to content

fix(resource): resolve 'file exists' errors on resource add#845

Merged
qin-ctx merged 2 commits intovolcengine:mainfrom
qin-ptr:patch-4
Mar 21, 2026
Merged

fix(resource): resolve 'file exists' errors on resource add#845
qin-ctx merged 2 commits intovolcengine:mainfrom
qin-ptr:patch-4

Conversation

@qin-ptr
Copy link
Copy Markdown
Contributor

@qin-ptr qin-ptr commented Mar 21, 2026

Description

Fix two bugs in resource_processor.py Phase 3.5 that caused mv to fail with "file exists" when adding resources, plus a minor ctx propagation fix in embedding_utils.py.

Related Issue

Fixes #846

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • Trailing slash in to URI: When to parameter ends with /, parent_uri was incorrectly computed as the target directory itself (instead of its parent). mkdir then pre-created the exact path that mv was about to rename into, causing 100% failure. Fixed by stripping trailing slash before rsplit.
  • Missing request context in _resolve_unique_uri: The Phase 3.5 call to _resolve_unique_uri ran outside bind_request_context, so stat checked the default account (/local/default/) instead of the actual account (e.g., /local/coral/). On repeated adds of the same resource name, it failed to detect the existing resource and returned the original URI, colliding with the existing path. Fixed by wrapping in bind_request_context(ctx).
  • embedding_utils.py: index_resource had ctx available but didn't pass it to viking_fs.exists() and viking_fs.read_file() calls for .abstract.md / .overview.md.

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

N/A

Additional Notes

Reproduction (Bug 1 - trailing slash): Call add_resource with to="viking://resources/foo/" — first add fails immediately.

Reproduction (Bug 2 - missing context): On a non-default account, run ov add-resource file.md twice — second add fails with "file exists".

qin-ctx and others added 2 commits March 21, 2026 18:25
Two bugs caused mv to fail with "file exists" during add_resource:

1. When `to` URI has a trailing slash, parent_uri was computed as the
   target directory itself instead of its parent. mkdir then created
   the exact path that mv was about to rename into.

2. _resolve_unique_uri in Phase 3.5 ran outside bind_request_context,
   so stat checked the default account path instead of the actual
   account. On repeated adds it failed to detect the existing resource
   and returned the original URI, causing mv to collide.

Also pass ctx to viking_fs calls in index_resource that were missing it.
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ qin-ctx
❌ qin-ptr
You have signed the CLA already but the status is still pending? Let us recheck it.

@qin-ctx qin-ctx changed the title Patch 4 fix(resource): resolve 'file exists' errors on resource add Mar 21, 2026
@github-actions
Copy link
Copy Markdown

Failed to generate code suggestions for PR

@qin-ctx qin-ctx merged commit fba4c17 into volcengine:main Mar 21, 2026
5 of 6 checks passed
@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenViking project Mar 21, 2026
zeattacker pushed a commit to zeattacker/OpenViking that referenced this pull request Mar 21, 2026
…ne#845)

* fix(resource): resolve 'file exists' errors on resource add

Two bugs caused mv to fail with "file exists" during add_resource:

1. When `to` URI has a trailing slash, parent_uri was computed as the
   target directory itself instead of its parent. mkdir then created
   the exact path that mv was about to rename into.

2. _resolve_unique_uri in Phase 3.5 ran outside bind_request_context,
   so stat checked the default account path instead of the actual
   account. On repeated adds it failed to detect the existing resource
   and returned the original URI, causing mv to collide.

Also pass ctx to viking_fs calls in index_resource that were missing it.

* Update GUIDE.md

---------

Co-authored-by: qin-ctx <qinhaojie.exe@bytedance.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

bug(resource): add_resource fails with 'file exists' due to trailing slash and missing account context

3 participants