Skip to content

ci: notify help-docs hub on docs changes #1084

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
saravmajestic wants to merge 5 commits into
base: main
Choose a base branch
from
+83 −0
Open

ci: notify help-docs hub on docs changes #1084

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f3242ad
ci: notify help-docs hub on docs changes (bidirectional sync)
saravmajestic Aug 7, 2026
5fc6c28
ci: resolve original PR author for hub notifications
saravmajestic Aug 7, 2026
1ceb638
ci: drop unused email lookup from hub notify payload
saravmajestic Aug 7, 2026
e897998
ci: harden notify (fallback, concurrency, jq payload, pinned action, …
saravmajestic Aug 7, 2026
cba8741
ci: address consensus review — secrets (not vars) for App ID, robust …
saravmajestic Aug 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions .github/workflows/notify-help-docs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
name: Notify help-docs (docs changed)

# When a push to the default branch changes published docs, tell the help-docs
# hub to promote the change up (product -> help-docs) as a review PR. Requires
# the shared docs-sync GitHub App as repo secrets: DOCS_SYNC_APP_ID and
# DOCS_SYNC_APP_PRIVATE_KEY — if either is unset the job skips (no red run).
# The loop guard checks every commit in the push for the [docs-sync] marker, so
# it holds across squash, rebase, and merge-commit strategies.

on:

@cubic-dev-ai cubic-dev-ai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The push trigger on main fires for any push that touches docs/docs/**, including direct pushes to the branch, not just merged pull requests. This is broader than the stated intent of notifying only when a docs PR merges. Consider a merged-only trigger (e.g., a closed pull_request event filtered on github.event.pull_request.merged == true) if direct pushes to main should not trigger the dispatch.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/notify-help-docs.yml, line 8:

<comment>The `push` trigger on `main` fires for any push that touches `docs/docs/**`, including direct pushes to the branch, not just merged pull requests. This is broader than the stated intent of notifying only when a docs PR merges. Consider a merged-only trigger (e.g., a closed `pull_request` event filtered on `github.event.pull_request.merged == true`) if direct pushes to main should not trigger the dispatch.</comment>

<file context>
@@ -0,0 +1,34 @@
+# GitHub App: set vars.DOCS_SYNC_APP_ID and secrets.DOCS_SYNC_APP_PRIVATE_KEY.
+# Sync-generated merges carry a [docs-sync] marker and are skipped (no loop).
+
+on:
+  push:
+    branches: [main]
</file context>

push:
branches: [main]
paths: ["docs/docs/**"]

@sahrizvi sahrizvi Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR — path filtering is bounded on very large pushes

GitHub evaluates paths: filters against a bounded changed-file list (3,000 files). A push whose diff exceeds that cap may not start this workflow even though docs/docs/** changed.

Acceptable if very large pushes are operationally prohibited; otherwise trigger on every push to main and detect docs changes inside the job.

Separately: the docs/docs/** scope itself is correct and deliberate — it matches docs_path: docs/docs for the code: entry in help-docs tools/oss_sources.yml and excludes docs/internal/. Four reviewers flagged it as a mismatch with docs.yml's docs/**; that was rejected.

@sahrizvi sahrizvi Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR — path filtering is bounded on very large pushes

Unchanged from round 1. GitHub evaluates paths: filters against a bounded changed-file list, so a push whose diff exceeds that cap may not start this workflow even though docs/docs/** changed.

Acceptable if very large pushes are operationally prohibited; otherwise trigger on every push to main and detect docs changes inside the job — which would also give the workflow a natural place to handle the zero-SHA and force-push ranges.

workflow_dispatch:

@coderabbitai coderabbitai Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

workflow=".github/workflows/notify-help-docs.yml"

if rg -q '^  workflow_dispatch:$' "$workflow" &&
   ! rg -q 'inputs\.(before|after)' "$workflow"; then
  echo "::error::workflow_dispatch has no explicit synchronization range"
  exit 1
fi

rg -n -C 3 \
  'workflow_dispatch|github\.event\.(before|commits)|github\.ref|BEFORE|AFTER|inputs\.(before|after)' \
  "$workflow"

Repository: AltimateAI/altimate-code

Length of output: 227

🏁 Script executed:

#!/bin/bash
set -euo pipefail

workflow=".github/workflows/notify-help-docs.yml"
echo "== workflow exists =="
git ls-files "$workflow" || true
echo "== excerpt =="
cat -n "$workflow"

Repository: AltimateAI/altimate-code

Length of output: 4089

🌐 Web query:

GitHub Actions manually running workflow workflow_dispatch github.event before commits selected branch ref after commit manual run

💡 Result:

When you manually trigger a workflow using the workflow_dispatch event, you can specify a target branch or tag via the ref parameter [1]. Within the workflow run, the following properties provide information about the execution context: - github.ref: This context variable contains the fully-formed reference of the branch or tag that was used to trigger the workflow (e.g., refs/heads/main) [2]. - github.sha: This contains the commit SHA that the workflow is running against [2]. For a manual workflow_dispatch run, this corresponds to the HEAD commit of the ref you specified [3]. - github.event.ref: When triggered via workflow_dispatch, this property also contains the reference (e.g., refs/heads/main) that was targeted by the manual run [4]. To access the commit SHA specifically chosen for the run, you can rely on the standard github.sha context, which is automatically set to the commit at the tip of the chosen ref at the time of the trigger [3]. If you need to perform actions relative to a specific commit or branch, you can use the github.ref or github.event.ref context to identify the source [4]. If your workflow requires handling a commit other than the current HEAD of a branch (e.g., a specific SHA), the workflow_dispatch event natively supports specifying a ref, but it does not support passing a specific SHA directly as a "ref" to the workflow trigger itself in the same way it handles branches or tags [1]. If you must target a specific arbitrary SHA, a common workaround is to use custom inputs (e.g., input: sha) in your workflow_dispatch configuration and then use those inputs within your workflow to check out or interact with that specific commit [5][6].

Citations:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

workflow=".github/workflows/notify-help-docs.yml"
if [ -f "$workflow" ]; then
  echo "== workflow excerpt =="
  cat -n "$workflow"
else
  echo "Workflow file not found: $workflow"
  echo "Matching notify-help-docs files:"
  git ls-files | grep -F "notify-help-docs" || true
fi

Repository: AltimateAI/altimate-code

Length of output: 4037

🌐 Web query:

GitHub push event github.event.commits workflow_dispatch before property not present manual runs

💡 Result:

The github.event.before property is not present in workflow runs triggered by the workflow_dispatch event because this event is manually triggered rather than being tied to a specific git commit history or push activity [1][2]. While events like push provide a commit-based history that includes before and after properties to represent changes [3], a workflow_dispatch event is an ad-hoc trigger that does not inherently know about previous states or commit deltas [1]. Consequently, if a GitHub Action (such as a path-filtering or diff-based tool) relies on github.event.before to determine what changed in the repository, it will fail or skip its logic because that information is missing in the manual event payload [1][2]. Common workarounds for this limitation include: - Removing strict dependency on base commit properties in your action configuration [1]. - Using Git commands within your workflow steps to manually calculate the relevant diff or SHA, such as finding the merge-base between the current branch and the default branch [1][2]. - Relying on GitHub-provided environment variables (like GITHUB_SHA) and branch references rather than event payload properties when performing comparisons [1].

Citations:

Provide explicit sync ranges for workflow_dispatch.

workflow_dispatch can run from a selected ref, but it does not provide github.event.before or github.event.commits. The job guard checks github.event.commits.*.message, while lines 56-57 send an empty BEFORE and use only the selected HEAD as AFTER. Restrict manual runs to main, require before and after inputs, inspect that range for [docs-sync], then use those inputs for before and after; otherwise remove workflow_dispatch.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/notify-help-docs.yml at line 14, Update the
workflow_dispatch configuration and its job guard to avoid relying on
unavailable push-event fields: either remove manual dispatch or restrict it to
main with required before and after inputs, inspect that explicit range for
[docs-sync], and pass those inputs as BEFORE and AFTER instead of deriving them
from github.event.

Source: MCP tools

@cubic-dev-ai cubic-dev-ai Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Manual re-fires dispatch an empty before revision, so the hub cannot form the source range it receives for normal promotions. Add an explicit base-SHA input (and use it for BEFORE) or prevent manual dispatch from sending a promotion.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/notify-help-docs.yml, line 14:

<comment>Manual re-fires dispatch an empty `before` revision, so the hub cannot form the source range it receives for normal promotions. Add an explicit base-SHA input (and use it for `BEFORE`) or prevent manual dispatch from sending a promotion.</comment>

<file context>
@@ -1,31 +1,44 @@
   push:
     branches: [main]
     paths: ["docs/docs/**"]
+  workflow_dispatch:
 
 permissions:
</file context>

@sahrizvi sahrizvi Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAJOR — workflow_dispatch sends an empty range; the hub receives it and does nothing

Flagged by all 7 reviewers. This trigger was added to close round-1 nit 16 ("a failed promotion cannot be manually re-fired, and there is no way to smoke-test"). It can do neither.

The workflow_dispatch payload has no before field, so line 56 evaluates to "" and the dispatch goes out as:

{"product":"code","before":"","after":"<sha>","source_pr":"","source_author":"","source_url":""}

At the hub, help-docs/.github/workflows/sync-docs.yml:299-312:

if [ -n "${{ steps.p.outputs.before }}" ] && \
   git cat-file -e "${{ steps.p.outputs.before }}^{commit}" 2>/dev/null; then
  CH="$(git diff --name-only …)"
else
  CH=""
fi
…
if [ "${{ github.event_name }}" = "repository_dispatch" ] && [ -z "$CH" ]; then
  echo "run=false" >> "$GITHUB_OUTPUT"

The empty before fails the -n test first, so git cat-file and git diff never run: CH="" → run=false → the sync step, the PR step, the source-PR comment and the Slack notify are all skipped. Both workflows report success having promoted nothing.

Fix — make an empty range mean full reconciliation rather than nothing. Either add explicit inputs:

  workflow_dispatch:
    inputs:
      before: { description: "Start SHA (blank = full reconcile)", required: false }
      after:  { description: "End SHA (blank = main HEAD)",        required: false }

or send a mode: full flag and teach the hub to sync the whole docs_path when it sees one. Either way the hub half is a merge prerequisite in help-docs #43 — this PR cannot fix it alone. The cheapest correct option is to drop workflow_dispatch here and use the hub's own workflow_dispatch (direction: up, product: code), which already works.


permissions:
contents: read
pull-requests: read

Comment on lines +16 to +19

@coderabbitai coderabbitai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 4 'permissions:|create-github-app-token|repositories:|permission-' .github
gh api /orgs/AltimateAI/installation --jq '.permissions'

Repository: AltimateAI/altimate-code

Length of output: 11267

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== workflow =="
cat -n .github/workflows/notify-help-docs.yml
echo
echo "== create-github-app-token action v1 metadata =="
git ls-files .github
tmpdir="$(mktemp -d)"
cd "$tmpdir"
git init >/dev/null 2>&1
git remote add origin https://github.com/actions/create-github-app-token.git >/dev/null 2>&1
git fetch --no-tags --depth 1 origin 9626c2827b2773687c9ceab62c3cf66ef99169 >/dev/null 2>&1 || true
git checkout --detach FETCH_HEAD >/dev/null 2>&1 || true
find . -maxdepth 3 -type f \( -name 'action.yml' -o -name 'README.md' \) -print
if [ -f action.yml ]; then cat -n action.yml; fi
echo
echo "== repository-dispatch metadata =="
git init >/dev/null 2>&1
git remote add origin https://github.com/peter-evans/repository-dispatch.git >/dev/null 2>&1
git fetch --no-tags --depth 1 origin v3 >/dev/null 2>&1 || true
git checkout --detach FETCH_HEAD >/dev/null 2>&1 || true
if [ -f action.yml ]; then cat -n action.yml; fi

Repository: AltimateAI/altimate-code

Length of output: 2587

🌐 Web query:

actions/create-github-app-token v1 permissions input permission-contents default README

💡 Result:

The actions/create-github-app-token action does not have a default value for the permission-contents input [1]. By default, if you do not specify any permission- inputs (including permission-contents), the generated GitHub App installation access token automatically inherits all of the permissions granted to the GitHub App installation itself [2][3][4]. When you choose to use the permission-contents input, you must explicitly set it to either read or write [1]. The action recommends explicitly listing the permissions required for your specific use case to follow the principle of least privilege [2][3]. If you set any permission- input (such as permission-contents), the token's permissions are restricted to only those explicitly defined in your workflow configuration [5][6]. Ensure that any permission you specify is also granted to the GitHub App installation, otherwise the action will return an error [2][3].

Citations:

Restrict the generated App token to the contents permission used by the dispatch.

repositories: help-docs scopes the repository, but actions/create-github-app-token@v1 still issues an installation token with all App installation permissions unless a permission-* input is provided. The workflow permissions: contents: read controls the job’s GITHUB_TOKEN, not steps.tok.outputs.token. Configure the App installation with Contents: write on AltimateAI/help-docs only; then pass permission-contents: write after keeping the action pinned to an audited commit SHA.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/notify-help-docs.yml around lines 13 - 15, Update the
token-generation step in the workflow to pass permission-contents: write while
retaining the audited commit-SHA pin for actions/create-github-app-token@v1 and
the AltimateAI/help-docs repository restriction. Keep the job-level permissions
unchanged; the App installation token must be scoped to Contents: write only.

Sources: MCP tools, Linters/SAST tools

jobs:
notify:

@sahrizvi sahrizvi Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR — no timeout-minutes on the job

Flagged by 6 reviewers. dispatch-code-review.yml:33 sets timeout-minutes: 2, and every other job in the repo sets one. This bounds a hung gh api call — it does not bound runner queue time, so it isn't a mitigation for the arc-runner-gke concern above.

Suggested change
notify:
notify:
timeout-minutes: 5

@sahrizvi sahrizvi Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAJOR — deleting concurrency did not fix the dropped-range bug; it moved it to where it fails silently

Round-1 M5 was: GitHub holds one pending run per concurrency group, so three rapid docs pushes evict the middle one and its [before, after] range is never promoted. The concurrency block was deleted. But the hub has its own group covering exactly these runs (sync-docs.yml:58-60):

concurrency:
  group: docs-sync-${{ github.event_name }}-${{ github.event.client_payload.product || inputs.product || 'push' }}
  cancel-in-progress: false

Every dispatch from this workflow lands on the single group docs-sync-repository_dispatch-code. Three rapid pushes now produce three near-simultaneous dispatches instead of three serialized ones, so the middle hub run is more likely to be evicted, not less. And the failure is quieter than before: no red check on altimate-code, and the surviving run's before starts after the lost range's after, so those files are simply never promoted.

Fix — this has to be fixed at the hub; there is no sender-side workaround:

  • queue: max on the hub's concurrency group if the org's Actions plan supports it, which removes the eviction outright; or
  • have the hub diff against its own last-successfully-synced SHA (a marker commit or tag it maintains) instead of trusting the before it was handed, so a lost dispatch is recovered by the next one; or
  • coalesce ranges at the hub.

Restoring concurrency here is not a fix — it reintroduces the identical one-pending-run eviction, just earlier in the chain.

if: ${{ !contains(toJSON(github.event.commits.*.message), '[docs-sync]') }}

@cubic-dev-ai cubic-dev-ai Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: A mixed push permanently drops ordinary docs updates whenever its commit list also contains a [docs-sync] commit. Narrow the suppression to pushes that are wholly sync-generated, or otherwise dispatch the non-sync docs range.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/notify-help-docs.yml, line 22:

<comment>A mixed push permanently drops ordinary docs updates whenever its commit list also contains a `[docs-sync]` commit. Narrow the suppression to pushes that are wholly sync-generated, or otherwise dispatch the non-sync docs range.</comment>

<file context>
@@ -1,31 +1,44 @@
   notify:
-    if: ${{ !contains(github.event.head_commit.message, '[docs-sync]') }}
-    runs-on: arc-runner-gke
+    if: ${{ !contains(toJSON(github.event.commits.*.message), '[docs-sync]') }}
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
</file context>

@sahrizvi sahrizvi Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAJOR (latent) — workflow_dispatch is not restricted to main; fixing the manual trigger activates this

on.push.branches: [main] constrains only the push trigger. workflow_dispatch can be fired from any branch, and when it is:

  • github.sha (line 57, AFTER) is that branch's head, not a commit on main;
  • the paths: ["docs/docs/**"] filter does not apply at all;
  • this loop guard is vacuous — github.event.commits does not exist.

The hub checks out the product repo at client_payload.after (sync-docs.yml:282), so the promoted content would come from an unmerged branch — violating the hub's premise that it only ever receives content already on the product's default branch.

Today this is inert: the empty before makes the hub set run=false, so it checks out the branch commit and then does nothing. That is the only thing preventing it — and the prescribed fix for the manual trigger (treat an empty range as a full reconciliation) activates it directly. Fix both in the same change:

    if: >-
      (github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main') ||
      (github.event_name == 'push' &&
       !contains(toJSON(github.event.commits.*.message), '[docs-sync]'))

This also makes the trigger/loop-guard interaction explicit instead of incidental.

runs-on: ubuntu-latest
timeout-minutes: 5
steps:
- name: Check docs-sync App is configured
id: config
env:
APP_ID: ${{ secrets.DOCS_SYNC_APP_ID }}
APP_KEY: ${{ secrets.DOCS_SYNC_APP_PRIVATE_KEY }}
run: |
if [ -n "$APP_ID" ] && [ -n "$APP_KEY" ]; then
echo "enabled=true" >> "$GITHUB_OUTPUT"
else
echo "enabled=false" >> "$GITHUB_OUTPUT"
echo "::notice::docs-sync App not configured — skipping dispatch."
fi

- name: Resolve merged PR (author for notifications)
id: src
if: steps.config.outputs.enabled == 'true'
env:
GH_TOKEN: ${{ github.token }}
run: |
J="$(gh api repos/${{ github.repository }}/commits/${{ github.sha }}/pulls --jq '.[0] // {}' 2>/dev/null || echo '{}')"

@sahrizvi sahrizvi Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR — this line collapses three distinct outcomes into one, and picks an arbitrary PR

  • Errors are indistinguishable from "no PR." A missing gh, an auth failure, and a secondary-rate-limit hit all produce a silent {} with nothing in the log. Direct pushes already return an empty array successfully, so swallowing every error is unnecessary.
  • .[0] is arbitrary. A commit can be associated with more than one PR; nothing checks that the PR was merged into main or is the most recent, so attribution can land on the wrong author.
  • The endpoint is eventually consistent. GET /commits/{sha}/pulls can lag several seconds after a merge, so on a fast runner attribution is lost silently and deterministically, not occasionally.

Also: ${{ github.repository }} and ${{ github.sha }} are interpolated straight into the run: script here, while the very next step correctly routes everything through env:. Both values are trusted so there's no injection today, but the split pattern invites the unsafe version to be copied for an untrusted field later.

J="$(gh api "repos/${GITHUB_REPOSITORY}/commits/${GITHUB_SHA}/pulls" \
      --jq 'map(select(.merged_at != null and .base.ref == "main")) | sort_by(.merged_at) | last // {}')" \
  || { echo "::warning::could not resolve merged PR for ${GITHUB_SHA}"; J='{}'; }

plus a retry with backoff (roughly 4 attempts over ~15s) while J is {}.

@sahrizvi sahrizvi Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR — this line is unchanged in every respect

Flagged by 6 reviewers, and byte-identical to the previous revision:

  • Errors are indistinguishable from "no PR." 2>/dev/null || echo '{}' means a missing gh, an auth failure and a rate-limit hit all produce a silent {} with nothing in the log. Direct pushes already return an empty array successfully, so swallowing every error is unnecessary.
  • .[0] is arbitrary. A commit can belong to several PRs; nothing filters on merged_at or base.ref, so attribution can land on the wrong author.
  • No retry against an endpoint that is eventually consistent for seconds after a merge.
  • ${{ }} interpolated into the shell while the very next step routes everything through env:. Both values are trusted so there's no injection here, but the split pattern invites the unsafe version to be copied for an untrusted field later.

Consequence at the hub: attribution lands on the wrong author, or on nobody.

J="$(gh api "repos/${GITHUB_REPOSITORY}/commits/${GITHUB_SHA}/pulls" \
      --jq 'map(select(.merged_at != null and .base.ref == "main")) | sort_by(.merged_at) | last // {}')" \
  || { echo "::warning::could not resolve merged PR for ${GITHUB_SHA}"; J='{}'; }

plus a short retry with backoff while J is {}.

{
echo "num=$(echo "$J" | jq -r '.number // ""')"
echo "login=$(echo "$J" | jq -r '.user.login // ""')"
echo "url=$(echo "$J" | jq -r '.html_url // ""')"

@sahrizvi sahrizvi Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR — direct pushes to main dispatch with all source fields empty

Every push to main triggers this workflow, not only merges. A direct push produces empty num/login/url, and the receiver gets a promotion request it can't attribute to anyone.

Fall back to the commit author:

[ -z "$NUM" ] && LOGIN="$(gh api "repos/$GITHUB_REPOSITORY/commits/$GITHUB_SHA" --jq '.author.login // ""')"

Do not gate the dispatch on num != '' — that would suppress legitimate direct docs changes.

@sahrizvi sahrizvi Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR — direct pushes to main still promote with no attribution at all

Flagged by 5 reviewers. Every push to main triggers this workflow, not only merges. A direct push yields empty num / login / url, and at the hub that silently disables both notification paths — the source-PR comment (sync-docs.yml:356) and the Slack notify (sync-docs.yml:364) are gated on src_pr != ''. The promotion PR opens with no source link and nobody is told.

Fall back to the pusher rather than gating the dispatch:

[ -z "$NUM" ] && LOGIN="${{ github.event.sender.login }}"

Do not gate the dispatch on num != '' — that would suppress legitimate direct docs changes.

} >> "$GITHUB_OUTPUT"

- name: Build dispatch payload
id: payload
if: steps.config.outputs.enabled == 'true'
env:
BEFORE: ${{ github.event.before }}

@sahrizvi sahrizvi Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR — github.event.before is forwarded without a recovery mode

Flagged by all 7 reviewers. On branch creation before is the all-zero SHA; after a force-push it can name a commit no longer reachable. The consumer does git diff $BEFORE..$AFTER, which breaks in both cases.

This repo already has the guard pattern — ci.yml:554:

if [[ "${{ github.event.before }}" == "0000000000000000000000000000000000000000" ]]; then

Normalize the zero-SHA to "" and forward github.event.created / github.event.forced in the payload — without them the receiver can't tell a new branch from a force-push, and can't decide when to fall back to a full reconciliation.

@sahrizvi sahrizvi Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR — before still forwarded raw; the failure mode is now a silent skip

Flagged by all 7 reviewers. Unchanged from round 1.

The hub's git cat-file -e guard (sync-docs.yml:299-300) means a zero-SHA (branch creation) or an unreachable SHA (force-push) no longer breaks the sync — it skips it, with run=false and no signal in either repo. That is a worse failure mode for an operator than a red check.

Normalize the zero-SHA and forward github.event.created / github.event.forced so the hub can distinguish "nothing to do" from "fall back to a full reconciliation." ci.yml:554 already has the zero-SHA pattern in this repo:

if [[ "${{ github.event.before }}" == "0000000000000000000000000000000000000000" ]]; then

AFTER: ${{ github.sha }}
SRC_PR: ${{ steps.src.outputs.num }}
SRC_AUTHOR: ${{ steps.src.outputs.login }}
SRC_URL: ${{ steps.src.outputs.url }}
run: |
JSON="$(jq -cn --arg product code \
--arg before "$BEFORE" --arg after "$AFTER" \
--arg source_pr "$SRC_PR" --arg source_author "$SRC_AUTHOR" --arg source_url "$SRC_URL" \
'{product:$product,before:$before,after:$after,source_pr:$source_pr,source_author:$source_author,source_url:$source_url}')"
echo "json=$JSON" >> "$GITHUB_OUTPUT"

- uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547 # v1

@sahrizvi sahrizvi Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAJOR — no guard for absent App credentials; the job hard-fails instead of skipping

Flagged independently by all 7 reviewers. Even with the credential reference fixed, there is no if: gate here. On a fork, a repo where the org secret isn't visible, or after a key rotation, create-github-app-token errors and an unrelated docs merge shows a failed check on main.

The repo's own sibling workflow already handles this — dispatch-code-review.yml:44-48:

if [ -z "$GH_TOKEN" ]; then
  echo "AUTOPILOT_DISPATCH_TOKEN not available — skipping centralized dispatch."
  exit 0
fi

secrets isn't available in a job-level if, so gate at step level — and gate every subsequent step, not just the token and dispatch, or a jq/runner failure still reddens an unconfigured workflow:

      - name: Check docs-sync configuration
        id: config
        env:
          APP_ID: ${{ secrets.DOCS_SYNC_APP_ID }}
          APP_KEY: ${{ secrets.DOCS_SYNC_APP_PRIVATE_KEY }}
        run: |
          if [[ -n "$APP_ID" && -n "$APP_KEY" ]]; then
            echo "enabled=true" >> "$GITHUB_OUTPUT"
          else
            echo "enabled=false" >> "$GITHUB_OUTPUT"
            echo "::notice::docs-sync App not configured; skipping dispatch."
          fi

then if: steps.config.outputs.enabled == 'true' on all remaining steps.

@sahrizvi sahrizvi Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT — still pinned to create-github-app-token v1

.github/actions/setup-git-committer/action.yml:22 uses @v2. Pick one. Worth bumping here and adding permission-contents: write in the same edit.

id: tok
if: steps.config.outputs.enabled == 'true'
with:
app-id: ${{ secrets.DOCS_SYNC_APP_ID }}
private-key: ${{ secrets.DOCS_SYNC_APP_PRIVATE_KEY }}
owner: AltimateAI

@cubic-dev-ai cubic-dev-ai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: actions/create-github-app-token issues a token that inherits all of the GitHub App installation's permissions by default; repositories: help-docs only scopes which repo the token can touch, not what it can do there. The job-level permissions: contents: read doesn't apply to this generated token either, since that only governs GITHUB_TOKEN. Add an explicit permission-contents: write (or the minimal set actually required by the dispatch) to avoid granting this token broader access than needed.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/notify-help-docs.yml, line 26:

<comment>`actions/create-github-app-token` issues a token that inherits all of the GitHub App installation's permissions by default; `repositories: help-docs` only scopes which repo the token can touch, not what it can do there. The job-level `permissions: contents: read` doesn't apply to this generated token either, since that only governs `GITHUB_TOKEN`. Add an explicit `permission-contents: write` (or the minimal set actually required by the dispatch) to avoid granting this token broader access than needed.</comment>

<file context>
@@ -0,0 +1,34 @@
+        with:
+          app-id: ${{ vars.DOCS_SYNC_APP_ID }}
+          private-key: ${{ secrets.DOCS_SYNC_APP_PRIVATE_KEY }}
+          owner: AltimateAI
+          repositories: help-docs
+      - uses: peter-evans/repository-dispatch@v3
</file context>

repositories: help-docs

@sahrizvi sahrizvi Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR — the App token inherits every installation permission

Scoping to owner: AltimateAI / repositories: help-docs is good, but no permission-* inputs are supplied, so the minted token carries every permission granted to the App installation. repository_dispatch needs only Contents: write on the target.

Suggested change
repositories: help-docs
repositories: help-docs
permission-contents: write

@sahrizvi sahrizvi Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR — the minted App token still inherits every installation permission

Flagged by 6 reviewers. owner: AltimateAI / repositories: help-docs is good scoping, but with no permission-* inputs the token carries everything the App installation was granted — per help-docs tools/DOCS_SYNC.md that is Contents:write and PullRequests:write. Creating a repository dispatch needs Contents: write only.

          permission-contents: write


- uses: peter-evans/repository-dispatch@ff45666b9427631e3450c54a1bcbee4d9ff4d7c0 # v3

@sahrizvi sahrizvi Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR — no failure signal beyond a red check

A failed dispatch surfaces only in the Actions tab. help-docs already ships tools/notify_slack.py; an if: failure() notify step here would close the loop on a sync that has silently stopped working.

if: steps.config.outputs.enabled == 'true'

@sahrizvi sahrizvi Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR — no failure signal beyond a red check, and the revision widened the gap

Unchanged from round 1, but there are now three separate paths that converge on "green everywhere, docs never promoted":

  1. absent configuration → the config gate skips, run is green;
  2. missing receiver → POST /dispatches returns 204, run is green;
  3. workflow_dispatch → empty before, hub sets run=false, both runs green.

A failed dispatch surfaces only in the Actions tab, and none of the three above surfaces anywhere at all. help-docs already ships tools/notify_slack.py; an if: failure() notify step here would close the loop on a sync that has silently stopped working.

with:
token: ${{ steps.tok.outputs.token }}
repository: AltimateAI/help-docs

@sahrizvi sahrizvi Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAJOR — merge-order prerequisite: the consumer doesn't exist yet

AltimateAI/help-docs has no repository_dispatch handler for promote-from-product on its default branch. The handler lives in help-docs PR #43 (feat/sync-docs-to-oss), which is still open. POST /dispatches returns 204 whether or not anything is listening, so this will "succeed" and do nothing, with no signal that the far end is missing.

The payload contract itself is correct — {product, before, after, source_pr, source_author, source_url} matches the consumer's reads at sync-docs.yml:232-237, product: code matches the code: key in tools/oss_sources.yml, and that entry's docs_path: docs/docs matches this workflow's paths: ["docs/docs/**"] filter. Good contract, wrong merge order.

Merge help-docs #43 first, or land this behind the credential guard so it's genuinely inert until both ends are live. If the ordering is already coordinated, state it as a merge prerequisite in the PR description.

@sahrizvi sahrizvi Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAJOR — the consumer is still not on help-docs' default branch, and the credentials are configured, so this fires into the void

Flagged by all 7 reviewers. Carried from round 1, and now sharper rather than softer:

  • DOCS_SYNC_APP_ID and DOCS_SYNC_APP_PRIVATE_KEY are present as repo secrets, so the new config gate passes, the token mints, and the dispatch is sent.
  • repository_dispatch handlers must live on the target's default branch. sync-docs.yml exists only on help-docs PR fix: address remaining code review issues from PR #39 #43, which is still open.
  • POST /dispatches returns 204 whether or not anything is listening.

Merging this first produces a workflow that reports success on every docs merge to main while doing nothing — the worst outcome, because the green check reads as "synced."

Fix: merge help-docs #43 first (or simultaneously) and state the ordering in the PR description. A sender-side preflight — gh api repos/AltimateAI/help-docs/contents/.github/workflows/sync-docs.yml, failing loudly if absent — would make the dependency self-enforcing.

event-type: promote-from-product
client-payload: ${{ steps.payload.outputs.json }}
Loading