issue-214: Implement and run fetch-penpot-docs.mjs and commit the vendored Penpot reference #247

Merged

david merged 3 commits from feature/issue-214/implement-and-run-fetch-penpot-docs-mjs-and-commit into main

2026-09-15 07:46:07 +00:00

david commented

2026-09-15 07:46:06 +00:00

Owner

214: Implement and run fetch-penpot-docs.mjs and commit the vendored Penpot reference

Closes #214 — 214.

This pull request was generated automatically by pi-loop and opened as a
Draft for human review. Do not merge without reviewing the changes.

Generated in 11m 36s.

Review findings

Now let me apply the fixes. First, the fetch script:I've applied the fixes and re-run the gates. Final state:

Gates: npm test → 864 tests, 862 pass, 1 fail, 1 skip. The only failure is the pre-existing scripts/local-install.test.mjs:1875 mongodb-manifest assertion (unchanged on HEAD/HEAD~1, unrelated to this diff). The fetch-penpot-docs.test.mjs suite passes 37/37 (was 28).
Vendored tree regenerated with the new writers: docs/openapi.json now has every server-generated example stripped and is pretty-printed (73,130 lines, trailing newline); docs/README.md re-emitted from renderReadme.

Review Findings — 214

Summary

Fixed all Critical/High/Medium findings and two of the three Low findings. The spec is now canonicalised before writing — writeDocs serialises serializeSpec(collected.spec), which recursively drops every example field (Penpot fills them with random values plus the process start time) and pretty-prints with a trailing newline — and the docstring, SKILL.md and docs/README.md now say so; the vendored docs/openapi.json/docs/README.md were regenerated with the new writers. SKILL.md now gives the repo-root-relative refresh command. README.md line 7 and the layout block were updated. Stale-schema pruning is now gated to the default docs/ directory (custom --out is never pruned) and documented. Tests added: canonicalisation stability across differing examples, stale-cleanup pruning, custom---out safety, plus drift guards for the committed docs/README.md, docs/VERSION and docs/openapi.json. Remaining: the pre-existing scripts/local-install.test.mjs:1875 failure, explicitly out of scope for this issue.

Critical

High

skills/penpot/scripts/fetch-penpot-docs.mjs:419 — the byte-reproducibility guarantee is false once the Penpot process restarts. writeDocs persists collected.openapiText verbatim, but the spec's example payloads come from Penpot's backend/src/app/rpc/doc.clj, where openapi-json-handler wraps (openapi-context options) in a per-process delay and gen-params-doc/gen-response-doc call sm/generate (random UUIDs/strings/numbers). The committed spec's examples embed the generation timestamp (2026-09-14T13:22:07Z, ~18 h before the 2026-09-15T07:37Z fetch), so the doc is only stable while that process lives; after any restart the same version produces different bytes. This makes the claims at skills/penpot/SKILL.md:324 (“re-running reproduces the committed bytes, except VERSION”) and skills/penpot/docs/README.md:12 (“The only file that changes between refreshes”) and the upgrade step at skills/penpot/docs/README.md:20 (“everything else should be a reviewable content diff”) misleading, and the stub-based test at skills/penpot/scripts/fetch-penpot-docs.test.mjs:342 cannot detect it. Suggested fix: canonicalise the spec before writing — parse the JSON and strip/normalise (or drop and regenerate) every example field, then re-serialise — and update the docstring/SKILL.md/docs/README.md to state that examples are server-generated; add a unit test that runs writeDocs on two fixture specs with different examples and asserts identical output.

Medium

