Skip to content

Commit 3f78539

Browse files
authored
Merge pull request #138 from USACE-RMC/ci/review-workflow-overhaul
Overhaul document review and draft publication workflows
2 parents cea062f + 7e54772 commit 3f78539

51 files changed

Lines changed: 4176 additions & 1278 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.claude/skills/git-conventions.md‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,24 +21,23 @@ Resolve LifeSim User's Guide v1.0 Level 3 QC findings
2121

2222
## Branch Naming
2323

24-
Whether a PR gets a review lane is decided by **content**, not by branch name: a PR
25-
that changes any file under `docs/` is a documentation PR; one that changes nothing
26-
under `docs/` has no lane and no review stages.
24+
An administrator assigns every PR a review lane using `/review classify`.
25+
Changed content determines the appropriate classification; branch names are descriptive.
2726

2827
### Documentation branches (changes under `docs/`)
2928

30-
The prefix routes the PR to a review lane. Use the full two-segment prefix — a bare
31-
`docs/{name}` does **not** match a lane and strands the PR in `stage:needs-lane`.
29+
These prefixes describe intent and do not assign or advance a review lane.
3230

33-
- `docs/new/{slug}` — new document (Peer → Lead Civil → Technical edit → Director)
31+
- `docs/new/{slug}` — new document (Peer → Lead Civil → Technical edit in the Content PR; separate Director review after draft merge)
3432
- `docs/major/{slug}-v{X.0}` — major revision (Peer → Lead Civil → Technical edit)
3533
- `docs/minor/{slug}-v{X.Y}` — minor revision (Peer → Technical edit)
3634
- `docs/fix/{slug}` — editorial fix (no review; admin self-merge)
3735
- `docs/dev/{slug}` — dev docs under `docs/dev/` (no review; admin self-merge)
3836

3937
### Non-documentation branches (components, plumbing, styling, scripts, CI)
4038

41-
No review lane, no review stages — `CI Build` is the only gate.
39+
Administrators assign the `code` lane to changes without document edits. It has no
40+
formal review stages, but both `CI Build` and `review-workflow` must pass before administrator merge.
4241

4342
- `feature/{descriptive-name}` — new components, new site capabilities, enhancements
4443
- `fix/{descriptive-name}` — bug fixes in components, styles, scripts, build tooling
@@ -47,8 +46,8 @@ No review lane, no review stages — `CI Build` is the only gate.
4746

4847
Use `feature/` rather than `enhancement/`; older branches used the latter.
4948

50-
A PR that touches both site code and content under `docs/` is a documentation PR —
51-
put it on the matching `docs/…` prefix.
49+
A PR mixing document content and site code is a specialized administrator-classified
50+
case. Authors should change only one document per PR; a branch prefix grants no exception.
5251

5352
## Pull Requests
5453

‎.claude/skills/new-doc/SKILL.md‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,7 @@ If the category IS `dev/`:
5858

5959
### 1f — Active / Draft Status
6060

61-
Ask: "Should this document be active (visible and clickable) or inactive (shows 'Coming Soon' badge)? (default: active)"
62-
63-
Record both `active` (true/false) and `draft` (inverse of active) values.
61+
Ask whether the draft should be active after its Content PR merges. Record `active` independently and always start a new document with `draft: true`. The later Publication PR removes draft status after Director approval or waiver.
6462

6563
### 1g — File Structure (dev only)
6664

