Skip to content

ci: stop the stale sweep from closing issues, drop stale tooling configs - #318

Open
alexander-sei wants to merge 2 commits into
mainfrom
ci/uci-followups
Open

ci: stop the stale sweep from closing issues, drop stale tooling configs#318
alexander-sei wants to merge 2 commits into
mainfrom
ci/uci-followups

Conversation

@alexander-sei

Copy link
Copy Markdown
Contributor

Follow-up to #317, acting on one valid finding from its own AI review plus some cleanup.

The fix

The stale sweep only set days-before-pr-stale, so issues fell through to UCI's defaults: stale after 8 days, closed 2 days later. With a daily cron and issues: write, that would have started auto-closing sei-js issues on a policy nobody picked. Issue staling is now off (-1), PR staling stays at 28 days. I kept issues: write rather than dropping it, since actions/stale reaches PRs through the issues API and removing the scope risks breaking PR staling instead.

Also keyed concurrency on github.ref instead of github.sha. Cron runs on an unchanged main share a SHA, so a manual dispatch would cancel a running sweep.

Removals

  • pr-to-slack-codex.yml, superseded by UCI's Codex pass. OPENAI_API_KEY, SLACK_BOT_TOKEN and SLACK_CHANNEL_ID are left unreferenced and can be revoked separately.
  • .windsurf/rules/docs.md. Windsurf-specific and stale, though it was 368 lines of Mintlify writing guidance that still applies, so it may be worth porting to AGENTS.md rather than just losing. Windsurf mentions in docs/ are untouched, those describe it as an MCP client.
  • .vscode/extensions.json, which recommended Prettier, ESLint and Angular Console for a Biome repo with no Angular. .vscode/ is now gitignored outright instead of allowlisting four files back in.

REVIEW.md

Drops the reference to the deleted Slack workflow. Narrows the CORS entry, since the bare 204 preflight only binds browsers and neither transport validates Origin or Host, so that gap should still get flagged. Fixes the submodule line: only release.yml checks them out, not the PR gate in checks.yml.

Left alone

The review also wanted concurrency on ai-review.yml and a comment filter on ai-assist.yml. Both are already handled inside UCI v0.0.15. Its ai-review.yml sets its own concurrency group, and the assistant's team check runs before the model and fails closed.

Separately, UCI's Codex pass is failing on sei-js: the proxy never starts, Run codex exec is skipped, and continue-on-error reports the job green anyway. sei-chain's equivalent runs fine, so PLATFORM_CODE_AGENT_OPENAI_API_KEY probably isn't reaching this repo. Review is Claude-only until that's sorted.

Made with Cursor

The stale check only configured `days-before-pr-stale`, so the issue side fell
through to UCI's defaults — stale after 8 days, closed 2 days later. A daily
cron with `issues: write` would have started closing community issues on a
policy nobody chose. Issue staling is now explicitly disabled; PR staling is
unchanged at 28 days. Also key the concurrency group on the ref rather than the
SHA, since consecutive cron runs on an unchanged default branch share a SHA and
a manual dispatch would cancel a running sweep.

Remove pr-to-slack-codex.yml, superseded by the UCI Codex pass, and drop the
stale .windsurf and .vscode configs (the latter recommended Prettier and ESLint
extensions for a repo that uses Biome). .vscode/ is now fully gitignored.

REVIEW.md drops its reference to the deleted Slack workflow, narrows the CORS
entry so missing Origin/Host validation stays fair to raise, and corrects the
submodule claim: only release.yml checks them out, not the PR gate.

Co-authored-by: Cursor <cursoragent@cursor.com>
@codecov-commenter

codecov-commenter commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.75%. Comparing base (2a9a4fd) to head (747ae91).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #318   +/-   ##
=======================================
  Coverage   79.75%   79.75%           
=======================================
  Files          83       83           
  Lines        1309     1309           
  Branches      160      217   +57     
=======================================
  Hits         1044     1044           
