Fix the PG version check so a detected issue is reported instead of failing the job

.github/scripts/check-pg-versions.py

@@ -14,7 +14,7 @@
 
 
 def git(*args, allow_missing=False):
-    result = subprocess.run(['git', *args], capture_output=True, text=True)
+    result = subprocess.run(['git', *args], capture_output=True, text=True, encoding='utf-8', errors='replace')
     if result.returncode and not (allow_missing and result.returncode == 1):
         raise RuntimeError(result.stderr.strip() or 'git command failed')
     return result.stdout
@@ -146,7 +146,7 @@ def layout(lines, values):
 
 def check(base, head):
     base = git('merge-base', base, head).strip()
-    changed = [path for path in git('diff', '--name-only', base + '..' + head).splitlines() if path.endswith(('.c', '.h'))]
+    changed = [path for path in git('diff', '--no-renames', '--name-only', base + '..' + head).splitlines() if path.endswith(('.c', '.h'))]
     if not changed:
         print('No C/H files changed')
         return 0

.github/scripts/test-check-pg-versions.py

@@ -1,5 +1,6 @@
-import subprocess,tempfile,pathlib,os
+import re,subprocess,tempfile,pathlib,os
 script=str(pathlib.Path(__file__).with_name('check-pg-versions.sh').resolve())
+workflow=pathlib.Path(__file__).parents[1]/'workflows'/'pg-version-check.yml'
 cases=[('unchanged',False,False,[1],[1],0),('missing bump',True,False,[1],[1],1),('bumped',True,False,[1],[2],0),('array missing',True,True,[4],[4],1),('array bumped',True,True,[4],[5],0),('conditional bumped',True,True,[4,1],[5,2],0),('conditional partial',True,True,[4,1],[5,1],1),('conditional decreased',True,True,[4,1],[3,2],1)]
 for label,changed,array,old,new,expected in cases:
  with tempfile.TemporaryDirectory() as d:
@@ -49,6 +50,23 @@ def registration(v):
   assert result.returncode==expected,result.stdout+result.stderr
 
 
+# A renamed + layout-changed struct header must still be checked: the old path's
+# definition is otherwise lost when git diff collapses the rename to the new name.
+with tempfile.TemporaryDirectory() as d:
+ def git(*a): return subprocess.run(['git','-c','user.name=CI Test','-c','user.email=ci@example.invalid',*a],cwd=d,check=True,capture_output=True,text=True).stdout.strip()
+ p=pathlib.Path(d);git('init')
+ (p/'config.h').write_text('typedef struct config_s {\n int old;\n} config_t;\n')
+ (p/'config.c').write_text('PG_REGISTER_WITH_RESET_FN(config_t, config, PG_CONFIG, 1);\n')
+ git('add','.');git('commit','-m','base')
+ git('mv','config.h','renamed_config.h')
+ (p/'renamed_config.h').write_text('typedef struct config_s {\n int old;\n int added;\n} config_t;\n')
+ git('add','.');git('commit','-m','head')
+ env={k:v for k,v in os.environ.items() if k not in ('GITHUB_BASE_REF','GITHUB_HEAD_REF')}
+ result=subprocess.run(['bash',script],cwd=d,capture_output=True,text=True,env=env)
+ print('renamed header missing bump','exit',result.returncode,'expected',1)
+ assert result.returncode==1,result.stdout+result.stderr
+
+
 # Advancing the base branch must not make changes outside the PR look like removals.
 with tempfile.TemporaryDirectory() as d:
  def git(*a): return subprocess.run(['git','-c','user.name=CI Test','-c','user.email=ci@example.invalid',*a],cwd=d,check=True,capture_output=True,text=True).stdout.strip()
