Skip to content

CI check over .md files in root folder - #1177

Open
dhruv8sh wants to merge 7 commits into
mainfrom
chore/check-root-markdown-formatting
Open

dhruv8sh wants to merge 7 commits into
mainfrom
chore/check-root-markdown-formatting

Conversation

@dhruv8sh

Copy link
Copy Markdown
Collaborator

Summary

  • Extend the format-docs CI job to check root-level Markdown (AGENTS.md, README.md, FAQ_POC.md, etc.) with Prettier, closing the gap where only docs/ was covered
  • Normalize existing formatting drift in the three root files that were failing (AGENTS.md, FAQ_POC.md, ProjectGovernance.md) — whitespace/list-marker only, no content changes
  • Document the new gate in AGENTS.md's CI Gates checklist

Changes

File Change
.github/workflows/format.yml Added a step to the format-docs job that runs docs/node_modules/.bin/prettier --config docs/.prettierrc --check "*.md" from the repo root, reusing the already-installed binary and the existing docs/.prettierrc config
AGENTS.md Reformatted (Prettier drift only); added item 8 to the CI Gates list documenting the new root-Markdown check
FAQ_POC.md Reformatted (Prettier drift only)
ProjectGovernance.md Reformatted (Prettier drift only)

Closes #1093

Test plan

  • Docs format: cd docs && npm run format
  • Root Markdown format: docs/node_modules/.bin/prettier --config docs/.prettierrc --check "*.md" (run from repo root)
  • Ran the format-docs and check-claude-md-symlink GitHub Actions jobs locally via act — both pass, including the new step
  • Verified CLAUDE.md still resolves as a symlink to AGENTS.md (git ls-tree mode 120000) after formatting AGENTS.md
  • 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
  • JS format: cd crates/trusted-server-js/lib && npm run format
  • WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
  • Manual testing via fastly compute serve
  • Other: docs-only change, no Rust/JS code touched — Rust/JS gates are unaffected and not re-run

Checklist

  • Changes follow AGENTS.md conventions
  • No unwrap() in production code — use expect("should ...")
  • Uses tracing macros (not println!)
  • New code has tests — N/A, CI workflow change and Markdown-only edits
  • No secrets or credentials committed

@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.

Review: approve with nits

Verified the gate empirically in a scratch worktree at the PR head (71264e87):

  • The gate has teeth. Run against main's root .md tree it exits 1 (AGENTS.md, CLAUDE.md, FAQ_POC.md, ProjectGovernance.md warned); run against this PR's tree it exits 0 (All matched files use Prettier code style!).
  • Correct binary. docs/node_modules/.bin/prettier is the lockfile-pinned 3.8.1 (docs/package-lock.json), not npx — which in this repo resolves a different version and produces false format failures. The step also sits after npm ci in the same job, so the binary exists, and working-directory: . resolving to the workspace root matches the in-file precedent of the "Retrieve Node.js version" step.
  • Glob coverage is complete. Copying all 7 regular root .md files plus a real CLAUDE.md -> AGENTS.md symlink into a scratch dir and appending a violation to each regular file warns on exactly 7 files. So README.md, CONTRIBUTING.md, CHANGELOG.md and TESTING.md are covered by the glob and were already compliant; nothing that should be covered is missed. CLAUDE.md is correctly skipped (prettier does not follow the symlink), so AGENTS.md isn't checked twice — and the dedicated symlink-guard job runs first anyway.
  • No ignore-file interference. No root .prettierignore; docs/.prettierignore isn't on the cwd path for this invocation; .gitignore has no .md entries.
  • Content changes are formatting-only. proseWrap: "preserve" means no reflow: AGENTS.md is table realignment plus the new checklist entry, ProjectGovernance.md is *- bullet markers, FAQ_POC.md is trailing-whitespace and double-space collapse. No semantic drift.

Nothing blocking. Four non-blocking notes inline, plus two things outside the diff:

  • *.md is non-recursive, which matches the PR title, but it leaves 19 tracked Markdown files ungated (.github/pull_request_template.md, four crate README.mds, 14 files under .claude/). Fine to defer — worth an issue if the intent is eventually "all Markdown is formatted".
  • Is format-docs a required status check? I couldn't read branch protection. If it isn't required, this gate is advisory only.

