-
Notifications
You must be signed in to change notification settings - Fork 5
Feat/sdk subgraph required #66
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
Conversation
WalkthroughAdds Changesets docs and config, CI workflows for PR validation/prerelease/release, a pnpm monorepo root and package metadata across packages. SDK API: Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor SDKUser as SDK User
participant EnsembleCreate as Ensemble.create
participant AgentSvc as AgentService
participant GQL as GraphQLClient
SDKUser->>EnsembleCreate: create(config { subgraphUrl, ... })
EnsembleCreate->>AgentSvc: new AgentService(registry, subgraphUrl, signer, ipfsSDK)
AgentSvc->>GQL: initialize(subgraphUrl)
note right of AgentSvc #DDEFEF: subgraph client mandatory and always initialized
SDKUser->>AgentSvc: getAgentRecords()/getAgentsByOwner()
AgentSvc->>GQL: query(...)
GQL-->>AgentSvc: results
AgentSvc-->>SDKUser: data
sequenceDiagram
autonumber
actor Dev as PR Author
participant GH as GitHub Actions
participant Job as PR Validation Job
participant CS as pnpm changeset
Dev->>GH: open/sync PR
GH->>Job: trigger validate
Job->>Job: checkout → setup pnpm/node → install → typecheck → build → test
alt actor != dependabot
Job->>CS: pnpm changeset status --verbose
CS-->>Job: output
alt "No changesets present"
Job-->>GH: fail PR (request changeset)
else
Job-->>GH: pass
end
else dependabot
Job-->>GH: skip changeset check
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
| subgraphUrl: string; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making subgraphUrl required in EnsembleConfig could affect anyone initializing the SDK without this field. Was this change to a required field intentional for all consumers?
Finding type: Type Inconsistency
Heads up!
Your free trial ends in 2 days.
To keep getting your PRs reviewed by Baz, update your team's subscription
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
packages/sdk/README.md (2)
151-156: Fix incorrect instantiation (remove “new” and await the Promise)
Ensemble.createappears to be an async factory (as shown later in the doc). The current snippet usesnewand doesn’t await, which will confuse users.-// creating the ensemble sdk -const ensemble = new Ensemble.create(config, signer); +// creating the ensemble sdk +const ensemble = await Ensemble.create(config, signer);
160-176: Correct method namespace and typos; await completion callThe API reference below states task methods live under
ensemble.tasks. Also several typos and a missingawaitcould mislead users.-const executeTask = async (task) => { - console.log(`receieved a new task ${task.id} to the agent proposal ${task.proposalId} by user ${task.issuer}`) - console.log(`task prompt: ${task.prompt}`) - - // TODO: Optionaly validate the task and propmpt - - // Task Execution - // This is a KOL tas to wrtie a tweet about the topic, so twitter client is used - runtime.character.topics = [task.prompt] - const tweet = await runtime.clients.twitter.post.generateNewTweet() - - // Competing the task with a result - ensemble.completeTask(task.id, `Done tweet about topic: ${tweet.url}`) -} +const executeTask = async (task) => { + console.log(`Received a new task ${task.id} for proposal ${task.proposalId} by user ${task.issuer}`); + console.log(`Task prompt: ${task.prompt}`); + + // TODO: Optionally validate the task and prompt + + // Task Execution + // This is a KOL task to write a tweet about the topic, so the Twitter client is used + runtime.character.topics = [task.prompt]; + const tweet = await runtime.clients.twitter.post.generateNewTweet(); + + // Completing the task with a result + await ensemble.tasks.completeTask(task.id, `Tweet posted: ${tweet.url}`); +}packages/sdk/src/types.ts (2)
94-110: Type inconsistency:TaskStatusenum (number) vsTaskData.status(bigint)
TaskData.status: bigintdoesn’t match the numeric enum. Mixingbigintwithnumbercomplicates comparisons and switch statements.Options:
- Make
TaskData.status: TaskStatus(preferred), or- Use a BigInt enum-like union:
type TaskStatusBI = 0n | 1n | 2n | 3n;and align usage accordingly, or- Change event and data models to use
BigNumberishconsistently if values originate onchain.Follow-up impacts: query mappers, tests, and any code doing
if (task.status === TaskStatus.COMPLETED).
164-168: AlignTaskCreationParams.proposalIdwith other typesElsewhere
proposalIdisbigint/BigNumberish. Here it’sstring, which will force ad-hoc casts.export interface TaskCreationParams { prompt: string; - proposalId: string; + proposalId: BigNumberish; }packages/sdk/src/services/AgentService.ts (3)
374-376: Review inconsistency with constructor changes.The method still checks for
subgraphClientinitialization, but this is now unnecessary since the constructor guarantees the client is always initialized. This check should be removed to align with the constructor changes.Apply this diff to remove the obsolete initialization check:
- if (!this.subgraphClient) { - throw new Error("Subgraph client is not initialized. Please provide a subgraphUrl in the config."); - }
540-542: Remove obsolete subgraph client check.Similar to the previous method, this initialization check is no longer needed since the constructor guarantees the client is always initialized.
Apply this diff to remove the obsolete initialization check:
- if (!this.subgraphClient) { - throw new Error("Subgraph client is not initialized. Please provide a subgraphUrl in the config."); - }
615-617: Remove obsolete subgraph client check.This initialization check should also be removed for consistency with the constructor changes.
Apply this diff to remove the obsolete initialization check:
- if (!this.subgraphClient) { - throw new Error("Subgraph client is not initialized. Please provide a subgraphUrl in the config."); - }
🧹 Nitpick comments (31)
packages/sdk/README.md (3)
139-149: Call out the new required config: consider clarifying “from which version” and that it must be a non-empty URLThe added
subgraphUrlline is good. To reduce support churn, explicitly state the minimum SDK version that requires it and that it must be a valid, non-empty URL string.subgraphUrl: process.env.SUBGRAPH_URL, // Required for agent queries + // Note: Required starting in @ensemble-ai/sdk >= 0.6.2. Must be a valid, non-empty URL.
326-333: Standardize environment variable naming for subgraph URLEarlier you use
process.env.SUBGRAPH_URLwhile this section introducesENSEMBLE_SUBGRAPH_URL. Recommend using a single name across the README to prevent misconfiguration.-ENSEMBLE_SUBGRAPH_URL=https://api.goldsky.com/api/public/project_cmcnps2k01akp01uobifl4bby/subgraphs/ensemble-subgraph/0.0.5/gn +SUBGRAPH_URL=https://api.goldsky.com/api/public/project_cmcnps2k01akp01uobifl4bby/subgraphs/ensemble-subgraph/0.0.5/gnIf you prefer the
ENSEMBLE_prefix, change the earlier code sample toprocess.env.ENSEMBLE_SUBGRAPH_URLaccordingly.
36-41: Grammar and clarity nits in “Agent Integration” introTighten language and fix typos for a more professional read.
-Agent can do many things, thay can create tasks and solve tasks, create new services, delegate work to other agents, and more. But in this manual, we want to integrate the agents as a service provider. There's two parts to the integration: +Agents can do many things: create and solve tasks, create new services, delegate work to other agents, and more. In this manual, we focus on integrating an agent as a service provider. There are two parts to the integration:packages/sdk/src/types.ts (3)
185-191: MakingsubgraphUrlrequired is correct; consider documenting non-empty URL requirement and adding runtime validationThe type change aligns with the SDK behavior. To catch misconfigurations early, add a brief JSDoc and (optionally) a runtime assertion in the SDK initialization path.
export interface EnsembleConfig { taskRegistryAddress: string; agentRegistryAddress: string; serviceRegistryAddress: string; network: NetworkConfig; - subgraphUrl: string; + /** + * Subgraph endpoint used for agent queries. + * Must be a valid, non-empty URL. Required since @ensemble-ai/sdk >= 0.6.2. + */ + subgraphUrl: string; }If you already use zod for schemas, consider a small
EnsembleConfigSchemawithz.string().url()forsubgraphUrl.
61-64: Type inconsistency:TaskRatedEvent.rating: numbervsTaskData.rating?: bigintThese should align to avoid lossy conversions and awkward casts.
- If ratings come from chain logs, prefer
bigint/BigNumberish.- If ratings are app-layer integers (e.g., 1–5), standardize on
numbereverywhere and convert at boundaries.
122-130: Minor: PreferBigNumberishforProposal.idfor symmetry
Proposal.idisbigintwhile other onchain identifiers often useBigNumberish. Consider aligning for consistency across interfaces..changeset/config.json (1)
1-14: Config looks solid and matches independent versioning
access: "public"andbaseBranch: "main"are standard.updateInternalDependencies: "patch"and the unsafe peer-dependent option help reduce churn.Optional tweaks to consider later:
- Set a snapshot prerelease template if you plan frequent alphas:
changeset pre enter alpha+snapshot.prereleaseTemplate.- If you want Changesets to commit during
version, set"commit": trueand let CI push. Currentfalseis fine if your workflows commit/push explicitly.package.json (2)
25-27: Make “clean” cross-platform
rm -rfwill fail on Windows shells commonly used by contributors. Prefer a cross-platform tool.Option A (no extra deps, uses git):
-"clean": "pnpm -r --parallel exec rm -rf dist node_modules", +"clean": "git clean -xfd -e \"!**/.vscode/**\""Option B (add rimraf):
+"devDependencies": { + "rimraf": "^5.0.5", + ... +}, -"clean": "pnpm -r --parallel exec rm -rf dist node_modules", +"clean": "pnpm -r exec rimraf dist node_modules"
26-27: Prettier format scope could include common file typesTo keep configs consistent, consider adding yml/yaml and root files.
-"format": "prettier --write \"packages/**/*.{ts,tsx,js,jsx,json,md}\"", +"format": "prettier --write \"**/*.{ts,tsx,js,jsx,json,md,yml,yaml}\"",.changeset/README.md (3)
36-39: Minor wording: hyphenate “backward-compatible” and tighten phrasingSmall wording improvement to match common style.
-- **minor** (0.x.0): New features, new API methods (backwards compatible) +- **minor** (0.x.0): New features, new API methods (backward-compatible)
80-86: Clarify the automated release steps reference to CIMake it explicit that CI opens the release PR after merging to main.
-3. CI creates "Release packages" PR +3. CI opens a "Release packages" PR with version bumps and changelogs
89-91: Explicitly tie to config:commit: falsebehaviorSince the config sets
"commit": false, helpful to note that CI will commit changes produced bychangeset:version.pnpm changeset:version # Update versions and changelogs -pnpm changeset:publish # Publish to npm +pnpm changeset:publish # Publish to npm +# Note: Our config uses `"commit": false`; CI will commit and push the versioned changes..github/workflows/pr-validation.yml (3)
39-47: Harden the changeset check; avoid subshell string test and use grep -q for reliabilityUsing command substitution with test [ "$()" ] works but is brittle. Prefer a straightforward pipeline with grep -q and an explicit negation path. This also avoids false positives from whitespace/newlines.
- - name: Check for changeset (skip for dependabot) + - name: Check for changeset (skip for dependabot) if: github.actor != 'dependabot[bot]' run: | - if [ "$(pnpm changeset status --verbose | grep 'No changesets present')" ]; then - echo "::error::Please add a changeset by running 'pnpm changeset'" - echo "::error::This helps us track changes and generate release notes" - exit 1 - else - echo "✅ Changeset found" - fi + if pnpm changeset status --verbose | grep -q "No changesets present"; then + echo "::error::Please add a changeset by running 'pnpm changeset'" + echo "::error::This helps us track changes and generate release notes" + exit 1 + else + echo "✅ Changeset found" + fi
1-8: Fix YAML lint issues: remove trailing spaces and add a final newlineSeveral lines have trailing spaces and there’s no newline at EOF. Clean these up to satisfy linters and keep history clean.
name: PR Validation @@ - fetch-depth: 0 - + fetch-depth: 0 @@ - version: 8 - + version: 8 @@ - cache: 'pnpm' - + cache: 'pnpm' @@ - run: pnpm install --frozen-lockfile - + run: pnpm install --frozen-lockfile @@ - run: pnpm typecheck - + run: pnpm typecheck @@ - run: pnpm build - + run: pnpm build @@ - run: pnpm test - + run: pnpm test @@ - echo "✅ Changeset found" - fi + echo "✅ Changeset found" + fi +Also applies to: 15-38, 48-48
7-9: Consider adding concurrency to cancel superseded runs on synchronize eventsOptional, but helps keep CI noise down on rapid pushes to the same PR.
jobs: + # Cancel in-flight runs on the same PR + concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true validate: runs-on: ubuntu-latest.github/workflows/release.yml (2)
44-53: Enable npm provenance attestation for published packagesYou already set id-token: write, which is required. Add NPM_CONFIG_PROVENANCE or pass --provenance so published artifacts include SLSA provenance. This is increasingly expected by npm consumers.
- name: Create Release Pull Request or Publish id: changesets uses: changesets/action@v1 with: publish: pnpm changeset:publish title: "chore: release packages" commit: "chore: release packages" env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + NPM_CONFIG_PROVENANCE: trueIf your
changeset:publishscript does not pass through tonpm publish --provenance, consider:- publish: pnpm changeset:publish + publish: pnpm changeset:publish -- --provenance
12-16: Fix YAML lint issues: remove trailing spaces and add a final newlineTrailing spaces on multiple lines and missing newline at EOF. Minor cleanup to keep workflows tidy.
permissions: contents: write pull-requests: write id-token: write - + @@ with: fetch-depth: 0 - + @@ with: version: 8 - + @@ cache: 'pnpm' registry-url: 'https://registry.npmjs.org' - + @@ - run: pnpm install --frozen-lockfile - + run: pnpm install --frozen-lockfile + @@ - run: pnpm build - + run: pnpm build + @@ - run: pnpm test - + run: pnpm test + @@ - NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} +Also applies to: 21-22, 26-27, 33-34, 36-37, 39-40, 42-43, 53-53
.github/workflows/prerelease.yml (3)
55-61: Commit step is resilient; consider also tagging the pre mode commit for clarityCurrent guard
|| exit 0is good. Optionally, append the tag to the commit body for traceability.- name: Commit prerelease changes run: | git config --local user.email "action@github.com" git config --local user.name "GitHub Action" git add . - git commit -m "chore: enter ${{ github.event.inputs.tag }} prerelease mode" || exit 0 + git commit -m "chore: enter ${{ github.event.inputs.tag }} prerelease mode" -m "changeset:pre:${{ github.event.inputs.tag }}" || exit 0
15-17: Add concurrency to avoid overlapping prerelease runs with the same refUseful when the workflow is triggered multiple times accidentally.
jobs: + concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true prerelease: runs-on: ubuntu-latest
18-21: Fix YAML lint issues: trailing spaces and final newlineClean up trailing spaces and add newline at EOF.
permissions: contents: write id-token: write - + @@ with: fetch-depth: 0 - + @@ with: version: 8 - + @@ cache: 'pnpm' registry-url: 'https://registry.npmjs.org' - + @@ - run: pnpm install --frozen-lockfile - + run: pnpm install --frozen-lockfile + @@ - run: pnpm build - + run: pnpm build + @@ - run: pnpm test - + run: pnpm test + @@ - run: pnpm changeset pre enter ${{ github.event.inputs.tag }} - + run: pnpm changeset pre enter ${{ github.event.inputs.tag }} + @@ - run: pnpm changeset:version - + run: pnpm changeset:version + @@ env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} - + @@ - name: Push changes - run: git push --follow-tags + run: git push --follow-tags +Also applies to: 26-27, 31-32, 38-39, 41-42, 44-45, 47-48, 50-51, 53-54, 61-61, 66-66, 68-68
packages/mcp-server/package.json (2)
21-27: Make clean script cross-platform; add files whitelist to reduce package size
rm -rffails on Windows; preferrimraf. Also consider whitelisting published files."scripts": { - "clean": "rm -rf dist", + "clean": "rimraf dist", "build": "tsc", "dev": "ts-node src/index.ts", "start": "node dist/src/index.js", "test": "jest --config jest.config.ts" }, + "files": [ + "dist", + "README.md", + "LICENSE" + ],Add devDependency:
"devDependencies": { + "rimraf": "^6.0.0",
1-4: Optional: declare engines to match CI Node versionHelps consumers avoid unsupported Node versions and aligns with workflows using Node 18.
"devDependencies": { "@jest/globals": "^29.7.0", @@ "typescript": "^5.4.2" - } + }, + "engines": { + "node": ">=18" + }Also applies to: 35-43
packages/cli/CHANGELOG.md (1)
7-14: Backticks and style nits in bulletsMinor readability nits: consistently wrap identifiers like subgraphUrl/AgentService in backticks; prefer em dash or colon for “now always initialized.”
The diff above already applies these tweaks; no additional changes required beyond that.
packages/cli/package.json (2)
29-29: Consider adding engines to communicate runtime requirementsCommunicating Node/pnpm requirements at the package level aids tooling and users.
"scripts": { "clean": "rm -rf dist", "build": "tsc",Add below the scripts block (exact placement doesn’t matter in JSON):
+ "engines": { + "node": ">=18.0.0" + },
10-13: Optional: avoid shipping TS sources under bin/You’re mapping the binary to
dist/bin/ensemble.jswhilefilesincludes"bin". This may publish the TypeScript sources (if present) unnecessarily."files": [ "dist", - "bin" + // "bin" not needed since the executable points to dist/bin ],CONTRIBUTING.md (2)
191-203: Specify a language for the directory tree fenced block (markdownlint MD040)Add a language hint to satisfy linters and improve rendering.
-``` +```text ensemble-framework/ ├── packages/ │ ├── sdk/ # @ensemble-ai/sdk ...--- `38-56`: **Add a dev tip for CLI usage in local development** Per prior learnings, contributors should prefer `pnpm dev` over invoking `ensemble` directly unless globally linked. Add this to reduce friction. We used your stored preference from long-term learnings to include this suggestion. ```diff ### Package Scripts ... ```bash # Build all packages pnpm build @@ # Type checking pnpm typecheck
+#### CLI development tip
+
+When developing the CLI locally, use the CLI package’s dev script instead of invokingensembledirectly (unless you’ve runnpm link):
+
+bash +# From repo root +pnpm --filter @ensemble-ai/cli dev <command> +# Example: +pnpm --filter @ensemble-ai/cli dev agents list +</blockquote></details> <details> <summary>packages/subgraph/package.json (1)</summary><blockquote> `19-28`: **Check subgraph slug consistency across scripts** `deploy` targets `ensemble-v2` while local scripts use `ensemble-hub`. If that’s intentional (different hosted slug vs. local), consider documenting it. If not, align the slug to prevent accidental deployments to the wrong target. If they should be the same: ```diff - "deploy": "graph deploy ensemble-v2", + "deploy": "graph deploy ensemble-hub",Or parameterize via an env var (document default in README):
- "deploy": "graph deploy ensemble-v2", + "deploy": "graph deploy ${GRAPH_SLUG:-ensemble-hub}",packages/sdk/package.json (3)
24-40: Prepublish hooks: prefer prepare/prepublishOnly and avoid duplicate typechain runs
prebuildrunstypechain, andprepublishalso runstypechain && build. NPM’sprepublishcan run on local installs unexpectedly. Preferprepare(runs on install from git) plusprepublishOnly(runs only on publish). Also avoid runningtypechaintwice."scripts": { - "clean": "rm -rf dist typechain", - "prebuild": "npm run typechain", - "prepublish": "npm run typechain && npm run build", + "clean": "rm -rf dist typechain", + "prepare": "npm run typechain && npm run build", + "prepublishOnly": "npm run build", "build": "tsc",
1-21: Tiny packaging optimization: add sideEffects for better tree-shakingMarking the package as side-effect free helps bundlers. Only do this if modules truly have no top-level side effects.
{ "name": "@ensemble-ai/sdk", - "version": "0.6.2", + "version": "0.7.0", "description": "TypeScript SDK for the Ensemble Framework", "main": "dist/index.js", "types": "dist/index.d.ts", + "sideEffects": false,
41-51: Declareethersas a peerDependency in the SDK packageTo avoid shipping a duplicate copy of
etherswhen consumers already install it, move it out of your regulardependenciesand intopeerDependencies(with a matching entry indevDependenciesfor local development).• File: packages/sdk/package.json
- Remove the
"ethers": "^6.9.0"entry from thedependenciesblock.- Add a
peerDependenciessection requiring"ethers": "^6.9.0".- Add
etherstodevDependenciesso you can still build and test locally.Suggested diff:
--- a/packages/sdk/package.json +++ b/packages/sdk/package.json @@ "dependencies": { "@jest/globals": "^29.7.0", "chai": "^4.3.6", "dotenv": "^16.4.7", - "ethers": "^6.9.0", "graphql": "^16.9.0", "graphql-request": "^7.1.2", "pinata-web3": "^0.5.4", "zod": "^4.0.17" }, + "peerDependencies": { + "ethers": "^6.9.0" + }, + "devDependencies": { + "ethers": "^6.9.0" + },This ensures that consuming applications install exactly one version of
ethersand eliminates the risk of version skew or bundle bloat.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
.changeset/README.md(1 hunks).changeset/config.json(1 hunks).github/workflows/pr-validation.yml(1 hunks).github/workflows/prerelease.yml(1 hunks).github/workflows/release.yml(1 hunks)CONTRIBUTING.md(1 hunks)package.json(1 hunks)packages/cli/CHANGELOG.md(1 hunks)packages/cli/package.json(3 hunks)packages/contracts/package.json(2 hunks)packages/mcp-server/package.json(1 hunks)packages/sdk/CHANGELOG.md(2 hunks)packages/sdk/README.md(1 hunks)packages/sdk/package.json(1 hunks)packages/sdk/src/ensemble.ts(1 hunks)packages/sdk/src/services/AgentService.ts(2 hunks)packages/sdk/src/types.ts(1 hunks)packages/sdk/test/agentService.test.ts(1 hunks)packages/sdk/test/ensemble.test.ts(2 hunks)packages/subgraph/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-12T08:16:42.572Z
Learnt from: CR
PR: ensemble-codes/ensemble-framework#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-12T08:16:42.572Z
Learning: Use the `ensemble` CLI directly only after running `npm link` or when the package is installed globally via npm.
Applied to files:
packages/cli/package.json
📚 Learning: 2025-08-12T08:16:42.572Z
Learnt from: CR
PR: ensemble-codes/ensemble-framework#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-12T08:16:42.572Z
Learning: When suggesting Ensemble CLI commands in development, use `pnpm dev <command>` (e.g., `pnpm dev agents list`) instead of invoking `ensemble` directly.
Applied to files:
packages/cli/package.json
🪛 YAMLlint (1.37.1)
.github/workflows/release.yml
[error] 16-16: trailing spaces
(trailing-spaces)
[error] 22-22: trailing spaces
(trailing-spaces)
[error] 27-27: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 37-37: trailing spaces
(trailing-spaces)
[error] 40-40: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 53-53: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/prerelease.yml
[error] 21-21: trailing spaces
(trailing-spaces)
[error] 27-27: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 39-39: trailing spaces
(trailing-spaces)
[error] 42-42: trailing spaces
(trailing-spaces)
[error] 45-45: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
[error] 51-51: trailing spaces
(trailing-spaces)
[error] 54-54: trailing spaces
(trailing-spaces)
[error] 61-61: trailing spaces
(trailing-spaces)
[error] 66-66: trailing spaces
(trailing-spaces)
[error] 68-68: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/pr-validation.yml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 20-20: trailing spaces
(trailing-spaces)
[error] 26-26: trailing spaces
(trailing-spaces)
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 38-38: trailing spaces
(trailing-spaces)
[error] 48-48: no new line character at the end of file
(new-line-at-end-of-file)
🪛 LanguageTool
.changeset/README.md
[grammar] ~29-~29: There might be a mistake here.
Context: ... bash pnpm changeset This will: 1. Ask which packages have changed 2. Ask w...
(QB_NEW_EN)
[grammar] ~36-~36: There might be a mistake here.
Context: ...mall improvements, documentation updates - minor (0.x.0): New features, new API m...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ..., new API methods (backwards compatible) - major (x.0.0): Breaking changes, API c...
(QB_NEW_EN)
[grammar] ~72-~72: There might be a mistake here.
Context: ...sdk**: Core SDK, follows semver strictly - @ensemble-ai/cli: CLI tool, minor bump...
(QB_NEW_EN)
[grammar] ~73-~73: There might be a mistake here.
Context: ...: CLI tool, minor bumps for new commands - @ensemble-ai/contracts: Smart contract...
(QB_NEW_EN)
[grammar] ~74-~74: There might be a mistake here.
Context: ...tracts, major bumps for contract changes - @ensemble-ai/subgraph: Subgraph indexe...
(QB_NEW_EN)
[grammar] ~75-~75: There might be a mistake here.
Context: ...ph indexer, syncs with contract versions - @ensemble-ai/mcp-server: MCP server, i...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ...ase Process ### Automated (Recommended) 1. Create changeset with your PR 2. Merge P...
(QB_NEW_EN)
[grammar] ~84-~84: There might be a mistake here.
Context: ...tes "Release packages" PR 4. Review and merge release PR 5. CI publishes to npm ### ...
(QB_NEW_EN)
packages/cli/CHANGELOG.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...### Patch Changes - 02eed68: fix: Make subgraphUrl required parameter in SDK configuration...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...on - SDK: subgraphUrl is now required in EnsembleConfig interface - SDK: Agent...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...e - SDK: AgentService constructor now requires subgraphUrl parameter - SDK: Updated ...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...ameter - SDK: Updated parameter order in AgentService constructor (subgraphUrl c...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...SDK: Removed conditional initialization of GraphQL client - now always initialized...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ... Documentation: Updated README examples with required subgraphUrl parameter This ...
(QB_NEW_EN)
CONTRIBUTING.md
[grammar] ~7-~7: There might be a mistake here.
Context: ... Table of Contents - Development Setup - Making Changes - [Rele...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...p](#development-setup) - Making Changes - Release Process - [Pa...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...ges](#making-changes) - Release Process - Package Structure -...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...](#release-process) - Package Structure - Guidelines ## Development...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ... ### Prerequisites - Node.js >= 18.0.0 - pnpm >= 8.0.0 - Git ### Installation `...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ...tes - Node.js >= 18.0.0 - pnpm >= 8.0.0 - Git ### Installation ```bash # Clone t...
(QB_NEW_EN)
[grammar] ~99-~99: There might be a mistake here.
Context: ... bash pnpm changeset This will: 1. Ask which packages have changed 2. Ask w...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ...mall improvements, documentation updates - minor: New features, new API methods (...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ..., new API methods (backwards compatible) - major: Breaking changes, API changes t...
(QB_NEW_EN)
[grammar] ~237-~237: There might be a mistake here.
Context: ... Breaking Changes For breaking changes: 1. Mark changeset as major 2. Document th...
(QB_NEW_EN)
[grammar] ~238-~238: There might be a mistake here.
Context: ... breaking changes: 1. Mark changeset as major 2. Document the breaking change clearly 3. ...
(QB_NEW_EN)
[grammar] ~255-~255: There might be a mistake here.
Context: ... Guidelines #### SDK (@ensemble-ai/sdk) - Follow semantic versioning strictly - Ma...
(QB_NEW_EN)
[grammar] ~261-~261: There might be a mistake here.
Context: ...public APIs #### CLI (@ensemble-ai/cli) - Maintain command backward compatibility ...
(QB_NEW_EN)
[grammar] ~267-~267: There might be a mistake here.
Context: ... #### Contracts (@ensemble-ai/contracts) - Test thoroughly on testnets before mainn...
(QB_NEW_EN)
[grammar] ~273-~273: There might be a mistake here.
Context: ...s #### Subgraph (@ensemble-ai/subgraph) - Version in sync with contract changes - ...
(QB_NEW_EN)
[grammar] ~274-~274: There might be a mistake here.
Context: ... - Version in sync with contract changes - Test queries thoroughly - Document schem...
(QB_NEW_EN)
[grammar] ~275-~275: There might be a mistake here.
Context: ...ntract changes - Test queries thoroughly - Document schema changes - Include deploy...
(QB_NEW_EN)
[grammar] ~276-~276: There might be a mistake here.
Context: ...ies thoroughly - Document schema changes - Include deployment URLs ## Getting Help...
(QB_NEW_EN)
packages/sdk/CHANGELOG.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...### Patch Changes - 02eed68: fix: Make subgraphUrl required parameter in SDK configuration...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...on - SDK: subgraphUrl is now required in EnsembleConfig interface - SDK: Agent...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...e - SDK: AgentService constructor now requires subgraphUrl parameter - SDK: Updated ...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...ameter - SDK: Updated parameter order in AgentService constructor (subgraphUrl c...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...SDK: Removed conditional initialization of GraphQL client - now always initialized...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ... Documentation: Updated README examples with required subgraphUrl parameter This ...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...ar error messages when write operations require signer ### Changed - All services now...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ... renamed**: socketio-eliza renamed to eliza - Update your agent registration to use `c...
(QB_NEW_EN)
[grammar] ~62-~62: There might be a mistake here.
Context: ...s)` ### Added - Zod validation schemas - SDK agent record fixes - Agent Record ty...
(QB_NEW_EN)
[grammar] ~63-~63: There might be a mistake here.
Context: ...idation schemas - SDK agent record fixes - Agent Record types fixes ### Changed -...
(QB_NEW_EN)
[grammar] ~68-~68: There might be a mistake here.
Context: ...nged - Integrating zod for agent record - SDK types cleaning - Bumping version ##...
(QB_NEW_EN)
[grammar] ~69-~69: There might be a mistake here.
Context: ...od for agent record - SDK types cleaning - Bumping version ## [0.5.5] - 2025-08-03...
(QB_NEW_EN)
[grammar] ~146-~146: There might be a mistake here.
Context: ...### Fixed - Hotfix: sdk install failure - Bugfix: add fixes for agent interaction ...
(QB_NEW_EN)
[grammar] ~215-~215: There might be a mistake here.
Context: ...### Changed - Releasing the sdk version - Updating documentation ## [0.2.3] - 202...
(QB_NEW_EN)
[grammar] ~238-~238: There might be a mistake here.
Context: ...2025-01-16 ### Added - Ensemble SDK v2 - Services registry - Create task improvem...
(QB_NEW_EN)
[grammar] ~239-~239: There might be a mistake here.
Context: ...d - Ensemble SDK v2 - Services registry - Create task improvements ### Fixed - F...
(QB_NEW_EN)
[grammar] ~244-~244: There might be a mistake here.
Context: ...ements ### Fixed - Fixing TaskRegistry - Fixing services and small fixes ## [0.1...
(QB_NEW_EN)
[grammar] ~280-~280: There might be a mistake here.
Context: ...- 2024-12-19 ### Added - Adding README - Adding documentation and integration det...
(QB_NEW_EN)
🪛 GitHub Actions: PR Validation
packages/sdk/src/ensemble.ts
[error] 20-20: TypeScript error TS2307: Cannot find module '../typechain' or its corresponding type declarations. (Command: pnpm -r typecheck)
🪛 markdownlint-cli2 (0.17.2)
CONTRIBUTING.md
191-191: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (17)
packages/sdk/src/types.ts (1)
185-191: I’m digging into thesetupSdk.tsscript to confirm whether itsconfigliteral includes the newsubgraphUrlfield. Running a targeted grep on theconfigdeclaration for type annotation and literal contents..github/workflows/release.yml (1)
7-8: Nice: concurrency guard will prevent overlapping releasesGood use of a workflow- and ref-scoped concurrency key. Nothing to change.
packages/sdk/CHANGELOG.md (1)
8-12: All AgentService instantiations updated to new constructor signature
- packages/sdk/src/ensemble.ts (around line 86):
new AgentService(agentRegistry, config.subgraphUrl, signer, ipfsSDK)- packages/sdk/test/agentService.test.ts (around line 33):
new AgentService(mockRegistry, "https://mock-subgraph-url.com", mockSigner)No other TypeScript call sites or README samples use
AgentService(, and all found usages match(registry, subgraphUrl, signer?, ipfsSDK?). No further changes needed.packages/mcp-server/package.json (1)
7-20: Metadata additions look solid and align with monorepo publishingRepository, publishConfig, license, homepage, and bugs are correctly set for public publishing on npm.
packages/cli/package.json (3)
14-18: Repository metadata looks goodAdding repository metadata (type/url/directory) is correct and will improve npm page linkage.
19-27: Publish config, license, homepage/bugs: LGTMThese are standard and correctly set for public publishing.
3-3: Version bump acknowledged; ensure aligned with changesetsCLI version 0.1.1 aligns with a patch-level doc/metadata update; no runtime changes here.
CONTRIBUTING.md (1)
58-89: All referenced scripts are presentI’ve verified that the scripts mentioned in CONTRIBUTING.md are defined in the project:
Root package.json includes
• test:sdk
• test:cli
• typecheckpackages/sdk/package.json includes build, test, and typecheck scripts
packages/cli/package.json includes build, test, and typecheck scripts
packages/contracts/package.json includes test scripts (test, test:coverage, test:gas)
No updates to the documentation are needed here.
packages/subgraph/package.json (1)
2-18: Publishing metadata looks solidRenaming to the scoped package, adding version/description, and public publish config are consistent with the monorepo changes.
packages/contracts/package.json (1)
2-24: LGTM! Comprehensive monorepo package configuration.The updates align the contracts package with monorepo standards by adding proper publishing metadata, repository information, and standardized licensing. The scoped package name, public access, and included files are all appropriately configured for npm publishing.
packages/sdk/test/ensemble.test.ts (2)
62-63: LGTM! Tests correctly updated for required subgraphUrl.The test configuration properly includes the required
subgraphUrlfield that aligns with the updated SDK API.
85-86: LGTM! Consistent test configuration.Both test cases now correctly include the required
subgraphUrlparameter.packages/sdk/src/ensemble.ts (1)
86-86: LGTM! Constructor parameter order correctly updated.The AgentService constructor call has been properly updated to match the new parameter order where
subgraphUrlis now the second parameter.packages/sdk/test/agentService.test.ts (1)
33-38: LGTM! Test updated to match new AgentService constructor.The test correctly instantiates AgentService with the new 4-parameter constructor signature where
subgraphUrlis now the second parameter.packages/sdk/src/services/AgentService.ts (3)
72-72: LGTM! Subgraph client is now guaranteed to be initialized.Making the subgraph client non-optional ensures it's always available after construction, eliminating potential runtime errors from uninitialized clients.
108-115: LGTM! Constructor properly enforces required subgraphUrl.The updated constructor signature makes
subgraphUrla required parameter and unconditionally initializes the GraphQL client, which eliminates the need for runtime checks throughout the service.
657-657: All subgraph operations are correctly wired upI’ve confirmed that:
- The
AgentServiceconstructor always initializesthis.subgraphClientwith the providedsubgraphUrl(lines 113–115)- Every subgraph-based method (
getAgentRecord,searchAgents,getAgentCount, and the newgetAgentRecords) usesthis.subgraphClient.request(...)and will throw only if the client initialization somehow failed- In
ensemble.ts,AgentServiceis instantiated withconfig.subgraphUrl(line 86), ensuring a valid URL is passed throughAs long as a valid
subgraphUrlis provided when constructingAgentService, all subgraph queries will execute as expected. No further changes are required here.
| ### Patch Changes | ||
|
|
||
| - 02eed68: fix: Make subgraphUrl required parameter in SDK configuration | ||
| - SDK: subgraphUrl is now required in EnsembleConfig interface | ||
| - SDK: AgentService constructor now requires subgraphUrl parameter | ||
| - SDK: Updated parameter order in AgentService constructor (subgraphUrl comes after registry) | ||
| - SDK: Removed conditional initialization of GraphQL client - now always initialized | ||
| - CLI: Already provides subgraphUrl from config, no breaking changes for CLI users | ||
| - Tests: Updated all test configurations to include subgraphUrl | ||
| - Documentation: Updated README examples with required subgraphUrl parameter | ||
|
|
||
| This ensures that agent query methods (`getAgentRecords`, `getAgentsByOwner`, etc.) always work correctly by guaranteeing subgraph connectivity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Explicitly mark SDK breaking changes; tighten wording
Making subgraphUrl required and reordering AgentService constructor args are breaking for SDK consumers. This should be called out to prevent silent breakages for downstreams skimming the CLI changelog.
Proposed edits to make the breaking nature unmistakable and improve consistency:
-### Patch Changes
+### Patch Changes
-- 02eed68: fix: Make subgraphUrl required parameter in SDK configuration
+- 02eed68: feat!: Make `subgraphUrl` a required parameter in the SDK configuration (**BREAKING – SDK**)
- SDK: subgraphUrl is now required in EnsembleConfig interface
- - SDK: AgentService constructor now requires subgraphUrl parameter
- - SDK: Updated parameter order in AgentService constructor (subgraphUrl comes after registry)
- - SDK: Removed conditional initialization of GraphQL client - now always initialized
+ - SDK: `AgentService` constructor now requires `subgraphUrl`
+ - SDK: Constructor parameter order updated (`subgraphUrl` comes after `registry`) (**BREAKING – SDK**)
+ - SDK: Removed conditional initialization of the GraphQL client — it is now always initialized
- CLI: Already provides subgraphUrl from config, no breaking changes for CLI users
- - Tests: Updated all test configurations to include subgraphUrl
- - Documentation: Updated README examples with required subgraphUrl parameter
+ - Tests: Updated all test configurations to include `subgraphUrl`
+ - Documentation: Updated README examples with required `subgraphUrl`
This ensures that agent query methods (`getAgentRecords`, `getAgentsByOwner`, etc.) always work correctly by guaranteeing subgraph connectivity.Follow-up: ensure the SDK’s own CHANGELOG also carries a clear “BREAKING” note with migration guidance.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### Patch Changes | |
| - 02eed68: fix: Make subgraphUrl required parameter in SDK configuration | |
| - SDK: subgraphUrl is now required in EnsembleConfig interface | |
| - SDK: AgentService constructor now requires subgraphUrl parameter | |
| - SDK: Updated parameter order in AgentService constructor (subgraphUrl comes after registry) | |
| - SDK: Removed conditional initialization of GraphQL client - now always initialized | |
| - CLI: Already provides subgraphUrl from config, no breaking changes for CLI users | |
| - Tests: Updated all test configurations to include subgraphUrl | |
| - Documentation: Updated README examples with required subgraphUrl parameter | |
| This ensures that agent query methods (`getAgentRecords`, `getAgentsByOwner`, etc.) always work correctly by guaranteeing subgraph connectivity. | |
| ### Patch Changes | |
| - 02eed68: feat!: Make `subgraphUrl` a required parameter in the SDK configuration (**BREAKING – SDK**) | |
| - SDK: subgraphUrl is now required in EnsembleConfig interface | |
| - SDK: `AgentService` constructor now requires `subgraphUrl` | |
| - SDK: Constructor parameter order updated (`subgraphUrl` comes after `registry`) (**BREAKING – SDK**) | |
| - SDK: Removed conditional initialization of the GraphQL client — it is now always initialized | |
| - CLI: Already provides subgraphUrl from config, no breaking changes for CLI users | |
| - Tests: Updated all test configurations to include `subgraphUrl` | |
| - Documentation: Updated README examples with required `subgraphUrl` | |
| This ensures that agent query methods (`getAgentRecords`, `getAgentsByOwner`, etc.) always work correctly by guaranteeing subgraph connectivity. |
🧰 Tools
🪛 LanguageTool
[grammar] ~7-~7: There might be a mistake here.
Context: ...### Patch Changes - 02eed68: fix: Make subgraphUrl required parameter in SDK configuration...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...on - SDK: subgraphUrl is now required in EnsembleConfig interface - SDK: Agent...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...e - SDK: AgentService constructor now requires subgraphUrl parameter - SDK: Updated ...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...ameter - SDK: Updated parameter order in AgentService constructor (subgraphUrl c...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...SDK: Removed conditional initialization of GraphQL client - now always initialized...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ... Documentation: Updated README examples with required subgraphUrl parameter This ...
(QB_NEW_EN)
🤖 Prompt for AI Agents
packages/cli/CHANGELOG.md lines 5-16: the entry describes making subgraphUrl
required and reordering AgentService constructor args but doesn't explicitly
mark these as BREAKING changes or provide migration guidance; update the
changelog to prepend a clear "BREAKING CHANGES" header for this bullet, tighten
wording to explicitly state that SDK consumers must update calls to the
AgentService constructor (new parameter order: registry, subgraphUrl, ...) and
that subgraphUrl is now mandatory in EnsembleConfig, and add a short migration
note showing the old vs new constructor signature and a reminder to update
dependent code; also add a follow-up note that the SDK package's own CHANGELOG
must include the same BREAKING section with migration steps.
| "main": "dist/src/index.js", | ||
| "bin": "dist/src/index.js", | ||
| "repository": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Bin entry likely needs a shim and shebang to be executable after build
Pointing bin directly to dist/src/index.js works only if that file has a Unix shebang and executable bit set in the published tarball. Since tsc typically emits plain JS without a shebang, provide a small bin shim checked into source with a shebang.
- "bin": "dist/src/index.js",
+ "bin": {
+ "mcp-server": "bin/mcp-server.js"
+ },Create packages/mcp-server/bin/mcp-server.js:
#!/usr/bin/env node
require('../dist/src/index.js');Optionally add a postbuild to ensure the shim is executable:
"scripts": {
"clean": "rimraf dist",
- "build": "tsc",
+ "build": "tsc",
+ "postbuild": "chmod +x bin/mcp-server.js || true",Verification script to flag missing shebang/executable:
Would you like me to open a follow-up PR adding the shim?
🏁 Script executed:
#!/bin/bash
# Check for bin shim and shebang
rg -n '^#!/usr/bin/env node' packages/mcp-server/bin/mcp-server.js || { echo "Missing shebang in bin shim"; exit 1; }Length of output: 220
Add an executable shim with a shebang for the mcp-server CLI
The bin field in packages/mcp-server/package.json still points directly to dist/src/index.js, and the suggested shim file (packages/mcp-server/bin/mcp-server.js) doesn’t exist—so the published package will not expose a working CLI. Please:
• Update the bin field in packages/mcp-server/package.json
- "bin": "dist/src/index.js",
+ "bin": {
+ "mcp-server": "bin/mcp-server.js"
+ },• Create packages/mcp-server/bin/mcp-server.js with a Unix shebang:
#!/usr/bin/env node
require('../dist/src/index.js');Make sure to commit this file so it’s published.
• Ensure the shim is executable after build by adding a postbuild step:
"scripts": {
"clean": "rimraf dist",
"build": "tsc",
+ "postbuild": "chmod +x bin/mcp-server.js || true",
…
},• (Optionally) Add a CI or lint check to verify the shim’s presence, shebang, and executable bit.
🤖 Prompt for AI Agents
In packages/mcp-server/package.json around lines 5-7 the "bin" currently points
to dist/src/index.js which prevents a proper CLI; update the "bin" to
"bin/mcp-server.js", add a new file packages/mcp-server/bin/mcp-server.js
containing a Unix shebang that requires the built entry
(require('../dist/src/index.js')), commit that shim so it ships, and add a
postbuild step in package.json to ensure the shim is executable (e.g. chmod +x
./bin/mcp-server.js) so the executable bit is preserved after build; optionally
add a CI or lint check to verify the shim exists, has the shebang, and is
executable.
| ## 0.6.2 | ||
|
|
||
| ### Patch Changes | ||
|
|
||
| - 02eed68: fix: Make subgraphUrl required parameter in SDK configuration | ||
| - SDK: subgraphUrl is now required in EnsembleConfig interface | ||
| - SDK: AgentService constructor now requires subgraphUrl parameter | ||
| - SDK: Updated parameter order in AgentService constructor (subgraphUrl comes after registry) | ||
| - SDK: Removed conditional initialization of GraphQL client - now always initialized | ||
| - CLI: Already provides subgraphUrl from config, no breaking changes for CLI users | ||
| - Tests: Updated all test configurations to include subgraphUrl | ||
| - Documentation: Updated README examples with required subgraphUrl parameter | ||
|
|
||
| This ensures that agent query methods (`getAgentRecords`, `getAgentsByOwner`, etc.) always work correctly by guaranteeing subgraph connectivity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Polish: add date and tighten wording for 0.6.2; this is user-facing
Changelog entries should include a date and use consistent, concise phrasing. Proposed edit keeps content intact while improving clarity and aligning with Keep a Changelog.
-## 0.6.2
-
-### Patch Changes
-
-- 02eed68: fix: Make subgraphUrl required parameter in SDK configuration
- - SDK: subgraphUrl is now required in EnsembleConfig interface
- - SDK: AgentService constructor now requires subgraphUrl parameter
- - SDK: Updated parameter order in AgentService constructor (subgraphUrl comes after registry)
- - SDK: Removed conditional initialization of GraphQL client - now always initialized
- - CLI: Already provides subgraphUrl from config, no breaking changes for CLI users
- - Tests: Updated all test configurations to include subgraphUrl
- - Documentation: Updated README examples with required subgraphUrl parameter
-
- This ensures that agent query methods (`getAgentRecords`, `getAgentsByOwner`, etc.) always work correctly by guaranteeing subgraph connectivity.
+## [0.6.2] - 2025-08-25
+
+### Patch
+
+- 02eed68: Make `subgraphUrl` a required SDK configuration parameter.
+ - SDK: `subgraphUrl` is now required on the `EnsembleConfig` interface.
+ - SDK: `AgentService` constructor now requires `subgraphUrl`; parameter order is `registry, subgraphUrl, signer?, ipfsSDK?`.
+ - SDK: GraphQL client is always initialized (removed conditional init).
+ - CLI: Already supplies `subgraphUrl`; no breaking change for CLI users.
+ - Tests: Updated all test configs to include `subgraphUrl`.
+ - Docs: README examples updated to show required `subgraphUrl`.
+
+This guarantees subgraph connectivity so agent query methods (for example, `getAgentRecords`, `getAgentsByOwner`) work reliably.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## 0.6.2 | |
| ### Patch Changes | |
| - 02eed68: fix: Make subgraphUrl required parameter in SDK configuration | |
| - SDK: subgraphUrl is now required in EnsembleConfig interface | |
| - SDK: AgentService constructor now requires subgraphUrl parameter | |
| - SDK: Updated parameter order in AgentService constructor (subgraphUrl comes after registry) | |
| - SDK: Removed conditional initialization of GraphQL client - now always initialized | |
| - CLI: Already provides subgraphUrl from config, no breaking changes for CLI users | |
| - Tests: Updated all test configurations to include subgraphUrl | |
| - Documentation: Updated README examples with required subgraphUrl parameter | |
| This ensures that agent query methods (`getAgentRecords`, `getAgentsByOwner`, etc.) always work correctly by guaranteeing subgraph connectivity. | |
| ## [0.6.2] - 2025-08-25 | |
| ### Patch | |
| - 02eed68: Make `subgraphUrl` a required SDK configuration parameter. | |
| - SDK: `subgraphUrl` is now required on the `EnsembleConfig` interface. | |
| - SDK: `AgentService` constructor now requires `subgraphUrl`; parameter order is `registry, subgraphUrl, signer?, ipfsSDK?`. | |
| - SDK: GraphQL client is always initialized (removed conditional init). | |
| - CLI: Already supplies `subgraphUrl`; no breaking change for CLI users. | |
| - Tests: Updated all test configs to include `subgraphUrl`. | |
| - Docs: README examples updated to show required `subgraphUrl`. | |
| This guarantees subgraph connectivity so agent query methods (for example, `getAgentRecords`, `getAgentsByOwner`) work reliably. |
🧰 Tools
🪛 LanguageTool
[grammar] ~7-~7: There might be a mistake here.
Context: ...### Patch Changes - 02eed68: fix: Make subgraphUrl required parameter in SDK configuration...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...on - SDK: subgraphUrl is now required in EnsembleConfig interface - SDK: Agent...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...e - SDK: AgentService constructor now requires subgraphUrl parameter - SDK: Updated ...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...ameter - SDK: Updated parameter order in AgentService constructor (subgraphUrl c...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...SDK: Removed conditional initialization of GraphQL client - now always initialized...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ... Documentation: Updated README examples with required subgraphUrl parameter This ...
(QB_NEW_EN)
🤖 Prompt for AI Agents
packages/sdk/CHANGELOG.md lines 3-16: the 0.6.2 entry lacks a release date and
uses verbose phrasing; update the header to include the release date
(YYYY-MM-DD) after the version, tighten and standardize wording to concise items
following Keep a Changelog style (e.g., "Added/Changed/Fixed: short
description"), condense bullet points into short, consistent lines (remove
redundant "SDK:" prefixes where implied), and ensure the summary sentence is
brief and user-facing.
| "version": "0.6.2", | ||
| "description": "TypeScript SDK for the Ensemble Framework", | ||
| "main": "dist/index.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SemVer: patch bump is insufficient for breaking SDK API changes
You’ve made subgraphUrl required in EnsembleConfig and changed AgentService constructor parameter order. Both are breaking for SDK consumers. Under semver (even in 0.y.z), breaking changes should bump the minor version at minimum. Recommend 0.7.0.
- "version": "0.6.2",
+ "version": "0.7.0",
- "description": "TypeScript SDK for the Ensemble Framework",
+ "description": "TypeScript SDK for the Ensemble Framework",Follow-up:
- Update the changeset for the SDK to “minor” (or “major” if you’re at >=1.0.0 later).
- Clearly mark the change as BREAKING in the SDK’s CHANGELOG with migration notes.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "version": "0.6.2", | |
| "description": "TypeScript SDK for the Ensemble Framework", | |
| "main": "dist/index.js", | |
| "version": "0.7.0", | |
| "description": "TypeScript SDK for the Ensemble Framework", | |
| "main": "dist/index.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/cli/src/utils/sdk.ts (3)
23-27: Harden validation: trim and verify http(s) URL to prevent subtle misconfigsA whitespace-only value or a malformed URL will currently slip through. Consider tightening the check.
Apply this diff within this block:
- // Validate required subgraphUrl - if (!config.subgraphUrl) { - throw new Error('subgraphUrl is required in CLI config. Please ensure your config includes a subgraphUrl.'); - } + // Validate required subgraphUrl + const subgraphUrl = (config.subgraphUrl ?? '').trim(); + if (!subgraphUrl) { + throw new Error( + 'subgraphUrl is required in CLI config. Provide it in your CLI config or via SUBGRAPH_URL env var.' + ); + } + try { + const { protocol } = new URL(subgraphUrl); + if (protocol !== 'http:' && protocol !== 'https:') { + throw new Error(); + } + } catch { + throw new Error(`Invalid subgraphUrl "${config.subgraphUrl}". Expected a valid http(s) URL.`); + }
7-21: Fail even earlier to avoid unnecessary work (optional)Minor UX nit: move the subgraphUrl validation to just after loading the config so we don’t spin up a provider/signer when we’re going to exit anyway.
Example placement:
export async function createSDKInstance(providedSigner?: ethers.Signer): Promise<Ensemble> { const config = await getConfig(); // validate before provider/signer creation const subgraphUrl = (config.subgraphUrl ?? '').trim(); if (!subgraphUrl) { throw new Error('subgraphUrl is required in CLI config.'); } // optionally: URL validity check here // Create provider const provider = new ethers.JsonRpcProvider(config.rpcUrl); // ... }
34-36: Network mapping is brittle; prefer explicit mapping or reading from providerUsing a binary ternary on a string risks mis-routing if config.network ever holds values like "base", "base-sepolia", or "staging".
Consider either reading the chainId from the provider you already created:
const { chainId } = await provider.getNetwork();or use an explicit map with a guard:
const CHAIN_IDS = { mainnet: 8453, sepolia: 84532 } as const; const chainId = CHAIN_IDS[config.network as keyof typeof CHAIN_IDS]; if (!chainId) throw new Error(`Unsupported network "${config.network}". Expected one of: ${Object.keys(CHAIN_IDS).join(', ')}`);If other parts of the repo already define a canonical network enum/map, import and reuse that instead to keep this consistent. Can you confirm the accepted values for config.network in the CLI config so we align this mapping?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/cli/src/utils/sdk.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/cli/src/utils/sdk.ts (1)
packages/mcp-server/src/config.ts (1)
config(4-18)
🔇 Additional comments (1)
packages/cli/src/utils/sdk.ts (1)
23-27: Good addition: runtime guard for required subgraphUrlThis aligns the CLI with the SDK’s new required field and fails fast before constructing the Ensemble config. Clear, actionable error message too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/sdk/package.json (1)
41-51: Don’t ship test tooling and script-only libs to consumersOur search confirms that
@jest/globals,jest, andchaiare not imported anywhere underpackages/sdk/src, making them purely test-time dependencies.dotenvis only imported inpackages/sdk/scripts/utils/setupSdk.ts, i.e. used only in SDK setup scripts.To prevent downstream installs from being bloated or conflicting, please:
- Remove
@jest/globals,chai,jest, anddotenvfrom the"dependencies"block inpackages/sdk/package.json(currently lines 41–51).- Add them (with the same versions) under
"devDependencies".- Ensure the existing
jestentry in devDependencies remains and that@jest/globals,chai, anddotenvare also listed there.Suggested diff:
--- packages/sdk/package.json @@ -41,11 +41,7 @@ "dependencies": { - "@jest/globals": "^29.7.0", - "chai": "^4.3.6", - "dotenv": "^16.4.7", "ethers": "^6.9.0", "graphql": "^16.9.0", "graphql-request": "^7.1.2", - "jest": "^29.7.0", "pinata-web3": "^0.5.4", "zod": "^4.0.17" }, @@ -52,6 +48,10 @@ "devDependencies": { "@babel/preset-typescript": "^7.26.0", "@graphprotocol/client-cli": "^3.0.7", + "@jest/globals": "^29.7.0", + "chai": "^4.3.6", + "dotenv": "^16.4.7", "@swc/jest": "^0.2.37", "@typechain/ethers-v6": "^0.5.1", "@types/chai": "^5.0.1", "@types/jest": "^29.5.14", "@types/node": "^20.10.0", "jest": "^29.7.0", "ts-jest": "^29.2.5", "typechain": "^8.3.2", "typedoc": "^0.27.4", "typescript": "^5.3.2" }Locations needing updates:
- packages/sdk/package.json (remove and reclassify deps in both sections).
♻️ Duplicate comments (1)
packages/sdk/package.json (1)
3-3: SemVer: breaking SDK API requires a minor bump or a Changesets-driven bumpYou made
subgraphUrlrequired and reorderedAgentServiceconstructor params—both are breaking. Publishing as 0.6.x is misleading. Either:
- bump to 0.7.0 here, or
- revert manual version edits and add a “minor” Changeset so CI produces a release PR that bumps to 0.7.0.
I recommend the second approach to align with your new Changesets workflow.
If you’re using Changesets, run this to confirm there is a “minor” changeset for @ensemble-ai/sdk:
#!/bin/bash # Verify there's at least one 'minor' changeset for @ensemble-ai/sdk rg -nP '\b"@ensemble-ai/sdk"\b' .changeset -C2 | sed -n '1,120p' rg -nP 'bump:\s*minor' .changeset -C2 | sed -n '1,120p'If you choose to fix directly in this PR, apply:
- "version": "0.6.2", + "version": "0.7.0",
🧹 Nitpick comments (6)
packages/sdk/package.json (6)
15-18: publishConfig: consider enabling npm provenanceOptional, but recommended for supply-chain trust. Add provenance so
npm publishincludes build attestation.Apply:
"publishConfig": { "access": "public", - "registry": "https://registry.npmjs.org/" + "registry": "https://registry.npmjs.org/", + "provenance": true },
19-23: Metadata looks good; add engines for runtime guardrailsTo avoid users installing on unsupported Node versions (GraphQL/ethers often expect >=18), declare engines.
"license": "MIT", "homepage": "https://github.com/ensemble-codes/ensemble-framework/tree/main/packages/sdk", "bugs": { "url": "https://github.com/ensemble-codes/ensemble-framework/issues" }, + "engines": { + "node": ">=18.17" + },Confirm the repo root contains a LICENSE file matching MIT so npm page links correctly.
25-25: Use cross-platform clean; avoid POSIX-only rm
rm -rffails on Windows runners and some dev machines. Preferrimraf."scripts": { - "clean": "rm -rf dist typechain", + "clean": "rimraf dist typechain", ... }, "devDependencies": { + "rimraf": "^5.0.5",Also applies to: 52-65
27-27: Prefer prepublishOnly (or prepack) over prepublish
prepublishruns on local install in older npm versions and can surprise contributors. UseprepublishOnlyfor publish-time, orprepackto prepare tarball contents.- "prepublish": "npm run typechain && npm run build", + "prepublishOnly": "npm run typechain && npm run build",
33-33: Typecheck shouldn’t mutate the tree; split generation from checkingRunning codegen as part of typecheck causes side effects in CI and local editors. Make codegen an explicit pre-step.
Option A (clean separation):
- "typecheck": "npm run typechain && tsc --noEmit", + "typecheck": "tsc --noEmit", + "pretypecheck": "npm run typechain",Option B (CI-only):
- Keep
typecheckpure and addtypecheck:cithat runs codegen first, then tsc.
41-51: Consider making pinata-web3 optional or a peer depIf Pinata integration is optional at runtime (as the PR description suggests), consider:
- lazy/dynamic import, and
- moving
pinata-web3topeerDependencies(optionallyoptionalDependencies) to reduce install size for users who don’t need it.Confirm whether core SDK paths require
pinata-web3unconditionally. If not, I can propose a small dynamic import pattern to keep it optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/sdk/package.json(1 hunks)
🔇 Additional comments (2)
packages/sdk/package.json (2)
4-4: Description tweak looks goodNo issues; reads clearly and matches the project branding.
10-14: Repository metadata: good additionURL and directory look correct for a monorepo package.
Please confirm the package path remains
packages/sdkafter this PR so the “directory” field doesn’t drift.
| currentMetadata = await response.json() as AgentMetadata; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to validate the response shape at runtime before asserting it as AgentMetadata, since this comes from an external API?
| currentMetadata = await response.json() as AgentMetadata; | |
| } | |
| if (response.ok) { | |
| const data = await response.json(); | |
| // Optionally validate data here before assignment | |
| currentMetadata = data as AgentMetadata; | |
| } |
Finding type: Type Inconsistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/sdk/src/services/AgentService.ts (3)
386-418: Use ID! for $id variable to match typical Subgraph schemasMost The Graph schemas define entity id args as ID!, not String!. Using String! can fail validation on stricter servers.
- const query = gql` - query GetAgent($id: String!) { + const query = gql` + query GetAgent($id: ID!) { agent(id: $id) { id name agentUri owner reputation metadata { id name description agentCategory attributes instructions prompts communicationType communicationParams imageUri twitter telegram dexscreener github website } proposals { id service price } } } `;
657-737: Build the GraphQL ‘where’ using variables (avoid string interpolation, injection, and quoting bugs)Directly concatenating filter strings is brittle and vulnerable to query breakage if values contain quotes/special chars. Prefer typed variables and an Agent_filter input. Also, avoid precision loss when scaling reputation—use ethers.parseUnits.
async getAgentRecords(filters: AgentFilterParams = {}): Promise<AgentRecord[]> { - - // Build where clause based on filters - const whereClause: string[] = []; - - if (filters.owner) { - // Validate and normalize the Ethereum address - let normalizedOwner: string; - try { - normalizedOwner = ethers.getAddress(filters.owner).toLowerCase(); - } catch (error) { - throw new Error(`Invalid Ethereum address for owner filter: ${filters.owner}`); - } - whereClause.push(`owner: "${normalizedOwner}"`); - } - - if (filters.name) { - whereClause.push(`name: "${filters.name}"`); - } - - if (filters.reputation_min !== undefined) { - whereClause.push(`reputation_gte: "${(filters.reputation_min * 1e18).toString()}"`); - } - - if (filters.reputation_max !== undefined) { - whereClause.push(`reputation_lte: "${(filters.reputation_max * 1e18).toString()}"`); - } - - if (filters.category) { - whereClause.push(`metadata_: { agentCategory: "${filters.category}" }`); - } - - const whereString = whereClause.length > 0 ? `where: { ${whereClause.join(', ')} }` : ''; - const firstString = filters.first ? `first: ${filters.first}` : 'first: 100'; - const skipString = filters.skip ? `skip: ${filters.skip}` : ''; - - const queryParams = [whereString, firstString, skipString].filter(Boolean).join(', '); - - const query = gql` - query GetAgentsByFilter { - agents(${queryParams}) { + // Build typed variables for the filter + const where: Record<string, unknown> = {}; + if (filters.owner) { + try { + const normalizedOwner = ethers.getAddress(filters.owner).toLowerCase(); + where.owner = normalizedOwner; + } catch { + throw new Error(`Invalid Ethereum address for owner filter: ${filters.owner}`); + } + } + if (filters.name) where.name = filters.name; + if (filters.reputation_min !== undefined) { + // Avoid precision loss vs. number * 1e18 + where.reputation_gte = ethers.parseUnits(String(filters.reputation_min), 18).toString(); + } + if (filters.reputation_max !== undefined) { + where.reputation_lte = ethers.parseUnits(String(filters.reputation_max), 18).toString(); + } + if (filters.category) { + where.metadata_ = { agentCategory: filters.category }; + } + + const query = gql` + query GetAgentsByFilter( + $where: Agent_filter + $first: Int = 100 + $skip: Int = 0 + ) { + agents(where: $where, first: $first, skip: $skip) { id name agentUri owner reputation metadata { id name description agentCategory attributes instructions prompts communicationType communicationParams imageUri twitter telegram dexscreener github website } proposals { id service price } } } `; try { - const result = await this.subgraphClient.request<AgentsQuery>(query, {}); + const result = await this.subgraphClient.request<AgentsQuery>(query, { + where, + first: filters.first ?? 100, + skip: filters.skip ?? 0, + }); return result.agents.map(agent => this.convertSubgraphAgentToRecord(agent)); } catch (error) { console.error("Error fetching agents by filter:", error); throw new Error(`Failed to fetch agents with filters: ${error}`); } }
975-1019: “status” is validated but never applied; results in silent no-op updates
statusappears invalidPropertiesand in the string-type switch but is not merged intoupdatedMetadata(Lines 906–921) nor handled byupdateAgentMetadata. This will always “succeed” without effect.Either implement
statusend-to-end in metadata or remove it from allowed properties. Quick fix (remove to avoid misleading callers):- const validProperties: AgentRecordProperty[] = [ - 'name', 'description', 'category', 'imageURI', 'attributes', - 'instructions', 'prompts', 'socials', 'communicationType', - 'communicationParams', 'status' - ]; + const validProperties: AgentRecordProperty[] = [ + 'name', 'description', 'category', 'imageURI', 'attributes', + 'instructions', 'prompts', 'socials', 'communicationType', + 'communicationParams' + ]; @@ - case 'status': if (typeof value !== 'string') { throw new AgentUpdateError(`Property ${property} must be a string`); } break;If
statusis intended, point me to its target storage (subgraph and/or on-chain) and I’ll add it properly.
🧹 Nitpick comments (9)
packages/sdk/src/services/AgentService.ts (9)
72-72: MakesubgraphClientimmutableIt’s constructed once and never reassigned. Mark it
readonlyfor clarity and to prevent accidental reassignment.- private subgraphClient: GraphQLClient; + private readonly subgraphClient: GraphQLClient;
108-115: Fail fast on bad subgraphUrl (validate and constrain to http/https)Given the constructor now guarantees a client, invalid/empty URLs should be rejected early with a crisp error. This prevents puzzling runtime failures later.
constructor( private readonly agentRegistry: AgentsRegistry, - subgraphUrl: string, + subgraphUrl: string, private signer?: ethers.Signer, private readonly ipfsSDK?: PinataSDK ) { - this.subgraphClient = new GraphQLClient(subgraphUrl); + // Validate URL early + try { + const u = new URL(subgraphUrl); + if (u.protocol !== "http:" && u.protocol !== "https:") { + throw new Error("subgraphUrl must use http(s) protocol"); + } + } catch (e) { + throw new Error(`Invalid subgraphUrl: "${subgraphUrl}". Provide a valid http(s) URL.`); + } + this.subgraphClient = new GraphQLClient(subgraphUrl); }
374-376: Remove redundant “subgraph client not initialized” guardsThe client is now guaranteed by the constructor; these checks are dead code and the message mentions “config,” which no longer applies at this level. Removing them reduces noise and unreachable branches.
async getAgentRecord(agentAddress: string): Promise<AgentRecord> { - if (!this.subgraphClient) { - throw new Error("Subgraph client is not initialized. Please provide a subgraphUrl in the config."); - }async searchAgents(searchTerm: string, first: number = 100, skip: number = 0): Promise<AgentData[]> { - if (!this.subgraphClient) { - throw new Error("Subgraph client is not initialized. Please provide a subgraphUrl in the config."); - }async getAgentCount(): Promise<number> { - if (!this.subgraphClient) { - throw new Error("Subgraph client is not initialized. Please provide a subgraphUrl in the config."); - }Also applies to: 540-543, 615-617
632-646: Count implementation may undercount beyond 1000; consider pagination or a counter entityUsing a single 1000-entity page will undercount on larger datasets. If aggregate count isn’t available in the schema, consider looping pages until fewer than page size are returned, or add a “Stats” entity in the subgraph that tracks counts.
Offer to provide a minimal paginated version if desired.
898-899: Avoid blind JSON casts; validate IPFS metadata before mergingCasting
response.json()to AgentMetadata suppresses type safety and can propagate malformed metadata. Validate at runtime (Zod) and fail or fallback gracefully.- currentMetadata = await response.json() as AgentMetadata; + const json = await response.json(); + // TODO: add AgentMetadata schema; placeholder shape check + // const parsed = validateAgentMetadata(json); // zod.safeParse(...) + // if (parsed.success) currentMetadata = parsed.data; + // else console.warn("Invalid metadata shape from IPFS:", parsed.error); + currentMetadata = json as AgentMetadata;If you share the AgentMetadata Zod schema, I can wire this fully.
481-497: Library-level console.log noise; prefer injected logger or debug flagThese logs (and similar earlier ones) will spam consumers. Consider a minimal logger interface (or debug env flag) and use it instead of unconditional console.log.
Example:
export type Logger = { debug?: (...a:any[]) => void; info?: (...a:any[]) => void; error?: (...a:any[]) => void }; constructor(..., private readonly logger: Logger = console) { ... } this.logger.debug?.(`updating agent ${agentAddress} ...`);
191-209: Ethers v6 error surfaces can differ; hard-codingerror.reasonrisks missing real causesStrengthen error extraction to cover
shortMessage,info?.error?.message, and nestedcause?.reason. This yields more actionable errors across providers.- } catch (error: any) { - console.error({ error }); - if (error.reason === "Agent already registered") { + } catch (error: any) { + console.error({ error }); + const reason = + error?.reason ?? + error?.shortMessage ?? + error?.info?.error?.message ?? + error?.cause?.reason ?? + error?.message; + if (reason === "Agent already registered") { throw new AgentAlreadyRegisteredError(error.reason); } else { throw error; } }Apply similar handling in
registerAgentWithServiceandupdateAgentMetadatacatch blocks.Also applies to: 265-288, 498-508
896-899: Hard-coded Pinata gateway; make IPFS gateway configurableDepending on a single gateway is brittle and can be slow regionally. Consider injecting an
ipfsGatewayBaseUrl(defaulting to a robust multi-gateway) and use it here.- const response = await fetch(`https://gateway.pinata.cloud/ipfs/${ipfsHash}`); + const base = this.ipfsGatewayBaseUrl ?? "https://ipfs.io"; + const response = await fetch(`${base.replace(/\/+$/,'')}/ipfs/${ipfsHash}`);If you want, I can plumb a new optional constructor arg and thread it through callers.
101-103: Tighten type for communicationType (avoidas any)Define a union/enum for known types (e.g., 'eliza' | 'openai' | ...) and map unknowns to a default. This prevents invalid values percolating.
- communicationType: (metadata?.communicationType as any) || 'eliza', + communicationType: (metadata?.communicationType as 'eliza' /* | '...' */) ?? 'eliza',If
CommunicationTypeexists in your types, use it here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/sdk/src/services/AgentService.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/sdk/src/services/AgentService.ts (1)
packages/sdk/src/types.ts (1)
AgentMetadata(79-79)
🔇 Additional comments (1)
packages/sdk/src/services/AgentService.ts (1)
108-115: AllAgentServicecall sites updated andsubgraphUrlplumbed correctlyVerified that every
new AgentServiceinvocation now supplies the requiredsubgraphUrlin the correct position:
- packages/sdk/test/agentService.test.ts:33–36
›agentService = new AgentService(mockRegistry, "https://mock-subgraph-url.com", mockSigner)- packages/sdk/src/ensemble.ts:86
›const agentService = new AgentService(agentRegistry, config.subgraphUrl, signer, ipfsSDK)The
EnsembleConfig.subgraphUrlproperty is defined as required inpackages/sdk/src/types.tsand is consistently passed throughEnsemble.create, example scripts, and documentation. No remaining instantiations use the old signature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (12)
.github/workflows/release.yml (4)
44-53: Enable npm provenance for published packagesTo attach build provenance/SLSA attestations when publishing to npm, set NPM_CONFIG_PROVENANCE=true on the publish step. Node 20 ships with npm 10, which supports this out of the box. This is increasingly required in orgs and helps downstream consumers verify package integrity.
Apply this diff to the publish step:
- name: Create Release Pull Request or Publish id: changesets uses: changesets/action@v1 with: publish: pnpm changeset:publish title: "chore: release packages" commit: "chore: release packages" env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + NPM_CONFIG_PROVENANCE: true
7-7: Harden concurrency to avoid duplicate runs on self-pusheschangesets/action can push version commits/tags back to main. Those additional pushes will retrigger this workflow. Consider enabling cancel-in-progress so the older run is cancelled when a newer commit arrives on the same ref.
-concurrency: ${{ github.workflow }}-${{ github.ref }} +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true
18-21: Pin actions to immutable SHAsFor supply-chain hardening, pin third-party actions to full commit SHAs instead of floating version tags. This prevents unexpected changes when upstream tags are moved.
Example:
-uses: actions/checkout@v4 +uses: actions/checkout@3df4ab11eba7bda6032a0b82a6bb43b11571feac # v4Repeat for pnpm/action-setup@v2, actions/setup-node@v4, and changesets/action@v1.
16-16: Strip trailing spaces and add newline at EOFYAMLlint flags trailing spaces on the noted lines and missing newline at EOF. These don’t break YAML but will fail strict linting.
I recommend running a whitespace cleanup (e.g.,
git stripspaceor your editor’s trim-on-save) and ensuring the file ends with a single newline.Also applies to: 22-22, 27-27, 34-34, 37-37, 40-40, 43-43
.github/workflows/pr-validation.yml (3)
39-48: Make the changeset gate more robustAvoid using command substitution with grep output for control flow. Use grep -q to rely on exit codes, and add set -euo pipefail for safety.
- - name: Check for changeset (skip for dependabot) + - name: Check for changeset (skip for dependabot) if: github.actor != 'dependabot[bot]' - run: | - if [ "$(pnpm changeset status --verbose | grep 'No changesets present')" ]; then - echo "::error::Please add a changeset by running 'pnpm changeset'" - echo "::error::This helps us track changes and generate release notes" - exit 1 - else - echo "✅ Changeset found" - fi + run: | + set -euo pipefail + if pnpm changeset status --verbose | grep -q 'No changesets present'; then + echo "::error::Please add a changeset by running 'pnpm changeset'" + echo "::error::This helps us track changes and generate release notes" + exit 1 + else + echo "✅ Changeset found" + fi
1-7: Consider adding concurrency with cancel-in-progress for PRsMultiple pushes to the same PR branch cause redundant in-flight runs. Concurrency with cancel-in-progress saves CI minutes and speeds feedback.
name: PR Validation on: pull_request: types: [opened, synchronize, reopened] +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.ref }} + cancel-in-progress: true
11-15: Trim trailing spaces and add newline at EOFYAMLlint reports trailing spaces on the indicated lines and a missing newline at EOF. Harmless functionally, but they’ll trip format checks.
Recommend enabling “trim trailing whitespace” in your editor and ensuring a single trailing newline.
Also applies to: 20-20, 26-26, 29-29, 32-32, 35-35, 38-38
.github/workflows/prerelease.yml (5)
62-66: Add npm provenance to prerelease publishesAs with release, attach provenance for prerelease tags as well.
- name: Publish prerelease run: pnpm changeset:publish --tag ${{ github.event.inputs.tag }} env: - NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + NPM_CONFIG_PROVENANCE: true
23-27: Ensure pushes succeed by keeping checkout credentialsPushing prerelease commits/tags relies on checkout’s persisted credentials. actions/checkout@v4 defaults to persist-credentials: true, but making it explicit avoids surprises if defaults change.
- name: Checkout code uses: actions/checkout@v4 with: fetch-depth: 0 + persist-credentials: true
67-68: Confirm whether explicit git push is neededchangesets publish typically pushes tags/commits it creates. The explicit git push here is usually harmless but may be redundant. If you observe duplicate pushes or extra triggers, consider removing it.
21-21: Clean up trailing spaces and add newline at EOFYAMLlint flags trailing spaces on multiple blank lines and a missing newline at EOF. Cosmetic but helps keep CI green and diffs clean.
Recommend running a whitespace trim and ensuring a single newline at the end of the file.
Also applies to: 27-27, 32-32, 39-39, 42-42, 45-45, 48-48, 51-51, 54-54, 61-61, 66-66, 68-68
28-37: Pin actions to SHAsSame supply-chain hardening guidance as the release workflow: pin actions to commit SHAs.
Example:
-uses: pnpm/action-setup@v2 +uses: pnpm/action-setup@a50a5e3c2b4e2e7f8b3f4f0f1a2b3c4d5e6f7a8b # v2Repeat for actions/checkout@v4 and actions/setup-node@v4.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/workflows/pr-validation.yml(1 hunks).github/workflows/prerelease.yml(1 hunks).github/workflows/release.yml(1 hunks)package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/pr-validation.yml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 20-20: trailing spaces
(trailing-spaces)
[error] 26-26: trailing spaces
(trailing-spaces)
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 38-38: trailing spaces
(trailing-spaces)
[error] 48-48: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/prerelease.yml
[error] 21-21: trailing spaces
(trailing-spaces)
[error] 27-27: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 39-39: trailing spaces
(trailing-spaces)
[error] 42-42: trailing spaces
(trailing-spaces)
[error] 45-45: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
[error] 51-51: trailing spaces
(trailing-spaces)
[error] 54-54: trailing spaces
(trailing-spaces)
[error] 61-61: trailing spaces
(trailing-spaces)
[error] 66-66: trailing spaces
(trailing-spaces)
[error] 68-68: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/release.yml
[error] 16-16: trailing spaces
(trailing-spaces)
[error] 22-22: trailing spaces
(trailing-spaces)
[error] 27-27: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 37-37: trailing spaces
(trailing-spaces)
[error] 40-40: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 53-53: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (2)
.github/workflows/release.yml (1)
35-42: No missing scripts in root package.jsonThe
build,test, andchangeset:publishscripts are all defined in the repo’s root package.json, matching what the workflow expects. No further changes are required..github/workflows/pr-validation.yml (1)
27-37: All required root scripts confirmed presentThe CI validation script ran successfully and found
typecheck,build, andtestdefined in the root package.json, so no further changes are needed here. ✅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/subgraph/package.json (5)
28-34: Move graph-cli to devDependencies to avoid bloating consumers.The CLI is only needed at build/test time. Keeping it under dependencies increases install size for downstream consumers if this package is published.
Apply:
"dependencies": { - "@graphprotocol/graph-cli": "0.95.0", "@graphprotocol/graph-ts": "0.32.0" }, "devDependencies": { - "matchstick-as": "0.5.0" + "@graphprotocol/graph-cli": "0.95.0", + "matchstick-as": "0.5.0" }
20-22: Avoid running codegen twice in build.Minor nit: reuse the codegen script to reduce duplication and keep logs consistent.
"scripts": { "codegen": "graph codegen", - "build": "graph codegen && graph build", + "build": "pnpm run codegen && graph build",
22-25: Clarify deploy target and align subgraph name.
- The generic
graph deploy ensemble-v2lacks product flags (--studioor--product hosted-service) or a Graph Node target like used in deploy-local. As-is, this is likely to fail or deploy to an unintended target.- Local scripts use
ensemble-hubwhiledeployusesensemble-v2. Consider standardizing to one slug or parameterize via env.Two common, safer options:
- Subgraph Studio deployment (recommended):
- "deploy": "graph deploy ensemble-v2", + "deploy:studio": "graph deploy --studio ensemble-hub",
- Self-hosted Graph Node (mirrors deploy-local target names):
- "deploy": "graph deploy ensemble-v2", + "deploy:node": "graph deploy --node http://localhost:8020/ --ipfs http://localhost:5001 ensemble-hub",If you intend a different remote (e.g., decentralized network gateway), please confirm the exact command and auth flow and we can adjust the script accordingly.
10-13: Add a files allowlist to control what gets published.Since
publishConfig.accessis public, it’s easy to unintentionally publish codegen, test artifacts, or cache dirs. Add a files list to keep the npm tarball lean."publishConfig": { "access": "public", "registry": "https://registry.npmjs.org/" }, + "files": [ + "schema.graphql", + "subgraph.yaml", + "src/**", + "abis/**", + "README.md", + "LICENSE" + ],If this package is not meant to be consumed from npm, consider setting
"private": trueinstead. Given your changesets-based releases, I’m assuming publication is intended.
18-19: Pin the toolchain to avoid CI drift.Adding Node engine and packageManager helps keep CI and local consistent, especially with changesets and pnpm.
"bugs": { "url": "https://github.com/ensemble-codes/ensemble-framework/issues" }, + "engines": { + "node": ">=18.18.0" + }, + "packageManager": "pnpm@9", "scripts": {Adjust versions to match what your CI uses.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/sdk/package.json(1 hunks)packages/subgraph/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/sdk/package.json
🔇 Additional comments (3)
packages/subgraph/package.json (3)
5-9: Package metadata looks solid.Repository info, homepage, bugs, and MIT license are correctly populated and consistent with the monorepo structure.
Also applies to: 14-18
14-14: Ensure license file exists and is MIT-compliant.You switched to MIT. Confirm that a LICENSE file exists at the repo root and that headers (if used) match.
29-34: Verify and align Graph Protocol dependency versionsThe pinned versions in
packages/subgraph/package.jsonare behind the latest published releases—please review changelogs for breaking changes and update as needed:
- Dependencies (
packages/subgraph/package.json):
- @graphprotocol/graph-cli: pinned 0.95.0 → latest 0.97.1
- @graphprotocol/graph-ts: pinned 0.32.0 → latest 0.38.1
- DevDependencies:
- matchstick-as: pinned 0.5.0 → latest 0.6.0
Relevant changelogs (verify for any required mapping, as-pect, or test-harness updates):
• graph-cli: https://github.com/graphprotocol/graph-tooling/releases
• graph-ts: https://github.com/graphprotocol/graph-ts/releases
• matchstick-as: https://github.com/LimeChain/matchstick/releases
Generated description
Below is a concise technical summary of the changes proposed in this PR:
graph LR subgraph "@ensemble-ai/cli" ["@ensemble-ai/cli"] classDef added stroke:#15AA7A classDef removed stroke:#CD5270 classDef modified stroke:#EDAC4C linkStyle default stroke:#CBD5E1,font-size:13px end subgraph "@ensemble-ai/sdk" ["@ensemble-ai/sdk"] create_("create"):::modified AgentService_constructor_("AgentService.constructor"):::modified SUBGRAPH_("SUBGRAPH"):::added AgentService_getAgentRecords_("AgentService.getAgentRecords"):::modified AgentService_updateAgentRecord_("AgentService.updateAgentRecord"):::modified IPFS_("IPFS"):::added create_ -- "Updated constructor call with required subgraphUrl parameter." --> AgentService_constructor_ AgentService_constructor_ -- "Always initializes subgraph client with provided URL." --> SUBGRAPH_ AgentService_getAgentRecords_ -- "Removes client check; always sends queries to subgraph." --> SUBGRAPH_ AgentService_getAgentRecords_ -- "Relies on guaranteed subgraph client for responses." --> SUBGRAPH_ AgentService_updateAgentRecord_ -- "Adds explicit typing for fetched IPFS metadata JSON." --> IPFS_ classDef added stroke:#15AA7A classDef removed stroke:#CD5270 classDef modified stroke:#EDAC4C linkStyle default stroke:#CBD5E1,font-size:13px endEnforce the
subgraphUrlas a required parameter within the@ensemble-ai/sdk's configuration andAgentServiceto ensure consistent connectivity for agent-related queries. Integrate Changesets into the monorepo's build system and GitHub Actions workflows to streamline automated versioning and package publishing, alongside updating package metadata and contributing guidelines.subgraphUrlas a mandatory configuration parameter for the@ensemble-ai/sdk'sAgentServiceandEnsembleConfigto ensure consistent subgraph connectivity for agent queries, updating related logic, tests, and documentation.Modified files (9)
Latest Contributors(2)
Modified files (12)
Latest Contributors(2)
Modified files (1)
Latest Contributors(1)
Summary by CodeRabbit
Breaking Changes
Documentation
Chores / CI
Tests
Validation