Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDK and command interface #86

Conversation

whilefoo
Copy link
Contributor

@whilefoo
Copy link
Contributor Author

@gentlementlegen can't request you for review

src/worker.ts Outdated Show resolved Hide resolved
@gentlementlegen
Copy link
Member

@whilefoo When I try to run this, I get the following error:
Manifest is invalid.

I am running the kernel from the related PR for LLM. Commands look like this in the manifest of this PR

  "commands": [
    {
      "name": "start",
      "ubiquity:example": "/start",
      "description": "Assign yourself and/or others to the issue/task.",
      "parameters": {
        "type": "object",
        "properties": {
          "teammates": {
            "description": "Users other than yourself to assign to the issue",
            "type": "array",
            "items": {
              "description": "Github username",
              "type": "string"
            }
          }
        }
      }
    },
    {
      "name": "stop",
      "ubiquity:example": "/stop",
      "description": "Unassign yourself from the issue/task.",
      "parameters": {
        "type": "object",
        "properties": {

        }
      }
    }
  ],

They are an array when they should be an object, why is that?

@whilefoo
Copy link
Contributor Author

Did you link to the new SDK using bun link @ubiquity-os/plugin-sdk?
I'm thinking if manifest and other types should be inside the kernel and the SDK imports the kernel, it makes more sense to make kernel the source of truth so to say.

They are an array when they should be an object, why is that?

Honestly I thought it's easier as an array to just put these objects into OpenAI functions but later realized it's neither easier nor harder, I can change back to the object if you think it's better

@gentlementlegen
Copy link
Member

@whilefoo The plus side of the object is that it prevents duplicate keys.

Yes I usually link with --watch so I don't have to think about it. We can get the code back to the kernel if you think that is beneficial, but as we discussed before we need to absolutely make sure there is no circular reference of any import otherwise that will break the compilation.

I had an awful time splitting the SDK itself, because of the following:

  • if everything is compiled within index.d.ts then every lib is included which is useless and can lead to circular imports or incompatible packages
  • splitting it with subpath is not possible except if using node16 option at least
  • node16 is really bad because it forces silly things like adding .js at the end of the import paths, and breaks some libs
  • there is the bundler option that mitigates that but doesn't solve everything
  • the trick I found was to use the future imports, that leverage part of the node16 and upwards features while being compatible with the plain node as seen here

Smashed my head on the desk a whole day to fix this.

@whilefoo
Copy link
Contributor Author

whilefoo commented Nov 20, 2024

Yes I usually link with --watch so I don't have to think about it. We can get the code back to the kernel if you think that is beneficial, but as we discussed before we need to absolutely make sure there is no circular reference of any import otherwise that will break the compilation.

I assume you built the SDK?
What I had to do is first use bun link @ubiquity-os/plugin-sdk in the SDK and then also run bun link @ubiquity-os/plugin-sdk in the plugin. Make sure to use bun install even if the plugin is using yarn, because you have to use the same package manager

  • the trick I found was to use the future imports, that leverage part of the node16 and upwards features while being compatible with the plain node as seen here

Ohh, that's why I couldn't import octokit subpath that I added, I didn't see that I also need to add typesVersion. I also tried to switching to different module resolutions but nothing worked

@gentlementlegen
Copy link
Member

gentlementlegen commented Nov 21, 2024

@whilefoo yes that's another reason why I switched to bun in the conversation rewards because it is easier to link with the same package manager. Usually using bun instead of yarn temporarily to install packages has not been a problem.

Yes if you don't add it node is incapable of resolving the path even if it is there and described in package.json, all will tell you to switch to node16 which then introduces too many problems.

@gentlementlegen gentlementlegen mentioned this pull request Nov 25, 2024
@gentlementlegen gentlementlegen merged commit 7b19d2c into ubiquity-os-marketplace:development Nov 25, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants