Repository navigation
Workspace file tools and WorkspaceLLMTaskWorker - #2
Conversation
…file access Adds planai.tools.filesystem (Workspace, make_file_tools, hash_files) so an LLM can read/write/edit/list/grep files inside a sandboxed per-job directory, and planai.workspace_task.WorkspaceLLMTaskWorker/WorkspaceTask to wire those tools into a CachedLLMTaskWorker whose working directory is discovered from task provenance. LLMTaskWorker gains get_tools()/get_cache_salt() hooks and a max_tool_rounds field (forwarded to generate_pydantic only when tools are in use), and CachedTaskWorker gains a _cache_hit_is_valid() hook so a cache hit missing its expected output files is treated as a miss and re-executed. Bumps version to 0.7.0 and documents the new worker in usage.rst. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Deploying planai with
|
| Latest commit: |
3f447df
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://24451b92.planai-bae.pages.dev |
| Branch Preview URL: | https://file-tools.planai-bae.pages.dev |
The comment step passed context.repo.name (undefined) as the repo, so every pull-request job ended in a 404 after its tests had passed. Jobs that comment now declare pull-requests/issues write permission. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
✅ Example tests passed for deepsearch (Python 3.10)! |
There was a problem hiding this comment.
🟡 Changes recommended
The filesystem jail can be bypassed via symlink traversal in list/grep/hash operations and the dependency constraint update conflicts with the existing poetry.lock resolution.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a workspace-scoped filesystem toolset for LLM workers and a WorkspaceLLMTaskWorker wrapper so jobs can pass file paths through a graph while keeping LLM file access sandboxed to a per-job directory.
Changes:
- Introduces
Workspace,make_file_tools(), andhash_files()for jailed read/write/edit/list/grep within a workspace directory. - Adds
WorkspaceTask+WorkspaceLLMTaskWorkerto bind per-task file tools and integrate workspace inputs/outputs with caching semantics. - Extends
LLMTaskWorkerandCachedTaskWorkerwith new hooks (get_tools,max_tool_rounds,get_cache_salt,_cache_hit_is_valid) and updates docs/tests/versioning.
File summaries
| File | Description |
|---|---|
| tests/planai/tools/test_filesystem.py | New unit tests for workspace path jailing and file-tool behavior. |
| tests/planai/test_workspace_task.py | New tests for workspace discovery, per-task tool binding, and cache-hit bypass when output files are missing. |
| tests/planai/test_utils.py | Minor test fixture tweak (control char literal normalization). |
| tests/planai/test_llm_task.py | Adds tests for get_tools() hook, max_tool_rounds, and cache_salt forwarding behavior. |
| tests/planai/test_cached_task.py | Adds tests for the new _cache_hit_is_valid() hook behavior. |
| src/planai/workspace_task.py | Implements WorkspaceTask and WorkspaceLLMTaskWorker integrating workspace + file tools + cache behavior. |
| src/planai/tools/filesystem.py | Implements Workspace sandbox plus filesystem tools and workspace file hashing. |
| src/planai/tools/init.py | Exposes filesystem tool APIs via planai.tools. |
| src/planai/provenance.py | Removes stray whitespace. |
| src/planai/llm_task.py | Adds per-task tool hook and tool-related parameters forwarding into llm_interface.generate_pydantic. |
| src/planai/dispatcher.py | Removes stray whitespace. |
| src/planai/cached_task.py | Adds _cache_hit_is_valid() hook and integrates it into cache-hit flow. |
| src/planai/_version.py | Bumps internal version to 0.7.0. |
| src/planai/init.py | Re-exports new workspace/file-tool and worker/task symbols. |
| pyproject.toml | Bumps package version to 0.7.0 and updates llm-interface dependency constraint. |
| docs/source/usage.rst | Documents WorkspaceLLMTaskWorker usage and file tools. |
Review details
Suppressed comments (2)
src/planai/tools/filesystem.py:301
- list_files() enumerates files via target.glob(pattern) and accepts p.is_file() results directly. If the workspace contains a symlinked directory that points outside the root, Path.glob()/is_file() can traverse and stat files outside the jail, violating the stated sandboxing guarantees.
Resolve each candidate through ws.resolve(rel) and only include it when that resolution succeeds and is a file.
try:
matches = sorted(p for p in target.glob(pattern) if p.is_file())
except (re.error, ValueError) as e:
return f"Error: Invalid pattern: {e}"
src/planai/tools/filesystem.py:350
- grep_files() builds its candidate list with target.glob(glob) + p.is_file() and then reads each path directly. With a symlinked directory inside the workspace that points outside, this can read file contents outside the jail.
Resolve each candidate through ws.resolve(rel) and only read from the resolved path when it stays within the workspace.
try:
candidates = sorted(p for p in target.glob(glob) if p.is_file())
except ValueError as e:
return f"Error: Invalid pattern: {e}"
- Files reviewed: 17/17 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| psutil = "^6.1.1" | ||
| waitress = "^3.0.2" | ||
| llm-interface = "^0.1.13" | ||
| llm-interface = ">=0.2.0,<0.3.0" |
There was a problem hiding this comment.
Known and intentional for now: 0.2.0 of llm-interface is what provides max_tool_rounds and cache_salt (provos/llm-interface#1). The plan is to merge and publish that release first, then regenerate poetry.lock here; pinning back to 0.1.13 would make the new kwargs silently no-ops.
| matched = {} | ||
| for pattern in globs: | ||
| for path in ws.root.glob(pattern): | ||
| if path.is_file(): | ||
| rel = path.relative_to(ws.root).as_posix() | ||
| matched[rel] = path |
There was a problem hiding this comment.
Fixed: every glob walk (hash_files, list_files, grep_files) now passes each match through Workspace.resolve() and skips entries whose real path is outside the root. Tests cover a symlinked file and a symlinked directory pointing outside the workspace.
Glob follows symlinks, so a link under the workspace could expose files outside it. Every match now goes through Workspace.resolve() and entries that escape are skipped. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
✅ Example tests passed for deepsearch (Python 3.10)! |
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
✅ Linting passed! |
|
✅ Example tests passed for deepsearch (Python 3.10)! |
|
✅ Tests passed for Python 3.10! |
|
✅ Tests passed for Python 3.12! |
|
✅ Tests passed for Python 3.11! |
Add a Workspaces and File Tools feature page and cover the new WorkspaceLLMTaskWorker, WorkspaceTask, Workspace, make_file_tools, hash_files, the get_tools()/max_tool_rounds hooks and the _cache_hit_is_valid cache hook on the LLM integration, caching, task worker, usage and API reference pages. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
✅ Linting passed! |
|
✅ Example tests passed for deepsearch (Python 3.10)! |
|
✅ Tests passed for Python 3.12! |
|
✅ Tests passed for Python 3.11! |
|
✅ Tests passed for Python 3.10! |
Address review findings on the file-tools branch: - Workers whose tools write files now pass a fresh cache_salt on every execution so llm-interface's response cache cannot replay an answer whose tool calls (the file writes) would be skipped, e.g. after a PlanAI cache hit was rejected for missing output files. Read-only workers keep using the cache key as the salt. - The workspace root is always part of the cache key, so identical payloads in different workspaces no longer share an entry. - extra_cache_key() no longer raises when no workspace is in the provenance chain; get_tools() reports that instead. - The lookup cache key is memoized per task on the worker thread and reused by get_cache_salt(); the store still recomputes it after the run on purpose (documented) so a worker that edits files it also hashes is found by a later run over the edited files. - Static tools that would shadow a file tool are rejected. - Workspace() no longer creates the directory as a side effect. - Absolute or empty glob patterns are rejected with ValueError instead of surfacing pathlib's NotImplementedError. - grep_files searches every file by default (was Markdown only) and truncates long matching lines (max_grep_line_chars). - edit_file preserves CRLF line endings; write_file writes verbatim. - hash_files raises a clear OSError naming the unreadable file. - Fix the mypy error on the cached-results Optional and annotate get_tools(). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
✅ Linting passed! |
|
✅ Example tests passed for deepsearch (Python 3.10)! |
|
✅ Tests passed for Python 3.11! |
Two graphs sharing one dispatcher stop it concurrently; one caller set _dispatch_thread to None between the other's None check and its is_alive() call, which made test_shared_dispatcher_shutdown flaky. Hold the thread in a local before joining it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
✅ Linting passed! |
|
✅ Example tests passed for deepsearch (Python 3.10)! |
|
✅ Tests passed for Python 3.11! |
|
✅ Tests passed for Python 3.12! |
|
✅ Tests passed for Python 3.10! |
Summary
Lets an LLM worker read, write, edit, list and grep files inside a per-job working directory, so tasks can pass file names through the graph instead of large text.
planai.tools.filesystem:Workspace(jails every path: no absolute paths, no.., no symlink escapes),make_file_tools()producingread_file,write_file,edit_file(exact single-match replace),list_files,grep_filesas llm-interface tools, andhash_files().WorkspaceLLMTaskWorker(aCachedLLMTaskWorker): finds the workspace from the nearest task with aworkspaceattribute in provenance, binds the file tools per task, folds declared input files into the cache key, passes acache_saltto llm-interface, and re-executes a cache hit whoseexpected_output_files()are missing.LLMTaskWorker.get_tools(task)hook (per-task tools) andmax_tool_rounds;CachedTaskWorker._cache_hit_is_valid()hook.WorkspaceTaskcarries the directory through provenance. Docs inusage.rst. Version 0.7.0.Dependency
Requires llm-interface >= 0.2.0 (provos/llm-interface#1) for
max_tool_roundsandcache_salt. Until that release is on PyPI andpoetry.lockis regenerated, thepoetry installstep of CI will fail; the code and tests do not otherwise depend on it (tests mockgenerate_pydantic).Test plan
pytest tests/planai: 250 passed (60 new)black,flake8cleanWorkspaceLLMTaskWorker→ file tools → structured result on claude-haiku-4-5