| Age | Commit message (Collapse) | Author |
|---|
|
The upstream anthropics/claude-code-action rejects bot-initiated
workflow runs with:
Workflow initiated by non-human actor: mergify (type: Bot).
Add bot to allowed_bots list or use '*' to allow all bots.
This leaves a failing required check on every Mergify backport PR
(#2025 today is the proximate symptom β circinus backport of #2016 is
blocked from auto-merging by this).
Per org policy, Mergify-authored PRs skip the bot-review flow entirely:
the underlying change was already reviewed on the source PR. Re-running
validation on the backport would also post duplicate findings.
Fix: extend the existing `secrets-check` step (kept the id for backwards
compat with the cascade of `if: steps.secrets-check.outputs.skip != 'true'`
gates) to also short-circuit when `github.event.pull_request.user.login`
equals `mergify[bot]`. Add a `skip_reason` output so the notify-on-PR
step can distinguish bot-skip (silent) from missing-secrets (post a
notice comment).
Implementation notes:
- PR author is bound through env: PR_AUTHOR (same pattern as secret
bindings) so the template-expansion happens before bash sees the
value as an already-quoted env variable.
- Uses `printf '...%s\n' "$PR_AUTHOR"` for the notice line so the
shell β not the YAML template engine β interpolates the author into
the workflow log output.
- Renamed step name to "Decide whether to skip validation" to reflect
the broader scope; step id stays `secrets-check`.
π€ Generated by [robots](https://vyos.io)
|
|
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)
|
|
|
|
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)
|
|
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)
|
|
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.
|
|
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.
|
|
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.
|
|
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
|
|
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)
|
|
guard)
Copilot finding on PR #1959 (line 114): the path-traversal hardening
block used `continue` (skip-with-warning) on detection of absolute /
traversal paths, but the non-regular-tree-entry check below uses
`exit 1`. The asymmetry meant a fork PR that smuggles in a path like
`docs/../../outside.md` would silently bypass Pass 1 (no file copied
into _changed_md/) and Pass 2 (LLM Read/Glob/Grep tools see no
content) β reducing validation coverage on exactly the inputs that
warrant the closest look.
Change `continue` to `exit 1` so both unsafe-input checks have
consistent visible-failure semantics. Maintainers must explicitly
address an offending path rather than have it skipped.
Mirrored byte-identically across all three open workflow PRs.
|
|
pin/no-id-token
PR #1968 merged on rolling while this PR was in CR review. It adds:
* has_md_changes output on prepare + job-level if-gate on validate
(fixes the empty-_changed_md/ crash on infrastructure-only PRs)
* fetch base by refs/heads/<ref> + use FETCH_HEAD instead of
origin/<ref> (fixes the tag-vs-branch ambiguity on vyos/vyos-
documentation where 'rolling' exists as both a branch and a tag)
* skip validate on deletion-only Markdown PRs
This commit pulls in rolling@HEAD's ai-validation.yml verbatim,
then re-applies the 4 still-needed fixes raised by CR/Copilot:
1. SHA-pin actions/checkout@v6 (x4) + actions/upload-artifact@v4 +
actions/download-artifact@v4 + anthropics/claude-code-action@v1
2. Pin reference-DB download to tag: ${{ env.REVIEWER_REF }}
(was latest: true) β aligns DB version with pinned reviewer code
3. Drop id-token: write from validate job permissions (no OIDC use)
Items already present in rolling@HEAD via PR #1968 (no further action
here):
* NUL/control-char rejection in changed-*.z (the corrected
grep -zPq '[\x01-\x1F\x7F]' form, plus the explanatory
comment block about why NUL is excluded from the rejection class)
* Job-level if-gate on validate so infrastructure-only PRs skip the
entire expensive validate chain
* mkdir _changed_md defensive step in validate
Result: 3 PRs (this one + the circinus/sagitta companions) now share a
single byte-identical workflow file that is also a strict superset of
rolling@HEAD's current file (3 fixes layered on top).
|
|
Two factual issues in prior wording (flagged by Copilot on #1969):
- "NUL inside the name" implied embedded NUL is something to reject;
NUL cannot appear in a git pathname (it is the tree-entry terminator).
- "Filesystems β¦ typically reject these" was wrong for LF/CR β POSIX
filesystems allow them and git stores them fine. The actual hazard
is in our line-delimited downstream tooling.
Consolidated the two adjacent comment blocks into one accurate
explanation. No behavior change.
π€ Generated by [robots](https://vyos.io)
|
|
Previous form `tr -d '\0\n\r' | grep -Pq [\x00-\x1F\x7F]` stripped
the very chars (LF, CR) it was meant to catch before the grep ran,
so a path containing newlines or carriage returns slipped through.
`grep -z` keeps NUL as the record delimiter (legitimate separator from
git diff -z) and the pattern excludes 0x00 while catching every other
control byte 0x01-0x1F + 0x7F. LF/CR inside any path now correctly
fail the guard.
Surfaced by Copilot on #1969; applied to all three branch copies
(rolling/circinus/sagitta) so the workflow stays in sync.
π€ Generated by [robots](https://vyos.io)
|
|
The token is used only for read-only repo operations (sparse-checkout
of reviewer branches.json, full checkout of vyos-1x, download of the
reference-DB release asset). Without an explicit permission-* input
the token inherits all installation permissions. Scope it down so a
compromise cannot mutate either repo.
Surfaced by CodeRabbit on #1960; applied to all three branch copies
(rolling via #1969 follow-up + circinus #1959 + sagitta #1960) so the
workflow stays in sync across the version-train branches.
π€ Generated by [robots](https://vyos.io)
|
|
drop id-token
Follow-up on the now-merged #1957 (ubuntu-latest switch). CodeRabbit
raised these findings on the paired add-to-circinus PR #1959 (where
the same file was being added to the circinus branch); since the file
contents are byte-identical across rolling/circinus/sagitta, applying
the same fixes here.
1. SHA-pin actions/checkout@v6 (x4), actions/upload-artifact@v4,
actions/download-artifact@v4, anthropics/claude-code-action@v1.
pull_request_target has secrets + repo write β GitHub security
guidance recommends full commit SHAs as the only immutable
release form.
2. Reject paths containing control characters (NUL/CR/LF) in
changed-md.z and changed-rst.z before `tr '\0' '\n'` converts
them to newline-delimited manifests. A fork PR committing
`docs/foo<LF>bar.md` would otherwise split into two logical
lines, masking the real file from line-based consumers.
3. Pin reference-DB download to `tag: ${{ env.REVIEWER_REF }}`
(was `latest: true`). Aligns DB version with the pinned
reviewer code; a future reviewer-v1.x.x release with a DB schema
change can't be silently picked up.
4. Drop `id-token: write` from validate job permissions. No OIDC
usage; copy-paste leftover.
Paired PRs on release branches (byte-identical file contents):
* circinus: #1959 (commit 025319ea)
* sagitta: #1960 (commit e5506317)
|
|
Copilot review on #1962 flagged that the type:choice UI constraint on
workflow_dispatch.inputs.variant is bypassable when invoked via API
(gh workflow run -f variant=β¦). An empty or arbitrary value would:
- Generate a malformed concurrency key (context7-refresh- with empty
suffix, since inputs.variant || head_branch || '' both go falsy)
- Pass garbage to Context7's API (404s gracefully, but still a wasted
runner minute and noisy)
Add an explicit allowlist case after VARIANT is computed. Fails loud
with a clear message before any downstream call.
π€ Generated by [robots](https://vyos.io)
|
|
ci: serialize update-version-tags runs to close back-to-back-push race
|
|
ci(ai-validation): switch to GitHub-hosted ubuntu-latest runners
|
|
Post-#1961, the rolling auto-refresh (default variant, no field) succeeds,
but workflow_dispatch for branch=circinus/sagitta still 404s. Further
empirical curls against the live API revealed:
POST /api/v1/refresh
{"libraryName":"/vyos/vyos-documentation","branch":"circinus"}
β HTTP 404 {"error":"branch_not_found","message":"Branch 'circinus' not found"}
POST /api/v1/refresh
{"libraryName":"/vyos/vyos-documentation","tag":"1.5"}
β HTTP 200 {"message":"Refresh started successfully"}
So Context7's API addresses variants by their REGISTRATION TYPE on the
dashboard:
- default variant (rolling) β omit both 'branch' and 'tag'
- tag-backed variants (1.5/1.4) β 'tag' field
- branch-backed non-default β 'branch' field
The dashboard shows 'rolling' with a branch icon and '1.5'/'1.4' with tag
icons. The 'branch' field only addresses entries registered as branches;
'tag' addresses entries registered as tags. This is undocumented in the
public GitHub Actions integration page but works against the live API.
Changes:
- Restore the variant mapping (circinus β 1.5, sagitta β 1.4) β that
matches the actual dashboard variant names. #1961 had dropped this in
favor of branch-name passthrough, which only worked for the default.
- Switch the non-default payload from 'branch: <name>' to 'tag: <name>'.
- workflow_dispatch input renamed back from 'branch' to 'variant'; choices
back to [rolling, '1.5', '1.4'].
- Restore the variant-keyed concurrency expression (with rewrite chain).
- Update the documentation comment to record the empirical API semantics.
Spec: ~/.claude/specs/2026-05-10-context7-github-actions-integration-design.md
π€ Generated by [robots](https://vyos.io)
|
|
Both Copilot and CodeRabbit flagged the same hole on PR #1958: GitHub's
"Re-run failed jobs" can execute retag in isolation, skipping
check_head. If the branch HEAD advanced since the original run, the
isolated retag would PATCH the tag to a stale github.sha.
Add the same HEAD-equivalence guard inside retag, immediately before
the PATCH/POST. Defense-in-depth β both jobs check, so neither full
re-runs nor selective retag re-runs can move the tag backward.
π€ Generated by [robots](https://vyos.io)
|
|
Copilot review on the paired add-to-circinus PR (#1959) flagged two
documentation drifts from the ubuntu-latest switch in this PR:
1. Line 65: comment referenced a 'cp loop' but the implementation has
used 'git show HEAD:<path>' as the bundling mechanism since
1ea164ff. Reworded to describe the bundling loop accurately.
2. Line 270: comment explained why setup-uv was used 'on Debian 12' β
stale now that the workflow runs on ubuntu-latest. Reworded to
describe the actual reason setup-uv is preferred (fast interpreter
provisioning + portable to self-hosted Debian if this workflow
ever moves back).
Documentation-only change. No behavioral effect.
|
|
The post-merge auto-fired runs from #1948/#1949/#1950 all returned 4xx
errors. Diagnostic curls against the live Context7 API revealed two
issues:
1. The 'branch' parameter addresses variants by their underlying Git
branch name (rolling/circinus/sagitta), not by their tag display name
(rolling/1.5/1.4). Sending 'branch: "1.5"' returns:
HTTP 404 {"error":"branch_not_found","message":"Branch '1.5' not found"}
2. The default variant refreshes when the 'branch' field is omitted
entirely. Sending 'branch: "rolling"' returns:
HTTP 400 {"error":"branch-not-found","message":"Failed to refresh library"}
But omitting the field returns:
HTTP 200 {"message":"Refresh started successfully"}
3. (Bonus, validating #1951 was wrong direction.) The libraryName must
include the leading slash: 'libraryName: "/vyos/vyos-documentation"'
per Context7's docs. Without the slash returns:
HTTP 404 {"error":"library_not_found"}
This commit restores the leading slash that #1951 incorrectly removed.
Changes:
- Restore leading slash on libraryName ('/' + github.repository).
- Drop the tag-name mapping in the case statement; pass head_branch
directly as the branch value.
- Omit the 'branch' field when head_branch is 'rolling' (default variant).
- workflow_dispatch input renamed from 'version' to 'branch'; choice
options changed from [rolling, '1.5', '1.4'] to [rolling, circinus, sagitta].
- Simplified concurrency expression (no longer needs the rewrite chain).
- Documentation comment updated to explain the branch-name addressing.
Spec: ~/.claude/specs/2026-05-10-context7-github-actions-integration-design.md
This is the 'fallback mapping' path that the spec's variant table already
documented as a contingency β pre-flight confirmed it's the correct path.
π€ Generated by [robots](https://vyos.io)
|
|
Agent-Logs-Url: https://github.com/vyos/vyos-documentation/sessions/74be7b98-780e-4cbf-8177-11534c4ec2d7
Co-authored-by: andamasov <12631358+andamasov@users.noreply.github.com>
|
|
PR #1953 added github.sha to the concurrency group key to prevent stale
"Re-run jobs" replays from cancelling the in-progress current-HEAD run.
Copilot review on the sagitta backport (#1955) caught the regression
that introduced: per-SHA groups mean back-to-back pushes A then B run in
parallel rather than serializing, and if run-A's force-PATCH lands
after run-B's, the tag rewinds to A.
Fix: per-branch group + cancel-in-progress: false.
- Concurrent runs serialize, so commit order is preserved on the tag.
- Stale "Re-run jobs" replays queue behind the current run, then hit
the HEAD-equivalence guard in the job body and exit 0 β the guard
(added in PR #1953) is the safeguard for that case, not the
concurrency group.
- Tag-move work is fast (~5s); serial execution under back-to-back
push bursts is acceptable.
π€ Generated by [robots](https://vyos.io)
|
|
The `vyos` org does not have self-hosted runners labeled `web` (those
live in the VyOS-Networks org pool and only serve repos there). Every
AI Validation run queued since #1947 merged sat in `queued` state
indefinitely with no runner picking it up β observed across all recent
PRs (#1955 mergify backports, #1956, plus several yuriy/* branches).
Switching both `prepare` and `validate` jobs to `runs-on: ubuntu-latest`:
* Removes the host-isolation half of the prepare-job rationale comment
and replaces it with the ephemeral-VM rationale (cross-run state
leakage is impossible on a fresh GitHub-hosted VM).
* Removes both `atos-actions/clean-self-hosted-runner` cleanup steps β
GitHub-hosted runners are ephemeral, the action is a no-op there at
best and a failure mode at worst (it expects self-hosted workspace
patterns that don't exist on hosted runners).
* Tweaks one comment that mentioned `/proc/<pid>/cmdline on the self-
hosted runner` to be runner-agnostic.
Also removes `.github/actionlint.yaml`. It was added in #1947 to silence
actionlint's "label 'web' is unknown" false positive β with no workflow
in this repo now using `[self-hosted, web]`, the file is dead code.
The canonical reference at `VyOS-Networks/vyos-docs-opus-reviewer/scripts/
ai-validation.yml` intentionally diverges: that repo IS in VyOS-Networks
and has access to the `web` self-hosted pool, so its canonical keeps
`runs-on: [self-hosted, web]` and the cleanup steps. The deployed
file's REFERENCE COPY header comment block in the reviewer repo will be
updated in a follow-up to note that the deployed file may use different
runners per host repo's pool availability.
No security regression β the trust boundary on prepare is enforced by
no-fork-code-execution, no-secrets-referenced, persist-credentials:false,
and the split-job artifact, all of which are unchanged. Adds the implicit
host-ephemerality guarantee of GitHub-hosted runners.
|
|
CodeRabbit minor finding on the paired canonical PR
(VyOS-Networks/vyos-docs-opus-reviewer#14): the `Notify on PR (when
skipping)` step posts a fresh `gh pr comment` on every `synchronize`
event. On a fork PR to a repo where the AI-validation secrets are not
configured, every push during PR iteration would duplicate the skip
notice, flooding the conversation thread.
Gate the step to fire only on `opened`/`reopened` β those are the
moments where the PR author benefits from being told once that
validation is skipped. Further pushes add no new information; the
workflow-run-page `::notice::` annotation is still emitted on every run
for maintainers.
`concurrency.cancel-in-progress: true` alone is not sufficient β most
synchronize events would be cancelled before the notify step ran, but
any run that completed the notify step before the next push still
posts the comment.
Paired canonical commit: VyOS-Networks/vyos-docs-opus-reviewer@ea88567
|
|
ci: harden update-version-tags against stale re-runs and silent failures
|
|
Copilot review on PR #1953 surfaced an edge case the HEAD guard alone
doesn't fully cover: with cancel-in-progress: true and a per-branch
concurrency group, a stale "Re-run jobs" replay can cancel the
in-progress run for the current branch HEAD. The stale re-run then
hits the HEAD guard and exits 0, leaving the tag un-advanced until
the next push.
Including github.sha in the concurrency group means different commits
land in different groups and never cancel each other. Same-SHA re-runs
still deduplicate (they share the group), and the HEAD guard handles
the case where a stale re-run beats the current-HEAD run to start.
π€ Generated by [robots](https://vyos.io)
|
|
Two improvements to .github/workflows/update-version-tags.yml, bundled
because they touch the same code block:
1. HEAD-equivalence guard. GitHub's "Re-run jobs" replays the original
event SHA, which for this workflow would move tag rolling/1.5/1.4
backward to a stale commit. Compare github.sha against the live
branch HEAD via the API and exit 0 with a log line if they differ.
2. PATCH-first with 404-only fallback to POST. The previous "GET probe
then PATCH or POST" pattern silently fell through to POST on any
gh-api error (auth, rate-limit, 5xx), which would attempt to create
a tag that already exists and mask the real failure. Now the
fallback to POST fires only on HTTP 404; every other error is
re-emitted to stderr and fails the job.
Backport to circinus and sagitta after merge.
π€ Generated by [robots](https://vyos.io)
|
|
Context7's API expects the bare repo identifier 'vyos/vyos-documentation'
(verified at https://context7.com/vyos/vyos-documentation), not the
leading-slash form. The auto-fired workflow_run cycle after #1950 / #1948
/ #1949 merged returned HTTP 404 on all three variants because of the
extra slash.
Spec: ~/.claude/specs/2026-05-10-context7-github-actions-integration-design.md
(spec narrative referenced Codex's round-1 advice to use leading slash;
that advice was empirically wrong against the live Context7 API).
π€ Generated by [robots](https://vyos.io)
|
|
ci: AI Validation rewrite β MyST + split-job + branch map
|
|
Without explicit timeouts, a stalled TCP handshake or slow server response
blocks the workflow indefinitely. --connect-timeout 10 bounds the TCP/connect
phase; --max-time 60 caps total request duration. Both are within reasonable
limits for a refresh POST that normally completes in well under a second.
π€ Generated by [robots](https://vyos.io)
|
|
User direction: GitHub-hosted ubuntu-latest is not available in this
environment. The runner pool is Debian 12 self-hosted (web-runner-01,
web-runner-02). prepare must run there too.
Pushing back on Copilot's defense-in-depth finding (line 35) with
explicit threat-model reasoning documented in the workflow comment:
- prepare does not execute fork code. Only git fetch / git diff /
git show / file reads. No pip install, no npm install, no build,
no test. Adding any of these would require a deliberate code
change in this file that a reviewer must approve.
- No secrets are referenced in prepare. Even a presence-check would
leak the value into the runner environment.
- persist-credentials: false on the merge-ref checkout keeps the
default GITHUB_TOKEN out of fork-readable .git/config.
- The atos-actions/clean-self-hosted-runner step (`if: always()`)
wipes the workspace after every job regardless of exit state.
The split-job artifact still bridges the trust boundary to validate.
validate remains the only place where secrets are referenced.
The skip-notice `gh pr comment` step from a22df7d is preserved β
that's an independent discoverability improvement.
π€ Generated by [robots](https://vyos.io)
|
|
Tag VyOS-Networks/vyos-docs-opus-reviewer/reviewer-v1.0.1 was
published after PR #13 merged on the reviewer side. v1.0.1 brings:
- Self-hosted runner workspace cleanup
- setup-uv (Debian 12 + Python 3.12 compat)
The branches.json map and Python source consumed via REVIEWER_REF
are unchanged from v1.0.0, so this bump is a no-op functionally
for the validate job β but it ensures the deployed workflow pulls
from a stable, post-cleanup-merge state of main rather than the
older v1.0.0 tag.
π€ Generated by [robots](https://vyos.io)
|
|
If CONTEXT7_API_KEY is unset or empty (e.g. secret not yet configured),
emit a clear error message rather than letting curl fail with a generic
auth error. The `:-` guard is needed because `set -u` would otherwise
abort before the `-z` test when the variable is truly unset.
π€ Generated by [robots](https://vyos.io)
|
|
Two more Copilot findings on b0fdd07:
1. line 35 β defense in depth: prepare on self-hosted is risky.
Even though the prepare job doesn't execute fork code (it only does
git diff / git show / file reads β never pip install, npm install,
build, or tests), Copilot's right that running fork content on a
self-hosted runner with internal-network access is the wrong
default. A future maintainer who innocently adds a "run linter"
step to prepare could turn it into an attack vector against the
VyOS internal network or a persistence mechanism on the host.
Moved prepare to runs-on: ubuntu-latest. GitHub-hosted runners are
ephemeral, isolated, and have no path to internal services. The
trusted validate job stays on [self-hosted, web]; the split-job
artifact bridges the trust boundary as before.
Side benefit: removes the runner-side dependency on jq/gh/git
for the prepare job (those are pre-installed on ubuntu-latest).
2. line 149 β skip-notice discoverability.
When secrets are missing the workflow only emits ::notice:: in
the run logs. Contributors checking the PR timeline have no
reason to click through to the run page. Added a new step that
posts an actual PR comment via gh pr comment when skip=true,
running with GH_TOKEN: ${{ github.token }} (the validate job
already has pull-requests: write).
The ::notice:: annotation is preserved alongside.
π€ Generated by [robots](https://vyos.io)
|
|
CR finally got credits and posted three substantive findings on b0fdd07.
1. CRITICAL β line 281, model: input is silently ignored.
anthropics/claude-code-action@v1 removed the top-level `model`
input; the migration guide says model selection now travels via
`claude_args: --model <name>`. With the old form, the action used
its DEFAULT model on every run instead of the pinned claude-opus-4-7,
defeating the version pin entirely. CR even ran a web query and
actionlint to verify (actionlint output: "input 'model' is not
defined in action 'anthropics/claude-code-action@v1'"). Moved
--model claude-opus-4-7 into claude_args.
2. MAJOR β line 135, secrets template-expanded into shell text.
`[ -z "${{ secrets.VYOS_APP_ID }}" ]` lets GH Actions do
${{ ... }} expansion BEFORE bash parses the script. A secret
containing a single quote, backtick, or $ would either break the
test syntactically or be evaluated by the shell. The same hygiene
that justifies the prepare/validate split applies here. Moved
the three secrets to an env: mapping; the script now reads
"$VYOS_APP_ID" etc., handed to bash as already-quoted env vars.
3. NIT β line 7, concurrency group brittle outside PR events.
`github.event.pull_request.number` is empty on workflow_dispatch
or schedule; the group would collapse to "ai-validation-" and
unrelated runs cancel each other. Defensive fix: fallback to
`github.ref`. Today the workflow only fires on
pull_request_target so this is purely future-proofing.
Same changes being synced to canonical scripts/ai-validation.yml
in vyos-docs-opus-reviewer PR #13.
π€ Generated by [robots](https://vyos.io)
|
|
Refreshes Context7's index of the VyOS documentation library on
completion of 'Update version tags', mapping rolling/circinus/sagitta to
Context7 variants rolling/1.5/1.4 respectively.
Triggered via workflow_run because GITHUB_TOKEN-driven tag pushes from
update-version-tags.yml do not fan out to downstream workflows.
workflow_dispatch added for ad-hoc and bootstrap refreshes.
Spec: ~/.claude/specs/2026-05-10-context7-github-actions-integration-design.md
π€ Generated by [robots](https://vyos.io)
|
|
Copilot finding on .github/workflows/ai-validation.yml:86:
The bundling loop currently *skips* non-regular tree entries
(symlinks mode 120000, submodules 160000) with a `::warning::` and
continues. A PR that converts a regular docs/**/*.md into a symlink
effectively bypasses both Pass 1 (no file copied into _changed_md/)
and Pass 2 (LLM Read/Glob/Grep sees no content) β reducing
validation coverage on exactly the kind of change that warrants
closer review.
Replaced the `continue` with `exit 1` on non-regular mode. The error
message instructs the PR author to convert the file back to a regular
.md (or get explicit maintainer waiver). Maintainers can still land
non-regular doc entries by adjusting the workflow, but the decision
becomes visible rather than silent.
Combined with the symlink-exfil mitigation (commit 1ea164ff): we no
longer copy symlink target content into the artifact AND we no longer
silently skip the change. Either it's a regular file we can validate,
or the workflow fails loudly.
Same change being applied to the canonical reference copy in
VyOS-Networks/vyos-docs-opus-reviewer PR #13.
Suppressed-by-Copilot finding (line 110, id-token: write):
Same pushback as previous rounds. claude-code-action@v1 uses OIDC
internally (verified via this repo's commit b18a399c). Keeping.
π€ Generated by [robots](https://vyos.io)
|
|
Two new findings on PR #1947:
1. Copilot β line 59, mkdir collision after cancellation:
`mkdir _changed_md` fails with EEXIST if the directory was left
behind by a previous run that concurrency.cancel-in-progress
killed before the post-job cleanup step could execute. On a
busy PR with rapid synchronize events this is a real
non-determinism. Replaced with `rm -rf _changed_md && mkdir -p`
so the bundling step is idempotent.
2. CodeRabbit β Major, scripts/ai-validation.yml:244 (mirrored
here on the deployed copy):
`uv pip install "git+https://x-access-token:${TOKEN}@..."` puts
the App token in process argv. On a self-hosted runner anyone
able to read /proc/<pid>/cmdline (any user with the same UID,
any root tool, any LSM audit log) sees the secret while uv/git
is running. This undercuts the persist-credentials:false
hardening on the surrounding checkouts.
Replaced the install with a two-step checkout + local-path
install:
- actions/checkout@v6 with persist-credentials:false fetches
the reviewer source into ./reviewer-src using the App token
as a transient http extraheader (not argv).
- `uv pip install ./reviewer-src` then installs from the local
path β no token anywhere on the command line.
Net trust boundary: same security posture as the existing
sparse-checkout of branches.json (line 100-110); no new attack
surface.
π€ Generated by [robots](https://vyos.io)
|
