Repository navigation
Conversation
Dependency ReviewThe following issues were found:
License Issues.github/workflows/maintenance-alert.yml
OpenSSF Scorecard
Scanned Files
|
Memory benchmarkCompared base
Interpretation: No metric shows a clear Base-to-PR difference beyond its measured run-to-run variation. Extension overhead above pi baseline
Values are medians of 6 alternating, paired runs. The value after Environment: pi
|
patlux
left a comment
patlux
left a comment
There was a problem hiding this comment.
The trust model of the alert workflow (default-branch code only, no artifacts/logs, job-level if + script-level checks) is right. Comments inline are about robustness of the threshold logic, API volume, and two nits.
Two cross-PR notes:
workflows: [Command Code catalog sync, Latest host compatibility]— the second only exists after #162. Harmless (GitHub just never triggers), but worth mentioning in the merge order.- The
trackedmap in the script and theworkflows:list in the yml are both keyed by the workflowname:strings. A rename in eithermodel-metadata.ymlorlatest-hosts.ymlsilently disables alerts. A comment at all three places would help.
| status: "completed", | ||
| per_page: 100, | ||
| }) | ||
| const eligible = recent |
There was a problem hiding this comment.
The threshold relies on listWorkflowRuns already containing the triggering run as completed. workflow_run.completed can fire before the listing API reflects that (eventual consistency). If the current run is missing, eligible[0]/eligible[1] are the two previous runs and the decision is off by one in either direction.
Make it explicit: treat run as the head and take the newest eligible item with run_number < run.run_number as the predecessor, independent of whether the API already lists run.
| ) | ||
| ) | ||
| return "stale" | ||
| const issues = await github.paginate(github.rest.issues.listForRepo, { |
There was a problem hiding this comment.
state: "all" + creator paginates every bot-created issue forever. Add a dedicated label (e.g. maintenance-alert) on create and filter by labels here — cheaper and visible to humans in the issue list.
Same idea for listWorkflowRuns above: you only need runs up to the predecessor; github.paginate supports an early done() callback.
| issue_number: issue.number, | ||
| state: "closed", | ||
| state_reason: "completed", | ||
| body: `${marker}\n\nRecovered in [run ${run.run_number}](https://github.com/${repo.owner}/${repo.repo}/actions/runs/${run.id}).\n\n${config.action}`, |
There was a problem hiding this comment.
Recovery overwrites the body, so the failing-run links disappear from the issue once it is closed. A short comment ("Recovered in run N") plus close keeps the history — that is one comment per incident, not spam.
| - name: Compare with the latest Command Code CLI | ||
| shell: bash | ||
| run: npm run check:commandcode-catalog | tee commandcode-catalog-report.md | ||
| run: npm run check:commandcode-catalog 2>&1 | tee commandcode-catalog-report.md |
There was a problem hiding this comment.
2>&1 into the Markdown report also captures npm's own stderr (npm warn, the npm error banner on failure) into the step summary. If the intent is to keep the script's stderr, run tsx directly or use npm run --silent.
|
|
||
| ## Unreleased | ||
|
|
||
| - Open one actionable maintenance issue after repeated scheduled catalog/latest-host failures, update it without comment spam and close it after recovery. Keep report stderr, avoid broad formatting in catalog automation and document weekly release triage without automating publication. |
There was a problem hiding this comment.
CI/maintainer tooling is not user-facing; CONTRIBUTING limits the changelog to user-facing behavior. I'd drop the entry and let RELEASE.md be the record.
pierreraby
left a comment
pierreraby
left a comment
There was a problem hiding this comment.
Approved. Verified locally at the exact head in an isolated worktree (npm ci): tsc --noEmit PASS, test-maintenance-alert 8/8, full test:unit exit 0 on re-run (401 checks; first run hit a timing flake in the unrelated test-abort mid-stream test, which passes in isolation and is untouched by this diff), Prettier + git diff --check PASS. I also had this head independently reviewed in a fresh context (APPROVE, no P0/P1): threshold/dedup/reopen/recovery logic correct (2 consecutive failures, update-not-spam, close-on-recovery), fork/PR/stale guards double-enforced (workflow if + script re-checks), trusted default-branch code only, minimal permissions, tests non-vacuous.
Non-blocking follow-ups from that review: (1) per-item head_branch re-check in eligible/stale guard (currently relies on the list query param, which the test fixture ignores); (2) assert the created body starts with the dedup marker; (3) pin the tracked workflow names against .github/workflows/*.yml so a rename cannot silently disable alerts. None block merge.
This branch has not been deployed
2 participants
Summary
Verification
No real alert issue created during tests; no release, permission/settings changes or merge performed.