Repository navigation
🌳 feat: List Attached Workspace Trees - #15527
Conversation
566ea90 to
3585e1f
Compare
|
@codex review\n\nPlease review the current PR head 3585e1fe95a75cda035c2ec37ca2ea3b22e5dc95. Confirm that this exact commit is the reviewed commit and ignore findings that apply only to earlier heads. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3585e1fe95
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
3585e1f to
9fec91f
Compare
|
@codex review\n\nPlease review the current PR head 9fec91f35d1568f1062447041f04f6ea6de1119b. Confirm that this exact commit is the reviewed commit and ignore findings that apply only to earlier heads. |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
9fec91f to
ccd4c34
Compare
There was a problem hiding this comment.
💡 Codex Review
https://github.com/danny-avila/LibreChat/blob/ccd4c344ef08cd7de571d21f8a145a67ebc7dada/api/server/services/ToolService.js#L2004-L2006
When list_workspace_files is the only call in a tool batch, this detector remains false because it recognizes only search_workspace. The agents controller passes a defined actionsEnabled boolean, so loadToolsForExecution then skips capability resolution, sets codeExecutionEnabled/statefulCodeSessions to false, and constructs the default code context; handleWorkspaceListCall consequently rejects the call because environmentType is no longer attached. Include the listing tool in this routing check, as is already done for workspace search.
https://github.com/danny-avila/LibreChat/blob/ccd4c344ef08cd7de571d21f8a145a67ebc7dada/packages/api/src/agents/handlers.ts#L2351
When the model follows a search_workspace match by passing the returned path to read_file, this output supplies src/app.ts even though attached files are routed only when the path starts with workspace/. The unprefixed path instead enters skill/sandbox resolution and commonly fails to read the matched file. Emit workspace/${match.path} here, matching handleWorkspaceListCall, so search results are directly usable by the advertised reader.
https://github.com/danny-avila/LibreChat/blob/ccd4c344ef08cd7de571d21f8a145a67ebc7dada/packages/api/src/code/workspace.ts#L175-L177
If the run is aborted after the worker has returned headers but while reader.read() is still waiting for body data, the stream rejects with an abort error here and this catch converts it to WorkspaceToolHttpError('invalid'). The outer cancellation check and all three workspace handlers can then no longer recognize the abort, so they return an ordinary tool error and the canceled agent run may continue. Preserve abort errors from the body reader just as executeWorkspaceTool already preserves aborts raised during fetchImpl.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
danny-avila
changed the title
|
Resolved the stack conflict and all three latest findings at current head 6b0c843: standalone list_workspace_files now participates in attached code-route resolution; search results use workspace/ paths; response-body cancellation is preserved from the lower layer. Listing output is additionally UTF-8 byte-bounded without partial paths. Focused verification: 335 package tests and 115 ToolService tests pass.\n\n@codex review commit 6b0c843 |
There was a problem hiding this comment.
🟡 Changes recommended
executeWorkspaceTool request validation currently rejects dot-segment paths like ./src, which contradicts the new tests and prevents intended attached-workspace usage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds a new model-facing discovery tool, list_workspace_files, to safely enumerate relative file paths inside attached code workspaces. It extends the existing attached-workspace HTTP bridge to support a list_files operation, ensures results are bounded/filterable, and wires the tool through agent registration/handler plumbing (including cancellation forwarding) and legacy /api service integration.
Changes:
- Add a
list_filesworkspace-tool request/response shape with strict validation, subtree confinement, and result limits. - Register
list_workspace_filesonly when workspace tools are enabled for attached environments, and route calls through the principal-selected Code API worker with abort forwarding. - Add/extend tests across the TypeScript and legacy JS layers to cover tool registration, handler behavior, filtering, and routing.
File summaries
| File | Description |
|---|---|
| packages/api/src/code/workspace.ts | Adds list_files protocol support and strengthens result validation (including subtree checks). |
| packages/api/src/code/workspace.spec.ts | Adds tests for dot-segment request paths and bounded file listing validation. |
| packages/api/src/agents/tools.ts | Introduces the list_workspace_files tool definition and registers it alongside other attached-workspace tools. |
| packages/api/src/agents/tools.spec.ts | Verifies historical tool-name retention and tool registration/shape for list_workspace_files. |
| packages/api/src/agents/handlers.ts | Implements list_workspace_files execution, filtering, truncation, and cancellation forwarding. |
| packages/api/src/agents/handlers.spec.ts | Adds handler tests for routing, truncation without partial paths, and filename filtering behavior. |
| packages/api/src/agents/background.ts | Ensures the new tool is excluded from background execution. |
| packages/api/src/agents/background.spec.ts | Tests that list_workspace_files stays in the foreground even if requested in background. |
| api/server/services/ToolService.js | Treats workspace listing as a code-execution tool for attached routing/capability gating. |
| api/server/services/Files/Code/process.js | Adds the legacy-layer listWorkspaceFiles implementation that calls executeWorkspaceTool. |
| api/server/services/Files/Code/process.spec.js | Tests authenticated forwarding and correct worker routing/headers for listings. |
| api/server/services/Endpoints/agents/skillDeps.js | Exposes listWorkspaceFiles in agent skill deps for handler wiring. |
| api/server/services/Endpoints/agents/skillDeps.spec.js | Verifies the dependency is present and forwarded through the deps object. |
| api/server/services/Endpoints/agents/initialize.spec.js | Updates expected tool-name sets to include list_workspace_files. |
| api/server/services/tests/ToolService.spec.js | Adds a regression test ensuring attached routing is preserved when listing is the only requested tool. |
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b0c843ac3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6df5914f97
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6df5914f97
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review exact head f90ad2c. This head depends on LibreChat-AI/code-interpreter#106 for the matching list continuation protocol. |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
565df56 to
65a52d2
Compare
|
@codex review Please review the current PR head 77c01e6ed606547f96d61c6a56b1a607c2227609. Confirm that this exact commit is the reviewed commit and ignore findings that apply only to earlier heads. |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
77c01e6 to
03cc788
Compare
Summary
I added model-facing file discovery for attached code environments so an agent can begin in an existing project, Git repository, or empty directory and learn the available workspace paths before reading them. This PR is stacked on #15525 and depends on LibreChat-AI/code-interpreter#92.
list_workspace_filesonly when the selected code environment is attached.Change Type
Testing
@librechat/apitests covering handlers, tool registration, and the workspace HTTP client.@librechat/codeworker test against a plain non-Git directory; the model-facing result wasworkspace/src/app.ts.Test Configuration:
127.0.0.1:23118127.0.0.1:26386primaryChecklist