Skip to content

Add workflow for project stability sync#11500

Open
stdevMac wants to merge 1 commit intomasterfrom
chore/project-stability-sync
Open

Add workflow for project stability sync#11500
stdevMac wants to merge 1 commit intomasterfrom
chore/project-stability-sync

Conversation

@stdevMac
Copy link
Copy Markdown
Member

@stdevMac stdevMac commented May 5, 2026

Changes

  • Add .github/workflows/project-stability-sync.yml to keep project 14 (Client Stability) board status in sync with issue and PR activity.
  • Handles the two transitions GitHub's built-in project workflows don't cover:
    • issues.assigned🏗 Triage (only when the item is currently Backlog or unset, so re-assignment never resets progress)
    • pull_request_target.closed (merged only) → linked issues to 🔍 Testing
  • Skips with a no-op when STABILITY_PROJECT_TOKEN is unset, so merging this without configuring the secret is safe.

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Notes on testing

The workflow logic is too thin for unit tests. Verify manually after adding STABILITY_PROJECT_TOKEN (a fine-grained PAT with Projects: read & write on the NethermindEth org) as a repo secret:

  1. Assign yourself to any open issue tracked in project 14 — it should move to 🏗 Triage.
  2. Open a draft PR with Closes #N against an in-progress issue, then merge it — the linked issue should move to 🔍 Testing.
  3. Re-assign an issue that's already In Progress — it should stay In Progress (the guard).

Documentation

Requires documentation update

  • Yes
  • No

Requires explanation in Release Notes

  • Yes
  • No

Remarks

  • Built-in project workflows cover the rest: Auto-add to project + Item added to project (new issues → Backlog), Pull request linked to issue (added by GitHub on 2025-11-06 — linked issue → In Progress), and Item closed (closed → Done). This Action only fills the two gaps GitHub doesn't expose as triggers.
  • Uses pull_request_target rather than pull_request so secrets are available on fork PRs. Safe here because the workflow never checks out PR code — it only reads event metadata and calls the GraphQL API.
  • All github.event.* values flow through env: vars per GitHub's workflow injection hardening guide.

@stdevMac stdevMac requested a review from rubo as a code owner May 5, 2026 17:37
@stdevMac stdevMac changed the title feat: add workflow for project stability sync Add workflow for project stability sync May 5, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 5, 2026

Claude finished @stdevMac's task in 2m 19s —— View job


Code Review

  • Gather context and read the workflow file
  • Analyze security, correctness, and edge cases
  • Post review findings with inline comments

Summary

The workflow is well-structured and follows GitHub's security hardening guidelines correctly — all github.event.* values flow through env: vars and into GraphQL variables rather than being interpolated directly into shell strings, which prevents injection. The pull_request_target usage is safe because no code is checked out. The graceful no-op when the secret is unset is a nice touch.

Two medium findings should be addressed before merge, plus two low-severity notes.

Findings

Severity Finding
Medium No timeout-minutes on the job — can hold a runner for 6 h on API hang (line 27)
Medium PR merged → Testing transition has no forward-only guard — a Done issue can regress to Testing (lines 114–115)
Low No concurrency group — rapid reassignments spawn racing runs (lines 17–26)
Low closingIssuesReferences(first:50) silently drops issues beyond 50 (line 95)

What looks good

  • Injection hardening via env: + GraphQL variables throughout
  • pull_request_target justified correctly (no code checkout, secrets needed for fork PRs)
  • ensure_in_project is idempotent — safe to call for items already tracked
  • current_status correctly returns "" for unset fields via // ""
  • set -euo pipefail ensures unexpected API errors surface as failures rather than silent no-ops
  • The Triage guard (cur == "" || cur == OPT_BACKLOG) correctly prevents re-assignment from resetting in-progress items

@stdevMac stdevMac force-pushed the chore/project-stability-sync branch from b7ca642 to 196fff1 Compare May 5, 2026 17:38
Comment thread .github/workflows/project-stability-sync.yml
Comment thread .github/workflows/project-stability-sync.yml
Comment thread .github/workflows/project-stability-sync.yml
Comment thread .github/workflows/project-stability-sync.yml
@stdevMac stdevMac force-pushed the chore/project-stability-sync branch from 196fff1 to ec5c982 Compare May 5, 2026 17:40
@stdevMac
Copy link
Copy Markdown
Member Author

stdevMac commented May 5, 2026

@claude review again

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 5, 2026

Claude finished @stdevMac's task in 1m 22s —— View job


Code Review

  • Gather context and read the workflow file
  • Re-analyze security, correctness, and edge cases
  • Post updated review findings

Summary

Re-review of the same file. The four findings from the first pass remain unaddressed. One new medium-severity finding was identified this time: the secret existence guard is incomplete. All other security properties continue to look correct (injection hardening via env: + GraphQL variables, pull_request_target without code checkout, idempotent ensure_in_project).

Findings

Severity Finding Status
Medium Incomplete secret guard — OPT_TRIAGE and OPT_TESTING not validated at line 33, causing ugly API failures instead of a clean skip (line 33–36) New
Medium No timeout-minutes on the job — defaults to 6 h on a hung GraphQL call (line 13) Unaddressed
Medium PR merged → Testing has no forward-only guard — a Done issue can regress to Testing (line 101–104) Unaddressed
Low No concurrency group — rapid reassignments can spawn racing runs (line 12) Unaddressed
Low closingIssuesReferences(first:50) silently drops linked issues beyond 50 (line 78) Unaddressed

What looks good

  • Injection hardening via env: + GraphQL variables throughout — no shell interpolation of event data
  • pull_request_target correctly justified: no code checkout, secrets needed for fork PRs
  • ensure_in_project is idempotent — safe to call for items already tracked
  • current_status correctly returns "" for unset fields via // ""
  • set -euo pipefail ensures unexpected API errors fail visibly rather than silently
  • Triage guard (cur == "" || cur == OPT_BACKLOG) correctly prevents re-assignment from resetting in-progress items
  • PR_MERGED check prevents closed-unmerged PRs from transitioning issues

Comment thread .github/workflows/project-stability-sync.yml Outdated
@stdevMac stdevMac force-pushed the chore/project-stability-sync branch from ec5c982 to 9ea9fd1 Compare May 5, 2026 17:46
@stdevMac
Copy link
Copy Markdown
Member Author

stdevMac commented May 5, 2026

@claude another review

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 5, 2026

Claude finished @stdevMac's task in 1m 42s —— View job


Code Review (Pass 3)

  • Read the workflow file
  • Check whether prior findings were addressed
  • Identify any new issues
  • Post findings

Summary

All three medium findings from the previous reviews have been addressed in the current file. The workflow is now in good shape. One new low-severity note was identified. The remaining first:50 pagination limit was previously flagged as awareness-only and remains unchanged.

Prior findings — status update

Severity Finding Status
Medium No timeout-minutes ✅ Fixed — timeout-minutes: 5 at line 19
Medium PR merged → Testing could regress Done items ✅ Fixed — guard at lines 112–113 checks OPT_DONE and skips
Medium Incomplete secret guard (OPT_TRIAGE, OPT_TESTING missing) ✅ Fixed — guard now includes all 7 required vars (lines 39–44)
Low No concurrency group ✅ Fixed — concurrency group added at lines 9–11
Low closingIssuesReferences(first:50) silently drops >50 linked issues Unchanged — acknowledged as awareness-only, no action needed

New finding

Severity Finding
Low OPT_DONE is required in the guard even for issues.assigned events that never use it — a missing OPT_DONE silently skips Triage sync with no diagnostic. Consider logging which var is missing (line 41)

What looks good

  • Injection hardening via env: + GraphQL variables throughout — no shell interpolation of event data
  • pull_request_target correctly justified: no code checkout, secrets needed for fork PRs
  • ensure_in_project is idempotent — safe to call for items already tracked
  • current_status correctly returns "" for unset fields via // ""
  • set -euo pipefail ensures API errors fail visibly
  • Triage guard (-z "$cur" || "$cur" == "$OPT_BACKLOG") correctly prevents re-assignment resets
  • Done-regression guard ("$cur" == "$OPT_DONE") prevents merged PRs from pulling Done issues back to Testing
  • concurrency: cancel-in-progress: false is the right call — both transitions set terminal states so all runs should complete


if [[ -z "${GH_TOKEN:-}" || -z "${PROJECT_ID:-}" || -z "${STATUS_FIELD_ID:-}" \
|| -z "${OPT_BACKLOG:-}" || -z "${OPT_TRIAGE:-}" || -z "${OPT_TESTING:-}" \
|| -z "${OPT_DONE:-}" ]]; then
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.

Low — OPT_DONE over-required for issues.assigned path

OPT_DONE is only used in the pull_request_target.closed branch, but it's required in the guard for both event types. If an operator configures all secrets except OPT_DONE, the issues.assigned → Triage sync silently skips with no indication of which secret is missing.

This is a conservative choice that's safe, but consider splitting the guard to only require the secrets actually needed per event type, or at minimum log which variable is missing:

Suggested change
|| -z "${OPT_DONE:-}" ]]; then
missing=""
for var in GH_TOKEN PROJECT_ID STATUS_FIELD_ID OPT_BACKLOG OPT_TRIAGE OPT_TESTING OPT_DONE; do
[[ -z "${!var:-}" ]] && missing="$missing $var"
done
if [[ -n "$missing" ]]; then
echo "Required secrets not configured ($missing); skipping." >&2
exit 0
fi

This way the operator immediately knows which secret to add rather than debugging a silent skip.

@stdevMac stdevMac force-pushed the chore/project-stability-sync branch from 9ea9fd1 to 4266ae3 Compare May 5, 2026 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant