Repository navigation
feat(project-files): let Chat read and write Project files - #9034
mzxchandra wants to merge 10 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
f7da653 to
8e82431
Compare
dc1b9e6 to
d66d69d
Compare
- Keep prepareProjectsForAccountDeletion returning the Project IDs to delete. The object return added an unused storageCleanupEventIds field; the durable cleanup outbox already processes those events, so restore the staging contract and its stronger integration assertions. - Exempt lib/mothership/generated from the relative-import convention. contracts:sync copies those wire contracts verbatim and byte-checks them, so their sibling imports must stay relative; drop their baseline rows. - Re-record the files page module-count baseline for the Project browser. - Type the test fixtures that regressed, including a queue-recovery fixture that persisted the invalid completion status 'success'.
Staging now holds resource-delegated Chat principals to content admission, so a Project callback read with no gate was refused and the sandbox got a 503. Run Project callbacks under the same workbench session gate as workspace callbacks (the gate's workspace is optional, since Projects have none) and share the withheld-result handling. The Project CLI read suite now runs reads under the tool executor's registry gate, as Chat does.
The chat resources API stored a Project-owned tab without checking the projects flag. Refuse new Project tabs while Projects are off; removing or reordering stored tabs stays possible so they can be cleaned up. Also drop the projects flag check in list_user_projects that could never run (listProjects already refuses while the flag is off) and the content delivery barrel export left unused by the shared sandbox delivery helper.
The Project files UI will be built for the project-first redesign, so this PR keeps only the backend and Chat transport behind the projects flag. Restore the workspace Files page, file viewer, Chat resource panel, add-resource and mention entries, and their client query hooks and owner adapters to staging. The previous tip is tagged project-files-ui-seed for that UI work. Kept: the shared ChatContext wire type (Project contexts and file owners reach the Chat server) and its exhaustive kind registry entry. Also drop the UI-only browser constants and response types nothing uses now, restore the files page module-count baseline, and type the use-chat fixtures that returned with the staging test.
Project grep and text reads admitted every Project artifact as opaque bytes, so any file carrying a secret was refused to Chat instead of having the secret masked, unlike the same file in a workspace. Share one text extraction core between workspace text reads and private file observation: parse, redact before a line window can cut a secret, then slice. Project text reads now go through readProjectFileText, which admits the source and every rendered input as complete text and returns text only. The complete artifact read stays private to that use case, so bytes never leave under a complete label; readProjectFileArtifact keeps admitting opaque bytes, and visual reads of a secret still refuse.
…s-product-v2 # Conflicts: # apps/sim/lib/mothership/request/application/recover-stream.ts
|
@cubic-dev-ai review this PR |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 116 files
Confidence score: 3/5
- In
workspace.ts, organization-target workbench receipts may not be scoped and audited to the selected workspace when the chat provides onlychatOrganizationId. PassorganizationId: context.chatOrganizationId ?? context.organizationId.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/lib/mothership/agent-cli/workspace.ts">
<violation number="1" location="apps/sim/lib/mothership/agent-cli/workspace.ts:59">
P2: Organization-target workbench receipts are scoped and audited to the selected workspace when the chat provides only `chatOrganizationId`. Pass `organizationId: context.chatOrganizationId ?? context.organizationId` so receipts retain organization scope across targets and audit records identify the owning organization.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
…tory #9057 hid the projects group from --help and `sim cli` search until launch, and the inventory printer dropped every hidden command, so the Mothership worker lost the Project file commands it routes --project to. List a hidden top-level group that is only unpublished, marked `unpublished`, so the worker can route to it once Sim enables Projects for a turn; other hidden commands stay out, and --help is unchanged.
[High impact] Adds file read and write access to the chat system. Fix the workspace file-tab regression and satisfy the test rule before merging. FindingsSummaryAdds Chat transport for discovering Projects and reading, searching, writing, uploading, and copying their files.
|
Workspace regressions while Projects are off: - Sandbox file copies between workspaces keep the workspace route when the lease has not negotiated the file-owner protocol; only pairs that name a Project owner need it. - Chat upload contexts keep their uploads/ path in the workspace owner adapter. Project transport: - Project CLI requests refuse mutations during benchmark reference generation, as workspace requests do. - Project grep defaults an omitted scope to files. - Project files with unrecorded or empty exact provenance are readable when the caller holds a registry; a missing registry is still refused. Mothership inventory: - Unpublished Project commands carry response shapes from the launch-ready Project OpenAPI definitions. - An operation whose delegated services exclude Chat no longer mints a Chat principal, so the refused-route list now matches runtime for Project file version history.
|
@cubic-dev-ai review this PR |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 121 files
Confidence score: 3/5
project-file-read.tstreats undecodable files as text because it checks fornullwhileworkspaceFileTextFormatreturnsundefined;files readtherefore fails on non-visual binary Project files. Check forundefinedas well.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/lib/mothership/agent-cli/project-file-read.ts">
<violation number="1" location="apps/sim/lib/mothership/agent-cli/project-file-read.ts:85">
P2: This check compares against `null`, but `workspaceFileTextFormat` returns `undefined` for undecodable files. As a result, `files read` on a non-visual binary Project file always attempts text extraction and fails instead of returning its binary metadata. Check for `undefined` (or use a truthiness check).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
| */ | ||
| const readsText = | ||
| !explicitVisual && | ||
| workspaceFileTextFormat({ name: resolved.file.name, type: resolvedType }) !== null && |
There was a problem hiding this comment.
P2: This check compares against null, but workspaceFileTextFormat returns undefined for undecodable files. As a result, files read on a non-visual binary Project file always attempts text extraction and fails instead of returning its binary metadata. Check for undefined (or use a truthiness check).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/sim/lib/mothership/agent-cli/project-file-read.ts, line 85:
<comment>This check compares against `null`, but `workspaceFileTextFormat` returns `undefined` for undecodable files. As a result, `files read` on a non-visual binary Project file always attempts text extraction and fails instead of returning its binary metadata. Check for `undefined` (or use a truthiness check).</comment>
<file context>
@@ -0,0 +1,184 @@
+ */
+ const readsText =
+ !explicitVisual &&
+ workspaceFileTextFormat({ name: resolved.file.name, type: resolvedType }) !== null &&
+ (textRange || !isDefaultVisual(resolvedType))
+ let file: typeof resolved.file
</file context>
| workspaceFileTextFormat({ name: resolved.file.name, type: resolvedType }) !== null && | |
| workspaceFileTextFormat({ name: resolved.file.name, type: resolvedType }) !== undefined && |
| const { workspaceId, ...rest } = resource | ||
| return { | ||
| ...rest, | ||
| owner: resource.owner ?? { entityType: 'workspace', entityId: workspaceId }, |
There was a problem hiding this comment.
Workspace tabs lose their address
normalizeChatResource removes workspaceId, but the browser also runs this helper through sanitizeChatResources and mergeChatResource. In organization Chat, workspace file tabs then lose the address their reader needs. Closing them sends neither workspaceId nor owner, so the saved tab does not match the DELETE request and returns after reload.
The HTTP and SSE presenter cannot fix this because the browser strips the restored address again. Keep the browser-facing address until its readers and close callbacks support owner, or keep this normalization server-only.
| it('stores a Project tab once Projects are on', async () => { | ||
| featureFlagsMockFns.mockIsFeatureEnabled.mockImplementation(async (flag) => flag === 'projects') | ||
| await changeChatResources.execute({ | ||
| principal, | ||
| input: { chatId: 'chat', change: { kind: 'upsert', resources: [projectFile] } }, | ||
| }) | ||
| expect(mocks.store).toHaveBeenCalledWith('chat', { kind: 'upsert', resources: [projectFile] }) |
There was a problem hiding this comment.
The new successful add and remove tests mock the surrounding authorization and storage calls, then only assert toHaveBeenCalledWith on storage. CLAUDE.md explicitly forbids tests that mock every collaborator and assert calls on those mocks. These tests do not show that a tab was saved or removed.
Cover these cases through the existing database-backed resource tests instead. This repository requirement must be satisfied before merging.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Summary
Lets Chat read, search, write, upload and copy Project files, on top of the Project files backend that #9015 through #9019 merged.
Everything stays behind the
projectsflag, which turns on only after the project-first redesign ships its Project files UI.Scope: backend and Chat transport only.
The Project file browser and other Project UI were removed from this PR, because the redesign will build that surface project-first rather than inside the workspace Files page.
The removed UI is preserved at the
project-files-ui-seedtag for that work.The only client change left is the shared
ChatContextwire type (Project contexts and file owners) and its kind registry entry.What Chat can do with Project files
projects list) and target one explicitly per command with--project <id>.Secret handling matches workspace files
readProjectFileTextadmits the source and every rendered input ascompletetext and returns text only, so bytes never leave under acompletelabel.readProjectFileArtifactstill admitsopaquebytes, so visual and byte reads of a file with a secret are still refused.Flag gating
list_user_projects, Project file use cases, and sandbox callbacks.list_user_projectsthat could never run was removed.Pairing with simstudioai/mothership#594
This PR and the worker companion merge back to back, Sim first.
Sim sends file-owner hints only when the worker's health check advertises the file-owner protocol, so merging Sim first changes nothing at runtime.
The worker byte-checks Sim's copies of the generated contracts, so the pair must land together; the companion pin will point at this PR's head on
staging.Type of Change
Testing
Reviewers: focus on
readProjectFileTextand the artifact view factory inartifacts.ts, the sharedextractFileTextFromBytes, and the Project sandbox callback gate.Checklist
🤖 Generated with Claude Code