| Age | Commit message (Collapse) | Author |
|---|
|
renders correctly
After the RSTβMyST migration, ~45 MD pages use the plain `{include}` MyST
directive to pull in `_include/need_improvement.txt`. The included file
still contained RST (`.. raw:: html`, `.. raw:: latex`, `:ref:`), and
MyST's built-in `{include}` parses the content with the *outer* file's
parser β i.e. as MyST. RST directives then render as literal paragraphs.
Visible symptom on docs.vyos.io/en/rolling (and across 1.5/1.4):
the VPP index, VPP subpages, haproxy, isis, bfd, policy, pki, salt,
flowtables, webproxy, and others all show:
.. raw:: latex
\iffalse
.. raw:: html
Call for Contributions
Help improve this section ...
For contribution guidelines, see :ref: documentation .
Fix: rewrite `_include/need_improvement.txt` as a native MyST admonition
(`:::{admonition} Call for Contributions / :class: error / ... / :::`)
and drop the now-unnecessary `{eval-rst}` wrapper at the single callsite
(`configuration/highavailability/index.md`) that wrapped the include for
RST processing.
The fix is companion to 9c815d68 (cmdincludemd RST plumbing) but targets
the plain-`{include}` path which doesn't route through our custom
extension.
π€ Generated by [robots](https://vyos.io)
|
|
The override shipped in 3d44fe06 used rule names `Flag PR title not
matching T-ID format` + `Flag commit message not matching T-ID format`.
Per Mergify `extends:` semantics, same-name rules in the child REPLACE
the parent's β and the central baseline in vyos/mergify uses a single
rule named `Flag T-ID format violation in PR title or commit messages`.
Different names β no replacement β parent rule keeps firing.
Symptom (surfaced on PR #2014, title `docs(readme): β¦`): every PR
without a `T<digits>:` token gets `invalid-title` labeled.
Fix: collapse to one rule, rename to match the parent exactly. The
condition (`closed AND -closed`) still never matches, so `toggle`
removes the label on every evaluation β picks up any stale
`invalid-title` from the broken-override window.
The parent rule does not toggle `invalid-commit-title`; the
second child rule for that label was extraneous and is dropped.
π€ Generated by [robots](https://vyos.io)
|
|
CodeRabbit review on 28224f3 flagged that is_docs_path() introduced
in 65a8e9f only matched repo-relative path strings. An absolute path
to docs/... (e.g., from a local invocation that pre-resolves paths,
or from tooling that uses git ls-files --full-path) would silently
fail the docs/ check and the file would be skipped.
Rewrite the helper to use os.path.commonpath against an absolute
docs/ root computed on each call. Both inputs are normalized to
absolute form, so repo-relative and absolute callers produce the
same result. ValueError from commonpath (mixed Windows drives or
empty input) is caught and treated as 'not a docs path'.
abs_docs is recomputed per call rather than captured at import time
so the helper picks up the actual cwd at invocation, matching the
existing assumption that CI / local runs invoke the linter from the
repo root.
Verified against 12 edge cases:
- repo-relative docs paths (docs, docs/foo.md, docs/sub/dir/foo.md,
./docs/foo.md) -> True.
- repo-relative meta paths (AGENTS.md, README.md,
.github/copilot-instructions.md, docs_other/foo.md) -> False.
- absolute paths inside docs/ -> True; inside repo root but outside
docs/ -> False.
- traversal attempts (../other/foo.md, docs/../AGENTS.md) -> False.
CI behavior unchanged: tj-actions/changed-files passes repo-relative
paths, which were already handled by the previous logic.
|
|
Two stale claims flagged by Copilot in PR #2014 (one comment marked the
other as 'same issue earlier in the file'):
- '## Lint' (L40-45): said 'vyoslinter (doc-linter.py from the
vyos/.github repo, via the lint-doc.yml workflow) on changed files
only'. The cross-repo dependency was dropped in 1bf386e β the
linter is now scripts/doc-linter.py in this repo, invoked from
.github/workflows/lint-doc.yml.
- '## CI' (L290-292): same stale claim in bullet form.
Also reflect 65a8e9f's scoping decision: the linter applies only to
files under docs/. Repo-root meta files (README.md, AGENTS.md,
.github/copilot-instructions.md) are explicitly out of scope, since
they are project meta rather than published documentation and don't
need to obey docs-publication conventions (80-char wrap, RFC IP
rules, suppression markers). Mention that exclusion in the CI bullet
so contributors know the linter won't catch issues there.
|
|
The linter targets published documentation sources; the auto-discover
fallback already walks `docs/` only. CI was passing root-level meta
files (README.md, AGENTS.md, .github/copilot-instructions.md β the
last is a symlink to AGENTS.md) which forced docs-publication
conventions (80-char wrap, RFC IP rules, suppression markers) onto
project meta that has no business obeying them.
Add an `is_docs_path()` guard in `main()` so the explicit-file-list
path matches the auto-discover behavior β only files under `docs/`
are linted. AGENTS.md and the Copilot-instruction symlink are now
out of scope.
Verified:
- `python3 scripts/doc-linter.py "['AGENTS.md', 'README.md', '.github/copilot-instructions.md']"` β exit 0 (all skipped).
- `python3 scripts/doc-linter.py "['docs/_test_lint.md']"` with a real public IP β still errors as expected.
π€ Generated by [robots](https://vyos.io)
|
|
Four follow-up findings from Copilot on commit e8d7e43, all valid:
- '### Formatting' bullet 'Inline code: use double backticks per RST
convention' was carried over from the RST era. The .md sources are
overwhelmingly single-backtick (~1900 single- vs ~80 double-backtick
inline-code spans across docs/). Rewrite to 'single backticks in
MyST (the canonical form). Double backticks only inside
{eval-rst} blocks and _include/*.txt snippets, per RST convention.'
- '### RST heading hierarchy' section read like a general authoring
rule. Add a preface scoping it explicitly to RST contexts
(_include/*.txt and {eval-rst} blocks) and noting that canonical
MyST pages use ATX (# / ## / ...) headings. Reword the post-block
prose to 'every embedded RST snippet that introduces a title'
instead of 'every RST file'.
- '### `.. TODO::` markers' heading and prose described the RST
form only. The .md sources use `{todo}` fenced directives
exclusively (6 triple-backtick + 1 colon-fenced occurrences across
docs/, zero `.. TODO::` in .md). Rename the section to
'`{todo}` markers', describe the MyST fenced form first, and
mention the RST form only for RST contexts.
- Drop the '% stop_vyoslinter' / '% start_vyoslinter' markers I
added around the '8.8.8.8' DNS example bullet under
'### IP addresses'. Replace the literal IP with a description-only
example ('a DNS server's address in a DNS forwarder example, or an
upstream peer's address in an EBGP example'). The bullet still
illustrates the rule (real public IPs need suppression) without
rendering bare '% stop_vyoslinter' lines in GitHub views.
Local doc-linter still exits 0 on AGENTS.md and README.md.
|
|
The previous README refresh sweep landed before the doc-linter was
extended to MyST in 7a3aa29, so AGENTS.md was never wrapped to the
80-char rule its own 'Source conventions' section documents, and the
source-format claims drifted out of sync once the RST->MyST migration
finished.
Content fixes (the inconsistency Copilot flagged on PR #2014):
- '## Project': drop the 'MyST Markdown for migrated pages and RST
for pages that haven't been migrated yet' framing. State plainly
that sources are MyST Markdown only, source_suffix is ['.md'],
and the RST archive under docs/_rst_legacy/ is not editable.
- '### Sphinx config': source_suffix entry updated from
['.rst', '.md'] to ['.md'].
- '### Source files': drop the 'docs/<page>.rst' non-migrated bullet
(no such pages exist). Add a 'docs/_include/<name>.txt' bullet
noting that the shared snippets stay RST because cmdincludemd
parses their content as RST.
- '**Editing rules:**': collapse 'Migrated page' / 'Non-migrated
page' into one 'Existing page' rule. Add a bullet pointing at the
_include/*.txt -> RST exception.
- '### Command directives': rewrite to lead with MyST fenced blocks
(the canonical form) and only mention RST forms for {eval-rst}
blocks and _include/*.txt snippets. Avoid using literal
quad-backtick code-in-code samples that would otherwise open
phantom fences in the source.
Linter compliance:
- Hard-wrap every paragraph and bullet outside fenced code blocks
to <=80 chars (~30 pre-existing violations).
- Wrap the long line about the docutils inline-markup link by
dropping the link entirely; the rule 'use double backticks' is
self-contained and AGENTS.md doesn't need the reference.
- Restructure '### Bot review workflow' table: shorten the
CodeRabbit row to 79 chars and move the 'auto-reviews are
disabled' note to a paragraph below the table.
- Wrap the '8.8.8.8' real-public-IP bullet under '### IP addresses'
with '% stop_vyoslinter' / '% start_vyoslinter' markers. The
surrounding section is literally documenting suppression markers,
so the meta-example reads naturally.
- The '### Configuration page structure' bullet referring to MD
fence syntax was using quad-backtick code-in-code; rewritten to
'`{cfgcmd}` fenced code blocks (in MD)' to avoid opening a
phantom fence in the linter's view of the source.
|
|
The doc-linter was extended to MyST .md files in 7a3aa29 (after the
previous README refresh in 3a544f6), so this PR is the first README
edit linted under MyST rules. The three pre-existing >80-char lines
(badge, Wayback link, rolling table row) now fail doc-lint. Wrap them:
- Switch the badge and Wayback links to reference-style. The badge
destination/image URLs become [badge]/[rtd]/[wayback] references at
the top of the file; renders identically on GitHub and in Sphinx.
- Trim the Wayback snapshot path from web/20200225171529/ to web/2020/
(year-level capture, same era as the original snapshot) to fit the
reference definition in 80 chars without a suppression marker. Using
% stop_vyoslinter here would render literally on GitHub, since the
README is GFM-rendered, not MyST.
- Shorten the rolling-branch table row from 'all new contributions
target this' to 'new contributions land here' (84 -> 78 chars).
Also address two Copilot review findings on this PR:
- Line 15 grammar: 'Pages are ... and build with Sphinx' ->
'Pages are ... and are built with Sphinx'.
- AGENTS.md still describes the source format as mixed MyST/RST
(AGENTS.md:7 and several follow-up sections). Updating AGENTS.md
in this PR would pull ~30 pre-existing long-line violations into
scope and require a bulk hard-wrap of the file, which is outside
the original README request. Leaving AGENTS.md for a follow-up PR
and noting it in the PR description.
|
|
The MyST migration is complete: source_suffix in docs/conf.py is now
['.md'] only, all canonical pages are .md, and the pre-migration RST
originals are archived under docs/_rst_legacy/ (excluded from the
build). Update the README accordingly:
- Add a 'Source format' section stating pages are MyST Markdown and
pointing at the _rst_legacy/ archive so newcomers know not to edit
it. Mention that the VyOS command directives are written as MyST
fenced blocks via myst_fence_as_directive, and that docs/_include/
*.txt snippets stay RST because cmdincludemd parses their content
as RST (per 9c815d6).
- Drop the stale 'MyST Markdown for migrated pages, RST for the rest'
line from the Contributing section; rephrase to point at the actual
topics AGENTS.md covers today (MyST conventions, command directives,
linter suppression markers, Copilot/CodeRabbit workflow).
|
|
docs: fix typos across configuration, contributing, and vpp docs
|
|
fix(ext): cmdincludemd parses .txt content as RST to fix broken interface docs
|
|
ci(mergify): disable inherited T-ID title/commit-message checks in this repo
|
|
The central baseline at vyos/mergify:.mergify.yml enforces a
`T<digits>: <text>` (optional `scope: ` prefix) format on PR titles
and every commit's first line β a convention from vyos/vyos-1x and
sibling code repos where engineering work tracks against a Phorge
task.
vyos-documentation has never enforced this. Recent merged PRs use
conventional-commits style (`fix(ext): β¦`, `ci(...): β¦`,
`chore(deps): β¦`) without an upstream T-ID. After the
`extends: mergify` adoption in #2005, the inherited rules
started labeling every such PR `invalid-title` and
`invalid-commit-title` β pure noise for this repo.
Override the two rules by name (Mergify `extends:` semantics:
same-name rules in the child REPLACE the parent's). Conditions are
constructed to never match (`closed and -closed`), so the `toggle`
action always falls into the "remove label" branch and also clears
the labels from any PR that picked them up since #2005 merged.
π€ Generated by [robots](https://vyos.io)
|
|
directives render
The `_include/*.txt` library is written in reStructuredText (`.. cfgcmd::`,
`.. code-block::`, `.. note::`, `.. cmdinclude::`). After the RSTβMyST
migration, the `{cmdincludemd}` fence on .md pages routed include content
through `MockState.nested_parse`, which in MyST 2.0 simply replays the
content through the MyST renderer (see myst_parser/mocking.py:153). RST
directives in the .txt file then rendered as literal paragraph text:
<p>.. cmdinclude:: /_include/interface-description.txt</p>
<p>.. cfgcmd:: set interfaces ethernet ...</p>
<p>.. code-block:: none</p>
User-visible symptom (reported on rolling docs): the Ethernet, dummy,
tunnel, bonding, bridge, macsec, vxlan, l2tpv3, pseudo-ethernet,
virtual-ethernet, and wireless pages all showed unprocessed `.. cfgcmd::`
and `.. cmdinclude::` directives instead of styled command boxes.
Fix: mirror MyST's own `{eval-rst}` plumbing
(`MockRSTParser().parse(text, doc)` per
`mdit_to_docutils/base.py:1655`). Build a fresh document that inherits
the outer document's settings + reporter, run `MockRSTParser` over the
substituted include content, and graft the resulting children back
into the calling document with explicit-target registration. Keeps the
Sphinx env available to `cfgcmd`/`opcmd`/`cmdinclude` directives.
Verified locally on rolling (Sphinx 7.x, myst-parser 2.0): zero literal
`cfgcmd::` / `cmdinclude::` / `code-block::` / `opcmd::` / `note::` /
`include::` strings remain in any built HTML page under
`configuration/`, `automation/`, `installation/`, `operation/`,
`vpp/`; all 11 affected interface pages now render proper
`cfgcmd-heading` / `cfgcmd-body` blocks (e.g. ethernet.html: 160,
wireless.html: 172, dummy.html: 4).
π€ Generated by [robots](https://vyos.io)
|
|
T8782: adopt central Mergify baseline via `extends: mergify`
|
|
Adds .github/mergify.yml that inherits from vyos/mergify:.mergify.yml.
The central baseline provides:
- defaults.actions.backport.ignore_conflicts: false (new in T8782 β
makes backport conflicts fail loudly instead of committing literal
`<<<<<<< / ======= / >>>>>>>` markers; see incident below)
- pull_request_rules β label conflicting PRs with `conflicts`
- commands_restrictions β restrict @Mergifyio slash commands to
@vyos/maintainers + vyosbot
Why this repo, why now: 2026-05-12, PR #1994 was backported via
`@Mergifyio backport sagitta circinus`. Cherry-pick conflicted in
docs/conf.py; Mergify's pre-T8782 default (`ignore_conflicts: true`)
committed the markers, the resulting PRs (#1998 / #1999) were merged
anyway, and Read the Docs builds on both branches broke with
`SyntaxError: invalid syntax (conf.py, line 309)`. Fix-forwards
#2000 (circinus) and #2001 (sagitta) restored both branches. Adopting
the central baseline closes the gap for this repo so the same failure
mode cannot recur via the Mergify-managed backport path.
Spec: https://vyos.atlassian.net/wiki/spaces/VYOS/pages/849477640
IS-432: https://vyos.atlassian.net/browse/IS-432
π€ Generated by [robots](https://vyos.io)
|
|
ci(ai-validation): stop retargeting origin to fork URL
|
|
The previous CR-driven fix (7a7a8002) re-pointed origin to the PR's HEAD
fork on the assumption that claude-code-action calls
`git fetch origin <head-branch>`. That assumption is wrong for fork PRs:
the action detects a fork PR and fetches `pull/<n>/head` instead, and
those refs only exist on the base repo. Retargeting origin to the fork
therefore breaks the fork-PR fetch path with:
PR #<n> is from a fork, fetching via refs/pull/<n>/head...
fatal: couldn't find remote ref pull/<n>/head
Observed on run 25689321499 (PR #1891 from natali-rs1985/vyos-documentation).
Leave origin pointed at vyos/vyos-documentation (the base repo) β that's
where actions/checkout left it after the merge-ref checkout, and where
`pull/<n>/head` resolves. Replace the step with an explanatory comment.
π€ Generated by [robots](https://vyos.io)
|
|
- webproxy.md: replace `ftp.univ-tlse1.fr` with `ftp.example.com` so the
hostname matches the documentation-reserved IP `192.0.2.249` (Copilot).
- ospf.md: insert missing preposition in `show ip ospf interface` and
`show ipv6 ospfv3 interface` op-cmd descriptions (CodeRabbit).
- ipsec.md: rewrite intro sentence ("can significantly speed up β¦") and
hyphenate `feature-rich` (CodeRabbit).
π€ Generated by [robots](https://vyos.io)
|
|
teslazonda/T8843-fix-recursive-build-directory-error
T8843: Fix recursive _build directory nesting error
|
|
- ipsec.md: "to speed-up" -> "to speed up"; drop article in "creates a
corresponding SAs and policies"; "there routes" -> "these routes".
- ospf.md: "Cost calculation wireguard interfaces" -> "Cost calculation
for WireGuard interfaces".
https://claude.ai/code/session_01RDpSBDLSWLKMGnyPCaKECB
|
|
scripts/doc-linter.py enforces an 80-character limit outside fenced code
blocks and treats any finding as a failure. Wrap long paragraphs in
files that this PR touched so CI passes:
- docs/configuration/protocols/ospf.md: wrap the WireGuard cost note.
- docs/configuration/protocols/traffic-engineering.md: wrap the IS-IS/
OSPF note.
- docs/contributing/debugging.md: wrap the Phabricator paragraph, and
guard the long [bootchart.conf]/[vyatta-cfg] link references at EOF
with `% stop_vyoslinter`/`% start_vyoslinter`.
- docs/vpp/configuration/ipsec.md: wrap the integration, requirements,
algorithm, ACL, VTI example, troubleshooting paragraphs.
Pure reformatting; no semantic changes.
https://claude.ai/code/session_01RDpSBDLSWLKMGnyPCaKECB
|
|
- terraformAWS.md: balance the orphan `.. stop_vyoslinter` directive
inside the eval-rst block and wrap the long MyST link references at
EOF with `% stop_vyoslinter`/`% start_vyoslinter`.
- webproxy.md: replace real-world IP 193.49.48.249 with doc-range
192.0.2.249 in the blacklist update example output.
- console-server.md: "some times" -> "sometimes".
- bgp.md: "command disable" -> "command disables"; add comma before
"use the delete form"; "Route dampening which described in" ->
"Route dampening, described in".
https://claude.ai/code/session_01RDpSBDLSWLKMGnyPCaKECB
|
|
Corrections include: adresses->addresses, oder->other, Them->The,
criterias->criteria, seeting->setting, carefull->careful,
beeing->being, reenable->re-enable, wich->which, derection->direction,
Whith->With, assosiate->associate, fpr->for, bootup->boot, trough->through.
https://claude.ai/code/session_01RDpSBDLSWLKMGnyPCaKECB
|
|
|
|
Bumps [urllib3](https://github.com/urllib3/urllib3) from 2.6.3 to 2.7.0.
- [Release notes](https://github.com/urllib3/urllib3/releases)
- [Changelog](https://github.com/urllib3/urllib3/blob/main/CHANGES.rst)
- [Commits](https://github.com/urllib3/urllib3/compare/2.6.3...2.7.0)
---
updated-dependencies:
- dependency-name: urllib3
dependency-version: 2.7.0
dependency-type: direct:production
...
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
|
|
|
|
vyos/yuriy/ai-validation-checkout-pr-in-validate-rolling
ci(ai-validation, rolling): merge-ref checkout in validate + retarget origin to fork (claude-code-action git workspace)
|
|
CR/Copilot pass on the merge-ref + retarget-origin revision raised:
1. (CR on #1991, line 278, Major) Re-resolving refs/pull/<n>/merge
in validate is racy. GitHub may advance the merge ref between
prepare and validate (rapid pushes), so Pass 1 (artifact from
prepare) and Pass 2 (validate workspace) can see different
revisions. concurrency.cancel-in-progress narrows the window but
does not make the ref immutable.
Fix: prepare outputs its post-checkout `git rev-parse HEAD` as
`merge_sha`, validate checks out THAT exact sha. Both jobs now
operate on the same revision regardless of subsequent pushes.
2. (Copilot on #1991, two findings on line 294) The merge-ref +
retarget commit introduced two NEW stray heredoc escapes:
`PR"'sclaude-code-action'"s`. Same class of error
as the previous-fix-cycle: shell heredoc escape sequences
survived verbatim into YAML.
Fix: replace with plain `PRs` / `claude-code-actions` in the
inline comment block and step name.
3. (Copilot on #1990/#1991, "PR title/description says HEAD repo +
head.sha") Metadata cleanup tracked separately β PR titles +
bodies updated to describe the merge-ref + retarget-origin
approach (no workflow file change for that part).
Mirrored byte-identically across the 3 open wave-5 PRs (#1990
rolling, #1991 circinus, #1993 sagitta follow-up). Canonical sync
once these merge.
|
|
CLAUDE.md/.claude (CR findings)
Two new CR findings on PR #1990 (both raised on the same review cycle):
1. (CR, line 296, Major, Heavy lift) Tree drift between Pass 1 and
Pass 2. The prior fix in this PR used `head.sha` (origin=fork) so
the action's `git fetch origin <head-ref>` resolves β but that
left validate's workspace tree at PR HEAD while prepare bundled
`diff-md.patch` + `_changed_md/` from `refs/pull/<n>/merge`. For
PRs where base also contributes content to a touched file, Pass 2
workspace-Read could disagree with Pass 1's view.
Fix: switch the validate checkout back to `refs/pull/<n>/merge`
(workspace tree now matches the prepare bundle) and address the
action's `git fetch origin <head-ref>` requirement separately by
`git remote set-url origin <fork-url>` after the checkout. The
workspace tree is unchanged by the remote retarget; only the fetch
destination is updated. For public forks (the only kind targeting
public vyos/vyos-documentation) the unauthenticated fetch succeeds.
2. (CR, line 316, Major) `anthropics/claude-code-action@v1` invokes
the Claude Code CLI, which auto-discovers and loads `CLAUDE.md`
and `.claude/**` (memory, skills, hooks, MCP, plugins) from the
workspace at session startup. A malicious fork could pre-place a
`CLAUDE.md` with prompt-injection content, or a `.claude/skill.md`
marked as auto-load. Documented at:
- code.claude.com/docs/en/claude-directory.md
- code.claude.com/docs/en/memory
Fix: add `CLAUDE.md` and `.claude` to the wipe list so Claude's
auto-discovery starts from a clean slate (workflow-controlled state
only). `--bare` would also disable this but would lose the
`mcp__github_inline_comment` MCP server the action provides, so
wiping reserved paths is the better fit.
Mirrored across rolling (#1990), circinus (#1991), and the sagitta
follow-up (#1993). Canonical sync once these merge.
|
|
validate-checkout step
After the first fix bundle landed on the three wave-5 PRs, three more
findings surfaced:
1. (CR on #1991, line 312) `rm -f` silently no-ops on directories. A
fork could commit `changed-md.txt/`, `diff-md.patch/`, or
`pass1-findings.json/` AS DIRECTORIES (not files), and the wipe
step would skip them β the subsequent artifact-download writes
into the real-file path would then collide with the fork-placed
directory, producing unpredictable behaviour.
Fix: use `rm -rf` for ALL reserved paths uniformly. The
distinction was cosmetic at best; making it uniform closes the
gap.
2. (Copilot on #1991, line 311) `.venv/` is also fork-attackable.
`astral-sh/setup-uv` with `activate-environment: true` creates
`${{ github.workspace }}/.venv` and prepends its `bin/` to PATH.
If a fork pre-populates `.venv/bin/`, those binaries land in
PATH for subsequent steps (Pass 1 runs `vyos-doc-review`, Pass 2
runs `claude-code-action` whose internals may shell out).
Fix: add `.venv` to the wipe list.
3. (Copilot on #1992, line 298) The inline comment block contains
a literal `PR's` β a stray shell heredoc escape sequence
that survived the commit verbatim. Confusing in YAML.
Fix: replace with plain `PRs`.
Mirrored byte-identically across rolling/circinus/sagitta + canonical
follow-up after this wave merges.
|
|
CR pass on #1990/#1991/#1992 raised 4 findings on the new validate-
side actions/checkout step. All valid; folding into one bundle:
1. Fork PR head-ref not found in base repo (#1990, #1991, #1992 each
raised this). The previous form checked out refs/pull/<n>/merge
from origin (vyos/vyos-documentation), leaving origin pointed at
the base repo. claude-code-action then runs
`git fetch origin <head-ref>` where <head-ref> is e.g.
`contributor/branch` β which does NOT exist in the base repo for
fork PRs, so the fetch fails the same way "no .git" failed before.
Fix: check out the PR HEAD repo (the fork for fork PRs) at the
head sha. origin now resolves to the head repo where the head
ref exists, so the action's fetch succeeds. github.token is the
default GITHUB_TOKEN which can read public forks (the only kind
of fork that can target vyos/vyos-documentation, since that repo
is public).
2. Untrusted PR content at workspace root can pre-create reserved
paths (#1991). The PRs tree at workspace root could in
principle contain `.reference-db/extracted/...`,
`pass1-findings.json`, `_changed_md/poisoned.md`, etc., which
subsequent steps would treat as workflow-generated. The fail-
closed gate only checks `[ ! -d .reference-db/extracted ]` so a
PR-placed empty `.reference-db/extracted/` would bypass it.
Fix: add a "Wipe reserved workspace paths" step right after the
checkout that `rm -rf`s the workflow-owned paths
(.reference-db, .vyos-1x, reviewer, reviewer-src, _changed_md)
and `rm -f`s the workflow-owned files (pass1-findings.json,
diff-md.patch, changed-*.txt) β every one of these is recreated
by the trusted producer step immediately following.
3. Checkout runs unconditionally even on skip=true paths (#1991).
The checkout only needs to run when validate will actually use
it (skip != true β Pass 2 runs).
Fix: gate the checkout (and the wipe step) on
`if: steps.secrets-check.outputs.skip != true`. Move them
AFTER the secrets-check step in step order so the conditional
is meaningful.
4. "NO credentials" wording misleading (#1992). actions/checkout
uses the GITHUB_TOKEN to download the tree β it is not an
unauthenticated fetch. `persist-credentials: false` only
suppresses writing the token into the resulting .git/config.
Fix: rename the step to "Checkout PR HEAD (no persisted
credentials)" and update the inline comment block to be explicit
that the token is used for the download but not persisted.
Mirrored byte-identically across rolling/circinus/sagitta + canonical
follow-up after this wave merges.
|
|
needs git workspace)
Smoke verify on PR #1977 (run 25659408343 after github_token fix
landed) reached Pass 2 with valid auth but failed at action setup:
fatal: could not open `docs/quick-start.md` for reading: No such
file or directory
fatal: not a git repository (or any of the parent directories): .git
Action failed with error: Command failed:
git fetch origin --depth=20 yuriy/docs-smoke-test-quickstart
The action expects to run inside a git checkout of the PR's repo so
it can `git fetch <head-ref>` and read changed files from the working
directory. Our split-job design checks out vyos-1x, reviewer-src, and
the reviewer-config sparse-checkout into named subdirs but leaves the
workspace root empty (we only have _changed_md/ via artifact download).
Add `actions/checkout` of `refs/pull/<n>/merge` as the FIRST step in
validate. Subsequent steps that create named subdirs (_changed_md/,
.vyos-1x/, reviewer-src/, reviewer/, .reference-db/) coexist with the
docs/scripts/.github content checked out here β no path collisions.
Trust model unchanged:
* persist-credentials: false (no token in fork-content .git/config)
* No shell step executes fork code (no pip/npm/make against the
workspace; Pass 1 runs the trusted reviewer CLI; Pass 2 only uses
allowlisted Read/Glob/Grep/mcp__github_inline_comment + Bash
restricted to `gh pr comment|diff|view`)
* The prepare job continues to provide the canonical bundled
changed-md set via the pr-input artifact; validate now ALSO has
the full PR tree available for cross-reference reads
Mirrored byte-identically across rolling/circinus/sagitta + canonical
follow-up.
|
|
vyos/yuriy/ai-validation-claude-action-github-token-rolling
ci(ai-validation): pass github_token to claude-code-action (skip OIDC exchange)
|
|
ci(context7): gate version-tag move + refresh on docs-only changes
|
|
Second smoke verify on PR #1977 (run 25658927427) reached Pass 2 but
failed with "Action failed with error: Invalid OIDC token". Diagnosis:
The action (anthropics/claude-code-action@v1) mints an OIDC token with
audience `claude-code-github-action` and POSTs it to
`api.anthropic.com/api/github/github-app-token-exchange` to receive
an Anthropic-managed GitHub App token for posting back to the PR.
The endpoint validated our OIDC token and rejected it ("Invalid
OIDC token") β this happens on repos where Anthropic-side OIDC
federation hasn't been provisioned for the GitHub org.
Bypass the exchange entirely by providing `github_token` to the
action: `setupGitHubToken()` in src/github/token.ts checks
`process.env.OVERRIDE_GITHUB_TOKEN` first and uses it directly if
set, skipping OIDC.
`${{ github.token }}` is auto-scoped to the current PR repo and gets
the validate job's permissions (pull-requests: write,
issues: write, contents: read) β exactly what the action needs.
`steps.app.outputs.token` (our App token) is NOT suitable here: it's
scoped to the VyOS-Networks org (vyos-1x, vyos-docs-opus-reviewer)
for the earlier reviewer-source / vyos-1x checkouts, and has no
write access on vyos/vyos-documentation PRs.
Side effect: comments now post as github-actions[bot] (the workflow's
own bot identity) rather than as claude[bot] (the Anthropic-managed
GitHub App). Acceptable trade-off, and arguably preferable β the
inline-review identity is now visibly the VyOS workflow rather than
an external bot.
Mirrored byte-identically across rolling/circinus/sagitta + canonical
PR (folded into the still-open VyOS-Networks/vyos-docs-opus-reviewer#17).
|
|
Add `paths:` filter to `Update version tags` push trigger so infra-only
pushes (workflows, scripts, Docker, README, etc.) skip the tag move and
therefore skip the downstream Context7 refresh. Context7 only ingests
`docs/**` (per context7.json `folders`), so refreshing on infra commits
wasted API calls and gave no benefit. `context7.json` is also covered
because changes to its `rules`/`folders`/`excludeFolders` affect what
Context7 returns even when no .md files moved.
The version tag's only consumer is Context7, so skipping the move on
infra-only pushes is semantically correct β the tag still represents
the latest docs state. `workflow_dispatch` on context7-refresh.yml
remains available to force a refresh out of band.
π€ Generated by [robots](https://vyos.io)
|
|
ci(ai-validation): restore id-token: write (claude-code-action@v1 uses OIDC)
|
|
Revert of the id-token drop from #1969. Smoke verify on PR #1977
(empty-commit retrigger run 25658256103) failed at the Pass 2 step:
Action failed with error: Could not fetch an OIDC token. Did you
remember to add `id-token: write` to your workflow permissions?
The earlier Copilot finding that motivated dropping the permission
("no step uses OIDC") was incomplete. No shell step in the workflow
invokes OIDC directly, but anthropics/claude-code-action@v1 itself
calls actions/core's `getIDToken()` internally β likely for the
Claude/Anthropic auth federation path. The required scope is the
third-party action's, not the workflow body's.
Adding id-token: write back with an inline comment block citing the
specific failure mode + the run ID where it was reproduced so this
isn't re-dropped on a future review pass.
Mirrored byte-identically across rolling/circinus/sagitta + canonical.
|
|
ci(ai-validation): pathspec :(glob) magic β top-level docs/*.md were silently bypassed
|
|
CRITICAL BUG discovered during smoke verify of PR #1977 (a draft PR
touching docs/quick-start.md). The prepare job ran SUCCESS but
validate SKIPPED because has_md_changes was false:
$ git diff origin/rolling...HEAD --name-only -- 'docs/**/*.md'
(empty)
$ git diff origin/rolling...HEAD --name-only -- ':(glob)docs/**/*.md'
docs/quick-start.md
Root cause: git's default pathspec syntax uses fnmatch with
FNM_PATHNAME (slashes are not crossed by `*`). The `**` glob is NOT
recognized in default pathspec β it's treated as plain `**`
(two consecutive asterisks). For `docs/**/*.md` this means git
requires at least one `/` between `docs/` and `*.md`, so:
- docs/configuration/service/foo.md β MATCHES (1+ subdir)
- docs/quick-start.md β NO MATCH (0 subdirs)
- docs/index.md β NO MATCH (0 subdirs)
- docs/cli.md β NO MATCH (0 subdirs)
- docs/copyright.md β NO MATCH (0 subdirs)
- docs/coverage.md β NO MATCH (0 subdirs)
- docs/404.md β NO MATCH (0 subdirs)
- docs/documentation.md β NO MATCH (0 subdirs)
7 top-level files silently bypass validation on every PR that
touches them. Adding the `:(glob)` magic prefix opts in to true
shell-style ** semantics (also recognised by git's pathspec
parser per gitignore-pattern rules):
- docs/**/*.md β :(glob)docs/**/*.md
- docs/**/*.rst β :(glob)docs/**/*.rst
Three pathspec usages updated in the prepare job; comment references
to "docs/**/*.md" left unchanged (they read as informal shorthand).
Paired PR on VyOS-Networks/vyos-docs-opus-reviewer adjusts the
canonical scripts/ai-validation.yml identically.
|
|
vyos/yuriy/ai-validation-align-comment-and-reviewer-ref-rolling
ci(ai-validation, rolling): align fail-closed-gate comment + bump REVIEWER_REF to v1.0.2
|
|
Copilot finding on #1974/#1975: the inline backtick code span
`[ -s diff-md.patch ] && [ ! -d .reference-db/extracted ]` was split
across two YAML comment lines, which renders poorly in Markdown
viewers (the backtick crosses the line boundary).
Reworded to put the relevant filesystem-presence check (`-d
.reference-db/extracted`) on a single line and drop the redundant
`-s diff-md.patch` half β the latter is the job-level gate
condition, not part of the fail-closed gate logic, so omitting it
from this rationale comment removes the wrap-required spread without
losing meaning. The `-d` check on its own is what defines "the gate
is a presence check, not a schema check".
Mirrored byte-identically across rolling/circinus/sagitta.
|
|
Two small follow-ups now that the DB-pin revert wave (#1971/#1972/#1973
+ canonical #15) has settled and `reviewer-v1.0.2` is tagged:
1. Comment alignment. The fail-closed-gate rationale comment that
landed in #1971/#1972 says "the new code will refuse to load an
older-format DB at the fail-closed gate". A Copilot finding on
#1973 pointed out this is imprecise: the workflow's fail-closed
gate is a presence check (`[ ! -d .reference-db/extracted ]`); it
does not load or inspect the DB. The actual schema-mismatch
protection lives inside the pinned reviewer Python code at
`Pass 1 β deterministic checks` (loaded via
`uv pip install ./reviewer-src` from `env.REVIEWER_REF`). The fix
landed on sagitta (#1973 / b09ea673). This commit ports the same
wording to this branch so the workflow file is byte-identical
across rolling/circinus/sagitta again.
2. REVIEWER_REF bump v1.0.1 -> v1.0.2. The canonical at
`VyOS-Networks/vyos-docs-opus-reviewer/scripts/ai-validation.yml`
already defaults to v1.0.2 since the canonical-sync PR #15 merged
and the `reviewer-v1.0.2` tag was pushed. v1.0.2 is functionally
identical to v1.0.1 β the Python package and `branches.json` are
unchanged; the bump just aligns this deployed copy with the
canonical default for hygiene. No behavioral change.
|
|
ci(ai-validation): revert DB-download `tag:` pin to `latest:true` (critical regression)
|
|
CRITICAL REGRESSION fix. PR #1969 changed the reference-DB download
from `latest: true` to `tag: ${{ env.REVIEWER_REF }}` (set to
`reviewer-v1.0.1`). Verification post-merge showed that:
- The `reviewer-v1.0.1` tag exists but has **no GitHub release**
attached:
$ gh api /repos/VyOS-Networks/vyos-docs-opus-reviewer/releases/tags/reviewer-v1.0.1
{"message": "Not Found", "status": "404"}
- Reference DBs are published to a separate release stream by the
matrixed `rebuild-reference` workflow, tagged
`ref-db-<UTC-timestamp>` (e.g. `ref-db-20260510-193946`). The
reviewer Python package and the reference DBs have independent
release cadences.
`robinraju/release-downloader` with `tag: reviewer-v1.0.1` therefore
404s. The download step has `continue-on-error: true` so the run
proceeds, but the fail-closed gate at "Fail if reference DB missing
while MyST files are in the diff" then errors every validate run on a
PR with MD changes.
Our own recent test PRs (#1947 / #1956 / #1957 / #1968 / #1969 / #1970
on rolling, plus the circinus/sagitta backports) all happened to be
infrastructure-only β `has_md_changes` was `false`, validate was
skipped at the job level, and the DB step was never invoked. The
regression was therefore invisible until the next real `.md`-changing
PR would have failed red.
Reverting to `latest: true` is the correct behavior. The reference DB
schema is intentionally backwards-compatible across reviewer Python
releases; the freshest DB is always usable. If the schema ever
changes incompatibly, the way to opt out is bump `REVIEWER_REF` and
the new reviewer code will refuse to load an older DB at the
fail-closed gate β _exactly_ what we have today, just one level up.
A paired PR on VyOS-Networks/vyos-docs-opus-reviewer/scripts/
ai-validation.yml ([#15](https://github.com/VyOS-Networks/vyos-docs-opus-reviewer/pull/15))
applies the same revert + bumps REVIEWER_REF default to v1.0.2 so
canonical and deployed stay aligned after the next reviewer tag.
A separate companion PR will backport this revert to circinus and
sagitta to keep the workflow file byte-identical across all three
release branches.
|
|
ci: refresh Context7 LTS variants via tag field, not branch field
|
|
ci(ai-validation): address 4 CR findings on the merged #1957
|
|
write + fail-closed gate
CodeRabbit / Copilot findings on the merged PR #1959 and the still-open
PRs #1960 + #1969:
1. NUL/control-char guard regressed. PR #1959 (merged into circinus
without this) β and likewise sagitta/rolling-followup β had the
guard inadvertently dropped during the rolling@HEAD rebase, because
rolling@HEAD never had the corrected form. Restoring the
user-corrected form: `LC_ALL=C grep -zPq "[\\x01-\\x1F\\x7F]"`
reads the NUL-delimited *.z files directly (NUL is the record
delimiter, not a forbidden byte) and rejects every other control
byte 0x01-0x1F + 0x7F. An earlier `tr -d "\\0\\n\\r" | grep ...`
form stripped the very bytes it was meant to reject before the
grep ran β see the inline comment block for the contract.
2. `gh pr comment` requires `issues: write`. The skip-notice step and
Pass 2 summary comment both post via `POST /repos/{owner}/{repo}/
issues/{number}/comments` β a PR conversation comment IS an issue
comment in GitHub's data model. With only `pull-requests: write`
the call can 403 on repos whose default GITHUB_TOKEN permission
split routes issue-comment writes through `issues:`. Adding
`issues: write` alongside the existing `pull-requests: write` keeps
every comment path working without expanding the trust surface
beyond what the original validate job needed.
3. Fail-closed gate used `[ -s changed-md.txt ]` (--diff-filter=ACMRT,
excludes deletions) but validate is now gated on `has_md_changes`
which is computed from `[ -s diff-md.patch ]` (unfiltered, deletion-
aware). A deletion-only PR with a missing reference DB would reach
validate (has_md_changes=true) but bypass the fail-closed gate
(changed-md.txt empty), masking the DB-missing condition. Switch
the gate to `[ -s diff-md.patch ]` so both signals agree.
A separate Copilot finding (line 170, "validate gated on has_md_changes
means RST-only PRs do not run") is pushed back on the PR thread as
intentional design β RST is legacy per the RSTβMyST migration, and
the RST bookkeeping in prepare exists to surface mixed-MD-RST PRs in
the Pass 2 prompt, not to drive validation on RST-only PRs.
|
|
CodeRabbit review on #1962 flagged that concurrency.group is evaluated
at workflow scheduling time, BEFORE the shell-level allowlist case in
the job runs. An API-triggered workflow_dispatch with arbitrary variant
value (e.g. 'gh workflow run -f variant=foobar') would produce
concurrency key 'context7-refresh-foobar' and bypass dedupe with
legitimate runs.
Gate inputs.variant in the expression against the same allowlist
('rolling'/'1.5'/'1.4'). Map head_branch values to their canonical
variant names so the workflow_run path also resolves cleanly. Fall
through to 'invalid' if neither path matches β the shell allowlist
then fails the run loud.
π€ Generated by [robots](https://vyos.io)
|