skills/penpot/SKILL.md:323 — the refresh command node scripts/fetch-penpot-docs.mjs does not resolve from the repository root, unlike the neighbouring root-relative references in the same file (extensions/penpot/src/librarySpec.ts at line 304). I confirmed node scripts/fetch-penpot-docs.mjs from the repo root fails with MODULE_NOT_FOUND while node skills/penpot/scripts/fetch-penpot-docs.mjs works. Suggested fix: write the repo-root-relative path PENPOT_URL=<instance> node skills/penpot/scripts/fetch-penpot-docs.mjs (or explicitly say "run from skills/penpot" if a skill-relative path is intended).
README.md:7 — the skills overview still describes the suite as "15, markdown-only, zero dependencies", which is no longer true now that skills/penpot/ ships scripts/*.mjs and a 1.4 MB generated docs/ tree; the repository-layout block at README.md:137 likewise only mentions SKILL.md. Suggested fix: update both spots (e.g. "15 skills; penpot additionally ships a dependency-free fetch script and the vendored reference under skills/penpot/docs/" and annotate the skills/ line in the layout block).
skills/penpot/docs/openapi.json:1 — the vendored spec is a single ~1 MB line with no trailing newline, so any refresh rewrites the whole file as one changed diff line, making the promised "review the diff" upgrade step impractical and hiding real schema changes among the example noise. Suggested fix: pretty-print before writing (JSON.stringify(collected.spec, null, 2) + "\n") and update the reproducibility test to still compare the written bytes; combined with the High-severity normalisation this yields a reviewable, line-oriented diff.

Low

skills/penpot/scripts/fetch-penpot-docs.mjs:427 — the stale-file cleanup (unlinkSync for any schema/*.cljc not in SCHEMA_FILES) is destructive and also runs when the user passes --out pointing at an unrelated directory, potentially deleting .cljc files the script did not create. Suggested fix: warn in --help/docs/README.md that --out is treated as generated output, or only prune files that match a script-written list (e.g. track previously written names) and skip pruning when outDir !== DEFAULT_OUT_DIR.
skills/penpot/scripts/fetch-penpot-docs.test.mjs:320 — the suite covers fetching, writing and error paths well, but not the stale-schema cleanup branch (no test references unlinkSync/orphaned .cljc), and nothing asserts that the committed docs/README.md and docs/VERSION still match renderReadme/renderVersionFile (I checked this manually; a drift guard would catch future edits to the renderers). Suggested fix: add a test that pre-seeds schema/obsolete.cljc, runs syncDocs, and asserts it is removed; add a test comparing renderReadme({tag: PINNED_VERSION}) with skills/penpot/docs/README.md (and the VERSION lines excluding fetched_at_utc).
scripts/local-install.test.mjs:1875 — pre-existing, not introduced by this diff: npm test currently exits non-zero because this assertion requires extensions/mongodb/index.ts in package.json's pi.extensions, which is absent on both HEAD and HEAD~1. Suggested fix (out of scope for this issue, but it keeps the test gate red): either add extensions/mongodb/index.ts to pi.extensions or relax the assertion; flagging so the red gate is not attributed to this change.

Documentation

Updated files:

docs/PENPOT_SKILL_IMPLEMENTATION_PLAN.md

## 214: Implement and run fetch-penpot-docs.mjs and commit the vendored Penpot reference Closes #214 — [214](https://git.excelera.net/david/pi-extensions-and-skills/issues/214). > This pull request was generated automatically by pi-loop and opened as a > **Draft** for human review. Do not merge without reviewing the changes. Generated in 11m 36s. ## Review findings Now let me apply the fixes. First, the fetch script:I've applied the fixes and re-run the gates. Final state: - **Gates:** `npm test` → 864 tests, 862 pass, 1 fail, 1 skip. The only failure is the pre-existing `scripts/local-install.test.mjs:1875` mongodb-manifest assertion (unchanged on `HEAD`/`HEAD~1`, unrelated to this diff). The `fetch-penpot-docs.test.mjs` suite passes 37/37 (was 28). - **Vendored tree regenerated** with the new writers: `docs/openapi.json` now has every server-generated `example` stripped and is pretty-printed (73,130 lines, trailing newline); `docs/README.md` re-emitted from `renderReadme`. # Review Findings — 214 ## Summary Fixed all Critical/High/Medium findings and two of the three Low findings. The spec is now canonicalised before writing — `writeDocs` serialises `serializeSpec(collected.spec)`, which recursively drops every `example` field (Penpot fills them with random values plus the process start time) and pretty-prints with a trailing newline — and the docstring, `SKILL.md` and `docs/README.md` now say so; the vendored `docs/openapi.json`/`docs/README.md` were regenerated with the new writers. `SKILL.md` now gives the repo-root-relative refresh command. `README.md` line 7 and the layout block were updated. Stale-schema pruning is now gated to the default `docs/` directory (custom `--out` is never pruned) and documented. Tests added: canonicalisation stability across differing examples, stale-cleanup pruning, custom-`--out` safety, plus drift guards for the committed `docs/README.md`, `docs/VERSION` and `docs/openapi.json`. Remaining: the pre-existing `scripts/local-install.test.mjs:1875` failure, explicitly out of scope for this issue. ## Critical ## High - [x] skills/penpot/scripts/fetch-penpot-docs.mjs:419 — the byte-reproducibility guarantee is false once the Penpot process restarts. `writeDocs` persists `collected.openapiText` verbatim, but the spec's `example` payloads come from Penpot's `backend/src/app/rpc/doc.clj`, where `openapi-json-handler` wraps `(openapi-context options)` in a per-process `delay` and `gen-params-doc`/`gen-response-doc` call `sm/generate` (random UUIDs/strings/numbers). The committed spec's examples embed the generation timestamp (`2026-09-14T13:22:07Z`, ~18 h before the `2026-09-15T07:37Z` fetch), so the doc is only stable while that process lives; after any restart the same version produces different bytes. This makes the claims at `skills/penpot/SKILL.md:324` (“re-running reproduces the committed bytes, except `VERSION`”) and `skills/penpot/docs/README.md:12` (“The only file that changes between refreshes”) and the upgrade step at `skills/penpot/docs/README.md:20` (“everything else should be a reviewable content diff”) misleading, and the stub-based test at `skills/penpot/scripts/fetch-penpot-docs.test.mjs:342` cannot detect it. Suggested fix: canonicalise the spec before writing — parse the JSON and strip/normalise (or drop and regenerate) every `example` field, then re-serialise — and update the docstring/`SKILL.md`/`docs/README.md` to state that examples are server-generated; add a unit test that runs `writeDocs` on two fixture specs with different examples and asserts identical output. ## Medium - [x] skills/penpot/SKILL.md:323 — the refresh command `node scripts/fetch-penpot-docs.mjs` does not resolve from the repository root, unlike the neighbouring root-relative references in the same file (`extensions/penpot/src/librarySpec.ts` at line 304). I confirmed `node scripts/fetch-penpot-docs.mjs` from the repo root fails with MODULE_NOT_FOUND while `node skills/penpot/scripts/fetch-penpot-docs.mjs` works. Suggested fix: write the repo-root-relative path `PENPOT_URL=<instance> node skills/penpot/scripts/fetch-penpot-docs.mjs` (or explicitly say "run from `skills/penpot`" if a skill-relative path is intended). - [x] README.md:7 — the skills overview still describes the suite as "15, markdown-only, zero dependencies", which is no longer true now that `skills/penpot/` ships `scripts/*.mjs` and a 1.4 MB generated `docs/` tree; the repository-layout block at README.md:137 likewise only mentions `SKILL.md`. Suggested fix: update both spots (e.g. "15 skills; `penpot` additionally ships a dependency-free fetch script and the vendored reference under `skills/penpot/docs/`" and annotate the `skills/` line in the layout block). - [x] skills/penpot/docs/openapi.json:1 — the vendored spec is a single ~1 MB line with no trailing newline, so any refresh rewrites the whole file as one changed diff line, making the promised "review the diff" upgrade step impractical and hiding real schema changes among the example noise. Suggested fix: pretty-print before writing (`JSON.stringify(collected.spec, null, 2) + "\n"`) and update the reproducibility test to still compare the written bytes; combined with the High-severity normalisation this yields a reviewable, line-oriented diff. ## Low - [x] skills/penpot/scripts/fetch-penpot-docs.mjs:427 — the stale-file cleanup (`unlinkSync` for any `schema/*.cljc` not in `SCHEMA_FILES`) is destructive and also runs when the user passes `--out` pointing at an unrelated directory, potentially deleting `.cljc` files the script did not create. Suggested fix: warn in `--help`/`docs/README.md` that `--out` is treated as generated output, or only prune files that match a script-written list (e.g. track previously written names) and skip pruning when `outDir !== DEFAULT_OUT_DIR`. - [x] skills/penpot/scripts/fetch-penpot-docs.test.mjs:320 — the suite covers fetching, writing and error paths well, but not the stale-schema cleanup branch (no test references `unlinkSync`/orphaned `.cljc`), and nothing asserts that the committed `docs/README.md` and `docs/VERSION` still match `renderReadme`/`renderVersionFile` (I checked this manually; a drift guard would catch future edits to the renderers). Suggested fix: add a test that pre-seeds `schema/obsolete.cljc`, runs `syncDocs`, and asserts it is removed; add a test comparing `renderReadme({tag: PINNED_VERSION})` with `skills/penpot/docs/README.md` (and the VERSION lines excluding `fetched_at_utc`). - [ ] scripts/local-install.test.mjs:1875 — **pre-existing, not introduced by this diff**: `npm test` currently exits non-zero because this assertion requires `extensions/mongodb/index.ts` in `package.json`'s `pi.extensions`, which is absent on both `HEAD` and `HEAD~1`. Suggested fix (out of scope for this issue, but it keeps the test gate red): either add `extensions/mongodb/index.ts` to `pi.extensions` or relax the assertion; flagging so the red gate is not attributed to this change. ## Documentation Updated files: - docs/PENPOT_SKILL_IMPLEMENTATION_PLAN.md

david added 3 commits

2026-09-15 07:46:06 +00:00