@@ -393,3 +391,13 @@ Next steps:
393391
- Add chapter files (02-*.mdx, 03-*.mdx, etc.)
394392
- Run `npm start` to preview locally
395393
```
394+
395+
## Step 9: Prepare review
396+
397+
Use a descriptive `docs/new/<slug>` branch. The prefix suggests intent; it does not assign review state. Keep the author PR to one document and directly related source, assets, registrations, and site changes. After the Content PR opens, an administrator records:
398+
399+
```text
400+
/review classify new <doc_location>
401+
```
402+
403+
The Content PR requires peer, Lead Civil, and manually initiated technical editing. It merges and deploys from `main` as an active draft before an administrator starts the separate Director Review PR. Do not remove `draft: true` in the Content PR and do not deploy the author branch.

‎.claude/skills/new-revision/SKILL.md‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ The corresponding value in the JSON is the `currentVersion` (e.g., `v1.0`).
2525

2626
Ask: "Is this a major revision or a minor revision?"
2727

28-
- **Major revision** — substantial changes warranting a new major version. Branch prefix: `docs/major/`. Goes through Lane 2 (Peer → Lead Civil → Technical edit). No Director review.
29-
- **Minor revision** — smaller updates warranting a minor version bump. Branch prefix: `docs/minor/`. Goes through Lane 3 (Peer → Technical edit). No Director review.
28+
- **Major revision** — substantial changes warranting a new major version. Descriptive branch prefix: `docs/major/`. Requires Peer → Lead Civil → Technical edit. No Director review.
29+
- **Minor revision** — smaller updates warranting a minor version bump. Descriptive branch prefix: `docs/minor/`. Requires Peer → Technical edit. No Director review.
3030

3131
### 1c — New version number
3232

@@ -161,5 +161,13 @@ Next steps:
161161
1. Edit the new version's MDX files to make your changes
162162
2. Update 00-version-history.mdx with a real description and your name
163163
3. Run `npm start` to preview locally
164-
4. When ready, commit and open a PR (the workflow will detect the branch prefix and start {Lane 2: Peer → Lead Civil | Lane 3: Peer review} automatically)
164+
4. When ready, commit and open one PR for this document
165165
```
166+
167+
After the PR opens, an administrator records the authoritative classification:
168+
169+
```text
170+
/review classify <major|minor> <doc_location>
171+
```
172+
173+
The branch prefix is descriptive and does not assign review state. The administrator assigns one named reviewer per human stage. Technical editing is manually initiated and reviews the changed content with enough surrounding context to assess it. Production builds only from administrator-merged `main`; do not deploy the revision branch.

‎.claude/skills/pr/SKILL.md‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,11 @@ Read `.claude/skills/git-conventions.md` for PR conventions.
6868
- Derived from the **overall purpose** of all commits on the branch, not just the latest commit
6969
- Example: "Redesign homepage with product tile grid layout"
7070

71-
### Body — choose the format based on the branch prefix
71+
### Body — choose the format based on the change
7272

73-
**If the current branch starts with `docs/`** → use the **Template Style** (Step 5a). The repo's PR template at `.github/pull_request_template.md` contains a checklist for document authors, including the Lane 1 "Technical edit comments addressed" checkbox that the stage-progression workflow watches for. Passing `--body` to `gh pr create` overrides the template entirely, so the skill must reproduce the template structure with the auto-generated content filled in.
73+
**If the change affects a document** → use the **Template Style** (Step 5a). Reproduce the repository template when passing `--body`, including the one-document scope, `doc_location`, related issues, validation, and review notes. Branch prefixes describe intent; an administrator's controller classification is authoritative.
7474

75-
**Otherwise (infrastructure, tooling, dependency, or any non-doc branch)** → use the **Summary Style** (Step 5b). These PRs are silently ignored by the review workflow and don't need the doc-author checklist.
75+
**Otherwise (infrastructure, tooling, dependency, or any non-doc branch)** → use the **Summary Style** (Step 5b). These PRs still require administrator classification as `code`, the review-workflow gate, and CI; they do not require formal document-review stages.
7676

7777
In both styles, the summary content should be based on **ALL** commits on the branch, not just the most recent one. Read through all the commit messages and the diff to understand the full scope. If `$ARGUMENTS` was provided, use it to focus the title and description.
7878

