Repository navigation
fix(files): let Mothership tool runs write workspace files - #9685
Merged
Merged
Conversation
`tools execute` authorizes the chat's delegated principal for `sim:tool-execution` and hands it to the tool layer unchanged, but every workspace-file use case admits only `sim:workspace-files`. Every File tool run directly from a Mothership chat therefore failed with a 403 "Delegated workspace access is no longer valid", which the agent relayed to the user as lost workspace access. The File tool handler now rebinds a delegated caller to the workspace-files audience (keeping subject, workspace, expiry ceiling and resource scope), as function-execute and payload materialization already do; the file use cases still check the subject's current workspace role. `image_generate` outside a workflow run returned only a URL for its copilot-storage upload, so the file-output processor downloaded and re-uploaded it, storing every image twice under two keys and returning `imageUrl` and `image` that disagreed. It now returns the stored file.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
 [Medium impact] The PR appears safe to merge. SummaryLets Mothership File tools use workspace-file operations and avoids storing generated images twice.
Reviews (3) · Last reviewed commit: "chore(files): import the principal facto..." · Reviewed by Greptile |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
…legations with 403 Generated images are now content-sniffed before they are stored, so every output field references the same verified object on both the workflow and chat paths; skipping the re-upload had otherwise skipped the sniff it did. A delegation minted for another workspace is no longer rebound (which threw and surfaced a 500); the workspace-file use cases refuse it with their ordinary 403. The regression test now uses the shared principal factory and uploads mock.
Collaborator
Author
Collaborator
Author
|
@cubic-dev-ai review this PR |
Contributor
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Collaborator
Author
Collaborator
Author
|
@cubic-dev-ai review this PR |
Contributor
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
This branch was previously deployed
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.
Summary
executeToolForCallerhands the chat's delegated principal (audiencesim:tool-execution) to in-process tools unchanged, and workspace-file use cases admit onlysim:workspace-files.executeFileToolnow rebinds a delegated caller withrebindWorkspaceFileDelegatedPrincipal, the same helper function-execute and payload materialization use. Subject, workspace, expiry ceiling and resource scope are kept, and the file use cases still check the subject's current role. A delegation for another workspace is left unchanged so they refuse it with their normal 403. File tools are the only consumer ofcallerPrincipal, and its TSDoc now says consumers must rebind.image_generateoutside a workflow run stored every image twice. The chat-path fallback returned only a URL, soFileToolProcessordownloaded and re-uploaded it under a second key.imageUrl/contentandimagethen pointed at different objects, and onlyimagewas content-sniffed.storeGeneratedImagenow sniffs once withresolveStoredFileMetadatabefore storing and returns the stored file asimageFileon both paths. The processor keeps it as is, so there is one object, one sniff, and consistent output fields.storeInternalToolFileResult. That keeps storage failures inside its fal "completed but delivery failed" metering reconciliation.Type of Change
Testing
lib/internal/file/delegated-caller-write.integration.tsdrives the realexecuteFileToolagainst Postgres with the principaltools executemints:expected 403 to be 200), green with the fix. Re-proved after the cleanup round.lib/internal/file/execute-tool.test.ts,lib/internal/image,tools/imageunit tests andbun run lintpass.bun run test,check:auditsandtype-checkare left to CI.Checklist
test-auditauthoring gate)