Skip to content

chore: dependabot PR hygiene (PR template + auto-CHANGELOG workflow)#58

Merged
cmeans-claude-dev[bot] merged 4 commits into
mainfrom
chore/dependabot-pr-hygiene
Apr 26, 2026
Merged

chore: dependabot PR hygiene (PR template + auto-CHANGELOG workflow)#58
cmeans-claude-dev[bot] merged 4 commits into
mainfrom
chore/dependabot-pr-hygiene

Conversation

@cmeans-claude-dev
Copy link
Copy Markdown
Contributor

@cmeans-claude-dev cmeans-claude-dev Bot commented Apr 26, 2026

Status: design APPROVED — round 3 addresses round-2 findings

QA approved option C (PR template + auto-CHANGELOG workflow on pull_request_target with App-token auth) on round 2 — see review at #58 (comment) and awareness entry 9e1ed4f1-fea1-46aa-9393-155f23ed728e.

Round 2 → Round 3 deltas

  • F1-r2 (substantive)CHANGELOG.md entry was stale (described round-1 design with @v2 floating tag, github-actions[bot] guard, no App-token mention). Rewritten to reflect round-2 reality: App-token auth, dependency-group precedence, SHA-pinned @v2.5.0 / @v3.1.1, cmeans-claude-dev[bot] loop guard, required BOT_APP_ID / BOT_APP_PRIVATE_KEY secrets, idempotency on (#N) preserving hand-prepended human entries.
  • O2-r2 (observation)actions/checkout@v4 was the only third-party action still tag-pinned. Now SHA-pinned to 34e114876b0b11c390a56381ad16ebd13914f8d5 (v4.3.1). All three third-party actions on this pull_request_target workflow are now pinned by SHA.
  • O3-r2 (observation)## Test plan section restored below.

Summary

Closes the QA gap surfaced by PR #55 — the first Dependabot-authored PR on this repo. QA round-1 found two issues that Dependabot can't fix on its own: the auto-generated body had no ## QA checklist, and the auto-bump didn't add the CHANGELOG entry that CLAUDE.md § "Adding a CHANGELOG entry on every PR" requires unconditionally.

  • .github/PULL_REQUEST_TEMPLATE.md — auto-fills the body of new PRs with ## Summary / ## Test plan / ## CHANGELOG sections plus the standard test-plan checklist (pytest, ruff check, ruff format --check, mypy). Helps human contributors only — Dependabot bypasses PR templates.
  • .github/workflows/dependabot-changelog.yml — covers Dependabot. Runs on pull_request_target (so it has write perms; Dependabot's pull_request event runs read-only). Filters to dependabot[bot]. Mints an App token (so pushes re-fire required CI checks), fetches Dependabot metadata, prepends a ### Changed entry under ## Unreleased, pushes a follow-up commit attributed to cmeans-claude-dev[bot].

Design notes

  • Why pull_request_target — Dependabot PRs run with restricted token perms; pull_request workflows can't push back. pull_request_target runs in the base branch's context with full perms.
  • Why App token, not GITHUB_TOKEN — anti-loop policy: pushes via secrets.GITHUB_TOKEN do not trigger downstream pull_request workflows. Required CI checks would never fire on the bot's commit, leaving the PR blocked at merge under the main-branch ruleset. App-token pushes do trigger downstream workflows.
  • Loop guard / idempotency — skip if last commit author is cmeans-claude-dev[bot] or if (#N) already in CHANGELOG.md. Handles @dependabot recreate / rebase and preserves hand-prepended human entries (e.g., the CVE-callout style used on PR Bump the uv group across 1 directory with 4 updates #55).
  • SHA pinsactions/create-github-app-token@v3.1.1, dependabot/fetch-metadata@v2.5.0, actions/checkout@v4.3.1. Defense-in-depth on the highest-privilege workflow trigger in the repo.
  • Manual override — a human can prepend a hand-written CHANGELOG entry to a Dependabot PR. The idempotency check on (#N) ensures the workflow won't double-write.

Operator prereq (one-time, already configured)

Two repo secrets at Settings → Secrets and variables → Actions:

  • BOT_APP_ID = 3223881 (the cmeans-claude-dev App ID)
  • BOT_APP_PRIVATE_KEY = full PEM contents of the App's private key

Configured by maintainer earlier in this PR cycle. Without these the workflow fails fast on the create-github-app-token step.

Test plan

  • python -c "import yaml; yaml.safe_load(open('.github/workflows/dependabot-changelog.yml'))" — YAML parses on 851a44f
  • All three third-party actions SHA-pinned: actions/checkout@v4.3.1, actions/create-github-app-token@v3.1.1, dependabot/fetch-metadata@v2.5.0
  • .github/PULL_REQUEST_TEMPLATE.md present and includes ## Summary / ## Test plan / ## CHANGELOG sections
  • CHANGELOG entry under ## Unreleased### Added references this PR (chore: dependabot PR hygiene (PR template + auto-CHANGELOG workflow) #58) and accurately reflects the round-3 implementation (App-token auth, group-name precedence, idempotency)
  • (Operator) BOT_APP_ID and BOT_APP_PRIVATE_KEY repo secrets configured (confirmed by maintainer earlier in this PR cycle)
  • After merge: wait for Dependabot to open its next PR (or @dependabot recreate on an existing one) and confirm:
    • The dependabot-changelog workflow runs to completion
    • A second commit lands on the Dependabot branch authored by cmeans-claude-dev[bot] (chore(changelog): record dep bumps from #N)
    • Required CI checks (lint, typecheck, test (3.11/3.12/3.13), version-sync) re-run on the bot's HEAD SHA — confirms App-token push triggers downstream workflows
    • The CHANGELOG line uses the Dependabot group name (e.g., "python" or "github-actions") not the ecosystem identifier
    • Re-running on the same PR (@dependabot rebase) does NOT add a second entry (idempotency)

Closes the QA gap surfaced by PR #55 (the first Dependabot-authored
PR on this repo): the Dependabot body had no `## QA` section and the
auto-bump didn't produce the unconditional CHANGELOG entry that
CLAUDE.md § "Adding a CHANGELOG entry on every PR" requires.

Two pieces:

1. `.github/PULL_REQUEST_TEMPLATE.md` — neutral scaffold with
   `## Summary` / `## Test plan` / `## CHANGELOG` sections plus the
   project-standard test-plan checklist (`pytest`, `ruff check`,
   `ruff format --check`, `mypy`). GitHub auto-fills this on every
   human-authored PR open. Dependabot bypasses the template (it
   supplies its own body), so the next piece picks up the slack for
   bot PRs.

2. `.github/workflows/dependabot-changelog.yml` — runs on
   `pull_request_target` (so it has write perms; Dependabot's
   `pull_request` event runs read-only). Filters to
   `github.event.pull_request.user.login == 'dependabot[bot]'`.
   Steps:
     - Checkout PR head ref.
     - Loop guard: skip if last commit author is
       `github-actions[bot]` OR `(#<PR>)` already present in
       CHANGELOG.md.
     - `dependabot/fetch-metadata@v2` to get the bump list.
     - Inline Python: parse `updated-dependencies-json`, compose
       `- **Bump <ecosystem> group: pkg vX→vY, ...** (#N)`, prepend
       under `## Unreleased` → `### Changed` (creating the section
       if missing).
     - Commit as `github-actions[bot]` and push to the Dependabot
       branch.

Why `pull_request_target` over `pull_request`: Dependabot PRs run
with restricted token permissions; `pull_request` workflows on a
Dependabot PR cannot push back. `pull_request_target` runs in the
base branch's context with full perms, which is what we need to
push a follow-up commit. Standard pattern for Dependabot
augmentation workflows.

Why a Python script inline: the CHANGELOG insertion needs to find
`## Unreleased` and the `### Changed` subsection within it (without
crossing into the next `## ` release heading) and create the
subsection if absent. That's awkward in awk/sed; Python keeps it
readable.

Verified against this repo's CHANGELOG.md by smoke-testing the
insertion logic with a synthetic deps payload — entry lands at the
correct line under `## Unreleased` → `### Changed`, immediately
after the section header's blank-line separator.
@cmeans-claude-dev cmeans-claude-dev Bot added the Ready for QA Dev work complete — QA can begin review label Apr 26, 2026
@github-actions github-actions Bot added the Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA label Apr 26, 2026
@github-actions github-actions Bot added Ready for QA Dev work complete — QA can begin review and removed Ready for QA Dev work complete — QA can begin review Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA labels Apr 26, 2026
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@cmeans cmeans added the QA Active QA is actively reviewing; Dev should not push changes label Apr 26, 2026
@github-actions github-actions Bot removed the Ready for QA Dev work complete — QA can begin review label Apr 26, 2026
Copy link
Copy Markdown
Owner

@cmeans cmeans left a comment

Choose a reason for hiding this comment

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

QA Round 1 — PR #58

Solid follow-up on #55. The workflow is a more durable answer than the PR template alone, since Dependabot bypasses templates. One blocker found and a few smaller items.

Verification on 2c8729f

Check Result
python3 -c "import yaml; yaml.safe_load(open('.github/workflows/dependabot-changelog.yml'))" parses
.github/PULL_REQUEST_TEMPLATE.md present (33 lines)
uv run pytest 499 passed, 94 deselected, 96.04% coverage
uv run ruff check src/ tests/ scripts/ clean
uv run ruff format --check 69 files clean
uv run mypy src/ scripts/ clean (29 files)
Remote CI rollup green (vdsm in progress at start, since completed)
Algorithm dry-run vs actual CHANGELOG.md (existing ### Changed) inserts at the right line, under existing ### Changed block
Algorithm dry-run, ## Unreleased exists but no ### Changed creates ### Changed block immediately after ## Unreleased, above existing ### Added
Idempotency: grep -qE "\(#${PR_NUMBER}\)" CHANGELOG.md works against PR #58's own entry — re-running would skip
Loop guard: git log -1 --pretty=%an against github-actions[bot] logic correct

Findings

1. [substantive — blocker] Bot push via GITHUB_TOKEN won't re-trigger required CI checks; Dependabot PRs become un-mergeable after the workflow runs.

The repo's main branch ruleset (gh api repos/cmeans/mcp-synology/rulesets/14717573) is enforcing with strict_required_status_checks_policy: true and these required contexts:

  • lint
  • typecheck
  • test (3.11), test (3.12), test (3.13)
  • version-sync

Sequence of events when this workflow runs on a Dependabot PR:

  1. Dependabot opens PR — fires pull_request → CI runs, attaches checks to SHA1
  2. dependabot-changelog fires (pull_request_target) → pushes chore(changelog): record dep bumps from #N as SHA2 using secrets.GITHUB_TOKEN
  3. PR HEAD is now SHA2. Per GitHub's anti-loop policy, GITHUB_TOKEN-authored pushes do not trigger pull_request workflows. So ci.yml does not re-run on SHA2
  4. Required checks are absent on SHA2 → PR is blocked at the merge step

The PR body's "After merge" test plan acknowledges the second commit will land but does not flag the merge-blocking side effect. Three ways to fix:

  • A. (recommended) Push from a PAT or GitHub App token. The repo already has a GitHub App configured for cmeans-claude-dev[bot]. Replacing secrets.GITHUB_TOKEN in actions/checkout@v4 and the implicit push remote with an App-token secret causes the resulting push to fire pull_request workflows normally. Standard pattern for Dependabot augmentation workflows.
  • B. Don't push back. Have the workflow post a PR comment titled "Suggested ## Unreleased entry" with the composed line; the maintainer (or QA) pastes it during review. Loses automation but avoids the merge-block entirely.
  • C. Document and re-trigger manually. Note in CLAUDE.md that Dependabot PRs need @dependabot recreate (or an empty commit) after the bot's CHANGELOG commit to re-fire CI. Operationally fragile.

I'd take A.

2. [substantive] Test plan checkboxes are all unchecked. The PR body's ## Test plan lists four pre-merge checkboxes (YAML parse, template visible, CHANGELOG entry under ### Added, post-merge bot behavior). Per project convention (every step ticked immediately after running), these should reflect what's been verified. The first three are verifiable now and pass; the fourth is genuinely post-merge.

Observations

3. [observation] package-ecosystem is the ecosystem identifier (pip, github-actions, ...), not the named group from dependabot.yml. PR #55's title was "Bump the uv group across 1 directory with 4 updates" — "uv" is the group name configured in dependabot.yml (per #57). The auto-entry would render as "Bump pip group: pytest 9.0.2→9.0.3, ..." which conflicts with the PR title. dependabot/fetch-metadata@v2 exposes dependency-group as a separate output — using it (with package-ecosystem as fallback) would keep the entry's group name aligned with the PR title.

4. [observation] No SHA pin on dependabot/fetch-metadata@v2. Repo convention is tag pinning across all workflows (e.g., actions/checkout@v4, astral-sh/setup-uv@v5), so this is internally consistent. But pull_request_target with contents: write is the highest-privilege workflow trigger in this repo — defense-in-depth would suggest SHA pinning the third-party action specifically. Optional, not blocking.

5. [observation] PR #58's CHANGELOG entry adds a second ### Added block to ## Unreleased. The existing ## Unreleased already had ### Added lower in the file (entries for #27, #24); the new entry creates another block at the top instead of consolidating. This matches the project's de-facto pre-existing pattern (current ## Unreleased has multiple ### Fixed and ### Changed blocks scattered through, from PRs #16, #17, #19, #21, #22, #23, #24, #26, #29, #30, #53), so the workflow itself isn't introducing a new problem — but the next release-prep PR should consolidate the duplicated subsections. Worth noting; not blocking this PR.

Verdict

QA Failed pending finding #1 (the merge-block blocker) and finding #2 (test plan ticks). Observations 3–5 are signoff-blockers per project convention but are smaller-surface fixes once #1 is settled.

@cmeans
Copy link
Copy Markdown
Owner

cmeans commented Apr 26, 2026

QA audit — round 1

Branch / SHA: `chore/dependabot-pr-hygiene` @ `2c8729f`

Commands run locally (reproducible):

```
git checkout chore/dependabot-pr-hygiene
uv sync --extra dev
uv run pytest # 499 passed, 94 deselected, 96.04% (gate 95%)
uv run ruff check src/ tests/ scripts/ # clean
uv run ruff format --check src/ tests/ scripts/ # 69 files clean
uv run mypy src/ scripts/ # clean (29 files)
python3 -c "import yaml; yaml.safe_load(open('.github/workflows/dependabot-changelog.yml'))" # parses
```

Workflow algorithm dry-run (against actual CHANGELOG.md):

  • `## Unreleased` exists, `### Changed` exists → entry inserts at line 15 inside the existing `### Changed` block (top of that section). Correct.
  • `## Unreleased` exists, `### Changed` absent → creates new `### Changed` immediately after `## Unreleased`, above any existing `### Added` / `### Fixed`. Correct.
  • Idempotency: re-running with `PR_NUMBER=58` would match the existing `(chore: dependabot PR hygiene (PR template + auto-CHANGELOG workflow) #58)` in the file and skip via `grep -qE`. Correct.

Branch protection check: `gh api repos/cmeans/mcp-synology/rulesets/14717573` shows `strict_required_status_checks_policy: true` with required contexts: `lint`, `typecheck`, `test (3.11)`, `test (3.12)`, `test (3.13)`, `version-sync`. This is what makes finding #1 a blocker — none of these will re-fire on the bot's CHANGELOG commit.

Outcome: QA Failed — see review comment for findings.

@cmeans cmeans added QA Failed QA found issues — needs dev attention and removed QA Active QA is actively reviewing; Dev should not push changes labels Apr 26, 2026
Addresses QA round-1 findings on PR #58.

Finding 1 (BLOCKER) — `secrets.GITHUB_TOKEN`-authored pushes don't
trigger downstream `pull_request` workflows (GitHub anti-loop
policy). On a Dependabot PR, the workflow's CHANGELOG commit would
land on the branch but `lint` / `typecheck` / `test (3.11/3.12/3.13)`
/ `version-sync` would not re-run on the new HEAD SHA. The
main-branch ruleset requires those checks → PR blocks at merge.

Fix: mint a token from the `cmeans-claude-dev` GitHub App via
`actions/create-github-app-token@v3.1.1` and use it for both
`actions/checkout` and the push. App-token pushes fire
`pull_request` workflows normally. Adds two repo-secrets prereqs:
- `BOT_APP_ID` (numeric App ID)
- `BOT_APP_PRIVATE_KEY` (PEM contents)
PR body documents the operator action.

Loop guard updated to match `cmeans-claude-dev[bot]` instead of
`github-actions[bot]` since the bot identity changes with the
auth swap.

Commit author/committer aligns to the bot via the numeric-user-id
noreply format (`<BOT_USER_ID>+cmeans-claude-dev[bot]@users.noreply.
github.com`) per the convention captured in
`claude-dev/github-app/config` — the comment there explains why
APP_ID does NOT work in this position (commits fail to resolve
back to the bot user, breaking `require_last_push_approval`
rulesets).

Observation 3 — `package-ecosystem` is the ecosystem identifier
(`pip`, `github-actions`), not the named group from
`dependabot.yml`. PR #55's title used "uv" (the configured group
name); the v1 workflow would have rendered "pip group" instead.

Fix: read `steps.meta.outputs.dependency-group` first, fall back
to `package-ecosystem`. Output now matches the PR title.

Observation 4 — SHA-pin third-party actions on this
high-privilege workflow:
- `actions/create-github-app-token@1b10c78c…` v3.1.1
- `dependabot/fetch-metadata@21025c705c…` v2.5.0
@github-actions github-actions Bot added Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA Ready for QA Dev work complete — QA can begin review and removed QA Failed QA found issues — needs dev attention Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA labels Apr 26, 2026
@cmeans cmeans added the QA Active QA is actively reviewing; Dev should not push changes label Apr 26, 2026
@github-actions github-actions Bot removed the Ready for QA Dev work complete — QA can begin review label Apr 26, 2026
Copy link
Copy Markdown
Owner

@cmeans cmeans left a comment

Choose a reason for hiding this comment

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

QA Round 2 — PR #58

Design verdict (responding to the awareness proposal)

(a) Option C — auto-CHANGELOG workflow + PR template — is the right choice. Approved.

  • Option A (manual) defeats automation and predictably trips the same finding on every Dependabot PR.
  • Option B (carve-out) softens an invariant CLAUDE.md deliberately restored after #13/#15/#16. Loses CVE searchability at release time.
  • Option C scales to grouped + non-grouped Dependabot PRs and keeps the rule unconditional. Right call.

(b) App-token auth design is acceptable. Independently verified:

Check Result
actions/create-github-app-token@1b10c78c… → upstream tag v3.1.1 matches (gh api .../git/refs/tags/v3.1.1)
dependabot/fetch-metadata@21025c70… → upstream tag v2.5.0 matches
Bot user id 272174644cmeans-claude-dev[bot] matches (gh api users/cmeans-claude-dev%5Bbot%5D returns id:272174644 type:Bot)
Workflow permissions: scoped to contents: read + pull-requests: read confirmed; intrinsic GITHUB_TOKEN no longer needs write — App token does the write path
App-token-authored pushes trigger downstream pull_request workflows per GitHub docs, yes — fixes the round-1 merge-block

App-installation permissions on this repo aren't visible from a non-App credential (gh api repos/.../installation returns 401 without a JWT) — that's an operator-side responsibility to keep scoped to the minimum the workflow needs (contents: write, pull-requests: read). Recommend the operator confirm via the App's settings page, but I can't gate signoff on something I can't see.

(c) Round-2 implementation is correct on the design pointsdependency-group preferred with package-ecosystem fallback (lines 102-104), loop guard switched to cmeans-claude-dev[bot] (line 65), idempotency (#N) regex (line 68), SHA pins on the two new third-party actions, reduced workflow permissions.

Verification on 8f1f0ae

Check Result
uv run pytest 499 passed, 94 deselected, 96.04% coverage
uv run ruff check src/ tests/ scripts/ clean
uv run ruff format --check 69 files clean
uv run mypy src/ scripts/ clean (29 source files)
python3 -c "import yaml; yaml.safe_load(...)" parses
Remote CI rollup all green

Round-1 findings explicitly closed:

  • F1 (merge-block via GITHUB_TOKEN push) — closed by App-token swap.
  • O3 (package-ecosystem vs named group) — closed by dependency-group preference.
  • O4 (SHA pins) — closed for the two new third-party actions.

Round-2 findings

1. [substantive — doc drift] The PR's own CHANGELOG.md entry is stale relative to round-2.

Current entry (CHANGELOG.md:5) still describes the round-1 design:

calls dependabot/fetch-metadata@v2 to enumerate the bump set ... Loop guard skips when the latest commit author is github-actions[bot]

Round-2 reality:

  • dependabot/fetch-metadata@v2.5.0 (SHA-pinned) — version bump
  • Loop guard checks cmeans-claude-dev[bot] — different identity
  • dependency-group preferred over package-ecosystem — new logic
  • App-token mechanism — the entire round-2 design pivot, not mentioned

A reader at release time will see a description that no longer matches the workflow on disk. Either rewrite the entry to reflect the actual implementation (one paragraph mentioning App-token, cmeans-claude-dev[bot] guard, @v2.5.0, dependency-group fallback) or shorten the entry to a non-implementation-specific summary that won't drift.

2. [observation] actions/checkout@v4 is still tag-pinned, not SHA-pinned.

Round 2 SHA-pinned the two new actions (create-github-app-token, fetch-metadata) but left the existing actions/checkout@v4 on a floating major-version tag. For a pull_request_target workflow that also uses an App token, defense-in-depth suggests pinning all three. Consistent with the rest of the repo, so optional. If pinned, prefer the latest v4.x SHA from actions/checkout's release page.

3. [observation] PR body has no test plan section.

The round-1 body had four test-plan checkboxes; round 2 replaced the body with the design-review prompt. Dev should restore the ## Test plan section (YAML parse, template visible, CHANGELOG references PR — these are checkable today; first-Dependabot-bot-run is post-merge) so QA has explicit pre-merge ticks per project convention. The design-review prompt can stay above the test plan, not replace it.

Process note

Acknowledged Dev pushed round-2 code before getting design sign-off and is treating the branch as reference-only pending approval. That's fine — the design IS approved (see top of this comment). Dev can iterate finding 1 (and optionally 2 + 3) on the same branch; no need to close and re-open.

Verdict

QA Failed pending finding 1 (CHANGELOG drift), with observations 2 and 3 to address before signoff per project convention. Design itself is approved — Dev can proceed with iterations on this branch.

@cmeans
Copy link
Copy Markdown
Owner

cmeans commented Apr 26, 2026

QA audit — round 2

Branch / SHA: `chore/dependabot-pr-hygiene` @ `8f1f0ae`

Independent SHA verification:
```
gh api repos/actions/create-github-app-token/git/refs/tags/v3.1.1

→ 1b10c78c7865c340bc4f6099eb2f838309f1e8c3 (matches workflow line 45)

gh api repos/dependabot/fetch-metadata/git/refs/tags/v2.5.0

→ 21025c705c08248db411dc16f3619e6b5f9ea21a (matches workflow line 78)

gh api 'users/cmeans-claude-dev%5Bbot%5D'

→ id: 272174644 login: cmeans-claude-dev[bot] type: Bot (matches workflow line 185)

```

Local stack:
```
uv sync --extra dev
uv run pytest # 499 passed, 94 deselected, 96.04% coverage
uv run ruff check src/ tests/ scripts/ # clean
uv run ruff format --check src/ tests/ scripts/ # 69 files clean
uv run mypy src/ scripts/ # clean (29 source files)
python3 -c "import yaml; yaml.safe_load(open('.github/workflows/dependabot-changelog.yml'))" # parses
```

Round-1 findings closed in round 2:

  • F1 (GITHUB_TOKEN merge-block): closed by App-token swap.
  • O3 (named-group vs ecosystem): closed by `dependency-group` preference w/ `package-ecosystem` fallback.
  • O4 (third-party SHA pinning): closed for the two newly-introduced actions.

Round-2 finding:

  • F1-r2 substantive: `CHANGELOG.md` entry on this PR is stale — describes round-1 design (`@v2`, `github-actions[bot]` guard, no App-token mention) instead of round-2 reality.

Outcome: QA Failed — design approved, finding 1 + observations 2/3 to address. See review comment for details, and awareness entry `9e1ed4f1-fea1-46aa-9393-155f23ed728e` for the design-side response.

@cmeans cmeans added QA Failed QA found issues — needs dev attention and removed QA Active QA is actively reviewing; Dev should not push changes labels Apr 26, 2026
F1-r2 (substantive): CHANGELOG entry rewritten to reflect the
round-2 implementation (App-token auth via
actions/create-github-app-token@v3.1.1, dependency-group with
package-ecosystem fallback, dependabot/fetch-metadata@v2.5.0,
loop-guard against cmeans-claude-dev[bot], idempotency on (#N),
required BOT_APP_ID + BOT_APP_PRIVATE_KEY secrets, preservation
of hand-prepended human CHANGELOG entries). Previous wording
described round 1 (@v2 floating tag, github-actions[bot] guard,
no App-token mention) and would have shipped an inaccurate
description into release notes.

O2-r2 (observation): SHA-pin actions/checkout to
34e114876b0b11c390a56381ad16ebd13914f8d5 (v4.3.1) for consistency
with the two third-party actions already SHA-pinned in this
workflow. All three third-party actions on this high-privilege
pull_request_target trigger are now pinned by SHA.

PR body restoration of the ## Test plan section (O3-r2) is
applied via gh pr edit, not via this commit.
@github-actions github-actions Bot added Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA and removed QA Failed QA found issues — needs dev attention Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA labels Apr 26, 2026
@github-actions github-actions Bot added the Ready for QA Dev work complete — QA can begin review label Apr 26, 2026
@cmeans cmeans added the QA Active QA is actively reviewing; Dev should not push changes label Apr 26, 2026
@github-actions github-actions Bot removed the Ready for QA Dev work complete — QA can begin review label Apr 26, 2026
Copy link
Copy Markdown
Owner

@cmeans cmeans left a comment

Choose a reason for hiding this comment

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

QA Round 3 — PR #58

Round-2 findings — verified closed on 851a44f

Finding Status Evidence
F1-r2 (CHANGELOG drift) ✅ closed Entry rewritten to describe round-3 reality: App-token mechanism, SHA-pinned @v3.1.1 / @v2.5.0 / @v4.3.1, dependency-group precedence with package-ecosystem fallback, cmeans-claude-dev[bot] loop guard, (#N) idempotency that preserves hand-prepended human entries (CVE-callout style on PR #55), BOT_APP_ID / BOT_APP_PRIVATE_KEY secret names. Matches the workflow on disk.
O2-r2 (actions/checkout not SHA-pinned) ✅ closed Now actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1. Independently verified: gh api repos/actions/checkout/git/refs/tags/v4.3.1 returns the same SHA. All three third-party actions on this pull_request_target workflow are now SHA-pinned.
O3-r2 (test plan section) ✅ closed PR body's ## Test plan is back, with 4 pre-merge-verifiable items ticked and 2 properly unticked (operator-secrets confirmation + post-merge first-Dependabot-bot-run).

Independent verification on 851a44f

Check Result
actions/create-github-app-token@1b10c78c… ↔ upstream v3.1.1 matches
actions/checkout@34e114876b… ↔ upstream v4.3.1 matches
dependabot/fetch-metadata@21025c70… ↔ upstream v2.5.0 matches
Idempotency guard against PR_NUMBER=58 would skip — (#58) present in CHANGELOG.md
Algorithm dry-run with DEPENDENCY_GROUP=python, ECOSYSTEM=pip composes "Bump python group: ..." — group preferred ✓
Insert position vs current ## Unreleased / ### Changed line 14 (under existing ### Changed) ✓
uv run pytest 499 passed, 94 deselected, 96.04% coverage
uv run ruff check src/ tests/ scripts/ clean
uv run ruff format --check 69 files clean
uv run mypy src/ scripts/ clean (29 source files)
python3 -c "import yaml; yaml.safe_load(...)" parses
python3 scripts/sync-server-json.py --check in sync (0.5.0)
Required CI checks all pass: lint, typecheck, test (3.11/3.12/3.13), version-sync
Optional CI validate-server-json pass; vdsm integration tests was in progress, not required

Verdict

Ready for QA Signoff. Round-2 findings F1-r2 / O2-r2 / O3-r2 all addressed. Round-1 findings remain closed. Implementation is internally consistent with the design proposal and the PR's own CHANGELOG entry. No new findings.

Two reminders for the maintainer at merge time:

  1. Operator-confirmable post-merge: test-plan items 5 and 6 (operator secrets actually wired up + first real Dependabot bot run lands a CHANGELOG commit and re-fires required CI) can only be verified once a Dependabot PR opens after this merges. The workflow will fail fast on the create-github-app-token step if BOT_APP_ID or BOT_APP_PRIVATE_KEY are missing.
  2. CHANGELOG ### Added duplication still present (round-1 O5 carryover): ## Unreleased has ### Added blocks at lines 5 and 19. Pre-existing project pattern, not introduced or worsened by this PR. Worth a single consolidate pass at the next release-prep PR; not blocking now.

QA Approved is the maintainer's decision per the project label-ownership rule.

@cmeans
Copy link
Copy Markdown
Owner

cmeans commented Apr 26, 2026

QA audit — round 3

Branch / SHA: `chore/dependabot-pr-hygiene` @ `851a44f`

SHA verification (all three third-party actions):
```
gh api repos/actions/create-github-app-token/git/refs/tags/v3.1.1 # → 1b10c78c7865c340bc4f6099eb2f838309f1e8c3 ✓
gh api repos/actions/checkout/git/refs/tags/v4.3.1 # → 34e114876b0b11c390a56381ad16ebd13914f8d5 ✓
gh api repos/dependabot/fetch-metadata/git/refs/tags/v2.5.0 # → 21025c705c08248db411dc16f3619e6b5f9ea21a ✓
```

Algorithm dry-runs against round-3 `CHANGELOG.md`:

```

Idempotency:

PR_NUMBER=58 → guard: would skip (#58 present)

Composition with named group + ecosystem fallback:

DEPENDENCY_GROUP=python ECOSYSTEM=pip → "Bump python group: foo 1.0→1.1" (group preferred ✓)

Insertion target:

existing ### Changed at file line 13 → entry would land at line 14 (top of section ✓)
```

Local stack:
```
uv sync --extra dev
uv run pytest # 499 passed, 94 deselected, 96.04% coverage
uv run ruff check src/ tests/ scripts/ # clean
uv run ruff format --check src/ tests/ scripts/ # 69 files clean
uv run mypy src/ scripts/ # clean (29 source files)
python3 -c "import yaml; yaml.safe_load(open('.github/workflows/dependabot-changelog.yml'))" # parses
python3 scripts/sync-server-json.py --check # in sync (0.5.0)
```

Round-2 findings closed: F1-r2 (CHANGELOG drift), O2-r2 (`actions/checkout` SHA pin), O3-r2 (test plan section). Round-1 findings remain closed. No new findings.

Outcome: Ready for QA Signoff. `QA Approved` remains the maintainer's call per project label-ownership.

@cmeans cmeans added Ready for QA Signoff QA passed — ready for maintainer final review and merge and removed QA Active QA is actively reviewing; Dev should not push changes labels Apr 26, 2026
Copy link
Copy Markdown
Owner

@cmeans cmeans left a comment

Choose a reason for hiding this comment

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

LGTM

@cmeans cmeans added QA Approved Manual QA testing completed and passed and removed Ready for QA Signoff QA passed — ready for maintainer final review and merge labels Apr 26, 2026
@cmeans-claude-dev cmeans-claude-dev Bot merged commit 1b16552 into main Apr 26, 2026
32 of 33 checks passed
@cmeans-claude-dev cmeans-claude-dev Bot deleted the chore/dependabot-pr-hygiene branch April 26, 2026 20:08
cmeans-claude-dev Bot added a commit that referenced this pull request Apr 26, 2026
Adds the three standard GitHub community-health files. Existing
.github/ISSUE_TEMPLATE/ and .github/FUNDING.yml already on the repo;
this fills in the remaining slots.

CONTRIBUTING.md
- Inbound = outbound Apache-2.0 § 5 licensing rule.
- No-bounties / no-paid-contributions policy.
- Dev workflow: uv sync --extra dev, pytest, ruff check / format,
  mypy --strict — commands include scripts/ to match CI exactly.
- Test mirror convention (src/.../listing.py ↔
  tests/.../test_listing.py) and integration-test pointer.
- QA-label flow: Awaiting CI → Ready for QA → QA Approved.
- PR-body example aligned with .github/PULL_REQUEST_TEMPLATE.md
  (same Test plan checklist, same `## CHANGELOG` wording, same
  tests/modules/<area>/test_<file>.py placeholder).

CODE_OF_CONDUCT.md
- Adopts Contributor Covenant 2.1 by reference.
- Routes private reports through GitHub Private Security
  Advisories with a Conduct title prefix.

SECURITY.md
- Declares 0.5.x supported.
- In-scope items grounded in real code: auth/keyring credential
  handling in core/auth.py; session-error retry in core/client.py
  (_SESSION_ERROR_CODES) + core/errors.py (SessionExpiredError,
  error_from_code()); File Station path validation; comma/
  backslash escaping in core/client.py (escape_path_param);
  permission-tier gating; async background-task cleanup
  (try/finally on Search, DirSize, CopyMove, Delete);
  config-loading invariants; publish-registry workflow integrity;
  GHA injection patterns.
- Out-of-scope items: upstream dep CVEs, DSM itself (→ Synology
  PSIRT), compromised-host scenarios, MCP-host bugs.

Also rolls forward .github/PULL_REQUEST_TEMPLATE.md (which landed
via #58 with the same scripts/-missing drift round-1 QA caught in
CONTRIBUTING.md). After merge, the PR template, the
CONTRIBUTING.md PR-body example, and CI all use the same commands
and placeholders.

Pattern ported from cmeans/pypi-winnow-downloads#20.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cmeans-claude-dev Bot added a commit that referenced this pull request Apr 26, 2026
After PR #58 merged, .github/workflows/dependabot-changelog.yml
joined the workflow set Dependabot watches via the github-actions
ecosystem. Updated the comment so the listed workflows match the
actual contents of .github/workflows/.

No functional change — Dependabot already tracks the new workflow
because it's matched by the ecosystem-wide directory glob.
cmeans-claude-dev Bot added a commit that referenced this pull request Apr 26, 2026
Adds .github/dependabot.yml so Dependabot can open weekly version-
update PRs for outdated dependencies.

Two ecosystems tracked:
- pip: runtime + [dev] + [vdsm] extras from pyproject.toml; uv.lock
  at the repo root keeps Dependabot's lockfile bumps reproducible.
- github-actions: every workflow file under .github/workflows/ (ci,
  vdsm, publish, test-publish, qa-gate, pr-labels, pr-labels-ci,
  dependabot-changelog) plus the local composite action at
  .github/actions/install-mcp-publisher.

Schedule weekly Monday 06:00 America/Chicago, single grouped PR per
ecosystem. Labels `dependencies` + ecosystem-tag (`python` /
`github-actions`) — all three labels created on the repo so
Dependabot doesn't silently skip them.

Commit subjects render as `chore(deps): bump <pkg>` via
`prefix: "chore"` + `include: "scope"` (Dependabot auto-appends the
(deps) scope; specifying chore(deps) explicitly in the prefix would
double it to chore(deps)(deps), which is the bug we caught in QA
round 3 — same misconfig is still live on cmeans/pypi-winnow-downloads
and worth fixing there as a follow-up).

No `docker` ecosystem because mcp-synology has no Dockerfile.

Repo-side switches (Dependabot alerts, security updates, secret
scanning, private vulnerability reporting) are already on; this PR
fills in the version-update config that the GitHub-side switches
don't cover. Once merged, Dependabot version-update PRs benefit
from the auto-CHANGELOG workflow shipped in #58.

Pattern ported from cmeans/pypi-winnow-downloads#21 with the
doubled-scope commit-prefix bug fixed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cmeans-claude-dev Bot pushed a commit that referenced this pull request Apr 26, 2026
…equests (#55)

First Dependabot-authored PR on this repo. Updates the uv group of
Python deps (pyproject.toml + uv.lock) for security pickups and
routine bumps:

- pytest 9.0.2 → 9.0.3 — direct dev dep. CVE-2025-71176 (insecure
  tmpdir).
- cryptography 46.0.5 → 46.0.7 — transitive (pyjwt, secretstorage).
  CVE-2026-39892 (buffer overflow); 46.0.7 wheels also ship
  OpenSSL 3.5.6.
- python-multipart 0.0.22 → 0.0.26 — transitive (mcp). Multipart
  parser hardening.
- requests 2.32.5 → 2.33.0 — transitive (docker, vdsm extra only).
  Drops Python 3.9 — irrelevant; project requires-python = ">=3.11".

No pyproject.toml constraints were widened — the diff is uv.lock-only.
499 unit tests pass at 96.04% coverage on the bumped lockfile.

CHANGELOG entry was added by hand on this branch (commit 670cd28)
because the PR predates #58, the auto-CHANGELOG workflow that will
take care of this on every future Dependabot PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

QA Approved Manual QA testing completed and passed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants