Skip to content

fix(ci): dependabot major-bump comment uses marker-based upsert - #114

Closed
EricAndrechek wants to merge 1 commit into
mainfrom
dependabot-comment-upsert
Closed

fix(ci): dependabot major-bump comment uses marker-based upsert#114
EricAndrechek wants to merge 1 commit into
mainfrom
dependabot-comment-upsert

Conversation

@EricAndrechek

Copy link
Copy Markdown
Member

What

The "Comment on major bumps" step in `dependabot-automerge.yml` calls `gh pr comment` unconditionally on every workflow run. Dependabot re-synchronizes its PRs (lockfile resolution, version metadata refetches), and each synchronize re-fires the workflow → a fresh duplicate of the same hold-for-review comment each time.

PR #106 currently has five identical copies of this comment from the same Dependabot bump cycle.

Fix

Apply the same marker-comment upsert that `housekeeping.yml` already uses for its title-lint comment: stash a hidden marker (``) in the body, look it up on the issue's comments list, and PATCH the existing comment in place. New comment only on the first run for the PR.

Test plan

  • Next Dependabot major-version bump: one comment, edited in place as the PR re-syncs (not duplicated).

🤖 Generated with Claude Code

The "Comment on major bumps" step in dependabot-automerge.yml called
`gh pr comment` unconditionally on every workflow run. Dependabot
synchronizes the PR repeatedly (re-resolving the lockfile, picking
up version metadata), and each synchronize re-fires the workflow,
so PR #106 accumulated a fresh duplicate of the same hold-for-review
comment each time.

Apply the same marker-comment upsert pattern that housekeeping.yml
already uses for its title-lint comment: stash a hidden marker
(`<!-- dependabot-major-bump-comment -->`) inside the body, look it
up on the issue's comments list, and PATCH the existing comment in
place if one exists. New comment only on the first run for the PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@gemini-code-assist

Copy link
Copy Markdown

Note

Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported.

@claude

claude Bot commented May 12, 2026

Copy link
Copy Markdown

Claude finished @EricAndrechek's task in 5m 36s —— View job


PR Review

  • Read AGENTS.md for repo conventions
  • Read full PR diff
  • Review prior comments
  • Analyze the workflow change
  • Post review findings

Summary

Correct fix for a real annoyance. The marker-comment upsert is a direct port of the pattern housekeeping.yml already uses for the title-lint comment, and the implementation is sound.

Security: NONE. REPO/PR_NUMBER come from GitHub Actions context (not user-controlled), and the workflow only fires when github.actor == 'dependabot[bot]'.


Findings

Correctness — LGTM

The YAML literal-block indentation strips cleanly: after the base-10-space YAML strip, the double-quoted bash string resolves to \nMajor-version bump — ... with no leading whitespace artifacts. The contains("$MARKER") lookup is correct.

The head -n1 is a good defensive guard against the degenerate case where two simultaneous workflow runs both POSTed before either could see the other's comment.

Migration artifact (FYI, not a blocker)

Existing PRs that already have the old-style comment (posted before this fix, without the marker in the body) will get one new comment on the first re-sync after deploy — EXISTING would be empty since the old body lacks the marker. From that point on subsequent syncs would find and PATCH the newly-posted marker comment. PR #106 would see one additional comment during the first run, then stabilize. Not a regression; just worth knowing.

CHANGELOG (minor nit)

AGENTS.md §"Documentation & Consistency Sync" says every notable change gets an entry under [Unreleased]. The Unreleased section already tracks other workflow fixes (board-state-sync removal, etc.), so a one-liner here would be consistent:

- **`dependabot-automerge.yml`**: major-bump comment uses marker-based upsert to prevent duplicates on PR re-syncs.

Your call — it's genuinely borderline for a pure-CI fix.


Ship it. No blocking issues. The CHANGELOG entry is the only thing worth considering before merge.

@github-actions github-actions Bot added github_actions Pull requests that update GitHub Actions code area/infra CI, build, deploy, Docker, release labels May 12, 2026
@github-actions
github-actions Bot requested a review from taitelee May 12, 2026 14:12
@EricAndrechek

Copy link
Copy Markdown
Member Author

Superseded by #115, which contains this exact marker-comment upsert change verbatim inside a larger orchestrator-removal rewrite of dependabot-automerge.yml. Closing to avoid two merges of the same fix.

— Co-authored with Claude Opus 4.7 (1M context)

@github-project-automation github-project-automation Bot moved this from Ready to Done in WaveHouse Task Board May 12, 2026
EricAndrechek added a commit that referenced this pull request May 12, 2026
)

## Summary

Delete `project-orchestrator.yml` (and the two composite actions +
helper script it owned), fold the only behaviour worth keeping
(reviewer-assign on first open) into `housekeeping.yml`, and rely on
GitHub's native Projects-v2 workflows for everything else the
orchestrator was doing on the board.

**Net diff: −887 lines.** Workflow runs per non-Dependabot PR push go
from ~8 to ~5.