Both inline suggestions were applied in isolation and as a batch against a scratch worktree and re-verified green (exit 0, no drift), then reverted.

Comment thread AGENTS.md Outdated
Comment thread FAQ_POC.md Outdated
Comment thread .github/workflows/format.yml
Comment thread .github/workflows/format.yml
Document the --write fix in the CI checklist, run the root Markdown
check after the VitePress dead-link build so both failures surface in
one CI round, and fix a couple of long-standing typos in FAQ_POC.md
touched by the same line.

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.

Summary

Reviewed 1851e98101e68c17d900506c2ad0ac145dd6dbb7 against 6cae7f5da8911c746cf873581885f90c3820dd96. No actionable issues found.

The new root-Markdown check passed in the exact PR workflow. A separate negative test with the lockfile-pinned Prettier 3.8.1 rejected deliberately malformed copies of all seven regular root .md files, while CLAUDE.md remained a symlink to AGENTS.md. All reported CI checks pass, and the active main ruleset requires format-docs.

@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

Adds a format-docs step that runs Prettier over root-level Markdown, closing the gap where only docs/ was covered, and normalizes existing drift in three root files. The gate itself works — I ran the exact command against the PR head, injected drift, confirmed it fails, ran the documented --write fix, and confirmed it passes again. All 20 CI checks pass.

One blocking finding: the step is ordered after the VitePress build, which defeats the stated purpose of the commit that introduced that ordering.

2 of the inline comments below carry a one-click GitHub suggestion — use Commit suggestion (or Add suggestion to batch) to apply them. The remaining findings are body-level because they concern the PR description or span the repository rather than a single line.

Blocking

🔧 wrench

  • Root Markdown check is skipped whenever the VitePress build fails — see inline at .github/workflows/format.yml:155

Non-blocking

🤔 thinking

  • Documented local command exits 127 without a prior npm ci — see inline at AGENTS.md:348

Cross-cutting / body-level findings

  • FAQ_POC.md is described as "Prettier drift only" but carries manual content edits — I reformatted the merge-base copy with this PR's own Prettier config and diffed it against the PR head. ProjectGovernance.md is byte-identical to pure Prettier output, and AGENTS.md differs only by the intentional CI-gate item 8. FAQ_POC.md line 5 has two edits Prettier would never make: punisherpublisher, and current form.Its a proofcurrent form. It is a proof. Both are correct fixes and commit 5c7d4f390 mentions them, but the PR description's table still reads "Reformatted (Prettier drift only)" for this file, which will lead a reviewer to skip the one line that actually changed meaning. Worth a description edit rather than a code change.

  • 🌱 The gate covers 8 of 27 non-docs/ Markdown files, and 4 uncovered ones are already drifted"*.md" is correctly quoted, so Prettier (not the shell) expands it and the scope is root depth-1 only. That is a reasonable scope for closing #1093. Running the same config over the rest shows these would fail if included today: crates/trusted-server-core/README.md, crates/trusted-server-integration-tests/README.md, .claude/agents/pr-reviewer.md, .claude/skills/deploying-trusted-server-to-fastly/SKILL.md. Flagging only so the remaining gap stays a deliberate follow-up rather than an assumed win.

Verified as safe

Three things worth recording, since they are the non-obvious risks in a change that globs *.md at the repository root:

  • The CLAUDE.md symlink is not at risk. CLAUDE.md is a 120000 symlink to AGENTS.md and is matched by the glob. Prettier deduplicates it: with drift injected, --list-different reported only AGENTS.md, and after --write the symlink was still CLAUDE.md -> AGENTS.md. The check-claude-md-symlink guard job cannot be tripped by this gate.
  • No .prettierignore gap. docs/.prettierignore lists only node_modules and .vitepress/*; at root depth-1 there is nothing it would need to exclude.
  • No double-checking of docs/. The existing npm run format runs with cwd=docs, so its scope and the new root glob do not overlap.

CI Status

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

Comment thread .github/workflows/format.yml Outdated
Comment thread AGENTS.md Outdated
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
@dhruv8sh
dhruv8sh requested a review from aram356 September 21, 2026 19:55
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.

Root Markdown is not covered by any format gate

4 participants