Teach the file-gc task to heal files whose stored data fails schema
or semantic validation instead of failing the nightly run: read once
(sanitize, migrate in memory, check the scheduled revn), run the
shared best-effort repair loop, then take the row lock and persist
the snapshot, marking the file trimmed. A new revision orphaned the
current file thumbnail (the list joins on revn), so a run that
changes bytes touches modified-at instead (the ETag covers it) and
no-op runs touch nothing; the change is detected against the decoded
pre-sanitize data, which get-file now returns as :orig-data. The
handler reports a result map with a keyword reason (:cleaned, :gone,
:stale-revn, :unfixable, :error) and the tiered-storage offload
submit runs outside the result handling, so a queue failure after a
good write stays truthful and only logs its own error. The
library-user scan moved out of the write transaction, so long scans
no longer trip idle-in-transaction timeouts.
Repair convergence lives in one shared loop used by file-gc, the
debug endpoint and the srepl helper: validate, apply the computed
repair changes and repeat up to ten times, returning the input
untouched when it throws or does not converge. file-gc revalidates
after the loop and raises :repair-did-not-converge, so a partial
cleanup is never stored as clean: the classifier reports :unfixable,
the file stays unmarked and the scheduler retries it. Validation
skips shapes without :shape-ref (copy roots legitimately carry
none), so the repair converges instead of crashing on the get-shape
precondition; a dedicated repair for copies that genuinely lost
their ref is future work.
Libraries load through the weak-map loader, and that loader was
broken: the threading macro passed :data to update as the map
itself, so every library load threw and the repair loop swallowed
the exception, silently skipping every repair for files with linked
libraries. The pipeline is now explicit: load, realize pointers
(they take [cfg file], so the reversed -> threading was returning
the Integrant system map) and then sanitize. Unconverted types broke
ref resolution (detaching copies) and hid real uses from
instance-of?, so libraries are sanitized on load, user files are
sanitized inside the library-user scan, and the srepl helper and the
debug endpoint sanitize too. Only migrate when migrations pend.
sanitize-file-data now returns the input instance unchanged when
neither the nil-entry drop nor the schema decode repairs anything:
lazy containers (objects-map) keep their identity and file-gc can
tell a real fix from a no-op by comparing with the pre-sanitize
data. The walk still happens on every call, and that cost is
documented on the loader.
Migration 0027 heals stored data durably with the same recipe the
read path applies: schema-decode unconverted types (precompiled
decoders, explicit component-objects pass) plus the legacy-57 nil
page cleanup that reappeared through a write path. clean-file keeps
only its post-decode fixes and documents the single sanitize pass.
Change-level guards reject :add-page without a valid page id
(structured error, not assert, so release CLJS still enforces it)
and conflicting id/page pairs.
Tests pin the contract with production-shaped corruption builders:
file-gc repair and the outcome matrix, loop convergence and give-up
branches, the :shape-ref guard both ways, page guards, sanitize
idempotence and identity (plain and objects-map), the 0027
migration, a remote copy resolved through the loaded library and the
non-convergence report; existing file suites move to the result map
through the run-file-gc! helper.
Closes#12433
Follow-up:
https://github.com/penpot/penpot/issues/12509
AI-assisted-by: muse-spark-1.3-contributor
AI-assisted-by: glm-5.3-flash
* ⚡ Improve performance and fix orphan detection in validate-file
- Add `*ref-shape-cache*` dynamic var to memoize `find-ref-shape`
lookups per page, avoiding repeated O(depth) ancestor walks.
- Add `*children-sets*` pre-computed maps for O(1) parent-child
containment checks, replacing linear `some` scans.
- Short-circuit `inside-component-main?` when the shape context
already implies a main component.
- Use single-pass reduce with early exit for duplicate detection
(children, swap slots) instead of count/distinct or frequencies.
- Guard `check-missing-slot` to skip expensive `find-near-match`
when the shape already has a swap slot.
- Refactor variant-set validation to use `run!` with direct `get`.
- Refactor `check-ref-cycles` to use a single `reduce-kv` pass.
- Fix `get-orphan-shapes`: the original `map` pipeline produced
nils so orphan shapes were never validated; rewrite with
`reduce-kv` for correct results.
- Add `validate-file-affected!` for change-scoped validation,
replacing full file validation in `process-changes-and-validate`
to only validate pages and components touched by the changes.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
* ✨ Improved validation
---------
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
Co-authored-by: alonso.torres <alonso.torres@kaleidos.net>
Steps to reproduce: paste an SVG authored in Inkscape (or any editor
that follows the inkscape:label convention) into a penpot file. The
group/element names visible in the source editor are dropped — penpot
shows generic auto-ids like 'g1234' or 'path5678' instead.
Root cause: parse-svg-element in common/src/app/common/files/shapes_
builder.cljc derived the shape name from (or (:id attrs) (tag->name
tag)). Inkscape stores user-given element labels in the inkscape:label
and sodipodi:label namespaced attributes while id holds an auto-
generated technical id, so the operator's chosen name was always
overridden by the technical id when present.
tubax/xml->clj (the SVG parser the import pipeline already uses for
upload, paste, and library import) keeps namespaced attributes as
:prefix:name keywords — the same shape this file already reads
:xlink:href from on line 134, and that app.common.svg uses for the
xlink: namespace at lines 300-307.
Fix: extract the name-resolution logic into a public resolve-element-
name helper that prefers :inkscape:label, then :sodipodi:label, then
:id, then (tag->name tag). Existing SVGs that don't carry either label
namespace fall through the same chain as before, so the behaviour for
non-Inkscape-authored SVGs is unchanged.
This restores the behaviour dfelinto's penpot-icon-generator-plugin
relies on (linked from the issue body) — that plugin reads element
names from the imported SVG to map Blender icons to penpot components.
Tests: 6 deftest blocks in common/test/common_tests/files/shapes_
builder_test.cljc covering the priority order (inkscape > sodipodi >
id > tag), each fallback in isolation, and the empty-attrs case.
Registered in common-tests.runner.
Closes#7869
Co-authored-by: Andrey Antukh <niwi@niwi.nz>
* 🔧 Validate only after propagation in tests
* 💄 Enhance some component sync traces
* 🔧 Add fake uuid generator for debugging
* 🐛 Remove old feature of advancing references when reset changes
Since long time ago, we only allow to reset changes in the top copy
shape. In this case the near and the remote shapes are the same, so
the advance-ref has no effect.
* 🐛 Fix some bugs and add validations, repair and migrations
Also added several utilities to debug and to create scripts that
processes files
* 🐛 Fix misplaced parenthesis passing propagate-fn to wrong function
The :propagate-fn keyword argument was incorrectly placed inside the
ths/get-shape call instead of being passed to tho/reset-overrides.
This caused reset-overrides to never propagate component changes,
making the test not validate what it intended.
* 🐛 Accept and forward :include-deleted? in find-near-match
Callers were passing :include-deleted? true but the parameter was not
in the destructuring, so it was silently ignored and the function
always hardcoded true. This made the API misleading and would cause
incorrect behavior if called with :include-deleted? false.
* 💄 Use set/union alias instead of fully-qualified clojure.set/union
The namespace already requires [clojure.set :as set], so use the alias
for consistency.
* 🐛 Add tests for reset-overrides with and without propagate-fn
Add two focused tests to comp_reset_test to cover the propagate-fn
path in reset-overrides:
- test-reset-with-propagation-updates-copies: verifies that resetting
an override on a nested copy inside a main and supplying propagate-fn
causes the canonical color to appear in all downstream copies.
- test-reset-without-propagation-does-not-update-copies: regression
guard for the misplaced-parenthesis bug; confirms that omitting
propagate-fn leaves copies with the overridden value because the
component sync never runs.
---------
Co-authored-by: Andrey Antukh <niwi@niwi.nz>
Return nil from get-prev-sibling when the shape is no longer present in
the parent ordering so delete undo generation falls back to index-based
restore instead of crashing on invalid vector access.
* 🎉 Add tokens to plugins API documentation
And add poc plugin example
* 📚 Document better the tokens value in plugins API
* 🔧 Refactor token validation schemas
* 🔧 Use automatic validation in token proxies
* 🔧 Use schemas to validate token creation
* 🔧 Use multi schema for token value
* 🔧 Use schema in token api methods
* 🐛 Fix review comments
---------
Co-authored-by: Andrey Antukh <niwi@niwi.nz>
* ✨ Update function implementation
* ✨ Add tests for a new function implementation
* ✨ Update function usages according to new signature
* ✨ Update docstrings
* ✨ Use simpler assertion
* 💄 Replace concat with cons on name-seq
* 🐛 Fix incorrect error handling on legacy workspace redirect
---------
Co-authored-by: Andrey Fedorov <oran9e.red@gmail.com>
