Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 22 additions & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# CODEOWNERS — reviewer ROUTING only (auto-requests reviewers on a PR).
#
# This file does NOT enforce "both teams must approve": when code-owner review
# is required, GitHub is satisfied by an approval from ANY single listed owner.
# The dual gate is enforced elsewhere:
# - Editorial (TW) gate: a branch ruleset requiring 1 approval from
# @OffchainLabs/technical-writing on docs/** (applied by eng/infra).
# - SME (technical) gate: the `sme-review-gate` status check (region/path aware,
# see .github/workflows/sme-review-gate.yml + scripts/sme-review-gate.ts).
#
# The per-domain SME teams referenced below must exist in the OffchainLabs org
# WITH write access, or GitHub ignores the entry. Until eng/infra creates them,
# those lines are inert — the technical-writing routing still works.

# Editorial review on all docs
/docs/** @OffchainLabs/technical-writing

# Pilot technical sections — also auto-request the relevant per-domain SME team
/docs/how-arbitrum-works/** @OffchainLabs/technical-writing @OffchainLabs/protocol-sme
/docs/stylus/** @OffchainLabs/technical-writing @OffchainLabs/stylus-sme
/docs/stylus-by-example/** @OffchainLabs/technical-writing @OffchainLabs/stylus-sme
/docs/launch-arbitrum-chain/** @OffchainLabs/technical-writing @OffchainLabs/chain-sme
151 changes: 151 additions & 0 deletions .github/SME_REVIEW_GATE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
# SME review gate — operator runbook

How the dual editorial + SME review gate works, and the admin steps to turn on
enforcement. Canonical design lives in Notion ("Solving TW's PR Bottleneck Issue").

## What it does

A docs PR should merge only when **both** an editorial reviewer (Technical Writing)
**and** the relevant **subject-matter expert(s) (SME)** have approved. The two gates
are enforced separately:

- **Editorial gate (TW)** — a branch ruleset rule "require review from specific teams"
requiring 1 approval from `@OffchainLabs/technical-writing` on `docs/**`.
- **SME gate** — the `sme-review-gate` status check (this repo's
`.github/workflows/sme-review-gate.yml` + `scripts/sme-review-gate.ts`). It figures
out which SME team(s) a PR needs and whether they've approved.

A PR needs an SME team when either:

1. **Region marker** — a changed line is inside `{/* sme:start team=<slug> */}` …
`{/* sme:end */}` (sub-document granularity), or
2. **Path fallback** — a changed file matches that team's globs in
`.github/sme-config.json` (so untagged technical edits still gate).

If a PR touches no SME content, the check passes on its own and editorial approval is enough.

## Status check conclusions

| Conclusion | Meaning |
| ----------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `neutral` | Report-only mode (`reportOnly: true`). Never blocks. |
| `success` | No SME content, or every required SME team approved. |
| `failure` | A required SME team has not approved yet. |
| `action_required` | Misconfiguration: malformed `sme:*` markers, an unknown team slug, or team membership could not be read (missing team / token). Fix setup — not a normal "awaiting approval". |

## Transient markers (post-merge cleanup)

SME markers are **transient per-PR signals**, not durable content. After a PR
merges to `master`, the `SME marker cleanup` workflow
(`.github/workflows/sme-marker-cleanup.yml`) strips the `{/* sme:* */}` markers
from the files that PR changed and commits the result directly to `master` as the
cleanup bot. A future edit to that area is re-tagged by its own author if the new
diff needs SME review. Always-on protection for whole technical sections still
comes from the path globs in `.github/sme-config.json`, which do not depend on
markers.

Enabling cleanup needs (admin): a dedicated bot/GitHub App token in the repo
secret `SME_CLEANUP_TOKEN` with `contents: write`, and that bot added to the
branch-ruleset **bypass list** so its commit isn't blocked by required reviews.

## Current state: report-only (Phase 0)

`.github/sme-config.json` has `"reportOnly": true`, so the check posts `neutral` and
**blocks nothing**. This lets the diff → region/path → team logic be validated on real
PRs before it gates merges. While in this state you will see SME teams reported as
`unverifiable` until the teams and token below exist — that is expected.

## Enabling enforcement (admin steps)

> Requires org owner / repo admin. Do these in order; the gate is reversible at every step.

1. **Create the per-domain SME teams** in the `OffchainLabs` org and give each **write**
access to `arbitrum-docs` (write access is required for their approvals to count):

- `protocol-sme` — owns `docs/how-arbitrum-works/**`
- `stylus-sme` — owns `docs/stylus/**`, `docs/stylus-by-example/**`
- `chain-sme` — owns `docs/launch-arbitrum-chain/**`

(These match `.github/sme-config.json` and `.github/CODEOWNERS`. Add/rename teams by
editing those two files in a PR.)

2. **Provision a GitHub App token.** The gate token must do two things at once: read org
team membership AND post the `sme-review-gate` check run. Only a **GitHub App** can hold
both — `members: read` (org) and `checks: write` (repo). No PAT (classic or fine-grained)
can create check runs, so a PAT is not an option here. Create a GitHub App with repo
permissions **Checks: read/write**, **Contents: read**, **Pull requests: read** and org
permission **Members: read**, install it on the repo, and store its App ID and private
key as the repo secrets **`SME_GATE_APP_ID`** and **`SME_GATE_APP_PRIVATE_KEY`**. The
workflow mints a short-lived installation token from them via
`actions/create-github-app-token`; if the secrets are absent it skips that step and
degrades to `GITHUB_TOKEN` (teams report `unverifiable` rather than failing).

3. **Confirm in report-only.** Open/refresh a PR touching a pilot section and check the
`sme-review-gate` run: required teams should now show `approved`/`awaiting` instead of
`unverifiable`. Fix any `action_required` marker problems it reports.

4. **Flip to enforcing.** In a PR, set `"reportOnly": false` in `.github/sme-config.json`.
The check now returns `success` / `failure` / `action_required`.

5. **Apply the ruleset.** Create the branch ruleset from the API body in
`.github/rulesets/docs-review-gates.json` (it requires the `sme-review-gate` check +
1 code-owner approval), then set its enforcement to **active**:

```shell
gh api -X POST repos/OffchainLabs/arbitrum-docs/rulesets \
--input .github/rulesets/docs-review-gates.json
# then, in the repo ruleset UI, switch enforcement from "Disabled" to "Active"
```

6. **Add the editorial (TW) gate.** In the same ruleset (or a second one), add the
**"Require review from specific teams"** rule via the repo UI:
`@OffchainLabs/technical-writing`, 1 approval, file path `docs/**`. (This rule is new
enough that the UI is the reliable way to configure it; it is not in the JSON above.)

7. **Pilot, then expand.** Keep the path scope limited to the pilot sections first. To
widen coverage later, add teams + globs to `.github/sme-config.json` and paths to
`.github/CODEOWNERS`.

## Contributor model: forks vs internal branches

External contributors open PRs from **forks**; internal contributors push branches to
**origin**. A plain `pull_request` workflow on a fork PR gets a read-only token, no
secrets, and no `checks: write` — so it couldn't post the check or read team membership.
The workflow therefore uses **`pull_request_target`**, which runs in the base-repo context
(full token + secrets) for fork PRs too. It is safe because the job checks out only the
trusted base ref and the script reads PR content over the API (`refs/pull/N/head`) — it
never checks out or runs the PR's code. Do not add a step that builds or executes PR code.

## Smoke-testing in a sandbox

To exercise the _blocking_ behavior without risking real PRs, use a throwaway private repo.

**One-time setup (admin):**

1. Create a private repo, e.g. `OffchainLabs/arbitrum-docs-gate-sandbox`.
2. Copy the gate files into its default branch: `.github/workflows/sme-review-gate.yml`,
`scripts/sme-review-gate.ts`, `.github/sme-config.json`, `.github/CODEOWNERS`
(plus a minimal `package.json` with `tsx`, or reuse this repo's). Keep `sme-config.json`
identical so `stylus-sme` maps to `docs/stylus/**`.
3. Create an `stylus-sme` team with **write** access, and set the `SME_GATE_APP_ID` +
`SME_GATE_APP_PRIVATE_KEY` secrets (a GitHub App with `members: read` + `checks: write`;
see step 2 of "Enabling enforcement" for the full permission list).

**Run the matrix:**

```shell
scripts/sme-gate-sandbox.sh --repo OffchainLabs/arbitrum-docs-gate-sandbox --dry-run
scripts/sme-gate-sandbox.sh --repo OffchainLabs/arbitrum-docs-gate-sandbox
```

This opens four internal-branch PRs (editorial-only, path-fallback, region-tag, malformed
marker). Flip `reportOnly: false` in the sandbox to see `success`/`failure`/`action_required`
instead of `neutral`, and apply the ruleset (active) to confirm the merge button actually
locks/unlocks. The script prints the manual rows it can't drive: an SME approval flipping the
check to success, dismissing it to re-block, and a **fork PR** (from a second account) to
confirm the `pull_request_target` path posts and resolves the check.

## Rollback

Set `"reportOnly": true` in `.github/sme-config.json` (instant, code-side), and/or set the
ruleset enforcement back to **Disabled** in the UI. Either fully unblocks merges.
10 changes: 10 additions & 0 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ Please fill out the form below to ensure your doc gets quickly approved and merg
- [ ] Codebase changes
- [ ] Not applicable

## SME review

<!-- Highly technical content needs subject-matter-expert (SME) approval in addition to editorial review. Tagging the parts that need SME eyes lets SMEs review just that subset. Check all that apply: -->

- [ ] No content here needs SME review (editorial only)
- [ ] I tagged the technical region(s) needing SME review with `{/* sme:start team=<team> */}` … `{/* sme:end */}` (teams: `protocol-sme`, `stylus-sme`, `chain-sme`)
- [ ] This PR only touches a technical section (`how-arbitrum-works/`, `stylus/`, `launch-arbitrum-chain/`), which auto-requires its SME team

<!-- See CONTRIBUTE.md ("Flagging content for SME review") for how tagging works. Tags are transient — they're auto-removed from master after merge, so re-tag in future PRs when needed. -->

## Checklist

<!-- Mark completed items with an "x" -->
Expand Down
30 changes: 30 additions & 0 deletions .github/rulesets/docs-review-gates.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"name": "Docs review gates",
"target": "branch",
"enforcement": "disabled",
"conditions": {
"ref_name": {
"include": ["~DEFAULT_BRANCH"],
"exclude": []
}
},
"rules": [
{
"type": "pull_request",
"parameters": {
"required_approving_review_count": 1,
"require_code_owner_review": true,
"dismiss_stale_reviews_on_push": false,
"require_last_push_approval": false,
"required_review_thread_resolution": false
}
},
{
"type": "required_status_checks",
"parameters": {
"strict_required_status_checks_policy": false,
"required_status_checks": [{ "context": "sme-review-gate" }]
}
}
]
}
11 changes: 11 additions & 0 deletions .github/sme-config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"_README": "Drives scripts/sme-review-gate.ts. team keys are GitHub team slugs under `org` and must have write access for Phase 1 enforcement; Phase 0 runs report-only. `paths` are gitignore-style globs (prefix/** supported). Region markers in MDX ({/* sme:start team=<slug> */} ... {/* sme:end */}) reference these same slugs.",
"org": "OffchainLabs",
"editorialTeam": "technical-writing",
"reportOnly": true,
"smeTeams": {
"protocol-sme": ["docs/how-arbitrum-works/**"],
"stylus-sme": ["docs/stylus/**", "docs/stylus-by-example/**"],
"chain-sme": ["docs/launch-arbitrum-chain/**"]
}
}
84 changes: 84 additions & 0 deletions .github/workflows/sme-marker-cleanup.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
name: SME marker cleanup
run-name: SME marker cleanup (post-merge of #${{ github.event.pull_request.number }})

# After a PR merges to master, strip the transient {/* sme:* */} markers it
# introduced. SME tags are per-PR signals, not durable content — see
# .github/SME_REVIEW_GATE.md and the design spec.
#
# SECURITY — why pull_request_target: it runs in the BASE repo context, so it has
# a token (SME_CLEANUP_TOKEN) that can push to master. It checks out the trusted
# base (master), never the PR head, and only runs the in-repo stripper over file
# paths. Do NOT add a step that builds or executes PR code.

on:
pull_request_target:
types: [closed]
branches: [master]

permissions:
contents: write
pull-requests: read

concurrency:
group: sme-marker-cleanup-master
cancel-in-progress: false

jobs:
strip:
if: github.event.pull_request.merged == true
runs-on: ubuntu-latest
steps:
- name: Checkout master
uses: actions/checkout@v4
with:
ref: master
fetch-depth: 0
token: ${{ secrets.SME_CLEANUP_TOKEN }}
persist-credentials: true

- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version: '22.x'

- name: Install dependencies
uses: OffchainLabs/actions/node-modules/install@main
with:
install-command: yarn install --frozen-lockfile

- name: Collect merged PR's markdown files
id: files
env:
GH_TOKEN: ${{ secrets.SME_CLEANUP_TOKEN }}
PR: ${{ github.event.pull_request.number }}
REPO: ${{ github.repository }}
run: |
gh api "repos/$REPO/pulls/$PR/files" --paginate --jq '.[].filename' \
| grep -E '\.mdx?$' > "$RUNNER_TEMP/sme-files.txt" || true
echo "count=$(wc -l < "$RUNNER_TEMP/sme-files.txt" | tr -d ' ')" >> "$GITHUB_OUTPUT"

- name: Strip SME markers
if: steps.files.outputs.count != '0'
run: |
mapfile -t FILES < "$RUNNER_TEMP/sme-files.txt"
yarn tsx scripts/strip-sme-markers.ts "${FILES[@]}"

- name: Commit and push if changed
if: steps.files.outputs.count != '0'
env:
PR: ${{ github.event.pull_request.number }}
run: |
if git diff --quiet; then
echo "No SME markers to strip."
exit 0
fi
git config user.name "arbitrum-docs-sme-bot"
git config user.email "sme-bot@users.noreply.github.com"
git commit -am "chore: strip transient SME markers (post-merge of #${PR})"
for attempt in 1 2 3; do
if git push origin master; then exit 0; fi
echo "push rejected, rebasing (attempt $attempt)"
git pull --rebase origin master
done
echo "Failed to push after 3 attempts" >&2
exit 1
Loading
Loading