+ Misses        265      259    -6     
- Partials        0        6    +6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A CI-cleanup PR that correctly disables issue staling (the real fix from #317's review) and removes three stale config files; REVIEW.md's rewritten submodule and CORS entries check out against the actual workflows. The concurrency change, however, is a no-op for the problem its comment claims to solve, and deleting pr-to-slack-codex.yml and .windsurf/rules/docs.md drops content whose replacements are (by the author's own account) not yet in place.

Findings: 0 blocking | 7 non-blocking | 2 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • Deleting pr-to-slack-codex.yml removes the repo's only AppSec review pass, and the PR body states its replacement is currently broken (Run codex exec skipped, continue-on-error reporting green). Per REVIEW_GUIDELINES §1 packages/mcp-server is the security surface, so this leaves that surface with no security-specific automated pass while CI still shows green. Worth linking a tracked issue for PLATFORM_CODE_AGENT_OPENAI_API_KEY before merging, or keeping the workflow until UCI's Codex pass is confirmed running.
  • .windsurf/rules/docs.md is the repo's only docs-writing guidance — I grepped for "mintlify" across all md/mdc/json and the sole remaining hit is docs/docs.json (the config, not guidance), and there is no AGENTS.md or CLAUDE.md at root to absorb it. docs/ is still a Mintlify site, so the 368 lines still apply. The PR body already flags this as "may be worth porting"; porting it in the same PR avoids it being silently lost.
  • Both second-opinion reviews produced no output: codex-review.md and cursor-review.md are empty files. This review is Claude-only, consistent with what the PR body says about the Codex pass on this repo.
  • issues: write is retained while issue staling is disabled. The stated rationale (actions/stale reaches PRs through the issues API) is defensible and I'd keep it, but it is now a scope the sweep no longer exercises for issues — worth a note if least-privilege gets revisited.
  • Verified accurate, no change needed: the REVIEW.md submodule correction matches reality (only release.yml sets submodules: recursive; checks.yml does a plain actions/checkout@v4), the narrowed CORS entry correctly notes neither transport validates Origin/Host, no dangling references to any deleted file remain, and .vscode/extensions.json was the only tracked file under .vscode/ so the broadened ignore rule untracks nothing. No changeset is required here (CI/docs only, no published package touched), consistent with REVIEW_GUIDELINES §3.
  • 2 suggestion(s)/nit(s) flagged inline on specific lines.

group: ${{ github.workflow }}-${{ github.sha }}
# Keyed on the ref, not the SHA: consecutive cron runs on an unchanged default
# branch share a SHA, and a manual dispatch would otherwise cancel a running sweep.
group: ${{ github.workflow }}-${{ github.ref }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[suggestion] This doesn't fix the scenario the comment describes. On a schedule event github.ref is the default branch ref (refs/heads/main), and a workflow_dispatch run from main resolves to the same refs/heads/main — so dispatch and cron still land in one group and, with cancel-in-progress: true, the dispatch still cancels a running sweep. Exactly as they did under github.sha, where both also share the head SHA of an unchanged main.

If anything the ref key collapses more runs than the SHA key did: two cron runs across a commit to main had distinct SHAs and would not have cancelled each other, but they share a ref and now will.

The change that actually protects an in-flight sweep is cancel-in-progress: false (a stale sweep is idempotent and cheap; there's little value in cancelling one). If you want cron and manual runs isolated instead, fold the event into the group:

group: ${{ github.workflow }}-${{ github.ref }}-${{ github.event_name }}

Either way the comment needs rewording, since it currently justifies the key by a property the key doesn't have.

# Issue triage is not automated here. Without this the UCI defaults apply
# (stale after 8 days, closed 2 days later), which would let a daily cron
# close community issues nobody agreed to close. -1 disables issue staling.
days-before-issue-stale: -1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[nit] Worth double-checking that stale-check.yml at UCI 6590124 actually declares days-before-issue-stale in its workflow_call.inputs — an input the callee doesn't declare fails the whole reusable-workflow call, and this only runs on a daily cron, so a break wouldn't surface until the next scheduled sweep. I couldn't fetch the UCI workflow from this environment to confirm. It fails loudly rather than silently, so this is low risk, but a one-off workflow_dispatch after merge would confirm it in seconds. The -1 value itself is right: that's actions/stale's documented "never stale" sentinel, and disabling staling also makes the close timer unreachable for issues.

Swapping the concurrency key from the SHA to the ref did not do what its
comment claimed. On both a schedule and a workflow_dispatch from main the ref
is refs/heads/main, and on an unchanged main the SHA matches too, so a manual
dispatch cancelled a running sweep either way. The ref key also collapses more
runs than the SHA key did, since two cron runs across a commit to main used to
have distinct SHAs.

cancel-in-progress: false is what actually protects an in-flight sweep.
Co-authored-by: Cursor <cursoragent@cursor.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CI-only cleanup that correctly fixes a real defect (issue staling was silently inheriting UCI's 8-day-stale/2-day-close defaults under a daily cron with issues: write), and the REVIEW.md edits are all factually accurate — I confirmed checks.yml does a plain checkout while only release.yml uses submodules: recursive, the registry postinstall runs git submodule update, and neither http-sse.ts nor streamable-http.ts reads Origin/Host. No blockers; the notable risk is that deleting pr-to-slack-codex.yml removes the repo's only working AppSec review pass while the PR itself reports the UCI Codex replacement as broken on this repo.

Findings: 0 blocking | 7 non-blocking | 2 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • Sequencing risk on removing pr-to-slack-codex.yml: the PR body states UCI's Codex pass is currently failing on sei-js (proxy never starts, Run codex exec skipped, continue-on-error reports green). REVIEW_GUIDELINES.md §1 names that Slack pass as a deliberate second AppSec opinion on the one package where a regression can cost user funds. Deleting it now leaves the repo with a single AI reviewer until PLATFORM_CODE_AGENT_OPENAI_API_KEY is sorted. Either land the deletion after UCI Codex is green here, or open a tracking issue and link it in the PR. Separately worth raising upstream: a continue-on-error job that reports green when its model step was skipped is a silent-failure mode — a review gate that cannot fail is indistinguishable from one that found nothing.
  • Both second-opinion passes produced no output — codex-review.md and cursor-review.md are empty files. This review is Claude-only, which is consistent with the PR description's note about the UCI Codex pass, but it means the merged review is a single perspective rather than three.
  • OPENAI_API_KEY, SLACK_BOT_TOKEN and SLACK_CHANNEL_ID are now unreferenced by any workflow. The PR notes they can be revoked separately — worth an actual tracking item, since an unused-but-live bot token in repo secrets is reachable by any future workflow (or any workflow a compromised PR could add).
  • .windsurf/rules/docs.md deleted 368 lines of Mintlify writing guidance with no replacement landing anywhere. There is no AGENTS.md or CLAUDE.md in the repo today, and docs/ is still an active Mintlify site (docs/docs.json, docs/contributing.mdx), so the guidance is simply lost rather than relocated. The PR body already flags this — recommend porting it in a follow-up rather than leaving it to git history. (The Windsurf mentions left in docs/ are unrelated: they describe Windsurf as an MCP client.)
  • No changeset, correctly — nothing under packages/** is touched, which REVIEW_GUIDELINES.md §3 exempts. Noting it only so the absence isn't re-raised.
  • 2 suggestion(s)/nit(s) flagged inline on specific lines.

# Issue triage is not automated here. Without this the UCI defaults apply
# (stale after 8 days, closed 2 days later), which would let a daily cron
# close community issues nobody agreed to close. -1 disables issue staling.
days-before-issue-stale: -1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[suggestion] Two things worth confirming before merge:

1. Does UCI v0.0.15 actually declare a days-before-issue-stale input? A reusable workflow rejects inputs it does not declare with an invalid-workflow error, which would break the sweep entirely rather than just the issue half. I could not fetch sei-protocol/uci from this sandbox to check the input schema, so this is a verification ask, not an assertion — if it resolves, ignore.

2. -1 disables marking, not closing. In actions/stale, the close path is gated on the issue already carrying the stale label, independent of days-before-stale. days-before-issue-close falls back to days-before-close (UCI's default per your comment: 2 days), so any issue that already picked up the stale label would still be closed on the next sweep — exactly the outcome this change is meant to prevent. #317 landed today and the cron is 0 12 * * *, so realistically zero or one sweep has run and the blast radius is probably empty, but the safe form is to also pass days-before-issue-close: -1 (if UCI exposes it) or to confirm no open issue currently carries the stale label.

Comment thread .gitignore
!.vscode/tasks.json
!.vscode/launch.json
!.vscode/extensions.json
.vscode/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[nit] Confirmed nothing is lost here: git ls-files .vscode/ is empty, so extensions.json was the only tracked file and the settings.json / tasks.json / launch.json negations were dead entries.

One forward-looking note — ignoring the directory (.vscode/) rather than its contents (.vscode/*) means a future !.vscode/settings.json will not work, since git does not descend into an excluded directory to re-include children. If you ever want to share a debug config, this line has to go back to .vscode/* first. Fine to leave as-is if the intent is that nothing under .vscode/ is ever shared.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants