Skip to content

fix(kanban): advance-deploy-env advances closing ISSUES, not just PRs (backend#1600) - #168

Merged
LukasWodka merged 3 commits into
developfrom
fix/1600-advance-closing-issues
Aug 6, 2026
Merged

fix(kanban): advance-deploy-env advances closing ISSUES, not just PRs (backend#1600)#168
LukasWodka merged 3 commits into
developfrom
fix/1600-advance-closing-issues

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Fixes the root cause of the recurring "On dev" board drift — see backend#1600.

Problem

advance-deploy-env advances only the PRs in a push. An issue closed by a develop-merged PR is parked at On dev by the closure-router and then never advanced when its code ships to staging/prod. Evidence: on 2026-08-06 all 20 drifted cards were closed Issues, 0 PRs.

Change (additive, low-risk)

  • The existing per-PR advance loop is unchanged.
  • New block after it: for each PR in the push, resolve pullRequest.closingIssuesReferences and advance each closing issue's card with the samerank() monotonic guard, archived-skip, and update_field helper.
  • Cross-repo aware: each closing issue is looked up in its own repo — essential here, since client/website/etc. PRs routinely close backend issues.
  • Dry-run mode: new dry_run input; update_field short-circuits under it, so both the PR and issue paths log intended moves without writing.
  • Uses a here-string (not a pipe) for the issue loop so RUN_FAILED still propagates and a failed issue update fails the run, matching the PR path.

⚠️ Before merge (core automation — moves every card)

  1. Dry-run test: temporarily set dry_run: true on one repo's caller (or a controlled run) and confirm the [DRY] would set … logs list the right issues + columns, with no writes.
  2. Confirm cross-repo closing issues (e.g. a client PR closing a backend issue) resolve and advance in the logs.
  3. Then flip dry_run off and roll out.

Backstop (kanban-reconcile sweep for already-drifted closed issues) is tracked separately in backend#1600.

Do not fast-merge — this is the workflow that advances the whole board.


Note

Medium Risk
Extends the workflow that bulk-updates kanban Status/Deploy on every develop/staging/prod push; wrong issue selection or state checks could move many cards, though behavior mirrors the existing PR loop and dry_run supports safe validation.

Overview
Fixes On dev drift for issue cards: after deploy pushes, the workflow still advances PRs on the board, and now also advances closed issues linked via each PR’s closingIssuesReferences, including cross-repo closers, using the same monotonic rank() guard, archived skip, and update_field path as PRs. Open closing issues are skipped so in-progress cards aren’t promoted when merges land on develop without auto-closing.

Adds a dry_runworkflow_call input: when enabled, update_field only logs [DRY] would set … for both PR and issue updates with no GraphQL writes.

Reviewed by Cursor Bugbot for commit b980f98. Bugbot is set up for automated code reviews on this repo. Configure here.

… PR (backend#1600)
advance-deploy-env advanced only PRs; issues closed by a develop-merged PR were
parked at On dev by the closure-router and never advanced when their code shipped,
drifting permanently. For each PR in the push, resolve closingIssuesReferences
(cross-repo aware) and advance each closing issue with the same monotonic guard,
archived-skip and (new) dry-run behaviour. Adds a dry_run input; update_field
short-circuits under it so both the PR and issue paths can be tested without writes.
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

Reviewing this from a parallel session that reached the same diagnosis independently — the core change is right and I'm dropping my version in favour of it. Three findings, one of which I'd treat as must-fix before merge.

🔴 Must-fix: this reverts a SHA pin

- - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0+ - uses: actions/checkout@v4

That undoes the D10 pinning work from backend#1491 / #1490 on the single most-consumed reusable workflow in the org. A mutable @v4 tag can be repointed by the upstream owner at any commit, and this file runs with PROJECTS_KANBAN_TOKEN in scope.

Nothing will catch it for you. I checked: tracebloc/.github runs only its own repo-local actionlint.yml — it does not call the code-quality reusable workflow, so the action-pins job added in #159 (which is exactly the check that fails on a non-SHA tracebloc/-or-actions/ ref) never runs here. The repo that publishes the org's pinning rule is the one repo not subject to it. I'm filing that gap separately; for this PR, please just restore the SHA.

🟠 Silent cap at 30 closing issues

closingIssuesReferences(first: 30) with no totalCount comparison. A PR closing more than 30 advances the first 30 and leaves the rest looking exactly like "this PR closed nothing" — the same quiet-miscount class this ticket exists to remove, and against the no-silent-caps rule the rest of the fleet follows. Cheap fix: ask for totalCount alongside nodes, and if it exceeds what you read, echo "::warning::..." + RUN_FAILED=1 so the run goes red rather than half-done-and-green.

🟡 Two structural notes (take or leave)

  1. N+1 queries. This adds two GraphQL round-trips per PR (one for the closing refs, one per issue for its project item). The PR loop above already queries pullRequest(number:)closingIssuesReferences { nodes { number repository { nameWithOwner } projectItems { ... } } } nests inside that existing query and yields the item ids in the same response, so the whole feature costs zero extra calls. On a 50-PR promotion push that is ~100 requests saved against a rate-limited token.
  2. prnum is reassigned inside the second loop to relabel update_field's warning. It works today only because the first loop has finished. If anyone ever merges the two loops, that mutation silently corrupts the iteration. Passing the label (and the item id) as parameters instead of mutating the caller's loop variable removes the trap — that also lets the archived-check + rank-guard + both writes live in one advance_item function rather than two copies that can drift apart.

✅ What's right, for the record

Cross-repo resolution via repository { owner { login } name } is correct and it is the case that actually matters — most closing links here are product-repo PR → backend issue. The monotonic guard is correctly reused, so Done/Cancelled (rank 10) can't be dragged back into the pipeline, and the here-string keeps RUN_FAILED in the parent shell. The dry_run input is a genuine improvement over what I had.

Corroborating measurement from my side, if useful in the PR body: of the 10 issues currently in On dev, 8 have a closing PR that already shipped further — backend#606→#1476 (FR on staging), #1437→.github#157, #1507→#1509 (Prod), #1550→#1551, #1555→client-runtime#277, #1556→.github#156 (Ready for prod), #1558→#1590. #1584/#1594 are correctly on dev and #1493 has no linked PR. Those 8 want a one-time backfill after this merges, since the fix only advances cards on future pushes.

The three pii-gate / pii-check failures are the known red-inert posture (fails closed on unset PII_DENYLIST, required nowhere) — not caused by this PR.

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 528d243. Configure here.

Comment thread.github/workflows/advance-deploy-env.yml
…igh)
PRs merge to develop (not the default branch), so closingIssuesReferences lists
still-OPEN issues; advancing those into deploy columns is wrong and reconcile
bounces them to Backlog. Gate the advancement on issue.state == CLOSED.
LukasWodka added a commit that referenced this pull request Aug 6, 2026
…1603) (#171)
* feat(ci): run the org's own code-quality suite on this repo (backend#1603)
This repo publishes code-quality.yml -- and its action-pins job -- to all 19
other repos, and was the only repo that never ran it. The exemption was
invisible because actionlint.yml looks like coverage: it type-checks every
expression, validates needs/uses wiring, and shellchecks every run: block.
What it does not do is look at action REFS. So the pinning rule this repo
defines was the one rule this repo was not subject to.
That is not theoretical. On 2026-08-06 .github#168 -- an otherwise correct
fix -- reverted actions/checkout from its pinned commit SHA back to the
mutable @v4 tag, in advance-deploy-env.yml, the most-consumed reusable
workflow in the org and one that runs with PROJECTS_KANBAN_TOKEN in scope.
action-pins exists to fail exactly that diff. It did not run. A human reading
the diff caught it, which is the review path D10 was written to stop
depending on.
Armed from day one rather than soft-failed into a backlog, because the tree
was measured first: shellcheck --severity=error clean over both shell
scripts, house-rules.sh clean over the same, and 24 `uses:` refs across 23
workflows with zero pin violations. ruff and gitleaks could not be run
locally and get their first honest look on this PR; whatever they surface is
fixed or baselined here rather than the gate being softened -- the same
bargain e2e-test-agent's caller records in its own comment.
action-pins-soft-fail is set to false explicitly rather than left to inherit
soft-fail. It is the check whose absence let #168 through, and every other
repo consumes these workflows at @main and inherits whatever refs they pin,
so this repo has a specific duty to enforce it on itself.
actionlint.yml stays separate. The two gates have deliberately different
postures -- actionlint blocks from day one against a tree cleaned in the same
change; code-quality ships soft-fail: true for repos still clearing backlog
-- and folding them together would force one posture onto both.
No paths: filter, so the check can be made required without leaving
workflow-free PRs waiting on a status that never reports.
Refs backend#1603, backend#1491 (D10), backend#1276.
* fix(ci): drop the action-pins inputs -- @main does not declare them yet
First push of this branch produced a startup_failure on the reusable call
(run 31086491251): every other check reported normally and `Code quality`
never started. Cause: `action-pins` and `action-pins-soft-fail` were added in
#159, which is on `develop`; `main` still carries the five-job version of
code-quality.yml. Callers must reference @main (Q3), and passing an input the
@main callee does not declare fails the entire call before any job runs.
Same merge-order trap as the #160 audit red: a change becomes consumable by
@main callers when it is PROMOTED, not when it is merged.
The other five jobs still run armed, so most of backend#1603 closes now. The
arming diff is written verbatim in the file so the follow-up is mechanical
once #159 reaches main.
Refs backend#1603.
This branch changed `actions/checkout@11d5960a…` to the mutable `@v4` tag in
advance-deploy-env.yml — the most-consumed reusable workflow in the org, and
one that runs with PROJECTS_KANBAN_TOKEN in scope. A floating major tag can
be repointed by the upstream owner at any commit, which is exactly what D10
pinning removes.
Nothing in CI would have caught it: tracebloc/.github runs only its own
actionlint.yml, which does not inspect action refs, and it does not call the
code-quality workflow whose action-pins job exists to fail this diff. That
gap is now backend#1603 (.github#171, merged) — but action-pins itself cannot
arm here until #159 reaches main, so for the moment this restore is the only
thing standing between the unpin and every repo that consumes these
workflows at @main.
No functional change to the backend#1600 fix.
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

Pushed the must-fix myself as b980f98b — restoring actions/checkout@11d5960a… — since you were actively pushing and this one shouldn't ride along to merge. No functional change to your #1600 fix; if you have local work, pull before your next push.

Context for why I didn't just leave it to CI: I checked, and nothing here would have caught it.tracebloc/.github runs only its own actionlint.yml, which doesn't inspect action refs, and it never called the code-quality workflow whose action-pins job exists to fail exactly this diff. I've filed that as backend#1603 and merged the caller (#171) — the suite now runs green on this repo — but action-pins itself still can't arm here until #159 reaches main, because a caller must reference @main and passing an input the @main callee doesn't declare kills the whole call with a startup_failure (measured: run 31086491251). So for now this restore is the only thing between the unpin and every repo consuming these workflows at @main.

The other two findings from my earlier review are still open and still yours to take or leave: the first: 30 silent cap (no totalCount check, so a PR closing >30 issues half-advances and reports green), and the N+1 query shape (the closing refs nest inside the pullRequest query the loop already makes, so the feature can cost zero extra round-trips).

Separately: I ran the one-time backfill for the already-stranded cards, since this fix only advances on future pushes — 7 issues moved to their closing PR's column, receipted on backend#1600. No overlap with anything this PR does at runtime.

@LukasWodka
LukasWodka merged commit c1b716c into developAug 6, 2026
7 of 9 checks passed
@LukasWodka
LukasWodka deleted the fix/1600-advance-closing-issues branch August 14, 2026 13:53
Sign up for freeto 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

@LukasWodka@saadqbal