diff --git a/.github/workflows/propagate-changes.yml b/.github/workflows/propagate-changes.yml index c7cd1206e..593835d22 100644 --- a/.github/workflows/propagate-changes.yml +++ b/.github/workflows/propagate-changes.yml @@ -97,6 +97,7 @@ jobs: } // Compare commits to see if src is ahead of base + let sensitiveFiles = []; try { const compare = await github.rest.repos.compareCommits({ owner: context.repo.owner, @@ -112,19 +113,28 @@ jobs: } // If files changed include history-rewrite or other sensitive scripts, - // avoid automatic propagation. This prevents bypassing checklist validation - // and manual review for potentially destructive changes. - let files = (compare.data.files || []).map(f => (f.filename || '').toLowerCase()); + // flag them for manual review below rather than skipping PR creation + // outright. A single unrelated sensitive-path file used to block the + // *entire* diff from propagating, even when it was bundled with + // otherwise-safe changes (CI/dependency fixes) — that already caused a + // dropped propagation once (PR #1261: 4 unrelated .github/skills/ files + // blocked the auto-PR, a human had to manually recreate it, and that + // manual PR was later closed unmerged). A blocked auto-PR is silent and + // fragile; an annotated one at least surfaces for review. + let originalFiles = (compare.data.files || []).map(f => f.filename || ''); + let files = originalFiles.map(f => f.toLowerCase()); // Fallback: if compare.files is empty/truncated, aggregate files from the commit list if (files.length === 0 && Array.isArray(compare.data.commits) && compare.data.commits.length > 0) { + originalFiles = []; for (const commit of compare.data.commits) { const commitData = await github.rest.repos.getCommit({ owner: context.repo.owner, repo: context.repo.repo, ref: commit.sha }); for (const f of (commitData.data.files || [])) { - files.push((f.filename || '').toLowerCase()); + originalFiles.push(f.filename || ''); } } - files = Array.from(new Set(files)); + originalFiles = Array.from(new Set(originalFiles)); + files = originalFiles.map(f => f.toLowerCase()); } // Load propagation config (list of sensitive paths) from .github/propagate-config.yml when available @@ -148,12 +158,14 @@ jobs: if (parsedPaths.length > 0) configPaths = parsedPaths.map(p => p.toLowerCase()); } catch (err) { core.info('No .github/propagate-config.yml or parse failure; using defaults.'); } - const sensitive = files.some(fn => configPaths.some(sp => fn.startsWith(sp) || fn.includes(sp))); - if (sensitive) { - const preview = files.slice(0, 25).join(', '); - const suffix = files.length > 25 ? ` …(+${files.length - 25} more)` : ''; - core.info(`${src} -> ${base} contains sensitive changes (${preview}${suffix}). Skipping automatic propagation.`); - return; + sensitiveFiles = files + .map((fn, idx) => (configPaths.some(sp => fn.startsWith(sp) || fn.includes(sp)) ? originalFiles[idx] : null)) + .filter(Boolean); + + if (sensitiveFiles.length > 0) { + const preview = sensitiveFiles.slice(0, 25).join(', '); + const suffix = sensitiveFiles.length > 25 ? ` …(+${sensitiveFiles.length - 25} more)` : ''; + core.info(`${src} -> ${base} touches sensitive paths (${preview}${suffix}). PR will still be created, flagged for manual review.`); } } catch (error) { // If base branch doesn't exist, etc. @@ -161,18 +173,30 @@ jobs: return; } - // Create PR + // Create PR. A sensitive-path match no longer blocks creation (see above) — + // it only prepends a review warning to the PR body. try { + const isSensitive = sensitiveFiles.length > 0; + let body = `Automated PR to propagate changes from ${src} into ${base}.\n\nTriggered by push to ${currentBranch}.`; + if (isSensitive) { + const preview = sensitiveFiles.slice(0, 25).join(', '); + const suffix = sensitiveFiles.length > 25 ? ` …(+${sensitiveFiles.length - 25} more)` : ''; + body = `⚠️ This propagation touches sensitive paths that need manual review before merging: \`${preview}${suffix}\`\n\n${body}`; + } + const pr = await github.rest.pulls.create({ owner: context.repo.owner, repo: context.repo.repo, title: `Propagate changes from ${src} into ${base}`, head: src, base: base, - body: `Automated PR to propagate changes from ${src} into ${base}.\n\nTriggered by push to ${currentBranch}.`, - draft: true, + body, + // Sensitive-path PRs stay draft for a human to review and merge + // manually. Safe PRs are opened ready-for-review and we attempt to + // enable native auto-merge on them below. + draft: isSensitive, }); - core.info(`Created PR #${pr.data.number} to merge ${src} into ${base}`); + core.info(`Created PR #${pr.data.number} to merge ${src} into ${base}${isSensitive ? ' (draft, flagged for manual review)' : ''}`); // Add an 'auto-propagate' label to the created PR and create the label if missing try { try { @@ -184,6 +208,35 @@ jobs: } catch (labelErr) { core.warning('Failed to ensure or add auto-propagate label: ' + labelErr.message); } + + // Attempt to enable GitHub's native auto-merge for safe (non-sensitive) + // propagation PRs, so they don't require a human to notice and click + // merge. This repo only allows merge commits (allow_squash_merge and + // allow_rebase_merge are both false), matching the existing nightly -> + // main weekly promotion policy of always using "Create a merge commit", + // so we request MERGE here too — never SQUASH or REBASE. + // + // NOTE: as of this change, the repo-level "Allow auto-merge" setting + // (Settings > General > Pull Requests > Allow auto-merge) is OFF, so + // this mutation will fail (as a caught, non-fatal warning) until that + // setting is turned on. Flipping that setting is a deliberate, separate + // repo-administration decision — this code is written to be correct and + // inert until then, not to attempt it itself. + if (!isSensitive) { + try { + await github.graphql( + `mutation($pullRequestId: ID!) { + enablePullRequestAutoMerge(input: { pullRequestId: $pullRequestId, mergeMethod: MERGE }) { + pullRequest { autoMergeRequest { enabledAt } } + } + }`, + { pullRequestId: pr.data.node_id } + ); + core.info(`Auto-merge enabled for PR #${pr.data.number}`); + } catch (mergeErr) { + core.warning(`Could not enable auto-merge for PR #${pr.data.number} — check repo Settings > Pull Requests > Allow auto-merge. (${mergeErr.message})`); + } + } } catch (error) { core.warning(`Failed to create PR from ${src} to ${base}: ${error.message}`); } @@ -198,5 +251,15 @@ jobs: core.info('Push originated from development (excluded). Skipping propagation back to development.'); } } else if (currentBranch === 'development') { - core.info('Push to development detected. No downstream propagation configured.'); + // development -> nightly is intentionally NOT handled here. That hop + // is already covered by the `sync-development-to-nightly` job in + // .github/workflows/nightly-build.yml, which runs on a daily cron + // (09:00 UTC) and fast-forwards (or force-resets) `nightly` to match + // `development` directly, bypassing PRs entirely. Opening a + // PR-based development -> nightly leg here would race that cron: if + // this leg's PR is still open when the next scheduled run fires, the + // cron's force-push would collapse the PR's diff to zero, leaving a + // dangling, unmergeable PR behind. See ARCHITECTURE.md's "Branch + // Promotion Chain" section for the full picture. + core.info('Push to development detected. Propagation to nightly is handled by the daily sync-development-to-nightly cron in nightly-build.yml, not this workflow.'); } diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index e9b8bd408..52f8d4c0e 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -1124,10 +1124,14 @@ services: **Branch Strategy:** - `main`: Stable production branch +- `development`: Integration branch; aggregates changes promoted from `main` plus ongoing work before they reach `nightly` +- `nightly`: Nightly build/package branch; promoted weekly to `main` via a manual, merge-commit-only promotion PR - `feature/*`: New feature development - `fix/*`: Bug fixes - `chore/*`: Maintenance tasks +See "Branch Promotion Chain" below for how changes flow `main` → `development` → `nightly` → `main` — note that each hop uses a *different* mechanism, not one uniform pipeline. + **Commit Convention:** - `feat:` New user-facing feature @@ -1148,6 +1152,55 @@ provisioning via Let's Encrypt DNS-01 challenge. Closes #123 ``` +### Branch Promotion Chain + +Charon promotes changes downstream through three long-lived branches: +`main` (stable/production) → `development` (integration) → `nightly` +(nightly builds) → back to `main` weekly. Each hop is driven by a +**different mechanism**, with a different trust/automation level — this is +deliberate, not an inconsistency to "fix" into one uniform pipeline: + +1. **`main` → `development`** (`.github/workflows/propagate-changes.yml`): + fires on every successful `Docker Build, Publish & Test` run on `main` + and opens an automated PR (labeled `auto-propagate`) into `development`. + - Diffs that touch a path listed under `sensitive_paths` in + `.github/propagate-config.yml` (e.g. `docs/plans/`, `.github/skills/`) + still get a PR, but it stays in draft with a warning in the PR body + naming the matched files, and does not get auto-merge — a human must + review and merge it. + - Diffs with no sensitive-path matches are opened ready-for-review and + the workflow attempts to enable GitHub's native auto-merge + (`mergeMethod: MERGE`, since this repo only allows merge commits). + Auto-merge only actually takes effect once the repo-level **Settings + → General → Pull Requests → Allow auto-merge** setting is turned on; + until then the mutation fails safely and just logs a warning. + - Loop prevention: the leg checks whether the triggering commit came + from a PR sourced in `development` and skips propagating back into it + (e.g. a `development` → `main`-sourced merge does not immediately + reopen a `main` → `development` PR). + - This workflow does **not** handle `development` → `nightly` — pushes + to `development` are deliberately a no-op here (see next item). + +2. **`development` → `nightly`** + (`.github/workflows/nightly-build.yml`, job `sync-development-to-nightly`): + a separate, pre-existing daily cron (09:00 UTC, plus `workflow_dispatch`) + that fast-forwards `nightly` to match `origin/development`, or, if a + fast-forward isn't possible, force-resets it (`git reset --hard + origin/development` + force-push). This bypasses PRs entirely — it is + not related to `propagate-changes.yml` above. An earlier draft of this + fix added a second, PR-based `development` → `nightly` leg to + `propagate-changes.yml`; that was dropped because it raced this cron — + the cron's next force-reset would silently collapse the PR's diff to + zero, leaving a dangling, unmergeable PR. `development` → `nightly` + therefore has exactly one mechanism: this cron. + +3. **`nightly` → `main`** (the weekly release, + `.github/workflows/weekly-nightly-promotion.yml`): a separate, manual + promotion PR, unrelated to either mechanism above. Always merged by hand + using **"Create a merge commit"** (never squash/rebase) — see the + "Weekly Promotion PRs" note in `CLAUDE.md`; squashing collapses commit + history the `auto-versioning` workflow needs to parse for version bumps. + ### Code Review Process 1. **Automated Checks (CI):**