[cuipinghuo]

Summary

• Adds bin/validate-docs.sh script and make validate-docs Makefile target for lightweight documentation validation that works without external repos or network access
• Validates AsciiDoc syntax (via asciidoctor if installed), broken xref: targets, broken include:: partials, and navigation references
• Adds a CI workflow (.github/workflows/validate-docs.yaml) that runs validation on PRs touching modules/**
• Documents the new command in CLAUDE.md under "Common Commands"

Closes #240

Jira: https://redhat.atlassian.net/browse/EC-1984

Test plan

• make validate-docs passes on current repo content
• Broken xrefs are detected (tested with deliberate broken xref)
• Broken includes are detected (tested with deliberate broken partial)
• Cross-component xrefs (e.g. xref:ecc:ROOT:...) are correctly skipped
• Script degrades gracefully when asciidoctor is not installed
• CI workflow runs successfully on this PR

🤖 Generated with Claude Code

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 74b6f184-4ef6-410c-aeec-a2637bf52c19

---

_{Comment @coderabbitai help to get the list of available commands.}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 2:58 AM UTC · Completed 3:07 AM UTC

Commit: cd01138 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-7 · Effort: high · Cost: $3.59

[fullsend-ai-review]

Review

Findings

Medium

• [protected-path] .github/workflows/validate-docs.yaml, CLAUDE.md — This PR modifies files under governance/infrastructure paths (.github/, CLAUDE.md). The linked issue (Add lightweight AsciiDoc validation for agent and CI use #240) and PR body explain the rationale for the change, so this is a raise-for-visibility notice: human approval is always required for protected-path modifications regardless of context.
  Remediation: none — merge decision belongs to a human reviewer.

Low

• [injection] bin/validate-docs.sh:46 — asciidoctor is invoked with --safe-mode=unsafe on .adoc files sourced from PRs. Trigger is pull_request (not pull_request_target), so fork code runs with a read-only GITHUB_TOKEN and no secrets — realistic attack surface is limited to reading ephemeral runner state and echoing it via output=$(asciidoctor ... 2>&1) into public workflow logs. Still, unsafe grants strictly more capability than this validation needs.
  Remediation: Use --safe-mode=safe (or server). safe still permits includes but restricts them to files under the base directory, which is what this workflow needs. If the symlinked partials resolution breaks under safe, use server. Reserve unsafe for trusted local author workflows.

• [ci-coverage] .github/workflows/validate-docs.yaml:8 — The workflow's paths filter (modules/**, antora.yml, bin/validate-docs.sh, Makefile) does not include .github/workflows/validate-docs.yaml itself. A PR that modifies only this workflow will not exercise it on that PR — edits to trigger config, action pins, or the install step land without self-validation.
  Remediation: Add .github/workflows/validate-docs.yaml to the paths list.

• [logic] bin/validate-docs.sh:86 — validate_xrefs globs every .adoc file under modules/, which includes nav.adoc and partials/contents.adoc. validate_nav then re-processes those same two files with identical xref-target logic. Any broken xref in either file is reported twice and counted twice toward the failure total, inflating the error count.
  Remediation: Exclude nav.adoc and partials/contents.adoc from validate_xrefs's find, or remove the redundant xref pass from validate_nav and let it only verify nav-file existence.

• [logic] bin/validate-docs.sh:82 — Bare same-page anchor xrefs (e.g. xref:my-section[label] with no .adoc, no #, no :) would be mis-flagged: grep captures xref:my-section[, trimming yields target=my-section, the :/{/empty guards do not fire, and the final -f ${PAGES_DIR}/my-section check fails. Currently no such xref exists in the codebase, so no false positive today — latent.
  Remediation: Skip targets that do not end in .adoc (or explicitly detect anchor-only forms) before the file-existence check.

• [supply-chain] .github/workflows/validate-docs.yaml:20 — step-security/harden-runner is configured with egress-policy: audit, which logs outbound network calls without blocking them. Given pull_request trigger (no secrets, contents: read) the impact is bounded, but block mode is the standard defense-in-depth posture once endpoints are known.
  Remediation: After a short audit-mode learning period, switch to egress-policy: block with an explicit allowed-endpoints list covering github.com:443, api.github.com:443, objects.githubusercontent.com:443, rubygems.org:443, index.rubygems.org:443.

• [usability] bin/validate-docs.sh:48 — asciidoctor runs against files under ${WORK_DIR}/pages/ (a mktemp directory), so any warning/error cites a path like /tmp/tmp.XXXX/pages/foo.adoc:LINE rather than modules/ROOT/pages/foo.adoc:LINE. Users and agents cannot jump directly to the offending line in the repo. Issue Add lightweight AsciiDoc validation for agent and CI use #240 explicitly wants agents to get pass/fail feedback while editing .adoc files, so this hurts the primary use case.
  Remediation: Post-process asciidoctor's output to rewrite ${WORK_DIR}/pages/ back to modules/ROOT/pages/ before echoing it (e.g., pipe through sed "s|${WORK_DIR}/pages/|modules/ROOT/pages/|g").

• [logic] bin/validate-docs.sh:37 — The partial$ → ../partials/ perl -pi rewrite is applied only to files under WORK_DIR/pages/, not to the partials themselves. Because WORK_DIR/partials is a symlink to the real modules/ROOT/partials/, if any partial ever contains include::partial$foo.adoc[], asciidoctor will fail to resolve it during syntax validation. No partial currently nests another partial — latent.
  Remediation: Copy partials/ into WORK_DIR/partials/ (instead of symlinking) and run the same perl -pi rewrite over it, or handle the partial$ prefix uniformly across all included files.

Info

• [permission-expansion] .github/workflows/validate-docs.yaml:14 — New workflow declares permissions: contents: read. Verified: top-level grants the minimum required for actions/checkout on a public repo; no job-level override; no secrets: are referenced; trigger is pull_request (not pull_request_target), so fork code runs without secrets or a write token. Least-privilege confirmed.

• [provenance-warning] N/A — Prior review context was discarded because provenance validation returned unverifiable-wrong-app. This review treats all findings as first-time assessments; no severity anchoring was applied.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run

Review

The PR cleanly implements the three items authorized by issue #240 (validate-docs target, CLAUDE.md docs, optional CI workflow). Design fits existing conventions (bash script under bin/, Makefile target). A handful of items are worth addressing before merge — most notably GNU-only tool flags that will break the documented local workflow on macOS, and asciidoctor --safe-mode=unsafe on repo-supplied content that becomes a real exposure if this workflow ever gains secrets or write permissions.

Findings

Medium

• [security/sandbox] bin/validate-docs.sh:40 — asciidoctor is invoked with --safe-mode=unsafe on .adoc files that originate from the PR head. In unsafe mode Asciidoctor permits include:: of absolute paths, follows symlinks (the script deliberately symlinks PARTIALS_DIR into the work dir), and enables ruby-based extensions and docinfo / allow-uri-read attributes. Today the blast radius is small (contents: read, no secrets, pull_request not pull_request_target), but this becomes a real exposure the moment this workflow gains a secret or write permission.
  Remediation: use --safe-mode=server (or safe) for validation; if a stricter mode breaks legitimate constructs, allow them via specific attributes rather than downgrading the whole sandbox.

• [correctness/portability] bin/validate-docs.sh:42 — sed -i 's|...|...|g' {} uses GNU sed's no-suffix in-place form, which fails on BSD/macOS sed (which requires an explicit backup suffix, e.g. sed -i ''). CLAUDE.md advertises this command as "Run this after editing any .adoc file to catch errors before pushing," so developers running it locally on macOS will get an unexpected error. CI on Ubuntu is unaffected.
  Remediation: use a portable form such as perl -pi -e 's|include::partial\$|include::../partials/|g', or emit transformed content to a new file rather than mutating in place.

• [correctness/portability] bin/validate-docs.sh — grep -noP uses the PCRE flag, a GNU extension not available on stock macOS BSD grep. Same local-workflow break as above. The regexes here are simple enough to be expressed as ERE.
  Remediation: replace grep -noP 'xref:([^\[]+)\[' with grep -noE 'xref:[^[]+\[' and adjust similarly for include::.

• [protected-path] .github/workflows/validate-docs.yaml, CLAUDE.md — the PR modifies files under protected paths (.github/, CLAUDE.md). Authorization exists (issue Add lightweight AsciiDoc validation for agent and CI use #240 explicitly requests both the CI workflow and the CLAUDE.md documentation), so this is a medium notice rather than a blocking concern: human approval is always required for protected-path changes regardless of context.

Low

• [correctness/xref-block-context] bin/validate-docs.sh — the xref/include regex scans raw file text without excluding AsciiDoc listing blocks (----), literal blocks (....), pass-through blocks (++++), or single-line/block comments (//, ////). Current docs happen not to contain xref: inside such blocks, but any future example that documents xref usage inside a [source,asciidoc] block or a comment will produce a spurious "broken xref" failure and fail CI.
  Remediation: strip listing/comment blocks before grepping, or (simpler) rely solely on asciidoctor's own xref/include resolution warnings and drop the regex-based checks.

• [correctness/xref-edge-cases] bin/validate-docs.sh — the xref regex produces false positives for attribute-substituted targets like xref:{page}[...] and for same-page anchors written as xref:#anchor[...]. In the latter case, ${target%%#*} yields the empty string, so the check becomes [[ ! -f "${PAGES_DIR}/" ]] — always true — and reports a spurious broken xref.
  Remediation: skip targets that start with { or that are empty after stripping the anchor.

• [correctness/nav-redundant] bin/validate-docs.sh — validate_nav largely duplicates validate_xrefs: the latter runs find "${MODULES_DIR}" -name '*.adoc', which already includes nav.adoc and partials/contents.adoc. Every broken nav xref will be reported twice, inflating the final error count.
  Remediation: either exclude nav.adoc/contents.adoc from validate_xrefs' find, or drop the duplicate xref loop in validate_nav and keep only the "nav file not found" guard.

• [correctness/workflow-paths] .github/workflows/validate-docs.yaml:8 — the paths filter omits .github/workflows/validate-docs.yaml itself, so PRs that modify only the workflow file won't trigger it and self-validate.
  Remediation: add .github/workflows/validate-docs.yaml to the paths list.

• [correctness/makefile-phony] Makefile:20 — no .PHONY: validate-docs declaration. If a file named validate-docs ever exists at repo root, make will consider the target up-to-date and skip the script. Minor hygiene issue.
  Remediation: add .PHONY: validate-docs (and ideally .PHONY: ec-docs-preview).

• [correctness/unused-variable] bin/validate-docs.sh:43 — local basename="${file##*/}" inside validate_syntax is assigned but never used.
  Remediation: delete the unused assignment.

