Skip to content

Integrate UCI: AI review, AI assistant, and stale check #13

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.

Sign up for GitHub

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

Open
alexander-sei wants to merge 2 commits into
base: main
Choose a base branch
from
+181 −0
Open

Integrate UCI: AI review, AI assistant, and stale check #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c24de9e
Integrate UCI: AI review, AI assistant, and stale check
alexander-sei Aug 3, 2026
60af3dd
Pin stale check to the UCI release commit; point README at REVIEW.md
alexander-sei Aug 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .github/workflows/ai-assist.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
name: AI Assistant
on:
issue_comment:

@github-actions github-actions Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] issue_comment fires on plain issues as well as PRs. Every comment on any issue in this repo will start this job, which then either no-ops or errors inside the reusable workflow depending on how it handles a missing pull_request field.

If the assistant is PR-only, gating at the caller is cheap and keeps the Actions list clean:

    if: github.event_name != 'issue_comment' || github.event.issue.pull_request

Skip if UCI's own preflight already handles this — worth a quick check, since the cost of being wrong is only noise.

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

@github-actions github-actions Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] issue_comment fires on every comment in the repo (issues and PRs, including comments on fork PRs, which run in base-repo context with full secret access). Combined with secrets: inherit + issues: write/pull-requests: write, the allowed-team gate inside UCI is the entire security boundary — good that it fails closed, per the PR description. Two follow-ups: (1) the org Members: Read grant is load-bearing, not optional, since without it the gate denies everyone; (2) consider passing the PLATFORM_CODE_AGENT_* secrets explicitly instead of secrets: inherit, so this comment-triggered workflow can't reach unrelated org secrets.

with:
# See: https://github.com/sei-protocol/uci/releases/tag/v0.0.15
uci-ref: 65901242783550521f25a19199a6b10e54550b97
allowed-team: 'sei-protocol/sei-core'
20 changes: 20 additions & 0 deletions .github/workflows/ai-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: AI Review
on:
pull_request:

@github-actions github-actions Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] pull_request (not pull_request_target) means fork PRs get a read-only GITHUB_TOKEN, no secrets: inherit values, and no OIDC token — so id-token: write / checks: write / pull-requests: write are all silently downgraded. On a public skill repo that expects outside contributions, every fork PR will surface a failing AI Review check rather than a skipped one. The trade-off is real (pull_request_target on untrusted diffs has its own risks, and is likely why UCI chose this), so the ask is just: confirm UCI exits cleanly (neutral/skip) when credentials are absent, and don't make this a required status check until that's verified.

@github-actions github-actions Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Flagging the fork consequence once here for the record: pull_request means external contributions get no secrets and a read-only token, so pull-requests: write / checks: write / secrets: inherit are all inert on fork PRs and the review will not post.

That is the correct trade-off — pull_request_target would run trusted workflow code with write access and secrets against untrusted head content — so no change requested to the trigger. It just means AI review is an internal-contributor tool here, which is worth being explicit about (see the README comment).

# `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 ]

@github-actions github-actions Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] labeled/unlabeled will start a workflow run for every label change on every PR — the comment is right that UCI's preflight gate only acts when the changed label is the skip label, but the caller job still spins up and reports a check run before that gate decides. Two things to watch: runner minutes on label churn, and (if AI Review ever becomes a required check) the gated-skip path must report success rather than leaving a pending check. Also note the ai: skip-review label doesn't exist on this repo yet — already on your checklist, just calling out that these two event types are inert until it's created.

@cursor cursor Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Label events cancel AI reviews

Medium Severity

pull_request types include labeled and unlabeled with no caller-level if limiting those actions to ai: skip-review. UCI’s reusable ai-review workflow uses PR-scoped concurrency with cancel-in-progress: true and, on the public workflow definition, only gates on draft status—not label name—so unrelated label edits can cancel an in-flight review and queue another run. The skip-label immediate-effect intent does not hold with this caller shape.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c24de9e. Configure here.

jobs:
ai-review:

@github-actions github-actions Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] No caller-level concurrency group, unlike uci-stale-check.yml in this same PR. With synchronize plus labeled/unlabeled in the trigger list, a push-then-relabel or a few rapid pushes can leave several review runs in flight on one PR, each spending model tokens and each potentially posting its own review and AI Review check run.

Unless UCI already dedupes internally at the reusable-workflow level, the standard guard is worth adding:

concurrency:
  group: ${{ github.workflow }}-${{ github.event.pull_request.number }}
  cancel-in-progress: true

Same consideration applies to ai-assist.yml, keyed on the issue/PR number, so two @seidroid comments in quick succession do not run concurrently.

# 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

@github-actions github-actions Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] No concurrency block here (unlike uci-stale-check.yml). With synchronize in the trigger list, a rapid push sequence can run several full AI reviews against the same PR concurrently — duplicate posted reviews plus duplicated model spend. A reusable workflow can declare its own concurrency, so UCI may already handle this; if it doesn't, group: ${{ github.workflow }}-${{ github.event.pull_request.number }} with cancel-in-progress: true is the fix. Same applies to ai-assist.yml.

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.
25 changes: 25 additions & 0 deletions .github/workflows/uci-stale-check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 }}

@github-actions github-actions Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Keying the group on github.sha does not do much for a cron workflow: github.sha is the default-branch head, so it stays constant until a new commit lands. The practical effects are that a workflow_dispatch run cancels an in-flight scheduled run on the same commit (and vice versa), while two scheduled runs on different commits — the case where overlap would matter least — get separate groups.

If the intent is "only one stale sweep at a time," group: ${{ github.workflow }} with cancel-in-progress: false expresses it more directly. Harmless either way at a daily cadence.

cancel-in-progress: true

jobs:
stale:
name: Stale
permissions:

@github-actions github-actions Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] A job-level permissions: block replaces the workflow-level one outright — there is no merging. So this job runs with issues: write and pull-requests: write but contents: none, and the contents: read on line 10 applies to nothing (it is the workflow default, and this is the only job).

If stale-check.yml does an actions/checkout or otherwise reads repo contents at any point, the daily cron fails — and it fails where nobody is looking, since there is no PR to annotate. Note both sibling files in this PR do include contents: read in their job blocks, which makes the omission here look like an oversight rather than deliberate tightening.

I could not fetch the reusable workflow to confirm it needs contents: read (network to sei-protocol/uci was blocked), so this is a "please verify" rather than a confirmed break. Adding it is a one-line, no-downside change:

    permissions:
      contents: read
      issues: write
      pull-requests: write

Also worth checking whether stale-check.yml needs secrets: inherit like the other two callers, or genuinely runs on the default GITHUB_TOKEN.

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
2 changes: 2 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,8 @@ Built from:
```
6. Open a pull request

Pull requests are reviewed automatically against [REVIEW.md](REVIEW.md), which records this repo's review standards — worth a read before a larger change.

@github-actions github-actions Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] This lands at the end of the Contributing section, whose step 1 is to fork — but ai-review.yml triggers on pull_request, and GitHub does not pass secrets to fork-triggered runs and forces GITHUB_TOKEN read-only regardless of the permissions: block. So for exactly the audience reading these steps, PRs will not be reviewed automatically; it only works for branches pushed to this repo.

Suggest either scoping the sentence ("PRs from repo branches are reviewed automatically…") or dropping the automation claim and keeping the useful half: that REVIEW.md records the review standards and is worth reading before a larger change. The pointer to REVIEW.md is valuable on its own.


## License

MIT — see [LICENSE](LICENSE)
112 changes: 112 additions & 0 deletions REVIEW.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
# Review guidelines for AI agents

Repo-specific conventions for automated PR review (Codex, Cursor, Claude, and
any other AI reviewer). This repo ships **prose that other AI agents consume as
ground truth**, not a running service: the `skill/` tree is the product, and
`install.sh` is the only executable of consequence. Calibrate accordingly — a
wrong number in a markdown table is a higher-severity defect here than it would
be in a normal codebase, and style nits on reference prose are near-worthless.

## 1. Factual accuracy about Sei is the primary correctness axis

Every concrete claim in `skill/**` gets replayed to users by an agent that
cannot check it. The highest-value finding in this repo is a Sei fact that is
wrong or stale. Scrutinise added or changed:

- chain IDs, network names, and `evm_chain_id` values,
- contract, precompile, and pointer addresses,
- gas costs and fee/denom values,
- RPC, explorer, faucet, and docs URLs,
- hardware, Node/Go, and toolchain version requirements,
- CLI flags and subcommands for `seid`, Foundry, and Hardhat.

Real regressions have shipped in each of those categories, so treat them as
load-bearing rather than incidental.

You usually cannot settle a network fact from the diff alone. When a value
looks suspect, **ask for the authoritative source rather than asserting the
value is wrong** — link to what you checked (`docs.sei.io`, `sei-chain`,
Seiscan) and say what disagrees. An unsourced numeric or version change to an
existing documented value is worth raising on its own.

Prefer official Sei sources over third-party aggregators. This repo migrated
off Seitrace in favour of Seiscan (Sourcify) for verification flows; flag new
Seitrace references as regressions.

## 2. A new reference file must be linked from `skill/SKILL.md`

`install.sh` does not carry a file manifest. In flatten mode it discovers
reference files by grepping the selected variant entry point for
`references/**.md` paths:

```bash
grep -Eo 'references/[^)"[:space:]]+\.md' "$SOURCE_DIR/$VARIANT_FILE" | sort -u
```

So a file added under `skill/references/` but not linked from a `SKILL*.md`
entry point is **silently omitted** from every flattened agent install
(`--agent cursor`, `--agent codex`, `--flatten`, …) while still appearing in the
Claude Code directory install, which copies the whole tree. That asymmetry is
invisible in CI and in the diff.

The invariant to check: every file under `skill/references/` is linked from
`skill/SKILL.md` (the full variant), **and** from each domain variant
(`SKILL-CONTRACTS.md`, `SKILL-FRONTEND.md`, `SKILL-ECOSYSTEM.md`) it belongs
to. Flag an added reference file that no entry point links, and flag a renamed
or moved reference whose links were not updated — a stale link degrades to a
missing file rather than an error, since the append loop skips paths that do not
exist.

## 3. YAML frontmatter on `SKILL*.md` is load-bearing

Two consumers parse it:

- `install.sh` strips frontmatter with awk by discarding everything up to the
**second** `---` line. There is no check that those two lines actually
delimit a frontmatter block, so an entry point whose frontmatter is malformed
or missing silently mis-slices — output starting at some later horizontal
rule, or an empty file if fewer than two `---` lines remain. It never fails
loudly. Any edit to the opening `---`/`name`/`description` block deserves a
close look.
- `tests/run.ts` reads `name` and `description` out of the frontmatter and
scores whether an agent selects the skill for a given prompt.

The `description` field is the skill's trigger surface: it is a long list of
paraphrased user requests, and trimming it changes activation behaviour even
though nothing about it looks like code. Treat a shortened or reworded
`description` as a behavioural change and say which prompts it may stop
matching.

## 4. `install.sh` portability constraints

The README commits to macOS, Linux, Git Bash on Windows, and WSL. That means:

- **bash 3.2 compatible** — macOS ships bash 3.2. No `mapfile`/`readarray`, no
associative arrays (`declare -A`), no `${var,,}` case conversion. The script
is currently clean of all of these; keep it that way.
- **no GNU-only flags** — coreutils on macOS and Git Bash are BSD/MinGW.
- The script runs under `set -e` only. If you flag a missing `set -u` or
`pipefail`, note that adding them is a behavioural change to every path, not a
free hardening win.

Also check that a new `--agent` target sets `AGENT_DEFAULT_OUTPUT`, is listed in
both `--help` and the `Supported agents` error string, and is documented in the
README table — the four are updated by hand and drift silently.

@github-actions github-actions Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] "the four" reads at first as "the four agent targets," but install.sh currently supports seven (cursor, copilot, windsurf, aider, openhands, codex, gemini) — you mean the four places that need updating. Suggest "all four sites are updated by hand and drift silently."

Everything else in this file I spot-checked against the tree and it is accurate: the grep -Eo 'references/…' discovery line matches install.sh:283 verbatim, the awk frontmatter strip at 288/291 does discard up to the second --- with no validation (so it yields empty output rather than an error when fewer than two remain), set -e on line 10 is indeed the only option set, and tests/run.ts does parse name/description out of the frontmatter. §2's invariant also holds right now — all 46 files under skill/references/ are linked from skill/SKILL.md, an exact match in both directions.


## 5. Known non-issues — do not flag these

- **Content duplicated across `SKILL.md` and the `SKILL-*.md` variants.** Each
variant is a standalone entry point for a separate install, so overlap is
intentional. The same reference file is deliberately linked from several
variants. Do not propose deduplicating them into shared includes.
- **`tests/` not running in CI.** It is a model-scored benchmark: it calls the
Anthropic API (`new Anthropic()`, so it needs `ANTHROPIC_API_KEY`) and grades
skill selection. It is not a deterministic unit-test suite. Do not ask for it
to gate the PR, and do not ask for exact-match assertions in place of model
scoring.
- **Long files and long lines under `skill/references/`.** Reference documents
are meant to be exhaustive and there is no line-length or file-size lint.
Size alone is not a finding.
- **Prose that repeats itself between `README.md` and `skill/`.** The README
describes the skill to humans; `skill/` addresses agents. Parallel wording is
expected — only flag it when the two actually contradict each other.
Loading