Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds two new GitHub Actions workflows: one creates a versioned docs branch in Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Event
participant WF as Docs Branch Create Workflow
participant GHAPI as GitHub API
participant MM as mattermost/mattermost
participant DOCS as mattermost/docs
GH->>WF: PR closed & merged (head branch matches v*...-documentation)
WF->>GHAPI: gh api -> list milestones (MM)
GHAPI-->>WF: milestones list
WF->>WF: select earliest milestone, extract vMAJOR.MINOR -> VERSION
WF->>GHAPI: check refs/heads/{VERSION}-documentation in DOCS
alt branch not present
WF->>GHAPI: get master sha from DOCS
WF->>GHAPI: create ref refs/heads/{VERSION}-documentation from sha
GHAPI-->>DOCS: branch created
end
WF->>WF: append step summary with merged branch and docs branch info
sequenceDiagram
participant GH as GitHub Event
participant WF as Docs PR Sync Workflow
participant GHAPI as GitHub API
participant DEV as Dev PR (origin repo)
GH->>WF: Docs PR closed (branch: docs/<repo>-pr-<number>)
WF->>WF: parse dev repo and PR number from branch name
WF->>GHAPI: remove label `Docs/Needed` on DEV PR (continue on fail)
WF->>GHAPI: add label `Docs/Done` on DEV PR
GHAPI-->>DEV: labels updated
WF->>GHAPI: post comment on DEV PR with merge/close status and docs PR link
GHAPI-->>DEV: comment posted
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
Newest code from mattermost has been published to preview environment for Git SHA 3b4b9f6 |
|
Newest code from mattermost has been published to preview environment for Git SHA 891878f |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/docs-branch-create.yml (1)
114-127: Summary may be misleading when branch already exists.The summary step runs regardless of whether the branch was created or already existed, but always displays "Docs Branch Created". Consider capturing the creation status and adjusting the summary text accordingly, or this could be addressed in a follow-up.
✨ Proposed enhancement
In the "Create docs branch" step, output a status:
if [ -n "$EXISTS" ]; then echo "::notice::Branch '${BRANCH}' already exists in mattermost/docs — nothing to do." + echo "created=false" >> "$GITHUB_OUTPUT" exit 0 fi # ... branch creation logic ... echo "✓ Created branch '${BRANCH}' in mattermost/docs from master (${SHA})" + echo "created=true" >> "$GITHUB_OUTPUT"Then in the Summary step, conditionally adjust the heading.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs-branch-create.yml around lines 114 - 127, The Summary step always prints "Docs Branch Created" even when the branch already existed; update the "Create docs branch" step to emit a status output (e.g., outputs.branch_created = "true" or "false" or similar) indicating whether a new branch was created, and then modify the Summary step (which uses MERGED_BRANCH and NEW_BRANCH) to read that new output and conditionally set the heading text (e.g., "Docs Branch Created" vs "Docs Branch Exists") and/or adjust wording in the table based on that output; reference the step name "Create docs branch" for where to add the output and the step that currently prints the summary ("Summary") to consume it..github/workflows/docs-pr-sync.yml (1)
94-113: Comment text implies label change even when potentially misleading.The comment states "label has been updated from
Docs/Needed→Docs/Done" in all cases. If the docs PR was closed without merging (and if that case receives different label treatment per the previous comment), the message should reflect the actual label state.✨ Conditional message text
gh pr comment "$DEV_PR" \ --repo "$DEV_REPO" \ - --body "${ICON} The associated docs PR ([mattermost/docs#${DOCS_PR_NUMBER}](https://github.com/mattermost/docs/pull/${DOCS_PR_NUMBER})) has been **${STATUS}**. This PR's label has been updated from \`Docs/Needed\` → \`Docs/Done\`." + --body "${ICON} The associated docs PR ([mattermost/docs#${DOCS_PR_NUMBER}](https://github.com/mattermost/docs/pull/${DOCS_PR_NUMBER})) has been **${STATUS}**.${LABEL_MSG}"Where
LABEL_MSGis set earlier based on merge status and actual label changes applied.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs-pr-sync.yml around lines 94 - 113, The comment always claims the label changed which can be misleading; modify the "Comment on dev PR" step to build a descriptive LABEL_MSG (based on DOCS_PR_MERGED and whatever label logic you set earlier) and use that LABEL_MSG in the gh pr comment body instead of the hardcoded "label has been updated from `Docs/Needed` → `Docs/Done`"; specifically update the variables STATUS and ICON logic to also set LABEL_MSG, then replace the static label sentence in the --body payload with the dynamic ${LABEL_MSG} so the posted message accurately reflects real label state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docs-branch-create.yml:
- Around line 41-44: The Log merged branch step interpolates the untrusted value
github.event.pull_request.head.ref directly into the shell, creating a shell
injection risk; change the workflow to pass that value as an environment
variable (e.g., HEAD_REF) on the step and reference the env var inside the run
block (use $HEAD_REF) instead of using ${{ github.event.pull_request.head.ref }}
directly—follow the same pattern used in docs-pr-sync.yml to
sanitize/encapsulate HEAD_REF for the Log merged branch step.
---
Nitpick comments:
In @.github/workflows/docs-branch-create.yml:
- Around line 114-127: The Summary step always prints "Docs Branch Created" even
when the branch already existed; update the "Create docs branch" step to emit a
status output (e.g., outputs.branch_created = "true" or "false" or similar)
indicating whether a new branch was created, and then modify the Summary step
(which uses MERGED_BRANCH and NEW_BRANCH) to read that new output and
conditionally set the heading text (e.g., "Docs Branch Created" vs "Docs Branch
Exists") and/or adjust wording in the table based on that output; reference the
step name "Create docs branch" for where to add the output and the step that
currently prints the summary ("Summary") to consume it.
In @.github/workflows/docs-pr-sync.yml:
- Around line 94-113: The comment always claims the label changed which can be
misleading; modify the "Comment on dev PR" step to build a descriptive LABEL_MSG
(based on DOCS_PR_MERGED and whatever label logic you set earlier) and use that
LABEL_MSG in the gh pr comment body instead of the hardcoded "label has been
updated from `Docs/Needed` → `Docs/Done`"; specifically update the variables
STATUS and ICON logic to also set LABEL_MSG, then replace the static label
sentence in the --body payload with the dynamic ${LABEL_MSG} so the posted
message accurately reflects real label state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 87dbd9b8-d84a-4d11-8869-e08ecb8684ef
📒 Files selected for processing (2)
.github/workflows/docs-branch-create.yml.github/workflows/docs-pr-sync.yml
|
Newest code from mattermost has been published to preview environment for Git SHA 144bd29 |
|
Newest code from mattermost has been published to preview environment for Git SHA d6a8e35 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.github/workflows/docs-branch-create.yml (1)
97-104: Make branch creation resilient to concurrent runs.Current check-then-create can still fail with a 422 if another run creates the branch between Line 98 and Line 109. Treat “already exists” on create as a success path.
Suggested idempotent pattern
- gh api repos/mattermost/docs/git/refs \ - --method POST \ - -f "ref=refs/heads/${BRANCH}" \ - -f "sha=${SHA}" + if gh api repos/mattermost/docs/git/refs \ + --method POST \ + -f "ref=refs/heads/${BRANCH}" \ + -f "sha=${SHA}" 2>/dev/null; then + echo "✓ Created branch '${BRANCH}' in mattermost/docs from master (${SHA})" + else + EXISTS_AFTER=$(gh api "repos/mattermost/docs/branches/${BRANCH}" --jq '.name' 2>/dev/null || echo "") + if [ -n "$EXISTS_AFTER" ]; then + echo "::notice::Branch '${BRANCH}' was created by another run — continuing." + exit 0 + fi + echo "::error::Failed to create branch '${BRANCH}'." + exit 1 + fi - - echo "✓ Created branch '${BRANCH}' in mattermost/docs from master (${SHA})"Also applies to: 109-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs-branch-create.yml around lines 97 - 104, The branch-exists check (EXISTS) is racy: between checking EXISTS and the gh api call that creates the branch the branch may be created by another run, causing a 422 and failing the job; update the branch-creation gh api invocation(s) so that a 422 / “already exists” response is treated as success (no-op) instead of an error — for example by capturing the HTTP status or error output from the gh api create branch call and returning success when the response indicates the branch already exists; apply this change to the create-branch call that follows the EXISTS check and the second create-branch occurrence mentioned (the calls that reference BRANCH and use gh api to create branches).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docs-branch-create.yml:
- Around line 35-37: The current conditional uses
contains(github.event.pull_request.head.ref, '-documentation') which also
matches non-release branches; tighten it by requiring the branch to be a release
branch too (e.g., use startsWith(github.event.pull_request.head.ref, 'release/')
&& contains(github.event.pull_request.head.ref, '-documentation') or use a
match/regex equivalent) so only branches following the release docs pattern
trigger the job; update the if condition that references
github.event.pull_request.head.ref accordingly.
- Around line 30-33: The job "create-next-version-branch" invokes the GitHub CLI
to create refs (gh api repos/mattermost/docs/git/refs POST) but does not declare
job-level permissions, so it can fail if default workflow permissions are
read-only; update the job (and the other similar jobs) to include explicit
permissions allowing write access to repository contents (e.g., add a
permissions block with contents: write) at the job level so the gh api call can
create refs successfully.
In @.github/workflows/docs-pr-sync.yml:
- Around line 13-17: Update the top-of-file comment to match the workflow
implementation: replace references to docs/enterprise-pr-* and "parses the dev
PR reference from the PR body" with the actual handled pattern docs/desktop-pr-*
and state that the workflow parses the dev PR reference from the branch name;
keep the note about removing docs/needed and adding docs/done so the comment
accurately documents the behavior of the workflow file.
- Around line 31-35: The current startsWith checks
(startsWith(github.event.pull_request.head.ref, ...)) allow branches like
docs/mattermost-pr-12-pr-999 and the parsing logic that extracts the PR id from
the last "-pr-" will pick the wrong number; replace the prefix checks with exact
regex matching using matches(...) to require full branch names such as
^docs/(mattermost|mattermost-mobile|desktop)-pr-[0-9]+$ and update the parsing
logic (the code that splits/parses after "-pr-") to extract the numeric ID
deterministically (e.g., use a single regex capture for the digits after the
single "-pr-" suffix) so only branches that exactly match
docs/<flavor>-pr-<number> are accepted and the correct PR number is used.
---
Nitpick comments:
In @.github/workflows/docs-branch-create.yml:
- Around line 97-104: The branch-exists check (EXISTS) is racy: between checking
EXISTS and the gh api call that creates the branch the branch may be created by
another run, causing a 422 and failing the job; update the branch-creation gh
api invocation(s) so that a 422 / “already exists” response is treated as
success (no-op) instead of an error — for example by capturing the HTTP status
or error output from the gh api create branch call and returning success when
the response indicates the branch already exists; apply this change to the
create-branch call that follows the EXISTS check and the second create-branch
occurrence mentioned (the calls that reference BRANCH and use gh api to create
branches).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 06f32224-0ea5-4c34-85d2-a6ec5f00c5fd
📒 Files selected for processing (2)
.github/workflows/docs-branch-create.yml.github/workflows/docs-pr-sync.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docs-branch-create.yml:
- Line 134: The workflow message uses the label string "docs/needed" but other
workflow docs-pr-sync.yml uses "Docs/Needed" (case-sensitive mismatch); update
the echo string that writes to GITHUB_STEP_SUMMARY (the line that references
NEW_BRANCH and uses the label text) to use the exact label casing "Docs/Needed"
so both workflows reference the same label; ensure any other occurrences of
"docs/needed" in this workflow are changed to "Docs/Needed" for consistency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b3aa0d76-0217-41f5-b3c9-83add9e8a536
📒 Files selected for processing (1)
.github/workflows/docs-branch-create.yml
|
Newest code from mattermost has been published to preview environment for Git SHA 36ab3e8 |
|
Newest code from mattermost has been published to preview environment for Git SHA 73d204b |
|
Newest code from mattermost has been published to preview environment for Git SHA bbbb872 |
|
Newest code from mattermost has been published to preview environment for Git SHA 2380c09 |
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
|
Newest code from mattermost has been published to preview environment for Git SHA 998a148 |
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/docs-pr-sync.yml (1)
31-33:⚠️ Potential issue | 🟡 MinorAvoid failing on unrelated
docs/*branches.Line 32 runs this job for any
docs/ref, but Line 46 rejects everything outside the three automation branch families. A normal contributor branch likedocs/fix-navwould therefore close with a red workflow even though nothing needs syncing. Narrow the gate to the generated prefixes, or make the parser exit cleanly when the ref is out of scope.Suggested guard
- if: | - startsWith(github.event.pull_request.head.ref, 'docs/') + if: | + startsWith(github.event.pull_request.head.ref, 'docs/mattermost-pr-') || + startsWith(github.event.pull_request.head.ref, 'docs/mattermost-mobile-pr-') || + startsWith(github.event.pull_request.head.ref, 'docs/desktop-pr-')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs-pr-sync.yml around lines 31 - 33, The current job-level if uses startsWith(github.event.pull_request.head.ref, 'docs/') which triggers on any docs/* branch and later your parser rejects refs outside the three automation branch families; narrow the gate by replacing that single startsWith check with a whitelist check for the actual generated prefixes (e.g. test the ref against an array of allowed prefixes using any(startsWith(...)) or a single regex that matches only the automation prefixes), or add an early workflow step that inspects github.event.pull_request.head.ref and exits successfully when the ref is not in the allowed prefixes so the job does not fail for unrelated contributor branches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docs-branch-create.yml:
- Around line 95-107: The workflow reports "Docs Branch Created" even when it
skipped creation; make the step set and check an explicit flag instead of
assuming creation: initialize a CREATED (or BRANCH_CREATED) env variable before
the EXISTS check, set it to true only when the POST/branch-creation command
actually runs, and leave it false when the script exits on existing branch
(EXISTS). Then drive the summary/notice text from that CREATED flag (print "Docs
Branch Created" only if true, otherwise print a no-op notice), and apply the
same flag-based logic to the other branch-creation block referenced in the diff.
In @.github/workflows/docs-pr-sync.yml:
- Around line 45-54: Remove the duplicated orphaned error-handling block that
follows the regex guard for HEAD_REF (the duplicate echo/exit/fi after setting
REPO_NAME and PR_NUMBER); the regex match guard using BASH_REMATCH should end
once REPO_NAME and PR_NUMBER are set, so delete the extra echo "::error::Could
not parse repo name and PR number from branch '$HEAD_REF'." plus the subsequent
exit 1 and stray fi to restore valid Bash syntax and leave only the original
if/fi guard that assigns REPO_NAME and PR_NUMBER.
---
Duplicate comments:
In @.github/workflows/docs-pr-sync.yml:
- Around line 31-33: The current job-level if uses
startsWith(github.event.pull_request.head.ref, 'docs/') which triggers on any
docs/* branch and later your parser rejects refs outside the three automation
branch families; narrow the gate by replacing that single startsWith check with
a whitelist check for the actual generated prefixes (e.g. test the ref against
an array of allowed prefixes using any(startsWith(...)) or a single regex that
matches only the automation prefixes), or add an early workflow step that
inspects github.event.pull_request.head.ref and exits successfully when the ref
is not in the allowed prefixes so the job does not fail for unrelated
contributor branches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f2b788c8-5d17-4c48-8662-628ca140190c
📒 Files selected for processing (2)
.github/workflows/docs-branch-create.yml.github/workflows/docs-pr-sync.yml
|
Newest code from mattermost has been published to preview environment for Git SHA 91e388f |
|
Newest code from mattermost has been published to preview environment for Git SHA 44de47e |
|
Newest code from mattermost has been published to preview environment for Git SHA 12a89dd |
|
Newest code from mattermost has been published to preview environment for Git SHA 0ec0b86 |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/docs-branch-create.yml (1)
19-21:⚠️ Potential issue | 🟡 MinorUse consistent label casing in the top-of-file behaviour notes.
The note still says
docs/needed; elsewhere this automation usesDocs/Needed. Keep casing consistent to prevent operator error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs-branch-create.yml around lines 19 - 21, Update the top-of-file behaviour note to use the same label casing used elsewhere by replacing the lowercase string "docs/needed" with the canonical "Docs/Needed" so the comment matches the automation's expected label casing and avoids operator confusion; modify the comment text that currently reads "docs/needed.yml in the code repos" (the top-of-file behaviour note) to "Docs/Needed.yml in the code repos"..github/workflows/docs-pr-sync.yml (1)
13-17:⚠️ Potential issue | 🟡 MinorAlign the behaviour header with actual branch and label values.
The header text is stale versus implementation: it omits
mattermost-mobileand uses lowercase label names while the workflow usesDocs/NeededandDocs/Done.📝 Suggested doc-only update
-# When a docs/mattermost-pr-* or docs/desktop-pr-* PR is closed +# When a docs/mattermost-pr-*, docs/mattermost-mobile-pr-*, or docs/desktop-pr-* PR is closed ... -# • Removes docs/needed -# • Adds docs/done +# • Removes Docs/Needed +# • Adds Docs/Done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs-pr-sync.yml around lines 13 - 17, Update the header comment to match the workflow implementation: include the missing branch pattern for mattermost-mobile (e.g., docs/mattermost-mobile-pr-*) and use the exact capitalized label names "Docs/Needed" and "Docs/Done" (instead of lowercase) so the header accurately reflects the branch and label values used by the workflow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docs-branch-create.yml:
- Around line 61-85: The selection logic for NEXT_TITLE must filter only
milestones whose .title contains a vMAJOR.MINOR token before sorting and picking
the first; update the gh api jq pipeline used to compute NEXT_TITLE (the block
assigning NEXT_TITLE) to add a select(.title | test("v[0-9]+\\.[0-9]+")) (or
equivalent regex-match) right after select(.state == "open") so only versioned
milestones are considered when sorting and taking first.
---
Duplicate comments:
In @.github/workflows/docs-branch-create.yml:
- Around line 19-21: Update the top-of-file behaviour note to use the same label
casing used elsewhere by replacing the lowercase string "docs/needed" with the
canonical "Docs/Needed" so the comment matches the automation's expected label
casing and avoids operator confusion; modify the comment text that currently
reads "docs/needed.yml in the code repos" (the top-of-file behaviour note) to
"Docs/Needed.yml in the code repos".
In @.github/workflows/docs-pr-sync.yml:
- Around line 13-17: Update the header comment to match the workflow
implementation: include the missing branch pattern for mattermost-mobile (e.g.,
docs/mattermost-mobile-pr-*) and use the exact capitalized label names
"Docs/Needed" and "Docs/Done" (instead of lowercase) so the header accurately
reflects the branch and label values used by the workflow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fd5af90a-3a7a-4d34-836d-afd82f6dd37f
📒 Files selected for processing (2)
.github/workflows/docs-branch-create.yml.github/workflows/docs-pr-sync.yml
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Newest code from mattermost has been published to preview environment for Git SHA b3c202b |
Process Flow
1. Release cycle begins
When v11.5-documentation is merged into master in mattermost/docs, docs-branch-create.yml fires. It finds the earliest open milestone in mattermost/mattermost (e.g. v11.6.0), derives the branch name v11.6-documentation, and creates it from master in mattermost/docs. This is the single active docs branch for the cycle.
2. Dev PR is opened
A developer opens a PR in one of the four code repos. The existing Documentation Impact Review workflow runs, analyzes the diff with Claude, and if documentation changes are warranted, applies the Docs/Needed label automatically.
3. Docs PR is created
The Docs/Needed label event triggers docs-needed.yml. It:
4. Docs are written and merged
The docs writer reviews and edits the draft. The docs PR (docs/mattermost-pr-1234 → v11.6-documentation) is merged by the docs team.
5. Dev PR labels are updated
Merging (or closing) the docs PR triggers docs-pr-sync.yml. It parses the source repo and PR number from the branch name, then on the original dev PR: removes Docs/Needed, adds Docs/Done, and posts a comment with the outcome.
6. Release ships
Once all docs PRs for the milestone are merged into v11.6-documentation, that branch is merged into master — which triggers docs-branch-create.yml again, starting the next cycle with v11.7-documentation.
Server PR: mattermost/mattermost#35979. Mobile/Desktop PRs to be opened.