ci: stop the stale sweep from closing issues, drop stale tooling configs - #318
ci: stop the stale sweep from closing issues, drop stale tooling configs#318alexander-sei wants to merge 2 commits into
Conversation
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 Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
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.ymlremoves the repo's only AppSec review pass, and the PR body states its replacement is currently broken (Run codex execskipped,continue-on-errorreporting green). Per REVIEW_GUIDELINES §1packages/mcp-serveris the security surface, so this leaves that surface with no security-specific automated pass while CI still shows green. Worth linking a tracked issue forPLATFORM_CODE_AGENT_OPENAI_API_KEYbefore merging, or keeping the workflow until UCI's Codex pass is confirmed running. .windsurf/rules/docs.mdis the repo's only docs-writing guidance — I grepped for "mintlify" across all md/mdc/json and the sole remaining hit isdocs/docs.json(the config, not guidance), and there is noAGENTS.mdorCLAUDE.mdat 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.mdandcursor-review.mdare empty files. This review is Claude-only, consistent with what the PR body says about the Codex pass on this repo. issues: writeis 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.ymlsetssubmodules: recursive;checks.ymldoes a plainactions/checkout@v4), the narrowed CORS entry correctly notes neither transport validatesOrigin/Host, no dangling references to any deleted file remain, and.vscode/extensions.jsonwas 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 }} |
There was a problem hiding this comment.
[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 |
There was a problem hiding this comment.
[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>
There was a problem hiding this comment.
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 execskipped,continue-on-errorreports 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 untilPLATFORM_CODE_AGENT_OPENAI_API_KEYis 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: acontinue-on-errorjob 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.mdandcursor-review.mdare 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_TOKENandSLACK_CHANNEL_IDare 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.mddeleted 368 lines of Mintlify writing guidance with no replacement landing anywhere. There is noAGENTS.mdorCLAUDE.mdin the repo today, anddocs/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 indocs/are unrelated: they describe Windsurf as an MCP client.)- No changeset, correctly — nothing under
packages/**is touched, whichREVIEW_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 |
There was a problem hiding this comment.
[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.
| !.vscode/tasks.json | ||
| !.vscode/launch.json | ||
| !.vscode/extensions.json | ||
| .vscode/ |
There was a problem hiding this comment.
[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.
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 andissues: 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 keptissues: writerather than dropping it, sinceactions/stalereaches PRs through the issues API and removing the scope risks breaking PR staling instead.Also keyed concurrency on
github.refinstead ofgithub.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_TOKENandSLACK_CHANNEL_IDare 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 toAGENTS.mdrather than just losing. Windsurf mentions indocs/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
OriginorHost, so that gap should still get flagged. Fixes the submodule line: onlyrelease.ymlchecks them out, not the PR gate inchecks.yml.Left alone
The review also wanted concurrency on
ai-review.ymland a comment filter onai-assist.yml. Both are already handled inside UCI v0.0.15. Itsai-review.ymlsets 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 execis skipped, andcontinue-on-errorreports the job green anyway. sei-chain's equivalent runs fine, soPLATFORM_CODE_AGENT_OPENAI_API_KEYprobably isn't reaching this repo. Review is Claude-only until that's sorted.Made with Cursor