feat: notify AM repo when WF is published - #1146
Conversation
WalkthroughPublish workflow now computes a final RC-style version (Python step querying PyPI and git), selects final_version from inputs/dev-calculated RC/tag/commit_sha, updates Poetry and publishes that version, creates an annotated RC tag on Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as "dev branch / CI"
participant PublishWF as "publish.yml"
participant PyPI as "PyPI API"
participant Git as "git / repo"
participant TriggerWF as "trigger-workflow.yml"
participant Downstream as "downstream repo"
Dev->>+PublishWF: start (commit_sha or tag or manual input)
PublishWF->>+PyPI: query releases & RCs
PyPI-->>-PublishWF: releases/versions
PublishWF->>Git: inspect tags & commits since last stable
PublishWF-->>PublishWF: compute next RC (Python step)
alt on dev branch
PublishWF->>Git: create annotated tag writer-framework-<version>
Git-->>PublishWF: push tag
end
PublishWF->>TriggerWF: workflow_call(event_type, merged_payload with final_version)
TriggerWF->>Downstream: repository_dispatch with client_payload
Note right of Downstream: receives final_version in payload
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![hackerone-code[bot]](https://avatars.githubusercontent.com/in/3434?s=60&v=4){kind=small}
There was a problem hiding this comment.
HackerOne Code Security Review
🟢 Scan Complete: 4 Issue(s)
🟢 Validation Complete: Any Issues detected were validated by one of our engineers. None were determined to require immediate action.
Here's how the code changes were interpreted and info about the tools used for scanning.
📖 Summary of Changes
The GitHub Actions workflows were modified to enhance branch-specific publishing and workflow triggering capabilities. The publish workflow now supports dev branch releases with improved version extraction and RC tag creation. The trigger workflow was refactored to become a reusable workflow with dynamic payload merging and added logging for better event tracking and flexibility.| File | Summary |
|---|---|
| .github/workflows/publish.yml | The workflow file was updated to support publishing from the 'dev' branch, with changes to version extraction logic, added RC version determination, and a new step to create and push RC tags when publishing from the development branch. |
| .github/workflows/trigger-workflow.yml | The workflow changed from a push-triggered action to a reusable workflow with optional inputs. It now dynamically merges commit SHA and extra payload using jq, and adds logging for the triggered event type and payload. |
ℹ️ Issues Detected
NOTE: These may not require action!
Below are unvalidated results from the Analysis Tools that ran during the latest scan for transparency. We investigate each of these for accuracy and relevance before surfacing them as a potential problem.
How will I know if something is a problem?
When validation completes, any concerns that warrant attention prior to merge will be posted as inline comments. These will show up in 2 ways:
- Expert review (most cases): Issues will be posted by experts who manually reviewed and validated them. These are real HackerOne engineers (not bots) reviewing through an integrated IDE-like tool. You can communicate with them like any other reviewer. They'll stay assigned and get notified with commit & comment updates.
- Automatically: In cases where our validation checks have highest confidence the problem is legitimate and urgent. These will include a description of contextual reasoning why & actionable next steps.
| File & Line | Issue |
|---|---|
.github/workflows/publish.yml Line 58 |
The script uses unsanitized git log output in a regex pattern, which could potentially lead to unexpected behavior if commit messages contain special regex characters. |
.github/workflows/publish.yml Line 45 |
The script makes an HTTP request to PyPI without verifying SSL certificates. This could allow for man-in-the-middle attacks where an attacker could intercept sensitive information or inject malicious responses. |
.github/workflows/trigger-workflow.yml Line 24 |
Using variable interpolation ${{...}} with github context data in a run: step could allow an attacker to inject their own code into the runner. This would allow them to steal secrets and code. github context data can have arbitrary user input and should be treated as untrusted. Instead, use an intermediate environment variable with env: to store the data and use the environment variable in the run: script. Be sure to use double-quotes the environment variable, like this: "$ENVVAR". |
.github/workflows/publish.yml Line 77 |
Using variable interpolation ${{...}} with github context data in a run: step could allow an attacker to inject their own code into the runner. This would allow them to steal secrets and code. github context data can have arbitrary user input and should be treated as untrusted. Instead, use an intermediate environment variable with env: to store the data and use the environment variable in the run: script. Be sure to use double-quotes the environment variable, like this: "$ENVVAR". |
🧰 Analysis tools
- [ ✅ ] HackerOne AI Code Analysis
- [ ✅ ] HackerOne AI Code Validation
- [ ✅ ] semgrep
- [ ✅ ] bandit
⏱️ Latest scan covered changes up to commit ab05041 (latest)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/publish.yml(2 hunks).github/workflows/trigger-workflow.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (3.11)
- GitHub Check: build (3.13)
- GitHub Check: build (3.12)
- GitHub Check: build (3.9)
- GitHub Check: build (3.10)
- GitHub Check: build (3.9)
- GitHub Check: build (3.11)
- GitHub Check: build (3.12)
- GitHub Check: build (3.13)
- GitHub Check: build (3.10)
- GitHub Check: tests (firefox)
- GitHub Check: tests (webkit)
- GitHub Check: tests (chromium)
🔇 Additional comments (3)
.github/workflows/publish.yml (1)
73-76: Verify reusable workflow invocation and input mapping.The step invokes the reusable workflow with
event_type: framework_published. Ensure this aligns with the input definition in.github/workflows/trigger-workflow.yml(which expectsevent_typeas a required string input). Verify that both the push-to-dev trigger and this workflow_call invocation correctly pass/default theevent_typevariable downstream..github/workflows/trigger-workflow.yml (2)
7-12: LGTM on workflow_call trigger definition.The reusable workflow trigger is properly defined with a required
event_typeinput, enabling external workflows to invoke this workflow with a custom event type. This supports the publish workflow's new framework_published event.
26-26: Let me check the publish.yml file to understand the complete context of how trigger-workflow.yml is called:Based on my verification of GitHub Actions expression language behavior and the workflow configuration:
Variable expansion works correctly in both trigger scenarios.
When the workflow is triggered by push, the whole inputs context is unavailable, which evaluates to an empty string. Empty string is a falsy value in GitHub Actions conditionals, so the
||operator correctly triggers the fallback. For string inputs you can use the || expression operator to set the default value, and this pattern${{ github.event.inputs.name || 'Octocat' }}solves the issue without hassle.Your implementation correctly defaults to
'framework_updated'when triggered via push to dev (wheregithub.event.inputsis unavailable) and uses the passed value when invoked via workflow_call from publish.yml.
715e4c1 to
4c1e494
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/publish.yml(2 hunks).github/workflows/trigger-workflow.yml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/trigger-workflow.yml
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/publish.yml
33-33: property "pr" is not defined in object type {}
(expression)
84-84: property "extract_version" is not defined in object type {bump_rc: {conclusion: string; outcome: string; outputs: {string => string}}}
(expression)
90-90: property "extract_version" is not defined in object type {bump_rc: {conclusion: string; outcome: string; outputs: {string => string}}}
(expression)
93-93: property "extract_version" is not defined in object type {bump_rc: {conclusion: string; outcome: string; outputs: {string => string}}}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (3.10)
- GitHub Check: build (3.11)
- GitHub Check: build (3.12)
- GitHub Check: build (3.9)
- GitHub Check: build (3.13)
- GitHub Check: build (3.13)
- GitHub Check: build (3.12)
- GitHub Check: build (3.10)
- GitHub Check: build (3.11)
- GitHub Check: tests (firefox)
- GitHub Check: build (3.9)
- GitHub Check: tests (webkit)
- GitHub Check: tests (chromium)
🔇 Additional comments (1)
.github/workflows/publish.yml (1)
97-100: The syntax in the review comment is actually valid; the real issue is the missing workflow file.GitHub Actions documentation confirms that
uses: ./.github/workflows/filename.ymlis the correct syntax for calling a reusable workflow in the same repository. The code at lines 97-100 follows this exact pattern correctly.However, the verification revealed that
trigger-workflow.ymldoes not exist in the repository—so while the syntax is valid, the referenced file is missing. The review comment's premise is flawed: it incorrectly identifies valid syntax as invalid and suggests unnecessary workarounds (like adding@${{ github.ref }}, using CLI, or treating it as an action directory).Actual issue: The file
.github/workflows/trigger-workflow.ymlmust be created for this step to function, or the reference must point to an existing workflow file.Likely an incorrect or invalid review comment.
4e4e32e to
794111b
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/publish.yml(2 hunks).github/workflows/trigger-workflow.yml(2 hunks)
🧰 Additional context used
🪛 GitHub Check: CodeQL
.github/workflows/trigger-workflow.yml
[warning] 27-27: Code injection
Potential code injection in ${{ github.event.inputs.event_type || 'framework_updated' }}, which may be controlled by an external user.
[warning] 28-28: Code injection
Potential code injection in ${{ github.sha }}, which may be controlled by an external user.
[warning] 29-29: Code injection
Potential code injection in ${{ github.event.inputs.extra_payload || '{}' }}, which may be controlled by an external user.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (3.12)
- GitHub Check: build (3.9)
- GitHub Check: build (3.13)
- GitHub Check: build (3.11)
- GitHub Check: build (3.10)
- GitHub Check: build (3.9)
- GitHub Check: build (3.12)
- GitHub Check: build (3.11)
- GitHub Check: build (3.13)
- GitHub Check: build (3.10)
- GitHub Check: tests (firefox)
- GitHub Check: tests (chromium)
- GitHub Check: tests (webkit)
🔇 Additional comments (5)
.github/workflows/trigger-workflow.yml (2)
4-13: Workflow structure converted correctly to reusable workflow_call.The migration from push-based trigger to
workflow_callwithevent_typeandextra_payloadinputs is properly structured and supports dynamic triggering from publish.yml.
27-29: CodeQL flags code injection; assess context for actual risk.CodeQL warns that
event_type,commit_sha, andextra_payloadare user-controlled inputs. In the current flow (called from publish.yml with hardcoded values), this is not exploitable. However, since this is now a reusableworkflow_call, future callers could provide malicious inputs.Mitigation: Consider adding input validation, especially for
event_type. For example, restrict it to a known pattern like[a-z_]+using a regex check before use.Current usage: The payload construction via
jq -n --argjsonand JSON quoting should safely handle the inputs; this is a defensive-coding enhancement rather than a blocking fix.Also applies to: 34-45
.github/workflows/publish.yml (3)
4-8: Trigger configuration and inputs updated correctly.Push trigger now covers the
devbranch andwriter-framework-*tags. Inputs properly renamed tocommit_shaand optionalversionoverride for explicit control. Aligns with the RC/versioning flow.Also applies to: 11-16
102-119: Version determination logic is sound.The fallback chain correctly prioritizes explicit version input → dev RC version → tag-based version. Output naming and step ID are consistent with consumer steps (122, 128, 131).
135-146: RC tag creation and push logic is correct.Creates and pushes annotated tag only on
devbranch, with proper git config and GitHub token. Version source from RC calculation step is correct.
794111b to
a737ca7
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/publish.yml(1 hunks).github/workflows/trigger-workflow.yml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/trigger-workflow.yml
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/publish.yml
154-154: reusable workflow call "./.github/workflows/trigger-workflow.yml@dev" at "uses" is not following the format "owner/repo/path/to/workflow.yml@ref" nor "./path/to/workflow.yml". see https://docs.github.com/en/actions/learn-github-actions/reusing-workflows for more details
(workflow-call)
157-157: context "steps" is not allowed here. available contexts are "github", "inputs", "matrix", "needs", "strategy", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details
(expression)
157-157: property "set_version" is not defined in object type {}
(expression)
🪛 GitHub Check: CodeQL
.github/workflows/publish.yml
[warning] 152-161: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{}}
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (3.10)
- GitHub Check: build (3.12)
- GitHub Check: build (3.13)
- GitHub Check: build (3.9)
- GitHub Check: build (3.11)
- GitHub Check: build (3.10)
- GitHub Check: build (3.12)
- GitHub Check: build (3.13)
- GitHub Check: build (3.9)
- GitHub Check: build (3.11)
- GitHub Check: tests (chromium)
- GitHub Check: tests (firefox)
- GitHub Check: tests (webkit)
6604ac2 to
fca0573
Compare
| needs: build | ||
| if: github.ref_name == 'dev' | ||
| uses: ./.github/workflows/trigger-workflow.yml | ||
| with: | ||
| event_type: framework_updated | ||
| extra_payload: | | ||
| { | ||
| "tag": "writer-framework-${{ needs.build.outputs.version }}", | ||
| "version": "${{ needs.build.outputs.version }}", | ||
| } |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
This autofix suggestion was applied.
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 months ago
To fix this issue, you should add a permissions: section in the workflow. The safest general approach is to set it at the workflow root (just below the name: and on: blocks), which will apply to all jobs unless overridden. This prevents all jobs from having unnecessary broad permissions. For jobs or steps that require elevated permissions (e.g., git push to create tags, or actions needing repository/contents write), set only the specific write permissions actually required for those steps or jobs. Based on the shown workflow, writing tags (git push) and using the default GITHUB_TOKEN for git operations means the job must have at least contents: write. For publishing to PyPI (using PYPI_TOKEN) and triggering downstream workflows, only read permission is required for contents.
Step by step:
- Add a root-level
permissions:block with minimal required permissions, likely:contents: writefor jobs that push tags (build)- If some jobs do not need write, limit them further (e.g., root could be
contents: read, but for simplicity and to avoid step-level overrides, root can becontents: write).
- Optionally, restrict further with job-level overwrites if needed.
- The
trigger-agent-managerjob usessecrets(but only triggers another workflow), socontents: readsuffices.
Best file location: At the root of .github/workflows/publish.yml, below name: publish and above or below on:.
| @@ -1,5 +1,8 @@ | ||
| name: publish | ||
|
|
||
| permissions: | ||
| contents: write | ||
|
|
||
| on: | ||
| push: | ||
| branches: |
UladzislauK-Writer
committed this autofix suggestion 9 months ago.
5fb7724 to
66e971b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/publish.yml (1)
114-124: Major: Missing explicit permissions block for GITHUB_TOKEN.The
trigger-agent-managerjob does not declare an explicitpermissionsblock. While this job uses a reusable workflow, it should still declare what permissions are required. This is a security best practice flagged by static analysis.trigger-agent-manager: needs: build if: github.ref_name == 'dev' + permissions: + contents: read uses: ./.github/workflows/trigger-workflow.yml with: event_type: framework_updated
🧹 Nitpick comments (1)
.github/workflows/publish.yml (1)
42-66: Recommended: Add error handling to the Python version-bump script.The inline Python script (lines 42-66) queries PyPI and inspects git history but lacks error handling for network failures, malformed API responses, or missing git data. Consider wrapping the main logic in a try-except block and providing meaningful error messages.
try: data = requests.get(f"https://pypi.org/pypi/writer/json", timeout=10).json() # ... rest of logic ... except requests.RequestException as e: print(f"Error fetching PyPI data: {e}", file=sys.stderr) sys.exit(1) except (KeyError, ValueError) as e: print(f"Error parsing PyPI response: {e}", file=sys.stderr) sys.exit(1)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/publish.yml(1 hunks).github/workflows/trigger-workflow.yml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/trigger-workflow.yml
🧰 Additional context used
🪛 GitHub Check: CodeQL
.github/workflows/publish.yml
[warning] 115-124: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{}}
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (3.12)
- GitHub Check: build (3.13)
- GitHub Check: build (3.9)
- GitHub Check: build (3.10)
- GitHub Check: build (3.11)
- GitHub Check: tests (webkit)
- GitHub Check: build (3.9)
- GitHub Check: build (3.12)
- GitHub Check: build (3.13)
- GitHub Check: build (3.10)
- GitHub Check: build (3.11)
- GitHub Check: tests (firefox)
- GitHub Check: tests (chromium)
🔇 Additional comments (2)
.github/workflows/publish.yml (2)
75-92: Verify version determination logic handles all expected paths correctly.The
Determine final versionstep implements three paths: manual input, dev branch RC, and tag-based. Ensure that:
- The dev branch always produces a valid RC version from
steps.bump_version.outputs.version- Tag-based versions correctly strip the
writer-framework-prefix- The fallback error message is informative when debugging
This logic appears sound, but it would benefit from additional logging or validation to catch configuration issues early.
60-66: Critical: Unguarded list access causes IndexError when no RC versions exist.Line 62 accesses
rc_versions[-1]without checking if the list is empty. When publishing the first RC version (no prior RC releases on PyPI), this will raise anIndexError.rc_versions = [v for v in data["releases"] if v.startswith(rc_prefix) and "rc" in v] - rc_num = int(re.search(r"rc(\d+)", rc_versions[-1]).group(1)) + 1 if rc_versions else 1 + rc_num = 1 + if rc_versions: + rc_num = int(re.search(r"rc(\d+)", rc_versions[-1]).group(1)) + 1 final = f"{rc_prefix}rc{rc_num}"Likely an incorrect or invalid review comment.
66e971b to
e198185
Compare
e198185 to
a9247bf
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/publish.yml (1)
114-124: Missing explicit permissions block for downstream workflow dispatch.The
trigger-agent-managerjob lacks apermissionsblock. GitHub Actions recommends explicitly declaring GITHUB_TOKEN permissions to minimize the blast radius of potential compromises. Since this job dispatches to an external repository, it should declare the minimal required permissions.Add a permissions block to this job:
trigger-agent-manager: needs: build if: github.ref_name == 'dev' + permissions: + contents: read uses: ./.github/workflows/trigger-workflow.yml
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/publish.yml(1 hunks).github/workflows/trigger-workflow.yml(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-23T12:26:44.366Z
Learnt from: UladzislauK-Writer
Repo: writer/writer-framework PR: 1139
File: .github/workflows/publish.yml:6-6
Timestamp: 2025-09-23T12:26:44.366Z
Learning: GitHub Actions tag filters support glob patterns with regex-like characters including [0-9]+, (rc[0-9]+)?, *, **, +, ?, ! and others as documented in the official GitHub Actions workflow syntax documentation. These patterns work correctly for triggering workflows based on tag names.
Applied to files:
.github/workflows/publish.yml
🪛 actionlint (1.7.8)
.github/workflows/trigger-workflow.yml
9-9: invalid value "choice" for input type of workflow_call event. it must be one of "boolean", "number", or "string"
(syntax-check)
10-10: unexpected key "options" for "inputs at workflow_call event" section. expected one of "default", "description", "required", "type"
(syntax-check)
🪛 GitHub Check: CodeQL
.github/workflows/publish.yml
[warning] 115-124: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{}}
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (3.9)
- GitHub Check: build (3.12)
- GitHub Check: build (3.11)
- GitHub Check: tests (webkit)
- GitHub Check: build (3.13)
- GitHub Check: build (3.10)
- GitHub Check: build (3.9)
- GitHub Check: build (3.11)
- GitHub Check: tests (chromium)
- GitHub Check: tests (firefox)
- GitHub Check: build (3.13)
- GitHub Check: build (3.12)
- GitHub Check: build (3.10)
🔇 Additional comments (6)
.github/workflows/publish.yml (4)
103-112: Verify git push authorization and tag fetch behavior.The RC tag creation and push logic looks correct, but ensure the following:
- The
actions/checkout@v4action (line 26) should fetch all tags by default. Verify this doesn't cause issues if tags haven't been fetched fromorigin.- The
GITHUB_TOKENmust have thecontents: writepermission to push tags. Ensure the job-level permissions (which are missing per the prior comment) includecontents: write.Without explicit permissions, the job inherits the default token permissions, which may or may not include tag-write access depending on repository settings.
53-56: Missing error handling for git log when tag doesn't exist.The git log command on line 53-56 assumes the tag
writer-framework-{latest}exists locally. If the repository was freshly cloned with limited tag fetching or if tags haven't been synchronized, this command will fail with a fatal error.The AI summary mentions this was addressed in past commits, but I don't see a try/except wrapper in the current code. Verify that:
actions/checkout@v4on line 26 fetches all tags (default behavior should includefetch-tags: true)- If the tag fetch is limited, add error handling:
try: log = subprocess.check_output( ["git", "log", f"writer-framework-{latest}..HEAD", "--pretty=%s"], text=True ) except subprocess.CalledProcessError: log = "" # Fallback: assume all commits are new
75-92: Final version determination logic is well-structured.The multi-priority version resolution strategy (explicit input → dev-branch-RC → tag-based → error) is clear and follows expected behavior. The step correctly references the earlier
bump_versionstep output and handles all trigger scenarios.
94-101: Poetry version update and publish steps correctly reference version output.The steps properly reference
steps.set_version.outputs.versionand pass required environment variables. The secret reference for PYPI_TOKEN follows standard patterns..github/workflows/trigger-workflow.yml (2)
25-46: Payload construction and API call look well-structured.The use of
jqto mergecommit_shaintoextra_payloadis robust and ensures valid JSON output. The curl command properly passes the merged payload to the GitHub repository dispatch API. The PAT token reference for authorization is appropriate.One minor note: ensure that the
AGENT_MANAGER_PATsecret has the necessaryrepoorpublic_reposcopes to trigger repository dispatch events in the external repository.
17-18: Permissions declaration is appropriately minimal.The
contents: readpermission is sufficient since the workflow only reads repository context and doesn't modify any repository content. Authorization for the external repository dispatch is properly delegated to theAGENT_MANAGER_PATsecret.
a9247bf to
ab05041
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/publish.yml (2)
103-112: RC tagging requires write permission to the repository.The step creates an annotated tag and pushes it to origin, which requires the
GITHUB_TOKENto havecontents: writepermission. However, neither the workflow root nor thebuildjob declares explicit permissions. While the default token may have these permissions depending on repository settings, it's a best practice to declare them explicitly.Add a permissions block at the workflow root or job level:
+ permissions: + contents: write + jobs: build: runs-on: ubuntu-latestWould you like me to add the explicit permissions block to the workflow?
53-56: Error handling for missing git tag is necessary — add try-except block or explicitly fetch tags.The review comment is correct. actions/checkout@v4 does not fetch tags by default — fetch-tags defaults to false. The workflow at line 26 uses
actions/checkout@v4without thefetch-tagsparameter, so the tagwriter-framework-{latest}will not be available locally. Thesubprocess.check_output()call at lines 53-56 will fail withCalledProcessErrorif the tag is missing, causing the workflow to crash.Add error handling:
- log = subprocess.check_output( - ["git", "log", f"writer-framework-{latest}..HEAD", "--pretty=%s"], - text=True - ) + try: + log = subprocess.check_output( + ["git", "log", f"writer-framework-{latest}..HEAD", "--pretty=%s"], + text=True, + stderr=subprocess.PIPE + ) + except subprocess.CalledProcessError as e: + print(f"Warning: git log failed, tag may not exist: {e.stderr}") + log = ""Alternatively, add
fetch-tags: trueto the checkout action at line 26.
🧹 Nitpick comments (1)
.github/workflows/publish.yml (1)
114-124: trigger-agent-manager job needs explicit permissions to dispatch workflows.The job calls the reusable workflow to dispatch an event to the agent manager repository. Since this crosses repository boundaries (dispatching to WriterInternal/be.agent-manager), explicit permissions should be declared. The
trigger-workflow.ymlreusable workflow uses a personal access token (AGENT_MANAGER_PAT), so it does not depend on the defaultGITHUB_TOKENpermissions here, but it is still a best practice to declare what the job intends to do.Add permissions to the job:
trigger-agent-manager: needs: build if: github.ref_name == 'dev' + permissions: + contents: read uses: ./.github/workflows/trigger-workflow.yml with: event_type: framework_updated extra_payload: | { "tag": "writer-framework-${{ needs.build.outputs.version }}", "version": "${{ needs.build.outputs.version }}" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/publish.yml(1 hunks).github/workflows/trigger-workflow.yml(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-23T12:26:44.366Z
Learnt from: UladzislauK-Writer
Repo: writer/writer-framework PR: 1139
File: .github/workflows/publish.yml:6-6
Timestamp: 2025-09-23T12:26:44.366Z
Learning: GitHub Actions tag filters support glob patterns with regex-like characters including [0-9]+, (rc[0-9]+)?, *, **, +, ?, ! and others as documented in the official GitHub Actions workflow syntax documentation. These patterns work correctly for triggering workflows based on tag names.
Applied to files:
.github/workflows/publish.yml
🪛 GitHub Check: CodeQL
.github/workflows/publish.yml
[warning] 115-124: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{}}
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (3.13)
- GitHub Check: build (3.12)
- GitHub Check: build (3.13)
- GitHub Check: build (3.10)
- GitHub Check: build (3.9)
- GitHub Check: build (3.11)
- GitHub Check: build (3.12)
- GitHub Check: build (3.11)
- GitHub Check: build (3.9)
- GitHub Check: tests (firefox)
- GitHub Check: tests (chromium)
- GitHub Check: tests (webkit)
- GitHub Check: build (3.10)
🔇 Additional comments (2)
.github/workflows/trigger-workflow.yml (2)
4-13: workflow_call syntax is correct.The input types are now properly defined as
string(not the invalidchoicetype from previous iterations). The descriptions are clear and the inputs align with whattrigger-workflow.ymlexpects.
27-45: JSON merging and dispatch payload look sound.Using
jqto safely mergecommit_shainto the extra payload JSON is a solid approach. Theevent_typeis properly quoted in the curl command, and the merged payload is injected as a JSON object. However, ensure that upstream callers validate theextra_payloadinput—if malformed JSON is passed,jqwill fail silently and the workflow will error. Consider adding a validation step or error handling if this becomes an issue.
3 participants
Summary by CodeRabbit