feat: a joined client node keeps its user's files in the pool #503

Merged
admin merged 2 commits from feat/joined-pool-files into dev 2026-10-19 13:56:24 +00:00
Owner

Part of #500.

A node that has joined another's pool as a client can now keep its user's files there through its own local API, the way it already submits jobs there (#474).

What changes

  • poolFiles (apps/sisyphusd/pool_files.go) is the local API's file store on a worker+client node. It stores and reads blobs through the worker's existing link to the coordinator, and pins the user's files there as this node's client pins. The file names stay in the node's own list.
  • Gating: every file call checks Available() first, the same rule as jobs. A worker alone gets FAILED_PRECONDITION ("not a client"); a node that is away gets UNAVAILABLE.
  • Coordinator: attemptOf no longer refuses an upload from a node that is both worker and client. Such a node uploads as a client, and is held to the client's share.
  • Chat attachments are refused for now, both in StoreFile and in the assistant, with a clear FAILED_PRECONDITION. They belong to other owners with their own retention, which is slice 2.
  • Private files are sealed on the node before the pool holds them, as before.

Tests

  • E2E TestAJoinedClientNodeKeepsItsFilesInThePool covers a worker-only refusal, then a client invitation. After it, the test stores, lists and fetches a file, checks the coordinator's client pin, and runs a wordcount job on the CID. It also checks that a private file is not held in the clear, that a chat attachment is refused, and that removing the file releases the pin.
  • Unit TestAJoinedNodesFileStoreKeepsOnlyTheUsersFiles and TestAnAssistantThatKeepsNoAttachmentsRefusesThem.
  • make cover: every statement is covered. gofmt and vet are clean.

Docs

docs/desktop-backend.md: files on a joined node.

@excho0, feel free to push to this branch if anything reads wrong to you.

Part of #500. A node that has joined another's pool as a **client** can now keep its user's files there through its own local API, the way it already submits jobs there (#474). ## What changes - **`poolFiles`** (`apps/sisyphusd/pool_files.go`) is the local API's file store on a worker+client node. It stores and reads blobs through the worker's existing link to the coordinator, and pins the user's files there as this node's client pins. The file names stay in the node's own list. - **Gating:** every file call checks `Available()` first, the same rule as jobs. A worker alone gets `FAILED_PRECONDITION` ("not a client"); a node that is away gets `UNAVAILABLE`. - **Coordinator:** `attemptOf` no longer refuses an upload from a node that is both worker and client. Such a node uploads as a client, and is held to the client's share. - **Chat attachments are refused for now**, both in `StoreFile` and in the assistant, with a clear `FAILED_PRECONDITION`. They belong to other owners with their own retention, which is slice 2. - **Private files** are sealed on the node before the pool holds them, as before. ## Tests - E2E `TestAJoinedClientNodeKeepsItsFilesInThePool` covers a worker-only refusal, then a client invitation. After it, the test stores, lists and fetches a file, checks the coordinator's client pin, and runs a `wordcount` job on the CID. It also checks that a private file is not held in the clear, that a chat attachment is refused, and that removing the file releases the pin. - Unit `TestAJoinedNodesFileStoreKeepsOnlyTheUsersFiles` and `TestAnAssistantThatKeepsNoAttachmentsRefusesThem`. - `make cover`: every statement is covered. gofmt and vet are clean. ## Docs `docs/desktop-backend.md`: files on a joined node. @excho0, feel free to push to this branch if anything reads wrong to you.
Owner

I added a focused regression test for the worker+client quota edge case: a node holding both roles cannot upload beyond the client's share. The focused Go test passes locally, and I recorded this as a pending checklist item on #500; I’ll leave it unchecked until the PR lands. Austin, please include the test when you squash-merge once CI is green, preserving my authorship for this contribution in the squash commit.

/cc @rvnminers-A-and-N

I added a focused regression test for the worker+client quota edge case: a node holding both roles cannot upload beyond the client's share. The focused Go test passes locally, and I recorded this as a pending checklist item on #500; I’ll leave it unchecked until the PR lands. Austin, please include the test when you squash-merge once CI is green, preserving my authorship for this contribution in the squash commit. /cc @rvnminers-A-and-N
Author
Owner

Thanks @excho0, that's exactly the edge the attemptOf change opened up: a worker+client node uploading past the client's share. Good to have it pinned down. I'll keep you as co-author in the squash commit once CI is green. I'll also carry your test onto the stacked follow-up for slice 2 (chat attachments on a joined node), which is about to open.

Thanks @excho0, that's exactly the edge the `attemptOf` change opened up: a worker+client node uploading past the client's share. Good to have it pinned down. I'll keep you as co-author in the squash commit once CI is green. I'll also carry your test onto the stacked follow-up for slice 2 (chat attachments on a joined node), which is about to open.
@@ -0,0 +80,4 @@
}
var errs []error
for _, c := range cids {
_, err := f.client.Unpin(context.Background(), &pb.UnpinBlobRequest{Cid: c.String()})
Owner

Potential failure-path consistency issue: localService.RemoveFile deletes the local catalogue row before this remote Unpin call. If the coordinator becomes unreachable after storeAvailable() but before this RPC completes, the API returns an error after the file has disappeared from ListFiles, while the coordinator can retain its permanent client pin (TtlSeconds: 0) and quota usage. Could we preserve a retry path (for example, unpin before deleting the row after verifying it exists, or keep a pending release) and cover a failed remote Unpin? This also applies to the stacked #505 path. /cc @rvnminers-A-and-N

Potential failure-path consistency issue: `localService.RemoveFile` deletes the local catalogue row before this remote `Unpin` call. If the coordinator becomes unreachable after `storeAvailable()` but before this RPC completes, the API returns an error after the file has disappeared from `ListFiles`, while the coordinator can retain its permanent client pin (`TtlSeconds: 0`) and quota usage. Could we preserve a retry path (for example, unpin before deleting the row after verifying it exists, or keep a pending release) and cover a failed remote Unpin? This also applies to the stacked #505 path. /cc @rvnminers-A-and-N
excho0 left a comment
Owner

I requested one retryability fix on the remote Unpin failure path. A temporary coordinator disconnect can currently drop the local file catalogue row while leaving the permanent client pin/quota behind; please preserve a recoverable release path and test it. The CI rerun itself is green. /cc @rvnminers-A-and-N

I requested one retryability fix on the remote Unpin failure path. A temporary coordinator disconnect can currently drop the local file catalogue row while leaving the permanent client pin/quota behind; please preserve a recoverable release path and test it. The CI rerun itself is green. /cc @rvnminers-A-and-N
Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Sisyphus/Sisyphus#503