@@ -64,3 +82,30 @@ def git(*a): return subprocess.run(['git','-c','user.name=CI Test','-c','user.em
  result=subprocess.run(['bash',script],cwd=d,capture_output=True,text=True,env=env)
  print('advanced base uses merge-base','exit',result.returncode,'expected',0)
  assert result.returncode==0,result.stdout+result.stderr
+
+# The workflow's own "Run PG version check script" step re-parses the checker's stdout
+# with a bash guard and (on the next step) a JS filter, both keyed on a literal string.
+# Run that guard for real, against the checker's real "issue found" output, so the two
+# can't silently drift apart the way they did across two commits in this same PR chain
+# (one added a '^### ' guard for the old bash script's Markdown headings, a later one
+# rewrote the checker in Python with no '###' anywhere in its output).
+run_block=re.search(r"- name: Run PG version check script\n(?:.*\n)*?        run: \|\n((?:( {10}.*)?\n)+)",workflow.read_text())
+assert run_block,'could not find the "Run PG version check script" step in ' + str(workflow)
+guard=run_block[1]
+assert 'check-pg-versions.sh' in guard and 'exit_code' in guard,'unexpected step contents:\n' + guard
+with tempfile.TemporaryDirectory() as d:
+ def git(*a): return subprocess.run(['git','-c','user.name=CI Test','-c','user.email=ci@example.invalid',*a],cwd=d,check=True,capture_output=True,text=True).stdout.strip()
+ p=pathlib.Path(d);git('init')
+ (p/'config.h').write_text('typedef struct config_s {\n int old;\n} config_t;\n')
+ (p/'config.c').write_text('PG_REGISTER_WITH_RESET_FN(config_t, config, PG_CONFIG, 1);\n')
+ git('add','.');git('commit','-m','base');base=git('rev-parse','HEAD')
+ (p/'config.h').write_text('typedef struct config_s {\n int old;\n int added;\n} config_t;\n')
+ git('add','.');git('commit','--allow-empty','-m','head, missing version bump')
+ git('update-ref','refs/remotes/origin/test-base',base)
+ wrapper='#!/bin/bash\nset -e\ncd ' + d + '\n' + guard.replace('.github/scripts/check-pg-versions.sh', script)
+ outputs=str(p/'github_output')
+ env=dict(os.environ,GITHUB_BASE_REF='test-base',GITHUB_HEAD_REF='feature',GITHUB_OUTPUT=outputs)
+ result=subprocess.run(['bash','-c',wrapper],capture_output=True,text=True,env=env)
+ print('workflow guard accepts a real detected issue','exit',result.returncode,'expected',0)
+ assert result.returncode==0,'the workflow step would hard-fail instead of posting a comment:\n'+result.stdout+result.stderr
+ assert 'exit_code=1' in pathlib.Path(outputs).read_text(),'workflow step did not record the issue for the comment step'

.github/workflows/pg-version-check-comment.yml

@@ -0,0 +1,122 @@
+name: Parameter Group Version Check Comment
+
+# Posts the PG version warning comment from a privileged context so it also works
+# for PRs opened from forks, whose pull_request GITHUB_TOKEN is read-only. The
+# unprivileged "Parameter Group Version Check" workflow uploads the checker output
+# and PR number as artifacts; this workflow only downloads them — it never executes
+# pull-request code.
+on:
+  workflow_run:
+    workflows: ["Parameter Group Version Check"]
+    types: [completed]
+
+jobs:
+  comment:
+    runs-on: ubuntu-latest
+    # Only act on pull_request-triggered runs that succeeded (a checker error fails
+    # the check job, so its conclusion is not 'success').
+    if: >
+      github.event.workflow_run.event == 'pull_request' &&
+      github.event.workflow_run.conclusion == 'success'
+    permissions:
+      actions: read
+      issues: write
+      pull-requests: write
+
+    steps:
+      - name: Download PG check result
+        uses: actions/download-artifact@v4
+        with:
+          name: pg-check-result
+          run-id: ${{ github.event.workflow_run.id }}
+          github-token: ${{ secrets.GITHUB_TOKEN }}
+
+      - name: Post or update comment
+        uses: actions/github-script@v7
+        with:
+          script: |
+            const fs = require('fs');
+            const prNumber = Number(fs.readFileSync('pr_number.txt', 'utf8').trim());
+            const output = fs.readFileSync('pg_output.txt', 'utf8');
+
+            if (!output.includes('PG version issue:')) {
+              console.log('No PG version issues to report; nothing to do.');
+              return;
+            }
+
+            let issuesContent = '';
+
+            try {
+              // Extract issues from output (everything after the warning line)
+              const lines = output.split('\n');
+              let capturing = false;
+              let issues = [];
+
+              for (const line of lines) {
+                if (line.includes('PG version issue:')) {
+                  capturing = true;
+                }
+                if (capturing) {
+                  issues.push(line);
+                }
+              }
+
+              issuesContent = issues.join('\n');
+            } catch (err) {
+              console.log('Error capturing issues:', err);
+              issuesContent = '*Unable to extract detailed issues*';
+            }
+
+            const commentBody = '## ⚠️ Parameter Group Version Check\n\n' +
+              'The following parameter groups may need version increments:\n\n' +
+              issuesContent + '\n\n' +
+              '**Why this matters:**\n' +
+              'Modifying PG struct fields without incrementing the version can cause settings corruption when users flash new firmware. The `pgLoad()` function validates versions and will use defaults if there\'s a mismatch, preventing corruption.\n\n' +
+              '**When to increment the version:**\n' +
+              '- ✅ Adding/removing fields\n' +
+              '- ✅ Changing field types or sizes\n' +
+              '- ✅ Reordering fields\n' +
+              '- ✅ Adding/removing packing attributes\n' +
+              '- ❌ Only changing default values in `PG_RESET_TEMPLATE`\n' +
+              '- ❌ Only changing comments\n\n' +
+              '**Reference:**\n' +
+              '- [Parameter Group Documentation](../docs/development/parameter_groups/)\n' +
+              '- Example: [PR #11236](https://github.com/iNavFlight/inav/pull/11236) (field removal requiring version increment)\n\n' +
+              '---\n' +
+              '*This is an automated check. False positives are possible. If you believe the version increment is not needed, please explain in a comment.*';
+
+            try {
+              // Check if we already commented
+              const { data: comments } = await github.rest.issues.listComments({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                issue_number: prNumber,
+              });
+
+              const botComment = comments.find(comment =>
+                comment.user.login === 'github-actions[bot]' &&
+                comment.body.includes('Parameter Group Version Check')
+              );
+
+              if (botComment) {
+                // Update existing comment
+                await github.rest.issues.updateComment({
+                  owner: context.repo.owner,
+                  repo: context.repo.repo,
+                  comment_id: botComment.id,
+                  body: commentBody
+                });
+                console.log('Updated existing PG version check comment');
+              } else {
+                // Post new comment
+                await github.rest.issues.createComment({
+                  owner: context.repo.owner,
+                  repo: context.repo.repo,
+                  issue_number: prNumber,
+                  body: commentBody
+                });
+                console.log('Posted new PG version check comment');
+              }
+            } catch (err) {
+              core.setFailed(`Failed to post comment: ${err}`);
+            }

.github/workflows/pg-version-check.yml

@@ -19,7 +19,6 @@ jobs:
     runs-on: ubuntu-latest
     permissions:
       contents: read
-      pull-requests: write
 
     steps:
       - name: Checkout PR code
@@ -42,7 +41,7 @@ jobs:
           # The output is captured and encoded to be passed between steps.
           output=$(bash .github/scripts/check-pg-versions.sh 2>&1)
           exit_code=$?
-          if [ "$exit_code" -gt 1 ] || { [ "$exit_code" -eq 1 ] && ! grep -q '^### ' <<< "$output"; }; then
+          if [ "$exit_code" -gt 1 ] || { [ "$exit_code" -eq 1 ] && ! grep -q '^PG version issue:' <<< "$output"; }; then
             printf '%s\n' "$output"
             exit 2
           fi
@@ -54,90 +53,20 @@ jobs:
           GITHUB_BASE_REF: ${{ github.base_ref }}
           GITHUB_HEAD_REF: ${{ github.head_ref }}
 
-      - name: Post comment if issues found
-        if: steps.pg_check.outputs.exit_code == '1'
-        uses: actions/github-script@v7
+      - name: Stage PG check result for the comment workflow
         env:
-          # Passed through the environment: inlining the multi-line script output
-          # into the JavaScript source breaks the string literal (SyntaxError).
+          # Multi-line output travels through the environment so shell
+          # metacharacters in it are not reinterpreted.
           PG_CHECK_OUTPUT: ${{ steps.pg_check.outputs.output }}
-        with:
-          script: |
-            // Use the captured output from the previous step
-            const output = process.env.PG_CHECK_OUTPUT || '';
-            let issuesContent = '';
-
-            try {
-              // Extract issues from output (everything after the warning line)
-              const lines = output.split('\n');
-              let capturing = false;
-              let issues = [];
-
-              for (const line of lines) {
-                if (line.includes('###')) {
-                  capturing = true;
-                }
-                if (capturing) {
-                  issues.push(line);
-                }
-              }
-
-              issuesContent = issues.join('\n');
-            } catch (err) {
-              console.log('Error capturing issues:', err);
-              issuesContent = '*Unable to extract detailed issues*';
-            }
-
-            const commentBody = '## ⚠️ Parameter Group Version Check\n\n' +
-              'The following parameter groups may need version increments:\n\n' +
-              issuesContent + '\n\n' +
-              '**Why this matters:**\n' +
-              'Modifying PG struct fields without incrementing the version can cause settings corruption when users flash new firmware. The `pgLoad()` function validates versions and will use defaults if there\'s a mismatch, preventing corruption.\n\n' +
-              '**When to increment the version:**\n' +
-              '- ✅ Adding/removing fields\n' +
-              '- ✅ Changing field types or sizes\n' +
-              '- ✅ Reordering fields\n' +
-              '- ✅ Adding/removing packing attributes\n' +
-              '- ❌ Only changing default values in `PG_RESET_TEMPLATE`\n' +
-              '- ❌ Only changing comments\n\n' +
-              '**Reference:**\n' +
-              '- [Parameter Group Documentation](../docs/development/parameter_groups/)\n' +
-              '- Example: [PR #11236](https://github.com/iNavFlight/inav/pull/11236) (field removal requiring version increment)\n\n' +
-              '---\n' +
-              '*This is an automated check. False positives are possible. If you believe the version increment is not needed, please explain in a comment.*';
-
-            try {
-              // Check if we already commented
-              const { data: comments } = await github.rest.issues.listComments({
-                owner: context.repo.owner,
-                repo: context.repo.repo,
-                issue_number: context.issue.number,
-              });
-
-              const botComment = comments.find(comment =>
-                comment.user.login === 'github-actions[bot]' &&
-                comment.body.includes('Parameter Group Version Check')
-              );
+        run: |
+          echo "${{ github.event.pull_request.number }}" > pr_number.txt
+          printf '%s\n' "$PG_CHECK_OUTPUT" > pg_output.txt
 
-              if (botComment) {
-                // Update existing comment
-                await github.rest.issues.updateComment({
-                  owner: context.repo.owner,
-                  repo: context.repo.repo,
-                  comment_id: botComment.id,
-                  body: commentBody
-                });
-                console.log('Updated existing PG version check comment');
-              } else {
-                // Post new comment
-                await github.rest.issues.createComment({
-                  owner: context.repo.owner,
-                  repo: context.repo.repo,
-                  issue_number: context.issue.number,
-                  body: commentBody
-                });
-                console.log('Posted new PG version check comment');
-              }
-            } catch (err) {
-              core.setFailed(`Failed to post comment: ${err}`);
-            }
+      - name: Upload PG check result
+        uses: actions/upload-artifact@v4
+        with:
+          name: pg-check-result
+          path: |
+            pr_number.txt
+            pg_output.txt
+          retention-days: 1