@@ -87,12 +87,12 @@ In both styles, the summary content should be based on **ALL** commits on the br
8787
3. Compute the list of affected MDX documents: from `git diff --name-only main...HEAD`, keep entries matching `docs/**/*.mdx`. If the base is not `main`, use that instead.
8888
4. Build the PR body by transforming the template:
8989
- Under `## Description`, replace the `<!-- Briefly describe what this PR does and why. -->` comment with the summary bullets.
90-
- Under `## Affected documents`, replace the bare `- ` line with the list of affected MDX files. Format each as a markdown link relative to the repo root: `- [filename.mdx](docs/full/path/filename.mdx)`. If there are no changed MDX files, write `- _No MDX files changed in this PR._`
90+
- Under `## Document scope`, identify the single document and its `doc_location`. List affected MDX files when useful. For an administrator infrastructure overhaul spanning guides, explicitly describe that scope instead of presenting it as a routine one-document author PR.
9191
- Leave the `## Related issue(s)` section's comment placeholder unchanged so the author can fill it in.
92-
- Leave **all checklist items unchecked**, including the Lane 1 Technical edit checkbox. The author checks them as they complete each item; the workflow specifically depends on the Technical edit checkbox being present and unchecked at PR open time.
93-
- Leave the `## Notes for reviewers` comment placeholder unchanged.
92+
- Mark validation items only when evidence supports them. Do not add a technical-edit advancement checkbox or manually request stage reviewers.
93+
- Under `## Review notes`, include any relevant activation requirements or reviewer context.
9494

95-
The result should be a complete copy of the template with the Description and Affected documents sections populated.
95+
The result should follow the current template with its Description and Document scope sections populated.
9696

9797
### Step 5b: Summary Style (for non-`docs/` branches)
9898

‎.claude/skills/technical-edit/SKILL.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ Tell the user:
5252
- How many files were reviewed
5353
- Which review prompt version was used
5454
- Remind the author to check the "Technical edit comments addressed" checkbox in the PR description when they're done addressing comments
55-
- Remind the author to reply to threads from the **Files changed** tab using **Start a review**, then submit once. Replying from the Conversation tab files each reply as its own standalone review and emails every PR subscriber separately.
55+
- Suggest batching related replies from **Files changed** with **Start a review** to reduce separate submissions and notification noise. Do not promise an exact email count.
5656

57-
Note that step 4 posts a single bundled review on purpose: one `POST /pulls/:pr/reviews` call with every comment in the `comments` array produces one notification. Never loop over `POST /pulls/:pr/comments` — that creates one standalone review per comment and floods every subscriber.
57+
Step 4 posts one bundled review so findings arrive coherently and avoid unnecessary separate notifications. Native GitHub delivery still depends on each user's settings.
58+
59+
For a new document, review the full document. For a major or minor revision, review changed content with enough surrounding context to assess it. Completion is explicit: after the edit finishes, an administrator records `/review complete editor`. Author replies and thread resolution are not stage gates.

‎.github/pull_request_template.md‎

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,24 @@
11
## Description
22

3-
<!-- Briefly describe what this PR does and why. -->
3+
<!-- Describe what this PR changes and why. -->
44

5-
## Affected documents
5+
## Document scope
66

7-
<!-- List the documents this PR changes. Write "None — non-documentation change"
8-
for component, styling, script, workflow, or other site-plumbing work. -->
7+
<!-- Name the one document changed by this author PR, or write "None — site/code change." Related source, assets, registry, and site changes may be included. -->
98

10-
-
9+
- Document:
10+
- Document location (`doc_location`, or `-`):
1111

1212
## Related issue(s)
1313

14-
<!-- Link related issues. Use "Closes #N" to auto-close on merge. -->
14+
<!-- Use "Closes #N" when appropriate. -->
1515

16-
## Pre-submission checklist
16+
## Validation
1717

18-
- [ ] I have previewed these changes locally or via the PR preview URL
19-
- [ ] My branch name uses an expected prefix — documentation: `docs/new/`, `docs/major/`, `docs/minor/`, `docs/fix/`, `docs/dev/`; everything else: `feature/`, `fix/`, `chore/`, `ci/`
20-
- [ ] I have updated `00-version-history.mdx` if this change warrants a version entry
21-
- [ ] I have assigned a specific peer reviewer via the Reviewers sidebar (if known)
18+
- [ ] I previewed the change locally or through the PR preview
19+
- [ ] I ran the checks appropriate to this change
20+
- [ ] I updated version history when the document revision warrants it
2221

23-
## Technical edit (Lanes 1, 2, and 3)
22+
## Review notes
2423

25-
<!-- Check the box below only after every technical edit comment has been addressed.
26-
Lane 1 (new document) then advances to Director review.
27-
Lanes 2 and 3 (major/minor revision) advance straight to ready-to-merge —
28-
neither lane includes a Director review.
29-
Non-documentation PRs can ignore this section entirely. -->
30-
31-
- [ ] Technical edit comments addressed
32-
33-
## Notes for reviewers
34-
35-
<!-- Anything reviewers should know. -->
24+
<!-- Call out areas that need attention. The administrator records classification and assignments with /review commands after the PR opens. Authors do not advance stages by editing this template. -->

‎.github/workflows/ci-build.yml‎

