From bc4291687b194271d337fbeb3c075b4d4f7394a7 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Wed, 7 Oct 2026 20:50:56 +0200 Subject: [PATCH] fix(openehr-specs): build publish previews only through the published image The local-script branch of publish ran spec_publish.sh -f -v without -q, so every {pkg} class-table include failed. On BASE that gave 70 "include file not found" errors and about 80% fewer table blocks in the HTML, while the script still printed "generated" and exited 0. The published ghcr.io/openehr/asciidoctor image passes -q in its entrypoint, and the same build there has no errors. - publish runs only the image: it checks for Docker and the sibling specifications-AA_GLOBAL checkout, snapshots the HTML timestamps, builds with --rm, then fails the build on a missing or unchanged output or on any ERROR or "include file not found" log line. The tool pre-check and the script's allowed-tools rule are removed. - authoring step 7 shows the image first and says to pass -q when using a local toolchain. - AGENTS.md, the plugin README, docs/testing.md and the CHANGELOG match. Co-Authored-By: Claude Sonnet 5.5 --- AGENTS.md | 2 +- CHANGELOG.md | 7 +-- docs/testing.md | 2 +- plugins/openehr-specs/README.md | 2 +- .../openehr-specs/skills/authoring/SKILL.md | 23 ++++++---- plugins/openehr-specs/skills/publish/SKILL.md | 46 ++++++++----------- 6 files changed, 40 insertions(+), 42 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ca0bb44..caa263b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -57,7 +57,7 @@ gracefully when one is absent (each says so in its prompt): - **`spec-reviewer`, `xref-auditor`** — need a `specifications-XX` checkout and (for attribute resolution) the sibling `specifications-AA_GLOBAL`. - **`xref-auditor`, `identifier-grounding`** — use **WebFetch** to read spec Markdown twins (`.html` → `.md`); `identifier-grounding` additionally prefers the **`openehr-assistant` MCP** (`type_specification_get`) when connected, falling back to the twin. - **`regen-classes`** — needs **Docker** (`ghcr.io/openehr/bmm-publisher`). -- **`publish`** — needs a sibling `specifications-AA_GLOBAL` checkout; it runs `bin/spec_publish.sh`, falls back to the published `ghcr.io/openehr/asciidoctor` Docker image when `asciidoctor`, `jq`, or `bc` is missing, and stops when Docker is missing too. It checks that each HTML output was rebuilt, because the script reports success even when it built nothing. +- **`publish`** — needs **Docker** and sibling `specifications-AA_GLOBAL` and component checkouts. It runs the published `ghcr.io/openehr/asciidoctor` image, which bundles the toolchain and `spec_publish.sh`; the boilerplate and references are still read from the AA_GLOBAL checkout. It checks that each HTML output was rebuilt and that the log has no missing includes, because the script reports success even when it built nothing or dropped the class tables. None are bundled (the `openehr-assistant` MCP is interactively authenticated, not redistributable); document them, don't assume them. diff --git a/CHANGELOG.md b/CHANGELOG.md index 02c582f..162c992 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,17 +9,18 @@ Notable changes to the plugins in this repository. The format follows [Keep a Ch - `publish` and `regen-classes` moved from `commands/` to `skills/` as user-only skills (`disable-model-invocation: true`). Claude Code treats `commands/` as the older format, so the plugin no longer ships that directory. The `/openehr-specs:publish` and `/openehr-specs:regen-classes` names are unchanged. - `scripts/validate.py` no longer checks a `commands/` directory; the user-only skills are validated as skills. - `publish`, `authoring`, and the release checklist run the published `ghcr.io/openehr/asciidoctor` image instead of `openehr/asciidoctor`, which is only the tag of a local `docker build`. +- `publish` runs only the published image; the local-script branch is gone. That branch ran `spec_publish.sh -f -v` without `-q`, so every `{pkg}` class-table include failed (70 errors on BASE, with about 80% of the table blocks missing from the HTML) while the script still printed "generated" and exited 0. The image's entrypoint passes `-q`; on BASE the same image builds with no errors. The AA_GLOBAL checkout is still required, because the boilerplate and reference definitions are read from it. `authoring` shows the image first and says to pass `-q` with a local toolchain. - `publish` and `regen-classes` keep only the runnable steps and guardrails; the background stays in `authoring` and `class-generation`. - `amendment-record` takes over the `amend` command: it accepts the same arguments (`argument-hint`), locates the amendment record, checks the working-tree diff, and stops after showing the diff. It no longer pre-approves `Read`, `Edit`, and `Bash` as the command did. ### Security -- `publish` and `regen-classes` no longer pre-approve every shell command while they run. `allowed-tools` was a bare `Bash`, which also covered `git commit` and `git push`. It now lists only the publisher script, the one Docker image each skill uses, and `docker --version`. A command that does not match a pattern asks for approval as usual. +- `publish` and `regen-classes` no longer pre-approve every shell command while they run. `allowed-tools` was a bare `Bash`, which also covered `git commit` and `git push`. It now lists only the Docker image each skill runs, plus `docker --version` for `regen-classes`. A command that does not match a pattern asks for approval as usual. ### Fixed -- `publish` no longer lists a `[spec-id]` argument: the publisher builds whole components, and it rebuilds every sibling repo when given none, so the skill now stops and asks when `$ARGUMENTS` is empty. Its guardrail names the release label (`-l Release-N.N.N`) as the release flag. -- `publish` no longer trusts the script's `generated ` line, which `spec_publish.sh` prints and exits 0 even when `asciidoctor` is missing or failed. It checks for `asciidoctor`, `jq`, and `bc` first and uses the Docker image when one is missing, then compares output timestamps before and after the build and reports each HTML file that was not rebuilt. `authoring` carries the same warning for a manual build. +- `publish` no longer lists a `[spec-id]` argument, because the publisher builds whole components. It stops and asks when `$ARGUMENTS` is empty: the image takes exactly one component, and the bare script would rebuild every sibling repo. It always builds as `development`; a `Release-N.N.N` build belongs to `governance`. +- `publish` no longer trusts the build's `generated ` line, which is printed and exits 0 even when `asciidoctor` failed or includes are missing. It compares output timestamps before and after the build, scans the log for `ERROR` and `include file not found` lines, and reports each HTML file that was not rebuilt. `authoring` carries the same warning for a manual build. - `regen-classes` keeps the layout rule (`legacy-adoc` or `asciidoc`) and stops and asks when `$ARGUMENTS` is empty. - `amendment-record`: a direct run states an assumed version bump and proceeds instead of pausing, and it stops after the diff only when invoked directly, not inside a release or authoring flow. diff --git a/docs/testing.md b/docs/testing.md index 6d39aa1..46a25d2 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -13,7 +13,7 @@ Install the marketplace from your working copy (inside a Claude Code session): Then exercise the skills against a checkout of a `specifications-XX` repo: confirm each skill triggers on its documented phrases (see the `description` frontmatter) and does **not** trigger on its anti-trigger cases. -The user-only action skills (`publish`, `regen-classes`) run only when you type them (`/openehr-specs:`): confirm that a plain request does not start one. Subagents are dispatched when a request matches their `description`, and you can also ask for one by name: confirm both. `regen-classes` needs Docker and `publish` needs a sibling `specifications-AA_GLOBAL` checkout (see the [plugin README](../plugins/openehr-specs/README.md)). +The user-only action skills (`publish`, `regen-classes`) run only when you type them (`/openehr-specs:`): confirm that a plain request does not start one. Subagents are dispatched when a request matches their `description`, and you can also ask for one by name: confirm both. Both need Docker, and `publish` also needs a sibling `specifications-AA_GLOBAL` checkout (see the [plugin README](../plugins/openehr-specs/README.md)). After editing skill content, reinstall (or restart the session) to pick up changes. diff --git a/plugins/openehr-specs/README.md b/plugins/openehr-specs/README.md index cd3ce93..aaac2a5 100644 --- a/plugins/openehr-specs/README.md +++ b/plugins/openehr-specs/README.md @@ -36,7 +36,7 @@ Two skills run only when you type them: they do not auto-trigger, and their desc | Skill | Argument | Action | |-------|----------|--------| | `/openehr-specs:regen-classes` | ` [-d ...]` | Regenerate class tables and diagrams via `bmm-publisher` (needs Docker) | -| `/openehr-specs:publish` | `` | Build a local HTML preview with the `AA_GLOBAL` publisher (needs a sibling `specifications-AA_GLOBAL` checkout; falls back to Docker when `asciidoctor`, `jq`, or `bc` is missing, and checks that the HTML was rebuilt) | +| `/openehr-specs:publish` | `` | Build a local HTML preview with the published `ghcr.io/openehr/asciidoctor` image (needs Docker and a sibling `specifications-AA_GLOBAL` checkout; checks that the HTML was rebuilt and the class tables resolved) | ## Subagents diff --git a/plugins/openehr-specs/skills/authoring/SKILL.md b/plugins/openehr-specs/skills/authoring/SKILL.md index 4b6903c..ec44180 100644 --- a/plugins/openehr-specs/skills/authoring/SKILL.md +++ b/plugins/openehr-specs/skills/authoring/SKILL.md @@ -225,19 +225,24 @@ Add the new specification entry to the `specifications` array, with `id`, `title ### Step 7: Preview locally Build a local HTML preview after creating or editing a spec. The user can run -`/openehr-specs:publish ` (user-only; it wraps the commands below), or run them -from the parent directory that contains all `specifications-*` repos. `-f` forces regeneration, `-v` -is verbose, and `XX` is the component (for example `RM`): +`/openehr-specs:publish ` (user-only; it wraps the command below), or run it from the +parent directory that contains all `specifications-*` repos, with `XX` as the component (for example `RM`): ```bash -./specifications-AA_GLOBAL/bin/spec_publish.sh -f -v XX -# Or via Docker: -docker run -u $(id -u):$(id -g) -v "$(pwd):/documents/" ghcr.io/openehr/asciidoctor development XX +docker run --rm -u $(id -u):$(id -g) -v "$(pwd):/documents/" ghcr.io/openehr/asciidoctor development XX ``` -The script prints `generated ` and exits 0 even when `asciidoctor` is missing or failed, so check -that each expected `docs/.html` is new or has a later modification time than before the build. -The preview does not tag or deploy a release; that is the **governance** skill's process. +The image bundles the toolchain, but the boilerplate and reference definitions are still read from the +sibling `specifications-AA_GLOBAL/` checkout. Its entrypoint is `spec_publish.sh -f -r -v -t -q -l`, so +the first argument is the release. To use a locally installed toolchain instead, run +`./specifications-AA_GLOBAL/bin/spec_publish.sh -f -q -v XX`; always pass `-q`, because without it +every `{pkg}` class-table include fails. + +The build prints `generated ` and exits 0 even when includes are missing or `asciidoctor` failed. +Check that each expected `docs/.html` has a later modification time than before the build and +that the log has no `ERROR` or `include file not found` lines. The build rewrites the tracked +`docs/*.html` artefacts; do not stage them. The preview does not tag or deploy a release; that is the +**governance** skill's process. ## Editing Existing Specifications diff --git a/plugins/openehr-specs/skills/publish/SKILL.md b/plugins/openehr-specs/skills/publish/SKILL.md index 64aeea8..0acb9a8 100644 --- a/plugins/openehr-specs/skills/publish/SKILL.md +++ b/plugins/openehr-specs/skills/publish/SKILL.md @@ -1,9 +1,8 @@ --- name: publish -description: Build a local HTML preview of an openEHR specification component via the AA_GLOBAL publisher +description: Build a local HTML preview of an openEHR specification component with the published AA_GLOBAL publishing image argument-hint: "" allowed-tools: - - "Bash(./specifications-AA_GLOBAL/bin/spec_publish.sh *)" - "Bash(docker run * ghcr.io/openehr/asciidoctor *)" - Read - Glob @@ -11,34 +10,27 @@ disable-model-invocation: true --- Build a local HTML preview of the component named in `$ARGUMENTS` (for example `RM`, `BASE` or `AM`) -with the publisher in `specifications-AA_GLOBAL`. The publisher builds a whole component, not a single -spec. This is the preview step of the `openehr-specs:authoring` skill. It never tags, commits, or -deploys a release; that is the `governance` skill. +with the published `ghcr.io/openehr/asciidoctor` image. This is the preview step of the +`openehr-specs:authoring` skill: it builds a whole component as the `development` release and never +tags, commits, or deploys (that is `governance`). If `$ARGUMENTS` is empty, ask which component and +stop: the image takes exactly one. -If `$ARGUMENTS` is empty, ask which component and stop: without one, the script rebuilds every sibling -`specifications-*` repo. - -The script prints `generated ` and exits 0 even when `asciidoctor` is missing or failed. Confirm -the outputs (steps 2 and 4) instead of trusting that line. - -1. Work from the parent directory that holds the sibling `specifications-AA_GLOBAL/` and - `specifications-/` checkouts. If either is missing, say which and stop. Run - `which asciidoctor jq bc`: the script needs all three. If one is missing, name it and use the Docker - fallback in step 3; if `which docker` finds nothing either, report both and stop. +1. Work from the parent directory that holds the sibling `specifications-AA_GLOBAL/` (the image reads + the boilerplate and references from it) and `specifications-/` checkouts. If either is + missing, say which and stop. If `which docker` finds nothing, say Docker is required and stop. 2. Snapshot the outputs: `stat -c '%Y %n' specifications-/docs/*.html` (ignore "No such file"; use `ls -l` where `stat -c` is unavailable). -3. Build: +3. Build. The entrypoint is `spec_publish.sh -f -r -v -t -q -l`, so the first argument is the release: ```bash - ./specifications-AA_GLOBAL/bin/spec_publish.sh -f -v $ARGUMENTS - # fallback when a tool is missing: - docker run -u $(id -u):$(id -g) -v "$(pwd):/documents/" ghcr.io/openehr/asciidoctor development $ARGUMENTS + docker run --rm -u $(id -u):$(id -g) -v "$(pwd):/documents/" ghcr.io/openehr/asciidoctor development $ARGUMENTS ``` -4. Verify. Glob `specifications-/docs/*/master.adoc`: each needs a rebuilt +4. Verify, because the build prints `generated ` and exits 0 even when includes are missing or + `asciidoctor` failed. Glob `specifications-/docs/*/master.adoc`: each needs a rebuilt `docs/.html` beside it, plus `docs/index.html` when `docs/index.adoc` exists. Repeat the `stat` - command. A file that is missing, or whose time did not change, was not built: list each as - `NOT BUILT` and report the build as failed. -5. Report where the HTML landed and every Asciidoctor warning or error verbatim (unresolved - attributes, missing includes, missing images). On failure, summarise the first actionable error and - point to `authoring` (includes, attributes) or the `xref-auditor` subagent (cross-references). - -Never pass a release label (`-l Release-N.N.N`); releasing belongs to `governance`. + command; a file that is missing or unchanged is `NOT BUILT`. Any `ERROR` or `include file not found` + log line (for example `{pkg}…` class tables) also fails the build. An `INFO` line, such as a possible + invalid bibliography reference, does not. +5. Report where the HTML landed and every error or warning verbatim. On failure, summarise the first + actionable error and point to `authoring` (includes, attributes) or the `xref-auditor` subagent + (cross-references). The build rewrites the tracked `docs/*.html` artefacts: tell the user, and do not + stage them.