Conflicting Sync error. (#857)

When I added the one-shot `sync()` method I put in a guard to prevent it from being called while an interval was running. This caused a bug in the interval, so I've modified the code to account for it.

Also updated some type docs to get rid of the warning.

.changeset/tall-birds-dress.md

@@ -0,0 +1,8 @@
+---
+"@web5/agent": patch
+"@web5/identity-agent": patch
+"@web5/proxy-agent": patch
+"@web5/user-agent": patch
+---
+
+Sync vs StartSync conflicting error.

packages/agent/src/connect.ts

@@ -185,9 +185,27 @@ export type ConnectPermissionRequest = {
   permissionScopes: DwnPermissionScope[];
 };
 
+/**
+ * Shorthand for the types of permissions that can be requested.
+ */
 export type Permission = 'write' | 'read' | 'delete' | 'query' | 'subscribe';
 
-function createPermissionRequestForProtocol(definition: DwnProtocolDefinition, permissions: Permission[]): ConnectPermissionRequest {
+/**
+ * The options for creating a permission request for a given protocol.
+ */
+export type ProtocolPermissionOptions = {
+  /** The protocol definition for the protocol being requested */
+  definition: DwnProtocolDefinition;
+
+  /** The permissions being requested for the protocol */
+  permissions: Permission[];
+};
+
+/**
+ * Creates a set of Dwn Permission Scopes to request for a given protocol.
+ * If no permissions are provided, the default is to request all permissions (write, read, delete, query, subscribe).
+ */
+function createPermissionRequestForProtocol({ definition, permissions }: ProtocolPermissionOptions): ConnectPermissionRequest {
   const requests: DwnPermissionScope[] = [];
 
   // In order to enable sync, we must request permissions for `MessagesQuery`, `MessagesRead` and `MessagesSubscribe`

packages/agent/src/sync-engine-level.ts

@@ -288,7 +288,8 @@ export class SyncEngineLevel implements SyncEngine {
         }
 
         try {
-          await this.sync();
+          await this.push();
+          await this.pull();
         } catch (error: any) {
           this.stopSync();
           reject(error);

packages/agent/tests/connect.spec.ts

@@ -822,7 +822,9 @@ describe('web5 connect', function () {
         }
       };
 
-      const permissionRequests = WalletConnect.createPermissionRequestForProtocol(protocol, []);
+      const permissionRequests = WalletConnect.createPermissionRequestForProtocol({
+        definition: protocol, permissions: []
+      });
 
       expect(permissionRequests.protocolDefinition).to.deep.equal(protocol);
       expect(permissionRequests.permissionScopes.length).to.equal(3); // only includes the sync permissions
@@ -846,7 +848,9 @@ describe('web5 connect', function () {
         }
       };
 
-      const permissionRequests = WalletConnect.createPermissionRequestForProtocol(protocol, ['write', 'read']);
+      const permissionRequests = WalletConnect.createPermissionRequestForProtocol({
+        definition: protocol, permissions: ['write', 'read']
+      });
 
       expect(permissionRequests.protocolDefinition).to.deep.equal(protocol);
 
@@ -871,7 +875,9 @@ describe('web5 connect', function () {
         }
       };
 
-      const permissionRequests = WalletConnect.createPermissionRequestForProtocol(protocol, ['write', 'read', 'delete', 'query', 'subscribe']);
+      const permissionRequests = WalletConnect.createPermissionRequestForProtocol({
+        definition: protocol, permissions: ['write', 'read', 'delete', 'query', 'subscribe']
+      });
 
       expect(permissionRequests.protocolDefinition).to.deep.equal(protocol);
 

packages/agent/tests/sync-engine-level.spec.ts

@@ -2415,23 +2415,28 @@ describe('SyncEngineLevel', () => {
     });
 
     describe('startSync()', () => {
-      it('calls sync() in each interval', async () => {
+      it('calls pull() and push() in each interval', async () => {
         await testHarness.agent.sync.registerIdentity({
           did: alice.did.uri,
         });
 
-        const syncSpy = sinon.stub(SyncEngineLevel.prototype, 'sync');
-        syncSpy.resolves();
+        const pullSpy = sinon.stub(SyncEngineLevel.prototype as any, 'pull');
+        pullSpy.resolves();
+
+        const pushSpy = sinon.stub(SyncEngineLevel.prototype as any, 'push');
+        pushSpy.resolves();
 
         const clock = sinon.useFakeTimers();
 
         testHarness.agent.sync.startSync({ interval: '500ms' });
 
         await clock.tickAsync(1_400); // just under 3 intervals
-        syncSpy.restore();
+        pullSpy.restore();
+        pushSpy.restore();
         clock.restore();
 
-        expect(syncSpy.callCount).to.equal(2, 'push');
+        expect(pullSpy.callCount).to.equal(2, 'push');
+        expect(pushSpy.callCount).to.equal(2, 'pull');
       });
 
       it('does not call sync() again until a sync round finishes', async () => {
@@ -2441,24 +2446,30 @@ describe('SyncEngineLevel', () => {
 
         const clock = sinon.useFakeTimers();
 
-        const syncSpy = sinon.stub(SyncEngineLevel.prototype, 'sync');
-        syncSpy.returns(new Promise((resolve) => {
+        const pullSpy = sinon.stub(SyncEngineLevel.prototype as any, 'pull');
+        pullSpy.returns(new Promise<void>((resolve) => {
           clock.setTimeout(() => {
             resolve();
           }, 1_500); // more than the interval
         }));
 
+        const pushSpy = sinon.stub(SyncEngineLevel.prototype as any, 'push');
+        pushSpy.resolves();
+
         testHarness.agent.sync.startSync({ interval: '500ms' });
 
         await clock.tickAsync(1_400); // less time than the push
 
-        expect(syncSpy.callCount).to.equal(1, 'sync');
+        expect(pullSpy.callCount).to.equal(1, 'pull');
+        expect(pullSpy.callCount).to.equal(1, 'push');
 
         await clock.tickAsync(600); //remaining time for a 2nd sync
 
-        expect(syncSpy.callCount).to.equal(2, 'sync');
+        expect(pullSpy.callCount).to.equal(2, 'pull');
+        expect(pushSpy.callCount).to.equal(2, 'push');
 
-        syncSpy.restore();
+        pullSpy.restore();
+        pushSpy.restore();
         clock.restore();
       });
     });

packages/api/src/web5.ts

@@ -36,12 +36,28 @@ export type DidCreateOptions = {
   dwnEndpoints?: string[];
 }
 
+/**
+ * Represents a permission request for a protocol definition.
+ */
 export type ConnectPermissionRequest = {
+  /**
+   * The protocol definition for the protocol being requested.
+   */
   protocolDefinition: DwnProtocolDefinition;
+  /**
+   * The permissions being requested for the protocol. If none are provided, the default is to request all permissions.
+   */
   permissions?: Permission[];
 }
 
+/**
+ * Options for connecting to a Web5 agent. This includes the ability to connect to an external wallet
+ */
 export type ConnectOptions = Omit<WalletConnectOptions, 'permissionRequests'> & {
+  /**
+   * The permissions that are being requested for the connected DID.
+   * This is used to create the {@link ConnectPermissionRequest} for the wallet connect flow.
+   */
   permissionRequests: ConnectPermissionRequest[];
 }
 
@@ -286,9 +302,12 @@ export class Web5 {
         // No connected identity found and connectOptions are provided, attempt to import a delegated DID from an external wallet
         try {
           const { permissionRequests, ...connectOptions } = walletConnectOptions;
-          const walletPermissionRequests = permissionRequests.map(({ protocolDefinition, permissions }) => WalletConnect.createPermissionRequestForProtocol(protocolDefinition, permissions ?? [
-            'read', 'write', 'delete', 'query', 'subscribe'
-          ]));
+          const walletPermissionRequests = permissionRequests.map(({ protocolDefinition, permissions }) => WalletConnect.createPermissionRequestForProtocol({
+            definition  : protocolDefinition,
+            permissions : permissions ?? [
+              'read', 'write', 'delete', 'query', 'subscribe'
+            ]}
+          ));
 
           const { delegatePortableDid, connectedDid, delegateGrants } = await WalletConnect.initClient({
             ...connectOptions,

packages/api/tests/web5.spec.ts

@@ -846,7 +846,7 @@ describe('web5 api', () => {
           const call = requestPermissionsSpy.getCall(0);
 
           // since no explicit permissions were provided, all permissions should be requested
-          expect(call.args[1]).to.have.members([
+          expect(call.args[0].permissions).to.have.members([
             'read', 'write', 'delete', 'query', 'subscribe'
           ]);
         }
@@ -920,14 +920,14 @@ describe('web5 api', () => {
           const call1 = requestPermissionsSpy.getCall(0);
 
           // since no explicit permissions were provided for the first protocol, all permissions should be requested
-          expect(call1.args[1]).to.have.members([
+          expect(call1.args[0].permissions).to.have.members([
             'read', 'write', 'delete', 'query', 'subscribe'
           ]);
 
           const call2 = requestPermissionsSpy.getCall(1);
 
           // only the provided permissions should be requested for the second protocol
-          expect(call2.args[1]).to.have.members([
+          expect(call2.args[0].permissions).to.have.members([
             'read', 'write'
           ]);
         }