• [supply-chain] .github/workflows/validate-docs.yaml:27 — sudo gem install asciidoctor --no-document installs the latest asciidoctor unpinned on every run. Other actions in this workflow are correctly pinned to full commit SHAs; the gem is not. A compromised or typosquatted release would execute in CI.
  Remediation: pin the version (sudo gem install asciidoctor -v <version> --no-document), or use Gemfile.lock + bundle install --deployment, or use the official asciidoctor container image pinned by digest.

• [style/bash-indent] bin/validate-docs.sh — the new script uses 4-space indentation; existing bin/ scripts (screenshot-helper.sh, screenshot-pruner.sh) use 2-space. Minor consistency nit.
  Remediation: reformat to 2-space indent to match the existing bin/ convention.

• [style/script-header] bin/validate-docs.sh — existing bin/ scripts include a comment block after set -euo pipefail describing purpose (#\n# <description>\n#). The new script omits this.
  Remediation: add a brief header comment describing the script's purpose.

---

Labels: PR adds CI workflow and enhancement per issue #240 (AsciiDoc validation tooling).

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

[simonbaird]

Lgtm.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 1:48 PM UTC · Completed 1:58 PM UTC

Commit: cd01138 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-7 · Effort: high · Cost: $5.26

[fullsend-ai-review]

Risk Assessment: moderate (2/5)

Details

4-file additive PR with 2 protected-path touches and a new CI workflow, tempered by documentation-only scope; linked issue #240 well-scoped with all acceptance criteria met and easily revertible implementation.

[fullsend-ai-review]

See the review comment for full details.

[st3penta]

LGTM

[simonbaird]

Lgtm.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 8:18 PM UTC · Completed 8:24 PM UTC

Commit: e8f0a9f · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-7 · Effort: high · Cost: $2.93

[fullsend-ai-retro]

Retro on PR #251 — no new proposals; corroborates 5 existing open issues

Workflow shape. Human-authored 4-file PR (docs validation tooling). Two review-agent runs (Sep 10, Sep 13), both CHANGES_REQUESTED. Two human approvals landed on Sep 15 and the PR merged with the review agent still in CHANGES_REQUESTED. No code-agent or fix-agent involvement. Total review cost: $8.85 ($3.59 + $5.26).

Root failure this retro observed. Between Run 1 and Run 2 the author (a) pushed fixes for most Run 1 findings and (b) inline-replied to the remaining --safe-mode=unsafe finding explaining server mode was already tried and blocks ../partials/ traversal. Run 2 nevertheless re-derived its entire severity map from scratch, re-suggested --safe-mode=safe|server verbatim, and cost ~46% more than Run 1 despite emitting fewer output tokens. Trace shows the cause: the reusable-workflow provenance check compared the prior review-comment app client_id against an empty expected value, set PRIOR_REVIEW_PROVENANCE=unverifiable-wrong-app, and dropped the whole prior-review comment thread (including the author's reply) from the agent's context.

Everything actionable here is already tracked. Per retro guidance, filing no new proposals; evidence for each is below.

• fullsend-ai/agents#931 & #203 — direct root cause. REVIEW_APP_CLIENT_ID was empty for this install; Run 2 log shows expected — discarding (wrong app). Confirms the failure mode on a per-repo install (conforma/.fullsend).
• fullsend-ai/fullsend#5265 & #3907 — author-response incorporation. cuipinghuo's inline reply on the --safe-mode thread was never surfaced to Run 2; the remediation text repeats a suggestion the author had already explained is infeasible.
• fullsend-ai/agents#1281 & fullsend-ai/fullsend#2959 — re-flagging across iterations. Run 2 re-raised 5 of 7 Run 1 findings; two low-severity items (ci-coverage workflow-paths omission, nav-redundant double-count) were identical text.
• fullsend-ai/fullsend#4313 & fullsend-ai/agents#797 — severity anchoring. Cost delta ($1.67 extra on Run 2, 87 vs. 54 tool calls, 12 vs. 7 sub-agent transcripts) is direct evidence that losing the prior severity anchor forces a full re-derivation. The --safe-mode finding did drop medium→low, but the challenger sub-agent arrived there independently, not by anchoring.
• fullsend-ai/agents#680 & fullsend-ai/fullsend#1791 — CHANGES_REQUESTED vs. human approval. Two human MEMBER/CONTRIBUTOR approvals (simonbaird, st3penta) merged over an unresolved agent CHANGES_REQUESTED with no explicit override step; the agent state was simply ignored.

Repo-specific bugs left unaddressed in the merged code (already flagged inline by Run 2, not filing separately since the review agent has them on record): bin/validate-docs.sh emits asciidoctor errors with /tmp/tmp.XXXX/pages/... paths that are not click-jumpable in the repo, and validate_xrefs + validate_nav double-count broken xrefs in nav.adoc / partials/contents.adoc. Both survived to main; a follow-up PR by the author or a future fix-agent run would clear them.

Agents repo discovered from run log: fullsend-ai/agents@v0.41.0 (commit 10560252b0ad).