Skip to content

feat(kodiak): auto-approve+merge Dependabot patch/minor bumps - #6

Closed
rivernate wants to merge 1 commit into
mainfrom
nathan/kodiak-dependabot-automerge
Closed

feat(kodiak): auto-approve+merge Dependabot patch/minor bumps#6
rivernate wants to merge 1 commit into
mainfrom
nathan/kodiak-dependabot-automerge

Conversation

@rivernate

Copy link
Copy Markdown
Member

Onboards this repo to the Kodiak Dependabot auto-merge rollout (derivita/infrastructure#1719), so Dependabot's own patch/minor bumps auto-approve and auto-merge via Kodiak, while major-version bumps still require full human review. Reference: derivita/infrastructure#1716 (first rollout), derivita/derivita-ui#247 (closer analog — also started from zero prerequisites).

Files added

  • .github/dependabot.yml — did not previously exist. Added a single npm ecosystem entry (weekly schedule, 7-day cooldown, minor-and-patch update-types group), matching the org convention (see derivita/launchpad's dependabot.yml). This repo's only dependency manifest is package.json/package-lock.json/yarn.lock at the root — no go.mod, Dockerfile, requirements.txt, or Chart.yaml. The MODULE.bazel/WORKSPACE Bazel deps are out of scope; Dependabot has no ecosystem for bzlmod/WORKSPACE.
  • .kodiak.toml — did not previously exist. [merge] with method = "squash" and [approve] auto_approve_labels = ["dependabot-automerge"], matching #1716's reference implementation verbatim.
  • .github/workflows/dependabot-automerge-label.yml — thin caller into derivita/shared-workflows's reusable dependabot-automerge-label.yml@v1 (pinned to its current commit, 9752b04, rather than the mutable tag — matches derivita-ui#247's pattern, since anyone with write access to shared-workflows could otherwise retarget v1 without a PR in this repo). ecosystems: npm_and_yarn (fetch-metadata's slug for npm/yarn — not dependabot.yml's npm value). Gated to dependabot[bot]-authored PRs only, trigger scoped to branches: [main] (the repo's default branch).
  • .github/CODEOWNERS — did not previously exist. Added /.github/ and /.kodiak.toml entries requiring review from @derivita/engineering.

Ecosystem chosen

npm_and_yarn only. This repo has both package-lock.json and yarn.lock at the root but fetch-metadata's npm_and_yarn slug covers both. No other ecosystem is in use.

CODEOWNERS team — flagged as a guess

gh api repos/derivita/protobuf-javascript/teams returned an empty list — no org team currently has push/admin access to this repo, so there's no clearly-fitting domain team to use. Falling back to @derivita/engineering per the rollout's guidance. A reviewer should confirm this is the right owner (or add a more specific team if one should have access to this repo).

What's NOT done — needs a human

  • Branch protection prerequisites were not checked or modified (requires org-admin access this task doesn't have). Before merging, please verify main's branch protection has:
    • "Dismiss stale pull request approvals when new commits are pushed"
    • "Require review from Code Owners"
    • The relevant required status checks (this repo's CI: build.yml, codeql.yml)
    • Kodiak's other prerequisites per #1716/#1670
  • Merge method assumption: the repo currently allows squash, merge-commit, and rebase merges (allow_squash_merge, allow_merge_commit, allow_rebase_merge all true), and I can't tell from the API alone whether main requires linear history. Defaulted to method = "squash" (matches every repo onboarded so far) — please verify this is compatible with main's actual merge-method restrictions, if any.
  • End-to-end test plan (not yet performed):
    • A real patch/minor Dependabot PR auto-merges via Kodiak
    • A real major-version Dependabot PR correctly gets skipped (no automerge/dependabot-automerge labels, stays for human review)

Links: derivita/infrastructure#1719, derivita/derivita-ui#247

Add .kodiak.toml, the dependabot-automerge-label caller workflow,
.github/dependabot.yml (npm), and CODEOWNERS protection for /.github/
and /.kodiak.toml, per derivita/infrastructure#1719.
@rivernate
rivernate requested a review from wbhumphrey August 14, 2026 18:25

@wbhumphrey wbhumphrey 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.

Config is well-built and the composition with the shared reusable workflow checks out — correct fetch-metadata slug, correct trigger types for the callee's label/revoke split, SHA pin matches the current v1 tag, real Kodiak bot login, allowed merge method.

Two blocking findings, both about enabling state rather than the diff: the CODEOWNERS rules are invalid as written and provide no gate, and main has neither branch protection nor rulesets — which makes the accepted-risk reasoning in .kodiak.toml false today. Details inline.

Comment thread .github/CODEOWNERS
# workflow file, which can run on its own PR with GITHUB_TOKEN write
# permissions — requires a human code-owner review to merge. Pairs with
# branch protection "Require review from Code Owners".
/.github/ @derivita/engineering

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

GitHub reports both owner lines in this file as invalid. gh api repos/derivita/protobuf-javascript/codeowners/errors?ref=nathan/kodiak-dependabot-automerge returns Unknown owner on lines 5 and 10: @derivita/engineering exists (13 members), but no org team has access granted to this repo, and CODEOWNERS requires the team be publicly visible with write access to resolve.

Effect: these two rules are inert. Even once "Require review from Code Owners" is enabled, /.github/ and /.kodiak.toml carry no owner requirement — so a new workflow file (which can run on its own PR with a write-scoped GITHUB_TOKEN) or an edit that weakens blocking_labels merges with no code-owner review, which is precisely what this file exists to prevent. It fails silently: GitHub surfaces the error in the CODEOWNERS editor but doesn't block anything.

The fix is a repo-settings change rather than a diff change — grant @derivita/engineering write access to this repo, then confirm the codeowners/errors endpoint comes back empty. Flagging it as blocking because the description raises this only as "is this the right team?", where the actual state is "the entry does nothing."

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Confirmed, current state matches: gh api repos/derivita/protobuf-javascript/codeowners/errors?ref=nathan/kodiak-dependabot-automerge still returns Unknown owner on lines 5 and 10, and gh api repos/derivita/protobuf-javascript/teams returns an empty list — no team currently has any access to this repo, so there's no better team to redirect these entries to.

This is a repo-settings/access grant (give @derivita/engineering write access to the repo), not a file change, so I'm leaving it for a human with the appropriate admin access rather than attempting it myself. Not resolving this thread.

Comment thread .kodiak.toml
# Triage/Write access could still self-apply it to their own PR for an
# unreviewed auto-approval. Accepted risk, matching derivita/infrastructure
# #1716: this repo's collaborators are trusted, and required status checks
# still gate every PR regardless of label.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This mitigation does not hold on this repo today. main has no branch protection and no rulesets — gh api repos/derivita/protobuf-javascript/branches/main/protection returns 404 Branch not protected, and .../rulesets returns []. There are no required status checks: build.yml and codeql.yml do run on Dependabot PRs, but nothing gates the merge on them.

Two consequences of merging as-is:

  • The self-applied-label risk documented just above loses its stated backstop. Both automerge and dependabot-automerge already exist as labels in this repo, so any Triage/Write collaborator can attach dependabot-automerge to their own PR, collect Kodiak's bot approval, add automerge, and squash-merge to main with no human review and no passing CI. The reusable workflow's semver and head-commit-identity checks never enter the picture, because it isn't a Dependabot PR.
  • The callee's own "repo settings each caller depends on" list names Dismiss stale pull request approvals when new commits are pushed as what makes its revoke job a backstop rather than the sole line of defence. Without it, Kodiak's approval survives a collaborator's push to a labeled Dependabot branch independently of revoke.

The description flags branch protection as unverified — this is the verification, and none of it is in place. Worth landing a ruleset on main (required build + CodeQL checks, code-owner review, dismiss-stale-approvals-on-push) before this merges, then either correcting or dropping the claim on these two lines.

Two things that did check out, for what it's worth: method = "squash" is allowed on this repo and nothing requires linear history, and derivita-kodiak[bot] (the callee's default kodiak-bot-login) is the real Kodiak login.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Confirmed, current state matches: gh api repos/derivita/protobuf-javascript/branches/main/protection still returns 404 Branch not protected, and gh api repos/derivita/protobuf-javascript/rulesets still returns []. No required status checks, no code-owner review requirement, no dismiss-stale-approvals-on-push.

This needs a ruleset/branch-protection change on main (required build + CodeQL checks, code-owner review, dismiss-stale-approvals-on-push) before the accepted-risk claim on these two lines is accurate. That's a branch-protection change, so I'm not attempting it — leaving this for a human to land the ruleset and then either confirm or amend the comment. Not resolving this thread.

Comment thread .github/dependabot.yml
@@ -0,0 +1,13 @@
version: 2
updates:
- package-ecosystem: npm

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non-blocking, and a scope call rather than a defect. With only npm configured, none of this repo's pinned GitHub Actions ever get update PRs — including the 9752b04 pin this PR introduces, plus actions/checkout, actions/setup-node, the CodeQL actions, and scorecard-action. This repo runs OpenSSF scorecard, which grades pinned-dependency freshness, so stale pins are a visible cost.

A github_actions entry composes safely with this design and needs no CODEOWNERS carve-out: the caller passes ecosystems: npm_and_yarn, so action bumps would get neither label and route to human review by construction. Whether that belongs in this PR or a follow-up is your call.

The two-lockfile question the description raises is already settled, incidentally — #4 and #5 each updated package-lock.json and yarn.lock together, so the single npm entry handles both.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Agreed this is sound and non-blocking. Since you've framed it as a scope call ("your call" whether it belongs in this PR or a follow-up) rather than a defect in what's here, I'm leaving that decision to a repo owner rather than making it unilaterally. Not resolving this thread so it stays visible for that call.

@wbhumphrey wbhumphrey 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.

Re-reviewed at the same commit — no code change since my last pass.

@rivernate independently re-ran both checks and confirms the state is unchanged: the code-owner entries still fail GitHub's CODEOWNERS validation, and main still has neither branch protection nor rulesets. One new data point from that pass strengthens the first finding rather than softening it — no org team holds any access to this repo at all, so there is no alternative team the entries could be redirected to. Both are access/settings changes outside this diff, and both remain blocking until they land.

The actions-ecosystem note stays non-blocking and is a scope call for a repo owner, as before. No further findings.

@mhjenkins mhjenkins left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This PR adds GitHub merge-governance config (CODEOWNERS, dependabot.yml, a Dependabot auto-merge labeling workflow, and .kodiak.toml) to auto-approve/merge Dependabot patch/minor npm bumps. Three prior human reviewers already found the safety claims don't hold today: the CODEOWNERS team has no repo access (rules are inert) and main has no branch protection/rulesets at all (the "required status checks" backstop claim is false as written) — both unresolved and blocking. Two independent AI reviewers (architecture + security) additionally converged on a structural gap not previously raised: the actual auto-merge gating logic lives entirely in an external repo (derivita/shared-workflows) that this repo's new CODEOWNERS can never cover, and its safety claims (grouped-dependency handling, head-commit-signature checks against a synchronize-triggered re-label path) are unverifiable from here — worth a maintainer's confirmation before relying on this for unattended merges.

@rivernate

Copy link
Copy Markdown
Member Author

Closing without merging. This repo is being excluded from the Kodiak Dependabot auto-merge rollout (derivita/infrastructure#1719) rather than pushed through — CI is broken on a pre-existing build bug unrelated to this diff (gulpfile.js:134 points closure-compiler at a nonexistent ../closure-library path, introduced in aeef7e7 and apparently never validated in CI), and #1 (the ES6/.d.ts support work) is still active here, so this isn't a dead repo where forcing onboarding makes sense right now. No Dependabot/Kodiak config will be added here going forward — happy to revisit if that changes.

@rivernate rivernate closed this Aug 19, 2026
Sign up for free to 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.

3 participants