Make checkSamples fail loudly on embed-code errors#84
Open
alexander-yevsyukov wants to merge 2 commits into
Open
Make checkSamples fail loudly on embed-code errors#84alexander-yevsyukov wants to merge 2 commits into
alexander-yevsyukov wants to merge 2 commits into
Conversation
`./gradlew checkSamples` was exiting 0 while verifying nothing, so CI's
"Check Code Embedding" workflow stayed green regardless of embedding drift.
Two compounding bugs:
1. Stale config: `docs/_settings/v1.embed-code.yml` set `docs-path` to
`../content/docs/1`, a directory removed in the site restructure. The
embed-code binary rejected it with an argument-validation error.
2. Swallowed failure: on argument-validation errors the embed-code binary
prints an ERROR line but still exits 0, so `set -e` in
`docs/_script/check-samples` had no non-zero status to trip on.
Changes:
- Repoint `docs-path` to `../content/docs` (top-level key and embed-mappings).
- Harden `check-samples` to fail closed: the check now passes only when the
binary exits 0 AND prints its success sentinel ("documentation files are
up-to-date with code files"). An argument error (exit 0), a stale embedding,
or any other silent-exit-0 failure is now reported and fails the build.
Verified via `./gradlew :checkSamples` (what CI runs): passes on a clean tree,
and now fails with exit 1 both on a deliberately stale embedding and on a bad
`docs-path`. The embeddings themselves were already up to date, so no
regeneration was needed.
The root cause (exit 0 on argument errors) lives upstream in
SpineEventEngine/embed-code-go; the guard protects this repo regardless.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reverts the docs-path change from the prior commit on this branch. Per the project's documentation versioning, the v1 embed-code config stays pointed at the `/1` subfolder, so the net change of this branch is just the fail-loud guard in check-samples. Because commit b184e4d ("Move the main documentation version to the root") moved v1 content to the root and no docs/content/docs/1 directory exists yet, checkSamples now fails loudly via that guard, reporting the missing path. Restoring `/1` is handled separately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
alexander-yevsyukov
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
./gradlew checkSamples— and CI's Check Code Embedding workflow — was passing while verifying nothing: on argument errors the embed-code binary prints an error but exits 0, anddocs/_script/check-samplestrusted that exit code. This PR makes the check fail loudly so those errors can no longer pass silently.What this changes
docs/_script/check-samples— a fail-closed guard. The check now passes only when the binary exits 0 and prints its success sentinel (documentation files are up-to-date with code files). A zero exit without that sentinel (an argument error), or any non-zero exit (stale embedding, missing code file), now fails with an actionable message.Fail-closed was chosen over blocklisting
ERRORoutput because the binary's exit-code discipline is inconsistent — argument errors exit 0, while stale embeddings and missing files panic with exit 1. Asserting the positive success signal catches every silent-exit-0 mode.The
docs/_settings/v1.embed-code.ymlconfig is intentionally left pointing at../content/docs/1per the project's documentation versioning, so the net diff of this PR ischeck-samplesonly./1is restoredCommit b184e4d ("Move the main documentation version to the root") moved all v1 content from
docs/content/docs/1/todocs/content/docs/without updating this config, anddocs/data/versions.ymlreflects that layout (v1 →content_path: docs). There is currently nodocs/content/docs/1/directory, so with the guard in placecheckSamplesnow correctly reports:This is the guard doing its job — surfacing a real, previously-hidden breakage rather than passing over it. Restoring the
/1directory (or otherwise reconciling the path) is handled separately. Until then this job is red by design.Behavior matrix (via
./gradlew :checkSamples, what CI runs)docs-path(current repo state)The success sentinel is present in all three committed binaries (
embed-code-ubuntu,embed-code-macos,embed-code-macos-x86_64), so the guard holds on the Ubuntu binary CI uses.Notes for reviewers
gradlew.bat(dirty before this work) was deliberately left out of this PR.🤖 Generated with Claude Code