Skip to content

Fix submodule detection, the CleanupArtifacts NameError and the skippable Bandit scan - #246

Merged
Paebbels merged 1 commit into
devfrom
claude/workflow-fixes
Aug 3, 2026
Merged

Fix submodule detection, the CleanupArtifacts NameError and the skippable Bandit scan#246
Paebbels merged 1 commit into
devfrom
claude/workflow-fixes

Conversation

@Paebbels

@Paebbels Paebbels commented Aug 2, 2026

Copy link
Copy Markdown
Member

New Features

Changes

  • No input, output or default was added, renamed or removed, so @r7 consumers need no adjustment.
  • CheckCodeQuality.yml's artifact input is now unused. It is kept declared, because removing an input from a stable release branch breaks every consumer that passes it — CompletePipeline does. See the note at the bottom.

Bug Fixes

  • PrepareJob.yml: submodules were never detected. The check tested for a file named .gitsubmodules; Git's file is .gitmodules. has_submodules was therefore always 'false', and git_submodule_count, git_submodule_names and git_submodule_paths kept their initial empty values — for every repository, since the workflow was introduced.

    The block already had the correct name in a local variable one line below:

    if [[ -f .gitsubmodules ]]; then
      has_submodules="true"
      git_modules_file=.gitmodules   # <- correct name, one line too late

    The variable is now assigned first and used for the test.

    Verified against a scratch repository with two submodules:

    has_submodules=true
    count=2
    names=libA:libB
    paths=deps/libA:deps/libB
    

    and has_submodules=false once .gitmodules is removed.

    Nothing consumes these four outputs today, which is why nobody hit it. It matters as soon as a pipeline wants to check out submodules conditionally.

  • CleanupArtifacts.yml: an unknown artifact ID raised NameError. Both compute steps call printf(...) in their case _: fallback, but the step runs shell: python, where printf is not a function:

    case _:
      printf(f"Name '{name}' not found in JSON dictionary.")

    So an artifact-json-ids entry that is not a key of the JSON dictionary — a typo, or a key removed from Parameters.yml while a consumer still lists it — aborted the step instead of reporting the name. That is precisely the case the branch exists for.

    Reproduced against the previous revision of the step:

    BEFORE the fix -> exit code: 1
        NameError: name 'printf' is not defined. Did you mean: 'print'?
    

    and against this one:

    Name 'typo_key' not found in JSON dictionary.
    Artifact to delete:
      pyX-UnitTestReportSummary-XML-*
      pyX-Packages
    

    The second run also confirms that a #-commented entry is still skipped and that the prefix/postfix forms still resolve.

  • CheckCodeQuality.yml: the security scan could be skipped silently. The Bandit step was guarded by if: inputs.artifact != '', although the step writes its report to a fixed path (report/bandit/report.xml) and never used that parameter. An empty artifact name skipped the scan, while the job installed bandit, ran to the end and reported success — a green security check that scanned nothing.

    artifact is declared required: true, but GitHub does not enforce required inputs that arrive as an empty string from an expression, which is how a consumer would realistically pass one.

    The guard is removed. The scan now runs whenever the job runs, i.e. whenever bandit: 'true'.

Documentation

Unit Tests

  • _Checking_CleanupArtifacts.yml now passes an unknown_key entry, so the case _: branch is exercised by the verification pipeline rather than only by a typo in production.

  • The PrepareJob submodule logic was executed offline against a scratch repository, in both states (.gitmodules present and absent) — output quoted above. The Actions repository itself has no submodules, so its own pipeline cannot cover the positive case.

  • Both changed shell scripts pass bash -n, and all four changed workflows parse as YAML.

  • The CheckCodeQuality change is covered by this repository's own pipeline: _Checking_JobTemplates.yml instantiates the template, and the Bandit findings in myPackage/myFramework are deliberate, so the step must run and report them.


Review (2026-08-03)

  • No description of past defects in the source. The three explanatory comments I had added — why .gitsubmodules was wrong, why the if: on the Bandit step is gone, what the unknown_key entry exercises — are removed. A fix belongs in the commit message and in this description; the code should read as if the defect never existed. The comment in _Checking_CleanupArtifacts.yml is reduced to the reviewer's one-liner, # Deliberate added 'unknown_key' for exception testing.

    The diff is now four hunks, all of them the actual change.

  • The unknown_key regression entry is kept, pending the reviewer's decision. It does not test for a typo: it covers the case _: branch, which is documented behaviour ("A key that is not present in the dictionary is reported and skipped") and is reachable without any typo — a key removed from Parameters.yml while a consumer on @r7 still lists it. Because shell: python aborts the whole step, an unknown ID meant no artifact of that set was deleted, not "one entry skipped". One line to drop if unwanted.

  • The submodule block was re-verified after the edit, since removing the comments touched that hunk: has_submodules=true count=2 names=libA:libB paths=deps/libA:deps/libB against a scratch repository with two submodules, false with empty outputs once .gitmodules is removed.


Related Issues and Pull-Requests

Note

CheckCodeQuality.yml's artifact input is now referenced nowhere. It is declared required: true, so it cannot be dropped without breaking consumers that pass it on @r7. Two options for a later major release: remove it, or give it a purpose by uploading the bandit XML report under that name. Left as is here, because either choice is an interface decision rather than a bug fix.

@codacy-production

codacy-production Bot commented Aug 2, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@codecov

codecov Bot commented Aug 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.22%. Comparing base (a46764f) to head (828b7bf).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #246   +/-   ##
=======================================
  Coverage   82.22%   82.22%           
=======================================
  Files           1        1           
  Lines          45       45           
  Branches        9        9           
=======================================
  Hits           37       37           
  Misses          4        4           
  Partials        4        4           
Flag Coverage Δ
unittests 82.22% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread .github/workflows/_Checking_CleanupArtifacts.yml
Comment thread .github/workflows/_Checking_CleanupArtifacts.yml Outdated
Comment thread .github/workflows/PrepareJob.yml Outdated
@Paebbels
Paebbels force-pushed the claude/workflow-fixes branch from 5808758 to 91378e7 Compare August 3, 2026 22:55
**`PrepareJob.yml`: submodules were never detected.** The check tested for a
file named `.gitsubmodules`; Git's file is `.gitmodules`. `has_submodules` was
therefore always `'false'` and `git_submodule_count`, `git_submodule_names` and
`git_submodule_paths` kept their initial empty values for every repository. The
block already had the correct name in a local variable one line below, which is
now used for the test as well.

Verified against a scratch repository with two submodules:

```
has_submodules=true
count=2
names=libA:libB
paths=deps/libA:deps/libB
```

and `has_submodules=false` once the file is removed.

**`CleanupArtifacts.yml`: an unknown artifact ID raised `NameError`.** Both
compute steps call `printf(...)` in their `case _:` fallback, but the step runs
`shell: python`, where `printf` is not a function. An `artifact-json-ids` entry
that is not a key of the JSON dictionary - a typo, or a key removed from
`Parameters.yml` while a consumer still lists it - aborted the step with
`NameError: name 'printf' is not defined` instead of reporting the name. That is
precisely the case the branch exists for.

Reproduced against the previous revision (exit code 1, `NameError`) and against
this one:

```
Name 'typo_key' not found in JSON dictionary.
Artifact to delete:
  pyX-UnitTestReportSummary-XML-*
  pyX-Packages
```

`_Checking_CleanupArtifacts.yml` now passes an `unknown_key` entry, so the
branch is exercised by the verification pipeline.

**`CheckCodeQuality.yml`: the security scan could be skipped silently.** The
`Bandit` step was guarded by `if: inputs.artifact != ''`, although the step
writes its report to a fixed path and never used that parameter. An empty
artifact name skipped the scan while the job still reported success. The guard
is removed; the scan now runs whenever the job runs.

Co-Authored-By: Patrick Lehmann <Paebbels@gmail.com>
@Paebbels
Paebbels force-pushed the claude/workflow-fixes branch from 91378e7 to 828b7bf Compare August 3, 2026 22:57
@Paebbels
Paebbels merged commit 051047c into dev Aug 3, 2026
154 of 159 checks passed
@Paebbels Paebbels mentioned this pull request Aug 3, 2026
@Paebbels
Paebbels deleted the claude/workflow-fixes branch August 3, 2026 23:15
@Paebbels Paebbels mentioned this pull request Aug 3, 2026
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.

2 participants