🐛 [scanner] fix: document catch-binding convention to prevent lint regressions#21360
Conversation
kubestellar-prow
Bot
added
the
dco-signoff: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
|
🐝 Hi @clubanderson! I'm Trusted users — org members and contributors with write access — can mention Automation may take a moment to start, and follow-up happens through workflow activity rather than chat replies. |
kubestellar-prow
Bot
added
the
size/XS
✅ Test Coverage CheckAll new source files in this PR have corresponding test files. Checked |
♿ Accessibility Audit (WCAG 2.1 AA)✅ No WCAG 2.1 AA violations detected in audited routes. Powered by axe-core. Target: WCAG 2.1 AA compliance. |
There was a problem hiding this comment.
Pull request overview
Fixes the failing CreateNamespaceModal unit test by ensuring a cluster is auto-selected when cluster options become available, aligning the modal’s initialization behavior with established patterns elsewhere in the web UI.
Changes:
- Add a
useEffectto auto-select the first available cluster when none is selected. - Update React imports to include
useEffect.
| useEffect(() => { | ||
| if (!cluster && clusters.length > 0) { | ||
| setCluster(clusters[0]) | ||
| } | ||
| }, [clusters, cluster]) |
❌ Playwright Tests Failed📊 View Full ReportDownload the To view the report locally: # Download and extract playwright-report.zip
npx playwright show-report path/to/playwright-report |
20b959c to
ddd45e5
Compare
clubanderson
left a comment
clubanderson
left a comment
There was a problem hiding this comment.
🚨 BLOCK — title/branch/description don't match the diff (quality review, COMMENT-only, self-approve blocked)
The PR is misfiled:
- Title: "auto-select first cluster in CreateNamespaceModal"
- Branch:
scanner/fix-21352 Fixes #21352 #21353- Actual diff: 4 lines in
.github/workflows/release.yml— changes/tmp/go-test-output.txt→go-test-output.txtin the release Go-test step
The body actually describes the workflow change correctly, but the title, branch, and Fixes issues advertise a completely different fix. Please either:
- Retitle to
[scanner] fix: use CWD instead of /tmp in release.yml go-test stepand re-verify theFixes #refs point at the right issues, or - Close this PR and reopen the intended CreateNamespaceModal change.
If the intent really is the workflow change, one substantive concern:
The claim "/tmp is blocked in the runtime environment" is unusual on GitHub-hosted runners — /tmp is world-writable and consistently available. This is only an issue on self-hosted or hardened runners. Please state which runner label the failing release job uses (runs-on: value from release.yml line ~180) and whether the block is documented policy — if it's a one-off flake, moving to CWD is a workaround for the symptom, not the cause. If it IS policy, other workflows in the repo also write to /tmp and should be swept in the same PR (there are Playwright artifact paths, kind kubeconfig temp paths, etc.).
Minor: tee go-test-output.txt writes to $GITHUB_WORKSPACE and the file gets included in any subsequent actions/upload-artifact glob — verify it's not accidentally uploaded to a release asset.
Not blocking on scope, but…
If it's really the workflow change, the tier label reads tier/3-restricted (typical for release-workflow edits) — good, that gates a human review.
quality agent · ACMM L4/L6 full · bead filed
…gressions (#21352 #21353) Root cause of nightly failure (run 29805735617): unused `catch (error)` binding in EnterpriseLayout.tsx introduced by #21342 caused a new `@typescript-eslint/no-unused-vars` violation beyond the baseline. The code fix (drop the binding → `catch { }`) landed in #21345. This follow-up: - Adds an inline comment on the no-unused-vars rule in eslint.config.js explaining the correct pattern so future contributors avoid the trap. - Improves the lint-baseline-check.mjs error message with a targeted tip for the most common offender (unused catch bindings). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Scanner Agent <scanner@agents.github.com>
ddd45e5 to
bc528e5
Compare
github-actions
Bot
added
tier/2-standard
ai-generated
clubanderson
changed the title
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
⏹️ Post-Merge Verification: cancelledCommit: |
|
Post-merge build verification passed ✅ Both Go and frontend builds compiled successfully against merge commit |
Fixes #21352
Fixes #21353
Root cause
The nightly Release workflow failed at commit
2ea59dd25(2026-07-21) because a new@typescript-eslint/no-unused-varslint violation was introduced inEnterpriseLayout.tsxby #21342. The catch bindingcatch (error)was declared but never referenced, exceeding the lint baseline count and causing the Lint frontend step to fail.The failing jobs in run #29805735617:
npm run lint:check) — actual failure stepExisting code fixes (already merged to main)
errorbinding inEnterpriseLayout.tsx— core lint fixThis PR — regression guard
To prevent the same class of mistake (naming a catch binding you never use) from silently slipping through again:
web/eslint.config.js— adds an inline comment on theno-unused-varsrule explaining the correct pattern: usecatch { }(no binding) when the error value is not needed; prefix with_only when a type annotation is required.web/scripts/lint-baseline-check.mjs— improves the CI failure message with a targeted tip for the most common offender (unused catch bindings), pointing developers directly to the fix.No rule severity or baseline changes — documentation-only, zero new lint violations.