From f4f75ca57dc769870dac1518dd427a443ca9fb3b Mon Sep 17 00:00:00 2001 From: alexander-sei Date: Wed, 5 Aug 2026 14:27:40 +0200 Subject: [PATCH 1/2] ci: integrate UCI AI review, AI assistant, and stale check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adopt the shared sei-protocol/uci workflows so sei-js gets the same automated PR review, @seidroid assistant, and stale triage that sei-chain and sei-skill run. Pinned to the v0.0.15 release commit rather than a floating tag. sei-chain currently trails on v0.0.13 (AI review/assist) and v0.0.9 (stale check), so these are pinned forward instead of copied verbatim. v0.0.15 adds the `ai: skip-review` label switch, which only takes effect immediately when the caller subscribes to `labeled`/`unlabeled` — hence those trigger types. Cursor's scout pass is disabled because Bugbot already reviews sei-js PRs. xreview is intentionally omitted; it was removed upstream after v0.0.15. Also add REVIEW.md, the repo guidelines the AI reviewer loads from the PR base branch, covering the mcp-server transport/wallet invariants, hand-maintained precompile ABIs, changeset expectations, and known non-issues that would otherwise generate false positives. Co-authored-by: Cursor --- .github/workflows/ai-assist.yml | 22 ++++++ .github/workflows/ai-review.yml | 20 +++++ .github/workflows/uci-stale-check.yml | 25 ++++++ REVIEW.md | 109 ++++++++++++++++++++++++++ 4 files changed, 176 insertions(+) create mode 100644 .github/workflows/ai-assist.yml create mode 100644 .github/workflows/ai-review.yml create mode 100644 .github/workflows/uci-stale-check.yml create mode 100644 REVIEW.md diff --git a/.github/workflows/ai-assist.yml b/.github/workflows/ai-assist.yml new file mode 100644 index 00000000..b21cd3f2 --- /dev/null +++ b/.github/workflows/ai-assist.yml @@ -0,0 +1,22 @@ +name: AI Assistant +on: + issue_comment: + types: [ created ] + pull_request_review_comment: + types: [ created ] + pull_request_review: + types: [ submitted ] +jobs: + assistant: + # See: https://github.com/sei-protocol/uci/releases/tag/v0.0.15 + uses: sei-protocol/uci/.github/workflows/ai-assistant.yml@65901242783550521f25a19199a6b10e54550b97 + permissions: + contents: read + pull-requests: write + issues: write + id-token: write + secrets: inherit + with: + # See: https://github.com/sei-protocol/uci/releases/tag/v0.0.15 + uci-ref: 65901242783550521f25a19199a6b10e54550b97 + allowed-team: 'sei-protocol/sei-core' diff --git a/.github/workflows/ai-review.yml b/.github/workflows/ai-review.yml new file mode 100644 index 00000000..7413f693 --- /dev/null +++ b/.github/workflows/ai-review.yml @@ -0,0 +1,20 @@ +name: AI Review +on: + pull_request: + # `labeled`/`unlabeled` let the `ai: skip-review` label take effect immediately rather + # than on the next push. UCI ignores those two events for every other label. + types: [ opened, ready_for_review, synchronize, reopened, labeled, unlabeled ] +jobs: + ai-review: + # See: https://github.com/sei-protocol/uci/releases/tag/v0.0.15 + uses: sei-protocol/uci/.github/workflows/ai-review.yml@65901242783550521f25a19199a6b10e54550b97 + permissions: + contents: read + pull-requests: write + checks: write + id-token: write + secrets: inherit + with: + # See: https://github.com/sei-protocol/uci/releases/tag/v0.0.15 + uci-ref: 65901242783550521f25a19199a6b10e54550b97 + enable-cursor: false # Disabled for now since there is a dedicated Bugbot flow built into Cursor currently enabled on repo. diff --git a/.github/workflows/uci-stale-check.yml b/.github/workflows/uci-stale-check.yml new file mode 100644 index 00000000..23057f32 --- /dev/null +++ b/.github/workflows/uci-stale-check.yml @@ -0,0 +1,25 @@ +name: UCI +run-name: UCI / Stale Check + +on: + workflow_dispatch: + schedule: + - cron: '0 12 * * *' + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.sha }} + cancel-in-progress: true + +jobs: + stale: + name: Stale + permissions: + issues: write + pull-requests: write + # See: https://github.com/sei-protocol/uci/releases/tag/v0.0.15 + uses: sei-protocol/uci/.github/workflows/stale-check.yml@65901242783550521f25a19199a6b10e54550b97 + with: + days-before-pr-stale: 28 diff --git a/REVIEW.md b/REVIEW.md new file mode 100644 index 00000000..c50f1378 --- /dev/null +++ b/REVIEW.md @@ -0,0 +1,109 @@ +# Review guidelines for AI agents + +Repo-specific conventions for automated PR review (Codex, Cursor, Claude, and +any other AI reviewer). This is a pnpm workspace that publishes six +independently versioned `@sei-js/*` packages to npm, so a defect ships to every +downstream dApp that upgrades rather than to a single deployment we control. +Calibrate accordingly: a wrong precompile address or a weakened wallet guard is +far more valuable to catch than a style nit, and several patterns below look +like bugs in isolation but are deliberate. + +## 1. `packages/mcp-server` is the security surface + +This package hands blockchain capabilities to an LLM client, so it is the one +place in the repo where a subtle regression can cost users funds. Two +invariants are load-bearing and are enforced in code rather than by convention: + +- **Wallet mode is stdio-only.** `validateSecurityConfig()` in + `src/server/transport/security.ts` halts the process when the wallet is + enabled and the transport is `streamable-http` or `http-sse`. HTTP + transports are reachable cross-origin, so a signing key behind one is a + drain-the-wallet primitive. Any change that narrows this check, makes it + non-fatal, or adds a transport that bypasses it is a finding. +- **SSE messages are bound to their session.** + `src/server/transport/http-sse.ts` keys `connections` by + `transport.sessionId` and requires a matching `?sessionId=` on + `POST {path}/message` (400 when absent, 404 when unknown). This replaced an + implementation that routed to the first connection in the map, which let one + client inject into another's stream. Treat any reintroduction of positional + or implicit session lookup as a regression, and keep the isolation tests in + `src/tests/server/transport/http-sse.test.ts` meaningful. + +Beyond those, scrutinise anything that widens what a caller controls: contract +ABIs reach `JSON.parse` from tool arguments in `src/core/tools.ts`, addresses +and call arguments arrive unvalidated from the model, and RPC endpoints are +overridable through `MAINNET_RPC_URL` / `TESTNET_RPC_URL` / `DEVNET_RPC_URL` in +`src/core/chains.ts`. Private keys are read from the environment in +`src/core/config.ts` and must never reach a log line, an error message, or a +tool response. + +Note that `.github/workflows/pr-to-slack-codex.yml` already runs a separate +AppSec pass to Slack. Overlapping findings are expected; don't suppress a real +issue because you assume the other reviewer caught it. + +## 2. Precompile addresses and ABIs are hand-maintained source + +`packages/precompiles/src/precompiles/*.ts` is not generated — there is no +codegen step or ABI pipeline in this repo. An address or ABI entry changed +there is a change to the source of truth, and a wrong value silently misroutes +every consumer's calls. You usually cannot settle these from the diff alone, so +when a value looks suspect, ask for the authoritative source rather than +asserting it is wrong: link what you checked (`docs.sei.io`, `sei-chain`, +Seiscan) and say what disagrees. An unsourced change to an existing documented +address, chain ID, or ABI signature is worth raising on its own. + +## 3. A publishable change needs a changeset + +`.changeset/config.json` sets `fixed: []` and `linked: []`, so every package +versions independently — do not expect or request a coordinated bump. Merging +to `main` opens a "Version Packages" PR, and merging that publishes. A +user-facing change to a published package with no `.changeset/*.md` file ships +the code without releasing it, which is the common miss on this repo. + +Ask for a changeset when a published package's behaviour, types, or +dependencies change. Docs-only, CI-only, and changes confined to +`packages/create-sei/templates/**` generally don't need one, though the repo +has deliberately added patch changesets across all six packages for +release-note visibility (the `@asyncapi` pinning in `pnpm.overrides` is the +precedent). Absence of a changeset on that kind of PR is a question, not a +defect. + +## 4. Known non-issues — do not flag these + +- **`console.error` used for informational messages.** Under the stdio + transport, stdout carries the JSON-RPC frames, so anything written there + corrupts the protocol. `console.error('MCP Server ready (stdio transport)')` + in `src/server/transport/stdio.ts` is correct. Never suggest converting + these to `console.log`. The inverse *is* a finding: a new `console.log` on a + path reachable from stdio breaks the transport. +- **The CORS middleware sets no `Access-Control-Allow-Origin`.** + `createCorsMiddleware()` answers preflights with a bare 204 and no CORS + headers. That is deny-by-default: a browser treats the missing header as a + failure and blocks the request. It is not an oversight and not a permissive + wildcard. +- **`process.exit(1)` inside `validateSecurityConfig()`.** Failing closed at + startup is the intent. Do not ask for a thrown error the caller might swallow. +- **`packages/registry/chain-registry` and `.../community-assetlist` are + missing from the tree.** Both are git submodules (`.gitmodules`) and are + listed in `.gitignore`; they are populated by the `registry` package's + `postinstall` and by CI's submodule checkout. Their JSON is vendored + upstream — review the TypeScript wrappers, not the data. +- **Biome findings are not enforced anywhere.** `biome.json` configures tabs, + 160-column lines, single quotes and no trailing commas, but no package + defines a `biome` script and `.github/workflows/checks.yml` runs only + `pnpm build:all` and `pnpm test:all`. Match the surrounding style; do not + file formatting-only findings as blocking. +- **Test file naming is inconsistent across packages.** `mcp-server` and + `create-sei` use `*.test.ts`; `precompiles`, `ledger`, `registry` and + `sei-global-wallet` use `*.spec.ts` under `__tests__/`. Follow the + convention of the package being changed rather than proposing a repo-wide + rename. +- **`mcp-server` has no Codecov target.** `codecov.yml` defines 80% project + targets for the other five packages only. Thin coverage on an mcp-server PR + is worth mentioning on its merits, but it does not fail a gate. +- **`noImplicitAny: false` in `tsconfig.base.json`.** This is a deliberate + repo-wide setting. Flag an untyped value when it actually causes an unsound + path, not because the compiler permitted it. +- **The `create-sei` templates are excluded from the root Biome config** and + carry their own toolchain. Do not apply root formatting rules to anything + under `packages/create-sei/templates/`. From 34e989c0aae128b45bbcbd284291a3361ff39904 Mon Sep 17 00:00:00 2001 From: alexander-sei Date: Wed, 5 Aug 2026 14:37:25 +0200 Subject: [PATCH 2/2] ci: keep contents:read on the UCI stale job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Job-level `permissions` replaces the workflow-level block instead of merging with it, so the `stale` job was running with `contents: none` and the workflow-level default applied to no job at all. Harmless today — UCI's stale-check only calls the issues/PRs API via `actions/stale` and never checks out — but it breaks silently the moment that changes. Co-authored-by: Cursor --- .github/workflows/uci-stale-check.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/uci-stale-check.yml b/.github/workflows/uci-stale-check.yml index 23057f32..a6c5f4b6 100644 --- a/.github/workflows/uci-stale-check.yml +++ b/.github/workflows/uci-stale-check.yml @@ -16,7 +16,10 @@ concurrency: jobs: stale: name: Stale + # Job-level `permissions` replaces the workflow-level block rather than merging, + # so `contents: read` has to be repeated here to survive. permissions: + contents: read issues: write pull-requests: write # See: https://github.com/sei-protocol/uci/releases/tag/v0.0.15