Skip to content

Propose documentation updates after code merges to main - #1225

Open
dhruv8sh wants to merge 16 commits into
mainfrom
feat/docs-proposal-automation
Open

dhruv8sh wants to merge 16 commits into
mainfrom
feat/docs-proposal-automation

Conversation

@dhruv8sh

@dhruv8sh dhruv8sh commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Code merges to main that change documented behavior currently rely on the author remembering to update docs/. A new Documentation proposal workflow runs GitHub Copilot CLI on each such merge and opens one reviewable PR per merge (docs/auto/<sha12>), never committing to main.
  • Copilot runs in a job whose token cannot push or open PRs. A second read-only validate job applies the patch, enforces a docs/guide/** / docs/index.md allowlist (regular files only, so no symlinks), and runs the docs lint, format, and build gates. Only then does a third publish job, which never builds or executes proposal content, re-check the allowlist and publish. Empty diffs open no PR, a retry updates the same branch and PR, and a closed proposal is never reopened.
  • Reviewers get one review comment per hunk. It cites the merged source Copilot used and carries a suggestion that reverts the hunk in one click (a hunk that creates a new page says to delete the file instead). The revert is built from the diff itself, never from model output, so it is always exact.

Changes

File Change
.github/workflows/docs-proposal.yml New three-job workflow. propose has contents: read and copilot-requests: write. validate has contents: read. publish has contents: write and pull-requests: write. Runs one merge at a time, and a manual run with a full sha retries a merge.
.github/workflows/format.yml New docs-proposal-scripts job running shellcheck, the Node tests, and the script tests on every PR
.github/workflows/deploy-docs.yml Drop the npm cache, so a cache saved by a job that ran generated content can't be restored in the privileged Pages deploy
AGENTS.md CI gate for the documentation proposal scripts
scripts/docs-proposal/hunks.mjs Dependency-free diff parser. Lists hunks with stable ids for Copilot and builds the per-hunk revert review. Model text is sanitized: backticks, newlines, and @ mentions are stripped. The rationale renders as an inert fenced text block, so links, HTML, #N references and closing keywords stay inactive.
scripts/docs-proposal/hunks.test.mjs node:test coverage for parsing, ids, every revert-anchor case, new-file hunks, evidence sanitizing, the inert rationale block, and the review payload
scripts/docs-proposal/propose.sh Resolves the base to a full SHA, renders the prompt (portable to bash 3.2), runs Copilot, normalizes edits with Prettier, writes the patch, then runs Copilot again for per-hunk evidence. A failed evidence run keeps the proposal with empty evidence.
scripts/docs-proposal/validate.sh Applies the patch, enforces the allowlist, and runs the docs gates without write credentials
scripts/docs-proposal/publish.sh Applies the patch, re-checks the allowlist, pushes the branch, creates or edits the PR (ignoring fork PRs that reuse the branch name), and posts the review. An emptied rerun comments on the PR before closing it.
scripts/docs-proposal/lib.sh Shared SHA validation, branch naming, allowlist, and PR body helpers
scripts/docs-proposal/prompt.md, evidence-prompt.md Copilot instructions: shipped behavior only, smallest accurate edit, fictional example.com values
scripts/docs-proposal/test.sh Bash tests against a temporary bare repo with stubbed gh and npm. Covers empty diff, stale-proposal close, disallowed path, symlinks, fork PRs, first publish, unchanged retry, updated proposal, closed proposal, the evidence run and its failure, a base revision expression, and an & in the work dir.
scripts/README.md Script rows plus org and repo prerequisites
.gitignore Ignore the .docs-proposal/ work dir
docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md, docs/superpowers/plans/2026-10-01-docs-proposal-automation.md Design decisions for the issue's open questions, and the implementation plan

Closes

Closes #1105

Test plan

  • cargo test-fastly && cargo test-axum
  • cargo clippy-fastly && cargo clippy-axum
  • cargo fmt --all -- --check
  • JS tests: cd crates/trusted-server-js/lib && npx vitest run. All 1185 pass on the pinned Node 24 behavior. Locally on Node 26 they need NODE_OPTIONS=--no-experimental-webstorage, because Node's built-in localStorage shadows jsdom's. That failure is unrelated to this PR.
  • JS format: cd crates/trusted-server-js/lib && npm run format
  • Docs format: cd docs && npm run format
  • WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1 (no Rust changes)
  • Manual testing via fastly compute serve (not applicable)
  • Other:
    • node --test scripts/docs-proposal/hunks.test.mjs: 19 of 19 pass.
    • scripts/docs-proposal/test.sh passes, including with LANG/LC_ALL unset. A mutation check confirmed it fails when either the unchanged-retry guard or the path allowlist is disabled.
    • shellcheck 0.11.0 is clean on all four scripts.
    • actionlint 1.7.12 is clean except two false positives: copilot-requests and concurrency.queue. Its latest release (2026-03) predates the copilot-requests permission, which GitHub's Copilot CLI Actions docs document, and it doesn't know the concurrency.queue key.
    • Every Copilot flag was checked against @github/copilot@1.0.90 --help.
    • propose.sh was dry-run in a temporary clone with stubbed copilot and npm.
    • cd docs && npm run build passes.

Not yet exercised: a real Copilot run, which cannot happen locally. After merge, run the workflow manually with a recent merge SHA to validate end to end.

Before the first run, a maintainer must enable:

  • the organization Copilot policy "Allow use of Copilot CLI billed to the organization";
  • the repository setting "Allow GitHub Actions to create and approve pull requests".

Proposal PRs are opened with GITHUB_TOKEN, so they don't trigger other workflows. The publish job runs the docs gates itself.

Checklist

  • Changes follow AGENTS.md conventions
  • No unwrap() in production code — use expect("should ...") (no Rust changes)
  • Uses log macros (not println!) (no Rust changes)
  • New code has tests
  • No secrets or credentials committed

Record the design and implementation plan for issue #1105.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Add a Documentation proposal workflow that runs GitHub Copilot CLI on each code merge to main, proposes edits to docs/guide or docs/index.md, and opens one pull request per merge on docs/auto/<sha12>. The publish job runs the docs gates first, skips empty diffs, and never reopens a closed proposal. It also posts a review comment on every hunk that cites the merged source and offers a suggestion reverting the hunk.

Closes #1105.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>

@prk-Jr prk-Jr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

Changes are needed to keep generated content away from publication credentials, preserve queued merges, and recover missing reviews after partial publication failures.

One inline comment includes a one-click suggestion; the other two describe coordinated fixes to apply manually.

Blocking

  • 🔧 [P1] Validate generated documentation without publication credentials — see inline at scripts/docs-proposal/publish.sh:46.
  • 🔧 [P2] Preserve pending merge runs — see inline at .github/workflows/docs-proposal.yml:29 (suggestion).
  • 🔧 [P2] Resume review publication after a partial failure — see inline at scripts/docs-proposal/publish.sh:61–64.

Validation

The 16 hunk tests, existing shell integration tests, and shellcheck pass locally. A harmless allowed Markdown SSR probe passed docs lint, format, and build while reading a dummy process environment variable. A temporary failure/retry fixture reproduced missing review recovery. The proposed concurrency YAML parses; queue: max is documented by GitHub. A real Copilot run has not been exercised.

CI Status

All 21 reported checks passed when queried during this review. A later refresh encountered a reviewer-side network failure.

  • integration tests: PASS
  • browser integration tests: PASS
  • integration tests (Fastly EC lifecycle): PASS
  • CodeQL: PASS
  • cargo test (ts CLI, native): PASS
  • Analyze (javascript-typescript): PASS
  • format-typescript: PASS (required)
  • docs-proposal scripts: PASS
  • Analyze (rust): PASS
  • cargo check (cloudflare native + wasm32-unknown-unknown): PASS
  • cargo test: PASS (required)
  • cargo test (axum native): PASS
  • cargo fmt: PASS (required)
  • CLAUDE.md symlink guard: PASS
  • format-docs: PASS (required)
  • cargo check/build/test (spin native + wasm32-wasip1): PASS
  • Analyze (javascript-typescript): PASS
  • cargo test (cross-adapter parity): PASS
  • prepare integration artifacts: PASS
  • vitest: PASS
  • Analyze (actions): PASS

Comment thread scripts/docs-proposal/publish.sh Outdated
Comment thread .github/workflows/docs-proposal.yml
Comment thread scripts/docs-proposal/publish.sh Outdated

@ChristianPavilonis ChristianPavilonis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review summary

Reviewed f70ce72181e839b0bd0d2c9a93bfb56a66694c21 against cb945254521ed6dab365072f7caec10c524e323b. All 14 changed files and the proposal-to-publication path were inspected. Two additional P2 findings are posted inline. Existing feedback was checked and is not duplicated.

Validation

  • node --test scripts/docs-proposal/hunks.test.mjs: all 16 passed.
  • scripts/docs-proposal/test.sh: passed.
  • bash -n scripts/docs-proposal/*.sh: passed.
  • git diff --check cb945254521ed6dab365072f7caec10c524e323b...f70ce72181e839b0bd0d2c9a93bfb56a66694c21: passed.
  • Docs lint, format, and build passed in temporary copies.
  • A scratch check exercised 300 real Git diffs; 691 revert suggestions restored the original text.
  • All 21 reported CI checks passed. Shellcheck passed in CI and was unavailable locally.

Real Copilot authentication, organization prerequisites, live workflow publication, and GitHub-side suggestion application remain untested. Repository files were not modified and no review work was delegated.

Comment thread scripts/docs-proposal/prompt.md Outdated
Comment thread .github/workflows/docs-proposal.yml
Split the docs gates into a validate job with a read-only token and no persisted credentials, since VitePress executes Vue in generated Markdown. The publish job now re-checks the path allowlist on a fresh checkout without building. Every job runs the scripts from the workflow's revision against a separate checkout of the merge commit, so historical merges can be retried.

Queue every pending run instead of cancelling all but one, and have Copilot inspect the full pushed range so merge commits and multi-commit pushes are not missed. A retry with an unchanged tree now reuses the pushed commit and posts the review if an earlier run failed before posting it.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>

@prk-Jr prk-Jr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

Second pass at ae8d9c5. The earlier findings from both reviews (credential split, queued merges, review recovery, merge-commit diffing, tooling checkout) are addressed in f22e366a, and I re-checked them. One new blocking issue: the retry path force-pushes over maintainer commits on the proposal branch, which the design itself asks maintainers to make. There's also a hardening gap in the Copilot tool allowlist.

Blocking

  • 🔧 [P1] Retries force-push over maintainer commits. See inline at scripts/docs-proposal/publish.sh:57-62 (suggestion). Reproduced.

Should fix

  • 🤔 [P2] shell(git grep:*) allows running any command via --open-files-in-pager. See inline at scripts/docs-proposal/propose.sh:39 (suggestion). Reproduced.
  • 🤔 [P2] Drop the npm cache in the Copilot job. See inline at .github/workflows/docs-proposal.yml:72-73 (suggestion).
  • 🤔 [P2] Rationale goes into the PR body without @-mention stripping. See inline at scripts/docs-proposal/lib.sh:66-68.

Nits / notes

  • ⛏ No timeout-minutes on the Copilot job. See inline at .github/workflows/docs-proposal.yml:44 (suggestion).
  • 📝 Merging this PR triggers the workflow itself (scripts/README.md matches scripts/**). The first run will fail unless the org Copilot policy and the "Allow GitHub Actions to create pull requests" setting are already on.
  • 📝 Self-trigger loop: none. Proposal PRs touch only docs/guide/** / docs/index.md and are opened with GITHUB_TOKEN.
  • 📝 The AGENTS.md "CI Gates" list doesn't mention the new docs-proposal scripts job.
  • 📝 The PR body says actionlint is clean except copilot-requests, but it also flags concurrency.queue. Both are actionlint false positives.
  • ⛏ scripts/README.md says "runs both proposal scripts", but there are three (propose, validate, publish).
  • ⛏ prompt.md / evidence-prompt.md hardcode .docs-proposal/, while propose.sh accepts any work dir.
  • 🌱 Cost: nearly every merge touches crates/**, so each runs Copilot twice. Consider skipping dependency-bump and bot merges.

Validation

  • scripts/docs-proposal/test.sh: pass
  • node --test scripts/docs-proposal/hunks.test.mjs: 16/16 pass
  • shellcheck scripts/docs-proposal/*.sh (0.11.0): clean
  • actionlint 1.7.12: only the two false positives above
  • Force-push repro against a bare repo: maintainer commit lost on rerun; kept with the suggested fix
  • CI: all 21 checks pass

A real Copilot run and a live publication remain untested.

Comment thread scripts/docs-proposal/publish.sh
Comment thread scripts/docs-proposal/propose.sh Outdated
Comment thread .github/workflows/docs-proposal.yml Outdated
Comment thread scripts/docs-proposal/lib.sh Outdated
Comment thread .github/workflows/docs-proposal.yml

@ChristianPavilonis ChristianPavilonis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review summary

Reviewed ae8d9c5a9ff581cfe462745bb6970b5a5277cdbe against 80483011e30a446ac741b4423e36f8d142c5d2ac. All 15 changed files and the proposal-to-publication flow were inspected. One additional P2 finding is posted inline. Existing branch-overwrite and hardening findings are not repeated.

Safety proof

  • Publication rechecks the path allowlist and does not invoke the docs gates. Executed evidence: the script integration tests rejected forbidden changes before publication and successful publication invoked no npm commands. Proven for the exercised script paths.
  • Generated revert suggestions restore the original changed text. Executed evidence: all 16 hunk tests passed, and a scratch check applied 506 generated revert suggestions across 300 real Git diffs, restoring the original lines in every case. Proven for the tested text cases.

Validation and review context

  • node --test scripts/docs-proposal/hunks.test.mjs: 16/16 passed.
  • scripts/docs-proposal/test.sh: passed.
  • for script in scripts/docs-proposal/*.sh; do bash -n "$script" || exit; done: passed.
  • git diff --check 80483011e30a446ac741b4423e36f8d142c5d2ac...ae8d9c5a9ff581cfe462745bb6970b5a5277cdbe: passed.
  • Scratch Node and Python checks: revert round trips passed; the manual-retry defect was reproduced using the real proposal, validation, and publication scripts with Copilot, npm, and GitHub calls stubbed.
  • CI: all 21 reported checks passed, with no failures or skipped checks. Shellcheck was unavailable locally but passed in CI.
  • Existing reviews, inline comments, replies, issue comments, and thread resolution states were inspected.
  • Real Copilot authentication, live workflow isolation/publication, and GitHub-side suggestion application remain untested.
  • Revision remained unchanged; worktree stayed clean. No files modified or review work delegated.

Comment thread scripts/docs-proposal/propose.sh
A retry for the same merge regenerated the proposal and force-pushed it, discarding commits maintainers added on top, including applied revert suggestions. An empty rerun also closed the pull request and deleted that branch. publish.sh now leaves a branch alone when its head is not the generated proposal commit, and pushes with a lease instead of a bare force.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
git grep --open-files-in-pager (or -O) runs an arbitrary command, so allowing shell(git grep:*) let content in the merged change steer Copilot into executing commands in the propose job. Copilot's built-in file reads cover what git grep was used for.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Copilot can write files in the propose job that propose.sh then executes, and validate builds generated Markdown that VitePress runs as Vue. Either job could therefore save a poisoned main-scoped setup-node cache entry that later runs restore. Neither job restores or saves a dependency cache now, and the design spells out that the job, not the tool allowlist, is the trust boundary.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Copilot CLI has no limit of its own, so a hung run inherited the 360-minute default and, with the concurrency queue, delayed every later merge's proposal. The propose, validate, and publish jobs now time out after 30, 15, and 10 minutes.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Evidence already had its @ mentions broken because it is model output, but up to 20 KB of Copilot's rationale went into the pull request body verbatim, so a rationale naming a team would ping it on every proposal. hunks.mjs gains a sanitize subcommand that inserts a zero width space after each @, which the PR body helper now pipes the rationale through.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
A manual dispatch had no base, so retrying a multi-commit push such as a rebase merge narrowed the inspected range to the final commit. The retry could then miss documented behavior, or produce an empty patch that closed and deleted a valid proposal.

The dispatch gains an optional base input. propose.sh records the resolved base in the artifact and warns when it discards a supplied base. publish.sh records the base in the pull request body and fails, naming the original base, when an open proposal was generated from a different range.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
AGENTS.md now lists the docs-proposal scripts job among the CI gates. scripts/README.md names all three scripts the workflow runs, and warns that the run triggered by merging the workflow fails unless the Copilot policy and pull request setting are already enabled.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
propose.sh accepts any work directory, but both prompts told Copilot to write its outputs under .docs-proposal/. The prompts now use a <work-dir> placeholder that propose.sh replaces with the work directory relative to the repository root.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
…tomation

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>

@ChristianPavilonis ChristianPavilonis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review summary

Reviewed b3e3b22066d8a72b6ef767e5beab272c0f016d10 against 7a0ecb4cbfa39af3d83f7e1ed2733d7eabdc4cb1, from feat/docs-proposal-automation into main. Inspected all 16 changed files and traced generation, artifacts, validation, publication, reviews, and retries. One P2 finding is posted inline.

Safety proof

Publication rejects forbidden paths without running the docs build. Executed evidence: scripts/docs-proposal/test.sh passed; forbidden changes were rejected through lib.sh:48 before publication, and successful publication invoked no npm commands. Proven for the exercised script paths.

Validation and review context

  • node --test scripts/docs-proposal/hunks.test.mjs: 17/17 passed.
  • scripts/docs-proposal/test.sh: passed.
  • for script in scripts/docs-proposal/*.sh; do bash -n "$script" || exit; done: passed.
  • shellcheck scripts/docs-proposal/*.sh, version 0.11.0: passed.
  • git diff --check 7a0ecb4cbfa39af3d83f7e1ed2733d7eabdc4cb1...b3e3b22066d8a72b6ef767e5beab272c0f016d10: passed.
  • In a scratch archive: npm ci --no-audit --no-fund && npm run lint && npm run format && npm run build: passed. Changed Markdown outside docs/ also passed Prettier.
  • Scratch revert verification: 644 suggestions across 299 real Git diffs restored the original text. An initial fixture failure came from local nonstandard diff prefixes; isolated Git configuration passed.
  • Actionlint 1.7.12 reported unsupported but documented queue and copilot-requests keys, plus unquoted-output ShellCheck warnings in format.yml. No behavioral finding arose from those diagnostics.
  • CI: all 23 reported checks passed at the reviewed head.
  • Existing feedback: inspected review bodies, comments, replies, and all 11 resolved threads. The inline finding adds concurrent-push and fetch-failure evidence beyond the earlier branch-overwrite feedback.
  • Residual risk: real Copilot authentication, live workflow isolation/publication, and GitHub-side suggestion application remain untested.
  • Repository unchanged; no review work delegated.

Comment thread scripts/docs-proposal/publish.sh Outdated
An empty rerun deleted the proposal branch via gh pr close --delete-branch, so a maintainer push after the head inspection was lost, and a failed fetch hid existing maintainer commits. Look up the branch with git ls-remote --exit-code and fail unless it is verifiably absent, delete it with --force-with-lease on the inspected head, and close the pull request only after the deletion succeeds.

Add regressions for a concurrent maintainer push, fetch failures on the empty and push paths, and the lease-guarded deletion.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>

@prk-Jr prk-Jr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

The design holds up well: Copilot runs in a job whose token can't publish, the allowed docs paths are re-checked downstream, and the revert suggestions are built from the diff rather than from model output. One blocking issue remains: the proposal-PR lookup also matches PRs from forks. Four non-blocking follow-ups cover rationale rendering, cache writes, and two edge cases.

2 of the inline comments below carry a one-click GitHub suggestion. Use Commit suggestion (or Add suggestion to batch) to apply them. The others describe the fix in prose because the change spans several files or several places in a file.

Blocking

🔧 wrench

  • Proposal lookup matches fork PRs with the same branch name: see inline at scripts/docs-proposal/publish.sh:29

Non-blocking

🤔 thinking

  • Rationale still renders closing keywords, #N references and links: see inline at scripts/docs-proposal/lib.sh:72
  • The propose job can still write a cache that deploy-docs restores: see inline at scripts/docs-proposal/propose.sh:67

⛏ nitpick

  • The revert suggestion for a new page leaves an empty file: see inline at scripts/docs-proposal/hunks.mjs:227
  • A non-SHA base input is only rejected at publish time: see inline at scripts/docs-proposal/propose.sh:29

👍 praise

  • The revert suggestion is built from the diff, not from model output, and its fence grows to fit any backtick run.
  • --force-with-lease everywhere, including the empty-lease "must not exist" case and the delete path.
  • An unchanged tree reuses the earlier commit, so a retry resumes a review that was never posted.
  • The allowed-paths check fails closed: --no-renames is set, and quoted paths (non-ASCII, newline, traversal) never match the regex.

Cross-cutting / body-level findings

  • 📝 actionlint errors are false positives: actionlint 1.7.12 rejects concurrency.queue: max and the copilot-requests: write permission, but both are valid GitHub syntax. No change needed.
  • 📝 Test count: hunks.test.mjs has 17 tests (all pass locally); the PR description says 16.

Verified locally at 0bc4bf7a: node --test scripts/docs-proposal/hunks.test.mjs (17/17), bash scripts/docs-proposal/test.sh (pass), shellcheck scripts/docs-proposal/*.sh (clean).

CI Status

All 23 checks pass, including docs-proposal scripts, Analyze (actions), CodeQL, cargo fmt (required), cargo test (required), format-typescript (required) and format-docs (required).

Comment thread scripts/docs-proposal/publish.sh Outdated
Comment thread scripts/docs-proposal/lib.sh Outdated
Comment thread scripts/docs-proposal/propose.sh
Comment thread scripts/docs-proposal/hunks.mjs
Comment thread scripts/docs-proposal/propose.sh

@ChristianPavilonis ChristianPavilonis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review summary

Reviewed 0bc4bf7a13eca8e68275055996a547fb171b74d0 against 7a0ecb4cbfa39af3d83f7e1ed2733d7eabdc4cb1, from feat/docs-proposal-automation into main. Inspected all 16 changed files and traced generation, artifacts, validation, publication, retries, and review suggestions.

No new, non-duplicate actionable findings. This approval records the result of this pass; it does not establish that existing unresolved feedback has been addressed.

Safety proof

  • Publication rejects forbidden paths without running the docs build. Executed evidence: scripts/docs-proposal/test.sh passed, exercising rejection through lib.sh:45–49 and verifying successful publication invokes no npm commands. Proven for the exercised script paths.
  • Added maintainer commits survive retries and raced deletion. Executed evidence: the script tests exercise publish.sh:60–71 against a temporary bare repository. Maintainer commits remain intact, concurrent deletion fails under the lease, and fetch failures prevent publication or closure. Proven for the exercised cases.

Validation and review context

  • node --test scripts/docs-proposal/hunks.test.mjs: 17/17 passed.
  • scripts/docs-proposal/test.sh: passed.
  • /home/pav/.local/share/mise/installs/shellcheck/0.11.0/shellcheck-v0.11.0/shellcheck scripts/docs-proposal/*.sh: passed.
  • for script in scripts/docs-proposal/*.sh; do bash -n "$script" || exit; done: passed.
  • git diff --check 7a0ecb4cbfa39af3d83f7e1ed2733d7eabdc4cb1...0bc4bf7a13eca8e68275055996a547fb171b74d0: passed.
  • In a temporary archive, npm ci --no-audit --no-fund && npm run lint && npm run format && npm run build: passed. Changed Markdown outside docs/ also passed Prettier.
  • Inline Python/Node round-trip check: 686 suggestions across 300 real Git text diffs restored the original lines.
  • Pinned @github/copilot@1.0.90 --help: confirmed the invoked flags.
  • Actionlint 1.7.7 reported unsupported queue and copilot-requests keys, both confirmed in current GitHub documentation, plus unquoted-output ShellCheck warnings. No behavioral finding arose from these diagnostics.
  • CI: all 23 reported checks passed. The format workflow's head SHA matches the reviewed revision.
  • Existing feedback: inspected reviews, inline comments, replies, issue comments, and thread resolution states. Five threads remain unresolved, covering fork-PR selection, rationale rendering, cache writes, new-page reverts, and base-input normalization. Existing findings are not duplicated.
  • Residual risk: real Copilot authentication, live workflow isolation/publication, cache isolation, and GitHub-side suggestion application remain unverified. Rust and browser suites were not rerun locally.
  • Revision unchanged; worktree clean. No repository files modified or review work delegated.

@aram356 aram356 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

Reviewed 0bc4bf7a1 against main (merge base 7a0ecb4cb). One blocking gap: the path allowlist checks paths but not file modes, so a symlink under docs/guide/ passes both checks and VitePress publishes its target. The rest are smaller fixes to the empty-rerun close path, test coverage, local portability of the new CI gate, and docs that trail the code.

7 of the inline comments below carry a one-click GitHub suggestion. Use Commit suggestion (or Add suggestion to batch for several at once) to apply them. The other 2 describe the fix in prose because it changes test.sh in more than one place or together with publish.sh.

Blocking

🔧 wrench

  • The path allowlist admits symlinks, so a proposal can publish pages the site excludes: see inline at scripts/docs-proposal/lib.sh:51

Non-blocking

🤔 thinking

  • An empty rerun probably closes the proposal without its explanation: see inline at scripts/docs-proposal/publish.sh:72-73
  • CI never runs the evidence half of propose.sh: see inline at scripts/docs-proposal/test.sh:92-95
  • A failed evidence run discards a finished proposal: see inline at scripts/docs-proposal/propose.sh:84
  • The committed plan still prescribes the designs the reviews removed: see inline at docs/superpowers/plans/2026-10-01-docs-proposal-automation.md:3
  • Proposal PRs can't merge until someone starts CI by hand: see inline at scripts/README.md:36-38

♻️ refactor

  • render_prompt writes ".docs-proposal"/rationale.md under bash 3.2: see inline at scripts/docs-proposal/propose.sh:38-42
  • The mention assertion depends on the caller's locale: see inline at scripts/docs-proposal/test.sh:50

⛏ nitpick

  • The spec's trigger bullet predates the base input and validate.sh: see inline at docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md:50-52

Cross-cutting / body-level findings

  • 🌱 Check that cited sources exist. Model citations are the weakest input to the per-hunk review. publish.sh could run git cat-file -e "$sha:<path>" on each evidence source, and check the line number against the file's length, then mark misses as "cited source not found" instead of passing them through. Not for this PR.
  • 📝 The PR description trails the code. It says "New two-job workflow" (there are three jobs now: propose, validate, publish), leaves validate.sh out of the Changes table, and says 16 of 16 hunk tests pass (there are 17).
  • 📝 Not repeated here: prk-Jr's five open threads at this head (fork-PR lookup, rationale rendering, cache writes, new-file revert, base normalization) still apply.

Validation

  • node --test scripts/docs-proposal/hunks.test.mjs: 17/17 pass. shellcheck scripts/docs-proposal/*.sh: clean.
  • scripts/docs-proposal/test.sh: passes on Linux bash 5.2 with LANG=C.UTF-8 (Docker node:24, matching the runners); 1 failure without LANG; 3 failures under macOS /bin/bash 3.2.57.
  • Symlink probe: docs_proposal_apply accepts a 120000 entry. npm ci, lint, format, and build run on the docs tree at this head show the symlinked superpowers/** spec published at dist/guide/zz-probe.html.
  • Every suggestion was applied alone to a clean checkout of this head and verified with shellcheck, the hunk tests, and test.sh on all three shells (or the Prettier gate for Markdown), with no drift between the applied bytes and the verified tree. All of them applied together, plus the two prose fixes, pass on all three shells.
  • Each proposed test fails without its fix: the symlink case fails both assertions without the lib.sh change, the evidence test fails three assertions against a broken hunks.mjs list call, and the new close assertion fails without the publish.sh change.
  • concurrency.queue: max is documented GitHub syntax (up to 100 pending runs), so actionlint's error there is a false positive.

CI Status

  • Analyze (actions): PASS
  • Analyze (javascript-typescript): PASS
  • Analyze (javascript-typescript): PASS
  • Analyze (python): PASS
  • Analyze (rust): PASS
  • browser integration tests: PASS
  • cargo check (cloudflare native + wasm32-unknown-unknown): PASS
  • cargo check/build/test (spin native + wasm32-wasip1): PASS
  • cargo fmt: PASS (required)
  • cargo test: PASS (required)
  • cargo test (axum native): PASS
  • cargo test (cross-adapter parity): PASS
  • cargo test (ts CLI, native) (macos-latest): PASS
  • cargo test (ts CLI, native) (ubuntu-latest): PASS
  • CLAUDE.md symlink guard: PASS
  • CodeQL: PASS
  • docs-proposal scripts: PASS
  • format-docs: PASS (required)
  • format-typescript: PASS (required)
  • integration tests: PASS
  • integration tests (Fastly EC lifecycle): PASS
  • prepare integration artifacts: PASS
  • vitest: PASS

Comment thread scripts/docs-proposal/lib.sh
Comment thread scripts/docs-proposal/publish.sh Outdated
Comment thread scripts/docs-proposal/test.sh
Comment thread scripts/docs-proposal/propose.sh Outdated
Comment thread scripts/docs-proposal/propose.sh
Comment thread scripts/docs-proposal/test.sh Outdated
Comment thread docs/superpowers/plans/2026-10-01-docs-proposal-automation.md
Comment thread scripts/README.md Outdated
Comment thread docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md Outdated
…onale

Ignore fork pull requests that reuse the proposal branch name, reject non-regular files such as symlinks, and render the rationale as an inert text block. Comment before closing an emptied proposal, keep a proposal when the evidence run fails, resolve a manual base to a full SHA, and fill prompts portably across bash versions.

Ask for deletion instead of an empty suggestion on new pages, drop the npm cache from the Pages deploy, mark the original plan superseded, and cover the evidence half, symlinks, and fork pull requests in the script tests.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
…tomation

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>

# Conflicts:
#	AGENTS.md
@dhruv8sh
dhruv8sh requested review from aram356 and prk-Jr October 10, 2026 06:31

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Automatically open PR for documentation changes when code merged to main

4 participants