Lines changed: 55 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,65 +1,70 @@
11
name: CI Build
2-
2+
run-name: 'Validate PR #${{ github.event.pull_request.number || inputs.pr }} at ${{ github.event.pull_request.head.sha || inputs.sha }}'
33
on:
44
pull_request:
55
types: [opened, synchronize, reopened]
6-
6+
workflow_dispatch:
7+
inputs:
8+
pr:
9+
description: 'PR number to validate (used by review automation)'
10+
required: true
11+
type: string
12+
sha:
13+
description: 'Exact PR head revision to validate'
14+
required: true
15+
type: string
716
permissions:
817
contents: read
9-
pull-requests: write
10-
statuses: write
11-
18+
pull-requests: read
19+
concurrency:
20+
group: ci-${{ github.event.pull_request.number || inputs.pr }}
21+
cancel-in-progress: true
1222
jobs:
1323
build:
14-
name: CI Build
24+
name: Build preview artifact
1525
runs-on: ubuntu-latest
26+
permissions:
27+
contents: read
28+
pull-requests: read
1629
steps:
30+
- name: Resolve PR
31+
id: pr
32+
uses: actions/github-script@v7
33+
with:
34+
script: |
35+
const n = context.payload.pull_request?.number || Number(context.payload.inputs?.pr);
36+
if (!Number.isSafeInteger(n) || n < 1) throw new Error('Invalid PR number');
37+
if (context.eventName === 'workflow_dispatch' && context.ref !== 'refs/heads/main') throw new Error('Dispatch from main only');
38+
const {data: pr} = await github.rest.pulls.get({...context.repo, pull_number:n});
39+
if (pr.state !== 'open') throw new Error('PR is closed');
40+
const expected = context.payload.pull_request?.head.sha || context.payload.inputs?.sha;
41+
if (expected !== pr.head.sha) throw new Error('PR changed before this build started; build the latest revision');
42+
core.setOutput('number', String(n));
43+
core.setOutput('sha', pr.head.sha);
44+
core.setOutput('repository', pr.head.repo.full_name);
1745
- uses: actions/checkout@v4
46+
with:
47+
repository: ${{ steps.pr.outputs.repository }}
48+
ref: ${{ steps.pr.outputs.sha }}
49+
persist-credentials: false
1850
- uses: actions/setup-node@v4
1951
with:
20-
node-version: '20'
21-
cache: 'npm'
52+
node-version: '22'
53+
cache: npm
2254
- run: npm ci
23-
- run: npm run build
24-
- name: Signal admin-may-merge for non-docs PRs
25-
if: success()
26-
uses: actions/github-script@v7
55+
- run: npm run test:review --if-present
56+
- name: Build preview
57+
run: npm run build
58+
env:
59+
DOCUSAURUS_URL: https://usace-rmc.github.io
60+
DOCUSAURUS_BASE_URL: /RMC-Software-Documentation-Previews/pr-${{ steps.pr.outputs.number }}/
61+
DOCUSAURUS_IS_PREVIEW: 'true'
62+
- uses: actions/upload-artifact@v4
2763
with:
28-
script: |
29-
const prNumber = context.payload.pull_request.number;
30-
const files = await github.paginate(github.rest.pulls.listFiles, {
31-
owner: context.repo.owner,
32-
repo: context.repo.repo,
33-
pull_number: prNumber,
34-
per_page: 100,
35-
});
36-
// Branch naming for this path is conventional, not enforced:
37-
// feature/, fix/, chore/, ci/. What actually decides the lane is
38-
// content -- a PR touching nothing under docs/ has no review lane
39-
// and needs only this build to pass.
40-
const touchesDocs = files.some(f => f.filename.startsWith('docs/'));
41-
if (touchesDocs) return; // docs PRs are handled by stage-progression.yml
42-
43-
// Flip the review-workflow commit status to success so branch
44-
// protection allows merge. This is the non-docs counterpart to
45-
// stage-progression.yml's status-setting for docs PRs.
46-
const headSha = context.payload.pull_request.head.sha;
47-
await github.rest.repos.createCommitStatus({
48-
owner: context.repo.owner,
49-
repo: context.repo.repo,
50-
sha: headSha,
51-
state: 'success',
52-
context: 'review-workflow',
53-
description: 'No doc changes — admin may merge',
54-
});
55-
56-
const marker = '<!-- ci-admin-merge-bot-comment -->';
57-
const sha = headSha.substring(0, 7);
58-
const body = `${marker}\n\n✅ **CI build passed** for commit \`${sha}\`\n\nThis PR changes no files under \`docs/\`, so it carries no review lane. **CI Build is the only gate** - there are no review stages to clear.\n\n@usace-rmc/docs-admin may merge.`;
59-
const comments = await github.rest.issues.listComments({ owner: context.repo.owner, repo: context.repo.repo, issue_number: prNumber });
60-
const existing = comments.data.find(c => c.user.type === 'Bot' && c.body.includes(marker));
61-
if (existing) {
62-
await github.rest.issues.updateComment({ owner: context.repo.owner, repo: context.repo.repo, comment_id: existing.id, body });
63-
} else {
64-
await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: prNumber, body });
65-
}
64+
name: preview-${{ steps.pr.outputs.number }}-${{ steps.pr.outputs.sha }}
65+
path: build/
66+
if-no-files-found: error
67+
retention-days: 7
68+
outputs:
69+
pr: ${{ steps.pr.outputs.number }}
70+
sha: ${{ steps.pr.outputs.sha }}

‎.github/workflows/deploy.yml‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,8 @@ on:
1212
- 'package.json'
1313
- 'package-lock.json'
1414
- 'scripts/**'
15+
- '.github/workflows/deploy.yml'
1516
workflow_dispatch:
16-
inputs:
17-
ref:
18-
description: 'Branch or ref to deploy (leave blank for main)'
19-
required: false
20-
type: string
2117

2218
permissions:
2319
contents: read
@@ -32,13 +28,19 @@ jobs:
3228
build:
3329
name: Build site
3430
runs-on: ubuntu-latest
31+
permissions:
32+
contents: read
3533
steps:
34+
- name: Require main
35+
if: github.ref != 'refs/heads/main'
36+
run: echo 'Production builds must run from main.' >&2; exit 1
3637
- uses: actions/checkout@v4
3738
with:
38-
ref: ${{ inputs.ref || github.ref }}
39+
ref: ${{ github.sha }}
40+
persist-credentials: false
3941
- uses: actions/setup-node@v4
4042
with:
41-
node-version: '20'
43+
node-version: '22'
4244
cache: 'npm'
4345
- run: npm ci
4446
- run: npm run build

0 commit comments

Comments
 (0)