Skip to content

fix(ci): restore actions/checkout + configurationFile to renovate workflow#6

Merged
njhensley merged 1 commit into
mainfrom
ci/renovate-restore-checkout-and-config
May 4, 2026
Merged

fix(ci): restore actions/checkout + configurationFile to renovate workflow#6
njhensley merged 1 commit into
mainfrom
ci/renovate-restore-checkout-and-config

Conversation

@njhensley

Copy link
Copy Markdown
Owner

Summary

Reverts toward NVIDIA/gpu-operator's known-working pattern after the persistent Repository has changed during renovation aborts that PR #5 (drop checkout) did not fix.

Motivation / Context

After PR #4 merged, every Renovate dispatch ended the same way: branch creation succeeded, then 0.3–0.5s later Renovate aborted with repository-changed. main was verified unchanged during the run window via the GitHub commits API, so this was Renovate misdetecting state, not an actual race with another commit.

A diff against gpu-operator's working renovate.yaml (which we modeled on originally) surfaced three drift points:

Aspect gpu-operator Ours (before this PR)
actions/checkout YES NO (removed in #5)
configurationFile: .github/renovate.json NOT PASSED (removed in #1 to dodge customManager doubling)
RENOVATE_DRY_RUN env not set '' on every run

This PR restores all three to the gpu-operator shape:

  1. Re-add actions/checkout. The action clones internally but the host-side git state planted by checkout keeps Renovate's pre/post hooks consistent. PR fix(ci): drop actions/checkout from renovate workflow #5's removal didn't fix the abort, ruling it out as the cause but suggesting it shouldn't have been removed either.
  2. Re-pass configurationFile: .github/renovate.json5. The customManager-doubling concern that originally led to dropping this came from RENOVATE_PLATFORM=local dry-runs; in production (platform=github), Renovate dedupes by manager identity. Without this input, the action runs Renovate without a global config, which is associated with the mid-run race conditions surfacing as repository-changed.
  3. Conditional RENOVATE_DRY_RUN. Was emitted on every run as '' (empty when not dry-run). Renovate's option parser may treat the var's presence differently from its unset state. New approach: a small preceding step writes RENOVATE_DRY_RUN=full to $GITHUB_ENV only when inputs.dryRun is true.

Fixes: persistent repository-changed aborts on every Renovate run since PR #4
Related: #1, #5

Type of Change

  • Bug fix
  • Build/CI/tooling

Component(s) Affected

  • Other: CI / dependency management

Implementation Notes

  • Kept our security improvements: renovate-version digest pin (43@sha256:00185c0d…), explicit permissions block, timeout, weekday cron.
  • The configurationFile: re-add is the riskiest change — if customManager doubling reproduces in production (it didn't in the dry-runs that matter), follow-up will move customManagers to a separate file under presets/ referenced via extends: so global+repo both load it but each defines it once.

Testing

actionlint .github/workflows/renovate.yaml   # clean

Post-merge validation:

  1. Delete the orphan renovate/build-tools branch.
  2. Single gh workflow run renovate.yaml --repo njhensley/aicr dispatch.
  3. Expected: branch created AND PR opened (no repository-changed abort).
  4. If renovate/build-tools PR shows up cleanly, success.
  5. If we see customManager doubling in the resulting PR (e.g., duplicate dep entries), we have a different problem to solve next.

Risk Assessment

  • Low — Each change is reverting toward a documented known-working pattern. Worst case: doubling reappears in production, addressed via a follow-up PR.

Checklist

  • Linter passes (actionlint)
  • I did not skip/disable tests to make CI green
  • Commits are cryptographically signed (git commit -S)

…kflow

Reverts toward the known-working NVIDIA/gpu-operator pattern after PRs
#5 (drop checkout) and earlier failed to clear the persistent
"Repository has changed during renovation" abort. Three concrete
changes:

1. **Add back actions/checkout.** The action documents that it clones
   internally, but the host-side git state planted by checkout
   appears to keep Renovate's pre/post hooks consistent. Removing it
   in PR #5 did not fix the abort, suggesting checkout was never the
   culprit.

2. **Pass configurationFile: .github/renovate.json5.** The earlier
   concern about customManager doubling came from
   `RENOVATE_PLATFORM=local` dry-runs; in production (platform=github)
   Renovate dedupes by manager identity, so the same-file case is
   benign. Without configurationFile, the action runs Renovate without
   a global config — and that mode is associated with mid-run race
   conditions that surface as "repository-changed" aborts right after
   a successful branch push.

3. **Set RENOVATE_DRY_RUN conditionally.** Previously the env var was
   emitted on every run as `''` (empty string when dryRun=false).
   Renovate's option parser may treat the *presence* of the env var
   differently from its unset state. The new "Configure dry-run mode"
   step writes the var to GITHUB_ENV only when explicitly requested,
   keeping the env clean for the common case.

Verified end-to-end via actionlint. Soft-launch loop continues — once
this lands, dispatch Renovate manually and check whether
`renovate/build-tools` (the only bundle past the cooldown filter)
finally produces a PR alongside its branch.
@njhensley njhensley merged commit 96f6959 into main May 4, 2026
21 checks passed
njhensley added a commit that referenced this pull request May 4, 2026
#7)

Root cause of the persistent "Repository has changed during renovation"
aborts since PR #4 — Renovate was calling POST /repos/{owner}/{repo}/
statuses/{sha} after each branch creation to write a stability status
check (tied to the cooldown / merge-confidence flow). The workflow's
permissions block had contents:write, pull-requests:write, issues:write
but not statuses:write, so the call 403'd with "integration-
unauthorized". Renovate's error handler maps that internally to
"repository-changed", which is why every prior debugging attempt
(checkout, configurationFile, dry-run env, digest pin) chased the
wrong symptom.

Confirmed via debug log on run 25346172853:
  Request failed with status code 403 (Forbidden):
  POST .../statuses/cf3dc7d1...
  "x-accepted-github-permissions": "statuses=write"
  DEBUG: Caught error setting branch status - aborting
  Error: integration-unauthorized
  DEBUG: Passing repository-changed error up

The earlier hypothesis fixes in PRs #5 and #6 weren't wrong per se
(actions/checkout and configurationFile aren't the cause, and
RENOVATE_DRY_RUN='' isn't either) — they just couldn't have fixed the
underlying permission gap. With statuses:write added, Renovate's
post-branch status-check call should succeed and the run should
complete normally.
njhensley added a commit that referenced this pull request May 4, 2026
Confirmed in production run 25346453345: the regex manager stats
showed `"regex": {"fileCount": 4, "depCount": 56}` instead of the
expected 2/28. With `configurationFile: .github/renovate.json5`
passed, the action mounts the file as Renovate's global config AND
Renovate auto-discovers the same file from the cloned working tree;
both loads register the customManagers and every annotation gets
extracted twice. The doubling produced 14 "Cannot find replaceString
in current file content. Was it already updated?" warnings across
the four created branches — Renovate's first pass applied the
replacement, the second pass tried to re-apply but the content no
longer matched.

PR #6 added `configurationFile:` chasing the "repository-changed"
aborts under the wrong hypothesis; PR #7 found and fixed the actual
cause (missing `statuses: write` permission). With #7 in place we
can drop `configurationFile:` cleanly. Earlier dry-run experiments in
RENOVATE_PLATFORM=local mode had already shown this doubling, but I
mis-attributed it to local-mode specifics. Production confirms the
behavior is the same.

`actions/checkout` stays — it's not the cause of any issue we've
seen, and the upstream gpu-operator pattern keeps it.
njhensley added a commit that referenced this pull request May 13, 2026
Audit findings #5 and #6: the pointer file was duplicating fingerprint,
criteriaMatch, phaseSummary, and attestedAt — nearly a copy of the
predicate, with no Go consumer reading the denormalized fields. Two
sources of truth with no good answer for which to trust on mismatch.

A pointer's job is to *locate* the signed bundle, not to summarize it.
A reviewer who wants fingerprint dimensions or per-phase pass/fail
counts fetches the bundle from PointerBundle.OCI and reads
predicate.json — that's the authoritative copy and the only one.

  Before                                 After
  ──────                                 ─────
  schemaVersion                          schemaVersion
  recipe                                 recipe
  attestations[]:                        attestations[]:
    bundle                                 bundle
    signer                                 signer       (omitempty)
    attestedAt                             attestedAt
    fingerprint        ──── DROPPED
    criteriaMatch      ──── DROPPED
    phaseSummary       ──── DROPPED  (also resolves audit #6:
                                      pointer phaseSummary block
                                      omitted .skipped despite
                                      predicate carrying it)
    logsBundle         ──── DROPPED  (feature deferred in 0277fec;
                                      will return as omitempty
                                      when log capture lands)

Wire changes:

  - PointerAttestation: 7 fields → 3.

  - PointerFingerprint, PointerCriteriaMatch, PointerPhaseStat,
    PointerLogsBundle types deleted along with their build helpers
    (pointerFingerprintFrom, pointerPhaseSummaryFrom).

  - PointerInputs.LogsBundle dropped — no caller ever populated it.

  - Schema version stays at 1.0.0; no committed pointer files exist
    yet and the design doc's '2.0.0 reserved for multi-instance'
    plan is preserved.

Test coverage:

  - TestBuildPointer_OmitsDenormalizedFields: asserts the rendered
    YAML has no fingerprint:, criteriaMatch:, phaseSummary:, or
    logsBundle: keys. Catches regressions where a future commit adds
    a field back without updating the design rationale.

  - Existing fingerprint round-trip and logsBundle round-trip tests
    deleted (they asserted denormalization that no longer happens).

Design doc (docs/design/007-recipe-evidence.md):

  - Pointer schema example shrunk to match.

  - Added 'The pointer is a locator, not a denormalized cache'
    paragraph documenting the rationale.

  - Verifier step 7 updated to read criteriaMatch from the predicate
    rather than the pointer.

  - Verifier step 11 (logs bundle verification) marked deferred
    pending log capture; spelled out that V1 pointers omit the field
    entirely so verifiers don't error on absence.
njhensley added a commit that referenced this pull request May 13, 2026
Audit findings #5 and #6: the pointer file was duplicating fingerprint,
criteriaMatch, phaseSummary, and attestedAt — nearly a copy of the
predicate, with no Go consumer reading the denormalized fields. Two
sources of truth with no good answer for which to trust on mismatch.

A pointer's job is to *locate* the signed bundle, not to summarize it.
A reviewer who wants fingerprint dimensions or per-phase pass/fail
counts fetches the bundle from PointerBundle.OCI and reads
predicate.json — that's the authoritative copy and the only one.

  Before                                 After
  ──────                                 ─────
  schemaVersion                          schemaVersion
  recipe                                 recipe
  attestations[]:                        attestations[]:
    bundle                                 bundle
    signer                                 signer       (omitempty)
    attestedAt                             attestedAt
    fingerprint        ──── DROPPED
    criteriaMatch      ──── DROPPED
    phaseSummary       ──── DROPPED  (also resolves audit #6:
                                      pointer phaseSummary block
                                      omitted .skipped despite
                                      predicate carrying it)
    logsBundle         ──── DROPPED  (feature deferred in 0277fec;
                                      will return as omitempty
                                      when log capture lands)

Wire changes:

  - PointerAttestation: 7 fields → 3.

  - PointerFingerprint, PointerCriteriaMatch, PointerPhaseStat,
    PointerLogsBundle types deleted along with their build helpers
    (pointerFingerprintFrom, pointerPhaseSummaryFrom).

  - PointerInputs.LogsBundle dropped — no caller ever populated it.

  - Schema version stays at 1.0.0; no committed pointer files exist
    yet and the design doc's '2.0.0 reserved for multi-instance'
    plan is preserved.

Test coverage:

  - TestBuildPointer_OmitsDenormalizedFields: asserts the rendered
    YAML has no fingerprint:, criteriaMatch:, phaseSummary:, or
    logsBundle: keys. Catches regressions where a future commit adds
    a field back without updating the design rationale.

  - Existing fingerprint round-trip and logsBundle round-trip tests
    deleted (they asserted denormalization that no longer happens).

Design doc (docs/design/007-recipe-evidence.md):

  - Pointer schema example shrunk to match.

  - Added 'The pointer is a locator, not a denormalized cache'
    paragraph documenting the rationale.

  - Verifier step 7 updated to read criteriaMatch from the predicate
    rather than the pointer.

  - Verifier step 11 (logs bundle verification) marked deferred
    pending log capture; spelled out that V1 pointers omit the field
    entirely so verifiers don't error on absence.
@njhensley njhensley deleted the ci/renovate-restore-checkout-and-config branch June 23, 2026 16:21
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