ci: enable and improve docs-sync workflow#3398
Conversation
Re-enable the docs-sync workflow by triggering on merged PRs to main and manual dispatch. Add diff content capture to provide Claude with actual code changes, not just file names. Specify the latest Claude model (sonnet-4-5) and rewrite the prompt with strict documentation rules (default to no changes, single canonical location, minimal edits, one example per concept). Include .cairo files in diff capture and docs pattern matching. Auto-merge generated documentation PRs after status checks pass. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
|
Ohayo, sensei! WalkthroughWorkflow updates unify diff calculation for manual and merged-PR runs, capture limited diffs for selected file types, add Cairo detection, simplify Claude analysis prompt and model, create and auto-merge docs PRs (including branch creation), and clean up the docs-repo clone after run. Changes
Sequence Diagram(s)sequenceDiagram
participant Source as Source Repo
participant Workflow as GitHub Actions
participant Claude as Claude API
participant Docs as Docs Repo
participant GH as GitHub (PR/Branch)
Workflow->>Source: Trigger (workflow_dispatch or PR closed)
Workflow->>Workflow: Compute DIFF_BASE/DIFF_HEAD, CHANGED_FILES
Workflow->>Workflow: Capture DIFF_CONTENT (rs,cairo,toml,md) <=60000 bytes
Workflow->>Claude: Send simplified prompt + "Diff of changed files"
Claude-->>Workflow: Analysis / suggested doc edits
Workflow->>Docs: Create branch, apply edits
Workflow->>GH: Create PR to docs repo
Workflow->>GH: gh pr merge --auto --squash (merge docs PR)
Workflow->>Workflow: Cleanup (remove docs-repo clone)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/docs-sync.yml (1)
144-157:⚠️ Potential issue | 🟠 MajorSame injection concern in git commit messages, sensei.
Lines 152–157 interpolate
github.event.pull_request.titledirectly into a shell string for the commit message. A PR title containing backticks,$(...), or double-quote characters could break the shell command or cause command injection. Use an environment variable instead of inline interpolation:Safer pattern
+ env: + GITHUB_TOKEN: ${{ secrets.CREATE_PR_TOKEN }} + PR_TITLE: ${{ github.event.pull_request.title }} + PR_NUMBER: ${{ github.event.pull_request.number }} + COMMIT_SHA: ${{ github.event.inputs.commit_sha }}Then reference
"$PR_TITLE"etc. in the shell script instead of${{ }}interpolation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs-sync.yml around lines 144 - 157, The commit message block interpolates workflow values directly into a shell string which can be injected; instead, capture values into environment variables (e.g. export or write to GITHUB_ENV like PR_TITLE="${{ github.event.pull_request.title }}" and COMMIT_SHA="${{ github.event.inputs.commit_sha }}" / PR_NUMBER="${{ github.event.pull_request.number }}"), then use the quoted variables in the git commit command (e.g. git commit -m "$COMMIT_MSG" or git commit -m "docs: Update documentation for dojo PR #$PR_NUMBER\n\nUpdates documentation to reflect changes made in: $PR_TITLE\n\nRelated dojo PR: dojoengine/dojo#$PR_NUMBER"), ensuring all variable expansions are double-quoted and construct the multi-line message with a safe printf or a here-doc to preserve newlines.
🧹 Nitpick comments (1)
.github/workflows/docs-sync.yml (1)
175-176:gh pr mergewithout a PR specifier is fragile, sensei.Lines 175 and 186 call
gh pr merge --auto --squashwithout specifying which PR to merge. TheghCLI infers the PR from the current branch, which should work here since you just created and pushed that branch. However, this is brittle — if thegh pr createoutput format or timing changes, or if there's a race condition, the wrong PR (or no PR) could be targeted.Capture the PR URL from
gh pr createand pass it explicitly:Proposed improvement
- gh pr create \ + PR_URL=$(gh pr create \ --title "docs: Update documentation for dojo PR #${{ github.event.pull_request.number }}" \ - --body "..." - gh pr merge --auto --squash + --body "...") + gh pr merge "$PR_URL" --auto --squashAlso applies to: 186-187
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs-sync.yml around lines 175 - 176, The workflow currently calls `gh pr merge --auto --squash` without a PR specifier which is brittle; update the `gh pr create` step to capture its output (the PR URL or number) into a variable and then pass that value explicitly to `gh pr merge` (e.g., `gh pr merge <pr> --auto --squash`); apply this change for both places where `gh pr merge --auto --squash` is called so the merge targets the exact PR returned by `gh pr create`.
🤖 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-sync.yml:
- Around line 33-49: The heredoc delimiter "EOF" is reused for both outputs and
can collide with raw git diff lines; replace the fixed "EOF" delimiter usage for
the GitHub Actions outputs with unique/randomized delimiters per output (e.g.,
generate DELIM1 and DELIM2 with uuidgen or $RANDOM) and use those when writing
"changed_files<<$DELIM1" / "diff_content<<$DELIM2" and their corresponding
terminators, ensuring CHANGED_FILES and DIFF_CONTENT are written between
matching, unique delimiters so the diff won't be truncated; update the echo
lines that reference the delimiters accordingly (look for the lines building
CHANGED_FILES, DIFF_CONTENT and the echo "changed_files<<EOF"/echo
"diff_content<<EOF" blocks).
- Around line 35-36: The DIFF_BASE assignment using `${{
github.event.inputs.commit_sha }}~1` can break for merge commits or the initial
commit; update the workflow to first verify the supplied SHA
(`github.event.inputs.commit_sha`) has a parent and only then set DIFF_BASE to
its parent, otherwise fall back to an explicitly provided base input (e.g.,
`github.event.inputs.base_sha`) or to a safe alternative (like `git rev-parse
--verify <sha>^`/error out with a clear message); change the DIFF_BASE/DIFF_HEAD
logic so DIFF_HEAD still uses `github.event.inputs.commit_sha` but DIFF_BASE is
computed conditionally after validating the commit parent existence.
---
Outside diff comments:
In @.github/workflows/docs-sync.yml:
- Around line 144-157: The commit message block interpolates workflow values
directly into a shell string which can be injected; instead, capture values into
environment variables (e.g. export or write to GITHUB_ENV like PR_TITLE="${{
github.event.pull_request.title }}" and COMMIT_SHA="${{
github.event.inputs.commit_sha }}" / PR_NUMBER="${{
github.event.pull_request.number }}"), then use the quoted variables in the git
commit command (e.g. git commit -m "$COMMIT_MSG" or git commit -m "docs: Update
documentation for dojo PR #$PR_NUMBER\n\nUpdates documentation to reflect
changes made in: $PR_TITLE\n\nRelated dojo PR: dojoengine/dojo#$PR_NUMBER"),
ensuring all variable expansions are double-quoted and construct the multi-line
message with a safe printf or a here-doc to preserve newlines.
---
Nitpick comments:
In @.github/workflows/docs-sync.yml:
- Around line 175-176: The workflow currently calls `gh pr merge --auto
--squash` without a PR specifier which is brittle; update the `gh pr create`
step to capture its output (the PR URL or number) into a variable and then pass
that value explicitly to `gh pr merge` (e.g., `gh pr merge <pr> --auto
--squash`); apply this change for both places where `gh pr merge --auto
--squash` is called so the merge targets the exact PR returned by `gh pr
create`.
| if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then | ||
| # Get changed files for the specified commit | ||
| git fetch origin | ||
| CHANGED_FILES=$(git diff --name-only ${{ github.event.inputs.commit_sha }}~1 ${{ github.event.inputs.commit_sha }}) | ||
| DIFF_BASE="${{ github.event.inputs.commit_sha }}~1" | ||
| DIFF_HEAD="${{ github.event.inputs.commit_sha }}" | ||
| else | ||
| # Get list of changed files in the merged PR | ||
| git fetch origin main | ||
| CHANGED_FILES=$(git diff --name-only ${{ github.event.pull_request.base.sha }} ${{ github.event.pull_request.merge_commit_sha }}) | ||
| DIFF_BASE="${{ github.event.pull_request.base.sha }}" | ||
| DIFF_HEAD="${{ github.event.pull_request.merge_commit_sha }}" | ||
| fi | ||
| CHANGED_FILES=$(git diff --name-only "$DIFF_BASE" "$DIFF_HEAD") | ||
| DIFF_CONTENT=$(git diff "$DIFF_BASE" "$DIFF_HEAD" -- '*.rs' '*.cairo' '*.toml' '*.md' | head -c 60000) | ||
| echo "changed_files<<EOF" >> $GITHUB_OUTPUT | ||
| echo "$CHANGED_FILES" >> $GITHUB_OUTPUT | ||
| echo "EOF" >> $GITHUB_OUTPUT | ||
| echo "diff_content<<EOF" >> $GITHUB_OUTPUT | ||
| echo "$DIFF_CONTENT" >> $GITHUB_OUTPUT | ||
| echo "EOF" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
Potential heredoc delimiter collision can silently truncate output, sensei.
The EOF delimiter used for both changed_files and diff_content GitHub Actions outputs (Lines 44–49) will break if the diff content itself contains a line that is exactly EOF. Given that you're capturing raw git diff output (which could contain anything — file contents, test fixtures, etc.), this is a real risk. A truncated diff means Claude gets incomplete context and may make wrong documentation decisions.
Use a randomized or unique delimiter instead:
Proposed fix
- echo "changed_files<<EOF" >> $GITHUB_OUTPUT
+ DELIMITER="GHADELIM_$(openssl rand -hex 8)"
+ echo "changed_files<<$DELIMITER" >> $GITHUB_OUTPUT
echo "$CHANGED_FILES" >> $GITHUB_OUTPUT
- echo "EOF" >> $GITHUB_OUTPUT
- echo "diff_content<<EOF" >> $GITHUB_OUTPUT
+ echo "$DELIMITER" >> $GITHUB_OUTPUT
+ DELIMITER2="GHADELIM_$(openssl rand -hex 8)"
+ echo "diff_content<<$DELIMITER2" >> $GITHUB_OUTPUT
echo "$DIFF_CONTENT" >> $GITHUB_OUTPUT
- echo "EOF" >> $GITHUB_OUTPUT
+ echo "$DELIMITER2" >> $GITHUB_OUTPUT📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then | |
| # Get changed files for the specified commit | |
| git fetch origin | |
| CHANGED_FILES=$(git diff --name-only ${{ github.event.inputs.commit_sha }}~1 ${{ github.event.inputs.commit_sha }}) | |
| DIFF_BASE="${{ github.event.inputs.commit_sha }}~1" | |
| DIFF_HEAD="${{ github.event.inputs.commit_sha }}" | |
| else | |
| # Get list of changed files in the merged PR | |
| git fetch origin main | |
| CHANGED_FILES=$(git diff --name-only ${{ github.event.pull_request.base.sha }} ${{ github.event.pull_request.merge_commit_sha }}) | |
| DIFF_BASE="${{ github.event.pull_request.base.sha }}" | |
| DIFF_HEAD="${{ github.event.pull_request.merge_commit_sha }}" | |
| fi | |
| CHANGED_FILES=$(git diff --name-only "$DIFF_BASE" "$DIFF_HEAD") | |
| DIFF_CONTENT=$(git diff "$DIFF_BASE" "$DIFF_HEAD" -- '*.rs' '*.cairo' '*.toml' '*.md' | head -c 60000) | |
| echo "changed_files<<EOF" >> $GITHUB_OUTPUT | |
| echo "$CHANGED_FILES" >> $GITHUB_OUTPUT | |
| echo "EOF" >> $GITHUB_OUTPUT | |
| echo "diff_content<<EOF" >> $GITHUB_OUTPUT | |
| echo "$DIFF_CONTENT" >> $GITHUB_OUTPUT | |
| echo "EOF" >> $GITHUB_OUTPUT | |
| if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then | |
| git fetch origin | |
| DIFF_BASE="${{ github.event.inputs.commit_sha }}~1" | |
| DIFF_HEAD="${{ github.event.inputs.commit_sha }}" | |
| else | |
| git fetch origin main | |
| DIFF_BASE="${{ github.event.pull_request.base.sha }}" | |
| DIFF_HEAD="${{ github.event.pull_request.merge_commit_sha }}" | |
| fi | |
| CHANGED_FILES=$(git diff --name-only "$DIFF_BASE" "$DIFF_HEAD") | |
| DIFF_CONTENT=$(git diff "$DIFF_BASE" "$DIFF_HEAD" -- '*.rs' '*.cairo' '*.toml' '*.md' | head -c 60000) | |
| DELIMITER="GHADELIM_$(openssl rand -hex 8)" | |
| echo "changed_files<<$DELIMITER" >> $GITHUB_OUTPUT | |
| echo "$CHANGED_FILES" >> $GITHUB_OUTPUT | |
| echo "$DELIMITER" >> $GITHUB_OUTPUT | |
| DELIMITER2="GHADELIM_$(openssl rand -hex 8)" | |
| echo "diff_content<<$DELIMITER2" >> $GITHUB_OUTPUT | |
| echo "$DIFF_CONTENT" >> $GITHUB_OUTPUT | |
| echo "$DELIMITER2" >> $GITHUB_OUTPUT |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/docs-sync.yml around lines 33 - 49, The heredoc delimiter
"EOF" is reused for both outputs and can collide with raw git diff lines;
replace the fixed "EOF" delimiter usage for the GitHub Actions outputs with
unique/randomized delimiters per output (e.g., generate DELIM1 and DELIM2 with
uuidgen or $RANDOM) and use those when writing "changed_files<<$DELIM1" /
"diff_content<<$DELIM2" and their corresponding terminators, ensuring
CHANGED_FILES and DIFF_CONTENT are written between matching, unique delimiters
so the diff won't be truncated; update the echo lines that reference the
delimiters accordingly (look for the lines building CHANGED_FILES, DIFF_CONTENT
and the echo "changed_files<<EOF"/echo "diff_content<<EOF" blocks).
| DIFF_BASE="${{ github.event.inputs.commit_sha }}~1" | ||
| DIFF_HEAD="${{ github.event.inputs.commit_sha }}" |
There was a problem hiding this comment.
commit_sha~1 may not resolve to the expected parent, sensei.
Line 35 uses ${{ github.event.inputs.commit_sha }}~1 as the diff base. The ~1 suffix works for linear history but can give unexpected results for merge commits (it follows only the first parent). If the manually supplied SHA is the initial commit, ~1 will fail entirely.
Consider using git diff against HEAD~1 only after verifying the commit exists and has a parent, or accept an optional base SHA input:
+ base_commit_sha:
+ description: "Base commit SHA (defaults to commit_sha~1)"
+ required: false
+ type: stringThen:
- DIFF_BASE="${{ github.event.inputs.commit_sha }}~1"
+ DIFF_BASE="${{ github.event.inputs.base_commit_sha || format('{0}~1', github.event.inputs.commit_sha) }}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/docs-sync.yml around lines 35 - 36, The DIFF_BASE
assignment using `${{ github.event.inputs.commit_sha }}~1` can break for merge
commits or the initial commit; update the workflow to first verify the supplied
SHA (`github.event.inputs.commit_sha`) has a parent and only then set DIFF_BASE
to its parent, otherwise fall back to an explicitly provided base input (e.g.,
`github.event.inputs.base_sha`) or to a safe alternative (like `git rev-parse
--verify <sha>^`/error out with a clear message); change the DIFF_BASE/DIFF_HEAD
logic so DIFF_HEAD still uses `github.event.inputs.commit_sha` but DIFF_BASE is
computed conditionally after validating the commit parent existence.
| direct_prompt: | | ||
| I need you to analyze the changes in this dojo repository PR and update the documentation in the dojoengine/book repository accordingly. | ||
| Analyze changes in this dojo repository and update documentation | ||
| in the dojoengine/book repository ONLY if user-facing behavior changed. | ||
|
|
||
| **Change Information:** | ||
| - Title: ${{ github.event.pull_request.title || format('Manual trigger for commit {0}', github.event.inputs.commit_sha) }} | ||
| - Description: ${{ github.event.pull_request.body || 'Manually triggered documentation sync' }} | ||
| - Files changed: ${{ steps.changed-files.outputs.changed_files }} | ||
| - Commit SHA: ${{ github.event.pull_request.merge_commit_sha || github.event.inputs.commit_sha }} | ||
|
|
||
| **Your tasks:** | ||
| 1. Review the changed files and PR description to understand what functionality was added, modified, or removed | ||
| 2. Check the docs-repo directory to see what documentation currently exists | ||
| 3. Determine if any existing documentation needs updates or if new documentation should be created | ||
| 4. If updates are needed: | ||
| - Create or update the appropriate documentation files in the docs-repo directory | ||
| - Ensure the documentation accurately reflects the current state of the dojo | ||
| - Follow the existing documentation style and structure | ||
| - Focus on user-facing changes, API changes, new features, or breaking changes | ||
|
|
||
| **Important guidelines:** | ||
| - Only create documentation updates if they are actually needed | ||
| - Don't document internal implementation details unless they affect usage | ||
| - If no documentation updates are needed, simply state that and exit | ||
| - DO NOT create git branches, commits, or PRs - just update the files | ||
|
|
||
| The docs repository is checked out in the `docs-repo` directory. Please analyze the dojo changes and update the documentation files accordingly. | ||
| **Diff of changed files:** | ||
| ${{ steps.changed-files.outputs.diff_content }} |
There was a problem hiding this comment.
PR title and body are interpolated unsanitized into the prompt — injection risk, sensei.
Lines 99–100 interpolate github.event.pull_request.title and github.event.pull_request.body directly into the YAML string via ${{ }}. A PR author could craft a title or body containing YAML special characters that break the workflow, or inject prompt instructions that manipulate Claude's behavior (prompt injection).
For the YAML/shell breakage angle: since this is inside a with: block and the direct_prompt is a YAML scalar, a PR body containing unescaped YAML could corrupt the workflow. For the prompt injection angle: a malicious PR body could instruct Claude to make arbitrary file edits in the docs repo.
Consider sanitizing or truncating the PR body, or passing it via an environment variable / file rather than inline interpolation.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/docs-sync.yml (2)
165-186:⚠️ Potential issue | 🟠 MajorOhayo sensei — auto-merging AI-generated docs PRs without human review is risky.
Lines 175 and 186 run
gh pr merge "$PR_URL" --auto --squashimmediately after PR creation. This means Claude's documentation changes will be merged automatically once branch protection checks pass (or immediately if none are configured on thedojoengine/bookrepo).If Claude misinterprets a change, produces inaccurate docs, or is influenced by prompt injection (as flagged earlier), those errors land in production docs with no human gate.
Consider either:
- Removing
--autoand letting a maintainer review/merge, or- At minimum, ensuring the
dojoengine/bookrepo has branch protection rules requiring at least one approval before merge.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs-sync.yml around lines 165 - 186, Change the workflow to stop auto-merging AI-generated docs PRs: remove the --auto flag from the two gh pr merge "$PR_URL" --auto --squash invocations (the gh pr merge lines that run immediately after creating PR_URL) so merges require a human maintainer to approve/merge, or alternatively enforce a repository branch-protection rule requiring at least one approval on the dojoengine/book repo before merges; update the workflow to use gh pr merge "$PR_URL" --squash (or simply leave merging to maintainers) and/or add a short comment/log notifying maintainers that a PR was created and needs review.
144-158:⚠️ Potential issue | 🟠 MajorShell injection via unescaped
${{ }}interpolation in commit messages, sensei.Lines 145, 148, 152, 155, and 157 interpolate
${{ github.event.inputs.commit_sha }}and${{ github.event.pull_request.title }}/.numberdirectly into shell strings. A PR title containing double quotes (e.g.,feat: support "new" syntax) will break thegit commit -mcommand — or worse, allow arbitrary shell execution.The safer pattern is to pass these values through environment variables:
Proposed fix (example for the PR-triggered commit)
+ env: + PR_TITLE: ${{ github.event.pull_request.title }} + PR_NUMBER: ${{ github.event.pull_request.number }} + COMMIT_SHA: ${{ github.event.inputs.commit_sha }} run: | ... if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then - git commit -m "docs: Update documentation for dojo commit ${{ github.event.inputs.commit_sha }} + git commit -m "docs: Update documentation for dojo commit ${COMMIT_SHA} Updates documentation to reflect changes made in commit: - ${{ github.event.inputs.commit_sha }} + ${COMMIT_SHA} Manually triggered documentation sync" else - git commit -m "docs: Update documentation for dojo PR #${{ github.event.pull_request.number }} + git commit -m "docs: Update documentation for dojo PR #${PR_NUMBER} Updates documentation to reflect changes made in: - ${{ github.event.pull_request.title }} + ${PR_TITLE} - Related dojo PR: dojoengine/dojo#${{ github.event.pull_request.number }}" + Related dojo PR: dojoengine/dojo#${PR_NUMBER}" fiThis applies equally to the
gh pr create --title/--bodyarguments on Lines 165–186.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs-sync.yml around lines 144 - 158, The git commit and gh pr create commands interpolate unescaped GitHub event values (e.g., ${{ github.event.inputs.commit_sha }}, ${{ github.event.pull_request.title }}, ${{ github.event.pull_request.number }}) directly into double-quoted shell strings, which can be exploited or break when values contain quotes; fix by assigning those event values to safe environment variables (e.g., COMMIT_SHA="${{ github.event.inputs.commit_sha }}", PR_TITLE="${{ github.event.pull_request.title }}", PR_BODY="...", PR_NUMBER="${{ github.event.pull_request.number }}") and then use those variables in commands with proper quoting and safer APIs: write multi-line commit/PR bodies to a temporary file and call git commit -F "$MSG_FILE" (instead of git commit -m "...") and call gh pr create --title "$PR_TITLE" --body "$PR_BODY" (or --body-file "$BODY_FILE") so the values are not interpolated into shell command text; update the git commit branches in the workflow where git commit -m and gh pr create --title/--body are used to use COMMIT_SHA, PR_TITLE, PR_BODY, PR_NUMBER and commit/PR file flags instead of direct ${{ }} interpolation.
🧹 Nitpick comments (1)
.github/workflows/docs-sync.yml (1)
18-21:id-token: writepermission appears unused, sensei.No step in this workflow uses OIDC token exchange. Dropping this permission tightens the security posture to least-privilege.
Proposed fix
permissions: contents: read pull-requests: read - id-token: write🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs-sync.yml around lines 18 - 21, Remove the unused OIDC permission by deleting the "id-token: write" entry from the workflow permissions block so the permissions become only "contents: read" and "pull-requests: read"; update the permissions section in .github/workflows/docs-sync.yml (the permissions: block) to remove id-token to enforce least-privilege.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/docs-sync.yml:
- Around line 165-186: Change the workflow to stop auto-merging AI-generated
docs PRs: remove the --auto flag from the two gh pr merge "$PR_URL" --auto
--squash invocations (the gh pr merge lines that run immediately after creating
PR_URL) so merges require a human maintainer to approve/merge, or alternatively
enforce a repository branch-protection rule requiring at least one approval on
the dojoengine/book repo before merges; update the workflow to use gh pr merge
"$PR_URL" --squash (or simply leave merging to maintainers) and/or add a short
comment/log notifying maintainers that a PR was created and needs review.
- Around line 144-158: The git commit and gh pr create commands interpolate
unescaped GitHub event values (e.g., ${{ github.event.inputs.commit_sha }}, ${{
github.event.pull_request.title }}, ${{ github.event.pull_request.number }})
directly into double-quoted shell strings, which can be exploited or break when
values contain quotes; fix by assigning those event values to safe environment
variables (e.g., COMMIT_SHA="${{ github.event.inputs.commit_sha }}",
PR_TITLE="${{ github.event.pull_request.title }}", PR_BODY="...", PR_NUMBER="${{
github.event.pull_request.number }}") and then use those variables in commands
with proper quoting and safer APIs: write multi-line commit/PR bodies to a
temporary file and call git commit -F "$MSG_FILE" (instead of git commit -m
"...") and call gh pr create --title "$PR_TITLE" --body "$PR_BODY" (or
--body-file "$BODY_FILE") so the values are not interpolated into shell command
text; update the git commit branches in the workflow where git commit -m and gh
pr create --title/--body are used to use COMMIT_SHA, PR_TITLE, PR_BODY,
PR_NUMBER and commit/PR file flags instead of direct ${{ }} interpolation.
---
Duplicate comments:
In @.github/workflows/docs-sync.yml:
- Around line 35-36: Replace the brittle DIFF_BASE calculation that uses
"commit_sha~1": add an optional workflow input (e.g., base_commit_sha) and set
DIFF_BASE to that when provided; if not provided, compute a safe parent for
DIFF_BASE by checking that the provided commit (commit_sha) actually has a
parent (e.g., inspect parents via git rev-list or rev-parse) and fail or fall
back with a clear error message if no parent exists (handle initial commit and
merge-commit cases), while leaving DIFF_HEAD as DIFF_HEAD="${{
github.event.inputs.commit_sha }}".
- Around line 94-105: The prompt injection risk arises from interpolating
github.event.pull_request.title and github.event.pull_request.body directly into
the direct_prompt string; instead sanitize or escape those values before
inclusion (e.g., assign to safe variables and run a JSON-escape or
whitespace/newline replacement), or fall back to a controlled field (like
github.event.inputs.commit_sha or a sanitized summary) when pull_request fields
are present; update the direct_prompt construction to reference the sanitized
variables (used where github.event.pull_request.title/body are currently
referenced) so untrusted PR text cannot inject arbitrary prompt instructions.
- Around line 44-49: Replace the fixed "EOF" heredoc delimiters used when
writing to $GITHUB_OUTPUT for CHANGED_FILES and DIFF_CONTENT with a randomized
delimiter to avoid truncation if the diff contains a literal "EOF": generate a
delimiter (e.g., DELIMITER=$(openssl rand -hex 8)), write the start marker using
that variable (echo "changed_files<<$DELIMITER" >> $GITHUB_OUTPUT and echo
"diff_content<<$DELIMITER" >> $GITHUB_OUTPUT), emit the variable's contents, and
terminate each heredoc with the same $DELIMITER so both the CHANGED_FILES and
DIFF_CONTENT heredocs are robust against delimiter collisions.
---
Nitpick comments:
In @.github/workflows/docs-sync.yml:
- Around line 18-21: Remove the unused OIDC permission by deleting the
"id-token: write" entry from the workflow permissions block so the permissions
become only "contents: read" and "pull-requests: read"; update the permissions
section in .github/workflows/docs-sync.yml (the permissions: block) to remove
id-token to enforce least-privilege.
Description
Enable the docs-sync workflow and improve its reliability by refining how it calculates diffs, determines when docs updates are needed, and integrates with Claude. The prompt now includes concrete rules about when documentation should be updated to avoid unnecessary changes.
Related issue
N/A
Tests
Added to documentation?
Checklist
Summary by CodeRabbit
Documentation
Chores