.github/workflows/publish-ocm-component-version.yml (2)

140-141: Add a brief comment explaining the URL encoding.

The shell parameter expansion ${COMPONENT_NAME//\//%2F} URL-encodes forward slashes. A brief comment would help readers unfamiliar with this syntax.

📝 Proposed comment

+          # URL-encode slashes in component name: ${var//pattern/replacement} replaces all '/' with '%2F'
           PKG_PATH="component-descriptors%2F${COMPONENT_NAME//\//%2F}"
           PACKAGE_URL="https://github.com/${GITHUB_REPOSITORY}/pkgs/container/${PKG_PATH}"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/publish-ocm-component-version.yml around lines 140 - 141,
Add a brief inline comment above the PKG_PATH/PACKAGE_URL assignments explaining
that the shell parameter expansion ${COMPONENT_NAME//\//%2F} replaces all
forward slashes with "%2F" to URL-encode the component name for the GitHub
Packages path; reference the PKG_PATH and PACKAGE_URL variables and place the
comment directly next to the PKG_PATH line so readers understand why the
expansion is used.

---

129-130: Clarify expected constructor format or adjust parsing.

The sed patterns ^name: and ^version: assume these fields appear at the start of lines, but typical component constructors nest these under a components: list:

components:
- name: github.com/acme.org/myapp
  version: 1.0.0

If the workflow expects a different single-component format (e.g., flat YAML without the components: wrapper), please add a comment documenting the expected structure to aid future maintainers.

♻️ Option A: Add a comment documenting expected format

+          # Expected constructor format (single component, flat structure):
+          #   name: <component-name>
+          #   version: <component-version>
+          #   ...
           COMPONENT_NAME=$(sed -n 's/^name:[[:space:]]*//p' "${CONSTRUCTOR_FILE}" | head -n 1)
           COMPONENT_VERSION=$(sed -n 's/^version:[[:space:]]*//p' "${CONSTRUCTOR_FILE}" | head -n 1)

♻️ Option B: Adjust regex to handle nested YAML format

-          COMPONENT_NAME=$(sed -n 's/^name:[[:space:]]*//p' "${CONSTRUCTOR_FILE}" | head -n 1)
-          COMPONENT_VERSION=$(sed -n 's/^version:[[:space:]]*//p' "${CONSTRUCTOR_FILE}" | head -n 1)
+          # Handle both flat and nested component constructor formats
+          COMPONENT_NAME=$(sed -n 's/^-\{0,1\}[[:space:]]*name:[[:space:]]*//p' "${CONSTRUCTOR_FILE}" | head -n 1)
+          COMPONENT_VERSION=$(sed -n 's/^[[:space:]]*version:[[:space:]]*//p' "${CONSTRUCTOR_FILE}" | head -n 1)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/publish-ocm-component-version.yml around lines 129 - 130,
The sed extraction for COMPONENT_NAME and COMPONENT_VERSION assumes fields start
at column 0; either document the expected flat constructor YAML format near the
COMPONENT_NAME/COMPONENT_VERSION assignment (mentioning CONSTRUCTOR_FILE must
contain top-level name: and version: keys) or change the sed patterns used to
strip leading whitespace and an optional list dash so they match nested entries
like "- name:" and "version:" indented under components; update the two sed
invocations that produce COMPONENT_NAME and COMPONENT_VERSION to use the more
permissive pattern that accepts leading spaces and an optional leading '-'
before the key.

.github/workflows/publish-ocm-component-version.yml (1)

34-37: Add explicit contents: read permission for checkout.

The job uses actions/checkout on Line [121]. Declaring contents: read explicitly avoids permission brittleness when token defaults are restricted.

💡 Proposed fix

     permissions:
       actions: read
+      contents: read
       packages: write    # Required for pushing to GHCR via docker login + OCM CLI

Also applies to: 121-126

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/publish-ocm-component-version.yml around lines 34 - 37,
The workflow's job permissions block currently sets actions: read and packages:
write but omits contents: read, which can break actions/checkout; update the
job-level permissions to include contents: read so the actions/checkout step can
access repository contents (modify the permissions block that contains actions:
read and packages: write to add contents: read) and mirror the same change for
the other similar permissions block used around the actions/checkout step.

.github/scripts/component-constructor.test.js (1)

172-186: Use try/finally around temp-file blocks to guarantee cleanup.

If an assertion fails before cleanup, temp artifacts remain behind.

💡 Proposed fix pattern

 {
     const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "ocm-test-"));
     const tmpFile = path.join(tmpDir, "component-constructor.yaml");
-
-    const testDoc = { name: "ocm.software/test", version: "0.1.0", provider: { name: "test" } };
-    fs.writeFileSync(tmpFile, yaml.dump(testDoc), "utf8");
-
-    const parsed = parseConstructorFile(tmpFile);
-    assert.strictEqual(parsed.name, "ocm.software/test", "Should parse name from YAML file");
-    assert.strictEqual(parsed.version, "0.1.0", "Should parse version from YAML file");
-
-    // Cleanup
-    fs.unlinkSync(tmpFile);
-    fs.rmdirSync(tmpDir);
+    try {
+        const testDoc = { name: "ocm.software/test", version: "0.1.0", provider: { name: "test" } };
+        fs.writeFileSync(tmpFile, yaml.dump(testDoc), "utf8");
+
+        const parsed = parseConstructorFile(tmpFile);
+        assert.strictEqual(parsed.name, "ocm.software/test", "Should parse name from YAML file");
+        assert.strictEqual(parsed.version, "0.1.0", "Should parse version from YAML file");
+    } finally {
+        if (fs.existsSync(tmpFile)) fs.unlinkSync(tmpFile);
+        if (fs.existsSync(tmpDir)) fs.rmdirSync(tmpDir);
+    }
 }

Apply the same pattern to the empty-file block.

Also applies to: 196-209

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/scripts/component-constructor.test.js around lines 172 - 186, The
test creates temp dirs/files and cleans them up manually but can leak artifacts
if assertions fail; wrap the temp-file test blocks that call
parseConstructorFile (the block creating tmpDir/tmpFile and the similar
"empty-file" block) in a try/finally where the finally always calls
fs.unlinkSync(tmpFile) and fs.rmdirSync(tmpDir) (guarding existence checks as
needed) so cleanup runs even on assertion failures; update both the block using
parseConstructorFile and the other empty-file test to use this try/finally
pattern.

.github/scripts/component-constructor.js (1)

154-156: Harden failure reporting for non-Error throws.

error.message can be empty/undefined if a non-Error value is thrown.

💡 Proposed fix

     } catch (error) {
-        core.setFailed(error.message);
+        const message = error instanceof Error ? error.message : String(error);
+        core.setFailed(message);
     }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/scripts/component-constructor.js around lines 154 - 156, The catch
block currently calls core.setFailed(error.message) which can be empty for
non-Error throws; update the catch handling in the catch (error) block to
normalize the thrown value before passing to core.setFailed (e.g., if error is
an Error use error.message or error.stack, otherwise convert to a string/JSON
via String(error) or JSON.stringify(error) and include it in the message) so
core.setFailed always receives a meaningful string; ensure you reference the
existing catch (error) and core.setFailed invocation when making the change.