## Why now

The orchestrator was doing two conceptually separate jobs squeezed into
one workflow:
1. **Reviewer assignment** — pick the non-author admin, assign them,
request their review.
2. **Task Board state machine** — mirror PR state into project #7's
Status field, with "opposite semantics" between PR cards and
linked-issue cards.

For a 4-person team, #1 is genuinely useful as automation and #2 is
mostly bookkeeping that GitHub already provides natively (or that's a
one-click manual operation on the rare event it doesn't). Each PR push
was triggering 3-4 orchestrator runs through `workflow_run` chains, and
the workflow has been the largest single source of cross-trigger
complexity (GraphQL `statusCheckRollup` perms, integration-token `NONE`
for private members, etc. — every recent CI fix has touched it).

## What the native Project workflows already handle

Project #7 has these enabled (verified via `gh api graphql ... {
projectV2 { workflows } }`):

- **Auto-add to project** — new PRs / issues land on the board
automatically.
- **Item added to project** — sets the default Status when added
(configurable in the project UI; defaults to your project's setup).
- **Pull request merged** — sets Status = Done on merge.
- **Item closed** — sets Status when an issue is closed.
- **Pull request linked to issue** — auto-links via `Closes #N`
keywords.
- **Auto-close issue** — closes linked issues when PR merges.
- **Auto-add sub-issues to project**.

That covers placement, initial Status, and Done-on-merge. The
orchestrator's remaining custom behaviours are dropped (see "trade-offs"
below).

## Files

**Deleted:**
- `.github/workflows/project-orchestrator.yml` (546 lines)
- `.github/actions/board-upsert-status/` (composite, only orchestrator +
the now-trimmed dependabot-automerge step used it)
- `.github/actions/set-linked-issues-status/` (composite, only
orchestrator used it)
- `.github/scripts/board-fetch-item.sh` (helper, only the above used it)

**Kept:**
- `.github/actions/assign-and-request-review/` — still used by
`dependabot-automerge.yml` and now also `housekeeping.yml`.

**Modified:**
- `housekeeping.yml` — added a reviewer-assign step that fires on
`pull_request_target: opened` / `ready_for_review` (NOT on `synchronize`
— composite is idempotent but firing per-push would re-spam reviewers
after `dismiss_stale_reviews_on_push` clears a request following
CHANGES_REQUESTED). Picks the non-author admin by the same parity logic
the orchestrator used. `continue-on-error: true` so a flaky
review-request can't mask the title-lint required check's exit code.
`ready_for_review` added to the workflow's trigger types so the draft →
ready flip pings the reviewer.
- `dependabot-automerge.yml` — drop the board-upsert step (native
handles it), drop the `PROJECT_BOARD_TOKEN` guard (no longer needed in
this workflow), drop the `board-config.env` load (no consumers left),
and apply the marker-comment upsert pattern to the major-bump comment
(same fix as #114 — folded in here since they touch the same step).
- `.github/board-config.env` — drop `STATUS_*` (no consumers left) and
`ADMINS` (was only read by orchestrator; `housekeeping.yml` and
`admin-approval.yml` both inline it). Keep `PROJECT_OWNER` / `NUMBER` /
`ID` and `PRIORITY_*` for `triage.yml`.

## Trade-offs you're explicitly accepting

| Lost behaviour | Replacement |
|---|---|
| Reviewer pinged after CI + Claude go green | Reviewer pinged at open.
They can ignore the ping until checks settle. |
| Status promoted to Ready only on bot-clean | Status = whatever the
project's "Item added" workflow sets. |
| `changes_requested` moves PR card to "In review" | One click in the
project UI when it happens. |
| Linked issues auto-mirror PR state (opposite semantics) | Manual / not
mirrored. The PR list itself shows what needs review. |
| Auto-flip draft → ready on bot-clean | Author manually clicks "Mark
ready" (informative signal). |
| `re_requested` re-fires review request | Author manually clicks
"Re-request review" (the GitHub default). |

## Conflict with #114

#114 (the standalone dependabot major-bump comment upsert fix) and this
PR both edit the same step. Whichever lands first, the other needs a
trivial rebase. The upsert pattern in this PR matches #114 exactly, so
if #114 lands first the conflict resolution is "take theirs"; if this
lands first, #114 closes as already-incorporated.

## Test plan

- [ ] Open a fresh non-Dependabot PR: `housekeeping` runs, assigns the
non-author admin, requests their review. No orchestrator runs fire.
- [ ] Open a major-version Dependabot PR: both admins assigned, comment
posted with marker, no board-upsert step in the run log.
- [ ] Open a Dependabot patch / minor PR: auto-approve + auto-merge as
before.
- [ ] Open a draft, push commits, mark ready: reviewer pinged at the
ready_for_review event, not earlier.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@EricAndrechek
EricAndrechek deleted the dependabot-comment-upsert branch June 5, 2026 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/infra CI, build, deploy, Docker, release github_actions Pull requests that update GitHub Actions code

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants