Integrate UCI: AI review, AI assistant, and stale check - #13
Integrate UCI: AI review, AI assistant, and stale check#13alexander-sei wants to merge 2 commits into
Conversation
This repo had no CI at all, so these are its first workflows. They adopt the same UCI reusable workflows sei-chain calls, wired to the subset that applies to a docs/skill repo — go-lint, go-test, and goreleaser have no Go to act on, release-check/release-publish key off a version.json this repo doesn't have, and backport needs release/** branches that don't exist here. All three callers pin UCI v0.0.15, the latest release. The AI workflows pin the release commit rather than the tag, and pass the same SHA as uci-ref: v0.0.14 moved the shared prompts from .github/seidroid to .github/seidroid/auto-review, so a uses:/uci-ref mismatch would silently fail to load them. Two deliberate differences from sei-chain's callers: - enable-cursor: false, because Cursor's built-in Bugbot already reviews PRs on this repo and the scout would duplicate it. Same reason sei-chain disables it. - The pull_request trigger adds labeled/unlabeled so the `ai: skip-review` label takes effect immediately instead of on the next push. UCI's preflight gate only runs on those two actions when the changed label is the skip label, so unrelated label edits don't queue extra reviews. REVIEW.md carries the repo-specific review standards that ai-review loads from the PR's base branch. Since the product here is prose other agents consume as ground truth, it puts factual accuracy first — the categories that have actually regressed before are gas values, hardware and toolchain requirements, and explorer references. It also documents two silent failure modes in install.sh: a reference file not linked from a SKILL*.md is dropped from every flattened agent install because the file list is grepped out of the entry point, and the awk frontmatter strip mis-slices rather than erroring on malformed frontmatter. Before the AI workflows can run, the seidroid GitHub App needs installing on this repo with read access to sei-protocol/uci, and the PLATFORM_CODE_AGENT secrets and org variables need to be in scope for sei-skill. The `ai: skip-review` label does not exist here yet. Co-authored-by: Cursor <cursoragent@cursor.com>
![github-actions[bot]](https://cdn.github.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean, well-documented first-CI PR: three UCI reusable-workflow callers plus a repo-specific REVIEW.md, with a thorough rationale for what was included and excluded. No blockers — every factual claim REVIEW.md makes about install.sh and tests/run.ts checks out against the tree. Remaining notes are operational: tag-vs-SHA pinning inconsistency in the stale check, fork-PR credential behaviour for ai-review, missing concurrency guards, and the fact that both second-opinion reviewer passes returned empty output on this run.
Findings: 0 blocking | 11 non-blocking | 5 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- Both second-opinion passes produced no output:
codex-review.mdandcursor-review.mdare both empty (1 byte).enable-cursor: falseexplains the Cursor gap by design, but the Codex pass returning nothing on the PR that wires up Codex is worth confirming before treating this run as a successful live test. REVIEW_GUIDELINES.md(the base-branch copy handed to this review) also arrived empty — expected, sinceREVIEW.mdisn't onmainyet, but it means the guidelines-loading path is still unverified end-to-end. Worth confirming UCI v0.0.15 reads repo standards from rootREVIEW.mdspecifically (vs..github/REVIEW.mdor another path): a name/path mismatch degrades silently to "no guidelines", which is indistinguishable from the empty file seen on this run. The first PR opened after merge is the real test.- I verified every factual claim REVIEW.md makes about this repo and they all hold:
install.sh:283is thegrep -Eo 'references/…' | sort -udiscovery loop,install.sh:288-292is the second----awk strip,set -eat line 10 with noset -u/pipefail,AGENT_DEFAULT_OUTPUTis the correct variable name, the script is clean ofmapfile/declare -A/${var,,}, andtests/run.ts:40-55parsesname/descriptionfrom frontmatter. Good — this file becomes ground truth for every future reviewer, so the accuracy matters. - REVIEW.md's "Known non-issues — do not flag these" section is a finding-suppression list that any future PR author can extend. That's legitimate repo policy and the design is safe (UCI loads it from the base branch, so a suppression only takes effect after it's been merged/reviewed), but it's worth stating that property explicitly somewhere so a future PR that edits REVIEW.md gets extra scrutiny rather than being treated as docs churn.
- Nothing links to
REVIEW.mdfromREADME.md. A one-line pointer would keep human contributors aware the file drives automated review. days-before-pr-stale: 28is set, but the job also requestsissues: writeand no issue-side input is passed. Ifstale-check.yml's issue defaults are active, issues on this repo will start getting staled/closed on a schedule nobody chose. Worth setting the issue input explicitly (or-1/disabled) rather than inheriting a default.- 5 suggestion(s)/nit(s) flagged inline on specific lines.
| permissions: | ||
| issues: write | ||
| pull-requests: write | ||
| uses: sei-protocol/uci/.github/workflows/stale-check.yml@v0.0.15 |
There was a problem hiding this comment.
[suggestion] Pinning inconsistency: this one uses the mutable tag v0.0.15 while ai-review.yml and ai-assist.yml pin the release commit 6590124…. The PR description makes the case for SHA pinning (the v0.0.14 prompt-path move), and tags can be re-pointed after the fact. Same-org repo so the risk is low, but pinning all three to the same SHA would make the set uniform and auditable — and would let one grep confirm every UCI reference moves together on the next bump.
| @@ -0,0 +1,20 @@ | |||
| name: AI Review | |||
| on: | |||
| pull_request: | |||
There was a problem hiding this comment.
[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.
| 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 ] |
There was a problem hiding this comment.
[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.
| pull-requests: write | ||
| checks: write | ||
| id-token: write | ||
| secrets: inherit |
There was a problem hiding this comment.
[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.
| pull-requests: write | ||
| issues: write | ||
| id-token: write | ||
| secrets: inherit |
There was a problem hiding this comment.
[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.
![cursor[bot]](https://cdn.github.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c24de9e. Configure here.
| 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 ] |
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit c24de9e. Configure here.
Addresses review feedback on #13. The stale check used the mutable v0.0.15 tag while the two AI callers pinned the release commit. Tags can be re-pointed after the fact, so pin all three to the same SHA — the set is now uniform, and one grep confirms every UCI reference moves together on the next bump. Also link REVIEW.md from the README contributing steps, so human contributors know it is what automated review grades against. Issue staling keeps UCI's defaults (stale at 8 days, closed after 2), matching sei-chain, rather than being disabled or lengthened. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks — addressed the two actionable items in 60af3dd and closing out the rest below. Fixed
Already handled upstream — no caller change needed
Confirmed, and worth recording
Deliberately unchanged
|
There was a problem hiding this comment.
Well-scoped, well-documented first CI for this repo: three UCI callers pinned to a commit SHA plus a genuinely useful REVIEW.md whose claims about install.sh and tests/run.ts I verified against the tree and found accurate. No blockers, but the stale-check job's permissions: block silently drops contents: read, the README's new "reviewed automatically" claim won't hold for fork PRs, and none of the workflows set a caller-level concurrency group.
Findings: 0 blocking | 13 non-blocking | 7 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- Both second-opinion passes produced no output:
codex-review.mdandcursor-review.mdare empty, as is the base-branchREVIEW_GUIDELINES.md. So this review is a single-model pass with no repo-specific guidelines applied — the Codex and Cursor findings are simply absent rather than clean. - I could not independently verify the pinned ref or the
workflow_callcontracts: network calls tosei-protocol/uciwere not permitted in this environment, so the claims in the test plan (SHA == v0.0.15,enable-cursor/allowed-team/days-before-pr-staleare real inputs at that ref, prompt paths resolve) are taken on trust. Everything I flag about the reusable workflows is from the caller side only. - Worth confirming the filename UCI v0.0.15 expects for repo-specific standards. The PR adds
REVIEW.md, but the harness for this very review was handed a file namedREVIEW_GUIDELINES.md. That may just be UCI stagingREVIEW.mdunder a generic name, and it is empty here for the expected reason (it is read from the base branch, where the file does not exist yet) — so this PR's own run cannot validate it. Since a name mismatch fails silently with an empty guidelines file and no error, it is worth checking against the reusable workflow directly rather than inferring from the first post-merge run. - Loading REVIEW.md from the base branch is the right call and should stay that way — it is what stops an outside contributor from editing the reviewer's own instructions in the same PR to suppress review of their changes. Flagging it so the property is deliberate rather than incidental if the ref is ever revisited. Relatedly, REVIEW.md §5 is by design a list of findings the reviewer should suppress; that is fine as a maintainer-controlled file on a protected branch, but each future addition to §5 narrows review coverage and deserves the same scrutiny as a code change.
- No deterministic CI is added, only AI review and a stale bot. The two silent failure modes REVIEW.md documents are both mechanically checkable in a few lines of shell, and a check would be far more reliable than asking a reviewer to remember: (1) §2's invariant — I confirmed all 46 files under
skill/references/are currently linked fromskill/SKILL.md, exactly matching, so acomm-style diff offindoutput against thegrep -Eo 'references/...'list would hold today and fail loudly on an unlinked or renamed file; (2) §3's frontmatter contract — asserting eachSKILL*.mdopens with---and has a second---would turn the awk mis-slice into an error. Abash -n install.shsmoke check is similarly cheap. This does not conflict with §5's "tests/should not gate the PR" — that is about the model-scored benchmark, which is a different thing. - Naming is inconsistent across the three new files:
name: UCI+run-name: UCI / Stale Checkin one, barename: AI Review/AI Assistantin the others. Cosmetic, and presumably mirrors sei-chain, but it makes the Actions list read oddly. - 7 suggestion(s)/nit(s) flagged inline on specific lines.
| jobs: | ||
| stale: | ||
| name: Stale | ||
| permissions: |
There was a problem hiding this comment.
[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: writeAlso worth checking whether stale-check.yml needs secrets: inherit like the other two callers, or genuinely runs on the default GITHUB_TOKEN.
| ``` | ||
| 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. |
There was a problem hiding this comment.
[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.
| @@ -0,0 +1,20 @@ | |||
| name: AI Review | |||
| on: | |||
| pull_request: | |||
There was a problem hiding this comment.
[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).
| # 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: |
There was a problem hiding this comment.
[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: trueSame consideration applies to ai-assist.yml, keyed on the issue/PR number, so two @seidroid comments in quick succession do not run concurrently.
| @@ -0,0 +1,22 @@ | |||
| name: AI Assistant | |||
| on: | |||
| issue_comment: | |||
There was a problem hiding this comment.
[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_requestSkip if UCI's own preflight already handles this — worth a quick check, since the cost of being wrong is only noise.
| contents: read | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.sha }} |
There was a problem hiding this comment.
[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.
|
|
||
| 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. |
There was a problem hiding this comment.
[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.
1 participant
Summary
Adds this repo's first CI: the UCI reusable workflows that
sei-chaincalls, wired to the subset that applies to a docs/skill repo, all pinned to v0.0.15 (the latest release). No xreview.ai-review.yml65901242783550521f25a19199a6b10e54550b97(v0.0.15)pull_requestai-assist.yml65901242783550521f25a19199a6b10e54550b97(v0.0.15)uci-stale-check.ymlv0.0.15workflow_dispatchPlus
REVIEW.md, the repo-specific review standardsai-reviewloads from the PR's base branch (analog of sei-chain #3774).Why these three and not the rest
go-lint,go-test, andgoreleaserhave no Go to act on.release-check/release-publishkey off aversion.jsonthis repo doesn't have, and it has no tags or releases.backportneedsrelease/**branches that don't exist here.seidroid-xreviewis intentionally excluded.Two deliberate differences from sei-chain's callers
enable-cursor: false— Cursor's built-in Bugbot already reviews PRs on this repo (see thecursor[bot]summaries on Make install additive by default; add --force for clean reinstall #7, Fix stale node RAM and Go version requirements in skill references #8, Refresh Sei skill: SIP-3 bridges/IBC, Mintlify docs, gas & oracle accuracy #9), so the Cursor scout would duplicate it. Same rationale sei-chain uses.labeled/unlabeledadded to thepull_requesttrigger — makes theai: skip-reviewlabel take effect immediately rather than on the next push. UCI's preflight gate only runs on those two actions when the changed label is the skip label, so unrelated label edits don't queue extra reviews.On the pinning
The AI workflows pin the release commit rather than the tag and pass the same SHA as
uci-ref. v0.0.14 moved the shared prompts from.github/seidroid/to.github/seidroid/auto-review/, so auses:/uci-refmismatch would silently fail to load them. Note sei-chain is still on v0.0.13 for these, so this repo is slightly ahead.REVIEW.md
The product here is prose that other agents consume as ground truth, so it puts factual accuracy first — the categories that have actually regressed before are gas values, hardware/toolchain requirements, and explorer references. It also documents two silent failure modes in
install.sh: a reference file not linked from aSKILL*.mdis dropped from every flattened agent install (the file list is grepped out of the entry point), and the awk frontmatter strip mis-slices rather than erroring on malformed frontmatter.Before this works
sei-protocol/sei-skillwith read access tosei-protocol/uci(to fetch prompts), plus org Members: Read for the assistant's team check — it fails closed and denies everyone without it.PLATFORM_CODE_AGENT_*secrets and org variables (PLATFORM_CODE_AGENT_ANTHROPIC_FDRL_ID,SEI_LABS_ANTHROPIC_ORG_ID,PLATFORM_CODE_AGENT_ANTHROPIC_SVAC_ID,PLATFORM_CODE_AGENT_USER_ID) are in scope for this repo, not just sei-chain.ai: skip-reviewlabel — it doesn't exist on this repo yet.Test plan
uses:,with:, andpermissions:verified against each reusable workflow'sworkflow_callinput contract at v0.0.15.uci-ref.enable-cursor,allowed-team, anddays-before-pr-staleconfirmed to be real inputs at v0.0.15.ai-review— expect oneAI Reviewcheck run and a single PR review once the app and secrets are in place.@seidroidon this PR to exercise the assistant.Made with Cursor